Skip to content

Commit 52804cd

Browse files
claude[bot]claude
andauthored
feat(spec)!: FlowSchema refuses a flow whose edges[] declares the same id twice (#14964) (#15716)
* 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M59rPZZFzqhfMUPFqqZTkf * chore(changeset): @objectstack/spec minor — duplicate edge ids refused at parse, ADR-0087 no-migration-prescription Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M59rPZZFzqhfMUPFqqZTkf * chore(changeset): drop the FROM → TO label — the remedy is a renumber, not a migration prescription Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M59rPZZFzqhfMUPFqqZTkf --------- Co-authored-by: os-dev <noreply@anthropic.com>
1 parent b25a5fc commit 52804cd

3 files changed

Lines changed: 196 additions & 0 deletions

File tree

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
feat(spec)!: `FlowSchema` refuses a flow whose `edges[]` declares the same id twice (#14964)
6+
7+
<!-- 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. -->
8+
9+
**BREAKING** accept-set narrowing on `FlowSchema` — a flow whose `edges[]`
10+
carries two edges with the same `id` is now **refused at parse time** — by
11+
`FlowSchema.parse` / `safeParse`, `defineFlow`, and every door that validates a
12+
flow through the schema (`objectstack validate`, the runtime publish gate, a
13+
stack's `flows[]`) — where it used to parse on green. Shipped as `minor` under
14+
the repo's launch-window convention for breaking changes. Maintainer ruling
15+
2026-09-05 on #14964 (director decision batch #40, verbatim 「同意」): option
16+
A — an `error`, not a `warning`; no opt-out, no transition window.
17+
18+
Every reader of an edge id assumes the ids in a flow are unique — a designer,
19+
a BPMN export, a flow diff, any traversal that dedupes by id — and nothing
20+
enforced it. A real duplicate (`id: 'e20'` on two edges of one flow) shipped
21+
through two releases of green CI in a downstream app and was inert only
22+
because the engine keys out-edges by `source`, never by `id`: the collision is
23+
invisible until something keys on ids, and then silently wrong rather than
24+
loudly broken. The id space is hand-authored, so the next author picking a
25+
"free" id from the sequence had no way to know it was taken.
26+
27+
**What changes** (`packages/spec/src/automation/flow.zod.ts`): a `superRefine`
28+
on the flow's `edges[]`. Each later occurrence of an already-declared id raises
29+
one `custom` issue, anchored at `edges[N].id` of the *later* edge and naming
30+
both positions, so the formatted error points at the edge to renumber:
31+
32+
```text
33+
✗ 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: …
34+
```
35+
36+
**What does NOT change:** `edges[].id` keeps its name, type and describe; the
37+
node vocabulary, the edge `type` enum and every other refusal are untouched;
38+
a flow with unique edge ids (or no edges) parses exactly as before. Node ids
39+
are not covered by this change.
40+
41+
The shape that is refused, and what the author does about it — a two-edge
42+
excerpt, the later edge renumbered:
43+
44+
```ts
45+
// before — parsed on green, both edges keyed 'e20'
46+
edges: [
47+
{ id: 'e20', source: 'qualify', target: 'convert' },
48+
{ id: 'e20', source: 'convert', target: 'end' },
49+
]
50+
51+
// after — refused at parse (edges.1.id: Duplicate edge id `e20` …); renumber the later one:
52+
edges: [
53+
{ id: 'e20', source: 'qualify', target: 'convert' },
54+
{ id: 'e21', source: 'convert', target: 'end' },
55+
]
56+
```
57+
58+
**Remedy.** Renumber the later edge to an id no other edge in that flow
59+
carries; nothing else in the flow needs to move. The census over this
60+
repository found no flow to migrate, so this is a release note, not a
61+
migration: no shipped example, fixture or seed in `packages/**` or
62+
`examples/**` declares a duplicate edge id, and the pinned objectui tree
63+
carries none in its authored flows. The one known downstream instance was
64+
renumbered before this change (hotcrm PR #1571).

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

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
// and falls through without iterating — so no `iteratorVariable` is ever set
2929
// and a `{item.…}` token downstream references nothing.
3030
import { LoopConfigSchema } from './control-flow.zod';
31+
import { formatZodError } from '../shared/error-map.zod';
3132

3233
describe('FlowNodeAction', () => {
3334
it('should accept all node action types', () => {
@@ -1876,3 +1877,107 @@ describe('unknown keys are rejected, not stripped (#4001)', () => {
18761877
});
18771878
});
18781879
});
1880+
1881+
describe('FlowSchema — edge ids are unique (#14964)', () => {
1882+
// The card's probe, reproduced: two edges differing only in source/target,
1883+
// both `id: 'dup'`, parsed on green through 17.2.0. The control beside it —
1884+
// an invalid edge `type` on the SAME schema instance — is what proves the
1885+
// acceptance was a missing rule rather than a disabled validator, so it is
1886+
// pinned here too: a refactor that silences the whole edge branch would
1887+
// otherwise read as "the duplicate is still refused".
1888+
const nodes: Flow['nodes'] = [
1889+
{ id: 'start', type: 'start', label: 'Start' },
1890+
{ id: 'assign', type: 'assignment', label: 'Assign' },
1891+
{ id: 'end', type: 'end', label: 'End' },
1892+
];
1893+
const flowWith = (edges: Flow['edges']): Flow => ({
1894+
name: 'edge_id_flow',
1895+
label: 'Edge id flow',
1896+
type: 'autolaunched',
1897+
nodes,
1898+
edges,
1899+
});
1900+
const duplicate = flowWith([
1901+
{ id: 'dup', source: 'start', target: 'assign' },
1902+
{ id: 'dup', source: 'assign', target: 'end' },
1903+
]);
1904+
1905+
it('refuses a flow whose edges[] declares one id twice — the issue names the id and BOTH positions, anchored on the later edge', () => {
1906+
const result = FlowSchema.safeParse(duplicate);
1907+
expect(result.success).toBe(false);
1908+
if (result.success) return;
1909+
expect(result.error.issues).toHaveLength(1);
1910+
const [issue] = result.error.issues;
1911+
expect(issue.code).toBe('custom');
1912+
expect(issue.path).toEqual(['edges', 1, 'id']);
1913+
expect(issue.message).toContain('Duplicate edge id `dup`');
1914+
expect(issue.message).toContain('`edges[1]` reuses the id already declared by `edges[0]`');
1915+
});
1916+
1917+
it('renders through formatZodError as a line that points at the edge to renumber', () => {
1918+
const result = FlowSchema.safeParse(duplicate);
1919+
expect(result.success).toBe(false);
1920+
if (result.success) return;
1921+
const rendered = formatZodError(result.error);
1922+
expect(rendered).toContain('Validation failed (1 issue):');
1923+
expect(rendered).toContain(
1924+
'✗ edges.1.id: Duplicate edge id `dup` — `edges[1]` reuses the id already declared by `edges[0]`',
1925+
);
1926+
});
1927+
1928+
it('raises one issue per later occurrence, each naming the FIRST declaration of that id', () => {
1929+
const result = FlowSchema.safeParse(flowWith([
1930+
{ id: 'a', source: 'start', target: 'assign' },
1931+
{ id: 'b', source: 'start', target: 'end' },
1932+
{ id: 'a', source: 'assign', target: 'end' },
1933+
{ id: 'b', source: 'assign', target: 'end', type: 'fault' },
1934+
{ id: 'a', source: 'start', target: 'end', type: 'back' },
1935+
]));
1936+
expect(result.success).toBe(false);
1937+
if (result.success) return;
1938+
expect(result.error.issues.map((i) => i.path)).toEqual([
1939+
['edges', 2, 'id'],
1940+
['edges', 3, 'id'],
1941+
['edges', 4, 'id'],
1942+
]);
1943+
expect(result.error.issues[0].message).toContain('`edges[2]` reuses the id already declared by `edges[0]`');
1944+
expect(result.error.issues[1].message).toContain('`edges[3]` reuses the id already declared by `edges[1]`');
1945+
expect(result.error.issues[2].message).toContain('`edges[4]` reuses the id already declared by `edges[0]`');
1946+
});
1947+
1948+
it('still refuses the card control — an invalid edge `type` — so the refusal above is not a vacuous pass', () => {
1949+
const result = FlowSchema.safeParse(flowWith([
1950+
{ id: 'e1', source: 'start', target: 'assign' },
1951+
{ id: 'e2', source: 'assign', target: 'end', type: 'bogus' as never },
1952+
]));
1953+
expect(result.success).toBe(false);
1954+
if (result.success) return;
1955+
expect(result.error.issues.map((i) => [i.code, i.path])).toEqual([['invalid_value', ['edges', 1, 'type']]]);
1956+
expect(result.error.issues.some((i) => i.message.includes('Duplicate edge id'))).toBe(false);
1957+
});
1958+
1959+
it('accepts the same flow once the ids are unique, keeping the ids in authored order', () => {
1960+
const unique = flowWith([
1961+
{ id: 'e1', source: 'start', target: 'assign' },
1962+
{ id: 'e2', source: 'assign', target: 'end' },
1963+
]);
1964+
const result = FlowSchema.safeParse(unique);
1965+
expect(result.success).toBe(true);
1966+
if (!result.success) return;
1967+
expect(result.data.edges.map((e) => e.id)).toEqual(['e1', 'e2']);
1968+
expect(defineFlow(unique).edges.map((e) => e.id)).toEqual(['e1', 'e2']);
1969+
});
1970+
1971+
it('defineFlow refuses the duplicate with the same anchored issue', () => {
1972+
let caught: unknown;
1973+
try {
1974+
defineFlow(duplicate);
1975+
} catch (error) {
1976+
caught = error;
1977+
}
1978+
const issues = (caught as { issues?: Array<{ code: string; path: PropertyKey[]; message: string }> })?.issues;
1979+
expect(issues).toBeDefined();
1980+
expect(issues!.map((i) => [i.code, i.path])).toEqual([['custom', ['edges', 1, 'id']]]);
1981+
expect(issues![0].message).toContain('Duplicate edge id `dup`');
1982+
});
1983+
});

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -880,6 +880,33 @@ export const FlowSchema = lazySchema(() => strictObject(
880880
// ADR-0010 — runtime protection envelope (internal — set by loader).
881881
...MetadataProtectionFields,
882882

883+
}).superRefine((flow, ctx) => {
884+
// Every reader of `edges[].id` assumes the ids are unique — a designer, a
885+
// BPMN export, a flow diff, any traversal that dedupes by id — while nothing
886+
// enforced it: two edges carrying one id parsed, shipped through green CI
887+
// twice, and were inert only because the engine keys out-edges by `source`
888+
// (#14964). The id space is hand-authored, so the next author picking a
889+
// "free" id from the sequence cannot tell it is taken. Refuse the collision
890+
// here, at parse time, naming the id and BOTH positions; the issue is
891+
// anchored on the later occurrence so the formatted error points at the
892+
// edge to renumber.
893+
const firstIndexById = new Map<string, number>();
894+
flow.edges.forEach((edge, index) => {
895+
const first = firstIndexById.get(edge.id);
896+
if (first === undefined) {
897+
firstIndexById.set(edge.id, index);
898+
return;
899+
}
900+
ctx.addIssue({
901+
code: 'custom',
902+
path: ['edges', index, 'id'],
903+
message:
904+
`Duplicate edge id \`${edge.id}\` — \`edges[${index}]\` reuses the id already declared by ` +
905+
`\`edges[${first}]\`; every edge id in a flow must be unique. Renumber one of them: an ` +
906+
'edge id is the handle a designer, a BPMN export or a flow diff keys on, so a collision ' +
907+
'is silently wrong there rather than loudly broken.',
908+
});
909+
});
883910
}));
884911

885912
/**

0 commit comments

Comments
 (0)