fix(hook): show combined output for failed steps by nkakouros · Pull Request #772 · jdx/hk

nkakouros · 2026-03-25T22:05:18Z

Summary

On failure, always show combined stdout+stderr output so diagnostic messages are never lost regardless of which stream the tool
writes to
Respects output_summary = "hide" even on failure
The output_summary setting now only filters output for successful steps (e.g., to surface deprecation warnings on stderr)

This is a framework-level fix rather than per-builtin configuration — tools that write diagnostics to stdout (eslint, flake8,
prettier, etc.) will now have their output visible when they fail, without changing any builtin definitions.

Original description - wrong approach

Details

Most linters and formatters write their error output to stdout, but output_summary defaults to stderr. This meant diagnostic output was invisible in the per-step summary for 75 builtins.

Affected categories:

All check_diff tools (diff output goes to stdout)
Linters: eslint, flake8, pylint, mypy, rubocop, golangci-lint, etc.
Type checkers: tsc, ty, mypy, sorbet, etc.
Formatters in check mode: prettier, black, ruff, shfmt, stylua, etc.

Tools that correctly write to stderr (cargo, go vet, deno, ghalint, taplo lint, etc.) were left with the default.

nkakouros · 2026-03-25T22:05:50Z

The alternative would be to switch the default for output_summary to stdout, but that would be a breaking change.

gemini-code-assist · 2026-03-25T22:05:54Z

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the visibility of diagnostic output for a significant number of built-in tools by configuring them to direct their standard output to the step summary. This resolves an issue where critical information from linters, formatters, and type checkers was not being displayed, improving the debugging and review experience for affected tools.

Highlights

Visibility of Diagnostic Output: The output_summary property has been explicitly set to stdout for numerous built-in tools. This change ensures that diagnostic messages, which many linters and formatters output to standard output, are now correctly captured and displayed in the per-step summary.
Broad Tool Coverage: This fix impacts a wide range of tools across various categories, including all check_diff tools, linters (e.g., eslint, flake8, pylint, rubocop), type checkers (e.g., tsc, mypy, sorbet), and formatters in check mode (e.g., prettier, black, ruff, shfmt). Previously, their output was often invisible due to the default stderr capture.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request adds output_summary = "stdout" to numerous Config.Step definitions across various pkl/builtins files. However, for specific tools such as markdownlint, shellcheck, sorbet, and yamllint, this configuration is incorrect. These tools output their diagnostic information to stderr, and setting output_summary to stdout would hide their output. The reviewer suggests removing these specific output_summary lines to maintain proper diagnostic visibility.

nkakouros · 2026-03-25T23:08:15Z

Failing checks are pre-existing.

jdx

output_summary is intended for displaying warnings (e.g., deprecation notices, version info) that tools emit on stderr even when they pass — it's not meant for showing diagnostic output from failures.

Setting it to stdout on these builtins would dump the tool's primary output into the summary on every run, including successes. For check_diff tools that's raw diff output; for linters it's the full error listing or "all clean" messages. None of that belongs in the summary.

The underlying issue (diagnostic output not being visible when a step fails) is real, but it needs a different solution — probably something at the framework level rather than changing output_summary on individual builtins.

This comment was generated by Claude Code.

nkakouros · 2026-03-26T00:17:14Z

Aha, I finally get what the setting is about:

  ✔ superlint
  ✔ yamllint
  ✗ eslint – ERROR

  superlint stderr:
  WARNING: v2.x is deprecated, upgrade to v3  <-- this here is still useful to show, despite success, right?

  eslint stdout:
  file.js:10: error: unused variable

I will attempt another fix then. How about showing the combined output whenever a step fails?

jdx · 2026-03-26T00:27:44Z

that's probably ok

greptile-apps · 2026-03-26T09:22:14Z

Greptile Summary

The PR correctly fixes diagnostic output visibility on step failure by always showing combined stdout+stderr, while properly respecting output_summary = "hide". The prior review concern has been addressed. No issues found.

Confidence Score: 5/5

Safe to merge — the fix is minimal, logically correct, and the prior review concern is resolved.

No P0 or P1 findings remain. The output_summary=hide concern from the prior review thread has been correctly addressed.

No files require special attention.

Important Files Changed

