From 91878c45daf5a8014d7a2e65646150dea99db7e5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 16:02:24 +0000 Subject: [PATCH 1/2] fix(lint): read every flow node list through `recordsOf` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `lintFlowPatterns` threw an uncaught `TypeError` on an ordinary flow whose `nodes:` list carried an empty YAML item: `Array.isArray` proves the list, never its members, and `nodes.find(n => n.type === 'start')` dereferenced the `null`. Seven readers across three modules held that spelling; all now coerce through `recordsOf`, the single home for this decision. The load-bearing half is which array is handed onward: `collectFlowGraphs` is transparent about members, so the COERCED array — not `flow.nodes` raw — is what it receives, or the crash relocates into `packages/spec` instead of going away. Its two `graph.nodes` readers are coerced too, because a nested region's list reaches them with only an `Array.isArray` behind it. `collectFlowVariableNames` guarded its `flow.variables` members and not its `graph.nodes` members, seven lines apart in one function; that half was reachable only at a region nest of exactly `MAX_REGION_DEPTH`. The three `validate-flow-template-paths.ts` readers were NOT throwing — they survived on an optional chain in the `.find` predicate, one character away from the reader that did. That half is a hardening. `recordsOf` gains no copy: the declaration count over `packages/lint/src` is unchanged at three (the canonical one plus the two ledgered). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012GKcPZbMoGq7WPzKLfRBTU --- .changeset/lint-flow-node-list-recordsof.md | 20 +++++++++++++ packages/lint/src/flow-variable-scope.ts | 11 +++++-- packages/lint/src/lint-flow-patterns.ts | 29 +++++++++++++++---- .../lint/src/non-record-object-entry.test.ts | 24 +++++++-------- .../lint/src/validate-flow-template-paths.ts | 18 ++++++++---- 5 files changed, 77 insertions(+), 25 deletions(-) create mode 100644 .changeset/lint-flow-node-list-recordsof.md diff --git a/.changeset/lint-flow-node-list-recordsof.md b/.changeset/lint-flow-node-list-recordsof.md new file mode 100644 index 0000000000..0d7b2067f1 --- /dev/null +++ b/.changeset/lint-flow-node-list-recordsof.md @@ -0,0 +1,20 @@ +--- +"@objectstack/lint": patch +--- + +Flow-node-list readers no longer throw on a non-record member — `lintFlowPatterns`, `collectFlowVariableNames` and the three record-change template-path readers now coerce through `recordsOf`. + +`lintFlowPatterns` crashed on an ordinary flow. A YAML `nodes:` list item left empty deserialises to `null`, and the rule read `nodes.find(n => n.type === 'start')` off a list it had only `Array.isArray`-checked, so an author's own metadata turned `objectstack validate` into an uncaught `TypeError` out of a function contractually typed `(stack) => Finding[]`: + +``` +TypeError: Cannot read properties of null (reading 'type') + at lint-flow-patterns.ts:1430 +``` + +`Array.isArray` proves the LIST, never its MEMBERS — the same sentence removed from `validate-expressions.ts` one file over. All seven readers now go through `recordsOf` (`object-graph.ts`), which stays the single home for this coercion; no new copy of the predicate is declared. + +- **`lintFlowPatterns`** — `flow.nodes` is coerced once, and that coerced array is what is handed on to `collectFlowGraphs`. That second half is the load-bearing one: `collectFlowGraphs` is transparent about members (it forwards the caller's array and re-exposes the same objects), so coercing only for the local read would have moved the crash into `packages/spec` rather than removing it. Its two `graph.nodes` readers are coerced as well, because a nested region's node list reaches them with only an `Array.isArray` behind it. +- **`collectFlowVariableNames`** — the `graph.nodes` walk had no member guard while the `flow.variables` walk seven lines above it did. Reachable today only at a region nest of exactly `MAX_REGION_DEPTH`; it now cannot throw at any depth. +- **The three record-change template-path readers** (`boundObjectOf`, `declaredExpandOf` and the per-flow start lookup in `validateFlowTemplatePaths`) were **not** throwing. They survived on an optional chain in the `.find` predicate — one character's difference from the reader that did throw, maintained by nothing and looking redundant next to the `Array.isArray` above it. They are coerced for the same reason and the optional chain goes with it. This half is a hardening, not a bug fix. + +A malformed member is dropped, in silence, exactly as `recordsOf` drops one everywhere else; the valid nodes standing beside it are still judged and the findings a flow draws are unchanged. diff --git a/packages/lint/src/flow-variable-scope.ts b/packages/lint/src/flow-variable-scope.ts index 2202963845..948eaee28f 100644 --- a/packages/lint/src/flow-variable-scope.ts +++ b/packages/lint/src/flow-variable-scope.ts @@ -69,6 +69,8 @@ import { firstUndeclaredReference } from '@objectstack/formula'; +import { recordsOf } from './object-graph.js'; + type AnyRec = Record; /** The node shape this module reads: `collectFlowGraphs`' element type, loosened. */ @@ -219,8 +221,13 @@ export function collectFlowVariableNames( } for (const graph of graphs) { - for (const item of graph.nodes) { - const flowNode = item as AnyRec; + // `recordsOf`, not a bare cast (#16751). Row 1 above guards each + // `flow.variables` member with `if (!item || typeof item !== 'object')` + // seven lines up; this loop did not, so a non-record region node made + // `flowNode.id` throw out of a collector that is contractually total. + // `collectFlowGraphs` only `Array.isArray`-checks a nested region's list, + // so the members reaching here carry no promise from the producer either. + for (const flowNode of recordsOf(graph.nodes)) { // Row 8 — the node id itself. if (typeof flowNode.id === 'string' && flowNode.id) names.add(flowNode.id); const rawConfig = flowNode.config; diff --git a/packages/lint/src/lint-flow-patterns.ts b/packages/lint/src/lint-flow-patterns.ts index f73cc7ef94..81e48eadb1 100644 --- a/packages/lint/src/lint-flow-patterns.ts +++ b/packages/lint/src/lint-flow-patterns.ts @@ -447,13 +447,19 @@ function findDataNodeAnywhere( edges: AnyRec[], ): { readonly node: AnyRec; readonly scope: string } | null { // A cast, not a parse — same contract as the main per-graph walk below: the - // walk touches only `type` / `config`, and the guarded arrays are passed so a - // malformed region cannot make this throw (this module never throws). + // walk touches only `type` / `config`, and the arrays handed in are the ones + // the caller already coerced through `recordsOf`, so a malformed member + // cannot make this throw (this module never throws). for (const graph of collectFlowGraphs({ nodes: nodes as unknown as FlowNodeParsed[], edges: edges as unknown as FlowEdgeParsed[], })) { - for (const node of graph.nodes as unknown as AnyRec[]) { + // `recordsOf`, not `as unknown as AnyRec[]` (#16751). The top-level list is + // clean by the caller's coercion, but a NESTED region's node list is only + // `Array.isArray`-checked by `collectFlowGraphs` before it becomes a graph + // — it carries the producer's word about its members, not a check. Same + // decision made once more where that guarantee stops, through the one home. + for (const node of recordsOf(graph.nodes)) { if (DATA_NODE_TYPES.has(typeof node.type === 'string' ? (node.type as string) : '')) { return { node, scope: graph.scope }; } @@ -1423,7 +1429,16 @@ export function lintFlowPatterns(stack: AnyRec): FlowLintFinding[] { const findings: FlowLintFinding[] = []; for (const flow of recordsOf(stack.flows)) { const flowName = typeof flow.name === 'string' ? flow.name : '(unnamed flow)'; - const nodes = Array.isArray(flow.nodes) ? (flow.nodes as AnyRec[]) : []; + // `Array.isArray` proves the LIST, never its MEMBERS. A YAML `nodes:` item + // left empty deserialises to `null`, and `nodes.find(n => n.type === …)` + // four lines down dereferenced it (#16751). Read through `recordsOf` — the + // one home for this coercion (`object-graph.ts`) — and note that THIS array + // is also what goes to `collectFlowGraphs` below, never `flow.nodes` raw: + // that producer is transparent about members (it forwards the caller's + // array and re-exposes the same objects), so coercing only for the local + // read relocates the crash into `packages/spec` instead of removing it — + // measured on #15793, and measured again here. + const nodes = recordsOf(flow.nodes); const edges = Array.isArray(flow.edges) ? (flow.edges as AnyRec[]) : []; // (a) #1874 — date-equality time condition on a record-change start node. @@ -1519,7 +1534,11 @@ export function lintFlowPatterns(stack: AnyRec): FlowLintFinding[] { edges: edges as unknown as FlowEdgeParsed[], })) { const at = graph.scope ? `flow '${flowName}' · ${graph.scope}` : `flow '${flowName}'`; - const graphNodes = graph.nodes as unknown as AnyRec[]; + // `recordsOf`, not `as unknown as AnyRec[]` (#16751) — the same reason as + // in `findDataNodeAnywhere`: the top-level graph is clean by the coercion + // at the call site above, and a nested region's node list arrives here + // with only `Array.isArray` behind it. + const graphNodes = recordsOf(graph.nodes); const graphEdges = graph.edges as unknown as AnyRec[]; // (b) #1315 — wrong interpolation syntax in any node's template values. Flow diff --git a/packages/lint/src/non-record-object-entry.test.ts b/packages/lint/src/non-record-object-entry.test.ts index f6e9edc766..9c58de47b2 100644 --- a/packages/lint/src/non-record-object-entry.test.ts +++ b/packages/lint/src/non-record-object-entry.test.ts @@ -399,7 +399,7 @@ const SWEPT_COLLECTIONS: readonly SweptCollection[] = [ * "nothing throws" would have had to be deleted or weakened on the day it was * written, and would then never have caught the next one. * - * Three rows have come out since it was written, each because the sweep went + * Four rows have come out since it was written, each because the sweep went * red demanding a throw that no longer happens — which is the both-directions * half earning its keep, since no removal started with anyone going looking: * @@ -413,19 +413,23 @@ const SWEPT_COLLECTIONS: readonly SweptCollection[] = [ * field readers already did. * - `flows[].nodes` / `validateStackExpressions` — the two casts #15793 * repaired, and the reason the two graph-shaped arms below exist at all. + * - `flows[].nodes` / `lintFlowPatterns` — the SAME two spellings one file + * over. `lint-flow-patterns.ts` inline-cast `flow.nodes` and then read + * `.type` off each member, and double-cast `graph.nodes` at two further + * readers; `flow-variable-scope.ts` walked `graph.nodes` with no member + * guard while guarding `flow.variables` seven lines up. All re-pointed at + * `recordsOf` by #16751, with the COERCED array — never `flow.nodes` raw — + * handed on to `collectFlowGraphs`, so the crash is removed rather than + * relocated into `packages/spec`. * - * ## The rows it holds today, both found by the arms that added them + * ## The row it holds today, found by the arm that added it * * It went from empty to two the moment a flow's inner node list became * addressable, which is the point #15793 was filed to make: this class was * closed three times over collections while the same defect stood untouched one - * addressing mode away. + * addressing mode away. #16751 has since taken the consumer-side half back out; + * what stands below is the producer-side one. * - * - `flows[].nodes` / `lintFlowPatterns` (#16751) — `lint-flow-patterns.ts` - * holds the SAME two spellings #15793 removed from `validate-expressions.ts` - * (`:1426` inline-casts `flow.nodes`, then `:1430` reads `.type` off each - * member; `:456` and `:1522` double-cast `graph.nodes`). Shallowly - * reachable — an ordinary flow with an empty YAML list item. * - `flows[].nodes[].config.body.nodes` / `validateStackExpressions` + * `lintFlowPatterns` (#16752) — neither rule's own reader is at fault here: * both throw from INSIDE `collectFlowGraphs`, whose region walk reads @@ -435,10 +439,6 @@ const SWEPT_COLLECTIONS: readonly SweptCollection[] = [ * `packages/spec` contract to tolerate malformed members. */ const RESIDUAL_THROWS: Readonly> = { - // 2026-09-08 — #16751. Removed when `lint-flow-patterns.ts` reads its node - // lists through `recordsOf`, as `validate-expressions.ts` now does. - 'flows[].nodes · null': ['lintFlowPatterns'], - 'flows[].nodes · undefined': ['lintFlowPatterns'], // 2026-09-08 — #16752. Both entries are ONE defect in `collectFlowGraphs`, // surfacing through the two rules that call it. Removed together. 'flows[].nodes[].config.body.nodes · null': ['lintFlowPatterns', 'validateStackExpressions'], diff --git a/packages/lint/src/validate-flow-template-paths.ts b/packages/lint/src/validate-flow-template-paths.ts index e1a9575766..9540d7638b 100644 --- a/packages/lint/src/validate-flow-template-paths.ts +++ b/packages/lint/src/validate-flow-template-paths.ts @@ -253,8 +253,14 @@ function isRecordTriggered(flow: AnyRec, startConfig: AnyRec): boolean { /** Resolve the object a record-change flow binds to, from its start node. */ function boundObjectOf(flow: AnyRec): string | undefined { - const nodes = Array.isArray(flow.nodes) ? (flow.nodes as AnyRec[]) : []; - const start = nodes.find((n) => n?.type === 'start'); + // `recordsOf`, not `Array.isArray` + cast (#16751). This site and the two + // below never threw — but only because each `.find` predicate happens to be + // spelled `n?.type`, one character away from the reader that did throw in + // `lint-flow-patterns.ts`. Nothing maintained that difference, and the `?.` + // reads as redundant beside an `Array.isArray`, so the coercion is made where + // it has a home and the optional chain goes with it. + const nodes = recordsOf(flow.nodes); + const start = nodes.find((n) => n.type === 'start'); if (!start) return undefined; const config = (start.config ?? {}) as AnyRec; const typed = (start.start ?? {}) as AnyRec; @@ -271,8 +277,8 @@ function boundObjectOf(flow: AnyRec): string | undefined { * or `string[]`; anything else yields the empty set. */ function declaredExpandOf(flow: AnyRec): Set { - const nodes = Array.isArray(flow.nodes) ? (flow.nodes as AnyRec[]) : []; - const start = nodes.find((n) => n?.type === 'start'); + const nodes = recordsOf(flow.nodes); + const start = nodes.find((n) => n.type === 'start'); const raw = ((start?.config ?? {}) as AnyRec).expand; if (typeof raw === 'string') return new Set(raw ? [raw] : []); if (Array.isArray(raw)) return new Set(raw.filter((r): r is string => typeof r === 'string' && r.length > 0)); @@ -295,8 +301,8 @@ export function validateFlowTemplatePaths(stack: AnyRec): FlowTemplatePathFindin flows.forEach((flow, flowIndex) => { const flowName = typeof flow.name === 'string' ? flow.name : `#${flowIndex}`; - const nodes = Array.isArray(flow.nodes) ? (flow.nodes as AnyRec[]) : []; - const start = (nodes.find((n) => n?.type === 'start')?.config ?? {}) as AnyRec; + const nodes = recordsOf(flow.nodes); + const start = (nodes.find((n) => n.type === 'start')?.config ?? {}) as AnyRec; if (!isRecordTriggered(flow, start)) return; const objectName = boundObjectOf(flow); From ea8564d0014aba8f1950c4e4c40327629cb70d9a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 16:22:24 +0000 Subject: [PATCH 2/2] =?UTF-8?q?test(lint):=20empty=20`RESIDUAL=5FTHROWS`?= =?UTF-8?q?=20=E2=80=94=20the=20nested-region=20rows=20were=20misattribute?= =?UTF-8?q?d?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-pointing the flow-node-list readers turned the sweep's `flows[].nodes[].config.body.nodes` arm red, demanding two throws that no longer happen. Reading the frames off the reverted tree showed why: neither was in `packages/spec`. `lintFlowPatterns` threw at its own `graph.nodes` reader (`lint-flow-patterns.ts:1529` on the reverted tree) and `validateStackExpressions` threw at `collectFlowVariableNames`' unguarded `graph.nodes` walk (`flow-variable-scope.ts:225`) — the two consumer sites this branch re-pointed. `collectFlowGraphs` forwards a non-record member of a nested list into the graph it yields rather than dereferencing it. So the rows had to go: the ratchet is exact in both directions and a row that demands a throw nobody performs is a lie. The docblock records the corrected attribution, and records just as plainly that this says nothing about whether the producer has a defect of its own reachable some other way. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012GKcPZbMoGq7WPzKLfRBTU --- .../lint/src/non-record-object-entry.test.ts | 41 +++++++++++-------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/packages/lint/src/non-record-object-entry.test.ts b/packages/lint/src/non-record-object-entry.test.ts index 9c58de47b2..b45c724acd 100644 --- a/packages/lint/src/non-record-object-entry.test.ts +++ b/packages/lint/src/non-record-object-entry.test.ts @@ -399,7 +399,7 @@ const SWEPT_COLLECTIONS: readonly SweptCollection[] = [ * "nothing throws" would have had to be deleted or weakened on the day it was * written, and would then never have caught the next one. * - * Four rows have come out since it was written, each because the sweep went + * Six rows have come out since it was written, each because the sweep went * red demanding a throw that no longer happens — which is the both-directions * half earning its keep, since no removal started with anyone going looking: * @@ -421,29 +421,34 @@ const SWEPT_COLLECTIONS: readonly SweptCollection[] = [ * `recordsOf` by #16751, with the COERCED array — never `flow.nodes` raw — * handed on to `collectFlowGraphs`, so the crash is removed rather than * relocated into `packages/spec`. + * - `flows[].nodes[].config.body.nodes` / `lintFlowPatterns` + + * `validateStackExpressions` — written down as ONE defect *inside* + * `collectFlowGraphs`, on the reading that its region walk dereferences a + * member of an inner list it had only `Array.isArray`-checked, and filed on + * that basis. Re-pointing the readers above turned this row red demanding + * two throws that no longer happen, and reading the frames off the reverted + * tree showed the ATTRIBUTION was wrong: neither was in `packages/spec`. + * `lintFlowPatterns` threw at its own `graph.nodes` reader, and + * `validateStackExpressions` threw at `collectFlowVariableNames`' unguarded + * `graph.nodes` walk — the two consumer sites #16751 re-pointed. + * `collectFlowGraphs` FORWARDS a non-record member of a nested list into the + * graph it yields rather than dereferencing it, so at this shape the crash + * was always the consumer's. That is a statement about this shape and this + * rule table only: it does not say the producer has no defect of its own + * reachable some other way, and nothing here closes that question. * - * ## The row it holds today, found by the arm that added it + * ## It holds no row today * * It went from empty to two the moment a flow's inner node list became * addressable, which is the point #15793 was filed to make: this class was * closed three times over collections while the same defect stood untouched one - * addressing mode away. #16751 has since taken the consumer-side half back out; - * what stands below is the producer-side one. - * - * - `flows[].nodes[].config.body.nodes` / `validateStackExpressions` + - * `lintFlowPatterns` (#16752) — neither rule's own reader is at fault here: - * both throw from INSIDE `collectFlowGraphs`, whose region walk reads - * `node.config` off a member of an inner list it checked only with - * `Array.isArray`. No coercion at either call site reaches that list, which - * is why #15793 stopped and filed the fork instead of widening a - * `packages/spec` contract to tolerate malformed members. + * addressing mode away. Both rows came back out on the change that re-pointed + * the last of the flow-node-list readers, and the table is empty again. Empty + * is this ratchet's resting state, not its retirement: it stays exact in both + * directions, so a rule that starts throwing on any swept collection reds here + * because it is not listed. */ -const RESIDUAL_THROWS: Readonly> = { - // 2026-09-08 — #16752. Both entries are ONE defect in `collectFlowGraphs`, - // surfacing through the two rules that call it. Removed together. - 'flows[].nodes[].config.body.nodes · null': ['lintFlowPatterns', 'validateStackExpressions'], - 'flows[].nodes[].config.body.nodes · undefined': ['lintFlowPatterns', 'validateStackExpressions'], -}; +const RESIDUAL_THROWS: Readonly> = {}; /** * Where a junk member still draws a finding no author's file justifies — the