Merge pull request #1687 from Runfusion/fix/pause-abort-worktree-leak-storm
Fix pause-abort worktree-slot leak + retry storm; add board auto-recovery
This commit is contained in:
9
.changeset/fix-pause-abort-leak-storm.md
Normal file
9
.changeset/fix-pause-abort-leak-storm.md
Normal file
@@ -0,0 +1,9 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
Fix the global pause/resume failure mode that stalled the board: a pause-abort that left a task back in `todo` was parked `status:"failed"` ("operator action required") and leaked its in-memory worktree slot, producing an instant re-fail retry storm and concurrency-starving the whole queue.
|
||||
|
||||
- Root cause: `handleGraphFailure` now treats a pause-abort that has re-queued a task to `todo` as benign (FN-6782) — it no longer parks it failed, clears the `pausedAborted` marker so the next dispatch starts clean, and releases the leaked worktree slot.
|
||||
- Auto-recovery: a new `recoverPausedAbortFailures` self-healing sweep clears any pause-abort park (`status:"failed"` with "operator action required") still on the board and requeues it for normal scheduling, so the board self-heals without operator intervention.
|
||||
- Defense-in-depth: a new `reapLeakedConcurrencySlots` self-healing sweep reclaims any in-memory worktree slot whose holder is no longer in-progress (the "in todo yet still a `maxWorktrees` holder" leak), gated by the executor's live-session refusal so it can never pull a worktree out from under a running agent. This recovers a leaked slot from any future/unknown path without an engine restart.
|
||||
@@ -0,0 +1,98 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import "./executor-test-helpers.js";
|
||||
import { TaskExecutor } from "../executor.js";
|
||||
import { createMockStore, resetExecutorMocks } from "./executor-test-helpers.js";
|
||||
import type { TaskDetail } from "@fusion/core";
|
||||
|
||||
const now = "2026-06-20T00:00:00.000Z";
|
||||
|
||||
function makeTask(overrides: Partial<TaskDetail> = {}): TaskDetail {
|
||||
return {
|
||||
id: "FN-6782-T",
|
||||
title: "pause-abort benign todo repro",
|
||||
description: "Reproduces FN-6782 benign requeue-to-todo classification",
|
||||
column: "todo",
|
||||
dependencies: [],
|
||||
steps: [{ name: "Implement", status: "pending" }],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
branch: null,
|
||||
baseBranch: "main",
|
||||
worktree: "/tmp/fusion-fn-6782-t",
|
||||
status: null,
|
||||
error: null,
|
||||
paused: false,
|
||||
userPaused: false,
|
||||
autoMerge: true,
|
||||
mergeRetries: 0,
|
||||
createdAt: now,
|
||||
updatedAt: now,
|
||||
...overrides,
|
||||
} as TaskDetail;
|
||||
}
|
||||
|
||||
function makeHarness(taskOverrides: Partial<TaskDetail> = {}) {
|
||||
const store = createMockStore();
|
||||
const task = makeTask(taskOverrides);
|
||||
store.getTask.mockResolvedValue(task);
|
||||
store.getSettings.mockResolvedValue({
|
||||
maxConcurrent: 2,
|
||||
maxWorktrees: 4,
|
||||
pollIntervalMs: 15000,
|
||||
autoMerge: true,
|
||||
maxAutoMergeRetries: 3,
|
||||
});
|
||||
const executor = new TaskExecutor(store, "/tmp/test", {});
|
||||
(executor as any).markPausedAborted(task.id, "hard-cancel");
|
||||
return { store, task, executor };
|
||||
}
|
||||
|
||||
async function invokeGraphFailure(executor: TaskExecutor, task: TaskDetail) {
|
||||
await (executor as any).handleGraphFailure(task, {
|
||||
disposition: "failed",
|
||||
outcome: "failure",
|
||||
visitedNodeIds: ["plan", "execute"],
|
||||
context: {},
|
||||
});
|
||||
}
|
||||
|
||||
function logText(store: ReturnType<typeof createMockStore>): string {
|
||||
return store.logEntry.mock.calls.map((call: unknown[]) => call[1]).join("\n");
|
||||
}
|
||||
|
||||
describe("pause-abort benign requeue-to-todo (FN-6782)", () => {
|
||||
beforeEach(() => {
|
||||
resetExecutorMocks();
|
||||
});
|
||||
|
||||
it("does NOT park a todo-column pause-abort as failed (no retry storm)", async () => {
|
||||
const { store, task, executor } = makeHarness({ column: "todo" });
|
||||
(executor as any).activeWorktrees.set(task.id, task.worktree);
|
||||
|
||||
await invokeGraphFailure(executor, task);
|
||||
|
||||
// FNXC:WorkflowLifecycle a todo pause-abort must NOT write status:"failed" — that was the storm trigger.
|
||||
const parkedFailed = store.updateTask.mock.calls.some(
|
||||
(call: unknown[]) => (call[1] as { status?: string } | undefined)?.status === "failed",
|
||||
);
|
||||
expect(parkedFailed).toBe(false);
|
||||
// FNXC:WorkflowLifecycle the benign-clear log must surface for observability.
|
||||
expect(logText(store)).toContain("benign, cleared for normal scheduling");
|
||||
// FNXC:WorkflowLifecycle the pausedAborted marker must be cleared so the next dispatch starts clean.
|
||||
expect((executor as any).pausedAborted.has(task.id)).toBe(false);
|
||||
// FNXC:WorkflowLifecycle the leaked worktree slot must be released to avoid board-wide concurrency blockage.
|
||||
expect((executor as any).activeWorktrees.has(task.id)).toBe(false);
|
||||
});
|
||||
|
||||
it("STILL parks a non-todo (in-review) pause-abort as operator-action failed", async () => {
|
||||
const { store, task, executor } = makeHarness({ column: "in-review" });
|
||||
|
||||
await invokeGraphFailure(executor, task);
|
||||
|
||||
const parkedFailed = store.updateTask.mock.calls.some(
|
||||
(call: unknown[]) => (call[1] as { status?: string } | undefined)?.status === "failed",
|
||||
);
|
||||
expect(parkedFailed).toBe(true);
|
||||
expect(logText(store)).toContain("operator action required");
|
||||
});
|
||||
});
|
||||
@@ -1878,51 +1878,65 @@ describe("TaskExecutor bounded recovery retries", () => {
|
||||
expect(store.handoffToReview).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each(["todo", "done"] as const)(
|
||||
"surfaces paused graph exits in already-advanced %s column without lifecycle movement",
|
||||
async (column) => {
|
||||
const store = createMockStore();
|
||||
const task = {
|
||||
id: "FN-001",
|
||||
title: "Test",
|
||||
description: "Test",
|
||||
column: "in-progress",
|
||||
status: undefined,
|
||||
dependencies: [],
|
||||
steps: [{ name: "Preflight", status: "pending" }],
|
||||
currentStep: 0,
|
||||
log: [{ timestamp: new Date().toISOString(), action: "Resuming execution after unpause" }],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
} as Task;
|
||||
store.getTask.mockResolvedValue({
|
||||
...task,
|
||||
column,
|
||||
paused: true,
|
||||
status: undefined,
|
||||
error: null,
|
||||
});
|
||||
const executor = new TaskExecutor(store, "/tmp/test", {});
|
||||
function advancedColumnTask(): Task {
|
||||
return {
|
||||
id: "FN-001",
|
||||
title: "Test",
|
||||
description: "Test",
|
||||
column: "in-progress",
|
||||
status: undefined,
|
||||
dependencies: [],
|
||||
steps: [{ name: "Preflight", status: "pending" }],
|
||||
currentStep: 0,
|
||||
log: [{ timestamp: new Date().toISOString(), action: "Resuming execution after unpause" }],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
} as Task;
|
||||
}
|
||||
|
||||
await (executor as any).handleGraphFailure(task, {
|
||||
visitedNodeIds: ["execute"],
|
||||
});
|
||||
// FN-6782: a paused graph exit that already landed back in `todo` is BENIGN —
|
||||
// it must NOT be parked `failed` (that re-fail loop was the retry storm). It
|
||||
// logs a benign line, clears the pause-abort marker, and leaves the task in
|
||||
// todo for normal scheduling. (Previously this was surfaced as an
|
||||
// operator-action failure; see the `done` case below for the still-surfaced path.)
|
||||
it("treats a paused graph exit re-queued to todo as benign without parking failed", async () => {
|
||||
const store = createMockStore();
|
||||
const task = advancedColumnTask();
|
||||
store.getTask.mockResolvedValue({ ...task, column: "todo", paused: true, status: undefined, error: null });
|
||||
const executor = new TaskExecutor(store, "/tmp/test", {});
|
||||
|
||||
const expectedMessage = `Workflow graph failure surfaced after paused task pause in '${column}' at node 'execute' — operator action required; retry or explicitly unpause/resume after inspecting the task`;
|
||||
expect(store.logEntry).toHaveBeenCalledWith("FN-001", expectedMessage, undefined, undefined);
|
||||
if (column === "done") {
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith(
|
||||
"FN-001",
|
||||
expect.objectContaining({ status: "failed" }),
|
||||
expect.anything(),
|
||||
);
|
||||
} else {
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { error: expectedMessage, status: "failed" }, undefined);
|
||||
}
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
expect(store.handoffToReview).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
await (executor as any).handleGraphFailure(task, { visitedNodeIds: ["execute"] });
|
||||
|
||||
const benignMessage = "Workflow graph run ended during task pause with task re-queued to todo — benign, cleared for normal scheduling";
|
||||
expect(store.logEntry).toHaveBeenCalledWith("FN-001", benignMessage, undefined, undefined);
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith(
|
||||
"FN-001",
|
||||
expect.objectContaining({ status: "failed" }),
|
||||
expect.anything(),
|
||||
);
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
expect(store.handoffToReview).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("surfaces a paused graph exit in an already-advanced done column without parking failed", async () => {
|
||||
const store = createMockStore();
|
||||
const task = advancedColumnTask();
|
||||
store.getTask.mockResolvedValue({ ...task, column: "done", paused: true, status: undefined, error: null });
|
||||
const executor = new TaskExecutor(store, "/tmp/test", {});
|
||||
|
||||
await (executor as any).handleGraphFailure(task, { visitedNodeIds: ["execute"] });
|
||||
|
||||
const expectedMessage = "Workflow graph failure surfaced after paused task pause in 'done' at node 'execute' — operator action required; retry or explicitly unpause/resume after inspecting the task";
|
||||
expect(store.logEntry).toHaveBeenCalledWith("FN-001", expectedMessage, undefined, undefined);
|
||||
// done/archived are terminal — surfaced via log only, never parked failed.
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith(
|
||||
"FN-001",
|
||||
expect.objectContaining({ status: "failed" }),
|
||||
expect.anything(),
|
||||
);
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
expect(store.handoffToReview).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
describe("merge-seam abort classification (FN-6568)", () => {
|
||||
/*
|
||||
|
||||
@@ -0,0 +1,183 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { EventEmitter } from "node:events";
|
||||
|
||||
const { logger } = vi.hoisted(() => ({ logger: { log: vi.fn(), warn: vi.fn(), error: vi.fn() } }));
|
||||
vi.mock("../logger.js", () => ({
|
||||
createLogger: vi.fn(() => logger),
|
||||
schedulerLog: { log: vi.fn(), warn: vi.fn(), error: vi.fn() },
|
||||
}));
|
||||
|
||||
vi.mock("../worktree-pool.js", () => ({
|
||||
WorktreePool: vi.fn(),
|
||||
RemovalReason: {},
|
||||
scanIdleWorktrees: vi.fn().mockResolvedValue([]),
|
||||
cleanupOrphanedWorktrees: vi.fn().mockResolvedValue(0),
|
||||
isUsableTaskWorktree: vi.fn().mockResolvedValue(true),
|
||||
removeWorktree: vi.fn().mockResolvedValue(undefined),
|
||||
resolveWorktreeBackend: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../merger.js", () => ({ classifyOwnedLandedEvidence: vi.fn() }));
|
||||
|
||||
import { SelfHealingManager } from "../self-healing.js";
|
||||
import type { Settings, Task, TaskStore } from "@fusion/core";
|
||||
|
||||
function createMockStore(overrides: Record<string, unknown> = {}): TaskStore & EventEmitter {
|
||||
const emitter = new EventEmitter();
|
||||
return Object.assign(emitter, {
|
||||
getSettings: vi.fn().mockResolvedValue({
|
||||
autoMerge: true,
|
||||
globalPause: false,
|
||||
enginePaused: false,
|
||||
maintenanceIntervalMs: 0,
|
||||
} as unknown as Settings),
|
||||
listTasks: vi.fn().mockResolvedValue([]),
|
||||
getTask: vi.fn(),
|
||||
updateTask: vi.fn().mockResolvedValue({} as Task),
|
||||
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||
moveTask: vi.fn().mockResolvedValue(undefined),
|
||||
recordRunAuditEvent: vi.fn().mockResolvedValue(undefined),
|
||||
getRootDir: vi.fn().mockReturnValue("/tmp/test-project"),
|
||||
...overrides,
|
||||
}) as unknown as TaskStore & EventEmitter;
|
||||
}
|
||||
|
||||
// A task that has sat in its column well past the reaper's 60s grace.
|
||||
function taskRow(id: string, column: string, extra: Record<string, unknown> = {}): Task {
|
||||
return {
|
||||
id,
|
||||
column,
|
||||
paused: false,
|
||||
columnMovedAt: "2026-05-20T12:00:00.000Z",
|
||||
updatedAt: "2026-05-20T12:00:00.000Z",
|
||||
steps: [],
|
||||
...extra,
|
||||
} as unknown as Task;
|
||||
}
|
||||
|
||||
describe("reapLeakedConcurrencySlots", () => {
|
||||
let store: TaskStore & EventEmitter;
|
||||
let clearPhantomExecutorBinding: ReturnType<typeof vi.fn>;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
// Well past the 60s grace relative to the task rows' columnMovedAt.
|
||||
vi.setSystemTime(new Date("2026-05-20T12:05:00.000Z"));
|
||||
clearPhantomExecutorBinding = vi.fn().mockReturnValue(true);
|
||||
store = createMockStore();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
function makeManager(holders: Array<{ taskId: string; worktreePath: string }>, executing: string[] = []) {
|
||||
return new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
listWorktreeHolders: () => holders,
|
||||
getExecutingTaskIds: () => new Set<string>(executing),
|
||||
clearPhantomExecutorBinding: clearPhantomExecutorBinding as (taskId: string) => boolean | void,
|
||||
});
|
||||
}
|
||||
|
||||
it("releases a leaked slot whose holder sits in todo and is not executing", async () => {
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(taskRow("FN-6756", "todo"));
|
||||
const manager = makeManager([{ taskId: "FN-6756", worktreePath: "/wt/pearl-lark" }]);
|
||||
|
||||
const reaped = await manager.reapLeakedConcurrencySlots();
|
||||
|
||||
expect(reaped).toBe(1);
|
||||
expect(clearPhantomExecutorBinding).toHaveBeenCalledWith("FN-6756");
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-6756",
|
||||
expect.stringContaining("released leaked worktree/concurrency slot"),
|
||||
);
|
||||
});
|
||||
|
||||
it("does NOT release a legit in-progress holder", async () => {
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(taskRow("FN-6750", "in-progress"));
|
||||
const manager = makeManager([{ taskId: "FN-6750", worktreePath: "/wt/proud-delta" }]);
|
||||
|
||||
const reaped = await manager.reapLeakedConcurrencySlots();
|
||||
|
||||
expect(reaped).toBe(0);
|
||||
expect(clearPhantomExecutorBinding).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does NOT release a holder that is still executing, even if its column looks reapable", async () => {
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(taskRow("FN-6700", "todo"));
|
||||
const manager = makeManager([{ taskId: "FN-6700", worktreePath: "/wt/fast-falcon" }], ["FN-6700"]);
|
||||
|
||||
const reaped = await manager.reapLeakedConcurrencySlots();
|
||||
|
||||
expect(reaped).toBe(0);
|
||||
expect(clearPhantomExecutorBinding).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does NOT release an in-review holder (conservative — it legitimately keeps its worktree)", async () => {
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(taskRow("FN-6744", "in-review"));
|
||||
const manager = makeManager([{ taskId: "FN-6744", worktreePath: "/wt/jade-grove" }]);
|
||||
|
||||
const reaped = await manager.reapLeakedConcurrencySlots();
|
||||
|
||||
expect(reaped).toBe(0);
|
||||
expect(clearPhantomExecutorBinding).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does NOT release a todo holder still inside the grace window", async () => {
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(
|
||||
taskRow("FN-6753", "todo", { columnMovedAt: "2026-05-20T12:04:30.000Z" }), // 30s < 60s grace
|
||||
);
|
||||
const manager = makeManager([{ taskId: "FN-6753", worktreePath: "/wt/swift-falcon" }]);
|
||||
|
||||
const reaped = await manager.reapLeakedConcurrencySlots();
|
||||
|
||||
expect(reaped).toBe(0);
|
||||
expect(clearPhantomExecutorBinding).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("skips entirely under global pause", async () => {
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
globalPause: true,
|
||||
enginePaused: false,
|
||||
} as unknown as Settings);
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(taskRow("FN-6756", "todo"));
|
||||
const manager = makeManager([{ taskId: "FN-6756", worktreePath: "/wt/pearl-lark" }]);
|
||||
|
||||
const reaped = await manager.reapLeakedConcurrencySlots();
|
||||
|
||||
expect(reaped).toBe(0);
|
||||
expect(clearPhantomExecutorBinding).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("reaps an orphan holder with no task row", async () => {
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(null);
|
||||
const manager = makeManager([{ taskId: "FN-ghost", worktreePath: "/wt/gone" }]);
|
||||
|
||||
const reaped = await manager.reapLeakedConcurrencySlots();
|
||||
|
||||
expect(reaped).toBe(1);
|
||||
expect(clearPhantomExecutorBinding).toHaveBeenCalledWith("FN-ghost");
|
||||
});
|
||||
|
||||
// FNXC:WorkflowLifecycle coderabbit Major (PR #1687): a holder that starts
|
||||
// executing AFTER the pre-loop snapshot but BEFORE the release must be spared
|
||||
// by the fresh executing-set re-check.
|
||||
it("does NOT release a holder that starts executing mid-sweep", async () => {
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(taskRow("FN-6760", "todo"));
|
||||
let call = 0;
|
||||
const manager = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
listWorktreeHolders: () => [{ taskId: "FN-6760", worktreePath: "/wt/calm-iris" }],
|
||||
// Pre-loop snapshot is empty; by the fresh re-check the task is executing.
|
||||
getExecutingTaskIds: () => new Set<string>(call++ === 0 ? [] : ["FN-6760"]),
|
||||
clearPhantomExecutorBinding: clearPhantomExecutorBinding as (taskId: string) => boolean | void,
|
||||
});
|
||||
|
||||
const reaped = await manager.reapLeakedConcurrencySlots();
|
||||
|
||||
expect(reaped).toBe(0);
|
||||
expect(clearPhantomExecutorBinding).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,157 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { EventEmitter } from "node:events";
|
||||
|
||||
vi.mock("node:fs", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("node:fs")>();
|
||||
return { ...actual, existsSync: vi.fn(actual.existsSync) };
|
||||
});
|
||||
|
||||
const { logger } = vi.hoisted(() => ({ logger: { log: vi.fn(), warn: vi.fn(), error: vi.fn() } }));
|
||||
vi.mock("../logger.js", () => ({
|
||||
createLogger: vi.fn(() => logger),
|
||||
schedulerLog: { log: vi.fn(), warn: vi.fn(), error: vi.fn() },
|
||||
}));
|
||||
|
||||
vi.mock("../worktree-pool.js", () => ({
|
||||
WorktreePool: vi.fn(),
|
||||
RemovalReason: {},
|
||||
scanIdleWorktrees: vi.fn().mockResolvedValue([]),
|
||||
cleanupOrphanedWorktrees: vi.fn().mockResolvedValue(0),
|
||||
isUsableTaskWorktree: vi.fn().mockResolvedValue(true),
|
||||
removeWorktree: vi.fn().mockResolvedValue(undefined),
|
||||
resolveWorktreeBackend: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../merger.js", () => ({ classifyOwnedLandedEvidence: vi.fn() }));
|
||||
|
||||
import { SelfHealingManager } from "../self-healing.js";
|
||||
import type { Settings, Task, TaskStore } from "@fusion/core";
|
||||
|
||||
const PARK_ERROR =
|
||||
"Workflow graph failure surfaced after paused engine abort during pause/resume in 'todo' at node 'execute' — operator action required; retry or explicitly unpause/resume after inspecting the task";
|
||||
|
||||
function createMockStore(tasks: Task[]): TaskStore & EventEmitter {
|
||||
const emitter = new EventEmitter();
|
||||
const byId = new Map(tasks.map((t) => [t.id, t]));
|
||||
return Object.assign(emitter, {
|
||||
getSettings: vi.fn().mockResolvedValue({
|
||||
autoMerge: true,
|
||||
globalPause: false,
|
||||
enginePaused: false,
|
||||
maintenanceIntervalMs: 0,
|
||||
} as unknown as Settings),
|
||||
listTasks: vi.fn().mockResolvedValue(tasks),
|
||||
getTask: vi.fn(async (id: string) => byId.get(id) ?? null),
|
||||
updateTask: vi.fn().mockResolvedValue({} as Task),
|
||||
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||
moveTask: vi.fn().mockResolvedValue(undefined),
|
||||
recordRunAuditEvent: vi.fn().mockResolvedValue(undefined),
|
||||
getRootDir: vi.fn().mockReturnValue("/tmp/test-project"),
|
||||
}) as unknown as TaskStore & EventEmitter;
|
||||
}
|
||||
|
||||
function parkTask(overrides: Partial<Task> = {}): Task {
|
||||
return {
|
||||
id: "FN-7000",
|
||||
column: "todo",
|
||||
paused: false,
|
||||
userPaused: false,
|
||||
status: "failed",
|
||||
error: PARK_ERROR,
|
||||
steps: [{ status: "pending" }],
|
||||
title: "parked task",
|
||||
...overrides,
|
||||
} as unknown as Task;
|
||||
}
|
||||
|
||||
describe("recoverPausedAbortFailures", () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
vi.setSystemTime(new Date("2026-06-20T02:30:00.000Z"));
|
||||
});
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("clears a todo-column pause-abort park to schedulable (status:null) without moving it", async () => {
|
||||
const store = createMockStore([parkTask({ id: "FN-7000", column: "todo" })]);
|
||||
const clearBinding = vi.fn().mockReturnValue(true);
|
||||
const manager = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
getExecutingTaskIds: () => new Set<string>(),
|
||||
clearPhantomExecutorBinding: clearBinding as (taskId: string) => boolean | void,
|
||||
});
|
||||
|
||||
const recovered = await manager.recoverPausedAbortFailures();
|
||||
|
||||
expect(recovered).toBe(1);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-7000", { status: null, error: null });
|
||||
// Already in todo — must NOT be moved.
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
// FNXC:WorkflowLifecycle A1 releases via the wired clearPhantomExecutorBinding,
|
||||
// not the dead releaseExecutorWorktreeOwnership option (PR #1687 review).
|
||||
expect(clearBinding).toHaveBeenCalledWith("FN-7000");
|
||||
expect(store.recordRunAuditEvent).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ mutationType: "task:auto-recover-paused-abort-park", target: "FN-7000" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("rehomes an in-progress pause-abort park back to todo", async () => {
|
||||
const store = createMockStore([parkTask({ id: "FN-7001", column: "in-progress" })]);
|
||||
const manager = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
getExecutingTaskIds: () => new Set<string>(),
|
||||
});
|
||||
|
||||
const recovered = await manager.recoverPausedAbortFailures();
|
||||
|
||||
expect(recovered).toBe(1);
|
||||
expect(store.moveTask).toHaveBeenCalledWith(
|
||||
"FN-7001",
|
||||
"todo",
|
||||
{ preserveProgress: true, moveSource: "engine", recoveryRehome: true },
|
||||
);
|
||||
});
|
||||
|
||||
it("skips paused, executing, in-review, and non-pause-abort failures", async () => {
|
||||
const store = createMockStore([
|
||||
parkTask({ id: "FN-A", paused: true }),
|
||||
parkTask({ id: "FN-B", column: "in-progress" }), // executing (below)
|
||||
parkTask({ id: "FN-C", column: "in-review" }), // in-review park left for operator
|
||||
parkTask({ id: "FN-D", error: "some other failure", status: "failed" }),
|
||||
]);
|
||||
const manager = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
getExecutingTaskIds: () => new Set<string>(["FN-B"]),
|
||||
});
|
||||
|
||||
const recovered = await manager.recoverPausedAbortFailures();
|
||||
|
||||
expect(recovered).toBe(0);
|
||||
expect(store.updateTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// FNXC:WorkflowLifecycle greptile P1 (PR #1687): the method self-guards on
|
||||
// global/engine pause at its own entry, so calling it directly (test/API path)
|
||||
// while the operator has frozen the board must be a no-op.
|
||||
it("self-guards: does nothing while globalPause is set", async () => {
|
||||
const store = createMockStore([parkTask({ id: "FN-7000", column: "todo" })]);
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
autoMerge: true,
|
||||
globalPause: true,
|
||||
enginePaused: false,
|
||||
maintenanceIntervalMs: 0,
|
||||
} as unknown as Settings);
|
||||
const manager = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
getExecutingTaskIds: () => new Set<string>(),
|
||||
});
|
||||
|
||||
const recovered = await manager.recoverPausedAbortFailures();
|
||||
|
||||
expect(recovered).toBe(0);
|
||||
expect(store.updateTask).not.toHaveBeenCalled();
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -158,7 +158,7 @@ import {
|
||||
isMissingWorktreeSessionStartFailure,
|
||||
} from "./restart-recovery-coordinator.js";
|
||||
import { BranchWorktreeAutoRecoveryHandler } from "./auto-recovery-handlers/branch-worktree.js";
|
||||
import { autoRecoverWorktreeSessionStartFailure, MAX_WORKTREE_SESSION_RETRIES } from "./self-healing.js";
|
||||
import { autoRecoverWorktreeSessionStartFailure, MAX_WORKTREE_SESSION_RETRIES, PAUSE_ABORT_PARK_ERROR_MARKER, PAUSE_ABORT_PARK_OPERATOR_MARKER } from "./self-healing.js";
|
||||
import { ContaminationAutoRecoveryHandler } from "./auto-recovery-handlers/contamination.js";
|
||||
import { createFileScopeAutoRecoveryHandler } from "./auto-recovery-handlers/file-scope.js";
|
||||
import { ReadonlyViolationError, filterCustomToolsForReadonly } from "./workflow-step-tool-policy.js";
|
||||
@@ -6640,8 +6640,39 @@ export class TaskExecutor {
|
||||
? "engine abort during pause/resume"
|
||||
: "task pause";
|
||||
if (live.column !== "in-progress") {
|
||||
// FN-6782: a pause/resume abort that has left the task back in `todo`
|
||||
// is benign — the work is simply re-queued for a fresh dispatch, not
|
||||
// stranded. Parking it `status: "failed"` (operator action required)
|
||||
// here is what caused the retry storm: the scheduler re-dispatches the
|
||||
// todo task, this branch re-fires on the still-set pausedAborted
|
||||
// marker, and it re-parks instantly with no backoff. Treat `todo` like
|
||||
// the in-progress benign case: clear the abort marker so the next
|
||||
// dispatch starts clean, log, and return WITHOUT parking failed. The
|
||||
// operator-action failure is preserved only for genuinely stranded
|
||||
// non-todo columns (e.g. in-review), per FN-6478.
|
||||
if (live.column === "todo") {
|
||||
this.clearPausedAborted(task.id);
|
||||
// FNXC:WorkflowLifecycle 2026-06-20-00:00: FN-6782 leak fix — a task
|
||||
// parked back to `todo` must not keep pinning its in-memory worktree
|
||||
// slot. The execute() finally does not delete activeWorktrees on this
|
||||
// early-return path, so without this release the slot leaks — a `todo`
|
||||
// task stays a maxWorktrees holder and concurrency-blocks the whole
|
||||
// queue (the FN-6756 "in todo yet still a holder, maxWorktrees=3/3"
|
||||
// symptom). Mirror clearPhantomExecutorBinding's release semantics.
|
||||
// Safe here: handleGraphFailure is terminal for this run (no seam
|
||||
// re-entry), and the next dispatch re-acquires a fresh worktree.
|
||||
this.activeWorktrees.delete(task.id);
|
||||
const todoBenign = `Workflow graph run ended during ${pauseProvenance} with task re-queued to todo — benign, cleared for normal scheduling`;
|
||||
executorLog.log(`${task.id}: ${todoBenign}`);
|
||||
await this.store.logEntry(task.id, todoBenign, undefined, this.getRunContextFor(task.id));
|
||||
await this.persistTokenUsage(task.id);
|
||||
return;
|
||||
}
|
||||
const failedNode = result.visitedNodeIds[result.visitedNodeIds.length - 1] ?? "unknown";
|
||||
const message = `Workflow graph failure surfaced after paused ${pauseProvenance} in '${live.column}' at node '${failedNode}' — operator action required; retry or explicitly unpause/resume after inspecting the task`;
|
||||
// FNXC:WorkflowLifecycle 2026-06-20-00:00: build the parked-failure
|
||||
// message from the shared markers so self-healing's recoverPausedAbortFailures
|
||||
// predicate cannot drift out of sync with this text (PR #1687 review).
|
||||
const message = `${PAUSE_ABORT_PARK_ERROR_MARKER} ${pauseProvenance} in '${live.column}' at node '${failedNode}' — ${PAUSE_ABORT_PARK_OPERATOR_MARKER}; retry or explicitly unpause/resume after inspecting the task`;
|
||||
executorLog.warn(`${task.id}: ${message}`);
|
||||
await this.store.logEntry(task.id, message, undefined, this.getRunContextFor(task.id));
|
||||
if (live.column !== "done" && live.column !== "archived" && live.status == null && live.error == null) {
|
||||
@@ -13958,6 +13989,25 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
* The requesting task is excluded from the check because `cleanupConflictingWorktree` is
|
||||
* only called for worktrees the requesting task is trying to displace.
|
||||
*/
|
||||
/**
|
||||
* FN-6782 leaked-slot reaper support: expose a read-only snapshot of the
|
||||
* in-memory `activeWorktrees` holders so SelfHealingManager can cross-check
|
||||
* each holder's task column and reclaim a slot whose holder is no longer
|
||||
* legitimately in-progress (the "in todo yet still maxWorktrees holder"
|
||||
* leak). Returns a copied array — never the live Map — so callers cannot
|
||||
* mutate executor state. The actual release still goes through
|
||||
* `clearPhantomExecutorBinding`, which refuses to detach live session
|
||||
* surfaces, so this introspection cannot by itself pull a worktree out from
|
||||
* under a running agent.
|
||||
*/
|
||||
listWorktreeHolders(): Array<{ taskId: string; worktreePath: string }> {
|
||||
const holders: Array<{ taskId: string; worktreePath: string }> = [];
|
||||
for (const [taskId, worktreePath] of this.activeWorktrees) {
|
||||
holders.push({ taskId, worktreePath });
|
||||
}
|
||||
return holders;
|
||||
}
|
||||
|
||||
private async findActiveWorktreeOwner(
|
||||
worktreePath: string,
|
||||
requestingTaskId: string,
|
||||
|
||||
@@ -459,6 +459,10 @@ export type DatabaseMutationType =
|
||||
| "task:auto-recover-worktree-metadata-rebound"
|
||||
| "task:auto-recover-worktree-metadata-cleared"
|
||||
| "task:auto-recover-worktree-metadata-skipped-active"
|
||||
// FNXC:Lifecycle FNXC_LOG 2026-06-20-00:00: FN-6782 — audit type for a global pause/resume park that was cleared and requeued by self-healing.
|
||||
| "task:auto-recover-paused-abort-park"
|
||||
// FNXC:Lifecycle FNXC_LOG 2026-06-20-00:00: audit type for reaping a leaked worktree/lease/semaphore slot whose holder left in-progress.
|
||||
| "task:reap-leaked-concurrency-slot"
|
||||
// task:auto-archived-ghost-bug metadata: { findings: Array<{ construct: { kind: string; raw: string; filePath?: string; line?: number }; matched: boolean; probeError?: string; output?: string }>; reason: string }
|
||||
// task:auto-archived-duplicate metadata: { siblingTaskIds: string[]; scores: Record<string, number> }
|
||||
| "task:auto-archived-ghost-bug"
|
||||
|
||||
@@ -788,6 +788,7 @@ export class InProcessRuntime
|
||||
recoverFailedPreMergeStep: (task) => this.executor.recoverFailedPreMergeWorkflowStep(task),
|
||||
getExecutingTaskIds: () => this.executor?.getExecutingTaskIds() ?? new Set<string>(),
|
||||
clearPhantomExecutorBinding: (taskId: string) => this.executor?.clearPhantomExecutorBinding(taskId),
|
||||
listWorktreeHolders: () => this.executor?.listWorktreeHolders() ?? [],
|
||||
recoverApprovedTriageTask: (task) => this.triageProcessor?.recoverApprovedTask(task) ?? Promise.resolve(false),
|
||||
getPlanningTaskIds: () => this.triageProcessor?.getProcessingTaskIds() ?? new Set<string>(),
|
||||
evictStaleTriageProcessing: () => this.triageProcessor?.evictStaleProcessing() ?? new Set<string>(),
|
||||
|
||||
@@ -242,8 +242,19 @@ export interface SelfHealingOptions {
|
||||
rootDir: string;
|
||||
/** Optional callback to release TaskExecutor in-memory worktree ownership for a task. */
|
||||
releaseExecutorWorktreeOwnership?: (taskId: string) => void;
|
||||
/** Optional callback to clear a demonstrably-stale executor binding without touching live sessions. */
|
||||
clearPhantomExecutorBinding?: (taskId: string) => void;
|
||||
/**
|
||||
* FN-6782: read-only snapshot of the executor's in-memory worktree holders
|
||||
* ({ taskId, worktreePath }), so the leaked-slot reaper can cross-check each
|
||||
* holder's task column and reclaim a slot whose holder is no longer in-progress.
|
||||
*/
|
||||
listWorktreeHolders?: () => Array<{ taskId: string; worktreePath: string }>;
|
||||
/**
|
||||
* Optional callback to clear a demonstrably-stale executor binding without
|
||||
* touching live sessions. Returns `true` if the binding was cleared, `false`
|
||||
* if the executor refused because a live session surface is still registered
|
||||
* (the leaked-slot reaper relies on this refusal signal).
|
||||
*/
|
||||
clearPhantomExecutorBinding?: (taskId: string) => boolean | void;
|
||||
/** Optional AgentStore for agent-level self-healing checks. */
|
||||
agentStore?: AgentStore;
|
||||
/** Canonical stale-lease recovery manager. */
|
||||
@@ -357,6 +368,12 @@ const STARVED_REFINEMENT_RECOVERY_GRACE_MS = 10 * 60_000;
|
||||
const STARVED_PEER_PROGRESS_THRESHOLD = 3;
|
||||
const STARVED_REFINEMENT_ESCALATION_COOLDOWN_MS = STARVED_REFINEMENT_RECOVERY_GRACE_MS * 4;
|
||||
const ORPHANED_EXECUTION_RECOVERY_GRACE_MS = 60_000;
|
||||
/**
|
||||
* FN-6782 leaked-slot reaper grace: a worktree holder whose task has sat in a
|
||||
* reapable column (todo/triage) shorter than this is left alone, so the reaper
|
||||
* never races a task mid-transition out of in-progress.
|
||||
*/
|
||||
const LEAKED_WORKTREE_SLOT_GRACE_MS = 60_000;
|
||||
export const VALIDATOR_RUN_STALE_MAX_AGE_MS = 6 * 60 * 60 * 1000;
|
||||
const ACTIVE_MERGE_STATUSES = new Set(["merging", "merging-pr", "merging-fix"]);
|
||||
const NON_TERMINAL_STEP_STATUSES = new Set(["pending", "in-progress"]);
|
||||
@@ -395,6 +412,16 @@ const ORPHANED_WITH_WORKTREE_GRACE_MS = 300_000;
|
||||
*/
|
||||
const MAX_TASK_DONE_RETRIES = 3;
|
||||
export const MAX_WORKTREE_SESSION_RETRIES = 3;
|
||||
/**
|
||||
* FNXC:WorkflowLifecycle 2026-06-20-00:00: single source of truth for the
|
||||
* pause-abort park error message markers. The executor's handleGraphFailure
|
||||
* builds the parked-failure message from these, and `recoverPausedAbortFailures`
|
||||
* matches on them — sharing the constants prevents the recovery predicate from
|
||||
* silently drifting if the message text is ever edited (greptile review on
|
||||
* PR #1687: a string-coupled predicate breaks with no compile-time signal).
|
||||
*/
|
||||
export const PAUSE_ABORT_PARK_ERROR_MARKER = "Workflow graph failure surfaced after paused";
|
||||
export const PAUSE_ABORT_PARK_OPERATOR_MARKER = "operator action required";
|
||||
/**
|
||||
* FNXC:AutoMergeRetries 2026-06-17-04:20:
|
||||
* Keep this export as the historical default seed for tests and dashboard fallback alignment, but SelfHealingManager must call resolveMaxAutoMergeRetries(settings) at decision points so configured projects do not recover or stall at the old fixed value.
|
||||
@@ -2111,6 +2138,7 @@ export class SelfHealingManager {
|
||||
{ name: "recover-misclassified-failures", fn: () => this.recoverMisclassifiedFailures() },
|
||||
{ name: "recover-missing-worktree-review-failures", fn: () => this.recoverMissingWorktreeReviewFailures() },
|
||||
{ name: "recover-no-progress-no-task-done", fn: () => this.recoverNoProgressNoTaskDoneFailures() },
|
||||
{ name: "recover-paused-abort-failures", fn: () => this.recoverPausedAbortFailures() },
|
||||
{ name: "recover-partial-progress-no-task-done", fn: () => this.recoverPartialProgressNoTaskDoneFailures() },
|
||||
{ name: "recover-orphaned-executions", fn: () => this.recoverOrphanedExecutions() },
|
||||
{ name: "recover-approved-triage", fn: () => this.recoverApprovedTriageTasks() },
|
||||
@@ -2135,6 +2163,10 @@ export class SelfHealingManager {
|
||||
{ name: "recover-stale-transition-pending", fn: () => this.runStaleTransitionPendingSweep() },
|
||||
{ name: "reconcile-self-defeating-deps", fn: () => this.reconcileSelfDefeatingDependencies() },
|
||||
{ name: "reconcile-dependency-blocking-leases", fn: () => this.reconcileDependencyBlockingLeases() },
|
||||
// FN-6782: reclaim in-memory worktree slots whose holder is no longer
|
||||
// in-progress (defense-in-depth for the pause-abort leak; conservative,
|
||||
// gated by clearPhantomExecutorBinding's live-session refusal).
|
||||
{ name: "reap-leaked-concurrency-slots", fn: () => this.reapLeakedConcurrencySlots() },
|
||||
{ name: "reconcile-dependency-cycles", fn: () => this.reconcileDependencyCycles().then(() => undefined) },
|
||||
{ name: "reclaim-pr-conflicts", fn: () => this.reclaimPrConflicts() },
|
||||
{ name: "reclaim-self-owned-branch-conflicts", fn: () => this.reclaimSelfOwnedBranchConflicts() },
|
||||
@@ -7874,6 +7906,130 @@ export class SelfHealingManager {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* FN-6782 / pause-abort auto-recovery: a global pause/resume cycle can leave a
|
||||
* task parked `status: "failed"` with the "operator action required" message
|
||||
* produced by the executor's genuine-pause-abort branch (executor.ts
|
||||
* handleGraphFailure). Historically that required a human to retry/unpause.
|
||||
* This sweep auto-recovers those parks: it clears the failed status/error and
|
||||
* rehomes the task to `todo` with `status: null`. The scheduler treats a
|
||||
* non-paused `todo` task with `status: null` as runnable (scheduler.ts builds
|
||||
* its dispatch set from `column === "todo" && !paused`; `status: "queued"` is
|
||||
* the *blocked* marker, not the runnable one), so clearing to null is what
|
||||
* makes the task schedulable again.
|
||||
*
|
||||
* Guards mirror the other failed-task recoverers: never touch a task that is
|
||||
* paused, currently executing, or whose error is not the pause-abort park.
|
||||
*/
|
||||
async recoverPausedAbortFailures(): Promise<number> {
|
||||
try {
|
||||
// FNXC:WorkflowLifecycle 2026-06-20-00:00: self-guard against global/engine
|
||||
// pause at the method entry, not just the batch-2 runner. This method is
|
||||
// public and exercised directly (tests, potential API path); without this,
|
||||
// calling it while paused would requeue tasks the operator intentionally
|
||||
// froze (greptile P1, PR #1687). Mirrors every peer recovery func.
|
||||
const settings = await this.store.getSettings();
|
||||
if (settings.globalPause || settings.enginePaused) return 0;
|
||||
|
||||
const executingIds = this.options.getExecutingTaskIds?.() ?? new Set<string>();
|
||||
const tasks = await this.store.listTasks({ slim: true });
|
||||
|
||||
const isPausedAbortPark = (t: Task): boolean =>
|
||||
t.status === "failed" &&
|
||||
typeof t.error === "string" &&
|
||||
t.error.includes(PAUSE_ABORT_PARK_OPERATOR_MARKER) &&
|
||||
t.error.includes(PAUSE_ABORT_PARK_ERROR_MARKER);
|
||||
|
||||
const parked = tasks.filter((t) =>
|
||||
isPausedAbortPark(t) &&
|
||||
!t.paused &&
|
||||
!t.userPaused &&
|
||||
!executingIds.has(t.id) &&
|
||||
// Only recover columns that are safe to requeue. done/archived parks are
|
||||
// terminal and in-review parks may carry merge state — leave those for
|
||||
// the existing review recoverers / operator inspection.
|
||||
(t.column === "todo" || t.column === "in-progress"),
|
||||
);
|
||||
|
||||
if (parked.length === 0) return 0;
|
||||
|
||||
log.warn(`Found ${parked.length} pause-abort park(s) requiring auto-recovery`);
|
||||
|
||||
let recovered = 0;
|
||||
for (const task of parked) {
|
||||
try {
|
||||
// FNXC:WorkflowLifecycle 2026-06-20-00:00: re-read AND re-validate the
|
||||
// FULL predicate against the refreshed row with a FRESH executing set
|
||||
// before mutating — the outer snapshot can go stale across awaits, so a
|
||||
// task that became ineligible (paused, user-paused, started executing,
|
||||
// or moved to a non-recoverable column) must not get a backward move
|
||||
// applied (coderabbit Major + greptile, PR #1687).
|
||||
const fresh = await this.store.getTask(task.id);
|
||||
const latestExecutingIds = this.options.getExecutingTaskIds?.() ?? new Set<string>();
|
||||
if (
|
||||
!fresh ||
|
||||
!isPausedAbortPark(fresh) ||
|
||||
fresh.paused ||
|
||||
fresh.userPaused ||
|
||||
latestExecutingIds.has(fresh.id) ||
|
||||
!(fresh.column === "todo" || fresh.column === "in-progress")
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
|
||||
await this.store.updateTask(task.id, { status: null, error: null });
|
||||
if (fresh.column !== "todo") {
|
||||
await this.store.moveTask(task.id, "todo", {
|
||||
preserveProgress: true,
|
||||
moveSource: "engine",
|
||||
recoveryRehome: true,
|
||||
});
|
||||
}
|
||||
// Release any in-memory worktree ownership the leaked park may still
|
||||
// pin, so the requeued task does not re-block the concurrency gate.
|
||||
// FNXC:WorkflowLifecycle 2026-06-20-00:00: use clearPhantomExecutorBinding
|
||||
// (wired + live-session-refusal guarded), NOT releaseExecutorWorktreeOwnership
|
||||
// which is a declared-but-never-wired option — it would silently no-op.
|
||||
this.options.clearPhantomExecutorBinding?.(task.id);
|
||||
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
"Auto-recovered: pause-abort park cleared — requeued for normal scheduling",
|
||||
);
|
||||
// FNXC:WorkflowLifecycle 2026-06-20-00:00: audit emission is strictly
|
||||
// best-effort — an audit throw AFTER the successful state mutation must
|
||||
// not drop into the per-task catch and falsely log "recovery failed" /
|
||||
// skip the recovered++ (coderabbit, PR #1687).
|
||||
try {
|
||||
await this.store.recordRunAuditEvent?.({
|
||||
taskId: task.id,
|
||||
agentId: "self-healing",
|
||||
runId: generateSyntheticRunId("self-healing", task.id),
|
||||
domain: "database",
|
||||
mutationType: "task:auto-recover-paused-abort-park",
|
||||
target: task.id,
|
||||
metadata: { fromColumn: fresh.column },
|
||||
});
|
||||
} catch (auditErr: unknown) {
|
||||
log.warn(`Pause-abort park audit emission failed for ${task.id}: ${auditErr instanceof Error ? auditErr.message : String(auditErr)}`);
|
||||
}
|
||||
log.log(`Recovered pause-abort park ${task.id}: ${task.title || task.description?.slice(0, 60) || "(untitled)"}`);
|
||||
recovered++;
|
||||
} catch (err: unknown) { const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
log.error(`Failed to recover pause-abort park ${task.id}: ${errorMessage}`);
|
||||
}
|
||||
}
|
||||
|
||||
if (recovered > 0) {
|
||||
log.log(`Recovered ${recovered} pause-abort park(s) → requeued to todo`);
|
||||
}
|
||||
return recovered;
|
||||
} catch (err: unknown) { const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
log.error(`Pause-abort park recovery failed: ${errorMessage}`);
|
||||
return 0;
|
||||
}
|
||||
}
|
||||
|
||||
async auditNoCommitsExpectedCandidates(): Promise<number> {
|
||||
try {
|
||||
const inReviewTasks = await this.store.listTasks({ column: "in-review", slim: true });
|
||||
@@ -7912,6 +8068,81 @@ export class SelfHealingManager {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* FN-6782 leaked-slot reaper (defense-in-depth). The source leak is closed in
|
||||
* the executor's pause-abort park path, but a worktree slot leaked by any
|
||||
* other/future path would silently pin `maxWorktrees` and concurrency-starve
|
||||
* the whole queue (the FN-6756 "in todo yet still maxWorktrees=3/3 holder"
|
||||
* symptom) until an engine restart. This sweep cross-checks each in-memory
|
||||
* worktree holder against its task column and reclaims slots whose holder is
|
||||
* no longer legitimately holding one.
|
||||
*
|
||||
* Conservative by construction — release happens ONLY when every guard agrees:
|
||||
* - the holder is NOT in the executor's executing set;
|
||||
* - its task is missing, or in `todo`/`triage` (a task waiting to run must
|
||||
* not pin a worktree). `in-progress` and `in-review` holders legitimately
|
||||
* retain their worktree; `done`/`archived` are handled by worktree-metadata
|
||||
* reconcile + merge cleanup, so they are left alone here;
|
||||
* - it has sat in the reapable column past a short grace (no mid-transition race);
|
||||
* - and finally `clearPhantomExecutorBinding` itself refuses (returns false)
|
||||
* if any live session surface is still registered — the last line of
|
||||
* defense against pulling a worktree out from under a running agent.
|
||||
*/
|
||||
async reapLeakedConcurrencySlots(): Promise<number> {
|
||||
const settings = await this.store.getSettings();
|
||||
if (settings.globalPause || settings.enginePaused) {
|
||||
return 0;
|
||||
}
|
||||
|
||||
const holders = this.options.listWorktreeHolders?.() ?? [];
|
||||
if (holders.length === 0) return 0;
|
||||
|
||||
const executingIds = this.options.getExecutingTaskIds?.() ?? new Set<string>();
|
||||
const now = Date.now();
|
||||
let reaped = 0;
|
||||
|
||||
for (const { taskId } of holders) {
|
||||
try {
|
||||
if (executingIds.has(taskId)) continue;
|
||||
|
||||
const task = await this.store.getTask(taskId).catch(() => null);
|
||||
const reapableColumn = !task || task.column === "todo" || task.column === "triage";
|
||||
if (!reapableColumn) continue;
|
||||
|
||||
if (task) {
|
||||
const since = new Date(task.columnMovedAt ?? task.updatedAt).getTime();
|
||||
if (Number.isFinite(since) && now - since < LEAKED_WORKTREE_SLOT_GRACE_MS) continue;
|
||||
}
|
||||
|
||||
// FNXC:WorkflowLifecycle 2026-06-20-00:00: re-check execution ownership
|
||||
// against a FRESH executing set immediately before releasing — the outer
|
||||
// `executingIds` snapshot predates this holder's `getTask` await, so a
|
||||
// task that started executing mid-sweep must not have its slot pulled
|
||||
// (coderabbit Major, PR #1687). clearPhantomExecutorBinding's live-session
|
||||
// refusal is the last line of defense, but this avoids racing it at all.
|
||||
const latestExecutingIds = this.options.getExecutingTaskIds?.() ?? new Set<string>();
|
||||
if (latestExecutingIds.has(taskId)) continue;
|
||||
|
||||
const released = this.options.clearPhantomExecutorBinding?.(taskId);
|
||||
// false = executor refused (live session surface); undefined = not wired.
|
||||
if (released !== true) continue;
|
||||
|
||||
reaped++;
|
||||
await this.store.logEntry(
|
||||
taskId,
|
||||
"Auto-recovered: released leaked worktree/concurrency slot (holder no longer in-progress)",
|
||||
);
|
||||
log.warn(`Reaped leaked worktree slot held by ${taskId} (column=${task?.column ?? "missing"})`);
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
log.error(`Leaked-slot reaper failed for ${taskId}: ${errorMessage}`);
|
||||
}
|
||||
}
|
||||
|
||||
if (reaped > 0) log.log(`Reaped ${reaped} leaked worktree slot(s)`);
|
||||
return reaped;
|
||||
}
|
||||
|
||||
/**
|
||||
* Recover executor tasks stranded in `in-progress` before a real session was
|
||||
* established, typically when the scheduler reserved a worktree path but the
|
||||
|
||||
Reference in New Issue
Block a user