pre-commit stash restore corrupts partially-staged files: stash patch written without trailing newline → git-apply "corrupt patch" → manual fallback splits lines

[ad1269]

Versions: hk 1.46.0 (latest), git 2.50.1 (Apple Git-155), macOS arm64, pkl 0.31.1

Edit: revised after reading the v1.46.0 source — the original "corrupt patch → manual fallback" causal chain was wrong; the manual merge IS the primary restore path, and I've now pinned the exact buggy branch. Repro also made portable (no inline comments, no sed).

Summary

Any file with both staged and unstaged hunks gets its unstaged hunks re-applied at the wrong position after hk run pre-commit: an edit to the last line comes back split onto a new line with a leading space. Reproduces 100%, with both stash = "patch-file" and stash = "git", even when no fix command modifies the file. Unstaged changes in other (non-partially-staged) files restore correctly.

Minimal repro

Prereqs: hk 1.46.0 + the pkl CLI on PATH (e.g. mise use -g hk pkl). Paste as a block (no inline comments — default zsh eats them):

git init t && cd t
cat > hk.pkl <<'EOF'
amends "package://github.com/jdx/hk/releases/download/v1.46.0/hk@1.46.0#/Config.pkl"
hooks { ["pre-commit"] { fix = true; stash = "patch-file"; steps {
    ["noop"] { glob = Regex(#"\.txt$"#); check_first = false; fix = "true" }
} } }
EOF
printf 'l1\nl2\nl3: tail\n' > f.txt
git add -A && git commit -m base
printf 'l1 STAGED\nl2\nl3: tail\n' > f.txt
git add f.txt
printf 'l1 STAGED\nl2\nl3: tail UNSTAGED\n' > f.txt
hk run pre-commit
cat f.txt

Expected (last line restored intact, ONE line):

l1 STAGED
l2
l3: tail UNSTAGED

Actual (last-line edit split onto a new line, note the leading space):

l1 STAGED
l2
l3: tail
 UNSTAGED

Root cause (source-pinned, v1.46.0)

The restore path for both stash modes is pop_stash's manual per-file merge (StashType::PatchFile is defined but never constructed). After merge::three_way_merge_hunks, src/git.rs has a special case that prefers "fixer + appended tail" when the worktree change looks like a pure tail insertion:

// Try strict prefix first
let mut tail_opt = w.strip_prefix(i);
// If that fails, allow a single trailing newline discrepancy
if tail_opt.is_none() && i.ends_with('\n') {
    tail_opt = w.strip_prefix(&i[..i.len().saturating_sub(1)]);
}

The newline-tolerant fallback misclassifies an edit to the last line as a tail insertion whenever the index snapshot ends with \n (i.e. nearly always):

• i = …l3: tail\n, w = …l3: tail UNSTAGED\n
• strict prefix fails (\n vs ), but stripping i's trailing newline makes …l3: tail a prefix of w
• → tail = " UNSTAGED\n" → result = fixer content (ending …l3: tail\n) + " UNSTAGED\n"

…which is exactly the observed corruption, leading space included. Edits to non-final lines of partially-staged files take the regular merge path and survive; it's specifically last-line edits that get split.

Secondary bug: the recovery backup patch is unparseable

The stash backup written to ~/.local/state/hk/patches/ (the stash_backup_count recovery mechanism) is missing its trailing newline — git apply --check on it fails with error: corrupt patch at line N. So when the merge above mangles a file, the documented recovery artifact can't be applied either. (Possibly related to the EOF-newline area touched by #293.)

Workaround we're using

stash = "none" plus a git-hook-side guard that rejects partially-staged commits. (stash = "none" alone silently stages unstaged hunks of fixer-touched files into the commit, so it isn't safe by itself.)

Happy to test a fix — this repro is scripted in our migration test matrix.

[ad1269]

I've raised #966 to fix this.

It addresses both bugs: the newline-tolerant fallback in the tail-insertion special case now only accepts the EOF-newline-removal case (last-line edits fall through to the regular three-way merge), and the backup patch gets its trailing newline restored so git apply can parse it. The repro from this discussion is included as a regression test covering both stash modes.

This comment was generated by Claude Code.