Skip to content

fix(spec): filter the edge members collectFlowGraphs hands out - #18906

Merged
os-elon-musk merged 3 commits into
mainfrom
claude/issue-18102-flow-edge-member-filter
Sep 18, 2026
Merged

os-elon-musk merged 3 commits into
mainfrom
claude/issue-18102-flow-edge-member-filter

Conversation

@os-elon-musk

Copy link
Copy Markdown
Collaborator

Fixes #18102

Clause-②: no

collectFlowGraphs handed out a FlowGraph whose edges could 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 — so FlowGraph.edges means 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.

card's reading re-derived at verdict
node side does nodes.filter(isRegionDict) around :773 control-flow.zod.ts:773 holds, exact
edges forwarded untouched in the same graphs.push :774 holds
a nested region's edge list admitted on Array.isArray alone around :786 :786 holds, exact

The reproduction from the card body, run on main at 0b31d90fb:

collectFlowGraphs({ nodes: [start, loop{ body: { nodes: [], edges: [null] } }], edges: [] })
  graph[0] scope=""                 path=[]                              edges=[]
  graph[1] scope="loop 'lp' body"   path=["nodes",1,"config","body"]     edges=[null]
  NON-RECORD EDGE MEMBERS HANDED OUT: [null]

The shape of the repair

  • A drop, in the producer. 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.
  • Not a refusal, and not a looser signature. No authoring door's accept set moves. FlowSchema.safeParse still returns an envelope rather than throwing (the walk runs inside the parse, so the repair has to stay a drop and a skip), the region safeParse refusal in validateControlFlow still owns and still reports the malformed region, and FlowGraph.path still 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.
  • No consumer indexes graph.edges positionally. Both packages/lint readers key findings by edge.id; the edge-id uniqueness rule in flow.zod.ts reads flow.edges raw, never graph.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.edges consumers in packages/lint". There are three graph.edges consumers on this tree, not two.

consumer guard
packages/lint/src/lint-flow-patterns.ts:1560 recordsOf(graph.edges)
packages/lint/src/validate-expressions.ts:1462 recordsOf(graph.edges)
packages/services/service-automation/src/engine.ts:9244 none — reads edge.id / edge.source / edge.target straight off each member

The third one is shielded only by call ORDER: inside registerFlow, canonicalizeStoredFlow runs validateControlFlow(parsed) at engine.ts:4021, which refuses the malformed region before validateFlowExpressions is reached at :4058. Measured:

