From 59bfcf3e569539d7648b267f3310a93ea37dda7e Mon Sep 17 00:00:00 2001 From: John Bolliger Date: Thu, 8 Oct 2026 12:00:28 -0700 Subject: [PATCH 1/5] Remove a branch's worktree before deleting the branch git refuses to delete a branch that any worktree has checked out, under -d and -D alike, so pruning such a branch always failed. The delete now runs `git worktree remove` first, with --force in force mode. A branch that -d will refuse as unmerged keeps its worktree until the user accepts the force retry. A branch checked out in the main worktree cannot be marked, since git cannot remove the main worktree. Co-Authored-By: Claude Opus 5.5 (1M context) --- main.go | 138 ++++++++++++++++++++++++++++++++++++++------------- main_test.go | 34 ++++++------- 2 files changed, 119 insertions(+), 53 deletions(-) diff --git a/main.go b/main.go index 68f8346..1136ad3 100644 --- a/main.go +++ b/main.go @@ -34,6 +34,8 @@ type branch struct { riskCommits int // commits whose patch is not in the base branch; -D discards them riskMeasured bool // riskCommits has been computed (0 is a meaningful value) isCurrent bool + worktree string // path of the worktree that has the branch checked out; "" when none does + mainWorktree bool // that worktree is the main one, which git cannot remove selected bool deleteRemote bool } @@ -66,6 +68,11 @@ func (b branch) safeDeletable() bool { return b.headMerged } +// deletable reports whether the list lets b be marked for deletion. git +// refuses to delete the branch checked out here, and the main worktree, unlike +// a linked one, cannot be removed to free the branch it holds. +func (b branch) deletable() bool { return !b.isCurrent && !b.mainWorktree } + // forcedDelete reports whether b will be deleted with -D rather than -d. Gone // branches always are: git reports no track info for them, so a safe delete // would turn on HEAD alone and refuse branches whose work is in the remote @@ -107,14 +114,17 @@ const ( ) type deleteResult struct { - br branch // the branch this deletion was run for - done bool // the async deletion for this branch has completed - localOK bool - localErr string - forceable bool // a safe (-d) delete failed and could be retried with -D - remoteTried bool - remoteOK bool - remoteErr string + br branch // the branch this deletion was run for + done bool // the async deletion for this branch has completed + // worktreeRemoved records that the linked worktree holding br was removed + // to free the branch for deletion. + worktreeRemoved bool + localOK bool + localErr string + forceable bool // a safe (-d) delete failed and could be retried with -D + remoteTried bool + remoteOK bool + remoteErr string // remoteSkipped records that the armed push was deliberately deferred // because the local delete failed — the one piece of state not derivable // from br, since arming is the caller's decision. @@ -317,7 +327,7 @@ var trackRe = regexp.MustCompile(`ahead (\d+)|behind (\d+)`) func loadBranches() ([]branch, error) { const format = "%(refname)%00%(objectname:short)%00%(committerdate:iso8601-strict)%00" + - "%(committerdate:relative)%00%(upstream)%00%(upstream:track)%00%(HEAD)%00%(contents:subject)" + "%(committerdate:relative)%00%(upstream)%00%(upstream:track)%00%(HEAD)%00%(worktreepath)%00%(contents:subject)" out, err := runGit("for-each-ref", "--format="+format, "refs/heads") if err != nil { return nil, err @@ -328,7 +338,7 @@ func loadBranches() ([]branch, error) { continue } f := strings.Split(line, "\x00") - if len(f) < 8 { + if len(f) < 9 { continue } b := branch{ @@ -337,7 +347,8 @@ func loadBranches() ([]branch, error) { committedRel: f[3], upstream: shortRef(f[4]), isCurrent: f[6] == "*", - subject: f[7], + worktree: f[7], + subject: f[8], } if t, terr := time.Parse(time.RFC3339, f[2]); terr == nil { b.committed = t @@ -528,8 +539,23 @@ type repoReads struct { remotes remoteRefs } +// mainWorktreePath returns the path of the repository's main worktree, which git +// always lists first. "" when the list cannot be read. +func mainWorktreePath() string { + out, err := runGit("worktree", "list", "--porcelain", "-z") + if err != nil { + return "" + } + first, _, _ := strings.Cut(out, "\x00") + path, ok := strings.CutPrefix(first, "worktree ") + if !ok { + return "" + } + return path +} + // loadRepo reads the branch list and everything independent of it in one round. -// Starting a git subprocess costs about 6ms, and none of these three waits on +// Starting a git subprocess costs about 6ms, and none of these four waits on // another, so the depth of the chain is what the user waits on — not the work // inside it. func loadRepo() ([]branch, repoReads, error) { @@ -537,13 +563,20 @@ func loadRepo() ([]branch, repoReads, error) { branches []branch err error reads repoReads + mainWT string ) var wg sync.WaitGroup - wg.Add(3) + wg.Add(4) go func() { defer wg.Done(); branches, err = loadBranches() }() go func() { defer wg.Done(); reads.headMerged = localMergedSet() }() go func() { defer wg.Done(); reads.remotes = loadRemoteRefs() }() + go func() { defer wg.Done(); mainWT = mainWorktreePath() }() wg.Wait() + // Marked here rather than in applyBranches so carryMarks, which runs + // first, already knows which branches cannot be marked. + for i := range branches { + branches[i].mainWorktree = mainWT != "" && branches[i].worktree == mainWT + } return branches, reads, err } @@ -600,9 +633,9 @@ var spinnerFrames = []string{"⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", " // deleteBranchCmd wraps the deleteBranch worker as a tea.Cmd so deletions run off // the update loop. It captures only a branch value (never the model), so each runs // independently and concurrently under tea.Batch. -func deleteBranchCmd(idx int, b branch, flag string, wantRemote bool) tea.Cmd { +func deleteBranchCmd(idx int, b branch, force, wantRemote bool) tea.Cmd { return func() tea.Msg { - return branchDeletedMsg{idx: idx, res: deleteBranch(b, flag, wantRemote)} + return branchDeletedMsg{idx: idx, res: deleteBranch(b, force, wantRemote)} } } @@ -926,11 +959,11 @@ func (m model) updateList(msg tea.KeyMsg) (tea.Model, tea.Cmd) { m.cursor = len(m.branches) // clampCursor lands it on the last visible row m.clampCursor() case " ": - if b := m.cur(); b != nil && !b.isCurrent { + if b := m.cur(); b != nil && b.deletable() { b.selected = !b.selected } case "r": - if b := m.cur(); b != nil && b.upstream != "" && !b.isCurrent { + if b := m.cur(); b != nil && b.upstream != "" && b.deletable() { b.deleteRemote = !b.deleteRemote } case "/": @@ -943,7 +976,7 @@ func (m model) updateList(msg tea.KeyMsg) (tea.Model, tea.Cmd) { // Select what the list shows: with a filter active, a marks only the // matching branches, which is what makes filter-then-select useful. for _, i := range m.viewIdx() { - if !m.branches[i].isCurrent { + if m.branches[i].deletable() { m.branches[i].selected = true } } @@ -1056,7 +1089,7 @@ func (m *model) startDeletions(includeRemote bool) tea.Cmd { for i, b := range sel { m.results[i] = deleteResult{br: b} wantRemote := includeRemote && b.deleteRemote && b.upstream != "" - cmds = append(cmds, deleteBranchCmd(i, b, b.deleteFlag(m.force), wantRemote)) + cmds = append(cmds, deleteBranchCmd(i, b, m.force, wantRemote)) } return tea.Batch(cmds...) } @@ -1140,18 +1173,29 @@ func (b branch) deleteFlag(force bool) string { // deleteBranch runs one branch's local delete and, when wantRemote is set, its // remote-branch push --delete. It is the worker deleteBranchCmd runs off the -// update loop, one cmd per branch. -func deleteBranch(b branch, flag string, wantRemote bool) deleteResult { +// update loop, one cmd per branch. force is the user's force mode: it picks the +// delete flag and lets a worktree with uncommitted changes be removed. +func deleteBranch(b branch, force, wantRemote bool) deleteResult { res := deleteResult{br: b, done: true} - if _, err := runGit("branch", flag, b.name); err != nil { - res.localErr = err.Error() - // Only an unmerged refusal is worth escalating to -D. Other failures — a - // branch held by another worktree, most commonly — fail identically under - // -D, so offering the retry would just mislabel them as lost commits. - res.forceable = flag == "-d" && strings.Contains(res.localErr, "not fully merged") - } else { - res.localOK = true + flag := b.deleteFlag(force) + switch { + case b.worktree != "" && flag == "-d" && !b.safeDeletable(): + // git checks the worktree before the merge, so -d would report the + // worktree, not the unmerged commits that the force retry can clear. + // The worktree stays until the user agrees to that retry. + res.localErr = fmt.Sprintf("the branch '%s' is not fully merged (worktree kept)", b.name) + case b.worktree != "" && !removeWorktree(&res, force): + default: + if _, err := runGit("branch", flag, b.name); err != nil { + res.localErr = err.Error() + } else { + res.localOK = true + } } + // Only an unmerged refusal is worth escalating to -D. Other failures — a + // worktree that will not be removed, most commonly — fail identically under + // -D, so offering the retry would just mislabel them as lost commits. + res.forceable = !res.localOK && flag == "-d" && strings.Contains(res.localErr, "not fully merged") // Never delete the remote copy while the local branch survives a refused // delete: that would strand its commits with nowhere else to exist. The push // is deferred until the force retry clears the local branch. @@ -1165,6 +1209,24 @@ func deleteBranch(b branch, flag string, wantRemote bool) deleteResult { return res } +// removeWorktree removes the linked worktree holding res's branch: git refuses to +// delete a branch any worktree has checked out, under -d and -D alike. Without +// force, git refuses a worktree with uncommitted changes or untracked files; +// with it, they are discarded. A worktree whose directory is already gone is +// removed either way. It reports whether the branch is now free to delete. +func removeWorktree(res *deleteResult, force bool) bool { + args := []string{"worktree", "remove"} + if force { + args = append(args, "--force") + } + if _, err := runGit(append(args, res.br.worktree)...); err != nil { + res.localErr = "worktree " + res.br.worktree + ": " + err.Error() + return false + } + res.worktreeRemoved = true + return true +} + // pushRemoteDelete deletes res's remote branch, clearing any deferral: the // results screen tests remoteSkipped first, so a stale flag would report the // remote as kept right after a successful push. @@ -1192,11 +1254,12 @@ func carryMarks(old, fresh []branch) []branch { } for i := range fresh { if p, ok := prev[fresh[i].name]; ok { - // Marks never land on the current branch: it cannot be deleted, so - // a carried mark would arm an operation the list refuses to offer - // (relevant after a switch, or when HEAD moved outside the TUI). - fresh[i].selected = p.selected && !fresh[i].isCurrent - fresh[i].deleteRemote = p.deleteRemote && !fresh[i].gone && !fresh[i].isCurrent + // Marks never land on a branch that cannot be deleted, such as the + // current one: a carried mark would arm an operation the list + // refuses to offer (relevant after a switch, or when HEAD moved + // outside the TUI). + fresh[i].selected = p.selected && fresh[i].deletable() + fresh[i].deleteRemote = p.deleteRemote && !fresh[i].gone && fresh[i].deletable() } } return fresh @@ -1326,6 +1389,11 @@ func (m *model) forceDeleteUnmerged() { if r.localOK || !r.forceable { continue } + // deleteBranch kept the worktree while -d was going to refuse the + // branch; the user has now agreed to the delete, so free it. + if r.br.worktree != "" && !r.worktreeRemoved && !removeWorktree(r, m.force) { + continue + } if _, err := runGit("branch", "-D", r.br.name); err != nil { r.localErr = err.Error() continue @@ -1590,7 +1658,7 @@ func (m *model) selectGone() string { gone, risky := 0, 0 for i := range m.branches { br := &m.branches[i] - if !br.gone || br.isCurrent { + if !br.gone || !br.deletable() { continue } gone++ diff --git a/main_test.go b/main_test.go index d50e9fd..71161d4 100644 --- a/main_test.go +++ b/main_test.go @@ -509,7 +509,7 @@ func TestDeleteBranchCmd(t *testing.T) { chdir(t, repo) // Local-only safe delete of the merged branch. - msg := deleteBranchCmd(2, branch{name: "feature/merged"}, "-d", false)() + msg := deleteBranchCmd(2, branch{name: "feature/merged"}, false, false)() dm, ok := msg.(branchDeletedMsg) if !ok { t.Fatalf("want branchDeletedMsg, got %T", msg) @@ -523,7 +523,7 @@ func TestDeleteBranchCmd(t *testing.T) { // Force delete + remote push of the tracked branch. tracked := branch{name: "feature/tracked", upstream: "origin/feature/tracked", ahead: 1} - dm2 := deleteBranchCmd(0, tracked, "-D", true)().(branchDeletedMsg) + dm2 := deleteBranchCmd(0, tracked, true, true)().(branchDeletedMsg) if !dm2.res.localOK { t.Fatalf("force local delete failed: %s", dm2.res.localErr) } @@ -1377,33 +1377,31 @@ func TestNonUnmergedFailureIsNotForceable(t *testing.T) { repo := setupRepo(t) chdir(t, repo) - // A second worktree holds feature/unmerged; git refuses to delete it under - // -d and -D alike. + // A second worktree holds the merged branch and an untracked file, so a + // safe delete must refuse to remove the worktree, and with it the branch. wt := t.TempDir() + "/wt" - git(t, repo, "worktree", "add", "-q", wt, "feature/unmerged") - - if _, err := runGit("branch", "-D", "feature/unmerged"); err == nil { - t.Fatal("precondition: -D should also fail for a branch held by a worktree") + git(t, repo, "worktree", "add", "-q", wt, "feature/merged") + if err := os.WriteFile(wt+"/scratch", []byte("unsaved work"), 0o644); err != nil { + t.Fatal(err) } - b := branch{name: "feature/unmerged"} - res := deleteBranch(b, "-d", false) + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + b := *find(m.branches, "feature/merged") + res := deleteBranch(b, false, false) if res.localOK { t.Fatalf("delete should have failed: %+v", res) } if res.forceable { - t.Fatalf("a worktree conflict must not be offered as force-retryable: %q", res.localErr) + t.Fatalf("a dirty worktree must not be offered as force-retryable: %q", res.localErr) } // End to end: the async path lands on the results screen, not the prompt. - m, err := initialModel() - if err != nil { - t.Fatal(err) - } - m.results = []deleteResult{{br: branch{name: "feature/unmerged"}}} + m.results = []deleteResult{{br: b}} m.state = stateDeleting - msg := deleteBranchCmd(0, *find(m.branches, "feature/unmerged"), "-d", false)() - nm, _ := m.Update(msg) + nm, _ := m.Update(deleteBranchCmd(0, b, false, false)()) if got := nm.(model).state; got != stateResult { t.Fatalf("state should be stateResult, got %v", got) } From 1e6e7c955aa46af2ec57cf320d1a135c4342f80f Mon Sep 17 00:00:00 2001 From: John Bolliger Date: Thu, 8 Oct 2026 12:01:20 -0700 Subject: [PATCH 2/5] Show worktree branches and the worktrees a delete removes Rows mark a branch checked out in another worktree with +, as `git branch` does. The confirm screen and the force prompt name each worktree the delete removes, and say whether uncommitted changes are refused or discarded. The results screen reports each removed worktree. The help screen and README describe the behavior. Co-Authored-By: Claude Opus 5.5 (1M context) --- README.md | 19 ++++++++++++++++-- main.go | 58 ++++++++++++++++++++++++++++++++++++++++++++----------- 2 files changed, 64 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 3739736..de62cdc 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,7 @@ git_pruner version # also --version, -v | -------------- | ------------------------------------------------------------- | | `↑`/`k`, `↓`/`j` | Move cursor | | `g` / `G` | Jump to top / bottom | -| `space` | Toggle selection (the current branch cannot be selected) | +| `space` | Toggle selection (the current branch, and a branch checked out in the main worktree, cannot be selected) | | `a` / `n` | Select all listed branches / clear selection | | `c` | Checkout the branch under the cursor (`git switch`) | | `/` | Filter branches by name; `enter` keeps it, `esc` clears it | @@ -85,7 +85,8 @@ precedence, so `y`, `R` and `n` still work while a list is scrolled. > [x] R * feature/foo ↑2↓1 ✓ 3 days ago a1b2c3d Fix the thing ``` -- `>` cursor, `[x]` selected, `R` remote deletion armed, `*` current branch +- `>` cursor, `[x]` selected, `R` remote deletion armed, `*` current branch, `+` checked out in + another worktree - ahead/behind shown as `↑N↓M` (`=` when in sync, `gone` in red when the upstream was deleted) - a green `✓` after the track column means the upstream is merged into the remote default branch — i.e. the remote is safe to delete @@ -157,6 +158,20 @@ claiming the work is unrecoverable. Deletions then run concurrently in the background on a live progress screen, and a results screen reports per-branch success or failure. +## Branches checked out in a worktree + +git refuses to delete a branch that any worktree has checked out, even with `-D`. git_pruner +marks such a branch with `+` and runs `git worktree remove` before it deletes the branch. The +confirmation screen names each worktree it will remove. + +- In safe mode, git refuses to remove a worktree with uncommitted changes or untracked files, + and the branch stays. Force mode (`f`) removes the worktree with `--force` and discards them. +- A worktree whose directory is already gone is removed in either mode. +- If the safe delete (`-d`) would refuse the branch as not fully merged, the worktree stays + until you accept the force delete prompt. +- A branch checked out in the main worktree cannot be selected: git cannot remove the main + worktree. This happens when you run git_pruner from a linked worktree. + ## Development ```sh diff --git a/main.go b/main.go index 1136ad3..8c4d49c 100644 --- a/main.go +++ b/main.go @@ -184,15 +184,16 @@ var ( // ---- styles ---- var ( - currentStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("10")) - cursorStyle = lipgloss.NewStyle().Bold(true).Foreground(lipgloss.Color("12")) - rowBgStyle = lipgloss.NewStyle().Background(lipgloss.Color("236")) // cursor row band - selStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("11")) - goneStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("9")) - dimStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("8")) - headerStyle = lipgloss.NewStyle().Bold(true) - okStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("10")) - errStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("9")) + currentStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("10")) + cursorStyle = lipgloss.NewStyle().Bold(true).Foreground(lipgloss.Color("12")) + rowBgStyle = lipgloss.NewStyle().Background(lipgloss.Color("236")) // cursor row band + selStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("11")) + worktreeStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("13")) + goneStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("9")) + dimStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("8")) + headerStyle = lipgloss.NewStyle().Bold(true) + okStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("10")) + errStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("9")) nameStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("14")) // cyan hashStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("3")) // yellow @@ -1692,9 +1693,14 @@ func (m model) renderRow(br branch, nameW int, isCursor bool) string { if br.deleteRemote { rem = errStyle.Render("R") } + // The checkout column follows `git branch`: * for the branch checked out + // here, + for one checked out in another worktree. cur := " " - if br.isCurrent { + switch { + case br.isCurrent: cur = currentStyle.Render("*") + case br.worktree != "": + cur = worktreeStyle.Render("+") } name := pad(truncate(br.name, nameW), nameW) @@ -1836,6 +1842,7 @@ func (m model) helpView() string { b.WriteString("\n") writeRows([][2]string{ {"*", "current branch (cannot be deleted)"}, + {"+", "checked out in another worktree (removed on delete)"}, {"[x]", "selected for deletion"}, {"R", "its remote branch will also be deleted"}, {"↑/↓", "commits ahead of / behind upstream"}, @@ -1845,7 +1852,10 @@ func (m model) helpView() string { b.WriteString("\n") b.WriteString(dimStyle.Render("Gone branches are deleted with -D. Any holding commits that are not in\n" + - "the default branch are left unselected by x/p and flagged on the confirm screen.")) + "the default branch are left unselected by x/p and flagged on the confirm screen.\n" + + "A branch checked out in a linked worktree is deleted after its worktree is removed;\n" + + "safe mode refuses a worktree with uncommitted changes, force mode discards them.\n" + + "A branch checked out in the main worktree cannot be deleted.")) b.WriteString("\n\n") commit, date := buildInfo() @@ -1901,6 +1911,9 @@ func (m model) confirmParts() (header, body, footer []string) { if br.deleteRemote && br.upstream != "" { body = append(body, " "+errStyle.Render(fmt.Sprintf("+ delete remote %s/%s", br.remoteName(), br.remoteBranch()))) } + if w := m.worktreeWarning(br); w != "" { + body = append(body, " "+errStyle.Render(w)) + } if w := m.riskWarning(br); w != "" { body = append(body, " "+errStyle.Render(w)) } @@ -1918,6 +1931,23 @@ func (m model) confirmParts() (header, body, footer []string) { func (m model) confirmView() string { return m.page(m.confirmParts()) } +// worktreeWarning states what deleting br does to the worktree that holds it, +// or "" when no worktree does. git refuses to delete a branch a worktree has +// checked out, so the worktree is removed first. +func (m model) worktreeWarning(br branch) string { + if br.worktree == "" { + return "" + } + if m.force { + return "+ remove worktree " + br.worktree + " (uncommitted changes will be lost)" + } + if br.forcedDelete(false) || br.safeDeletable() { + return "+ remove worktree " + br.worktree + " (refused if it has uncommitted changes)" + } + // -d will refuse the branch, so the worktree stays until the force retry. + return "+ remove worktree " + br.worktree + " if you then force delete" +} + // riskWarning states the cost of deleting br, or "" when the delete is clean. // It covers every branch git's safe delete would refuse plus gone branches, // which take the -D path regardless: under -D the unmerged commits are @@ -1969,6 +1999,9 @@ func (m model) forcePromptParts() (header, body, footer []string) { if r.remoteSkipped { body = append(body, " "+errStyle.Render(fmt.Sprintf("+ remote %s/%s will be deleted once the branch is gone", r.br.remoteName(), r.br.remoteBranch()))) } + if r.br.worktree != "" && !r.worktreeRemoved { + body = append(body, " "+errStyle.Render("+ worktree "+r.br.worktree+" will be removed first")) + } } footer = []string{ @@ -1983,6 +2016,9 @@ func (m model) forcePromptView() string { return m.page(m.forcePromptParts()) } // appendResultLines adds one completed deletion result (local, then remote if // tried) to dst. Shared by the results screen and the live deleting screen. func appendResultLines(dst []string, r deleteResult) []string { + if r.worktreeRemoved { + dst = append(dst, okStyle.Render(" ✓ ")+"removed worktree "+r.br.worktree) + } if r.localOK { dst = append(dst, okStyle.Render(" ✓ ")+"deleted local "+r.br.name) } else { From 9d644bacdc287aea3a0c0af780096487022a9348 Mon Sep 17 00:00:00 2001 From: John Bolliger Date: Thu, 8 Oct 2026 12:02:07 -0700 Subject: [PATCH 3/5] Test deleting branches held by worktrees Cover a clean worktree and one whose directory is gone, a dirty worktree in safe and force mode, an unmerged branch that keeps its worktree until the force retry, and a branch held by the main worktree. Co-Authored-By: Claude Opus 5.5 (1M context) --- main_test.go | 203 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 203 insertions(+) diff --git a/main_test.go b/main_test.go index 71161d4..9851359 100644 --- a/main_test.go +++ b/main_test.go @@ -2771,3 +2771,206 @@ func TestDiffRerendersOnResize(t *testing.T) { t.Fatalf("the view must show delta's lines and name it:\n%s", o) } } + +// result returns m's deletion result for the named branch, failing if none ran. +func result(t *testing.T, m model, name string) deleteResult { + t.Helper() + for _, r := range m.results { + if r.br.name == name { + return r + } + } + t.Fatalf("no deletion result for %s: %+v", name, m.results) + return deleteResult{} +} + +// confirmBody returns the body lines of m's confirmation screen. +func (m model) confirmBody() []string { + _, body, _ := m.confirmParts() + return body +} + +// realTempDir is t.TempDir with symlinks resolved, so its paths match the +// ones git reports (macOS puts temp dirs under /var, a link to /private/var). +func realTempDir(t *testing.T) string { + t.Helper() + dir, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + return dir +} + +// dirExists reports whether path is present on disk. +func dirExists(path string) bool { + _, err := os.Stat(path) + return err == nil +} + +// git refuses to delete a branch any worktree has checked out, under -d and -D +// alike, so the delete has to remove the worktree first. That holds for a +// worktree whose directory was deleted by hand too: git still records it until +// it is pruned. +func TestWorktreeBranchDeletes(t *testing.T) { + repo := setupLocalRepo(t) + chdir(t, repo) + git(t, repo, "branch", "feature/stale") + wt := filepath.Join(realTempDir(t), "wt") + stale := filepath.Join(realTempDir(t), "stale") + git(t, repo, "worktree", "add", "-q", wt, "feature/merged") + git(t, repo, "worktree", "add", "-q", stale, "feature/stale") + if err := os.RemoveAll(stale); err != nil { + t.Fatal(err) + } + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + for _, name := range []string{"feature/merged", "feature/stale"} { + b := find(m.branches, name) + if b.worktree == "" || b.mainWorktree { + t.Fatalf("precondition: %s should be held by a linked worktree: %+v", name, b) + } + b.selected = true + } + m.force = false + m = startAndDrain(t, m, false) + + wantState(t, m, stateResult, "both deletes should succeed") + for _, name := range []string{"feature/merged", "feature/stale"} { + if r := result(t, m, name); !r.worktreeRemoved || !r.localOK { + t.Fatalf("%s: want worktree removed and branch deleted: %+v", name, r) + } + if find(m.branches, name) != nil { + t.Fatalf("%s should be gone after the delete", name) + } + } + if dirExists(wt) { + t.Fatal("the worktree directory should be removed") + } + if out := git(t, repo, "worktree", "list", "--porcelain"); strings.Contains(out, "feature/") { + t.Fatalf("git should no longer record either worktree:\n%s", out) + } + if body := strings.Join(appendResultLines(nil, result(t, m, "feature/merged")), "\n"); !strings.Contains(body, "removed worktree "+wt) { + t.Fatalf("the results must name the removed worktree:\n%s", body) + } +} + +// A worktree with uncommitted work survives a safe delete, and the branch with +// it; force mode is the user's say-so to discard that work. +func TestDirtyWorktreeNeedsForce(t *testing.T) { + repo := setupLocalRepo(t) + chdir(t, repo) + wt := filepath.Join(realTempDir(t), "wt") + git(t, repo, "worktree", "add", "-q", wt, "feature/merged") + if err := os.WriteFile(filepath.Join(wt, "scratch"), []byte("unsaved work"), 0o644); err != nil { + t.Fatal(err) + } + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + find(m.branches, "feature/merged").selected = true + m.force = false + if body := strings.Join(m.confirmBody(), "\n"); !strings.Contains(body, "remove worktree "+wt+" (refused if it has uncommitted changes)") { + t.Fatalf("the safe confirm must say the worktree goes only if clean:\n%s", body) + } + m = startAndDrain(t, m, false) + + wantState(t, m, stateResult, "a dirty worktree is not a force-prompt case") + if r := result(t, m, "feature/merged"); r.localOK || r.worktreeRemoved || !strings.Contains(r.localErr, wt) { + t.Fatalf("safe mode must refuse the dirty worktree and name it: %+v", r) + } + if !dirExists(filepath.Join(wt, "scratch")) || find(m.branches, "feature/merged") == nil { + t.Fatal("the worktree, its files and the branch must all survive a refused delete") + } + + find(m.branches, "feature/merged").selected = true + m.force = true + if body := strings.Join(m.confirmBody(), "\n"); !strings.Contains(body, "uncommitted changes will be lost") { + t.Fatalf("the force confirm must warn that changes are discarded:\n%s", body) + } + m = startAndDrain(t, m, false) + if r := result(t, m, "feature/merged"); !r.localOK || !r.worktreeRemoved { + t.Fatalf("force mode should remove the dirty worktree and the branch: %+v", r) + } + if dirExists(wt) { + t.Fatal("the worktree directory should be removed under force") + } +} + +// When -d is going to refuse the branch as unmerged, removing its worktree +// first would leave the user with neither: the branch survives, the worktree +// does not. The worktree stays until the user accepts the force retry. +func TestUnmergedWorktreeKeptUntilForce(t *testing.T) { + repo := setupLocalRepo(t) + chdir(t, repo) + wt := filepath.Join(realTempDir(t), "wt") + git(t, repo, "worktree", "add", "-q", wt, "feature/unmerged") + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + find(m.branches, "feature/unmerged").selected = true + m.force = false + m = startAndDrain(t, m, false) + + wantState(t, m, stateForcePrompt, "an unmerged branch must still raise the force prompt") + if r := result(t, m, "feature/unmerged"); r.worktreeRemoved || !r.forceable { + t.Fatalf("the worktree must be kept and the delete forceable: %+v", r) + } + if !dirExists(wt) { + t.Fatal("the worktree must survive the refused -d") + } + if _, body, _ := m.forcePromptParts(); !strings.Contains(strings.Join(body, "\n"), "worktree "+wt+" will be removed first") { + t.Fatalf("the force prompt must name the worktree it removes:\n%s", strings.Join(body, "\n")) + } + + m = press(t, m, key("y")) + if r := result(t, m, "feature/unmerged"); !r.localOK || !r.worktreeRemoved { + t.Fatalf("the force retry should remove the worktree and the branch: %+v", r) + } + if dirExists(wt) || find(m.branches, "feature/unmerged") != nil { + t.Fatal("the worktree and the branch should both be gone after the force retry") + } +} + +// Run from a linked worktree, the branch checked out in the main worktree is as +// undeletable as the current one: git cannot remove the main worktree to free it. +func TestMainWorktreeBranchCannotBeMarked(t *testing.T) { + repo := setupLocalRepo(t) + addOrigin(t, repo, "main") + git(t, repo, "push", "-q", "-u", "origin", "feature/merged") + git(t, repo, "branch", "-q", "-u", "origin/feature/merged", "main") + git(t, repo, "push", "-q", "origin", "--delete", "feature/merged") + git(t, repo, "fetch", "-q", "--prune") + wt := filepath.Join(realTempDir(t), "wt") + git(t, repo, "worktree", "add", "-q", wt, "feature/unmerged") + chdir(t, wt) + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + b := find(m.branches, "main") + if b == nil || !b.mainWorktree || b.isCurrent { + t.Fatalf("precondition: main should be held by the main worktree: %+v", b) + } + for i := range m.branches { + if m.branches[i].name == "main" { + m.cursor = i + } + } + m = press(t, m, key(" "), key("a"), key("x")) + for _, b := range m.selectedBranches() { + if b.name == "main" || b.isCurrent { + t.Fatalf("%s must not be selectable from a linked worktree", b.name) + } + } + if row := m.renderRow(*find(m.branches, "main"), 12, false); !strings.Contains(row, worktreeStyle.Render("+")) { + t.Fatalf("a branch held by another worktree must show the + marker:\n%q", row) + } +} From cfa51e6805531612ea90401e8eeb632e1d1937ae Mon Sep 17 00:00:00 2001 From: John Bolliger Date: Thu, 8 Oct 2026 12:08:47 -0700 Subject: [PATCH 4/5] Simplify the worktree delete checks Add safeDeleteRefuses and worktreePending so deleteBranch, worktreeWarning, forceDeleteUnmerged and the force prompt share one test each instead of repeating the conditions. The kept-worktree case sets forceable directly rather than building an error string for the merge check to find. Co-Authored-By: Claude Opus 5.5 (1M context) --- main.go | 48 +++++++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/main.go b/main.go index 8c4d49c..4573f99 100644 --- a/main.go +++ b/main.go @@ -79,6 +79,12 @@ func (b branch) deletable() bool { return !b.isCurrent && !b.mainWorktree } // default but not in the local checkout — the headline prune case. func (b branch) forcedDelete(force bool) bool { return force || b.gone } +// safeDeleteRefuses reports whether the delete of b under the given force mode +// will run -d and be refused as not fully merged. +func (b branch) safeDeleteRefuses(force bool) bool { + return !b.forcedDelete(force) && !b.safeDeletable() +} + type sortField int const ( @@ -548,11 +554,10 @@ func mainWorktreePath() string { return "" } first, _, _ := strings.Cut(out, "\x00") - path, ok := strings.CutPrefix(first, "worktree ") - if !ok { - return "" + if path, ok := strings.CutPrefix(first, "worktree "); ok { + return path } - return path + return "" } // loadRepo reads the branch list and everything independent of it in one round. @@ -1180,23 +1185,24 @@ func deleteBranch(b branch, force, wantRemote bool) deleteResult { res := deleteResult{br: b, done: true} flag := b.deleteFlag(force) switch { - case b.worktree != "" && flag == "-d" && !b.safeDeletable(): + case b.worktree != "" && b.safeDeleteRefuses(force): // git checks the worktree before the merge, so -d would report the // worktree, not the unmerged commits that the force retry can clear. // The worktree stays until the user agrees to that retry. - res.localErr = fmt.Sprintf("the branch '%s' is not fully merged (worktree kept)", b.name) + res.localErr = "not fully merged (worktree kept)" + res.forceable = true case b.worktree != "" && !removeWorktree(&res, force): default: if _, err := runGit("branch", flag, b.name); err != nil { res.localErr = err.Error() + // Only an unmerged refusal is worth escalating to -D. Other + // failures fail identically under -D, so offering the retry would + // just mislabel them as lost commits. + res.forceable = flag == "-d" && strings.Contains(res.localErr, "not fully merged") } else { res.localOK = true } } - // Only an unmerged refusal is worth escalating to -D. Other failures — a - // worktree that will not be removed, most commonly — fail identically under - // -D, so offering the retry would just mislabel them as lost commits. - res.forceable = !res.localOK && flag == "-d" && strings.Contains(res.localErr, "not fully merged") // Never delete the remote copy while the local branch survives a refused // delete: that would strand its commits with nowhere else to exist. The push // is deferred until the force retry clears the local branch. @@ -1228,6 +1234,10 @@ func removeWorktree(res *deleteResult, force bool) bool { return true } +// worktreePending reports whether a linked worktree still holds r's branch, so +// a force retry has to remove it first. +func (r deleteResult) worktreePending() bool { return r.br.worktree != "" && !r.worktreeRemoved } + // pushRemoteDelete deletes res's remote branch, clearing any deferral: the // results screen tests remoteSkipped first, so a stale flag would report the // remote as kept right after a successful push. @@ -1392,7 +1402,7 @@ func (m *model) forceDeleteUnmerged() { } // deleteBranch kept the worktree while -d was going to refuse the // branch; the user has now agreed to the delete, so free it. - if r.br.worktree != "" && !r.worktreeRemoved && !removeWorktree(r, m.force) { + if r.worktreePending() && !removeWorktree(r, m.force) { continue } if _, err := runGit("branch", "-D", r.br.name); err != nil { @@ -1935,17 +1945,17 @@ func (m model) confirmView() string { return m.page(m.confirmParts()) } // or "" when no worktree does. git refuses to delete a branch a worktree has // checked out, so the worktree is removed first. func (m model) worktreeWarning(br branch) string { - if br.worktree == "" { + switch { + case br.worktree == "": return "" - } - if m.force { + case m.force: return "+ remove worktree " + br.worktree + " (uncommitted changes will be lost)" - } - if br.forcedDelete(false) || br.safeDeletable() { + case br.safeDeleteRefuses(false): + // The worktree stays until the force retry. + return "+ remove worktree " + br.worktree + " if you then force delete" + default: return "+ remove worktree " + br.worktree + " (refused if it has uncommitted changes)" } - // -d will refuse the branch, so the worktree stays until the force retry. - return "+ remove worktree " + br.worktree + " if you then force delete" } // riskWarning states the cost of deleting br, or "" when the delete is clean. @@ -1999,7 +2009,7 @@ func (m model) forcePromptParts() (header, body, footer []string) { if r.remoteSkipped { body = append(body, " "+errStyle.Render(fmt.Sprintf("+ remote %s/%s will be deleted once the branch is gone", r.br.remoteName(), r.br.remoteBranch()))) } - if r.br.worktree != "" && !r.worktreeRemoved { + if r.worktreePending() { body = append(body, " "+errStyle.Render("+ worktree "+r.br.worktree+" will be removed first")) } } From b75e8aff7f3eaca59555b6c76d4156aa81b775d6 Mon Sep 17 00:00:00 2001 From: John Bolliger Date: Thu, 8 Oct 2026 12:28:27 -0700 Subject: [PATCH 5/5] Always remove a branch's worktree with everything in it A worktree is scratch space for its branch, so the delete now runs `git worktree remove --force` in safe and force mode alike. Safe mode no longer refuses a worktree with uncommitted changes. A locked worktree is still refused, and an unmerged branch still keeps its worktree until the force retry, since the commits are the work at risk there. Co-Authored-By: Claude Opus 5.5 (1M context) --- README.md | 17 ++++++++--------- git.go | 22 +++++++++------------- main_test.go | 46 +++++++++++++++++----------------------------- model.go | 2 +- view.go | 13 +++++-------- 5 files changed, 40 insertions(+), 60 deletions(-) diff --git a/README.md b/README.md index 44a4d4f..1f4f043 100644 --- a/README.md +++ b/README.md @@ -178,19 +178,18 @@ claiming the work is unrecoverable. ## Branches checked out in a worktree git refuses to delete a branch that any worktree has checked out, even with `-D`. git_pruner -marks such a branch with `+` and runs `git worktree remove` before it deletes the branch. The -confirmation screen names each worktree it will remove. +marks such a branch with `+` and runs `git worktree remove --force` before it deletes the +branch. The confirmation screen names each worktree it will remove. -- In safe mode, git refuses to remove a worktree with uncommitted changes or untracked files, - and the branch stays. Force mode (`f`) removes the worktree with `--force` and discards them. -- A worktree whose directory is already gone is removed in either mode. +- A worktree is scratch space for its branch. The remove deletes the worktree folder and + everything in it, uncommitted changes included, in safe and force mode alike. +- A worktree whose folder is already gone is removed too. - If the safe delete (`-d`) would refuse the branch as not fully merged, the worktree stays - until you accept the force delete prompt. -- git also deletes the worktree's ignored files, such as `.env`, in both modes. git does not - count them as changes, so the confirmation screen warns about them. + until you accept the force delete prompt. The branch's commits are the work at risk there. +- git refuses to remove a locked worktree (`git worktree lock`), and the branch stays. - A branch checked out in the main worktree is locked: git cannot remove the main worktree. This happens when you run git_pruner from a linked worktree. -- Script mode removes worktrees the same way as safe mode. +- Script mode removes worktrees the same way. ## Selecting merged, old branches diff --git a/git.go b/git.go index 55d2d7a..218ac4f 100644 --- a/git.go +++ b/git.go @@ -631,8 +631,8 @@ func (b branch) deleteFlag(force bool) string { // deleteBranch runs one branch's local delete and, when wantRemote is set, its // remote-branch push --delete. It is the worker deleteBranchCmd runs off the -// update loop, one cmd per branch. force is the user's force mode: it picks the -// delete flag and lets a worktree with uncommitted changes be removed. +// update loop, one cmd per branch. force is the user's force mode, which picks +// the delete flag. func deleteBranch(b branch, force, wantRemote bool) deleteResult { res := deleteResult{br: b, done: true} if b.remoteOnly { @@ -650,7 +650,7 @@ func deleteBranch(b branch, force, wantRemote bool) deleteResult { // The worktree stays until the user agrees to that retry. res.localErr = "not fully merged (worktree kept)" res.forceable = true - case b.worktree != "" && !removeWorktree(&res, force): + case b.worktree != "" && !removeWorktree(&res): default: if _, err := runGit("branch", flag, b.name); err != nil { res.localErr = err.Error() @@ -676,16 +676,12 @@ func deleteBranch(b branch, force, wantRemote bool) deleteResult { } // removeWorktree removes the linked worktree holding res's branch: git refuses to -// delete a branch any worktree has checked out, under -d and -D alike. Without -// force, git refuses a worktree with uncommitted changes or untracked files; -// with it, they are discarded. A worktree whose directory is already gone is -// removed either way. It reports whether the branch is now free to delete. -func removeWorktree(res *deleteResult, force bool) bool { - args := []string{"worktree", "remove"} - if force { - args = append(args, "--force") - } - if _, err := runGit(append(args, res.br.worktree)...); err != nil { +// delete a branch any worktree has checked out, under -d and -D alike. A +// worktree is scratch space for its branch, so it goes with everything in it, +// uncommitted changes included. A locked worktree is refused: someone locked it +// on purpose. It reports whether the branch is now free to delete. +func removeWorktree(res *deleteResult) bool { + if _, err := runGit("worktree", "remove", "--force", res.br.worktree); err != nil { res.localErr = "worktree " + res.br.worktree + ": " + cleanText(err.Error()) return false } diff --git a/main_test.go b/main_test.go index 61603f9..cbf1712 100644 --- a/main_test.go +++ b/main_test.go @@ -1387,13 +1387,11 @@ func TestNonUnmergedFailureIsNotForceable(t *testing.T) { repo := setupRepo(t) chdir(t, repo) - // A second worktree holds the merged branch and an untracked file, so a - // safe delete must refuse to remove the worktree, and with it the branch. + // A locked worktree holds the merged branch: git refuses to remove it, so + // the branch cannot be freed under -d or -D. wt := t.TempDir() + "/wt" git(t, repo, "worktree", "add", "-q", wt, "feature/merged") - if err := os.WriteFile(wt+"/scratch", []byte("unsaved work"), 0o644); err != nil { - t.Fatal(err) - } + git(t, repo, "worktree", "lock", wt) m, err := initialModel() if err != nil { @@ -1405,7 +1403,7 @@ func TestNonUnmergedFailureIsNotForceable(t *testing.T) { t.Fatalf("delete should have failed: %+v", res) } if res.forceable { - t.Fatalf("a dirty worktree must not be offered as force-retryable: %q", res.localErr) + t.Fatalf("a locked worktree must not be offered as force-retryable: %q", res.localErr) } // End to end: the async path lands on the results screen, not the prompt. @@ -2881,14 +2879,17 @@ func TestWorktreeBranchDeletes(t *testing.T) { } } -// A worktree with uncommitted work survives a safe delete, and the branch with -// it; force mode is the user's say-so to discard that work. -func TestDirtyWorktreeNeedsForce(t *testing.T) { +// A worktree is scratch space for its branch: even a safe delete removes it +// with everything in it, changed and untracked files included. +func TestDirtyWorktreeIsRemoved(t *testing.T) { repo := setupLocalRepo(t) chdir(t, repo) wt := filepath.Join(realTempDir(t), "wt") git(t, repo, "worktree", "add", "-q", wt, "feature/merged") - if err := os.WriteFile(filepath.Join(wt, "scratch"), []byte("unsaved work"), 0o644); err != nil { + if err := os.WriteFile(filepath.Join(wt, "a"), []byte("changed"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(wt, "scratch"), []byte("untracked"), 0o644); err != nil { t.Fatal(err) } @@ -2898,30 +2899,17 @@ func TestDirtyWorktreeNeedsForce(t *testing.T) { } find(m.branches, "feature/merged").selected = true m.force = false - if body := strings.Join(m.confirmBody(), "\n"); !strings.Contains(body, "remove worktree "+wt+" (refused if it has uncommitted changes; ignored files will be lost)") { - t.Fatalf("the safe confirm must say the worktree goes only if clean:\n%s", body) + if body := strings.Join(m.confirmBody(), "\n"); !strings.Contains(body, "remove worktree "+wt+" and everything in it") { + t.Fatalf("the confirm must say the worktree goes with its contents:\n%s", body) } m = startAndDrain(t, m, false) - wantState(t, m, stateResult, "a dirty worktree is not a force-prompt case") - if r := result(t, m, "feature/merged"); r.localOK || r.worktreeRemoved || !strings.Contains(r.localErr, wt) { - t.Fatalf("safe mode must refuse the dirty worktree and name it: %+v", r) - } - if !dirExists(filepath.Join(wt, "scratch")) || find(m.branches, "feature/merged") == nil { - t.Fatal("the worktree, its files and the branch must all survive a refused delete") - } - - find(m.branches, "feature/merged").selected = true - m.force = true - if body := strings.Join(m.confirmBody(), "\n"); !strings.Contains(body, "uncommitted and ignored files will be lost") { - t.Fatalf("the force confirm must warn that changes are discarded:\n%s", body) - } - m = startAndDrain(t, m, false) + wantState(t, m, stateResult, "a dirty worktree is removed, not refused") if r := result(t, m, "feature/merged"); !r.localOK || !r.worktreeRemoved { - t.Fatalf("force mode should remove the dirty worktree and the branch: %+v", r) + t.Fatalf("safe mode should remove the dirty worktree and the branch: %+v", r) } - if dirExists(wt) { - t.Fatal("the worktree directory should be removed under force") + if dirExists(wt) || find(m.branches, "feature/merged") != nil { + t.Fatal("the worktree and the branch should both be gone") } } diff --git a/model.go b/model.go index c27640c..ea83af7 100644 --- a/model.go +++ b/model.go @@ -1206,7 +1206,7 @@ func (m *model) forceDeleteUnmerged() { } // deleteBranch kept the worktree while -d was going to refuse the // branch; the user has now agreed to the delete, so free it. - if r.worktreePending() && !removeWorktree(r, m.force) { + if r.worktreePending() && !removeWorktree(r) { continue } if _, err := runGit("branch", "-D", r.br.name); err != nil { diff --git a/view.go b/view.go index 93d9ee4..b90fb77 100644 --- a/view.go +++ b/view.go @@ -546,8 +546,8 @@ func (m model) helpView() string { b.WriteString("\n") b.WriteString(dimStyle.Render("Gone branches are deleted with -D. Any holding commits that are not in\n" + "the default branch are left unselected by x/p and flagged on the confirm screen.\n" + - "A branch in a linked worktree is deleted after its worktree is removed; safe mode\n" + - "refuses a worktree with uncommitted changes, force mode discards them.\n" + + "A branch in a linked worktree is deleted after its worktree is removed, with\n" + + "everything in it.\n" + "Protect more branches with: git config --add pruner.protect 'release/*'")) b.WriteString("\n\n") @@ -639,19 +639,16 @@ func (m model) confirmView() string { return m.page(m.confirmParts()) } // worktreeWarning states what deleting br does to the worktree that holds it, // or "" when no worktree does. git refuses to delete a branch a worktree has -// checked out, so the worktree is removed first. git also deletes the -// worktree's ignored files, such as .env, which it never counts as changes. +// checked out, so the worktree is removed first, with everything in it. func (m model) worktreeWarning(br branch) string { switch { case br.worktree == "": return "" - case m.force: - return "+ remove worktree " + br.worktree + " (uncommitted and ignored files will be lost)" - case br.safeDeleteRefuses(false): + case br.safeDeleteRefuses(m.force): // The worktree stays until the force retry. return "+ remove worktree " + br.worktree + " if you then force delete" default: - return "+ remove worktree " + br.worktree + " (refused if it has uncommitted changes; ignored files will be lost)" + return "+ remove worktree " + br.worktree + " and everything in it" } }