diff --git a/mobile/src/screens/ProjectsScreen.tsx b/mobile/src/screens/ProjectsScreen.tsx index 3f6f9da6eb..90875110b2 100644 --- a/mobile/src/screens/ProjectsScreen.tsx +++ b/mobile/src/screens/ProjectsScreen.tsx @@ -260,7 +260,7 @@ export function ProjectsScreen(): JSX.Element { // Show confirmation dialog Alert.alert( "Delete Workspace?", - `This will permanently remove "${metadata.name}" from ${metadata.projectName}.\n\nThis action cannot be undone.`, + `This will permanently remove "${metadata.name}" from ${metadata.projectName} and delete the local branch "${metadata.name}".\n\nThis action cannot be undone.`, [ { text: "Cancel", style: "cancel" }, { @@ -280,7 +280,7 @@ export function ProjectsScreen(): JSX.Element { // Show force delete option Alert.alert( "Workspace Has Changes", - `${errorMsg}\n\nForce delete will discard these changes permanently.`, + `${errorMsg}\n\nForce delete will discard these changes permanently and delete the local branch "${metadata.name}".`, [ { text: "Cancel", style: "cancel" }, { diff --git a/src/browser/components/ArchivedWorkspaces.tsx b/src/browser/components/ArchivedWorkspaces.tsx index 916e91ecb2..3588aac563 100644 --- a/src/browser/components/ArchivedWorkspaces.tsx +++ b/src/browser/components/ArchivedWorkspaces.tsx @@ -518,7 +518,9 @@ export const ArchivedWorkspaces: React.FC = ({ {selectedIds.size} selected {bulkDeleteConfirm ? ( <> - Delete permanently? + + Delete permanently (also deletes local branches)? + - Delete selected permanently + + Delete selected permanently (local branches too) + - Delete permanently + Delete permanently (local branch too) diff --git a/src/browser/components/ForceDeleteModal.tsx b/src/browser/components/ForceDeleteModal.tsx index b470d468ba..fcfee88e98 100644 --- a/src/browser/components/ForceDeleteModal.tsx +++ b/src/browser/components/ForceDeleteModal.tsx @@ -70,7 +70,7 @@ export const ForceDeleteModal: React.FC = ({ This action cannot be undone - Force deleting will permanently remove the workspace and{" "} + Force deleting will permanently remove the workspace and its local branch, and{" "} {error.includes("unpushed commits:") ? "discard the unpushed commits shown above" : "may discard uncommitted work or lose data"} diff --git a/src/browser/utils/commands/sources.ts b/src/browser/utils/commands/sources.ts index da91614224..1a96d49eb4 100644 --- a/src/browser/utils/commands/sources.ts +++ b/src/browser/utils/commands/sources.ts @@ -153,7 +153,13 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi subtitle: workspaceDisplayName, section: section.workspaces, run: async () => { - const ok = confirm("Remove current workspace? This cannot be undone."); + const branchName = + selectedMeta?.name ?? + selected.namedWorkspacePath.split("/").pop() ?? + selected.namedWorkspacePath; + const ok = confirm( + `Remove current workspace? This will delete the worktree and local branch "${branchName}". This cannot be undone.` + ); if (ok) await p.onRemoveWorkspace(selected.workspaceId); }, }); @@ -305,7 +311,10 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi (m) => m.id === vals.workspaceId ); const workspaceName = meta ? `${meta.projectName}/${meta.name}` : vals.workspaceId; - const ok = confirm(`Remove workspace ${workspaceName}? This cannot be undone.`); + const branchName = meta?.name ?? workspaceName.split("/").pop() ?? workspaceName; + const ok = confirm( + `Remove workspace ${workspaceName}? This will delete the worktree and local branch "${branchName}". This cannot be undone.` + ); if (ok) { await p.onRemoveWorkspace(vals.workspaceId); } diff --git a/src/node/runtime/WorktreeRuntime.test.ts b/src/node/runtime/WorktreeRuntime.test.ts index b6d1623756..4225d12081 100644 --- a/src/node/runtime/WorktreeRuntime.test.ts +++ b/src/node/runtime/WorktreeRuntime.test.ts @@ -90,7 +90,7 @@ describe("WorktreeRuntime.resolvePath", () => { }); describe("WorktreeRuntime.deleteWorkspace", () => { - it("deletes agent branches when removing worktrees", async () => { + it("deletes non-agent branches when removing worktrees (force)", async () => { const rootDir = await fsPromises.realpath( await fsPromises.mkdtemp(path.join(os.tmpdir(), "worktree-runtime-delete-")) ); @@ -106,7 +106,7 @@ describe("WorktreeRuntime.deleteWorkspace", () => { const runtime = new WorktreeRuntime(srcBaseDir); const initLogger = createNullInitLogger(); - const branchName = "agent_explore_aaaaaaaaaa"; + const branchName = "feature_aaaaaaaaaa"; const createResult = await runtime.createWorkspace({ projectPath, branchName, @@ -116,16 +116,80 @@ describe("WorktreeRuntime.deleteWorkspace", () => { }); expect(createResult.success).toBe(true); if (!createResult.success) return; + if (!createResult.workspacePath) { + throw new Error("Expected workspacePath from createWorkspace"); + } + const workspacePath = createResult.workspacePath; - const before = execSync(`git branch --list "${branchName}"`, { + // Make the branch unmerged (so -d would fail); force delete should still delete it. + execSync("bash -lc 'echo \"change\" >> README.md'", { + cwd: workspacePath, + stdio: "ignore", + }); + execSync("git add README.md", { cwd: workspacePath, stdio: "ignore" }); + execSync('git commit -m "change"', { cwd: workspacePath, stdio: "ignore" }); + + const deleteResult = await runtime.deleteWorkspace(projectPath, branchName, true); + expect(deleteResult.success).toBe(true); + + const after = execSync(`git branch --list "${branchName}"`, { cwd: projectPath, stdio: ["ignore", "pipe", "ignore"], }) .toString() .trim(); - expect(before).toContain(branchName); + expect(after).toBe(""); + } finally { + await fsPromises.rm(rootDir, { recursive: true, force: true }); + } + }, 20_000); - const deleteResult = await runtime.deleteWorkspace(projectPath, branchName, true); + it("deletes merged branches when removing worktrees (safe delete)", async () => { + const rootDir = await fsPromises.realpath( + await fsPromises.mkdtemp(path.join(os.tmpdir(), "worktree-runtime-delete-")) + ); + + try { + const projectPath = path.join(rootDir, "repo"); + await fsPromises.mkdir(projectPath, { recursive: true }); + initGitRepo(projectPath); + + const srcBaseDir = path.join(rootDir, "src"); + await fsPromises.mkdir(srcBaseDir, { recursive: true }); + + const runtime = new WorktreeRuntime(srcBaseDir); + const initLogger = createNullInitLogger(); + + const branchName = "feature_merge_aaaaaaaaaa"; + const createResult = await runtime.createWorkspace({ + projectPath, + branchName, + trunkBranch: "main", + directoryName: branchName, + initLogger, + }); + expect(createResult.success).toBe(true); + if (!createResult.success) return; + if (!createResult.workspacePath) { + throw new Error("Expected workspacePath from createWorkspace"); + } + const workspacePath = createResult.workspacePath; + + // Commit on the workspace branch. + execSync("bash -lc 'echo \"merged-change\" >> README.md'", { + cwd: workspacePath, + stdio: "ignore", + }); + execSync("git add README.md", { cwd: workspacePath, stdio: "ignore" }); + execSync('git commit -m "merged-change"', { + cwd: workspacePath, + stdio: "ignore", + }); + + // Merge into main so `git branch -d` succeeds. + execSync(`git merge "${branchName}"`, { cwd: projectPath, stdio: "ignore" }); + + const deleteResult = await runtime.deleteWorkspace(projectPath, branchName, false); expect(deleteResult.success).toBe(true); const after = execSync(`git branch --list "${branchName}"`, { @@ -139,4 +203,63 @@ describe("WorktreeRuntime.deleteWorkspace", () => { await fsPromises.rm(rootDir, { recursive: true, force: true }); } }, 20_000); + + it("does not delete protected branches", async () => { + const rootDir = await fsPromises.realpath( + await fsPromises.mkdtemp(path.join(os.tmpdir(), "worktree-runtime-delete-")) + ); + + try { + const projectPath = path.join(rootDir, "repo"); + await fsPromises.mkdir(projectPath, { recursive: true }); + initGitRepo(projectPath); + + // Move the main worktree off main so we can add a separate worktree on main. + execSync("git checkout -b other", { cwd: projectPath, stdio: "ignore" }); + + const srcBaseDir = path.join(rootDir, "src"); + await fsPromises.mkdir(srcBaseDir, { recursive: true }); + + const runtime = new WorktreeRuntime(srcBaseDir); + const initLogger = createNullInitLogger(); + + const branchName = "main"; + const createResult = await runtime.createWorkspace({ + projectPath, + branchName, + trunkBranch: "main", + directoryName: branchName, + initLogger, + }); + expect(createResult.success).toBe(true); + if (!createResult.success) return; + if (!createResult.workspacePath) { + throw new Error("Expected workspacePath from createWorkspace"); + } + const workspacePath = createResult.workspacePath; + + const deleteResult = await runtime.deleteWorkspace(projectPath, branchName, true); + expect(deleteResult.success).toBe(true); + + // The worktree directory should be removed. + let worktreeExists = true; + try { + await fsPromises.access(workspacePath); + } catch { + worktreeExists = false; + } + expect(worktreeExists).toBe(false); + + // But protected branches (like main) should never be deleted. + const after = execSync(`git branch --list "${branchName}"`, { + cwd: projectPath, + stdio: ["ignore", "pipe", "ignore"], + }) + .toString() + .trim(); + expect(after).toBe("main"); + } finally { + await fsPromises.rm(rootDir, { recursive: true, force: true }); + } + }, 20_000); }); diff --git a/src/node/runtime/WorktreeRuntime.ts b/src/node/runtime/WorktreeRuntime.ts index a3185388e0..55c79e8b66 100644 --- a/src/node/runtime/WorktreeRuntime.ts +++ b/src/node/runtime/WorktreeRuntime.ts @@ -9,7 +9,7 @@ import type { WorkspaceForkResult, InitLogger, } from "./Runtime"; -import { listLocalBranches, cleanStaleLock } from "@/node/git"; +import { listLocalBranches, cleanStaleLock, getCurrentBranch } from "@/node/git"; import { checkInitHookExists, getMuxEnv } from "./initHook"; import { execAsync } from "@/node/utils/disposableExec"; import { getBashPath } from "@/node/utils/main/bashPath"; @@ -222,21 +222,112 @@ export class WorktreeRuntime extends LocalBaseRuntime { // In-place workspaces are identified by projectPath === workspaceName // These are direct workspace directories (e.g., CLI/benchmark sessions), not git worktrees const isInPlace = projectPath === workspaceName; - const shouldDeleteBranch = !isInPlace && workspaceName.startsWith("agent_"); + + // For git worktree workspaces, workspaceName is the branch name. + // Now that archiving exists, deleting a workspace should also delete its local branch by default. + const shouldDeleteBranch = !isInPlace; const tryDeleteBranch = async () => { if (!shouldDeleteBranch) return; + + const branchToDelete = workspaceName.trim(); + if (!branchToDelete) { + log.debug("Skipping git branch deletion: empty workspace name", { + projectPath, + workspaceName, + }); + return; + } + + let localBranches: string[]; + try { + localBranches = await listLocalBranches(projectPath); + } catch (error) { + log.debug("Failed to list local branches; skipping branch deletion", { + projectPath, + workspaceName: branchToDelete, + error: getErrorMessage(error), + }); + return; + } + + if (!localBranches.includes(branchToDelete)) { + log.debug("Skipping git branch deletion: branch does not exist locally", { + projectPath, + workspaceName: branchToDelete, + }); + return; + } + + // Never delete protected/trunk branches. + const protectedBranches = new Set(["main", "master", "trunk", "develop", "default"]); + + // If there's only one local branch, treat it as protected (likely trunk). + if (localBranches.length === 1) { + protectedBranches.add(localBranches[0]); + } + + const currentBranch = await getCurrentBranch(projectPath); + if (currentBranch) { + protectedBranches.add(currentBranch); + } + + // If origin/HEAD points at a local branch, also treat it as protected. + try { + using originHeadProc = execAsync( + `git -C "${projectPath}" symbolic-ref refs/remotes/origin/HEAD` + ); + const { stdout } = await originHeadProc.result; + const ref = stdout.trim(); + const prefix = "refs/remotes/origin/"; + if (ref.startsWith(prefix)) { + protectedBranches.add(ref.slice(prefix.length)); + } + } catch { + // No origin/HEAD (or not a git repo) - ignore + } + + if (protectedBranches.has(branchToDelete)) { + log.debug("Skipping git branch deletion: protected branch", { + projectPath, + workspaceName: branchToDelete, + }); + return; + } + + // Extra safety: don't delete a branch still checked out by any worktree. + try { + using worktreeProc = execAsync(`git -C "${projectPath}" worktree list --porcelain`); + const { stdout } = await worktreeProc.result; + const needle = `branch refs/heads/${branchToDelete}`; + const isCheckedOut = stdout.split("\n").some((line) => line.trim() === needle); + if (isCheckedOut) { + log.debug("Skipping git branch deletion: branch still checked out by a worktree", { + projectPath, + workspaceName: branchToDelete, + }); + return; + } + } catch (error) { + // If the worktree list fails, proceed anyway - git itself will refuse to delete a checked-out branch. + log.debug("Failed to check worktree list before branch deletion; proceeding", { + projectPath, + workspaceName: branchToDelete, + error: getErrorMessage(error), + }); + } + const deleteFlag = force ? "-D" : "-d"; try { using deleteProc = execAsync( - `git -C "${projectPath}" branch ${deleteFlag} "${workspaceName}"` + `git -C "${projectPath}" branch ${deleteFlag} "${branchToDelete}"` ); await deleteProc.result; } catch (error) { // Best-effort: workspace deletion should not fail just because branch cleanup failed. log.debug("Failed to delete git branch after removing worktree", { projectPath, - workspaceName, + workspaceName: branchToDelete, error: getErrorMessage(error), }); } @@ -260,7 +351,7 @@ export class WorktreeRuntime extends LocalBaseRuntime { } } - // Best-effort: if this looks like an agent workspace, also delete the branch. + // Best-effort: also delete the local branch. await tryDeleteBranch(); return { success: true, deletedPath }; } @@ -282,7 +373,7 @@ export class WorktreeRuntime extends LocalBaseRuntime { ); await proc.result; - // Best-effort: if this looks like an agent workspace, also delete the branch. + // Best-effort: also delete the local branch. await tryDeleteBranch(); return { success: true, deletedPath }; } catch (error) { @@ -327,7 +418,7 @@ export class WorktreeRuntime extends LocalBaseRuntime { }); await rmProc.result; - // Best-effort: if this looks like an agent workspace, also delete the branch. + // Best-effort: also delete the local branch. await tryDeleteBranch(); return { success: true, deletedPath }; } catch (rmError) { diff --git a/tests/ipc/removeWorkspace.test.ts b/tests/ipc/removeWorkspace.test.ts index b3f7e42066..9a3645330e 100644 --- a/tests/ipc/removeWorkspace.test.ts +++ b/tests/ipc/removeWorkspace.test.ts @@ -172,6 +172,13 @@ describeIntegration("Workspace deletion integration tests", () => { } expect(deleteResult.success).toBe(true); + // Local worktree runtime should also delete the local branch. + if (type === "local") { + using branchProc = execAsync(`git -C "${tempGitRepo}" branch --list "${branchName}"`); + const { stdout } = await branchProc.result; + expect(stdout.trim()).toBe(""); + } + // Verify workspace is no longer in config const config = env.config.loadConfigOrDefault(); const project = config.projects.get(tempGitRepo);