Skip to content

fix(step): refuse to rebase or merge from a worktree mid-operation - #3558

Merged
max-sixty merged 6 commits into
mainfrom
rebase-in-progress-precheck
Jul 24, 2026
Merged

fix(step): refuse to rebase or merge from a worktree mid-operation#3558
max-sixty merged 6 commits into
mainfrom
rebase-in-progress-precheck

Conversation

@max-sixty

@max-sixty max-sixty commented Jul 23, 2026

Copy link
Copy Markdown
Owner

The bug

With a rebase in progress (a conflicted stop, or a killed git), wt step rebase main reported success over a conflicted tree:

$ 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:

✗ 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.

$ 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:

$ 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:

$ 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:

↳ 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:

$ 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:

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

`wt step rebase main` reported "Already up to date with main" and exited 0
while a conflicted rebase sat unfinished on disk. Mid-rebase, HEAD is
detached on a linear extension of the target, so `is_rebased_onto` returns
true and `handle_rebase` returns `UpToDate` before anything consults
`worktree_state`.

Both commit-replaying commands now check for an open operation first.
`wt merge` gets the same gate ahead of its branch check, where a mid-rebase
detached HEAD previously produced "Cannot merge: not on a branch (detached
HEAD)" and a `git switch <branch>` hint that abandons the rebase rather than
finishing it.

The gate covers every state `worktree_state` reports, not just rebase.
Measured on the other states 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 of them told the user what to do next.

The refusal names no operation and prescribes no remedy, deferring to
`git status`:

    ✗ Cannot rebase: a git operation is already in progress
    ↳ To see what is in progress and how to finish it, run git status

An earlier draft carried a per-operation table of `--continue`/`--abort`
hints. It was wrong on its first contact with the long tail: `git am` borrows
the `rebase-apply` directory, so an am session was reported as a rebase and
sent to `git rebase --continue`, which git refuses outright with "It looks
like 'git am' is in progress". Adding an `Am` arm would have treated the
enumeration as the fixable part. `git status` already names the operation,
the branch, the progress, and every way out including `--skip`, in git's
words and git's translations, for states worktrunk has no variant for — so
the table is gone rather than extended.

`worktree_state` returns a structured `InProgressOperation` instead of 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.

Known gap, unchanged by this commit: resolving a cherry-pick with `git commit`
instead of `git cherry-pick --continue` clears `CHERRY_PICK_HEAD` but leaves
the remaining picks in `.git/sequencer/todo`, which `worktree_state` does not
read, so the gate does not fire there.

> _This was written by Claude Code on behalf of Maximilian_

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@max-sixty
max-sixty force-pushed the rebase-in-progress-precheck branch from 20471c2 to e5ed07f Compare July 23, 2026 23:25
max-sixty and others added 4 commits July 23, 2026 16:41
Resolves src/commands/step/rebase.rs against #3539, which extracted the
failed-rebase classification into `classify_rebase_failure`. Its
`is_rebasing` check reads the structured `InProgressOperation` this branch
introduces instead of matching "REBASING" on a display string.

> _This was written by Claude Code on behalf of Maximilian_

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The title already says an operation is open, and `git status` is the
reflexive next command; spelling it out added a line without adding
information.

> _This was written by Claude Code on behalf of Maximilian_

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
`RebaseConflict` prints git's stderr in the gutter when it has it, and
substituted two hint lines of its own when it didn't:

    ↳ To continue after resolving conflicts, run git rebase --continue
    ↳ To abort, run git rebase --abort

That is the per-operation remedy table this branch already deleted, in
miniature — one operation's commands, minus `--skip` — on the one path
that reaches it: the safety check for a rebase git reported as finished,
where worktrunk knows least about what is actually open. `git status`
names the open operation and every way out of it.

> _This was written by Claude Code on behalf of Maximilian_

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
`Repository::worktree_state` and `WorkingTree::is_rebasing`/`is_merging`
probed the same state files for the same answer — the first for the
commit-replaying gate, the second for `wt list`'s gutter symbols. They are
now one method on `WorkingTree`, which `Repository::operation_in_progress`
delegates to; the rename also retires a name that collided with `wt list`'s
unrelated `WorktreeState`.

