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
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { existsSync } from "node:fs";
import { rm } from "node:fs/promises";
import { TRPCError } from "@trpc/server";
import { eq } from "drizzle-orm";
import { z } from "zod";
Expand Down Expand Up @@ -26,6 +27,18 @@ import { isMainWorkspace } from "./is-main-workspace";
*/
const destroysInFlight = new Set<string>();

/**
* `git worktree remove` fails with this when the path is no longer a
* registered worktree (its `.git/worktrees/<id>` metadata is gone) even
* though the directory may still exist on disk. This happens when a
* concurrent `git worktree prune` (run by `workspaces.create` on every
* invocation, including idempotent window opens/refreshes) or a prior
* partially-completed delete deregistered the worktree first. Retrying
* `git worktree remove` can never succeed in this state, so we treat it as
* already-deregistered and clean up the leftover directory ourselves.
*/
const NOT_A_WORKING_TREE = /is not a working tree/i;

/** @internal — exposed for tests to introspect / clear the guard. */
export const __testDestroysInFlight = destroysInFlight;

Expand Down Expand Up @@ -315,7 +328,23 @@ async function runDestroy(
} catch (err) {
const message = err instanceof Error ? err.message : String(err);
if (!existsSync(local.worktreePath)) {
// Directory already gone — nothing left to remove.
worktreeRemoved = true;
} else if (NOT_A_WORKING_TREE.test(message)) {
// Deregistered but lingering: prune stale metadata, then remove
// the leftover directory directly so the delete is idempotent.
await git.raw(["worktree", "prune"]).catch(() => undefined);
try {
await rm(local.worktreePath, { recursive: true, force: true });
worktreeRemoved = true;
} catch (rmErr) {
const rmMessage =
rmErr instanceof Error ? rmErr.message : String(rmErr);
throw new TRPCError({
code: "INTERNAL_SERVER_ERROR",
message: `Failed to remove leftover worktree directory at ${local.worktreePath}: ${rmMessage}`,
});
}
} else {
throw new TRPCError({
code: "INTERNAL_SERVER_ERROR",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
existsSync,
mkdirSync,
mkdtempSync,
renameSync,
rmSync,
writeFileSync,
} from "node:fs";
Expand Down Expand Up @@ -361,6 +362,50 @@ describe("workspaceCleanup.destroy integration", () => {
expect(branches.all).not.toContain(scenario.branch);
});

test("deregistered worktree whose directory lingers is removed without error", async () => {
// Reproduce the >50% deletion failure: something deregistered the
// worktree from git's metadata (a concurrent `git worktree prune` from
// workspaces.create, or a prior partial delete) while the directory
// stayed on disk. `git worktree remove` then fails permanently with
// "is not a working tree". Simulate by pruning the metadata while the
// directory is briefly moved aside, then restoring the directory.
const stash = `${scenario.worktreePath}.stash`;
renameSync(scenario.worktreePath, stash);
await scenario.repo.git.raw(["worktree", "prune"]);
renameSync(stash, scenario.worktreePath);

// Precondition: git no longer registers the path, but it exists on disk.
const before = await scenario.repo.git.raw([
"worktree",
"list",
"--porcelain",
]);
expect(before).not.toContain(scenario.worktreePath);
expect(existsSync(scenario.worktreePath)).toBe(true);

const result = await scenario.host.trpc.workspaceCleanup.destroy.mutate({
workspaceId: scenario.featureWorkspaceId,
deleteBranch: true,
});
expect(result.success).toBe(true);
expect(result.worktreeRemoved).toBe(true);
expect(result.branchDeleted).toBe(true);
expect(result.warnings).toEqual([]);
expect(existsSync(scenario.worktreePath)).toBe(false);
expect(
scenario.host.apiCalls.some(
(c) => c.path === "v2Workspace.delete.mutate",
),
).toBe(true);

const remaining = scenario.host.db
.select()
.from(workspaces)
.where(eq(workspaces.id, scenario.featureWorkspaceId))
.all();
expect(remaining).toHaveLength(0);
});

test("cloud delete failure after local removal keeps row so delete can retry", async () => {
let cloudDeleteCalls = 0;
scenario.host.setApi("v2Workspace.delete.mutate", () => {
Expand Down
53 changes: 53 additions & 0 deletions packages/host-service/test/workspace-cleanup.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { beforeEach, describe, expect, mock, test } from "bun:test";
import {
existsSync,
mkdirSync,
mkdtempSync,
rmSync,
Expand Down Expand Up @@ -32,6 +33,7 @@ interface ContextSpec {
revListCount?: string | (() => Promise<string>);
gitFactoryThrows?: boolean;
worktreeRemove?: () => Promise<unknown>;
worktreePrune?: () => Promise<unknown>;
branchDelete?: () => Promise<unknown>;
dbDeleteThrows?: boolean;
noApi?: boolean;
Expand All @@ -57,6 +59,7 @@ function makeCtx(spec: ContextSpec): HostServiceContext {
: (spec.revListCount ?? "0\n"),
);
const worktreeRemove = mock(spec.worktreeRemove ?? (async () => undefined));
const worktreePrune = mock(spec.worktreePrune ?? (async () => undefined));
const branchDelete = mock(spec.branchDelete ?? (async () => undefined));

const git = mock(async () => {
Expand All @@ -65,6 +68,8 @@ function makeCtx(spec: ContextSpec): HostServiceContext {
status,
raw: mock(async (args: string[]) => {
if (args[0] === "rev-list") return await revList();
if (args[0] === "worktree" && args[1] === "prune")
return await worktreePrune();
if (args[0] === "worktree") return await worktreeRemove();
if (args[0] === "branch") return await branchDelete();
throw new Error(`unexpected git raw: ${args.join(" ")}`);
Expand Down Expand Up @@ -394,6 +399,54 @@ describe("workspaceCleanup.destroy cleanup ordering", () => {
}
});

test("deregistered worktree ('is not a working tree') is cleaned up idempotently and delete proceeds", async () => {
const tmp = mkdtempSync(join(tmpdir(), "workspace-delete-"));
writeFileSync(join(tmp, ".keep"), "");
let cloudCallCount = 0;
let pruneCount = 0;
try {
const ctx = makeCtx({
workspace: {
id: "ws-1",
projectId: "p-1",
worktreePath: tmp,
branch: "feature",
},
project: { id: "p-1", repoPath: "/repo" },
cloudType: "worktree",
cloudDelete: async () => {
cloudCallCount += 1;
},
// Directory still on disk, but git no longer registers it — a
// concurrent prune/partial-delete deregistered it. Retrying
// `git worktree remove` can never succeed.
worktreeRemove: async () => {
throw new Error(`fatal: '${tmp}' is not a working tree`);
},
worktreePrune: async () => {
pruneCount += 1;
},
});
const caller = workspaceCleanupRouter.createCaller(ctx);

const result = await caller.destroy({
workspaceId: "ws-1",
deleteBranch: false,
force: true,
});

expect(result.success).toBe(true);
expect(result.worktreeRemoved).toBe(true);
expect(result.cloudDeleted).toBe(true);
expect(cloudCallCount).toBe(1);
expect(pruneCount).toBe(1);
// Leftover directory removed from disk.
expect(existsSync(tmp)).toBe(false);
} finally {
rmSync(tmp, { recursive: true, force: true });
}
});

test("git open failure blocks cloud delete while the worktree path still exists", async () => {
const tmp = mkdtempSync(join(tmpdir(), "workspace-delete-"));
let cloudCallCount = 0;
Expand Down
Loading