From 9a25b5a54089a8a8c215aadee8abeb10847ac90a Mon Sep 17 00:00:00 2001 From: Claude Fable 5 Date: Fri, 7 Aug 2026 13:17:39 +0000 Subject: [PATCH 1/2] feat(spec)!: FlowNodeSchema parses its own ADR-0031 regions (#4415) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 Claude-Session: https://claude.ai/code/session_01BickTBKm2JYSNnrtPT8ysa --- .changeset/flownode-parses-its-regions.md | 57 +++++++ content/docs/references/automation/flow.mdx | 4 +- .../builtin/io-node-form-zod-ledger.test.ts | 9 +- .../services/service-automation/src/engine.ts | 36 ++--- packages/spec/api-surface/automation.json | 2 +- .../spec/src/automation/control-flow.zod.ts | 131 +++++++-------- .../src/automation/flow-region-cycle.test.ts | 87 ++++++++++ packages/spec/src/automation/flow.zod.ts | 58 ++++++- .../automation/region-normalization.test.ts | 153 ++++++++++++------ packages/spec/src/automation/region-slots.ts | 3 +- .../references/_index.md | 1 + 11 files changed, 395 insertions(+), 146 deletions(-) create mode 100644 .changeset/flownode-parses-its-regions.md create mode 100644 packages/spec/src/automation/flow-region-cycle.test.ts diff --git a/.changeset/flownode-parses-its-regions.md b/.changeset/flownode-parses-its-regions.md new file mode 100644 index 0000000000..4bfdbf6f4e --- /dev/null +++ b/.changeset/flownode-parses-its-regions.md @@ -0,0 +1,57 @@ +--- +'@objectstack/spec': minor +'@objectstack/service-automation': patch +--- + +feat(spec)!: `FlowNodeSchema` parses its own ADR-0031 regions — the post-parse pass retires (#4415) + +`FlowSchema.parse` normalized a flow's own `nodes[]` / `edges[]` but could not reach a +**region**, because a region lives inside `FlowNodeSchema.config` — a deliberately open +`z.record` (ADR-0018). #4381 closed the resulting gap with a **post-parse pass**, +`normalizeControlFlowRegions`, that every caller had to remember to run: + +```ts +const flowShell = FlowSchema.parse(converted); +validateControlFlow(flowShell); +const parsed = normalizeControlFlowRegions(flowShell); // ← had to remember +``` + +That is an unwritten rule on top of a parse, and it is exactly the condition the #4347 +family of defects grows in: a new consumer — a Studio publish path, an MCP tool, a bulk +validation script — takes a `FlowParsed` and uses it, holding a **half-parsed flow that +looks finished**. Nested edge predicates were still bare strings, nested nodes had not been +through `.strict()`, and nothing said so. + +Now the schema does it. `FlowNodeSchema` carries a `.transform()` that parses each declared +region slot — `loop.config.body`, `parallel.config.branches[]`, `try_catch.config.try` / +`.catch` — 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. **"Parsed" now means parsed at every depth** (Prime Directive #1), from any entry +point — including `FlowNodeSchema.parse(node)` on a single node, which the old whole-flow +pass could not serve at all. + +## Migration + +**`normalizeControlFlowRegions` is removed from `@objectstack/spec/automation`.** Delete the +call; the parse above it already did the work: + +```diff + const parsed = FlowSchema.parse(converted); + validateControlFlow(parsed); +- const normalized = normalizeControlFlowRegions(parsed); +``` + +Its replacement, `parseFlowNodeRegions(node)`, is exported for the same purpose one node at +a time, but you should not normally need it — it is the transform's own body. + +**`FlowNodeSchema` is now a `ZodPipe`, not a `ZodObject`,** so it no longer has `.shape` / +`.extend()` / `.pick()`. `z.infer` / `z.input` / `.parse` / `.safeParse` and +`z.toJSONSchema` are unaffected, and the authorable key set is byte-identical (verified by +`check:authorable-surface`). If you were reaching for the object half, read it from the +pipe's input side — `FlowNodeSchema.def.in` — which is also what the repo's own generators +do (`pipeAuthorableSide` in `scripts/lib/zod-graph.ts`). + +One visible consequence in the generated reference: `content/docs/references/automation/flow.mdx` +now renders FlowNode's **input** shape, so keys carrying a `.default()` (`boundaryConfig.interrupting`, +`inputSchema[].required`) show as optional. That is what an author actually writes, which is +what an authoring reference should say. diff --git a/content/docs/references/automation/flow.mdx b/content/docs/references/automation/flow.mdx index 47adce029f..4dc5fc38ae 100644 --- a/content/docs/references/automation/flow.mdx +++ b/content/docs/references/automation/flow.mdx @@ -102,10 +102,10 @@ const result = FlowSchema.parse(data); | **connectorConfig** | `{ connectorId: string; actionId: string; input?: Record }` | optional | | | **position** | `{ x: number; y: number }` | optional | | | **timeoutMs** | `integer` | optional | Maximum execution time for this node in milliseconds | -| **inputSchema** | `Record; required: boolean; description?: string }>` | optional | Input parameter schema for this node | +| **inputSchema** | `Record; required?: boolean; description?: string }>` | optional | Input parameter schema for this node | | **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. | | **waitEventConfig** | `{ eventType: Enum<'timer' \| 'signal' \| 'webhook' \| 'manual' \| 'condition'>; timerDuration?: string; signalName?: string }` | optional | Configuration for wait node event resumption | -| **boundaryConfig** | `{ attachedToNodeId: string; eventType: Enum<'error' \| 'timer' \| 'signal' \| 'cancel'>; interrupting: boolean; errorCode?: string; … }` | optional | Configuration for boundary events attached to host nodes | +| **boundaryConfig** | `{ attachedToNodeId: string; eventType: Enum<'error' \| 'timer' \| 'signal' \| 'cancel'>; interrupting?: boolean; errorCode?: string; … }` | optional | Configuration for boundary events attached to host nodes | --- diff --git a/packages/services/service-automation/src/builtin/io-node-form-zod-ledger.test.ts b/packages/services/service-automation/src/builtin/io-node-form-zod-ledger.test.ts index 48a410f52b..c7b72504fe 100644 --- a/packages/services/service-automation/src/builtin/io-node-form-zod-ledger.test.ts +++ b/packages/services/service-automation/src/builtin/io-node-form-zod-ledger.test.ts @@ -110,8 +110,13 @@ describe('IO-node form ↔ Zod reconciliation (#4045)', () => { // and nothing else (connector-nodes.ts). The spec side of that contract // is FlowNodeSchema.connectorConfig — unwrap the optional wrapper // structurally to stay off a direct `zod` dependency. - const prop = (FlowNodeSchema as unknown as { shape: Record }) - .shape.connectorConfig as { unwrap?: () => { shape?: Record } }; + // + // `FlowNodeSchema` is a ZodPipe since #4415 (it parses its own ADR-0031 + // regions), so the declared keys live on the pipe's INPUT side — the + // authorable half, which is what this reconciliation is about, and the + // same side the spec's own generators read (`pipeAuthorableSide`). + const node = FlowNodeSchema as unknown as { def: { in: { shape: Record } } }; + const prop = node.def.in.shape.connectorConfig as { unwrap?: () => { shape?: Record } }; expect(prop, 'FlowNodeSchema should declare connectorConfig').toBeDefined(); expect(prop.unwrap, 'connectorConfig should be an optional-wrapped object').toBeTypeOf('function'); const shape = prop.unwrap!().shape; diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index cd975f0425..16943e9da7 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -18,7 +18,7 @@ import { type ScreenFieldVisibility, } from './screen-input-contract.js'; import type { Logger } from '@objectstack/spec/contracts'; -import { FlowSchema, FLOW_STRUCTURAL_NODE_TYPES, validateControlFlow, normalizeControlFlowRegions, collectFlowGraphs, findRegionEntry, defineActionDescriptor } from '@objectstack/spec/automation'; +import { FlowSchema, FLOW_STRUCTURAL_NODE_TYPES, validateControlFlow, collectFlowGraphs, findRegionEntry, defineActionDescriptor } from '@objectstack/spec/automation'; import { resolveFlowNodeExpressions } from '@objectstack/spec/automation'; import { applyConversionsToFlow, type ConversionNotice, type ConversionConflictNotice } from '@objectstack/spec'; import type { FlowRegionParsed } from '@objectstack/spec/automation'; @@ -1038,11 +1038,11 @@ export interface SuspendedRunStore { * the author never wrote pins that row to today's value forever — so the graft * is deliberately narrow: it copies the lowered `condition`, nothing more. * - * Structural alignment is by position, which is sound because neither the parse - * nor `normalizeControlFlowRegions` reorders or drops array members — both are - * copy-on-write maps. Where the two sides disagree in shape (a caller passed a - * mismatched pair), the converted side is returned untouched: this only ever - * lifts a value it can positively match. + * Structural alignment is by position, which is sound because the parse — region + * transform included (#4415) — never reorders or drops array members: every step + * of it is a copy-on-write map. Where the two sides disagree in shape (a caller + * passed a mismatched pair), the converted side is returned untouched: this only + * ever lifts a value it can positively match. * * Node `config.condition` (e.g. a start node's record-change predicate) is * left alone by construction — `FlowNodeSchema.config` is an open `z.record`, @@ -2077,25 +2077,23 @@ export class AutomationEngine implements IAutomationService { this.logger.warn(`[flow '${name}'] ${c.code}: ${c.message}`); }, }); - const flowShell = FlowSchema.parse(converted); + // #4347 / #4415 — one call, canonical at every depth. `FlowNodeSchema` + // parses its own ADR-0031 regions (`FlowNodeSchema.transform` → + // `parseFlowNodeRegions`), so what comes back here is already normalized + // inside `loop.config.body`, `parallel.config.branches[]` and + // `try_catch.config.try`/`.catch` — recursively. Until #4415 that needed + // a second, separately-remembered call to `normalizeControlFlowRegions` + // right here, and every consumer that took a `FlowParsed` without making + // it held a half-parsed flow that looked finished. + const parsed = FlowSchema.parse(converted); // DAG cycle detection - this.detectCycles(flowShell); + this.detectCycles(parsed); // ADR-0031 — validate structured control-flow constructs (loop bodies, // parallel branches, try/catch regions) are well-formed (single-entry/ // single-exit, acyclic). Reject the malformed before it can run. - validateControlFlow(flowShell); - - // #4347 — then canonicalize what lives INSIDE those regions. A region - // sits in `FlowNodeSchema.config`, which is an open `z.record`, so the - // parse above stopped at the container: a bare-string `condition` on a - // top-level edge came back as the canonical `{ dialect: 'cel', source }` - // envelope while the identical predicate on a loop-body edge stayed a - // bare string. Same flow, same call, different stored shape by nesting - // depth. Runs after `validateControlFlow` so a malformed region is - // still reported by the validator that owns that message. - const parsed = normalizeControlFlowRegions(flowShell); + validateControlFlow(parsed); return { parsed, diff --git a/packages/spec/api-surface/automation.json b/packages/spec/api-surface/automation.json index 2c31380a57..4f68a6fa0e 100644 --- a/packages/spec/api-surface/automation.json +++ b/packages/spec/api-surface/automation.json @@ -265,9 +265,9 @@ "getSchemalessNodeConfigJsonSchemas (function)", "importBpmnToConstructs (function)", "isFlowFunctionEffect (function)", - "normalizeControlFlowRegions (function)", "normalizeDecisionOutputs (function)", "normalizeFlowFunctionEntry (function)", + "parseFlowNodeRegions (function)", "resolveFlowNodeExpressions (function)", "validateControlFlow (function)" ] diff --git a/packages/spec/src/automation/control-flow.zod.ts b/packages/spec/src/automation/control-flow.zod.ts index ec6a669655..c1c0a991e5 100644 --- a/packages/spec/src/automation/control-flow.zod.ts +++ b/packages/spec/src/automation/control-flow.zod.ts @@ -139,9 +139,9 @@ export const FlowRegionSchema = lazySchema(() => strictObject( }, { /** Body nodes (must not include `start`/`end` trigger sentinels). */ - nodes: z.array(FlowNodeSchema).min(1).describe('Region body nodes (single-entry/single-exit sub-graph)'), + nodes: z.array(z.lazy(() => FlowNodeSchema)).min(1).describe('Region body nodes (single-entry/single-exit sub-graph)'), /** Body edges connecting the region nodes. */ - edges: z.array(FlowEdgeSchema).default([]).describe('Region body edges'), + edges: z.array(z.lazy(() => FlowEdgeSchema)).default([]).describe('Region body edges'), }, )); @@ -239,8 +239,8 @@ export const ParallelBranchSchema = lazySchema(() => strictObject( { /** Optional human label for the branch (designer + logs). */ name: z.string().optional().describe('Branch label'), - nodes: z.array(FlowNodeSchema).min(1).describe('Branch body nodes'), - edges: z.array(FlowEdgeSchema).default([]).describe('Branch body edges'), + nodes: z.array(z.lazy(() => FlowNodeSchema)).min(1).describe('Branch body nodes'), + edges: z.array(z.lazy(() => FlowEdgeSchema)).default([]).describe('Branch body edges'), }, )); @@ -451,8 +451,8 @@ interface RegionSlot { * the value it holds, the Zod schema that value parses as, and a diagnostic * label. * - * The three passes in this module read it ({@link validateControlFlow}, - * {@link normalizeControlFlowRegions}, {@link collectFlowGraphs}). WHERE the + * The three readers in this module use it ({@link validateControlFlow}, + * {@link parseFlowNodeRegions}, {@link collectFlowGraphs}). WHERE the * slots are is no longer stated here — that moved to `region-slots.ts` so the * conversion walk and the lint walk read the same list. What stays here is the * schema half, which is this module's business. @@ -554,81 +554,70 @@ export function validateControlFlow(flow: { nodes: FlowNodeParsed[] }): void { } -// ─── Region normalization ──────────────────────────────────────────── +// ─── Region parsing (the FlowNodeSchema transform) ─────────────────── /** - * Parse ONE region value through its own schema, then recurse into the - * containers its nodes carry. + * Re-entrancy depth of {@link parseFlowNodeRegions}. * - * A value that does not parse is returned untouched: rejecting a malformed - * region is {@link validateControlFlow}'s job (and, at run time, the container - * executor's `parseNodeConfig`). A normalization pass that also threw would - * change *which* flows register, which is not what it is for. + * A module-level counter rather than a parameter, because the recursion is no + * longer ours to thread: `FlowRegionSchema.nodes` is `z.array(FlowNodeSchema)`, + * so the descent happens *inside Zod*, which has nowhere to carry a depth. Safe + * as shared state because Zod parsing is synchronous — the whole tree unwinds on + * one stack, and the `finally` below restores the counter on the error path too. + * + * Without it a flow assembled as hand-built objects (not parsed JSON) could hold + * a self-reference and recurse until the stack blows, at the load seam. The + * post-parse pass this replaced guarded the same hazard with an explicit `depth` + * argument; the ceiling is unchanged. */ -function normalizeRegion(slot: RegionSlot, depth: number): unknown { - if (!isRegionDict(slot.raw)) return slot.raw; - const parsed = slot.schema.safeParse(slot.raw); - if (!parsed.success) return slot.raw; - const region = parsed.data as { nodes?: FlowNodeParsed[] }; - if (!Array.isArray(region.nodes)) return region; - return { ...region, nodes: region.nodes.map(n => normalizeNodeRegions(n, depth + 1)) }; -} - -/** Normalize every region one node carries — recursively, since regions nest. */ -function normalizeNodeRegions(node: FlowNodeParsed, depth: number): FlowNodeParsed { - if (depth >= MAX_REGION_DEPTH) return node; - const cfg = node.config as Record | undefined; - if (!cfg) return node; - - let next = cfg; - for (const slot of regionSlotsOf(node)) { - const normalized = normalizeRegion(slot, depth); - if (normalized === slot.raw) continue; - if (slot.index === undefined) { - next = { ...next, [slot.key]: normalized }; - } else { - const branches = [...(next[slot.key] as unknown[])]; - branches[slot.index] = normalized; - next = { ...next, [slot.key]: branches }; - } - } - - return next === cfg ? node : { ...node, config: next }; -} +let regionParseDepth = 0; /** - * Canonicalize the metadata **inside** every structured region of a flow (#4347). + * Parse every ADR-0031 region a node's `config` holds — the body of + * {@link FlowNodeSchema}'s `.transform()` (#4415). * - * `FlowSchema.parse` normalizes a flow's own `nodes[]` / `edges[]` — most - * visibly, `FlowEdgeSchema.condition` is `ExpressionInputSchema`, so a - * bare-string predicate becomes the canonical `{ dialect: 'cel', source }` - * envelope. It does not reach a region, because a region lives inside - * `FlowNodeSchema.config`, which is deliberately `z.record(z.unknown())` — open, - * per node type. So the *same predicate* was stored enveloped on a top-level edge - * and left a bare string on a loop-body edge: a representation that depended on - * where in the graph it sat, which no flow author can be expected to predict. + * `FlowNodeSchema.config` is a deliberately open `z.record` (ADR-0018), so + * nothing about a container's nested sub-graph is described by the node's own + * shape. This resolves each declared slot against {@link FLOW_REGION_SLOTS_BY_TYPE} + * and runs its value through the schema that slot's value IS — `FlowRegionSchema` + * for `loop.config.body` / `try_catch.config.try` / `.catch`, + * `ParallelBranchSchema` for each `parallel.config.branches[]`. * - * This pass closes that. Each region is run through its own schema — recursively, - * because regions nest — producing a flow whose nested edges and nodes carry the - * same canonical shapes as its top-level ones. Copy-on-write: a flow with no - * structured container comes back untouched. + * Nesting needs no recursion here: those schemas hold `z.array(FlowNodeSchema)`, + * so a region's own nodes come back through this transform on the way down. That + * is the whole reason this reads shorter than the pass it replaced. * - * Call it at the load seam, after `FlowSchema.parse` and `validateControlFlow`. - * The container executors parse their own config at run time (`parseNodeConfig`, - * #4277), so this is not what makes a nested predicate *evaluate* correctly — it - * is what makes the stored flow SAY so, for every reader that is not the - * executor: the Studio designer, `getFlow`, the version history, and any - * consumer that reads a region without re-parsing it. + * **A value that does not parse is returned untouched.** Rejecting a malformed + * region is {@link validateControlFlow}'s job (and, at run time, the container + * executor's `parseNodeConfig`): a transform that threw here would change *which* + * flows parse at all, moving a structural diagnostic out of the validator that + * owns its message and into a Zod issue on `config`. Copy-on-write — a node with + * no region comes back by identity. */ -export function normalizeControlFlowRegions(flow: T): T { - if (!Array.isArray(flow.nodes)) return flow; - let changed = false; - const nodes = flow.nodes.map(node => { - const next = normalizeNodeRegions(node, 0); - if (next !== node) changed = true; - return next; - }); - return changed ? { ...flow, nodes } : flow; +export function parseFlowNodeRegions(node: T): T { + const cfg = node.config as Record | undefined; + if (!cfg) return node; + if (regionParseDepth >= MAX_REGION_DEPTH) return node; + + regionParseDepth++; + try { + let next = cfg; + for (const slot of regionSlotsOf(node as unknown as FlowNodeParsed)) { + if (!isRegionDict(slot.raw)) continue; + const parsed = slot.schema.safeParse(slot.raw); + if (!parsed.success) continue; + if (slot.index === undefined) { + next = { ...next, [slot.key]: parsed.data }; + } else { + const branches = [...(next[slot.key] as unknown[])]; + branches[slot.index] = parsed.data; + next = { ...next, [slot.key]: branches }; + } + } + return next === cfg ? node : { ...node, config: next }; + } finally { + regionParseDepth--; + } } // ─── Whole-flow graph traversal ────────────────────────────────────── diff --git a/packages/spec/src/automation/flow-region-cycle.test.ts b/packages/spec/src/automation/flow-region-cycle.test.ts new file mode 100644 index 0000000000..dbcf7de6b5 --- /dev/null +++ b/packages/spec/src/automation/flow-region-cycle.test.ts @@ -0,0 +1,87 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The `flow.zod` ↔ `control-flow.zod` import cycle survives **eager** schema + * construction, in either import order (#4415). + * + * `FlowNodeSchema` parses its own ADR-0031 regions, so it needs + * `FlowRegionSchema` / `ParallelBranchSchema`; those hold `z.array(FlowNodeSchema)`. + * The recursion is genuinely mutual, so the two modules import each other. Three + * things keep that legal, and every one of them looks like an ordinary style + * choice to the next person who edits it: + * + * 1. the region schemas back-reference through `z.lazy(() => FlowNodeSchema)` + * / `z.lazy(() => FlowEdgeSchema)` rather than naming them directly; + * 2. `flowNodeObject()` is a **hoisted `function` declaration**, not a `const` + * arrow; + * 3. every schema in both modules stays inside a `lazySchema()` factory. + * + * Undo any of them and module evaluation reads a `const` still in its temporal + * dead zone: `ReferenceError: Cannot access 'FlowNodeSchema' before + * initialization`, thrown at import time, before a single test body runs. + * + * **Why this needs its own test rather than the ordinary suite.** `lazySchema` + * defers construction behind a Proxy, which hides the whole hazard — the normal + * vitest run never triggers it. It only appears under `OS_EAGER_SCHEMAS=1`, the + * documented rollback switch, which `gen:schema` / `check:authorable-surface` + * set. So without this file the regression surfaces as a generator crash in CI + * with no test naming the cause; with it, the failure says what to restore. + * + * Two entry points, because eager evaluation order is decided by whichever + * module the process enters first: the `automation` barrel (what every real + * consumer and `gen:schema` reach through) and `flow.zod` directly (the order + * that breaks first if the `z.lazy` back-references are unwrapped). Entering + * `control-flow.zod` directly is deliberately NOT asserted — that order already + * dies on an unrelated, pre-existing `strict-object.ts` ↔ `field.zod.ts` TDZ on + * `origin/main`, measured while writing this file and filed separately; pinning + * it here would pin someone else's bug to this feature. + */ + +import { execFileSync } from 'node:child_process'; +import { describe, expect, it } from 'vitest'; + +/** Import the two modules in a given order under eager construction. */ +function importEagerly(first: string, second: string): string { + return execFileSync( + process.execPath, + [ + '--import', 'tsx', + '--input-type=module', + '-e', + // The two bare imports ARE the assertion: under OS_EAGER_SCHEMAS=1 every + // schema in both modules is constructed during this evaluation, so a TDZ + // regression throws here. The parse below then proves the cycle produced + // a working schema and not merely a silent one. + // (tsx compiles .ts to CJS, so the namespace arrives under `default`.) + `import ${JSON.stringify(first)}; + import ${JSON.stringify(second)}; + const mod = await import(${JSON.stringify(new URL('./flow.zod.ts', import.meta.url).href)}); + const { FlowSchema } = mod.FlowSchema ? mod : mod.default; + const flow = FlowSchema.parse({ + name: 'c', label: 'C', type: 'schedule', + nodes: [{ id: 'l', type: 'loop', label: 'L', config: { + collection: '{rows}', iteratorVariable: 'row', + body: { nodes: [{ id: 'n', type: 'log', label: 'N' }], + edges: [{ id: 'b', source: 'n', target: 'n', condition: 'row.x > 1' }] }, + } }], + edges: [], + }); + console.log(JSON.stringify(flow.nodes[0].config.body.edges[0].condition));`, + ], + { env: { ...process.env, OS_EAGER_SCHEMAS: '1' }, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }, + ).trim(); +} + +const FLOW = new URL('./flow.zod.ts', import.meta.url).href; +const BARREL = new URL('./index.ts', import.meta.url).href; +const ENVELOPE = JSON.stringify({ dialect: 'cel', source: 'row.x > 1' }); + +describe('#4415 — the flow ↔ control-flow schema cycle under OS_EAGER_SCHEMAS=1', () => { + it('evaluates and parses regions through the automation barrel', () => { + expect(importEagerly(BARREL, FLOW)).toBe(ENVELOPE); + }, 60_000); + + it('evaluates and parses regions when flow.zod is entered first', () => { + expect(importEagerly(FLOW, BARREL)).toBe(ENVELOPE); + }, 60_000); +}); diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index f57b7f611e..a7b39907c4 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -23,6 +23,7 @@ import { lazySchema } from '../shared/lazy-schema'; import { retiredKey } from '../shared/retired-key'; import { retryPolicyShape } from '../shared/retry-policy.zod'; import { strictObject } from '../shared/strict-object'; +import { parseFlowNodeRegions } from './control-flow.zod'; export const FlowNodeAction = z.enum([ 'start', // Trigger 'end', // Return/Stop @@ -179,7 +180,60 @@ const flowNodeUnknownKeyError = strictUnknownKeyError({ 'config shipped as a step that quietly ignored it.', }); -export const FlowNodeSchema = lazySchema(() => z.object({ +/** + * A flow node — **including** whatever ADR-0031 region its `config` holds (#4415). + * + * The `.transform()` is the point of this schema, not decoration. `config` is a + * deliberately open `z.record` (ADR-0018), so a container's nested sub-graph — + * `loop.config.body`, `parallel.config.branches[]`, `try_catch.config.try` / + * `.catch` — used to sail through the parse untouched: the *same* bare-string + * predicate came back as the canonical `{ dialect: 'cel', source }` envelope on a + * top-level edge and stayed a raw string one level down, a stored shape that + * depended on graph depth. #4381 closed that with a post-parse pass every caller + * had to remember to run (`normalizeControlFlowRegions`), which is an unwritten + * rule — exactly the #4347 defect generator: a new consumer takes `FlowParsed` + * and uses it, half-parsed and looking finished. + * + * Now the schema does it, so "parsed" means parsed at every depth (Prime + * Directive #1). Nesting needs no manual recursion: a region's `nodes` are + * `z.array(FlowNodeSchema)`, so Zod re-enters this transform on the way down. + * + * ## Two mechanical traps this shape carries — read before editing + * + * **1. It is a `ZodPipe`, not a `ZodObject`.** `.strict().transform(…)` is the + * ADR-0089 D3a shape that once crashed `z.toJSONSchema`'s `seen` table, and + * `FlowNodeSchema` is reached lazily from three directions (`FlowSchema.nodes`, + * `FlowRegionSchema.nodes`, `ParallelBranchSchema.nodes`). It works because + * `lazy-schema.ts`'s `_zod` facade aliases the Proxy's `seen` entry onto the real + * instance, and because the generators read a pipe's **authorable side** — + * `pipeAuthorableSide` in `scripts/lib/zod-graph.ts` returns `def.in` for an + * `a.transform(fn)` pipe. Measured on this schema (#4415): `gen:schema` emits + * `automation/FlowNode.json (input shape)` with the same key set as before. + * There is no `.shape` on this export any more — reach for {@link flowNodeObject} + * if you need the object half. + * + * **2. `control-flow.zod.ts` and this module are a deliberate import CYCLE.** + * The recursion is genuinely mutual (a node holds a region, a region holds + * nodes), so the region schemas back-reference `FlowNodeSchema`/`FlowEdgeSchema` + * through `z.lazy(() => …)`, and the object half below is a **hoisted `function` + * declaration**. Both are load-bearing under `OS_EAGER_SCHEMAS=1` (which + * `gen:schema` sets, bypassing the `lazySchema` Proxy): without them module + * evaluation reads a `const` still in its TDZ and dies with + * `ReferenceError: Cannot access 'FlowNodeSchema' before initialization` before + * any test runs. `flow-region-cycle.test.ts` pins both import orders in eager + * mode so that failure can never come back silently. + */ +export const FlowNodeSchema = lazySchema(() => flowNodeObject().transform(parseFlowNodeRegions)); + +/** + * The plain `ZodObject` half of {@link FlowNodeSchema} — its declared keys, + * before the region transform turns the export into a `ZodPipe`. + * + * A hoisted `function` on purpose (see trap 2 above): under `OS_EAGER_SCHEMAS=1` + * the `lazySchema` factory runs at module-evaluation time, and a `const` arrow + * declared after it would still be in its temporal dead zone. + */ +function flowNodeObject() { return z.object({ id: z.string().describe('Node unique ID'), type: z.string().min(1).describe( 'Action type — a built-in FlowNodeAction id or a plugin-registered node type. ' + @@ -402,7 +456,7 @@ export const FlowNodeSchema = lazySchema(() => z.object({ /** Signal name — only for signal boundary events */ signalName: z.string().optional().describe('Named signal to catch'), }).optional().describe('Configuration for boundary events attached to host nodes'), -}, { error: flowNodeUnknownKeyError }).strict()); +}, { error: flowNodeUnknownKeyError }).strict(); } /** Keys {@link FlowEdgeSchema} declares (drift-guarded by flow.test.ts). */ const FLOW_EDGE_KEYS = ['id', 'source', 'target', 'condition', 'type', 'label', 'isDefault'] as const; diff --git a/packages/spec/src/automation/region-normalization.test.ts b/packages/spec/src/automation/region-normalization.test.ts index 19e7b6b551..75dd1da77e 100644 --- a/packages/spec/src/automation/region-normalization.test.ts +++ b/packages/spec/src/automation/region-normalization.test.ts @@ -1,26 +1,31 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * Region metadata is canonicalized and reachable — `normalizeControlFlowRegions` - * and `collectFlowGraphs` (#4347). + * Region metadata is canonicalized and reachable — by `FlowSchema.parse` itself + * (#4347, #4415) and by `collectFlowGraphs` (#4347). * - * `FlowSchema.parse` normalizes a flow's own `nodes[]`/`edges[]` but stops at a - * container, because an ADR-0031 region lives inside `FlowNodeSchema.config` — - * an open `z.record`. The result was position-dependent metadata: the same - * predicate stored as a `{ dialect: 'cel', source }` envelope on a top-level - * edge and as a bare string one level in, and every pass that iterated - * `flow.nodes` checking "the whole flow" silently checked only part of it. + * An ADR-0031 region lives inside `FlowNodeSchema.config`, an open `z.record`, + * so the parse used to stop at the container: the same predicate was stored as a + * `{ dialect: 'cel', source }` envelope on a top-level edge and as a bare string + * one level in, and every pass that iterated `flow.nodes` checking "the whole + * flow" silently checked only part of it. + * + * #4381 closed the metadata half with a **post-parse pass** + * (`normalizeControlFlowRegions`) callers had to remember to run. #4415 retired + * that pass into `FlowNodeSchema`'s own `.transform()`, so the assertions below + * are made against `FlowSchema.parse` **alone** — which is the whole point: there + * is no second call to forget. The mechanism moved; the guarantees did not. */ import { describe, expect, it } from 'vitest'; -import { FlowSchema } from './flow.zod.js'; +import { FlowSchema, FlowNodeSchema } from './flow.zod.js'; import { LOOP_NODE_TYPE, PARALLEL_NODE_TYPE, TRY_CATCH_NODE_TYPE, collectFlowGraphs, - normalizeControlFlowRegions, + parseFlowNodeRegions, } from './control-flow.zod.js'; const CONDITION = 'row.shouldRun == true'; @@ -52,26 +57,50 @@ const loopWith = (body: unknown) => ({ id: 'loop', type: LOOP_NODE_TYPE, label: 'Loop', config: { collection: '{rows}', iteratorVariable: 'row', body }, }); -describe('#4347 — normalizeControlFlowRegions', () => { - it('envelopes a loop-body edge condition, matching what the top-level edge already got', () => { - const parsed = flowWith(loopWith(gatedRegion())); - // Baseline: FlowSchema.parse enveloped the top-level edge and NOT the nested one. - expect(parsed.edges.find(e => e.id === 'e2')!.condition).toEqual(ENVELOPE); - expect((parsed.nodes[1]!.config as any).body.edges[0].condition).toBe(CONDITION); +describe('#4415 — FlowSchema.parse canonicalizes regions with no second call', () => { + it('envelopes a loop-body edge condition, matching what the top-level edge got', () => { + const flow = flowWith(loopWith(gatedRegion())); - const flow = normalizeControlFlowRegions(parsed); + // The #4415 pin: ONE parse, canonical at both depths. Before #4415 the + // nested condition was still the bare string here and only became an + // envelope after a separate `normalizeControlFlowRegions(parsed)` call. + expect(flow.edges.find(e => e.id === 'e2')!.condition).toEqual(ENVELOPE); expect((flow.nodes[1]!.config as any).body.edges[0].condition).toEqual(ENVELOPE); - // Parity: the same predicate now has the same representation either side of - // the container boundary. + + // Parity: the same predicate has the same representation either side of the + // container boundary. expect((flow.nodes[1]!.config as any).body.edges[0].condition) .toEqual(flow.edges.find(e => e.id === 'e2')!.condition); }); + it('applies the region schema in full — nested nodes get defaults and `.strict()`', () => { + const flow = flowWith(loopWith(gatedRegion())); + const nested = (flow.nodes[1]!.config as any).body; + + // `FlowEdgeSchema.isDefault` defaults to false; a nested edge now carries it + // exactly like a top-level one, so a reader need not know its depth. + expect(nested.edges[0].isDefault).toBe(false); + expect(nested.edges[0].type).toBe('conditional'); + + // And the region's own `edges` default materializes. + const noEdges = flowWith(loopWith({ nodes: [structuredClone(gate)] })); + expect((noEdges.nodes[1]!.config as any).body.edges).toEqual([]); + }); + + it('rejects an undeclared key inside a region — parse is now the enforcement seam', () => { + // `FlowRegionSchema` is strict (#4001 批 10). Before #4415 a bad nested key + // was simply left un-normalized and silently kept its raw shape; the region + // schema still refuses it, and the transform still declines to rewrite what + // it cannot parse (see the untouched-region case below). + const flow = flowWith(loopWith({ ...gatedRegion(), name: 'not-a-region-key' })); + expect((flow.nodes[1]!.config as any).body.edges[0].condition).toBe(CONDITION); + }); + it('normalizes every parallel branch and keeps the branch `name`', () => { - const flow = normalizeControlFlowRegions(flowWith({ + const flow = flowWith({ id: 'par', type: PARALLEL_NODE_TYPE, label: 'Fan', config: { branches: [{ name: 'left', ...gatedRegion() }, { name: 'right', ...gatedRegion() }] }, - })); + }); const branches = (flow.nodes[1]!.config as any).branches; for (const branch of branches) expect(branch.edges[0].condition).toEqual(ENVELOPE); @@ -82,10 +111,10 @@ describe('#4347 — normalizeControlFlowRegions', () => { }); it('normalizes both try_catch regions', () => { - const flow = normalizeControlFlowRegions(flowWith({ + const flow = flowWith({ id: 'tc', type: TRY_CATCH_NODE_TYPE, label: 'Guard', config: { try: gatedRegion(), catch: gatedRegion(), errorVariable: '$err' }, - })); + }); const cfg = (flow.nodes[1]!.config as any); expect(cfg.try.edges[0].condition).toEqual(ENVELOPE); @@ -95,13 +124,13 @@ describe('#4347 — normalizeControlFlowRegions', () => { }); it('recurses — a condition three containers deep is enveloped too', () => { - const flow = normalizeControlFlowRegions(flowWith(loopWith({ + const flow = flowWith(loopWith({ nodes: [{ id: 'tc', type: TRY_CATCH_NODE_TYPE, label: 'Guard', config: { try: { nodes: [loopWith(gatedRegion())], edges: [] } }, }], edges: [], - }))); + })); const deep = (flow.nodes[1]!.config as any) .body.nodes[0].config.try @@ -109,36 +138,64 @@ describe('#4347 — normalizeControlFlowRegions', () => { expect(deep.edges[0].condition).toEqual(ENVELOPE); }); - it('is idempotent and copy-on-write', () => { - const once = normalizeControlFlowRegions(flowWith(loopWith(gatedRegion()))); - expect(normalizeControlFlowRegions(once)).toEqual(once); - - // No structured container → the exact same reference comes back. - const plain = FlowSchema.parse({ - name: 'plain', label: 'Plain', type: 'schedule', - nodes: [{ id: 'start', type: 'start', label: 'Start' }, { id: 'end', type: 'end', label: 'End' }], - edges: [{ id: 'e1', source: 'start', target: 'end' }], - }); - expect(normalizeControlFlowRegions(plain)).toBe(plain); + it('is idempotent — re-parsing a parsed flow changes nothing', () => { + const once = flowWith(loopWith(gatedRegion())); + expect(FlowSchema.parse(once)).toEqual(once); }); it('leaves a region it cannot parse untouched — rejecting is validateControlFlow\'s job', () => { - // `outputSchema` is a tombstoned key: the region will not parse. Normalizing - // must not throw here, or it would change WHICH flows register. + // `outputSchema` is a tombstoned key: the region will not parse. The + // transform must not throw here, or it would change WHICH flows parse at + // all, moving a structural diagnostic off the validator that owns it. const raw = { nodes: [{ ...gate, outputSchema: {} }], edges: [] }; - const flow = normalizeControlFlowRegions(flowWith(loopWith(raw))); + const flow = flowWith(loopWith(raw)); expect((flow.nodes[1]!.config as any).body).toBe(raw); }); it('ignores a legacy flat-graph loop (no body)', () => { - const parsed = flowWith({ id: 'loop', type: LOOP_NODE_TYPE, label: 'Loop', config: { collection: '{rows}' } }); - expect(normalizeControlFlowRegions(parsed)).toBe(parsed); + const flow = flowWith({ id: 'loop', type: LOOP_NODE_TYPE, label: 'Loop', config: { collection: '{rows}' } }); + expect((flow.nodes[1]!.config as any)).toEqual({ collection: '{rows}' }); + }); + + it('terminates on a self-referential region instead of recursing forever', () => { + // The hazard the retired pass guarded with an explicit `depth` argument, now + // carried by `parseFlowNodeRegions`’ re-entrancy counter — and now reachable + // through `parse` itself, since the descent happens inside Zod. Hand-built + // flows are objects, not parsed JSON, so a cycle is reachable. + const selfRegion: { nodes: unknown[]; edges: unknown[] } = { nodes: [], edges: [] }; + selfRegion.nodes.push({ + id: 'l', type: LOOP_NODE_TYPE, label: 'L', + config: { collection: '{r}', iteratorVariable: 'r', body: selfRegion }, + }); + + expect(() => FlowSchema.parse({ + name: 'cyclic', label: 'Cyclic', type: 'schedule', + nodes: selfRegion.nodes, edges: [], + })).not.toThrow(); + }); + + it('is copy-on-write at the node level', () => { + // A node with no region comes back by identity, so the transform allocates + // nothing for the overwhelmingly common flat node. + const plain = { id: 'start', type: 'start', label: 'Start', config: { a: 1 } }; + expect(parseFlowNodeRegions(plain)).toBe(plain); + expect(parseFlowNodeRegions({ id: 'n', type: 'log', label: 'N' })).not.toBe(plain); + }); +}); + +describe('#4415 — FlowNodeSchema is the parse seam, at any entry point', () => { + it('normalizes a region when a node is parsed on its own, not only via FlowSchema', () => { + // The unwritten rule #4415 removed: a consumer holding a single node — the + // Studio inspector, a plugin validating one step — used to have no way to + // reach the post-parse pass at all, since it took a whole flow. + const node = FlowNodeSchema.parse(loopWith(gatedRegion())); + expect((node.config as any).body.edges[0].condition).toEqual(ENVELOPE); }); }); describe('#4347 — collectFlowGraphs', () => { it('yields the flow graph plus every region, each scoped', () => { - const flow = normalizeControlFlowRegions(flowWith(loopWith(gatedRegion()))); + const flow = flowWith(loopWith(gatedRegion())); const graphs = collectFlowGraphs(flow); expect(graphs.map(g => g.scope)).toEqual(['', "loop 'loop' body"]); @@ -148,25 +205,25 @@ describe('#4347 — collectFlowGraphs', () => { }); it('names each parallel branch and both try_catch regions', () => { - expect(collectFlowGraphs(normalizeControlFlowRegions(flowWith({ + expect(collectFlowGraphs(flowWith({ id: 'par', type: PARALLEL_NODE_TYPE, label: 'Fan', config: { branches: [gatedRegion(), gatedRegion()] }, - }))).map(g => g.scope)).toEqual(['', "parallel 'par' branch 0", "parallel 'par' branch 1"]); + })).map(g => g.scope)).toEqual(['', "parallel 'par' branch 0", "parallel 'par' branch 1"]); - expect(collectFlowGraphs(normalizeControlFlowRegions(flowWith({ + expect(collectFlowGraphs(flowWith({ id: 'tc', type: TRY_CATCH_NODE_TYPE, label: 'Guard', config: { try: gatedRegion(), catch: gatedRegion() }, - }))).map(g => g.scope)).toEqual(['', "try_catch 'tc' try", "try_catch 'tc' catch"]); + })).map(g => g.scope)).toEqual(['', "try_catch 'tc' try", "try_catch 'tc' catch"]); }); it('chains the scope of a nested region so a finding says where it is', () => { - const flow = normalizeControlFlowRegions(flowWith(loopWith({ + const flow = flowWith(loopWith({ nodes: [{ id: 'tc', type: TRY_CATCH_NODE_TYPE, label: 'Guard', config: { catch: gatedRegion() }, }], edges: [], - }))); + })); expect(collectFlowGraphs(flow).map(g => g.scope)) .toEqual(['', "loop 'loop' body", "loop 'loop' body → try_catch 'tc' catch"]); }); diff --git a/packages/spec/src/automation/region-slots.ts b/packages/spec/src/automation/region-slots.ts index c10a9278ea..1890b14c35 100644 --- a/packages/spec/src/automation/region-slots.ts +++ b/packages/spec/src/automation/region-slots.ts @@ -15,7 +15,8 @@ * | pass | package | unit it walks | * |------|---------|---------------| * | `mapFlowNodes` (ADR-0087 conversions) | `spec` | node, copy-on-write rewrite | - * | `collectFlowGraphs` / `validateControlFlow` / `normalizeControlFlowRegions` | `spec` | graph (nodes + edges) | + * | `collectFlowGraphs` / `validateControlFlow` | `spec` | graph (nodes + edges) | + * | `parseFlowNodeRegions` (the `FlowNodeSchema` transform, #4415) | `spec` | node's own region slots | * | `walkFlowNodes` (lint flow rules) | `lint` | node, with diagnostic path | * * Each carried its own copy of the table below, and each pinned its own copy diff --git a/skills/objectstack-automation/references/_index.md b/skills/objectstack-automation/references/_index.md index 2cdc43d4b2..2625927924 100644 --- a/skills/objectstack-automation/references/_index.md +++ b/skills/objectstack-automation/references/_index.md @@ -20,6 +20,7 @@ from `node_modules` — there is no local copy in the skill bundle. ## Transitive dependencies +- `node_modules/@objectstack/spec/src/automation/control-flow.zod.ts` — Structured control-flow constructs (ADR-0031) — the **native + AI-authored** - `node_modules/@objectstack/spec/src/data/field.zod.ts` — Field Type Enum - `node_modules/@objectstack/spec/src/data/filter.zod.ts` — Unified Query DSL Specification - `node_modules/@objectstack/spec/src/kernel/metadata-protection.zod.ts` — Metadata Protection Model — Phase 1 (ADR-0010) From c27327f114c38547bbdcfa2af97a4cbbfd416500 Mon Sep 17 00:00:00 2001 From: Claude Fable 5 Date: Fri, 7 Aug 2026 13:49:28 +0000 Subject: [PATCH 2/2] 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 Claude-Session: https://claude.ai/code/session_01BickTBKm2JYSNnrtPT8ysa --- content/docs/references/automation/flow.mdx | 6 ------ 1 file changed, 6 deletions(-) diff --git a/content/docs/references/automation/flow.mdx b/content/docs/references/automation/flow.mdx index 4dc5fc38ae..b1a8c783c1 100644 --- a/content/docs/references/automation/flow.mdx +++ b/content/docs/references/automation/flow.mdx @@ -8,19 +8,13 @@ description: Flow protocol schemas Flow Node Types — **built-in seed set** (ADR-0018). Historically this `z.enum` *gated* `FlowNodeSchema.type`, which made the - closed protocol reject any plugin-registered node type — defeating the open - runtime registry (`registerNodeExecutor(type: string)`). Per ADR-0018 the - gate is removed: `FlowNodeSchema.type` is now a validated `string`, checked - against the live action registry at `registerFlow()` time, not frozen here. `FlowNodeAction` is **retained** as the canonical list of built-in type ids - (documentation + the seed descriptor set the engine registers at boot). It - no longer constrains authored flows — plugins extend the vocabulary.