With one detector, the last gap is worth closing. A multi-commit
`git cherry-pick` or `git revert` that stops on a conflict writes
`CHERRY_PICK_HEAD`/`REVERT_HEAD`, but resolving that stop with `git commit`
instead of `--continue` removes the file and leaves the rest of the sequence
queued in `sequencer/todo`. Git reads that file to keep calling the sequence
in progress; detection now reads its first instruction word the same way, so
`wt merge` and `wt step rebase` refuse where they previously reported being
up to date.

Measured before trusting it, since a refusal the user can't clear is worse
than the gap: the sequencer directory is gone once the sequence finishes and
gone after `--quit`/`--abort`, a single-commit cherry-pick never creates one,
and a rebase keeps its todo under `rebase-merge/`. An empty or unrecognized
todo reports nothing.

Detection also now runs in the order `git status` consults the files, which
swaps the old `wt list` rebase-before-merge order; no rebase backend writes
`MERGE_HEAD`, so nothing can reach the difference. `wt list` maps the
operations with no gutter symbol to `None`, so its output is unchanged.

> _This was written by Claude Code on behalf of Maximilian_

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing as a draft — flagging anything that looks worth a quick fix. Mark ready for a full review.

The detection consolidation reads cleanly, the queued-sequencer case is handled conservatively (empty/unrecognized todo → None, so the failure direction stays the old behavior), and the resume path is intact — after a conflicted wt merge, git rebase --continue clears the state so a re-run isn't refused. No stale callers of the removed worktree_state/is_rebasing/is_merging remain.

One minor doc nit inline.

Comment thread src/git/repository/mod.rs Outdated
The method is defined on `RepositoryCliExt` in the binary crate, so the
intra-doc link resolved to the `Repository` type page, where it doesn't
appear. Inline code says the same thing without sending readers there.

