diff --git a/packages/host-service/src/trpc/router/workspace-cleanup/workspace-cleanup.ts b/packages/host-service/src/trpc/router/workspace-cleanup/workspace-cleanup.ts index 202e4189750..1331c9fd0e1 100644 --- a/packages/host-service/src/trpc/router/workspace-cleanup/workspace-cleanup.ts +++ b/packages/host-service/src/trpc/router/workspace-cleanup/workspace-cleanup.ts @@ -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"; @@ -26,6 +27,18 @@ import { isMainWorkspace } from "./is-main-workspace"; */ const destroysInFlight = new Set(); +/** + * `git worktree remove` fails with this when the path is no longer a + * registered worktree (its `.git/worktrees/` 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; @@ -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", diff --git a/packages/host-service/test/integration/workspace-cleanup.integration.test.ts b/packages/host-service/test/integration/workspace-cleanup.integration.test.ts index fc153535c18..fc3f8ae1116 100644 --- a/packages/host-service/test/integration/workspace-cleanup.integration.test.ts +++ b/packages/host-service/test/integration/workspace-cleanup.integration.test.ts @@ -4,6 +4,7 @@ import { existsSync, mkdirSync, mkdtempSync, + renameSync, rmSync, writeFileSync, } from "node:fs"; @@ -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", () => { diff --git a/packages/host-service/test/workspace-cleanup.test.ts b/packages/host-service/test/workspace-cleanup.test.ts index a5d7d43cb39..2157dd87b2a 100644 --- a/packages/host-service/test/workspace-cleanup.test.ts +++ b/packages/host-service/test/workspace-cleanup.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, mock, test } from "bun:test"; import { + existsSync, mkdirSync, mkdtempSync, rmSync, @@ -32,6 +33,7 @@ interface ContextSpec { revListCount?: string | (() => Promise); gitFactoryThrows?: boolean; worktreeRemove?: () => Promise; + worktreePrune?: () => Promise; branchDelete?: () => Promise; dbDeleteThrows?: boolean; noApi?: boolean; @@ -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 () => { @@ -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(" ")}`); @@ -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;