Skip to content

fix: never strand a merged branch when the post-merge restack fails - #143

Merged
rohoswagger merged 2 commits into
mainfrom
fix/merge-cleanup-orphans
Aug 11, 2026
Merged

fix: never strand a merged branch when the post-merge restack fails#143
rohoswagger merged 2 commits into
mainfrom
fix/merge-cleanup-orphans

Conversation

@rohoswagger

Copy link
Copy Markdown
Owner

The bug

Both merges in this session left orphans behind — fix/needs-restack-from-git and fix/merge-restack-shared-engine still had branches and worktrees after their PRs merged, and ez sync reported Everything is up to date.

merge_branch ran in this order (merge.rs:410-429):

state.remove_branch(branch);                       // entry gone from stack.json
state.save()?;                                     // persisted
move_to_main_root_for_targets(...)?;
let (restacked, pushed) =
    fetch_restack_and_push_remaining(...)?;        // ✗ bails here
cleanup_merged_branch(...)?;                       // never runs

The metadata is deleted before the step that can fail, and cleanup sits behind that ?. So a restack failure produces a branch and worktree with no stack entry — and ez sync builds its cleanup candidates from state.branches (sync.rs:453-467), so an untracked branch is never a candidate. The orphan is invisible to every command that could remove it.

This is not specific to the stale-entry bug that triggered it. Post-#142, an ordinary rebase conflict in the post-merge restack reproduces it exactly.

Fixes

1. Clean up before restacking — sequential path and native-stack path both. The branch is merged and its entry is already gone; whether some sibling rebases cleanly has no bearing on deleting it. We're already back in the main root by that point, so the move is safe.

2. A safety net in sync for orphans that already exist or that a future ordering bug creates. prune_orphaned_ez_worktrees removes the worktree, local branch, and remote branch for untracked-but-merged branches, running after the tracked pass so nothing is considered twice.

Scope is deliberately narrow, because this deletes branches ez isn't tracking:

Situation Action
Untracked, worktree under .worktrees/, merged Cleaned
Untracked, worktree under .worktrees/, not merged Kept + warned, with an ez track hint
Worktree outside .worktrees/ (Superconductor, manual git worktree add) Untouched
No worktree at all Untouched
Trunk in a second worktree Untouched

.worktrees/ is the proof that ez created it, so it is ez's litter to collect. Merge detection prefers the PR's merged flag and falls back to is_ancestor — a closed-but-unmerged PR is not merged and keeps its branch.

Tests

  • merged_branch_is_cleaned_up_even_when_the_post_merge_restack_fails — real repo, real remote, fake gh. A sibling branch is set up to conflict with the new trunk so the post-merge restack genuinely fails; asserts the call still errors and that the merged branch's worktree, local branch, and stack entry are all gone.
  • orphaned_ez_worktrees_only_claims_untracked_branches_in_ez_owned_worktrees — pins the safety scope table above.

487 unit tests + all integration suites green, clippy clean.

`ez merge` removed the branch from stack.json and saved, then restacked the
rest of the stack, then cleaned up the merged branch — with the restack behind
a `?`. A restack failure therefore skipped cleanup after the entry was already
gone, leaving a branch and worktree that no command can see: `ez sync` builds
its cleanup candidates from `state.branches`, so an untracked branch is never
a candidate. It reports "Everything is up to date" while the litter sits there.

Two fixes:

1. Clean up before restacking, in both the sequential and native-stack paths.
   The branch is merged and its entry is already gone; whether some sibling
   rebases cleanly has no bearing on deleting it.

2. Give `ez sync` a safety net for orphans that already exist, or that some
   future ordering bug creates. `prune_orphaned_ez_worktrees` removes the
   worktree, branch, and remote branch for untracked branches that are merged.

The safety net is deliberately narrow: only worktrees under `.worktrees/`,
which ez creates and owns. External worktrees and plain local branches are
left alone however merged they look, and an orphan that is not provably
merged is reported with a `ez track` hint rather than deleted — an orphan
with unmerged commits is lost work, not litter.
Review findings on the orphan-cleanup path added in the previous commit.

