Skip to content
Merged
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
72 changes: 68 additions & 4 deletions apps/server/src/vcs/GitVcsDriverCore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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", () => {
Expand Down Expand Up @@ -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", () => {
Expand Down
46 changes: 43 additions & 3 deletions apps/server/src/vcs/GitVcsDriverCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void, never>;
Expand Down Expand Up @@ -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,
});
});

Expand Down
Loading