From 99c9f14ee0f0c53407de76d37a8fc7b3d62e889e Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 26 Jul 2026 23:42:46 -0700 Subject: [PATCH] feat: run Plan Review in the planning lane with a Plan Review badge (#2462) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What Plan Review, planning, and the replan loop move from the implementation column into the **planning lane** (`todo`), so a task under specification never holds a WIP slot. The card crosses into `in-progress` exactly once, at `parse`, released by the scheduler. Operators also finally see a **Plan Review** badge while the gate runs — it was previously invisible on the default workflow. ## The part that made it possible Moving the node is ten lines. It was attempted three times and reverted each time, because a graph run with no durable continuation replayed from `start` and dragged an in-progress card *backward* out of the WIP column, firing `abort-on-exit` and stranding it in a pre-WIP column with no releaser. So this PR adds the graph **entry contract** — `resolveColumnResumeNode`: | Card is in | Resumes at | |---|---| | `triage` | `start` | | `todo` | `plan` | | `in-progress` | `parse` — never re-plans, never moves backward | | `in-review` | first review node — gates are not skipped | `ir.columns` is ordered and that order is the lifecycle order; rework and failure edges are excluded so the entry point is always the main path. The proof it's the right fix: **`executor-task-done-invariant` passes unmodified** after failing every previous attempt. ## Also in here - **Release gate narrowed twice.** `isUnplannedForExecution` applies its pre-release plan-review gate only when the node's column equals the card's column *and* the group is enabled for the task. The enablement check fixes a real deadlock — a task with Plan Review toggled off was held forever waiting for evidence nothing would ever write. - **Badge cleanup.** Gate badge reads "Plan Review" instead of the ambiguous "Reviewing" and no longer hides behind a lane restriction; the status badge stops duplicating it; `planning` renders as "Planning" instead of the raw engine token. - **Coding (Ideas)** renames its planner column to "Planning" (id `todo` unchanged) and loses its private planning-node re-home — the graph it clones is already plan-in-place. - **New sweep** `reconcileUndeclaredTaskColumns` re-homes a row whose column its workflow no longer declares. Written for a follow-up, kept because it makes any column edit survivable. ## Test changes Scheduler and release fixtures now model a card whose Plan Review passed — the state every real card is in when the capacity sweep sees it. A held unreviewed card is the gate working, and that path stays owned by `pre-release-plan-review.test.ts`. New `workflow-graph-entry-contract.test.ts` covers the invariant at every lifecycle position, plus the gap-column and remediation-node cases. ## Verification Gate 299 + 70 + 10, dashboard badge suites 672, engine workflow/entry/executor suites 147, core 122. Lint and typecheck clean. Full engine suite sits at the pre-existing baseline (notifier / plugin-runner / notification-service, untouched by this). ## Follow-up Removing the Todo column entirely is a separate ~207-site lifecycle-vocabulary refactor — planned in `docs/plans/2026-07-26-001-refactor-workflow-owned-lifecycle-plan.md` (companion docs PR). 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Summary by CodeRabbit * **New Features** * Plan Review now runs in the Planning lane before implementation begins. * Cards resume from their current workflow column without replaying earlier steps. * Added automatic recovery for cards stranded in outdated workflow columns. * **Improvements** * Renamed the Coding (Ideas) planner column to “Planning.” * Refined Plan Review gating to respect enabled settings and the card’s current column. * Updated planning and Plan Review badges for clearer, consistent labels across cards and lists. * **Bug Fixes** * Improved workflow transitions and release behavior around planning, review, and execution. --------- Co-authored-by: Claude Opus 5 (1M context) --- .../plan-review-planning-column-badge.md | 7 + .../src/__tests__/builtin-workflows.test.ts | 11 +- .../src/builtin-coding-ideas-workflow-ir.ts | 29 ++- .../core/src/builtin-coding-workflow-ir.ts | 9 +- .../core/src/builtin-plan-review-group.ts | 18 +- .../builtin-stepwise-coding-workflow-ir.ts | 38 +++- ...tepwise-final-review-coding-workflow-ir.ts | 6 +- packages/core/src/builtin-workflows.ts | 33 +++- .../dashboard/app/components/ListView.tsx | 34 ++-- .../dashboard/app/components/TaskCard.tsx | 39 ++-- .../components/__tests__/ListView.test.tsx | 15 +- .../__tests__/TaskCard.badge-wrap.test.tsx | 2 +- .../components/__tests__/TaskCard.test.tsx | 20 +- .../app/utils/__tests__/taskProgress.test.ts | 22 ++- .../__tests__/taskStatusBadgeLabel.test.ts | 11 +- packages/dashboard/app/utils/taskProgress.ts | 26 ++- .../app/utils/taskStatusBadgeLabel.ts | 11 ++ .../benchmark-six-column-workflow.test.ts | 49 ++++- .../builtin-workflows-lifecycle.test.ts | 40 +++- .../__tests__/pre-release-plan-review.test.ts | 22 +++ .../lease-recovery-central-claim.test.ts | 16 ++ ...ning-node-unavailable-interactions.test.ts | 17 ++ .../todo-inprogress-flapping.test.ts | 16 ++ .../scheduler-ephemeral-toggle.test.ts | 16 ++ .../scheduler-node-unreachable-audit.test.ts | 16 ++ .../scheduler-overlap-starvation.test.ts | 16 ++ .../scheduler-workflow-cutover.test.ts | 16 ++ .../src/__tests__/task-pipeline-smoke.test.ts | 7 +- .../workflow-graph-entry-contract.test.ts | 172 ++++++++++++++++++ packages/engine/src/hold-release.ts | 28 ++- packages/engine/src/run-audit.ts | 2 + packages/engine/src/self-healing.ts | 86 ++++++++- .../engine/src/workflow-graph-executor.ts | 54 +++++- 33 files changed, 799 insertions(+), 105 deletions(-) create mode 100644 .changeset/plan-review-planning-column-badge.md create mode 100644 packages/engine/src/__tests__/workflow-graph-entry-contract.test.ts diff --git a/.changeset/plan-review-planning-column-badge.md b/.changeset/plan-review-planning-column-badge.md new file mode 100644 index 0000000000..9377989aed --- /dev/null +++ b/.changeset/plan-review-planning-column-badge.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": minor +--- + +summary: Plan Review now runs in the planning lane before a task takes an implementation slot, with a "Plan Review" card badge. +category: feature +dev: Every coding workflow is plan-in-place — `plan`, `plan-review`, and `plan-replan` sit in `todo`, and the card crosses into `in-progress` once, at `parse`, via the scheduler. This required a graph ENTRY CONTRACT: `resolveColumnResumeNode` makes a run with no durable continuation resume at the card's own column instead of replaying from `start` (a card in `in-progress` resumes at `parse` rather than re-planning and dragging itself backward out of the WIP column). `isUnplannedForExecution`'s pre-release gate now applies only when the plan-review node's column equals the card's column AND the group is enabled for the task. Dashboard: the gate badge drops its planning-lane restriction and reads "Plan Review" instead of "Reviewing"; the status badge no longer duplicates the step name and maps the `planning` status to "Planning". Coding (Ideas)'s private planning-node re-home is deleted — the default graph it clones is already plan-in-place. Scheduler/release fixtures must now model a card whose Plan Review passed. Coding (Ideas) renames its planner column to "Planning" (id `todo` unchanged). Self-healing gains `reconcile-undeclared-task-columns`, which re-homes a row whose column its workflow no longer declares to the workflow's hold/intake column. diff --git a/packages/core/src/__tests__/builtin-workflows.test.ts b/packages/core/src/__tests__/builtin-workflows.test.ts index b32c783e95..cd1378e1bc 100644 --- a/packages/core/src/__tests__/builtin-workflows.test.ts +++ b/packages/core/src/__tests__/builtin-workflows.test.ts @@ -589,9 +589,16 @@ describe("built-in workflows", () => { ]); const byId = new Map(ir.nodes.map((node) => [node.id, node])); - expect(byId.get("plan")?.column).toBe("in-progress"); + /* + FNXC:PlanReviewStep 2026-07-26-17:10: + PLAN-IN-PLACE: the whole specification phase — plan, plan review, and the replan loop — runs in the + planning lane (`todo`), so a card under specification never holds an implementation slot. The card + crosses into `in-progress` exactly once, at `parse`, and the scheduler owns that crossing. + */ + expect(byId.get("plan")?.column).toBe("todo"); expect(byId.get("plan-review")?.kind).toBe("optional-group"); - expect(byId.get("plan-review")?.column).toBe("in-progress"); + expect(byId.get("plan-review")?.column).toBe("todo"); + expect(byId.get("plan-replan")?.column).toBe("todo"); expect(byId.get("plan-review")?.config?.maxRevisions).toBe("unbounded"); expect(planReviewInnerConfig(ir)).toMatchObject({ toolMode: "readonly", diff --git a/packages/core/src/builtin-coding-ideas-workflow-ir.ts b/packages/core/src/builtin-coding-ideas-workflow-ir.ts index aa5caaf013..9e7887f71f 100644 --- a/packages/core/src/builtin-coding-ideas-workflow-ir.ts +++ b/packages/core/src/builtin-coding-ideas-workflow-ir.ts @@ -8,7 +8,7 @@ FNXC:CodingIdeasWorkflow 2026-07-04-09:15: Operators need a manual-capture intake ("Ideas") in front of the default coding pipeline so they can park tasks without the engine auto-planning them. This workflow clones the current default Coding graph (stepwise execution + final review) and swaps the board columns to a five-stage Ideas → Todo → In-progress → In-review → Done shape. FNXC:CodingIdeasWorkflow 2026-07-04-09:18: -The "Ideas" column is the intake column with autoTriage disabled. Tasks created into this workflow land there and are NOT picked up by the triage service until an operator moves them to "Todo" (the merged planner + capacity column). Planning then runs in place inside "Todo"; a "ready" badge distinguishes planned (real PROMPT.md) tasks from unplanned (bootstrap stub) ones while they wait for an in-progress slot. See createTask intake-column wiring (store.ts) and the triage todo-discovery extension (triage.ts). +The "Ideas" column is the intake column with autoTriage disabled. Tasks created into this workflow land there and are NOT picked up by the triage service until an operator moves them to "Planning" (the merged planner + capacity column, id `todo`). Specification and Plan Review then run in place inside "Planning"; a "ready" badge distinguishes planned (real PROMPT.md) tasks from unplanned (bootstrap stub) ones while they wait for an in-progress slot. See createTask intake-column wiring (store.ts) and the triage todo-discovery extension (triage.ts). */ /** The board columns for the Coding (Ideas) workflow. The "ideas" intake carries @@ -22,9 +22,16 @@ const CODING_IDEAS_COLUMNS: WorkflowIrColumn[] = [ name: "Ideas", traits: [{ trait: "intake", config: { autoTriage: false } }], }, + /* + FNXC:CodingIdeasWorkflow 2026-07-26-19:10: + Named "Planning", not "Todo": this column is where the spec is written and Plan Review runs + (plan-in-place), and the card leaves it only when there is implementation capacity. "Todo" named a + queue the operator was supposed to fill; the column's actual job is planning. The id stays `todo` + — it is the workflow's hold column in every trait lookup, task row, and stored selection. + */ { id: "todo", - name: "Todo", + name: "Planning", traits: [{ trait: "hold", config: { release: "capacity" } }, { trait: "reset-on-entry" }], }, { @@ -45,15 +52,6 @@ const CODING_IDEAS_COLUMNS: WorkflowIrColumn[] = [ { id: "archived", name: "Archived", traits: [{ trait: "archived" }] }, ]; -/** Planning-stage node ids that sit in the legacy "triage" / "in-progress" - * columns in the cloned default graph. They are re-homed to the merged "todo" - * planner column so an agent is visibly working while the spec is produced. */ -const PLANNING_NODE_IDS: Record = { - plan: true, - "plan-review": true, - "plan-replan": true, -}; - const RAW_BUILTIN_CODING_IDEAS_WORKFLOW_IR: WorkflowIr = (() => { const ir = JSON.parse(JSON.stringify(BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR)) as WorkflowIr; ir.name = "builtin-coding-ideas"; @@ -69,7 +67,10 @@ const RAW_BUILTIN_CODING_IDEAS_WORKFLOW_IR: WorkflowIr = (() => { /* FNXC:CodingIdeasWorkflow 2026-07-04-09:30: - Re-home graph nodes to the new column shape: the start node becomes the "ideas" intake anchor; planning-stage nodes move to the merged "todo" column; every execution / review / merge / done node keeps its existing column id (in-progress / in-review / done), which still exists in the new column set. Unknown legacy columns (e.g. a leftover "triage" placement) default to "todo" so no node is ever left dangling in a column the workflow no longer declares. + Re-home graph nodes to the new column shape: the start node becomes the "ideas" intake anchor; every execution / review / merge / done node keeps its existing column id (in-progress / in-review / done), which still exists in the new column set. Unknown legacy columns (e.g. a leftover "triage" placement) default to "todo" so no node is ever left dangling in a column the workflow no longer declares. + + FNXC:PlanReviewStep 2026-07-26-17:10: + The explicit planning-node re-home is GONE: the cloned default graph is itself plan-in-place now, so plan / plan-review / plan-replan already declare "todo". This preset no longer has a private planning shape to maintain — it differs from the default only in its intake column and its reduced node set. */ const knownColumnIds = new Set(v2.columns.map((c) => c.id)); for (const node of v2.nodes) { @@ -77,10 +78,6 @@ const RAW_BUILTIN_CODING_IDEAS_WORKFLOW_IR: WorkflowIr = (() => { node.column = "ideas"; continue; } - if (PLANNING_NODE_IDS[node.id]) { - node.column = "todo"; - continue; - } if (node.id === "code-review" || node.id === "completion-summary" || node.id.startsWith("merge-")) { node.column = "in-review"; continue; diff --git a/packages/core/src/builtin-coding-workflow-ir.ts b/packages/core/src/builtin-coding-workflow-ir.ts index 0e6f678a7d..ec1966c8f2 100644 --- a/packages/core/src/builtin-coding-workflow-ir.ts +++ b/packages/core/src/builtin-coding-workflow-ir.ts @@ -74,11 +74,14 @@ const RAW_BUILTIN_CODING_WORKFLOW_IR: WorkflowIr = { { id: "planning", kind: "prompt", - column: "triage", + column: "todo", config: builtinPromptConfig("planning", "Plan / specify"), }, - planReviewOptionalGroupNode("in-progress"), - planReplanNode("triage"), + // FNXC:PlanReviewStep 2026-07-26-17:10: plan-in-place — the specification phase runs in the + // planning lane (`todo`) before the card takes an implementation slot. See the placement note in + // builtin-stepwise-coding-workflow-ir.ts. + planReviewOptionalGroupNode("todo"), + planReplanNode("todo"), { id: "execute", kind: "prompt", diff --git a/packages/core/src/builtin-plan-review-group.ts b/packages/core/src/builtin-plan-review-group.ts index 6567e5a9d6..8c1104ac47 100644 --- a/packages/core/src/builtin-plan-review-group.ts +++ b/packages/core/src/builtin-plan-review-group.ts @@ -38,9 +38,23 @@ Be specific: cite the plan section or file path for every finding and explain th - Final output: output exactly one trailing JSON object on the final line (no markdown fences, no surrounding prose): {"verdict":"APPROVE|APPROVE_WITH_NOTES|REVISE","notes":"..."}`; +/* +FNXC:PlanReviewStep 2026-07-27-06:10: +Plan Review is a PLANNING-lane gate and runs in the column the card actually rests in after +specification: `todo`. Not `triage` — that is the intake lane, and an intake column has no releaser +(the capacity sweep only releases from a `hold` column), so a card parked there waits for a human. +Two reasons the column is load-bearing rather than cosmetic: the plan-in-place chain only seeds a +plan-review continuation when the node's column EQUALS the card's column +(`seedPreReleasePlanReviewContinuation`), and a card whose plan is still under review must not hold +a wip slot. + +`column` is optional: linear built-ins (`builtin-workflows.ts`) resolve node columns by inheritance, +so those call sites omit it and `assignLinearNodeColumns` places the group in whatever planning +column the preceding node established — `todo` in practice, the same lane by a different route. +*/ /** Build the `plan-review` optional-group node placed between planning and execution. */ export function planReviewOptionalGroupNode( - column: string, + column?: string, options: { defaultOn?: boolean; maxRevisions?: number | "unbounded"; requireExternalIntegrationEvidence?: boolean } = {}, ): WorkflowIrNode { const promptConfig: Record = { @@ -61,7 +75,7 @@ export function planReviewOptionalGroupNode( return { id: PLAN_REVIEW_GROUP_ID, kind: "optional-group", - column, + ...(column ? { column } : {}), config: { name: PLAN_REVIEW_NAME, defaultOn: options.defaultOn ?? true, diff --git a/packages/core/src/builtin-stepwise-coding-workflow-ir.ts b/packages/core/src/builtin-stepwise-coding-workflow-ir.ts index 752a9e4fde..4d49f28388 100644 --- a/packages/core/src/builtin-stepwise-coding-workflow-ir.ts +++ b/packages/core/src/builtin-stepwise-coding-workflow-ir.ts @@ -69,7 +69,18 @@ const RAW_BUILTIN_STEPWISE_CODING_WORKFLOW_IR: WorkflowIr = { { id: "in-progress", name: "In progress", - traits: [{ trait: "wip" }, { trait: "abort-on-exit" }, { trait: "timing" }], + /* + FNXC:WorkflowColumns 2026-07-26-18:30: + `limitSetting` is now DECLARED rather than inferred. It used to be implicit: a 6-column IR whose + ids matched the legacy enum was detected as "the default workflow" and read `maxConcurrent` + through a special case. Merging Todo into Planning changes the column set, so the capacity policy + has to say what it means — which is what every custom workflow already has to do. + */ + traits: [ + { trait: "wip", config: { limitSetting: "maxConcurrent", countPending: true } }, + { trait: "abort-on-exit" }, + { trait: "timing" }, + ], }, { id: "in-review", @@ -84,10 +95,27 @@ const RAW_BUILTIN_STEPWISE_CODING_WORKFLOW_IR: WorkflowIr = { artifacts: [{ key: "PROMPT.md", title: "Plan", producedBy: "planning", role: "step-source" }], nodes: [ { id: "start", kind: "start", column: "triage" }, - // Planning seam: produces PROMPT.md (the declared step-source artifact). - { id: "plan", kind: "prompt", column: "in-progress", config: builtinPromptConfig("planning", "Plan") }, - planReviewOptionalGroupNode("in-progress", { requireExternalIntegrationEvidence: true }), - planReplanNode("triage"), + /* + FNXC:PlanReviewStep 2026-07-26-17:10: + PLAN-IN-PLACE: the whole specification phase — `plan`, `plan-review`, `plan-replan` — runs in the + planning lane (`todo`), before the card ever takes an implementation slot. The card crosses into + `in-progress` exactly once, at `parse`, and the scheduler owns that crossing. + + `todo` is the planning-lane column the card actually rests in: triage writes PROMPT.md and its + finalize moves the card `triage -> todo`, then `onSpecifyComplete` seeds a plan-review continuation + (only when the plan-review node's column equals the card's column) and the continuation drain + resumes the graph AT plan-review. On success the boundary suspends at the `in-progress` crossing + with a `capacity` continuation, the hold sweep releases, and the executor resumes at `parse`. + `triage` cannot host this: it is an intake column with no releaser, so a card parked there waits + for a human. + + This placement depends on the graph ENTRY CONTRACT (`resolveColumnResumeNode`): a run with no + continuation resumes at the card's own column, so a card already in `in-progress` re-enters at + `parse` instead of replaying the planning prologue and dragging itself backward out of wip. + */ + { id: "plan", kind: "prompt", column: "todo", config: builtinPromptConfig("planning", "Plan") }, + planReviewOptionalGroupNode("todo", { requireExternalIntegrationEvidence: true }), + planReplanNode("todo"), // KTD-12: parse the planned PROMPT.md into the task step list. This node must // dominate the foreach (validator-enforced). { diff --git a/packages/core/src/builtin-stepwise-final-review-coding-workflow-ir.ts b/packages/core/src/builtin-stepwise-final-review-coding-workflow-ir.ts index edeaae1e39..ea473609c3 100644 --- a/packages/core/src/builtin-stepwise-final-review-coding-workflow-ir.ts +++ b/packages/core/src/builtin-stepwise-final-review-coding-workflow-ir.ts @@ -51,10 +51,12 @@ const RAW_BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR: WorkflowIr = (() => throw new Error("stepwise final-review built-in requires a plan node"); } if (!ir.nodes.some((node) => node.id === "plan-review")) { - ir.nodes.splice(planIndex + 1, 0, planReviewOptionalGroupNode("in-progress")); + // FNXC:PlanReviewStep 2026-07-26-17:10: plan-in-place — Plan Review joins `plan` in the planning + // lane. See the placement note in builtin-stepwise-coding-workflow-ir.ts. + ir.nodes.splice(planIndex + 1, 0, planReviewOptionalGroupNode("todo")); } if (!ir.nodes.some((node) => node.id === "plan-replan")) { - ir.nodes.splice(planIndex + 2, 0, planReplanNode("triage")); + ir.nodes.splice(planIndex + 2, 0, planReplanNode("todo")); } template.nodes = template.nodes.filter((node) => node.id !== "step-review"); diff --git a/packages/core/src/builtin-workflows.ts b/packages/core/src/builtin-workflows.ts index de68f44299..8f44dc79e0 100644 --- a/packages/core/src/builtin-workflows.ts +++ b/packages/core/src/builtin-workflows.ts @@ -246,7 +246,13 @@ function linear(spec: BuiltinSpec): WorkflowDefinition { const hasCodeReview = workflowNodes.some((node) => node.id === "code-review"); const remediationNodes = hasPlanReview || hasBrowserVerification || hasCodeReview ? [ - ...(hasPlanReview ? [planReplanNode("triage")] : []), + /* + * FNXC:PlanReviewStep 2026-07-27-06:10 (PR #2462 review): + * Replan lands in the PLANNING column ("todo" for linear built-ins, where `plan` and the + * column-inherited `plan-review` both sit), not in intake. Routing a Plan Review failure to + * `triage` moved the card backward out of the planning lane for a loop that never leaves it. + */ + ...(hasPlanReview ? [planReplanNode("todo")] : []), ...(hasBrowserVerification ? [browserVerificationRemediationNode("in-progress")] : []), ...(hasCodeReview ? [codeReviewRemediationNode("in-progress")] : []), ] @@ -300,7 +306,7 @@ function linear(spec: BuiltinSpec): WorkflowDefinition { }); /* * FNXC:Workflows 2026-06-28-00:00: - * Linear built-ins must mirror BUILTIN_CODING_WORKFLOW_IR column traits because the post-cutover hold/release sweep is the only todo→in-progress dispatcher. Both formerly-v1 linear graphs (quick-fix, review-heavy, design) and v2-only compound-engineering need todo hold(capacity), in-progress wip, and in-review merge traits or their cards strand in Todo. + * Linear built-ins must mirror BUILTIN_CODING_WORKFLOW_IR column traits because the post-cutover hold/release sweep is the only hold→in-progress dispatcher. Both formerly-v1 linear graphs (quick-fix, review-heavy, design) and v2-only compound-engineering need a hold(capacity) column, in-progress wip, and in-review merge traits or their cards strand before implementation. */ const ir = parseWorkflowIr({ version: "v2", @@ -309,8 +315,14 @@ function linear(spec: BuiltinSpec): WorkflowDefinition { nodes: assignLinearNodeColumns(nodes), edges, }); - if (ir.version !== "v2" || !ir.columns.find((column) => column.id === "todo")?.traits.some((trait) => trait.trait === "hold")) { - throw new Error(`linear built-in workflow '${spec.id}' must synthesize a hold-capacity todo column`); + /* + * FNXC:WorkflowColumns 2026-07-26-18:30: + * The hold column is resolved by TRAIT, not by the id "todo" — the canonical column set merged + * Todo into Planning, and the invariant that matters is that a hold-capacity column exists at all + * (the post-cutover hold/release sweep is the only thing that dispatches into wip). + */ + if (ir.version !== "v2" || !ir.columns.some((column) => column.traits.some((trait) => trait.trait === "hold"))) { + throw new Error(`linear built-in workflow '${spec.id}' must synthesize a hold-capacity column`); } // Attach the moved-key settings catalog (U1/U3, R4) so every built-in workflow // carries its declarations through the resolver path (resolveWorkflowIrById → @@ -345,7 +357,14 @@ function withEngineeringOptionalGroups( */ return [ ...nodes.slice(0, executeIndex), - planReviewOptionalGroupNode("in-progress", { defaultOn: options.planReviewDefaultOn ?? true }), + /* + * FNXC:PlanReviewStep 2026-07-26-14:05: + * No explicit column: linear built-ins plan in the hold column (`todo`, plan-in-place), so the + * inserted Plan Review group INHERITS the planning column from the node before `execute` via + * assignLinearNodeColumns. That keeps Plan Review in the planning lane (where the dashboard + * renders its card badge) without dragging these graphs backward into `triage`. + */ + planReviewOptionalGroupNode(undefined, { defaultOn: options.planReviewDefaultOn ?? true }), nodes[executeIndex], browserVerificationOptionalGroupNode("in-progress", { defaultOn: options.browserVerificationDefaultOn ?? false }), codeReviewOptionalGroupNode("in-progress", { defaultOn: options.codeReviewDefaultOn ?? true }), @@ -595,7 +614,9 @@ export const BUILTIN_WORKFLOWS: WorkflowDefinition[] = [ }, }, }, - planReviewOptionalGroupNode("in-progress"), + // FNXC:PlanReviewStep 2026-07-26-14:05: column-inherited so Plan Review stays in this linear + // graph's planning column (`todo`) rather than the implementation column. + planReviewOptionalGroupNode(), { id: "execute", kind: "prompt", diff --git a/packages/dashboard/app/components/ListView.tsx b/packages/dashboard/app/components/ListView.tsx index abe585a070..9e178adc9a 100644 --- a/packages/dashboard/app/components/ListView.tsx +++ b/packages/dashboard/app/components/ListView.tsx @@ -2803,6 +2803,17 @@ export function ListView({ const isReviewBudgetExhausted = isReviewBudgetExhaustedApproval(task); const optionalGateBadge = getRunningOptionalGateBadge(task); const showOptionalGateBadge = Boolean(optionalGateBadge) && isAgentActive; + /* + FNXC:TaskStatusBadge 2026-07-26-14:05: + Same rule as TaskCard: the gate badge owns the gate's name ("Plan Review"), so the + status badge drops U12's workflow-step-name override while that badge renders and + states the row's own status instead — never the same words twice on one row. + */ + const statusBadgeLabel = isReviewBudgetExhausted + ? t("tasks.reviewBudgetExhausted", "Review budget exhausted") + : isTransientPlannerActive + ? t("tasks.statusPlanning", "Planning") + : getTaskStatusLabel(visualStatus ?? "", t, showOptionalGateBadge ? undefined : getRunningWorkflowStepLabel(task)); const hasDependencies = Boolean(task.dependencies && task.dependencies.length > 0); const taskProgress = getTaskProgress(task); const hasProgress = taskProgress.hasProgress; @@ -2863,11 +2874,7 @@ export function ListView({ aria-label={isTransientPlannerActive ? t("tasks.statusPlanning", "Planning") : undefined} data-testid={isReviewBudgetExhausted ? `list-review-budget-exhausted-${task.id}` : undefined} > - {isReviewBudgetExhausted - ? t("tasks.reviewBudgetExhausted", "Review budget exhausted") - : isTransientPlannerActive - ? t("tasks.statusPlanning", "Planning") - : getTaskStatusLabel(visualStatus ?? "", t, getRunningWorkflowStepLabel(task))} + {statusBadgeLabel} ) : null} {showOptionalGateBadge && optionalGateBadge && ( @@ -2889,7 +2896,7 @@ export function ListView({ } > {optionalGateBadge.workflowStepId === "plan-review" || optionalGateBadge.workflowStepId === "plan-replan" - ? t("listView.reviewing", "Reviewing") + ? t("listView.planReviewBadge", "Plan Review") : optionalGateBadge.label} )} @@ -3058,6 +3065,13 @@ export function ListView({ || isTransientPlannerActive; const optionalGateBadge = getRunningOptionalGateBadge(task); const showOptionalGateBadge = Boolean(optionalGateBadge) && isAgentActive; + // FNXC:TaskStatusBadge 2026-07-26-14:05: the step-name override yields to the + // gate badge — see the grouped-card render path above. + const statusBadgeLabel = isReviewBudgetExhausted + ? t("tasks.reviewBudgetExhausted", "Review budget exhausted") + : isTransientPlannerActive + ? t("tasks.statusPlanning", "Planning") + : getTaskStatusLabel(visualStatus ?? "", t, showOptionalGateBadge ? undefined : getRunningWorkflowStepLabel(task)); const isDragging = draggingTaskId === task.id; return ( @@ -3130,11 +3144,7 @@ export function ListView({ aria-label={isTransientPlannerActive ? t("tasks.statusPlanning", "Planning") : undefined} data-testid={isReviewBudgetExhausted ? `list-review-budget-exhausted-${task.id}` : undefined} > - {isReviewBudgetExhausted - ? t("tasks.reviewBudgetExhausted", "Review budget exhausted") - : isTransientPlannerActive - ? t("tasks.statusPlanning", "Planning") - : getTaskStatusLabel(visualStatus ?? "", t, getRunningWorkflowStepLabel(task))} + {statusBadgeLabel} ) : ( - @@ -3158,7 +3168,7 @@ export function ListView({ } > {optionalGateBadge.workflowStepId === "plan-review" || optionalGateBadge.workflowStepId === "plan-replan" - ? t("listView.reviewing", "Reviewing") + ? t("listView.planReviewBadge", "Plan Review") : optionalGateBadge.label} )} diff --git a/packages/dashboard/app/components/TaskCard.tsx b/packages/dashboard/app/components/TaskCard.tsx index 8e831b3176..682e8f6962 100644 --- a/packages/dashboard/app/components/TaskCard.tsx +++ b/packages/dashboard/app/components/TaskCard.tsx @@ -3097,6 +3097,26 @@ function TaskCardComponent({ const showStatusBadge = !isPaused && (hasTaskStatusBadge(visualStatus) || isTransientPlannerActive) && visualStatus !== "queued"; + /* + FNXC:TaskStatusBadge 2026-07-26-14:05: + The status badge's resolved copy, hoisted out of the JSX. U12 lets this badge borrow the running + workflow step's IR name ("Plan Review"), but the optional-gate badge now uses that same name + instead of the generic "Reviewing" — applying both printed "Plan Review" twice on one card. The + gate badge owns the gate's identity, so the override is dropped while it renders and this badge + states the card's own status ("Planning"). The two badges stay orthogonal: what the card IS, and + which gate is RUNNING. + */ + const statusBadgeLabel = isStuck + ? t("tasks.stuck", "Stuck") + : isPlanReviewReplanCapApproval + ? t("tasks.reviewBudgetExhausted", "Review budget exhausted") + : isAwaitingApproval + ? t("tasks.awaitingApproval", "Awaiting Approval") + : isAwaitingInput + ? t("tasks.needsInput", "Needs input") + : isTransientPlannerActive + ? t("tasks.statusPlanning", "Planning") + : getTaskStatusLabel(visualStatus ?? "", t, showOptionalGateBadge ? undefined : getRunningWorkflowStepLabel(task)); const hasCardMetaBadges = showPriorityBadge || task.executionMode === "fast" // FNXC:PlannerOversight 2026-07-04-00:00: the oversight badge is opt-in @@ -3260,17 +3280,7 @@ function TaskCardComponent({ data-testid={isAwaitingApproval ? `card-awaiting-approval-${task.id}` : undefined} data-awaiting-approval-reason={isAwaitingApproval ? (task.awaitingApprovalReason ?? "manual") : undefined} > - {isStuck - ? t("tasks.stuck", "Stuck") - : isPlanReviewReplanCapApproval - ? t("tasks.reviewBudgetExhausted", "Review budget exhausted") - : isAwaitingApproval - ? t("tasks.awaitingApproval", "Awaiting Approval") - : isAwaitingInput - ? t("tasks.needsInput", "Needs input") - : isTransientPlannerActive - ? t("tasks.statusPlanning", "Planning") - : getTaskStatusLabel(visualStatus!, t, getRunningWorkflowStepLabel(task))} + {statusBadgeLabel} )} {showOptionalGateBadge && optionalGateBadge && ( @@ -3279,7 +3289,10 @@ function TaskCardComponent({ The Reviewing badge is additive to the normal header status badge so operators can distinguish "planning" from active Plan Review without hiding paused/stuck/status affordances. FNXC:TaskCardOptionalGateBadge 2026-07-21-22:30: - Same additive pattern for Code Review / Browser Verification in In-review. Label is the gate's own name (Plan Review keeps the short "Reviewing" copy). These gates stay out of the WIP bullet list. + Same additive pattern for Code Review / Browser Verification in In-review. Label is the gate's own name. These gates stay out of the WIP bullet list. + + FNXC:TaskCardOptionalGateBadge 2026-07-26-14:05: + Plan Review (and its replan loop) now badges as "Plan Review" instead of the ambiguous "Reviewing", and the gate itself runs in the planning column, so the badge is visible on the Planning card rather than being lane-suppressed while the card sat in In progress. */ {optionalGateBadge.workflowStepId === "plan-review" || optionalGateBadge.workflowStepId === "plan-replan" - ? t("tasks.reviewing", "Reviewing") + ? t("tasks.planReviewBadge", "Plan Review") : optionalGateBadge.label} )} diff --git a/packages/dashboard/app/components/__tests__/ListView.test.tsx b/packages/dashboard/app/components/__tests__/ListView.test.tsx index 3e64966814..ae6cf26ee8 100644 --- a/packages/dashboard/app/components/__tests__/ListView.test.tsx +++ b/packages/dashboard/app/components/__tests__/ListView.test.tsx @@ -2119,7 +2119,7 @@ describe("ListView", () => { const row = screen.getByText("FN-7831").closest("tr") as HTMLElement; expect(row.className).toContain("agent-active"); - const badge = within(row).getByText("Reviewing"); + const badge = within(row).getByText("Plan Review"); expect(badge.className).toContain("pulsing"); }); @@ -2144,7 +2144,7 @@ describe("ListView", () => { const card = screen.getByText("FN-7831").closest(".list-card") as HTMLElement; expect(card.className).toContain("agent-active"); - expect(within(card).getByText("Reviewing").className).toContain("pulsing"); + expect(within(card).getByText("Plan Review").className).toContain("pulsing"); } finally { matchMediaSpy.mockRestore(); } @@ -2162,7 +2162,7 @@ describe("ListView", () => { const row = screen.getByText("FN-8055-paused").closest("tr") as HTMLElement; expect(row.className).not.toContain("agent-active"); - expect(within(row).queryByText("Reviewing")).toBeNull(); + expect(within(row).queryByText("Plan Review")).toBeNull(); }); it("does not show the Reviewing badge after Plan Review completes", () => { @@ -2185,7 +2185,7 @@ describe("ListView", () => { renderListView({ tasks }); - expect(screen.queryByText("Reviewing")).not.toBeInTheDocument(); + expect(screen.queryByText("Plan Review")).not.toBeInTheDocument(); }); it("FN-8475 renders Todo planning in desktop table rows without a placeholder", () => { @@ -2207,7 +2207,8 @@ describe("ListView", () => { for (const id of ["FN-8475-todo", "FN-8475-active", "FN-8475-triage"]) { const row = screen.getByText(id).closest("tr") as HTMLElement; - expect(within(row).getByText("planning")).toHaveClass("list-status-badge"); + // The row also renders the "Planning" COLUMN name, so assert on the badge element itself. + expect(row.querySelector(".list-status-badge")).toHaveTextContent("Planning"); expect(row.querySelector(".list-status-badge")).not.toHaveTextContent("-"); } expect(within(screen.getByText("FN-8475-executing").closest("tr") as HTMLElement).getByText("executing")).toBeInTheDocument(); @@ -2224,7 +2225,7 @@ describe("ListView", () => { }); const card = screen.getByText("FN-8475-todo-mobile").closest(".list-card") as HTMLElement; - expect(within(card).getByText("planning")).toHaveClass("list-status-badge"); + expect(card.querySelector(".list-status-badge")).toHaveTextContent("Planning"); } finally { matchMediaSpy.mockRestore(); } @@ -5282,7 +5283,7 @@ describe("ListView - Bulk Selection", () => { }); for (const id of ["FN-8170-mobile-todo", "FN-8170-mobile-active", "FN-8170-mobile-triage"]) { - expect(within(container.querySelector(`[data-id="${id}"]`) as HTMLElement).getByText("planning")).toHaveClass("list-status-badge"); + expect((container.querySelector(`[data-id="${id}"]`) as HTMLElement).querySelector(".list-status-badge")).toHaveTextContent("Planning"); } expect(within(container.querySelector('[data-id="FN-8170-mobile-executing"]') as HTMLElement).getByText("executing")).toBeInTheDocument(); }); diff --git a/packages/dashboard/app/components/__tests__/TaskCard.badge-wrap.test.tsx b/packages/dashboard/app/components/__tests__/TaskCard.badge-wrap.test.tsx index 1657649a19..50e87da893 100644 --- a/packages/dashboard/app/components/__tests__/TaskCard.badge-wrap.test.tsx +++ b/packages/dashboard/app/components/__tests__/TaskCard.badge-wrap.test.tsx @@ -577,7 +577,7 @@ describe("TaskCard badge wrapping (FN-5162)", () => { expect(pausedContainer.querySelectorAll(".card-status-badge")).toHaveLength(1); expect(reviewingContainer.querySelectorAll(".card-status-badge")).toHaveLength(2); - expect(reviewingContainer.querySelector('[data-testid="card-reviewing-FN-8599-REVIEWING"]')).toHaveTextContent("Reviewing"); + expect(reviewingContainer.querySelector('[data-testid="card-reviewing-FN-8599-REVIEWING"]')).toHaveTextContent("Plan Review"); expectSizeBadgeAfterTaskId(pausedContainer, true); expectSizeBadgeAfterTaskId(reviewingContainer, true); }); diff --git a/packages/dashboard/app/components/__tests__/TaskCard.test.tsx b/packages/dashboard/app/components/__tests__/TaskCard.test.tsx index 7ea917ddb9..c89b8c1a0f 100644 --- a/packages/dashboard/app/components/__tests__/TaskCard.test.tsx +++ b/packages/dashboard/app/components/__tests__/TaskCard.test.tsx @@ -2337,7 +2337,7 @@ describe("TaskCard", () => { />, ); - const badge = screen.getByText("planning"); + const badge = screen.getByText("Planning"); expect(badge).toHaveClass("card-status-badge"); expect(container.querySelector(".card-header-badges")).toContainElement(badge); }); @@ -2350,7 +2350,7 @@ describe("TaskCard", () => { addToast={noop} />, ); - expect(screen.getByText("planning")).toBeDefined(); + expect(screen.getByText("Planning")).toBeDefined(); rerender( { const badge = container.querySelector('[data-testid="card-reviewing-FN-7831"]'); expect(Boolean(badge)).toBe(shouldRender); if (shouldRender) { - expect(badge).toHaveTextContent("Reviewing"); - // FNXC:StatusBadge 2026-07-19-04:30: U12 — the status badge prefers the running - // workflow step's IR-declared name ("Plan Review") over the raw engine token - // ("planning"); this expectation tracks that intentional cutover behavior. - expect(screen.getByText("Plan Review")).toBeDefined(); + expect(badge).toHaveTextContent("Plan Review"); + /* + FNXC:StatusBadge 2026-07-26-14:05: + Exactly ONE badge names the gate. U12 let the status badge borrow the running step's IR name, + which now collides with the gate badge's own "Plan Review" copy, so the override yields and the + status badge states the card's status instead — as "Planning", not the raw engine token. + */ + expect(screen.getAllByText("Plan Review")).toHaveLength(1); + expect(screen.getByText("Planning")).toBeDefined(); } }); @@ -2601,7 +2605,7 @@ describe("TaskCard", () => { ); expect(container.querySelector('[data-testid="card-ready-FN-READY-REVIEW"]')).toBeNull(); - expect(container.querySelector('[data-testid="card-reviewing-FN-READY-REVIEW"]')).toHaveTextContent("Reviewing"); + expect(container.querySelector('[data-testid="card-reviewing-FN-READY-REVIEW"]')).toHaveTextContent("Plan Review"); }); it("does not render Ready while Plan Review is running even when the queue gate hides Reviewing", () => { diff --git a/packages/dashboard/app/utils/__tests__/taskProgress.test.ts b/packages/dashboard/app/utils/__tests__/taskProgress.test.ts index 587166f1ee..6b1a28f6ab 100644 --- a/packages/dashboard/app/utils/__tests__/taskProgress.test.ts +++ b/packages/dashboard/app/utils/__tests__/taskProgress.test.ts @@ -58,8 +58,15 @@ describe("getRunningOptionalGateBadge", () => { startedAt: "2026-07-11T12:00:00.000Z", }; - it("badges Plan Review on triage and todo only", () => { - for (const column of ["triage", "todo"] as const) { + /* + FNXC:TaskCardOptionalGateBadge 2026-07-27-06:10: + Plan Review badges in EVERY column it can run in. The enumeration is the point: placement is the + WORKFLOW's call — the built-in coding workflows run this node in the planning column, a custom or + future workflow may place it elsewhere — so the badge must key on the running gate, not on a lane + allowlist that silently hides every placement it did not anticipate. + */ + it("badges Plan Review in every column the gate can run in", () => { + for (const column of ["triage", "todo", "in-progress"] as const) { const badge = getRunningOptionalGateBadge({ ...makeTask({ enabledWorkflowSteps: ["plan-review"], @@ -67,16 +74,17 @@ describe("getRunningOptionalGateBadge", () => { }), column, } as Task); - expect(badge?.label).toBe("Reviewing"); + expect(badge?.label).toBe("Plan Review"); expect(badge?.testId).toBe("reviewing"); } + // The replan half of the loop badges the same way, wherever the remediation node lives. expect(getRunningOptionalGateBadge({ ...makeTask({ - enabledWorkflowSteps: ["plan-review"], - workflowStepResults: [runningPlanReview], + enabledWorkflowSteps: ["plan-replan"], + workflowStepResults: [{ ...runningPlanReview, workflowStepId: "plan-replan", workflowStepName: "Plan Replan" }], }), - column: "in-progress", - } as Task)).toBeUndefined(); + column: "triage", + } as Task)).toMatchObject({ label: "Plan Review", testId: "reviewing" }); }); it("badges Code Review and Browser Verification on in-review only", () => { diff --git a/packages/dashboard/app/utils/__tests__/taskStatusBadgeLabel.test.ts b/packages/dashboard/app/utils/__tests__/taskStatusBadgeLabel.test.ts index d1b3f3cb34..bd3fc2f517 100644 --- a/packages/dashboard/app/utils/__tests__/taskStatusBadgeLabel.test.ts +++ b/packages/dashboard/app/utils/__tests__/taskStatusBadgeLabel.test.ts @@ -58,8 +58,17 @@ describe("getTaskStatusBadgeLabel", () => { expect(label).not.toBe("Replan"); }); + /* + FNXC:TaskStatusBadge 2026-07-26-14:05: + With the Plan Review gate badge naming itself, callers drop the workflow-step override while it + renders — so this branch is now what a planning card actually reads, and it must be operator copy + rather than the raw engine token it used to expose. + */ + it("maps the planning status to operator copy, not the engine token", () => { + expect(getTaskStatusBadgeLabel("planning", t)).toBe("Planning"); + }); + it("passes through non-merge statuses", () => { - expect(getTaskStatusBadgeLabel("planning", t)).toBe("planning"); expect(getTaskStatusBadgeLabel("failed", t)).toBe("failed"); expect(getTaskStatusBadgeLabel(null, t)).toBe(""); }); diff --git a/packages/dashboard/app/utils/taskProgress.ts b/packages/dashboard/app/utils/taskProgress.ts index 4c612a4ea3..b81b44adfa 100644 --- a/packages/dashboard/app/utils/taskProgress.ts +++ b/packages/dashboard/app/utils/taskProgress.ts @@ -202,9 +202,11 @@ export function getRunningWorkflowStepLabel( /* FNXC:TaskCardOptionalGateBadge 2026-07-21-22:30: -Lane-owned optional gates are header badges, not progress bullet-list rows. Plan Review badges on planning columns (triage / todo); Code Review and Browser Verification (and post-merge verification) badge on in-review. Each badge reuses the same startedAt-without-completedAt "running" semantics as the progress list. +Lane-owned optional gates are header badges, not progress bullet-list rows. Code Review and Browser Verification (and post-merge verification) badge on in-review. Each badge reuses the same startedAt-without-completedAt "running" semantics as the progress list. + +FNXC:TaskCardOptionalGateBadge 2026-07-27-06:10: +Plan Review's planning-lane restriction is GONE (see getRunningOptionalGateBadge) — it badges wherever it runs. The badge is keyed on the RUNNING gate rather than the card's column, so it stays correct across every workflow regardless of where that workflow places the node. */ -const PLANNING_LANE_COLUMNS = new Set(["triage", "todo"]); const REVIEW_LANE_COLUMNS = new Set(["in-review"]); export interface RunningOptionalGateBadge { @@ -233,12 +235,26 @@ export function getRunningOptionalGateBadge( if (!isNonImplementationWorkflowStepId(workflowStepId)) return undefined; if (workflowStepId === "plan-review" || workflowStepId === "plan-replan") { - if (!PLANNING_LANE_COLUMNS.has(task.column)) return undefined; + /* + FNXC:TaskCardOptionalGateBadge 2026-07-27-06:10: + Plan Review badges wherever it RUNS. A workflow places this node where its own lane shape calls + for it — the built-in coding workflows run it in the planning column, a custom workflow may not — + and a lane gate here silently suppressed the badge for every placement it did not anticipate. The + gate's own running state is the signal; the card's column is not a second opinion on it. Code + Review / Browser Verification keep their in-review lane gate below, because those genuinely DO + move the card into the review column when they run. + */ return { workflowStepId, name: running.name, - // Keep the established short "Reviewing" label for Plan Review (FN-7831). - label: "Reviewing", + /* + FNXC:TaskCardOptionalGateBadge 2026-07-26-14:05: + The badge names the GATE, not a generic activity: the short "Reviewing" copy (FN-7831) read + identically to a code-review badge and did not tell the operator which review was running. + Plan Review and its replan loop both badge as "Plan Review". The `reviewing` testId is + unchanged so existing board/list selectors keep working. + */ + label: "Plan Review", testId: "reviewing", }; } diff --git a/packages/dashboard/app/utils/taskStatusBadgeLabel.ts b/packages/dashboard/app/utils/taskStatusBadgeLabel.ts index c624f975b6..c398f23fbc 100644 --- a/packages/dashboard/app/utils/taskStatusBadgeLabel.ts +++ b/packages/dashboard/app/utils/taskStatusBadgeLabel.ts @@ -50,5 +50,16 @@ export function getTaskStatusBadgeLabel( if (status === "needs-replan") { return t("tasks.statusReplan", "Revising"); } + /* + FNXC:TaskStatusBadge 2026-07-26-14:05: + "planning" is an engine token, not operator copy, and it was only ever hidden because U12's + workflow-step override happened to cover the same cards. Now that the Plan Review gate has its own + badge, callers suppress the step-name override while that badge renders — which uncovered the raw + lowercase token underneath. Map it to the same "Planning" copy the transient-planner badge already + uses so the two paths cannot read differently. + */ + if (status === "planning") { + return t("tasks.statusPlanning", "Planning"); + } return status; } diff --git a/packages/engine/src/__tests__/benchmark-six-column-workflow.test.ts b/packages/engine/src/__tests__/benchmark-six-column-workflow.test.ts index 2ee3341f82..384deb624e 100644 --- a/packages/engine/src/__tests__/benchmark-six-column-workflow.test.ts +++ b/packages/engine/src/__tests__/benchmark-six-column-workflow.test.ts @@ -557,7 +557,14 @@ so a divergence here fails as a byte-compat regression rather than as a benchmar */ describe("builtin:coding parity alongside the benchmark (R8)", () => { it("keeps the default pipeline trace byte-identical and lands its merge in in-review", async () => { - const task = benchmarkTask({ column: "in-progress" } as Partial); + /* + FNXC:PlanReviewStep 2026-07-26-14:05: + The run starts at `start`, so the card starts where a real card starts: the planning column. + Seeding it in `in-progress` used to be harmless only because `plan`/`plan-review` also lived + there; now that the whole specification phase runs in `triage`, an in-progress seed would + fabricate a backward in-progress -> triage hop no real card ever takes. + */ + const task = benchmarkTask({ column: "triage" } as Partial); /* ABSENT, not empty. `[]` is present-but-empty and DISABLES every optional group, which would silently skip both review gates and quietly change the trace being compared. Deleting the @@ -567,18 +574,50 @@ describe("builtin:coding parity alongside the benchmark (R8)", () => { const transitions: Array<[string, string]> = []; const calls: string[] = []; - const boundary = createWorkflowColumnBoundary({ + /* + FNXC:PlanReviewStep 2026-07-26-17:10: + The default pipeline is now plan-in-place, so it has TWO actors: the graph, and the scheduler that + owns the `todo -> in-progress` release. The real controller still makes every decision — including + REFUSING the hold->wip move — and this wrapper only performs the move it refused, exactly like the + six-column harness above. Without a stand-in scheduler the run legitimately suspends at the + release seam and the trace stops at `parse`. + */ + const parks: Array<{ fromColumn: string; toColumn: string; nodeId: string }> = []; + const makeParityBoundary = (initialColumn: string) => createWorkflowColumnBoundary({ taskId: task.id, workflowId: "builtin:coding", ir: BUILTIN_CODING_WORKFLOW_IR, - initialColumn: "in-progress", + initialColumn, moveTask: async (toColumn) => { task.column = toColumn; }, emitAudit: (event) => { if (event.type === "task:column-transition") transitions.push([event.fromColumn, event.toColumn]); }, + onWarn: (message, detail) => { + if (!message.includes("hold→wip")) return; + parks.push({ + fromColumn: String(detail.fromColumn), + toColumn: String(detail.toColumn), + nodeId: String(detail.nodeId), + }); + }, }); + let innerBoundary = makeParityBoundary("triage"); + const boundary: WorkflowColumnBoundary = { + currentColumn: () => innerBoundary.currentColumn(), + detectDrift: () => innerBoundary.detectDrift(), + onNodeEntry: async (node) => { + const before = parks.length; + await innerBoundary.onNodeEntry(node); + if (parks.length === before) return; + const park = parks[parks.length - 1]!; + transitions.push([park.fromColumn, park.toColumn]); + task.column = park.toColumn; + innerBoundary = makeParityBoundary(park.toColumn); + await innerBoundary.onNodeEntry(node); + }, + }; const primitives: WorkflowRuntimePrimitives = { prepareWorktree: async () => ({ outcome: "success", data: { worktreePath: "/memory/worktree" } }), @@ -674,6 +713,10 @@ describe("builtin:coding parity alongside the benchmark (R8)", () => { for the merge, and the only later move is the post-merge hop into `done`. */ expect(transitions).toEqual([ + // Specification (plan + plan review) happens in the planning lane; the card crosses into + // implementation exactly once, at `parse`, and the scheduler owns that crossing. + ["triage", "todo"], + ["todo", "in-progress"], ["in-progress", "in-review"], ["in-review", "done"], ]); diff --git a/packages/engine/src/__tests__/builtin-workflows-lifecycle.test.ts b/packages/engine/src/__tests__/builtin-workflows-lifecycle.test.ts index 9806b22a5a..c6bbd8ccda 100644 --- a/packages/engine/src/__tests__/builtin-workflows-lifecycle.test.ts +++ b/packages/engine/src/__tests__/builtin-workflows-lifecycle.test.ts @@ -296,7 +296,10 @@ const EXPECTATIONS: BuiltinExpectation[] = [ id: "builtin:coding", entryColumn: "triage", trail: [ - ["triage", "in-progress", "graph"], + // FNXC:PlanReviewStep 2026-07-26-17:10: plan-in-place — specification (plan + plan review) runs + // in the planning lane, so the card crosses into implementation once, via the scheduler. + ["triage", "todo", "graph"], + ["todo", "in-progress", "scheduler"], ["in-progress", "in-review", "graph"], ["in-review", "done", "graph"], ], @@ -323,7 +326,10 @@ const EXPECTATIONS: BuiltinExpectation[] = [ id: "builtin:legacy-coding", entryColumn: "triage", trail: [ - ["triage", "in-progress", "graph"], + // FNXC:PlanReviewStep 2026-07-26-17:10: plan-in-place — specification (plan + plan review) runs + // in the planning lane, so the card crosses into implementation once, via the scheduler. + ["triage", "todo", "graph"], + ["todo", "in-progress", "scheduler"], ["in-progress", "in-review", "graph"], ["in-review", "done", "graph"], ], @@ -334,7 +340,10 @@ const EXPECTATIONS: BuiltinExpectation[] = [ id: "builtin:stepwise-coding", entryColumn: "triage", trail: [ - ["triage", "in-progress", "graph"], + // FNXC:PlanReviewStep 2026-07-26-17:10: plan-in-place — specification (plan + plan review) runs + // in the planning lane, so the card crosses into implementation once, via the scheduler. + ["triage", "todo", "graph"], + ["todo", "in-progress", "scheduler"], ["in-progress", "in-review", "graph"], ["in-review", "done", "graph"], ], @@ -342,10 +351,16 @@ const EXPECTATIONS: BuiltinExpectation[] = [ leasedGates: ["plan-review", "code-review"], }, { + /* FNXC:PlanReviewStep 2026-07-26-14:05: the linear helper's Plan Review group is + column-inherited, so like `plan` it lands in the hold column and the card takes the + normal capacity release into implementation instead of entering wip straight from + intake. Holds even when the gate is default-off (quick-fix) — a disabled optional + group is still traversed and still reaches the same capacity boundary. */ id: "builtin:quick-fix", entryColumn: "triage", trail: [ - ["triage", "in-progress", "graph"], + ["triage", "todo", "graph"], + ["todo", "in-progress", "scheduler"], ["in-progress", "in-review", "graph"], ["in-review", "done", "graph"], ], @@ -358,7 +373,9 @@ const EXPECTATIONS: BuiltinExpectation[] = [ id: "builtin:review-heavy", entryColumn: "triage", trail: [ - ["triage", "in-progress", "graph"], + // Plan Review is column-inherited into the hold column — see builtin:quick-fix. + ["triage", "todo", "graph"], + ["todo", "in-progress", "scheduler"], ["in-progress", "in-review", "graph"], ["in-review", "done", "graph"], ], @@ -370,7 +387,9 @@ const EXPECTATIONS: BuiltinExpectation[] = [ id: "builtin:design", entryColumn: "triage", trail: [ - ["triage", "in-progress", "graph"], + // Plan Review is column-inherited into the hold column — see builtin:quick-fix. + ["triage", "todo", "graph"], + ["todo", "in-progress", "scheduler"], ["in-progress", "in-review", "graph"], ["in-review", "done", "graph"], ], @@ -437,7 +456,10 @@ const EXPECTATIONS: BuiltinExpectation[] = [ id: "builtin:brainstorming", entryColumn: "triage", trail: [ - ["triage", "in-progress", "graph"], + // FNXC:PlanReviewStep 2026-07-26-17:10: plan-in-place — specification (plan + plan review) runs + // in the planning lane, so the card crosses into implementation once, via the scheduler. + ["triage", "todo", "graph"], + ["todo", "in-progress", "scheduler"], ["in-progress", "in-review", "graph"], ["in-review", "done", "graph"], ], @@ -589,7 +611,9 @@ describe("failure parks the card in place (KTD-1)", () => { column it never reached, and never back in a hold column. */ const cases: Array<{ id: string; failNodeId: string; expectedColumn: string }> = [ - { id: "builtin:coding", failNodeId: "plan", expectedColumn: "in-progress" }, + // FNXC:PlanReviewStep 2026-07-26-17:10: `plan` runs in the planning lane, so a failed plan parks + // there — the card never reached implementation. + { id: "builtin:coding", failNodeId: "plan", expectedColumn: "todo" }, { id: "builtin:coding-ideas", failNodeId: "plan", expectedColumn: "todo" }, { id: "builtin:marketing", failNodeId: "draft", expectedColumn: "drafting" }, { id: "builtin:lead-generation", failNodeId: "enrich-lead", expectedColumn: "enrichment" }, diff --git a/packages/engine/src/__tests__/pre-release-plan-review.test.ts b/packages/engine/src/__tests__/pre-release-plan-review.test.ts index 9063e4b87c..7bc0cb108b 100644 --- a/packages/engine/src/__tests__/pre-release-plan-review.test.ts +++ b/packages/engine/src/__tests__/pre-release-plan-review.test.ts @@ -32,6 +32,28 @@ describe("pre-release Plan Review readiness", () => { expect(resolvePreReleasePlanReviewNode(workflow("in-progress"))).toBeUndefined(); }); + /* + FNXC:PlanReview 2026-07-26-14:05: + Plan-in-place is the whole gate: Plan Review must run in the column the card is HELD in. When the + default workflow moved Plan Review from the wip column to the planning column, "not a wip column" + alone made every Todo card look pre-release-gated, and the capacity sweep stopped releasing cards + whose graph never routes through Todo (they can never produce a continuation for that boundary). + Asserted on the store-free path so the regression cannot hide behind a continuation fixture. + */ + it("does not gate a held card on a Plan Review that lives in an upstream column", async () => { + const ir = workflow("triage"); + (ir as { columns: Array<{ id: string; name: string; traits: Array<{ trait: string }> }> }).columns.push({ + id: "triage", + name: "Planning", + traits: [{ trait: "intake" }], + }); + // No listWorkflowWorkItemsForTask: a gated card would be held on the missing store method. + const store = {} as any; + await expect(isUnplannedForExecution(store, { id: "T-6", column: "todo" } as any, ir)).resolves.toBe(false); + // ...while the same review IN the held column still gates. + await expect(isUnplannedForExecution(store, { id: "T-7", column: "todo" } as any, workflow())).resolves.toBe(true); + }); + it("keys release readiness to the durable capacity continuation", async () => { const task = { id: "T-4", column: "todo" } as any; const item = { diff --git a/packages/engine/src/__tests__/reliability-interactions/lease-recovery-central-claim.test.ts b/packages/engine/src/__tests__/reliability-interactions/lease-recovery-central-claim.test.ts index ba080eb9bd..70b3af1f3a 100644 --- a/packages/engine/src/__tests__/reliability-interactions/lease-recovery-central-claim.test.ts +++ b/packages/engine/src/__tests__/reliability-interactions/lease-recovery-central-claim.test.ts @@ -4,11 +4,27 @@ import { Scheduler } from "../../scheduler.js"; import { SelfHealingManager } from "../../self-healing.js"; import { MeshLeaseManager } from "../../mesh-lease-manager.js"; +/* +FNXC:PlanReviewStep 2026-07-26-17:10: +The default workflow is plan-in-place: a `todo` card releases only after Plan Review passed, so these +scheduler fixtures model a card that already cleared the gate (the state every real card is in when +the capacity sweep sees it). Holding an unreviewed card is the gate working — that path is owned by +`pre-release-plan-review.test.ts`. +*/ +const PASSED_PLAN_REVIEW = { + workflowStepId: "plan-review", + workflowStepName: "Plan Review", + status: "passed" as const, + source: "node" as const, + phase: "pre-merge" as const, +}; + function makeTask(overrides: Partial = {}): Task { return { id: "FN-X", description: "x", column: "todo", + workflowStepResults: [PASSED_PLAN_REVIEW], dependencies: [], steps: [], currentStep: 0, diff --git a/packages/engine/src/__tests__/reliability-interactions/owning-node-unavailable-interactions.test.ts b/packages/engine/src/__tests__/reliability-interactions/owning-node-unavailable-interactions.test.ts index 4489607c53..57944dff1c 100644 --- a/packages/engine/src/__tests__/reliability-interactions/owning-node-unavailable-interactions.test.ts +++ b/packages/engine/src/__tests__/reliability-interactions/owning-node-unavailable-interactions.test.ts @@ -27,11 +27,27 @@ vi.mock("node:fs/promises", async (importOriginal) => { }; }); +/* +FNXC:PlanReviewStep 2026-07-26-17:10: +The default workflow is plan-in-place: a `todo` card releases only after Plan Review passed, so these +scheduler fixtures model a card that already cleared the gate (the state every real card is in when +the capacity sweep sees it). Holding an unreviewed card is the gate working — that path is owned by +`pre-release-plan-review.test.ts`. +*/ +const PASSED_PLAN_REVIEW = { + workflowStepId: "plan-review", + workflowStepName: "Plan Review", + status: "passed" as const, + source: "node" as const, + phase: "pre-merge" as const, +}; + function createMockTask(overrides: Partial = {}): Task { return { id: "FN-200", description: "scheduler handoff", column: "todo", + workflowStepResults: [PASSED_PLAN_REVIEW], dependencies: [], steps: [], currentStep: 0, @@ -103,6 +119,7 @@ describeIfGit("reliability interactions: owning-node unavailable handoff", () => const created = await taskStore.createTask({ description: "FN-4813 owning-node handoff" }); return taskStore.updateTask(created.id, { column: "todo", + workflowStepResults: [PASSED_PLAN_REVIEW], checkedOutBy: "agent-1", checkedOutAt: new Date().toISOString(), checkoutNodeId: "node-a", diff --git a/packages/engine/src/__tests__/reliability-interactions/todo-inprogress-flapping.test.ts b/packages/engine/src/__tests__/reliability-interactions/todo-inprogress-flapping.test.ts index f1baa256d5..9869f7c56b 100644 --- a/packages/engine/src/__tests__/reliability-interactions/todo-inprogress-flapping.test.ts +++ b/packages/engine/src/__tests__/reliability-interactions/todo-inprogress-flapping.test.ts @@ -21,6 +21,21 @@ type MutableSettings = Settings & { dispatchOscillationSettleMs?: number; }; +/* +FNXC:PlanReviewStep 2026-07-26-17:10: +The default workflow is plan-in-place: a `todo` card releases only after Plan Review passed, so these +scheduler fixtures model a card that already cleared the gate (the state every real card is in when +the capacity sweep sees it). Holding an unreviewed card is the gate working — that path is owned by +`pre-release-plan-review.test.ts`. +*/ +const PASSED_PLAN_REVIEW = { + workflowStepId: "plan-review", + workflowStepName: "Plan Review", + status: "passed" as const, + source: "node" as const, + phase: "pre-merge" as const, +}; + function makeTask(rootDir: string, overrides: Partial = {}): Task { return { id: "FN-5941", @@ -36,6 +51,7 @@ function makeTask(rootDir: string, overrides: Partial = {}): Task { steps: [{ id: "s1", title: "step", status: "in-progress" } as any], currentStep: 1, log: [], + workflowStepResults: [PASSED_PLAN_REVIEW], createdAt: new Date("2026-01-01T00:00:00.000Z").toISOString(), updatedAt: new Date("2026-01-01T00:00:00.000Z").toISOString(), columnMovedAt: new Date("2026-01-01T00:00:00.000Z").toISOString(), diff --git a/packages/engine/src/__tests__/scheduler-ephemeral-toggle.test.ts b/packages/engine/src/__tests__/scheduler-ephemeral-toggle.test.ts index f7c92fadf9..2153007b3a 100644 --- a/packages/engine/src/__tests__/scheduler-ephemeral-toggle.test.ts +++ b/packages/engine/src/__tests__/scheduler-ephemeral-toggle.test.ts @@ -14,6 +14,21 @@ vi.mock("node:fs/promises", async (importOriginal) => { return { ...actual, readFile: vi.fn() }; }); +/* +FNXC:PlanReviewStep 2026-07-26-17:10: +The default workflow is plan-in-place: a `todo` card releases only after Plan Review passed, so these +scheduler fixtures model a card that already cleared the gate (the state every real card is in when +the capacity sweep sees it). Holding an unreviewed card is the gate working — that path is owned by +`pre-release-plan-review.test.ts`. +*/ +const PASSED_PLAN_REVIEW = { + workflowStepId: "plan-review", + workflowStepName: "Plan Review", + status: "passed" as const, + source: "node" as const, + phase: "pre-merge" as const, +}; + function makeTask(overrides: Partial = {}): Task { return { id: "FN-100", @@ -23,6 +38,7 @@ function makeTask(overrides: Partial = {}): Task { steps: [], currentStep: 0, log: [], + workflowStepResults: [PASSED_PLAN_REVIEW], createdAt: "2026-01-01T00:00:00.000Z", updatedAt: "2026-01-01T00:00:00.000Z", ...overrides, diff --git a/packages/engine/src/__tests__/scheduler-node-unreachable-audit.test.ts b/packages/engine/src/__tests__/scheduler-node-unreachable-audit.test.ts index 32416f4317..a7665ed8cc 100644 --- a/packages/engine/src/__tests__/scheduler-node-unreachable-audit.test.ts +++ b/packages/engine/src/__tests__/scheduler-node-unreachable-audit.test.ts @@ -14,11 +14,27 @@ vi.mock("node:fs/promises", async (importOriginal) => { return { ...actual, readFile: vi.fn() }; }); +/* +FNXC:PlanReviewStep 2026-07-26-17:10: +The default workflow is plan-in-place: a `todo` card releases only after Plan Review passed, so these +scheduler fixtures model a card that already cleared the gate (the state every real card is in when +the capacity sweep sees it). Holding an unreviewed card is the gate working — that path is owned by +`pre-release-plan-review.test.ts`. +*/ +const PASSED_PLAN_REVIEW = { + workflowStepId: "plan-review", + workflowStepName: "Plan Review", + status: "passed" as const, + source: "node" as const, + phase: "pre-merge" as const, +}; + function createTask(overrides: Partial = {}): Task { return { id: "FN-1", description: "x", column: "todo", + workflowStepResults: [PASSED_PLAN_REVIEW], dependencies: [], steps: [], currentStep: 0, diff --git a/packages/engine/src/__tests__/scheduler-overlap-starvation.test.ts b/packages/engine/src/__tests__/scheduler-overlap-starvation.test.ts index 6b0ef455a8..4cbea214db 100644 --- a/packages/engine/src/__tests__/scheduler-overlap-starvation.test.ts +++ b/packages/engine/src/__tests__/scheduler-overlap-starvation.test.ts @@ -2,6 +2,21 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { filterPathsByIgnoreList, Scheduler } from "../scheduler.js"; import type { Agent, AgentStore, Settings, Task, TaskStore } from "@fusion/core"; +/* +FNXC:PlanReviewStep 2026-07-26-17:10: +The default workflow is plan-in-place: a `todo` card releases only after Plan Review passed, so these +scheduler fixtures model a card that already cleared the gate (the state every real card is in when +the capacity sweep sees it). Holding an unreviewed card is the gate working — that path is owned by +`pre-release-plan-review.test.ts`. +*/ +const PASSED_PLAN_REVIEW = { + workflowStepId: "plan-review", + workflowStepName: "Plan Review", + status: "passed" as const, + source: "node" as const, + phase: "pre-merge" as const, +}; + function makeTask(overrides: Partial = {}): Task { return { id: "FN-001", @@ -12,6 +27,7 @@ function makeTask(overrides: Partial = {}): Task { steps: [], currentStep: 0, log: [], + workflowStepResults: [PASSED_PLAN_REVIEW], createdAt: "2026-01-01T00:00:00.000Z", updatedAt: "2026-01-01T00:00:00.000Z", ...overrides, diff --git a/packages/engine/src/__tests__/scheduler-workflow-cutover.test.ts b/packages/engine/src/__tests__/scheduler-workflow-cutover.test.ts index d14229b716..e46144def3 100644 --- a/packages/engine/src/__tests__/scheduler-workflow-cutover.test.ts +++ b/packages/engine/src/__tests__/scheduler-workflow-cutover.test.ts @@ -15,6 +15,21 @@ vi.mock("node:fs/promises", async (importOriginal) => { return { ...actual, readFile: vi.fn() }; }); +/* +FNXC:PlanReviewStep 2026-07-26-17:10: +The default workflow is plan-in-place: a `todo` card releases only after Plan Review passed, so these +scheduler fixtures model a card that already cleared the gate (the state every real card is in when +the capacity sweep sees it). Holding an unreviewed card is the gate working — that path is owned by +`pre-release-plan-review.test.ts`. +*/ +const PASSED_PLAN_REVIEW = { + workflowStepId: "plan-review", + workflowStepName: "Plan Review", + status: "passed" as const, + source: "node" as const, + phase: "pre-merge" as const, +}; + function task(overrides: Partial = {}): Task { return { id: "FN-100", @@ -25,6 +40,7 @@ function task(overrides: Partial = {}): Task { steps: [], currentStep: 0, log: [], + workflowStepResults: [PASSED_PLAN_REVIEW], createdAt: "2026-06-23T00:00:00.000Z", updatedAt: "2026-06-23T00:00:00.000Z", ...overrides, diff --git a/packages/engine/src/__tests__/task-pipeline-smoke.test.ts b/packages/engine/src/__tests__/task-pipeline-smoke.test.ts index d9cdd4f3e9..fc4fa28c20 100644 --- a/packages/engine/src/__tests__/task-pipeline-smoke.test.ts +++ b/packages/engine/src/__tests__/task-pipeline-smoke.test.ts @@ -121,8 +121,13 @@ describe("task pipeline smoke", () => { expect(selectionReads).toBe(1); expect(result.context[WORKFLOW_RUN_ID_CONTEXT_KEY]).toBe("FN-7228-SMOKE:builtin:coding"); expect(result.context[WORKFLOW_ID_CONTEXT_KEY]).toBe("builtin-stepwise-final-review-coding"); + /* + FNXC:WorkflowGraphEntry 2026-07-26-17:10: + No `start`: this card is in `todo`, and a run with no continuation now resumes at the card's own + column instead of replaying the pipeline from the first column. `start` lives in `triage`, a + column this card has already left, so the trace begins at the first planning-lane node. + */ expect(result.visitedNodeIds).toEqual([ - "start", "plan", "plan-review", "plan-review::plan-review-step", diff --git a/packages/engine/src/__tests__/workflow-graph-entry-contract.test.ts b/packages/engine/src/__tests__/workflow-graph-entry-contract.test.ts new file mode 100644 index 0000000000..5dd9d56de3 --- /dev/null +++ b/packages/engine/src/__tests__/workflow-graph-entry-contract.test.ts @@ -0,0 +1,172 @@ +import { describe, expect, it } from "vitest"; +import { + BUILTIN_CODING_WORKFLOW_IR, + BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR, + getBuiltinWorkflow, + parseWorkflowIr, + type WorkflowIr, +} from "@fusion/core"; +import { resolveColumnResumeNode, WorkflowGraphExecutor } from "../workflow-graph-executor.js"; +import type { WorkflowRuntimePrimitives } from "../runtime-primitives.js"; +import type { TaskDetail, TaskStep } from "@fusion/core"; + +/* +FNXC:WorkflowGraphEntry 2026-07-26-17:10: +THE GRAPH ENTRY CONTRACT. A run with no durable continuation resumes at the card's OWN column instead +of replaying the pipeline from `start`. Before this, every continuation-less run (self-healing graph +re-entry, a fresh dispatch, an operator drag into a processing column) re-entered at the first node of +the FIRST column and dragged the card backward through columns it had already left — aborting its live +session via `abort-on-exit`, and stranding it in any pre-wip column with no releaser. That backward +drag is the reason planning nodes had to live in the implementation column. + +These assert the INVARIANT across every lifecycle position a card can hold, not just the plan-in-place +case that motivated it: behind (resume forward), at, and past each column, on the real built-in IRs. +*/ + +const codingIr = parseWorkflowIr(getBuiltinWorkflow("builtin:coding")!.ir as never); + +describe("workflow graph entry contract — resume at the card's own column", () => { + it("enters the planning prologue only for a card still in the planning lane", () => { + // Intake: nothing is behind it, so the run starts at the graph's own start node. + expect(resolveColumnResumeNode(codingIr, "triage")?.id).toBe("start"); + // Planning lane: the specification phase is exactly what this card still needs. + expect(resolveColumnResumeNode(codingIr, "todo")?.id).toBe("plan"); + }); + + it("never re-plans a card that already reached implementation", () => { + const resumed = resolveColumnResumeNode(codingIr, "in-progress"); + expect(resumed?.id).toBe("parse"); + expect(resumed?.column).toBe("in-progress"); + // The regression this exists to prevent: resuming at a planning node would move the card + // backward out of the wip column and abort its session. + expect(["plan", "plan-review", "plan-replan"]).not.toContain(resumed?.id); + }); + + it("re-enters a review-column card at the FIRST review node so no gate is skipped", () => { + const resumed = resolveColumnResumeNode(codingIr, "in-review"); + expect(resumed?.column).toBe("in-review"); + // Entering at the merge region instead would silently skip Code Review. + expect(resumed?.id).toBe("browser-verification"); + }); + + it("resolves the same way for the other built-in coding IRs", () => { + expect(resolveColumnResumeNode(BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR, "in-progress")?.id).toBe("parse"); + // The base IR names its planning seam `planning`; the contract is about columns, not ids. + expect(resolveColumnResumeNode(BUILTIN_CODING_WORKFLOW_IR, "todo")?.id).toBe("planning"); + expect(resolveColumnResumeNode(BUILTIN_CODING_WORKFLOW_IR, "in-progress")?.id).toBe("execute"); + }); + + it("skips forward when the card rests in a column the pipeline has no node for", () => { + const ir = parseWorkflowIr({ + version: "v2", + name: "gap-column", + columns: [ + { id: "intake", name: "Intake", traits: [{ trait: "intake" }] }, + // No node declares `staging` — a card parked here must resume at the next node forward. + { id: "staging", name: "Staging", traits: [{ trait: "hold", config: { release: "capacity" } }] }, + { id: "work", name: "Work", traits: [{ trait: "wip" }] }, + { id: "done", name: "Done", traits: [{ trait: "complete" }] }, + ], + nodes: [ + { id: "start", kind: "start", column: "intake" }, + { id: "build", kind: "prompt", column: "work" }, + { id: "end", kind: "end", column: "done" }, + ], + edges: [ + { from: "start", to: "build", condition: "success" }, + { from: "build", to: "end", condition: "success" }, + ], + } as WorkflowIr); + expect(resolveColumnResumeNode(ir, "staging")?.id).toBe("build"); + }); + + it("never resumes at a remediation node reached only by a failure or rework edge", () => { + // `plan-replan` sits in the planning lane and is reachable only from a plan-review FAILURE, + // so a planning-lane card must still resume at `plan` — remediation is not an entry point. + expect(resolveColumnResumeNode(codingIr, "todo")?.id).not.toBe("plan-replan"); + // Same shape in the implementation column, where the code-review remediation node lives. + expect(resolveColumnResumeNode(codingIr, "in-progress")?.id).not.toBe("code-review-remediation"); + }); + + it("falls back to the start node for an unknown column or a v1 IR", () => { + expect(resolveColumnResumeNode(codingIr, "not-a-column")).toBeUndefined(); + expect(resolveColumnResumeNode(codingIr, undefined)).toBeUndefined(); + expect(resolveColumnResumeNode({ version: "v1", name: "legacy", nodes: [], edges: [] } as never, "todo")) + .toBeUndefined(); + }); +}); + +/* +FNXC:WorkflowGraphEntry 2026-07-27-06:10 (PR #2462 review): +The resolver tests above prove the DECISION; this one proves the executor actually asks. A run that +reached `run()` and ignored the resolver — or that reintroduced the backward `columnBoundary` move — +would satisfy every assertion above and still strand the card, which is exactly the regression the +entry contract exists to prevent. Assert on the real traversal, not on the helper. +*/ +describe("workflow graph entry contract — the executor honors it", () => { + const promptWithOneStep = "# Task\n\n## Steps\n\n### Step 0: Implement\n- [ ] do it\n"; + + function silentPrimitives(calls: string[]): WorkflowRuntimePrimitives { + const ok = { outcome: "success" as const }; + return { + prepareWorktree: async () => ({ outcome: "success", data: { worktreePath: "/memory/worktree" } }), + readArtifact: async (_c, _t, key) => (key === "PROMPT.md" ? promptWithOneStep : undefined), + writeArtifact: async (_c, _t, key) => ({ outcome: "success", data: { key } }), + runPlanningSession: async () => { + calls.push("planning-session"); + return { outcome: "success", data: { approved: true, artifactKeys: ["PROMPT.md"] } }; + }, + runCodingSession: async () => ({ outcome: "success", data: { taskDone: true, modifiedFiles: [] } }), + runTaskStep: async () => ({ outcome: "success", baselineSha: "b", checkpointId: "c" }), + resetTaskStep: async () => ({ ok: true }), + runReview: async () => ({ outcome: "success", data: { verdict: "APPROVE" } }), + runVerification: async () => ({ outcome: "success", data: { verdict: "skipped" } }), + updateSteps: async (_c, target: TaskDetail, steps: TaskStep[]) => { + target.steps = steps; + return { outcome: "success", data: { count: steps.length } }; + }, + transitionTask: async () => ok, + requestMerge: async () => ({ outcome: "success", value: "merged", data: { status: "merged" } }), + abortRun: async () => ok, + audit: () => undefined, + } as unknown as WorkflowRuntimePrimitives; + } + + it("resumes an in-progress card at `parse` and never re-enters a planning node", async () => { + const calls: string[] = []; + const task = { + id: "FN-ENTRY", + title: "Entry contract", + description: "", + column: "in-progress", + dependencies: [], + steps: [], + currentStep: 0, + log: [], + prompt: promptWithOneStep, + workflowStepResults: [], + createdAt: "2026-07-27T00:00:00.000Z", + updatedAt: "2026-07-27T00:00:00.000Z", + } as unknown as TaskDetail; + + const executor = new WorkflowGraphExecutor({ + primitives: silentPrimitives(calls), + parseStepsDeps: { + readArtifact: async (_target, key) => (key === "PROMPT.md" ? promptWithOneStep : undefined), + writeSteps: async (target: TaskDetail, steps: TaskStep[]) => { + target.steps = steps; + }, + }, + } as never); + + // No continuation node id — the "replay from start" path the contract governs. + const result = await executor.run(task, { experimentalFeatures: {} } as never, codingIr); + + expect(result.visitedNodeIds[0]).toBe("parse"); + for (const planningNode of ["start", "plan", "plan-review", "plan-replan"]) { + expect(result.visitedNodeIds, `must not re-enter ${planningNode}`).not.toContain(planningNode); + } + // The planning primitive is the loudest possible proof: a re-planned card would call it. + expect(calls).not.toContain("planning-session"); + }); +}); diff --git a/packages/engine/src/hold-release.ts b/packages/engine/src/hold-release.ts index 1cc437ff31..354dd925a6 100644 --- a/packages/engine/src/hold-release.ts +++ b/packages/engine/src/hold-release.ts @@ -47,6 +47,7 @@ import { TransitionRejectionError, resolveWorkflowIrForTask, isUnplannedSeedPrompt, + isWorkflowOptionalGroupEnabled, resolveEffectiveAutoMerge, type TaskStore, type Task, @@ -178,8 +179,33 @@ export async function isUnplannedForExecution(store: TaskStore, task: Task, ir: capacity boundary. Releasing first would skip the gate. This does not fire when Plan Review already lives in a WIP column. */ + /* + FNXC:PlanReview 2026-07-26-14:05: + The gate is PLAN-IN-PLACE only: it applies when Plan Review runs in the very column the card is + held in (Coding (Ideas) / the benchmark's Plan Review in Todo), which is the same condition the + other two consumers of this resolver already require (`seedPreReleasePlanReviewContinuation`, + `evaluateStrandedHoldContinuation`). Without the column check, moving the default workflow's Plan + Review out of the wip column into the planning column turned "non-wip" into "pre-release" for every + card in Todo — including cards whose graph never routes through Todo at all — and the capacity + sweep stopped releasing them (no continuation for a boundary they never reach). A review node in an + upstream column the card has already left is not something this sweep gates on. + */ + /* + FNXC:PlanReview 2026-07-26-17:10: + The gate also requires Plan Review to be ENABLED for this task. It exists to stop a card entering + implementation before its plan gate ran; a task whose plan-review group is toggled OFF has no such + gate, and holding it produced a deadlock — nothing would ever record the evidence the hold was + waiting for. + */ const preReleaseReview = resolvePreReleasePlanReviewNode(ir); - if (preReleaseReview) { + const preReleaseReviewEnabled = preReleaseReview + ? isWorkflowOptionalGroupEnabled( + task.enabledWorkflowSteps, + preReleaseReview.id, + (preReleaseReview.config as { defaultOn?: boolean } | undefined)?.defaultOn ?? false, + ) + : false; + if (preReleaseReview && preReleaseReviewEnabled && preReleaseReview.column === task.column) { // Compatibility for tasks planned before durable continuations existed and // for narrow store adapters that expose only the legacy review result. const legacyPassed = task.workflowStepResults?.some( diff --git a/packages/engine/src/run-audit.ts b/packages/engine/src/run-audit.ts index de8437985f..ad1ac304c9 100644 --- a/packages/engine/src/run-audit.ts +++ b/packages/engine/src/run-audit.ts @@ -612,6 +612,8 @@ export type DatabaseMutationType = reason }, where `reason` is a fixed adoption-table note — never row prose. */ | "task:reconcile-legacy-adoption" + // FNXC:WorkflowColumns 2026-07-26-18:30: a row re-homed out of a column its workflow no longer declares. + | "task:reconcile-undeclared-column" /** * An UNMAPPABLE legacy status: the row is parked `paused` for a human with its status * deliberately left in place so the operator can see what it carried. Same metadata shape. diff --git a/packages/engine/src/self-healing.ts b/packages/engine/src/self-healing.ts index 1ba21ec7d3..b683c58d7d 100644 --- a/packages/engine/src/self-healing.ts +++ b/packages/engine/src/self-healing.ts @@ -31,7 +31,7 @@ import { existsSync, mkdirSync, readdirSync, readFileSync, realpathSync, rmSync, import { readFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { isAbsolute, join, relative, resolve } from "node:path"; -import { resolveColumnFlags, IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, IN_REVIEW_STALL_TERMINAL_LOG_PREFIX, allowsAutoMergeProcessing, resolveEffectiveAutoMerge, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, evaluateNoCommitsNoOpFinalize, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isWorkflowColumnsEnabled, isWorkspaceTask, isSharedBranchGroupMemberIntegration, isNearDuplicateCanonicalInactive, parseExplicitDuplicateMarker, flagTriageDuplicate, isTriageDuplicateKeepAcknowledged, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, resolveWorkflowIrForTask, resolveReboundTarget, planLegacyAdoption, resolveOrphanedPendingStepResults, classifyReviewLease, PLAN_REVIEW_LEASE_STALENESS_MS, DEFAULT_MAX_POST_REVIEW_FIXES, ACTIVE_WORKFLOW_WORK_ITEM_STATES, AWAITING_APPROVAL_PAUSE_REASON, type Agent, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult, type WorkflowStepResult } from "@fusion/core"; +import { resolveColumnFlags, IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, IN_REVIEW_STALL_TERMINAL_LOG_PREFIX, allowsAutoMergeProcessing, resolveEffectiveAutoMerge, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, evaluateNoCommitsNoOpFinalize, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isWorkflowColumnsEnabled, isWorkspaceTask, isSharedBranchGroupMemberIntegration, isNearDuplicateCanonicalInactive, parseExplicitDuplicateMarker, flagTriageDuplicate, isTriageDuplicateKeepAcknowledged, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, resolveWorkflowIrForTask, resolveReboundTarget, workflowHasColumn, planLegacyAdoption, resolveOrphanedPendingStepResults, classifyReviewLease, PLAN_REVIEW_LEASE_STALENESS_MS, DEFAULT_MAX_POST_REVIEW_FIXES, ACTIVE_WORKFLOW_WORK_ITEM_STATES, AWAITING_APPROVAL_PAUSE_REASON, type Agent, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult, type WorkflowStepResult } from "@fusion/core"; import { finalizePlanningSegment } from "@fusion/core"; import type { MeshLeaseManager } from "./mesh-lease-manager.js"; import { createLogger, schedulerLog } from "./logger.js"; @@ -1523,6 +1523,10 @@ export class SelfHealingManager { // legacy `planning`/`needs-replan` row is judged by recovery rules that no longer // have a writer for that status. { name: "adopt-legacy-task-rows", fn: () => this.adoptLegacyTaskRows().then(() => undefined) }, + // FNXC:WorkflowColumns 2026-07-26-18:30: immediately after status adoption and before every + // column-reasoning step below — a row in an undeclared column carries NO trait flags, so each + // of those steps would silently classify it as "not my case" and leave it stranded. + { name: "reconcile-undeclared-task-columns", fn: () => this.reconcileUndeclaredTaskColumns().then(() => undefined) }, // FNXC:OrphanedPendingSteps 2026-07-22-16:20 (FN-8492 incident): runs right after // adoption and BEFORE every in-review recovery step below — those reason about // step-result completeness, and an orphaned `pending` result reads as "work in @@ -6705,6 +6709,86 @@ export class SelfHealingManager { } } + /** + * FNXC:WorkflowColumns 2026-07-26-18:30: + * Re-home a card whose column its workflow no longer declares. + * + * A workflow's column set is editable, and the built-in coding workflows just merged Todo into + * Planning — so a live board can hold rows pointing at a column that is gone. Such a row is + * invisible to every trait-driven sweep (`findColumn` returns undefined, so it carries no hold, wip, + * or intake flags), which means nothing schedules it, nothing releases it, and it renders in a + * column the board no longer draws. It is stranded in the most literal sense. + * + * The repair is the same one KTD-10 already defines for a recovered card: `resolveReboundTarget` + * (the workflow's hold column, else its intake column, else its first). Deliberately conservative — + * it only ever touches a row whose column is UNDECLARED, never one the operator merely disagrees + * with, and it leaves operator parks (`userPaused`) alone. + */ + async reconcileUndeclaredTaskColumns(): Promise { + try { + const pageSize = 500; + let offset = 0; + let rehomed = 0; + + for (;;) { + const tasks = await this.store.listTasks({ slim: true, includeArchived: false, limit: pageSize, offset }); + for (const task of tasks) { + if (task.userPaused === true) continue; + let ir; + try { + ir = await resolveWorkflowIrForTask(this.store, task.id); + } catch { + continue; // An unresolvable workflow is its own fault path; do not guess a column. + } + if (!ir || workflowHasColumn(ir, task.column)) continue; + const target = resolveReboundTarget(ir); + if (!target || target === task.column) continue; + + try { + await this.store.moveTask(task.id, target as Task["column"], { + moveSource: "engine", + /* + FNXC:WorkflowColumns 2026-07-27-06:10 (PR #2462 review): + `recoveryRehome`, not just `bypassGuards`. Adjacency is checked SEPARATELY from the + guards (moves.ts: only `recoveryRehome` skips `resolveAllowedColumns`), and this card's + SOURCE column is undeclared too — so `resolveAllowedColumns(ir, fromColumn)` returns [] + and every target is rejected. Without this flag the sweep threw on every candidate and + left the card exactly where it was stranded: a repair that never repaired anything. + */ + recoveryRehome: true, + bypassGuards: true, + preserveProgress: true, + }); + rehomed += 1; + await createRunAuditor(this.store, { + runId: generateSyntheticRunId("reconcile-undeclared-column", task.id), + agentId: "self-healing", + taskId: task.id, + taskLineageId: task.lineageId, + phase: "reconcile-undeclared-column", + }).database({ + type: "task:reconcile-undeclared-column", + target: task.id, + // ids/outcomes only. + metadata: { taskId: task.id, priorColumn: task.column, toColumn: target }, + }).catch(() => undefined); + } catch (error) { + log.warn( + `reconcileUndeclaredTaskColumns: failed for ${task.id}: ${error instanceof Error ? error.message : String(error)}`, + ); + } + } + if (tasks.length < pageSize) break; + offset += tasks.length; + } + if (rehomed > 0) log.log(`Re-homed ${rehomed} task(s) out of a column their workflow no longer declares`); + return rehomed; + } catch (error) { + log.error(`reconcileUndeclaredTaskColumns failed: ${error instanceof Error ? error.message : String(error)}`); + return 0; + } + } + /** * FNXC:StrandedHoldContinuation 2026-07-26-12:00: * A real prompt can survive a hard planning cancel after its continuation is diff --git a/packages/engine/src/workflow-graph-executor.ts b/packages/engine/src/workflow-graph-executor.ts index d8c5dd6c81..bb054d88f9 100644 --- a/packages/engine/src/workflow-graph-executor.ts +++ b/packages/engine/src/workflow-graph-executor.ts @@ -390,6 +390,58 @@ function hasTaskProjection(patch: WorkflowTaskProjection): boolean { return Object.keys(patch).length > 0; } +/* +FNXC:WorkflowGraphEntry 2026-07-26-17:10: +THE GRAPH ENTRY CONTRACT: a run with no durable continuation resumes at the card's OWN COLUMN, not +at `start`. + +Post-cutover a node's column IS the card's lifecycle position, so replaying from `start` contradicted +that invariant: any run without a continuation (self-healing's graph re-entry, a fresh dispatch, an +operator drag into a processing column) re-entered at the first node of the FIRST column and dragged +the card backward through columns it had already left — firing `abort-on-exit` on its live session, +and stranding it in any pre-wip column that has no releaser. That backward drag is what previously +forced planning nodes to live in the implementation column. + +The rule: enter at the first node, in pipeline order from `start`, whose column is not BEHIND the +card's current column (`ir.columns` is ordered, and that order is the lifecycle order). A card in the +planning lane still runs the planning prologue; a card already in implementation resumes at the first +implementation node instead of re-planning; a card in review re-enters at the first review node, so +review gates are never skipped. + +Traversal follows the forward pipeline only — `rework` back-edges and failure edges are excluded — so +the entry point is the main path, never a remediation node that merely happens to sit in the column. +*/ +export function resolveColumnResumeNode(ir: WorkflowIr, column: string | undefined): WorkflowIrNode | undefined { + if (!column || ir.version !== "v2") return undefined; + const columnOrder = new Map(ir.columns.map((entry, index) => [entry.id, index])); + const taskColumnIndex = columnOrder.get(column); + if (taskColumnIndex === undefined) return undefined; + + const startNode = ir.nodes.find((node) => node.kind === "start"); + if (!startNode) return undefined; + const nodeById = new Map(ir.nodes.map((node) => [node.id, node])); + + const queue: string[] = [startNode.id]; + const seen = new Set([startNode.id]); + while (queue.length > 0) { + const id = queue.shift()!; + const node = nodeById.get(id); + if (!node) continue; + const nodeColumnIndex = node.column ? columnOrder.get(node.column) : undefined; + // `>=` and not `===`: a card can rest in a column the pipeline has no node for + // (a pure hold column in some workflows), and must then resume at the next node forward. + if (nodeColumnIndex !== undefined && nodeColumnIndex >= taskColumnIndex) return node; + for (const edge of ir.edges) { + if (edge.from !== id || edge.kind === "rework") continue; + if (edge.condition && edge.condition !== "success") continue; + if (seen.has(edge.to)) continue; + seen.add(edge.to); + queue.push(edge.to); + } + } + return undefined; +} + export class WorkflowGraphExecutor { private readonly maxRetriesPerNode: number; @@ -418,7 +470,7 @@ export class WorkflowGraphExecutor { ): Promise { const startNode = startNodeId ? ir.nodes.find((node) => node.id === startNodeId) - : ir.nodes.find((node) => node.kind === "start"); + : resolveColumnResumeNode(ir, task.column) ?? ir.nodes.find((node) => node.kind === "start"); if (!startNode) throw new WorkflowIrError("Workflow IR missing start node"); const nodeMap = new Map(ir.nodes.map((node) => [node.id, node]));