diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index 33279fe00690..cff5d6d47f6b 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -739,13 +739,13 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { Effect.gen(function* () { const cwd = yield* makeTmpDir(); const pathService = yield* Path.Path; - const missingWorktree = pathService.join(cwd, "missing-worktree"); + const fileSystem = yield* FileSystem.FileSystem; + const notAWorktree = pathService.join(cwd, "not-a-worktree"); + yield* fileSystem.makeDirectory(notAWorktree); const driver = yield* GitVcsDriver.GitVcsDriver; yield* driver.initRepo({ cwd }); - const error = yield* driver - .removeWorktree({ cwd, path: missingWorktree }) - .pipe(Effect.flip); + const error = yield* driver.removeWorktree({ cwd, path: notAWorktree }).pipe(Effect.flip); assert.deepInclude(error, { _tag: "GitCommandError", @@ -755,9 +755,22 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { cwd, }); assert.notProperty(error, "cause"); + assert.notProperty(error, "stderr"); assert.notInclude(error.detail, "Git command failed in"); }), ); + + it.effect("treats removing an already-gone worktree as a no-op", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const pathService = yield* Path.Path; + const missingWorktree = pathService.join(cwd, "missing-worktree"); + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* driver.initRepo({ cwd }); + + yield* driver.removeWorktree({ cwd, path: missingWorktree }); + }), + ); }); describe("review diff previews", () => { @@ -1487,6 +1500,57 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { assert.equal(yield* fileSystem.exists(worktreePath), false); }), ); + + it.effect("removes the same worktree path twice without failing", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const { initialBranch } = yield* initRepoWithCommit(cwd); + const pathService = yield* Path.Path; + const worktreePath = pathService.join(yield* makeTmpDir("git-worktrees-"), "shared"); + const driver = yield* GitVcsDriver.GitVcsDriver; + + yield* driver.createWorktree({ + cwd, + path: worktreePath, + refName: initialBranch, + newRefName: "feature/shared", + }); + + // Two threads can record the same worktree path; the second delete + // must be a no-op instead of exit 128. + yield* driver.removeWorktree({ cwd, path: worktreePath }); + yield* driver.removeWorktree({ cwd, path: worktreePath }); + }), + ); + + it.effect("prunes stale registrations when removing an already-gone worktree", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const { initialBranch } = yield* initRepoWithCommit(cwd); + const pathService = yield* Path.Path; + const fileSystem = yield* FileSystem.FileSystem; + const worktreesRoot = yield* makeTmpDir("git-worktrees-"); + const stalePath = pathService.join(worktreesRoot, "stale"); + const driver = yield* GitVcsDriver.GitVcsDriver; + + yield* driver.createWorktree({ + cwd, + path: stalePath, + refName: initialBranch, + newRefName: "feature/stale", + }); + // Delete the directory behind git's back so the registration goes stale. + yield* fileSystem.remove(stalePath, { recursive: true }); + + yield* driver.removeWorktree({ + cwd, + path: pathService.join(worktreesRoot, "never-registered"), + }); + + const registered = yield* git(cwd, ["worktree", "list", "--porcelain"]); + assert.notInclude(registered, "stale"); + }), + ); }); describe("remote operations", () => { diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 77f8de11b81f..19e58fea63df 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -432,6 +432,17 @@ function isUnbornHeadStderr(stderr: string): boolean { ); } +// Matches `git worktree remove` on a path git no longer tracks: "is not a +// working tree" when the registration is gone, "cannot remove working tree" +// when older gits fail validation on a registered-but-deleted directory. +function isMissingWorktreeStderr(stderr: string): boolean { + const normalized = stderr.toLowerCase(); + return ( + normalized.includes("is not a working tree") || + normalized.includes("cannot remove working tree") + ); +} + interface Trace2Monitor { readonly env: NodeJS.ProcessEnv; readonly flush: Effect.Effect; @@ -3011,9 +3022,38 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* args.push("--force"); } args.push(input.path); - yield* executeGit("GitVcsDriver.removeWorktree", input.cwd, args, { - timeoutMs: 15_000, - fallbackErrorDetail: "git worktree remove failed", + const result = yield* executeGitWithStableDiagnostics( + "GitVcsDriver.removeWorktree", + input.cwd, + args, + { timeoutMs: 15_000, allowNonZeroExit: true }, + ); + if (result.exitCode === 0) { + return; + } + // Threads can share a worktree path, and worktrees get removed or pruned + // outside the app, so a worktree that is already gone is a no-op rather + // than an error. Prune so no stale registration lingers to block a later + // `worktree add` at the same path. + const alreadyGone = + isMissingWorktreeStderr(result.stderr) && + !(yield* fileSystem.exists(input.path).pipe(Effect.orElseSucceed(() => false))); + if (alreadyGone) { + yield* pruneWorktrees({ cwd: input.cwd }); + return; + } + // Raw stderr stays out of both the wire error and the log (it can carry + // secrets); log bounded diagnostics so a genuine failure is visible + // server-side. + yield* Effect.logWarning( + `GitVcsDriver.removeWorktree: git worktree remove exited with code ${result.exitCode} for ${input.path} (stderr length ${result.stderr.length}).`, + ); + return yield* new GitCommandError({ + ...gitCommandContext({ operation: "GitVcsDriver.removeWorktree", cwd: input.cwd, args }), + detail: "git worktree remove failed", + ...(result.exitCode === null ? {} : { exitCode: result.exitCode }), + stdoutLength: result.stdout.length, + stderrLength: result.stderr.length, }); });