feat: Add opt-in linked worktree cleanup - #188
AlextheYounga wants to merge 1 commit into
Conversation
Allow finish and delete to remove a linked worktree before deleting its branch when explicitly enabled by flag or branch-type config. Preserve existing behavior by default. Refuse to remove dirty worktrees unless force removal is explicitly requested. Relates gittower#175
There was a problem hiding this comment.
Pull request overview
This PR adds opt-in linked worktree cleanup to finish and delete, allowing a topic branch to be deleted even when it’s checked out in a linked worktree (by removing that worktree first), while keeping the current default behavior unchanged unless explicitly enabled via flags or gitflow.<type>.* config.
Changes:
- Add Git-layer support to locate a linked worktree for a branch, detect dirty state, and remove the worktree.
- Wire
--remove-worktree/--force-remove-worktree(and corresponding config keys) intofinishanddelete, including a finish preflight to avoid merging when cleanup would later refuse. - Add integration tests for
finish/deletebehavior and update manpages +docs/gitflow-config.5.mdfor the new options.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/internal/git/worktree_test.go | Adds Git-layer tests for worktree lookup, dirty detection, and removal. |
| test/cmd/finish_worktree_test.go | Adds end-to-end tests covering finish behavior with linked worktrees (opt-in, override, dirty/force). |
| test/cmd/delete_worktree_test.go | Adds end-to-end tests covering delete behavior with linked worktrees (opt-in, dirty protection). |
| internal/git/repo.go | Introduces WorktreeForBranch, RemoveWorktree, and WorktreeHasChanges Git wrapper methods. |
| internal/config/resolver.go | Extends finish option resolution to include remove/force-remove worktree booleans. |
| cmd/worktree_cleanup.go | Adds shared helper logic for locating/removing linked worktrees and producing actionable dirty-worktree errors. |
| cmd/topicbranch.go | Plumbs new flags through branch-type commands and updates retention option wiring. |
| cmd/shorthand.go | Plumbs new flags through shorthand finish/delete commands. |
| cmd/finish.go | Adds preflight for dirty worktrees and integrates worktree cleanup into the branch deletion step. |
| cmd/delete.go | Adds optional worktree cleanup before local branch deletion. |
| docs/gitflow-config.5.md | Documents new gitflow.<type>.{finish,delete}.{remove-worktree,force-remove-worktree} keys. |
| docs/git-flow-finish.1.md | Documents new finish flags and adds an example for finishing a branch held by a worktree. |
| docs/git-flow-delete.1.md | Documents new delete flags and adds a configuration example section. |
| docs/git-flow-config.1.md | Adds the new keys to the config command’s documented key list. |
| **gitflow.*type*.finish.remove-worktree** | ||
| : Remove a linked worktree holding the branch before the branch is deleted on finish. Git refuses to delete a branch checked out in a linked worktree; enabling this removes that worktree first so the finish can complete. Corresponds to `--remove-worktree`/`--no-remove-worktree`. | ||
| : *Type*: boolean | ||
| : *Default*: false | ||
|
|
||
| **gitflow.*type*.finish.force-remove-worktree** | ||
| : Force-remove a linked worktree that has uncommitted or untracked changes (those changes are lost) instead of failing. Only meaningful when `gitflow.*type*.finish.remove-worktree` is enabled. Corresponds to `--force-remove-worktree`/`--no-force-remove-worktree`. | ||
| : *Type*: boolean | ||
| : *Default*: false |
| output, err := r.gitCmd(args...).CombinedOutput() | ||
| if err != nil { | ||
| return fmt.Errorf("failed to remove worktree at '%s': %s", path, strings.TrimSpace(string(output))) | ||
| } | ||
| return nil | ||
| } |
|
Thanks for this — the opt-in instinct is right, and the concern behind it is the same one raised in #175, so I've replied there with where I'd like the design to land: git-flow tracks which worktrees it created and only removes those, while worktrees you made yourself are detached rather than removed. That avoids both a destructive default and a Let's keep the design discussion in #175 so it stays in one place, and use this PR for the code once the default is settled. Holding off on a full review until then, since the outcome changes the shape of the flag and config surface. |
alexrinass
left a comment
alexrinass
left a comment
There was a problem hiding this comment.
The design discussion in #175 has settled and the worktree specs are updated, so the requirements this PR implements have changed. Requesting changes on that basis rather than on the code — the part that moved is the surface this PR exposes, so a detailed code review would be reviewing something we already know has to change.
What changed
git-flow removes only the worktrees it created itself, recorded by a marker written at creation time and never inferred from the path. A worktree you created yourself is never removed: finish and delete detach its HEAD off the branch, delete the branch, and leave the directory and everything in it exactly as it was. That replaces the opt-in flags and the four gitflow.<type>.<command>.* keys with --keep-worktree and --force-worktree, and it removes the reason to keep cleanup off by default — the destructive case is gone, while someone using a worktree for a single feature still gets a finish that completes instead of failing after the merge. Full text in #175, with the shared model in #171 and #172.
What carries over from your branch: the dirty-worktree preflight, the rule that a dirty worktree is never removed without an explicit force, and the git-layer helpers for locating a branch's worktree and detecting changes.
Where that leaves this PR
The marker has to be written where the worktree is created, so cleanup now sits on top of #172 (foundation, git layer, provenance) and #173 (start), and neither is merged. I'll be working on those next. Rather than reshaping this branch blindly against specs whose foundation isn't in place yet, I'd wait for them to land and then re-express the cleanup on top — less wasted effort, and the flag and config surface will be settled by then. Glad to have you pick it up again at that point if you want it.
|
Thanks for this, and apologies it sat for a while — worktree support turned into a larger piece of work and this PR ended up overlapping the middle of it. Since this was opened, epic #171 has landed four specs: #172 added the worktree foundation (path templating, a provenance marker, a Git worktree operations layer, and a The first is mechanical. This PR adds The second is a design difference. The epic settled on git-flow removing only the worktrees it created itself, recorded through a The cleanup work itself is specified in #175, which is not currently being worked on, so this area is unowned rather than taken. If you would like to carry it forward, the shape that would fit is to build on the existing helpers in |
|
Closing this per the review response window in CONTRIBUTING.md — it's been over a week since my last comment with no response, and in the meantime the worktree epic (#172–#176) landed and made this branch's code no longer apply cleanly. I've opened #245 as a successor, built on the design settled across this thread and #175 (free only the worktrees git-flow itself created, detach the rest), and credited you via |
|
Hey @alexrinass, terribly sorry for late response here. It's been a busy week. I very much appreciate the co-author commits, you are a man of high integrity, and I am glad to have my name on this great project! |
feat: Add opt-in linked worktree cleanup
Lets you finish or delete a branch that's checked out in a linked worktree, without making workspace deletion the default.
Git refuses to delete a branch that's checked out in a worktree, so
finishanddeletefail with a "used by worktree" error whenever a branch lives in a second worktree. This adds opt-in cleanup: pass--remove-worktree(or setgitflow.<type>.finish.remove-worktree/gitflow.<type>.delete.remove-worktree) and git-flow removes that worktree right before deleting the branch. A dirty worktree is left alone unless you also pass--force-remove-worktree.Relates #175
I actually didn't know of this particular Issue #175 until I had already completed the branch and tested, then I saw the conversation. Sorry about that. But on looking into the proposed solution, I took a slightly different, inverse route. I think an opt-in approach might be more prudent to deleting worktrees by default.
I am currently running this change on my machine and it's working fine.
Why opt-in
#175 proposes automatic removal by default. I went the other way on purpose — a worktree is a workspace, and it often holds local state you still want (editor buffers, build artifacts, notes, scratch files). Removing it silently as part of a routine finish feels too destructive. So this keeps today's behavior as the default and makes cleanup an explicit choice, either per command or per branch type via config.
A few intentional differences from #175:
--keep-worktree/ detached-HEAD behavior. If you want the worktree gone, it's gone; if you don't, don't enable it.cd:navigation when you're sitting inside the worktree being removed. I haven't wired in the shell-integration protocol from Worktree navigation: checkout and shell-init #174.finishchecks an enabled, non-forced worktree removal before its merge flow begins, so a dirty worktree leaves the branch, remote branch, and target branch unchanged.Testing
--force-remove-worktree.finishtests: opt-in cleanup, default unchanged,--no-remove-worktreeoverriding config,--remove-worktreeenabling it, dirty worktree erroring with actionable guidance, and force removal completing.deletetests: opt-in cleanup, default unchanged, and dirty-worktree protection.go build ./..., andgo vet ./...passed; full validation of this branch is pending the isolated Git test-environment work.--remove-worktree— the worktree and branch were removed cleanly after the merge.Review focus
cmd/worktree_cleanup.go— shared helper: finds the worktree, removes it, prints progress, and turns the dirty-worktree case into an actionable error.internal/git/repo.go—WorktreeForBranch(porcelain parsing) andRemoveWorktree.internal/config/resolver.go,cmd/topicbranch.go,cmd/shorthand.go— flag and config precedence.cmd/finish.goandcmd/delete.go— where removal sits in the flow, right before local branch deletion.