diff --git a/.changeset/22088-flow-edge-unresolved-or-repeated-refused.md b/.changeset/22088-flow-edge-unresolved-or-repeated-refused.md new file mode 100644 index 00000000000..bc528b3ba54 --- /dev/null +++ b/.changeset/22088-flow-edge-unresolved-or-repeated-refused.md @@ -0,0 +1,39 @@ +--- +'@objectstack/spec': minor +--- + +feat(spec)!: `FlowSchema` refuses an edge whose `source` or `target` names no node of its graph, and an edge that repeats an earlier one + +Clause-②: no (narrowing) + + + +**BREAKING**: an accept-set narrowing on a published authoring surface, shipped as `minor` under the launch-window convention for accept-set narrowings. + +**Why.** `FlowSchema` held node ids and edge ids unique, and checked nothing else about an edge. A flow whose edge named a node it no longer held (`start → node_1` over nodes `[start, end]`), or that held `start → node_1` three times, passed `FlowSchema.parse`, `objectstack validate` and the metadata save door, and publish answered 200 with `_diagnostics.valid: true`. Then it ran. The dangling edge carried the run nowhere, silently, and the repeated edge ran its target once per copy: one record update created three identical records. The Studio flow designer produced both shapes after a node was removed. + +**What is refused.** Both rules run in the region walk the node-id rule already uses, at every depth it reaches. The issue's `code` is `custom`, and each issue is anchored on the edge to fix: + +- **An endpoint that names no node of the edge's own graph**, at `edges.N.source` / `edges.N.target`. For an edge inside a `loop` / `parallel` / `try_catch` region the path is the region path, such as `nodes.N.config.body.edges.M.target`. The graph is the flow's own `nodes` for a top-level edge, and the region body's `nodes` for a region edge, because the engine resolves an endpoint there alone. So a top-level edge into a region node is refused too, and the message names the graph the node does live in. The node-id space is still one across the flow for uniqueness. +- **A repeated edge**, at `edges.N`, naming the earlier copy. Repeated means the key the engine selects on: the same `source`, `target`, `type`, `condition` (its dialect and source, so a bare CEL string and its envelope are one condition) and branch `label`. An `isDefault` copy of an unconditional edge is a repeat. A repeat is judged only between edges whose endpoints both resolve. + +That covers `FlowSchema`, `defineFlow`, `defineStack` (`STACK_SCHEMA_INVALID`, 422), `os validate`, `os compile`, an artifact's parse, `AutomationEngine.registerFlow` (which parses first) and the metadata save door (`422 INVALID_METADATA`, in draft and in publish mode). + +**What is still accepted, byte for byte.** Two nodes joined by edges the engine tells apart: different conditions, a `fault` edge beside a default one, or `approve` and `reject` branch labels into one node. Every flow whose edges all resolve in their own graph and repeat nothing. + +## FROM → TO + +| you wrote | write instead | +|:--|:--| +| an edge into a node that is not in the same `nodes` list (`target: 'node_1'`, no `node_1`) | point it at the node it was meant to reach, or delete the edge | +| a top-level edge into a node inside a region body | an edge into the region's container node; the region's own edges reach the nodes inside it | +| the same `source` → `target` edge twice, with the same `type`, `condition` and `label` | one edge. Delete the later copy; an edge meant to take its own route needs its own `condition` or branch `label` | + +**The one-line fix: delete the edge the refusal names, or re-point its endpoint.** Deleting a dangling edge changes nothing a run did, with one exception. A conditioned edge into a missing node still counted as the branch taken when its condition held, so where a flow relied on that, point the edge at a node that ends the branch. Deleting a repeated copy runs its target once per traversal instead of once per copy, which is the defect being removed. + +**Who is affected, measured.** At `aa71c4d9d`, every flow the examples ship (`app-showcase`, `app-crm`, `app-todo`: 35 flows, 55 graphs counting region bodies, 131 edges) has no dangling endpoint and no edge pair sharing a `source` and `target` at all. The flows the packages ship (the `os generate` and `os explain` templates, the new-flow seed, the `@objectstack/verify` fixture) and the platform test checklist's flow bodies are clean by reading. The CLI's golden eval corpus carries no flow. Deployed metadata and other repositories were not measured. Where such an edge already sits in a stored flow, the whole flow is refused at registration: at boot it is skipped with a warn naming it, its trigger not armed, while the flows beside it register. + +### The kit + +- **The refusal.** Two blocks in `FlowSchema`'s `superRefine`, after the edge-id rule, over `collectFlowGraphs`. No new error code: the issue is the same `custom` issue the id rules raise. +- **The ledger.** The D3 semantic entry `flow-edge-unresolved-or-repeated-refused` (protocol 18) and its step-18 rationale fragment. No key is removed, so there is no tombstone, and there is no D2 conversion: a dangling endpoint carries no intent a rewrite could recover, and dropping a copy changes how often its target runs. diff --git a/packages/metadata-protocol/src/protocol.invalid-metadata-422-face-inventory.test.ts b/packages/metadata-protocol/src/protocol.invalid-metadata-422-face-inventory.test.ts index 631bc0c460b..1a1e87cd16a 100644 --- a/packages/metadata-protocol/src/protocol.invalid-metadata-422-face-inventory.test.ts +++ b/packages/metadata-protocol/src/protocol.invalid-metadata-422-face-inventory.test.ts @@ -634,3 +634,101 @@ describe('[#21689] a hook with no `body` and no function in `handler` is refused expect(rows.size).toBe(0); }); }); + +// ═══════════════════════════════════════════════════════════════════════════ +// 9. #22088 — the `flow` door refuses an edge that names no node, and a repeat +// ═══════════════════════════════════════════════════════════════════════════ +// +// The card's reach was this door: the Studio flow designer saved a flow whose +// edges named a node it no longer held, and a flow holding one edge three +// times; the save answered 2xx, publish then promoted it, and the repeated +// edge ran its target once per copy. `FlowSchema` now refuses both, so THIS +// gate — the one `PUT /api/v1/meta/flow/:name` reaches, built here as that +// route builds it (`writeFace: 'meta-envelope'`, the actor named), and the +// draft save the designer's save-then-publish loop starts with — refuses them +// with the ADR-0112 envelope, each issue located at the edge, and stores +// nothing, so there is no draft left for publish to promote. Rides this +// file's pinned engine double, as sections 4 to 8 do. ⛔ No check of its own +// lives in `protocol.ts`: the refusal is the registered type schema's. + +async function saveFlowAsAdministrator(protocol: any, item: Record, mode?: 'draft'): Promise { + try { + return await protocol.saveMetaItem({ + type: 'flow', + name: item.name, + item, + writeFace: 'meta-envelope', + actor: 'usr_admin', + ...(mode ? { mode } : {}), + }); + } catch (e: any) { + return e; + } +} + +describe('[#22088] a flow edge that names no node, or repeats an earlier edge, is refused at the metadata door', () => { + const flowWith = (nodes: Array>, edges: Array>) => ({ + name: 'repro_edges', + label: 'Repro edges', + type: 'autolaunched', + nodes, + edges, + }); + const start = { id: 'start', type: 'start', label: 'Start' }; + const end = { id: 'end', type: 'end', label: 'End' }; + const node1 = { id: 'node_1', type: 'assignment', label: 'Node 1' }; + + it.each([ + ['publish', undefined], + ['draft', 'draft'], + ] as const)('%s mode, the dangling flow — 422 INVALID_METADATA at each endpoint naming no node, nothing stored', async (_label, mode) => { + const { protocol, rows } = makeProtocol(); + const err = await saveFlowAsAdministrator(protocol, flowWith([start, end], [ + { id: 'e1', source: 'start', target: 'node_1' }, + { id: 'e2', source: 'node_1', target: 'end' }, + ]), mode); + + expect(err).toBeInstanceOf(Error); + expect({ code: err.code, status: err.status }).toEqual({ code: 'INVALID_METADATA', status: 422 }); + const issues = err.issues as Array<{ code?: string; path?: string; message: string }>; + expect(issues.map((i) => [i.code, i.path])).toEqual([ + ['custom', 'edges.0.target'], + ['custom', 'edges.1.source'], + ]); + expect(issues[0]!.message).toContain("`target: 'node_1'`"); + expect(rows.size).toBe(0); + }); + + it.each([ + ['publish', undefined], + ['draft', 'draft'], + ] as const)('%s mode, the repeated edge — 422 INVALID_METADATA at each later copy, nothing stored', async (_label, mode) => { + const { protocol, rows } = makeProtocol(); + const err = await saveFlowAsAdministrator(protocol, flowWith([start, node1, end], [ + { id: 'e1', source: 'start', target: 'node_1' }, + { id: 'edge_1', source: 'start', target: 'node_1' }, + { id: 'edge_3', source: 'start', target: 'node_1' }, + { id: 'edge_2', source: 'node_1', target: 'end' }, + ]), mode); + + expect(err).toBeInstanceOf(Error); + expect({ code: err.code, status: err.status }).toEqual({ code: 'INVALID_METADATA', status: 422 }); + const issues = err.issues as Array<{ code?: string; path?: string; message: string }>; + expect(issues.map((i) => [i.code, i.path])).toEqual([ + ['custom', 'edges.1'], + ['custom', 'edges.2'], + ]); + expect(rows.size).toBe(0); + }); + + it('CONTROL — the same flow with one edge per hop, each into a declared node, is stored', async () => { + const { protocol, rows } = makeProtocol(); + const result = await saveFlowAsAdministrator(protocol, flowWith([start, node1, end], [ + { id: 'e1', source: 'start', target: 'node_1' }, + { id: 'edge_2', source: 'node_1', target: 'end' }, + ])); + + expect(result instanceof Error ? `${result.message} ${JSON.stringify((result as any).issues ?? [])}` : 'stored').toBe('stored'); + expect([...rows.values()].map((r) => [r.type, r.name])).toEqual([['flow', 'repro_edges']]); + }); +}); diff --git a/packages/spec/src/automation/control-flow.zod.ts b/packages/spec/src/automation/control-flow.zod.ts index 35d0c265d21..b030f9e57f0 100644 --- a/packages/spec/src/automation/control-flow.zod.ts +++ b/packages/spec/src/automation/control-flow.zod.ts @@ -434,7 +434,10 @@ export function analyzeRegion(region: { nodes: FlowNodeParsed[]; edges?: FlowEdg ids.add(n.id); } - // Edge integrity + in/out degree. + // Edge integrity + in/out degree. Within `MAX_REGION_DEPTH` a parsed flow + // never arrives here with an edge naming no node of its region: `FlowSchema` + // refuses it at parse, anchored at the edge (#22088). Beyond the ceiling, and + // for a raw-region caller (`bpmn-mapping`), these two lines are the refusal. const hasIncoming = new Set(); const hasOutgoing = new Set(); const adj = new Map(); diff --git a/packages/spec/src/automation/flow.test.ts b/packages/spec/src/automation/flow.test.ts index a85f79a30d1..fffcc87a9c4 100644 --- a/packages/spec/src/automation/flow.test.ts +++ b/packages/spec/src/automation/flow.test.ts @@ -2212,13 +2212,18 @@ describe('FlowSchema — top-level node ids are unique', () => { }); it('raises one issue per later occurrence, each naming the FIRST declaration of that id', () => { - const result = FlowSchema.safeParse(flowWith([ - { id: 'a', type: 'start', label: 'Start' }, - { id: 'b', type: 'assignment', label: 'B' }, - { id: 'a', type: 'assignment', label: 'A again' }, - { id: 'b', type: 'assignment', label: 'B again' }, - { id: 'a', type: 'end', label: 'A once more' }, - ])); + // Edges of its own (#22088): the shared `start → n → end` pair names no + // node of THIS node list, and an edge must resolve in its graph. + const result = FlowSchema.safeParse({ + ...flowWith([ + { id: 'a', type: 'start', label: 'Start' }, + { id: 'b', type: 'assignment', label: 'B' }, + { id: 'a', type: 'assignment', label: 'A again' }, + { id: 'b', type: 'assignment', label: 'B again' }, + { id: 'a', type: 'end', label: 'A once more' }, + ]), + edges: [{ id: 'e1', source: 'a', target: 'b' }], + }); expect(result.success).toBe(false); if (result.success) return; expect(result.error.issues.map((i) => i.path)).toEqual([ @@ -2572,3 +2577,190 @@ describe('FlowSchema — one node-id space across the top-level nodes[] and ever ]); }); }); + +describe('FlowSchema — an edge resolves in its own graph, and is followed once', () => { + // #22088. The card's two reproductions, both from flows the Studio designer + // saved and publish answered 200 for: a draft whose edges named a node it no + // longer held (`start → node_1`, `node_1 → end` over nodes `[start, end]`), + // and a flow holding `start → node_1` three times, which ran `node_1` once + // per edge. "Repeated" is the key the engine SELECTS on — `source`, `target`, + // `type`, `condition` (dialect + source) and branch `label`; the controls + // below pin that an edge differing in any of them is still accepted. + 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: 'repro_edges', + label: 'Repro edges', + type: 'autolaunched', + nodes, + edges, + }); + const issuesOf = (flow: Flow) => { + const result = FlowSchema.safeParse(flow); + expect(result.success).toBe(false); + if (result.success) return []; + return result.error.issues; + }; + + it('refuses the card\'s dangling draft — one issue per endpoint that names no node, anchored at that endpoint', () => { + const issues = issuesOf(flowOf([start, end], [ + { id: 'e1', source: 'start', target: 'node_1' }, + { id: 'e2', source: 'node_1', target: 'end' }, + ])); + expect(issues.map((i) => [i.code, i.path])).toEqual([ + ['custom', ['edges', 0, 'target']], + ['custom', ['edges', 1, 'source']], + ]); + expect(issues[0].message).toContain("`target: 'node_1'`"); + expect(issues[0].message).toContain("the flow's top-level graph"); + expect(issues[1].message).toContain("`source: 'node_1'`"); + }); + + it('an edge with BOTH endpoints missing is refused at both, and never also as a repeat', () => { + const issues = issuesOf(flowOf([start, end], [ + { id: 'e1', source: 'start', target: 'end' }, + { id: 'g1', source: 'ghost_a', target: 'ghost_b' }, + { id: 'g2', source: 'ghost_a', target: 'ghost_b' }, + ])); + expect(issues.map((i) => i.path)).toEqual([ + ['edges', 1, 'source'], + ['edges', 1, 'target'], + ['edges', 2, 'source'], + ['edges', 2, 'target'], + ]); + }); + + it('refuses the card\'s repeated edge — each later copy, anchored on the copy, naming the first', () => { + const issues = issuesOf(flowOf([start, step('node_1'), end], [ + { id: 'e1', source: 'start', target: 'node_1' }, + { id: 'edge_1', source: 'start', target: 'node_1' }, + { id: 'edge_3', source: 'start', target: 'node_1' }, + { id: 'edge_2', source: 'node_1', target: 'end' }, + ])); + expect(issues.map((i) => [i.code, i.path])).toEqual([ + ['custom', ['edges', 1]], + ['custom', ['edges', 2]], + ]); + for (const issue of issues) { + expect(issue.message).toContain('`start` → `node_1`'); + expect(issue.message).toContain('`edges[0]` `e1`'); + } + }); + + it('renders through formatZodError as lines that point at the edge to fix', () => { + const result = FlowSchema.safeParse(flowOf([start, end], [{ id: 'e1', source: 'start', target: 'node_1' }])); + expect(result.success).toBe(false); + if (result.success) return; + const rendered = formatZodError(result.error); + expect(rendered).toContain('Validation failed (1 issue):'); + expect(rendered).toContain("✗ edges.0.target: Edge `e1` (`edges[0]`) has `target: 'node_1'`"); + }); + + it('the same condition is one condition in either spelling — a bare CEL string and its envelope repeat each other', () => { + const issues = issuesOf(flowOf([start, end], [ + { id: 'e1', source: 'start', target: 'end', condition: 'record.amount > 1' }, + { id: 'e2', source: 'start', target: 'end', condition: { dialect: 'cel', source: 'record.amount > 1' } }, + ])); + expect(issues.map((i) => i.path)).toEqual([['edges', 1]]); + }); + + it('an `isDefault` copy of an unconditional edge is a repeat — both are taken whenever no conditioned sibling holds', () => { + const issues = issuesOf(flowOf([start, end], [ + { id: 'e1', source: 'start', target: 'end' }, + { id: 'e2', source: 'start', target: 'end', isDefault: true }, + ])); + expect(issues.map((i) => i.path)).toEqual([['edges', 1]]); + }); + + it('CONTROL — one pair of nodes joined by edges the engine tells apart (condition, fault type, branch label) is accepted', () => { + const result = FlowSchema.safeParse(flowOf([start, step('check'), end], [ + { id: 'e0', source: 'start', target: 'check' }, + { id: 'hi', source: 'check', target: 'end', condition: 'record.amount > 100' }, + { id: 'lo', source: 'check', target: 'end', condition: 'record.amount < 0' }, + { id: 'flt', source: 'check', target: 'end', type: 'fault' }, + { id: 'yes', source: 'check', target: 'end', label: 'approve' }, + { id: 'no', source: 'check', target: 'end', label: 'reject' }, + ])); + expect(result.success).toBe(true); + if (!result.success) return; + expect(result.data.edges.map((e) => e.id)).toEqual(['e0', 'hi', 'lo', 'flt', 'yes', 'no']); + }); + + const loopOf = (bodyNodes: FlowNode[], bodyEdges: FlowEdge[]): FlowNode => ({ + id: 'sweep', type: 'loop', label: 'Sweep', + config: { collection: '{items}', body: { nodes: bodyNodes, edges: bodyEdges } }, + }); + + it('a region edge resolves in its region body — an endpoint on the top-level graph is refused at the region path, naming where that node lives', () => { + const issues = issuesOf(flowOf([start, loopOf([step('b1')], [{ id: 'out', source: 'b1', target: 'end' }]), end], [ + { id: 'e1', source: 'start', target: 'sweep' }, + { id: 'e2', source: 'sweep', target: 'end' }, + ])); + expect(issues.map((i) => [i.code, i.path])).toEqual([ + ['custom', ['nodes', 1, 'config', 'body', 'edges', 0, 'target']], + ]); + expect(issues[0].message).toContain("`loop 'sweep' body`"); + expect(issues[0].message).toContain("`end` is a node of the flow's top-level graph"); + }); + + it('a top-level edge into a region node is refused — the node-id space is one for uniqueness, not for resolution', () => { + const issues = issuesOf(flowOf([start, loopOf([step('b1')], []), end], [ + { id: 'e1', source: 'start', target: 'b1' }, + { id: 'e2', source: 'start', target: 'sweep' }, + { id: 'e3', source: 'sweep', target: 'end' }, + ])); + expect(issues.map((i) => i.path)).toEqual([['edges', 0, 'target']]); + expect(issues[0].message).toContain("`b1` is a node of `loop 'sweep' body`"); + }); + + it('a repeated edge inside a region body is refused at the region path', () => { + const issues = issuesOf(flowOf([start, loopOf([step('b1'), step('b2')], [ + { id: 'be1', source: 'b1', target: 'b2' }, + { id: 'be2', source: 'b1', target: 'b2' }, + ]), end], [ + { id: 'e1', source: 'start', target: 'sweep' }, + { id: 'e2', source: 'sweep', target: 'end' }, + ])); + expect(issues.map((i) => i.path)).toEqual([['nodes', 1, 'config', 'body', 'edges', 1]]); + expect(issues[0].message).toContain("`loop 'sweep' body → edges[0]` `be1`"); + }); + + it('a region its own schema refused is judged at the AUTHORED edge index, past a non-record member', () => { + // `label` is required on every node, so this body fails `FlowRegionSchema` + // and stays raw; `FlowGraph.edges` would drop the `null` and renumber. + const issues = issuesOf(flowOf([start, { + id: 'sweep', type: 'loop', label: 'Sweep', + config: { + collection: '{items}', + body: { nodes: [{ id: 'b1', type: 'assignment' }], edges: [null, { id: 'x', source: 'b1', target: 'ghost' }] }, + }, + } as never, end], [ + { id: 'e1', source: 'start', target: 'sweep' }, + { id: 'e2', source: 'sweep', target: 'end' }, + ])); + expect(issues.map((i) => i.path)).toEqual([['nodes', 1, 'config', 'body', 'edges', 1, 'target']]); + }); + + it('CONTROL — a well-formed multi-region flow is accepted end to end, edges in authored order', () => { + const flow = flowOf([ + start, + loopOf([step('b1'), step('b2')], [{ id: 'be1', source: 'b1', target: 'b2' }]), + { + id: 'fan', type: 'parallel', label: 'Fan out', + config: { branches: [{ nodes: [step('left')] }, { nodes: [step('right')] }] }, + }, + end, + ], [ + { id: 'e1', source: 'start', target: 'sweep' }, + { id: 'e2', source: 'sweep', target: 'fan' }, + { id: 'e3', source: 'fan', target: 'end' }, + ]); + const result = FlowSchema.safeParse(flow); + expect(result.success).toBe(true); + if (!result.success) return; + expect(result.data.edges.map((e) => e.id)).toEqual(['e1', 'e2', 'e3']); + expect(() => validateControlFlow(result.data)).not.toThrow(); + expect(defineFlow(flow).edges.map((e) => e.id)).toEqual(['e1', 'e2', 'e3']); + }); +}); diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index e070c2ec6c0..6f05aaecab0 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -1548,8 +1548,148 @@ export const FlowSchema = lazySchema(() => strictObject( 'is silently wrong there rather than loudly broken.', }); }); + + // An edge resolves inside the graph that declares it, and is followed once + // (#22088). Two refusals over one walk, both anchored on the edge to fix: + // + // - ENDPOINTS. `source` and `target` name nodes of the graph the edge is + // declared in — the flow's own `nodes[]` for a top-level edge, the region + // body's for an edge inside an ADR-0031 region. The engine resolves both + // there and nowhere else: traversal looks the target up in the graph it is + // walking, and a region runs against a view of its own nodes and edges. + // So an endpoint naming no node of that graph — a typo, a node since + // removed, a node of another region — carries the run nowhere, silently. + // `analyzeRegion` refused the region half at `registerFlow`; the top-level + // half had no refusal anywhere, and a draft holding `start → node_1` with + // no `node_1` published 200 with `_diagnostics.valid: true`. The node-id + // space is ONE for uniqueness (above) but not for resolution, so a + // top-level edge into a region node is refused here as well. + // + // - REPEATED EDGES. The engine runs a target once per out-edge it selects + // (`traverseNext`), so two edges it cannot tell apart run the target + // twice — measured: three `start → node_1` edges ran one `create_record` + // three times per trigger. "Cannot tell apart" is the key the engine + // selects on, and nothing wider: the same `source` and `target`, the same + // `type` (a `fault` edge is followed only on failure), the same + // `condition` (its dialect and source, the two parts the evaluator reads) + // and the same branch `label` (a `decision` or `approval` narrows its + // out-edges to the label it selected, so `approve` and `reject` may both + // reach one node). The later copy is refused, naming the earlier one. + // + // Judged per graph at every depth `collectFlowGraphs` reaches; past + // `MAX_REGION_DEPTH` a region stays `validateControlFlow`'s, whose + // `analyzeRegion` still refuses an endpoint there (a repeat there is not + // judged). A repeat is judged only between edges whose endpoints both + // resolve: the endpoint refusal already names a dangling edge, and "runs the + // target again" is false for an edge that runs nothing. Indexed over the + // AUTHORED edge list, never `graph.edges`, which drops a non-record member + // of a raw region and would shift the anchor. + const graphs = collectFlowGraphs(flow); + const scopeByNodeId = new Map(); + for (const graph of graphs) { + for (const node of graph.nodes) { + const id: unknown = (node as { id?: unknown }).id; + if (typeof id === 'string' && !scopeByNodeId.has(id)) scopeByNodeId.set(id, graph.scope); + } + } + for (const graph of graphs) { + const nodeIds = new Set(); + for (const node of graph.nodes) { + const id: unknown = (node as { id?: unknown }).id; + if (typeof id === 'string') nodeIds.add(id); + } + const graphName = graph.scope ? `\`${graph.scope}\`` : "the flow's top-level graph"; + const at = (index: number) => (graph.scope ? `${graph.scope} → edges[${index}]` : `edges[${index}]`); + const firstIndexBySelection = new Map(); + const edges = authoredEdgesAt(flow, graph.path); + const idAt = (index: number): string => { + const id: unknown = (edges[index] as { id?: unknown }).id; + return typeof id === 'string' ? ` \`${id}\`` : ''; + }; + edges.forEach((edge, index) => { + if (typeof edge !== 'object' || edge === null || Array.isArray(edge)) return; + const { id, source, target, type, condition, label } = edge as Record; + const named = typeof id === 'string' ? `Edge \`${id}\`` : 'An edge'; + let resolves = true; + for (const [endpoint, value] of [['source', source], ['target', target]] as const) { + if (typeof value !== 'string') { + resolves = false; + continue; + } + if (nodeIds.has(value)) continue; + resolves = false; + const elsewhere = scopeByNodeId.get(value); + ctx.addIssue({ + code: 'custom', + path: [...graph.path, 'edges', index, endpoint], + message: + `${named} (\`${at(index)}\`) has \`${endpoint}: '${value}'\`, which is not a node of ${graphName}. ` + + 'An edge connects two nodes of the graph that declares it — a top-level edge two top-level ' + + 'nodes, a region edge two nodes of that region body — and the engine resolves the endpoint ' + + 'there alone, so this edge carries the run nowhere, silently. ' + + (elsewhere === undefined + ? `No node in this flow has the id \`${value}\`. ` + : `\`${value}\` is a node of ${elsewhere ? `\`${elsewhere}\`` : "the flow's top-level graph"}, ` + + 'a different graph: a node inside a region is reached through its container node, never by ' + + 'an edge from outside it. ') + + `Point \`${endpoint}\` at a node declared in that graph, or delete the edge — removing a node ` + + 'removes the edges that name it.', + }); + } + if (!resolves) return; + const selection = JSON.stringify([source, target, type ?? 'default', edgeConditionKey(condition), label ?? null]); + const first = firstIndexBySelection.get(selection); + if (first === undefined) { + firstIndexBySelection.set(selection, index); + return; + } + ctx.addIssue({ + code: 'custom', + path: [...graph.path, 'edges', index], + message: + `Repeated edge${idAt(index)} — \`${at(index)}\` connects \`${String(source)}\` → \`${String(target)}\` ` + + `exactly as \`${at(first)}\`${idAt(first)} does, with the same \`type\`, \`condition\` and \`label\`. ` + + 'The engine follows every out-edge it selects, so the copy runs ' + + `\`${String(target)}\` a second time — a node that writes a record writes it once per copy — or, ` + + 'where only one out-edge may be taken, the copy is never taken at all. Delete the repeated edge; ' + + 'an edge meant to take its own route needs its own `condition` or branch `label`.', + }); + }); + } })); +/** + * 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. + * `FlowGraph.edges` drops a non-record member of a raw region, so its indices + * can differ from the author's, and a Zod issue is anchored at the author's + * index (#22088). A hoisted `function` for the same reason + * {@link ledgerPathSegments} is one. + */ +function authoredEdgesAt(flow: unknown, path: readonly (string | number)[]): readonly unknown[] { + let holder: unknown = flow; + for (const key of path) { + if (typeof holder !== 'object' || holder === null) return []; + holder = (holder as Record)[key]; + } + const edges = typeof holder === 'object' && holder !== null ? (holder as { edges?: unknown }).edges : undefined; + return Array.isArray(edges) ? edges : []; +} + +/** + * The part of an edge `condition` the engine's evaluator reads — its dialect + * and its `source` — so two edges are told apart exactly where traversal tells + * them apart (#22088). A parsed condition is already the envelope; a raw + * region's bare string is the `cel` shorthand it would have normalized to. + */ +function edgeConditionKey(condition: unknown): unknown { + if (condition == null) return null; + if (typeof condition === 'string') return ['cel', condition]; + if (typeof condition !== 'object') return condition; + const { dialect, source } = condition as { dialect?: unknown; source?: unknown }; + return [dialect ?? null, source ?? null]; +} + /** * A ledger path as the resolver fills it in (`conditions[0].expression`, * `fields[2].visibleWhen`) → the Zod issue path segments it names diff --git a/packages/spec/src/migrations/entries/semantic/18.flow-edge-unresolved-or-repeated-refused.ts b/packages/spec/src/migrations/entries/semantic/18.flow-edge-unresolved-or-repeated-refused.ts new file mode 100644 index 00000000000..44f4714b824 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.flow-edge-unresolved-or-repeated-refused.ts @@ -0,0 +1,76 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// Two refusals in one entry because they are one walk and one remedy family: +// an edge must resolve in the graph that declares it, and must not be a copy +// the engine cannot tell from an earlier edge. Neither has a D2 conversion — +// a dangling endpoint carries no intent a rewrite could recover, and dropping +// a repeated edge changes how many times its target runs. +// +// No backticks in `surface` — build-upgrade-guide.ts renders it inside a code +// span already, and a nested backtick would close it. +export const entry: SemanticMigration = { + id: 'flow-edge-unresolved-or-repeated-refused', + surface: + 'a flow edge whose source or target is not the id of a node in the graph that declares it — ' + + 'the flow\'s own nodes for a top-level edge, the region body\'s nodes for an edge inside a ' + + 'loop, parallel or try_catch region, so a top-level edge into a region node is one of them — ' + + 'and a later edge of the same graph with the same source, target, type, condition and branch ' + + 'label as an earlier one. Reachable wherever a flow is authored or stored: defineStack flows ' + + 'sources, defineFlow, an exported stack passed to objectstack validate, a flow saved from the ' + + 'Studio flow designer after a node was removed (its edges were left behind, and a node added ' + + 'later under the reused id picked them up), and a flow row already sitting in sys_metadata', + replacement: + 'an edge whose `source` and `target` are node ids declared in the same graph as the edge: ' + + 're-point the endpoint at the node it was meant to reach, or delete the edge. Deleting a ' + + 'dangling edge changes nothing a run did, with one exception: a conditioned edge into a ' + + 'missing node still counted as the branch taken when its condition held, so a default ' + + 'sibling was passed over and, on an exclusive `decision`, the later conditioned siblings ' + + 'were skipped — where a flow relied on that, point the edge at a node that ends the branch. ' + + 'For a repeated edge, delete the later copy: the target then runs once per traversal ' + + 'instead of once per copy — a CHANGE of behaviour wherever the copies ran it more than once, ' + + 'which is the defect being removed. An edge meant to take its own route needs its own ' + + '`condition` or branch `label`', + reason: + 'The engine resolves an edge\'s endpoints in the graph that declares it — traversal looks the ' + + 'target up there, and a region runs against a view of its own nodes and edges — and runs a ' + + 'target once per out-edge it selects. `FlowSchema` held node ids and edge ids unique and ' + + 'checked neither that an edge names a node of its graph nor that it is not a copy of another, ' + + 'so a draft holding an edge into a node it no longer had, or one edge three times, passed ' + + '`FlowSchema.parse`, `objectstack validate` and the metadata save door, published with ' + + '`_diagnostics.valid: true`, and ran: the dangling edge carried the run nowhere, silently, ' + + 'and the repeated edge ran its target once per copy (one record update created three ' + + 'identical records). The parse now refuses both at every depth the region walk reaches: an ' + + 'endpoint at `edges.N.source` / `edges.N.target` (or the region path ' + + '`nodes.N.config.body.edges.M.target`), naming the missing id and, when it is a node of ' + + 'another graph, that graph; a repeated edge at `edges.N`, naming the earlier copy. Repeated ' + + 'means the key the engine selects on — `source`, `target`, `type`, `condition` (its dialect ' + + 'and source) and branch `label` — so two nodes joined by edges with different conditions, a ' + + '`fault` edge beside a default one, or `approve` and `reject` branches into one node stay ' + + 'legal. A region edge naming no node of its region was already refused at registration by ' + + 'the region analysis; the top-level half had no refusal anywhere. ' + + '⚠️ No D2 conversion: a dangling endpoint carries no intent a rewrite could recover, and ' + + 'dropping a repeated edge changes how many times its target runs. ' + + '⚠️ Where such an edge already sits the whole flow is refused: registered from the metadata ' + + 'registry or `sys_metadata` at boot it is skipped with a `warn` naming it, its trigger not ' + + 'armed, while the flows beside it register; a `defineStack` flows source throws ' + + '`StackSchemaInvalidError` for the whole stack; an artifact file is refused whole at load. ' + + 'ADR-0087, ADR-0031.', + acceptanceCriteria: + 'Grep every flow in `defineStack` flows sources, exported stacks and every flow row in ' + + '`sys_metadata` — including the `edges` of each `loop` / `parallel` / `try_catch` region ' + + 'body — for an edge whose `source` or `target` is not the `id` of a node in the `nodes` list ' + + 'beside that `edges` list, and for two edges in one `edges` list with the same `source`, ' + + '`target`, `type`, `condition` and `label`. Each refusal names the edge: `FlowSchema.parse` ' + + 'anchors a `custom` issue at `edges.N.source`, `edges.N.target` or `edges.N` (or the region ' + + 'path `nodes.N.config.body.edges.M…`), and `objectstack validate` prints the same path. For a ' + + 'dangling endpoint, point it at the node the edge was meant to reach or delete the edge; for ' + + 'a repeated edge, delete the later copy. Two proofs. (1) For a stack authored in config ' + + 'files, `objectstack validate` is clean. (2) Boot the stack and confirm each flow REGISTERS: ' + + 'no `failed to register flow` warn for it (the three boot paths spell it `[Automation] failed ' + + 'to register flow`, `[Automation] flow re-sync: failed to register flow` and `[Automation] ' + + 'cold-boot flow bind: failed to register flow`) — that warn line is the locator for a row ' + + 'that exists only in `sys_metadata`. A flow whose edges all resolve in their own graph and ' + + 'repeat nothing parses and registers byte-identically to before.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 50ef39e36ac..ebfa86a22c9 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -5631,6 +5631,22 @@ const STEP18_RATIONALE: readonly RationaleFragment[] = [ + 'it; `os migrate meta --stored` lists each one for review, and `mode: \'inclusive\'` is ' + 'the one-line fix where a node meant every branch.', }, + { + id: 'flow-edge-unresolved-or-repeated-refused', + order: 86, + text: + 'It also refuses, at parse, a flow edge that does not resolve in its own graph or that repeats ' + + 'an earlier one. An edge\'s `source` and `target` must name nodes of the graph that declares ' + + 'it — the flow\'s own nodes, or the region body\'s for an edge inside a region — because the ' + + 'engine resolves them there alone, and a dangling edge carried the run nowhere, silently; and ' + + 'an edge with the same `source`, `target`, `type`, `condition` and branch `label` as an earlier ' + + 'edge of that graph is refused, because the engine runs a target once per out-edge it selects ' + + 'and a copy ran it again. Both are judged in the region walk the node-id rule uses, so ' + + '`objectstack validate`, `registerFlow` and the metadata save door agree. No key is removed, ' + + 'so there is no tombstone, and no D2 conversion exists: a dangling endpoint carries no intent ' + + 'a rewrite could recover, and dropping a copy changes how often its target runs. Its D3 record ' + + 'is the semantic entry `flow-edge-unresolved-or-repeated-refused`.', + }, { id: 'flow-write-node-stored-metadata-target-refused', order: 74, @@ -13456,6 +13472,78 @@ const step18: MigrationStep = { + "slot phrase. A flow that boots without that warn is unaffected; every structural " + 'condition carrying a non-blank `source` parses byte-identically to before.', }, + // Two refusals in one entry because they are one walk and one remedy family: + // an edge must resolve in the graph that declares it, and must not be a copy + // the engine cannot tell from an earlier edge. Neither has a D2 conversion — + // a dangling endpoint carries no intent a rewrite could recover, and dropping + // a repeated edge changes how many times its target runs. + // + // No backticks in `surface` — build-upgrade-guide.ts renders it inside a code + // span already, and a nested backtick would close it. + { + id: 'flow-edge-unresolved-or-repeated-refused', + surface: + 'a flow edge whose source or target is not the id of a node in the graph that declares it — ' + + 'the flow\'s own nodes for a top-level edge, the region body\'s nodes for an edge inside a ' + + 'loop, parallel or try_catch region, so a top-level edge into a region node is one of them — ' + + 'and a later edge of the same graph with the same source, target, type, condition and branch ' + + 'label as an earlier one. Reachable wherever a flow is authored or stored: defineStack flows ' + + 'sources, defineFlow, an exported stack passed to objectstack validate, a flow saved from the ' + + 'Studio flow designer after a node was removed (its edges were left behind, and a node added ' + + 'later under the reused id picked them up), and a flow row already sitting in sys_metadata', + replacement: + 'an edge whose `source` and `target` are node ids declared in the same graph as the edge: ' + + 're-point the endpoint at the node it was meant to reach, or delete the edge. Deleting a ' + + 'dangling edge changes nothing a run did, with one exception: a conditioned edge into a ' + + 'missing node still counted as the branch taken when its condition held, so a default ' + + 'sibling was passed over and, on an exclusive `decision`, the later conditioned siblings ' + + 'were skipped — where a flow relied on that, point the edge at a node that ends the branch. ' + + 'For a repeated edge, delete the later copy: the target then runs once per traversal ' + + 'instead of once per copy — a CHANGE of behaviour wherever the copies ran it more than once, ' + + 'which is the defect being removed. An edge meant to take its own route needs its own ' + + '`condition` or branch `label`', + reason: + 'The engine resolves an edge\'s endpoints in the graph that declares it — traversal looks the ' + + 'target up there, and a region runs against a view of its own nodes and edges — and runs a ' + + 'target once per out-edge it selects. `FlowSchema` held node ids and edge ids unique and ' + + 'checked neither that an edge names a node of its graph nor that it is not a copy of another, ' + + 'so a draft holding an edge into a node it no longer had, or one edge three times, passed ' + + '`FlowSchema.parse`, `objectstack validate` and the metadata save door, published with ' + + '`_diagnostics.valid: true`, and ran: the dangling edge carried the run nowhere, silently, ' + + 'and the repeated edge ran its target once per copy (one record update created three ' + + 'identical records). The parse now refuses both at every depth the region walk reaches: an ' + + 'endpoint at `edges.N.source` / `edges.N.target` (or the region path ' + + '`nodes.N.config.body.edges.M.target`), naming the missing id and, when it is a node of ' + + 'another graph, that graph; a repeated edge at `edges.N`, naming the earlier copy. Repeated ' + + 'means the key the engine selects on — `source`, `target`, `type`, `condition` (its dialect ' + + 'and source) and branch `label` — so two nodes joined by edges with different conditions, a ' + + '`fault` edge beside a default one, or `approve` and `reject` branches into one node stay ' + + 'legal. A region edge naming no node of its region was already refused at registration by ' + + 'the region analysis; the top-level half had no refusal anywhere. ' + + '⚠️ No D2 conversion: a dangling endpoint carries no intent a rewrite could recover, and ' + + 'dropping a repeated edge changes how many times its target runs. ' + + '⚠️ Where such an edge already sits the whole flow is refused: registered from the metadata ' + + 'registry or `sys_metadata` at boot it is skipped with a `warn` naming it, its trigger not ' + + 'armed, while the flows beside it register; a `defineStack` flows source throws ' + + '`StackSchemaInvalidError` for the whole stack; an artifact file is refused whole at load. ' + + 'ADR-0087, ADR-0031.', + acceptanceCriteria: + 'Grep every flow in `defineStack` flows sources, exported stacks and every flow row in ' + + '`sys_metadata` — including the `edges` of each `loop` / `parallel` / `try_catch` region ' + + 'body — for an edge whose `source` or `target` is not the `id` of a node in the `nodes` list ' + + 'beside that `edges` list, and for two edges in one `edges` list with the same `source`, ' + + '`target`, `type`, `condition` and `label`. Each refusal names the edge: `FlowSchema.parse` ' + + 'anchors a `custom` issue at `edges.N.source`, `edges.N.target` or `edges.N` (or the region ' + + 'path `nodes.N.config.body.edges.M…`), and `objectstack validate` prints the same path. For a ' + + 'dangling endpoint, point it at the node the edge was meant to reach or delete the edge; for ' + + 'a repeated edge, delete the later copy. Two proofs. (1) For a stack authored in config ' + + 'files, `objectstack validate` is clean. (2) Boot the stack and confirm each flow REGISTERS: ' + + 'no `failed to register flow` warn for it (the three boot paths spell it `[Automation] failed ' + + 'to register flow`, `[Automation] flow re-sync: failed to register flow` and `[Automation] ' + + 'cold-boot flow bind: failed to register flow`) — that warn line is the locator for a row ' + + 'that exists only in `sys_metadata`. A flow whose edges all resolve in their own graph and ' + + 'repeat nothing parses and registers byte-identically to before.', + }, // ONE entry for the family, not one per node type: every member is the same // decision — a node config its executor cannot run is refused where the flow // is built, by one judge (`flowNodeConfigRefusals`), instead of registering