An orphan has no recorded PR number, so the only way to find its PR is
`get_pr_statuses_for`, which queries `headRefName` and takes the most recently
created match. That resolves by branch *name*, not by head SHA or head repo.
Trusting its `merged` flag alone meant a recycled branch name — or, in a fork
workflow, another contributor's merged `feat/login` — authorized deleting a
worktree and force-deleting a branch that still held unpushed commits. The
tracked cleanup path avoids this by looking PRs up by number; that is not
available here, so git is the corroboration instead: the tip must already be
in trunk, or the branch's diff against trunk must be empty (squash merge).

Also from review:

- Stop deleting the remote branch. The evidence is about the local tip and
  says nothing about commits pushed from elsewhere, and sync's tracked cleanup
  does not delete remote refs either — only `ez delete` and `ez merge` do.
- Anchor worktree ownership to `{main_root}/.worktrees/` instead of a bare
  `contains("/.worktrees/")`, which claimed every sibling worktree when the
  repo itself lives under a `.worktrees/` directory.
- Move out of the worktree before removing it when it contains the cwd, as the
  tracked loop already does; otherwise the rest of the sync runs from a
  deleted directory.
- Re-read `git worktree list` inside the prune rather than reusing the map
  captured before the cleanup loop. A branch cleaned by the tracked pass is
  out of `state.branches` and its worktree is gone, so the stale entry matched
  every orphan rule and produced a spurious warning on the happy path.
@rohoswagger

Copy link
Copy Markdown
Owner Author

Review pass applied

Ran a multi-lens review over the diff. The merge reordering came back clean — verified that deleting the merged branch cannot strand its children (reparent_children_preserving_parent_head leaves each child's parent_head at a commit that is by construction an ancestor of the child's own ref, so it stays reachable, and effective_old_base returns the right replay range), that move_to_main_root_for_targets still precedes cleanup so the chdir-before-remove invariant holds, and that the earlier remote deletion changes nothing GitHub-visible.

The new sync deletion path needed four fixes, now in c0bb5e3:

Critical — name-matched PR authorized deletion. get_pr_statuses_for queries pullRequests(headRefName:"<branch>", first:1, orderBy:CREATED_AT DESC) — it resolves by name, with no head-SHA or head-repo check. Taking pr.merged as sufficient meant a recycled branch name, or a fork workflow where two contributors use the same name, would delete a worktree and force-delete a branch holding unpushed commits. The tracked path avoids this by looking PRs up by number, which orphans can't do. Git now has to agree: tip in trunk, or an empty diff against trunk for squash merges.

High — remote branch deleted on weaker evidence than the local one. Nothing verified origin/<branch> was in trunk, so a commit pushed from another machine after the merge would become unreachable. Also inconsistent with sync itself, which never deletes remote refs (only ez delete and ez merge do). Dropped.

Medium — unanchored ownership check. contains("/.worktrees/") claimed every sibling worktree when the repo is cloned under a .worktrees/ directory, contradicting the doc comment. Now anchored to {main_root}/.worktrees/.

Medium — no chdir-out before removing the current worktree, which the tracked loop handles deliberately. Running ez sync from inside an orphan would have deleted its own cwd, then run restacks and state.save() from a deleted directory.

Plus one I caught before dispatching: the prune reused the worktree map captured before the cleanup loop, so every normally-cleaned branch was re-examined as an orphan and warned about on the ordinary happy path. It now re-reads git worktree list itself.

Not fixed, noted deliberately: --force extends to untracked worktrees without recording what it discarded; cleanup failure now aborts the restack rather than running after it (recoverable with ez restack, and strictly better than the orphan it replaces).

490 unit tests + all integration suites green, clippy clean. Four new tests pin the fixes.

@rohoswagger
rohoswagger merged commit 2922e84 into main Aug 11, 2026
6 checks passed
rohoswagger added a commit that referenced this pull request Aug 11, 2026
Three stale-metadata and cleanup fixes since v0.3.2 (#141, #142, #143, #144).

Also backfills the 0.3.0–0.3.2 rows in the version history table, which were
never recorded.
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.

1 participant