From 7a2b2022cd34785ac603e4101271684f3106d9f0 Mon Sep 17 00:00:00 2001
From: jdx <216188+jdx@users.noreply.github.com>
Date: Thu, 30 Apr 2026 08:31:49 -0500
Subject: [PATCH] fix(hook): do not stage fixes when fail_on_fix=true
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When `fail_on_fix=true` is set on a hook, the step's auto-staging would
silently re-stage the fixer's output over the user's explicit `git add`
choices. After the failed commit the user couldn't see the proposed
fix as an unstaged change for review, and a re-commit would silently
succeed with the fix baked in — defeating the entire point of
`fail_on_fix`.

Force `should_stage=false` during fix runs when `fail_on_fix=true`. The
fixer's output stays in the worktree as an unstaged change, the user's
staged changes are preserved, and a re-commit will fail again until the
user explicitly accepts the fix.

Closes #888

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/hook.rs           |  2 ++
 test/fail_on_fix.bats | 50 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/src/hook.rs b/src/hook.rs
index 368b165b1..4da4881cb 100644
--- a/src/hook.rs
+++ b/src/hook.rs
@@ -826,6 +826,8 @@ impl Hook {
             return Ok(());
         }
         let run_type = self.run_type(&opts);
+        // fail_on_fix exists to surface fixes for review; staging would defeat that.
+        let should_stage = should_stage && !(self.fail_on_fix && matches!(run_type, RunType::Fix));
         let repo = Arc::new(Mutex::new(Git::new()?));
         let groups = self.get_step_groups(&opts);
         let stash_method = if let Some(stash_str) = &opts.stash {
diff --git a/test/fail_on_fix.bats b/test/fail_on_fix.bats
index 7f58377a7..a201a081e 100644
--- a/test/fail_on_fix.bats
+++ b/test/fail_on_fix.bats
@@ -117,3 +117,53 @@ EOF
 
     hk run fix
 }
+
+@test "fail_on_fix=true preserves staged changes and surfaces fix as unstaged (#888)" {
+    cat <<EOF > hk.pkl
+amends "$PKL_PATH/Config.pkl"
+hooks {
+    ["pre-commit"] {
+        fix = true
+        fail_on_fix = true
+        steps {
+            ["normalize"] {
+                glob = "*.json"
+                fix = #"for f in {{ files }}; do tr -d ' ' < "\$f" > "\$f.tmp" && mv "\$f.tmp" "\$f"; done"#
+            }
+        }
+    }
+}
+EOF
+    # Initial committed state has spaces that the fixer will strip.
+    echo '{"a": 1}' > a.json
+    echo "original" > b.md
+    git add hk.pkl a.json b.md
+    git commit -m "initial commit"
+    hk install
+
+    # User makes intentional changes to both files, but only stages a.json.
+    echo '{"a": 2}' > a.json
+    echo "modified" > b.md
+    git add a.json
+
+    # Pre-commit must fail with fail_on_fix.
+    run git commit -m "update"
+    assert_failure
+
+    # The user's staged change to a.json must survive: index still differs from HEAD
+    # in the test value, NOT in the formatting (which is the fixer's contribution).
+    run git diff --cached --name-only
+    assert_output "a.json"
+    run git diff --cached a.json
+    assert_output --partial '"a": 2'
+    refute_output --partial '{"a":2}'
+
+    # The fix should now be visible as an unstaged change on a.json (whitespace removed).
+    run git diff --name-only
+    assert_line "a.json"
+    assert_line "b.md"
+
+    # b.md unstaged change must be preserved.
+    run cat b.md
+    assert_output "modified"
+}