From ee32adf1c600ea8fcd0ed631265dd0aff6602676 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 19:44:10 +0000 Subject: [PATCH 1/9] feat(spec)!: refuse a top-level flow node no edge path from start reaches Claude-Session: https://claude.ai/code/session_01FEkQ5oEQNYg7j3ueo5t6pV Co-Authored-By: Claude --- packages/spec/src/automation/flow.test.ts | 131 +++++++++++++++++++--- packages/spec/src/automation/flow.zod.ts | 74 ++++++++++++ 2 files changed, 190 insertions(+), 15 deletions(-) diff --git a/packages/spec/src/automation/flow.test.ts b/packages/spec/src/automation/flow.test.ts index 09721f08eea..9e31ae84c65 100644 --- a/packages/spec/src/automation/flow.test.ts +++ b/packages/spec/src/automation/flow.test.ts @@ -2367,12 +2367,20 @@ describe('FlowSchema — top-level node ids are unique', () => { }); it('accepts the same flow once the ids are unique, keeping the nodes in authored order', () => { - const unique = flowWith([ - { id: 'start', type: 'start', label: 'Start' }, - { id: 'n', type: 'assignment', label: 'Assign A' }, - { id: 'n2', type: 'assignment', label: 'Assign B' }, - { id: 'end', type: 'end', label: 'End' }, - ]); + // `n2` is wired in: a node no edge reaches is refused at parse. + const unique = { + ...flowWith([ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'n', type: 'assignment', label: 'Assign A' }, + { id: 'n2', type: 'assignment', label: 'Assign B' }, + { id: 'end', type: 'end', label: 'End' }, + ]), + edges: [ + { id: 'e1', source: 'start', target: 'n' }, + { id: 'e2', source: 'n', target: 'n2' }, + { id: 'e3', source: 'n2', target: 'end' }, + ], + }; const result = FlowSchema.safeParse(unique); expect(result.success).toBe(true); if (!result.success) return; @@ -2645,15 +2653,23 @@ describe('FlowSchema — one node-id space across the top-level nodes[] and ever }); it('accepts a flow whose ids are unique across the whole flow, keeping every region node in authored order', () => { - const unique = flowWith([ - { id: 'start', type: 'start', label: 'Start' }, - loopOver([step('sweep_first'), step('sweep_second')], [{ id: 'b1', source: 'sweep_first', target: 'sweep_second' }]), - { - id: 'p', type: 'parallel', label: 'Fan out', - config: { branches: [{ nodes: [step('left')] }, { nodes: [step('right')] }] }, - }, - { id: 'end', type: 'end', label: 'End' }, - ]); + // `p` is wired in: a node no edge reaches is refused at parse. + const unique = { + ...flowWith([ + { id: 'start', type: 'start', label: 'Start' }, + loopOver([step('sweep_first'), step('sweep_second')], [{ id: 'b1', source: 'sweep_first', target: 'sweep_second' }]), + { + id: 'p', type: 'parallel', label: 'Fan out', + config: { branches: [{ nodes: [step('left')] }, { nodes: [step('right')] }] }, + }, + { id: 'end', type: 'end', label: 'End' }, + ]), + edges: [ + { id: 'e1', source: 'start', target: 'n' }, + { id: 'e2', source: 'n', target: 'p' }, + { id: 'e3', source: 'p', target: 'end' }, + ], + }; const result = FlowSchema.safeParse(unique); expect(result.success).toBe(true); if (!result.success) return; @@ -2852,3 +2868,88 @@ describe('FlowSchema — an edge resolves in its own graph, and is followed once expect(defineFlow(flow).edges.map((e) => e.id)).toEqual(['e1', 'e2', 'e3']); }); }); + +describe('FlowSchema — every top-level node is reachable from the start node', () => { + // A run starts at the `start` node and follows edges; nothing else schedules + // a top-level node. A node no edge path reaches parsed, published and never + // ran. Refused at `nodes[i]`, for each such node. + const start: FlowNode = { id: 'start', type: 'start', label: 'Start' }; + const end: FlowNode = { id: 'end', type: 'end', label: 'End' }; + const step = (id: string): FlowNode => ({ id, type: 'assignment', label: id }); + const flowOf = (nodes: FlowNode[], edges: FlowEdge[]): Flow => ({ + name: 'reach_flow', + label: 'Reach flow', + type: 'autolaunched', + nodes, + edges, + }); + + it('refuses an action node with no in-edge, anchored at the node, naming it and the start node', () => { + const result = FlowSchema.safeParse(flowOf([start, step('orphan'), end], [ + { id: 'e1', source: 'start', target: 'end' }, + ])); + expect(result.success).toBe(false); + if (result.success) return; + expect(result.error.issues.map((i) => [i.code, i.path])).toEqual([['custom', ['nodes', 1]]]); + const [issue] = result.error.issues; + expect(issue.message).toContain('Node `orphan` (`assignment`, `nodes[1]`) is not reachable'); + expect(issue.message).toContain('from the start node `start`'); + }); + + it('refuses every node of a disconnected chain — an in-edge from an unreachable node is no path', () => { + const result = FlowSchema.safeParse(flowOf([start, step('a'), step('b'), end], [ + { id: 'e1', source: 'start', target: 'end' }, + { id: 'e2', source: 'a', target: 'b' }, + { id: 'e3', source: 'b', target: 'end' }, + ])); + expect(result.success).toBe(false); + if (result.success) return; + expect(result.error.issues.map((i) => i.path)).toEqual([['nodes', 1], ['nodes', 2]]); + }); + + it('refuses through every door that parses a flow — defineFlow throws', () => { + expect(() => defineFlow(flowOf([start, step('orphan'), end], [ + { id: 'e1', source: 'start', target: 'end' }, + ]))).toThrow(/Node `orphan`/); + }); + + it('CONTROL — a node reached only by a `fault` edge or a conditional edge is reachable', () => { + const result = FlowSchema.safeParse(flowOf([start, step('work'), step('recover'), step('big'), end], [ + { id: 'e1', source: 'start', target: 'work' }, + { id: 'e2', source: 'work', target: 'end' }, + { id: 'e3', source: 'work', target: 'recover', type: 'fault' }, + { id: 'e4', source: 'recover', target: 'end' }, + { id: 'e5', source: 'start', target: 'big', condition: 'record.amount > 1' }, + { id: 'e6', source: 'big', target: 'end' }, + ])); + expect(result.success).toBe(true); + }); + + it('CONTROL — a `boundary_event`, and a node reached only through one, are not judged', () => { + const result = FlowSchema.safeParse(flowOf([ + start, + step('work'), + { + id: 'on_error', type: 'boundary_event', label: 'On error', + boundaryConfig: { attachedToNodeId: 'work', eventType: 'error' }, + }, + step('recover'), + end, + ], [ + { id: 'e1', source: 'start', target: 'work' }, + { id: 'e2', source: 'work', target: 'end' }, + { id: 'e3', source: 'on_error', target: 'recover' }, + { id: 'e4', source: 'recover', target: 'end' }, + ])); + expect(result.success).toBe(true); + }); + + it('CONTROL — a flow with no start node, and a graph with a dangling edge, are left to their own refusals', () => { + expect(FlowSchema.safeParse(flowOf([step('a'), end], [{ id: 'e1', source: 'a', target: 'end' }])).success) + .toBe(true); + const dangling = FlowSchema.safeParse(flowOf([start, end], [{ id: 'e1', source: 'start', target: 'node_1' }])); + expect(dangling.success).toBe(false); + if (dangling.success) return; + expect(dangling.error.issues.map((i) => i.path)).toEqual([['edges', 0, 'target']]); + }); +}); diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index 3219078af4c..b0af6a93c29 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -1910,8 +1910,82 @@ export const FlowSchema = lazySchema(() => strictObject( }); }); } + + for (const refusal of unreachableTopLevelNodeRefusals(flow.nodes, flow.edges)) { + ctx.addIssue({ code: 'custom', path: ['nodes', refusal.index], message: refusal.message }); + } })); +/** + * The top-level nodes no edge path from the `start` node reaches, each with + * the refusal an author meets at `nodes[i]`. + * + * The engine runs a flow from its first `start` node and follows out-edges + * from there (`service-automation`'s `executeNode` → `traverseNext`); nothing + * else ever schedules a top-level node. So a node no edge path reaches — an + * edge forgotten while wiring, a node left behind when its in-edge was + * deleted — parses, registers, publishes and never runs. Refused here so the + * author meets it at every door the endpoint refusal above reaches. + * + * Scope, declared rather than discovered: + * + * - The TOP-LEVEL graph only. A region body (`loop` / `try_catch` / + * `parallel`) is already single-entry and acyclic under ADR-0031 + * (`analyzeRegion`), and in a DAG with one source every node is reachable + * from it, so a region cannot hold an unreachable node. + * - A flow with no `start` node is not judged: it has no run to reach anything + * from, and the engine refuses to run it (`FLOW_NO_START_NODE`). + * - Every edge counts, whatever its `type` or `condition`: a `fault` edge or a + * conditional edge is a path the engine can take. + * - A graph holding an edge whose endpoint names no top-level node is not + * judged: the endpoint rule above refuses that edge, and the node it was + * meant to reach would otherwise be reported a second time, for the same + * fault, under a cure (add an edge) that is not the one to apply. + * - A `boundary_event` is exempt, and so is a node reached only through one: + * it is attached to its host by `boundaryConfig.attachedToNodeId`, not + * entered by an edge, and its own contract owns its refusal. + */ +function unreachableTopLevelNodeRefusals( + nodes: ReadonlyArray<{ id: string; type: string }>, + edges: ReadonlyArray<{ source: string; target: string }>, +): Array<{ index: number; message: string }> { + const start = nodes.find((node) => node.type === 'start'); + if (start === undefined) return []; + const ids = new Set(nodes.map((node) => node.id)); + if (edges.some((edge) => !ids.has(edge.source) || !ids.has(edge.target))) return []; + const outgoing = new Map(); + for (const edge of edges) { + const targets = outgoing.get(edge.source) ?? []; + targets.push(edge.target); + outgoing.set(edge.source, targets); + } + const reached = new Set(); + const visit = (from: readonly string[]): void => { + const queue = [...from]; + while (queue.length > 0) { + const id = queue.shift()!; + if (reached.has(id)) continue; + reached.add(id); + queue.push(...(outgoing.get(id) ?? [])); + } + }; + visit([start.id]); + visit(nodes.filter((node) => node.type === 'boundary_event').map((node) => node.id)); + const refusals: Array<{ index: number; message: string }> = []; + nodes.forEach((node, index) => { + if (reached.has(node.id)) return; + refusals.push({ + index, + message: + `Node \`${node.id}\` (\`${node.type}\`, \`nodes[${index}]\`) is not reachable: no edge path leads to it ` + + `from the start node \`${start.id}\`. A run starts at the start node and follows edges, so this node ` + + 'never runs, while the flow registers and reports itself as configured. Add an edge into it from a ' + + 'node that runs, or delete the node.', + }); + }); + return refusals; +} + /** * The edge list a `FlowGraph` was read from, as the author wrote it — * `graph.path` is the key path from the flow root to the object holding it. From 1a0e8dbe217be391c3bfefcc856fe450cba0a839 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 20:02:43 +0000 Subject: [PATCH 2/9] test: wire the fixtures that held a node no edge reaches Claude-Session: https://claude.ai/code/session_01FEkQ5oEQNYg7j3ueo5t6pV Co-Authored-By: Claude --- .../src/builtin/notify-currency-read-fault.test.ts | 5 ++++- .../src/canonicalize-stored-flow.test.ts | 5 ++++- .../src/fault-edge-guard-containment.test.ts | 5 ++++- .../src/automation/flow-region-pause-and-end.test.ts | 8 +++++++- packages/spec/src/conversions/conversions.test.ts | 10 +++++----- .../src/shared/retired-key-removal-version.pin.test.ts | 2 +- 6 files changed, 25 insertions(+), 10 deletions(-) diff --git a/packages/services/service-automation/src/builtin/notify-currency-read-fault.test.ts b/packages/services/service-automation/src/builtin/notify-currency-read-fault.test.ts index 9202f191d64..233c5420d18 100644 --- a/packages/services/service-automation/src/builtin/notify-currency-read-fault.test.ts +++ b/packages/services/service-automation/src/builtin/notify-currency-read-fault.test.ts @@ -131,7 +131,10 @@ describe('notify — a currency hole refused after a FAILED tenant-settings read it('under errorHandling retry the run succeeds once the read does, and the title formats in the default', async () => { const flow = flowWith({ errorHandling: { strategy: 'retry', maxRetries: 2, backoffMs: 1 } }); - flow.edges = flow.edges.filter((e) => e.type !== 'fault'); + // Without its fault edge the handler is reached by nothing, which the + // parse refuses, so it goes with the edge. + flow.edges = flow.edges.filter((e) => e.type !== 'fault' && e.source !== 'handled'); + flow.nodes = flow.nodes.filter((n) => n.id !== 'handled'); const { engine, messaging } = engineWith(flow); let reads = 0; engine.setTenantCurrencyResolver(() => { diff --git a/packages/services/service-automation/src/canonicalize-stored-flow.test.ts b/packages/services/service-automation/src/canonicalize-stored-flow.test.ts index cc0b3740d41..7ff9b50282e 100644 --- a/packages/services/service-automation/src/canonicalize-stored-flow.test.ts +++ b/packages/services/service-automation/src/canonicalize-stored-flow.test.ts @@ -246,13 +246,16 @@ describe('canonicalizeStoredFlow — every site registerFlow refuses after the p engine.registerNodeExecutor(stampExecutor); return engine; }; + // The nodes are chained `start → … → end`: a top-level node no edge reaches + // is refused at parse, and that is not the check these cases judge. + const chain = (ids: string[]) => ids.slice(1).map((target, i) => ({ id: `e${i}`, source: ids[i], target })); const flowOf = (nodes: unknown[], extra: Record = {}) => ({ name: 'fd_stored', label: 'Stored', type: 'autolaunched', status: 'active', nodes: [{ id: 'start', type: 'start', label: 'Start' }, ...nodes, { id: 'end', type: 'end', label: 'End' }], - edges: [], + edges: chain(['start', ...nodes.map((n) => (n as { id: string }).id), 'end']), ...extra, }); const notify = (title: string) => ({ diff --git a/packages/services/service-automation/src/fault-edge-guard-containment.test.ts b/packages/services/service-automation/src/fault-edge-guard-containment.test.ts index 3607e0073f9..241cd4faecc 100644 --- a/packages/services/service-automation/src/fault-edge-guard-containment.test.ts +++ b/packages/services/service-automation/src/fault-edge-guard-containment.test.ts @@ -280,7 +280,10 @@ describe('#3863 — a fault edge must not swallow a guard refusal', () => { it('a guard refusal without any fault edge is fatal, as before', async () => { const flow = guardedDeleteWithHandler(); flow.name = 'no_handler'; - flow.edges = flow.edges.filter((e) => e.type !== 'fault'); + // Without its fault edge the handler is reached by nothing, which the + // parse refuses, so it goes with the edge. + flow.edges = flow.edges.filter((e) => e.type !== 'fault' && e.source !== 'handler'); + flow.nodes = flow.nodes.filter((n) => n.id !== 'handler'); engine.registerFlow('no_handler', flow); const result = await engine.execute('no_handler', { diff --git a/packages/spec/src/automation/flow-region-pause-and-end.test.ts b/packages/spec/src/automation/flow-region-pause-and-end.test.ts index 6b6f580aca7..5a658e788c7 100644 --- a/packages/spec/src/automation/flow-region-pause-and-end.test.ts +++ b/packages/spec/src/automation/flow-region-pause-and-end.test.ts @@ -57,7 +57,13 @@ const pausingNode = (type: string, id = 'pauser'): FlowNode => ({ const step = (id: string): FlowNode => ({ id, type: 'assignment', label: id }); -const flowWith = (nodes: FlowNode[], edges: FlowEdge[] = []): Flow => ({ +// Unless a test passes its own edges, the nodes are chained from `start` in +// order: a top-level node no edge reaches is refused at parse, and that rule is +// not the one these tests judge. +const chainFromStart = (nodes: FlowNode[]): FlowEdge[] => + nodes.map((node, i) => ({ id: `chain_${i}`, source: i === 0 ? 'start' : nodes[i - 1].id, target: node.id })); + +const flowWith = (nodes: FlowNode[], edges: FlowEdge[] = chainFromStart(nodes)): Flow => ({ name: 'region_contents', label: 'Region contents', type: 'autolaunched', diff --git a/packages/spec/src/conversions/conversions.test.ts b/packages/spec/src/conversions/conversions.test.ts index 37a2ba34e83..5189bfcf88d 100644 --- a/packages/spec/src/conversions/conversions.test.ts +++ b/packages/spec/src/conversions/conversions.test.ts @@ -456,7 +456,7 @@ describe('conversion layer (ADR-0087 D2)', () => { { id: 's', type: 'start', label: 'S' }, { id: 'g', type: 'get_record', label: 'G', config: { objectName: 'lead', object: 'lead' } }, ], - edges: [], + edges: [{ id: 'e1', source: 's', target: 'g' }], }], }; let refusal: unknown; @@ -482,7 +482,7 @@ describe('conversion layer (ADR-0087 D2)', () => { { id: 's', type: 'start', label: 'S' }, { id: 'h', type: 'http_request', label: 'H', config: { url: 'https://example.com' } }, ], - edges: [], + edges: [{ id: 'e1', source: 's', target: 'h' }], }], }; let refusal: unknown; @@ -723,7 +723,7 @@ describe('conversion layer (ADR-0087 D2)', () => { name: 'settle', label: 'Settle', type: 'autolaunched', - edges: [], + edges: [{ id: 'e1', source: 'n1', target: 'w' }], nodes: [ { id: 'n1', type: 'start', label: 'Start' }, { id: 'w', type: 'wait', label: 'Wait', ...node }, @@ -839,7 +839,7 @@ describe('conversion layer (ADR-0087 D2)', () => { name: 'notify_slack', label: 'Notify Slack', type: 'autolaunched', - edges: [], + edges: [{ id: 'e1', source: 'n1', target: 'c' }], nodes: [ { id: 'n1', type: 'start', label: 'Start' }, { id: 'c', type: 'connector_action', label: 'Call', ...node }, @@ -892,7 +892,7 @@ describe('conversion layer (ADR-0087 D2)', () => { name: 'settle', label: 'Settle', type: 'autolaunched', - edges: [], + edges: [{ id: 'e1', source: 'n1', target: String(node.id) }], nodes: [ { id: 'n1', type: 'start', label: 'Start' }, { label: 'Step', ...node }, diff --git a/packages/spec/src/shared/retired-key-removal-version.pin.test.ts b/packages/spec/src/shared/retired-key-removal-version.pin.test.ts index c6b69a906fb..2d7928f7736 100644 --- a/packages/spec/src/shared/retired-key-removal-version.pin.test.ts +++ b/packages/spec/src/shared/retired-key-removal-version.pin.test.ts @@ -247,7 +247,7 @@ const flowWithNode = (node: Record) => ({ name: 'settle', label: 'Settle', type: 'autolaunched', - edges: [], + edges: [{ id: 'e1', source: 'n1', target: 'n2' }], nodes: [{ id: 'n1', type: 'start', label: 'Start' }, { id: 'n2', label: 'Step', ...node }], }); From 5ccbebe30dae464ff17033a5a978317650b5a337 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 20:11:37 +0000 Subject: [PATCH 3/9] test(metadata-protocol): wire the flow fixtures that held a node no edge reaches Claude-Session: https://claude.ai/code/session_01FEkQ5oEQNYg7j3ueo5t6pV Co-Authored-By: Claude --- .../src/protocol.metadata-redaction.test.ts | 13 +++++++---- ...runtime-authoring-gate.flow-region.test.ts | 22 ++++++++++++------- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts b/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts index 2d60e0616c6..420f492c902 100644 --- a/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts +++ b/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts @@ -766,7 +766,9 @@ function relocate(served: any, secret?: string) { config: { triggerType: 'api', hookId: 'intake', ...(secret !== undefined ? { secret } : {}) }, }, ], - edges: [{ id: 'e1', source: 'begin_v2', target: 'finish' }], + // `e2` keeps the old start node reached: a node no edge reaches is + // refused at parse, which is not the refusal these tests are about. + edges: [{ id: 'e1', source: 'begin_v2', target: 'finish' }, { id: 'e2', source: 'begin_v2', target: 'begin' }], }; } @@ -906,7 +908,8 @@ function storedCalloutFlow() { }, }, ], - edges: [], + // Wired: a top-level node no edge reaches is refused at parse. + edges: [{ id: 's1', source: 'begin', target: 'each' }, { id: 's2', source: 'each', target: 'fan' }], }; } @@ -1042,6 +1045,7 @@ function storedMovesFlow() { const flow: any = storedCalloutFlow(); flow.name = 'moves'; flow.nodes.push({ id: 'call', type: 'http', label: 'Call', config: { url: 'https://c', durable: true, signingSecret: SIGNING } }); + flow.edges.push({ id: 's3', source: 'fan', target: 'call' }); return flow; } @@ -1054,6 +1058,7 @@ function movedOutOfLoop(served: any) { return { ...served, nodes: [begin, { ...each, config: { ...each.config, body: { nodes: [NOOP('body_noop')], edges: [] } } }, fan, call, perRow], + edges: [...served.edges, { id: 's4', source: 'call', target: perRow.id }], }; } @@ -1178,7 +1183,7 @@ describe('#20590 round 2 — the relocation counts nodes, not every object carry const stored = storedMovesFlow(); const incoming = movedWithEdgeTwin(redactMetadataItem('flow', stored)); const out: any = carryForwardRedactedValues('flow', incoming, stored); - expect(out.edges.map((e: any) => e.id)).toEqual(['per_row']); + expect(out.edges.map((e: any) => e.id)).toEqual(['s1', 's2', 's3', 's4', 'per_row']); expect(out.nodes[4].id).toBe('per_row'); expect(out.nodes[4].config.signingSecret).toBe(SIGNING); expect(allStrings(redactMetadataItem('flow', out))).not.toContain(SIGNING); @@ -1197,7 +1202,7 @@ describe('#20590 round 2 — the relocation counts nodes, not every object carry await protocol.saveMetaItem({ type: 'flow', name: 'moves', item: movedWithEdgeTwin(editable) }); const at = JSON.parse(Array.from(rows.values()).find((r) => r.name === 'moves' && r.state === 'active')!.metadata); - expect(at.edges.map((e: any) => e.id)).toEqual(['per_row']); + expect(at.edges.map((e: any) => e.id)).toEqual(['s1', 's2', 's3', 's4', 'per_row']); expect(at.nodes[4].config.signingSecret).toBe(SIGNING); expect(allStrings(await protocol.getMetaItem({ type: 'flow', name: 'moves' }))).not.toContain(SIGNING); }); diff --git a/packages/metadata-protocol/src/runtime-authoring-gate.flow-region.test.ts b/packages/metadata-protocol/src/runtime-authoring-gate.flow-region.test.ts index 3268c1c3f66..2c09449f9fb 100644 --- a/packages/metadata-protocol/src/runtime-authoring-gate.flow-region.test.ts +++ b/packages/metadata-protocol/src/runtime-authoring-gate.flow-region.test.ts @@ -21,14 +21,20 @@ const RULE = 'flow-region-shape-invalid'; type Rec = Record; -/** start → the given nodes (unwired) → end; region cases need no top-level edges. */ -const flowOf = (name: string, nodes: Rec[]) => ({ - name, - label: name, - type: 'autolaunched', - nodes: [{ id: 'start', type: 'start', label: 'Start' }, ...nodes, { id: 'end', type: 'end', label: 'End' }], - edges: [] as Rec[], -}); +/** + * start → the given nodes → end, chained in order: a top-level node no edge + * reaches is refused at parse, and that is not the rule these cases judge. + */ +const flowOf = (name: string, nodes: Rec[]) => { + const ids = ['start', ...nodes.map((n) => String(n.id)), 'end']; + return { + name, + label: name, + type: 'autolaunched', + nodes: [{ id: 'start', type: 'start', label: 'Start' }, ...nodes, { id: 'end', type: 'end', label: 'End' }], + edges: ids.slice(1).map((target, i) => ({ id: `e${i}`, source: ids[i], target })) as Rec[], + }; +}; const oneStep = (id: string) => ({ nodes: [{ id, type: 'assignment', label: id }], edges: [] }); From 12d73bfad535e3f6843254336ef8096d0c7e2aa7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 20:23:15 +0000 Subject: [PATCH 4/9] revert: take the parse-level reachability rule back out (it moves to lint) Claude-Session: https://claude.ai/code/session_01FEkQ5oEQNYg7j3ueo5t6pV Co-Authored-By: Claude --- .../src/protocol.metadata-redaction.test.ts | 13 +- ...runtime-authoring-gate.flow-region.test.ts | 22 ++- .../notify-currency-read-fault.test.ts | 5 +- .../src/canonicalize-stored-flow.test.ts | 5 +- .../src/fault-edge-guard-containment.test.ts | 5 +- .../flow-region-pause-and-end.test.ts | 8 +- packages/spec/src/automation/flow.test.ts | 131 ++---------------- packages/spec/src/automation/flow.zod.ts | 74 ---------- .../spec/src/conversions/conversions.test.ts | 10 +- .../retired-key-removal-version.pin.test.ts | 2 +- 10 files changed, 37 insertions(+), 238 deletions(-) diff --git a/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts b/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts index 420f492c902..2d60e0616c6 100644 --- a/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts +++ b/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts @@ -766,9 +766,7 @@ function relocate(served: any, secret?: string) { config: { triggerType: 'api', hookId: 'intake', ...(secret !== undefined ? { secret } : {}) }, }, ], - // `e2` keeps the old start node reached: a node no edge reaches is - // refused at parse, which is not the refusal these tests are about. - edges: [{ id: 'e1', source: 'begin_v2', target: 'finish' }, { id: 'e2', source: 'begin_v2', target: 'begin' }], + edges: [{ id: 'e1', source: 'begin_v2', target: 'finish' }], }; } @@ -908,8 +906,7 @@ function storedCalloutFlow() { }, }, ], - // Wired: a top-level node no edge reaches is refused at parse. - edges: [{ id: 's1', source: 'begin', target: 'each' }, { id: 's2', source: 'each', target: 'fan' }], + edges: [], }; } @@ -1045,7 +1042,6 @@ function storedMovesFlow() { const flow: any = storedCalloutFlow(); flow.name = 'moves'; flow.nodes.push({ id: 'call', type: 'http', label: 'Call', config: { url: 'https://c', durable: true, signingSecret: SIGNING } }); - flow.edges.push({ id: 's3', source: 'fan', target: 'call' }); return flow; } @@ -1058,7 +1054,6 @@ function movedOutOfLoop(served: any) { return { ...served, nodes: [begin, { ...each, config: { ...each.config, body: { nodes: [NOOP('body_noop')], edges: [] } } }, fan, call, perRow], - edges: [...served.edges, { id: 's4', source: 'call', target: perRow.id }], }; } @@ -1183,7 +1178,7 @@ describe('#20590 round 2 — the relocation counts nodes, not every object carry const stored = storedMovesFlow(); const incoming = movedWithEdgeTwin(redactMetadataItem('flow', stored)); const out: any = carryForwardRedactedValues('flow', incoming, stored); - expect(out.edges.map((e: any) => e.id)).toEqual(['s1', 's2', 's3', 's4', 'per_row']); + expect(out.edges.map((e: any) => e.id)).toEqual(['per_row']); expect(out.nodes[4].id).toBe('per_row'); expect(out.nodes[4].config.signingSecret).toBe(SIGNING); expect(allStrings(redactMetadataItem('flow', out))).not.toContain(SIGNING); @@ -1202,7 +1197,7 @@ describe('#20590 round 2 — the relocation counts nodes, not every object carry await protocol.saveMetaItem({ type: 'flow', name: 'moves', item: movedWithEdgeTwin(editable) }); const at = JSON.parse(Array.from(rows.values()).find((r) => r.name === 'moves' && r.state === 'active')!.metadata); - expect(at.edges.map((e: any) => e.id)).toEqual(['s1', 's2', 's3', 's4', 'per_row']); + expect(at.edges.map((e: any) => e.id)).toEqual(['per_row']); expect(at.nodes[4].config.signingSecret).toBe(SIGNING); expect(allStrings(await protocol.getMetaItem({ type: 'flow', name: 'moves' }))).not.toContain(SIGNING); }); diff --git a/packages/metadata-protocol/src/runtime-authoring-gate.flow-region.test.ts b/packages/metadata-protocol/src/runtime-authoring-gate.flow-region.test.ts index 2c09449f9fb..3268c1c3f66 100644 --- a/packages/metadata-protocol/src/runtime-authoring-gate.flow-region.test.ts +++ b/packages/metadata-protocol/src/runtime-authoring-gate.flow-region.test.ts @@ -21,20 +21,14 @@ const RULE = 'flow-region-shape-invalid'; type Rec = Record; -/** - * start → the given nodes → end, chained in order: a top-level node no edge - * reaches is refused at parse, and that is not the rule these cases judge. - */ -const flowOf = (name: string, nodes: Rec[]) => { - const ids = ['start', ...nodes.map((n) => String(n.id)), 'end']; - return { - name, - label: name, - type: 'autolaunched', - nodes: [{ id: 'start', type: 'start', label: 'Start' }, ...nodes, { id: 'end', type: 'end', label: 'End' }], - edges: ids.slice(1).map((target, i) => ({ id: `e${i}`, source: ids[i], target })) as Rec[], - }; -}; +/** start → the given nodes (unwired) → end; region cases need no top-level edges. */ +const flowOf = (name: string, nodes: Rec[]) => ({ + name, + label: name, + type: 'autolaunched', + nodes: [{ id: 'start', type: 'start', label: 'Start' }, ...nodes, { id: 'end', type: 'end', label: 'End' }], + edges: [] as Rec[], +}); const oneStep = (id: string) => ({ nodes: [{ id, type: 'assignment', label: id }], edges: [] }); diff --git a/packages/services/service-automation/src/builtin/notify-currency-read-fault.test.ts b/packages/services/service-automation/src/builtin/notify-currency-read-fault.test.ts index 233c5420d18..9202f191d64 100644 --- a/packages/services/service-automation/src/builtin/notify-currency-read-fault.test.ts +++ b/packages/services/service-automation/src/builtin/notify-currency-read-fault.test.ts @@ -131,10 +131,7 @@ describe('notify — a currency hole refused after a FAILED tenant-settings read it('under errorHandling retry the run succeeds once the read does, and the title formats in the default', async () => { const flow = flowWith({ errorHandling: { strategy: 'retry', maxRetries: 2, backoffMs: 1 } }); - // Without its fault edge the handler is reached by nothing, which the - // parse refuses, so it goes with the edge. - flow.edges = flow.edges.filter((e) => e.type !== 'fault' && e.source !== 'handled'); - flow.nodes = flow.nodes.filter((n) => n.id !== 'handled'); + flow.edges = flow.edges.filter((e) => e.type !== 'fault'); const { engine, messaging } = engineWith(flow); let reads = 0; engine.setTenantCurrencyResolver(() => { diff --git a/packages/services/service-automation/src/canonicalize-stored-flow.test.ts b/packages/services/service-automation/src/canonicalize-stored-flow.test.ts index 7ff9b50282e..cc0b3740d41 100644 --- a/packages/services/service-automation/src/canonicalize-stored-flow.test.ts +++ b/packages/services/service-automation/src/canonicalize-stored-flow.test.ts @@ -246,16 +246,13 @@ describe('canonicalizeStoredFlow — every site registerFlow refuses after the p engine.registerNodeExecutor(stampExecutor); return engine; }; - // The nodes are chained `start → … → end`: a top-level node no edge reaches - // is refused at parse, and that is not the check these cases judge. - const chain = (ids: string[]) => ids.slice(1).map((target, i) => ({ id: `e${i}`, source: ids[i], target })); const flowOf = (nodes: unknown[], extra: Record = {}) => ({ name: 'fd_stored', label: 'Stored', type: 'autolaunched', status: 'active', nodes: [{ id: 'start', type: 'start', label: 'Start' }, ...nodes, { id: 'end', type: 'end', label: 'End' }], - edges: chain(['start', ...nodes.map((n) => (n as { id: string }).id), 'end']), + edges: [], ...extra, }); const notify = (title: string) => ({ diff --git a/packages/services/service-automation/src/fault-edge-guard-containment.test.ts b/packages/services/service-automation/src/fault-edge-guard-containment.test.ts index 241cd4faecc..3607e0073f9 100644 --- a/packages/services/service-automation/src/fault-edge-guard-containment.test.ts +++ b/packages/services/service-automation/src/fault-edge-guard-containment.test.ts @@ -280,10 +280,7 @@ describe('#3863 — a fault edge must not swallow a guard refusal', () => { it('a guard refusal without any fault edge is fatal, as before', async () => { const flow = guardedDeleteWithHandler(); flow.name = 'no_handler'; - // Without its fault edge the handler is reached by nothing, which the - // parse refuses, so it goes with the edge. - flow.edges = flow.edges.filter((e) => e.type !== 'fault' && e.source !== 'handler'); - flow.nodes = flow.nodes.filter((n) => n.id !== 'handler'); + flow.edges = flow.edges.filter((e) => e.type !== 'fault'); engine.registerFlow('no_handler', flow); const result = await engine.execute('no_handler', { diff --git a/packages/spec/src/automation/flow-region-pause-and-end.test.ts b/packages/spec/src/automation/flow-region-pause-and-end.test.ts index 5a658e788c7..6b6f580aca7 100644 --- a/packages/spec/src/automation/flow-region-pause-and-end.test.ts +++ b/packages/spec/src/automation/flow-region-pause-and-end.test.ts @@ -57,13 +57,7 @@ const pausingNode = (type: string, id = 'pauser'): FlowNode => ({ const step = (id: string): FlowNode => ({ id, type: 'assignment', label: id }); -// Unless a test passes its own edges, the nodes are chained from `start` in -// order: a top-level node no edge reaches is refused at parse, and that rule is -// not the one these tests judge. -const chainFromStart = (nodes: FlowNode[]): FlowEdge[] => - nodes.map((node, i) => ({ id: `chain_${i}`, source: i === 0 ? 'start' : nodes[i - 1].id, target: node.id })); - -const flowWith = (nodes: FlowNode[], edges: FlowEdge[] = chainFromStart(nodes)): Flow => ({ +const flowWith = (nodes: FlowNode[], edges: FlowEdge[] = []): Flow => ({ name: 'region_contents', label: 'Region contents', type: 'autolaunched', diff --git a/packages/spec/src/automation/flow.test.ts b/packages/spec/src/automation/flow.test.ts index 9e31ae84c65..09721f08eea 100644 --- a/packages/spec/src/automation/flow.test.ts +++ b/packages/spec/src/automation/flow.test.ts @@ -2367,20 +2367,12 @@ describe('FlowSchema — top-level node ids are unique', () => { }); it('accepts the same flow once the ids are unique, keeping the nodes in authored order', () => { - // `n2` is wired in: a node no edge reaches is refused at parse. - const unique = { - ...flowWith([ - { id: 'start', type: 'start', label: 'Start' }, - { id: 'n', type: 'assignment', label: 'Assign A' }, - { id: 'n2', type: 'assignment', label: 'Assign B' }, - { id: 'end', type: 'end', label: 'End' }, - ]), - edges: [ - { id: 'e1', source: 'start', target: 'n' }, - { id: 'e2', source: 'n', target: 'n2' }, - { id: 'e3', source: 'n2', target: 'end' }, - ], - }; + const unique = flowWith([ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'n', type: 'assignment', label: 'Assign A' }, + { id: 'n2', type: 'assignment', label: 'Assign B' }, + { id: 'end', type: 'end', label: 'End' }, + ]); const result = FlowSchema.safeParse(unique); expect(result.success).toBe(true); if (!result.success) return; @@ -2653,23 +2645,15 @@ describe('FlowSchema — one node-id space across the top-level nodes[] and ever }); it('accepts a flow whose ids are unique across the whole flow, keeping every region node in authored order', () => { - // `p` is wired in: a node no edge reaches is refused at parse. - const unique = { - ...flowWith([ - { id: 'start', type: 'start', label: 'Start' }, - loopOver([step('sweep_first'), step('sweep_second')], [{ id: 'b1', source: 'sweep_first', target: 'sweep_second' }]), - { - id: 'p', type: 'parallel', label: 'Fan out', - config: { branches: [{ nodes: [step('left')] }, { nodes: [step('right')] }] }, - }, - { id: 'end', type: 'end', label: 'End' }, - ]), - edges: [ - { id: 'e1', source: 'start', target: 'n' }, - { id: 'e2', source: 'n', target: 'p' }, - { id: 'e3', source: 'p', target: 'end' }, - ], - }; + const unique = flowWith([ + { id: 'start', type: 'start', label: 'Start' }, + loopOver([step('sweep_first'), step('sweep_second')], [{ id: 'b1', source: 'sweep_first', target: 'sweep_second' }]), + { + id: 'p', type: 'parallel', label: 'Fan out', + config: { branches: [{ nodes: [step('left')] }, { nodes: [step('right')] }] }, + }, + { id: 'end', type: 'end', label: 'End' }, + ]); const result = FlowSchema.safeParse(unique); expect(result.success).toBe(true); if (!result.success) return; @@ -2868,88 +2852,3 @@ describe('FlowSchema — an edge resolves in its own graph, and is followed once expect(defineFlow(flow).edges.map((e) => e.id)).toEqual(['e1', 'e2', 'e3']); }); }); - -describe('FlowSchema — every top-level node is reachable from the start node', () => { - // A run starts at the `start` node and follows edges; nothing else schedules - // a top-level node. A node no edge path reaches parsed, published and never - // ran. Refused at `nodes[i]`, for each such node. - const start: FlowNode = { id: 'start', type: 'start', label: 'Start' }; - const end: FlowNode = { id: 'end', type: 'end', label: 'End' }; - const step = (id: string): FlowNode => ({ id, type: 'assignment', label: id }); - const flowOf = (nodes: FlowNode[], edges: FlowEdge[]): Flow => ({ - name: 'reach_flow', - label: 'Reach flow', - type: 'autolaunched', - nodes, - edges, - }); - - it('refuses an action node with no in-edge, anchored at the node, naming it and the start node', () => { - const result = FlowSchema.safeParse(flowOf([start, step('orphan'), end], [ - { id: 'e1', source: 'start', target: 'end' }, - ])); - expect(result.success).toBe(false); - if (result.success) return; - expect(result.error.issues.map((i) => [i.code, i.path])).toEqual([['custom', ['nodes', 1]]]); - const [issue] = result.error.issues; - expect(issue.message).toContain('Node `orphan` (`assignment`, `nodes[1]`) is not reachable'); - expect(issue.message).toContain('from the start node `start`'); - }); - - it('refuses every node of a disconnected chain — an in-edge from an unreachable node is no path', () => { - const result = FlowSchema.safeParse(flowOf([start, step('a'), step('b'), end], [ - { id: 'e1', source: 'start', target: 'end' }, - { id: 'e2', source: 'a', target: 'b' }, - { id: 'e3', source: 'b', target: 'end' }, - ])); - expect(result.success).toBe(false); - if (result.success) return; - expect(result.error.issues.map((i) => i.path)).toEqual([['nodes', 1], ['nodes', 2]]); - }); - - it('refuses through every door that parses a flow — defineFlow throws', () => { - expect(() => defineFlow(flowOf([start, step('orphan'), end], [ - { id: 'e1', source: 'start', target: 'end' }, - ]))).toThrow(/Node `orphan`/); - }); - - it('CONTROL — a node reached only by a `fault` edge or a conditional edge is reachable', () => { - const result = FlowSchema.safeParse(flowOf([start, step('work'), step('recover'), step('big'), end], [ - { id: 'e1', source: 'start', target: 'work' }, - { id: 'e2', source: 'work', target: 'end' }, - { id: 'e3', source: 'work', target: 'recover', type: 'fault' }, - { id: 'e4', source: 'recover', target: 'end' }, - { id: 'e5', source: 'start', target: 'big', condition: 'record.amount > 1' }, - { id: 'e6', source: 'big', target: 'end' }, - ])); - expect(result.success).toBe(true); - }); - - it('CONTROL — a `boundary_event`, and a node reached only through one, are not judged', () => { - const result = FlowSchema.safeParse(flowOf([ - start, - step('work'), - { - id: 'on_error', type: 'boundary_event', label: 'On error', - boundaryConfig: { attachedToNodeId: 'work', eventType: 'error' }, - }, - step('recover'), - end, - ], [ - { id: 'e1', source: 'start', target: 'work' }, - { id: 'e2', source: 'work', target: 'end' }, - { id: 'e3', source: 'on_error', target: 'recover' }, - { id: 'e4', source: 'recover', target: 'end' }, - ])); - expect(result.success).toBe(true); - }); - - it('CONTROL — a flow with no start node, and a graph with a dangling edge, are left to their own refusals', () => { - expect(FlowSchema.safeParse(flowOf([step('a'), end], [{ id: 'e1', source: 'a', target: 'end' }])).success) - .toBe(true); - const dangling = FlowSchema.safeParse(flowOf([start, end], [{ id: 'e1', source: 'start', target: 'node_1' }])); - expect(dangling.success).toBe(false); - if (dangling.success) return; - expect(dangling.error.issues.map((i) => i.path)).toEqual([['edges', 0, 'target']]); - }); -}); diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index b0af6a93c29..3219078af4c 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -1910,82 +1910,8 @@ export const FlowSchema = lazySchema(() => strictObject( }); }); } - - for (const refusal of unreachableTopLevelNodeRefusals(flow.nodes, flow.edges)) { - ctx.addIssue({ code: 'custom', path: ['nodes', refusal.index], message: refusal.message }); - } })); -/** - * The top-level nodes no edge path from the `start` node reaches, each with - * the refusal an author meets at `nodes[i]`. - * - * The engine runs a flow from its first `start` node and follows out-edges - * from there (`service-automation`'s `executeNode` → `traverseNext`); nothing - * else ever schedules a top-level node. So a node no edge path reaches — an - * edge forgotten while wiring, a node left behind when its in-edge was - * deleted — parses, registers, publishes and never runs. Refused here so the - * author meets it at every door the endpoint refusal above reaches. - * - * Scope, declared rather than discovered: - * - * - The TOP-LEVEL graph only. A region body (`loop` / `try_catch` / - * `parallel`) is already single-entry and acyclic under ADR-0031 - * (`analyzeRegion`), and in a DAG with one source every node is reachable - * from it, so a region cannot hold an unreachable node. - * - A flow with no `start` node is not judged: it has no run to reach anything - * from, and the engine refuses to run it (`FLOW_NO_START_NODE`). - * - Every edge counts, whatever its `type` or `condition`: a `fault` edge or a - * conditional edge is a path the engine can take. - * - A graph holding an edge whose endpoint names no top-level node is not - * judged: the endpoint rule above refuses that edge, and the node it was - * meant to reach would otherwise be reported a second time, for the same - * fault, under a cure (add an edge) that is not the one to apply. - * - A `boundary_event` is exempt, and so is a node reached only through one: - * it is attached to its host by `boundaryConfig.attachedToNodeId`, not - * entered by an edge, and its own contract owns its refusal. - */ -function unreachableTopLevelNodeRefusals( - nodes: ReadonlyArray<{ id: string; type: string }>, - edges: ReadonlyArray<{ source: string; target: string }>, -): Array<{ index: number; message: string }> { - const start = nodes.find((node) => node.type === 'start'); - if (start === undefined) return []; - const ids = new Set(nodes.map((node) => node.id)); - if (edges.some((edge) => !ids.has(edge.source) || !ids.has(edge.target))) return []; - const outgoing = new Map(); - for (const edge of edges) { - const targets = outgoing.get(edge.source) ?? []; - targets.push(edge.target); - outgoing.set(edge.source, targets); - } - const reached = new Set(); - const visit = (from: readonly string[]): void => { - const queue = [...from]; - while (queue.length > 0) { - const id = queue.shift()!; - if (reached.has(id)) continue; - reached.add(id); - queue.push(...(outgoing.get(id) ?? [])); - } - }; - visit([start.id]); - visit(nodes.filter((node) => node.type === 'boundary_event').map((node) => node.id)); - const refusals: Array<{ index: number; message: string }> = []; - nodes.forEach((node, index) => { - if (reached.has(node.id)) return; - refusals.push({ - index, - message: - `Node \`${node.id}\` (\`${node.type}\`, \`nodes[${index}]\`) is not reachable: no edge path leads to it ` + - `from the start node \`${start.id}\`. A run starts at the start node and follows edges, so this node ` + - 'never runs, while the flow registers and reports itself as configured. Add an edge into it from a ' + - 'node that runs, or delete the node.', - }); - }); - return refusals; -} - /** * The edge list a `FlowGraph` was read from, as the author wrote it — * `graph.path` is the key path from the flow root to the object holding it. diff --git a/packages/spec/src/conversions/conversions.test.ts b/packages/spec/src/conversions/conversions.test.ts index 5189bfcf88d..37a2ba34e83 100644 --- a/packages/spec/src/conversions/conversions.test.ts +++ b/packages/spec/src/conversions/conversions.test.ts @@ -456,7 +456,7 @@ describe('conversion layer (ADR-0087 D2)', () => { { id: 's', type: 'start', label: 'S' }, { id: 'g', type: 'get_record', label: 'G', config: { objectName: 'lead', object: 'lead' } }, ], - edges: [{ id: 'e1', source: 's', target: 'g' }], + edges: [], }], }; let refusal: unknown; @@ -482,7 +482,7 @@ describe('conversion layer (ADR-0087 D2)', () => { { id: 's', type: 'start', label: 'S' }, { id: 'h', type: 'http_request', label: 'H', config: { url: 'https://example.com' } }, ], - edges: [{ id: 'e1', source: 's', target: 'h' }], + edges: [], }], }; let refusal: unknown; @@ -723,7 +723,7 @@ describe('conversion layer (ADR-0087 D2)', () => { name: 'settle', label: 'Settle', type: 'autolaunched', - edges: [{ id: 'e1', source: 'n1', target: 'w' }], + edges: [], nodes: [ { id: 'n1', type: 'start', label: 'Start' }, { id: 'w', type: 'wait', label: 'Wait', ...node }, @@ -839,7 +839,7 @@ describe('conversion layer (ADR-0087 D2)', () => { name: 'notify_slack', label: 'Notify Slack', type: 'autolaunched', - edges: [{ id: 'e1', source: 'n1', target: 'c' }], + edges: [], nodes: [ { id: 'n1', type: 'start', label: 'Start' }, { id: 'c', type: 'connector_action', label: 'Call', ...node }, @@ -892,7 +892,7 @@ describe('conversion layer (ADR-0087 D2)', () => { name: 'settle', label: 'Settle', type: 'autolaunched', - edges: [{ id: 'e1', source: 'n1', target: String(node.id) }], + edges: [], nodes: [ { id: 'n1', type: 'start', label: 'Start' }, { label: 'Step', ...node }, diff --git a/packages/spec/src/shared/retired-key-removal-version.pin.test.ts b/packages/spec/src/shared/retired-key-removal-version.pin.test.ts index 2d7928f7736..c6b69a906fb 100644 --- a/packages/spec/src/shared/retired-key-removal-version.pin.test.ts +++ b/packages/spec/src/shared/retired-key-removal-version.pin.test.ts @@ -247,7 +247,7 @@ const flowWithNode = (node: Record) => ({ name: 'settle', label: 'Settle', type: 'autolaunched', - edges: [{ id: 'e1', source: 'n1', target: 'n2' }], + edges: [], nodes: [{ id: 'n1', type: 'start', label: 'Start' }, { id: 'n2', label: 'Step', ...node }], }); From 1f280319b9cd9be63f6083aa7950c153670699c9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 20:37:09 +0000 Subject: [PATCH 5/9] fix(lint)!: refuse a flow node no edge path from the start node reaches A run starts at the start node and follows edges, so a top-level node no edge path reaches never executes, while the flow parses, registers and reports itself as configured. lintFlowPatterns now reports it as `flow-node-unreachable` (severity error), which gates os validate/build/lint and the flow publish doors; a draft save still stores, with the finding in its diagnostics. Every edge type counts as a path. boundary_event nodes and the nodes only they lead to are exempt (reached by attachment), and region bodies are not judged (single-entry DAGs). A flow with no start node or a dangling edge is left to the flow parse. It is lint rather than a FlowSchema rule because the save door parses every save, drafts included, and the Studio designer's node delete can leave a successor unwired. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5oEQNYg7j3ueo5t6pV --- .changeset/31224-flow-node-unreachable.md | 21 +++ packages/lint/src/index.ts | 1 + packages/lint/src/lint-flow-patterns.test.ts | 145 +++++++++++++++++- packages/lint/src/lint-flow-patterns.ts | 88 +++++++++++ packages/lint/src/rule-explanations.ts | 23 +++ .../protocol.draft-publish-verdict.test.ts | 18 +++ 6 files changed, 290 insertions(+), 6 deletions(-) create mode 100644 .changeset/31224-flow-node-unreachable.md diff --git a/.changeset/31224-flow-node-unreachable.md b/.changeset/31224-flow-node-unreachable.md new file mode 100644 index 00000000000..1cb73bda575 --- /dev/null +++ b/.changeset/31224-flow-node-unreachable.md @@ -0,0 +1,21 @@ +--- +'@objectstack/lint': minor +--- + +fix(lint)!: a flow node no edge path from the start node reaches is refused at build and publish (`flow-node-unreachable`), because a run never executes it + +Clause-②: yes (narrowing) — `os validate`, `os build`, `os lint` and the flow save and publish doors refuse a flow they accepted before; `@objectstack/lint` also exports the rule id `FLOW_NODE_UNREACHABLE`, and `RULE_EXPLANATIONS` gains its entry. +Metadata-protocol: narrowed +Skills-docs: none — `skills/**` and `content/docs/**` teach only wired top-level flows; the edgeless examples in `content/docs/automation/flows.mdx` are region bodies, which this rule does not judge. The reasoning ships as the `os explain flow-node-unreachable` entry. + + + +**BREAKING** accept-set narrowing in `os validate` / `os build` / `os lint` and at the flow publish doors (`PUT /api/v1/meta/flow/NAME` live save, the per-item and package publish; Studio, REST and MCP authors), through the gating `lintFlowPatterns` rule. + +- **Before.** A flow whose top-level node had no edge path from the start node (`nodes: [start, stamp, end]`, `edges: [start → end]`) parsed, built, saved, published and registered, and reported itself as configured. The engine starts a run at the start node and follows edges, so `stamp` never ran, and nothing said so. +- **Now.** `flow-node-unreachable` (severity `error`) names each such node: `flow 'NAME' · node 'ID' (TYPE)`, why it never runs, and the fix. A draft save still stores, with the finding in its `_diagnostics`, so a designer that deletes a node and leaves its successor unwired can keep saving; the publish refuses it. +- **What counts as reaching a node.** Every edge type: default, conditional, fault and back. A `boundary_event` node, and the nodes only it leads to, are not judged (they are reached by attachment, not by an edge). +- **Not judged.** Nodes inside a region body (a region is a single-entry acyclic graph that the region validator already holds, so all of it is reached); a flow with no start node; a flow with an edge whose endpoint names no node (the flow parse refuses that first). +- **Why lint and not the flow parse.** The metadata save door parses `FlowSchema` on every save, drafts included, and the Studio designer's node delete can leave a successor unwired. A parse rule would refuse that draft save outright. + +**What an author does.** Nothing for a flow whose nodes are all wired from the start node. For a refused node: add an edge into it from a node that runs, or delete the node. A refused node never ran, so neither choice changes what the flow did. diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index a6881787fe8..c834de82d49 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -1042,6 +1042,7 @@ export { FLOW_DEFAULT_EDGE_WITH_CONDITION, FLOW_MULTIPLE_DEFAULT_EDGES, FLOW_INERT_NODE_CONDITION, + FLOW_NODE_UNREACHABLE, FLOW_MULTI_WRITE_UNFILTERED, FLOW_LOOP_BODY_UNCONTAINED, FLOW_TRY_CATCH_WITHOUT_CATCH, diff --git a/packages/lint/src/lint-flow-patterns.test.ts b/packages/lint/src/lint-flow-patterns.test.ts index e2f2e0ca713..a028cd21b73 100644 --- a/packages/lint/src/lint-flow-patterns.test.ts +++ b/packages/lint/src/lint-flow-patterns.test.ts @@ -33,6 +33,7 @@ import { FLOW_DEFAULT_EDGE_WITH_CONDITION, FLOW_MULTIPLE_DEFAULT_EDGES, FLOW_INERT_NODE_CONDITION, + FLOW_NODE_UNREACHABLE, FLOW_MULTI_WRITE_UNFILTERED, FLOW_LOOP_BODY_UNCONTAINED, FLOW_TRY_CATCH_WITHOUT_CATCH, @@ -91,7 +92,7 @@ const filterFlow = (filter: unknown) => ({ { id: 'start', type: 'start', config: { triggerType: 'schedule', schedule: 'cron:0 9 * * *' } }, { id: 'query', type: 'get_record', config: { objectName: 'contract', filter } }, ], - edges: [], + edges: [{ id: 'e1', source: 'start', target: 'query' }], }], }); @@ -220,7 +221,7 @@ describe('lintFlowPatterns — phantom aggregation capability (#1870)', () => { { id: 'start', type: 'start', config: {} }, { id: 'sum', type: 'script', config }, ], - edges: [], + edges: [{ id: 'e1', source: 'start', target: 'sum' }], }], }); @@ -250,7 +251,7 @@ const nodeFlow = (config: Record) => ({ { id: 'start', type: 'start', config: {} }, { id: 'create', type: 'create_record', config }, ], - edges: [], + edges: [{ id: 'e1', source: 'start', target: 'create' }], }], }); const rules = (s: any) => lintFlowPatterns(s).map((f) => f.rule); @@ -351,6 +352,7 @@ describe('lintFlowPatterns — approval revise loop (ADR-0044)', () => { const edges = [ { source: 'start', target: 'mgr' }, { source: 'mgr', target: 'ok', label: 'approve' }, + { source: 'mgr', target: 'no', label: 'reject' }, { source: 'mgr', target: 'wait', label: 'revise' }, ]; const fnds = lintFlowPatterns(approvalFlow(edges)); @@ -370,7 +372,11 @@ describe('lintFlowPatterns — approval revise loop (ADR-0044)', () => { { source: 'mgr', target: 'ok', label: 'approve' }, { source: 'mgr', target: 'no', label: 'reject' }, ]; - expect(lintFlowPatterns(approvalFlow(edges))).toEqual([]); + // No revise edge, so no revise node either: one left in place with no edge + // into it would draw `flow-node-unreachable` (it could never run). + const stack = approvalFlow(edges) as any; + stack.flows[0].nodes = stack.flows[0].nodes.filter((n: any) => n.id !== 'wait'); + expect(lintFlowPatterns(stack)).toEqual([]); }); // #3823 — the shape ADR-0044 D3 prescribed until its 2026-07-28 amendment. @@ -416,7 +422,7 @@ describe('lintFlowPatterns — user-less runAs unscoped (#1888 / ADR-0049 / ADR- { id: 'start', type: 'start', config: opts.startConfig ?? { triggerType: 'schedule', cron: '0 8 * * *' } }, { id: 'op', type: opts.nodeType ?? 'create_record', config: { objectName: 'thing', fields: { a: 1 } } }, ], - edges: [], + edges: [{ id: 'e1', source: 'start', target: 'op' }], }], }); @@ -528,7 +534,7 @@ describe('lintFlowPatterns — user-less runAs unscoped (#1888 / ADR-0049 / ADR- { id: 'start', type: 'start', config: { schedule: { type: 'interval', intervalMs: 60000 } } }, { id: 'notify', type: 'notify', config: { topic: 'x' } }, ], - edges: [], + edges: [{ id: 'e1', source: 'start', target: 'notify' }], }], })).toHaveLength(0); }); @@ -3710,3 +3716,130 @@ describe('[#22161] one-line fix — the twelve flow anti-pattern ids', () => { } }); }); + +describe('lintFlowPatterns — a node no edge path from start reaches (flow-node-unreachable)', () => { + type Edge = { id: string; source: string; target: string; type?: string; condition?: string }; + const reachFlow = (nodes: Array>, edges: Edge[]) => ({ + flows: [{ name: 'orphan', type: 'autolaunched', nodes, edges }], + }); + const START = { id: 'start', type: 'start', label: 'Start' }; + const END = { id: 'end', type: 'end', label: 'End' }; + const unreachable = (stack: unknown) => + lintFlowPatterns(stack as never).filter((f) => f.rule === FLOW_NODE_UNREACHABLE); + + it('refuses an action node no edge reaches, as an error that names it, why it never runs and the fix', () => { + const found = unreachable(reachFlow( + [START, { id: 'stamp', type: 'assignment', label: 'Stamp' }, END], + [{ id: 'e1', source: 'start', target: 'end' }], + )); + expect(found).toHaveLength(1); + expect(found[0]).toMatchObject({ rule: FLOW_NODE_UNREACHABLE, severity: 'error' }); + expect(found[0].where).toBe("flow 'orphan' · node 'stamp' (assignment)"); + expect(found[0].message).toMatch(/no edge path leads to this node from the start node 'start', so it never runs/); + expect(found[0].hint).toMatch(/Add an edge into it from a node that runs, or delete the node/); + }); + + it('refuses a node whose only in-edge comes from another unreached node (one finding each)', () => { + const found = unreachable(reachFlow( + [START, { id: 'a', type: 'assignment' }, { id: 'b', type: 'assignment' }, END], + [{ id: 'e1', source: 'start', target: 'end' }, { id: 'e2', source: 'a', target: 'b' }], + )); + expect(found.map((f) => f.where)).toEqual([ + "flow 'orphan' · node 'a' (assignment)", + "flow 'orphan' · node 'b' (assignment)", + ]); + }); + + it('refuses a cycle cut off from start', () => { + const found = unreachable(reachFlow( + [START, { id: 'a', type: 'assignment' }, { id: 'b', type: 'assignment' }, END], + [ + { id: 'e1', source: 'start', target: 'end' }, + { id: 'e2', source: 'a', target: 'b' }, + { id: 'e3', source: 'b', target: 'a', type: 'back' }, + ], + )); + expect(found).toHaveLength(2); + }); + + describe('does NOT flag (controls)', () => { + it('a node reached over a conditional, fault or back edge (every edge type counts)', () => { + expect(unreachable(reachFlow( + [ + START, + { id: 'call', type: 'http', config: { url: 'https://example.test' } }, + { id: 'big', type: 'assignment' }, + { id: 'recover', type: 'assignment' }, + END, + ], + [ + { id: 'e1', source: 'start', target: 'call' }, + { id: 'e2', source: 'call', target: 'big', type: 'conditional', condition: 'true' }, + { id: 'e3', source: 'call', target: 'recover', type: 'fault' }, + { id: 'e4', source: 'big', target: 'end' }, + { id: 'e5', source: 'recover', target: 'end' }, + ], + ))).toEqual([]); + }); + + it('a boundary_event node and the nodes only it leads to (reached by attachment, not by an edge)', () => { + expect(unreachable(reachFlow( + [ + START, + { id: 'task', type: 'wait', waitEventConfig: { eventType: 'timer', timerDuration: 'PT1H' } }, + { id: 'timeout', type: 'boundary_event', config: { attachedToNodeId: 'task' } }, + { id: 'escalate', type: 'assignment' }, + END, + ], + [ + { id: 'e1', source: 'start', target: 'task' }, + { id: 'e2', source: 'task', target: 'end' }, + { id: 'e3', source: 'timeout', target: 'escalate' }, + ], + ))).toEqual([]); + }); + + it('a flow with no start node (a different defect, judged elsewhere)', () => { + expect(unreachable(reachFlow( + [{ id: 'a', type: 'assignment' }, END], + [], + ))).toEqual([]); + }); + + it('a flow with a dangling edge (the flow parse refuses that first; reachability is not guessed)', () => { + expect(unreachable(reachFlow( + [START, { id: 'a', type: 'assignment' }, END], + [{ id: 'e1', source: 'start', target: 'end' }, { id: 'e2', source: 'ghost', target: 'a' }], + ))).toEqual([]); + }); + + it('nodes inside a region body (a region is a single-entry DAG, so every node in it is reached)', () => { + expect(unreachable(reachFlow( + [ + START, + { + id: 'each', type: 'loop', + config: { + collection: '{records}', iteratorVariable: 'r', + body: { + nodes: [{ id: 'x', type: 'assignment' }, { id: 'y', type: 'assignment' }], + edges: [{ id: 'b1', source: 'x', target: 'y' }], + }, + }, + }, + END, + ], + [{ id: 'e1', source: 'start', target: 'each' }, { id: 'e2', source: 'each', target: 'end' }], + ))).toEqual([]); + }); + }); + + it('is in the gating flow rule, so its error reaches os lint/build and the save and publish doors', () => { + const rule = AUTHORING_RULES.find((r) => r.name === 'lintFlowPatterns'); + expect(rule, 'the authoring rule that runs lintFlowPatterns').toBeDefined(); + expect(rule!.tier).toBe('gating'); + expect(rule!.runtimeTypes).toContain('flow'); + const served = rule!.run(reachFlow([START, { id: 'stamp', type: 'assignment' }, END], [{ id: 'e1', source: 'start', target: 'end' }]) as never); + expect(served).toContainEqual(expect.objectContaining({ rule: FLOW_NODE_UNREACHABLE, severity: 'error' })); + }); +}); diff --git a/packages/lint/src/lint-flow-patterns.ts b/packages/lint/src/lint-flow-patterns.ts index 39d76c3487f..4a65fd61218 100644 --- a/packages/lint/src/lint-flow-patterns.ts +++ b/packages/lint/src/lint-flow-patterns.ts @@ -238,6 +238,11 @@ export const FLOW_DEFAULT_EDGE_WITH_CONDITION = 'flow-default-edge-with-conditio export const FLOW_MULTIPLE_DEFAULT_EDGES = 'flow-multiple-default-edges'; /** #4414 — `config.condition` on a node whose executor never reads it. */ export const FLOW_INERT_NODE_CONDITION = 'flow-inert-node-condition'; +/** + * A top-level node no edge path from the start node reaches: it never runs. + * See {@link scanUnreachableNodes}. + */ +export const FLOW_NODE_UNREACHABLE = 'flow-node-unreachable'; /** * #15429 — a `decision` `config.mode` the spec refuses: a value outside * `'exclusive' | 'inclusive'`, or a legal `mode` beside a non-empty @@ -1862,6 +1867,83 @@ function scanTryCatchWithoutCatch( * never throws. A finding marked `severity: 'error'` fails the build, and since * #5383 it can be raised by a node inside a `loop` body too. */ +/** + * {@link FLOW_NODE_UNREACHABLE}: a top-level node no edge path from the start + * node reaches, one `error` finding per node. + * + * The engine runs a flow from its first `start` node and follows out-edges from + * there (`service-automation`'s `executeNode` → `traverseNext`); nothing else + * schedules a top-level node. So a node no path reaches (an edge forgotten while + * wiring, a node left behind when its in-edges were deleted) registers, + * publishes, reports itself configured and never runs: inert on every run, + * which is this module's bar for `error`. + * + * Why lint and not the flow parse: the save door parses `FlowSchema` on every + * save, a draft included, and the Studio designer produces this shape mid-edit + * (removing a node that has more or fewer than one in-edge and out-edge drops + * its edges without bridging them, which leaves its successors unwired). A + * parse refusal would refuse that draft. As a finding here it fails `os lint` / + * `os build`, and on the runtime authoring gate (`runtimeTypes: ['flow']`) a + * live save and a publish, while a draft is kept with it in `_diagnostics`. + * + * Scope, declared rather than discovered: + * + * - The top-level graph only (see the call site). + * - A flow with no `start` node is not judged: nothing is reached from + * nothing, and the engine refuses to run it (`FLOW_NO_START_NODE`). + * - Every edge counts, whatever its `type` or `condition`: a `fault` edge or a + * conditional edge is a path the engine can take. + * - A flow with an edge whose endpoint names no top-level node is not judged: + * the flow parse refuses that edge, and the node it was meant to reach would + * be reported a second time, for the same fault, under a cure (add an edge) + * that is not the one to apply. + * - A `boundary_event` is exempt, and so is a node reached only through one: it + * is attached to its host by `boundaryConfig.attachedToNodeId`, not entered by + * an edge, and its own contract owns its refusal. + */ +function scanUnreachableNodes( + flowName: string, + nodes: AnyRec[], + edges: AnyRec[], + findings: FlowLintFinding[], +): void { + const start = nodes.find((n) => n.type === 'start'); + if (!start || typeof start.id !== 'string') return; + const ids = new Set(nodes.map((n) => n.id).filter((id): id is string => typeof id === 'string')); + const outgoing = new Map(); + for (const e of edges) { + if (typeof e.source !== 'string' || typeof e.target !== 'string') return; + if (!ids.has(e.source) || !ids.has(e.target)) return; + const targets = outgoing.get(e.source) ?? []; + targets.push(e.target); + outgoing.set(e.source, targets); + } + const reached = new Set(); + const visit = (from: readonly string[]): void => { + const queue = [...from]; + while (queue.length > 0) { + const id = queue.shift()!; + if (reached.has(id)) continue; + reached.add(id); + queue.push(...(outgoing.get(id) ?? [])); + } + }; + visit([start.id]); + visit(nodes.filter((n) => n.type === 'boundary_event' && typeof n.id === 'string').map((n) => n.id as string)); + for (const node of nodes) { + if (typeof node.id !== 'string' || reached.has(node.id)) continue; + findings.push({ + where: `flow '${flowName}' · node '${node.id}' (${String(node.type)})`, + message: + `no edge path leads to this node from the start node '${start.id}', so it never runs: a run starts ` + + `at the start node and follows edges, and the flow still registers and reports itself as configured.`, + hint: `Add an edge into it from a node that runs, or delete the node.`, + rule: FLOW_NODE_UNREACHABLE, + severity: 'error', + }); + } +} + export function lintFlowPatterns(stack: AnyRec): FlowLintFinding[] { const findings: FlowLintFinding[] = []; for (const flow of recordsOf(stack.flows)) { @@ -1967,6 +2049,12 @@ export function lintFlowPatterns(stack: AnyRec): FlowLintFinding[] { } } + // (a5) A top-level node no edge path from the start node reaches never runs. + // Flow-level for the reason the trigger rules are: a run enters the + // top-level graph at its start node, and a region body is already + // single-entry (ADR-0031), so nothing one level down can be unreached. + scanUnreachableNodes(flowName, nodes, edges, findings); + // (b)–(e) #5383 — every graph in the flow, not just the top-level one: its // own `nodes`/`edges` plus each nested ADR-0031 region, each scanned // against ITS OWN edge list. `scope` is empty for the flow's own graph, diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index 04afb87da47..a506a03db01 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -2019,6 +2019,28 @@ const FLOW_GATE_REACH = 'the findings the stored stack already had, so a dead flow already stored never blocks another flow\'s ' + 'save; what is refused is the dead flow\'s own publish.'; +const FLOW_NODE_UNREACHABLE_EXPLANATION: RuleExplanation = { + rule: 'flow-node-unreachable', + covers: 'why an unreached node never runs and what reaches it', + paragraphs: [ + 'A flow run starts at the start node and follows edges: the engine executes a node, then the edges ' + + 'out of it. A top-level node no edge path from the start node leads to is never executed, while the ' + + 'flow still parses, registers and reports itself as configured, so nothing tells the author the step ' + + 'is dead.', + 'Every edge type counts as a path: default, conditional, fault and back. A `boundary_event` node is ' + + 'reached by attachment to its host node rather than by an edge, so it and the nodes only it leads to ' + + 'are not judged. Nodes inside a region body (`loop`, `parallel`, `try_catch` and the rest) are not ' + + 'judged here either: a region is a single-entry acyclic graph, which the region validator already ' + + 'holds, so every node in it is reached. A flow with no start node, or with an edge whose endpoint ' + + 'names no node, is not judged: the flow parse refuses those shapes first.', + 'The rule lives in lint rather than in the flow parse because the metadata save door parses every ' + + 'save, drafts included: a designer that deletes a node and leaves its successor unwired must still ' + + 'be able to save the draft. The draft stores with the finding in its diagnostics, and publish ' + + 'refuses it.', + FLOW_GATE_REACH, + ], +}; + const FLOW_TIME_RELATIVE_DESCRIPTOR_INVALID_EXPLANATION: RuleExplanation = { rule: 'flow-time-relative-descriptor-invalid', covers: 'why a refused timeRelative descriptor never binds', @@ -5824,6 +5846,7 @@ export const RULE_EXPLANATIONS: Readonly> = Obje [SHARING_RULE_RUNTIME_VARIABLE_CONDITION_EXPLANATION.rule]: SHARING_RULE_RUNTIME_VARIABLE_CONDITION_EXPLANATION, [SHARING_RULE_OBJECT_NOT_SHAREABLE_EXPLANATION.rule]: SHARING_RULE_OBJECT_NOT_SHAREABLE_EXPLANATION, [SHARING_RULE_OBJECT_CONTROLLED_BY_PARENT_EXPLANATION.rule]: SHARING_RULE_OBJECT_CONTROLLED_BY_PARENT_EXPLANATION, + [FLOW_NODE_UNREACHABLE_EXPLANATION.rule]: FLOW_NODE_UNREACHABLE_EXPLANATION, [FLOW_TIME_RELATIVE_DESCRIPTOR_INVALID_EXPLANATION.rule]: FLOW_TIME_RELATIVE_DESCRIPTOR_INVALID_EXPLANATION, [FLOW_TIME_RELATIVE_DESCRIPTOR_UNROUTABLE_EXPLANATION.rule]: FLOW_TIME_RELATIVE_DESCRIPTOR_UNROUTABLE_EXPLANATION, [FLOW_TIME_RELATIVE_OBJECT_MISMATCH_EXPLANATION.rule]: FLOW_TIME_RELATIVE_OBJECT_MISMATCH_EXPLANATION, diff --git a/packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts b/packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts index fd4f0297b07..e316d7f4f14 100644 --- a/packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts +++ b/packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts @@ -480,6 +480,24 @@ const CASES: Array<{ rule: string; type: string; item: { name: string } & Record ], }, }, + { + // A step no edge path from start reaches: it saved, read valid:true and + // PUBLISHED, and never ran. Refused by lint rather than the flow parse, + // so the draft below still saves. + rule: 'flow-node-unreachable', + type: 'flow', + item: { + name: 'orphan_step', + label: 'Orphan Step', + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'stamp', type: 'assignment', label: 'Stamp' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [{ id: 'e1', source: 'start', target: 'end' }], + }, + }, { // [#29233] A loop whose body closes on itself (body a → b → a, the // card's reproduction). It saved, read valid:true and PUBLISHED, and From 0c11e91bf154f988cb951430342abe1b0041236b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 20:39:52 +0000 Subject: [PATCH 6/9] test(lint): attach the boundary_event in the reachability control by boundaryConfig Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5oEQNYg7j3ueo5t6pV --- packages/lint/src/lint-flow-patterns.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/lint/src/lint-flow-patterns.test.ts b/packages/lint/src/lint-flow-patterns.test.ts index a028cd21b73..c393dea5d1a 100644 --- a/packages/lint/src/lint-flow-patterns.test.ts +++ b/packages/lint/src/lint-flow-patterns.test.ts @@ -3787,7 +3787,7 @@ describe('lintFlowPatterns — a node no edge path from start reaches (flow-node [ START, { id: 'task', type: 'wait', waitEventConfig: { eventType: 'timer', timerDuration: 'PT1H' } }, - { id: 'timeout', type: 'boundary_event', config: { attachedToNodeId: 'task' } }, + { id: 'timeout', type: 'boundary_event', boundaryConfig: { attachedToNodeId: 'task', eventType: 'timer' } }, { id: 'escalate', type: 'assignment' }, END, ], From 737b4ec8af070af03a1911a6a1abcd7ed3b5a44e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 21:10:36 +0000 Subject: [PATCH 7/9] test(metadata-protocol): wire the redaction flows that held a node no edge reaches The save door's flow lint now refuses a top-level node no edge path from the start node reaches, which is not the refusal these round trips are about. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5oEQNYg7j3ueo5t6pV --- .../src/protocol.metadata-redaction.test.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts b/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts index 2d60e0616c6..0f0727a7926 100644 --- a/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts +++ b/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts @@ -766,7 +766,10 @@ function relocate(served: any, secret?: string) { config: { triggerType: 'api', hookId: 'intake', ...(secret !== undefined ? { secret } : {}) }, }, ], - edges: [{ id: 'e1', source: 'begin_v2', target: 'finish' }], + // `e2` keeps the old start node reached: the save door's flow lint + // refuses a node no edge reaches (`flow-node-unreachable`), which is + // not the refusal these tests are about. + edges: [{ id: 'e1', source: 'begin_v2', target: 'finish' }, { id: 'e2', source: 'begin_v2', target: 'begin' }], }; } @@ -906,7 +909,8 @@ function storedCalloutFlow() { }, }, ], - edges: [], + // Wired: the save door's flow lint refuses a top-level node no edge reaches. + edges: [{ id: 's1', source: 'begin', target: 'each' }, { id: 's2', source: 'each', target: 'fan' }], }; } @@ -1042,6 +1046,7 @@ function storedMovesFlow() { const flow: any = storedCalloutFlow(); flow.name = 'moves'; flow.nodes.push({ id: 'call', type: 'http', label: 'Call', config: { url: 'https://c', durable: true, signingSecret: SIGNING } }); + flow.edges.push({ id: 's3', source: 'fan', target: 'call' }); return flow; } @@ -1054,6 +1059,7 @@ function movedOutOfLoop(served: any) { return { ...served, nodes: [begin, { ...each, config: { ...each.config, body: { nodes: [NOOP('body_noop')], edges: [] } } }, fan, call, perRow], + edges: [...served.edges, { id: 's4', source: 'call', target: perRow.id }], }; } @@ -1178,7 +1184,7 @@ describe('#20590 round 2 — the relocation counts nodes, not every object carry const stored = storedMovesFlow(); const incoming = movedWithEdgeTwin(redactMetadataItem('flow', stored)); const out: any = carryForwardRedactedValues('flow', incoming, stored); - expect(out.edges.map((e: any) => e.id)).toEqual(['per_row']); + expect(out.edges.map((e: any) => e.id)).toEqual(['s1', 's2', 's3', 's4', 'per_row']); expect(out.nodes[4].id).toBe('per_row'); expect(out.nodes[4].config.signingSecret).toBe(SIGNING); expect(allStrings(redactMetadataItem('flow', out))).not.toContain(SIGNING); @@ -1197,7 +1203,7 @@ describe('#20590 round 2 — the relocation counts nodes, not every object carry await protocol.saveMetaItem({ type: 'flow', name: 'moves', item: movedWithEdgeTwin(editable) }); const at = JSON.parse(Array.from(rows.values()).find((r) => r.name === 'moves' && r.state === 'active')!.metadata); - expect(at.edges.map((e: any) => e.id)).toEqual(['per_row']); + expect(at.edges.map((e: any) => e.id)).toEqual(['s1', 's2', 's3', 's4', 'per_row']); expect(at.nodes[4].config.signingSecret).toBe(SIGNING); expect(allStrings(await protocol.getMetaItem({ type: 'flow', name: 'moves' }))).not.toContain(SIGNING); }); From d3db24b18b145d587eea50ec97ad696a96ba33fa Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 22:22:04 +0000 Subject: [PATCH 8/9] test(metadata-protocol): pay the lint load at collection in the redaction test check:test-source-alias asks an edited test file that reaches the save door's dynamic lint import to import it at module top. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5oEQNYg7j3ueo5t6pV --- .../src/protocol.metadata-redaction.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts b/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts index 0f0727a7926..33049b30664 100644 --- a/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts +++ b/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts @@ -50,6 +50,11 @@ import { hasMetadataRedactor, redactMetadataItem, } from './index.js'; +// `@objectstack/lint` is reached through the save door's author-time gate, a +// dynamic `import()` (`metadata-diagnostics.ts`). Paid here, at collection, so no +// case's `testTimeout` window pays that first load +// (`scripts/check-test-source-alias.mjs`, "The source reach"). +import '@objectstack/lint'; // [#20611] Not on the package entry — the gate's half of the carry-forward, // consumed inside this package only. import { redactedPathsCarriedForward } from './metadata-redaction.js'; From 3284c658f3d3a98615ff573cc234f0e8191bc3ba Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 22:46:26 +0000 Subject: [PATCH 9/9] fix(lint): give a second start node its own cure, and call run with its context A second `start` node is unreached by construction (a run starts at the first one), so its hint says to delete the extra start rather than wire an edge into it; the os explain entry says the same. The registration test passes the `{}` context `AuthoringRule.run` takes, which check:test-typecheck asks for. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5oEQNYg7j3ueo5t6pV --- .changeset/31224-flow-node-unreachable.md | 1 + packages/lint/src/lint-flow-patterns.test.ts | 15 ++++++++++++++- packages/lint/src/lint-flow-patterns.ts | 6 +++++- packages/lint/src/rule-explanations.ts | 3 +++ 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/.changeset/31224-flow-node-unreachable.md b/.changeset/31224-flow-node-unreachable.md index 1cb73bda575..da37beb4575 100644 --- a/.changeset/31224-flow-node-unreachable.md +++ b/.changeset/31224-flow-node-unreachable.md @@ -14,6 +14,7 @@ Skills-docs: none — `skills/**` and `content/docs/**` teach only wired top-lev - **Before.** A flow whose top-level node had no edge path from the start node (`nodes: [start, stamp, end]`, `edges: [start → end]`) parsed, built, saved, published and registered, and reported itself as configured. The engine starts a run at the start node and follows edges, so `stamp` never ran, and nothing said so. - **Now.** `flow-node-unreachable` (severity `error`) names each such node: `flow 'NAME' · node 'ID' (TYPE)`, why it never runs, and the fix. A draft save still stores, with the finding in its `_diagnostics`, so a designer that deletes a node and leaves its successor unwired can keep saving; the publish refuses it. +- **A second start node.** A run starts at the first start node, so a second one is reported too; its hint says to delete the extra start rather than wire an edge into it. - **What counts as reaching a node.** Every edge type: default, conditional, fault and back. A `boundary_event` node, and the nodes only it leads to, are not judged (they are reached by attachment, not by an edge). - **Not judged.** Nodes inside a region body (a region is a single-entry acyclic graph that the region validator already holds, so all of it is reached); a flow with no start node; a flow with an edge whose endpoint names no node (the flow parse refuses that first). - **Why lint and not the flow parse.** The metadata save door parses `FlowSchema` on every save, drafts included, and the Studio designer's node delete can leave a successor unwired. A parse rule would refuse that draft save outright. diff --git a/packages/lint/src/lint-flow-patterns.test.ts b/packages/lint/src/lint-flow-patterns.test.ts index c393dea5d1a..b0694f6e65d 100644 --- a/packages/lint/src/lint-flow-patterns.test.ts +++ b/packages/lint/src/lint-flow-patterns.test.ts @@ -3750,6 +3750,19 @@ describe('lintFlowPatterns — a node no edge path from start reaches (flow-node ]); }); + it('refuses a second start node with the cure that fits it: delete the extra start', () => { + const found = unreachable(reachFlow( + [START, { id: 'start2', type: 'start' }, { id: 'b', type: 'assignment' }, END], + [{ id: 'e1', source: 'start', target: 'end' }, { id: 'e2', source: 'start2', target: 'b' }], + )); + expect(found.map((f) => f.where)).toEqual([ + "flow 'orphan' · node 'start2' (start)", + "flow 'orphan' · node 'b' (assignment)", + ]); + expect(found[0].hint).toMatch(/delete this extra start node/); + expect(found[1].hint).toMatch(/Add an edge into it from a node that runs/); + }); + it('refuses a cycle cut off from start', () => { const found = unreachable(reachFlow( [START, { id: 'a', type: 'assignment' }, { id: 'b', type: 'assignment' }, END], @@ -3839,7 +3852,7 @@ describe('lintFlowPatterns — a node no edge path from start reaches (flow-node expect(rule, 'the authoring rule that runs lintFlowPatterns').toBeDefined(); expect(rule!.tier).toBe('gating'); expect(rule!.runtimeTypes).toContain('flow'); - const served = rule!.run(reachFlow([START, { id: 'stamp', type: 'assignment' }, END], [{ id: 'e1', source: 'start', target: 'end' }]) as never); + const served = rule!.run(reachFlow([START, { id: 'stamp', type: 'assignment' }, END], [{ id: 'e1', source: 'start', target: 'end' }]) as never, {}); expect(served).toContainEqual(expect.objectContaining({ rule: FLOW_NODE_UNREACHABLE, severity: 'error' })); }); }); diff --git a/packages/lint/src/lint-flow-patterns.ts b/packages/lint/src/lint-flow-patterns.ts index 4a65fd61218..e6b6d672d4f 100644 --- a/packages/lint/src/lint-flow-patterns.ts +++ b/packages/lint/src/lint-flow-patterns.ts @@ -1937,7 +1937,11 @@ function scanUnreachableNodes( message: `no edge path leads to this node from the start node '${start.id}', so it never runs: a run starts ` + `at the start node and follows edges, and the flow still registers and reports itself as configured.`, - hint: `Add an edge into it from a node that runs, or delete the node.`, + // A second `start` node is unreached by construction (a run starts at the + // first one), and an edge into it is not the cure: remove the extra start. + hint: node.type === 'start' + ? `A run starts at the first start node only: delete this extra start node, and wire what follows it from a node that runs.` + : `Add an edge into it from a node that runs, or delete the node.`, rule: FLOW_NODE_UNREACHABLE, severity: 'error', }); diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index a506a03db01..412e1679a38 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -2027,6 +2027,9 @@ const FLOW_NODE_UNREACHABLE_EXPLANATION: RuleExplanation = { 'out of it. A top-level node no edge path from the start node leads to is never executed, while the ' + 'flow still parses, registers and reports itself as configured, so nothing tells the author the step ' + 'is dead.', + 'A run starts at the FIRST start node, so a second start node is reported too, with a different cure: ' + + 'delete the extra start node rather than wiring an edge into it, and wire what followed it from a ' + + 'node that runs.', 'Every edge type counts as a path: default, conditional, fault and back. A `boundary_event` node is ' + 'reached by attachment to its host node rather than by an edge, so it and the nodes only it leads to ' + 'are not judged. Nodes inside a region body (`loop`, `parallel`, `try_catch` and the rest) are not ' +