feat(hook): add `fail_on_fix` option by jdx · Pull Request #725 · jdx/hk

jdx · 2026-03-06T12:44:26Z

Summary

Adds fail_on_fix boolean option to hooks that causes the hook to fail when fix commands modify files
Useful with stage = false in pre-commit hooks: fixes are applied but the commit is blocked so users can review changes before staging manually
Defaults to false to preserve existing behavior

Closes #723

Test plan

New bats tests: fail_on_fix=true fails on modified files, passes when nothing changes, default behavior unchanged
Existing stage_setting and stage_default tests still pass

🤖 Generated with Claude Code

Note

Medium Risk
Adds new hook execution behavior that can force fix runs to error based on file content changes, which may impact workflows when enabled and introduces extra file I/O/hashing during fix runs.

Overview
Adds a new hook config flag, fail_on_fix (default false), to make fix hooks fail if any files were actually modified by fix commands—useful for stage = false workflows where users want fixes applied but commits blocked until manual review.

Implements this by snapshotting pre-run file content hashes for the hook’s file set and comparing after step groups complete in RunType::Fix, then returning an error and logging the modified file list when changes are detected. Adds Bats coverage for failing/passing cases and for ignoring pre-existing unrelated unstaged changes.

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

When `fail_on_fix = true` on a hook, the hook will fail if fix commands modify files. This is useful with `stage = false` in pre-commit hooks to apply fixes but block the commit, letting users review changes before staging manually. Closes #723 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

gemini-code-assist · 2026-03-06T12:44:42Z

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 introduces a new fail_on_fix configuration option for hooks, providing greater control over the behavior of fix commands. This feature allows developers to configure hooks to automatically apply fixes while preventing the commit from proceeding until the user has explicitly reviewed and staged the changes. This improves the developer experience by ensuring that automated fixes are always acknowledged before being committed, especially in pre-commit scenarios.

Highlights

New fail_on_fix option: Introduced a boolean option for hooks that, when true, causes the hook to fail if fix commands modify files.
Enhanced pre-commit workflow: This option is particularly useful in pre-commit hooks with stage = false, allowing fixes to be applied but blocking the commit until the user manually reviews and stages the changes.
Default behavior preserved: The fail_on_fix option defaults to false, ensuring existing hook configurations continue to function as before.

Changelog

pkl/Config.pkl
- Added the fail_on_fix option with its documentation.
src/hook.rs
- Implemented the fail_on_fix logic, including adding the field to the Hook struct and the conditional failure check.
test/fail_on_fix.bats
- Added new tests to cover the fail_on_fix functionality.

Activity

The author, jdx, created this pull request to add the fail_on_fix option.
The pull request description provides a detailed summary, explains the option's utility, and outlines the test plan.
The changes address and close issue `fix=true` and `stage=false` in pre-commit hook not working as expected #723.
The PR was generated with Claude Code.

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.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

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 introduces a fail_on_fix option for hooks, a valuable feature for workflows where developers wish to review automatic fixes before committing. The implementation is clean and effective, adding a new field to the configuration and the Hook struct, with the primary logic in the hook's run method. This logic correctly identifies unstaged files after a fix run and fails the hook when fail_on_fix is active. The feature is also well-supported by new bats tests that cover the essential scenarios. I have one minor suggestion to enhance code conciseness.

gemini-code-assist · 2026-03-06T12:46:14Z

+                    .unstaged_files
+                    .iter()
+                    .map(|p| format!("  {}", p.display()))
+                    .collect::<Vec<_>>()


For improved conciseness and readability, you can directly use join from the itertools trait (which is already imported in this file) on the iterator. This avoids the intermediate collection into a Vec.

greptile-apps · 2026-03-06T12:49:44Z

Greptile Summary

This PR adds a fail_on_fix boolean option to hooks that causes the hook to exit with a non-zero status when fix commands leave files in a modified (unstaged) state, making it easy to apply auto-fixes while still blocking a commit so the user can review the changes before re-staging. The feature is implemented cleanly across the PKL schema, the Rust Hook struct, and is covered by three new bats tests.

Key changes:

pkl/Config.pkl – new fail_on_fix: Boolean = false property with documentation and a usage example.
src/hook.rs – new pub fail_on_fix: bool field (serde default); post-step-group check that reads repo.status() and returns an error when unstaged files are found (gated on RunType::Fix).
test/fail_on_fix.bats – three scenarios: fixer modifies files → failure; no-op fixer → success; default (no fail_on_fix) with fixer → success.

