feat: Free a branch's worktree on finish and delete - #245
Merged
Merged
Conversation
Closes the worktree lifecycle for topic branches (#175, part of #171, refs #45): finish and delete now free the worktree a branch was checked out in as part of deleting the branch, instead of leaving a stale checkout behind or failing outright because Git refuses to delete a branch that is still checked out somewhere. What "freeing" a worktree means depends on its provenance, read from the marker recorded at creation time (never from matching the path against the template): a worktree git-flow created is removed and its marker cleared; one the user made by hand is kept, with its HEAD detached from the branch so the directory and every file in it, including uncommitted work, survive untouched. Two new CLI-only flags control this on both commands: --keep-worktree routes even a git-flow-created worktree through the detach path instead of removing it, and --force-worktree/-W allows removing a git-flow-created worktree that has uncommitted or untracked changes. Neither flag has a git config equivalent, matching checkout's --worktree/--force. Both commands run a pre-flight before any destructive step: a worktree with a merge, rebase, or bisect in progress can be neither removed nor detached, and a git-flow-created worktree slated for removal refuses uncommitted or untracked changes without --force-worktree. Finish runs this pre-flight before the merge starts and again, identically, at the top of a resumed --continue, so a finish that reaches branch deletion either way never arrives with an unfreeable worktree; the two flags' resolved values are persisted onto the merge state for --continue to read back. Freeing a worktree is skipped entirely when the branch itself is being kept (--keep/--keeplocal on finish): it only ever happens because the branch is about to disappear. Running finish or delete from inside the very worktree being freed first redirects the operation to a repo handle bound elsewhere — the parent branch's own worktree if it has one, else the main worktree for finish; always the main worktree for delete, which has no merge target to prefer — so the worktree is left untouched until the free step, the same way the merge's own checkouts of the parent (and, for finish, its auto-updated children) would otherwise either repurpose that worktree before the free step ever saw it, or fail outright against a branch already checked out elsewhere. If the invoking shell is standing inside the worktree being removed, its replacement is written to GIT_FLOW_CD_FILE; detaching never navigates, since the directory never moves. Branches with no worktree, or checked out in the main worktree, are unaffected by any of this. A branch with no worktree, or one checked out in the main worktree, passed through unaffected, per existing test coverage for both commands. Adds internal/git's WorktreeOperationInProgress, checking a worktree's own git-dir for MERGE_HEAD/rebase-merge/rebase-apply/BISECT_LOG, plus a WorktreeOperationInProgressError, and extends WorktreeDirtyError with an optional Flag field so finish/delete can name --force-worktree instead of 'worktree remove's own --force. Co-authored-by: Alex Younger <thealexyounger@proton.me> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds OPTIONS entries, a Worktree Cleanup subsection, and examples for --keep-worktree and --force-worktree/-W on both git-flow-finish(1) and git-flow-delete(1), and cross-references git-flow-worktree(1) from both. Extends finish's exit-status description of code 6 to cover the new worktree-dirty and operation-in-progress refusals. No gitflow-config.5.md changes: per the settled scoping decision for #175, both flags are CLI-only with no git config equivalent. Closes #175 Co-authored-by: Alex Younger <thealexyounger@proton.me> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Delete redirection can produce incorrect mergedness checks and prevent post-delete hooks from executing.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds provenance-aware worktree cleanup to branch finish and delete workflows.
Changes:
- Removes managed worktrees or detaches unmanaged/kept worktrees.
- Adds cleanup flags, validation, navigation, and resumable finish state.
- Adds documentation and integration coverage.
File summaries
| File | Description |
|---|---|
cmd/worktree_cleanup.go |
Implements shared cleanup logic. |
cmd/finish.go |
Integrates cleanup into finish state machine. |
cmd/delete.go |
Integrates cleanup into deletion. |
cmd/topicbranch.go |
Registers cleanup flags. |
cmd/shorthand.go |
Adds flags to shorthand commands. |
internal/git/worktree.go |
Detects in-progress worktree operations. |
internal/errors/errors.go |
Adds cleanup-specific errors. |
internal/mergestate/mergestate.go |
Persists cleanup choices. |
docs/git-flow-finish.1.md |
Documents finish behavior. |
docs/git-flow-delete.1.md |
Documents delete behavior. |
test/cmd/finish_worktree_test.go |
Tests finish cleanup scenarios. |
test/cmd/delete_worktree_test.go |
Tests delete cleanup scenarios. |
test/internal/git/worktree_ops_test.go |
Tests operation detection. |
Review details
- Files reviewed: 13/13 changed files
- Comments generated: 6
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
An independent review (a second AI reviewer plus Copilot) surfaced four correctness gaps in the #175 worktree cleanup, all in the redirect mechanism finish and delete use when invoked from inside a branch's own worktree. Delete's redirect ran too late. It happened inside performDelete, after hooks.WithHooks had already captured the pre-redirect repo handle for the whole operation, including the post-delete hook. A delete that removed the worktree the user was standing in then ran its post-hook against a working directory that no longer existed. The redirect now happens in executeDelete, before WithHooks is called, so pre-hook, operation, and post-hook all agree on the same surviving repo. Delete's mergedness check could run against the wrong branch. Once redirected to the main worktree (because the parent has no dedicated worktree of its own), the existing "checkout parent if currently on the branch" guard never fired, since the branch being deleted is by definition no longer the current one after a redirect. `git branch -d` then checked mergedness against whatever the main worktree happened to have checked out — not the topic's actual parent — and could refuse a branch that was genuinely merged. redirectPreferringParentWorktree (the renamed, delete-shared form of finish's own redirect helper) now reports whether it redirected, and delete uses that to still ensure HEAD reflects the parent before the check runs. finish --continue silently dropped the worktree flags. A refusal named --force-worktree as the way out, but re-running '--continue --force-worktree' parsed the flag and then discarded it — only the persisted choice from the initial invocation was ever read. The flags now OR into the persisted state (both only ever make cleanup MORE permissive, so there is no unsafe direction to guard against), and the merged choice is written back so a later conflict-and-continue round keeps it. --continue also gained its own redirect, mirroring the initial run's: it is a fresh process invocation, and the user may still be sitting in the worktree the initial run redirected away from, while the conflict they are resolving lives wherever that redirect landed. Both additions are gated on state.Action != "integrate", which shares this state machine but never reaches worktree handling at all. WorktreeOperationInProgress missed cherry-pick and revert. A hand-made worktree mid-cherry-pick passed preflight cleanly (the detach path skips the dirty check by design, and a cherry-pick leaves no untracked files of its own), so DetachWorktree's checkout --detach would fail on the unresolved index — a refused cleanup after the branch was already merged, the exact case preflight exists to prevent. CHERRY_PICK_HEAD and REVERT_HEAD join the existing merge/rebase/bisect markers. A worktree lookup error (repo.WorktreeForBranch on the parent branch) was also silently swallowed at two of the three sites that read it for navigation/redirect purposes, both defaulting to "no parent worktree, use main" on failure rather than surfacing the error — risking landing an operation in the main worktree while the parent was actually checked out elsewhere. Both now propagate. Adds four tests: a delete-from-inside-own-worktree scenario that diverges main and develop to make the mergedness bug reproducible, a post-delete hook that proves it ran outside the removed worktree, a continue-then-force-worktree round trip, and a bisect-detection test for the primitive (only merge and rebase had coverage before). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Extends the "merge, rebase, or bisect in progress" wording in both git-flow-finish(1) and git-flow-delete(1) to also name cherry-pick and revert, matching the primitive's now-wider detection. Also fixes a git-flow-delete(1) sentence that read as if the removed worktree's OWN path were written to GIT_FLOW_CD_FILE; what is written is the main worktree's path, which is what the sentence already correctly said everywhere else. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Rebase and resumed linked-worktree finishes can fail, while rejected deletion can prematurely remove or detach the worktree.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
cmd/delete.go:210
- The worktree is freed before non-forced branch deletion is known to succeed. For a clean but unmerged branch, removal/detachment succeeds and the following
git branch -drefuses, leaving the branch alive but its managed worktree gone (or its handmade worktree unexpectedly detached). Check deletion eligibility before freeing the worktree; after that validation succeeds, delete in a way that cannot re-run the same check after cleanup.
freedRepo, err := freeWorktreeForBranch(repo, fullBranchName, worktreeOpts, "")
if err != nil {
return err
}
repo = freedRepo
- Files reviewed: 14/14 changed files
- Comments generated: 5
- Review effort level: Balanced
A second Copilot review round found four further correctness gaps in the #175 worktree redirect mechanism, all confirmed against the code (not taken on the reviewer's word) and each pinned with a regression test that fails without its fix and passes with it. Rebase, abort, and the --ff-only recovery path all tried to check the topic branch out again on the (possibly redirected) operating repo. That fails outright once the branch has its own separate worktree — exactly the situation a #175 redirect creates on purpose, specifically so the merge's own checkouts leave that worktree alone. Rebase genuinely needs to run wherever the branch already is (refs are shared across every worktree of one repository, so it doesn't matter which handle does it); abort and the --ff-only recovery checkout have nothing to do in that case, since the branch is already exactly where it needs to be. All three now share one small helper, topicWorktreeIfSeparate, that decides which applies. finish --continue and --abort could fail to find a redirected operation at all. Merge state is deliberately keyed per-worktree (TestMergeStateNotSharedBetweenWorktrees pins that as intentional, so the fix must not switch to shared storage), but a redirected initial run leaves its state in the parent's own worktree, or main — not in the topic's own worktree a fresh --continue process reopens from, if the user is still standing where they started. executeFinish now tries that same redirect as a fallback when nothing is found locally, before dispatching to --continue/--abort. handleContinue's own redirect (added in the first review round) is now provably redundant, since its caller always hands it the right repo, so it's removed rather than left as dead weight. delete's worktree preflight ran too late — after the parent checkout and the ffParent fast-forward had already mutated state, contrary to the "pre-flight before any destructive step" promise finish already keeps. Moved earlier, ahead of both. delete also froze the worktree before knowing whether 'git branch -d' would actually succeed: a clean-but-unmerged branch (no uncommitted changes, just commits not yet merged into its parent) had its worktree removed or detached, and only then found the branch itself correctly refused deletion — a refusal that "worked" but still cost the user their worktree. Git has no dry-run for branch -d, so a plain ancestor check (branch must be an ancestor of whatever is now checked out, which the steps above already arrange to be the parent whenever that mattered) runs first when not forced; a false negative there just means the real branch -d call further down, unreached in the cases that matter, makes the final call. Also adds cherry-pick and revert coverage for WorktreeOperationInProgress, which had markers for both since the first review round but no dedicated tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A round-1 doc fix for the same sentence introduced a new bug: it attributed "has no merge target to prefer" to finish, when finish is exactly the command that DOES have one (the parent branch) and prefers its worktree; delete is the one with no such target, which is why it always offers the main worktree. Reworded to state the distinction the right way round. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
Author
Fixed in ed98b55 — before freeing the worktree, delete now checks whether the branch is an ancestor of whatever's currently checked out (mirroring |
main removed worktree.IsManaged (576055a, before this branch caught up with it) once ListMarkers' bulk read became its only caller; this branch's two single-branch call sites, added independently, went in before that landed and never got the memo. Replaced with a small isManaged helper that reads the same bulk marker list and checks membership — the only query left for a single branch — at the same one-git-process cost IsManaged itself had. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Linked-worktree rebase recovery is broken, and delete can remove a worktree before Git subsequently refuses branch deletion.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 5
- Review effort level: Balanced
A third review round found that running the rebase step in the topic's own separate worktree (added last round to fix the original "already used by worktree" checkout failure) does not actually work: it splits one operation's state across two git-dirs. mergestate's JSON lands in the redirected worktree, while git's own rebase state lands in the topic worktree — and mid-rebase, that worktree's detached HEAD makes WorktreeForBranch blind to it. Verified by building and reproducing directly rather than reasoning about it: --continue fails with "no merge in progress" (the state lookup never finds it), and --abort exits 0 having silently done nothing — the rebase, the detached HEAD, and the stranded merge.json are all left exactly as they were. Consulted a second model on whether to patch the three consequences individually or revert the mechanism. Patching would mean coordinating at least six places that assume the operation's state and the conflicted working tree live in the same worktree: the --continue/ --abort lookup, mergestate's own staleness check, HasConflicts, RebaseContinue/RebaseAbort's strategy dispatch, preflight's in-progress detection (which would otherwise treat finish's own rebase as a foreign blocker), and --abort's return-to-topic checkout. That is exactly the shape of problem CLAUDE.md's anti-over-engineering clause exists to head off — especially for a combination that never worked before #175 either: the checkout would have failed the same way, just with an undocumented git error instead of a clear one. executeFinish now refuses rebase-strategy finish up front whenever the topic branch has its own separate worktree, alongside the existing --ff-only+squash mutual-exclusion check (same "usage error before any mutation" shape). --ff-only is exempt, since the rebase call is always skipped under it. handleMergeStep's rebase case reverts to a plain checkout, since the guard makes the separate-worktree case unreachable there. topicWorktreeIfSeparate loses the *git.Repo it used to return for the rebase call site to use — nothing needs it anymore — and is renamed topicHasSeparateWorktree to match its now-boolean-only contract; its two remaining callers (the same --abort and --ff-only-recovery checkouts that already needed it) are unaffected in behavior, since both only ever used the boolean. Real rebase-in-worktree support is left to a follow-up issue. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The pre-check that confirms a branch can actually be deleted before its worktree is freed compared it against HEAD only, mirroring 'git branch -d's no-upstream rule. That is only half of git's actual rule: when the branch has a configured upstream, 'git branch -d' checks mergedness against that instead. A branch already merged into its parent but not into a divergent configured upstream would pass this pre-check, have its worktree freed, and then have the real 'git branch -d' call refuse anyway — worktree already gone. Tries GetTrackingBranch first now, falling back to the current branch only when the lookup reports no upstream configured — the same fallback 'git branch -d' itself takes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The EXIT STATUS section stopped at code 5, never documenting the validation refusals (dirty worktree, operation in progress) added by #175, even though the finish manpage already covers its own code 6. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Worktree redirection currently breaks pre-finish hook semantics, the rebase/fast-forward combination, and failure-safe cleanup ordering.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
cmd/finish.go:280
- The
--ff-onlyexemption does not actually avoid the linked-worktree failure. The rebase arm still callsrepo.Checkout(state.FullBranchName)before it conditionally skips onlyRebaseWithOptions; after redirecting, Git rejects that checkout because the topic remains checked out in its linked worktree. Remove this exemption (or skip the entire topic checkout/rebase path under--ff-only) and add coverage for--rebase --ff-onlywith a linked topic worktree.
if resolvedOptions.MergeStrategy == strategyRebase && !resolvedOptions.RequireFastForward {
- Files reviewed: 14/14 changed files
- Comments generated: 3
- Review effort level: Balanced
A fourth review round found two more places where the #175 redirect mechanism reordered something it shouldn't have. The pre-finish hook ran on the wrong worktree. executeFinish redirected away from the topic's own worktree before calling finishBranch, which runs RunPreHook — but hooks run with their working directory set to the repo handle's worktree, and a version-bump hook (the documented, tested use case) is supposed to commit on the topic branch itself. Redirected too early, that commit landed on the parent's worktree (or main) instead, silently going nowhere near the eventual merge. finishBranch now takes the unredirected repo, runs the pre-hook first, and only then redirects before saving merge state and entering the state machine — the hook sees exactly what it saw before #175 existed. The topic's worktree was freed before remote branch deletion could fail. deleteBranchesIfNeeded deletes the remote branch first and returns immediately if that's rejected, leaving the local branch alone — an existing, pinned behavior (TestFinishClearsMergeStateWhenBranch DeletionFails). But the worktree-free step ran ahead of that call, so a rejected remote deletion left the local branch correctly intact with its worktree already gone anyway. Split into deleteRemoteBranchIfNeeded and deleteLocalBranchIfNeeded and reordered handleDeleteBranchStep around the worktree-free step: clear merge state (moved to the very top, so it still clears unconditionally, preserving the pinned test), delete remote, free worktree, delete local. Both regressions were confirmed by temporarily reintroducing the old ordering/timing and watching the new tests fail before restoring the fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The Worktree Cleanup section still read as if finish handled every combination automatically, and exit code 6 didn't mention the one it doesn't: rebase strategy against a topic branch with its own separate worktree, refused since round 3. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Detached rebase/bisect worktrees bypass cleanup guards, and linked-worktree ff-only rebases still attempt an invalid checkout.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 4
- Review effort level: Balanced
A fifth review round found three more places the redirect mechanism needed to account for, one of them a real oversight in an earlier fix rather than anything new. Redirecting into a busy destination could clobber it. Merge state is per-worktree, so two topic branches sharing a parent that has its own worktree both redirect to the same destination — nothing checked whether it already held a different operation's recovery state before overwriting it. Confirmed this isn't just "fails differently": without the guard, the second finish corrupts the first one's state outright (its own --continue afterward reports no merge in progress). finishBranch now checks IsMergeInProgress on the redirected repo before saving state, and reports the actual owner it found there. The --ff-only exemption from round 3's rebase-worktree refusal was incomplete. It skipped the rebase call under --ff-only, correctly, but missed that the checkout right before it isn't gated the same way — so the original "already used by worktree" failure still fired for --rebase --ff-only against a topic with its own separate worktree, the one combination the exemption was supposed to let through. The worktree preflight didn't re-run after the pre-finish hook. Round 4 moved the hook to run on the topic's own worktree, correctly, but the hook itself can dirty that worktree or start an operation there, and the only preflight check ran before it. Confirmed directly: without a second check after the hook, the merge actually completes and only the raw 'git worktree remove' call fails afterward — the exact "refusal can never follow a completed merge" violation the whole preflight exists to prevent. Re-running the same check right after the hook closes it. A fourth finding (detached HEAD hiding a hand-made worktree's own unrelated rebase/bisect from WorktreeForBranch) is the same root cause already tracked in #246 — folded in there rather than patched here, since a real fix needs the same new primitive #246 already scopes (resolving a detached worktree's original branch via git's rebase/ bisect metadata). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
I love it! |
A from-scratch audit of the redirect mechanism, run after five review rounds kept finding issues in it, found one more root cause with three consequences — all in the auto-update-children path, which no round or test had ever combined with worktrees before. --continue/--abort's state lookup recomputed where the initial run's redirect would have landed (the parent's own worktree, or main) instead of looking for the state directly. That works while the merge step itself is conflicted, since the parent is still checked out at the redirect target — but once handleUpdateChildrenStep has moved on and checked a child base branch out at that same location instead, WorktreeForBranch(parent) finds nothing there anymore and the recompute misses the state entirely. Confirmed directly: without the fix, --abort silently no-ops, leaving the conflict untouched while reporting success. findFinishStateAcrossWorktrees replaces the recompute with a direct search across every worktree for the matching state. Nothing refused a child base branch due for auto-update that has its own separate worktree. handleUpdateChildrenStep's checkout of it on the redirected repo would fail outright — but only after the merge (and any tag) had already completed, confirmed directly (exit 128, "already used by worktree", well after a completed merge). refuseIfChildWorktreeConflicts now checks every auto-update child before the merge starts, with a new ChildBranchWorktreeError naming the branch and its worktree. handleDeleteBranchStep's worktree-free step re-derived its CD-file destination via a fresh WorktreeForBranch(parent) lookup instead of trusting repo's own current worktree — the same staleness as the first finding, since a child checked out there makes that lookup return nothing. Confirmed directly: the CD file named the main worktree instead of wherever finish actually landed. Now derived from repo's own worktree instead. Also removes a verbatim-duplicated comment block left over from an earlier round's edit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
--keep-worktreeand--force-worktree/-Wtofinishanddelete, which free a topic branch's worktree as part of finishing or deleting the branch instead of leaving a stale checkout behind or failing outright because Git refuses to delete a branch that is still checked out somewhere.What "freeing" a worktree means depends on its provenance, read from the marker recorded at creation time (#172), never inferred from the worktree's path: a worktree git-flow created is removed and its marker cleared; one created by hand (
git worktree add) is kept, with its HEAD detached from the branch so the directory and every file in it, including uncommitted work, survive untouched.--keep-worktreeroutes even a git-flow-created worktree through the detach path;--force-worktree/-Wallows removing a git-flow-created worktree that has uncommitted or untracked changes. Both are CLI-only, with no git config equivalent, matchingcheckout's existing--worktree/--force. Both commands pre-flight the worktree before any destructive step (a worktree with a merge, rebase, bisect, cherry-pick, or revert in progress refuses either path; a dirty one refuses removal without--force-worktree), including a repeated check at the top of a resumedfinish --continue, so a refused cleanup can never follow a completed merge. If you're standing inside the worktree being removed, its replacement — wherever the operation actually landed, ordinarily the parent branch's own worktree if it has one, else the main worktree for finish, always the main worktree for delete — is written toGIT_FLOW_CD_FILE. Changes touchcmd/worktree_cleanup.go(new, shared by both commands),cmd/finish.go,cmd/delete.go,cmd/integrate.go,cmd/topicbranch.go,cmd/shorthand.go,internal/git/worktree.go(newWorktreeOperationInProgress),internal/errors/errors.go, andinternal/mergestate/mergestate.go.Closes #175
Closes #230
Running
finish/deletefrom inside the branch's own worktree needs a redirect step so the operation's own checkouts don't repurpose or fail against that worktree before the free step sees it (redirectPreferringParentWorktreeincmd/worktree_cleanup.go, preferring the parent's own worktree over main when it has one). That mechanism is also what #230 was asking for as its preferred fix (option 1 there: "perform the merge in the worktree that already has the base branch checked out") — it applies tofinishinvoked from inside a linked worktree generally, not only the branch-going-away case #175 describes, so this closes both.This PR supersedes #188 by @AlextheYounga. Their PR predated the worktree infrastructure that has since landed (#172–#176) and, per the design discussion on #188 and #175, settled on a different approach — freeing only the worktrees git-flow itself created and detaching (rather than removing) any others, versus #188's opt-in blanket removal — so none of the original code carries over unchanged. The commits implementing this feature carry their
Co-authored-bycredit for the original report, the working implementation that surfaced the gap, and the design conversation that shaped where this landed.Remarks
checkout's existing worktree flags (see the scoping decision recorded incmd/worktree_cleanup.go's doc comments).--force-worktree's contract is "discard uncommitted or untracked changes," not "abandon whatever operation is running."delete's post-delete hook could run against an already-removed worktree;delete's mergedness check could run against the wrong branch after redirecting to main;finish --continue --force-worktreesilently discarded the flag instead of overriding the earlier refusal; and--continueitself didn't redirect, so a conflict resolved from the original (pre-redirect) worktree could misbehave. Each has a dedicated regression test. Full detail in thefix:commit's body.--abort, and the--ff-onlyrecovery path) tried to re-checkout the topic branch on the redirected repo and failed outright;--continue/--abortcould report "no merge in progress" entirely when run from the original worktree after a redirect, since merge state is deliberately per-worktree;delete's worktree preflight ran after mutating steps, not before; anddeletecould free a worktree for a branch whose deletion then correctly failed as unmerged. All four fixed and regression-tested; full detail in the secondfix:commit's body.--continue/--abortunable to find or resolve it. Consulted a second model, which reproduced the failure directly (built and ran it) before recommending reverting the mechanism rather than patching its consequences — the coherent fix needs six coordinated state-machine changes for one narrow combination.finish --rebasenow refuses cleanly when the topic has its own separate worktree instead; real support is filed as finish --rebase: support a topic branch checked out in its own worktree #246. The same round also founddelete's mergedness pre-check ignored a configured upstream (fixed) and a doc gap for the new exit code (fixed).finish's worktree-free step ran before remote branch deletion could fail, the same class of bug already fixed ondelete's side. Both fixed and regression-tested (verified against the old behavior before being accepted, same as every round before this one).--ff-onlyexemption from the rebase-worktree refusal was incomplete (my own oversight — it skipped the rebase call but not the checkout before it); and the worktree preflight needed to re-run after the pre-finish hook, since round 4's fix let the hook run somewhere it could dirty. A fourth finding (detached HEAD hiding a hand-made worktree's own unrelated rebase/bisect) is the same root cause as finish --rebase: support a topic branch checked out in its own worktree #246 and was folded into it rather than patched separately.--continue/--abort's recompute-based state lookup missed a conflict during the child-update step, since by then the redirect target is checked out on the child, not the parent, andWorktreeForBranch(parent)finds nothing there anymore (fixed by searching every worktree directly instead of recomputing); a child base branch due for auto-update that has its own separate worktree was only caught when its checkout failed outright, after the merge and any tag had already completed (now refused up front, before the merge starts); and the worktree-free step'sGIT_FLOW_CD_FILEdestination had the same staleness as the first finding, sending a user to the main worktree instead of wherever finish actually landed. All three regression-tested the same way as every round before this one.updatecommand (internal/update.UpdateBranchFromParentWithMessage, shared with finish's own child-update step) has no worktree awareness at all outside of finish's redirect — naming a branch checked out in a different worktree fails with a raw git error rather than a clean refusal. Filed as update: fails with a raw git error when the branch lives in a different worktree #247 rather than folded in here, since it is a gap inupdateon its own, not something this PR's redirect mechanism introduced or is scoped to fix.Review focus:
cmd/worktree_cleanup.go— the shared pre-flight/free/redirect logiccmd/delete.go— the redirect now runs beforehooks.WithHooks, and the mergedness-check fixcmd/finish.go— the--continueflag override and redirectinternal/git/worktree.go—WorktreeOperationInProgress