fix(engine): stop requeueing review tasks into a worktree that no longer exists
Unusable-worktree recovery preserved task.worktree whenever the failing path
differed from it, treating the difference as proof the recorded worktree was
live. The reported strand had both gone: an AI-merge clean room refused as an
"incomplete worktree" while the task worktree had already been removed, so
every requeue re-dispatched into a missing directory ("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.
Also splits the planning-stage guard: only the DURABLE replan parks
(needs-replan, plan-review-unavailable) outrank sticky execution stamps.
"planning" is the transient planner claim, so a stamp landing on it still means
execution won the FN-8361 race.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
7
.changeset/dead-worktree-recovery-loop.md
Normal file
7
.changeset/dead-worktree-recovery-loop.md
Normal file
@@ -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.
|
||||
@@ -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: {
|
||||
|
||||
@@ -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<string>()),
|
||||
getRegisteredWorktreeBranchMap: vi.fn().mockResolvedValue(new Map<string, string>()),
|
||||
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"),
|
||||
|
||||
@@ -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<Task, "column" | "worktree" | "steps" | "status">
|
||||
& Partial<Pick<Task, "firstExecutionAt" | "executionStartedAt">>,
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user