feat(spec)!: FlowSchema refuses a flow whose top-level nodes[] declares the same id twice (#15713) (#16155)

* feat(spec)!: FlowSchema refuses a flow whose top-level nodes[] declares the same id twice

The existing superRefine on FlowSchema gains a pass over the flow's own
top-level nodes[], the same shape as the edges[] pass: a later occurrence of
an already-declared id raises one custom issue anchored on the LATER node,
naming the id and both positions, so the formatted error points at the node
to rename. Region bodies stay analyzeRegion's to judge; whether the two id
spaces are one is not decided here.

Pins: the duplicate refused (issue shape, formatZodError line, one issue per
later occurrence naming the first declaration), both card controls (missing
label, unknown key) still refused with no duplicate issue beside them, a
unique-id flow accepted in authored order, defineFlow refuses, node+edge
duplicates in one flow raise one issue each in the same shape, and the scope
boundary (a region node reusing a top-level id is outside this rule).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6HeZvT9wdSJD1ZxJb5Eno

* docs(spec): name the deferred id-space decision (#16134) at the rule's scope boundary

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6HeZvT9wdSJD1ZxJb5Eno

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: huangyiirene

.changeset/flow-node-id-uniqueness.md

@@ -0,0 +1,75 @@
+---
+"@objectstack/spec": minor
+---
+
+feat(spec)!: `FlowSchema` refuses a flow whose top-level `nodes[]` declares the same id twice (#15713)
+
+<!-- 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, and every flow whose top-level node ids are unique parses byte-identically. The only newly refused shape is two top-level nodes sharing one id — 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 repo at `1f2a02ba` (997 literal `nodes[]` arrays, 2,186 nodes, 2,011 literal ids under `packages/**` and `examples/**` including tests; 79 arrays / 256 nodes excluding tests; AST scan with a planted-duplicate control that reads 1) found zero instances, so there is no in-repo file to name. -->
+
+**BREAKING** accept-set narrowing on `FlowSchema` — a flow whose top-level
+`nodes[]` carries two nodes with the same `id` is now **refused at parse time**
+— by `FlowSchema.parse` / `safeParse`, `defineFlow`, and every door that
+validates a flow through the schema (`objectstack validate`, the runtime
+publish gate, a stack's `flows[]`) — where it used to parse on green. Shipped
+as `minor` under the repo's launch-window convention for breaking changes. The
+exact parallel of #14964 (edge ids, maintainer ruling 2026-09-05, option A —
+an `error`, not a `warning`; no opt-out, no transition window), applied to the
+other hand-authored id space in the same schema.
+
+Every edge's `source` / `target` names a node by id, and the engine's traversal
+picks out-edges by `source` — with two nodes sharing an id, every edge from
+that id is ambiguous and whichever node wins is decided by array order,
+silently. A designer, a BPMN export and a flow diff key on node ids the same
+way they key on edge ids. Only region bodies (`loop` / `try_catch` / `parallel`
+sub-graphs) were checked, by `analyzeRegion` at `registerFlow()`; the flow's
+own top-level `nodes[]` parsed with the collision intact — measured on
+`origin/main` `1f2a02ba` with two lit controls on the same schema instance (a
+node missing its `label` → refused at `nodes.1.label`; an unknown key on a node
+→ `unrecognized_keys`).
+
+**What changes** (`packages/spec/src/automation/flow.zod.ts`): the existing
+`superRefine` on `FlowSchema` gains a pass over the top-level `nodes[]`, the
+same shape as the `edges[]` pass. Each later occurrence of an already-declared
+id raises one `custom` issue, anchored at `nodes[N].id` of the *later* node and
+naming both positions, so the formatted error points at the node to rename:
+
+```text
+✗ nodes.2.id: Duplicate node id `n` — `nodes[2]` reuses the id already declared by `nodes[1]`; every node id in a flow must be unique. Rename one of them: …
+```
+
+**What does NOT change:** `nodes[].id` keeps its name, type and describe; the
+open node-type vocabulary (ADR-0018), the region rules (`analyzeRegion`) and
+every other refusal are untouched; a flow with unique top-level node ids
+parses exactly as before. The rule judges the flow's **own top-level**
+`nodes[]` only — a region body's nodes remain `analyzeRegion`'s to judge, and
+whether a region node may reuse a top-level node id (one id space or two) is a
+separate decision (#16134) this change neither takes nor pre-empts.
+
+The shape that is refused, and what the author does about it — a four-node
+excerpt, the later node renamed and its edge re-pointed:
+
+```ts
+// before — parsed on green, two nodes keyed 'n'
+nodes: [
+  { id: 'start', type: 'start', label: 'Start' },
+  { id: 'n', type: 'assignment', label: 'Assign A' },
+  { id: 'n', type: 'assignment', label: 'Assign B' },
+  { id: 'end', type: 'end', label: 'End' },
+]
+
+// after — refused at parse (nodes.2.id: Duplicate node id `n` …); rename the later one
+// and point the edges that meant it at the new id:
+nodes: [
+  { 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' },
+]
+```
+
+**Remedy.** Rename the later node to an id no other top-level node in that
+flow carries, then re-point at the new id the edges whose `source` / `target`
+meant that node; nothing else in the flow needs to move. The census over this
+repository found no flow to migrate, so this is a release note, not a
+migration: no shipped example, fixture or seed in `packages/**` or
+`examples/**` declares a duplicate top-level node id.

packages/spec/src/automation/flow.test.ts

@@ -1981,3 +1981,169 @@ describe('FlowSchema — edge ids are unique (#14964)', () => {
     expect(issues![0].message).toContain('Duplicate edge id `dup`');
   });
 });
+
+describe('FlowSchema — top-level node ids are unique (#15713)', () => {
+  // The card's probe, reproduced: a four-node flow with `nodes[1].id ===
+  // nodes[2].id === 'n'`, parsed on green (`origin/main` 1f2a02ba re-measured
+  // before this rule landed). The two controls beside it — a node missing its
+  // required `label`, and an unknown key on a node, on the SAME schema instance
+  // — are what proved the acceptance was a missing rule rather than a disabled
+  // validator, so both are pinned here too. An invalid node `type` is NOT a
+  // control on this schema: the node-type vocabulary is open by contract
+  // (ADR-0018), so that lever never fires — recorded so nobody reuses it.
+  const edges: Flow['edges'] = [
+    { id: 'e1', source: 'start', target: 'n' },
+    { id: 'e2', source: 'n', target: 'end' },
+  ];
+  const flowWith = (nodes: Flow['nodes']): Flow => ({
+    name: 'node_id_flow',
+    label: 'Node id flow',
+    type: 'autolaunched',
+    nodes,
+    edges,
+  });
+  const duplicate = flowWith([
+    { id: 'start', type: 'start', label: 'Start' },
+    { id: 'n', type: 'assignment', label: 'Assign A' },
+    { id: 'n', type: 'assignment', label: 'Assign B' },
+    { id: 'end', type: 'end', label: 'End' },
+  ]);
+
+  it('refuses a flow whose top-level nodes[] declares one id twice — the issue names the id and BOTH positions, anchored on the later node', () => {
+    const result = FlowSchema.safeParse(duplicate);
+    expect(result.success).toBe(false);
+    if (result.success) return;
+    expect(result.error.issues).toHaveLength(1);
+    const [issue] = result.error.issues;
+    expect(issue.code).toBe('custom');
+    expect(issue.path).toEqual(['nodes', 2, 'id']);
+    expect(issue.message).toContain('Duplicate node id `n`');
+    expect(issue.message).toContain('`nodes[2]` reuses the id already declared by `nodes[1]`');
+  });
+
+  it('renders through formatZodError as a line that points at the node to rename', () => {
+    const result = FlowSchema.safeParse(duplicate);
+    expect(result.success).toBe(false);
+    if (result.success) return;
+    const rendered = formatZodError(result.error);
+    expect(rendered).toContain('Validation failed (1 issue):');
+    expect(rendered).toContain(
+      '✗ nodes.2.id: Duplicate node id `n` — `nodes[2]` reuses the id already declared by `nodes[1]`',
+    );
+  });
+
+  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' },
+    ]));
+    expect(result.success).toBe(false);
+    if (result.success) return;
+    expect(result.error.issues.map((i) => i.path)).toEqual([
+      ['nodes', 2, 'id'],
+      ['nodes', 3, 'id'],
+      ['nodes', 4, 'id'],
+    ]);
+    expect(result.error.issues[0].message).toContain('`nodes[2]` reuses the id already declared by `nodes[0]`');
+    expect(result.error.issues[1].message).toContain('`nodes[3]` reuses the id already declared by `nodes[1]`');
+    expect(result.error.issues[2].message).toContain('`nodes[4]` reuses the id already declared by `nodes[0]`');
+  });
+
+  it('a duplicate node id and a duplicate edge id in one flow raise one issue each, nodes first, same shape', () => {
+    const result = FlowSchema.safeParse({
+      ...duplicate,
+      edges: [
+        { id: 'dup', source: 'start', target: 'n' },
+        { id: 'dup', source: 'n', target: 'end' },
+      ],
+    });
+    expect(result.success).toBe(false);
+    if (result.success) return;
+    expect(result.error.issues.map((i) => [i.code, i.path])).toEqual([
+      ['custom', ['nodes', 2, 'id']],
+      ['custom', ['edges', 1, 'id']],
+    ]);
+    expect(result.error.issues[0].message).toMatch(/^Duplicate node id `n` — `nodes\[2\]` reuses the id already declared by `nodes\[1\]`; every node id in a flow must be unique\. /);
+    expect(result.error.issues[1].message).toMatch(/^Duplicate edge id `dup` — `edges\[1\]` reuses the id already declared by `edges\[0\]`; every edge id in a flow must be unique\. /);
+  });
+
+  it('still refuses card control A — a node missing its required `label` — so the refusal above is not a vacuous pass', () => {
+    const result = FlowSchema.safeParse(flowWith([
+      { id: 'start', type: 'start', label: 'Start' },
+      { id: 'n', type: 'assignment' } as never,
+      { id: 'end', type: 'end', label: 'End' },
+    ]));
+    expect(result.success).toBe(false);
+    if (result.success) return;
+    expect(result.error.issues.map((i) => [i.code, i.path])).toEqual([['invalid_type', ['nodes', 1, 'label']]]);
+    expect(result.error.issues.some((i) => i.message.includes('Duplicate node id'))).toBe(false);
+  });
+
+  it('still refuses card control B — an unknown key on a node — with `unrecognized_keys`', () => {
+    const result = FlowSchema.safeParse(flowWith([
+      { id: 'start', type: 'start', label: 'Start' },
+      { id: 'n', type: 'assignment', label: 'Assign', bogusKey: 1 } as never,
+      { id: 'end', type: 'end', label: 'End' },
+    ]));
+    expect(result.success).toBe(false);
+    if (result.success) return;
+    expect(result.error.issues.map((i) => i.code)).toEqual(['unrecognized_keys']);
+    expect(result.error.issues.some((i) => i.message.includes('Duplicate node id'))).toBe(false);
+  });
+
+  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' },
+    ]);
+    const result = FlowSchema.safeParse(unique);
+    expect(result.success).toBe(true);
+    if (!result.success) return;
+    expect(result.data.nodes.map((n) => n.id)).toEqual(['start', 'n', 'n2', 'end']);
+    expect(defineFlow(unique).nodes.map((n) => n.id)).toEqual(['start', 'n', 'n2', 'end']);
+  });
+
+  it('defineFlow refuses the duplicate with the same anchored issue', () => {
+    let caught: unknown;
+    try {
+      defineFlow(duplicate);
+    } catch (error) {
+      caught = error;
+    }
+    const issues = (caught as { issues?: Array<{ code: string; path: PropertyKey[]; message: string }> })?.issues;
+    expect(issues).toBeDefined();
+    expect(issues!.map((i) => [i.code, i.path])).toEqual([['custom', ['nodes', 2, 'id']]]);
+    expect(issues![0].message).toContain('Duplicate node id `n`');
+  });
+
+  // Scope boundary, pinned so the rule cannot silently widen: it judges the
+  // flow's OWN top-level `nodes[]`. A region body (`loop.config.body.nodes`) is
+  // `analyzeRegion`'s to judge, at `registerFlow()`, and whether a region node
+  // may reuse a top-level id — one id space or two — is an open decision
+  // (#16134) that this rule neither takes nor pre-empts. This pin records
+  // today's accept set at that boundary; the decision, when taken, moves it
+  // deliberately.
+  it('judges the top-level nodes[] only — a region node reusing a top-level id is outside this rule', () => {
+    const result = FlowSchema.safeParse(flowWith([
+      { id: 'start', type: 'start', label: 'Start' },
+      {
+        id: 'n', type: 'loop', label: 'Loop',
+        config: {
+          collection: '{items}',
+          body: {
+            nodes: [{ id: 'start', type: 'assignment', label: 'Body step (reuses a top-level id)' }],
+          },
+        },
+      },
+      { id: 'end', type: 'end', label: 'End' },
+    ]));
+    expect(result.success).toBe(true);
+    if (!result.success) return;
+    expect(result.data.nodes.map((n) => n.id)).toEqual(['start', 'n', 'end']);
+  });
+});