Issue found:
The fail_on_fix check inspects all unstaged files after the fixers run rather than only files that were changed by the fixers. When stage = false (the recommended pairing), stash_method defaults to StashMethod::None, so pre-existing unstaged changes in the working tree are never stashed. This means any user who has unrelated unstaged edits will see a spurious failure and a misleading warning even when the fixers made no modifications at all — contradicting the documented behaviour ("fail when fix commands modify files").

Confidence Score: 3/5

Safe to merge for clean worktrees, but can produce false-positive failures for users who regularly have unrelated unstaged changes alongside stage = false.
The implementation logic for the common case (fixer modifies files → fail) is correct and well-tested. However, the post-fix status check does not filter out pre-existing unstaged files, leading to false positives that contradict the documented behaviour. This is a correctness gap in a common real-world workflow (partial staging), not merely a style concern.
Pay close attention to src/hook.rs lines 677–693 (the fail_on_fix check) and the missing test case in test/fail_on_fix.bats.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[hk run hook] --> B{RunType is Fix?}
    B -- No --> C[Run step groups in Check mode]
    B -- Yes --> D{stash_method not None?}
    D -- Yes --> E[Stash pre-existing unstaged changes]
    D -- No --> F[Pre-existing unstaged changes remain]
    E --> G[Run step groups - fixers execute]
    F --> G
    G --> H{result ok AND fail_on_fix true?}
    H -- No --> I[Skip fail_on_fix check]
    H -- Yes --> J[repo.status after fix]
    J --> K{unstaged_files empty?}
    K -- Yes --> L[result stays Ok]
    K -- No --> M[warn and result set to Err\nNOTE: includes pre-existing\nunstaged files if stash is None]
    L --> N[pop_stash if applicable]
    M --> N
    I --> N
    C --> O[Return result]
    N --> O

_{Last reviewed commit: 4e0e6b0}

greptile-apps · 2026-03-06T12:49:47Z

