diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index dc58fc2543c..6390cfa4eb3 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -1,22 +1,28 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; import { assert, it, describe } from "@effect/vitest"; +import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; +import * as Fiber from "effect/Fiber"; import * as Layer from "effect/Layer"; import * as Path from "effect/Path"; import * as PlatformError from "effect/PlatformError"; import * as Scope from "effect/Scope"; +import * as TestClock from "effect/testing/TestClock"; import { GitCommandError } from "@t3tools/contracts"; import { ServerConfig } from "../config.ts"; -import { splitNullSeparatedGitStdoutPaths } from "./GitVcsDriverCore.ts"; +import { + parsePorcelainWorktreePaths, + splitNullSeparatedGitStdoutPaths, +} from "./GitVcsDriverCore.ts"; import * as GitVcsDriver from "./GitVcsDriver.ts"; const ServerConfigLayer = ServerConfig.layerTest(process.cwd(), { prefix: "t3-git-vcs-driver-test-", }); const TestLayer = GitVcsDriver.layer.pipe( - Layer.provide(ServerConfigLayer), + Layer.provideMerge(ServerConfigLayer), Layer.provideMerge(NodeServices.layer), ); @@ -184,6 +190,48 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { assert.notInclude(error.detail, "Git command failed in"); }), ); + + it.effect("does not filesystem-delete a path that is not a registered worktree", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + yield* initRepoWithCommit(cwd); + const pathService = yield* Path.Path; + const fileSystem = yield* FileSystem.FileSystem; + const decoyPath = pathService.join(cwd, "not-a-worktree"); + yield* fileSystem.makeDirectory(decoyPath, { recursive: true }); + yield* writeTextFile(decoyPath, "keep.txt", "keep\n"); + const driver = yield* GitVcsDriver.GitVcsDriver; + + const error = yield* driver + .removeWorktree({ cwd, path: decoyPath, force: true }) + .pipe(Effect.flip); + + assert.equal(error._tag, "GitCommandError"); + assert.equal(yield* fileSystem.exists(pathService.join(decoyPath, "keep.txt")), true); + }), + ); + }); + + describe("porcelain worktree parsing", () => { + it.effect("parses worktree paths from porcelain output", () => + Effect.sync(() => { + assert.deepStrictEqual( + parsePorcelainWorktreePaths( + [ + "worktree /repo", + "HEAD abc", + "branch refs/heads/main", + "", + "worktree /repo-feature", + "HEAD def", + "branch refs/heads/feature", + "", + ].join("\n"), + ), + ["/repo", "/repo-feature"], + ); + }), + ); }); describe("review diff previews", () => { @@ -590,8 +638,66 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { assert.equal(created.worktree.refName, "feature/worktree"); assert.equal(yield* git(worktreePath, ["branch", "--show-current"]), "feature/worktree"); - yield* driver.removeWorktree({ cwd, path: worktreePath }); + yield* driver.removeWorktree({ cwd, path: worktreePath, force: true }); + const fileSystem = yield* FileSystem.FileSystem; + assert.equal(yield* fileSystem.exists(worktreePath), false); + }), + ); + + it.effect("retries transient filesystem failures while force-removing a worktree", () => + 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-"), + "feature-worktree", + ); + const driver = yield* GitVcsDriver.GitVcsDriver; + + yield* driver.createWorktree({ + cwd, + path: worktreePath, + refName: initialBranch, + newRefName: "feature/worktree-retry", + }); + const fileSystem = yield* FileSystem.FileSystem; + const serverConfig = yield* ServerConfig; + const firstRemoveAttempt = yield* Deferred.make(); + let removeAttempts = 0; + const transientFailure = PlatformError.systemError({ + _tag: "Unknown", + module: "FileSystem", + method: "remove", + pathOrDescriptor: worktreePath, + description: "Directory not empty", + }); + const flakyFileSystem = FileSystem.FileSystem.of({ + ...fileSystem, + remove: (target, options) => + Effect.suspend(() => { + if (target === worktreePath && removeAttempts++ === 0) { + return Deferred.succeed(firstRemoveAttempt, undefined).pipe( + Effect.andThen(Effect.fail(transientFailure)), + ); + } + return fileSystem.remove(target, options); + }), + }); + const retryingDriver = yield* GitVcsDriver.make.pipe( + Effect.provideService(FileSystem.FileSystem, flakyFileSystem), + Effect.provideService(ServerConfig, serverConfig), + ); + + const removalFiber = yield* retryingDriver + .removeWorktree({ cwd, path: worktreePath, force: true }) + .pipe(Effect.forkScoped); + yield* Deferred.await(firstRemoveAttempt); + yield* TestClock.adjust("100 millis"); + yield* Fiber.join(removalFiber); + + assert.equal(removeAttempts, 2); assert.equal(yield* fileSystem.exists(worktreePath), false); }), ); diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index a406cbce549..bbe2836e871 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -13,6 +13,7 @@ import * as Path from "effect/Path"; import * as PlatformError from "effect/PlatformError"; import * as Ref from "effect/Ref"; import * as Result from "effect/Result"; +import * as Schedule from "effect/Schedule"; import * as Schema from "effect/Schema"; import * as Scope from "effect/Scope"; import * as Semaphore from "effect/Semaphore"; @@ -38,6 +39,14 @@ import { import { ServerConfig } from "../config.ts"; const DEFAULT_TIMEOUT_MS = 30_000; +// Worktrees routinely contain multi-GB install artifacts from setup scripts +// (node_modules, .repos, etc). Deleting those via `git worktree remove` alone +// routinely exceeds short command timeouts on developer machines. +const REMOVE_WORKTREE_DIRECTORY_TIMEOUT_MS = 5 * 60_000; +const REMOVE_WORKTREE_GIT_TIMEOUT_MS = 60_000; +const REMOVE_WORKTREE_DIRECTORY_RETRY_SCHEDULE = Schedule.spaced("100 millis").pipe( + Schedule.take(50), +); const DEFAULT_MAX_OUTPUT_BYTES = 1_000_000; const OUTPUT_TRUNCATED_MARKER = "\n\n[truncated]"; const PREPARED_COMMIT_PATCH_MAX_OUTPUT_BYTES = 49_000; @@ -227,6 +236,19 @@ export function splitNullSeparatedGitStdoutPaths( return splitNullSeparatedPaths(result.stdout, result.stdoutTruncated); } +export function parsePorcelainWorktreePaths(stdout: string): string[] { + const paths: string[] = []; + for (const line of stdout.split("\n")) { + if (line.startsWith("worktree ")) { + const worktreePath = line.slice("worktree ".length).trim(); + if (worktreePath.length > 0) { + paths.push(worktreePath); + } + } + } + return paths; +} + function sanitizeRemoteName(value: string): string { const sanitized = value .trim() @@ -2379,8 +2401,86 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* args.push("--force"); } args.push(input.path); + const commandContext = gitCommandContext({ + operation: "GitVcsDriver.removeWorktree", + cwd: input.cwd, + args, + }); + + // When forcing, wipe the working tree via the filesystem first so git only + // has to clean up worktree registration metadata. That keeps the git step + // fast even for multi-GB trees and avoids interrupting `git worktree remove` + // mid-delete after a short timeout. + // + // Only do this after confirming the path is a registered *linked* worktree. + // Otherwise a mistyped/non-worktree path would be recursively deleted before + // `git worktree remove` could reject it. + // + // Listing is best-effort: timeout/failure here skips the filesystem + // pre-delete and still falls through to `git worktree remove`. + if (input.force) { + const worktreeList = yield* executeGit( + "GitVcsDriver.removeWorktree.list", + input.cwd, + ["worktree", "list", "--porcelain"], + { + timeoutMs: 5_000, + allowNonZeroExit: true, + fallbackErrorDetail: "git worktree list failed", + }, + ).pipe(Effect.orElseSucceed(() => null)); + if (worktreeList !== null && worktreeList.exitCode === 0) { + const registeredPaths = parsePorcelainWorktreePaths(worktreeList.stdout); + const canonicalize = (value: string) => + fileSystem.realPath(value).pipe(Effect.orElseSucceed(() => path.resolve(value))); + const [canonicalTarget, ...canonicalRegistered] = yield* Effect.all( + [canonicalize(input.path), ...registeredPaths.map(canonicalize)], + { concurrency: "unbounded" }, + ); + // Porcelain lists the main working tree first; never filesystem-delete it. + const linkedWorktreePaths = new Set(canonicalRegistered.slice(1)); + if (linkedWorktreePaths.has(canonicalTarget)) { + const exists = yield* fileSystem + .exists(input.path) + .pipe(Effect.orElseSucceed(() => false)); + if (exists) { + yield* fileSystem.remove(input.path, { recursive: true, force: true }).pipe( + Effect.retry({ + schedule: REMOVE_WORKTREE_DIRECTORY_RETRY_SCHEDULE, + while: (cause) => + cause.reason._tag === "Busy" || + cause.reason._tag === "WouldBlock" || + cause.reason._tag === "Unknown", + }), + Effect.mapError( + (cause) => + new GitCommandError({ + ...commandContext, + detail: "Failed to delete worktree directory.", + cause, + }), + ), + Effect.timeoutOption(REMOVE_WORKTREE_DIRECTORY_TIMEOUT_MS), + Effect.flatMap((result) => + Option.match(result, { + onNone: () => + Effect.fail( + new GitCommandError({ + ...commandContext, + detail: "Timed out deleting worktree directory.", + }), + ), + onSome: () => Effect.void, + }), + ), + ); + } + } + } + } + yield* executeGit("GitVcsDriver.removeWorktree", input.cwd, args, { - timeoutMs: 15_000, + timeoutMs: REMOVE_WORKTREE_GIT_TIMEOUT_MS, fallbackErrorDetail: "git worktree remove failed", }); });