diff --git a/.changeset/dead-worktree-recovery-loop.md b/.changeset/dead-worktree-recovery-loop.md new file mode 100644 index 0000000000..22b55c94a2 --- /dev/null +++ b/.changeset/dead-worktree-recovery-loop.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: A review task whose worktree was removed now gets a fresh one instead of failing on every retry. +category: fix +dev: `autoRecoverWorktreeSessionStartFailure` only preserves `task.worktree` when that path is still a usable checkout (exists + `.git`, via `isUsableWorktreeDirectory`/`hasRequiredWorktreeFiles`). Previously a failing path that merely DIFFERED from `task.worktree` — e.g. an AI-merge clean room refused as an "incomplete worktree" — was read as proof the recorded worktree was live, so a removed worktree was carried into every requeue until the retry budget was exhausted. diff --git a/packages/engine/src/__tests__/replan-target.test.ts b/packages/engine/src/__tests__/replan-target.test.ts index 20e7bb11f0..7cb0e2b44d 100644 --- a/packages/engine/src/__tests__/replan-target.test.ts +++ b/packages/engine/src/__tests__/replan-target.test.ts @@ -140,6 +140,23 @@ const planningGuardCases: PlanningGuardCase[] = [ }, stillPlanning: false, }, + /* + FNXC:WorkflowReplan 2026-07-26-07:40: + `planning` is the TRANSIENT planner claim, not a durable park: when a stamp lands on a + `planning` row, execution won the FN-8361 race and recovery must not clear the status out from + under it. Only the durable park statuses outrank the stamps. + */ + { + label: "planning row claimed by execution mid-race (FN-8361)", + task: { + column: "triage", + steps: [], + status: "planning", + worktree: "/tmp/claimed", + firstExecutionAt: "2026-07-26T04:35:29.068Z", + }, + stillPlanning: false, + }, { label: "triage card parked by a reviewer outage after an execution attempt", task: { diff --git a/packages/engine/src/__tests__/self-healing.test.ts b/packages/engine/src/__tests__/self-healing.test.ts index 0c54898757..51ac79006b 100644 --- a/packages/engine/src/__tests__/self-healing.test.ts +++ b/packages/engine/src/__tests__/self-healing.test.ts @@ -63,7 +63,10 @@ vi.mock("node:fs", async (importOriginal) => { }; }); -vi.mock("../worktree-pool.js", () => ({ +vi.mock("../worktree-pool.js", async () => { + const { existsSync: fsExistsSync } = await import("node:fs"); + const { join: joinPath } = await import("node:path"); + return { WorktreePool: vi.fn(), // FN-4811: Must mirror the production `RemovalReason` const in worktree-backend.ts // exactly — every key referenced as `RemovalReason.X` in production code (self-healing, @@ -87,13 +90,21 @@ vi.mock("../worktree-pool.js", () => ({ scanOrphanedBranches: vi.fn().mockResolvedValue([]), cleanupOrphanedWorktrees: vi.fn().mockResolvedValue(0), isUsableTaskWorktree: vi.fn().mockResolvedValue(true), + /* + FNXC:MissingWorktreeRecovery 2026-07-26-07:15: + Keep the real `.git` probe: the unusable-worktree recovery decides whether to PRESERVE + `task.worktree` from it, so a blanket `true` would re-hide the MG-047 strand (a recorded + worktree that is gone must be cleared, not carried into the next dispatch). + */ + hasRequiredWorktreeFiles: vi.fn((worktreePath: string) => fsExistsSync(joinPath(worktreePath, ".git"))), classifyTaskWorktree: vi.fn().mockResolvedValue({ ok: false, classification: "missing", reason: "test-default" }), getRegisteredWorktreePaths: vi.fn().mockResolvedValue(new Set()), getRegisteredWorktreeBranchMap: vi.fn().mockResolvedValue(new Map()), removeWorktree: vi.fn().mockResolvedValue(undefined), relocateReclaimableWorktreeIntoRoot: vi.fn(async ({ sourcePath }: { sourcePath: string }) => ({ kind: "ready", path: sourcePath, relocated: false })), resolveWorktreeBackend: vi.fn(), -})); + }; +}); const { selfHealingLoggerMock } = vi.hoisted(() => ({ selfHealingLoggerMock: { @@ -117,7 +128,7 @@ import { HEARTBEAT_ERROR_RECOVERY_METADATA_KEY, HEARTBEAT_ERROR_RETRY_EXHAUSTED_ import type { TaskStore, Settings, Task, AgentStore, Agent, NotificationProvider } from "@fusion/core"; import { EventEmitter } from "node:events"; import { execSync } from "node:child_process"; -import { existsSync, mkdtempSync, readdirSync, rmSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from "node:fs"; import { readFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -3365,7 +3376,15 @@ describe("SelfHealingManager", () => { expect(source).not.toMatch(/Refusing to start coding agent/); }); - it("requeues failed in-review tasks with unusable-worktree session-start errors", async () => { + /* + FNXC:MissingWorktreeRecovery 2026-07-26-07:15: + The mismatch-preserve branch needs a worktree that is REALLY there (temp dir + `.git`), because + the recovery now proves usability before carrying `task.worktree` forward. A fictional path is + indistinguishable from the MG-047 strand where the recorded worktree had been removed. + */ + it("requeues failed in-review tasks with unusable-worktree session-start errors, preserving a live task worktree", async () => { + const liveWorktree = mkdtempSync(join(tmpdir(), "fusion-live-worktree-")); + writeFileSync(join(liveWorktree, ".git"), "gitdir: /tmp/test-project/.git/worktrees/fn-3900\n"); const managerWithRecovery = new SelfHealingManager(store, { rootDir: "/tmp/test-project", }); @@ -3376,7 +3395,7 @@ describe("SelfHealingManager", () => { column: "in-review", paused: false, status: "failed", - worktree: "/tmp/project/.worktrees/fn-3900-stale", + worktree: liveWorktree, branch: "fusion/fn-3900", sessionFile: "/tmp/project/.fusion/sessions/fn-3900.json", error: "Refusing to start coding agent in missing worktree: /tmp/other/.worktrees/fn-3900", @@ -3392,7 +3411,7 @@ describe("SelfHealingManager", () => { status: null, error: null, worktreeSessionRetryCount: 1, - worktree: "/tmp/project/.worktrees/fn-3900-stale", + worktree: liveWorktree, branch: "fusion/fn-3900", sessionFile: null, }); @@ -3402,7 +3421,7 @@ describe("SelfHealingManager", () => { ); expect(store.logEntry).toHaveBeenCalledWith( "FN-3900", - expect.stringContaining("/tmp/project/.worktrees/fn-3900-stale"), + expect.stringContaining(liveWorktree), ); expect(store.logEntry).toHaveBeenCalledWith( "FN-3900", @@ -3410,10 +3429,36 @@ describe("SelfHealingManager", () => { ); expect(store.moveTask).toHaveBeenCalledWith("FN-3900", "todo", { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); + rmSync(liveWorktree, { recursive: true, force: true }); managerWithRecovery.stop(); }); - it("requeues incomplete-worktree failures and clears stale worktree metadata", async () => { + /* + FNXC:MissingWorktreeRecovery 2026-07-26-07:15: + Reported strand (in-review MG-047): the session-start refusal named an AI-merge clean room + ("incomplete worktree") while the RECORDED task worktree had already been removed. Preserving + the recorded path because it merely differed from the failing one re-dispatched the card into a + directory that no longer existed ("Working directory does not exist … Cannot execute bash + commands") on every retry, until the budget burned out and the card parked failed in review. + Surfaces: the failing path may be a clean room or the task worktree, and the recorded worktree + may be gone entirely or present-but-incomplete (no `.git`) — both must clear the metadata. + */ + it.each([ + { + label: "recorded task worktree no longer exists", + seed: (base: string) => join(base, "removed-by-cleanup"), + }, + { + label: "recorded task worktree exists but has no .git", + seed: (base: string) => { + const dir = join(base, "incomplete"); + mkdirSync(dir, { recursive: true }); + return dir; + }, + }, + ])("requeues incomplete-worktree failures and clears stale worktree metadata when the $label", async ({ seed }) => { + const base = mkdtempSync(join(tmpdir(), "fusion-dead-worktree-")); + const recordedWorktree = seed(base); const managerWithRecovery = new SelfHealingManager(store, { rootDir: "/tmp/test-project", }); @@ -3424,10 +3469,11 @@ describe("SelfHealingManager", () => { column: "in-review", paused: false, status: "failed", - worktree: "/tmp/project/.worktrees/noble-eagle-stale", + worktree: recordedWorktree, branch: "fusion/FN-4559", sessionFile: "/tmp/project/.fusion/sessions/FN-4559.json", - error: "Refusing to start coding agent in incomplete worktree: /tmp/project/.worktrees/noble-eagle", + // The refusal names the AI-merge clean room, NOT the recorded task worktree. + error: "Refusing to start coding agent in incomplete worktree: /tmp/project/.worktrees/.ai-merge/fusion-ai-merge-fn-4559-TGahla", steps: [{ status: "done" }, { status: "pending" }], log: [], }, @@ -3440,10 +3486,11 @@ describe("SelfHealingManager", () => { status: null, error: null, worktreeSessionRetryCount: 1, - worktree: "/tmp/project/.worktrees/noble-eagle-stale", - branch: "fusion/FN-4559", + worktree: null, + branch: null, sessionFile: null, }); + rmSync(base, { recursive: true, force: true }); expect(store.logEntry).toHaveBeenCalledWith( "FN-4559", expect.stringContaining("Auto-recovered"), diff --git a/packages/engine/src/replan-target.ts b/packages/engine/src/replan-target.ts index 42794330cc..3f0dad9546 100644 --- a/packages/engine/src/replan-target.ts +++ b/packages/engine/src/replan-target.ts @@ -43,6 +43,16 @@ legal from every legacy column and eligibleTriageTasks re-specifies unconditiona /** Statuses that explicitly park a card for (re)planning, whichever column holds it. */ const PLANNING_STAGE_STATUSES = new Set(["planning", "needs-replan", "plan-review-unavailable"]); +/* +FNXC:WorkflowReplan 2026-07-26-07:40: +The DURABLE subset of the above: a card parked here was deliberately sent back by Plan Review (or +by a reviewer outage) and stays parked until a planner re-specifies it. Only these outrank the +execution timestamps below. `planning` is deliberately excluded — it is the TRANSIENT in-flight +planner claim, and a fresh execution stamp on a `planning` row means execution won the race that +FN-8361 guards (recovery must not clear the status out from under the claiming executor). +*/ +const REPLAN_PARK_STATUSES = new Set(["needs-replan", "plan-review-unavailable"]); + export function hasAdvancedPastPlanning( task: Pick & Partial>, @@ -57,17 +67,17 @@ export function hasAdvancedPastPlanning( } /* FNXC:WorkflowReplan 2026-07-26-06:10: - An explicit parked-for-planning STATUS outranks execution evidence, because that evidence is - STICKY while a replan is a legitimate BACKWARD move. `firstExecutionAt`/`executionStartedAt` are - never cleared once implementation starts, so a card that executed, failed Plan Review, and was - rebounded to a planner lane (`needs-replan`) read as "advanced past planning" forever: triage's - discovery filter (`column === "triage" && isTaskStillInPlanningStage`) never re-admitted it and - the card sat in triage/needs-replan permanently — "stuck in planning" on the board (FN-8594). It - hit every triage-column workflow (builtin:coding, the default); plan-in-place Ideas cards escaped - only because todo discovery admits `needs-replan` without consulting this guard. + A DURABLE parked-for-replan status outranks execution evidence, because that evidence is STICKY + while a replan is a legitimate BACKWARD move. `firstExecutionAt`/`executionStartedAt` are never + cleared once implementation starts, so a card that executed, failed Plan Review, and was rebounded + to a planner lane (`needs-replan`) read as "advanced past planning" forever: triage's discovery + filter (`column === "triage" && isTaskStillInPlanningStage`) never re-admitted it and the card sat + in triage/needs-replan permanently — "stuck in planning" on the board (FN-8594). It hit every + triage-column workflow (builtin:coding, the default); plan-in-place Ideas cards escaped only + because todo discovery admits `needs-replan` without consulting this guard. This check covers BOTH planner lanes — the "triage" column and the plan-in-place "todo" lane. */ - if (task.status != null && PLANNING_STAGE_STATUSES.has(task.status)) { + if (task.status != null && REPLAN_PARK_STATUSES.has(task.status)) { return false; } /* @@ -91,6 +101,11 @@ export function hasAdvancedPastPlanning( if (task.column === "triage") { return false; } + // Plan-in-place planner lane ("todo"): a card explicitly parked for planning has not advanced. + // Reached only by `planning` here — the durable park statuses already returned above. + if (task.status != null && PLANNING_STAGE_STATUSES.has(task.status)) { + return false; + } return (task.steps?.length ?? 0) > 0; } diff --git a/packages/engine/src/self-healing.ts b/packages/engine/src/self-healing.ts index aab3a4a9c2..f19045ab00 100644 --- a/packages/engine/src/self-healing.ts +++ b/packages/engine/src/self-healing.ts @@ -35,7 +35,8 @@ import { finalizePlanningSegment } from "@fusion/core"; import type { MeshLeaseManager } from "./mesh-lease-manager.js"; import { createLogger, schedulerLog } from "./logger.js"; import { mergeEffectiveSettings } from "./effective-settings.js"; -import { RemovalReason, classifyTaskWorktree, getRegisteredWorktreeBranchMap, getRegisteredWorktreePaths, isUsableTaskWorktree, relocateReclaimableWorktreeIntoRoot, removeWorktree, resolveWorktreeBackend, scanIdleWorktrees, scanOrphanedBranches } from "./worktree-pool.js"; +import { RemovalReason, classifyTaskWorktree, getRegisteredWorktreeBranchMap, getRegisteredWorktreePaths, hasRequiredWorktreeFiles, isUsableTaskWorktree, relocateReclaimableWorktreeIntoRoot, removeWorktree, resolveWorktreeBackend, scanIdleWorktrees, scanOrphanedBranches } from "./worktree-pool.js"; +import { isUsableWorktreeDirectory } from "./step-runner.js"; import { classifyMissingWorktreeSessionStartFailure, extractMissingWorktreePathFromSessionStartFailure, @@ -592,8 +593,25 @@ export async function autoRecoverWorktreeSessionStartFailure( const staleWorktree = task.worktree; const missingWorktreePath = extractMissingWorktreePathFromSessionStartFailure(opts.failure); - const hasMismatchedLiveWorktree = + /* + FNXC:MissingWorktreeRecovery 2026-07-26-07:15: + A failing path that DIFFERS from `task.worktree` does not prove the recorded worktree is live. + The reported strand (in-review MG-047) had both gone: an AI-merge clean room refused as an + "incomplete worktree" while the recorded task worktree had already been removed. The mismatch + branch preserved that dead path, so every requeue re-dispatched into a directory that no longer + existed ("Working directory does not exist: …" / "Cannot execute bash commands") until the retry + budget burned out and the card parked failed in review. Preserve the recorded worktree only when + it is STILL a usable checkout; otherwise clear it so the next dispatch builds a fresh one from + the branch. Filesystem-only probe (exists + `.git`, the same first two gates as + classifyTaskWorktree) — recovery must not spawn git, and this runs on a failure path. + */ + const recordedWorktreeStillUsable = typeof staleWorktree === "string" && staleWorktree.length > 0 + && isUsableWorktreeDirectory(staleWorktree) + && hasRequiredWorktreeFiles(staleWorktree); + const hasMismatchedLiveWorktree = + recordedWorktreeStillUsable + && typeof staleWorktree === "string" && staleWorktree.length > 0 && typeof missingWorktreePath === "string" && missingWorktreePath.length > 0 && resolve(staleWorktree) !== resolve(missingWorktreePath); const noProgress = !hasStepProgress(task);