Merge pull request #1205 from plarson/fix/incomplete-stuck-loop-parking
fix: park incomplete stuck-loop exhaustions
This commit is contained in:
5
.changeset/quiet-lions-park.md
Normal file
5
.changeset/quiet-lions-park.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
Park incomplete stuck-loop exhausted tasks in todo instead of routing them through review or merge.
|
||||
@@ -350,6 +350,59 @@ describe("TaskExecutor bounded recovery retries", () => {
|
||||
}));
|
||||
});
|
||||
|
||||
it("does not clobber self-healing parked incomplete-task pause metadata during abort cleanup", async () => {
|
||||
const store = createMockStore();
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
id: "FN-001",
|
||||
title: "Test",
|
||||
description: "Test",
|
||||
column: "todo",
|
||||
status: "queued",
|
||||
paused: true,
|
||||
userPaused: false,
|
||||
pausedReason: undefined,
|
||||
branch: "fusion/fn-001",
|
||||
worktree: null,
|
||||
dependencies: [],
|
||||
steps: [{ name: "Testing & Verification", status: "in-progress" }],
|
||||
currentStep: 6,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test", {});
|
||||
mockedCreateFnAgent.mockRejectedValue(new Error("Aborted"));
|
||||
(executor as any).pausedAborted.add("FN-001");
|
||||
|
||||
await executor.execute({
|
||||
id: "FN-001",
|
||||
title: "Test",
|
||||
description: "Test",
|
||||
column: "in-progress",
|
||||
recoveryRetryCount: 1,
|
||||
branch: "fusion/fn-001",
|
||||
dependencies: [],
|
||||
steps: [{ name: "Testing & Verification", status: "in-progress" }],
|
||||
currentStep: 6,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith("FN-001", expect.objectContaining({
|
||||
worktree: undefined,
|
||||
branch: undefined,
|
||||
}));
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith("FN-001", "todo");
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-001",
|
||||
"Execution abort cleanup skipped — incomplete stuck-loop task is already parked with progress preserved",
|
||||
undefined,
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
it("does NOT consume retry budget for stuck-task-detector kills", async () => {
|
||||
const store = createMockStore();
|
||||
|
||||
|
||||
@@ -6,8 +6,19 @@ import { TaskExecutor } from "../../executor.js";
|
||||
import { SelfHealingManager } from "../../self-healing.js";
|
||||
import { StuckTaskDetector } from "../../stuck-task-detector.js";
|
||||
|
||||
function createStore(task: Task, settings: Record<string, unknown> = {}): TaskStore & EventEmitter {
|
||||
const emitter = new EventEmitter() as TaskStore & EventEmitter;
|
||||
type MockTaskStore = TaskStore & EventEmitter & {
|
||||
getSettings: ReturnType<typeof vi.fn>;
|
||||
getTask: ReturnType<typeof vi.fn>;
|
||||
listTasks: ReturnType<typeof vi.fn>;
|
||||
updateTask: ReturnType<typeof vi.fn>;
|
||||
moveTask: ReturnType<typeof vi.fn>;
|
||||
handoffToReview: ReturnType<typeof vi.fn>;
|
||||
logEntry: ReturnType<typeof vi.fn>;
|
||||
recordRunAuditEvent: ReturnType<typeof vi.fn>;
|
||||
};
|
||||
|
||||
function createStore(task: Task, settings: Record<string, unknown> = {}): MockTaskStore {
|
||||
const emitter = new EventEmitter() as MockTaskStore;
|
||||
(emitter as any).getSettings = vi.fn().mockResolvedValue({
|
||||
autoMerge: true,
|
||||
globalPause: false,
|
||||
@@ -178,7 +189,7 @@ describe("reliability interactions: non-progress churn", () => {
|
||||
manager.stop();
|
||||
});
|
||||
|
||||
it("preserves STUCK_LOOP_EXHAUSTED when the churn signal does not fire", async () => {
|
||||
it("parks incomplete STUCK_LOOP_EXHAUSTED tasks in todo when the churn signal does not fire", async () => {
|
||||
const task = baseTask({ id: "FN-5168-LOOP", stuckKillCount: 6 });
|
||||
const store = createStore(task);
|
||||
const manager = new SelfHealingManager(store, { rootDir: "/tmp/repo" });
|
||||
@@ -198,8 +209,16 @@ describe("reliability interactions: non-progress churn", () => {
|
||||
|
||||
await detector.killAndRetry(task.id, 60_000);
|
||||
|
||||
expect(task.error).toBe("STUCK_LOOP_EXHAUSTED: stuck kill budget exhausted (7/6) after last reason=loop.");
|
||||
expect(task.column).toBe("in-review");
|
||||
expect(task.error).toBeNull();
|
||||
expect(task.status).toBeNull();
|
||||
expect(task.column).toBe("todo");
|
||||
expect(task.paused).toBe(true);
|
||||
expect(task.userPaused).toBe(true);
|
||||
expect(task.pausedReason).toBe("stuck-loop-exhausted-incomplete-steps");
|
||||
expect(task.stuckKillCount).toBe(7);
|
||||
expect(task.steps).toEqual([{ name: "Implement", status: "in-progress" }]);
|
||||
expect(task.log?.some((entry) => entry.action.includes("incomplete task exhausted stuck kill budget"))).toBe(true);
|
||||
expect(store.handoffToReview).not.toHaveBeenCalled();
|
||||
|
||||
manager.stop();
|
||||
});
|
||||
|
||||
@@ -424,6 +424,73 @@ describe("SelfHealingManager", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("parks incomplete stuck-loop exhaustion in todo without review handoff", async () => {
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
id: "FN-001",
|
||||
column: "in-progress",
|
||||
stuckKillCount: 6,
|
||||
steps: [
|
||||
{ name: "Preflight", status: "done" },
|
||||
{ name: "Delivery", status: "in-progress" },
|
||||
],
|
||||
} as unknown as Task);
|
||||
|
||||
manager.start();
|
||||
|
||||
const result = await manager.checkStuckBudget("FN-001", "loop");
|
||||
|
||||
expect(result).toBe(false);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-001", expect.objectContaining({
|
||||
stuckKillCount: 7,
|
||||
paused: true,
|
||||
userPaused: true,
|
||||
pausedReason: "stuck-loop-exhausted-incomplete-steps",
|
||||
}));
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "todo", { preserveProgress: true });
|
||||
expect(store.updateTask).toHaveBeenLastCalledWith("FN-001", expect.objectContaining({
|
||||
stuckKillCount: 7,
|
||||
paused: true,
|
||||
userPaused: true,
|
||||
pausedReason: "stuck-loop-exhausted-incomplete-steps",
|
||||
}));
|
||||
expect(store.handoffToReview).not.toHaveBeenCalled();
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-001",
|
||||
"STUCK_LOOP_EXHAUSTED: incomplete task exhausted stuck kill budget (7/6), last reason=loop. Parked in todo with progress preserved; manual review/resume required before retry.",
|
||||
);
|
||||
});
|
||||
|
||||
it("leaves incomplete stuck-loop exhaustion paused when todo parking fails", async () => {
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
id: "FN-001",
|
||||
column: "in-progress",
|
||||
stuckKillCount: 6,
|
||||
steps: [
|
||||
{ name: "Preflight", status: "done" },
|
||||
{ name: "Delivery", status: "in-progress" },
|
||||
],
|
||||
} as unknown as Task);
|
||||
(store.moveTask as ReturnType<typeof vi.fn>).mockRejectedValueOnce(new Error("database is busy"));
|
||||
|
||||
manager.start();
|
||||
|
||||
const result = await manager.checkStuckBudget("FN-001", "loop");
|
||||
|
||||
expect(result).toBe(false);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-001", expect.objectContaining({
|
||||
stuckKillCount: 7,
|
||||
paused: true,
|
||||
userPaused: true,
|
||||
pausedReason: "stuck-loop-exhausted-incomplete-steps",
|
||||
}));
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "todo", { preserveProgress: true });
|
||||
expect(store.handoffToReview).not.toHaveBeenCalled();
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-001",
|
||||
"STUCK_LOOP_EXHAUSTED: incomplete task exhausted stuck kill budget (7/6), last reason=loop. Failed to move task to todo (database is busy); task remains paused for manual intervention.",
|
||||
);
|
||||
});
|
||||
|
||||
it("terminalizes no-progress churn without incrementing stuck kill budget", async () => {
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
id: "FN-001",
|
||||
|
||||
@@ -2,7 +2,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { evaluateSpecStaleness, getPromptPath } from "../spec-staleness.js";
|
||||
import { stat } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
import type { Settings } from "@fusion/core";
|
||||
import type { Settings, Task } from "@fusion/core";
|
||||
|
||||
vi.mock("node:fs/promises", () => ({
|
||||
stat: vi.fn(),
|
||||
@@ -131,6 +131,33 @@ describe("evaluateSpecStaleness", () => {
|
||||
expect(result.reason).toContain("moved to triage for re-planning");
|
||||
});
|
||||
|
||||
it("skips stale-spec rerouting for parked tasks with preserved execution progress", async () => {
|
||||
const now = 100_000_000_000;
|
||||
const mtime = now - defaultMaxAgeMs - 1000;
|
||||
mockStat.mockResolvedValue({ mtimeMs: mtime } as Awaited<ReturnType<typeof stat>>);
|
||||
|
||||
const settings = createMockSettings({ specStalenessEnabled: true });
|
||||
const task = {
|
||||
id: "FN-249",
|
||||
column: "todo",
|
||||
currentStep: 6,
|
||||
paused: true,
|
||||
pausedReason: "stuck-loop-exhausted-incomplete-steps",
|
||||
steps: [
|
||||
{ name: "Implement", status: "done" },
|
||||
{ name: "Testing & Verification", status: "in-progress" },
|
||||
{ name: "Documentation & Delivery", status: "pending" },
|
||||
],
|
||||
} as Task;
|
||||
|
||||
const result = await evaluateSpecStaleness({ settings, promptPath, nowMs: now, task });
|
||||
|
||||
expect(result.isStale).toBe(false);
|
||||
expect(result.skipped).toBe(true);
|
||||
expect(result.reason).toContain("preserved execution progress");
|
||||
expect(mockStat).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("uses custom specStalenessMaxAgeMs when set and valid", async () => {
|
||||
const now = 100_000_000_000;
|
||||
const customMaxAge = 60 * 60 * 1000; // 1 hour
|
||||
|
||||
@@ -3157,7 +3157,7 @@ export class TaskExecutor {
|
||||
if (!isActiveTask) {
|
||||
const tasksDir = join(this.store.getFusionDir(), "tasks");
|
||||
const promptPath = getPromptPath(tasksDir, task.id);
|
||||
const staleness = await evaluateSpecStaleness({ settings, promptPath });
|
||||
const staleness = await evaluateSpecStaleness({ settings, promptPath, task });
|
||||
if (staleness.isStale) {
|
||||
executorLog.warn(`Task ${task.id} specification is stale — ${staleness.reason}`);
|
||||
// Move to triage first, then set status so the task enters triage with needs-replan
|
||||
@@ -4917,6 +4917,21 @@ export class TaskExecutor {
|
||||
return;
|
||||
}
|
||||
this.pausedAborted.delete(task.id);
|
||||
const latestTask = await this.store.getTask(task.id);
|
||||
if (
|
||||
latestTask?.column === "todo" &&
|
||||
latestTask.paused === true &&
|
||||
((latestTask.currentStep ?? 0) > 0 || latestTask.steps?.some((step) => step.status === "done" || step.status === "in-progress"))
|
||||
) {
|
||||
executorLog.log(`${task.id} paused-abort cleanup skipped — incomplete task is already parked with progress preserved`);
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
"Execution abort cleanup skipped — incomplete stuck-loop task is already parked with progress preserved",
|
||||
undefined,
|
||||
this.getRunContextFor(task.id),
|
||||
);
|
||||
return;
|
||||
}
|
||||
if (await this.shouldFinalizeCompletedTask(task.id, taskDone)) {
|
||||
if (await this.shouldDeferCompletionForGlobalPause(task.id, "paused after completion")) {
|
||||
return;
|
||||
|
||||
@@ -1417,7 +1417,7 @@ export class Scheduler {
|
||||
// so they receive fresh specification before execution. This guard runs after
|
||||
// filesystem validation so missing/unreadable files skip staleness checks entirely.
|
||||
const promptPath = getPromptPath(this.store.getTasksDir(), task.id);
|
||||
const staleness = await evaluateSpecStaleness({ settings, promptPath });
|
||||
const staleness = await evaluateSpecStaleness({ settings, promptPath, task });
|
||||
if (staleness.isStale) {
|
||||
schedulerLog.warn(`Task ${task.id} specification is stale — ${staleness.reason}`);
|
||||
await this.store.moveTask(task.id, "triage");
|
||||
|
||||
@@ -995,12 +995,16 @@ export class SelfHealingManager {
|
||||
// ── Stuck kill budget ─────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Check whether a stuck-killed task should be re-queued or marked as failed.
|
||||
* Called by StuckTaskDetector's `beforeRequeue` callback.
|
||||
* Check whether a stuck-killed task should be re-queued, parked for manual
|
||||
* intervention, or marked as failed. Called by StuckTaskDetector's
|
||||
* `beforeRequeue` callback.
|
||||
*
|
||||
* Terminal contract for stuck-loop exhaustion and no-progress churn:
|
||||
* - `STUCK_LOOP_EXHAUSTED`: increments the kill budget until exhausted, then
|
||||
* marks the task failed and parks it in `in-review`.
|
||||
* - `STUCK_LOOP_EXHAUSTED`: increments the kill budget until exhausted. Once
|
||||
* exhausted, tasks with incomplete steps are moved back to `todo` with
|
||||
* progress preserved and pause metadata reapplied for manual resume or
|
||||
* decomposition; tasks with only terminal steps keep the legacy failed
|
||||
* `in-review` handoff path.
|
||||
* - `STUCK_NO_PROGRESS_CHURN`: skips the budget entirely and terminalizes on
|
||||
* the first trigger with operator guidance to decompose or rescope.
|
||||
*
|
||||
@@ -1065,6 +1069,40 @@ export class SelfHealingManager {
|
||||
const newCount = (task.stuckKillCount ?? 0) + 1;
|
||||
|
||||
if (newCount > maxKills) {
|
||||
const hasIncompleteSteps = !!task.steps?.some((step) => NON_TERMINAL_STEP_STATUSES.has(step.status));
|
||||
|
||||
if (hasIncompleteSteps) {
|
||||
log.warn(`${taskId} exceeded stuck kill budget (${newCount}/${maxKills}, reason=${reason}) with incomplete steps — parking in todo`);
|
||||
await this.store.updateTask(taskId, {
|
||||
stuckKillCount: newCount,
|
||||
paused: true,
|
||||
userPaused: true,
|
||||
pausedReason: "stuck-loop-exhausted-incomplete-steps",
|
||||
} as Partial<Task> & { userPaused: boolean });
|
||||
let parkedInTodo = true;
|
||||
let moveErrMessage = "";
|
||||
try {
|
||||
await this.store.moveTask(taskId, "todo", { preserveProgress: true });
|
||||
await this.store.updateTask(taskId, {
|
||||
stuckKillCount: newCount,
|
||||
paused: true,
|
||||
userPaused: true,
|
||||
pausedReason: "stuck-loop-exhausted-incomplete-steps",
|
||||
} as Partial<Task> & { userPaused: boolean });
|
||||
} catch (moveErr: unknown) {
|
||||
parkedInTodo = false;
|
||||
moveErrMessage = moveErr instanceof Error ? moveErr.message : String(moveErr);
|
||||
log.warn(`${taskId} moveTask(todo) failed (${moveErrMessage}) after incomplete STUCK_LOOP_EXHAUSTED terminalization — task remains paused for manual intervention`);
|
||||
}
|
||||
await this.store.logEntry(
|
||||
taskId,
|
||||
parkedInTodo
|
||||
? `STUCK_LOOP_EXHAUSTED: incomplete task exhausted stuck kill budget (${newCount}/${maxKills}), last reason=${reason}. Parked in todo with progress preserved; manual review/resume required before retry.`
|
||||
: `STUCK_LOOP_EXHAUSTED: incomplete task exhausted stuck kill budget (${newCount}/${maxKills}), last reason=${reason}. Failed to move task to todo (${moveErrMessage}); task remains paused for manual intervention.`,
|
||||
);
|
||||
return false;
|
||||
}
|
||||
|
||||
// Budget exhausted — mark as permanently failed
|
||||
log.warn(`${taskId} exceeded stuck kill budget (${newCount}/${maxKills}, reason=${reason}) — marking failed`);
|
||||
const exhaustedError =
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
|
||||
import { stat } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
import type { Settings } from "@fusion/core";
|
||||
import type { Settings, Task } from "@fusion/core";
|
||||
|
||||
/** Default maximum age for a specification before it is considered stale (6 hours in ms). */
|
||||
const DEFAULT_SPEC_STALENESS_MAX_AGE_MS = 6 * 60 * 60 * 1000;
|
||||
@@ -49,6 +49,12 @@ export interface EvaluateSpecStalenessOptions {
|
||||
* Defaults to `Date.now()` when not provided.
|
||||
*/
|
||||
nowMs?: number;
|
||||
/**
|
||||
* Optional task metadata. When provided, evaluation skips already-started,
|
||||
* parked work so preserved progress is not sent back through triage solely
|
||||
* because the original PROMPT.md mtime exceeded the staleness threshold.
|
||||
*/
|
||||
task?: Pick<Task, "id" | "column" | "status" | "currentStep" | "steps" | "pausedReason">;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -83,10 +89,22 @@ export interface EvaluateSpecStalenessOptions {
|
||||
* @param options - Evaluation options including settings and PROMPT.md path
|
||||
* @returns Spec staleness decision with staleness flag, metrics, and skip indicator
|
||||
*/
|
||||
export function shouldSkipSpecStalenessForPreservedProgress(
|
||||
task: EvaluateSpecStalenessOptions["task"] | undefined,
|
||||
): boolean {
|
||||
if (!task || task.column === "triage" || task.status === "needs-replan" || task.status === "planning") {
|
||||
return false;
|
||||
}
|
||||
if ((task.currentStep ?? 0) > 0) {
|
||||
return true;
|
||||
}
|
||||
return !!task.steps?.some((step) => step.status === "done" || step.status === "in-progress");
|
||||
}
|
||||
|
||||
export async function evaluateSpecStaleness(
|
||||
options: EvaluateSpecStalenessOptions,
|
||||
): Promise<SpecStalenessResult> {
|
||||
const { settings, promptPath, nowMs } = options;
|
||||
const { settings, promptPath, nowMs, task } = options;
|
||||
|
||||
// Disabled mode: strict no-op — no file access
|
||||
if (settings.specStalenessEnabled !== true) {
|
||||
@@ -99,6 +117,16 @@ export async function evaluateSpecStaleness(
|
||||
};
|
||||
}
|
||||
|
||||
if (shouldSkipSpecStalenessForPreservedProgress(task)) {
|
||||
return {
|
||||
isStale: false,
|
||||
ageMs: undefined,
|
||||
maxAgeMs: undefined,
|
||||
reason: "Specification staleness skipped for task with preserved execution progress",
|
||||
skipped: true,
|
||||
};
|
||||
}
|
||||
|
||||
// Resolve max age with fallback to default
|
||||
const configuredMaxAgeMs = settings.specStalenessMaxAgeMs;
|
||||
const maxAgeMs =
|
||||
|
||||
Reference in New Issue
Block a user