diff --git a/packages/engine/src/__tests__/workflow-graph-resume-node.test.ts b/packages/engine/src/__tests__/workflow-graph-resume-node.test.ts new file mode 100644 index 0000000000..5fbb40f10a --- /dev/null +++ b/packages/engine/src/__tests__/workflow-graph-resume-node.test.ts @@ -0,0 +1,83 @@ +/* +FNXC:WorkflowExecution 2026-08-08-01:40: +A durable continuation may name a node that is NOT a resumable graph node. + +A principal fence written for a node inside a foreach template stores the TEMPLATE node id +(`step-execute`), with the materialized instance in `nodeInstanceId` (`steps#0:step-execute`). +The template node lives under the foreach's `config.template`, never in `ir.nodes` — so handing +it to the interpreter as a start node resolves to nothing. Before this was pinned, that threw +`WorkflowIrError("Workflow IR missing start node")`, the executor's catch turned it into a +terminal graph failure, and a healthy card was parked on every dispatch while the message sent +readers to inspect a workflow definition that was perfectly fine. + +Two contracts are pinned here, using the REAL builtin coding IR rather than a fixture so the +nesting is the production one: + 1. `step-execute` is genuinely a template-only node — the premise the executor's guard rests + on. If the builtin graph ever hoists it to the top level, the guard is dead code and this + says so. + 2. Resuming at an unknown node id fails with a message that names the resume id, and is + distinguishable from a genuinely start-less IR. +*/ + +import { describe, expect, it } from "vitest"; +import { resolveDefaultWorkflowIr, WorkflowIrError } from "@fusion/core"; +import type { TaskDetail, WorkflowIr, WorkflowIrNode } from "@fusion/core"; + +import { WorkflowGraphExecutor } from "../workflows/workflow-graph-executor.js"; + +/* +The IR `builtin:coding` actually resolves to. Deliberately the production resolver rather than a +raw constant: `builtin:coding` maps to the stepwise-final-review graph, NOT the legacy +`BUILTIN_CODING_WORKFLOW_IR` (which has no foreach at all), and two move-path resolvers have +already drifted apart on exactly that point. +*/ +const DEFAULT_IR = resolveDefaultWorkflowIr(); + +const task = { id: "FN-RESUME", column: "in-progress" } as TaskDetail; + +function settingsOn() { + return { experimentalFeatures: { workflowGraphExecutor: true } } as never; +} + +/** Template nodes of the `steps` foreach in the real builtin coding workflow. */ +function foreachTemplateNodeIds(ir: WorkflowIr): string[] { + const foreach = ir.nodes.find((node) => node.kind === "foreach"); + const template = (foreach?.config as { template?: { nodes?: WorkflowIrNode[] } } | undefined)?.template; + return (template?.nodes ?? []).map((node) => node.id); +} + +describe("graph resume node resolution", () => { + it("step-execute is a foreach TEMPLATE node, not a top-level graph node", () => { + const topLevel = DEFAULT_IR.nodes.map((node) => node.id); + expect(foreachTemplateNodeIds(DEFAULT_IR)).toContain("step-execute"); + // The premise of the executor's resume guard: a fence for this node names something the + // interpreter cannot start at. + expect(topLevel).not.toContain("step-execute"); + expect(topLevel).toContain("steps"); + expect(topLevel).toContain("parse"); + }); + + it("resuming at a template node id reports the resume id, not a missing start node", async () => { + const executor = new WorkflowGraphExecutor({}); + const error = await executor + .run(task, settingsOn(), DEFAULT_IR, "step-execute") + .then(() => null, (e: unknown) => e as Error); + + expect(error).toBeInstanceOf(WorkflowIrError); + expect(error?.message).toContain("step-execute"); + // The old message pointed at the wrong thing and cost real debugging time. + expect(error?.message).not.toBe("Workflow IR missing start node"); + }); + + it("still reports a genuinely start-less IR as a missing start node", async () => { + const executor = new WorkflowGraphExecutor({}); + const startless: WorkflowIr = { version: "v1", name: "startless", nodes: [], edges: [] } as WorkflowIr; + + const error = await executor + .run(task, settingsOn(), startless) + .then(() => null, (e: unknown) => e as Error); + + expect(error).toBeInstanceOf(WorkflowIrError); + expect(error?.message).toBe("Workflow IR missing start node"); + }); +}); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 3884a4ee00..9577008336 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -7291,7 +7291,38 @@ export class TaskExecutor { "workflow:node-instance-id": continuation.nodeInstanceId ?? continuation.nodeId, } : undefined; - result = await runner.run(detail, settings, continuation?.nodeId, continuationContext); + /* + * FNXC:WorkflowExecution 2026-08-08-01:40: + * Only a TOP-LEVEL node id is a legal resume point. + * + * A fence written for a node inside a foreach template stores the TEMPLATE node id + * (`step-execute`) with the materialized instance in `nodeInstanceId` + * (`steps#0:step-execute`). The template node is not in `ir.nodes` — it exists only + * under the `steps` foreach's `config.template` — so handing it to the interpreter as + * a start node resolves to nothing and throws `WorkflowIrError`, which the catch below + * converts into a terminal graph failure. That parks a healthy card on every dispatch. + * + * Fall back to the graph ENTRY CONTRACT instead: with no explicit start node the run + * re-enters at the card's own column (`resolveColumnResumeNode`), so an in-progress + * card re-enters at `parse`, which sees the foreach already expanded and hands control + * back to `steps`. The instance itself resumes from its own durable row in + * `workflow_run_step_instances`, so nothing is replayed and no progress is lost — this + * is the same path a run with no continuation at all already takes. + * + * Self-healing by construction: an already-persisted template-node continuation (there + * are such rows in the field) resumes correctly on its next dispatch without migration. + */ + const resumeNodeId = continuation?.nodeId + && columnAgentIr?.nodes.some((candidate) => candidate.id === continuation?.nodeId) + ? continuation.nodeId + : undefined; + if (continuation?.nodeId && resumeNodeId === undefined) { + executorLog.debug( + `[workflow-graph] ${task.id}: continuation node '${continuation.nodeId}' is not a top-level graph node ` + + `(instance '${continuation.nodeInstanceId ?? "none"}') — re-entering at the column resume node`, + ); + } + result = await runner.run(detail, settings, resumeNodeId, continuationContext); } catch (err) { if (continuation) { await this.store.transitionWorkflowWorkItem(continuation.id, "failed", { diff --git a/packages/engine/src/workflows/workflow-graph-executor.ts b/packages/engine/src/workflows/workflow-graph-executor.ts index e6d1825a8b..72c89d51ab 100644 --- a/packages/engine/src/workflows/workflow-graph-executor.ts +++ b/packages/engine/src/workflows/workflow-graph-executor.ts @@ -534,7 +534,22 @@ export class WorkflowGraphExecutor { const startNode = startNodeId ? ir.nodes.find((node) => node.id === startNodeId) : resolveColumnResumeNode(ir, task.column) ?? ir.nodes.find((node) => node.kind === "start"); - if (!startNode) throw new WorkflowIrError("Workflow IR missing start node"); + /* + * FNXC:WorkflowExecution 2026-08-08-01:40: + * Name WHICH lookup failed. One message covered two unrelated causes: a genuinely + * malformed IR with no `start` node, and a caller asking to resume at a node id this IR + * does not contain — most often a foreach TEMPLATE node id, which lives under the + * foreach's `config.template` rather than in `ir.nodes`. Reporting the second as "missing + * start node" sends the reader to inspect a workflow definition that is perfectly fine. + */ + if (!startNode) { + throw new WorkflowIrError( + startNodeId + ? `Workflow IR has no top-level node '${startNodeId}' to resume at` + + " (a foreach template node is not a resumable graph node)" + : "Workflow IR missing start node", + ); + } const nodeMap = new Map(ir.nodes.map((node) => [node.id, node])); const outgoingMap = new Map();