From 8140397c0104e2f76702a60d2d4765447d055b71 Mon Sep 17 00:00:00 2001 From: os-dev Date: Sat, 5 Sep 2026 03:01:53 +0000 Subject: [PATCH 1/3] feat(spec)!: FlowSchema refuses a flow whose edges[] declares the same id twice (#14964) A superRefine on the flow's edges[] refuses a duplicate edge id at parse time with an issue naming the id and both positions, anchored on the later edge so the formatted error points at the one to renumber. Pins: the duplicate refused, the card's control (invalid edge type) still refused, a unique-id flow accepted, the rendered message shape. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01M59rPZZFzqhfMUPFqqZTkf --- packages/spec/src/automation/flow.test.ts | 105 ++++++++++++++++++++++ packages/spec/src/automation/flow.zod.ts | 27 ++++++ 2 files changed, 132 insertions(+) diff --git a/packages/spec/src/automation/flow.test.ts b/packages/spec/src/automation/flow.test.ts index 485bd0a60f..03a4e9b19d 100644 --- a/packages/spec/src/automation/flow.test.ts +++ b/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`'); + }); +}); diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index abc5dc708f..a0d5cf2688 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/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(); + 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.', + }); + }); })); /** From a4da73eec3f62ceff8a38cb6c699ae63dd875a66 Mon Sep 17 00:00:00 2001 From: os-dev Date: Sat, 5 Sep 2026 03:10:19 +0000 Subject: [PATCH 2/3] =?UTF-8?q?chore(changeset):=20@objectstack/spec=20min?= =?UTF-8?q?or=20=E2=80=94=20duplicate=20edge=20ids=20refused=20at=20parse,?= =?UTF-8?q?=20ADR-0087=20no-migration-prescription?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01M59rPZZFzqhfMUPFqqZTkf --- .changeset/flow-edge-id-uniqueness.md | 63 +++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 .changeset/flow-edge-id-uniqueness.md diff --git a/.changeset/flow-edge-id-uniqueness.md b/.changeset/flow-edge-id-uniqueness.md new file mode 100644 index 0000000000..d2dcccb98c --- /dev/null +++ b/.changeset/flow-edge-id-uniqueness.md @@ -0,0 +1,63 @@ +--- +"@objectstack/spec": minor +--- + +feat(spec)!: `FlowSchema` refuses a flow whose `edges[]` declares the same id twice (#14964) + + + +**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. + +## FROM → TO + +```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). From 260c4d9864b34179d37135a8cb7bd6b964db8a25 Mon Sep 17 00:00:00 2001 From: os-dev Date: Sat, 5 Sep 2026 03:35:46 +0000 Subject: [PATCH 3/3] =?UTF-8?q?chore(changeset):=20drop=20the=20FROM=20?= =?UTF-8?q?=E2=86=92=20TO=20label=20=E2=80=94=20the=20remedy=20is=20a=20re?= =?UTF-8?q?number,=20not=20a=20migration=20prescription?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01M59rPZZFzqhfMUPFqqZTkf --- .changeset/flow-edge-id-uniqueness.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.changeset/flow-edge-id-uniqueness.md b/.changeset/flow-edge-id-uniqueness.md index d2dcccb98c..a69e7ad3c7 100644 --- a/.changeset/flow-edge-id-uniqueness.md +++ b/.changeset/flow-edge-id-uniqueness.md @@ -38,7 +38,8 @@ 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. -## FROM → TO +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'