mirror of
https://github.com/max-sixty/worktrunk.git
synced 2026-09-14 20:00:38 +08:00
Maximilian Roos
321ffe838c
`wt step relocate --commit` committed conflict markers, the bug #3579 fixed everywhere else. It stages with `git add -A` and calls `CommitGenerator::commit_staged_changes` directly, so it reached neither of that PR's two gates: ```console $ git merge side # CONFLICT (content): Merge conflict in f $ wt step relocate --commit ◎ Committing changes in feature... ✓ Committed changes @ eb4cf19 $ git show HEAD:f <<<<<<< HEAD feature ======= side >>>>>>> side ``` `MERGE_HEAD` is gone and the working tree is clean afterwards, so the broken merge reads as finished. ## The gate belongs at the staging step, not the commit The obvious funnel is `commit_staged_changes`, the one call all three staging paths share. It cannot work, and the reason rules the shape out rather than merely disfavoring it: every caller stages *before* that call, and `git add -A` collapses an unmerged path's three index stages into one entry. After it, `git diff --diff-filter=U` returns nothing and git itself stops refusing. ```console $ git ls-files -u 100644 df967b9… 1 f 100644 ba2906d… 2 f 100644 2299c37… 3 f $ git add -A $ git diff --name-only --diff-filter=U # empty $ git commit -m x # succeeds ``` A check there passes vacuously in exactly the case it exists for. So the funnel sits one step earlier. `WorkingTree::stage(StageMode, action)` refuses, then stages, with nothing in between. It is now the only path to `git add` against a worktree's real index (every other `git add` writes a throwaway index via `temp_index`), so a fifth entrypoint can neither forget the gate nor order it after the staging that destroys the evidence. It also replaces three hand-rolled copies of the `StageMode` to `git add` match, so the callers come out smaller than they went in. ## The second gate is not belt-and-suspenders The callers keep their early refusals, which guard a different window: those run before hooks, prompts, and the LLM call, so a doomed commit runs no project commands. That placement leaves a hole the early refusal structurally cannot see, because a `pre-commit` hook is arbitrary project code, free to conflict the index after the gate has passed. ```console $ cat .config/wt.toml pre-commit = "git merge side" $ wt step commit --yes ◎ Running pre-commit project hook ✓ Committed changes @ 9e89c76 # markers and all ``` So `wt step commit` carried the same bug, reachable through any project config. That path isn't experimental, which may make it the more consequential half of this PR. `test_step_commit_refuses_hook_created_conflict` pins it. ## relocate skips rather than aborting An unresolved conflict is a per-worktree blocker the user has to fix by hand, like a locked worktree. relocate is a bulk command whose every other blocker (locked, uncommitted, template_error, occupied target) skips one worktree and carries on. Aborting the run would leave the other N-1 unrelocated, and would make passing `--commit` strictly less capable than omitting it. ```console $ wt step relocate --commit ▲ Skipping feature (1 path with unresolved conflicts) conflict.txt ✓ Relocated other: ~/code/wrong-clean → ~/code/myproject.other ``` `--format=json` records it as `reason: "unmerged"`, alongside the existing codes. Relocating a conflicted worktree *without* `--commit` is unchanged. Confirmed by hand that `git worktree move` carries the unmerged index, `MERGE_HEAD`, and the markers across, and that git still refuses to commit at the new path. Nothing stages there, so nothing is lost. ## Detection stays git's Nothing greps file contents for `<<<<<<<`. The whole chain is one structured question, `git diff --name-only --diff-filter=U -z`, which asks git which paths its index records as unmerged: the same question `git commit` asks itself. A content grep would false-positive on this very repo, whose docstrings contain the markers. The only marker-content checks in the diff are two test assertions, that the markers stay on disk for the user and never reach a commit. ## Merged with #3587 #3587 landed while this was in flight (as did #3578, whose `wt step push` refusal test shares the same spot in `merge.rs`) and hoisted `ensure_no_unmerged_paths("merge")` to `wt merge`'s entry, so merge refuses in its own name. The two are complementary: that PR gives each entry point a gate in the name the user typed, and this one guarantees the staging step itself is gated no matter who reaches it. Both sides edited the same docstring and added a test at the same spot; the merge keeps both paragraphs, lists all four entry points, and keeps both tests. The interaction leaves one narrow seam. `wt merge --no-squash`, with a `pre-commit` hook that conflicts the index, now refuses with `Cannot commit` rather than `Cannot merge`, because the entry gate already passed and the funnel names the step it fired in. Fixing the wording would mean threading a caller-supplied noun into `CommitOptions`, which #3587 considered and rejected as the wrong shape, so it stays. The default squash path isn't affected: squash stages before its hooks, so the funnel fires ahead of them. ## Verification Three integration tests, each asserting HEAD is unmoved rather than only matching the message: - `test_relocate_refuses_unmerged_paths`, the reported bug. - `test_relocate_unmerged_skips_only_that_worktree`: one conflicted worktree, one merely dirty. The sibling still relocates, and JSON carries `reason: "unmerged"`. - `test_step_commit_refuses_hook_created_conflict`, driven from a `pre-commit` hook, so it can only pass through the check inside `stage`. Gate green on the merged head (4557/4557, lints, fmt, doc sync), plus `cargo nextest run --features shell-integration-tests` (4546/4546 pre-merge) and `cargo clippy --all-targets --features shell-integration-tests`, which the gate doesn't compile. > _This was written by Claude Code on behalf of Maximilian_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>