From 6700041a1ec864663737119dedb6330dc6561f35 Mon Sep 17 00:00:00 2001 From: Masayoshi Wada Date: Thu, 3 Sep 2026 10:48:00 +0900 Subject: [PATCH 1/5] skip the base resolution a forced removal never uses --- main.go | 9 ++++++--- main_test.go | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/main.go b/main.go index f4e6679..dbac7de 100644 --- a/main.go +++ b/main.go @@ -168,9 +168,12 @@ func cmdRemove(stderr io.Writer, args []string, cwd string) error { if err != nil { return err } - base, err := invocationBase(ctx, cwd) - if err != nil { - return err + // A forced removal judges nothing against a base, so it takes none. + var base removeBase + if !*force { + if base, err = invocationBase(ctx, cwd); err != nil { + return err + } } // A single branch keeps the plain one-line error; the per-branch prefix // and the summary would only repeat it. diff --git a/main_test.go b/main_test.go index bad547c..42ff547 100644 --- a/main_test.go +++ b/main_test.go @@ -284,6 +284,22 @@ func TestRunRemoveForce(t *testing.T) { assertRemoved(t, repo, wt, "topic") } +func TestRunRemoveForceWithUnbornPrimaryHead(t *testing.T) { + repo := newTestRepo(t) + ctx, _ := loadRepoWithRoot(t, repo) + wt := mustResolve(t, ctx, repo, "topic") + // `git switch --orphan` leaves the primary checkout on an unborn HEAD + // that resolves to no commit. A forced removal judges nothing against + // a base, so it must not need one. + gitT(t, repo, "switch", "-q", "--orphan", "orphan") + + code, _, stderr := runEda(t, repo, "", "remove", "--force", "topic") + if code != 0 { + t.Fatalf("remove --force: exit=%d stderr=%q", code, stderr) + } + assertRemoved(t, repo, wt, "topic") +} + func TestRunStatus(t *testing.T) { repo := newTestRepo(t) ctx, _ := loadRepoWithRoot(t, repo) From 29754e5ab6f109151bafd0b64a150543ee4bdfd2 Mon Sep 17 00:00:00 2001 From: Masayoshi Wada Date: Thu, 3 Sep 2026 10:48:37 +0900 Subject: [PATCH 2/5] resolve the fallback HEAD up front but fail only when a branch needs it --- main.go | 9 +++----- main_test.go | 16 ++++++++++++++ remove.go | 57 +++++++++++++++++++++++++++++++------------------- remove_test.go | 24 +++++++++++++++++++++ 4 files changed, 78 insertions(+), 28 deletions(-) diff --git a/main.go b/main.go index dbac7de..56025cf 100644 --- a/main.go +++ b/main.go @@ -312,12 +312,9 @@ func cmdHook(stdin io.Reader, stdout, stderr io.Writer, args []string, cwd strin } // The hook has no invoking worktree to take a base from (where it // runs is not where the session started); the primary checkout's - // HEAD is the fallback base. - primaryHead, err := headCommit(ctx.PrimaryPath) - if err != nil { - return err - } - if err := removeWorktree(ctx, removeBase{PrimaryHead: primaryHead}, branch, false); err != nil { + // HEAD is the fallback base, needed only without an upstream. + base := removeBase{PrimaryHead: resolveHead(ctx.PrimaryPath)} + if err := removeWorktree(ctx, base, branch, false); err != nil { var refusal refusalError if errors.As(err, &refusal) { // Keeping a worktree that still holds work is a valid diff --git a/main_test.go b/main_test.go index 42ff547..92b4720 100644 --- a/main_test.go +++ b/main_test.go @@ -503,6 +503,22 @@ func TestRunHookWorktreeRemoveKeepsDuplicateBranch(t *testing.T) { assertKept(t, repo, dup, "agent-abc") } +func TestRunHookWorktreeRemoveWithUnbornPrimaryHead(t *testing.T) { + repo := newTestRepo(t) + ctx, _ := loadRepoWithRoot(t, repo) + wt := mustResolve(t, ctx, repo, "agent-abc") + gitT(t, repo, "branch", "-q", "--set-upstream-to=main", "agent-abc") + // The primary HEAD is the hook's fallback base; unborn, it cannot be + // resolved, but a branch judged by its upstream never needs it. + gitT(t, repo, "switch", "-q", "--orphan", "orphan") + + code, _, stderr := runEda(t, repo, removeHookInput(repo, wt), "hook", "worktree-remove") + if code != 0 { + t.Fatalf("hook worktree-remove: exit=%d stderr=%q", code, stderr) + } + assertRemoved(t, repo, wt, "agent-abc") +} + func TestRunHookWorktreeRemoveFromOtherDirectory(t *testing.T) { // The session cwd follows `cd`, so by the time the session ends it may // be in another repository or outside any; worktree_path alone must diff --git a/remove.go b/remove.go index e9f30fc..3e1acf8 100644 --- a/remove.go +++ b/remove.go @@ -30,19 +30,37 @@ func managedWorktree(ctx *repoContext, path string) bool { // removeBase is the HEAD a branch without a resolvable upstream is judged // against, fixed once when the command starts so that every branch of one // invocation sees the same commits, whatever earlier removals deleted. +// Resolving a HEAD may fail (unborn, as after `git switch --orphan`); the +// failure is kept as well and surfaces only for a branch that needs that +// HEAD, so branches judged by their upstream are unaffected. The zero value +// is the base of a forced removal, which judges nothing. type removeBase struct { // Top is the realpath of the worktree the command was run in, empty - // when it was not run in one (the hook), and Head is the OID its HEAD - // pointed to. + // when it was not run in one (the hook), and Head is what its HEAD + // resolved to. Top string - Head string - // PrimaryHead is the OID of the primary checkout's HEAD. It replaces - // Head for the worktree the command was run in, and is the base for - // everything when there is no such worktree. - PrimaryHead string + Head resolvedHead + // PrimaryHead is what the primary checkout's HEAD resolved to. It + // replaces Head for the worktree the command was run in, and is the + // base for everything when there is no such worktree. + PrimaryHead resolvedHead } -// invocationBase resolves the removeBase of a command run in cwd. +// resolvedHead is the outcome of resolving a HEAD to a commit: the OID, or +// the error to report should that HEAD be needed as a base. +type resolvedHead struct { + OID string + Err error +} + +func resolveHead(dir string) resolvedHead { + oid, err := headCommit(dir) + return resolvedHead{OID: oid, Err: err} +} + +// invocationBase resolves the removeBase of a command run in cwd. It fails +// when cwd is not in a worktree; a HEAD that does not resolve is recorded, +// not reported. func invocationBase(ctx *repoContext, cwd string) (removeBase, error) { out, err := runGit(cwd, "rev-parse", "--show-toplevel") if err != nil { @@ -54,24 +72,16 @@ func invocationBase(ctx *repoContext, cwd string) (removeBase, error) { if err != nil { return removeBase{}, fmt.Errorf("resolve invoking worktree path: %w", err) } - head, err := headCommit(cwd) - if err != nil { - return removeBase{}, err - } - primaryHead, err := headCommit(ctx.PrimaryPath) - if err != nil { - return removeBase{}, err - } - return removeBase{Top: top, Head: head, PrimaryHead: primaryHead}, nil + return removeBase{Top: top, Head: resolveHead(cwd), PrimaryHead: resolveHead(ctx.PrimaryPath)}, nil } // headFor returns the base OID for the worktree at path and a description -// of it for diagnostics. -func (b removeBase) headFor(path string) (oid, desc string) { +// of it for diagnostics, or the error resolving that HEAD produced. +func (b removeBase) headFor(path string) (oid, desc string, err error) { if b.Top == "" || path == b.Top { - return b.PrimaryHead, "the HEAD of the primary checkout" + return b.PrimaryHead.OID, "the HEAD of the primary checkout", b.PrimaryHead.Err } - return b.Head, "the HEAD of " + b.Top + return b.Head.OID, "the HEAD of " + b.Top, b.Head.Err } // removeWorktree deletes a worktree and its branch as a pair. Unless force @@ -132,7 +142,10 @@ func removeWorktree(ctx *repoContext, base removeBase, branch string, force bool return err } if rev == "" { - rev, desc = base.headFor(entry.Path) + rev, desc, err = base.headFor(entry.Path) + if err != nil { + return fmt.Errorf("cannot judge branch %q against %s: %w", branch, desc, err) + } } reachable, err := isAncestor(ctx.PrimaryPath, "refs/heads/"+branch, rev) if err != nil { diff --git a/remove_test.go b/remove_test.go index e4516d3..0ccdddb 100644 --- a/remove_test.go +++ b/remove_test.go @@ -252,6 +252,30 @@ func TestRemoveWorktreeLocalUpstream(t *testing.T) { assertRemoved(t, repo, wt, "topic") } +func TestRemoveWorktreeUnbornPrimaryHead(t *testing.T) { + repo := newTestRepo(t) + ctx, _ := loadRepoWithRoot(t, repo) + wt := mustResolve(t, ctx, repo, "topic") + // `git switch --orphan` leaves the primary checkout on an unborn HEAD + // that resolves to no commit. + gitT(t, repo, "switch", "-q", "--orphan", "orphan") + + // Without an upstream the unborn HEAD is the base, and resolving it + // fails only now, for this branch. + err := removeFrom(t, repo, repo, "topic", false) + if err == nil || !strings.Contains(err.Error(), "cannot resolve HEAD") { + t.Fatalf("branch judged by the unborn HEAD must fail on it, got %v", err) + } + assertKept(t, repo, wt, "topic") + + // Judged by its upstream, the branch never needs that HEAD. + gitT(t, repo, "branch", "-q", "--set-upstream-to=main", "topic") + if err := removeFrom(t, repo, repo, "topic", false); err != nil { + t.Fatalf("branch reachable from its upstream must be removable: %v", err) + } + assertRemoved(t, repo, wt, "topic") +} + func TestRemoveWorktreeGoneUpstream(t *testing.T) { repo := newTestClone(t) ctx, _ := loadRepoWithRoot(t, repo) From 942adc8a45a258a12b538537b2b3b03e1e478e6b Mon Sep 17 00:00:00 2001 From: Masayoshi Wada Date: Thu, 3 Sep 2026 10:55:28 +0900 Subject: [PATCH 3/5] pin down that a forced removal takes no base and both fallback HEADs can fail --- main_test.go | 45 +++++++++++++++++++++++++++++++++++++++++++++ remove_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/main_test.go b/main_test.go index 92b4720..773dd5c 100644 --- a/main_test.go +++ b/main_test.go @@ -300,6 +300,51 @@ func TestRunRemoveForceWithUnbornPrimaryHead(t *testing.T) { assertRemoved(t, repo, wt, "topic") } +func TestRunRemoveForceNeedsNoInvokingWorktree(t *testing.T) { + repo := newTestRepo(t) + ctx, _ := loadRepoWithRoot(t, repo) + wt := mustResolve(t, ctx, repo, "topic") + + // Inside .git the repository loads but there is no worktree to take a + // base from; only a forced removal, which takes none, can proceed. + gitDir := filepath.Join(repo, ".git") + if code, _, stderr := runEda(t, gitDir, "", "remove", "topic"); code == 0 || !strings.Contains(stderr, "work tree") { + t.Fatalf("remove without a worktree to judge from must fail: exit=%d stderr=%q", code, stderr) + } + assertKept(t, repo, wt, "topic") + if code, _, stderr := runEda(t, gitDir, "", "remove", "--force", "topic"); code != 0 { + t.Fatalf("remove --force: exit=%d stderr=%q", code, stderr) + } + assertRemoved(t, repo, wt, "topic") +} + +func TestRunRemoveMultipleWithUnbornPrimaryHead(t *testing.T) { + repo := newTestRepo(t) + ctx, _ := loadRepoWithRoot(t, repo) + wtA := mustResolve(t, ctx, repo, "a") + wtB := mustResolve(t, ctx, repo, "b") + wtC := mustResolve(t, ctx, repo, "c") + gitT(t, repo, "branch", "-q", "--set-upstream-to=main", "a") + gitT(t, repo, "branch", "-q", "--set-upstream-to=main", "c") + gitT(t, repo, "switch", "-q", "--orphan", "orphan") + + // The unborn HEAD fails only b, which has no upstream to be judged by; + // a before it and c after it are removed through their upstreams. + code, _, stderr := runEda(t, repo, "", "remove", "a", "b", "c") + if code == 0 { + t.Fatal("a branch judged by the unborn HEAD must fail the command") + } + assertRemoved(t, repo, wtA, "a") + assertKept(t, repo, wtB, "b") + assertRemoved(t, repo, wtC, "c") + if !strings.Contains(stderr, "remove b:") || !strings.Contains(stderr, "cannot resolve HEAD") { + t.Errorf("stderr must report the unresolvable HEAD for b, got %q", stderr) + } + if !strings.Contains(stderr, "failed to remove 1 of 3") { + t.Errorf("stderr must summarize the failures, got %q", stderr) + } +} + func TestRunStatus(t *testing.T) { repo := newTestRepo(t) ctx, _ := loadRepoWithRoot(t, repo) diff --git a/remove_test.go b/remove_test.go index 0ccdddb..6f7206a 100644 --- a/remove_test.go +++ b/remove_test.go @@ -276,6 +276,31 @@ func TestRemoveWorktreeUnbornPrimaryHead(t *testing.T) { assertRemoved(t, repo, wt, "topic") } +func TestRemoveWorktreeUnbornInvokingHead(t *testing.T) { + repo := newTestRepo(t) + ctx, _ := loadRepoWithRoot(t, repo) + wt := mustResolve(t, ctx, repo, "topic") + wtOther := mustResolve(t, reload(t, repo), repo, "other") + // The primary HEAD stays resolvable; only the worktree the command runs + // in is left on an unborn HEAD. + gitT(t, wtOther, "switch", "-q", "--orphan", "orphan") + + err := removeFrom(t, repo, wtOther, "topic", false) + if err == nil || !strings.Contains(err.Error(), "cannot resolve HEAD") { + t.Fatalf("branch judged by the unborn invoking HEAD must fail on it, got %v", err) + } + if !strings.Contains(err.Error(), "the HEAD of "+wtOther) { + t.Errorf("error must name the invoking HEAD, got %q", err) + } + assertKept(t, repo, wt, "topic") + + gitT(t, repo, "branch", "-q", "--set-upstream-to=main", "topic") + if err := removeFrom(t, repo, wtOther, "topic", false); err != nil { + t.Fatalf("branch reachable from its upstream must be removable: %v", err) + } + assertRemoved(t, repo, wt, "topic") +} + func TestRemoveWorktreeGoneUpstream(t *testing.T) { repo := newTestClone(t) ctx, _ := loadRepoWithRoot(t, repo) From cb3e31425738ae0f3d61a674ac363eb5236742a8 Mon Sep 17 00:00:00 2001 From: Masayoshi Wada Date: Thu, 3 Sep 2026 11:56:05 +0900 Subject: [PATCH 4/5] drop the test that pins a forced removal from inside .git --- main_test.go | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/main_test.go b/main_test.go index 773dd5c..6cd7bf7 100644 --- a/main_test.go +++ b/main_test.go @@ -300,24 +300,6 @@ func TestRunRemoveForceWithUnbornPrimaryHead(t *testing.T) { assertRemoved(t, repo, wt, "topic") } -func TestRunRemoveForceNeedsNoInvokingWorktree(t *testing.T) { - repo := newTestRepo(t) - ctx, _ := loadRepoWithRoot(t, repo) - wt := mustResolve(t, ctx, repo, "topic") - - // Inside .git the repository loads but there is no worktree to take a - // base from; only a forced removal, which takes none, can proceed. - gitDir := filepath.Join(repo, ".git") - if code, _, stderr := runEda(t, gitDir, "", "remove", "topic"); code == 0 || !strings.Contains(stderr, "work tree") { - t.Fatalf("remove without a worktree to judge from must fail: exit=%d stderr=%q", code, stderr) - } - assertKept(t, repo, wt, "topic") - if code, _, stderr := runEda(t, gitDir, "", "remove", "--force", "topic"); code != 0 { - t.Fatalf("remove --force: exit=%d stderr=%q", code, stderr) - } - assertRemoved(t, repo, wt, "topic") -} - func TestRunRemoveMultipleWithUnbornPrimaryHead(t *testing.T) { repo := newTestRepo(t) ctx, _ := loadRepoWithRoot(t, repo) From 80d13cec741b0cbbe34ab3610a898c3f200f3f59 Mon Sep 17 00:00:00 2001 From: Masayoshi Wada Date: Thu, 3 Sep 2026 11:56:05 +0900 Subject: [PATCH 5/5] give a forced removeFrom no base, as cmdRemove does --- remove_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/remove_test.go b/remove_test.go index 6f7206a..423f3b4 100644 --- a/remove_test.go +++ b/remove_test.go @@ -36,11 +36,16 @@ func baseFrom(t *testing.T, ctx *repoContext, cwd string) removeBase { return base } -// removeFrom runs removeWorktree as a remove command started in cwd would. +// removeFrom runs removeWorktree as a remove command started in cwd would: +// a forced removal takes no base, like cmdRemove. func removeFrom(t *testing.T, repo, cwd, branch string, force bool) error { t.Helper() ctx := reload(t, repo) - return removeWorktree(ctx, baseFrom(t, ctx, cwd), branch, force) + var base removeBase + if !force { + base = baseFrom(t, ctx, cwd) + } + return removeWorktree(ctx, base, branch, force) } // newTestClone clones a fresh test repository so branches can have a