Filename	Overview
src/step/output.rs	Failure path now always shows combined output (respecting `hide`); success path preserves prior per-variant behavior. Logic is correct and the prior review concern is resolved.

_{Greploops — Automatically fix all review issues by running /greploops in Claude Code. It iterates: fix, push, re-review, repeat until 5/5 confidence.
Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal.}

_{Reviews (3): Last reviewed commit: "Merge branch 'main' into fix/builtins-ou..." | Re-trigger Greptile}

…_summary The output_summary setting controls which stream is captured for the end-of-run summary. It defaults to "stderr" which is useful for surfacing side-channel warnings (e.g., deprecation notices) from successful steps. However, many tools write their diagnostic errors to stdout, not stderr, so failing steps had invisible output. Instead of configuring output_summary per-builtin, always show combined stdout+stderr output when a step fails. The output_summary setting now only applies to successful steps, except for output_summary = "hide" which is respected even on failure. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

nkakouros · 2026-03-29T15:12:24Z

Failing ci check is fixed in #782.

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]>

## Summary - 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. - Uses combined *content* on failure (preserving #772's intent) but keeps the configured *label* so the header matches `output_summary`. - Fixes CI failures on #806. ## Test plan - [x] `test/output_summary_no_duplicate.bats` passes locally (both tests) - [ ] CI passes on this PR 🤖 Generated with [Claude Code](https://claude.com/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. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit f0090e4. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>  Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>

@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 Mar 25, 2026

View reviewed changes

Comment thread pkl/builtins/markdown_lint.pkl Outdated

Comment thread pkl/builtins/shellcheck.pkl Outdated

Comment thread pkl/builtins/sorbet.pkl Outdated

Comment thread pkl/builtins/yamllint.pkl Outdated

nkakouros force-pushed the fix/builtins-output-summary branch 2 times, most recently from 16e900e to 77c976c Compare March 25, 2026 22:57

jdx requested changes Mar 25, 2026

View reviewed changes

nkakouros force-pushed the fix/builtins-output-summary branch from 77c976c to 4486910 Compare March 26, 2026 09:18

greptile-apps Bot reviewed Mar 26, 2026

View reviewed changes

Comment thread src/step/output.rs Outdated

nkakouros marked this pull request as draft March 26, 2026 20:05

nkakouros force-pushed the fix/builtins-output-summary branch from 4486910 to bb23b5b Compare March 29, 2026 14:36

nkakouros changed the title ~~fix: fix output_summary for tools writing diagnostics to stdout~~ fix(hook): show combined output for failed steps Mar 29, 2026

nkakouros marked this pull request as ready for review March 29, 2026 14:56

Merge branch 'main' into fix/builtins-output-summary

896da3f

jdx merged commit 5f8cfaa into jdx:main Apr 9, 2026
18 checks passed

jdx mentioned this pull request Apr 9, 2026

chore: release v1.41.1 #806

Merged

jdx mentioned this pull request Apr 10, 2026

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

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(hook): show combined output for failed steps#772

fix(hook): show combined output for failed steps#772
jdx merged 2 commits intojdx:mainfrom
nkakouros:fix/builtins-output-summary

nkakouros commented Mar 25, 2026 •

edited

Loading

Uh oh!

nkakouros commented Mar 25, 2026

Uh oh!

gemini-code-assist Bot commented Mar 25, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nkakouros commented Mar 25, 2026

Uh oh!

jdx left a comment

Uh oh!

nkakouros commented Mar 26, 2026

Uh oh!

jdx commented Mar 26, 2026

Uh oh!

greptile-apps Bot commented Mar 26, 2026 •

edited

Loading

Uh oh!

Uh oh!

nkakouros commented Mar 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

nkakouros commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Original description - wrong approach

Uh oh!

nkakouros commented Mar 25, 2026

Uh oh!

gemini-code-assist Bot commented Mar 25, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nkakouros commented Mar 25, 2026

Uh oh!

jdx left a comment

Choose a reason for hiding this comment

Uh oh!

nkakouros commented Mar 26, 2026

Uh oh!

jdx commented Mar 26, 2026

Uh oh!

greptile-apps Bot commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

Uh oh!

nkakouros commented Mar 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nkakouros commented Mar 25, 2026 •

edited

Loading

greptile-apps Bot commented Mar 26, 2026 •

edited

Loading