> _This was written by Claude Code on behalf of Maximilian_

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@max-sixty
max-sixty marked this pull request as ready for review July 24, 2026 14:27
@max-sixty
max-sixty merged commit 84cbb04 into main Jul 24, 2026
40 checks passed
@max-sixty
max-sixty deleted the rebase-in-progress-precheck branch July 24, 2026 14:39
max-sixty added a commit that referenced this pull request Jul 24, 2026
…or every operation (#3579)

Two follow-ups from #3558, which stopped `wt merge` and `wt step rebase`
from running out of a half-finished operation. Probing the sibling
commands for the same root cause found one that was worse, and the
symbol work is what that PR's detection change made possible.

## `wt step squash` committed conflict markers

Mid-merge, invoked directly, it didn't refuse. It generated a commit
message for the unresolved markers and committed them:

```console
$ git merge side
CONFLICT (content): Merge conflict in f
$ wt step squash --yes
◎ Generating commit message and committing changes... (1 file, +6, no squashing needed)
  Merge branch 'side'

  This resolves a merge conflict between main and side branches.
✓ Committed changes @ e6262c6
$ git show HEAD:f
<<<<<<< HEAD
main
||||||| 56e6eb3
base
=======
side
>>>>>>> side
```

`MERGE_HEAD` is gone and the working tree is clean, so the broken merge
reads as complete. `wt merge` was never exposed — its own gate stops it
before it reaches squash — so this was the direct-invocation path only.

## The index is what knows

The obvious fix is the gate #3558 added, and it is not sufficient. The
hazard is narrower than "an operation is open", and also wider.

`git add -A` collapses an unmerged path's three index stages into one
entry. That resolves the conflict as far as the index is concerned,
while `<<<<<<<` is still on disk — and it takes with it git's own
refusal to commit an unmerged index, which is what would otherwise have
stopped this. So the exposure is exactly the commands that stage on the
user's behalf, and it does not need an operation to be open at all:

```console
$ git stash pop
CONFLICT (content): Merge conflict in f
$ ls .git/MERGE_HEAD
ls: .git/MERGE_HEAD: No such file or directory
$ wt step commit --yes
✓ Committed changes @ 1f5387c        # markers and all
```

A conflicted `git stash pop` leaves unmerged paths with no state file
written, so no reading of `.git/` can see it.
`WorkingTree::ensure_no_unmerged_paths` reads the index instead — `git
diff --diff-filter=U`, the same question `git commit` asks — and both
staging paths call it before staging:

```console
$ wt step commit
✗ Cannot commit: 1 path with unresolved conflicts
   ┃ f
```

The paths are worth carrying where the operation refusal carried
nothing: they are the one thing the user needs and git isn't being
asked.

`wt step squash` additionally takes the operation gate, ahead of its
branch check, so mid-rebase it names the open rebase rather than blaming
the detached HEAD and offering `git switch <branch>` — the one command
that throws the rebase away, which is the same wrong remedy #3558
removed from `wt merge`.

Both guards run before the pre-commit hooks and the LLM call. Neither is
worth running for a commit that can't happen, and a refused commit runs
no project commands — checked with a `pre-commit = "touch HOOK_RAN"`
project config that never fires.

`wt step commit --dry-run` is deliberately not gated. It mutates
nothing, and guarding it displaced
`test_step_commit_dry_run_propagates_git_add_failure`, which pins a
distinct error path; a tested behavior is worth more than cosmetic
parity.

## One symbol for every operation

The Status column had `⤴` for rebase and `⤵` for merge, and nothing for
the other three states git can leave open. A worktree stopped
mid-cherry-pick, mid-revert, or mid-bisect rendered as idle — including
the case #3558 closed, a multi-commit cherry-pick whose stop was
resolved with `git commit`, which leaves a clean tree, no
`CHERRY_PICK_HEAD`, and only the queued sequencer to say anything is
wrong.

Three more glyphs was the obvious fix and the wrong one. What the reader
does about any of the five is identical: run `git status`, then finish
or abort it. Splitting the column across a glyph per operation asked
them to distinguish states that lead to the same next step, and the
split is what left the other three invisible. So they collapse to one:

```console
$ wt list
  Branch  Status  …  Message
@ main       ↻^   …  resolved by hand
```

`git status` names which operation it is, in git's own words — the same
division of labor as the refusal message. `--format=json` keeps the
identity the symbol drops, so a consumer that needs it still has it:
`operation_state` gains `cherry_pick`, `revert`, and `bisect` alongside
`rebase` and `merge`.

That retires `ActiveGitOperation`, which existed only to re-encode
`InProgressOperation` down to the two states the gutter knew.
`WorktreeData.git_operation` is now
`Option<Option<InProgressOperation>>` — outer `None` is "not loaded" —
matching its neighbour `has_working_tree_conflicts`. `GitOperationTask`
also stops swallowing errors: it reports a failed probe through the same
`ctx.error` channel every other task uses, rather than reporting "no
operation" for a probe that didn't answer.

## Verification

Three integration tests, each asserting HEAD is unmoved rather than only
matching the message:

- `test_step_squash_refuses_mid_merge` — the case that committed
markers.
- `test_step_commit_refuses_unmerged_paths` — driven from a conflicted
`git stash pop`, so it can only pass through the index read; the
operation check cannot see that state.
- `test_list_shows_symbol_for_bisect` — bisect because it is the only
operation that leaves HEAD on the branch and the tree clean, so nothing
else in the Status cell stands in for it. Snapshots pin both the symbol
and `"operation_state": "bisect"`.

Confirmed by hand against real repos: mid-merge, mid-rebase, and the
conflicted-stash-pop state all refuse; a mid-merge commit whose
conflicts *are* resolved still succeeds, as does a clean squash; a
queued cherry-pick and a stopped revert both render `↻` and name
themselves in JSON.

Full gate green (4500/4500 tests, lints, fmt, doc sync), plus `cargo
test --features shell-integration-tests` (2148/2148) 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 <noreply@anthropic.com>
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