Skip to content

Commit d91dff4

Browse files
claude[bot]claude
andauthored
fix(lint): read a flow's node lists through recordsOf, not an inline cast (#16765)
`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. Claude-Session: https://claude.ai/code/session_012GKcPZbMoGq7WPzKLfRBTU Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1167b4c commit d91dff4

3 files changed

Lines changed: 130 additions & 9 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
"@objectstack/lint": patch
3+
---
4+
5+
`validateStackExpressions` no longer throws on a non-record entry of a flow's `nodes` list.
6+
7+
An empty item in a YAML `nodes:` list deserialises to `null`, so this is an authorable shape — the same one #15552, #15636 and #15742 closed for stack collections and for `objects[].fields`. Here it crashed the linter instead of producing a finding: `flow.nodes: [null, ...]` threw `Cannot read properties of null (reading 'type')`, which presents to an author as a broken tool rather than as a problem with their metadata.
8+
9+
Both of the file's inline casts now read through `recordsOf`, the one home of this coercion, instead of asserting that `Array.isArray` proves anything about a list's MEMBERS:
10+
11+
- The flow walk reads `flow.nodes` through `recordsOf`, and — the half that actually removes the crash — hands that coerced array to `collectFlowGraphs` rather than the raw flow. `collectFlowGraphs` declares its input as already-parsed `FlowNodeParsed[]` and is transparent about members, so passing raw authored metadata was calling it out of contract; coercing only the local variable relocated the throw into `@objectstack/spec` instead of ending it. The producer's contract is unchanged, deliberately: widening it to tolerate malformed members is the wrong direction.
12+
- The per-graph walk reads `graph.nodes` through `recordsOf` in place of an `as unknown as` double cast. A nested region's node list is only `Array.isArray`-checked before it becomes a graph, so that list carries the producer's word about its members and not a check.
13+
14+
A non-record member is dropped whole and in silence, exactly as the file's sibling field readers already did; a flow standing beside the junk entry is still judged, and a `nodes` list holding a plain string still reports exactly what it reported before.

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

Lines changed: 94 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,60 @@ const underObject = (key: string, valid?: AnyRec): SweptCollection => ({
293293
valid,
294294
});
295295

296+
/**
297+
* A judgeable flow node: the `start` node the flow readers resolve the
298+
* record-change target against, so a control built from it is not merely empty.
299+
*/
300+
const VALID_NODE: AnyRec = { id: 'start', type: 'start', config: { objectName: 'crm_account' } };
301+
302+
/**
303+
* A flow's own node list — `stack.flows[].nodes` (#15793).
304+
*
305+
* ## Why this needed a THIRD addressing mode rather than one more row
306+
*
307+
* The two above address a collection by NAME: a top-level stack key, or one
308+
* sub-collection key on an object. A flow's node list is neither. It is the
309+
* `nodes` of a member of `stack.flows`, and #15793's whole diagnosis was that
310+
* this sweep *could not express* it — which is why #15552, #15636 and #15742
311+
* closed this defect class three times without ever reaching the two casts
312+
* #15793 repaired. The blind spot was in the ADDRESSING, not in the rule table.
313+
*
314+
* The constructor is a three-line sibling of `underObject` because
315+
* `SweptCollection.stack` was already an arbitrary builder; what was missing
316+
* was only the will to write a second shape of one. That is worth saying
317+
* plainly: the gap looked structural and was not.
318+
*/
319+
const underFlow = (key: string, valid?: AnyRec): SweptCollection => ({
320+
label: `flows[].${key}`,
321+
stack: (members) => ({ objects: [VALID_OBJECT], flows: [{ name: 'crm_flow', edges: [], [key]: members }] }),
322+
valid,
323+
});
324+
325+
/**
326+
* A NESTED region's node list — `flows[].nodes[].config.body.nodes` (#15793).
327+
*
328+
* The graph-shaped half proper, and a different reachability question from
329+
* `underFlow`. An ADR-0031 container keeps a whole sub-graph in its `config`,
330+
* and `collectFlowGraphs` turns each into its own `FlowGraph` after checking
331+
* only `Array.isArray` on the inner list — so a non-record member here is one
332+
* the PRODUCER picked up, not one a caller passed in, and no coercion at the
333+
* call site can reach it. Kept in the sweep with its throw recorded below
334+
* rather than left unexpressed: an unaddressable shape is exactly what let this
335+
* class survive three closures.
336+
*/
337+
const underNestedRegion = (valid?: AnyRec): SweptCollection => ({
338+
label: 'flows[].nodes[].config.body.nodes',
339+
stack: (members) => ({
340+
objects: [VALID_OBJECT],
341+
flows: [{
342+
name: 'crm_flow',
343+
edges: [],
344+
nodes: [VALID_NODE, { id: 'lp', type: 'loop', config: { collection: 'x', body: { nodes: members, edges: [] } } }],
345+
}],
346+
}),
347+
valid,
348+
});
349+
296350
const SWEPT_COLLECTIONS: readonly SweptCollection[] = [
297351
// Top-level, read through `recordsOf(stack.X)` by the re-pointed readers.
298352
topLevel('objects', VALID_OBJECT),
@@ -320,6 +374,11 @@ const SWEPT_COLLECTIONS: readonly SweptCollection[] = [
320374
topLevel('mappings'),
321375
// Top-level, read by `validateSecurityPosture` through `recordsOf`.
322376
topLevel('positions'),
377+
// A flow's inner graph — the shape no addressing mode could reach until
378+
// #15793 added one. `underFlow` is the flow's own list; `underNestedRegion`
379+
// is a container's sub-graph, which only the producer can hand out.
380+
underFlow('nodes', VALID_NODE),
381+
underNestedRegion(VALID_NODE),
323382
// Per-object sub-collections the same readers walk.
324383
underObject('fields', VALID_FIELD),
325384
underObject('actions'),
@@ -340,11 +399,9 @@ const SWEPT_COLLECTIONS: readonly SweptCollection[] = [
340399
* "nothing throws" would have had to be deleted or weakened on the day it was
341400
* written, and would then never have caught the next one.
342401
*
343-
* It is EMPTY today, and that is a measurement, not an aspiration: no rule in
344-
* the table throws on a non-record member of any collection swept here. Two
345-
* rows have come out since it was written, each because the sweep went red
346-
* demanding a throw that no longer happens — which is the both-directions half
347-
* earning its keep, since neither removal started with anyone going looking:
402+
* Three rows have come out since it was written, each because the sweep went
403+
* red demanding a throw that no longer happens — which is the both-directions
404+
* half earning its keep, since no removal started with anyone going looking:
348405
*
349406
* - `stack.datasets` — `indexDatasets` in `validate-chart-bindings.ts`,
350407
* re-pointed by #15741.
@@ -354,8 +411,39 @@ const SWEPT_COLLECTIONS: readonly SweptCollection[] = [
354411
* the list through `recordsOf` (#15742), which drops a non-record member of
355412
* the array shape whole and in silence, exactly as the file's two sibling
356413
* field readers already did.
414+
* - `flows[].nodes` / `validateStackExpressions` — the two casts #15793
415+
* repaired, and the reason the two graph-shaped arms below exist at all.
416+
*
417+
* ## The rows it holds today, both found by the arms that added them
418+
*
419+
* It went from empty to two the moment a flow's inner node list became
420+
* addressable, which is the point #15793 was filed to make: this class was
421+
* closed three times over collections while the same defect stood untouched one
422+
* addressing mode away.
423+
*
424+
* - `flows[].nodes` / `lintFlowPatterns` (#16751) — `lint-flow-patterns.ts`
425+
* holds the SAME two spellings #15793 removed from `validate-expressions.ts`
426+
* (`:1426` inline-casts `flow.nodes`, then `:1430` reads `.type` off each
427+
* member; `:456` and `:1522` double-cast `graph.nodes`). Shallowly
428+
* reachable — an ordinary flow with an empty YAML list item.
429+
* - `flows[].nodes[].config.body.nodes` / `validateStackExpressions` +
430+
* `lintFlowPatterns` (#16752) — neither rule's own reader is at fault here:
431+
* both throw from INSIDE `collectFlowGraphs`, whose region walk reads
432+
* `node.config` off a member of an inner list it checked only with
433+
* `Array.isArray`. No coercion at either call site reaches that list, which
434+
* is why #15793 stopped and filed the fork instead of widening a
435+
* `packages/spec` contract to tolerate malformed members.
357436
*/
358-
const RESIDUAL_THROWS: Readonly<Record<string, readonly string[]>> = {};
437+
const RESIDUAL_THROWS: Readonly<Record<string, readonly string[]>> = {
438+
// 2026-09-08 — #16751. Removed when `lint-flow-patterns.ts` reads its node
439+
// lists through `recordsOf`, as `validate-expressions.ts` now does.
440+
'flows[].nodes · null': ['lintFlowPatterns'],
441+
'flows[].nodes · undefined': ['lintFlowPatterns'],
442+
// 2026-09-08 — #16752. Both entries are ONE defect in `collectFlowGraphs`,
443+
// surfacing through the two rules that call it. Removed together.
444+
'flows[].nodes[].config.body.nodes · null': ['lintFlowPatterns', 'validateStackExpressions'],
445+
'flows[].nodes[].config.body.nodes · undefined': ['lintFlowPatterns', 'validateStackExpressions'],
446+
};
359447

360448
/**
361449
* Where a junk member still draws a finding no author's file justifies — the

packages/lint/src/validate-expressions.ts

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1124,7 +1124,11 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
11241124
// ── Flows ──────────────────────────────────────────────────────────
11251125
for (const flow of recordsOf(stack.flows)) {
11261126
const flowName = typeof flow.name === 'string' ? flow.name : '(unnamed flow)';
1127-
const nodes = Array.isArray(flow.nodes) ? (flow.nodes as AnyRec[]) : [];
1127+
// `Array.isArray` proves the LIST, never its MEMBERS — the sentence #15742
1128+
// removed from one reader below in this same file. A YAML `nodes:` item
1129+
// left empty deserialises to `null`, and `nodes.find(n => n.type === …)` on
1130+
// the very next line dereferenced it (#15793).
1131+
const nodes = recordsOf(flow.nodes);
11281132
// The record-change target object — `record.*` refs resolve against it.
11291133
const startNode = nodes.find(n => n.type === 'start');
11301134
const startCfg = (startNode?.config ?? {}) as AnyRec;
@@ -1137,7 +1141,16 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
11371141
// `objectstack validate` and shipped. This is the author-time half of the
11381142
// same traversal the engine's registration pass now does; `scope` names the
11391143
// region so the located message still points at one edge.
1140-
const graphs = collectFlowGraphs(flow as { nodes?: FlowNodeParsed[] });
1144+
//
1145+
// Handed the COERCED `nodes`, never `flow` raw (#15793). `collectFlowGraphs`
1146+
// declares its input as `FlowNodeParsed[]` — already-parsed nodes — and is
1147+
// transparent about members: it forwards the caller's array and re-exposes
1148+
// that same object. So it neither admits nor rejects a non-record member;
1149+
// passing raw authored metadata is calling it OUT OF CONTRACT, and it then
1150+
// dereferences `node.config` in its own region walk. Coercing only the
1151+
// local `nodes` above does not fix the crash, it relocates it into
1152+
// `packages/spec` — measured. Contract-first the caller is what changes.
1153+
const graphs = collectFlowGraphs({ ...flow, nodes } as { nodes?: FlowNodeParsed[] });
11411154

11421155
// [#14089] The flattened-scope shadowing pass needs the flow's COMPLETE
11431156
// variable set before any condition is judged, so it is a separate walk over
@@ -1202,7 +1215,13 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
12021215

12031216
for (const graph of graphs) {
12041217
const at = graph.scope ? `flow '${flowName}' · ${graph.scope}` : `flow '${flowName}'`;
1205-
for (const node of graph.nodes as unknown as AnyRec[]) {
1218+
// `recordsOf`, not `as unknown as AnyRec[]` (#15793). A NESTED region's
1219+
// node list is only `Array.isArray`-checked by `collectFlowGraphs` before
1220+
// it becomes a graph, so this list carries the producer's word about its
1221+
// members and not a check. The top-level graph is clean by the coercion
1222+
// at the call site above; this is the same decision made once more where
1223+
// that guarantee stops, through the file's one home for it.
1224+
for (const node of recordsOf(graph.nodes)) {
12061225
const cfg = (node.config ?? {}) as AnyRec;
12071226
const nodeCondWhere = `${at} · node '${node.id}' (${node.type}) condition`;
12081227
if (!checkStructuralCondition(nodeCondWhere, cfg.condition).refused) {

0 commit comments

Comments
 (0)