packages/spec/src/automation/flow.zod.ts

@@ -922,15 +922,44 @@ export const FlowSchema = lazySchema(() => strictObject(
   ...MetadataProtectionFields,
 
 }).superRefine((flow, ctx) => {
-  // Every reader of `edges[].id` assumes the ids are unique — a designer, a
-  // BPMN export, a flow diff, any traversal that dedupes by id — while nothing
-  // enforced it: two edges carrying one id parsed, shipped through green CI
-  // twice, and were inert only because the engine keys out-edges by `source`
-  // (#14964). The id space is hand-authored, so the next author picking a
-  // "free" id from the sequence cannot tell it is taken. Refuse the collision
-  // here, at parse time, naming the id and BOTH positions; the issue is
-  // anchored on the later occurrence so the formatted error points at the
-  // edge to renumber.
+  // Two hand-authored id spaces, one rule each, one shape — a later occurrence
+  // of an already-declared id raises one `custom` issue anchored on the LATER
+  // element and naming BOTH positions, so the formatted error points at the
+  // one to rename.
+  //
+  // Nodes (#15713): every edge's `source` / `target` names a node by id and
+  // the engine picks out-edges by `source`, so two top-level nodes sharing an
+  // id make every edge from that id ambiguous — whichever node wins is decided
+  // by array order, silently. Only region bodies were checked (`analyzeRegion`
+  // in `control-flow.zod.ts`, at `registerFlow()`); the flow's own top-level
+  // `nodes[]` parsed with the collision intact. This pass judges the top-level
+  // array ALONE: a region's nodes are judged by `analyzeRegion`, and whether the
+  // two spaces are one is a separate decision (#16134), not taken here.
+  const firstNodeIndexById = new Map<string, number>();
+  flow.nodes.forEach((node, index) => {
+    const first = firstNodeIndexById.get(node.id);
+    if (first === undefined) {
+      firstNodeIndexById.set(node.id, index);
+      return;
+    }
+    ctx.addIssue({
+      code: 'custom',
+      path: ['nodes', index, 'id'],
+      message:
+        `Duplicate node id \`${node.id}\` — \`nodes[${index}]\` reuses the id already declared by ` +
+        `\`nodes[${first}]\`; every node id in a flow must be unique. Rename one of them: a ` +
+        "node id is the handle every edge's `source`/`target` resolves and a designer, a BPMN " +
+        'export or a flow diff keys on, so a collision routes edges by array order silently ' +
+        'rather than failing loudly.',
+    });
+  });
+
+  // Edges (#14964): every reader of `edges[].id` assumes the ids are unique —
+  // a designer, a BPMN export, a flow diff, any traversal that dedupes by id —
+  // while nothing enforced it: two edges carrying one id parsed, shipped
+  // through green CI twice, and were inert only because the engine keys
+  // out-edges by `source`. The id space is hand-authored, so the next author
+  // picking a "free" id from the sequence cannot tell it is taken.
   const firstIndexById = new Map<string, number>();
   flow.edges.forEach((edge, index) => {
     const first = firstIndexById.get(edge.id);