Skip to content

fix(lint): read a flow's node lists through recordsOf, not an inline cast - #16765

Merged
baozhoutao merged 2 commits into
mainfrom
claude/issue-15793-flow-nodes-non-record
Sep 8, 2026
Merged

fix(lint): read a flow's node lists through recordsOf, not an inline cast#16765
baozhoutao merged 2 commits into
mainfrom
claude/issue-15793-flow-nodes-non-record

Conversation

@claude

@claude claude Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Fixes #15793

validateStackExpressions threw on a non-record entry of a flow's nodes list. An empty item in a YAML nodes: list deserialises to null, so this is an authorable shape — the same one #15552 / #15636 / #15742 closed for stack collections and for objects[].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 :1127 and :1205. The card's three-line reproduction was re-run on origin/main before anything changed, and reproduces exactly:

flow.nodes [null]   : THREW Cannot read properties of null (reading 'type')
flow.nodes ["str"]  : OK, 0 finding(s)
control: clean flow : OK, 0 finding(s)

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 for collection-coercion-single-copy.test.ts to count.

There is a third edit, and measurement is what demanded it: collectFlowGraphs is 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 into packages/spec:

coercing only the local `nodes`  -> THREW Cannot read properties of null (reading 'config')
                                     at regionSlotsOf     control-flow.zod.ts:515
                                     at collectFlowGraphs control-flow.zod.ts:726
handing the coerced array in     -> OK

This is exactly the contract reading the dispatch ruling settled on: collectFlowGraphs declares readonly 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.ts gains 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.stack was already an arbitrary builder, so each constructor is a three-line sibling of underObject. 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:

arm thrower filed
flows[].nodes lintFlowPatternslint-flow-patterns.ts holds the SAME two spellings, three times over #16751
flows[].nodes[].config.body.nodes validateStackExpressions + lintFlowPatterns, both from inside collectFlowGraphs #16752

Both 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

  • Ablation — the arm was proven to fail without the fix, not merely to pass with it. Mutating both spellings back on disk (blob hash 153d7a65 to d7401e71, anchor counts inverted before the run) makes the flows[].nodes arm fail 4 tests, naming validateStackExpressions as a thrower beyond the ledgered lintFlowPatterns. Restore verified by blob hash equal to HEAD and an empty git diff HEAD, not by an exit code.
  • pnpm --filter @objectstack/lint test102 files / 3569 tests passed, on the merged head.
  • pnpm --filter @objectstack/lint typecheck — clean; test layer compiles, debt ledger unchanged.
  • 55 of 55 derived gate families run and reconciled with dispatch-gates --ran (55 derived, 55 run, 0 NOT-MEASURED, 0 UNRUN). 53 exit 0. Two exit 3 = PREREQUISITE NOT METcheck:dual-build-cjs-loads and check:type-check-debt both need a repo-wide pnpm build and 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.
  • All of the above re-run on the final head after merging origin/main, at d4055961f.

origin/main was merged in before opening: sibling PR #16732 landed and added a rule to AUTHORING_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.

验收备注


Generated by Claude Code

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

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/releases/v16.mdx (via validateStackExpressions (symbol, a top-level function))

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 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 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; 102 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 — 5 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 6b7d709b8776fa2340fe7b4ce6e9b601ae256fe2packageMentionDocs.

Which tree this was computed on

This run read content/docs from 7f8c34cfaac727343134d565a6ab7fb23a1b7914 — the merge of head d4055961f71787ef9565653f6ce41e0bdf3508b4 into base 6b7d709b8776fa2340fe7b4ce6e9b601ae256fe2, 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 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

⚠️ 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 6b7d709b8776fa2340fe7b4ce6e9b601ae256fe2 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

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.

lint: validateStackExpressions throws on a non-record entry of a flow's nodes list — two inline casts no collection sweep can reach

2 participants