Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions .changeset/flownode-parses-its-regions.md
Original file line number Diff line number Diff line change
@@ -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.
4 changes: 2 additions & 2 deletions content/docs/references/automation/flow.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,10 @@ const result = FlowSchema.parse(data);
| **connectorConfig** | `{ connectorId: string; actionId: string; input?: Record<string, any> }` | optional | |
| **position** | `{ x: number; y: number }` | optional | |
| **timeoutMs** | `integer` | optional | Maximum execution time for this node in milliseconds |
| **inputSchema** | `Record<string, { type: Enum<'string' \| 'number' \| 'boolean' \| 'object' \| 'array'>; required: boolean; description?: string }>` | optional | Input parameter schema for this node |
| **inputSchema** | `Record<string, { type: Enum<'string' \| 'number' \| 'boolean' \| 'object' \| 'array'>; 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 |


---
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown> })
.shape.connectorConfig as { unwrap?: () => { shape?: Record<string, unknown> } };
//
// `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<string, unknown> } } };
const prop = node.def.in.shape.connectorConfig as { unwrap?: () => { shape?: Record<string, unknown> } };
expect(prop, 'FlowNodeSchema should declare connectorConfig').toBeDefined();
expect(prop.unwrap, 'connectorConfig should be an optional-wrapped object').toBeTypeOf('function');
const shape = prop.unwrap!().shape;
Expand Down
36 changes: 17 additions & 19 deletions packages/services/service-automation/src/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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`,
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion packages/spec/api-surface/automation.json
Original file line number Diff line number Diff line change
Expand Up @@ -265,9 +265,9 @@
"getSchemalessNodeConfigJsonSchemas (function)",
"importBpmnToConstructs (function)",
"isFlowFunctionEffect (function)",
"normalizeControlFlowRegions (function)",
"normalizeDecisionOutputs (function)",
"normalizeFlowFunctionEntry (function)",
"parseFlowNodeRegions (function)",
"resolveFlowNodeExpressions (function)",
"validateControlFlow (function)"
]
Expand Down
131 changes: 60 additions & 71 deletions packages/spec/src/automation/control-flow.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
},
));

Expand Down Expand Up @@ -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'),
},
));

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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<string, unknown> | 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<T extends { nodes: FlowNodeParsed[] }>(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<T extends { type: string; config?: unknown }>(node: T): T {
const cfg = node.config as Record<string, unknown> | 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 ──────────────────────────────────────
Expand Down
Loading
Loading