fix: prevent stuck-kill from stranding approved plans as unplanned (#2326)
## Summary Fixes a race where a stuck-kill during Plan Review could leave an approved task stranded in `todo` as **unplanned**, so the scheduler refused release to `in-progress` for several minutes (observed on FN-1312: ~6m39s idle after Plan Review APPROVE). ### What went wrong 1. Finalize clears `status` early, then runs Plan Review. 2. Stuck-kill disposes the main triage session near the 30m processing threshold. 3. Stale eviction only treated the main session as live, so the card left `processing` while Plan Review / finalize was still running. 4. A second planner claimed `status: "planning"` and rewrote `PROMPT.md`. 5. The first finalize moved `triage → todo` without clearing planning statuses. 6. Hold-release saw planning/`needs-replan` and blocked: *“card is unplanned and cannot enter processing column in-progress”*. ### Fix In `packages/engine/src/triage.ts`: - Track **finalizing** handoffs and **live Plan Review subagents** as live planning work. - Keep them in `getProcessingTaskIds`, refuse concurrent `specifyTask`, and skip stale-processing eviction while they are live. - **Defer** stuck-abort requeue during finalize (bump kill count only; do not force `needs-replan`). - Recover approved drafts with `status: null` (post early-clear), not only `status: "planning"`. - Re-assert `status: null` after the successful release move to todo. ### Tests Regression coverage in `triage.test.ts` for eviction retention, processing-id inclusion, null-status recovery, needs-replan non-recovery, and deferred stuck-abort during finalize. ## Test plan - [x] `pnpm --filter @fusion/engine exec vitest run src/__tests__/triage.test.ts -t "evictStaleProcessing|specified triage recovery|stuck|recoverApproved|markStuckAborted|defers stuck-abort"` (related cases green) - [ ] CI gate on this PR - [ ] Optional: reproduce stuck-kill mid–Plan Review and confirm todo releases without a second full replan ## Notes Secondary report (stuck-kill immediately after all implementation steps complete, then 16s auto-recovery) is **out of scope** here; existing recovery already continued by skipping completed steps. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved recovery for approved tasks when Plan Review has completed, including correct handling of `null`-status drafts. * Avoided recovering unplanned seed drafts and tasks that should enter a replanning flow. * Refined stuck-abort requeue/cleanup to respect in-progress Plan Review and finalize handoffs. * Prevented stale-processing eviction from removing tasks while Plan Review/subagent finalization is still active. * Blocked new triage discovery and task specification when finalize/Plan Review work is ongoing, preventing handoff disruption. * Preserved durable task state during handoff completion (approval/failure/replan outcomes). * **Tests** * Added expanded triage recovery and stuck-abort/stale-eviction regression coverage. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
gsxdsm
@@ -3676,12 +3676,274 @@ describe("specified triage recovery", () => {
|
||||
noCommitsExpected: true,
|
||||
});
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "todo");
|
||||
// FNXC:TriageStuckKill 2026-07-18-21:05: terminal status clear after release move (FN-1312).
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { status: null, error: null });
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-001",
|
||||
"Auto-recovered specified task stuck in planning — moved to todo",
|
||||
);
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:TriageStuckKill 2026-07-18-22:30:
|
||||
Null status alone is not proof of an approved plan (Greptile P1). Only recover null-status
|
||||
triage cards when Plan Review already recorded a passed verdict.
|
||||
*/
|
||||
it("recovers a null-status triage task only when Plan Review already passed", async () => {
|
||||
const store = createMockStore({
|
||||
getSettings: vi.fn().mockResolvedValue({
|
||||
maxConcurrent: 2,
|
||||
maxWorktrees: 4,
|
||||
pollIntervalMs: 10000,
|
||||
groupOverlappingFiles: false,
|
||||
autoMerge: true,
|
||||
requirePlanApproval: false,
|
||||
} as Settings),
|
||||
});
|
||||
|
||||
const processor = new TriageProcessor(store, rootDir);
|
||||
const recovered = await processor.recoverApprovedTask({
|
||||
id: "FN-001",
|
||||
description: "Recovered triage task",
|
||||
column: "triage",
|
||||
status: null,
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
workflowStepResults: [
|
||||
{
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
phase: "pre-merge",
|
||||
status: "passed",
|
||||
verdict: "APPROVE",
|
||||
},
|
||||
],
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
updatedAt: "2026-01-01T00:02:00.000Z",
|
||||
} as any);
|
||||
|
||||
expect(recovered).toBe(true);
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "todo");
|
||||
});
|
||||
|
||||
it("does not recover a null-status triage draft that never passed Plan Review", async () => {
|
||||
const store = createMockStore({
|
||||
getSettings: vi.fn().mockResolvedValue({
|
||||
maxConcurrent: 2,
|
||||
maxWorktrees: 4,
|
||||
pollIntervalMs: 10000,
|
||||
groupOverlappingFiles: false,
|
||||
autoMerge: true,
|
||||
requirePlanApproval: false,
|
||||
} as Settings),
|
||||
});
|
||||
|
||||
const processor = new TriageProcessor(store, rootDir);
|
||||
const recovered = await processor.recoverApprovedTask({
|
||||
id: "FN-001",
|
||||
description: "Unapproved draft after early status clear",
|
||||
column: "triage",
|
||||
status: null,
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
updatedAt: "2026-01-01T00:02:00.000Z",
|
||||
});
|
||||
|
||||
expect(recovered).toBe(false);
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not recover needs-replan cards (those require a real replan, not handoff recovery)", async () => {
|
||||
const store = createMockStore({
|
||||
getSettings: vi.fn().mockResolvedValue({
|
||||
maxConcurrent: 2,
|
||||
maxWorktrees: 4,
|
||||
pollIntervalMs: 10000,
|
||||
groupOverlappingFiles: false,
|
||||
autoMerge: true,
|
||||
requirePlanApproval: false,
|
||||
} as Settings),
|
||||
});
|
||||
|
||||
const processor = new TriageProcessor(store, rootDir);
|
||||
const recovered = await processor.recoverApprovedTask({
|
||||
id: "FN-001",
|
||||
description: "Rejected plan under revision",
|
||||
column: "triage",
|
||||
status: "needs-replan",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
updatedAt: "2026-01-01T00:02:00.000Z",
|
||||
});
|
||||
|
||||
expect(recovered).toBe(false);
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("defers stuck-abort requeue while Plan Review finalize is still in flight", async () => {
|
||||
const store = createMockStore({
|
||||
getSettings: vi.fn().mockResolvedValue({
|
||||
maxConcurrent: 2,
|
||||
maxWorktrees: 4,
|
||||
pollIntervalMs: 10000,
|
||||
groupOverlappingFiles: false,
|
||||
autoMerge: true,
|
||||
maxStuckKills: 6,
|
||||
} as Settings),
|
||||
getTask: vi.fn().mockResolvedValue({
|
||||
id: "FN-001",
|
||||
description: "mid finalize",
|
||||
column: "triage",
|
||||
status: null,
|
||||
stuckKillCount: 0,
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
updatedAt: "2026-01-01T00:02:00.000Z",
|
||||
}),
|
||||
});
|
||||
|
||||
const processor = new TriageProcessor(store, rootDir);
|
||||
(processor as any).finalizing.add("FN-001");
|
||||
|
||||
await (processor as any).handleStuckAbortRequeue(
|
||||
{
|
||||
id: "FN-001",
|
||||
description: "mid finalize",
|
||||
column: "triage",
|
||||
status: null,
|
||||
stuckKillCount: 0,
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
updatedAt: "2026-01-01T00:02:00.000Z",
|
||||
},
|
||||
"catch",
|
||||
);
|
||||
|
||||
// Must not force needs-replan while the in-flight finalize owns the handoff.
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { stuckKillCount: 1 });
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith(
|
||||
"FN-001",
|
||||
expect.objectContaining({ status: "needs-replan" }),
|
||||
);
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:TriageStuckKill 2026-07-18-22:30:
|
||||
After a successful release, outer stuckAborted cleanup must not re-apply needs-replan
|
||||
(Greptile P1 post-handoff race on PR #2326).
|
||||
*/
|
||||
it("does not write needs-replan when stuck-abort cleanup runs after handoff to todo", async () => {
|
||||
const store = createMockStore({
|
||||
getSettings: vi.fn().mockResolvedValue({
|
||||
maxConcurrent: 2,
|
||||
maxWorktrees: 4,
|
||||
pollIntervalMs: 10000,
|
||||
groupOverlappingFiles: false,
|
||||
autoMerge: true,
|
||||
maxStuckKills: 6,
|
||||
} as Settings),
|
||||
getTask: vi.fn().mockResolvedValue({
|
||||
id: "FN-001",
|
||||
description: "already released",
|
||||
column: "todo",
|
||||
status: null,
|
||||
stuckKillCount: 0,
|
||||
dependencies: [],
|
||||
steps: [{ name: "step-1", status: "pending" }],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
updatedAt: "2026-01-01T00:02:00.000Z",
|
||||
}),
|
||||
});
|
||||
|
||||
const processor = new TriageProcessor(store, rootDir);
|
||||
|
||||
await (processor as any).handleStuckAbortRequeue(
|
||||
{
|
||||
id: "FN-001",
|
||||
description: "already released",
|
||||
column: "todo",
|
||||
status: null,
|
||||
stuckKillCount: 0,
|
||||
dependencies: [],
|
||||
steps: [{ name: "step-1", status: "pending" }],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
updatedAt: "2026-01-01T00:02:00.000Z",
|
||||
},
|
||||
"catch",
|
||||
);
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { stuckKillCount: 1 });
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith(
|
||||
"FN-001",
|
||||
expect.objectContaining({ status: "needs-replan" }),
|
||||
);
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:TriageStuckKill 2026-07-18-22:50:
|
||||
Plan-in-place workflows plan inside todo with status:"planning". Stuck-abort must requeue
|
||||
those cards, not treat every todo row as a completed handoff (CodeRabbit on PR #2326).
|
||||
*/
|
||||
it("requeues plan-in-place todo cards still in planning status after stuck-abort", async () => {
|
||||
await writeFile(
|
||||
join(rootDir, ".fusion", "tasks", "FN-001", "PROMPT.md"),
|
||||
"# Task: FN-001\n\n**Size:** M\n\n## Mission\n\nDraft still being planned in todo\n",
|
||||
);
|
||||
|
||||
const planningTodo = {
|
||||
id: "FN-001",
|
||||
description: "plan-in-place mid plan",
|
||||
column: "todo" as const,
|
||||
status: "planning" as const,
|
||||
stuckKillCount: 0,
|
||||
dependencies: [] as string[],
|
||||
steps: [] as Array<{ name: string; status: string }>,
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
updatedAt: "2026-01-01T00:02:00.000Z",
|
||||
};
|
||||
|
||||
const store = createMockStore({
|
||||
getSettings: vi.fn().mockResolvedValue({
|
||||
maxConcurrent: 2,
|
||||
maxWorktrees: 4,
|
||||
pollIntervalMs: 10000,
|
||||
groupOverlappingFiles: false,
|
||||
autoMerge: true,
|
||||
maxStuckKills: 6,
|
||||
} as Settings),
|
||||
getTask: vi.fn().mockResolvedValue(planningTodo),
|
||||
});
|
||||
|
||||
const processor = new TriageProcessor(store, rootDir);
|
||||
await (processor as any).handleStuckAbortRequeue(planningTodo, "catch");
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith(
|
||||
"FN-001",
|
||||
expect.objectContaining({ status: "needs-replan", stuckKillCount: 1 }),
|
||||
);
|
||||
});
|
||||
|
||||
it("stamps source metadata from sanitized effective write scope during recovery", async () => {
|
||||
await writeFile(
|
||||
join(rootDir, ".fusion", "tasks", "FN-001", "PROMPT.md"),
|
||||
@@ -7117,6 +7379,60 @@ describe("evictStaleProcessing", () => {
|
||||
expect(processor.getProcessingTaskIds().has("FN-001")).toBe(false);
|
||||
expect(processor.getProcessingTaskIds().has("FN-002")).toBe(true);
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:TriageStuckKill 2026-07-18-21:05:
|
||||
FN-1312: after stuck-kill of the main triage session, Plan Review still runs as a
|
||||
subagent and finalize is mid-handoff. Eviction at the 30m threshold must not drop
|
||||
those cards or self-healing/poll will start a concurrent planner.
|
||||
*/
|
||||
it("retains stuck-aborted tasks that still have a live Plan Review subagent", () => {
|
||||
const store = createMockStore();
|
||||
const processor = new TriageProcessor(store, "/tmp/root");
|
||||
|
||||
vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z"));
|
||||
(processor as any).processing.add("FN-1312");
|
||||
(processor as any).processingSince.set("FN-1312", Date.now());
|
||||
(processor as any).stuckAborted.add("FN-1312");
|
||||
(processor as any).activeSubagentSessions.set("FN-1312", new Set([{ dispose: vi.fn() }]));
|
||||
|
||||
vi.setSystemTime(new Date("2026-01-01T00:31:00.000Z"));
|
||||
|
||||
const evicted = processor.evictStaleProcessing();
|
||||
|
||||
expect(evicted).toEqual(new Set());
|
||||
expect(processor.getProcessingTaskIds().has("FN-1312")).toBe(true);
|
||||
});
|
||||
|
||||
it("retains stuck-aborted tasks currently inside finalizeApprovedTask", () => {
|
||||
const store = createMockStore();
|
||||
const processor = new TriageProcessor(store, "/tmp/root");
|
||||
|
||||
vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z"));
|
||||
(processor as any).processing.add("FN-1312");
|
||||
(processor as any).processingSince.set("FN-1312", Date.now());
|
||||
(processor as any).stuckAborted.add("FN-1312");
|
||||
(processor as any).finalizing.add("FN-1312");
|
||||
|
||||
vi.setSystemTime(new Date("2026-01-01T00:31:00.000Z"));
|
||||
|
||||
const evicted = processor.evictStaleProcessing();
|
||||
|
||||
expect(evicted).toEqual(new Set());
|
||||
expect(processor.getProcessingTaskIds().has("FN-1312")).toBe(true);
|
||||
});
|
||||
|
||||
it("includes finalizing and subagent tasks in getProcessingTaskIds even when not in processing", () => {
|
||||
const store = createMockStore();
|
||||
const processor = new TriageProcessor(store, "/tmp/root");
|
||||
|
||||
(processor as any).finalizing.add("FN-finalizing");
|
||||
(processor as any).activeSubagentSessions.set("FN-subagent", new Set([{ dispose: vi.fn() }]));
|
||||
|
||||
const ids = processor.getProcessingTaskIds();
|
||||
expect(ids.has("FN-finalizing")).toBe(true);
|
||||
expect(ids.has("FN-subagent")).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
// ── Agent Delegation Tool Tests ──────────────────────────────────────
|
||||
|
||||
@@ -120,7 +120,7 @@ import type {
|
||||
AgentSession,
|
||||
} from "@earendil-works/pi-coding-agent";
|
||||
import { ModelFallbackExhaustedError, describeModel, formatModelMarkerDetails, promptWithFallback } from "./pi.js";
|
||||
import { isTaskStillInPlanningStage } from "./replan-target.js";
|
||||
import { hasAdvancedPastPlanning, isTaskStillInPlanningStage } from "./replan-target.js";
|
||||
import {
|
||||
createResolvedAgentSession,
|
||||
extractRuntimeHint,
|
||||
@@ -243,6 +243,18 @@ export class TriageProcessor {
|
||||
* verdicts. Mirrors `TaskExecutor.activeSubagentSessions`.
|
||||
*/
|
||||
private activeSubagentSessions = new Map<string, Set<AgentSession>>();
|
||||
/**
|
||||
* FNXC:TriageStuckKill 2026-07-18-21:05:
|
||||
* Tasks currently inside `finalizeApprovedTask` (PROMPT hygiene → Plan Review →
|
||||
* column handoff). The main planning session is often already untracked/disposed by
|
||||
* the time Plan Review runs, so without this set a stuck-kill + stale-processing
|
||||
* eviction can drop the card from `processing` and let a concurrent second planner
|
||||
* claim `status:"planning"` while the first finalize still moves triage→todo — the
|
||||
* move does not clear planning statuses, so the scheduler then holds the card as
|
||||
* unplanned until the second planner finishes (FN-1312: 6m+ idle after Plan Review
|
||||
* APPROVE). Finalizing tasks stay in getProcessingTaskIds and are not rediscovered.
|
||||
*/
|
||||
private finalizing = new Set<string>();
|
||||
/** Tasks aborted due to globalPause (to avoid reporting as errors). */
|
||||
private pauseAborted = new Set<string>();
|
||||
/** Tasks killed by the stuck task detector (to avoid reporting as errors). */
|
||||
@@ -642,9 +654,28 @@ export class TriageProcessor {
|
||||
/**
|
||||
* Return a snapshot of tasks currently being specified by this processor.
|
||||
* Used by self-healing maintenance to avoid recovering live sessions.
|
||||
*
|
||||
* FNXC:TriageStuckKill 2026-07-18-21:05:
|
||||
* Include Plan Review subagents and in-flight finalize handoffs so self-healing
|
||||
* and poll rediscovery cannot start a second planner while the first session is
|
||||
* still completing Plan Review → todo after a stuck-kill of the main session.
|
||||
*/
|
||||
getProcessingTaskIds(): Set<string> {
|
||||
return new Set(this.processing);
|
||||
const ids = new Set(this.processing);
|
||||
for (const taskId of this.finalizing) ids.add(taskId);
|
||||
for (const taskId of this.activeSubagentSessions.keys()) {
|
||||
const sessions = this.activeSubagentSessions.get(taskId);
|
||||
if (sessions && sessions.size > 0) ids.add(taskId);
|
||||
}
|
||||
return ids;
|
||||
}
|
||||
|
||||
/** True when this processor still owns live work for `taskId` (main, subagent, or finalize). */
|
||||
private hasLivePlanningWork(taskId: string): boolean {
|
||||
if (this.finalizing.has(taskId)) return true;
|
||||
const subagents = this.activeSubagentSessions.get(taskId);
|
||||
if (subagents && subagents.size > 0) return true;
|
||||
return this.activeSessions.has(taskId) && !this.stuckAborted.has(taskId);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -674,9 +705,15 @@ export class TriageProcessor {
|
||||
/*
|
||||
FNXC:Triage 2026-07-16-18:29:
|
||||
Stale-processing eviction must retain a task with a live, non-aborted triage session (`activeSessions.has(id) && !stuckAborted.has(id)`). Removing it would drop genuinely active planning from `getProcessingTaskIds()` and let self-healing prematurely finalize it to todo/awaiting-approval, clear planning status, or nudge priority. Hung promises without a session and stuck-aborted/disposed sessions remain evictable.
|
||||
|
||||
FNXC:TriageStuckKill 2026-07-18-21:05:
|
||||
Also retain Plan Review subagents and finalize handoffs after the main session is
|
||||
stuck-killed. Stuck kill often fires near the 30m threshold (same clock as this
|
||||
eviction), so without this guard the card is rediscovered while finalize is still
|
||||
moving triage→todo and a concurrent planner leaves `status:"planning"` on a
|
||||
todo card the scheduler refuses to release (FN-1312).
|
||||
*/
|
||||
const hasLiveSession = this.activeSessions.has(taskId) && !this.stuckAborted.has(taskId);
|
||||
if (hasLiveSession) continue;
|
||||
if (this.hasLivePlanningWork(taskId)) continue;
|
||||
|
||||
planLog.warn(
|
||||
`${taskId} has been in processing for ${Math.round((now - since) / 60_000)}min ` +
|
||||
@@ -686,18 +723,35 @@ export class TriageProcessor {
|
||||
this.processingSince.delete(taskId);
|
||||
this.activeSessions.delete(taskId);
|
||||
this.stuckAborted.delete(taskId);
|
||||
this.finalizing.delete(taskId);
|
||||
evicted.add(taskId);
|
||||
}
|
||||
|
||||
return evicted;
|
||||
}
|
||||
|
||||
/** True when Plan Review already recorded a passed verdict on this task. */
|
||||
private hasPassedPlanReview(task: Pick<Task, "workflowStepResults">): boolean {
|
||||
return task.workflowStepResults?.some(
|
||||
(result) => result.workflowStepId === PLAN_REVIEW_GROUP_ID && result.status === "passed",
|
||||
) === true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Recover a triage task whose PROMPT.md was already written but the final
|
||||
* handoff out of `status: "planning"` never completed.
|
||||
* handoff out of planning never completed.
|
||||
*
|
||||
* FNXC:TriageStuckKill 2026-07-18-21:05:
|
||||
* Classic path: status is `planning`. Extended path: status is null after finalize's
|
||||
* early clear ONLY when Plan Review already passed — null alone must not promote an
|
||||
* unapproved draft (a lightly-edited seed can fail the exact seed equality check).
|
||||
* Do not recover `needs-replan` / `plan-review-unavailable`.
|
||||
*/
|
||||
async recoverApprovedTask(task: Task): Promise<boolean> {
|
||||
if (task.column !== "triage" || task.status !== "planning") {
|
||||
const recoverableStatus =
|
||||
task.status === "planning"
|
||||
|| (task.status == null && this.hasPassedPlanReview(task));
|
||||
if (task.column !== "triage" || !recoverableStatus) {
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -724,6 +778,12 @@ export class TriageProcessor {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Bootstrap / refinement seeds are not approved specs — leave them for normal planning.
|
||||
if (isUnplannedSeedPrompt(written, task.id, task.title, task.description)) {
|
||||
planLog.warn(`${task.id} planning recovery skipped — PROMPT.md is still an unplanned seed`);
|
||||
return false;
|
||||
}
|
||||
|
||||
const deterministicSpecFailure = await this.validateGeneratedPrompt(task.id, written);
|
||||
if (deterministicSpecFailure) {
|
||||
planLog.warn(`${task.id} planning recovery skipped — PROMPT.md failed deterministic validation (${deterministicSpecFailure})`);
|
||||
@@ -784,6 +844,15 @@ export class TriageProcessor {
|
||||
/*
|
||||
FNXC:Triage 2026-06-27-00:00:
|
||||
A stuck-killed planning session that already wrote a usable PROMPT.md or plan task document must resume in revision mode on the next poll, not re-triage from scratch. Reuse stuckKillCount and maxStuckKills for the triage retry budget so repeated stuck resumes escalate to manual intervention instead of looping forever.
|
||||
|
||||
FNXC:TriageStuckKill 2026-07-18-21:05:
|
||||
Do not invalidate an already-approved plan. Finalize clears `status` to null before Plan
|
||||
Review, so a stuck-kill mid-review used to skip recoverApprovedTask (which required
|
||||
status:"planning") and force needs-replan — even when Plan Review had just APPROVEd and
|
||||
the card was about to move to todo. That left the scheduler holding an "unplanned" todo
|
||||
card until a second planner rewrote PROMPT.md (FN-1312). If Plan Review already passed
|
||||
or a valid draft exists after the early status clear, complete the handoff instead of
|
||||
replan-invalidating.
|
||||
*/
|
||||
const freshTask = await this.store.getTask(task.id).catch((err: unknown) => {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
@@ -791,6 +860,56 @@ export class TriageProcessor {
|
||||
return task;
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:TriageStuckKill 2026-07-18-21:05:
|
||||
If finalize is still running Plan Review after the main session was killed, leave the
|
||||
card alone — the in-flight finalize owns the handoff. Setting needs-replan here races
|
||||
the APPROVE path and strands the card unplanned in todo.
|
||||
*/
|
||||
if (this.finalizing.has(task.id) || (this.activeSubagentSessions.get(task.id)?.size ?? 0) > 0) {
|
||||
planLog.log(
|
||||
`${task.id} killed by stuck detector during Plan Review/finalize — deferring requeue to the in-flight handoff (${context})`,
|
||||
);
|
||||
await this.store.updateTask(task.id, {
|
||||
stuckKillCount: (freshTask.stuckKillCount ?? task.stuckKillCount ?? 0) + 1,
|
||||
}).catch((err: unknown) => {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
planLog.warn(`${task.id}: failed to increment stuckKillCount during deferred stuck-detector ${context} cleanup: ${msg}`);
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:TriageStuckKill 2026-07-18-22:30:
|
||||
Finalize can succeed (move to todo + clear status) and clear `finalizing` before the
|
||||
outer stuckAborted catch runs — e.g. dispose of an already-killed main session throws
|
||||
after handoff. Recovery then fails (column is no longer triage) and the draft path
|
||||
would write needs-replan, re-stranding an approved plan (Greptile P1 on PR #2326).
|
||||
|
||||
FNXC:TriageStuckKill 2026-07-18-22:50:
|
||||
Do NOT treat every `todo` card as released. Plan-in-place workflows plan inside `todo`
|
||||
with status:"planning"/"needs-replan"; those must still requeue (CodeRabbit on PR #2326).
|
||||
hasAdvancedPastPlanning covers execution columns and released todo (steps/worktree).
|
||||
Released handoffs with status cleared but no steps yet are also preserved: todo without
|
||||
a planning-stage status means the scheduler can claim the card.
|
||||
*/
|
||||
const planningStageStatus =
|
||||
freshTask.status === "planning"
|
||||
|| freshTask.status === "needs-replan"
|
||||
|| freshTask.status === "plan-review-unavailable";
|
||||
const releasedToTodo = freshTask.column === "todo" && !planningStageStatus;
|
||||
if (hasAdvancedPastPlanning(freshTask) || releasedToTodo) {
|
||||
const nextStuckKillCount = (freshTask.stuckKillCount ?? task.stuckKillCount ?? 0) + 1;
|
||||
planLog.log(
|
||||
`${task.id} killed by stuck detector after planning handoff completed (column=${freshTask.column}, status=${freshTask.status ?? "null"}) — preserving released state (${context})`,
|
||||
);
|
||||
await this.store.updateTask(task.id, { stuckKillCount: nextStuckKillCount }).catch((err: unknown) => {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
planLog.warn(`${task.id}: failed to increment stuckKillCount after post-handoff stuck-detector ${context} cleanup: ${msg}`);
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
const recovered = await this.recoverApprovedTask(freshTask).catch((err: unknown) => {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
planLog.warn(`${task.id}: planning recovery failed during stuck-detector ${context} cleanup: ${msg}`);
|
||||
@@ -937,7 +1056,7 @@ export class TriageProcessor {
|
||||
}
|
||||
|
||||
const eligibleTriageTasks = allTasks.filter(
|
||||
(t) => t.column === "triage" && !this.processing.has(t.id) && !t.paused
|
||||
(t) => t.column === "triage" && !this.processing.has(t.id) && !this.hasLivePlanningWork(t.id) && !t.paused
|
||||
// Skip tasks awaiting manual plan approval — they should not be auto-discovered
|
||||
&& t.status !== "awaiting-approval"
|
||||
// Skip failed specifications until the user explicitly retries them.
|
||||
@@ -955,7 +1074,7 @@ export class TriageProcessor {
|
||||
2. Refinement seeds (`# {title}\n\n{description}`, no id prefix) previously failed the strict bootstrap-stub equality, so a promoted refinement skipped planning entirely; isUnplannedSeedPrompt accepts both seed shapes.
|
||||
*/
|
||||
const eligibleTodoTasksRaw = allTasks.filter(
|
||||
(t) => t.column === "todo" && !this.processing.has(t.id) && !t.paused
|
||||
(t) => t.column === "todo" && !this.processing.has(t.id) && !this.hasLivePlanningWork(t.id) && !t.paused
|
||||
&& t.status !== "awaiting-approval"
|
||||
&& t.status !== "failed"
|
||||
&& t.status !== "stuck-killed"
|
||||
@@ -1086,7 +1205,13 @@ export class TriageProcessor {
|
||||
* quality gate before execution; triage does not inject a separate review tool.
|
||||
*/
|
||||
async specifyTask(task: Task): Promise<void> {
|
||||
if (this.processing.has(task.id)) return;
|
||||
/*
|
||||
FNXC:TriageStuckKill 2026-07-18-21:05:
|
||||
Refuse a second planner when finalize/Plan Review is still live even if
|
||||
`processing` was cleared by a stuck-kill eviction race. Concurrent claim is
|
||||
what leaves `status:"planning"` on a todo card after Plan Review APPROVE.
|
||||
*/
|
||||
if (this.processing.has(task.id) || this.hasLivePlanningWork(task.id)) return;
|
||||
this.processing.add(task.id);
|
||||
this.processingSince.set(task.id, Date.now());
|
||||
|
||||
@@ -2771,6 +2896,30 @@ export class TriageProcessor {
|
||||
recoveryLogAction?: string;
|
||||
preservePromptContent?: boolean;
|
||||
} = {},
|
||||
): Promise<void> {
|
||||
/*
|
||||
FNXC:TriageStuckKill 2026-07-18-21:05:
|
||||
Mark the card finalizing for the whole Plan Review → column handoff so stuck-kill
|
||||
eviction and poll rediscovery cannot start a concurrent planner (FN-1312).
|
||||
*/
|
||||
this.finalizing.add(task.id);
|
||||
try {
|
||||
await this.finalizeApprovedTaskBody(task, writtenInput, settings, options);
|
||||
} finally {
|
||||
this.finalizing.delete(task.id);
|
||||
}
|
||||
}
|
||||
|
||||
private async finalizeApprovedTaskBody(
|
||||
task: Task,
|
||||
writtenInput: string,
|
||||
settings: Settings,
|
||||
options: {
|
||||
isReplan?: boolean;
|
||||
feedback?: string;
|
||||
recoveryLogAction?: string;
|
||||
preservePromptContent?: boolean;
|
||||
} = {},
|
||||
): Promise<void> {
|
||||
let written = writtenInput;
|
||||
const explicitDuplicateMarker = parseExplicitDuplicateMarker(written);
|
||||
@@ -3207,6 +3356,45 @@ export class TriageProcessor {
|
||||
await this.store.moveTask(task.id, "todo");
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:TriageStuckKill 2026-07-18-21:05:
|
||||
Re-assert status:null after the release move. finalize clears status early (before Plan
|
||||
Review); triage→todo does not clear planning statuses; a concurrent stuck-kill requeue
|
||||
or rediscovered planner can stamp status:"planning" between those points. Without this
|
||||
terminal clear the scheduler holds the card as unplanned after Plan Review APPROVE
|
||||
(FN-1312).
|
||||
|
||||
FNXC:TriageStuckKill 2026-07-18-22:30:
|
||||
Only clear planning-stage statuses under the task lock. Do not wipe a concurrent
|
||||
operator/engine write of failed, awaiting-approval, or a genuine later needs-replan
|
||||
that is not the mid-handoff planner race (Greptile P2 on PR #2326). Concurrent
|
||||
`status:"planning"` from a rediscovered second planner is still cleared.
|
||||
*/
|
||||
if (typeof this.store.updateTaskAtomic === "function") {
|
||||
await this.store.updateTaskAtomic(task.id, (live) => {
|
||||
if (
|
||||
live.status === "planning"
|
||||
|| live.status === "plan-review-unavailable"
|
||||
|| live.status == null
|
||||
) {
|
||||
return { status: null, error: null };
|
||||
}
|
||||
// Leave needs-replan/failed/awaiting-approval and other durable statuses alone.
|
||||
return null;
|
||||
});
|
||||
} else {
|
||||
const live = await Promise.resolve(this.store.getTask(task.id)).catch(() => null);
|
||||
if (
|
||||
live
|
||||
&& (live.status === "planning" || live.status === "plan-review-unavailable" || live.status == null)
|
||||
) {
|
||||
await this.store.updateTask(task.id, { status: null, error: null });
|
||||
} else if (!live) {
|
||||
// Minimal test stores often omit getTask; still clear the mid-handoff planning stamp.
|
||||
await this.store.updateTask(task.id, { status: null, error: null });
|
||||
}
|
||||
}
|
||||
|
||||
if (shouldApplyPromptDeclaredTitle && promptDeclaredTitle) {
|
||||
await this.store.updateTask(task.id, { title: promptDeclaredTitle });
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user