fix(tools): harden worktree branch deletion

Refuse symbolic branch refs and delete with --no-deref, recheck that the branch is not checked out right before deletion, require write access to the Git common directory, pin the resolved merge target for preview staleness checks, and keep branch errors in every partial-failure report.

commit: dd4a900fb6dd339c5d38589f9be844175ca1a92f
author: brice · 4d ago · 2026-10-18 18:04 UTC
parent: 2c1ebcb
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

2 files · +280 -11

M packages/agent/tools/manage_worktrees.go +78 -11


@@ -49,9 +49,14 @@ type manageWorktreesPlan struct {
49	49		// into is the requested merge target; empty means the current HEAD.
50	50		into string
51	51		deleteBranch bool
52		-	selected []managedWorktree
53		-	all []managedWorktree
54		-	text string
	52	+	// intoCommit and intoRef pin the resolved merge target so a target that
	53	+	// moves between preview and execute is detected. intoRef is the symbolic
	54	+	// full name (empty for a commit or detached HEAD).
	55	+	intoCommit string
	56	+	intoRef string
	57	+	selected []managedWorktree
	58	+	all []managedWorktree
	59	+	text string
55	60		}
56	61
57	62		type manageWorktreesPreview struct {

@@ -104,8 +109,13 @@ func (t *ManageWorktreesTool) Execute(ctx context.Context, raw json.RawMessage,
104	109		}
105	110		var removed, deleted []string
106	111		var branchErrs []string
	112	+	// summary reports progress made so far; it is attached to every failure
	113	+	// return so a partial cleanup is never silently dropped.
107	114		summary := func() string {
108	115		var parts []string
	116	+	if len(branchErrs) > 0 {
	117	+	parts = append(parts, strings.Join(branchErrs, "; "))
	118	+	}
109	119		if len(removed) > 0 {
110	120		parts = append(parts, "removed worktrees: "+strings.Join(removed, ", "))
111	121		}

@@ -140,7 +150,7 @@ func (t *ManageWorktreesTool) Execute(ctx context.Context, raw json.RawMessage,
140	150		}
141	151		_, _ = manageWorktreesGit(ctx, plan.repoRoot, "worktree", "prune")
142	152		if len(branchErrs) > 0 {
143		-	return core.ToolResult{}, fmt.Errorf("manage_worktrees: %s (worktrees were removed; the listed branches were kept)%s", strings.Join(branchErrs, "; "), summary())
	153	+	return core.ToolResult{}, fmt.Errorf("manage_worktrees: worktrees were removed but some branches were kept%s", summary())
144	154		}
145	155		result := manageWorktreesResult(plan, "removed")
146	156		if len(deleted) > 0 {

@@ -150,25 +160,64 @@ func (t *ManageWorktreesTool) Execute(ctx context.Context, raw json.RawMessage,
150	160		}
151	161		result.Content = []provider.Content{provider.TextBlock{Text: text}}
152	162		}
153		-	details := result.Details.(map[string]any)
154		-	details["removed"] = removed
155		-	details["deleted_branches"] = deleted
	163	+	if details, ok := result.Details.(map[string]any); ok {
	164	+	details["removed"] = removed
	165	+	details["deleted_branches"] = deleted
	166	+	}
156	167		return result, nil
157	168		}
158	169
159	170		// deleteMergedBranch deletes refs/heads/<branch> only while it still points at
160		-	// head, so a branch that moved after inspection is never discarded.
	171	+	// head, is a plain (non-symbolic) ref, and is not checked out in any worktree.
	172	+	// The checks run immediately before deletion because state can change after
	173	+	// planning and after the worktree was removed.
161	174		func deleteMergedBranch(ctx context.Context, repoRoot, branch, head string) error {
162	175		if branch == "" \|\| head == "" {
163	176		return errors.New("branch or inspected commit is unknown")
164	177		}
165		-	_, err := manageWorktreesGit(ctx, repoRoot, "update-ref", "-d", "refs/heads/"+branch, head)
	178	+	ref := "refs/heads/" + branch
	179	+	// A symbolic branch ref would make update-ref delete its target.
	180	+	if _, err := manageWorktreesGit(ctx, repoRoot, "symbolic-ref", "-q", ref); err == nil {
	181	+	return errors.New("branch ref is symbolic and was kept")
	182	+	} else if createWorktreeGitExitCode(err) != 1 {
	183	+	return fmt.Errorf("inspect branch ref: %w", err)
	184	+	}
	185	+	where, err := branchCheckedOutAt(ctx, repoRoot, ref)
166	186		if err != nil {
	187	+	return err
	188	+	}
	189	+	if where != "" {
	190	+	return fmt.Errorf("branch is checked out in %q", filepath.ToSlash(where))
	191	+	}
	192	+	if _, err := manageWorktreesGit(ctx, repoRoot, "update-ref", "--no-deref", "-d", ref, head); err != nil {
167	193		return fmt.Errorf("branch changed since inspection or cannot be deleted: %w", err)
168	194		}
169	195		return nil
170	196		}
171	197
	198	+	// branchCheckedOutAt returns the path of a worktree (primary included) that has
	199	+	// ref checked out, or "" when none does.
	200	+	func branchCheckedOutAt(ctx context.Context, repoRoot, ref string) (string, error) {
	201	+	output, err := manageWorktreesGit(ctx, repoRoot, "worktree", "list", "--porcelain")
	202	+	if err != nil {
	203	+	return "", fmt.Errorf("list worktrees: %w", err)
	204	+	}
	205	+	for _, block := range strings.Split(strings.TrimSpace(output), "\n\n") {
	206	+	path, checkedOut := "", false
	207	+	for _, line := range strings.Split(block, "\n") {
	208	+	if strings.HasPrefix(line, "worktree ") {
	209	+	path = strings.TrimPrefix(line, "worktree ")
	210	+	} else if line == "branch "+ref {
	211	+	checkedOut = true
	212	+	}
	213	+	}
	214	+	if checkedOut {
	215	+	return path, nil
	216	+	}
	217	+	}
	218	+	return "", nil
	219	+	}
	220	+
172	221		func (t *ManageWorktreesTool) plan(ctx context.Context, raw json.RawMessage) (manageWorktreesPlan, error) {
173	222		var args manageWorktreesArgs
174	223		if err := json.Unmarshal(raw, &args); err != nil {

@@ -228,7 +277,7 @@ func (t *ManageWorktreesTool) plan(ctx context.Context, raw json.RawMessage) (ma
228	277		if err != nil {
229	278		return manageWorktreesPlan{}, err
230	279		}
231		-	plan := manageWorktreesPlan{repoRoot: repoRoot, action: args.Action, into: into, deleteBranch: args.DeleteBranch, all: items}
	280	+	plan := manageWorktreesPlan{repoRoot: repoRoot, action: args.Action, into: into, intoCommit: intoCommit, intoRef: intoRef, deleteBranch: args.DeleteBranch, all: items}
232	281		if args.Action == "cleanup" {
233	282		if len(args.Paths) == 0 {
234	283		return manageWorktreesPlan{}, errors.New("manage_worktrees: cleanup requires at least one explicit path")

@@ -266,6 +315,24 @@ func (t *ManageWorktreesTool) plan(ctx context.Context, raw json.RawMessage) (ma
266	315		}
267	316		}
268	317		if args.DeleteBranch {
	318	+	// Deleting a branch writes to Git ref storage, which lives outside the
	319	+	// worktree path. Require write access to it and fail closed. Jail mode
	320	+	// is a guardrail, not a sandbox: it cannot stop git from touching refs
	321	+	// once permitted, so this only enforces the declared scopes.
	322	+	commonDir, err := manageWorktreesGit(ctx, repoRoot, "rev-parse", "--git-common-dir")
	323	+	if err != nil {
	324	+	return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: resolve git directory: %w", err)
	325	+	}
	326	+	if !filepath.IsAbs(commonDir) {
	327	+	commonDir = filepath.Join(repoRoot, commonDir)
	328	+	}
	329	+	commonDir, err = canonical(commonDir)
	330	+	if err != nil {
	331	+	return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: resolve git directory: %w", err)
	332	+	}
	333	+	if err := t.Sandbox.CheckWritePath(commonDir); err != nil {
	334	+	return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: %w", err)
	335	+	}
269	336		for _, item := range plan.selected {
270	337		if intoRef == "refs/heads/"+item.Branch {
271	338		return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: branch %q is the merge target and cannot be deleted", item.Branch)

@@ -441,7 +508,7 @@ func manageWorktreesResult(plan manageWorktreesPlan, state string) core.ToolResu
441	508		}
442	509
443	510		func sameManageWorktreesPlan(a, b manageWorktreesPlan) bool {
444		-	if a.repoRoot != b.repoRoot \|\| a.action != b.action \|\| a.into != b.into \|\| a.deleteBranch != b.deleteBranch \|\| a.text != b.text \|\| len(a.selected) != len(b.selected) \|\| len(a.all) != len(b.all) {
	511	+	if a.repoRoot != b.repoRoot \|\| a.action != b.action \|\| a.into != b.into \|\| a.intoCommit != b.intoCommit \|\| a.intoRef != b.intoRef \|\| a.deleteBranch != b.deleteBranch \|\| a.text != b.text \|\| len(a.selected) != len(b.selected) \|\| len(a.all) != len(b.all) {
445	512		return false
446	513		}
447	514		for index := range a.selected {

 	// into is the requested merge target; empty means the current HEAD.
 	into         string
 	deleteBranch bool
-	selected     []managedWorktree
+	// intoCommit and intoRef pin the resolved merge target so a target that
-	all          []managedWorktree
+	// moves between preview and execute is detected. intoRef is the symbolic
-	text         string
+	// full name (empty for a commit or detached HEAD).
+	intoCommit string
+	intoRef    string
+	selected   []managedWorktree
+	all        []managedWorktree
+	text       string
 }
 type manageWorktreesPreview struct {
 	}
 	var removed, deleted []string
 	var branchErrs []string
+	// summary reports progress made so far; it is attached to every failure
+	// return so a partial cleanup is never silently dropped.
 	summary := func() string {
 		var parts []string
+		if len(branchErrs) > 0 {
+			parts = append(parts, strings.Join(branchErrs, "; "))
+		}
 		if len(removed) > 0 {
 			parts = append(parts, "removed worktrees: "+strings.Join(removed, ", "))
 		}
 	}
 	_, _ = manageWorktreesGit(ctx, plan.repoRoot, "worktree", "prune")
 	if len(branchErrs) > 0 {
-		return core.ToolResult{}, fmt.Errorf("manage_worktrees: %s (worktrees were removed; the listed branches were kept)%s", strings.Join(branchErrs, "; "), summary())
+		return core.ToolResult{}, fmt.Errorf("manage_worktrees: worktrees were removed but some branches were kept%s", summary())
 	}
 	result := manageWorktreesResult(plan, "removed")
 	if len(deleted) > 0 {
 		}
 		result.Content = []provider.Content{provider.TextBlock{Text: text}}
 	}
-	details := result.Details.(map[string]any)
+	if details, ok := result.Details.(map[string]any); ok {
-	details["removed"] = removed
+		details["removed"] = removed
-	details["deleted_branches"] = deleted
+		details["deleted_branches"] = deleted
+	}
 	return result, nil
 }
 // deleteMergedBranch deletes refs/heads/<branch> only while it still points at
-// head, so a branch that moved after inspection is never discarded.
+// head, is a plain (non-symbolic) ref, and is not checked out in any worktree.
+// The checks run immediately before deletion because state can change after
+// planning and after the worktree was removed.
 func deleteMergedBranch(ctx context.Context, repoRoot, branch, head string) error {
 	if branch == "" || head == "" {
 		return errors.New("branch or inspected commit is unknown")
 	}
-	_, err := manageWorktreesGit(ctx, repoRoot, "update-ref", "-d", "refs/heads/"+branch, head)
+	ref := "refs/heads/" + branch
+	// A symbolic branch ref would make update-ref delete its target.
+	if _, err := manageWorktreesGit(ctx, repoRoot, "symbolic-ref", "-q", ref); err == nil {
+		return errors.New("branch ref is symbolic and was kept")
+	} else if createWorktreeGitExitCode(err) != 1 {
+		return fmt.Errorf("inspect branch ref: %w", err)
+	}
+	where, err := branchCheckedOutAt(ctx, repoRoot, ref)
 	if err != nil {
+		return err
+	}
+	if where != "" {
+		return fmt.Errorf("branch is checked out in %q", filepath.ToSlash(where))
+	}
+	if _, err := manageWorktreesGit(ctx, repoRoot, "update-ref", "--no-deref", "-d", ref, head); err != nil {
 		return fmt.Errorf("branch changed since inspection or cannot be deleted: %w", err)
 	}
 	return nil
 }
+// branchCheckedOutAt returns the path of a worktree (primary included) that has
+// ref checked out, or "" when none does.
+func branchCheckedOutAt(ctx context.Context, repoRoot, ref string) (string, error) {
+	output, err := manageWorktreesGit(ctx, repoRoot, "worktree", "list", "--porcelain")
+	if err != nil {
+		return "", fmt.Errorf("list worktrees: %w", err)
+	}
+	for _, block := range strings.Split(strings.TrimSpace(output), "\n\n") {
+		path, checkedOut := "", false
+		for _, line := range strings.Split(block, "\n") {
+			if strings.HasPrefix(line, "worktree ") {
+				path = strings.TrimPrefix(line, "worktree ")
+			} else if line == "branch "+ref {
+				checkedOut = true
+			}
+		}
+		if checkedOut {
+			return path, nil
+		}
+	}
+	return "", nil
+}
 func (t *ManageWorktreesTool) plan(ctx context.Context, raw json.RawMessage) (manageWorktreesPlan, error) {
 	var args manageWorktreesArgs
 	if err := json.Unmarshal(raw, &args); err != nil {
 	if err != nil {
 		return manageWorktreesPlan{}, err
 	}
-	plan := manageWorktreesPlan{repoRoot: repoRoot, action: args.Action, into: into, deleteBranch: args.DeleteBranch, all: items}
+	plan := manageWorktreesPlan{repoRoot: repoRoot, action: args.Action, into: into, intoCommit: intoCommit, intoRef: intoRef, deleteBranch: args.DeleteBranch, all: items}
 	if args.Action == "cleanup" {
 		if len(args.Paths) == 0 {
 			return manageWorktreesPlan{}, errors.New("manage_worktrees: cleanup requires at least one explicit path")
 			}
 		}
 		if args.DeleteBranch {
+			// Deleting a branch writes to Git ref storage, which lives outside the
+			// worktree path. Require write access to it and fail closed. Jail mode
+			// is a guardrail, not a sandbox: it cannot stop git from touching refs
+			// once permitted, so this only enforces the declared scopes.
+			commonDir, err := manageWorktreesGit(ctx, repoRoot, "rev-parse", "--git-common-dir")
+			if err != nil {
+				return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: resolve git directory: %w", err)
+			}
+			if !filepath.IsAbs(commonDir) {
+				commonDir = filepath.Join(repoRoot, commonDir)
+			}
+			commonDir, err = canonical(commonDir)
+			if err != nil {
+				return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: resolve git directory: %w", err)
+			}
+			if err := t.Sandbox.CheckWritePath(commonDir); err != nil {
+				return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: %w", err)
+			}
 			for _, item := range plan.selected {
 				if intoRef == "refs/heads/"+item.Branch {
 					return manageWorktreesPlan{}, fmt.Errorf("manage_worktrees: branch %q is the merge target and cannot be deleted", item.Branch)
 }
 func sameManageWorktreesPlan(a, b manageWorktreesPlan) bool {
-	if a.repoRoot != b.repoRoot || a.action != b.action || a.into != b.into || a.deleteBranch != b.deleteBranch || a.text != b.text || len(a.selected) != len(b.selected) || len(a.all) != len(b.all) {
+	if a.repoRoot != b.repoRoot || a.action != b.action || a.into != b.into || a.intoCommit != b.intoCommit || a.intoRef != b.intoRef || a.deleteBranch != b.deleteBranch || a.text != b.text || len(a.selected) != len(b.selected) || len(a.all) != len(b.all) {
 		return false
 	}
 	for index := range a.selected {

M packages/agent/tools/manage_worktrees_test.go +202 -0


@@ -140,3 +140,205 @@ func TestManageWorktreesRejectsCleanupWithoutExplicitPaths(t *testing.T) {
140	140		func newManageWorktreesTestTool(cwd string) *ManageWorktreesTool {
141	141		return &ManageWorktreesTool{CWD: cwd, Sandbox: NewSandbox(cwd)}
142	142		}
	143	+
	144	+	// addMergedSideWorktree creates branch "side" at the primary HEAD in a new
	145	+	// worktree, so it is merged into HEAD and eligible for cleanup.
	146	+	func addMergedSideWorktree(t *testing.T, repo, branch string) string {
	147	+	t.Helper()
	148	+	path := filepath.Join(t.TempDir(), branch)
	149	+	gitTestOutput(t, repo, "worktree", "add", "-b", branch, path)
	150	+	return path
	151	+	}
	152	+
	153	+	func branchExists(t *testing.T, repo, branch string) bool {
	154	+	t.Helper()
	155	+	return gitTestOutputAllowExit(t, repo, "show-ref", "--verify", "--quiet", "refs/heads/"+branch).exitCode == 0
	156	+	}
	157	+
	158	+	func TestDeleteMergedBranchRefusesSymbolicRef(t *testing.T) {
	159	+	if _, err := exec.LookPath("git"); err != nil {
	160	+	t.Skip("git is required")
	161	+	}
	162	+	repo := initWorktreeTestRepo(t)
	163	+	mainBranch := gitTestOutput(t, repo, "branch", "--show-current")
	164	+	head := gitTestOutput(t, repo, "rev-parse", "HEAD")
	165	+	gitTestOutput(t, repo, "branch", "side")
	166	+	gitTestOutput(t, repo, "symbolic-ref", "refs/heads/side", "refs/heads/"+mainBranch)
	167	+
	168	+	err := deleteMergedBranch(context.Background(), repo, "side", head)
	169	+	if err == nil \|\| !strings.Contains(err.Error(), "symbolic") {
	170	+	t.Fatalf("error = %v, want symbolic-ref rejection", err)
	171	+	}
	172	+	if !branchExists(t, repo, mainBranch) {
	173	+	t.Fatal("symbolic ref deletion removed the target branch")
	174	+	}
	175	+	}
	176	+
	177	+	func TestManageWorktreesDeleteBranchKeepsTargetWhenBranchBecomesSymref(t *testing.T) {
	178	+	if _, err := exec.LookPath("git"); err != nil {
	179	+	t.Skip("git is required")
	180	+	}
	181	+	repo := initWorktreeTestRepo(t)
	182	+	mainBranch := gitTestOutput(t, repo, "branch", "--show-current")
	183	+	path := addMergedSideWorktree(t, repo, "side")
	184	+	tool := newManageWorktreesTestTool(repo)
	185	+	args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{path}, "delete_branch": true})
	186	+
	187	+	// The worktree is already removed when the delete progress is reported;
	188	+	// swap the branch for a symref to the primary branch at the same commit.
	189	+	progress := func(msg string) {
	190	+	if strings.HasPrefix(msg, "Deleting branch") {
	191	+	gitTestOutput(t, repo, "symbolic-ref", "refs/heads/side", "refs/heads/"+mainBranch)
	192	+	}
	193	+	}
	194	+	_, err := tool.Execute(context.Background(), args, progress)
	195	+	if err == nil \|\| !strings.Contains(err.Error(), "symbolic") \|\| !strings.Contains(err.Error(), "removed worktrees:") {
	196	+	t.Fatalf("error = %v, want symbolic-ref branch failure", err)
	197	+	}
	198	+	if !branchExists(t, repo, mainBranch) {
	199	+	t.Fatal("primary branch was deleted through a symbolic ref")
	200	+	}
	201	+	}
	202	+
	203	+	func TestManageWorktreesDeleteBranchRefusesBranchCheckedOutAfterRemoval(t *testing.T) {
	204	+	if _, err := exec.LookPath("git"); err != nil {
	205	+	t.Skip("git is required")
	206	+	}
	207	+	for _, where := range []string{"primary", "worktree"} {
	208	+	t.Run(where, func(t *testing.T) {
	209	+	repo := initWorktreeTestRepo(t)
	210	+	path := addMergedSideWorktree(t, repo, "side")
	211	+	tool := newManageWorktreesTestTool(repo)
	212	+	args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{path}, "delete_branch": true})
	213	+	progress := func(msg string) {
	214	+	if !strings.HasPrefix(msg, "Deleting branch") {
	215	+	return
	216	+	}
	217	+	if where == "primary" {
	218	+	gitTestOutput(t, repo, "checkout", "-q", "side")
	219	+	} else {
	220	+	gitTestOutput(t, repo, "worktree", "add", filepath.Join(t.TempDir(), "again"), "side")
	221	+	}
	222	+	}
	223	+	_, err := tool.Execute(context.Background(), args, progress)
	224	+	if err == nil \|\| !strings.Contains(err.Error(), `delete branch "side"`) \|\| !strings.Contains(err.Error(), "checked out") {
	225	+	t.Fatalf("error = %v, want checked-out branch failure", err)
	226	+	}
	227	+	if !branchExists(t, repo, "side") {
	228	+	t.Fatal("checked-out branch was deleted")
	229	+	}
	230	+	})
	231	+	}
	232	+	}
	233	+
	234	+	func TestManageWorktreesDeleteBranchRequiresRefStorageWriteAccess(t *testing.T) {
	235	+	if _, err := exec.LookPath("git"); err != nil {
	236	+	t.Skip("git is required")
	237	+	}
	238	+	repo := initWorktreeTestRepo(t)
	239	+	path := addMergedSideWorktree(t, repo, "side")
	240	+	scoped := func(write ...string) *ManageWorktreesTool {
	241	+	var p PermissionSet
	242	+	p.Bash.Mode = "ask"
	243	+	p.FS.Read = []string{repo, filepath.Dir(path)}
	244	+	p.FS.Write = write
	245	+	sandbox := NewSandbox(repo)
	246	+	sandbox.SetPermissions(&p)
	247	+	return &ManageWorktreesTool{CWD: repo, Sandbox: sandbox}
	248	+	}
	249	+	args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{path}, "delete_branch": true})
	250	+
	251	+	tool := scoped(path)
	252	+	if _, err := tool.Preview(context.Background(), args); err == nil \|\| !strings.Contains(err.Error(), "outside declared scopes") {
	253	+	t.Fatalf("preview error = %v, want ref-storage write denial", err)
	254	+	}
	255	+	if _, err := tool.Execute(context.Background(), args, nil); err == nil \|\| !strings.Contains(err.Error(), "outside declared scopes") {
	256	+	t.Fatalf("execute error = %v, want ref-storage write denial", err)
	257	+	}
	258	+	if _, err := os.Stat(path); err != nil {
	259	+	t.Fatalf("worktree was removed despite denial: %v", err)
	260	+	}
	261	+	if !branchExists(t, repo, "side") {
	262	+	t.Fatal("branch was deleted despite denial")
	263	+	}
	264	+
	265	+	// Without delete_branch only the worktree path needs write access.
	266	+	keep := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{path}})
	267	+	if _, err := tool.Preview(context.Background(), keep); err != nil {
	268	+	t.Fatalf("cleanup without delete_branch: %v", err)
	269	+	}
	270	+	// Granting the common git directory allows the full operation.
	271	+	tool = scoped(path, filepath.Join(repo, ".git"))
	272	+	if _, err := tool.Execute(context.Background(), args, nil); err != nil {
	273	+	t.Fatal(err)
	274	+	}
	275	+	if branchExists(t, repo, "side") {
	276	+	t.Fatal("branch remains after permitted cleanup")
	277	+	}
	278	+	}
	279	+
	280	+	func TestManageWorktreesDetectsMovedMergeTargetAfterPreview(t *testing.T) {
	281	+	if _, err := exec.LookPath("git"); err != nil {
	282	+	t.Skip("git is required")
	283	+	}
	284	+	repo := initWorktreeTestRepo(t)
	285	+	path := addMergedSideWorktree(t, repo, "side")
	286	+	gitTestOutput(t, repo, "branch", "target")
	287	+	tool := newManageWorktreesTestTool(repo)
	288	+	args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{path}, "into": "target", "delete_branch": true})
	289	+	if _, err := tool.Preview(context.Background(), args); err != nil {
	290	+	t.Fatal(err)
	291	+	}
	292	+	// Advance the unchecked-out target; side remains an ancestor, so the
	293	+	// worktree stays eligible and only the pinned target differs.
	294	+	tree := gitTestOutput(t, repo, "rev-parse", "target^{tree}")
	295	+	moved := gitTestOutput(t, repo, "commit-tree", tree, "-p", "target", "-m", "advance")
	296	+	gitTestOutput(t, repo, "update-ref", "refs/heads/target", moved)
	297	+
	298	+	_, err := tool.Execute(context.Background(), args, nil)
	299	+	if err == nil \|\| !strings.Contains(err.Error(), "changed since preview") {
	300	+	t.Fatalf("error = %v, want stale-state rejection", err)
	301	+	}
	302	+	if _, statErr := os.Stat(path); statErr != nil {
	303	+	t.Fatalf("worktree was removed: %v", statErr)
	304	+	}
	305	+	if !branchExists(t, repo, "side") {
	306	+	t.Fatal("branch was deleted")
	307	+	}
	308	+	}
	309	+
	310	+	func TestManageWorktreesPartialFailureReportsBranchErrorsAndProgress(t *testing.T) {
	311	+	if _, err := exec.LookPath("git"); err != nil {
	312	+	t.Skip("git is required")
	313	+	}
	314	+	repo := initWorktreeTestRepo(t)
	315	+	first := addMergedSideWorktree(t, repo, "first")
	316	+	second := addMergedSideWorktree(t, repo, "second")
	317	+	// A stale lock makes deleting "first" fail after its worktree is removed.
	318	+	lock := filepath.Join(repo, ".git", "refs", "heads", "first.lock")
	319	+	if err := os.WriteFile(lock, nil, 0o644); err != nil {
	320	+	t.Fatal(err)
	321	+	}
	322	+	tool := newManageWorktreesTestTool(repo)
	323	+	args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{first, second}, "delete_branch": true})
	324	+	// Dirty the second worktree so its removal fails.
	325	+	progress := func(msg string) {
	326	+	if strings.HasPrefix(msg, "Removing worktree") && strings.Contains(msg, filepath.ToSlash(second)) {
	327	+	if err := os.WriteFile(filepath.Join(second, "untracked.txt"), []byte("x"), 0o644); err != nil {
	328	+	t.Error(err)
	329	+	}
	330	+	}
	331	+	}
	332	+	_, err := tool.Execute(context.Background(), args, progress)
	333	+	if err == nil {
	334	+	t.Fatal("expected partial failure")
	335	+	}
	336	+	for _, want := range []string{"remove ", `delete branch "first"`, "removed worktrees: " + filepath.ToSlash(first)} {
	337	+	if !strings.Contains(err.Error(), want) {
	338	+	t.Fatalf("error missing %q:\n%v", want, err)
	339	+	}
	340	+	}
	341	+	if _, statErr := os.Stat(second); statErr != nil {
	342	+	t.Fatalf("second worktree should remain: %v", statErr)
	343	+	}
	344	+	}


@@ -140,3 +140,205 @@ func TestManageWorktreesRejectsCleanupWithoutExplicitPaths(t *testing.T) {
140	func newManageWorktreesTestTool(cwd string) *ManageWorktreesTool {	140		func newManageWorktreesTestTool(cwd string) *ManageWorktreesTool {
141	return &ManageWorktreesTool{CWD: cwd, Sandbox: NewSandbox(cwd)}	141		return &ManageWorktreesTool{CWD: cwd, Sandbox: NewSandbox(cwd)}
142	}	142		}
		143	+
		144	+	// addMergedSideWorktree creates branch "side" at the primary HEAD in a new
		145	+	// worktree, so it is merged into HEAD and eligible for cleanup.
		146	+	func addMergedSideWorktree(t *testing.T, repo, branch string) string {
		147	+	t.Helper()
		148	+	path := filepath.Join(t.TempDir(), branch)
		149	+	gitTestOutput(t, repo, "worktree", "add", "-b", branch, path)
		150	+	return path
		151	+	}
		152	+
		153	+	func branchExists(t *testing.T, repo, branch string) bool {
		154	+	t.Helper()
		155	+	return gitTestOutputAllowExit(t, repo, "show-ref", "--verify", "--quiet", "refs/heads/"+branch).exitCode == 0
		156	+	}
		157	+
		158	+	func TestDeleteMergedBranchRefusesSymbolicRef(t *testing.T) {
		159	+	if _, err := exec.LookPath("git"); err != nil {
		160	+	t.Skip("git is required")
		161	+	}
		162	+	repo := initWorktreeTestRepo(t)
		163	+	mainBranch := gitTestOutput(t, repo, "branch", "--show-current")
		164	+	head := gitTestOutput(t, repo, "rev-parse", "HEAD")
		165	+	gitTestOutput(t, repo, "branch", "side")
		166	+	gitTestOutput(t, repo, "symbolic-ref", "refs/heads/side", "refs/heads/"+mainBranch)
		167	+
		168	+	err := deleteMergedBranch(context.Background(), repo, "side", head)
		169	+	if err == nil \|\| !strings.Contains(err.Error(), "symbolic") {
		170	+	t.Fatalf("error = %v, want symbolic-ref rejection", err)
		171	+	}
		172	+	if !branchExists(t, repo, mainBranch) {
		173	+	t.Fatal("symbolic ref deletion removed the target branch")
		174	+	}
		175	+	}
		176	+
		177	+	func TestManageWorktreesDeleteBranchKeepsTargetWhenBranchBecomesSymref(t *testing.T) {
		178	+	if _, err := exec.LookPath("git"); err != nil {
		179	+	t.Skip("git is required")
		180	+	}
		181	+	repo := initWorktreeTestRepo(t)
		182	+	mainBranch := gitTestOutput(t, repo, "branch", "--show-current")
		183	+	path := addMergedSideWorktree(t, repo, "side")
		184	+	tool := newManageWorktreesTestTool(repo)
		185	+	args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{path}, "delete_branch": true})
		186	+
		187	+	// The worktree is already removed when the delete progress is reported;
		188	+	// swap the branch for a symref to the primary branch at the same commit.
		189	+	progress := func(msg string) {
		190	+	if strings.HasPrefix(msg, "Deleting branch") {
		191	+	gitTestOutput(t, repo, "symbolic-ref", "refs/heads/side", "refs/heads/"+mainBranch)
		192	+	}
		193	+	}
		194	+	_, err := tool.Execute(context.Background(), args, progress)
		195	+	if err == nil \|\| !strings.Contains(err.Error(), "symbolic") \|\| !strings.Contains(err.Error(), "removed worktrees:") {
		196	+	t.Fatalf("error = %v, want symbolic-ref branch failure", err)
		197	+	}
		198	+	if !branchExists(t, repo, mainBranch) {
		199	+	t.Fatal("primary branch was deleted through a symbolic ref")
		200	+	}
		201	+	}
		202	+
		203	+	func TestManageWorktreesDeleteBranchRefusesBranchCheckedOutAfterRemoval(t *testing.T) {
		204	+	if _, err := exec.LookPath("git"); err != nil {
		205	+	t.Skip("git is required")
		206	+	}
		207	+	for _, where := range []string{"primary", "worktree"} {
		208	+	t.Run(where, func(t *testing.T) {
		209	+	repo := initWorktreeTestRepo(t)
		210	+	path := addMergedSideWorktree(t, repo, "side")
		211	+	tool := newManageWorktreesTestTool(repo)
		212	+	args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{path}, "delete_branch": true})
		213	+	progress := func(msg string) {
		214	+	if !strings.HasPrefix(msg, "Deleting branch") {
		215	+	return
		216	+	}
		217	+	if where == "primary" {
		218	+	gitTestOutput(t, repo, "checkout", "-q", "side")
		219	+	} else {
		220	+	gitTestOutput(t, repo, "worktree", "add", filepath.Join(t.TempDir(), "again"), "side")
		221	+	}
		222	+	}
		223	+	_, err := tool.Execute(context.Background(), args, progress)
		224	+	if err == nil \|\| !strings.Contains(err.Error(), `delete branch "side"`) \|\| !strings.Contains(err.Error(), "checked out") {
		225	+	t.Fatalf("error = %v, want checked-out branch failure", err)
		226	+	}
		227	+	if !branchExists(t, repo, "side") {
		228	+	t.Fatal("checked-out branch was deleted")
		229	+	}
		230	+	})
		231	+	}
		232	+	}
		233	+
		234	+	func TestManageWorktreesDeleteBranchRequiresRefStorageWriteAccess(t *testing.T) {
		235	+	if _, err := exec.LookPath("git"); err != nil {
		236	+	t.Skip("git is required")
		237	+	}
		238	+	repo := initWorktreeTestRepo(t)
		239	+	path := addMergedSideWorktree(t, repo, "side")
		240	+	scoped := func(write ...string) *ManageWorktreesTool {
		241	+	var p PermissionSet
		242	+	p.Bash.Mode = "ask"
		243	+	p.FS.Read = []string{repo, filepath.Dir(path)}
		244	+	p.FS.Write = write
		245	+	sandbox := NewSandbox(repo)
		246	+	sandbox.SetPermissions(&p)
		247	+	return &ManageWorktreesTool{CWD: repo, Sandbox: sandbox}
		248	+	}
		249	+	args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{path}, "delete_branch": true})
		250	+
		251	+	tool := scoped(path)
		252	+	if _, err := tool.Preview(context.Background(), args); err == nil \|\| !strings.Contains(err.Error(), "outside declared scopes") {
		253	+	t.Fatalf("preview error = %v, want ref-storage write denial", err)
		254	+	}
		255	+	if _, err := tool.Execute(context.Background(), args, nil); err == nil \|\| !strings.Contains(err.Error(), "outside declared scopes") {
		256	+	t.Fatalf("execute error = %v, want ref-storage write denial", err)
		257	+	}
		258	+	if _, err := os.Stat(path); err != nil {
		259	+	t.Fatalf("worktree was removed despite denial: %v", err)
		260	+	}
		261	+	if !branchExists(t, repo, "side") {
		262	+	t.Fatal("branch was deleted despite denial")
		263	+	}
		264	+
		265	+	// Without delete_branch only the worktree path needs write access.
		266	+	keep := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{path}})
		267	+	if _, err := tool.Preview(context.Background(), keep); err != nil {
		268	+	t.Fatalf("cleanup without delete_branch: %v", err)
		269	+	}
		270	+	// Granting the common git directory allows the full operation.
		271	+	tool = scoped(path, filepath.Join(repo, ".git"))
		272	+	if _, err := tool.Execute(context.Background(), args, nil); err != nil {
		273	+	t.Fatal(err)
		274	+	}
		275	+	if branchExists(t, repo, "side") {
		276	+	t.Fatal("branch remains after permitted cleanup")
		277	+	}
		278	+	}
		279	+
		280	+	func TestManageWorktreesDetectsMovedMergeTargetAfterPreview(t *testing.T) {
		281	+	if _, err := exec.LookPath("git"); err != nil {
		282	+	t.Skip("git is required")
		283	+	}
		284	+	repo := initWorktreeTestRepo(t)
		285	+	path := addMergedSideWorktree(t, repo, "side")
		286	+	gitTestOutput(t, repo, "branch", "target")
		287	+	tool := newManageWorktreesTestTool(repo)
		288	+	args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{path}, "into": "target", "delete_branch": true})
		289	+	if _, err := tool.Preview(context.Background(), args); err != nil {
		290	+	t.Fatal(err)
		291	+	}
		292	+	// Advance the unchecked-out target; side remains an ancestor, so the
		293	+	// worktree stays eligible and only the pinned target differs.
		294	+	tree := gitTestOutput(t, repo, "rev-parse", "target^{tree}")
		295	+	moved := gitTestOutput(t, repo, "commit-tree", tree, "-p", "target", "-m", "advance")
		296	+	gitTestOutput(t, repo, "update-ref", "refs/heads/target", moved)
		297	+
		298	+	_, err := tool.Execute(context.Background(), args, nil)
		299	+	if err == nil \|\| !strings.Contains(err.Error(), "changed since preview") {
		300	+	t.Fatalf("error = %v, want stale-state rejection", err)
		301	+	}
		302	+	if _, statErr := os.Stat(path); statErr != nil {
		303	+	t.Fatalf("worktree was removed: %v", statErr)
		304	+	}
		305	+	if !branchExists(t, repo, "side") {
		306	+	t.Fatal("branch was deleted")
		307	+	}
		308	+	}
		309	+
		310	+	func TestManageWorktreesPartialFailureReportsBranchErrorsAndProgress(t *testing.T) {
		311	+	if _, err := exec.LookPath("git"); err != nil {
		312	+	t.Skip("git is required")
		313	+	}
		314	+	repo := initWorktreeTestRepo(t)
		315	+	first := addMergedSideWorktree(t, repo, "first")
		316	+	second := addMergedSideWorktree(t, repo, "second")
		317	+	// A stale lock makes deleting "first" fail after its worktree is removed.
		318	+	lock := filepath.Join(repo, ".git", "refs", "heads", "first.lock")
		319	+	if err := os.WriteFile(lock, nil, 0o644); err != nil {
		320	+	t.Fatal(err)
		321	+	}
		322	+	tool := newManageWorktreesTestTool(repo)
		323	+	args := mustJSON(t, map[string]any{"action": "cleanup", "paths": []string{first, second}, "delete_branch": true})
		324	+	// Dirty the second worktree so its removal fails.
		325	+	progress := func(msg string) {
		326	+	if strings.HasPrefix(msg, "Removing worktree") && strings.Contains(msg, filepath.ToSlash(second)) {
		327	+	if err := os.WriteFile(filepath.Join(second, "untracked.txt"), []byte("x"), 0o644); err != nil {
		328	+	t.Error(err)
		329	+	}
		330	+	}
		331	+	}
		332	+	_, err := tool.Execute(context.Background(), args, progress)
		333	+	if err == nil {
		334	+	t.Fatal("expected partial failure")
		335	+	}
		336	+	for _, want := range []string{"remove ", `delete branch "first"`, "removed worktrees: " + filepath.ToSlash(first)} {
		337	+	if !strings.Contains(err.Error(), want) {
		338	+	t.Fatalf("error missing %q:\n%v", want, err)
		339	+	}
		340	+	}
		341	+	if _, statErr := os.Stat(second); statErr != nil {
		342	+	t.Fatalf("second worktree should remain: %v", statErr)
		343	+	}
		344	+	}