validateControlFlow(the card's repro)
  THROWS Error: loop 'lp' body: invalid region — Too small: expected array to have >=1 items;
                                                Invalid input: expected object, received null

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, answered

analyzeRegion (:412, for (const e of edges) at :442) throws on a non-record edge member, measured on all five shapes:

null       THROWS TypeError: Cannot read properties of null (reading 'source')
undefined  THROWS TypeError: Cannot read properties of undefined (reading 'source')
a string   no throw — errors: ["edge 'undefined' source 'undefined' is not a region node", …]
a number   no throw — same
an array   no throw — same

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:624 hands it parsed.data from a successful slot.schema.safeParse — the failure branch throws first, so its edges members are always parsed records;
  • findRegionEntry ← AutomationEngine.runRegion(region: FlowRegionParsed) — parsed;
  • bpmn-mapping.ts:111 casts a raw cfg(c).body region 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 is FlowEdgeParsed[], 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 — the packages/spec-side pin the card said was the one still missing. Both halves of that claim were verified first:

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/main merged in at 600b1e2ab before 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:

anchor  "edges: keptEdges.length === edges.length ? edges : keptEdges,"  x1 -> x0
replace "edges,"                                                        x0 -> x1
blob    38cf36cb5fdc -> b293f7924509        (the mutation landed)
result  23 failing tests, all inside the #18102 describe, zero elsewhere
restore blob == HEAD (38cf36cb5fdc) and `git diff HEAD` empty

Direction: red, as expected. The suite resolves ./control-flow.zod.js relatively inside its own package, so the mutation reddened it with no rebuild — this ablation has no dist/ leg to prove.

Tests.

run result
pnpm --filter @objectstack/spec test 488 files / 14159 tests passed
pnpm --filter @objectstack/spec typecheck pass (test layer compiles; debt ledger unmoved)
pnpm --filter @objectstack/lint test 104 files / 3909 tests passed
... service-automation exec vitest run src/nested-region-parity.test.ts 22 tests passed

The packages/lint run is the load-bearing one for the cross-package direction: non-record-object-entry.test.ts keeps RESIDUAL_THROWS and RESIDUAL_INVENTED exact 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:

node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands
  -> 81 families
81 run, every exit code captured to a file BEFORE any pipe, all 0
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --ran ran.txt
  -> 81 derived, 81 run, 0 NOT-MEASURED, 0 UNRUN
     (a DERIVED zero — all 81 recorded an exit code and none of them is 3)

Two conditions were cleared rather than reported as reds, because neither measured anything: check-plugin-teardown-shape --self-test exited 3 on a shallow clone that could not reach its pinned fixture (cleared by git fetch --unshallow, as its own text prescribes), and four gates exited 3 with PREREQUISITE NOT MET before the package closure was built (cleared by turbo run build over ./packages/*). The whole set was then re-run at the final 369c4cca5, 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 lint is eslint . --no-inline-config, a repo-wide scan CI owns. The three facts that make the narrow run a measurement:

  1. Population, read from eslint.config.mjs itself — its file-matching objects declare **/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs} and narrower per-root subsets of it, minus NEVER_LINTED. A .md path matches no config object, which eslint states in its own report for the changeset (File ignored because no matching configuration was supplied).
  2. Count, read from --format json — 3 paths handed in, 3 in the report, 2 linted: 0 errors, 0 warnings, 0 suppressed. Exit 0.
  3. Invariance — eslint.config.mjs states, with its own measured positive control, that this repo "never enables type-aware linting (no parserOptions.project, no typed @typescript-eslint rules) 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.

  • Two packages/lint comments now misstate the producer. lint-flow-patterns.ts:1555 and validate-expressions.ts:1459 each say the edge side of this walk "is not" member-filtered, as the justification for their recordsOf coercion. 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: collectFlowGraphs hands out a FlowGraph whose edges can hold a non-record — the node side #16752 filtered, the edge side it did not #18102".
  • non-record-object-entry.test.ts:378 carries the same stale sentence in underNestedRegionEdges' docblock. Same reasoning; the arm's behaviour is unaffected and the file is untouched.
  • The edge-id uniqueness rule is top-level only. flow.zod.ts:1164 walks flow.edges and 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.ts casts raw region config into analyzeRegion. cfg(c).body as Region at :313 (and the try/catch/branches siblings) is a bare cast off an open z.record, so a malformed region reaches analyzeRegion as a TypeError where 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.
  • lint: lintFlowPatterns throws on a non-record member of a flow's edges list — 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

`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>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 18, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

2 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
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 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 — 136 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 84ba4a847600dbecb55fcd2e31322ada19496f58 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 1306253a18e812b2ada40a8ecb206eab85503101 — the merge of head 369c4cca589bca85d772afbd17bbc04cb04fb681 into base 84ba4a847600dbecb55fcd2e31322ada19496f58, 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 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

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

Copy link
Copy Markdown
Collaborator Author

Seat provenance: needs:contract-review was mis-hung on this pair by the seat and has been cleared from BOTH carriers. This red was the seat's, not the diff's.

domain:spec seat 3, session_019srGWGCBBCBHqcDoRZpQRh, R1.

What happened, in order

  1. At 2026-09-18T05:12Z this seat added needs:contract-review to PR fix(spec): filter the edge members collectFlowGraphs hands out #18906 and card spec: collectFlowGraphs hands out a FlowGraph whose edges can hold a non-record — the node side #16752 filtered, the edge side it did not #18102.
  2. Check Changeset then went red. Its log names the mechanism: it read Clause-②: no from the PR body, and the carrier label made it require at least minor for a widening — 「the needs:contract-review carrier is a PR label and the Clause-②: line is a PR-body line」.
  3. ⭐ The decisive reading: an earlier Check Changeset run on this same commit 369c4cca58 succeeded (job 105489421788), and the failing run (105489658889) came after the label write. The diff did not change between them. ⇒ the label was the variable.
  4. At 2026-09-18T05:20Z the label was removed from PR fix(spec): filter the edge members collectFlowGraphs hands out #18906 and card spec: collectFlowGraphs hands out a FlowGraph whose edges can hold a non-record — the node side #16752 filtered, the edge side it did not #18102, two seconds apart, deliberately: 「一次合法的清除留下两条相隔数秒的移除,一次剥标只留一条」 — a one-sided removal is indistinguishable from a strip.

The seat's error, stated precisely

needs:contract-review is the carrier for a Clause-②: yes declaration, and the discipline says ⛔ 不前瞻预挂. This pair declares Clause-②: **no**.

What actually applies here is a different rule: the clause-② path limb fires because the diff touches packages/spec/src/**, and that limb requires an in-seat clause-② review PASS on record before the PR may enqueue. ⇒ The seat conflated "a review is owed" with "the yes carrier is owed". Only the first is true. The gate's own route 2 names the remedy exactly — 「the level is right and the DECLARATION is wrong -> correct it at the producer: the needs:contract-review carrier is the review seat's to place and to clear」 — and ⛔ it warns against adding a tolerance to route around a declaration its author did not mean, which is why nothing about the changeset or the PR body was touched.

What is NOT being changed, and why

  • ⛔ The changeset stays @objectstack/spec: patch. The level is correct: this filters a member the producer forwards and moves no authoring door's accept set, which is a narrowing. Raising it to minor to silence the gate would be paying for the seat's mistake with a wrong release level.
  • ⛔ Clause-②: no stays. It is correct on the diff.
  • ⛔ No re-run was spent. Removing a label fires its own event, so the enforcing run arrives on its own.

Still owed before this PR may enqueue

An isolated at-tier clause-② review is running against head 369c4cca58 (the path limb's requirement). Its PASS will be recorded before any enqueue, and ⛔ this seat does not self-review it: the seat measured its own served model below CONTRACT_REVIEW_TIER.

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

@os-elon-musk
os-elon-musk marked this pull request as ready for review September 18, 2026 05:31
@os-elon-musk
os-elon-musk added this pull request to the merge queue Sep 18, 2026
Merged via the queue into main with commit 02bdeaa Sep 18, 2026
43 of 44 checks passed
@os-elon-musk
os-elon-musk deleted the claude/issue-18102-flow-edge-member-filter branch September 18, 2026 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spec: collectFlowGraphs hands out a FlowGraph whose edges can hold a non-record — the node side #16752 filtered, the edge side it did not

2 participants