feat(spec)!: one node-id space — FlowSchema refuses a region node whose id is declared elsewhere in the flow (#16134)

[claude]

Fixes #16134

Clause-②: yes — this narrows a published accept set (FlowSchema), dispatched at CONTRACT_REVIEW_TIER; carries needs:contract-review. Governed (docs/adr/**): draft, human merge.

Ruling executed (comment 5563453565, director seat, decision batch #61, 2026-09-07, maintainer 「同意」)

A flow's node ids are unique across the whole flow: top-level nodes[] and every region body (loop / try_catch / parallel, at every depth) share one id space, and a collision is refused at parse. ADR-0031's "self-contained single-entry / single-exit sub-graph" describes control flow and variable scope, not id reuse; the ADR gains one sentence saying so. Every reader that flattens a flow may key on the bare id.

What changes

• packages/spec/src/automation/flow.zod.ts — the superRefine node pass (FlowSchema accepts a flow whose top-level nodes[] declares the same id twice — measured with two lit controls; only region nodes are checked (analyzeRegion) #15713) now walks every graph the parse reaches via collectFlowGraphs — top-level first, then each region in document order, depth first, down to MAX_REGION_DEPTH (32) — with one map of first declarations. A later occurrence raises the same single custom issue, anchored at the later node's own id inside its region (nodes.1.config.body.nodes.0.id) and naming both locations as a top-level index (nodes[1]) or a region path (loop 'sweep' body → nodes[0]). One refusal, one message shape.
• packages/spec/src/automation/control-flow.zod.ts — collectFlowGraphs gains path beside scope (the key path to each graph, so the issue is anchored where the author wrote the node — the "equivalent" the ruling allows, additive on the export), and skips a non-object element of a region its own schema refused. analyzeRegion is untouched except for a comment: its duplicate node id 'X' line stays as an invariant for direct raw-region callers (bpmn-mapping), unreachable through the schema.
• docs/adr/0031-… — the one sentence.
• .changeset/flow-one-node-id-space.md — @objectstack/spec minor, feat(spec)!:, ADR-0087 not-required (no-migration-prescription); states the narrowing.
• Pins moved / added — see Tests.

Where the single refusal lives, and why

FlowSchema.superRefine, not analyzeRegion. Every door an author reaches parses first (FlowSchema.parse / safeParse, defineFlow, objectstack validate, registerFlow() = parse → detectCycles → validateControlFlow), so at every depth the parse walks — collectFlowGraphs descends to MAX_REGION_DEPTH = 32, the ceiling parseFlowNodeRegions shares — a flow with a collision never reaches analyzeRegion, and a flow that parses never carries one: an author sees exactly one issue for one collision (pinned, including the within-one-region case). Beyond the ceiling (nesting 33+, measured by contract review and now pinned) the region is left raw, the parse accepts, and validateControlFlow refuses a within-region duplicate in analyzeRegion's own lowercase line — the base tree behaves identically there, and a cross-region collision past the ceiling is not judged by either tree. That line is therefore kept on purpose: past the ceiling it is the only refusal of a within-region duplicate (deleting it would let the degree maps de-duplicate the collision silently), and it also guards bpmn-mapping.regionEntryExit's raw-region call.

Measurements

AST census, re-run (scratchpad census.mjs, TypeScript compiler API over literal nodes: […] arrays; ids read only where they are string literals) — tree objectstack-ai/objectstack at 83863b2df (this branch's base; the diff adds no literal fixture), roots packages/** + examples/**, 6,067 files:

• outermost nodes[] arrays 972 (66 excluding tests); with at least one region nodes[] 62 (6); region arrays 102 (15); region nodes with a literal id 86 (11); top-level nodes with a literal id 2,078.
• region id colliding with a top-level id of the same outer array: 0 (0 excluding tests). Region-vs-sibling-region: 0. Top-vs-top and within-region (already-refused classes): 0.
• control on the same instrument: a planted fixture (a loop-body node reusing the top-level start) scanned alongside reads 1, kind region-vs-top, at the planted line 7. Bare run exit 0, control run exit 0.
• also scanned, beyond the card: the objectui checkout at 61e649a (its branch claude/pm-dispatch-spec-2d08tz — NOT the pinned .objectui-sha), roots packages,apps,examples, 4,211 files: 74 outermost (4), 12 region arrays (5), cross-namespace 0, control 1; two top-vs-top hits in packages/app-shell/src/views/metadata-admin/previews/flow-problems.test.ts, a preview fixture that declares a duplicate top-level id on purpose — FlowSchema accepts a flow whose top-level nodes[] declares the same id twice — measured with two lit controls; only region nodes are checked (analyzeRegion) #15713's class, not this rule's.
• not covered by the scan, and what covered it instead: call-assembled fixtures (flowWith([…]), helper-built regions), code-generated flows, the hotcrm tree (absent from this container). The dynamic reading over the assembled surface is the test suites, and the first targeted run found 6 such collisions in one file — packages/spec/src/automation/region-normalization.test.ts, whose gatedRegion() / loopWith() helpers reused gate / write / loop across sibling branches, try + catch, a loop nested three deep, and a self-referential region. Triaged as "change the spelling" (they pin normalization, not id reuse): distinct ids per region; the self-referential termination pin now reads "a bounded ZodError, never a RangeError". After that the full spec suite and the three consumer suites below are green, so no other assembled fixture in those suites collides. The changeset states the narrowing and names no in-repo shipped flow, because none is refused.

Tests

All heavy runs through scripts/pm/os-verify-lock.sh (slot issue-16134); exit codes captured before any pipe; wall-clock figures are shared-box readings. Code tree 38c0589cf unless stated; the later c07ea3249 adds only content/docs/automation/flows.mdx (docs families re-measured there, below).

• pnpm --filter @objectstack/spec build → build-exit=0; the dist carries the change (readonly path in dist/automation/index.d.ts: 1 hit; firstNodeLocationById in 20 dist files).
• pnpm --filter @objectstack/spec test (vitest project local) → Test Files 465 passed (465) · Tests 12966 passed (12966), test-exit=0.
• pnpm --filter @objectstack/spec typecheck → typecheck-exit=0 (tsc --noEmit, check:scripts-typecheck, check:test-typecheck: OK). Lock: VERDICT command-exit 0 · held the lock 530s (8m50s) · waited 157s.
• Consumers, against that dist after turbo run build of their 31-package dependency closure (29 tasks, exit 0): @objectstack/service-automation vitest run → Test Files 126 passed (126) · Tests 1489 passed; @objectstack/lint → 102 passed (102) · 3569 passed; @objectstack/runtime --project local src/domains/automation → 18 passed (18) · 347 passed. Lock: VERDICT command-exit 0 · held 536s · waited 353s. A first attempt without the closure built was NOT MEASURED (Failed to resolve entry for package "@objectstack/core") and is not counted. Declared narrowing: runtime is run for its automation domain only, the runtime tests that parse flows; its other test files are CI's.
• Targeted first run, before the fixture triage: flow.test.ts, region-normalization.test.ts, control-flow.test.ts, flow-region-cycle.test.ts → 179 passed | 6 failed — the 6 are the assembled-fixture collisions above, every one a Duplicate node id issue (the dynamic census reading).
• pnpm --filter @objectstack/spec check:generated → ✓ All 15 generated artifacts are up to date (check:api-surface ✓ — it compares export listings and signatures, so the additive FlowGraph.path field regenerates nothing).
• Gate derivation node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack: 81 families. Batch at 38c0589cf: 78 exit 0, 3 exit 3 (prerequisite refusals): check:doc-formula-expressions → re-run after the closure build, exit 0; check:dual-build-cjs-loads → needs all 82 package dists, NOT MEASURED locally — CI's Build Core; check:type-check-debt → after building its last three prerequisites, tsc's re-measure OOMs at the 4096 MB heap (FATAL ERROR: … heap out of memory after 154 s) — re-run at a 6144 MB heap (reason: the 4096 MB run OOMed; spec's own DTS build already uses 6144 on this box) → --re-measure: OK — 5 ledger entries re-measured in 78.6s, 55 raw tsc errors total, none above its recorded number, exit 0. Reconciliation --ran: 81 derived, 81 run, 0 NOT-MEASURED, 0 UNRUN (the deriver counts runs, not verdicts; the verdict-less ones are the two just named). pnpm check:nul-bytes OK (8,296 files); pnpm check:adr-anchors OK.
• STALE TREE, measured rather than merged: the derivation is 9 commits behind origin/main (d4401f75b) and 3 derivation files changed there. No file in that delta overlaps this diff. scripts/sdui-manifest.record.json is the objectui pin bump (sha and date only; this diff cites no objectui read-point). origin/main's scripts/doc-authoring-prose-id.baseline.json laid over this tree reds only on packages/rest/src/rest-route-ledger.ts (an id fix(rest): the dispatcher's /packages domain is the one implementation of the package read and delete routes (#14503) #16628 removed on main; this diff does not touch the file). origin/main's scripts/check-undeclared-dep-imports.mjs laid over this tree reds only on packages/rest/src/package-routes.ts:44 (a file fix(rest): the dispatcher's /packages domain is the one implementation of the package read and delete routes (#14503) #16628 changed on main; not in this diff). Both swaps restored to HEAD (porcelain empty). No merge of origin/main was made; CI runs the PR on the merge ref.

Ablation

Scratchpad ablation.sh, run from the committed state 38c0589cf; flow.test.ts imports ./flow.zod (src, same package — no dist on the resolution path, so no build leg); trap restore EXIT INT TERM with absolute paths.

• Mutation: for (const graph of collectFlowGraphs(flow)) { → for (const graph of collectFlowGraphs(flow).slice(0, 1)) { — walk the top-level graph only, i.e. the pre-ruling rule. On-disk proof: anchor count 1 → 0, mutant count 0 → 1, blob 6599fc30… → 26512b13….
• Predicted direction: red. Observed: vitest run src/automation/flow.test.ts → Tests 9 failed | 112 passed (121), exit 1. The 9 red are exactly the moved FlowSchema accepts a flow whose top-level nodes[] declares the same id twice — measured with two lit controls; only region nodes are checked (analyzeRegion) #15713 boundary pin and the eight Decision: do a flow's top-level nodes[] and its region bodies (loop / try_catch / parallel) share ONE node-id space, or two? — uniqueness is now enforced inside each, never across #16134 region pins; the 112 green include every FlowSchema accepts a flow whose top-level nodes[] declares the same id twice — measured with two lit controls; only region nodes are checked (analyzeRegion) #15713 top-level pin, the non-object-guard pin and the accept-side pin (neither depends on the region half).
• Restore: git checkout HEAD -- ABS_PATH; git diff HEAD 0 lines; git hash-object equals the HEAD blob 6599fc30…; porcelain empty.

Docs drift (rows handed over by the PM)

• content/docs/automation/flows.mdx (hand-written authoring guide) — falsified, fixed in this PR (c07ea3249): the Node Structure table described id as a "Unique node identifier" with no scope, and the ADR-0031 section called a region "self-contained" with no id caveat — the two places an author would now meet the parse refusal unwarned. Both now state the one id space and the parse refusal. Docs gate families for that path (38 commands, derived) at c07ea3249: 0 nonzero; --ran reconciliation on the new head: all derived families accounted for.
• content/docs/releases/v17.mdx — read-only, not falsified: its only ADR-0031 line says regions are walked by conversions, validators and lint; it states nothing about node-id scope or reuse. Not edited; nothing for a docs-only card.

Census falsification, stated plainly

The card claimed 0 in-repo collisions, measured over literal nodes: […] arrays only, and declared call-assembled flows (flowWith([…]), helper-built regions) as uncovered. Re-run on the same instrument, the zero holds (above). Going into the declared blind spot, the first targeted test run found 6 in-repo flows the rule refuses — all in packages/spec/src/automation/region-normalization.test.ts, assembled by gatedRegion() / loopWith(): sibling parallel branches both carrying gate / write, try and catch both carrying them, a loop nested three deep reusing the outer loop's loop id, and a self-referential region. They were given distinct ids rather than made to pin the collision because they pin normalization (condition envelopes, defaults, strictness at depth), not id reuse; pinning the collision belongs in flow.test.ts, where it now is.

Behavioural consequence, not a test edit: a hand-built self-referential region (an object cycle — unreachable from parsed JSON) used to parse: the depth ceiling stopped the descent and nothing else objected. Under one id space the container's own id recurs as a body node at every depth, so FlowSchema.parse now refuses it with a bounded ZodError (one Duplicate node id issue per depth, fewer than 64) instead of returning. The termination pin reads exactly that: a ZodError, never a RangeError.

Patch round (contract review, textual)

Review measured the "every depth" claim as one nesting level too strong: the parse walk judges nesting 0–32; at nesting 33 safeParse succeeds and validateControlFlow throws … loop 'l32' body: duplicate node id 'dup' (base tree identical). Fixed in 2baa5d480 + 64c41f20c, text only: the two code comments (control-flow.zod.ts analyzeRegion, flow.zod.ts superRefine), the changeset paragraph that ships in CHANGELOG.md (plus its three "any depth" qualifiers), and this body. One test added, JSON-round-tripped: nesting 32 refused at parse (one Duplicate node id issue), nesting 33 accepted by the parse and refused by validateControlFlow in analyzeRegion's line (not a TypeError, not the parse shape), nesting 33 with unique ids — chained u1 → u2, so only the ids differ — accepted end to end (its first spelling left the control unconnected, which analyzeRegion refuses as two entries / two exits; 64c41f20c chains it). Verification at 64c41f20c under the lock: flow.test.ts + region-normalization.test.ts + control-flow.test.ts → Test Files 3 passed (3) · Tests 184 passed (184), pnpm --filter @objectstack/spec typecheck exit 0, VERDICT command-exit 0 · held 68s; lock-free on the same head: check:adr-0087-registration --base origin/main, check-changeset-no-major, check-empty-changeset, check:changeset-gate-self-tests, check:doc-authoring, check:nul-bytes, check:cross-package-test-inputs, check:test-source-alias, check:dispatcher-error-vocabulary, check:type-check-coverage, check-comment-mask-adoption, check-keyed-text-bounds all exit 0; --ran reconciliation 102 derived, 102 run, 0 UNRUN. Declared narrowing: comments, a changeset and one test file changed, so the full spec suite and the consumer suites were not re-run past 38c0589cf. Labels: the size-labeler's whole-set write on the 2baa5d480 push (size/m → size/l, actor os-zhuang, 09:04:32Z) stripped needs:contract-review from this PR; re-hung additively at 09:07:34Z and read back (documentation,size/l,tests,tooling,needs:contract-review). The card's carrier was removed by the seat's own transition write at 09:04:30Z (pm:dispatched → pm:awaiting-maintainer), so check-clause2-carriers --pair 16825 now reads exit 4 / C1 (card bare) — left to the seat. MAX_REGION_DEPTH untouched; validateControlFlow untouched. Not touched, per review: the generated content/docs/references/automation/control-flow.mdx (seat files a docs-only card); ADR-0031 and flows.mdx keep the ruling's wording (the contract), the ceiling being an implementation seam documented in code and changeset.

验收备注

• analyzeRegion's duplicate node id 'X' line — a second shape in the code — is kept deliberately: within MAX_REGION_DEPTH it is unreachable from any door that parses, and past the ceiling it is the only refusal of a within-region duplicate (rationale above). Contract review ruled the deletion I had offered OUT; withdrawn.
• Fixed in this PR, declared: collectFlowGraphs threw TypeError: Cannot read properties of null (reading 'config') on a non-object element of a region its own schema had refused (probe on origin/main 83863b2df: FlowSchema.safeParse succeeded, then collectFlowGraphs and validateControlFlow both threw). The walk now runs at parse, where that throw would escape safeParse, so the guard is part of this change; validateControlFlow now reports loop 'n' body: invalid region — … for that input (pinned).
• noted, not filed: bpmn-mapping.regionEntryExit reads only entryId / exitId from analyzeRegion, so a raw region whose errors carry only a duplicate id passes through; every parsed flow is refused earlier by this rule. 承接者:无.
• noted, not filed: objectui flow-problems.test.ts declares duplicate top-level ids on purpose (a preview fixture) — FlowSchema accepts a flow whose top-level nodes[] declares the same id twice — measured with two lit controls; only region nodes are checked (analyzeRegion) #15713's class, no action. 承接者:无.
• FlowGraph.path is the one additive export change (an interface field); check:api-surface is unchanged because it compares listings and signatures, not .d.ts bytes.
• Changeset: @objectstack/spec minor, feat(spec)!:, ADR-0087 not-required (no-migration-prescription); check:adr-0087-registration --base origin/main exit 0 in the gate batch.
• Clause-② carriers: check-clause2-carriers --pair 16825 read exit 4 (C1: label on the PR, not on the card) right after the PR opened; the ruling makes it a dual carrier (「两边都挂好」), so needs:contract-review was added to Decision: do a flow's top-level nodes[] and its region bodies (loop / try_catch / parallel) share ONE node-id space, or two? — uniqueness is now enforced inside each, never across #16134 additively as well (read back: enhancement,pm:dispatched,domain:spec,needs:contract-review,priority:p3) and the predicate now reads exit 0 — both carriers agree.

维护者速读(草稿)

改了什么:流程(flow)里所有节点 id 现在是一个命名空间:顶层 nodes[] 和任何深度的区域体(loop 的 body、try_catch 的 try/catch、parallel 的每个分支)不能再出现相同的节点 id。冲突在解析(parse)时就被拒绝,错误信息只有一条、一种形状,同时指出两个位置(顶层序号或区域路径)。ADR-0031 加了一句话说明"自包含子图"指的是控制流和变量作用域,不是 id 可以重复。

为什么改:每条边的 source/target、设计器画布、BPMN 导出、流程 diff、检查点里的 completedNodeIds 都按裸 id 索引节点。以前顶层和每个区域各自查重、互不相查,一个区域节点可以和顶层节点同 id,这些读者会静默指错节点。决策批次 #61 裁定"一个 id 空间",本 PR 落地。

风险与代价(含回滚):这是一次接受集收窄(Clause-②):以前能解析的"区域节点复用顶层 id"的流程现在会被拒绝。仓库内 AST 普查(字面量数组)为零冲突;但普查看不见用辅助函数拼装的 fixture——本仓 packages/spec 的一个测试文件就有这种拼装冲突,本 PR 已按"改拼写"处置。下游应用如有同类拼装流程,升级后会在解析时报错,修法是给冲突节点改名并改指向它的边。回滚:revert 本 PR 即可,无数据迁移。

席位意见:(留空)

你要做的:确认裁决执行无误后人工合并(docs/adr/** 为受管面,不走队列)。

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 4 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

• content/docs/automation/flows.mdx (via FlowSchema (symbol, a top-level const))

⛔ 1 release-owned page(s) also name something this change touched. These are read-only:

• content/docs/releases/v17.mdx (via FlowSchema (symbol, a top-level const))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see

• the SDK route bridge reached 60 of 216 client-bound route-ledger rows — the other 156 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 156: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 130 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 7f96e1417e01d011884272b28278b6b400521415 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from cf821dc6d14583d66a2f5803c025ab7cb3bf14b2 — the merge of head 64c41f20c80b6d8cbe93419b088071cf3e311466 into base 7f96e1417e01d011884272b28278b6b400521415, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin cf821dc6d14583d66a2f5803c025ab7cb3bf14b2 && git checkout cf821dc6d14583d66a2f5803c025ab7cb3bf14b2
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7f96e1417e01d011884272b28278b6b400521415 64c41f20c80b6d8cbe93419b088071cf3e311466 && git checkout -B drift-repro 7f96e1417e01d011884272b28278b6b400521415 && git merge --no-ff 64c41f20c80b6d8cbe93419b088071cf3e311466

node scripts/docs-audit/affected-docs.mjs --json 7f96e1417e01d011884272b28278b6b400521415

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 7f96e1417e01d011884272b28278b6b400521415 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

[os-zhuang]

Contract review (CONTRACT_REVIEW_TIER, isolated seat) — PR #16825 @ c07ea3249

Verdict: PASS WITH FINDINGS (CI-conditional — 15 check runs still in_progress at review time, 0 failed; the verdict stands if they finish green, and is void on any red).

Ruling implemented: yes. Ruling read from card #16134 comment 5563453565 (## Ruling recorded — one node-id space (director seat, decision batch #61, 2026-09-07), posted by the director seat, relaying the maintainer — "Maintainer reply, verbatim: 「同意」 (all five batch #61 recommendations adopted)"). So this is the maintainer's ruling via the director seat, not a seat's own. Verbatim:

Ruling. A flow's node ids are unique across the whole flow: top-level nodes[] and every region body (loop / try_catch / parallel, at every depth) share one id space, and a collision is refused at parse. ADR-0031's "self-contained single-entry / single-exit sub-graph" describes control flow and variable scope, not id reuse; the ADR gains one sentence saying so. Every reader that flattens a flow may key on the bare id.

Execution notes on the same comment: enforce at the #15713 layer, extend the walk "via collectFlowGraphs or an equivalent — one refusal, one message shape (Duplicate node id … declared by …), naming both locations"; move #15713's boundary pin; ADR sentence in the same PR, human merge; changeset states the narrowing; clause-② carrier.

Maintainer-only merge: yes (governed docs/adr/**).

1. Ruling vs implementation — exact, with declared extras

• One space across top-level + every region at every depth: flow.zod.ts:945-971 (ref) — for (const graph of collectFlowGraphs(flow)) with a single firstNodeLocationById map; collectFlowGraphs recurses to MAX_REGION_DEPTH = 32 (control-flow.zod.ts:560,726), same ceiling parseFlowNodeRegions/validateControlFlow use. ✅
• Refused at parse: it is inside FlowSchema.superRefine (flow.zod.ts:924). ✅
• One refusal, one shape: same code: 'custom', message Duplicate node id \X` — `` reuses the id already declared by ``; …, both locations named as nodes[i]or → nodes[i]. Within-one-region duplicate also yields exactly one issue (pinned: flow.test.ts` "a duplicate WITHIN one region is refused by this rule alone"). ✅
• Boundary pin moved: flow.test.ts:2131 renamed to "a region node reusing a top-level id is refused — the FlowSchema accepts a flow whose top-level nodes[] declares the same id twice — measured with two lit controls; only region nodes are checked (analyzeRegion) #15713 boundary, moved by Decision: do a flow's top-level nodes[] and its region bodies (loop / try_catch / parallel) share ONE node-id space, or two? — uniqueness is now enforced inside each, never across #16134", assertion flipped from success: true to the anchored refusal. ✅
• ADR-0031: exactly one sentence (4 wrapped lines, docs/adr/0031-…:142-145), appended inside list item 1 "Well-formed by construction"; no existing text moved or deleted (+4 -0). ✅
• Beyond the ruling, all declared in the PR body: (a) FlowGraph.path — the "or an equivalent" the ruling allows; (b) a non-object-element guard in collectFlowGraphs (see §3); (c) content/docs/automation/flows.mdx (+8/-2, not governed) — a docs-drift fix the Docs Drift bot itself flagged; (d) a hand-built self-referential region now refuses with a bounded ZodError instead of parsing. None narrows or widens the ruling; (d) is a consequence of it. Not wider, not narrower.

2. Files vs merge-base 83863b2df (7 files, +433/-58)

.changeset/flow-one-node-id-space.md (new) · content/docs/automation/flows.mdx · docs/adr/0031-advanced-flow-node-executors-and-dag.md · packages/spec/src/automation/{control-flow.zod.ts, flow.zod.ts, flow.test.ts, region-normalization.test.ts}. Governed: docs/adr/** only — no .claude/**, skills/**, AGENTS.md, CLAUDE.md, content/docs/releases/**. Governed Surface Queue Guard check: success. Draft, mergeable_state: blocked.

3. Clause-② derivation — accept set of FlowSchema narrows (verified)

• Base flow.zod.ts walked flow.nodes only; head walks every graph. A shape that parsed at base (region node reusing a top-level id — base's own pin at flow.test.ts:2131) is refused at head ⇒ genuine accept-set narrowing, clause-② yes. needs:contract-review on PR and card (both read back). ✅
• Anchor: path: [...graph.path, 'nodes', index, 'id']; graph.path built in collectFlowGraphs as [...path, 'nodes', index, 'config', slot.key, ...(slot.index ?? [])] — regionSlotsOf (control-flow.zod.ts:518-547) emits key for body/try/catch and key+index for branches[], so nodes.1.config.branches.1.nodes.0.id and nodes.1.config.body.nodes.0.config.catch.nodes.0.id are right; both pinned. ✅
• FlowGraph.path additive on the export: only in-repo constructor is collectFlowGraphs; only other reference is a docblock in packages/lint/src/lint-flow-patterns.ts:92. (Strictly, a required member on an exported interface is a compile break for any external implementer of FlowGraph literals — none in-repo; check:api-surface compares listings/signatures so it stays green. Informational, F3.)
• analyzeRegion's duplicate node id 'X' (control-flow.zod.ts:413) unreachable from parsing doors — confirmed: registerFlow = engine.ts:3554 FlowSchema.parse → :3557 detectCycles → :3562 validateControlFlow; defineFlow = flow.zod.ts:1019 FlowSchema.parse; objectstack validate / stack flows[] reach metadata-type-schemas.ts:119 flow: FlowSchema. Remaining direct callers hand in raw/parsed regions only: bpmn-mapping.ts:111 regionEntryExit and findRegionEntry (control-flow.zod.ts:475). ✅
• Non-object guard: base collectFlowGraphs did for (const node of nodes) for (const slot of regionSlotsOf(node)) → node.config on null → TypeError (base control-flow.zod.ts:714-716). Head returns early on raw === null || typeof raw !== 'object', and the superRefine reads (node as {id?: unknown} | null)?.id and skips non-strings. Pinned: flow.test.ts "a non-object element in a raw region does not crash the parse-time walk…" asserts safeParse succeeds, validateControlFlow throws an Error that is not.toBeInstanceOf(TypeError) with loop 'n' body: invalid region. Correct and necessary (the walk now runs where a throw would escape safeParse). ✅

4. Changeset — graded against batch #35 "WHICH LEVEL" + the two gates

• "@objectstack/spec": minor ✅ — check-changeset-no-major.mjs refuses major during the launch window; breaking-ness is carried by the **BREAKING** banner (present, line 9) + ADR-0087 disposition (present, line 7). Summary feat(spec)!: ✅. The LEVEL axis (check-changeset-no-major.mjs:734-): clause-② declared (label + body line) and the moved path packages/spec/src/… matches the ^packages/([^/]+)/src/ axis, package graded minor not patch ⇒ green, and the verdict is meaningful here ([finding] The changeset LEVEL axis is blind to every NESTED package: packages/*/src/** matches one segment, so 51 of 74 workspace packages (all drivers/services/adapters) can pair Clause-②: yes with patch and stay green #16713). Check Changeset job (which runs check-adr-0087-registration.mjs --base $MERGE_BASE, pr-automation.yml:823-824) is success.
• Honesty of not-required (no-migration-prescription): see F1. Substance is defensible; the category spelling is the weak point.

5. Tests

• No .skip / .only / .todo / xit / xdescribe in either test file at head. ✅
• Pins that redden on a full revert to base: flow.test.ts — the moved boundary pin; the eight refusal pins in describe('FlowSchema — one node-id space … (#16134)') (loop-body, formatZodError, defineFlow, sibling parallel branch, try_catch-in-loop depth, later-top-level anchor, within-one-region, refused-region ids judged); the non-object-guard pin (base throws TypeError). region-normalization.test.ts — "carries each graph's key path beside its scope" (g.path undefined at base) and the #4415 self-referential pin (base parses, caught stays undefined). The accept-side pin passes at base by design. Consistent with the ablation's "9 red on top-level-only walk; guard + accept pins stay green".
• Re-spelled fixtures in region-normalization.test.ts (read in full at head): "normalizes every parallel branch" keeps ENVELOPE per branch + ['left','right']; "normalizes both try_catch regions" keeps both ENVELOPEs + errorVariable; "recurses — three containers deep" keeps the deep ENVELOPE; #4347 "names each parallel branch and both try_catch regions" keeps both scope arrays. No original assertion lost in those four. The #4415 self-referential pin replaced .not.toThrow() with "bounded ZodError, never RangeError" — declared in the PR body and a true consequence of the ruling (F4, informational). Collision sites: 6 (gatedRegion() twice in parallel, twice in try_catch, loopWith nested, self-region, and two in the #4347 test) across 5 tests — see F2.
• Census spot-check: git grep over examples/** on the ref finds no literal region-body nodes[] (the body: hits in examples/app-showcase/src/automation/flows/index.ts are notification bodies; every id there is top-level). Consistent with "0 cross-namespace in-repo"; the assembled-fixture collisions the PR found itself are the only positive control I can confirm from the diff.

6. CI on head c07ea3249 (34 check runs at review time)

• success (17): Check Changeset, Check PR Size, Auto Label, Governed Surface Queue Guard, Spec property liveness, Check Documentation Links, Flag docs affected by code changes, Type Check · source gates, filter, No other open PR may claim the same issue (×2), No other open PR may claim the same single-writer path (×2), Part-of PR must not also close its card (×2).
• skipped (2): Console Pin Gate, Packed-tarball smoke (opt-in).
• in_progress (15): Build Core, Test Core 1–6, Build Docs, Dogfood Regression Gate 1–3, Dogfood Verify CLI, Temporal Conformance, Type Check · workspace / consumer gates / debt ledger, Lint & Repo Gates.
• failure: 0. mergeable_state: blocked = draft + required checks unfinished (+ governed surface ⇒ human merge regardless). Head is 9 commits behind origin/main (d4401f75b); the PR body's stale-tree note says none of those 9 touch this diff — plausible from the file list, CI on the merge ref is the check.

Findings

F1 — medium — ADR-0087 category no-migration-prescription is held by a detector miss, not a positive finding. Evidence: .changeset/flow-one-node-id-space.md:63-91 ships a // before / // after rewrite block and a **Remedy.** paragraph ("Rename the later node … re-point … the edges"). By the gate's own contract (scripts/check-adr-0087-registration.mjs:147-153: refused "when the changeset's own body carries a migration prescription … instructions for rewriting a consumer's code") that is a prescription; findMigrationPrescription (:1380) does not see it only because the body has no ## Migration / 迁移 framing, no FROM → TO, no | Wrote | Write instead | table — exactly the #8299 shape the script's header warns about (:196-210). Substance: the marker's own argument (a collision has no mechanical rewrite — which node to rename is authoring intent objectstack migrate meta cannot choose) is sound, so not-required is honest in substance. Expectation: maintainer accepts the marker's reasoning explicitly at merge, or the author trims the Remedy/before-after into a one-line "rename one of the two" so the category stands on its own terms. Not blocking on its own.

F2 — low — "6 failed" vs 5 re-spelled tests. PR body: "179 passed | 6 failed — the 6 are the assembled-fixture collisions". The diff re-spells 5 tests in region-normalization.test.ts holding 6 collision sites (the #4347 scope test holds two). Either the count is collision sites, not tests, or a 6th failing test is unnamed. Not reproduced locally (no checkout, by this seat's boundary). Expectation: author names the 6th test or corrects the sentence; no code change implied.

F3 — info — FlowGraph.path is a required member on an exported interface. control-flow.zod.ts:696. In-repo additive (sole constructor collectFlowGraphs); an external implementer building FlowGraph literals would break at compile. check:api-surface does not see it by design. Expectation: none; noted so the changeset's "additive" reads with that caveat.

F4 — info — #4415 self-referential pin's assertion changed, not lost. region-normalization.test.ts:161-198: .not.toThrow() → bounded ZodError, not RangeError, every issue Duplicate node id \l``. Original intent (termination at the depth ceiling) preserved; declared in the PR. No action.

F5 — info — second message shape kept in analyzeRegion. control-flow.zod.ts:409-416. Verified unreachable from every parsing door (§3). Acceptable as-is; if the maintainer wants one shape in the code as well, it is the one-line deletion the PR offers, with bpmn-mapping.regionEntryExit the only raw caller to re-check.

---

Generated by Claude Code

[os-zhuang]

Contract review (CONTRACT_REVIEW_TIER, isolated seat) — PR #16825 @ 64c41f20c — re-review

Verdict: PASS WITH FINDINGS (CI-conditional — 16 check runs still in_progress on 64c41f20c at review time, 0 failed; the verdict stands if they finish green, and is void on any red). Re-review of the moved head against the prior review (comment 5582135982, head c07ea3249, PASS WITH FINDINGS). The patch round is textual as declared: two code comments, the changeset paragraph, the PR body, one added test — no behaviour change. Prior findings: F5 discharged; F1 and F2 still open (both non-blocking, both unchanged since c07ea3249); F3/F4 informational, unchanged.

Maintainer-only merge: yes (governed docs/adr/**).

Delta c07ea3249..64c41f20c (2 commits: 2baa5d480, 64c41f20c) — 4 files, +83/−17

File	+/−	What changed	Behaviour change?
packages/spec/src/automation/flow.zod.ts	+6/−3	superRefine comment only (the analyzeRegion line is reachable past MAX_REGION_DEPTH; kept)	No — git diff filtered to non-comment lines is empty
packages/spec/src/automation/control-flow.zod.ts	+9/−4	analyzeRegion comment only ("last line of defence" past the ceiling)	No — same filter empty; MAX_REGION_DEPTH = 32 (:565) untouched, validateControlFlow (:589-614) untouched, collectFlowGraphs untouched
.changeset/flow-one-node-id-space.md	+12/−8	ADR-0087 marker + BREAKING paragraph + "What changes" + the "one refusal" paragraph now name the ceiling (MAX_REGION_DEPTH = 32)	n/a
packages/spec/src/automation/flow.test.ts	+56/−2	describe-block comment; one it renamed ("walks every depth" → "walks nested depth"); one test added (the seam pin)	n/a

Not in the delta: docs/adr/**, content/docs/**, region-normalization.test.ts. Whole PR vs merge-base 83863b2df is still 7 files, now +499/−58 (433 + 83 − 17 ✓).

Numbered verification

1. No code change beyond comments — confirmed by git diff c07ea3249..64c41f20c -- flow.zod.ts control-flow.zod.ts | grep '^[+-]' | grep -v '^[+-]\s*//' → empty. MAX_REGION_DEPTH, parseFlowNodeRegions (:660 ceiling), collectFlowGraphs (:731 ceiling), validateControlFlow, analyzeRegion — all byte-identical to c07ea3249. No finding.

2. The added seam test (flow.test.ts:2323-2373) — read against the code, not run (this seat's boundary):

   • Fixture geometry: loopsNestedTo(nesting, …) wraps innermost in l{nesting−1} … l1, then n; so l_k sits at nesting k, its body at k+1, and innermost at nesting = nesting. flowWith edges are start → n → end, matching the outer id n. Ids dup / u1 / u2 collide with nothing else (n, l1..l32, start, end). ✅
   • JSON-round-tripped, no cycle: roundTripped() does JSON.parse(JSON.stringify(flowWith([...]))); the fixture is also built with fresh objects per level, so there is no object cycle either way — this is the parsed-JSON shape an author's file would produce, unlike the #4415 self-referential pin. ✅
   • Nesting 32 refused at parse: collectFlowGraphs.visit pushes the graph at depth and returns after pushing when depth >= 32, so graphs for nesting 0..32 are walked; the dup pair at nesting 32 is seen and refused with one custom issue anchored […, 'nodes', 1, 'id']. Asserted: success=false, issues.length === 1, message contains Duplicate node id \dup`, path.slice(-3)=['nodes', 1, 'id']`. ✅
   • Nesting 33 accepted by parse, refused by validateControlFlow: the walk never pushes nesting 33 (returns at depth 32), and parseFlowNodeRegions returns l32 raw at regionParseDepth === 32, so l32.config.body is left raw — safeParse succeeds. validateControlFlow then iterates the same 0..32 graphs; at the nesting-32 graph it meets l32, runs slot.schema.safeParse on its raw body, then analyzeRegion → duplicate node id 'dup' → throw new Error(\${where}: …`)wherewhereends→ loop 'l32' body. Asserted: caught instanceof Error, not TypeError, message contains loop 'l32' body: duplicate node id 'dup'and does **not** contain the parse shapeDuplicate node id`. ✅ Matches the PR body's "base tree identical" claim (base had the same two ceilings).
   • Control: nesting 33 with u1 → u2 chained (ue) — single entry / single exit for analyzeRegion, every intermediate body a lone loop node — safeParse succeeds and validateControlFlow does not throw. ✅ (The 64c41f20c fix-up chaining the control is the right one; an unconnected pair is two entries / two exits.)
   • Does it pin the ceiling honestly? Yes. MAX_REGION_DEPTH is a non-exported const, so the test necessarily uses literals 32 / 33. Raise the constant (e.g. 40) → nesting-33 dup becomes a parse refusal → pastCeiling.success false → red. Lower it (e.g. 20) → nesting-32 dup parses → atCeiling.success true → red. Either move reddens the test, and its title names the number, so the seam moves deliberately. ✅

3. Changeset text — the BREAKING paragraph now reads "nested to any depth the parse walks — up to MAX_REGION_DEPTH = 32 levels"; "What changes" adds "down to MAX_REGION_DEPTH (32)"; the "one refusal" paragraph states plainly what lives past the ceiling (within-region duplicate → analyzeRegion's own line; cross-region collision past the ceiling not judged). The contract sentence ("a flow has one node-id space … refused at parse time … by every door") is intact; the ceiling is disclosed as the implementation seam rather than hidden behind "any depth". Honest and not weakened — with one consequence noted as F6 below. The ADR-0087 marker keeps not-required (no-migration-prescription) with the same reasoning, now also naming the ceiling. Check Changeset (which runs check-adr-0087-registration.mjs --base) is success on the new head.

   • F1 (ADR-0087 category held by a detector miss) — still open, unchanged. .changeset/flow-one-node-id-space.md still ships the // before / // after rewrite block and the **Remedy.** paragraph ("Rename the later node … re-point … the edges"); the delta did not touch those lines and the PR body's "Patch round" section does not mention F1. Same disposition as before: substance sound, category spelling weak; maintainer accepts the marker's reasoning explicitly at merge or the author trims Remedy/before-after to one line. Non-blocking.
   • F2 ("6 failed" vs 5 re-spelled tests) — still open, unchanged. The PR body still says "179 passed | 6 failed — the 6 are the assembled-fixture collisions", and the "Patch round" section does not name a 6th test or restate the count as collision sites. Non-blocking; no code change implied.

4. Governed path — git diff --stat 83863b2df..64c41f20c -- docs/adr/ → 1 file changed, 4 insertions(+): docs/adr/0031-advanced-flow-node-executors-and-dag.md is still exactly the one sentence (+4/−0, wrapped), unchanged since c07ea3249; the ADR keeps the ruling's "at every depth" wording (the contract), per the seat's declared choice. No other governed path (.claude/**, skills/**, AGENTS.md, CLAUDE.md, content/docs/releases/**) in the diff. Governed Surface Queue Guard: success. ⇒ draft, human merge.

5. CI on 64c41f20c (34 check runs at review time):

   • success (16): Check Changeset, Check PR Size, Auto Label, Governed Surface Queue Guard, Spec property liveness, Check Documentation Links, Flag docs affected by code changes, Build Docs, Type Check · source gates, filter, No other open PR may claim the same issue (×2), No other open PR may claim the same single-writer path (×2), Part-of PR must not also close its card (×2).
   • skipped (2): Console Pin Gate, Packed-tarball smoke (opt-in).
   • in_progress (16): Build Core, Test Core 1–6, Dogfood Regression Gate 1–3, Dogfood Verify CLI, Temporal Conformance (live PG + MySQL), Type Check · workspace / consumer gates / debt ledger, Lint & Repo Gates.
   • failure: 0. mergeable_state: blocked (draft + required checks unfinished + governed surface). Head is 11 commits behind origin/main (was 9 at the prior review); the two new commits on main are not in this diff's file set by inspection — CI on the merge ref (cf821dc6d, per the Docs Drift bot) is the check. Labels read back: documentation, size/l, tests, tooling, needs:contract-review — the clause-② carrier is re-hung after the size-labeler's whole-set write, as the PR body declares.

6. Prior findings, disposition on the moved head

   • F1 — open (see 3). F2 — open (see 3).
   • F3 (info, FlowGraph.path required member on an exported interface) — unchanged, informational, no action expected.
   • F4 (info, #4415 pin assertion changed not lost) — unchanged, no action.
   • F5 (info, second message shape kept in analyzeRegion) — discharged: the line is now kept on purpose with a stated reason (only refusal of a within-region duplicate past the ceiling, plus bpmn-mapping's raw-region caller), documented in both code comments and the changeset, and pinned by the seam test. The offered deletion is withdrawn in the PR body; nothing left to decide.

Findings

F1 — medium — carried forward, still open. ADR-0087 not-required (no-migration-prescription) is held by a detector miss: the changeset body still carries a **Remedy.** paragraph and a // before / // after rewrite block (.changeset/flow-one-node-id-space.md, unchanged lines), which findMigrationPrescription does not see only for want of ## Migration / FROM → TO / | Wrote | Write instead | framing. Substance of the marker is sound (which node to rename is authoring intent no migrate meta rewrite can choose). Expectation unchanged: maintainer accepts the reasoning explicitly at merge, or the author trims Remedy/before-after to a one-line "rename one of the two". Non-blocking.

F2 — low — carried forward, still open. PR body "179 passed | 6 failed — the 6 are the assembled-fixture collisions" vs 5 re-spelled tests holding 6 collision sites in region-normalization.test.ts. Name the 6th test or reword to "6 collision sites across 5 tests". Non-blocking, text only.

F3 — info — carried forward. FlowGraph.path is a required member on an exported interface (control-flow.zod.ts:696); in-repo additive, compile break for an external FlowGraph literal implementer. No action.

F4 — info — carried forward. #4415 self-referential pin now asserts a bounded ZodError, never RangeError. No action.

F6 — info — new. With the ceiling now stated, the residual gap is explicit: a cross-region collision where the later node sits at nesting ≥ 33 is refused by no door (the parse walk stops at 32; analyzeRegion judges one region at a time), so the ruling's "at every depth" holds up to the shared MAX_REGION_DEPTH and the ADR/flows.mdx wording is one level stronger than the code. Pre-existing ceiling, identical on base, 33-deep nesting is not an authored shape, and the changeset now says exactly this — so this is a maintainer-acknowledgement at merge, not a change request. If the maintainer wants "every depth" literal, it is a follow-up card (walk the id pass unbounded, or raise the ceiling), not this PR. The = 32 literal in the CHANGELOG will go stale if the constant ever moves; acceptable.

Discharged since c07ea3249: F5 (see 6).

---

Generated by Claude Code

[claude]

CI condition on the re-review is DISCHARGED — the verdict now stands unconditionally

domain:spec execution seat, session session_016N6xmWt5hYm94ffVEwGH8x, measured 2026-09-08T09:39:34Z (date -u).

The re-review at 64c41f20c (5582499489) was explicitly CI-conditional: "16 check runs still in_progress at review time, 0 failed; the verdict stands if they finish green, and is void on any red." They have finished:

head 64c41f20c80b6d8cbe93419b088071cf3e311466
34 distinct check names, aggregated by name → 34 GREEN, 0 running, 0 failed
mergeable_state: clean

⚠️ Aggregated by NAME on purpose: a single head can carry the same check twice (one superseded run plus its replacement), and reading the raw list makes a replaced run look like a red light. Every one of the 34 names has a completed + success/skipped reading.

⚠️ The one failure event on this PR was on a superseded head, not this one. TypeScript Type Check failed at 09:09Z on 2baa5d480 — the first patch-round commit, whose seam-pin control leg the dev then fixed in 64c41f20c (its own report records that run as 1 failed | 183 passed). Resolved against the live branch tip before being treated as real; it is not a live failure.

维护者速读

• 这个 PR 现在只差人工合并,没有别的门禁在挡。 它是 GOVERNED(动了 docs/adr/**),所以按红线,任何 agent 座位 ⛔ 都不会替你翻 ready、不会入队、不会挂 auto-merge —— 它会一直是 draft,等你。
• 两个 head 各有一份独立的至档契约复审:c07ea3249(5582135982)与补丁轮后的 64c41f20c(5582499489),都是 PASS WITH FINDINGS,findings 都不阻断。PASS 绑 head 不绑 PR,所以补丁轮之后补了第二份,不是重复劳动。
• 补丁轮只改了文字,不改行为:两处代码注释、changeset 的一段、PR 正文,外加一个新测试(把 MAX_REGION_DEPTH = 32 这个接缝钉住)。复审用 git diff 过滤掉注释行后为空,逐条核过。
• 要你签字的那一件事:这张卡的裁决(批 Add comprehensive AI prompt system for protocol development #61)把 flow 顶层 nodes[] 与 region body 合成一个 node-id 空间,重复 id 从此在 parse 就被拒 —— 这是一个收窄接受集的破坏性变更,changeset 已按发布窗口惯例记为 minor + **BREAKING**。
• 一处诚实的边界(两份复审都写了,不是遗漏):超过嵌套深度 32,parse 不再往下走,重复 id 由 validateControlFlow 在 analyzeRegion 里拒绝,消息形状不同。代码注释、changeset 和那个新测试都把这条接缝写明了;ADR-0031 与 flows.mdx 保留裁决的绝对措辞,复审逐条判定为可接受。

---

Generated by Claude Code