Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 109 additions & 3 deletions apps/server/src/vcs/GitVcsDriverCore.test.ts
Original file line number Diff line number Diff line change
@@ -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),
);

Expand Down Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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<void>();
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);
}),
);
Expand Down
102 changes: 101 additions & 1 deletion apps/server/src/vcs/GitVcsDriverCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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;
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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,
}),
),
);
}
}
Comment thread
cursor[bot] marked this conversation as resolved.
}
}
Comment thread
cursor[bot] marked this conversation as resolved.

yield* executeGit("GitVcsDriver.removeWorktree", input.cwd, args, {
timeoutMs: 15_000,
timeoutMs: REMOVE_WORKTREE_GIT_TIMEOUT_MS,
fallbackErrorDetail: "git worktree remove failed",
});
});
Expand Down
Loading