diff --git a/apps/backend/src/domain/mapper/from-integration-data.ts b/apps/backend/src/domain/mapper/from-integration-data.ts index a289647e5..99a8a81ca 100644 --- a/apps/backend/src/domain/mapper/from-integration-data.ts +++ b/apps/backend/src/domain/mapper/from-integration-data.ts @@ -2,6 +2,7 @@ import { type BaseNode, NODE_ERROR_POLICIES, type NodeErrorPolicy, + type NodeRole, type WorkflowDefinition, type WorkflowEdgeDefinition, } from '@workflow-builder/types/workflow-execution/execution-model'; @@ -16,6 +17,12 @@ type FrontendEdge = WorkflowSnapshot['edges'][number]; // out of sync with the runner's union. const ERROR_POLICIES: ReadonlySet = new Set(NODE_ERROR_POLICIES); +// The editor's kind for an entrypoint node, mirroring `NodeType.StartNode` in +// `@workflowbuilder/sdk`. Spelled out as a literal rather than imported: the +// backend deliberately does not depend on the React SDK. If the SDK renames the +// kind, this has to move with it. +const START_NODE_KIND = 'start-node'; + // Structural pass-through. The backend treats nodes as opaque `{ id, type, config }`; // the worker narrows `config` against its own concrete node union when it dispatches // the executor. Unknown types reach the worker and fail there as `node_failed`. @@ -25,18 +32,21 @@ export function mapToExecutionModel(workflowId: string, data: WorkflowSnapshot): return { workflowId, nodes, edges }; } -// `errorPolicy` is authored in the UI as a regular JSONForms property -// (via `sharedProperties` in the SDK), so it arrives nested in -// `data.properties`. The runner expects it at the top level of `BaseNode`, -// so we lift it here and keep `config` free of runner-only fields. +// Two runner-level fields are lifted out of the frontend shape here so `config` +// stays free of them. `errorPolicy` is authored in the UI as a regular JSONForms +// property (via `sharedProperties` in the SDK) and arrives nested in +// `data.properties`. `role` is derived from the editor's node kind, which lives +// on the node itself rather than in its properties. function mapNode(node: FrontendNode): BaseNode { const { errorPolicy: rawErrorPolicy, ...config } = node.data.properties ?? {}; const errorPolicy = isErrorPolicy(rawErrorPolicy) ? rawErrorPolicy : undefined; + const role: NodeRole | undefined = node.type === START_NODE_KIND ? 'start' : undefined; return { id: node.id, type: node.data.type, config, ...(errorPolicy === undefined ? {} : { errorPolicy }), + ...(role === undefined ? {} : { role }), }; } diff --git a/apps/backend/src/domain/mapper/snapshot-schema.test.ts b/apps/backend/src/domain/mapper/snapshot-schema.test.ts index 1e890f2ac..52a469890 100644 --- a/apps/backend/src/domain/mapper/snapshot-schema.test.ts +++ b/apps/backend/src/domain/mapper/snapshot-schema.test.ts @@ -16,6 +16,16 @@ describe('workflowSnapshotSchema', () => { expect(workflowSnapshotSchema.safeParse(snapshot).success).toBe(true); }); + it("keeps the editor's node kind so entrypoints stay identifiable", () => { + const snapshot = { + nodes: [{ id: 'n1', type: 'start-node', data: { type: 'my-product/trigger' } }], + edges: [], + }; + + const result = workflowSnapshotSchema.parse(snapshot); + expect(result.nodes[0]!.type).toBe('start-node'); + }); + it('rejects a node missing `id`', () => { const snapshot = { nodes: [{ data: { type: 'x/y' } }], @@ -151,6 +161,23 @@ describe('mapToExecutionModel', () => { ]); }); + it("lifts the editor's start-node kind to role 'start'", () => { + const result = mapToExecutionModel('wf-1', { + nodes: [ + { id: 'n1', type: 'start-node', data: { type: 'my-product/trigger' } }, + { id: 'n2', type: 'node', data: { type: 'my-product/action' } }, + { id: 'n3', data: { type: 'my-product/action' } }, + ], + edges: [], + }); + + // Only the start node carries a role; the runner reads it to pick entrypoints + // instead of inferring them from in-degree. + expect(result.nodes[0]!.role).toBe('start'); + expect(result.nodes[1]!.role).toBeUndefined(); + expect(result.nodes[2]!.role).toBeUndefined(); + }); + it('passes unknown node types through unchanged — backend does not know any vocabulary', () => { // The whole point of the structural mapper: a type the backend has never // heard of reaches the worker, where the registry-miss becomes a diff --git a/apps/backend/src/domain/mapper/snapshot-schema.ts b/apps/backend/src/domain/mapper/snapshot-schema.ts index f3c428c25..fd0e81ec3 100644 --- a/apps/backend/src/domain/mapper/snapshot-schema.ts +++ b/apps/backend/src/domain/mapper/snapshot-schema.ts @@ -8,6 +8,12 @@ import { z } from 'zod'; const frontendNodeSchema = z.object({ id: z.string(), + // The editor's node kind (`start-node`, `node`, `decision-node`, ...), as + // opposed to `data.type`, which is the product's own vocabulary. Kept because + // the runner needs to know which nodes are entrypoints: zod strips whatever it + // is not told about, and dropping this is what let a node with no incoming + // edge act as a second trigger. + type: z.string().optional(), data: z.object({ type: z.string(), properties: z.record(z.string(), z.unknown()).optional(), diff --git a/apps/execution-worker/src/engines/temporal/workflows/sequenced-event-emitter.test.ts b/apps/execution-worker/src/engines/temporal/workflows/sequenced-event-emitter.test.ts index fd68432a9..782175631 100644 --- a/apps/execution-worker/src/engines/temporal/workflows/sequenced-event-emitter.test.ts +++ b/apps/execution-worker/src/engines/temporal/workflows/sequenced-event-emitter.test.ts @@ -55,6 +55,10 @@ function node(id: string): TestNode { return { id, type: 'test/node', config: {} }; } +function startNode(id: string): TestNode { + return { id, type: 'test/node', config: {}, role: 'start' }; +} + type TestEdge = WorkflowExecutionInput['definition']['edges'][number]; function edge(id: string, source: string, target: string): TestEdge { @@ -84,7 +88,7 @@ const runner: ActivityRunnerPort = { // A→{B,C,D}→E: one four-wide wave, so four node_started emits are in flight at once. function fanOutGraph(): WorkflowExecutionInput { return makeInput( - [node('A'), node('B'), node('C'), node('D'), node('E')], + [startNode('A'), node('B'), node('C'), node('D'), node('E')], [ edge('e1', 'A', 'B'), edge('e2', 'A', 'C'), @@ -204,7 +208,7 @@ describe('createSequencedEventEmitter', () => { const outcome = await runGraph( makeInput( - [node('A'), { ...node('B'), errorPolicy: 'continue' }, node('C')], + [startNode('A'), { ...node('B'), errorPolicy: 'continue' }, node('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C')], ), runner, diff --git a/packages/execution-core/README.md b/packages/execution-core/README.md index dc7610b94..ffb343f5b 100644 --- a/packages/execution-core/README.md +++ b/packages/execution-core/README.md @@ -54,6 +54,7 @@ The mapped-type registry refuses to compile if a key drifts away from the union ``` src/ ├── graph-runner.ts # Topological scheduler over nodes/edges — engine-agnostic, generic in TNode +├── resolve-start-node.ts # Entry-shape rule: exactly one `role: 'start'` node, no orphans ├── execution-context.ts # Readonly context passed to every node executor ├── ports/ │ ├── workflow-engine.port.ts # submit(), cancel() — implemented by adapters (TemporalEngine, …) diff --git a/packages/execution-core/src/graph-runner.replay-determinism.test.ts b/packages/execution-core/src/graph-runner.replay-determinism.test.ts index 6f102d88d..10d7acda3 100644 --- a/packages/execution-core/src/graph-runner.replay-determinism.test.ts +++ b/packages/execution-core/src/graph-runner.replay-determinism.test.ts @@ -80,6 +80,10 @@ function trigger(id: string): TestNode { return { id, type: 'test/node', config: {} }; } +function start(id: string): TestNode { + return { id, type: 'test/node', config: {}, role: 'start' }; +} + function edge(id: string, source: string, target: string, sourceHandle?: string): WorkflowEdgeDefinition { return { id, sourceNodeId: source, targetNodeId: target, sourceHandle }; } @@ -129,7 +133,7 @@ const RUNS = 10; describe('runGraph — replay determinism (re-execution equivalence)', () => { it('linear A→B→C — every run produces the same activity order, events, statuses', async () => { - const input = makeInput([trigger('A'), trigger('B'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C')]); + const input = makeInput([start('A'), trigger('B'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C')]); const records = await runNTimes(input, {}, RUNS); expectAllRunsIdentical(records); @@ -144,7 +148,7 @@ describe('runGraph — replay determinism (re-execution equivalence)', () => { // Promise.all resolves with results in input order. The runner reads // them positionally, so the recorded event sequence must be identical // even though B and C run concurrently. - const input = makeInput([trigger('A'), trigger('B'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'A', 'C')]); + const input = makeInput([start('A'), trigger('B'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'A', 'C')]); const records = await runNTimes(input, {}, RUNS); expectAllRunsIdentical(records); @@ -156,7 +160,7 @@ describe('runGraph — replay determinism (re-execution equivalence)', () => { it('diamond A→{B,C}→D — fan-in join sees both upstreams in deterministic order', async () => { const input = makeInput( - [trigger('A'), trigger('B'), trigger('C'), trigger('D')], + [start('A'), trigger('B'), trigger('C'), trigger('D')], [edge('e1', 'A', 'B'), edge('e2', 'A', 'C'), edge('e3', 'B', 'D'), edge('e4', 'C', 'D')], ); @@ -169,7 +173,7 @@ describe('runGraph — replay determinism (re-execution equivalence)', () => { // The pruning decision in `propagate` comes from `nextPort` (data) and // `sourceHandle` (data) — both injected, no randomness possible. Pin it. const input = makeInput( - [trigger('D'), trigger('B'), trigger('C')], + [start('D'), trigger('B'), trigger('C')], [edge('e1', 'D', 'B', 'X'), edge('e2', 'D', 'C', 'Y')], ); @@ -182,7 +186,7 @@ describe('runGraph — replay determinism (re-execution equivalence)', () => { // The catch branch builds errorPayload from the thrown error. Across // replays the activity returns the same error (cached in history), so // the same payload must surface. Pin it. - const input = makeInput([trigger('A'), trigger('B'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C')]); + const input = makeInput([start('A'), trigger('B'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C')]); const records = await runNTimes(input, { B: { throws: { message: 'slow down', code: 'rate_limited' } } }, RUNS); expectAllRunsIdentical(records); @@ -198,7 +202,7 @@ describe('runGraph — replay determinism (re-execution equivalence)', () => { // switched to Set or to Object.keys (no longer Map) could re-order the // stalled-nodes list and change the message. Pin both. const input = makeInput( - [trigger('A'), trigger('B'), trigger('C')], + [start('A'), trigger('B'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C'), edge('e3', 'C', 'B')], ); @@ -212,10 +216,17 @@ describe('runGraph — replay determinism (re-execution equivalence)', () => { it('asymmetric fan-in — depth-mismatched join waits for both, every run', async () => { // The scheduler's job is exactly this case (B depth 1, Aprime depth 2, // join at C). Replay determinism here doubles as a regression pin for - // the scheduling algorithm. + // the scheduling algorithm. S fans out to the two legs, since a second + // root is no longer a legal shape. const input = makeInput( - [trigger('A'), trigger('Aprime'), trigger('B'), trigger('C')], - [edge('e1', 'A', 'Aprime'), edge('e2', 'Aprime', 'C'), edge('e3', 'B', 'C')], + [start('S'), trigger('A'), trigger('Aprime'), trigger('B'), trigger('C')], + [ + edge('e1', 'S', 'A'), + edge('e2', 'A', 'Aprime'), + edge('e3', 'Aprime', 'C'), + edge('e4', 'S', 'B'), + edge('e5', 'B', 'C'), + ], ); const records = await runNTimes(input, {}, RUNS); diff --git a/packages/execution-core/src/graph-runner.test.ts b/packages/execution-core/src/graph-runner.test.ts index f14afcac5..9d23e6682 100644 --- a/packages/execution-core/src/graph-runner.test.ts +++ b/packages/execution-core/src/graph-runner.test.ts @@ -82,6 +82,12 @@ function trigger(id: string, errorPolicy?: NodeErrorPolicy): TestNode { : { id, type: 'test/node', config: {}, errorPolicy }; } +function start(id: string, errorPolicy?: NodeErrorPolicy): TestNode { + return errorPolicy === undefined + ? { id, type: 'test/node', config: {}, role: 'start' } + : { id, type: 'test/node', config: {}, role: 'start', errorPolicy }; +} + function edge(id: string, source: string, target: string, sourceHandle?: string): WorkflowEdgeDefinition { return { id, sourceNodeId: source, targetNodeId: target, sourceHandle }; } @@ -109,7 +115,7 @@ describe('runGraph — topological scheduling', () => { const events = makeEvents(); const outcome = await runGraph( - makeInput([trigger('A'), trigger('B'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C')]), + makeInput([start('A'), trigger('B'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C')]), runner.port, events.port, ); @@ -126,7 +132,7 @@ describe('runGraph — topological scheduling', () => { const events = makeEvents(); await runGraph( - makeInput([trigger('A'), trigger('B'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'A', 'C')]), + makeInput([start('A'), trigger('B'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'A', 'C')]), runner.port, events.port, ); @@ -145,7 +151,7 @@ describe('runGraph — topological scheduling', () => { await runGraph( makeInput( - [trigger('A'), trigger('B'), trigger('C'), trigger('D')], + [start('A'), trigger('B'), trigger('C'), trigger('D')], [edge('e1', 'A', 'B'), edge('e2', 'A', 'C'), edge('e3', 'B', 'D'), edge('e4', 'C', 'D')], ), runner.port, @@ -159,17 +165,27 @@ describe('runGraph — topological scheduling', () => { expect(runner.callOrder.filter((id) => id === 'D')).toHaveLength(1); }); - it('asymmetric fan-in A→Aprime→C, B→C — C waits for BOTH', async () => { - // The canonical fan-in bug: B is depth 1, Aprime is depth 2. + it('asymmetric fan-in S→A→Aprime→C, S→B→C — C waits for BOTH', async () => { + // The canonical fan-in bug: from the start, B is depth 1 and Aprime is depth 2. // Old BFS scheduled C in wave 2 alongside Aprime → C ran without nodeOutputs[Aprime]. // New algorithm: C waits until BOTH B and Aprime complete. + // + // The asymmetry used to come from B being a second root. With exactly one start + // required, S fans out to the two legs of differing depth instead — same shape + // for the scheduler, one legal entrypoint. const runner = makeRunner(); const events = makeEvents(); await runGraph( makeInput( - [trigger('A'), trigger('Aprime'), trigger('B'), trigger('C')], - [edge('e1', 'A', 'Aprime'), edge('e2', 'Aprime', 'C'), edge('e3', 'B', 'C')], + [start('S'), trigger('A'), trigger('Aprime'), trigger('B'), trigger('C')], + [ + edge('e1', 'S', 'A'), + edge('e2', 'A', 'Aprime'), + edge('e3', 'Aprime', 'C'), + edge('e4', 'S', 'B'), + edge('e5', 'B', 'C'), + ], ), runner.port, events.port, @@ -183,23 +199,7 @@ describe('runGraph — topological scheduling', () => { expect(indexAprime).toBeLessThan(indexC); expect(indexB).toBeLessThan(indexC); // C sees both upstreams in nodeOutputs - expect(runner.contexts.C).toEqual({ A: 'out-A', Aprime: 'out-Aprime', B: 'out-B' }); - }); - - it('multi-entrypoint — independent roots run together', async () => { - const runner = makeRunner(); - const events = makeEvents(); - - await runGraph( - makeInput([trigger('R1'), trigger('R2'), trigger('Out')], [edge('e1', 'R1', 'Out'), edge('e2', 'R2', 'Out')]), - runner.port, - events.port, - ); - - // Both R1 and R2 run before Out - expect(runner.callOrder.slice(0, 2).sort()).toEqual(['R1', 'R2']); - expect(runner.callOrder.at(-1)).toBe('Out'); - expect(runner.contexts.Out).toEqual({ R1: 'out-R1', R2: 'out-R2' }); + expect(runner.contexts.C).toEqual({ S: 'out-S', A: 'out-A', Aprime: 'out-Aprime', B: 'out-B' }); }); it('decision routing — node reachable only via pruned branch is skipped silently', async () => { @@ -210,7 +210,7 @@ describe('runGraph — topological scheduling', () => { const events = makeEvents(); await runGraph( - makeInput([trigger('D'), trigger('B'), trigger('C')], [edge('e1', 'D', 'B', 'X'), edge('e2', 'D', 'C', 'Y')]), + makeInput([start('D'), trigger('B'), trigger('C')], [edge('e1', 'D', 'B', 'X'), edge('e2', 'D', 'C', 'Y')]), runner.port, events.port, ); @@ -231,7 +231,7 @@ describe('runGraph — topological scheduling', () => { await runGraph( makeInput( - [trigger('D'), trigger('B'), trigger('C'), trigger('E')], + [start('D'), trigger('B'), trigger('C'), trigger('E')], [edge('e1', 'D', 'B', 'X'), edge('e2', 'D', 'C', 'Y'), edge('e3', 'B', 'E'), edge('e4', 'C', 'E')], ), runner.port, @@ -252,7 +252,7 @@ describe('runGraph — topological scheduling', () => { await runGraph( makeInput( - [trigger('D'), trigger('B'), trigger('C'), trigger('Cprime'), trigger('E')], + [start('D'), trigger('B'), trigger('C'), trigger('Cprime'), trigger('E')], [ edge('e1', 'D', 'B', 'X'), edge('e2', 'D', 'C', 'Y'), @@ -277,7 +277,7 @@ describe('runGraph — topological scheduling', () => { const events = makeEvents(); const outcome = await runGraph( - makeInput([trigger('A'), trigger('B'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C')]), + makeInput([start('A'), trigger('B'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C')]), runner.port, events.port, ); @@ -290,21 +290,27 @@ describe('runGraph — topological scheduling', () => { expect(outcome).toEqual({ status: 'failed', error: { message: 'boom' } }); }); - it('fails the run when there is no entrypoint', async () => { + it('a graph rejected by the start-node rule fails the run before any node executes', async () => { + // The rule itself — missing start, duplicate starts, an edge back into the start, + // orphans — is covered in resolve-start-node.test.ts. What matters here is the + // wiring: a rejected graph fails the execution, reports the reason, and runs + // nothing. Orphan is the motivating shape: its only edge was deleted, so inferring + // roots from in-degree would run it in wave 1 with no upstream output and still + // feed its result downstream. const runner = makeRunner(); const events = makeEvents(); - // Cycle with no in-degree-zero node const outcome = await runGraph( - makeInput([trigger('A'), trigger('B')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'A')]), + makeInput([start('T'), trigger('A'), trigger('Orphan')], [edge('e1', 'T', 'A')]), runner.port, events.port, ); - expect(outcome).toEqual({ status: 'failed', error: { message: 'Workflow has no entrypoint node' } }); + const message = 'Workflow has orphaned nodes (no incoming edge and not a start node): Orphan'; + expect(outcome).toEqual({ status: 'failed', error: { message } }); expect(runner.callOrder).toEqual([]); expect(events.events.map((event) => event.type)).toEqual(['execution_started', 'execution_failed']); - expect(events.statuses.at(-1)).toEqual({ status: 'failed', errorMessage: 'Workflow has no entrypoint node' }); + expect(events.statuses.at(-1)).toEqual({ status: 'failed', errorMessage: message }); }); it('cycle reachable from an entrypoint fails the workflow with a stalled-node message', async () => { @@ -318,7 +324,7 @@ describe('runGraph — topological scheduling', () => { const outcome = await runGraph( makeInput( - [trigger('A'), trigger('B'), trigger('C')], + [start('A'), trigger('B'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C'), edge('e3', 'C', 'B')], ), runner.port, @@ -351,7 +357,7 @@ describe('runGraph — topological scheduling', () => { }; const events = makeEvents(); - await runGraph(makeInput([trigger('A'), trigger('D')], [edge('e1', 'A', 'D')]), runner, events.port); + await runGraph(makeInput([start('A'), trigger('D')], [edge('e1', 'A', 'D')]), runner, events.port); const nodeFailed = events.events.find((event) => event.type === 'node_failed' && event.nodeId === 'D'); expect(nodeFailed?.payload).toEqual({ @@ -377,7 +383,7 @@ describe('runGraph — topological scheduling', () => { }; const events = makeEvents(); - await runGraph(makeInput([trigger('A'), trigger('B')], [edge('e1', 'A', 'B')]), runner, events.port); + await runGraph(makeInput([start('A'), trigger('B')], [edge('e1', 'A', 'B')]), runner, events.port); const nodeFailed = events.events.find((event) => event.type === 'node_failed' && event.nodeId === 'B'); expect(nodeFailed?.payload).toEqual({ error: { message: 'boom' } }); @@ -401,7 +407,7 @@ describe('runGraph — topological scheduling', () => { }; const events = makeEvents(); - await runGraph(makeInput([trigger('A'), trigger('B')], [edge('e1', 'A', 'B')]), runner, events.port); + await runGraph(makeInput([start('A'), trigger('B')], [edge('e1', 'A', 'B')]), runner, events.port); const nodeFailed = events.events.find((event) => event.type === 'node_failed' && event.nodeId === 'B'); expect(nodeFailed?.payload).toEqual({ @@ -424,7 +430,7 @@ describe('runGraph — topological scheduling', () => { }; const events = makeEvents(); - await runGraph(makeInput([trigger('A')], []), runner, events.port); + await runGraph(makeInput([start('A')], []), runner, events.port); const nodeFailed = events.events.find((event) => event.type === 'node_failed'); expect(nodeFailed?.payload).toEqual({ error: { message: 'rate limit exceeded' } }); @@ -450,7 +456,7 @@ describe('runGraph — topological scheduling', () => { }; const events = makeEvents(); - await runGraph(makeInput([trigger('A')], []), runner, events.port); + await runGraph(makeInput([start('A')], []), runner, events.port); const nodeFailed = events.events.find((event) => event.type === 'node_failed'); expect(nodeFailed).toBeDefined(); @@ -475,7 +481,7 @@ describe('runGraph — topological scheduling', () => { }; const events = makeEvents(); - await runGraph(makeInput([trigger('A')], []), runner, events.port); + await runGraph(makeInput([start('A')], []), runner, events.port); const nodeFailed = events.events.find((event) => event.type === 'node_failed'); expect(nodeFailed?.payload).toEqual({ @@ -525,7 +531,7 @@ describe('runGraph — replay safety (sandbox-safe)', () => { const events = makeEvents(); await runGraph( - makeInput([trigger('A'), trigger('B'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'A', 'C')]), + makeInput([start('A'), trigger('B'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'A', 'C')]), runner.port, events.port, ); @@ -538,7 +544,7 @@ describe('runGraph — replay safety (sandbox-safe)', () => { const runner = makeRunner({ B: { throws: 'boom' } }); const events = makeEvents(); - await runGraph(makeInput([trigger('A'), trigger('B')], [edge('e1', 'A', 'B')]), runner.port, events.port); + await runGraph(makeInput([start('A'), trigger('B')], [edge('e1', 'A', 'B')]), runner.port, events.port); expect(events.statuses.at(-1)?.status).toBe('failed'); expect(events.events.some((event) => event.type === 'node_failed' && event.nodeId === 'B')).toBe(true); @@ -553,7 +559,7 @@ describe('runGraph — replay safety (sandbox-safe)', () => { await runGraph( makeInput( - [trigger('A'), trigger('B'), trigger('C')], + [start('A'), trigger('B'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C'), edge('e3', 'C', 'B')], ), runner.port, @@ -567,19 +573,21 @@ describe('runGraph — replay safety (sandbox-safe)', () => { expectNoConsoleWrites(); }); - it('a missing entrypoint writes nothing to console — surfaces via execution_failed event', async () => { + it('a missing start node writes nothing to console — surfaces via execution_failed event', async () => { const runner = makeRunner(); const events = makeEvents(); const outcome = await runGraph( - makeInput([trigger('A'), trigger('B')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'A')]), + makeInput([trigger('A'), trigger('B')], [edge('e1', 'A', 'B')]), runner.port, events.port, ); expect(outcome.status).toBe('failed'); const failedEvent = events.events.find((event) => event.type === 'execution_failed'); - expect(failedEvent?.payload).toEqual({ error: { message: 'Workflow has no entrypoint node' } }); + expect(failedEvent?.payload).toEqual({ + error: { message: 'Workflow has no start node: exactly one node must be marked as the start node' }, + }); expectNoConsoleWrites(); }); }); @@ -590,7 +598,7 @@ describe('runGraph — errorPolicy', () => { const events = makeEvents({ type: 'node_started', nodeId: 'B', message: 'events table unreachable' }); const outcome = await runGraph( - makeInput([trigger('A'), trigger('B'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C')]), + makeInput([start('A'), trigger('B'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C')]), runner.port, events.port, ); @@ -607,7 +615,7 @@ describe('runGraph — errorPolicy', () => { const events = makeEvents({ type: 'node_started', nodeId: 'B', message: 'events table unreachable' }); const outcome = await runGraph( - makeInput([trigger('A'), trigger('B', 'continue'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C')]), + makeInput([start('A'), trigger('B', 'continue'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C')]), runner.port, events.port, ); @@ -623,7 +631,7 @@ describe('runGraph — errorPolicy', () => { const outcome = await runGraph( makeInput( - [trigger('A'), trigger('B', 'errorRoute'), trigger('Recover'), trigger('Success')], + [start('A'), trigger('B', 'errorRoute'), trigger('Recover'), trigger('Success')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'Recover', 'errorRoute'), edge('e3', 'B', 'Success')], ), runner.port, @@ -640,7 +648,7 @@ describe('runGraph — errorPolicy', () => { const events = makeEvents(); await runGraph( - makeInput([trigger('A'), trigger('B', 'fail'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C')]), + makeInput([start('A'), trigger('B', 'fail'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C')]), runner.port, events.port, ); @@ -654,7 +662,7 @@ describe('runGraph — errorPolicy', () => { const events = makeEvents(); const outcome = await runGraph( - makeInput([trigger('A'), trigger('B', 'continue'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C')]), + makeInput([start('A'), trigger('B', 'continue'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C')]), runner.port, events.port, ); @@ -679,7 +687,7 @@ describe('runGraph — errorPolicy', () => { const events = makeEvents(); await runGraph( - makeInput([trigger('A'), trigger('B', 'continue'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C')]), + makeInput([start('A'), trigger('B', 'continue'), trigger('C')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'C')]), runner, events.port, ); @@ -697,7 +705,7 @@ describe('runGraph — errorPolicy', () => { const outcome = await runGraph( makeInput( - [trigger('A'), trigger('B', 'errorRoute'), trigger('Success'), trigger('Recovery')], + [start('A'), trigger('B', 'errorRoute'), trigger('Success'), trigger('Recovery')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'Success', 'success'), edge('e3', 'B', 'Recovery', 'errorRoute')], ), runner.port, @@ -720,7 +728,7 @@ describe('runGraph — errorPolicy', () => { await runGraph( makeInput( [ - trigger('A'), + start('A'), trigger('B', 'errorRoute'), trigger('Success'), trigger('SuccessPrime'), @@ -749,7 +757,7 @@ describe('runGraph — errorPolicy', () => { await runGraph( makeInput( - [trigger('A'), trigger('B', 'continue'), trigger('C'), trigger('D')], + [start('A'), trigger('B', 'continue'), trigger('C'), trigger('D')], [edge('e1', 'A', 'B'), edge('e2', 'A', 'C'), edge('e3', 'B', 'D'), edge('e4', 'C', 'D')], ), runner.port, @@ -776,7 +784,7 @@ describe('runGraph — errorPolicy', () => { await runGraph( makeInput( - [trigger('A'), trigger('B', 'continue'), trigger('C', 'fail')], + [start('A'), trigger('B', 'continue'), trigger('C', 'fail')], [edge('e1', 'A', 'B'), edge('e2', 'A', 'C')], ), runner.port, @@ -793,7 +801,7 @@ describe('runGraph — errorPolicy', () => { const runner = makeRunner({ A: { throws: 'boom' } }); const events = makeEvents(); - await runGraph(makeInput([trigger('A', 'errorRoute')], []), runner.port, events.port); + await runGraph(makeInput([start('A', 'errorRoute')], []), runner.port, events.port); expect(events.events.some((event) => event.type === 'node_failed' && event.nodeId === 'A')).toBe(true); expect(events.statuses.at(-1)?.status).toBe('completed'); @@ -808,7 +816,7 @@ describe('runGraph — errorPolicy', () => { await runGraph( makeInput( - [trigger('A'), trigger('B', 'continue'), trigger('Success'), trigger('ErrorBranch')], + [start('A'), trigger('B', 'continue'), trigger('Success'), trigger('ErrorBranch')], [edge('e1', 'A', 'B'), edge('e2', 'B', 'Success'), edge('e3', 'B', 'ErrorBranch', 'errorRoute')], ), runner.port, @@ -828,7 +836,7 @@ describe('runGraph — errorPolicy', () => { await runGraph( makeInput( - [trigger('A'), trigger('Success'), trigger('ErrorBranch')], + [start('A'), trigger('Success'), trigger('ErrorBranch')], [edge('e1', 'A', 'Success'), edge('e2', 'A', 'ErrorBranch', 'errorRoute')], ), runner.port, diff --git a/packages/execution-core/src/graph-runner.ts b/packages/execution-core/src/graph-runner.ts index 1a0045bc6..d09191144 100644 --- a/packages/execution-core/src/graph-runner.ts +++ b/packages/execution-core/src/graph-runner.ts @@ -9,6 +9,7 @@ import type { ExecutionContext } from './execution-context'; import type { ActivityRunnerPort } from './ports/activity-runner.port'; import type { EventEmitterPort } from './ports/event-emitter.port'; import type { WorkflowExecutionInput } from './ports/workflow-engine.port'; +import { resolveStartNode } from './resolve-start-node'; // `sourceHandle` reserved for the 'errorRoute' error policy. Edges tagged // with this value fire ONLY when the upstream node failed with policy @@ -24,7 +25,8 @@ const RESERVED_ERROR_HANDLE = 'errorRoute'; // Temporal adapter raises an ApplicationFailure so the Workflow Execution shows as // Failed rather than Completed). Note a node failing under errorPolicy 'continue' or // 'errorRoute' is absorbed by the graph and still yields `{ status: 'completed' }` — -// only an unhandled node failure, a stall, or a missing entrypoint fails the run. +// only an unhandled node failure, a stall, or a malformed start (missing, duplicated, +// or with orphaned nodes alongside it) fails the run. export type RunGraphOutcome = { status: 'completed' } | { status: 'failed'; error: { message: string; code?: string } }; // Topological scheduler. A node becomes ready only when ALL of its incoming @@ -50,10 +52,11 @@ export async function runGraph( await events.emitEvent(input.executionId, 'execution_started', { workflowId: input.workflowId }); - const entrypoints = input.definition.nodes.filter((node) => (inDegree.get(node.id) ?? 0) === 0); - if (entrypoints.length === 0) { - return await failExecution(input.executionId, events, { message: 'Workflow has no entrypoint node' }); + const entry = resolveStartNode(input.definition.nodes, inDegree); + if ('error' in entry) { + return await failExecution(input.executionId, events, { message: entry.error }); } + const startNode = entry.startNode; // pendingPredecessors counts incoming edges not yet resolved (completed OR pruned). // liveIncoming counts incoming edges that resolved via a non-pruned route. @@ -66,7 +69,7 @@ export async function runGraph( status: new Map(input.definition.nodes.map((node) => [node.id, 'pending'])), }; - let ready: TNode[] = entrypoints; + let ready: TNode[] = [startNode]; const nodeOutputs: Record = {}; while (ready.length > 0) { diff --git a/packages/execution-core/src/resolve-start-node.test.ts b/packages/execution-core/src/resolve-start-node.test.ts new file mode 100644 index 000000000..7144e992c --- /dev/null +++ b/packages/execution-core/src/resolve-start-node.test.ts @@ -0,0 +1,187 @@ +import { describe, expect, it } from 'vitest'; + +import type { BaseNode, WorkflowEdgeDefinition } from '@workflow-builder/types/workflow-execution/execution-model'; + +import { resolveStartNode } from './resolve-start-node'; + +type TestNode = BaseNode & { type: 'test/node' }; + +function node(id: string): TestNode { + return { id, type: 'test/node', config: {} }; +} + +function start(id: string): TestNode { + return { id, type: 'test/node', config: {}, role: 'start' }; +} + +function edge(id: string, source: string, target: string): WorkflowEdgeDefinition { + return { id, sourceNodeId: source, targetNodeId: target }; +} + +// Mirrors `computeInDegrees` in graph-runner.ts. Kept local rather than imported so +// these tests exercise `resolveStartNode` against a plain map, independent of how the +// runner happens to build one. +function inDegreeOf(nodes: TestNode[], edges: WorkflowEdgeDefinition[]): Map { + const inDegree = new Map(nodes.map((node_) => [node_.id, 0])); + for (const { targetNodeId } of edges) { + if (inDegree.has(targetNodeId)) inDegree.set(targetNodeId, (inDegree.get(targetNodeId) ?? 0) + 1); + } + return inDegree; +} + +function resolve(nodes: TestNode[], edges: WorkflowEdgeDefinition[] = []) { + return resolveStartNode(nodes, inDegreeOf(nodes, edges)); +} + +describe('resolveStartNode — the accepted shape', () => { + it('returns the declared start node', () => { + const nodes = [start('T'), node('A'), node('B')]; + + const result = resolve(nodes, [edge('e1', 'T', 'A'), edge('e2', 'A', 'B')]); + + expect(result).toEqual({ startNode: nodes[0] }); + }); + + it('accepts a start node listed after the nodes it feeds', () => { + const nodes = [node('A'), node('B'), start('T')]; + + const result = resolve(nodes, [edge('e1', 'T', 'A'), edge('e2', 'A', 'B')]); + + expect(result).toEqual({ startNode: nodes[2] }); + }); + + it('accepts a lone start node with no edges at all', () => { + const nodes = [start('T')]; + + expect(resolve(nodes)).toEqual({ startNode: nodes[0] }); + }); + + it('accepts a node that is unreachable but still has an incoming edge', () => { + // B and C form a cycle disconnected from the start. Neither sits at in-degree 0, + // so this is not an orphan; the runner's post-loop stall check owns this case and + // reports it with the list of nodes that never became ready. + const nodes = [start('T'), node('A'), node('B'), node('C')]; + + const result = resolve(nodes, [edge('e1', 'T', 'A'), edge('e2', 'B', 'C'), edge('e3', 'C', 'B')]); + + expect(result).toEqual({ startNode: nodes[0] }); + }); +}); + +describe('resolveStartNode — rejected shapes', () => { + it('rejects a graph where no node is marked as the start', () => { + const result = resolve([node('A'), node('B')], [edge('e1', 'A', 'B')]); + + expect(result).toEqual({ + error: 'Workflow has no start node: exactly one node must be marked as the start node', + }); + }); + + it('rejects an empty graph', () => { + expect(resolve([])).toEqual({ + error: 'Workflow has no start node: exactly one node must be marked as the start node', + }); + }); + + it('rejects two start nodes, naming both', () => { + const result = resolve([start('T1'), start('T2'), node('Out')], [edge('e1', 'T1', 'Out'), edge('e2', 'T2', 'Out')]); + + expect(result).toEqual({ + error: 'Workflow has 2 start nodes, but exactly one is allowed: T1, T2', + }); + }); + + it('rejects three start nodes, counting and naming all of them in node order', () => { + const result = resolve([start('T1'), node('A'), start('T2'), start('T3')], [edge('e1', 'T1', 'A')]); + + expect(result).toEqual({ + error: 'Workflow has 3 start nodes, but exactly one is allowed: T1, T2, T3', + }); + }); + + it('rejects an edge back into the start node', () => { + const result = resolve([start('T'), node('A')], [edge('e1', 'T', 'A'), edge('e2', 'A', 'T')]); + + expect(result).toEqual({ + error: 'Start node "T" has incoming edges: the start node must have none', + }); + }); + + it('rejects a node left with no incoming edge alongside a valid start', () => { + // The motivating case: Orphan's only edge was deleted. Inferring roots from + // in-degree would run it in the first wave with no upstream output. + const result = resolve([start('T'), node('A'), node('Orphan')], [edge('e1', 'T', 'A')]); + + expect(result).toEqual({ + error: 'Workflow has orphaned nodes (no incoming edge and not a start node): Orphan', + }); + }); + + it('names every orphan, not just the first', () => { + const result = resolve([start('T'), node('A'), node('Orphan1'), node('Orphan2')], [edge('e1', 'T', 'A')]); + + expect(result).toEqual({ + error: 'Workflow has orphaned nodes (no incoming edge and not a start node): Orphan1, Orphan2', + }); + }); +}); + +describe('resolveStartNode — which rule reports first', () => { + // The checks are ordered cheapest-to-most-specific, and each message is only + // actionable if the ones before it already hold. Pinned so a reordering has to be + // deliberate: an author fixing the reported problem should not hit a different + // message for a problem they did not introduce. + + it('reports the missing start before any orphan', () => { + // With no start declared, every in-degree-0 node looks like an orphan. Reporting + // those first would name nodes that are fine once a start exists. + const result = resolve([node('A'), node('B')], []); + + expect(result).toEqual({ + error: 'Workflow has no start node: exactly one node must be marked as the start node', + }); + }); + + it('reports duplicate starts before an edge back into one of them', () => { + const result = resolve([start('T1'), start('T2')], [edge('e1', 'T2', 'T1')]); + + expect(result).toEqual({ + error: 'Workflow has 2 start nodes, but exactly one is allowed: T1, T2', + }); + }); + + it('reports an edge back into the start before any orphan', () => { + const result = resolve([start('T'), node('A'), node('Orphan')], [edge('e1', 'T', 'A'), edge('e2', 'A', 'T')]); + + expect(result).toEqual({ + error: 'Start node "T" has incoming edges: the start node must have none', + }); + }); +}); + +describe('resolveStartNode — in-degree map edge cases', () => { + it('treats a node missing from the map as having no incoming edges', () => { + // The runner always passes a complete map, but a partial one must not silently + // promote an unlisted node to "has predecessors" and hide it from the orphan check. + const nodes = [start('T'), node('Orphan')]; + + const result = resolveStartNode(nodes, new Map([['T', 0]])); + + expect(result).toEqual({ + error: 'Workflow has orphaned nodes (no incoming edge and not a start node): Orphan', + }); + }); + + it('does not mutate the map it is given', () => { + const nodes = [start('T'), node('A')]; + const inDegree = inDegreeOf(nodes, [edge('e1', 'T', 'A')]); + + resolveStartNode(nodes, inDegree); + + // The runner copies this map into its scheduler state right after calling us. + expect([...inDegree]).toEqual([ + ['T', 0], + ['A', 1], + ]); + }); +}); diff --git a/packages/execution-core/src/resolve-start-node.ts b/packages/execution-core/src/resolve-start-node.ts new file mode 100644 index 000000000..7c1df01bf --- /dev/null +++ b/packages/execution-core/src/resolve-start-node.ts @@ -0,0 +1,48 @@ +import type { BaseNode } from '@workflow-builder/types/workflow-execution/execution-model'; + +// Errors carry a message rather than a code: they name the offending node ids, and +// nothing downstream branches on them — the runner turns whichever arrives into the +// same terminal failure. +type StartNodeResolution = { startNode: TNode } | { error: string }; + +// Validates the graph's entry shape and returns the one node the run begins at. +// +// Every runnable workflow has exactly one start node, declared by the author rather +// than inferred from the graph. Inferring roots from in-degree cannot tell an +// intentional trigger from a node whose only incoming edge was deleted: the orphan +// runs in the first wave with no upstream output, and that output still flows +// downstream. Requiring the declaration turns each of those shapes into a named error +// before any node runs. +export function resolveStartNode( + nodes: TNode[], + inDegree: Map, +): StartNodeResolution { + const startNodes = nodes.filter((node) => node.role === 'start'); + + if (startNodes.length === 0) { + return { error: 'Workflow has no start node: exactly one node must be marked as the start node' }; + } + if (startNodes.length > 1) { + const ids = startNodes.map((node) => node.id).join(', '); + return { error: `Workflow has ${startNodes.length} start nodes, but exactly one is allowed: ${ids}` }; + } + + const startNode = startNodes[0]!; + + // An edge back into the start node would leave it waiting on a predecessor it is + // supposed to precede. Caught here rather than left to the stall check, which would + // report it as a cycle several waves later. + if ((inDegree.get(startNode.id) ?? 0) > 0) { + return { error: `Start node "${startNode.id}" has incoming edges: the start node must have none` }; + } + + // Anything else sitting at in-degree 0 is unreachable, since the run begins at the + // start node alone. + const orphans = nodes.filter((node) => node.id !== startNode.id && (inDegree.get(node.id) ?? 0) === 0); + if (orphans.length > 0) { + const ids = orphans.map((node) => node.id).join(', '); + return { error: `Workflow has orphaned nodes (no incoming edge and not a start node): ${ids}` }; + } + + return { startNode }; +} diff --git a/packages/types/src/workflow-execution/execution-model.ts b/packages/types/src/workflow-execution/execution-model.ts index 723335023..6c8d48e5e 100644 --- a/packages/types/src/workflow-execution/execution-model.ts +++ b/packages/types/src/workflow-execution/execution-model.ts @@ -11,6 +11,17 @@ export const NODE_ERROR_POLICIES = ['fail', 'continue', 'errorRoute'] as const; export type NodeErrorPolicy = (typeof NODE_ERROR_POLICIES)[number]; +// Structural role a node plays in the graph, independent of its product type. +// `start` marks the execution entrypoint. Every runnable workflow declares exactly +// one: the runner begins there instead of inferring roots from in-degree, so a node +// left with no incoming edge is an authoring mistake rather than a second trigger. +// +// Unlike `NODE_ERROR_POLICIES` this needs no runtime tuple: a role is derived by +// the backend mapper from the editor's node kind, never validated against a value +// a client sent, so nothing has to check membership at runtime. Adding a role +// stays a one-line change. +export type NodeRole = 'start'; + // Minimal contract every node carries through the runner. Concrete node types // in worker packages narrow `config` via discriminated unions on `type`. export type BaseNode = { @@ -18,6 +29,7 @@ export type BaseNode = { type: string; config: unknown; errorPolicy?: NodeErrorPolicy; + role?: NodeRole; }; export type WorkflowEdgeDefinition = {