Skip to content

Commit a79e171

Browse files
centdixclaude
andcommitted
fix: harden backend worktree removal
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent cabbeac commit a79e171

3 files changed

Lines changed: 145 additions & 9 deletions

File tree

backend/src/__tests__/git-adapter.test.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
BunGitGateway,
77
parseGitWorktreePorcelain,
88
readGitWorktreeStatus,
9+
removeGitWorktree,
910
resolveWorktreeGitDir,
1011
resolveWorktreeRoot,
1112
} from "../adapters/git";
@@ -259,3 +260,71 @@ describe("BunGitGateway", () => {
259260
expect(dirtyStatus.currentCommit).toBe(cleanStatus.currentCommit);
260261
});
261262
});
263+
264+
describe("removeGitWorktree", () => {
265+
it("cleans up the leftover directory when git already unregistered the worktree", () => {
266+
const removedPaths: string[] = [];
267+
268+
removeGitWorktree(
269+
{
270+
repoRoot: "/repo",
271+
worktreePath: "/repo/__worktrees/feature-a",
272+
force: true,
273+
},
274+
{
275+
tryRunGit: () => ({
276+
ok: false,
277+
stderr: "error: failed to delete '/repo/__worktrees/feature-a': Directory not empty",
278+
}),
279+
listWorktrees: () => [
280+
{
281+
path: "/repo",
282+
head: "abc123",
283+
branch: "main",
284+
detached: false,
285+
bare: false,
286+
},
287+
],
288+
removeDirectory: (path) => {
289+
removedPaths.push(path);
290+
},
291+
},
292+
);
293+
294+
expect(removedPaths).toEqual(["/repo/__worktrees/feature-a"]);
295+
});
296+
297+
it("surfaces the git error when the worktree is still registered", () => {
298+
expect(() => {
299+
removeGitWorktree(
300+
{
301+
repoRoot: "/repo",
302+
worktreePath: "/repo/__worktrees/feature-a",
303+
force: true,
304+
},
305+
{
306+
tryRunGit: () => ({
307+
ok: false,
308+
stderr: "error: failed to delete '/repo/__worktrees/feature-a': Directory not empty",
309+
}),
310+
listWorktrees: () => [
311+
{
312+
path: "/repo",
313+
head: "abc123",
314+
branch: "main",
315+
detached: false,
316+
bare: false,
317+
},
318+
{
319+
path: "/repo/__worktrees/feature-a",
320+
head: "def456",
321+
branch: "feature-a",
322+
detached: false,
323+
bare: false,
324+
},
325+
],
326+
},
327+
);
328+
}).toThrow("git worktree remove --force /repo/__worktrees/feature-a failed");
329+
});
330+
});

backend/src/adapters/git.ts

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { rmSync } from "node:fs";
12
import { resolve } from "node:path";
23

