Repository navigation
fix(spec): filter the edge members collectFlowGraphs hands out - #18906
Conversation
`FlowGraph.edges` is declared `readonly FlowEdgeParsed[]`, and the walk forwarded it untouched — four lines from the node-side member filter the same walk has carried since #16752. A nested region's edge list is admitted on `Array.isArray` alone, which proves the LIST and never its MEMBERS, so a `body: { edges: [null] }` (what a YAML `edges:` item left empty deserialises to) became a graph whose `edges` held a member its own declared element type excludes. One filter per list, side by side, copy-on-write per list. The accept set of every authoring door is unchanged: this is a drop in the producer, not a refusal, and not a looser signature. Claude-Session: https://claude.ai/code/session_019srGWGCBBCBHqcDoRZpQRh Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019srGWGCBBCBHqcDoRZpQRh Co-authored-by: Claude <noreply@anthropic.com>
…ow-edge-member-filter
📓 Docs Drift Check2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 136 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 1306253a18e812b2ada40a8ecb206eab85503101 && git checkout 1306253a18e812b2ada40a8ecb206eab85503101
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 84ba4a847600dbecb55fcd2e31322ada19496f58 369c4cca589bca85d772afbd17bbc04cb04fb681 && git checkout -B drift-repro 84ba4a847600dbecb55fcd2e31322ada19496f58 && git merge --no-ff 369c4cca589bca85d772afbd17bbc04cb04fb681
node scripts/docs-audit/affected-docs.mjs --json 84ba4a847600dbecb55fcd2e31322ada19496f58 |
|
Seat provenance:
What happened, in order
The seat's error, stated precisely
What actually applies here is a different rule: the clause-② path limb fires because the diff touches What is NOT being changed, and why
Still owed before this PR may enqueueAn isolated at-tier clause-② review is running against head Written 2026-09-18T05:20Z. The two job ids, their conclusions and the gate's log text were read at 2026-09-18T05:19Z. Seat post: #18883. Generated by Claude Code |
Fixes #18102
Clause-②: no
collectFlowGraphshanded out aFlowGraphwhoseedgescould hold a non-record. The node side of that same walk has been member-filtered since the repair #16752 landed; the edge side was forwarded untouched, four lines away. One filter per list, side by side, in the same walk — soFlowGraph.edgesmeans what its declared type says.What the card claimed, re-derived on this tree
All three readings hold, and the line numbers were re-derived rather than copied.
nodes.filter(isRegionDict)around:773control-flow.zod.ts:773edgesforwarded untouched in the samegraphs.push:774Array.isArrayalone around:786:786The reproduction from the card body, run on
mainat0b31d90fb:The shape of the repair
edges.filter(isRegionDict), through the same single predicate the node side uses — extended in this PR to name its third caller, so the two lists the walk hands out cannot drift from each other. Copy-on-write per list: a well-formed flow is handed back the very same arrays, by identity, and pays nothing.FlowSchema.safeParsestill returns an envelope rather than throwing (the walk runs inside the parse, so the repair has to stay a drop and a skip), the regionsafeParserefusal invalidateControlFlowstill owns and still reports the malformed region, andFlowGraph.pathstill indexes the RAW node list so a Zod issue stays anchored where the author wrote it. The declared element type is unchanged; it is now true.graph.edgespositionally. Bothpackages/lintreaders key findings byedge.id; the edge-id uniqueness rule inflow.zod.tsreadsflow.edgesraw, nevergraph.edges. So a dropped member changes the index and never whether an edge was judged.Latent, not live — measured, and NOT for the reason the card gave
The card's reachability argument rests on "#16910 re-pointed both
graph.edgesconsumers inpackages/lint". There are threegraph.edgesconsumers on this tree, not two.packages/lint/src/lint-flow-patterns.ts:1560recordsOf(graph.edges)packages/lint/src/validate-expressions.ts:1462recordsOf(graph.edges)packages/services/service-automation/src/engine.ts:9244edge.id/edge.source/edge.targetstraight off each memberThe third one is shielded only by call ORDER: inside
registerFlow,canonicalizeStoredFlowrunsvalidateControlFlow(parsed)atengine.ts:4021, which refuses the malformed region beforevalidateFlowExpressionsis reached at:4058. Measured:So the conclusion "no reachable throw today" survives — by one belt more than was counted, and a belt made of validator ordering rather than of a member check. That is exactly the asymmetry this card was filed about, one layer up. After this change the declared type carries it, and the next consumer of this list needs neither a coercion nor a call-order argument. ⛔ Not written up as a crash report: no throw was measured on the tree.
analyzeRegion— the card's open question, answeredanalyzeRegion(:412,for (const e of edges)at:442) throws on a non-record edge member, measured on all five shapes:But it is not a consumer of
FlowGraph.edges, and the card's "second consumer" framing does not hold. Nothing on this tree routes producer output into it. Its three in-repo entry points:validateControlFlow:624hands itparsed.datafrom a successfulslot.schema.safeParse— the failure branch throws first, so itsedgesmembers are always parsed records;findRegionEntry←AutomationEngine.runRegion(region: FlowRegionParsed)— parsed;bpmn-mapping.ts:111casts a rawcfg(c).bodyregion in, which is a pre-existing raw-cast surface of that file and untouched here.It reads an edge list; it does not read
FlowGraph.edges. Its declared parameter type isFlowEdgeParsed[], so a throw on input that type excludes is not a defect at that door — hardening it would be the lenient-consumer direction Prime Directive #12 refuses, and the direction #16752's own note refuses ("the repair is a drop, never a looser signature"). Left alone deliberately.Where the pin went
Beside #16752's, in
packages/spec/src/automation/region-normalization.test.ts— thepackages/spec-side pin the card said was the one still missing. Both halves of that claim were verified first:packages/lint/src/non-record-object-entry.test.tsalready carries theflows[].nodes[].config.body.edgesarm (underNestedRegionEdges, inSWEPT_COLLECTIONS), so no arm is added there and that file is untouched;collectFlowGraphsdereferences a non-record member of a NESTED region's node list — its own walk, not the caller's #16752's own spec-side pin is thedescribe('#16752 — a non-record member never reaches a returned graph')block inregion-normalization.test.ts, not incontrol-flow.test.ts. The newdescribe('#18102 — …')sits directly after it, under the same#4347 — collectFlowGraphsparent.31 new cases: five shapes × (three nesting depths + an unguarded-dereference arm + an anti-vacuity arm), plus the card's own reproduction and a per-list identity arm.
Verification
Union run at
369c4cca5(the merge commit;origin/mainmerged in at600b1e2abbefore opening this PR).Reverse verification — the pin can fail. Mutation through
scripts/ablation-replace.mjs, anchor hit asserted on disk, restore proven by blob equality with HEAD:Direction: red, as expected. The suite resolves
./control-flow.zod.jsrelatively inside its own package, so the mutation reddened it with no rebuild — this ablation has nodist/leg to prove.Tests.
pnpm --filter @objectstack/spec testpnpm --filter @objectstack/spec typecheckpnpm --filter @objectstack/lint test... service-automation exec vitest run src/nested-region-parity.test.tsThe
packages/lintrun is the load-bearing one for the cross-package direction:non-record-object-entry.test.tskeepsRESIDUAL_THROWSandRESIDUAL_INVENTEDexact in BOTH directions, so a producer change that flipped a row there would red it. Both stay empty.Gates. Derived from the actual changed paths, never from a named list:
Two conditions were cleared rather than reported as reds, because neither measured anything:
check-plugin-teardown-shape --self-testexited 3 on a shallow clone that could not reach its pinned fixture (cleared bygit fetch --unshallow, as its own text prescribes), and four gates exited 3 withPREREQUISITE NOT METbefore the package closure was built (cleared byturbo run buildover./packages/*). The whole set was then re-run at the final369c4cca5, so no ratchet reading predates the last commit.⛔ Derived families are not a complete account of CI on this PR: the 50 artifact-roster families, the 11 wide-population families, the 5 path-scheduled CI jobs and the always-runs tail are each outside that total, and a green from a
--self-test-only family grades the checker, not this diff.Lint, as a proven narrowing rather than a repo scan.
pnpm lintiseslint . --no-inline-config, a repo-wide scan CI owns. The three facts that make the narrow run a measurement:eslint.config.mjsitself — its file-matching objects declare**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}and narrower per-root subsets of it, minusNEVER_LINTED. A.mdpath matches no config object, which eslint states in its own report for the changeset (File ignored because no matching configuration was supplied).--format json— 3 paths handed in, 3 in the report, 2 linted: 0 errors, 0 warnings, 0 suppressed. Exit 0.eslint.config.mjsstates, with its own measured positive control, that this repo "never enables type-aware linting (noparserOptions.project, no typed@typescript-eslintrules) for ANY file, test or not". With no rule reading a type, nothing in this diff can move the verdict on a file this diff does not contain.Acceptance notes
Observations, none of them filed — each is noted here for whoever next touches the file.
packages/lintcomments now misstate the producer.lint-flow-patterns.ts:1555andvalidate-expressions.ts:1459each say the edge side of this walk "is not" member-filtered, as the justification for theirrecordsOfcoercion. Their conclusion stays right (belt and braces, ⛔ not one repair written twice) — only the parenthetical is now stale. Left untouched because the dispatch's declared file surface does not include those two files and a stale comment is not one of the three filing classes. Exact edit needed: in both, "the edge side is not" becomes "both sides are since spec:collectFlowGraphshands out aFlowGraphwhoseedgescan hold a non-record — the node side #16752 filtered, the edge side it did not #18102".non-record-object-entry.test.ts:378carries the same stale sentence inunderNestedRegionEdges' docblock. Same reasoning; the arm's behaviour is unaffected and the file is untouched.flow.zod.ts:1164walksflow.edgesand no region's, so two region edges may share an id where two top-level edges may not. A coverage asymmetry, not an error: no example fails, nothing is mis-documented. Not filed.bpmn-mapping.tscasts raw region config intoanalyzeRegion.cfg(c).body as Regionat:313(and thetry/catch/branchessiblings) is a bare cast off an openz.record, so a malformed region reachesanalyzeRegionas aTypeErrorwhere that file's own located diagnostic belongs. Reachability was NOT measured, so this stays an un-rehearsed observation rather than a card. Successor: none — no open PR touches that file.lintFlowPatternsthrows on a non-record member of a flow'sedgeslist — the sibling list #16751 did not cover #16910 is not addressed here, as the card and triage both state.维护者速读(草稿)
改了什么。
packages/spec里那个把一张 flow 拆成「每个子图」的函数,原先对节点列表做了成员过滤、对边列表没做。这次给边列表补上同一个过滤,一条路一个过滤器,并排放着。为什么改。 它对外声明「我交出去的边一定是一条边」,实际可能交出一个
null—— 作者在 YAML 里把edges:下面一项写空了,反序列化出来就是null。同一个函数、同一次遍历,节点那一半修了、边这一半没修,声明与事实不符的那一半就还在。今天没人被它崩到,但每个现在和将来的消费者都只差一次不设防的取值。本次实测还发现:挡住它的其实不止卡上写的那两道消费端强制转换,还有第三个消费者靠「校验顺序」侥幸躲开 —— 也就是说,原来的安全边界比卡上记的更薄。风险与代价(含回滚)。 行为变化只发生在「声明里本来就说不可能存在」的输入上;结构良好的 flow 按引用原样返回,零代价。不新增任何拒收,作者能写的东西一点没少。回滚 = revert 本 PR 单个 commit,无数据迁移、无配置、无兼容窗口。
席位意见。
你要做的。 确认这里只补过滤、不新增拒收(这是本卡的裁决前提),然后按常规流程合。若要顺手清掉上面 Acceptance notes 里那三处已过期的注释,那是另一个 PR 的事。
Generated by Claude Code