Skip to content

Commit ae05f2e

Browse files
claude[bot]claude
andauthored
fix(spec): make what collectFlowGraphs RETURNS match its declared FlowNodeParsed[] (#16922)
* fix(spec): drop a non-record member from what collectFlowGraphs hands out `collectFlowGraphs` declares each `FlowGraph.nodes` as `readonly FlowNodeParsed[]`, then pushed the list verbatim. That list is one the walk picks up ITSELF out of a container's open `z.record` config, after an `Array.isArray` that proves the LIST and never its MEMBERS — so an empty YAML list item under a `loop` body reached a returned graph as `null`, at every depth, and no coercion at a call site could reach it. Filter what is handed out and skip what is walked through one predicate (`isRegionDict`), preserving array identity when nothing is dropped. The walk runs inside `FlowSchema`'s parse, so this stays a drop and a skip: a throw here would escape `safeParse` rather than become an issue. `path` stays indexed over the raw list, so a Zod issue is still anchored where the author wrote the node. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016N6xmWt5hYm94ffVEwGH8x * test(lint): retire the two graph-shaped RESIDUAL_THROWS rows the producer fix earned Re-measured with all four rows still present: the two `flows[].nodes[].config.body.nodes` rows went red demanding a throw that no longer happens, while both `flows[].nodes` rows stayed green — those are `lint-flow-patterns.ts` reading `flow.nodes` itself, untouched by a producer repair. Remove the two that stopped being earned, keep the two that did not, and correct the prose that described the retired pair as live. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016N6xmWt5hYm94ffVEwGH8x * chore(changeset): declare the collectFlowGraphs return-contract repair Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016N6xmWt5hYm94ffVEwGH8x --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9a89a00 commit ae05f2e

4 files changed

Lines changed: 181 additions & 29 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
`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.
6+
7+
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.
8+
9+
- **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.
10+
- **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.
11+
- **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

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -330,9 +330,11 @@ const underFlow = (key: string, valid?: AnyRec): SweptCollection => ({
330330
* and `collectFlowGraphs` turns each into its own `FlowGraph` after checking
331331
* only `Array.isArray` on the inner list — so a non-record member here is one
332332
* 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.
333+
* call site can reach it. That is what #16752 repaired, at the producer: the
334+
* walk now drops a non-record member from the graph it hands out, and this arm
335+
* carries no `RESIDUAL_THROWS` row. It stays in the sweep as the pin on that
336+
* repair — an unaddressable shape is exactly what let this class survive three
337+
* closures, so the addressing is the part worth keeping.
336338
*/
337339
const underNestedRegion = (valid?: AnyRec): SweptCollection => ({
338340
label: 'flows[].nodes[].config.body.nodes',
@@ -399,7 +401,7 @@ const SWEPT_COLLECTIONS: readonly SweptCollection[] = [
399401
* "nothing throws" would have had to be deleted or weakened on the day it was
400402
* written, and would then never have caught the next one.
401403
*
402-
* Three rows have come out since it was written, each because the sweep went
404+
* Four rows have come out since it was written, each because the sweep went
403405
* red demanding a throw that no longer happens — which is the both-directions
404406
* half earning its keep, since no removal started with anyone going looking:
405407
*
@@ -413,36 +415,38 @@ const SWEPT_COLLECTIONS: readonly SweptCollection[] = [
413415
* field readers already did.
414416
* - `flows[].nodes` / `validateStackExpressions` — the two casts #15793
415417
* repaired, and the reason the two graph-shaped arms below exist at all.
418+
* - `flows[].nodes[].config.body.nodes` / `validateStackExpressions` +
419+
* `lintFlowPatterns` (#16752) — neither rule's own reader was ever at fault
420+
* here, and neither was repaired: the throw was `collectFlowGraphs`'
421+
* (`packages/spec`), which handed out a `FlowGraph` whose `nodes` held the
422+
* junk member it had picked up out of a container's open `z.record` config.
423+
* Both rules stopped throwing the moment the PRODUCER stopped handing it
424+
* out, which is what #15793 predicted when it refused to widen the
425+
* `packages/spec` contract and filed the fork instead. ⛔ Note what did NOT
426+
* fix it: #16134 had already stopped that walk DEREFERENCING the member, and
427+
* both rows survived it — a guard against reading junk is not a guard
428+
* against passing it on.
416429
*
417-
* ## The rows it holds today, both found by the arms that added them
430+
* ## The row it holds today
418431
*
419432
* It went from empty to two the moment a flow's inner node list became
420433
* addressable, which is the point #15793 was filed to make: this class was
421434
* closed three times over collections while the same defect stood untouched one
422-
* addressing mode away.
435+
* addressing mode away. The graph-shaped pair is gone; the shallow one is not.
423436
*
424437
* - `flows[].nodes` / `lintFlowPatterns` (#16751) — `lint-flow-patterns.ts`
425438
* holds the SAME two spellings #15793 removed from `validate-expressions.ts`
426439
* (`:1426` inline-casts `flow.nodes`, then `:1430` reads `.type` off each
427440
* 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.
441+
* reachable — an ordinary flow with an empty YAML list item. The two
442+
* `graph.nodes` casts are covered from the producer side since #16752, so
443+
* what is left to repair here is the `flow.nodes` read the rule does itself.
436444
*/
437445
const RESIDUAL_THROWS: Readonly<Record<string, readonly string[]>> = {
438446
// 2026-09-08 — #16751. Removed when `lint-flow-patterns.ts` reads its node
439447
// lists through `recordsOf`, as `validate-expressions.ts` now does.
440448
'flows[].nodes · null': ['lintFlowPatterns'],
441449
'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'],
446450
};
447451

448452
/**

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

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,12 @@ export function findRegionEntry(region: { nodes: FlowNodeParsed[]; edges?: FlowE
486486

487487
// ─── Where the containers keep their regions ─────────────────────────
488488

489-
/** A dict — region-shaped enough to reach its `nodes` / `edges`. */
489+
/**
490+
* A dict — region-shaped enough to reach its `nodes` / `edges`, and the same
491+
* test a member of a node list must pass to be a node at all. One spelling for
492+
* both, so what {@link collectFlowGraphs} walks cannot drift from what it hands
493+
* out.
494+
*/
490495
function isRegionDict(v: unknown): v is Record<string, unknown> {
491496
return typeof v === 'object' && v !== null && !Array.isArray(v);
492497
}
@@ -699,6 +704,16 @@ export interface FlowGraph {
699704
* `[...path, 'nodes', i, 'id']` — rather than described in prose (#16134).
700705
*/
701706
readonly path: readonly (string | number)[];
707+
/**
708+
* Every member is a record. A node list read out of a container's open
709+
* `z.record` config can hold whatever the author typed — an empty YAML list
710+
* item deserialises to `null` — and a region its own schema refused is left
711+
* RAW for {@link validateControlFlow} to name. The walk therefore drops a
712+
* non-record member rather than hand out an array that does not match this
713+
* declared type (#16752). Only this array is narrowed: the schema refusal
714+
* that owns the malformed region still fires, and {@link path} still indexes
715+
* the RAW list, so a finding stays anchored where the author wrote it.
716+
*/
702717
readonly nodes: readonly FlowNodeParsed[];
703718
readonly edges: readonly FlowEdgeParsed[];
704719
}
@@ -727,18 +742,31 @@ export function collectFlowGraphs(
727742
path: readonly (string | number)[],
728743
depth: number,
729744
): void => {
730-
graphs.push({ scope, path, nodes, edges });
745+
// A region its own schema refused is left RAW by `parseFlowNodeRegions` for
746+
// `validateControlFlow` to name, so an element here can be whatever the
747+
// author typed — `null` included. What is HANDED OUT and what is WALKED both
748+
// drop it, through the one predicate above.
749+
//
750+
// Handed out (#16752): `FlowGraph.nodes` is declared `readonly
751+
// FlowNodeParsed[]`, and an array whose members every caller must re-check
752+
// is not that array. This list is one the walk picked up out of an open
753+
// `z.record` config ITSELF — no caller ever held it, so no coercion at a
754+
// call site can reach it. Identity is preserved when nothing is dropped.
755+
//
756+
// Walked (#16134): skip a non-record rather than read `.config` off it —
757+
// this walk runs inside `FlowSchema`'s parse, where a thrown TypeError would
758+
// escape `safeParse`, which is why this is a skip and not a throw. The schema
759+
// refusal that owns the malformed region still fires, reached now where the
760+
// throw used to pre-empt it.
761+
const kept = nodes.filter((node) => isRegionDict(node));
762+
graphs.push({ scope, path, nodes: kept.length === nodes.length ? nodes : kept, edges });
731763
if (depth >= MAX_REGION_DEPTH) return;
764+
// Indexed over the RAW list, never `kept`: `path` anchors a Zod issue where
765+
// the author wrote the node, so dropping a member must not renumber the
766+
// siblings that outlive it. `Array.isArray` on the inner list below proves
767+
// the LIST, never its MEMBERS — the sentence removed from four lint readers.
732768
nodes.forEach((node, index) => {
733-
// A region its own schema refused is left RAW by `parseFlowNodeRegions`
734-
// for `validateControlFlow` to name, so an element here can be whatever
735-
// the author typed — `null` included. Skip what is not a node object
736-
// rather than read `.config` off it: this walk runs inside `FlowSchema`'s
737-
// parse (#16134), where a thrown TypeError would escape `safeParse`. The
738-
// schema refusal that owns the malformed region still fires — reached now,
739-
// where the throw used to pre-empt it.
740-
const raw: unknown = node;
741-
if (raw === null || typeof raw !== 'object') return;
769+
if (!isRegionDict(node)) return;
742770
for (const slot of regionSlotsOf(node)) {
743771
if (!isRegionDict(slot.raw) || !Array.isArray(slot.raw.nodes)) continue;
744772
visit(

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

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,4 +279,113 @@ describe('#4347 — collectFlowGraphs', () => {
279279
expect(graphs.length).toBeGreaterThan(1);
280280
expect(graphs.length).toBeLessThan(64);
281281
});
282+
283+
/**
284+
* #16752 — what the walk HANDS OUT matches its declared
285+
* `readonly FlowNodeParsed[]`.
286+
*
287+
* The list in question is one `collectFlowGraphs` picks up ITSELF, out of a
288+
* container's open `z.record` config, and casts after an `Array.isArray` that
289+
* proves the LIST and never its MEMBERS — the sentence #15552 / #15636 /
290+
* #15742 / #15793 removed from four lint readers. No caller ever holds this
291+
* array, so no coercion at a call site can reach it; the guard belongs here.
292+
*
293+
* #16134 already stopped the walk DEREFERENCING a non-record member (a
294+
* `TypeError` thrown here escapes `FlowSchema.safeParse` rather than becoming
295+
* an issue). It did not stop the walk HANDING IT OUT: `nodes` was pushed
296+
* verbatim, so every graph over a junk-bearing list carried the junk — at
297+
* every depth, not only at the `MAX_REGION_DEPTH` ceiling the filing found.
298+
*
299+
* ⛔ The repair is a drop, never a looser signature: widening the declared
300+
* type to tolerate malformed members is the direction #15793 refused on the
301+
* anti-AI-error axis. As with the four repairs above, a dropped member
302+
* renumbers the ones behind it in `graph.nodes` — a difference in the index,
303+
* never in whether a node was judged; `graph.path`, which anchors a Zod issue
304+
* where the author wrote it, is pinned below to stay indexed over the RAW
305+
* list.
306+
*/
307+
describe('#16752 — a non-record member never reaches a returned graph', () => {
308+
/**
309+
* The five shapes a raw node list holds that are not a node. `null` is the
310+
* one an author writes by accident (an empty YAML list item deserialises to
311+
* it); `an array` is the one a bare `typeof x === 'object'` test admits, so
312+
* it pins that the drop is a record test and not an object test.
313+
*/
314+
const NON_NODES: readonly (readonly [string, unknown])[] = [
315+
['null', null],
316+
['undefined', undefined],
317+
['a string', 'x'],
318+
['a number', 42],
319+
['an array', []],
320+
];
321+
322+
const isRecord = (v: unknown): boolean => typeof v === 'object' && v !== null && !Array.isArray(v);
323+
324+
/**
325+
* `depth` nested loop bodies, the innermost holding `junk` beside two real
326+
* nodes. Hand-built rather than parsed on purpose: a region its own schema
327+
* refuses is left RAW by `parseFlowNodeRegions`, and raw is the state this
328+
* walk has to survive.
329+
*/
330+
const nestedJunk = (depth: number, junk: unknown) => {
331+
let region: Record<string, unknown> = {
332+
nodes: [junk, { ...gate, id: 'gate_in' }, { ...write, id: 'write_in' }],
333+
edges: [],
334+
};
335+
for (let i = depth; i > 0; i--) {
336+
region = { nodes: [loopWith(region, `lp${i}`)], edges: [] };
337+
}
338+
return { nodes: region.nodes as never, edges: [] };
339+
};
340+
341+
describe.each(NON_NODES)('with %s in the innermost body', (_label, junk) => {
342+
// 0 is the flow's own list, 1 the shape the card reproduced, and 32 the
343+
// depth ceiling — where `visit` pushes a graph and returns without ever
344+
// walking its members, so the junk was handed out with nothing having
345+
// looked at it.
346+
it.each([0, 1, 32])('hands out only records at nesting %i', (depth) => {
347+
const graphs = collectFlowGraphs(nestedJunk(depth, junk));
348+
expect(graphs.flatMap(g => g.nodes).filter(n => !isRecord(n))).toEqual([]);
349+
});
350+
351+
it('still hands out the real nodes standing beside it', () => {
352+
// Anti-vacuity: the drop takes what cannot be read, not the list. A
353+
// guard that emptied every graph would pass the assertion above.
354+
const graphs = collectFlowGraphs(nestedJunk(1, junk));
355+
expect(graphs[graphs.length - 1]!.nodes.map(n => n.id)).toEqual(['gate_in', 'write_in']);
356+
});
357+
358+
it('lets `FlowSchema.safeParse` return an envelope rather than throw (#16134)', () => {
359+
// This walk runs inside the parse, so the repair has to stay a drop and
360+
// a skip; a throw here escapes `safeParse` instead of becoming an issue.
361+
const result = FlowSchema.safeParse({
362+
name: 'repro', label: 'Repro', type: 'schedule',
363+
nodes: [
364+
{ id: 'start', type: 'start', label: 'Start' },
365+
loopWith({ nodes: [junk], edges: [] }),
366+
],
367+
edges: [],
368+
});
369+
expect(typeof result.success).toBe('boolean');
370+
});
371+
});
372+
373+
it('leaves `path` indexed over the RAW list, so a finding stays where the author wrote it', () => {
374+
// The container is at authored index 1 whether or not a non-record
375+
// precedes it: dropping a member must not renumber its siblings in the
376+
// key path a Zod issue is anchored on (#16134).
377+
const graphs = collectFlowGraphs({
378+
nodes: [null, loopWith(gatedRegion())] as never,
379+
edges: [],
380+
});
381+
expect(graphs.map(g => g.path)).toEqual([[], ['nodes', 1, 'config', 'body']]);
382+
});
383+
384+
it('hands back the very same array when there is nothing to drop', () => {
385+
// Copy-on-write, as `parseFlowNodeRegions` is: a well-formed flow pays
386+
// nothing for the guard.
387+
const nodes = [{ ...gate }, { ...write }];
388+
expect(collectFlowGraphs({ nodes: nodes as never, edges: [] })[0]!.nodes).toBe(nodes);
389+
});
390+
});
282391
});

0 commit comments

Comments
 (0)