authorgravatar for me@t3.ggTheo Browne <me@t3.gg> 2026-08-24 04:11:14-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-08-24 04:11:14-07:00
loge6a109b9f74552eb735968986845e500b98c3c73
treeb8af28ac2c777c0a6572381ebd335ef286175f2c
parent01fc7d228dd6794ffb35b57752d0218559de5e89
signature Signed by PGP key B5690EEEBB952194

fix(server): thread delete no longer fails on already-removed worktrees (#8076)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

2 files changed, 111 insertions(+), 7 deletions(-)

apps/server/src/vcs/GitVcsDriverCore.test.ts+68-4
...@@ -739,13 +739,13 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {...@@ -739,13 +739,13 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {
739 Effect.gen(function* () {739 Effect.gen(function* () {
740 const cwd = yield* makeTmpDir();740 const cwd = yield* makeTmpDir();
741 const pathService = yield* Path.Path;741 const pathService = yield* Path.Path;
742 const missingWorktree = pathService.join(cwd, "missing-worktree");742 const fileSystem = yield* FileSystem.FileSystem;
743 const notAWorktree = pathService.join(cwd, "not-a-worktree");
744 yield* fileSystem.makeDirectory(notAWorktree);
743 const driver = yield* GitVcsDriver.GitVcsDriver;745 const driver = yield* GitVcsDriver.GitVcsDriver;
744 yield* driver.initRepo({ cwd });746 yield* driver.initRepo({ cwd });
745747
746 const error = yield* driver748 const error = yield* driver.removeWorktree({ cwd, path: notAWorktree }).pipe(Effect.flip);
747 .removeWorktree({ cwd, path: missingWorktree })
748 .pipe(Effect.flip);
749749
750 assert.deepInclude(error, {750 assert.deepInclude(error, {
751 _tag: "GitCommandError",751 _tag: "GitCommandError",
...@@ -755,9 +755,22 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {...@@ -755,9 +755,22 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {
755 cwd,755 cwd,
756 });756 });
757 assert.notProperty(error, "cause");757 assert.notProperty(error, "cause");
758 assert.notProperty(error, "stderr");
758 assert.notInclude(error.detail, "Git command failed in");759 assert.notInclude(error.detail, "Git command failed in");
759 }),760 }),
760 );761 );
762
763 it.effect("treats removing an already-gone worktree as a no-op", () =>
764 Effect.gen(function* () {
765 const cwd = yield* makeTmpDir();
766 const pathService = yield* Path.Path;
767 const missingWorktree = pathService.join(cwd, "missing-worktree");
768 const driver = yield* GitVcsDriver.GitVcsDriver;
769 yield* driver.initRepo({ cwd });
770
771 yield* driver.removeWorktree({ cwd, path: missingWorktree });
772 }),
773 );
761 });774 });
762775
763 describe("review diff previews", () => {776 describe("review diff previews", () => {
...@@ -1487,6 +1500,57 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {...@@ -1487,6 +1500,57 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {
1487 assert.equal(yield* fileSystem.exists(worktreePath), false);1500 assert.equal(yield* fileSystem.exists(worktreePath), false);
1488 }),1501 }),
1489 );1502 );
1503
1504 it.effect("removes the same worktree path twice without failing", () =>
1505 Effect.gen(function* () {
1506 const cwd = yield* makeTmpDir();
1507 const { initialBranch } = yield* initRepoWithCommit(cwd);
1508 const pathService = yield* Path.Path;
1509 const worktreePath = pathService.join(yield* makeTmpDir("git-worktrees-"), "shared");
1510 const driver = yield* GitVcsDriver.GitVcsDriver;
1511
1512 yield* driver.createWorktree({
1513 cwd,
1514 path: worktreePath,
1515 refName: initialBranch,
1516 newRefName: "feature/shared",
1517 });
1518
1519 // Two threads can record the same worktree path; the second delete
1520 // must be a no-op instead of exit 128.
1521 yield* driver.removeWorktree({ cwd, path: worktreePath });
1522 yield* driver.removeWorktree({ cwd, path: worktreePath });
1523 }),
1524 );
1525
1526 it.effect("prunes stale registrations when removing an already-gone worktree", () =>
1527 Effect.gen(function* () {
1528 const cwd = yield* makeTmpDir();
1529 const { initialBranch } = yield* initRepoWithCommit(cwd);
1530 const pathService = yield* Path.Path;
1531 const fileSystem = yield* FileSystem.FileSystem;
1532 const worktreesRoot = yield* makeTmpDir("git-worktrees-");
1533 const stalePath = pathService.join(worktreesRoot, "stale");
1534 const driver = yield* GitVcsDriver.GitVcsDriver;
1535
1536 yield* driver.createWorktree({
1537 cwd,
1538 path: stalePath,
1539 refName: initialBranch,
1540 newRefName: "feature/stale",
1541 });
1542 // Delete the directory behind git's back so the registration goes stale.
1543 yield* fileSystem.remove(stalePath, { recursive: true });
1544
1545 yield* driver.removeWorktree({
1546 cwd,
1547 path: pathService.join(worktreesRoot, "never-registered"),
1548 });
1549
1550 const registered = yield* git(cwd, ["worktree", "list", "--porcelain"]);
1551 assert.notInclude(registered, "stale");
1552 }),
1553 );
1490 });1554 });
14911555
1492 describe("remote operations", () => {1556 describe("remote operations", () => {
apps/server/src/vcs/GitVcsDriverCore.ts+43-3
...@@ -432,6 +432,17 @@ function isUnbornHeadStderr(stderr: string): boolean {...@@ -432,6 +432,17 @@ function isUnbornHeadStderr(stderr: string): boolean {
432 );432 );
433}433}
434434
435// Matches `git worktree remove` on a path git no longer tracks: "is not a
436// working tree" when the registration is gone, "cannot remove working tree"
437// when older gits fail validation on a registered-but-deleted directory.
438function isMissingWorktreeStderr(stderr: string): boolean {
439 const normalized = stderr.toLowerCase();
440 return (
441 normalized.includes("is not a working tree") ||
442 normalized.includes("cannot remove working tree")
443 );
444}
445
435interface Trace2Monitor {446interface Trace2Monitor {
436 readonly env: NodeJS.ProcessEnv;447 readonly env: NodeJS.ProcessEnv;
437 readonly flush: Effect.Effect<void, never>;448 readonly flush: Effect.Effect<void, never>;
...@@ -3011,9 +3022,38 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*...@@ -3011,9 +3022,38 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
3011 args.push("--force");3022 args.push("--force");
3012 }3023 }
3013 args.push(input.path);3024 args.push(input.path);
3014 yield* executeGit("GitVcsDriver.removeWorktree", input.cwd, args, {3025 const result = yield* executeGitWithStableDiagnostics(
3015 timeoutMs: 15_000,3026 "GitVcsDriver.removeWorktree",
3016 fallbackErrorDetail: "git worktree remove failed",3027 input.cwd,
3028 args,
3029 { timeoutMs: 15_000, allowNonZeroExit: true },
3030 );
3031 if (result.exitCode === 0) {
3032 return;
3033 }
3034 // Threads can share a worktree path, and worktrees get removed or pruned
3035 // outside the app, so a worktree that is already gone is a no-op rather
3036 // than an error. Prune so no stale registration lingers to block a later
3037 // `worktree add` at the same path.
3038 const alreadyGone =
3039 isMissingWorktreeStderr(result.stderr) &&
3040 !(yield* fileSystem.exists(input.path).pipe(Effect.orElseSucceed(() => false)));
3041 if (alreadyGone) {
3042 yield* pruneWorktrees({ cwd: input.cwd });
3043 return;
3044 }
3045 // Raw stderr stays out of both the wire error and the log (it can carry
3046 // secrets); log bounded diagnostics so a genuine failure is visible
3047 // server-side.
3048 yield* Effect.logWarning(
3049 `GitVcsDriver.removeWorktree: git worktree remove exited with code ${result.exitCode} for ${input.path} (stderr length ${result.stderr.length}).`,
3050 );
3051 return yield* new GitCommandError({
3052 ...gitCommandContext({ operation: "GitVcsDriver.removeWorktree", cwd: input.cwd, args }),
3053 detail: "git worktree remove failed",
3054 ...(result.exitCode === null ? {} : { exitCode: result.exitCode }),
3055 stdoutLength: result.stdout.length,
3056 stderrLength: result.stderr.length,
3017 });3057 });
3018 });3058 });
30193059