fix(merge): measure squash/rebase against the target's upstream, not a stale local ref - #3549
Conversation
`wt merge` resolves its target to the *local* default-branch ref and squashes/rebases the span `merge-base(target, HEAD)..HEAD`. When the primary checkout's local `main` is behind `origin/main` and the feature descends from the newer `origin/main` tip, that merge-base falls back at the stale local tip, so the span sweeps in every commit already on `origin/main`. The squash folds them into one commit and the local fast-forward lands it — corrupting the primary checkout's `main`, and (if later pushed) duplicating already-upstream content under new SHAs (#3519). Add a local-only guard (no fetch, consistent with local-first merge): refuse the merge when the target branch has an upstream and `merge-base(HEAD, upstream)` is not an ancestor of the local target — the exact condition under which the span re-folds already-upstream commits. It stays quiet on a legitimately-diverged local target (the span is only the feature's own commits) and on repos with no upstream, so it doesn't regress `test_merge_removes_branch_when_local_main_diverged_from_upstream`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
worktrunk-bot
left a comment
There was a problem hiding this comment.
Reviewing as a draft — flagging anything that looks worth a quick fix. Mark ready for a full review.
The core guard is sound. I traced it against both the bug scenario (fires) and test_merge_removes_branch_when_local_main_diverged_from_upstream (feature forks from the shared base → merge-base(HEAD, upstream) is still an ancestor of local main → stays quiet), so the no-regression claim holds. Local-only, no fetch, thorough docstring — all consistent with the local-first posture.
One edge worth a decision before this lands: the guard fires unconditionally, before any merge-mode branching, but it isn't dangerous in every mode. In --no-commit --no-rebase (preserve-exact-graph) there's no squash and no rebase, so HEAD keeps its real ancestry base → C1 → C2 → F with the upstream SHAs, and the finish path fast-forwards local main to HEAD (handle_push with PushKind::MergeFastForward, gated by the is_ancestor_by_sha(target_sha, source_sha) check just above it). In the stale-behind scenario local main is an ancestor of HEAD, so that fast-forward would incorporate C1/C2 by their real SHAs — no new commit, no duplication, a clean result. The guard refuses it anyway, and the "would fold already-upstream commits into main" wording is inaccurate for that path (nothing is folded — it fast-forwards).
The dangerous mode is the default squash (and --rebase), where merge-base(local-main, HEAD)..HEAD gets rewritten into a new SHA — that's the case the PR body analyzes, and the guard is right to block it. So the question is just scope: gate the guard on the modes that actually rewrite (squash_enabled || rebase) so the safe --no-commit --no-rebase fast-forward isn't caught, or keep the broad refusal deliberately (defensible — it nudges the user to update local main first regardless of mode) and reword the message so it doesn't claim folding in a mode that fast-forwards. Either is fine; worth being an explicit choice rather than an artifact of guard placement.
Nit: WORKTRUNK_TEST_PARENT_SHELL: "" in the two help snapshots is an expected catch-up (it's in the standard test env block and 26 other snapshots already carry it; touching the merge help text just regenerated these two) — no action needed, noting it so it doesn't read as a stray change.
…the local ref is stale Rework of the up-front refusal: span_upstream (the same predicate) now redirects measurement instead of blocking. wt step squash (span, count, message, reset base, previews) and wt step rebase (the onto) measure against the target's upstream whenever the branch's history extends past the local ref into it, so already-upstream commits are never folded or replayed. wt merge announces the carry and fast-forwards the local target through the upstream commits by their real SHAs; the refusal remains only for a target that has diverged from its upstream, where no fast-forward exists. Also closes the wt step squash → wt merge bypass of the refusal (the standalone squash folded upstream commits into the branch, after which the merge saw a clean span) and unblocks the FF-only modes it over-refused. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
> _This was written by Claude Code on behalf of max_
) ## Summary Follow-ups from #3549 (the stale-local-ref fix, fixing #3519), recorded as inline TODOs rather than a separate tracking issue: - **`src/commands/worktree/push.rs`**: `MergeContext::prepare`'s `commit_count` mixes carried fast-forwarded upstream commits with the branch's own squash commit when the target was behind its upstream, so the merge success line ("Merged to main (N commits, ...)") overstates the branch's own contribution. Splitting it needs a carried-count threaded through `MergeContext` — deferred as cosmetic. - **`src/commands/step/squash.rs`**: `preview_squash` (backing `--dry-run`/`--show-prompt`) shares the upstream-measured base with `handle_squash`, but only the mutating path has a test asserting it (`test_step_squash_measures_against_upstream_when_local_main_stale`). A snapshot test pinning the preview output in the same stale topology would close this gap. No behavior changes — comments only. ## Test plan - [x] `cargo check` - [x] `cargo clippy --all-targets` - [x] `cargo fmt --check` 🤖 Generated with [Claude Code](https://claude.com/claude-code) > _This was written by Claude Code on behalf of Maximilian Roos_ Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…3558) ## The bug With a rebase in progress (a conflicted stop, or a killed `git`), `wt step rebase main` reported success over a conflicted tree: ```console $ git rebase main # stops on a conflict; HEAD detached mid-replay $ wt step rebase main ○ Already up to date with main $ echo $? 0 ``` `handle_rebase` asked `is_rebased_onto` first. Mid-rebase, HEAD sits on a linear extension of the target, so that returns true and the function returns `UpToDate` before anything consults `worktree_state`. `wt merge` from the same state failed differently but no better: ```console ✗ Cannot merge: not on a branch (detached HEAD) ↳ To switch to a branch, run git switch <branch> ``` The hint names the one command that throws the rebase away instead of finishing it. ## The fix Both commit-replaying commands now check for an open operation before doing anything else. `wt merge` runs the check ahead of its branch check, so the open operation is what gets named rather than the detached HEAD it causes. ```console $ wt step rebase main ✗ Cannot rebase: a git operation is already in progress ``` The gate covers every state detection reports, not just rebase. Measured on the others beforehand: a rebase started mid-bisect ran and stacked a second operation on the first, and one started mid-merge or mid-cherry-pick died inside git's autostash (`Cannot save the current index state`, `f: needs merge`). None told the user what to do next. ## Why it prescribes no remedy An earlier draft of this branch carried a per-operation table of `--continue`/`--abort` hints. It was wrong on first contact with the long tail: `git am` borrows the `rebase-apply` directory, so an open am session was reported as a rebase and pointed at `git rebase --continue`, which git refuses outright: ```console $ git rebase --continue fatal: It looks like 'git am' is in progress. Cannot rebase. ``` Adding an `Am` arm would have treated the enumeration as the fixable part. `git status` already answers this better than the table did, in git's own words and translations, for states worktrunk has no variant for: ```console $ git status interactive rebase in progress; onto b162044 You are currently rebasing branch 'feat' on 'b162044'. (fix conflicts and then run "git rebase --continue") (use "git rebase --skip" to skip this patch) (use "git rebase --abort" to check out the original branch) ``` It names the operation, the branch, the progress, and every way out including `--skip`, which the table never offered. So the table is gone rather than extended, and the refusal is a single line: it says what is blocked and why, and leaves the rest to the command every git user reaches for next. Detection still returns a structured `InProgressOperation` rather than a display string, but now purely as detection: the only caller that reads a variant is the one classifying a rebase worktrunk itself just started. The neighbouring `RebaseConflict` error took the same treatment. It forwards git's stderr in the gutter when it has it, and otherwise substituted two hint lines of its own: ```console ↳ To continue after resolving conflicts, run git rebase --continue ↳ To abort, run git rebase --abort ``` That is the operation table again in miniature — one operation's remedies, minus `--skip` — and the one path that reaches it is the safety check for a rebase git *reported as finished*, where worktrunk knows least about what is actually open. It now prints its title, plus git's own words when git said any. ## One detector Two places probed the same files for the same answer: `Repository::worktree_state` for the gate, and `WorkingTree::is_rebasing`/`is_merging` for the `wt list` gutter symbols. They are now one method, `WorkingTree::operation_in_progress`, which `Repository::operation_in_progress` delegates to — the rename also retires a name that collided with `wt list`'s unrelated `WorktreeState`. Detection now runs in the order `git status` consults the state files, which is a change from the old `wt list` path: it checked rebase before merge. Nothing can reach the difference — no rebase backend writes `MERGE_HEAD`, checked against all three — but matching git's order is one less thing to reason about. `wt list` maps the operations without a gutter symbol (cherry-pick, revert, bisect) to `None`, so its output is unchanged. ## The queued sequencer, now closed Having one detector is what made the last gap worth closing. `git cherry-pick A B` that stops on A writes `CHERRY_PICK_HEAD`; resolving that stop with `git commit` instead of `git cherry-pick --continue` removes the file and leaves B queued: ```console $ git status On branch main Cherry-pick currently in progress. (run "git cherry-pick --continue" to continue) $ ls .git/CHERRY_PICK_HEAD ls: .git/CHERRY_PICK_HEAD: No such file or directory $ cat .git/sequencer/todo pick 1f07533 side1 pick 6b32540 side2 ``` Git reads that same file to keep calling the sequence in progress, so detection reads it too: the first instruction word, `pick` or `revert`, exactly as git's own `sequencer_get_last_command` does. The reason this waited for its own change is the risk of refusing a `wt merge` the user can't un-refuse, so the lifecycle is measured rather than assumed. The directory is gone once the sequence finishes, and gone after `--quit` or `--abort`; a single-commit cherry-pick never creates one; a rebase keeps its todo elsewhere, under `rebase-merge/`. An empty or unrecognized todo reports nothing, so the failure direction is the old behavior rather than a refusal with no way out. ## Verification The repro above, plus the merge, am, cherry-pick, revert, and bisect states, checked by hand against real repos: all five refuse, including am, which is now correct precisely because nothing here claims to know what am needs. Three integration tests drive the real binary from a genuinely stopped operation (`test_step_rebase_refuses_mid_rebase`, `test_merge_refuses_mid_rebase`, `test_merge_refuses_mid_merge`). `test_merge_refuses_mid_merge` is what holds the broadened scope: a conflicted merge keeps HEAD on the branch, so it is the case the detached-HEAD check waves through. Narrowing the gate back to rebase-only turns it red, which I verified by making that edit and re-running. The sequencer case is driven end to end from a real stopped pick — `test_operation_in_progress_reads_the_queued_sequencer` commits the resolution by hand, asserts `CHERRY_PICK_HEAD` is gone, and still expects a cherry-pick, so it can only pass through the sequencer read. It then runs `git cherry-pick --quit` and expects the state to clear, which is the false-refusal half. Running the same sequence against the built binary refuses where it previously reported `○ Already up to date with main`. Confirmed unchanged: already-up-to-date, fast-forward, true-rebase, and rebase-conflict output; a plain detached HEAD still gets the `git switch` hint, which is the right remedy when no operation is open; a sibling worktree mid-rebase does not block a clean one; `wt list`'s gutter symbols. Full gate green (4494/4494 tests, lints, fmt), plus `cargo clippy --all-targets --features shell-integration-tests`, which the gate doesn't compile. ## Merged with main `main` gained #3539, which extracted the failed-rebase classification into `classify_rebase_failure`, and #3549, which adds `span_upstream` — both inside `handle_rebase`. The conflict was semantic as well as textual: #3539's `is_rebasing` check still read the old string API (`worktree_state()?.is_some_and(|s| s.starts_with("REBASING"))`), which this branch removes, so resolving the markers alone left a type error. The resolution keeps #3539's classification and reads the structured enum instead: ```rust let state = repo.operation_in_progress()?; let is_rebasing = matches!(state, Some(InProgressOperation::Rebase)); return Err(classify_rebase_failure(e, is_rebasing, &integration_target)); ``` All five of #3539's classification tests pass alongside the gate. > _This was written by Claude Code on behalf of Maximilian_ --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Fixes #3519.
The bug
wt mergeresolves its target to the local default-branch ref and squashes/rebases the spanmerge-base(local-target, HEAD)..HEAD. When the primary checkout's localmainis behindorigin/mainand the branch descends from the newerorigin/maintip (e.g. created with--base origin/main), that merge-base falls back at the stale local tip, so the span sweeps in every commit already onorigin/main. The squash folds them into a single commit and the local fast-forward lands it — corrupting the primary checkout'smainand, if that ref is later pushed, duplicating already-upstream content under new SHAs. Reproduced deterministically before fixing: a 1-commit feature offorigin/main, with localmaintwo commits behind, squashed "3 commits" into a new SHA on localmain— matching the report.The same mis-measured span reaches every rewrite entry point: a standalone
wt step squashfolds the upstream commits into the branch (the reporter's 1-commit PR becoming 13), andwt step rebasereplays them onto the stale ref when it fires.The fix: measure against the upstream; let the merge carry
One local-only predicate (no fetch — worktrunk is local-first),
Repository::span_upstream: the target's upstream, returned exactly when the branch's history extends past the local target ref into it — i.e.merge-base(HEAD, upstream)is not an ancestor of the local target. The rewrite pipeline responds by measuring there instead of guessing from the stale ref:wt step squash(and the squash insidewt merge) computes its span, commit count, message, andreset --softbase against the upstream — only the branch's own commits fold, and the target ref is never touched. The--dry-run/--show-promptpreviews use the same base.wt step rebase(and the rebase insidewt merge) rebases onto the upstream, so a non-linear branch replays only its own commits instead of duplicating upstream ones onto the stale ref. In the plain stale topology it no-ops, as before.wt mergeannounces the situation up front, then completes: the fast-forward carries localmainthrough the already-fetched upstream commits by their real SHAs to the result — the same graph a fresh localmainwould have produced:The one refusal left is forced by topology rather than chosen by policy: a target that has diverged from its upstream (its own commits and behind, with the branch based past it) can never fast-forward in any mode — reconciling the target's own commits is the user's call, so the merge stops before any rewrite or approval prompt:
This is the reporter's suggested direction ("use
origin/<default>as the merge/rebase target"), minus the network: the already-fetched remote-tracking ref is the measurement base, andwt mergestill never touches the wire. It also extends the rule the rest of worktrunk already applies —IntegrationTargets/ the preview panes' comparison base treat the upstream as the truth when the local default lags it; the merge pipeline was the one mutating surface that didn't.Quiet on everything that must keep working: a legitimately-diverged local target with the branch forked from the shared base (the supported offline workflow — existing regression test unchanged), targets with no upstream, orphan branches, and explicit non-branch targets like
wt step squash origin/main.Tests
test_merge_carries_stale_local_main_through_upstream— the wt merge trusts stale local default-branch ref, can squash unrelated already-upstream commits into it #3519 topology end-to-end: merge succeeds, the carry is announced, the squash folds only the branch's own commits and sits directly on theorigin/maintip, andorigin/mainis an ancestor of the mergedmain(real SHAs, no duplication).test_step_squash_measures_against_upstream_when_local_main_stale— the standalone squash folds only the branch's own commits and never moves the target ref (this scenario previously corruptedmainthrough a squash-then-merge sequence).test_step_rebase_targets_upstream_when_local_main_stale— a branch that mergedorigin/mainin rebases onto the upstream: linearized, real upstream SHAs retained, target ref untouched.test_merge_refuses_diverged_target_when_branch_based_on_upstream— the forced refusal: nothing mutated, branch survives.test_merge_removes_branch_when_local_main_diverged_from_upstream(legitimate divergence, branch from shared base) still passes — no regression.Docs: the merge help page (primary source in
src/cli/mod.rs) describes the upstream-aware measurement, the carry, and the diverged refusal; generated mirrors and help snapshots regenerated.