34
export interface GitWorktreeEntry {
@@ -33,6 +34,16 @@ export interface GitWorktreeStatus {
3334
currentCommit: string | null;
3435
}
3536

37+
export type TryGitCommandResult =
38+
| { ok: true; stdout: string }
39+
| { ok: false; stderr: string };
40+
41+
export interface RemoveGitWorktreeDeps {
42+
tryRunGit?: (args: string[], cwd: string) => TryGitCommandResult;
43+
listWorktrees?: (cwd: string) => GitWorktreeEntry[];
44+
removeDirectory?: (path: string) => void;
45+
}
46+
3647
export interface GitGateway {
3748
resolveWorktreeRoot(cwd: string): string;
3849
resolveWorktreeGitDir(cwd: string): string;
@@ -60,7 +71,7 @@ function runGit(args: string[], cwd: string): string {
6071
return new TextDecoder().decode(result.stdout).trim();
6172
}
6273

63-
function tryRunGit(args: string[], cwd: string): { ok: true; stdout: string } | { ok: false; stderr: string } {
74+
function tryRunGit(args: string[], cwd: string): TryGitCommandResult {
6475
const result = Bun.spawnSync(["git", ...args], {
6576
cwd,
6677
stdout: "pipe",
@@ -84,6 +95,18 @@ function errorMessage(error: unknown): string {
8495
return error instanceof Error ? error.message : String(error);
8596
}
8697

98+
function isRegisteredWorktree(entries: GitWorktreeEntry[], worktreePath: string): boolean {
99+
const resolvedPath = resolve(worktreePath);
100+
return entries.some((entry) => resolve(entry.path) === resolvedPath);
101+
}
102+
103+
function removeDirectory(path: string): void {
104+
rmSync(path, {
105+
recursive: true,
106+
force: true,
107+
});
108+
}
109+
87110
function currentCheckoutRef(cwd: string): { ref: string; branch: string | null } {
88111
const symbolicRef = tryRunGit(["symbolic-ref", "--quiet", "--short", "HEAD"], cwd);
89112
if (symbolicRef.ok && symbolicRef.stdout.length > 0) {
@@ -180,6 +203,32 @@ export function readGitWorktreeStatus(cwd: string): GitWorktreeStatus {
180203
};
181204
}
182205

206+
export function removeGitWorktree(
207+
opts: RemoveGitWorktreeOptions,
208+
deps: RemoveGitWorktreeDeps = {},
209+
): void {
210+
const args = ["worktree", "remove"];
211+
if (opts.force) args.push("--force");
212+
args.push(opts.worktreePath);
213+
214+
const result = (deps.tryRunGit ?? tryRunGit)(args, opts.repoRoot);
215+
if (result.ok) {
216+
return;
217+
}
218+
219+
const failure = `git ${args.join(" ")} failed: ${result.stderr || "exit 1"}`;
220+
const remainingWorktrees = (deps.listWorktrees ?? listGitWorktrees)(opts.repoRoot);
221+
if (isRegisteredWorktree(remainingWorktrees, opts.worktreePath)) {
222+
throw new Error(failure);
223+
}
224+
225+
try {
226+
(deps.removeDirectory ?? removeDirectory)(opts.worktreePath);
227+
} catch (error) {
228+
throw new Error(`${failure}; cleanup failed: ${errorMessage(error)}`);
229+
}
230+
}
231+
183232
export class BunGitGateway implements GitGateway {
184233
resolveWorktreeRoot(cwd: string): string {
185234
return resolveWorktreeRoot(cwd);
@@ -204,10 +253,7 @@ export class BunGitGateway implements GitGateway {
204253
}
205254

206255
removeWorktree(opts: RemoveGitWorktreeOptions): void {
207-
const args = ["worktree", "remove"];
208-
if (opts.force) args.push("--force");
209-
args.push(opts.worktreePath);
210-
runGit(args, opts.repoRoot);
256+
removeGitWorktree(opts);
211257
}
212258

213259
deleteBranch(repoRoot: string, branch: string, force = false): void {

backend/src/server.ts

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ const reconciliationService = new ReconciliationService({
5757
portProbe,
5858
runtime: projectRuntime,
5959
});
60+
const removingBranches = new Set<string>();
6061
const lifecycleService = new LifecycleService({
6162
projectRoot: PROJECT_DIR,
6263
controlBaseUrl: `http://127.0.0.1:${PORT}`,
@@ -184,10 +185,27 @@ function catching(label: string, fn: () => Promise<Response>): Promise<Response>
184185
});
185186
}
186187

188+
function ensureBranchNotRemoving(branch: string): void {
189+
if (removingBranches.has(branch)) {
190+
throw new LifecycleError(`Worktree is being removed: ${branch}`, 409);
191+
}
192+
}
193+
194+
async function withRemovingBranch<T>(branch: string, fn: () => Promise<T>): Promise<T> {
195+
ensureBranchNotRemoving(branch);
196+
removingBranches.add(branch);
197+
try {
198+
return await fn();
199+
} finally {
200+
removingBranches.delete(branch);
201+
}
202+
}
203+
187204
async function resolveTerminalWorktree(branch: string): Promise<{
188205
worktreeId: string;
189206
attachTarget: TerminalAttachTarget;
190207
}> {
208+
ensureBranchNotRemoving(branch);
191209
await reconciliationService.reconcile(PROJECT_DIR);
192210
const state = projectRuntime.getWorktreeByBranch(branch);
193211
if (!state) {
@@ -348,13 +366,16 @@ async function apiCreateWorktree(req: Request): Promise<Response> {
348366
}
349367

350368
async function apiDeleteWorktree(name: string): Promise<Response> {
351-
log.info(`[worktree:rm] name=${name}`);
352-
await lifecycleService.removeWorktree(name);
353-
log.debug(`[worktree:rm] done name=${name}`);
354-
return jsonResponse({ ok: true });
369+
return withRemovingBranch(name, async () => {
370+
log.info(`[worktree:rm] name=${name}`);
371+
await lifecycleService.removeWorktree(name);
372+
log.debug(`[worktree:rm] done name=${name}`);
373+
return jsonResponse({ ok: true });
374+
});
355375
}
356376

357377
async function apiOpenWorktree(name: string): Promise<Response> {
378+
ensureBranchNotRemoving(name);
358379
log.info(`[worktree:open] name=${name}`);
359380
const result = await lifecycleService.openWorktree(name);
360381
log.debug(`[worktree:open] done name=${name} worktreeId=${result.worktreeId}`);

0 commit comments

Comments
 (0)