feat(FN-4031): recover stale review sessions and unblock dependent tasks on
Adds defensive recovery logic for stale review sessions: the executor now guards against resuming in an invalid worktree context, the self-healing manager recovers stale session paths and unblocks dependent tasks, and the restart recovery coordinator is updated to classify these cases correctly. Inc Fusion-Task-Id: FN-4031
This commit is contained in:
@@ -8,6 +8,7 @@ import { TaskExecutor, buildExecutionPrompt } from "../executor.js";
|
||||
import { createFnAgent } from "../pi.js";
|
||||
import { reviewStep as mockedReviewStepFn } from "../reviewer.js";
|
||||
import { execSync } from "node:child_process";
|
||||
import { writeFile, rm } from "node:fs/promises";
|
||||
import { findWorktreeUser, aiMergeTask } from "../merger.js";
|
||||
import { WorktreePool } from "../worktree-pool.js";
|
||||
import { generateWorktreeName, slugify } from "../worktree-names.js";
|
||||
@@ -1326,6 +1327,44 @@ describe("TaskExecutor pause behavior", () => {
|
||||
expect(mockedSessionManager.create).toHaveBeenCalled();
|
||||
expect(mockedSessionManager.open).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not resume stale sessionFile when persisted worktree path mismatches live task worktree", async () => {
|
||||
const store = createMockStore();
|
||||
const sessionFilePath = "/tmp/fn-4031-stale-session.jsonl";
|
||||
await writeFile(sessionFilePath, JSON.stringify({ cwd: "/tmp/test/.worktrees/bright-wren" }), "utf-8");
|
||||
|
||||
mockedExistsSync.mockImplementation((p) => String(p) !== "/tmp/test/.worktrees/fn-001");
|
||||
|
||||
mockedCreateFnAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
sessionFile: "/tmp/sessions/new_session.jsonl",
|
||||
} as any);
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
await executor.execute({
|
||||
id: "FN-001",
|
||||
title: "Stale resumed session",
|
||||
description: "Test stale worktree session mismatch fallback",
|
||||
column: "in-progress",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
worktree: "/tmp/test/.worktrees/fn-001",
|
||||
sessionFile: sessionFilePath,
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
expect(mockedSessionManager.open).not.toHaveBeenCalled();
|
||||
expect(mockedSessionManager.create).toHaveBeenCalledWith("/tmp/test/.worktrees/fn-001");
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { sessionFile: null });
|
||||
|
||||
await rm(sessionFilePath, { force: true });
|
||||
});
|
||||
});
|
||||
|
||||
describe("swallowed async store failure observability", () => {
|
||||
|
||||
@@ -2,6 +2,7 @@ import { describe, expect, it, vi } from "vitest";
|
||||
import type { TaskStore, Task } from "@fusion/core";
|
||||
import {
|
||||
RestartRecoveryCoordinator,
|
||||
extractMissingWorktreePathFromSessionStartFailure,
|
||||
isMissingWorktreeSessionStartFailure,
|
||||
isRecoverableMissingWorktreeReviewFailure,
|
||||
} from "../restart-recovery-coordinator.js";
|
||||
@@ -29,6 +30,11 @@ describe("RestartRecoveryCoordinator", () => {
|
||||
expect(isMissingWorktreeSessionStartFailure("Deterministic test verification failed")).toBe(false);
|
||||
});
|
||||
|
||||
it("extracts missing-worktree path from session-start failure", () => {
|
||||
expect(extractMissingWorktreePathFromSessionStartFailure("Refusing to start coding agent in missing worktree: /tmp/wt")).toBe("/tmp/wt");
|
||||
expect(extractMissingWorktreePathFromSessionStartFailure("other error")).toBeNull();
|
||||
});
|
||||
|
||||
it("identifies recoverable in-review missing-worktree failures with step progress", () => {
|
||||
const task = createTask({
|
||||
column: "in-review",
|
||||
|
||||
@@ -1589,8 +1589,8 @@ describe("SelfHealingManager", () => {
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-3900", {
|
||||
status: null,
|
||||
error: null,
|
||||
worktree: null,
|
||||
branch: null,
|
||||
worktree: "/tmp/project/.worktrees/fn-3900-stale",
|
||||
branch: "fusion/fn-3900",
|
||||
sessionFile: null,
|
||||
});
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
@@ -4366,6 +4366,27 @@ describe("clearStaleBlockedBy", () => {
|
||||
manager.stop();
|
||||
});
|
||||
|
||||
it("FN-4013 signature: clears blockedBy when in-review blocker failed from missing-worktree session start", async () => {
|
||||
const store = createRunningStore();
|
||||
const taskA = createTask("FN-4013", { blockedBy: "FN-3908", dependencies: ["FN-3908"] });
|
||||
const taskB = createTask("FN-3908", {
|
||||
column: "in-review",
|
||||
status: "failed",
|
||||
mergeRetries: 0,
|
||||
error: "Refusing to start coding agent in missing worktree: /Users/eclipxe/Projects/kb/.worktrees/bright-wren",
|
||||
steps: [{ status: "done" }, { status: "pending" }] as any,
|
||||
});
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([taskA, taskB]);
|
||||
|
||||
const manager = new SelfHealingManager(store, { rootDir: "/tmp/test-project" });
|
||||
const recovered = await manager.clearStaleBlockedBy();
|
||||
|
||||
expect(recovered).toBe(1);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-4013", { blockedBy: null, status: null });
|
||||
expect(store.logEntry).toHaveBeenCalledWith("FN-4013", expect.stringContaining("missing-worktree session start"));
|
||||
manager.stop();
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ settings: { globalPause: true }, label: "globalPause" },
|
||||
{ settings: { enginePaused: true }, label: "enginePaused" },
|
||||
|
||||
@@ -151,6 +151,42 @@ const WORKFLOW_SCRIPT_OUTPUT_MAX_CHARS = 4_000;
|
||||
|
||||
class NonRetryableWorktreeError extends Error {}
|
||||
|
||||
const SESSION_WORKTREE_PATH_REGEX = /([A-Za-z]:)?[^"'\s]*\.worktrees[\\/][^"'\s]+/g;
|
||||
|
||||
function normalizeWorktreePath(pathValue: string): string {
|
||||
return resolvePath(pathValue).replace(/\\/g, "/").replace(/\/+$/, "");
|
||||
}
|
||||
|
||||
async function extractPersistedSessionWorktreePath(sessionFile: string): Promise<string | null> {
|
||||
try {
|
||||
const content = await readFile(sessionFile, "utf-8");
|
||||
const matches = content.match(SESSION_WORKTREE_PATH_REGEX) ?? [];
|
||||
if (matches.length === 0) return null;
|
||||
|
||||
const normalizedCounts = new Map<string, number>();
|
||||
for (const match of matches) {
|
||||
const normalized = normalizeWorktreePath(match);
|
||||
normalizedCounts.set(normalized, (normalizedCounts.get(normalized) ?? 0) + 1);
|
||||
}
|
||||
|
||||
let best: { path: string; count: number } | null = null;
|
||||
for (const [path, count] of normalizedCounts.entries()) {
|
||||
if (!best || count > best.count) best = { path, count };
|
||||
}
|
||||
return best?.path ?? null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function isSessionWorktreeCompatible(
|
||||
persistedWorktreePath: string | null,
|
||||
currentWorktreePath: string,
|
||||
): boolean {
|
||||
if (!persistedWorktreePath) return true;
|
||||
return persistedWorktreePath === normalizeWorktreePath(currentWorktreePath);
|
||||
}
|
||||
|
||||
function truncateWorkflowScriptOutput(output: string): string {
|
||||
if (output.length <= WORKFLOW_SCRIPT_OUTPUT_MAX_CHARS) return output;
|
||||
return `... output truncated to last ${WORKFLOW_SCRIPT_OUTPUT_MAX_CHARS} characters ...\n${output.slice(-WORKFLOW_SCRIPT_OUTPUT_MAX_CHARS)}`;
|
||||
@@ -3183,8 +3219,26 @@ export class TaskExecutor {
|
||||
|
||||
// Determine whether we're resuming a previous session (pause/resume)
|
||||
// or starting fresh. Use file-based sessions so conversation state
|
||||
// persists across pause/unpause cycles.
|
||||
const isResuming = !!task.sessionFile && existsSync(task.sessionFile);
|
||||
// persists across pause/unpause cycles. Resume is allowed only when
|
||||
// persisted session metadata still matches the task's live worktree.
|
||||
let isResuming = !!task.sessionFile && existsSync(task.sessionFile);
|
||||
if (isResuming) {
|
||||
const persistedWorktreePath = await extractPersistedSessionWorktreePath(task.sessionFile!);
|
||||
if (!isSessionWorktreeCompatible(persistedWorktreePath, worktreePath)) {
|
||||
executorLog.warn(
|
||||
`${task.id}: stale sessionFile worktree mismatch (session=${persistedWorktreePath}, task=${worktreePath}); starting fresh session`,
|
||||
);
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`Detected stale persisted session metadata (worktree mismatch: ${persistedWorktreePath} vs ${worktreePath}) — discarded resume state and started fresh session`,
|
||||
undefined,
|
||||
this.currentRunContext,
|
||||
);
|
||||
await this.store.updateTask(task.id, { sessionFile: null });
|
||||
isResuming = false;
|
||||
}
|
||||
}
|
||||
|
||||
const sessionManager = isResuming
|
||||
? SessionManager.open(task.sessionFile!)
|
||||
: SessionManager.create(worktreePath);
|
||||
|
||||
@@ -15,11 +15,21 @@ function isNoTaskDoneFailure(task: Task): boolean {
|
||||
&& task.error.toLowerCase().includes("without calling fn_task_done");
|
||||
}
|
||||
|
||||
const MISSING_WORKTREE_SESSION_PREFIX = "Refusing to start coding agent in missing worktree:";
|
||||
|
||||
export function isMissingWorktreeSessionStartFailure(error: unknown): boolean {
|
||||
if (typeof error !== "string") {
|
||||
return false;
|
||||
}
|
||||
return error.includes("Refusing to start coding agent in missing worktree:");
|
||||
return error.includes(MISSING_WORKTREE_SESSION_PREFIX);
|
||||
}
|
||||
|
||||
export function extractMissingWorktreePathFromSessionStartFailure(error: unknown): string | null {
|
||||
if (typeof error !== "string") return null;
|
||||
const idx = error.indexOf(MISSING_WORKTREE_SESSION_PREFIX);
|
||||
if (idx < 0) return null;
|
||||
const pathPart = error.slice(idx + MISSING_WORKTREE_SESSION_PREFIX.length).trim();
|
||||
return pathPart.length > 0 ? pathPart : null;
|
||||
}
|
||||
|
||||
export function isRecoverableMissingWorktreeReviewFailure(task: Task): boolean {
|
||||
|
||||
@@ -21,7 +21,7 @@ import { getTaskMergeBlocker, isEphemeralAgent, type AgentStore, type TaskStore,
|
||||
import type { MeshLeaseManager } from "./mesh-lease-manager.js";
|
||||
import { createLogger } from "./logger.js";
|
||||
import { getRegisteredWorktreePaths, scanIdleWorktrees, scanOrphanedBranches } from "./worktree-pool.js";
|
||||
import { isRecoverableMissingWorktreeReviewFailure } from "./restart-recovery-coordinator.js";
|
||||
import { extractMissingWorktreePathFromSessionStartFailure, isMissingWorktreeSessionStartFailure, isRecoverableMissingWorktreeReviewFailure } from "./restart-recovery-coordinator.js";
|
||||
import { classifyError, extractMissingModulePath, isOperatorActionableAgentError, isStaleWorktreeModuleResolutionError } from "./transient-error-detector.js";
|
||||
|
||||
const log = createLogger("self-healing");
|
||||
@@ -1243,6 +1243,12 @@ export class SelfHealingManager {
|
||||
(blocker.mergeRetries ?? 0) >= MAX_AUTO_MERGE_RETRIES
|
||||
) {
|
||||
reason = `blocker ${blockerId} in-review + failed (mergeRetries ${blocker.mergeRetries ?? 0}/${MAX_AUTO_MERGE_RETRIES})`;
|
||||
} else if (
|
||||
blocker.column === "in-review" &&
|
||||
blocker.status === "failed" &&
|
||||
isMissingWorktreeSessionStartFailure(blocker.error)
|
||||
) {
|
||||
reason = `blocker ${blockerId} in-review + failed (missing-worktree session start)`;
|
||||
} else if (task.dependencies.length > 0 && !unresolvedDeps.includes(blockerId)) {
|
||||
reason = `blocker ${blockerId} not among unresolved dependencies`;
|
||||
}
|
||||
@@ -2650,16 +2656,24 @@ export class SelfHealingManager {
|
||||
for (const task of candidates) {
|
||||
try {
|
||||
const staleWorktree = task.worktree;
|
||||
const missingWorktreePath = extractMissingWorktreePathFromSessionStartFailure(task.error);
|
||||
const hasMismatchedLiveWorktree =
|
||||
typeof staleWorktree === "string" && staleWorktree.length > 0 &&
|
||||
typeof missingWorktreePath === "string" && missingWorktreePath.length > 0 &&
|
||||
resolve(staleWorktree) !== resolve(missingWorktreePath);
|
||||
|
||||
await this.store.updateTask(task.id, {
|
||||
status: null,
|
||||
error: null,
|
||||
worktree: null,
|
||||
branch: null,
|
||||
worktree: hasMismatchedLiveWorktree ? staleWorktree : null,
|
||||
branch: hasMismatchedLiveWorktree ? task.branch ?? null : null,
|
||||
sessionFile: null,
|
||||
});
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`Auto-recovered: retry/verification session targeted missing worktree${staleWorktree ? ` (${staleWorktree})` : ""} — cleared stale session metadata and requeued to todo`,
|
||||
hasMismatchedLiveWorktree
|
||||
? `Auto-recovered: stale resume referenced missing worktree (${missingWorktreePath}) while live task worktree is ${staleWorktree} — cleared stale session metadata and requeued to todo`
|
||||
: `Auto-recovered: retry/verification session targeted missing worktree${staleWorktree ? ` (${staleWorktree})` : ""} — cleared stale session metadata and requeued to todo`,
|
||||
);
|
||||
await this.store.moveTask(task.id, "todo", { preserveProgress: true });
|
||||
recovered++;
|
||||
|
||||
Reference in New Issue
Block a user