Skip to content

fix(merge): refuse unresolved conflicts in merge's own name - #3587

Merged
max-sixty merged 1 commit into
mainfrom
rebase-in-progress-precheck
Jul 25, 2026
Merged

fix(merge): refuse unresolved conflicts in merge's own name#3587
max-sixty merged 1 commit into
mainfrom
rebase-in-progress-precheck

Conversation

@max-sixty

Copy link
Copy Markdown
Owner

Follow-up to #3579, which added the index gate that keeps conflict markers out of a commit. The gate works on wt merge; it just doesn't refuse in merge's name.

$ git stash pop
CONFLICT (content): Merge conflict in f
$ wt merge --yes
✗ Cannot squash: 1 path with unresolved conflicts
  f
$ wt merge --yes --no-squash
✗ Cannot commit: 1 path with unresolved conflicts
  f

wt merge stages through two different steps, so it borrowed whichever noun it happened to delegate to. Its sibling gate on the same command says Cannot merge, so one command named itself three ways depending on flags.

Threading a caller-supplied noun into handle_squash and CommitOptions would fix the wording, and it's the wrong shape: wt merge already runs ensure_no_operation_in_progress("merge") at its entry, in its own name. Each command gates for itself; the index gate is the one that hadn't been hoisted yet. So it moves up next to its sibling:

repo.ensure_no_operation_in_progress("merge")?;
current_wt.ensure_no_unmerged_paths("merge")?;

The inner guards stay where they are — wt step commit and wt step squash are entry points too, and they keep answering in their own names.

Refusing at the entry also puts it ahead of the approval prompts and the hook plan, rather than partway through the flow.

--no-commit changes which error it hits. It used to reach the dirty-tree check first:

$ wt merge --yes --no-commit
✗ Cannot merge with --no-commit: feature has uncommitted changes
  UU f
↳ Commit or stash changes first

Committing is the one thing that must not happen here, and stashing a conflicted index isn't something git will do either.

Verification

Every entry point, against a real repo stopped on a conflicted git stash pop, with HEAD unmoved in each:

before after
wt merge --yes Cannot squash Cannot merge
wt merge --yes --no-squash Cannot commit Cannot merge
wt merge --yes --no-commit Cannot merge with --no-commit: … uncommitted changes Cannot merge
wt step squash --yes Cannot squash unchanged
wt step commit --yes Cannot commit unchanged

test_merge_refuses_unmerged_paths pins the first row. It's driven from the conflicted-stash-pop helper, which writes no state file, so it can only pass through the index read — and it asserts HEAD is unmoved, not just the message.

Full gate green (4548/4548, lints, fmt, doc sync) on the rebased head, plus cargo clippy --all-targets --features shell-integration-tests.

This was written by Claude Code on behalf of Maximilian

`wt merge` inherited the index gate from whichever step it delegated to,
so it answered `Cannot squash` (via `wt step squash`) or `Cannot commit`
(via the direct-commit path) for a command the user typed as `merge`.

Its sibling operation gate already ran at merge's entry in merge's own
name; the index gate is the one that hadn't been hoisted. Moving it next
to `ensure_no_operation_in_progress("merge")` restores the pattern —
each entry point gates in its own name — and refuses ahead of the
approval prompts instead of partway through the flow.

`--no-commit` changes which error it hits: the dirty-tree check used to
answer "commit your changes", which is the wrong advice when the changes
are conflict markers.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@max-sixty
max-sixty merged commit b6b7457 into main Jul 25, 2026
39 checks passed
@max-sixty
max-sixty deleted the rebase-in-progress-precheck branch July 25, 2026 01:41
max-sixty added a commit that referenced this pull request Jul 25, 2026
#3587 hoisted `ensure_no_unmerged_paths("merge")` to `wt merge`'s entry so it
refuses in its own name. Both sides edited the same docstring and added a test
at the same spot.

Docstring: keeps both paragraphs and lists all four entry points. The two
additions are complementary — #3587's explains why `action` is caller-supplied,
this branch's explains that every entry gate is the *early* refusal and points
at `stage` for the one adjacent to `git add`.

Tests: keeps both, entry-point refusals grouped before the funnel test.
max-sixty added a commit that referenced this pull request Jul 25, 2026
`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>
max-sixty added a commit that referenced this pull request Jul 25, 2026
Eleven commits landed on `main` while #3590 sat in CI, so they ship in
v0.69.2 with no changelog entry. Most are user-facing, and three are
data-loss fixes — exactly the drift the release skill's step-12 check
exists to catch. The tag is held until this lands.

Entries added, most-impactful first:

- **#3589** — two ways shell integration deleted user data on a
substring guess: an rc line that only *quotes* the init command, and a
user's own `conf.d/wt.fish` deleted outright by `install`'s legacy
cleanup. Plus forge detection moving from `host.contains("github")` to
label-wise matching.
- **#3585 + #3591** — truncate-in-place rc writes replaced by
write-temp-then-rename, then generalized to every user-file write.
- **#3578** — despite its `docs(step):` subject this carries three
behavior fixes, including `wt step push` succeeding mid-rebase and
moving the target branch onto a half-replayed history.
- **#3588 + #3587** — conflict markers can no longer reach a commit via
`wt step relocate --commit`, and `wt merge` refuses in its own name.
- **#3554** — OpenCode marker writes could land in another session's
worktree (thanks @4i3n6, who both reported and fixed it).

Not documented, per convention: #3574 (doc comments only, no runtime or
docs-site surface) and the three dependency bumps.

Also corrects `.github/CLAUDE.md`, which lists three required status
checks; there are four — `fast-checks` is required too, confirmed
against the branch-protection API.

Entries verified against the diffs by subagent. Two corrections came
back and are folded in: the #3589 entry originally promised "two of them
deleted" and described only one, and the #3578 entry mis-quoted the
success line as `✓ Pushed to main` when it carries a commit count.

> _This was written by Claude Code on behalf of max_
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants