fix: treat check_list_files stderr as informational, not an error by jdx · Pull Request #425 · jdx/hk

jdx · 2025-11-18T21:39:32Z

Summary

Change check_list_files to only read files from stdout and treat stderr as informational output (warnings, debug info, etc.). The only error condition is when the command exits non-zero with no files in stdout, which indicates a tool failure rather than "files need fixing".

Problem

Previously, check_list_files had overly strict error handling:

Exit 0 with stderr → treated as error
Exit non-zero with empty stdout + stderr → treated as error

This prevented tools from outputting warnings or debug information to stderr, and incorrectly flagged some legitimate scenarios as errors.

Solution

Updated error handling to be more permissive:

Exit Code	Files in stdout	Stderr	Result
0	Empty	Empty	✅ Success - no files need fixing
0	Empty	Non-empty	✅ Success - stderr is just warnings
0	Has files	Any	✅ Success with ⚠️ warning (misconfiguration)
Non-zero	Empty	Any	❌ Error - tool failed
Non-zero	Has files	Any	✅ Run fixer on those files

Key changes:

Files are only read from stdout (never stderr)
Stderr is logged as debug output for visibility
Exit 0 with files in stdout generates a warning (tool should exit non-zero when files need fixing)
The ONLY error case: non-zero exit with no files (indicates tool crashed/failed)

Changes

src/step.rs:
- Removed error check for exit 0 + stderr
- Added warning for exit 0 + files (misconfiguration)
- Updated error handling for non-zero exit with no files
- Regular check commands continue to work as before
test/check_list_files_empty_with_stderr.bats:
- Updated test 1: Exit 0 + stderr is now success (was error)
- Added test 5: Exit 0 + files shows warning

Test Plan

✅ All tests pass with HK_LIBGIT2=0
✅ All tests pass with HK_LIBGIT2=1
✅ Related tests continue to pass: output_summary_check_first.bats, check_fix_suggestion.bats, stage_job_files.bats

🤖 Generated with Claude Code

Note

Make check_list_files read files only from stdout and treat stderr as informational, refine error handling (incl. diff fallback) and add warnings; update tests accordingly.

Core behavior – src/step.rs:
- Treat stderr as informational for both check_list_files and check_diff; read files only from stdout.
- When check_first fails:
  - check_diff: if no parseable files, keep all original files.
  - check_list_files: if files list is empty, treat as error only when the command failed; log stderr and return original error.
- Add debug logging of check stderr; warn when check_list_files exits 0 but outputs files (possible misconfiguration).
Tests – test/check_list_files_empty_with_stderr.bats:
- Update test to accept exit 0 + stderr as success and not run fixes.
- Add tests for non-zero exit with no files (error) and for exit 0 returning files (warn, no fixes).

^{Written by Cursor Bugbot for commit 015a485. This will update automatically on new commits. Configure here.}

Change check_list_files to only treat non-zero exit with no files as an error. Previously, stderr output was treated as an error even when the command exited successfully, which was overly strict. Problem: - check_list_files with exit 0 + stderr was treated as an error - This prevented tools from outputting warnings/info to stderr - The only real error case is: non-zero exit with no files (tool failure) Solution: - When exit 0: stderr is logged as debug only (informational) - When exit 0 with files in stdout: warn about misconfiguration - When exit non-zero with no files: error (tool failed) - When exit non-zero with files: run fixer on those files - Regular check commands continue to work as before Changes: - src/step.rs: Removed stderr error checks, added warning for exit 0 + files - test/check_list_files_empty_with_stderr.bats: Updated all test cases and added new test for the misconfiguration warning All tests pass with both HK_LIBGIT2=0 and HK_LIBGIT2=1. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

@thejcannon

## [1.21.1](https://github.com/jdx/hk/compare/v1.21.0..v1.21.1) - 2025-11-19 ### 🐛 Bug Fixes - **(ruff)** Make `ruff` respect user config `exclude` by [@thejcannon](https://github.com/thejcannon) in [#421](#421) - **(ruff_format)** Pass `--force-exclude` to `ruff format` (as well) by [@thejcannon](https://github.com/thejcannon) in [#428](#428) - Fix --check docstring by [@thejcannon](https://github.com/thejcannon) in [#423](#423) - Configuration Read Support YML File Extension by [@hcoona](https://github.com/hcoona) in [#427](#427) - treat check_list_files stderr as informational, not an error by [@jdx](https://github.com/jdx) in [#425](#425) ### 🚜 Refactor - Enable `trailing-whitespace` in this repo by [@thejcannon](https://github.com/thejcannon) in [#429](#429) ### 📚 Documentation - Don't suggest configuring hk in config env by [@thejcannon](https://github.com/thejcannon) in [#424](#424) ### New Contributors - @thejcannon made their first contribution in [#428](#428) - @hcoona made their first contribution in [#427](#427)  --- > [!NOTE] > Bump to 1.21.1 with updated docs/templates, corrected --check help text, and minor dependency refresh. > > - **Release/versioning**: > - Bump crate to `1.21.1` in `Cargo.toml`, `Cargo.lock`, `hk.usage.kdl`, and docs (`docs/cli/index.md`, `docs/cli/commands.json`). > - Update all `pkl` package URLs and examples to `v1.21.1` (init template in `src/cli/init.rs`, docs, examples). > - Add `CHANGELOG.md` entry for `1.21.1`. > - **CLI/help text**: > - Fix `--check` help to “Run check command instead of fix command” across CLI specs and docs (`docs/cli/*`, `hk.usage.kdl`, `docs/cli/commands.json`). > - **Code/messages**: > - Refresh Pkl error example version in `src/config.rs`. > - **Dependencies**: > - Update `clap`/`clap_builder` to `4.5.52` and `unit-prefix` to `0.5.2` in `Cargo.lock`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 3ed921a. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup>  Co-authored-by: mise-en-dev <[email protected]>

jdx and others added 2 commits November 18, 2025 15:39

Merge branch 'main' into fix/check-list-files-stderr-handling

015a485

jdx merged commit 3a44ff9 into main Nov 19, 2025
19 of 25 checks passed

jdx deleted the fix/check-list-files-stderr-handling branch November 19, 2025 17:09

jdx mentioned this pull request Nov 19, 2025

chore: release v1.21.1 #422

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

fix: treat check_list_files stderr as informational, not an error#425

fix: treat check_list_files stderr as informational, not an error#425
jdx merged 2 commits intomainfrom
fix/check-list-files-stderr-handling

jdx commented Nov 18, 2025 •

edited by cursor bot

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Comments

Conversation

jdx commented Nov 18, 2025 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem

Solution

Changes

Test Plan

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jdx commented Nov 18, 2025 •

edited by cursor bot

Loading