fix(lint): read every flow node list through recordsOf

.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.

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<string, unknown>;
 
 /** 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;

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

packages/lint/src/non-record-object-entry.test.ts

@@ -401,7 +401,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
+ * Five 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:
  *
@@ -416,8 +416,7 @@ const SWEPT_COLLECTIONS: readonly SweptCollection[] = [
  *  - `flows[].nodes` / `validateStackExpressions` — the two casts #15793
  *    repaired, and the reason the two graph-shaped arms below exist at all.
  *  - `flows[].nodes[].config.body.nodes` / `validateStackExpressions` +
- *    `lintFlowPatterns` (#16752) — neither rule's own reader was ever at fault
- *    here, and neither was repaired: the throw was `collectFlowGraphs`'
+ *    `lintFlowPatterns` (#16752) — the throw was `collectFlowGraphs`'
  *    (`packages/spec`), which handed out a `FlowGraph` whose `nodes` held the
  *    junk member it had picked up out of a container's open `z.record` config.
  *    Both rules stopped throwing the moment the PRODUCER stopped handing it
@@ -427,27 +426,44 @@ const SWEPT_COLLECTIONS: readonly SweptCollection[] = [
  *    both rows survived it — a guard against reading junk is not a guard
  *    against passing it on.
  *
- * ## The row it holds today
+ *    ⭐ These two arms are now green for TWO independent reasons, and both were
+ *    measured. #16751 landed alongside and re-pointed the two CONSUMER readers
+ *    this shape reached — `lint-flow-patterns.ts`' own `graph.nodes` reader and
+ *    `collectFlowVariableNames`' unguarded `graph.nodes` walk — and on the tree
+ *    before either fix those were the frames the throws carried, one in each
+ *    rule, neither inside `packages/spec`. So "neither rule's own reader was at
+ *    fault" is the wrong reading of this pair in one direction only: the
+ *    producer's declared `FlowNodeParsed[]` really did lie about what it
+ *    returned, AND the consumers really did dereference without a guard.
+ *    Neither repair makes the other unnecessary — remove the producer fix and
+ *    the junk is handed out again to every other consumer; remove the consumer
+ *    coercion and these two readers are back to trusting a declared element
+ *    type. Read the pair as belt and braces, ⛔ not as one change that turned
+ *    out to be redundant.
+ *  - `flows[].nodes` / `lintFlowPatterns` (#16751) — 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`, with the COERCED array — never `flow.nodes` raw — handed on
+ *    to `collectFlowGraphs`, measured: hand the raw one on and the crash
+ *    RELOCATES to the graph-node reader instead of going away. This is the
+ *    shallow half, and the producer repair above never covered it: `flow.nodes`
+ *    is a list the RULE reads itself, before any producer sees 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. The graph-shaped pair is gone; the shallow one is not.
- *
- *  - `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. The two
- *    `graph.nodes` casts are covered from the producer side since #16752, so
- *    what is left to repair here is the `flow.nodes` read the rule does itself.
+ * addressing mode away. The graph-shaped pair went out when the producer
+ * stopped handing a junk member out, the shallow one when the last flow-node
+ * list reader stopped trusting `Array.isArray` about members, 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<Record<string, readonly string[]>> = {
-  // 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'],
-};
+const RESIDUAL_THROWS: Readonly<Record<string, readonly string[]>> = {};
 
 /**
  * Where a junk member still draws a finding no author's file justifies — the

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<string> {
-  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);