fix(hook): preserve configured output_summary label on failure by jdx · Pull Request #808 · jdx/hk

jdx · 2026-04-10T00:18:32Z

Summary

PR fix(hook): show combined output for failed steps #772 forced Combined output on failure to avoid losing diagnostics, but this changed the summary header label (e.g., lint output: instead of lint stderr:), breaking fix(hook): remove dup output and preserve check_first diagnostics #784's tests.
Uses combined content on failure (preserving fix(hook): show combined output for failed steps #772's intent) but keeps the configured label so the header matches output_summary.
Fixes CI failures on chore: release v1.41.1 #806.

Test plan

test/output_summary_no_duplicate.bats passes locally (both tests)
CI passes on this PR

🤖 Generated with Claude Code

Note

Low Risk
Low risk, small change limited to end-of-run output summarization; main risk is subtle behavior differences in how failure output is labeled (and stored) for downstream consumers/tests.

Overview
On step failure, the summary now saves combined output content to avoid losing diagnostics, but preserves the step’s configured output_summary mode/label instead of forcing Combined.

output_summary = "hide" is still respected even on failure, preventing any summary output from being recorded.

^{Reviewed by Cursor Bugbot for commit f0090e4. Bugbot is set up for automated code reviews on this repo. Configure here.}

PR #772 forced Combined output on failure to avoid losing diagnostics, but this changed the summary header label (e.g., "lint output:" instead of "lint stderr:"), breaking #784's tests. Use combined content on failure but keep the configured label. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

greptile-apps · 2026-04-10T00:20:55Z

Greptile Summary

This PR fixes a label regression introduced by #772: on failure, it was forcing OutputSummary::Combined as both the content and the label, so steps configured with output_summary = \"stderr\" showed step output: instead of step stderr:, breaking #784's tests. The fix passes self.output_summary.clone() as the label while still using combined content — preserving diagnostics on failure without changing the visible header.

Confidence Score: 5/5

Safe to merge — minimal, targeted fix with no regressions introduced.

Single-line change in a well-tested code path. The fix correctly preserves the configured label while keeping combined content on failure, exactly matching the intent of both #772 and #784. All four OutputSummary variants behave correctly, Hide is still respected on failure, and the existing bats tests directly validate the restored behaviour.

No files require special attention.

Important Files Changed

Filename	Overview
src/step/output.rs	Single-line fix: passes the configured `output_summary` variant as the label instead of hard-coding `Combined` on failure; combined content is still used for completeness. Logic is correct for all four `OutputSummary` variants.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[save_output_summary called] --> B{is_check_first\ncheck that failed?}
    B -- yes --> C[return early\n no output saved]
    B -- no --> D{is_failure AND\noutput_summary != Hide?}
    D -- yes --> E["append_step_output\nlabel = self.output_summary\ncontent = combined\n(preserves label, captures all streams)"]
    D -- no --> F{output_summary value}
    F -- Stderr --> G[label=Stderr, content=stderr]
    F -- Stdout --> H[label=Stdout, content=stdout]
    F -- Combined --> I[label=Combined, content=combined]
    F -- Hide --> J[no output saved]

_{Reviews (1): Last reviewed commit: "fix(hook): preserve configured output_su..." | Re-trigger Greptile}

gemini-code-assist

Code Review

This pull request updates the step output logic in src/step/output.rs to respect the configured output summary label even during failure scenarios, while still honoring the 'hide' opt-out. I have no feedback to provide as there are no review comments to address.

@nkakouros

### 🐛 Bug Fixes - **(hook)** remove dup output and preserve check_first diagnostics by [@nkakouros](https://github.com/nkakouros) in [#784](#784) - **(hook)** show combined output for failed steps by [@nkakouros](https://github.com/nkakouros) in [#772](#772) - **(hook)** preserve configured output_summary label on failure by [@jdx](https://github.com/jdx) in [#808](#808) ### 📦️ Dependency Updates - update rust crate pklr to 0.4.1 by [@jhult](https://github.com/jhult) in [#805](#805) ### New Contributors - @jhult made their first contribution in [#805](#805)  --- > [!NOTE] > **Low Risk** > Primarily a version bump and doc/metadata regeneration, plus routine dependency lockfile updates; minimal behavioral risk beyond potential upstream crate changes. > > **Overview** > Bumps `hk` to **v1.41.1** and adds the corresponding `CHANGELOG.md` release entry. > > Regenerates/version-aligns published artifacts and docs (CLI docs JSON/markdown, `hk.usage.kdl`, and all docs/examples that reference the release `package://.../v1.41.1` URLs). > > Updates `Cargo.lock` for dependency bumps (notably `pklr 0.4.1`, `indexmap 2.14.0`/`hashbrown 0.17.0`, and `tokio 1.51.1`). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 0a2a092. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>  Co-authored-by: mise-en-dev <[email protected]>

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [hk](https://github.com/jdx/hk) | patch | `1.41.0` → `1.41.1` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>jdx/hk (hk)</summary> ### [`v1.41.1`](https://github.com/jdx/hk/blob/HEAD/CHANGELOG.md#1411---2026-04-10) [Compare Source](jdx/hk@v1.41.0...v1.41.1) ##### 🐛 Bug Fixes - **(hook)** remove dup output and preserve check\_first diagnostics by [@nkakouros](https://github.com/nkakouros) in [#784](jdx/hk#784) - **(hook)** show combined output for failed steps by [@nkakouros](https://github.com/nkakouros) in [#772](jdx/hk#772) - **(hook)** preserve configured output\_summary label on failure by [@jdx](https://github.com/jdx) in [#808](jdx/hk#808) ##### 📦️ Dependency Updates - update rust crate pklr to 0.4.1 by [@jhult](https://github.com/jhult) in [#805](jdx/hk#805) ##### New Contributors - [@jhult](https://github.com/jhult) made their first contribution in [#805](jdx/hk#805) </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

gemini-code-assist Bot reviewed Apr 10, 2026

View reviewed changes

jdx merged commit d84ab90 into main Apr 10, 2026
23 checks passed

jdx deleted the fix/output-summary-label branch April 10, 2026 00:32

jdx mentioned this pull request Apr 10, 2026

chore: release v1.41.1 #806

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(hook): preserve configured output_summary label on failure#808

fix(hook): preserve configured output_summary label on failure#808
jdx merged 1 commit intomainfrom
fix/output-summary-label

jdx commented Apr 10, 2026 •

edited by cursor Bot

Loading

Uh oh!

greptile-apps Bot commented Apr 10, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

jdx commented Apr 10, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

greptile-apps Bot commented Apr 10, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jdx commented Apr 10, 2026 •

edited by cursor Bot

Loading