diff --git a/packages/engine/src/__tests__/worktree-backend.test.ts b/packages/engine/src/__tests__/worktree-backend.test.ts index 21eb85e9b6..90df828ba0 100644 --- a/packages/engine/src/__tests__/worktree-backend.test.ts +++ b/packages/engine/src/__tests__/worktree-backend.test.ts @@ -938,7 +938,12 @@ describe("removeWorktree", () => { taskId: "FN-327", reason: RemovalReason.MergerCleanup, }), - ).resolves.toBeUndefined(); + ).resolves.toMatchObject({ + removed: false, + harmless: true, + classification: "not-registered-after-prune", + message: expect.stringContaining("no registered worktree remains after prune"), + }); expect(execMock).toHaveBeenNthCalledWith( 2, @@ -964,6 +969,28 @@ describe("removeWorktree", () => { ); }); + it("does not downgrade non-temp merger cleanup failures even when porcelain would be absent", async () => { + const worktreePath = "/repo/.worktrees/fn-327"; + const validationError = { + message: `Command failed: git worktree remove --force ${worktreePath}`, + stderr: `fatal: validation failed, cannot remove working tree: '${worktreePath}/.git' is not a .git file, error code 2`, + status: 2, + }; + execMock.mockRejectedValueOnce(validationError); + + await expect( + removeWorktree({ + rootDir: "/repo", + worktreePath, + settings: {}, + taskId: "FN-327", + reason: RemovalReason.MergerCleanup, + }), + ).rejects.toBe(validationError); + + expect(execMock).toHaveBeenCalledTimes(1); + }); + it("keeps FN-343 remove failures visible when the temp path remains registered after prune", async () => { const tempPath = "/var/folders/demo/T/fusion-ai-merge-fn-327-A5uY3j"; const validationError = { diff --git a/packages/engine/src/merger.ts b/packages/engine/src/merger.ts index 4271c6616d..79fe7deb54 100644 --- a/packages/engine/src/merger.ts +++ b/packages/engine/src/merger.ts @@ -7314,13 +7314,16 @@ async function removePostMergeWorktree( settings: Partial, ): Promise { try { - await removeWorktree({ + const outcome = await removeWorktree({ rootDir, worktreePath: postMergeWorktree, settings, taskId, reason: RemovalReason.MergerPostMerge, }); + if (outcome && "harmless" in outcome && outcome.harmless) { + mergerLog.warn(`${taskId}: post-merge worktree cleanup remove failed, but no registered worktree remains after prune for ${postMergeWorktree}: ${outcome.message}`); + } } catch (err: unknown) { mergerLog.warn(`${taskId}: failed to remove post-merge worktree ${postMergeWorktree}: ${getCommandErrorMessage(err)}`); } @@ -10569,7 +10572,7 @@ export async function aiMergeTask( metadata: { taskId, reason: RemovalReason.MergerCleanup, kind: "merger" }, }); } else { - await removeWorktree({ + const outcome = await removeWorktree({ rootDir, worktreePath, settings, @@ -10577,7 +10580,10 @@ export async function aiMergeTask( audit, reason: RemovalReason.MergerCleanup, }); - result.worktreeRemoved = true; + if (outcome && "harmless" in outcome && outcome.harmless) { + mergerLog.warn(`${taskId}: merge worktree cleanup remove failed, but no registered worktree remains after prune for ${worktreePath}: ${outcome.message}`); + } + result.worktreeRemoved = outcome?.removed ?? true; } if (result.worktreeRemoved) { try { diff --git a/packages/engine/src/run-audit.ts b/packages/engine/src/run-audit.ts index ee131c30b6..fcdd1d19ee 100644 --- a/packages/engine/src/run-audit.ts +++ b/packages/engine/src/run-audit.ts @@ -92,6 +92,8 @@ export type GitMutationType = | "worktree:create" | "worktree:remove" | "worktree:remove-fallback" + | "worktree:remove-classified-harmless" + | "worktree:remove-leaked-registered-worktree" | "worktree:reuse" | "worktree:incomplete-detected" | "worktree:reanchored" diff --git a/packages/engine/src/worktree-backend.ts b/packages/engine/src/worktree-backend.ts index 6abc8dbd07..74a811cfac 100644 --- a/packages/engine/src/worktree-backend.ts +++ b/packages/engine/src/worktree-backend.ts @@ -30,6 +30,133 @@ const NATIVE_TIMEOUT_MS = 120_000; const REMOVE_TIMEOUT_MS = 60_000; const MAX_BUFFER = 10 * 1024 * 1024; + +export type WorktreeRemoveOutcome = + | { removed: true; classification: "removed" } + | { + removed: false; + harmless: true; + classification: "not-registered-after-prune"; + message: string; + stderrPreview: string; + pathExists: boolean; + gitFileExists: boolean; + }; + +const HARMLESS_MERGE_REMOVE_ERROR_PATTERNS = [ + /validation failed, cannot remove working tree/i, + /is not a \.git file/i, + /is not a working tree/i, + /not a git repository/i, + /No such file or directory/i, +] as const; + +function previewError(error: unknown): string { + const stderr = getErrorStderr(error); + const message = error instanceof Error ? error.message : String(error); + return (stderr || message).slice(0, 4096); +} + +function normalizeComparablePath(value: string): string { + const resolved = resolve(value); + return resolved.startsWith("/private/var/") ? resolved.slice("/private".length) : resolved; +} + +function porcelainContainsWorktree(stdout: string, worktreePath: string): boolean { + const target = normalizeComparablePath(worktreePath); + const privateTarget = target.startsWith("/var/") ? `/private${target}` : target; + for (const line of stdout.split("\n")) { + if (!line.startsWith("worktree ")) continue; + const candidate = normalizeComparablePath(line.slice("worktree ".length).trim()); + if (candidate === target || candidate === privateTarget) return true; + } + return false; +} + +function isMergeTempCleanupCandidate(input: { worktreePath: string; reason: RemovalReason }, error: unknown): boolean { + if (input.reason !== RemovalReason.MergerCleanup && input.reason !== RemovalReason.MergerPostMerge) return false; + const base = basename(input.worktreePath); + const looksLikeFusionMergeTemp = base.startsWith("fusion-ai-merge-") || base.startsWith("post-merge-"); + if (!looksLikeFusionMergeTemp) return false; + const detail = previewError(error); + return HARMLESS_MERGE_REMOVE_ERROR_PATTERNS.some((pattern) => pattern.test(detail)); +} + +async function classifyHarmlessMergeRemoveFailure(input: { + rootDir: string; + worktreePath: string; + reason: RemovalReason; + taskId?: string; + audit?: RunAuditor; +}, error: unknown): Promise { + if (!isMergeTempCleanupCandidate(input, error)) return null; + + const stderrPreview = previewError(error); + const pathExists = existsSync(input.worktreePath); + const gitFileExists = existsSync(resolve(input.worktreePath, ".git")); + + await execAsync("git worktree prune", { + cwd: input.rootDir, + encoding: "utf-8", + timeout: NATIVE_TIMEOUT_MS, + maxBuffer: MAX_BUFFER, + }); + + const { stdout } = await execAsync("git worktree list --porcelain", { + cwd: input.rootDir, + encoding: "utf-8", + timeout: 10_000, + maxBuffer: MAX_BUFFER, + }); + const registeredAfterPrune = porcelainContainsWorktree(String(stdout ?? ""), input.worktreePath); + + if (registeredAfterPrune) { + await input.audit?.git({ + type: "worktree:remove-leaked-registered-worktree", + target: input.worktreePath, + metadata: { + taskId: input.taskId, + reason: input.reason, + registeredAfterPrune: true, + stderrPreview, + pathExists, + gitFileExists, + }, + }); + return null; + } + + const message = pathExists + ? "cleanup remove failed, but no registered worktree remains after prune; leftover directory was not deleted automatically" + : "cleanup remove failed, but no registered worktree remains after prune"; + await input.audit?.git({ + type: "worktree:remove-classified-harmless", + target: input.worktreePath, + metadata: { + taskId: input.taskId, + reason: input.reason, + classification: "not-registered-after-prune", + registeredAfterPrune: false, + stderrPreview, + pathExists, + gitFileExists, + nextAction: pathExists + ? "inspect the leftover temp directory before deleting filesystem residue" + : "no operator action required", + }, + }); + + return { + removed: false, + harmless: true, + classification: "not-registered-after-prune", + message, + stderrPreview, + pathExists, + gitFileExists, + }; +} + /** * worktrunk CLI mapping (verified 2026-05-15 from README + worktrunk.dev docs): * - create -> `wt switch --create [--base ]` @@ -822,7 +949,7 @@ export async function removeWorktree(input: { liveOwnerProbe?: LiveBindingProbe; processActiveProbe?: ProcessActiveProbe; reconcileMinIdleMs?: number; -}): Promise { +}): Promise { const logger = { log: (_message: string): void => {}, warn: (_message: string): void => {}, @@ -896,8 +1023,11 @@ export async function removeWorktree(input: { target: input.worktreePath, }); } - return; + return { removed: true, classification: "removed" }; } catch (error) { + const classified = await classifyHarmlessMergeRemoveFailure(input, error); + if (classified) return classified; + if (!(error instanceof WorktrunkOperationError) || input.settings.worktrunk?.onFailure !== "fallback-native") { throw error; } @@ -915,8 +1045,15 @@ export async function removeWorktree(input: { }); const native = new NativeWorktreeBackend({ logger, settings: input.settings }); - await native.remove(removeInput); - await input.audit?.git({ type: "worktree:remove", target: input.worktreePath }); + try { + await native.remove(removeInput); + await input.audit?.git({ type: "worktree:remove", target: input.worktreePath }); + return { removed: true, classification: "removed" }; + } catch (nativeError) { + const classified = await classifyHarmlessMergeRemoveFailure(input, nativeError); + if (classified) return classified; + throw nativeError; + } } }