From 2043ca08b97493eab4b17482402d392ab3990737 Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Mon, 18 May 2026 15:15:59 -0700 Subject: [PATCH] fix(host-service): make workspace destroy tolerate locked+missing worktrees MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `git worktree remove --force` fails with "cannot remove a locked working tree" when a workspace's worktree is locked and its directory has been manually deleted — the existing substring matcher didn't catch this and the step pushed a confusing "Failed to remove worktree" warning even though the dir was already gone. Escalate to `--force --force` (we're past the commit point) and add an `existsSync` fallback: if the worktree dir is gone, the user's intent is satisfied regardless of what git printed (locked, locale-translated, future phrasing). The substring matcher stays as defense for the rare race where the dir disappears mid-remove. Closes SUPER-779. --- .../workspace-cleanup/workspace-cleanup.ts | 24 ++++++++++++++-- .../workspace-cleanup.integration.test.ts | 28 +++++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) 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 91f05f27f74..ceaadc6c1f4 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,3 +1,4 @@ +import { existsSync } from "node:fs"; import { TRPCError } from "@trpc/server"; import { eq } from "drizzle-orm"; import { z } from "zod"; @@ -267,7 +268,9 @@ async function runDestroy(ctx: HostServiceContext, input: DestroyInput) { warnings.push(`Failed to dispose terminal sessions: ${message}`); } - // 3b. Worktree (always --force: we're past the commit point) + // 3b. Worktree (always --force --force: we're past the commit point, + // and double-force unlocks the rare locked-worktree case the user + // can hit by manually `rm -rf`-ing a worktree that ended up locked.) // 3c. Optional branch delete let worktreeRemoved = false; let branchDeleted = false; @@ -287,11 +290,26 @@ async function runDestroy(ctx: HostServiceContext, input: DestroyInput) { if (git) { try { - await git.raw(["worktree", "remove", "--force", local.worktreePath]); + await git.raw([ + "worktree", + "remove", + "--force", + "--force", + local.worktreePath, + ]); worktreeRemoved = true; } catch (err) { const message = err instanceof Error ? err.message : String(err); - if ( + // If the worktree dir is already gone, the user's goal is met + // regardless of what git complains about — locale-translated + // messages, future git phrasing, or "locked working tree" with + // the dir already rm'd. The substring matcher below stays as + // belt-and-braces for the rare race where the dir disappears + // between this check and the git invocation, but `existsSync` + // is the authoritative signal. + if (!existsSync(local.worktreePath)) { + worktreeRemoved = true; + } else if ( message.includes("is not a working tree") || message.includes("No such file or directory") || message.includes("does not exist") || 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 f8f4fccdeba..7478a23adf7 100644 --- a/packages/host-service/test/integration/workspace-cleanup.integration.test.ts +++ b/packages/host-service/test/integration/workspace-cleanup.integration.test.ts @@ -192,6 +192,34 @@ describe("workspaceCleanup.destroy integration", () => { expect(worktreeList).toContain(otherWorktreePath); }); + test("missing worktree that was locked is still removed without warnings", async () => { + // A locked worktree whose dir was manually deleted is the scenario + // that breaks the substring-based error matcher: git says + // "fatal: cannot remove a locked working tree" and single `--force` + // is not enough. `--force --force` plus the existsSync fallback + // closes the loop so the user always gets a clean delete. + await scenario.repo.git.raw(["worktree", "lock", scenario.worktreePath]); + rmSync(scenario.worktreePath, { recursive: true, force: 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([]); + + const worktreeList = await scenario.repo.git.raw([ + "worktree", + "list", + "--porcelain", + ]); + expect(worktreeList).not.toContain(scenario.worktreePath); + const branches = await scenario.repo.git.branchLocal(); + expect(branches.all).not.toContain(scenario.branch); + }); + test("returns success when no local workspace row exists, still calls cloud delete", async () => { await scenario.dispose(); const fresh = await createBasicScenario({