|
| 1 | +--- |
| 2 | +"@objectstack/spec": minor |
| 3 | +--- |
| 4 | + |
| 5 | +feat(spec)!: `FlowSchema` refuses a region node whose id is already declared elsewhere in the flow — one node-id space across the top-level `nodes[]` and every region body (#16134) |
| 6 | + |
| 7 | +<!-- adr-0087: not-required (no-migration-prescription) No authorable key is renamed, retired or re-typed: `nodes[].id` keeps its name, its type and its describe at every depth, and every flow whose node ids are unique across the whole flow parses byte-identically. The only newly refused shape is a region node (`loop.config.body`, `try_catch.config.try` / `.catch`, `parallel.config.branches[]`, at any depth the parse walks — nesting up to `MAX_REGION_DEPTH` = 32) carrying an id that a top-level node or a node in another region already declares — a collision, not a spelling — and its remedy is to rename one of the two (and re-point the edges that meant it), which is authoring intent no `objectstack migrate meta` rewrite can choose for the author. The census over this repository at `83863b2df` (AST scan of `packages/**` and `examples/**`: 972 outermost literal `nodes[]` arrays including tests, 66 excluding; 102 region arrays / 86 region nodes with a literal id, 15 / 11 excluding tests; a planted region-reuses-top-level-id control reads 1 at its planted line) found zero cross-region or region-vs-top-level collisions, so there is no in-repo file to name. --> |
| 8 | + |
| 9 | +**BREAKING** accept-set narrowing on `FlowSchema` — a flow has **one node-id |
| 10 | +space**. A node inside an ADR-0031 region body (`loop.config.body`, |
| 11 | +`try_catch.config.try` / `.catch`, each `parallel.config.branches[]`, nested to |
| 12 | +any depth the parse walks — up to `MAX_REGION_DEPTH` = 32 levels) whose `id` is |
| 13 | +already declared by a top-level node, or by a node in |
| 14 | +any other region of the same flow, is now **refused at parse time** — by |
| 15 | +`FlowSchema.parse` / `safeParse`, `defineFlow`, and every door that validates a |
| 16 | +flow through the schema (`objectstack validate`, the runtime publish gate, a |
| 17 | +stack's `flows[]`) — where it used to parse on green. Shipped as `minor` under |
| 18 | +the repo's launch-window convention for breaking changes. Maintainer ruling |
| 19 | +(director seat, decision batch #61, 2026-09-07, 「同意」): ADR-0031's |
| 20 | +"self-contained single-entry / single-exit sub-graph" describes control flow and |
| 21 | +variable scope, not id reuse; every reader that flattens a flow may key on the |
| 22 | +bare id. The ADR gains one sentence saying so in this same change. |
| 23 | + |
| 24 | +Before this change uniqueness was enforced **inside** each array — the |
| 25 | +top-level `nodes[]` by `FlowSchema` (#15713) and each region body by |
| 26 | +`analyzeRegion` at `registerFlow()` — and never **across** them: a loop-body |
| 27 | +node could carry the same `id` as a top-level node, or as a node in a sibling |
| 28 | +branch, and both rules stayed green. Every edge's `source` / `target` names a |
| 29 | +node by id, and the designer canvas, the BPMN export, a flow diff and a |
| 30 | +checkpoint's `completedNodeIds` all key on the bare id, so such a collision was |
| 31 | +silently wrong wherever a flow is flattened. |
| 32 | + |
| 33 | +**What changes** (`packages/spec/src/automation/flow.zod.ts`): the existing |
| 34 | +`superRefine` pass over `nodes[]` now walks every graph the parse reaches via |
| 35 | +`collectFlowGraphs` — the top-level graph first, then each region in document |
| 36 | +order, depth first, down to `MAX_REGION_DEPTH` (32) — keeping one map of first |
| 37 | +declarations. A later occurrence |
| 38 | +raises the same single `custom` issue as before, anchored at the later node's |
| 39 | +own `id` (inside the region, e.g. `nodes.1.config.body.nodes.0.id`) and naming |
| 40 | +both locations — a top-level index (`nodes[1]`) or a region path |
| 41 | +(`loop 'sweep' body → nodes[0]`): |
| 42 | + |
| 43 | +```text |
| 44 | +✗ nodes.1.config.body.nodes.0.id: Duplicate node id `start` — `loop 'n' body → nodes[0]` reuses the id already declared by `nodes[0]`; every node id in a flow must be unique. Rename one of them: … |
| 45 | +``` |
| 46 | + |
| 47 | +One refusal, one message shape, at every depth the parse walks: within |
| 48 | +`MAX_REGION_DEPTH` an author never sees two issues for one collision. A region |
| 49 | +nested beyond that ceiling is left raw by the parse and stays |
| 50 | +`validateControlFlow`'s, in its own line — there `analyzeRegion`'s |
| 51 | +`duplicate node id 'X'` is the only refusal of a within-region duplicate (a |
| 52 | +cross-region collision past the ceiling is not judged), and the same line |
| 53 | +guards `bpmn-mapping`'s raw-region caller, so it is kept on purpose. |
| 54 | +`collectFlowGraphs` gains a `path` field beside `scope` — the same location as |
| 55 | +a key path — so the issue can be anchored where the author wrote the node; it |
| 56 | +also now skips a non-object element in a region its own schema refused (such a |
| 57 | +region is left raw for `validateControlFlow` to name), where it used to throw a |
| 58 | +`TypeError` from inside that validator. |
| 59 | + |
| 60 | +**What does NOT change:** `nodes[].id` keeps its name, type and describe; the |
| 61 | +open node-type vocabulary (ADR-0018), the region rules (edge integrity, |
| 62 | +single-entry / single-exit, acyclicity) and every other refusal are untouched; |
| 63 | +a flow whose node ids are unique across the whole flow parses exactly as |
| 64 | +before, region nodes included, in authored order. |
| 65 | + |
| 66 | +The shape that is refused, and what the author does about it — the region node |
| 67 | +renamed, and any region edge that meant it re-pointed: |
| 68 | + |
| 69 | +```ts |
| 70 | +// before — parsed on green, `start` declared twice (top level + loop body) |
| 71 | +nodes: [ |
| 72 | + { id: 'start', type: 'start', label: 'Start' }, |
| 73 | + { id: 'sweep', type: 'loop', label: 'Sweep', config: { collection: '{items}', body: { |
| 74 | + nodes: [{ id: 'start', type: 'assignment', label: 'First step' }], |
| 75 | + } } }, |
| 76 | + { id: 'end', type: 'end', label: 'End' }, |
| 77 | +] |
| 78 | + |
| 79 | +// after — refused at parse (nodes.1.config.body.nodes.0.id: Duplicate node id `start` …); |
| 80 | +// rename the region node and point the region's edges that meant it at the new id: |
| 81 | +nodes: [ |
| 82 | + { id: 'start', type: 'start', label: 'Start' }, |
| 83 | + { id: 'sweep', type: 'loop', label: 'Sweep', config: { collection: '{items}', body: { |
| 84 | + nodes: [{ id: 'sweep_first', type: 'assignment', label: 'First step' }], |
| 85 | + } } }, |
| 86 | + { id: 'end', type: 'end', label: 'End' }, |
| 87 | +] |
| 88 | +``` |
| 89 | + |
| 90 | +**Remedy.** Rename the later node to an id nothing else in that flow carries — |
| 91 | +no top-level node, no node in any region — then re-point at the new id the |
| 92 | +edges whose `source` / `target` meant it; nothing else in the flow needs to |
| 93 | +move. The census over this repository found no flow to migrate, so this is a |
| 94 | +release note, not a migration: no shipped example, fixture or seed in |
| 95 | +`packages/**` or `examples/**` declares a region node id that collides with a |
| 96 | +top-level or another region's node id. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?v=4&size=40){kind=avatar}
0 commit comments