fix(engine): park incomplete stuck-loop exhaustions
This commit is contained in:
Phil Larson
@@ -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",
|
||||
|
||||
@@ -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 =
|
||||
|
||||
Reference in New Issue
Block a user