+        if result.is_ok() && self.fail_on_fix && matches!(run_type, RunType::Fix) {
+            let status = repo.lock().await.status(None)?;
+            if !status.unstaged_files.is_empty() {
+                let file_list = status
+                    .unstaged_files
+                    .iter()
+                    .map(|p| format!("  {}", p.display()))
+                    .collect::<Vec<_>>()
+                    .join("\n");
+                warn!(
+                    "Files were fixed but not staged (fail_on_fix=true). Review the changes and stage them manually:\n{}",
+                    file_list
+                );
+                result = Err(eyre::eyre!(
+                    "fail_on_fix: files were modified by fix commands"
+                ));
+            }


False positive when pre-existing unstaged files exist

The fail_on_fix check reads all unstaged files from the repository after the fixers run, but it does not distinguish between files that were modified by the fixers and files that were already unstaged before the hook ran.

When stage = false (the primary documented use-case), stash_method defaults to StashMethod::None (line 355), so pre-existing unstaged changes are never stashed (line 637-657 skips stashing when method is None). If the user has any unrelated unstaged changes in the working tree, status.unstaged_files will be non-empty even when the fixers made zero modifications — causing the hook to fail when the documented behaviour ("fail when fix commands modify files") says it should pass.

The test "fail_on_fix=true passes when no files are modified" only covers a clean working tree; it does not exercise the case where pre-existing unstaged files coexist with a no-op fixer.

A more accurate implementation would capture the set of unstaged files before the step groups run, then compare against the post-fix set, failing only when there is a meaningful delta:

// Before running groups, snapshot pre-existing unstaged files let pre_unstaged: BTreeSet<_> = repo.lock().await.status(None)? .unstaged_files.into_iter().collect(); // ... run groups ... // fail_on_fix: only fail when fixers actually changed something if result.is_ok() && self.fail_on_fix && matches!(run_type, RunType::Fix) { let status = repo.lock().await.status(None)?; let newly_modified: Vec<_> = status.unstaged_files.iter() .filter(|p| !pre_unstaged.contains(*p)) .collect(); // Also check files that existed before but now have different content // (e.g. via git diff --name-only pre_unstaged ∩ post_unstaged) if !newly_modified.is_empty() { ... result = Err(eyre::eyre!("fail_on_fix: files were modified by fix commands")); } }

Without this guard, any user who runs hk with fail_on_fix = true while their worktree contains unrelated unstaged edits will see a spurious failure and a misleading "Files were fixed but not staged" warning even though the fixers touched nothing.

Address review feedback: - Use file content hashing instead of unstaged file set comparison to correctly detect fixer modifications even on already-unstaged files - Avoid early return via `?` that could skip pop_stash cleanup - Add test for pre-existing unstaged files not causing false positives Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Treat deleted files as modified (unwrap_or(true) instead of false) - Skip pre-run file hashing when run_type is Check (not just Fix) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

cursor

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

- Use neutral warning message that doesn't assume stage=false - Fix no-op test to actually exercise the fail_on_fix code path by creating an unstaged change so hk picks up the file Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@jdx

### 🚀 Features - **(hook)** add `fail_on_fix` option by [@jdx](https://github.com/jdx) in [#725](#725) ### 🐛 Bug Fixes - **(builtins)** remove redundant check/check_diff from builtins by [@nkakouros](https://github.com/nkakouros) in [#726](#726) ### 📦️ Dependency Updates - update anthropics/claude-code-action digest to 26ec041 by [@renovate[bot]](https://github.com/renovate[bot]) in [#720](#720) - update jdx/mise-action digest to e79ddf6 by [@renovate[bot]](https://github.com/renovate[bot]) in [#721](#721) - update actions-rust-lang/setup-rust-toolchain digest to a0b538f by [@renovate[bot]](https://github.com/renovate[bot]) in [#719](#719) - update rust crate tokio to v1.50.0 by [@renovate[bot]](https://github.com/renovate[bot]) in [#722](#722)  --- > [!NOTE] > **Medium Risk** > Primarily a release/version bump, but it also updates `Cargo.lock` (including TLS/crypto-related crates like `aws-lc-*`), which could introduce build or runtime regressions. > > **Overview** > Prepares the `v1.38.0` release by adding the `1.38.0` section to `CHANGELOG.md` and bumping the project version to `1.38.0` across `Cargo.toml`, CLI usage/docs (`hk.usage.kdl`, `docs/cli/*`), and various docs/examples. > > Updates documentation Pkl `amends`/`import` example URLs to reference the `v1.38.0` release artifacts, and refreshes `Cargo.lock` with updated dependency versions (notably `aws-lc-*`, `windows-sys`, `getrandom`, `socket2`, `tokio-macros`, `uuid`). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 40b9c73. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup>  Co-authored-by: mise-en-dev <123107610+mise-en-dev@users.noreply.github.com>

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [hk](https://github.com/jdx/hk) | minor | `1.36.0` → `1.38.0` | 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.38.0`](https://github.com/jdx/hk/blob/HEAD/CHANGELOG.md#1380---2026-03-06) [Compare Source](jdx/hk@v1.37.0...v1.38.0) ##### 🚀 Features - **(hook)** add `fail_on_fix` option by [@jdx](https://github.com/jdx) in [#725](jdx/hk#725) ##### 🐛 Bug Fixes - **(builtins)** remove redundant check/check\_diff from builtins by [@nkakouros](https://github.com/nkakouros) in [#726](jdx/hk#726) ##### 📦️ Dependency Updates - update anthropics/claude-code-action digest to [`26ec041`](jdx/hk@26ec041) by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#720](jdx/hk#720) - update jdx/mise-action digest to [`e79ddf6`](jdx/hk@e79ddf6) by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#721](jdx/hk#721) - update actions-rust-lang/setup-rust-toolchain digest to [`a0b538f`](jdx/hk@a0b538f) by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#719](jdx/hk#719) - update rust crate tokio to v1.50.0 by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#722](jdx/hk#722) ### [`v1.37.0`](https://github.com/jdx/hk/blob/HEAD/CHANGELOG.md#1370---2026-03-03) [Compare Source](jdx/hk@v1.36.0...v1.37.0) ##### 🚀 Features - **(hook)** add env support to hooks by [@jdx](https://github.com/jdx) in [#709](jdx/hk#709) - parse Go-style diffs by [@jdx](https://github.com/jdx) in [#704](jdx/hk#704) ##### 🐛 Bug Fixes - **(builtins)** strip extra trailing newlines in end-of-file-fixer by [@jdx](https://github.com/jdx) in [#708](jdx/hk#708) - **(docs)** correctly document what --all is about by [@nkakouros](https://github.com/nkakouros) in [#715](jdx/hk#715) - **(git)** exclude untracked files from unstaged\_files set by [@nkakouros](https://github.com/nkakouros) in [#716](jdx/hk#716) - **(hkrc)** config format and load order by [@ivy](https://github.com/ivy) in [#710](jdx/hk#710) - **(release)** write release notes to file instead of capturing stdout by [@jdx](https://github.com/jdx) in [#688](jdx/hk#688) - **(release)** make release notes editorialization non-blocking by [@jdx](https://github.com/jdx) in [#690](jdx/hk#690) - **(step)** gate check\_diff forced check\_first on Fix mode only by [@nkakouros](https://github.com/nkakouros) in [#717](jdx/hk#717) ##### 📚 Documentation - **(shanty)** add audio player with sea shanty recording by [@jdx](https://github.com/jdx) in [67a25ad](jdx/hk@67a25ad) - document config file search paths by [@ivy](https://github.com/ivy) in [#701](jdx/hk#701) - require AI disclosure on GitHub comments by [@jdx](https://github.com/jdx) in [#703](jdx/hk#703) ##### 🔍 Other Changes - replace gen-release-notes script with communique by [@jdx](https://github.com/jdx) in [#700](jdx/hk#700) - add autofix.ci workflow by [@jdx](https://github.com/jdx) in [#705](jdx/hk#705) ##### 📦️ Dependency Updates - lock file maintenance by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#686](jdx/hk#686) - update taiki-e/upload-rust-binary-action digest to [`f391289`](jdx/hk@f391289) by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#692](jdx/hk#692) - update anthropics/claude-code-action digest to [`c22f7c3`](jdx/hk@c22f7c3) by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#691](jdx/hk#691) - update rust crate libc to v0.2.181 by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#694](jdx/hk#694) - update rust crate clap to v4.5.58 by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#693](jdx/hk#693) - lock file maintenance by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#695](jdx/hk#695) - update anthropics/claude-code-action digest to [`edd85d6`](jdx/hk@edd85d6) by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#698](jdx/hk#698) - update rust crate clap to v4.5.60 by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#699](jdx/hk#699) - lock file maintenance by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#702](jdx/hk#702) - lock file maintenance by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#711](jdx/hk#711) ##### New Contributors - [@ivy](https://github.com/ivy) made their first contribution in [#710](jdx/hk#710) - [@nkakouros](https://github.com/nkakouros) made their first contribution in [#715](jdx/hk#715) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, 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 6, 2026

View reviewed changes

cursor Bot reviewed Mar 6, 2026

View reviewed changes

Comment thread src/hook.rs Outdated

greptile-apps Bot reviewed Mar 6, 2026

View reviewed changes

cursor Bot reviewed Mar 6, 2026

View reviewed changes

Comment thread src/hook.rs Outdated

Comment thread src/hook.rs Outdated

fix(hook): handle deleted files and skip hashing in check mode

c943233

- Treat deleted files as modified (unwrap_or(true) instead of false) - Skip pre-run file hashing when run_type is Check (not just Fix) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

cursor Bot reviewed Mar 6, 2026

View reviewed changes

Comment thread src/hook.rs

Comment thread test/fail_on_fix.bats

jdx merged commit 421aabd into main Mar 6, 2026
19 checks passed

jdx deleted the feat/fail-on-fix branch March 6, 2026 19:55

jdx mentioned this pull request Mar 6, 2026

chore: release v1.38.0 #687

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(hook): add `fail_on_fix` option#725

feat(hook): add `fail_on_fix` option#725
jdx merged 4 commits intomainfrom
feat/fail-on-fix

jdx commented Mar 6, 2026 •

edited by cursor Bot

Loading

Uh oh!

gemini-code-assist Bot commented Mar 6, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Mar 6, 2026

Uh oh!

Uh oh!

greptile-apps Bot commented Mar 6, 2026

Uh oh!

greptile-apps Bot Mar 6, 2026

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

jdx commented Mar 6, 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!

gemini-code-assist Bot commented Mar 6, 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!

gemini-code-assist Bot Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps Bot commented Mar 6, 2026

Greptile Summary

Confidence Score: 3/5

Flowchart

Uh oh!

greptile-apps Bot Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jdx commented Mar 6, 2026 •

edited by cursor Bot

Loading