fix(spec): make what collectFlowGraphs RETURNS match its declared FlowNodeParsed[]

.changeset/collect-flow-graphs-drops-non-record-members.md

@@ -0,0 +1,11 @@
+---
+"@objectstack/spec": patch
+---
+
+`collectFlowGraphs` now honours the `readonly FlowNodeParsed[]` it declares: a member of a region's node list that is not a record is dropped from the `FlowGraph` it hands out, instead of being passed through verbatim.
+
+An ADR-0031 container keeps a whole sub-graph inside `FlowNodeSchema.config`, a deliberately open `z.record`, so `collectFlowGraphs` re-derives those inner node lists at run time and checks them with `Array.isArray` — which proves the LIST and never its MEMBERS. An empty item in a YAML `nodes:` list under a `loop` body deserialises to `null`, and that `null` reached `graph.nodes` on every returned graph, at every depth. No caller could prevent it: this is an array the walk picks up itself, so no coercion at a call site ever holds it. Filed as #16752.
+
+- **What changed.** The walk filters what it hands out and skips what it descends into, through one predicate. Array identity is preserved when nothing is dropped, so a well-formed flow allocates nothing new.
+- **What deliberately did NOT change.** The declared input type is untouched — widening it to tolerate malformed members was refused on the anti-AI-error axis, and this is the opposite move: the producer now keeps the promise it already made. The schema refusal that owns a malformed region still fires, unchanged; this walk runs inside `FlowSchema`'s parse, where a thrown `TypeError` would escape `safeParse`, so the repair is a drop and a skip and never a throw. `FlowGraph.path` still indexes the raw authored list, so a Zod issue stays anchored where the author wrote the node.
+- **Visible consequence.** As with the sibling repairs that read their lists through a record filter, a dropped member renumbers the ones behind it *within* `graph.nodes` — a difference in the index, never in whether a node was judged, and only in a list that was already malformed.

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

