Skip to content

Commit 4d552af

Browse files
hotlongclaude
andauthored
feat(spec)!: FlowNodeSchema parses its own ADR-0031 regions — the post-parse pass retires (#4415) (#6333)
* feat(spec)!: FlowNodeSchema parses its own ADR-0031 regions (#4415) `FlowSchema.parse` could not reach a region — regions live inside `FlowNodeSchema.config`, a deliberately open `z.record` (ADR-0018) — so #4381 closed the gap with a post-parse pass (`normalizeControlFlowRegions`) every caller had to remember to run. That unwritten rule is the condition the #4347 defect family grows in: a new consumer takes a `FlowParsed` and uses it, half-parsed and looking finished. `FlowNodeSchema` now carries a `.transform()` that parses each declared region slot through the schema that slot's value is. Nesting needs no manual recursion: a region's `nodes` are `z.array(FlowNodeSchema)`, so Zod re-enters the transform on the way down. The post-parse pass and its `registerFlow` call site retire. Premise measured first, per the maintainer ruling — the ZodPipe is digested by all three named generators (toJSONSchema walker, form generation, the lazy-schema seen-table path). Two mechanical prerequisites the measurement surfaced: the region schemas back-reference through `z.lazy()`, and the object half is a hoisted function declaration, both load-bearing under `OS_EAGER_SCHEMAS=1`; pinned by `flow-region-cycle.test.ts`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BickTBKm2JYSNnrtPT8ysa * chore(spec): regenerate the flow reference after merging main Net delta vs origin/main is exactly the two input-shape lines #4415 intends (`inputSchema[].required`, `boundaryConfig.interrupting` render optional now that FlowNode is read from the pipe's authorable IN side). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BickTBKm2JYSNnrtPT8ysa --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent a5ca08d commit 4d552af

11 files changed

Lines changed: 395 additions & 146 deletions

File tree

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
---
2+
'@objectstack/spec': minor
3+
'@objectstack/service-automation': patch
4+
---
5+
6+
feat(spec)!: `FlowNodeSchema` parses its own ADR-0031 regions — the post-parse pass retires (#4415)
7+
8+
`FlowSchema.parse` normalized a flow's own `nodes[]` / `edges[]` but could not reach a
9+
**region**, because a region lives inside `FlowNodeSchema.config` — a deliberately open
10+
`z.record` (ADR-0018). #4381 closed the resulting gap with a **post-parse pass**,
11+
`normalizeControlFlowRegions`, that every caller had to remember to run:
12+
13+
```ts
14+
const flowShell = FlowSchema.parse(converted);
15+
validateControlFlow(flowShell);
16+
const parsed = normalizeControlFlowRegions(flowShell); // ← had to remember
17+
```
18+
19+
That is an unwritten rule on top of a parse, and it is exactly the condition the #4347
20+
family of defects grows in: a new consumer — a Studio publish path, an MCP tool, a bulk
21+
validation script — takes a `FlowParsed` and uses it, holding a **half-parsed flow that
22+
looks finished**. Nested edge predicates were still bare strings, nested nodes had not been
23+
through `.strict()`, and nothing said so.
24+
25+
Now the schema does it. `FlowNodeSchema` carries a `.transform()` that parses each declared
26+
region slot — `loop.config.body`, `parallel.config.branches[]`, `try_catch.config.try` /
27+
`.catch` — through the schema that slot's value *is*. Nesting needs no manual recursion: a
28+
region's `nodes` are `z.array(FlowNodeSchema)`, so Zod re-enters the transform on the way
29+
down. **"Parsed" now means parsed at every depth** (Prime Directive #1), from any entry
30+
point — including `FlowNodeSchema.parse(node)` on a single node, which the old whole-flow
31+
pass could not serve at all.
32+
33+
## Migration
34+
35+
**`normalizeControlFlowRegions` is removed from `@objectstack/spec/automation`.** Delete the
36+
call; the parse above it already did the work:
37+
38+
```diff
39+
const parsed = FlowSchema.parse(converted);
40+
validateControlFlow(parsed);
41+
- const normalized = normalizeControlFlowRegions(parsed);
42+
```
43+
44+
Its replacement, `parseFlowNodeRegions(node)`, is exported for the same purpose one node at
45+
a time, but you should not normally need it — it is the transform's own body.
46+
47+
**`FlowNodeSchema` is now a `ZodPipe`, not a `ZodObject`,** so it no longer has `.shape` /
48+
`.extend()` / `.pick()`. `z.infer` / `z.input` / `.parse` / `.safeParse` and
49+
`z.toJSONSchema` are unaffected, and the authorable key set is byte-identical (verified by
50+
`check:authorable-surface`). If you were reaching for the object half, read it from the
51+
pipe's input side — `FlowNodeSchema.def.in` — which is also what the repo's own generators
52+
do (`pipeAuthorableSide` in `scripts/lib/zod-graph.ts`).
53+
54+
One visible consequence in the generated reference: `content/docs/references/automation/flow.mdx`
55+
now renders FlowNode's **input** shape, so keys carrying a `.default()` (`boundaryConfig.interrupting`,
56+
`inputSchema[].required`) show as optional. That is what an author actually writes, which is
57+
what an authoring reference should say.

content/docs/references/automation/flow.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,10 @@ const result = FlowSchema.parse(data);
9696
| **connectorConfig** | `{ connectorId: string; actionId: string; input?: Record<string, any> }` | optional | |
9797
| **position** | `{ x: number; y: number }` | optional | |
9898
| **timeoutMs** | `integer` | optional | Maximum execution time for this node in milliseconds |
99-
| **inputSchema** | `Record<string, { type: Enum<'string' \| 'number' \| 'boolean' \| 'object' \| 'array'>; required: boolean; description?: string }>` | optional | Input parameter schema for this node |
99+
| **inputSchema** | `Record<string, { type: Enum<'string' \| 'number' \| 'boolean' \| 'object' \| 'array'>; required?: boolean; description?: string }>` | optional | Input parameter schema for this node |
100100
| **outputSchema** | `never` | optional | [REMOVED] `flow.nodes[].outputSchema` was removed in @objectstack/spec 17.0.0 (#3896 audit close-out) — it was never validated: the engine does not check node outputs against it, so it documented a contract nothing enforced. Delete the key. Downstream nodes read prior outputs via expressions (`{{nodeId.field}}`) regardless of any declaration. |
101101
| **waitEventConfig** | `{ eventType: Enum<'timer' \| 'signal' \| 'webhook' \| 'manual' \| 'condition'>; timerDuration?: string; signalName?: string }` | optional | Configuration for wait node event resumption |
102-
| **boundaryConfig** | `{ attachedToNodeId: string; eventType: Enum<'error' \| 'timer' \| 'signal' \| 'cancel'>; interrupting: boolean; errorCode?: string; … }` | optional | Configuration for boundary events attached to host nodes |
102+
| **boundaryConfig** | `{ attachedToNodeId: string; eventType: Enum<'error' \| 'timer' \| 'signal' \| 'cancel'>; interrupting?: boolean; errorCode?: string; … }` | optional | Configuration for boundary events attached to host nodes |
103103

104104

105105
---

packages/services/service-automation/src/builtin/io-node-form-zod-ledger.test.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,13 @@ describe('IO-node form ↔ Zod reconciliation (#4045)', () => {
110110
// and nothing else (connector-nodes.ts). The spec side of that contract
111111
// is FlowNodeSchema.connectorConfig — unwrap the optional wrapper
112112
// structurally to stay off a direct `zod` dependency.
113-
const prop = (FlowNodeSchema as unknown as { shape: Record<string, unknown> })
114-
.shape.connectorConfig as { unwrap?: () => { shape?: Record<string, unknown> } };
113+
//
114+
// `FlowNodeSchema` is a ZodPipe since #4415 (it parses its own ADR-0031
115+
// regions), so the declared keys live on the pipe's INPUT side — the
116+
// authorable half, which is what this reconciliation is about, and the
117+
// same side the spec's own generators read (`pipeAuthorableSide`).
118+
const node = FlowNodeSchema as unknown as { def: { in: { shape: Record<string, unknown> } } };
119+
const prop = node.def.in.shape.connectorConfig as { unwrap?: () => { shape?: Record<string, unknown> } };
115120
expect(prop, 'FlowNodeSchema should declare connectorConfig').toBeDefined();
116121
expect(prop.unwrap, 'connectorConfig should be an optional-wrapped object').toBeTypeOf('function');
117122
const shape = prop.unwrap!().shape;

packages/services/service-automation/src/engine.ts

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {
1818
type ScreenFieldVisibility,
1919
} from './screen-input-contract.js';
2020
import type { Logger } from '@objectstack/spec/contracts';
21-
import { FlowSchema, FLOW_STRUCTURAL_NODE_TYPES, validateControlFlow, normalizeControlFlowRegions, collectFlowGraphs, findRegionEntry, defineActionDescriptor } from '@objectstack/spec/automation';
21+
import { FlowSchema, FLOW_STRUCTURAL_NODE_TYPES, validateControlFlow, collectFlowGraphs, findRegionEntry, defineActionDescriptor } from '@objectstack/spec/automation';
2222
import { resolveFlowNodeExpressions } from '@objectstack/spec/automation';
2323
import { applyConversionsToFlow, type ConversionNotice, type ConversionConflictNotice } from '@objectstack/spec';
2424
import type { FlowRegionParsed } from '@objectstack/spec/automation';
@@ -1038,11 +1038,11 @@ export interface SuspendedRunStore {
10381038
* the author never wrote pins that row to today's value forever — so the graft
10391039
* is deliberately narrow: it copies the lowered `condition`, nothing more.
10401040
*
1041-
* Structural alignment is by position, which is sound because neither the parse
1042-
* nor `normalizeControlFlowRegions` reorders or drops array members — both are
1043-
* copy-on-write maps. Where the two sides disagree in shape (a caller passed a
1044-
* mismatched pair), the converted side is returned untouched: this only ever
1045-
* lifts a value it can positively match.
1041+
* Structural alignment is by position, which is sound because the parse — region
1042+
* transform included (#4415) — never reorders or drops array members: every step
1043+
* of it is a copy-on-write map. Where the two sides disagree in shape (a caller
1044+
* passed a mismatched pair), the converted side is returned untouched: this only
1045+
* ever lifts a value it can positively match.
10461046
*
10471047
* Node `config.condition` (e.g. a start node's record-change predicate) is
10481048
* left alone by construction — `FlowNodeSchema.config` is an open `z.record`,
@@ -2077,25 +2077,23 @@ export class AutomationEngine implements IAutomationService {
20772077
this.logger.warn(`[flow '${name}'] ${c.code}: ${c.message}`);
20782078
},
20792079
});
2080-
const flowShell = FlowSchema.parse(converted);
2080+
// #4347 / #4415 — one call, canonical at every depth. `FlowNodeSchema`
2081+
// parses its own ADR-0031 regions (`FlowNodeSchema.transform` →
2082+
// `parseFlowNodeRegions`), so what comes back here is already normalized
2083+
// inside `loop.config.body`, `parallel.config.branches[]` and
2084+
// `try_catch.config.try`/`.catch` — recursively. Until #4415 that needed
2085+
// a second, separately-remembered call to `normalizeControlFlowRegions`
2086+
// right here, and every consumer that took a `FlowParsed` without making
2087+
// it held a half-parsed flow that looked finished.
2088+
const parsed = FlowSchema.parse(converted);
20812089

20822090
// DAG cycle detection
2083-
this.detectCycles(flowShell);
2091+
this.detectCycles(parsed);
20842092

20852093
// ADR-0031 — validate structured control-flow constructs (loop bodies,
20862094
// parallel branches, try/catch regions) are well-formed (single-entry/
20872095
// single-exit, acyclic). Reject the malformed before it can run.
2088-
validateControlFlow(flowShell);
2089-
2090-
// #4347 — then canonicalize what lives INSIDE those regions. A region
2091-
// sits in `FlowNodeSchema.config`, which is an open `z.record`, so the
2092-
// parse above stopped at the container: a bare-string `condition` on a
2093-
// top-level edge came back as the canonical `{ dialect: 'cel', source }`
2094-
// envelope while the identical predicate on a loop-body edge stayed a
2095-
// bare string. Same flow, same call, different stored shape by nesting
2096-
// depth. Runs after `validateControlFlow` so a malformed region is
2097-
// still reported by the validator that owns that message.
2098-
const parsed = normalizeControlFlowRegions(flowShell);
2096+
validateControlFlow(parsed);
20992097

21002098
return {
21012099
parsed,

packages/spec/api-surface/automation.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,9 +265,9 @@
265265
"getSchemalessNodeConfigJsonSchemas (function)",
266266
"importBpmnToConstructs (function)",
267267
"isFlowFunctionEffect (function)",
268-
"normalizeControlFlowRegions (function)",
269268
"normalizeDecisionOutputs (function)",
270269
"normalizeFlowFunctionEntry (function)",
270+
"parseFlowNodeRegions (function)",
271271
"resolveFlowNodeExpressions (function)",
272272
"validateControlFlow (function)"
273273
]

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

Lines changed: 60 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,9 @@ export const FlowRegionSchema = lazySchema(() => strictObject(
139139
},
140140
{
141141
/** Body nodes (must not include `start`/`end` trigger sentinels). */
142-
nodes: z.array(FlowNodeSchema).min(1).describe('Region body nodes (single-entry/single-exit sub-graph)'),
142+
nodes: z.array(z.lazy(() => FlowNodeSchema)).min(1).describe('Region body nodes (single-entry/single-exit sub-graph)'),
143143
/** Body edges connecting the region nodes. */
144-
edges: z.array(FlowEdgeSchema).default([]).describe('Region body edges'),
144+
edges: z.array(z.lazy(() => FlowEdgeSchema)).default([]).describe('Region body edges'),
145145
},
146146
));
147147

@@ -239,8 +239,8 @@ export const ParallelBranchSchema = lazySchema(() => strictObject(
239239
{
240240
/** Optional human label for the branch (designer + logs). */
241241
name: z.string().optional().describe('Branch label'),
242-
nodes: z.array(FlowNodeSchema).min(1).describe('Branch body nodes'),
243-
edges: z.array(FlowEdgeSchema).default([]).describe('Branch body edges'),
242+
nodes: z.array(z.lazy(() => FlowNodeSchema)).min(1).describe('Branch body nodes'),
243+
edges: z.array(z.lazy(() => FlowEdgeSchema)).default([]).describe('Branch body edges'),
244244
},
245245
));
246246

@@ -451,8 +451,8 @@ interface RegionSlot {
451451
* the value it holds, the Zod schema that value parses as, and a diagnostic
452452
* label.
453453
*
454-
* The three passes in this module read it ({@link validateControlFlow},
455-
* {@link normalizeControlFlowRegions}, {@link collectFlowGraphs}). WHERE the
454+
* The three readers in this module use it ({@link validateControlFlow},
455+
* {@link parseFlowNodeRegions}, {@link collectFlowGraphs}). WHERE the
456456
* slots are is no longer stated here — that moved to `region-slots.ts` so the
457457
* conversion walk and the lint walk read the same list. What stays here is the
458458
* schema half, which is this module's business.
@@ -554,81 +554,70 @@ export function validateControlFlow(flow: { nodes: FlowNodeParsed[] }): void {
554554
}
555555

556556

557-
// ─── Region normalization ────────────────────────────────────────────
557+
// ─── Region parsing (the FlowNodeSchema transform) ───────────────────
558558

559559
/**
560-
* Parse ONE region value through its own schema, then recurse into the
561-
* containers its nodes carry.
560+
* Re-entrancy depth of {@link parseFlowNodeRegions}.
562561
*
563-
* A value that does not parse is returned untouched: rejecting a malformed
564-
* region is {@link validateControlFlow}'s job (and, at run time, the container
565-
* executor's `parseNodeConfig`). A normalization pass that also threw would
566-
* change *which* flows register, which is not what it is for.
562+
* A module-level counter rather than a parameter, because the recursion is no
563+
* longer ours to thread: `FlowRegionSchema.nodes` is `z.array(FlowNodeSchema)`,
564+
* so the descent happens *inside Zod*, which has nowhere to carry a depth. Safe
565+
* as shared state because Zod parsing is synchronous — the whole tree unwinds on
566+
* one stack, and the `finally` below restores the counter on the error path too.
567+
*
568+
* Without it a flow assembled as hand-built objects (not parsed JSON) could hold
569+
* a self-reference and recurse until the stack blows, at the load seam. The
570+
* post-parse pass this replaced guarded the same hazard with an explicit `depth`
571+
* argument; the ceiling is unchanged.
567572
*/
568-
function normalizeRegion(slot: RegionSlot, depth: number): unknown {
569-
if (!isRegionDict(slot.raw)) return slot.raw;
570-
const parsed = slot.schema.safeParse(slot.raw);
571-
if (!parsed.success) return slot.raw;
572-
const region = parsed.data as { nodes?: FlowNodeParsed[] };
573-
if (!Array.isArray(region.nodes)) return region;
574-
return { ...region, nodes: region.nodes.map(n => normalizeNodeRegions(n, depth + 1)) };
575-
}
576-
577-
/** Normalize every region one node carries — recursively, since regions nest. */
578-
function normalizeNodeRegions(node: FlowNodeParsed, depth: number): FlowNodeParsed {
579-
if (depth >= MAX_REGION_DEPTH) return node;
580-
const cfg = node.config as Record<string, unknown> | undefined;
581-
if (!cfg) return node;
582-
583-
let next = cfg;
584-
for (const slot of regionSlotsOf(node)) {
585-
const normalized = normalizeRegion(slot, depth);
586-
if (normalized === slot.raw) continue;
587-
if (slot.index === undefined) {
588-
next = { ...next, [slot.key]: normalized };
589-
} else {
590-
const branches = [...(next[slot.key] as unknown[])];
591-
branches[slot.index] = normalized;
592-
next = { ...next, [slot.key]: branches };
593-
}
594-
}
595-
596-
return next === cfg ? node : { ...node, config: next };
597-
}
573+
let regionParseDepth = 0;
598574

599575
/**
600-
* Canonicalize the metadata **inside** every structured region of a flow (#4347).
576+
* Parse every ADR-0031 region a node's `config` holds — the body of
577+
* {@link FlowNodeSchema}'s `.transform()` (#4415).
601578
*
602-
* `FlowSchema.parse` normalizes a flow's own `nodes[]` / `edges[]` — most
603-
* visibly, `FlowEdgeSchema.condition` is `ExpressionInputSchema`, so a
604-
* bare-string predicate becomes the canonical `{ dialect: 'cel', source }`
605-
* envelope. It does not reach a region, because a region lives inside
606-
* `FlowNodeSchema.config`, which is deliberately `z.record(z.unknown())` — open,
607-
* per node type. So the *same predicate* was stored enveloped on a top-level edge
608-
* and left a bare string on a loop-body edge: a representation that depended on
609-
* where in the graph it sat, which no flow author can be expected to predict.
579+
* `FlowNodeSchema.config` is a deliberately open `z.record` (ADR-0018), so
580+
* nothing about a container's nested sub-graph is described by the node's own
581+
* shape. This resolves each declared slot against {@link FLOW_REGION_SLOTS_BY_TYPE}
582+
* and runs its value through the schema that slot's value IS — `FlowRegionSchema`
583+
* for `loop.config.body` / `try_catch.config.try` / `.catch`,
584+
* `ParallelBranchSchema` for each `parallel.config.branches[]`.
610585
*
611-
* This pass closes that. Each region is run through its own schema — recursively,
612-
* because regions nest — producing a flow whose nested edges and nodes carry the
613-
* same canonical shapes as its top-level ones. Copy-on-write: a flow with no
614-
* structured container comes back untouched.
586+
* Nesting needs no recursion here: those schemas hold `z.array(FlowNodeSchema)`,
587+
* so a region's own nodes come back through this transform on the way down. That
588+
* is the whole reason this reads shorter than the pass it replaced.
615589
*
616-
* Call it at the load seam, after `FlowSchema.parse` and `validateControlFlow`.
617-
* The container executors parse their own config at run time (`parseNodeConfig`,
618-
* #4277), so this is not what makes a nested predicate *evaluate* correctly — it
619-
* is what makes the stored flow SAY so, for every reader that is not the
620-
* executor: the Studio designer, `getFlow`, the version history, and any
621-
* consumer that reads a region without re-parsing it.
590+
* **A value that does not parse is returned untouched.** Rejecting a malformed
591+
* region is {@link validateControlFlow}'s job (and, at run time, the container
592+
* executor's `parseNodeConfig`): a transform that threw here would change *which*
593+
* flows parse at all, moving a structural diagnostic out of the validator that
594+
* owns its message and into a Zod issue on `config`. Copy-on-write — a node with
595+
* no region comes back by identity.
622596
*/
623-
export function normalizeControlFlowRegions<T extends { nodes: FlowNodeParsed[] }>(flow: T): T {
624-
if (!Array.isArray(flow.nodes)) return flow;
625-
let changed = false;
626-
const nodes = flow.nodes.map(node => {
627-
const next = normalizeNodeRegions(node, 0);
628-
if (next !== node) changed = true;
629-
return next;
630-
});
631-
return changed ? { ...flow, nodes } : flow;
597+
export function parseFlowNodeRegions<T extends { type: string; config?: unknown }>(node: T): T {
598+
const cfg = node.config as Record<string, unknown> | undefined;
599+
if (!cfg) return node;
600+
if (regionParseDepth >= MAX_REGION_DEPTH) return node;
601+
602+
regionParseDepth++;
603+
try {
604+
let next = cfg;
605+
for (const slot of regionSlotsOf(node as unknown as FlowNodeParsed)) {
606+
if (!isRegionDict(slot.raw)) continue;
607+
const parsed = slot.schema.safeParse(slot.raw);
608+
if (!parsed.success) continue;
609+
if (slot.index === undefined) {
610+
next = { ...next, [slot.key]: parsed.data };
611+
} else {
612+
const branches = [...(next[slot.key] as unknown[])];
613+
branches[slot.index] = parsed.data;
614+
next = { ...next, [slot.key]: branches };
615+
}
616+
}
617+
return next === cfg ? node : { ...node, config: next };
618+
} finally {
619+
regionParseDepth--;
620+
}
632621
}
633622

634623
// ─── Whole-flow graph traversal ──────────────────────────────────────

0 commit comments

Comments
 (0)