[Bennyjitsu]

Squash reset atomicity

wt step squash / wt merge --squash: a commit failure after the soft
reset (e.g. blocked by a pre-commit hook) left the branch's original commits
collapsed into anonymous staged changes instead of restored. Fixed by
capturing the pre-reset SHA and rolling back to it on failure.

Also fixes has_staged_changes(), which silently reported "no staged
changes" on a real git failure instead of propagating it — the bug that made
the rollback's own error-check path unreachable.

Update (this revision)

The PR originally also added an --author flag to merge/step commit/
step squash, bundled with this fix since they touched the same functions.
Per this review's feedback: GIT_AUTHOR_NAME/GIT_AUTHOR_EMAIL env vars
already propagate through every commit path the flag touched (git commit,
commit-tree via env, squash), since nothing here scrubs the environment —
verified live against all three paths before removing it. The flag also had
a real correctness bug (its author-spec parser didn't match git commit --author's own semantics, and the same value was accepted on one merge
path but rejected on another), not worth fixing in code that turned out to
be unnecessary. It's gone; this PR is now just the atomicity fix.

Also fixed: the error-wrapping bug this review flagged in
restore_after_failed_reset — the rollback outcome now rides as context on
top of the original error instead of flattening it into a Display string,
so a double failure (commit fails, then the rollback also fails) no longer
loses the original git error from the output. Proven with a direct unit
test (confirmed failing against the prior code, passing against the fix).

Full test suite passes (4,935 tests, 0 failed), full pre-merge gate
(clippy, doc-sync) green.

Update 2 (this revision)

Fixed a third bug this review flagged: the no-net-changes early return in
handle_squash (commits that squash down to zero net diff) left the branch
soft-reset to the merge base while reporting success, instead of restoring
pre_reset_sha like the other two post-reset exits already do. wt merge's
cleanup then reads that state as "same commit as target" and deletes the
worktree and the branch, discarding the branch's original (unmerged)
commits with no safety backup taken.

This path predates the PR — it wasn't introduced by the atomicity fix above
— but it's the same function, so fixed here rather than filed separately.
test_merge_squash_empty_changes now correctly shows the net-zero commits
being merged into the target, instead of the branch being silently deleted
as "already merged".

[worktrunk-bot]

The squash-atomicity half is a real fix and the rollback itself is correct — git reset --soft restores HEAD while leaving the index and working tree exactly as the stage step left them, so the branch comes back intact. Two things hold the verdict.

--author's entire behaviour is already reachable without a new flag. Every git child wt spawns inherits the environment — Cmd never clears it, and scrub_git_discovery_env_vars strips only the path vars (GIT_DIR, GIT_WORK_TREE, GIT_COMMON_DIR, GIT_INDEX_FILE, GIT_OBJECT_DIRECTORY). So GIT_AUTHOR_NAME="Bot" GIT_AUTHOR_EMAIL="bot@example.com" wt merge --no-ff already sets the author on all four paths this PR threads --author through, with the committer left ambient — which is exactly what the flag's help text promises. I verified against git 2.55 that both git commit and git commit-tree honour those two variables and leave the committer alone.

That makes the flag ergonomics rather than capability, and CLAUDE.md is explicit that "a command, option, user-facing concept, or configuration key is a product decision. Its user value must justify the code and documentation surface it adds." Here that surface is a flag on three commands, three doc pages, a hand-rolled ident parser that doesn't actually match the git commit --author it's modelled on (inline), and a new Repository::run_command_with_env. Whether the ergonomics are worth it is a maintainer call, not mine — so this is a COMMENT rather than an approval, not a request to drop the feature.

#4192 already fixes has_staged_changes, with the same exit-code split plus two flags this version omits. Details inline; rebasing onto it removes the conflict and is strictly more correct.

One thing to know before splitting the PR: the two halves are coupled through the tests. test_step_squash_failed_commit_restores_original_history uses an invalid --author as its failure injector, so the atomicity fix would need a different one — a .git/hooks/pre-commit exiting non-zero fires in the same place, since the squash commit doesn't pass --no-verify, and that is also the trigger the PR description names as the original bug.

[worktrunk-bot]

#4192 makes this same exit-code change to this same function, and additionally passes --no-relative and --ignore-submodules=none, with a regression test for the diff.relative case.

Those flags matter here rather than being incidental: git diff --cached is porcelain, so it reads display configuration that changes what it reports. diff.relative is the configuration CLAUDE.md's Data Safety section records as having "once made wt remove delete an unmerged branch", and "Plumbing for Output wt Consumes" is the rule this call sits under — wt consumes the answer as a boolean, so the invocation needs its behaviour pinned.

Rebasing onto #4192 and dropping this hunk gets the fix plus the flags and removes the conflict between the two PRs.

[worktrunk-bot]

The third post-reset exit still leaves the branch rewound, and it reports success. The new comment calls the !has_staged path "a normal, intentional outcome left as pre-existing behavior", but that path returns Ok(SquashResult::NoNetChanges) with the branch still soft-reset to the merge base. Pre-existing, yes — but the PR now captures pre_reset_sha three lines above it, and the two exits it does cover are the same reset leaking the same history.

Where that costs the user is wt merge. The reset leaves the branch pointing at the merge base, so the cleanup step reads it as "same commit as main" and removes the worktree and deletes the branch. That check is worktrunk's guard against deleting unmerged work, and the squash's own reset is what makes an unmerged branch satisfy it. With a clean worktree no safety backup is taken, and deleting the branch and the worktree takes both reflogs that named those commits with them — afterwards git fsck is the only thing that still finds them.