@@ -330,9 +330,11 @@ const underFlow = (key: string, valid?: AnyRec): SweptCollection => ({
  * and `collectFlowGraphs` turns each into its own `FlowGraph` after checking
  * only `Array.isArray` on the inner list — so a non-record member here is one
  * the PRODUCER picked up, not one a caller passed in, and no coercion at the
- * call site can reach it. Kept in the sweep with its throw recorded below
- * rather than left unexpressed: an unaddressable shape is exactly what let this
- * class survive three closures.
+ * call site can reach it. That is what #16752 repaired, at the producer: the
+ * walk now drops a non-record member from the graph it hands out, and this arm
+ * carries no `RESIDUAL_THROWS` row. It stays in the sweep as the pin on that
+ * repair — an unaddressable shape is exactly what let this class survive three
+ * closures, so the addressing is the part worth keeping.
  */
 const underNestedRegion = (valid?: AnyRec): SweptCollection => ({
   label: 'flows[].nodes[].config.body.nodes',
@@ -399,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.
  *
- * 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,36 +415,38 @@ 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[].config.body.nodes` / `validateStackExpressions` +
+ *    `lintFlowPatterns` (#16752) — neither rule's own reader was ever at fault
+ *    here, and neither was repaired: 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
+ *    out, which is what #15793 predicted when it refused to widen the
+ *    `packages/spec` contract and filed the fork instead. ⛔ Note what did NOT
+ *    fix it: #16134 had already stopped that walk DEREFERENCING the member, and
+ *    both rows survived it — a guard against reading junk is not a guard
+ *    against passing it on.
  *
- * ## The rows it holds today, both found by the arms that added them
+ * ## The row it holds 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.
+ * 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.
- *  - `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.
+ *    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.
  */
 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'],
-  // 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'],
 };
 
 /**

packages/spec/src/automation/control-flow.zod.ts

@@ -486,7 +486,12 @@ export function findRegionEntry(region: { nodes: FlowNodeParsed[]; edges?: FlowE
 
 // ─── Where the containers keep their regions ─────────────────────────
 
-/** A dict — region-shaped enough to reach its `nodes` / `edges`. */
+/**
+ * A dict — region-shaped enough to reach its `nodes` / `edges`, and the same
+ * test a member of a node list must pass to be a node at all. One spelling for
+ * both, so what {@link collectFlowGraphs} walks cannot drift from what it hands
+ * out.
+ */
 function isRegionDict(v: unknown): v is Record<string, unknown> {
   return typeof v === 'object' && v !== null && !Array.isArray(v);
 }
@@ -699,6 +704,16 @@ export interface FlowGraph {
    * `[...path, 'nodes', i, 'id']` — rather than described in prose (#16134).
    */
   readonly path: readonly (string | number)[];
+  /**
+   * Every member is a record. A node list read out of a container's open
+   * `z.record` config can hold whatever the author typed — an empty YAML list
+   * item deserialises to `null` — and a region its own schema refused is left
+   * RAW for {@link validateControlFlow} to name. The walk therefore drops a
+   * non-record member rather than hand out an array that does not match this
+   * declared type (#16752). Only this array is narrowed: the schema refusal
+   * that owns the malformed region still fires, and {@link path} still indexes
+   * the RAW list, so a finding stays anchored where the author wrote it.
+   */
   readonly nodes: readonly FlowNodeParsed[];
   readonly edges: readonly FlowEdgeParsed[];
 }
@@ -727,18 +742,31 @@ export function collectFlowGraphs(
     path: readonly (string | number)[],
     depth: number,
   ): void => {
-    graphs.push({ scope, path, nodes, edges });
+    // A region its own schema refused is left RAW by `parseFlowNodeRegions` for
+    // `validateControlFlow` to name, so an element here can be whatever the
+    // author typed — `null` included. What is HANDED OUT and what is WALKED both
+    // drop it, through the one predicate above.
+    //
+    // Handed out (#16752): `FlowGraph.nodes` is declared `readonly
+    // FlowNodeParsed[]`, and an array whose members every caller must re-check
+    // is not that array. This list is one the walk picked up out of an open
+    // `z.record` config ITSELF — no caller ever held it, so no coercion at a
+    // call site can reach it. Identity is preserved when nothing is dropped.
+    //
+    // Walked (#16134): skip a non-record rather than read `.config` off it —
+    // this walk runs inside `FlowSchema`'s parse, where a thrown TypeError would
+    // escape `safeParse`, which is why this is a skip and not a throw. The schema
+    // refusal that owns the malformed region still fires, reached now where the
+    // throw used to pre-empt it.
+    const kept = nodes.filter((node) => isRegionDict(node));
+    graphs.push({ scope, path, nodes: kept.length === nodes.length ? nodes : kept, edges });
     if (depth >= MAX_REGION_DEPTH) return;
+    // Indexed over the RAW list, never `kept`: `path` anchors a Zod issue where
+    // the author wrote the node, so dropping a member must not renumber the
+    // siblings that outlive it. `Array.isArray` on the inner list below proves
+    // the LIST, never its MEMBERS — the sentence removed from four lint readers.
     nodes.forEach((node, index) => {
-      // A region its own schema refused is left RAW by `parseFlowNodeRegions`
-      // for `validateControlFlow` to name, so an element here can be whatever
-      // the author typed — `null` included. Skip what is not a node object
-      // rather than read `.config` off it: this walk runs inside `FlowSchema`'s
-      // parse (#16134), where a thrown TypeError would escape `safeParse`. The
-      // schema refusal that owns the malformed region still fires — reached now,
-      // where the throw used to pre-empt it.
-      const raw: unknown = node;
-      if (raw === null || typeof raw !== 'object') return;
+      if (!isRegionDict(node)) return;
       for (const slot of regionSlotsOf(node)) {
         if (!isRegionDict(slot.raw) || !Array.isArray(slot.raw.nodes)) continue;
         visit(

packages/spec/src/automation/region-normalization.test.ts

@@ -279,4 +279,113 @@ describe('#4347 — collectFlowGraphs', () => {
     expect(graphs.length).toBeGreaterThan(1);
     expect(graphs.length).toBeLessThan(64);
   });
+
+  /**
+   * #16752 — what the walk HANDS OUT matches its declared
+   * `readonly FlowNodeParsed[]`.
+   *
+   * The list in question is one `collectFlowGraphs` picks up ITSELF, out of a
+   * container's open `z.record` config, and casts after an `Array.isArray` that
+   * proves the LIST and never its MEMBERS — the sentence #15552 / #15636 /
+   * #15742 / #15793 removed from four lint readers. No caller ever holds this
+   * array, so no coercion at a call site can reach it; the guard belongs here.
+   *
+   * #16134 already stopped the walk DEREFERENCING a non-record member (a
+   * `TypeError` thrown here escapes `FlowSchema.safeParse` rather than becoming
+   * an issue). It did not stop the walk HANDING IT OUT: `nodes` was pushed
+   * verbatim, so every graph over a junk-bearing list carried the junk — at
+   * every depth, not only at the `MAX_REGION_DEPTH` ceiling the filing found.
+   *
+   * ⛔ The repair is a drop, never a looser signature: widening the declared
+   * type to tolerate malformed members is the direction #15793 refused on the
+   * anti-AI-error axis. As with the four repairs above, a dropped member
+   * renumbers the ones behind it in `graph.nodes` — a difference in the index,
+   * never in whether a node was judged; `graph.path`, which anchors a Zod issue
+   * where the author wrote it, is pinned below to stay indexed over the RAW
+   * list.
+   */
+  describe('#16752 — a non-record member never reaches a returned graph', () => {
+    /**
+     * The five shapes a raw node list holds that are not a node. `null` is the
+     * one an author writes by accident (an empty YAML list item deserialises to
+     * it); `an array` is the one a bare `typeof x === 'object'` test admits, so
+     * it pins that the drop is a record test and not an object test.
+     */
+    const NON_NODES: readonly (readonly [string, unknown])[] = [
+      ['null', null],
+      ['undefined', undefined],
+      ['a string', 'x'],
+      ['a number', 42],
+      ['an array', []],
+    ];
+
+    const isRecord = (v: unknown): boolean => typeof v === 'object' && v !== null && !Array.isArray(v);
+
+    /**
+     * `depth` nested loop bodies, the innermost holding `junk` beside two real
+     * nodes. Hand-built rather than parsed on purpose: a region its own schema
+     * refuses is left RAW by `parseFlowNodeRegions`, and raw is the state this
+     * walk has to survive.
+     */
+    const nestedJunk = (depth: number, junk: unknown) => {
+      let region: Record<string, unknown> = {
+        nodes: [junk, { ...gate, id: 'gate_in' }, { ...write, id: 'write_in' }],
+        edges: [],
+      };
+      for (let i = depth; i > 0; i--) {
+        region = { nodes: [loopWith(region, `lp${i}`)], edges: [] };
+      }
+      return { nodes: region.nodes as never, edges: [] };
+    };
+
+    describe.each(NON_NODES)('with %s in the innermost body', (_label, junk) => {
+      // 0 is the flow's own list, 1 the shape the card reproduced, and 32 the
+      // depth ceiling — where `visit` pushes a graph and returns without ever
+      // walking its members, so the junk was handed out with nothing having
+      // looked at it.
+      it.each([0, 1, 32])('hands out only records at nesting %i', (depth) => {
+        const graphs = collectFlowGraphs(nestedJunk(depth, junk));
+        expect(graphs.flatMap(g => g.nodes).filter(n => !isRecord(n))).toEqual([]);
+      });
+
+      it('still hands out the real nodes standing beside it', () => {
+        // Anti-vacuity: the drop takes what cannot be read, not the list. A
+        // guard that emptied every graph would pass the assertion above.
+        const graphs = collectFlowGraphs(nestedJunk(1, junk));
+        expect(graphs[graphs.length - 1]!.nodes.map(n => n.id)).toEqual(['gate_in', 'write_in']);
+      });
+
+      it('lets `FlowSchema.safeParse` return an envelope rather than throw (#16134)', () => {
+        // This walk runs inside the parse, so the repair has to stay a drop and
+        // a skip; a throw here escapes `safeParse` instead of becoming an issue.
+        const result = FlowSchema.safeParse({
+          name: 'repro', label: 'Repro', type: 'schedule',
+          nodes: [
+            { id: 'start', type: 'start', label: 'Start' },
+            loopWith({ nodes: [junk], edges: [] }),
+          ],
+          edges: [],
+        });
+        expect(typeof result.success).toBe('boolean');
+      });
+    });
+
+    it('leaves `path` indexed over the RAW list, so a finding stays where the author wrote it', () => {
+      // The container is at authored index 1 whether or not a non-record
+      // precedes it: dropping a member must not renumber its siblings in the
+      // key path a Zod issue is anchored on (#16134).
+      const graphs = collectFlowGraphs({
+        nodes: [null, loopWith(gatedRegion())] as never,
+        edges: [],
+      });
+      expect(graphs.map(g => g.path)).toEqual([[], ['nodes', 1, 'config', 'body']]);
+    });
+
+    it('hands back the very same array when there is nothing to drop', () => {
+      // Copy-on-write, as `parseFlowNodeRegions` is: a well-formed flow pays
+      // nothing for the guard.
+      const nodes = [{ ...gate }, { ...write }];
+      expect(collectFlowGraphs({ nodes: nodes as never, edges: [] })[0]!.nodes).toBe(nodes);
+    });
+  });
 });