fix(lint): read a flow's node lists through recordsOf, not an inline cast - #16765
Conversation
…cast `validateStackExpressions` threw on a non-record entry of a flow's `nodes` list: `Array.isArray` proves the LIST, never its MEMBERS, and an empty item in a YAML `nodes:` list deserialises to `null`. The next statement read `.type` off it, so the linter crashed instead of reporting a finding. Both inline casts now read through `recordsOf` — the one home of this coercion, so no copy is added for `collection-coercion-single-copy.test.ts` to count. The coerced array is also what is handed to `collectFlowGraphs`: that function declares already-parsed `FlowNodeParsed[]` and forwards members untouched, so passing the raw flow was calling it out of contract, and coercing only the local variable relocated the crash into `packages/spec` rather than removing it. Measured both ways. The pin is a new addressing mode in `non-record-object-entry.test.ts` rather than a local test. The sweep drove collections only, which is why this class was closed three times without reaching these two lines; it now addresses a flow's own node list and a nested region's sub-graph. Both arms found more of the same class on their first run, recorded in `RESIDUAL_THROWS` and filed as #16751 and #16752. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012GKcPZbMoGq7WPzKLfRBTU
…ow-nodes-non-record
📓 Docs Drift CheckThis PR changes 1 package(s): ⛔ 1 release-owned page(s) name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 5 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 7f8c34cfaac727343134d565a6ab7fb23a1b7914 && git checkout 7f8c34cfaac727343134d565a6ab7fb23a1b7914
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 6b7d709b8776fa2340fe7b4ce6e9b601ae256fe2 d4055961f71787ef9565653f6ce41e0bdf3508b4 && git checkout -B drift-repro 6b7d709b8776fa2340fe7b4ce6e9b601ae256fe2 && git merge --no-ff d4055961f71787ef9565653f6ce41e0bdf3508b4
node scripts/docs-audit/affected-docs.mjs --json 6b7d709b8776fa2340fe7b4ce6e9b601ae256fe2
|
Fixes #15793
validateStackExpressionsthrew on a non-record entry of a flow'snodeslist. An empty item in a YAMLnodes:list deserialises tonull, so this is an authorable shape — the same one #15552 / #15636 / #15742 closed for stack collections and forobjects[].fields. Here it crashed the linter instead of producing a finding, which presents to an author as a broken tool rather than as a problem with their metadata.Premise re-check (owed at start)
The card's line numbers were from triage on
cc5b3dd. Both casts were re-located by text, not by line — they had moved to:1127and:1205. The card's three-line reproduction was re-run onorigin/mainbefore anything changed, and reproduces exactly:After the fix, all three rows are
OK, 0 finding(s). The middle row is unchanged, so nothing started reporting that did not report before.The fix, and the half that is not obvious
Both inline casts now read through
recordsOf— the single home of this coercion, so no copy is added forcollection-coercion-single-copy.test.tsto count.There is a third edit, and measurement is what demanded it:
collectFlowGraphsis now handed the coerced array, not the raw flow. Re-pointing only the local variable does not fix the card's own reproduction — it relocates the crash intopackages/spec:This is exactly the contract reading the dispatch ruling settled on:
collectFlowGraphsdeclaresreadonly FlowNodeParsed[]and forwards members untouched, so passing raw authored metadata was calling it out of contract. The producer's signature is unchanged, deliberately — widening it to tolerate malformed members is the wrong direction and would be a change on a published surface.The pin: a new addressing mode, not a local test
The card's diagnosis was that the sweep cannot express "a flow's inner node list", and that blind spot is why this class was closed three times without touching these two lines.
non-record-object-entry.test.tsgains two graph-shaped arms rather than a one-off test:flows[].nodes— the flow's own list.flows[].nodes[].config.body.nodes— a container's sub-graph, which only the producer can hand out.The PM's hunch that this would need a second addressing mode was right; the hunch that it would be a big change was not.
SweptCollection.stackwas already an arbitrary builder, so each constructor is a three-line sibling ofunderObject. The gap looked structural and was not — it was in the addressing, never in the rule table, and nobody had written a second shape of builder.Both arms found more of the same class on their first run
This is the pin earning its keep immediately, and the strongest argument against the local-test fallback:
flows[].nodeslintFlowPatterns—lint-flow-patterns.tsholds the SAME two spellings, three times overflows[].nodes[].config.body.nodesvalidateStackExpressions+lintFlowPatterns, both from insidecollectFlowGraphsBoth are recorded in
RESIDUAL_THROWS, which the harness documents as findings with a filed card, exact in both directions: when either is fixed its row becomes a lie and the test reds until it is deleted.Verification
153d7a65tod7401e71, anchor counts inverted before the run) makes theflows[].nodesarm fail 4 tests, namingvalidateStackExpressionsas a thrower beyond the ledgeredlintFlowPatterns. Restore verified by blob hash equal to HEAD and an emptygit diff HEAD, not by an exit code.pnpm --filter @objectstack/lint test— 102 files / 3569 tests passed, on the merged head.pnpm --filter @objectstack/lint typecheck— clean; test layer compiles, debt ledger unchanged.dispatch-gates --ran(55 derived, 55 run, 0 NOT-MEASURED, 0 UNRUN). 53 exit 0. Two exit 3 =PREREQUISITE NOT MET—check:dual-build-cjs-loadsandcheck:type-check-debtboth need a repo-widepnpm buildand say in terms that this is not a pass and not a finding, nothing was measured; they are CI's to run.pnpm lint— the full repo-wide eslint scan, exit 0. No narrowing was claimed or needed.origin/main, atd4055961f.origin/mainwas merged in before opening: sibling PR #16732 landed and added a rule toAUTHORING_RULES, which these arms drive. Contention re-measured this fire — all 22 open PRs' changed-file lists fetched paged to exhaustion, zero hits on either of my two files, control 22 of 22 non-empty.验收备注
collectFlowGraphs, filed at spec:collectFlowGraphsdereferences a non-record member of a NESTED region's node list — its own walk, not the caller's #16752.visitpushes a graph before theMAX_REGION_DEPTHguard returns, so at exactly nesting 32 aFlowGraphis returned to callers with anullinnodes, never having been walked:nesting 31 : THREW/nesting 32 : OK, graphs whose nodes hold a NON-RECORD: 1. This is why thegraph.nodesguard in this PR is not dead code — it is the one route by which a non-record legitimately arrives in a returned graph.collectFlowGraphsitself (slot.raw.nodes,Array.isArray-checked only), so no coercion at any call site reaches it. Whether the guard belongs upstream is adomain:specrouting question; per the dispatch ruling this seat stopped and filed spec:collectFlowGraphsdereferences a non-record member of a NESTED region's node list — its own walk, not the caller's #16752 rather than editingpackages/spec.collectFlowVariableNames(flow-variable-scope.ts:225) casts eachgraph.nodesmember without a guard, three lines below a loop that does guardflow.variablesmembers. It is currently masked by the spec-side throw and observable only at the depth ceiling. Not fixed here — the claim's file surface is this file and its test, and the breach is reported instead. Filed withlintFlowPatternsat lint: two more flow-node-list readers throw on a non-record member —lintFlowPatternsandcollectFlowVariableNames#16751.graph.edgescarries the sameas unknown as AnyRec[]double cast two loops below the node walk. Not measured for a crash and not in this card's scope. Carrier: whoever takes lint: two more flow-node-list readers throw on a non-record member —lintFlowPatternsandcollectFlowVariableNames#16751, which touches the same walks.Generated by Claude Code