Not introduced by this diff, so it needn't be fixed here. It also isn't mechanical, which is why I'd rather flag it than push a fix: restoring pre_reset_sha on this path makes the "No changes after squashing N commits" line literally true and keeps the branch intact, but it also changes what wt merge does next — it would go on to merge two commits that net to zero into main instead of treating the branch as already there. Which of those is wanted is a call for you.

Reproduction, against this PR's merged tree

A branch whose commits cancel out. No config needed — the fallback commit-message path reproduces it too.

git init -q -b main . && echo base > base.txt && git add -A && git commit -qm base
wt switch -c feature            # then cd into the feature worktree
echo x > tmp.txt && git add -A && git commit -qm "add tmp"
git rm -q tmp.txt && git commit -qm "remove tmp"
wt merge --yes --no-hooks

◎ Squashing 2 commits into a single commit...
◎ Generating squash commit message...
○ No changes after squashing 2 commits
○ Already up to date with main (no new commits, no rebase needed)
◎ Removing feature worktree & branch in background (same commit as main, _)

Afterwards, in the primary worktree:

$ git reflog --all
a4b3a6c refs/heads/main@{0}: commit (initial): base
a4b3a6c HEAD@{0}: commit (initial): base

$ git fsck --lost-found
dangling commit 723f67fbbbf77219d18f65c136dbaff6a0be86ca

wt step squash on its own stops a step earlier: exit 0, No changes after squashing 2 commits, and the branch has silently moved from its tip to the merge base.

[worktrunk-bot]

The restore leaves faithful state — reset --soft moves only HEAD, so the index keeps whatever wt.stage put there and the branch returns to its real tip — but nothing asserts that.

test_step_squash_no_net_changes_json builds exactly the scenario this commit fixes (three commits that cancel out, then wt step squash) and checks only that the JSON reports no_net_changes. That assertion passed before this commit too, with the branch silently rewound to the merge base, and it will pass again if the restore is ever dropped. The only thing pinning the new behaviour is merge_squash_empty_changes.snap, and a snapshot gets re-accepted with cargo insta test --accept without anyone weighing what moved.

The commit-failure path already has the direct check, in test_step_squash_failed_commit_restores_original_history: rev-parse feature before and after. The same two lines close this one.

// before the `wt step squash` invocation
let pre_squash_head = repo.git_output(&["rev-parse", "feature"]);

// after the existing outcome assertion
assert_eq!(
    repo.git_output(&["rev-parse", "feature"]),
    pre_squash_head,
    "a no-net-change squash must leave the branch tip where it was"
);

That test sits outside this PR's diff, so GitHub won't take it as an applyable suggestion — say the word and I'll push it.

[max-sixty]

I think there is actually something to do here, but it looks quite different to this

[worktrunk-bot]

Agreed the shape here isn't right. For whatever you build instead, here is what I could verify still holds on main at v0.79.0 (83cc7a76b) — both reproduced with a debug build of that commit, not against this PR's tree.

wt merge deletes a branch whose commits are not in the target. This is the one I'd scope the work around. When a branch's commits net to zero diff, the soft reset at squash.rs:307 moves the branch to the merge base and the early return at squash.rs:318 leaves it there. Cleanup then reads "same commit as main" and removes the worktree and the branch. The backup is gated on has_staged at squash.rs:259, which is false here, so nothing is written to refs/wt, and deleting the branch and worktree takes both reflogs that named the commits. git fsck is what's left.

The check that fires is worktrunk's guard against deleting unmerged work, and the squash's own reset is what makes an unmerged branch satisfy it. merge_squash_empty_changes.snap already records the deletion as expected output.

A failed commit leaves the branch rewound. With a pre-commit hook that exits 1, wt step squash prints ✗ Failed to create squash commit and leaves the branch at the merge base with the contents as anonymous staged changes. Recoverable from the reflog, unlike the above, but the branch is left rewound on a path that reported failure.

What I can't settle is the semantics, which is probably the part that makes it look different: restoring the branch on the no-net-changes path keeps it intact, but then wt merge goes on to merge commits that net to zero into main rather than treating the branch as already there. Happy to build whichever you'd prefer, or to leave it if you're taking it somewhere else.

Reproductions, against a debug build of 83cc7a76b

Branch deletion — feature has two commits that cancel out, neither in main:

git init -q -b main repo && cd repo
echo base > base.txt && git add -A && git commit -qm base
wt switch -c feature            # then cd into the feature worktree
echo x > tmp.txt && git add -A && git commit -qm "add tmp"
git rm -q tmp.txt && git commit -qm "remove tmp"
wt merge --yes --no-hooks

○ No changes after squashing 2 commits
○ Already up to date with main (no new commits, no rebase needed)
◎ Removing feature worktree & branch in background (same commit as main, _)

Afterwards, from the primary worktree:

$ git rev-parse --verify feature
fatal: Needed a single revision
$ git merge-base --is-ancestor 42fa48a main && echo yes || echo NO
NO
$ git for-each-ref refs/wt
(empty)

Failed commit — same setup with two ordinary commits, plus printf '#!/bin/sh\nexit 1\n' > .git/hooks/pre-commit:

$ git log --oneline main..feature      # before
41cdd7a commit B
e0a7a1a commit A
$ wt step squash --yes --no-hooks
✗ Failed to create squash commit
$ git log --oneline main..feature      # after — empty
$ git diff --cached --name-only
a.txt
b.txt