fix: resume the graph at a top-level node, not a foreach template 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 and is never in ir.nodes, so handing it to the
interpreter as a start node resolved to nothing and threw WorkflowIrError.
executeWorkflowGraph's catch turned that into a terminal graph failure, so a
healthy card was parked on every dispatch:
[workflow-graph] FN-8825 could not resolve workflow — parking task instead of
legacy fallback: interpreter-error: Workflow IR missing start node
Latent since FN-8764 introduced these fences, and reachable only once a
step-execute fence could become the task's sole active continuation — which the
atomic-handover change in dd40691ca2 made routine.
The executor now passes a continuation node id as the resume point only when the
task's resolved IR actually contains it. Otherwise it falls back to the graph
entry contract: with no explicit start node the run re-enters at the card's own
column, so an in-progress card re-enters at parse, finds the foreach already
expanded, and hands control back to steps. The instance resumes from its own row
in workflow_run_step_instances, so nothing is replayed. Already-persisted
template-node continuations therefore heal on their next dispatch with no
migration.
Also splits the error message. One string covered a genuinely malformed IR and a
caller asking to resume at an unknown node, and reporting the second as "missing
start node" sends the reader to inspect a workflow definition that is fine.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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");
|
||||
});
|
||||
});
|
||||
@@ -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", {
|
||||
|
||||
@@ -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<string, WorkflowIrEdge[]>();
|
||||
|
||||
Reference in New Issue
Block a user