feat(spec)!: FlowSchema refuses a flow whose edges[] declares the same id twice (#14964)

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

@@ -0,0 +1,64 @@
+---
+"@objectstack/spec": minor
+---
+
+feat(spec)!: `FlowSchema` refuses a flow whose `edges[]` declares the same id twice (#14964)
+
+<!-- adr-0087: not-required (no-migration-prescription) No authorable key is renamed, retired or re-typed: `edges[].id` keeps its name, its type and its describe, and every flow whose edge ids are unique parses byte-identically. The only newly refused shape is two edges sharing one id — a collision, not a spelling — and its remedy is to renumber one of the two, which is authoring intent no `objectstack migrate meta` rewrite can choose for the author. The Zone-2 census over this repo (776 `edges[]` arrays, 1,098 edges under `packages/**` and `examples/**`, with a lit control) found zero instances, so there is no in-repo file to name. -->
+
+**BREAKING** accept-set narrowing on `FlowSchema` — a flow whose `edges[]`
+carries two edges 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. Maintainer ruling
+2026-09-05 on #14964 (director decision batch #40, verbatim 「同意」): option
+A — an `error`, not a `warning`; no opt-out, no transition window.
+
+Every reader of an edge id assumes the ids in a flow are unique — a designer,
+a BPMN export, a flow diff, any traversal that dedupes by id — and nothing
+enforced it. A real duplicate (`id: 'e20'` on two edges of one flow) shipped
+through two releases of green CI in a downstream app and was inert only
+because the engine keys out-edges by `source`, never by `id`: the collision is
+invisible until something keys on ids, and then silently wrong rather than
+loudly broken. The id space is hand-authored, so the next author picking a
+"free" id from the sequence had no way to know it was taken.
+
+**What changes** (`packages/spec/src/automation/flow.zod.ts`): a `superRefine`
+on the flow's `edges[]`. Each later occurrence of an already-declared id raises
+one `custom` issue, anchored at `edges[N].id` of the *later* edge and naming
+both positions, so the formatted error points at the edge to renumber:
+
+```text
+✗ edges.7.id: Duplicate edge id `e20` — `edges[7]` reuses the id already declared by `edges[3]`; every edge id in a flow must be unique. Renumber one of them: …
+```
+
+**What does NOT change:** `edges[].id` keeps its name, type and describe; the
+node vocabulary, the edge `type` enum and every other refusal are untouched;
+a flow with unique edge ids (or no edges) parses exactly as before. Node ids
+are not covered by this change.
+
+The shape that is refused, and what the author does about it — a two-edge
+excerpt, the later edge renumbered:
+
+```ts
+// before — parsed on green, both edges keyed 'e20'
+edges: [
+  { id: 'e20', source: 'qualify', target: 'convert' },
+  { id: 'e20', source: 'convert', target: 'end' },
+]
+
+// after — refused at parse (edges.1.id: Duplicate edge id `e20` …); renumber the later one:
+edges: [
+  { id: 'e20', source: 'qualify', target: 'convert' },
+  { id: 'e21', source: 'convert', target: 'end' },
+]
+```
+
+**Remedy.** Renumber the later edge to an id no other edge in that flow
+carries; 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 edge id, and the pinned objectui tree
+carries none in its authored flows. The one known downstream instance was
+renumbered before this change (hotcrm PR #1571).

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

@@ -28,6 +28,7 @@ import {
 // and falls through without iterating — so no `iteratorVariable` is ever set
 // and a `{item.…}` token downstream references nothing.
 import { LoopConfigSchema } from './control-flow.zod';
+import { formatZodError } from '../shared/error-map.zod';
 
 describe('FlowNodeAction', () => {
   it('should accept all node action types', () => {
@@ -1876,3 +1877,107 @@ describe('unknown keys are rejected, not stripped (#4001)', () => {
     });
   });
 });
+
+describe('FlowSchema — edge ids are unique (#14964)', () => {
+  // The card's probe, reproduced: two edges differing only in source/target,
+  // both `id: 'dup'`, parsed on green through 17.2.0. The control beside it —
+  // an invalid edge `type` on the SAME schema instance — is what proves the
+  // acceptance was a missing rule rather than a disabled validator, so it is
+  // pinned here too: a refactor that silences the whole edge branch would
+  // otherwise read as "the duplicate is still refused".
+  const nodes: Flow['nodes'] = [
+    { id: 'start', type: 'start', label: 'Start' },
+    { id: 'assign', type: 'assignment', label: 'Assign' },
+    { id: 'end', type: 'end', label: 'End' },
+  ];
+  const flowWith = (edges: Flow['edges']): Flow => ({
+    name: 'edge_id_flow',
+    label: 'Edge id flow',
+    type: 'autolaunched',
+    nodes,
+    edges,
+  });
+  const duplicate = flowWith([
+    { id: 'dup', source: 'start', target: 'assign' },
+    { id: 'dup', source: 'assign', target: 'end' },
+  ]);
+
+  it('refuses a flow whose edges[] declares one id twice — the issue names the id and BOTH positions, anchored on the later edge', () => {
+    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(['edges', 1, 'id']);
+    expect(issue.message).toContain('Duplicate edge id `dup`');
+    expect(issue.message).toContain('`edges[1]` reuses the id already declared by `edges[0]`');
+  });
+
+  it('renders through formatZodError as a line that points at the edge to renumber', () => {
+    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(
+      '✗ edges.1.id: Duplicate edge id `dup` — `edges[1]` reuses the id already declared by `edges[0]`',
+    );
+  });
+
+  it('raises one issue per later occurrence, each naming the FIRST declaration of that id', () => {
+    const result = FlowSchema.safeParse(flowWith([
+      { id: 'a', source: 'start', target: 'assign' },
+      { id: 'b', source: 'start', target: 'end' },
+      { id: 'a', source: 'assign', target: 'end' },
+      { id: 'b', source: 'assign', target: 'end', type: 'fault' },
+      { id: 'a', source: 'start', target: 'end', type: 'back' },
+    ]));
+    expect(result.success).toBe(false);
+    if (result.success) return;
+    expect(result.error.issues.map((i) => i.path)).toEqual([
+      ['edges', 2, 'id'],
+      ['edges', 3, 'id'],
+      ['edges', 4, 'id'],
+    ]);
+    expect(result.error.issues[0].message).toContain('`edges[2]` reuses the id already declared by `edges[0]`');
+    expect(result.error.issues[1].message).toContain('`edges[3]` reuses the id already declared by `edges[1]`');
+    expect(result.error.issues[2].message).toContain('`edges[4]` reuses the id already declared by `edges[0]`');
+  });
+
+  it('still refuses the card control — an invalid edge `type` — so the refusal above is not a vacuous pass', () => {
+    const result = FlowSchema.safeParse(flowWith([
+      { id: 'e1', source: 'start', target: 'assign' },
+      { id: 'e2', source: 'assign', target: 'end', type: 'bogus' as never },
+    ]));
+    expect(result.success).toBe(false);
+    if (result.success) return;
+    expect(result.error.issues.map((i) => [i.code, i.path])).toEqual([['invalid_value', ['edges', 1, 'type']]]);
+    expect(result.error.issues.some((i) => i.message.includes('Duplicate edge id'))).toBe(false);
+  });
+
+  it('accepts the same flow once the ids are unique, keeping the ids in authored order', () => {
+    const unique = flowWith([
+      { id: 'e1', source: 'start', target: 'assign' },
+      { id: 'e2', source: 'assign', target: 'end' },
+    ]);
+    const result = FlowSchema.safeParse(unique);
+    expect(result.success).toBe(true);
+    if (!result.success) return;
+    expect(result.data.edges.map((e) => e.id)).toEqual(['e1', 'e2']);
+    expect(defineFlow(unique).edges.map((e) => e.id)).toEqual(['e1', 'e2']);
+  });
+
+  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', ['edges', 1, 'id']]]);
+    expect(issues![0].message).toContain('Duplicate edge id `dup`');
+  });
+});

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

@@ -880,6 +880,33 @@ export const FlowSchema = lazySchema(() => strictObject(
   // ADR-0010 — runtime protection envelope (internal — set by loader).
   ...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.
+  const firstIndexById = new Map<string, number>();
+  flow.edges.forEach((edge, index) => {
+    const first = firstIndexById.get(edge.id);
+    if (first === undefined) {
+      firstIndexById.set(edge.id, index);
+      return;
+    }
+    ctx.addIssue({
+      code: 'custom',
+      path: ['edges', index, 'id'],
+      message:
+        `Duplicate edge id \`${edge.id}\` — \`edges[${index}]\` reuses the id already declared by ` +
+        `\`edges[${first}]\`; every edge id in a flow must be unique. Renumber one of them: an ` +
+        'edge id is the handle a designer, a BPMN export or a flow diff keys on, so a collision ' +
+        'is silently wrong there rather than loudly broken.',
+    });
+  });
 }));
 
 /**