From 55961968af7c7ef5c2cae6cde9754fa917ce4cc1 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Sun, 4 Oct 2026 20:18:20 -0400 Subject: [PATCH 1/4] fix(server): removing a worktree on Windows no longer leaves junctions behind Git for Windows does not descend into NTFS junctions such as pnpm's node_modules links. `git worktree remove` exits 0 but leaves the junctions and their parent folders, so a resumed thread finds a stub instead of recreating its checkout. removeWorktree now unlinks those links without following them and removes the folders that leaves empty. Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/server/src/vcs/GitVcsDriverCore.test.ts | 38 +++++++++++++++++ apps/server/src/vcs/GitVcsDriverCore.ts | 45 ++++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index 17ae6e309fee..9002da9594d1 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -2883,6 +2883,44 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { }), ); + it.effect("unlinks ignored directory links without deleting their targets", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const { initialBranch } = yield* initRepoWithCommit(cwd); + yield* writeTextFile(cwd, ".gitignore", "linked\n"); + yield* git(cwd, ["add", ".gitignore"]); + yield* git(cwd, ["commit", "-m", "ignore links"]); + const fileSystem = yield* FileSystem.FileSystem; + const pathService = yield* Path.Path; + const outside = yield* makeTmpDir("git-worktree-link-target-"); + yield* writeTextFile(outside, "keep.txt", "keep\n"); + const worktreePath = pathService.join(yield* makeTmpDir("git-worktrees-"), "linked"); + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* driver.createWorktree({ + cwd, + path: worktreePath, + refName: initialBranch, + newRefName: "feature/linked", + }); + // A junction on Windows, which Git for Windows leaves behind; a symlink elsewhere. + NodeFS.symlinkSync(outside, pathService.join(worktreePath, "linked"), "junction"); + NodeFS.mkdirSync(pathService.join(worktreePath, "linked-parent")); + NodeFS.symlinkSync( + outside, + pathService.join(worktreePath, "linked-parent", "linked"), + "junction", + ); + + yield* driver.removeWorktree({ cwd, path: worktreePath }); + + assert.equal(yield* fileSystem.exists(worktreePath), false); + assert.equal( + yield* fileSystem.readFileString(pathService.join(outside, "keep.txt")), + "keep\n", + ); + }), + ); + it.effect("allows worktree removal to run longer than the default command timeout", () => Effect.gen(function* () { const delegate = yield* ChildProcessSpawner.ChildProcessSpawner; diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 935fb3d24a72..ad5a8d639ddb 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -3646,6 +3646,28 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* input.branch, ]); + /** + * Unlinks links under `target` without following them and removes the + * directories that leaves empty. Returns false when anything else remains. + */ + const removeLeftoverLinks = ( + target: string, + ): Effect.Effect => + Effect.gen(function* () { + if (Option.isSome(yield* fileSystem.readLink(target).pipe(Effect.option))) { + yield* fileSystem.remove(target); + return true; + } + if ((yield* fileSystem.stat(target)).type !== "Directory") return false; + let empty = true; + for (const name of yield* fileSystem.readDirectory(target)) { + if (!(yield* removeLeftoverLinks(path.join(target, name)))) empty = false; + } + // Node's rm refuses a directory without `recursive`, even an empty one. + if (empty) yield* fileSystem.remove(target, { recursive: true }); + return empty; + }); + const removeWorktree: GitVcsDriver.GitVcsDriver["Service"]["removeWorktree"] = Effect.fn( "removeWorktree", )(function* (input) { @@ -3667,6 +3689,29 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* }, ); if (result.exitCode === 0) { + // Git for Windows never descends into NTFS junctions, such as pnpm's + // node_modules links. It reports success but leaves them and their parent + // directories behind, so a resumed thread would find a stub instead of + // recreating its checkout. Git has already unregistered the worktree, so + // anything left here is logged rather than returned: a retry could never + // succeed. + const leftover = path.resolve(input.cwd, input.path); + if (yield* fileSystem.exists(leftover).pipe(Effect.orElseSucceed(() => false))) { + const removed = yield* removeLeftoverLinks(leftover).pipe( + Effect.catch((error) => + Effect.logWarning("GitVcsDriver.removeWorktree: failed to delete leftover links", { + path: leftover, + error, + }).pipe(Effect.as(true)), + ), + ); + if (!removed) { + yield* Effect.logWarning( + "GitVcsDriver.removeWorktree: kept files written after git removed the worktree", + { path: leftover }, + ); + } + } return; } // Threads can share a worktree path, and worktrees get removed or pruned From 15b753d0dee500756d4f89f94b9e6167ed4b7d18 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Sun, 4 Oct 2026 21:10:51 -0400 Subject: [PATCH 2/4] fix(server): only remove leftover worktree folders while they are empty Listing a folder and then removing it recursively deletes anything written in between. rmdir refuses a folder that is no longer empty. Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/server/src/vcs/GitVcsDriverCore.ts | 6 +++--- apps/server/src/vcs/removeEmptyDirectory.ts | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 apps/server/src/vcs/removeEmptyDirectory.ts diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index ad5a8d639ddb..65922f0913ec 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -35,6 +35,7 @@ import { parseT3ProjectFile } from "@t3tools/shared/t3ProjectFile"; import { resolveProjectFileBackedSetting } from "@t3tools/shared/projectSettings"; import { gitCommandDuration, gitCommandsTotal, withMetrics } from "../observability/Metrics.ts"; import * as GitVcsDriver from "./GitVcsDriver.ts"; +import { removeEmptyDirectory } from "./removeEmptyDirectory.ts"; import { parseRemoteNames, parseRemoteNamesInGitOrder, @@ -3663,9 +3664,8 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* for (const name of yield* fileSystem.readDirectory(target)) { if (!(yield* removeLeftoverLinks(path.join(target, name)))) empty = false; } - // Node's rm refuses a directory without `recursive`, even an empty one. - if (empty) yield* fileSystem.remove(target, { recursive: true }); - return empty; + // Only an empty directory goes, so files written since the listing stay. + return empty && (yield* removeEmptyDirectory(target)); }); const removeWorktree: GitVcsDriver.GitVcsDriver["Service"]["removeWorktree"] = Effect.fn( diff --git a/apps/server/src/vcs/removeEmptyDirectory.ts b/apps/server/src/vcs/removeEmptyDirectory.ts new file mode 100644 index 000000000000..710b1afcf116 --- /dev/null +++ b/apps/server/src/vcs/removeEmptyDirectory.ts @@ -0,0 +1,14 @@ +// @effect-diagnostics nodeBuiltinImport:off - Effect's FileSystem has no rmdir. +import * as NodeFSP from "node:fs/promises"; + +import * as Effect from "effect/Effect"; + +/** + * Removes `path` only while it is an empty directory. Succeeds with false when + * anything else is there, including files written after the caller looked. + */ +export const removeEmptyDirectory = (path: string) => + Effect.tryPromise(() => NodeFSP.rmdir(path)).pipe( + Effect.as(true), + Effect.orElseSucceed(() => false), + ); From 77ff6218f400dba3e943d61413bbb3456206ec41 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Mon, 5 Oct 2026 01:23:40 -0400 Subject: [PATCH 3/4] fix(server): keep unexpected rmdir errors in worktree removal warnings removeEmptyDirectory treated every rmdir failure as a folder that still had files, so a permission error was logged as "kept files". Only ENOTEMPTY means that now; other errors reach the existing warning with their details. Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/server/src/vcs/GitVcsDriverCore.ts | 3 ++- apps/server/src/vcs/removeEmptyDirectory.ts | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 65922f0913ec..2de147ce8fd5 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -1,4 +1,5 @@ import * as Cache from "effect/Cache"; +import * as Cause from "effect/Cause"; import * as Data from "effect/Data"; import * as Crypto from "effect/Crypto"; import * as DateTime from "effect/DateTime"; @@ -3653,7 +3654,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* */ const removeLeftoverLinks = ( target: string, - ): Effect.Effect => + ): Effect.Effect => Effect.gen(function* () { if (Option.isSome(yield* fileSystem.readLink(target).pipe(Effect.option))) { yield* fileSystem.remove(target); diff --git a/apps/server/src/vcs/removeEmptyDirectory.ts b/apps/server/src/vcs/removeEmptyDirectory.ts index 710b1afcf116..2ce80dfe588a 100644 --- a/apps/server/src/vcs/removeEmptyDirectory.ts +++ b/apps/server/src/vcs/removeEmptyDirectory.ts @@ -6,9 +6,13 @@ import * as Effect from "effect/Effect"; /** * Removes `path` only while it is an empty directory. Succeeds with false when * anything else is there, including files written after the caller looked. + * Other failures, such as a permission error, stay errors. */ export const removeEmptyDirectory = (path: string) => Effect.tryPromise(() => NodeFSP.rmdir(path)).pipe( Effect.as(true), - Effect.orElseSucceed(() => false), + Effect.catchIf( + (error) => (error.cause as NodeJS.ErrnoException | undefined)?.code === "ENOTEMPTY", + () => Effect.succeed(false), + ), ); From 344b395bb8ac4e5351f57458caa5e6d13efba129 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Mon, 5 Oct 2026 14:05:08 -0400 Subject: [PATCH 4/4] fix(server): clean up the same worktree path Git removed Git also accepts a worktree's short name and matches it against registered worktrees, while the leftover cleanup resolved the argument against cwd. With a worktree named `short` elsewhere and an unrelated `short` folder in the repository, Git removed the worktree and cleanup then unlinked the unrelated folder's links. removeWorktree now resolves the path once and gives Git and the cleanup the same absolute path. Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/server/src/vcs/GitVcsDriverCore.test.ts | 27 ++++++++++++++++++++ apps/server/src/vcs/GitVcsDriverCore.ts | 15 ++++++----- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index 9002da9594d1..5a5ed60f371b 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -2921,6 +2921,33 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { }), ); + it.effect("leaves a folder alone that only shares the worktree's short name", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const { initialBranch } = yield* initRepoWithCommit(cwd); + const pathService = yield* Path.Path; + const fileSystem = yield* FileSystem.FileSystem; + const driver = yield* GitVcsDriver.GitVcsDriver; + const worktreePath = pathService.join(yield* makeTmpDir("git-worktrees-"), "short"); + yield* driver.createWorktree({ + cwd, + path: worktreePath, + refName: initialBranch, + newRefName: "feature/short", + }); + // Unrelated to the worktree, but resolving "short" against cwd lands here. + const outside = yield* makeTmpDir("git-worktree-link-target-"); + const unrelated = pathService.join(cwd, "short"); + NodeFS.mkdirSync(unrelated); + NodeFS.symlinkSync(outside, pathService.join(unrelated, "linked"), "junction"); + + yield* Effect.result(driver.removeWorktree({ cwd, path: "short" })); + + assert.isTrue(NodeFS.lstatSync(pathService.join(unrelated, "linked")).isSymbolicLink()); + assert.equal(yield* fileSystem.exists(worktreePath), true); + }), + ); + it.effect("allows worktree removal to run longer than the default command timeout", () => Effect.gen(function* () { const delegate = yield* ChildProcessSpawner.ChildProcessSpawner; diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 2de147ce8fd5..0fc67020976b 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -3672,11 +3672,15 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* const removeWorktree: GitVcsDriver.GitVcsDriver["Service"]["removeWorktree"] = Effect.fn( "removeWorktree", )(function* (input) { + // Git also accepts a worktree's short name, which can match a different + // folder than the path resolved against `input.cwd`. Resolve it once so Git + // and the leftover cleanup below act on the same directory. + const target = path.resolve(input.cwd, input.path); const args = ["worktree", "remove"]; if (input.force) { args.push("--force"); } - args.push(input.path); + args.push(target); const result = yield* executeGitWithStableDiagnostics( "GitVcsDriver.removeWorktree", input.cwd, @@ -3696,12 +3700,11 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* // recreating its checkout. Git has already unregistered the worktree, so // anything left here is logged rather than returned: a retry could never // succeed. - const leftover = path.resolve(input.cwd, input.path); - if (yield* fileSystem.exists(leftover).pipe(Effect.orElseSucceed(() => false))) { - const removed = yield* removeLeftoverLinks(leftover).pipe( + if (yield* fileSystem.exists(target).pipe(Effect.orElseSucceed(() => false))) { + const removed = yield* removeLeftoverLinks(target).pipe( Effect.catch((error) => Effect.logWarning("GitVcsDriver.removeWorktree: failed to delete leftover links", { - path: leftover, + path: target, error, }).pipe(Effect.as(true)), ), @@ -3709,7 +3712,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* if (!removed) { yield* Effect.logWarning( "GitVcsDriver.removeWorktree: kept files written after git removed the worktree", - { path: leftover }, + { path: target }, ); } }