diff --git a/.changeset/fix-triage-replan-steps-wedge.md b/.changeset/fix-triage-replan-steps-wedge.md new file mode 100644 index 0000000000..858e313500 --- /dev/null +++ b/.changeset/fix-triage-replan-steps-wedge.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix tasks getting stuck in Planning forever after a plan review asks for revisions. +category: fix +dev: `hasAdvancedPastPlanning` (replan-target.ts) counted `steps.length > 0` as proof a task had advanced past planning. Replan cards legitimately retain the steps their previous planning pass materialized, so the guard at triage's `specifyTask` claim silently skipped its `status:"planning"` write, re-claimed the card every poll, and starved healthy cards out of `maxTriageConcurrent`. Steps are no longer advancement evidence in a planner lane ("triage", or "todo" for plan-in-place workflows carrying a planning status); worktrees and execution/terminal columns still are, preserving FN-7977. The primary claim path now warns instead of skipping silently. diff --git a/packages/engine/src/__tests__/replan-target.test.ts b/packages/engine/src/__tests__/replan-target.test.ts index 4e0146ed26..1bfe555183 100644 --- a/packages/engine/src/__tests__/replan-target.test.ts +++ b/packages/engine/src/__tests__/replan-target.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from "vitest"; -import type { TaskStore } from "@fusion/core"; +import type { Task, TaskStep, TaskStore } from "@fusion/core"; import { hasAdvancedPastPlanning, isTaskStillInPlanningStage, moveTaskToReplanColumn, resolveReplanTargetColumn } from "../replan-target.js"; /* @@ -18,18 +18,85 @@ function storeWithSelection(workflowId: string | undefined): TaskStore { } as unknown as TaskStore; } +/* +FNXC:WorkflowReplan 2026-07-16-05:35: +Regression surfaces for the steps>0 planner wedge. A replan card retains the steps its +previous planning pass materialized, so steps must never imply "advanced" while the card is +parked in a planner lane. Enumerated surfaces: the "triage" column (with and without an +explicit needs-replan status), the plan-in-place "todo" planner lane used by Coding (Ideas), +every parked-for-planning status, and the advancement signals that must still fire +(worktree, execution/terminal columns, planned-and-queued todo cards). +*/ +type PlanningGuardCase = { + label: string; + task: Pick; + stillPlanning: boolean; +}; + +const planStep = (name: string): TaskStep => ({ name, status: "pending" }); + +const planningGuardCases: PlanningGuardCase[] = [ + { label: "empty triage task", task: { column: "triage", steps: [] }, stillPlanning: true }, + { label: "unplanned todo seed", task: { column: "todo", steps: [] }, stillPlanning: true }, + { label: "todo task with a worktree", task: { column: "todo", worktree: "/tmp/FN-1", steps: [] }, stillPlanning: false }, + { + label: "planned-and-queued todo task with materialized steps", + task: { column: "todo", steps: [planStep("step-1")] }, + stillPlanning: false, + }, + { label: "in-progress task", task: { column: "in-progress", steps: [] }, stillPlanning: false }, + { label: "in-review task", task: { column: "in-review", steps: [] }, stillPlanning: false }, + { label: "completed task", task: { column: "done", steps: [] }, stillPlanning: false }, + { label: "archived task", task: { column: "archived", steps: [] }, stillPlanning: false }, + + // A triage card sits in the planner column by definition — nothing executes out of triage, + // so steps materialized by its previous planning pass must never read as advancement. + { + label: "triage replan card carrying steps from its previous planning pass", + task: { column: "triage", steps: [planStep("step-1")], status: "needs-replan" }, + stillPlanning: true, + }, + { + label: "triage card carrying steps with no explicit status", + task: { column: "triage", steps: [planStep("step-1"), planStep("step-2")] }, + stillPlanning: true, + }, + { + label: "triage card parked by a reviewer outage", + task: { column: "triage", steps: [planStep("step-1")], status: "plan-review-unavailable" }, + stillPlanning: true, + }, + + // Plan-in-place workflows (Coding (Ideas)) park replans in the merged "todo" planner lane, + // carrying a real spec — the planning status is what separates them from queued work. + { + label: "plan-in-place todo replan card carrying steps", + task: { column: "todo", steps: [planStep("step-1")], status: "needs-replan" }, + stillPlanning: true, + }, + { + label: "plan-in-place todo card parked by a reviewer outage", + task: { column: "todo", steps: [planStep("step-1")], status: "plan-review-unavailable" }, + stillPlanning: true, + }, + + // FN-7977's protections must survive: real advancement still outranks a planning status. + { + label: "triage card an executor already claimed a worktree for", + task: { column: "triage", worktree: "/tmp/FN-1", steps: [planStep("step-1")], status: "needs-replan" }, + stillPlanning: false, + }, + { + label: "card that reached execution while a planning recovery was in flight", + task: { column: "in-progress", steps: [planStep("step-1")], status: "needs-replan" }, + stillPlanning: false, + }, +]; + describe("planning-stage guard", () => { - it.each([ - [{ column: "triage", worktree: null, steps: [] }, true, "empty triage task"], - [{ column: "todo", worktree: null, steps: [] }, true, "unplanned todo seed"], - [{ column: "todo", worktree: "/tmp/FN-1", steps: [] }, false, "todo task with a worktree"], - [{ column: "todo", worktree: null, steps: [{ id: "step-1" }] }, false, "todo task with materialized steps"], - [{ column: "in-progress", worktree: null, steps: [] }, false, "in-progress task"], - [{ column: "in-review", worktree: null, steps: [] }, false, "in-review task"], - [{ column: "done", worktree: null, steps: [] }, false, "completed task"], - ] as const)("recognizes %s", (task, expected) => { - expect(isTaskStillInPlanningStage(task)).toBe(expected); - expect(hasAdvancedPastPlanning(task)).toBe(!expected); + it.each(planningGuardCases)("recognizes $label", ({ task, stillPlanning }) => { + expect(isTaskStillInPlanningStage(task)).toBe(stillPlanning); + expect(hasAdvancedPastPlanning(task)).toBe(!stillPlanning); }); }); diff --git a/packages/engine/src/replan-target.ts b/packages/engine/src/replan-target.ts index 1eb7439d2e..665e7e7482 100644 --- a/packages/engine/src/replan-target.ts +++ b/packages/engine/src/replan-target.ts @@ -24,21 +24,54 @@ legal from every legacy column and eligibleTriageTasks re-specifies unconditiona * FNXC:WorkflowReplan 2026-07-15-13:15: * FN-7977: a planning/provider recovery may finish after another engine lane has * started execution. Recovery callers must prove the live row is still planning - * before writing planning state; worktrees, materialized steps, and execution or - * terminal columns are durable evidence that the task has advanced. + * before writing planning state; worktrees and execution or terminal columns are + * durable evidence that the task has advanced. + * + * FNXC:WorkflowReplan 2026-07-16-05:35: + * Materialized steps are NOT advancement evidence for a card still parked in a planner + * lane. Triage materializes steps when it finalizes a spec, so every replan (Plan Review + * REVISE -> needs-replan) legitimately carries the steps of its previous planning pass. + * Counting steps>0 as "advanced" made the primary triage claim in specifyTask() skip its + * status:"planning" write on every poll: the card was re-claimed forever, never planned, + * and — because wedged cards keep occupying maxTriageConcurrent slots — starved every + * healthy card queued behind them. Both planner surfaces must stay plannable: the "triage" + * column, and plan-in-place workflows (Coding (Ideas)) that park needs-replan cards in + * "todo" carrying a real spec. A planned-and-queued "todo" card with no planning status is + * still genuinely advanced, so steps remain the deciding signal there. */ -export function hasAdvancedPastPlanning(task: Pick): boolean { - return ( + +/** Statuses that explicitly park a card for (re)planning, whichever column holds it. */ +const PLANNING_STAGE_STATUSES = new Set(["planning", "needs-replan", "plan-review-unavailable"]); + +export function hasAdvancedPastPlanning( + task: Pick, +): boolean { + if ( task.column === "in-progress" || task.column === "in-review" || task.column === "done" || task.column === "archived" - || task.worktree != null - || (task.steps?.length ?? 0) > 0 - ); + ) { + return true; + } + // A worktree proves an executor claimed the card, even while it still sits in a planner lane. + if (task.worktree != null) { + return true; + } + // The planner column itself is never "advanced" — nothing executes out of triage. + if (task.column === "triage") { + return false; + } + // Plan-in-place planner lane ("todo"): a card explicitly parked for planning has not advanced. + if (task.status != null && PLANNING_STAGE_STATUSES.has(task.status)) { + return false; + } + return (task.steps?.length ?? 0) > 0; } -export function isTaskStillInPlanningStage(task: Pick): boolean { +export function isTaskStillInPlanningStage( + task: Pick, +): boolean { return !hasAdvancedPastPlanning(task); } diff --git a/packages/engine/src/triage.ts b/packages/engine/src/triage.ts index 129f46eff3..02ced2b0f5 100644 --- a/packages/engine/src/triage.ts +++ b/packages/engine/src/triage.ts @@ -1012,7 +1012,20 @@ export class TriageProcessor { const agentWork = async () => { // Set status only after the semaphore slot has been acquired, so // tasks waiting in the queue don't appear as "planning". + /* + FNXC:Triage 2026-07-16-05:35: + A skip on this PRIMARY claim path is an anomaly, not a benign scheduler race: poll() + already proved the card is an eligible planner candidate, so failing the guard here + means it is re-claimed every poll, never planned, and holds a maxTriageConcurrent slot + against healthy cards. Recovery-write skips stay silent by design (see + updatePlanningStateIfStillCurrent); this one must be visible — the FN-7977 steps>0 + wedge stalled the whole planner for hours precisely because it logged nothing. + */ if (!await this.updatePlanningStateIfStillCurrent(task, { status: "planning" })) { + planLog.warn( + `${task.id}: planning claim skipped — live row is no longer in the planning stage; ` + + "it will be re-claimed on the next poll", + ); return; }