diff --git a/.changeset/15646-structured-region-pause-and-end-refused.md b/.changeset/15646-structured-region-pause-and-end-refused.md new file mode 100644 index 00000000000..7b62c00f4ab --- /dev/null +++ b/.changeset/15646-structured-region-pause-and-end-refused.md @@ -0,0 +1,42 @@ +--- +'@objectstack/spec': minor +--- + +**BREAKING for authored metadata** — an ADR-0031 structured region body (`loop.config.body`, a `parallel` branch, `try_catch`'s `try` / `catch`) now refuses two node populations at parse: a node whose TYPE parks the run on every execution, and an `end` node (#15646, absorbing #18112). + +Clause-②: yes + +The flow accept set shrinks for five node types inside region bodies — shapes the runtime never honoured. Both refusals are the authoring-time enforcement of a limit the engine already holds at run time and #3267 ruled 禁: **a region body runs synchronously inside the enclosing run, so it can neither park that run nor terminate it.** + +``` +✗ nodes.1.config.body.nodes.0.type: A `approval` node may not sit inside a structured region — + `loop 'sweep' body → try_catch 'guard' try` is a region body and the `approval` node `sign_off` + is inside it. A region body runs synchronously and cannot durably pause … +``` + +**What is refused** + +- **A node that pauses on EVERY execution** — `screen`, `wait`, `approval`, `approval_revise`. +- **An `end` node**, whatever its `outcome`. An `end` in a region was a no-op, and a refusing one was converted into a region error at the boundary; neither is what the author wrote. + +**⛔ What is deliberately NOT refused: `subflow` and `map`.** Their shipped executors also declare `supportsPause: true`, but they pause exactly when the child flow their `config.flowName` names pauses — a **different metadata record**, not in hand while this flow is parsed. Refusing them by type would also refuse `loop { map(synchronous child) }`, a shape that runs correctly today and is covered by an existing regression suite. A parse-time rule refuses what is statically wrong; a region-contained node that actually suspends is a fact only the run holds. **Nothing an author wrote with a region-nested `map` or `subflow` needs editing for this release.** + +**Why it was silent, measured.** The engine converts a suspension raised inside a region into an error — but the executor has already written its progress state into the ENCLOSING scope by then. Contain that error in a `try_catch` and the residue is read back as progress by the next entry to the same node. On a real `AutomationEngine`, `loop { try_catch { map(pausing child) } }` over 3 iterations × 2 items: not one item's subflow completed, only two of three iterations reached the catch, and iteration 3 read `started === collection.length`, ran nothing, and returned `success` with `summary.failed = 0`. ⚠️ Read that for the MECHANISM, not for this change's reach — the shape it was measured on is a `map`, and making that run's refusal loud is a separate change to the automation engine, not this one. + +### Migration — FROM → TO + +| You wrote | Write instead | +| --- | --- | +| `loop { body: [ …, end ] }` | `loop { body: [ … ] } → end` — give the region a normal exit and put the terminator, with its `outcome` / `message`, on the top-level graph | +| `loop { body: [ wait ] }` | a top-level `wait`, with the top-level graph as the repeating construct — a region body cannot park the run, so the nested form never waited | +| `parallel { branches: [ [ approval ] , … ] }` | put the `approval` on the top-level graph and fan out around it, or split the branch's pausing half into a `subflow` the top-level graph calls | + +The one-line fix is always the same: **move the node onto the top-level graph and route the region's exit to it.** ⛔ Not mechanically convertible — hoisting a node out of a region is a graph rewrite (new edges, a changed exit, sometimes a deleted container) and which shape the author meant is an intent no artifact records, so this ships as an ADR-0087 D3 structured TODO rather than a D2 conversion. + + + +**⚠️ Two boundaries this refusal does not reach, stated rather than discovered.** A pausing node type contributed by a **plugin** is not refused: ADR-0018 left the node-type namespace open and a parse has no registry. A region nested past **`MAX_REGION_DEPTH` (32)** is not judged: the parse walk stops there, and unlike a duplicate node id there is no second spec refusal behind it. For both, the engine's run-time refusal is the only one — unchanged by this change, and not fixed by it. + +⛔ No engine source is edited. What the refusal does to the run time is stated rather than left to be discovered: `AutomationEngine.registerFlow` and the ADR-0087 stored-row rehydration seam both go through `FlowSchema.parse` (`canonicalizeStoredFlow`), so a flow carrying a refused shape no longer registers or rehydrates — it is met at LOAD, not at the region boundary, and a stored row that carries one stops loading until it is rewritten. The engine's own run-time refusals for these shapes stay in place but are reachable only through the two boundaries above; for the `end` arm those are the only remaining path, because the refusal signal it answers is raised at exactly one site — an `end` node whose `outcome` is `refused`. + +**Published surface.** `FLOW_PAUSE_CAPABLE_NODE_TYPES` is published with the four types above. ⚠️ Read its contents, not its name: it is the UNCONDITIONALLY pausing set, not every type that can pause — `subflow` and `map` declare `supportsPause: true` and are deliberately absent, for the reason above. The identifier is unchanged, so this release removes no export. diff --git a/packages/services/service-automation/src/end-node-refused-outcome.test.ts b/packages/services/service-automation/src/end-node-refused-outcome.test.ts index 6928d4bf661..56935aec6d5 100644 --- a/packages/services/service-automation/src/end-node-refused-outcome.test.ts +++ b/packages/services/service-automation/src/end-node-refused-outcome.test.ts @@ -384,48 +384,70 @@ describe('#15788 — the region boundary, made loud', () => { * throw as the try region FAILING: the author's refusal would run the catch * handler and the run would still record `completed`. * - * ⛔ Nothing an author had is narrowed. Before #15788 an `end` inside a - * region was a no-op whatever its `outcome`, so this shape has never once - * been honoured; whether a refusal should instead propagate out of a region - * is a real question the #14945 ruling does not answer. + * ⚠️ #15646 moved that refusal one door EARLIER, and this case moved with + * it. `FlowSchema` now refuses an `end` node inside any ADR-0031 region + * body at parse, and `registerFlow` parses (`canonicalizeStoredFlow` → + * `FlowSchema.parse`), so the fixture below no longer registers at all and + * the run this case used to drive is unreachable. The fixture is + * deliberately unchanged, so this case still fails the day the shape + * becomes declarable again — a re-home, ⛔ not a deletion. + * + * ⚠️ What it no longer covers, stated rather than left to be discovered: + * `runRegion`'s own `isRefusalSignal` arm. `FlowRefusalSignal` is raised at + * exactly one site — an `end` node whose parsed `outcome` is `refused` — + * and the class is not exported, so with the parse refusal in place no + * authored flow reaches that arm except past `MAX_REGION_DEPTH` (32), + * where the spec's region walk stops. The engine code stays as the refusal + * for that one remaining seam, and nothing in this package exercises it. + * + * ⛔ Nothing an author had is narrowed by either door. Before #15788 an + * `end` inside a region was a no-op whatever its `outcome`, so this shape + * has never once been honoured; whether a refusal should instead propagate + * out of a region is a real question the #14945 ruling does not answer. */ - it('a refusing `end` inside a `loop` body fails the run loudly instead of vanishing', async () => { + it('a refusing `end` inside a `loop` body is refused at REGISTRATION, before any run', () => { const { engine } = engineWithStore(); installBuiltinNodes(engine, { logger: createTestLogger(), getService: () => undefined } as never); - engine.registerFlow('in_region', { - name: 'in_region', label: 'in_region', type: 'autolaunched', - successMessage: SUCCESS_TEXT, - nodes: [ - { id: 'start', type: 'start', label: 'Start' }, - { - id: 'sweep', type: 'loop', label: 'Sweep', - config: { - collection: '{items}', - iteratorVariable: 'item', - body: { - nodes: [{ id: 'nope', type: 'end', label: 'Nope', config: { outcome: 'refused', message: REFUSAL_TEMPLATE } }], - edges: [], + + let caught: unknown; + try { + engine.registerFlow('in_region', { + name: 'in_region', label: 'in_region', type: 'autolaunched', + successMessage: SUCCESS_TEXT, + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'sweep', type: 'loop', label: 'Sweep', + config: { + collection: '{items}', + iteratorVariable: 'item', + body: { + nodes: [{ id: 'nope', type: 'end', label: 'Nope', config: { outcome: 'refused', message: REFUSAL_TEMPLATE } }], + edges: [], + }, }, }, - }, - ], - edges: [{ id: 'e0', source: 'start', target: 'sweep' }], - } as never); - - // `items` rides on the trigger record, which the engine flattens into - // the variable map — so `{items}` resolves without declaring an input. - const result = await engine.execute('in_region', { - event: 'manual', object: 'account', record: { ...ACME, items: [1] }, - } as unknown as AutomationContext); + ], + edges: [{ id: 'e0', source: 'start', target: 'sweep' }], + } as never); + } catch (err) { + caught = err; + } - expect(result.success).toBe(false); - expect(result.status).toBe('failed'); + // Refused, and refused by the REGION rule specifically — a bare "it + // threw" would be satisfied by any unrelated parse error. + const issues = (caught as { issues?: Array<{ path: Array; message: string }> } | undefined)?.issues ?? []; + const refusal = issues.find((i) => i.message.includes('may not sit inside a structured region')); + expect(refusal, `no region refusal among ${JSON.stringify(issues)}`).toBeDefined(); + // Anchored where the author wrote the node, not on the container. + expect(refusal!.path).toEqual(['nodes', 1, 'config', 'body', 'nodes', 0, 'type']); // The message names the shape and the one-line fix, per "absence must // be loud" — ⛔ not a bare stringified sentinel. - expect(result.error).toContain('structured region'); - expect(result.error).toContain("outcome: 'refused'"); - // ⛔ And it is NOT recorded as a refusal: the run did not refuse, the - // engine declined to honour a shape it cannot express. - expect(result.refusalMessage).toBeUndefined(); + expect(refusal!.message).toContain('An `end` node may not sit inside a structured region'); + expect(refusal!.message).toContain('Put the `end` on the top-level graph'); + + // …and the refusal is terminal: nothing was registered, so no later + // call can reach the shape by another door. + expect(engine.getFlowVersionHistory('in_region')).toEqual([]); }); }); diff --git a/packages/spec/api-surface-declarations/automation.txt b/packages/spec/api-surface-declarations/automation.txt index 23d9c94895b..283e0f069ad 100644 --- a/packages/spec/api-surface-declarations/automation.txt +++ b/packages/spec/api-surface-declarations/automation.txt @@ -12,8 +12,8 @@ # excluded: documentation drift is `check:docs`'s axis, not this one. # # entry: ./automation -# exported names: 273 -# declarations: 279 +# exported names: 274 +# declarations: 280 # # GENERATED — ⛔ never hand-edited. Regenerate after a real build: # pnpm --filter @objectstack/spec build && pnpm --filter @objectstack/spec gen:api-surface-declarations @@ -215,8 +215,8 @@ declare const ActionRefSchema: z.ZodUnion; // ── ApprovalDecision (type) ── @@ -311,8 +311,8 @@ declare const ApprovalNodeConfigSchema: z.ZodObject<{ lockRecord: z.ZodDefault; approvalStatusField: z.ZodOptional; onEmptyApprovers: z.ZodDefault>; @@ -988,6 +988,9 @@ declare const FLOW_BUILTIN_NODE_TYPES: readonly string[]; // ── FLOW_NODE_EXPRESSION_PATHS (const) ── declare const FLOW_NODE_EXPRESSION_PATHS: readonly FlowNodeExpressionPath[]; +// ── FLOW_PAUSE_CAPABLE_NODE_TYPES (const) ── +declare const FLOW_PAUSE_CAPABLE_NODE_TYPES: readonly string[]; + // ── FLOW_REGION_CONFIG_KEYS (const) ── declare const FLOW_REGION_CONFIG_KEYS: ReadonlySet; diff --git a/packages/spec/api-surface/automation.json b/packages/spec/api-surface/automation.json index 9dc1216ccf7..6189e5214c2 100644 --- a/packages/spec/api-surface/automation.json +++ b/packages/spec/api-surface/automation.json @@ -111,6 +111,7 @@ "ExecutionStepSkipReasonSchema (const)", "FLOW_BUILTIN_NODE_TYPES (const)", "FLOW_NODE_EXPRESSION_PATHS (const)", + "FLOW_PAUSE_CAPABLE_NODE_TYPES (const)", "FLOW_REGION_CONFIG_KEYS (const)", "FLOW_REGION_SLOTS (const)", "FLOW_REGION_SLOTS_BY_TYPE (const)", diff --git a/packages/spec/export-origins/automation.json b/packages/spec/export-origins/automation.json index 26082654c07..1eaea1c7be5 100644 --- a/packages/spec/export-origins/automation.json +++ b/packages/spec/export-origins/automation.json @@ -107,6 +107,7 @@ "ExecutionStepSkipReasonSchema": "src/automation/execution.zod.ts#ExecutionStepSkipReasonSchema (const)", "FLOW_BUILTIN_NODE_TYPES": "src/automation/flow.zod.ts#FLOW_BUILTIN_NODE_TYPES (const)", "FLOW_NODE_EXPRESSION_PATHS": "src/automation/flow-node-expression-paths.ts#FLOW_NODE_EXPRESSION_PATHS (const)", + "FLOW_PAUSE_CAPABLE_NODE_TYPES": "src/automation/flow.zod.ts#FLOW_PAUSE_CAPABLE_NODE_TYPES (const)", "FLOW_REGION_CONFIG_KEYS": "src/automation/region-slots.ts#FLOW_REGION_CONFIG_KEYS (const)", "FLOW_REGION_SLOTS": "src/automation/region-slots.ts#FLOW_REGION_SLOTS (const)", "FLOW_REGION_SLOTS_BY_TYPE": "src/automation/region-slots.ts#FLOW_REGION_SLOTS_BY_TYPE (const)", diff --git a/packages/spec/src/automation/end-node-outcome.test.ts b/packages/spec/src/automation/end-node-outcome.test.ts index 086edab1070..a98ec3e026d 100644 --- a/packages/spec/src/automation/end-node-outcome.test.ts +++ b/packages/spec/src/automation/end-node-outcome.test.ts @@ -21,7 +21,6 @@ import { describe, it, expect } from 'vitest'; import { EndConfigSchema } from './builtin-node-config.zod'; import { FlowSchema, FlowNodeSchema, defineFlow, type Flow } from './flow.zod'; -import { validateControlFlow } from './control-flow.zod'; import { ExecutionLogSchema, ExecutionStatus } from './execution.zod'; import { formatZodError } from '../shared/error-map.zod'; @@ -218,13 +217,15 @@ describe('FlowSchema applies the `end` contract — the structural node\'s only expect(issues?.map((i) => [i.code, i.path])).toEqual([['custom', ['nodes', 1, 'config', 'message']]]); }); - it('a region-nested `end` is checked at the region door: the flow parse leaves the region raw, validateControlFlow refuses it by name', () => { - // `parseFlowNodeRegions` deliberately leaves a region it cannot parse - // untouched (the registration walk owns nested diagnostics, #4389), so the - // FLOW parse alone does not surface a nested refusal — the same boundary - // every other nested node key has. `validateControlFlow` re-parses the - // region through `FlowNodeSchema`, where this contract now lives, and - // throws with the same sentence. + it('a region-nested `end` is refused by the FLOW parse itself (#15646/#18112) — the region-door reading this test used to pin is unreachable, because the shape is gone', () => { + // ⚠️ REPLACED, not re-spelled. This case used to assert that the flow parse + // was GREEN here and that `validateControlFlow` was the door — a true + // reading of `parseFlowNodeRegions` leaving a refused region raw (#4389). + // #15646 removes its subject: an `end` node inside a structured region body + // is refused at parse, whatever its `config`, so there is no longer a + // region-nested `end` whose CONFIG can be judged one door later. Keeping the + // old assertion by weakening it would have pinned an `end`-in-region shape + // that is now undeclarable. const nested: Flow = { name: 'nested_refusal', label: 'Nested refusal', @@ -249,16 +250,31 @@ describe('FlowSchema applies the `end` contract — the structural node\'s only ], }; const parsed = FlowSchema.safeParse(nested); - expect(parsed.success).toBe(true); - if (!parsed.success) return; - let message = ''; - try { - validateControlFlow(parsed.data); - } catch (error) { - message = (error as Error).message; - } - expect(message).toContain("loop 'each' body"); - expect(message).toContain("`outcome: 'refused'` requires a `message`"); + expect(parsed.success).toBe(false); + if (parsed.success) return; + expect(parsed.error.issues.map((i) => [i.code, i.path])).toEqual([ + ['custom', ['nodes', 1, 'config', 'body', 'nodes', 0, 'type']], + ]); + expect(parsed.error.issues[0].message).toContain( + "An `end` node may not sit inside a structured region — `loop 'each' body` is a region body and the `end` node `inner_end` is inside it", + ); + + // CONTROL — the identical `end` node, identical malformed config, on the + // TOP-LEVEL graph: refused by the `end` CONFIG contract instead, at + // `config.message`. So the reading above is the region rule firing, not this + // fixture being malformed in some way that would fail anywhere. + const topLevel = FlowSchema.safeParse({ + ...nested, + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'inner_end', type: 'end', label: 'Inner end', config: { outcome: 'refused' } }, + ], + edges: [{ id: 'e1', source: 'start', target: 'inner_end' }], + } as Flow); + expect(topLevel.success).toBe(false); + if (topLevel.success) return; + expect(topLevel.error.issues.map((i) => i.path)).toEqual([['nodes', 1, 'config', 'message']]); + expect(topLevel.error.issues[0].message).toContain("`outcome: 'refused'` requires a `message`"); }); }); diff --git a/packages/spec/src/automation/flow-region-pause-and-end.test.ts b/packages/spec/src/automation/flow-region-pause-and-end.test.ts new file mode 100644 index 00000000000..8ed2314ff5e --- /dev/null +++ b/packages/spec/src/automation/flow-region-pause-and-end.test.ts @@ -0,0 +1,315 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * What a structured region body may NOT contain — #15646, absorbing #18112. + * + * Director seat ruling 5714239196 (maintainer 「同意,其他也同意」) and its scope + * addition 5714981966 (maintainer 「146 同意」) put the rule here; ruling + * 5724940095, decision batch #153 item 1 letter **D** (maintainer 「其他同意」), + * fixed its POPULATION and is quoted verbatim in the PR: + * + * 「inside `loop` / `parallel` branch / `try_catch` (try and catch) bodies at + * any depth, the node types `screen`, `wait`, `approval`, `approval_revise` + * and `end` are refused by `FlowSchema.superRefine`, the message naming the + * node and the region. `map` and `subflow` are ⛔ not refused by type.」 + * + * Both halves are authoring-time enforcement of the limit #3267 ruled 禁: a + * region body runs synchronously inside the enclosing run, so it can neither + * park that run nor terminate it. The engine already refuses both AT RUN TIME + * (`durable pause inside a structured region … is not supported`, and the + * #15788 refusing-`end` conversion) — this moves the refusal to where the + * author is standing, for the half a parse can actually judge. + * + * ⭐ The `map` / `subflow` exclusion is load-bearing and has its own pins in + * "the declared boundaries" below: those two pause exactly when the child flow + * they name pauses, so a type-keyed refusal would also refuse + * `loop { map(synchronous child) }` — a shape that runs correctly today. The + * run-time half of ruling D is what meets them. + * + * Every refusal case here fails without the rule: the shapes below all parsed + * green before it. + */ +import { describe, it, expect } from 'vitest'; +import { + FlowSchema, + FLOW_PAUSE_CAPABLE_NODE_TYPES, + defineFlow, + type Flow, + type FlowNode, + type FlowEdge, +} from './flow.zod'; +import { APPROVAL_NODE_TYPE, APPROVAL_REVISE_NODE_TYPE } from './approval.zod'; +import { validateControlFlow } from './control-flow.zod'; +import { formatZodError } from '../shared/error-map.zod'; + +/** A `wait` node owes a `waitEventConfig` block, so every fixture carries one. */ +const pausingNode = (type: string, id = 'pauser'): FlowNode => ({ + id, + type, + label: `A ${type}`, + ...(type === 'wait' ? { waitEventConfig: { eventType: 'timer' as const, timerDuration: 'PT1H' } } : {}), + ...(type === 'map' ? { config: { collection: '{items}', flowName: 'per_item' } } : {}), + ...(type === 'subflow' ? { config: { flowName: 'child' } } : {}), +}); + +const step = (id: string): FlowNode => ({ id, type: 'assignment', label: id }); + +const flowWith = (nodes: FlowNode[], edges: FlowEdge[] = []): Flow => ({ + name: 'region_contents', + label: 'Region contents', + type: 'autolaunched', + nodes: [{ id: 'start', type: 'start', label: 'Start' }, ...nodes], + edges, +}); + +const loopOver = (bodyNodes: FlowNode[], id = 'sweep'): FlowNode => ({ + id, type: 'loop', label: 'Sweep', + config: { collection: '{items}', body: { nodes: bodyNodes, edges: [] } }, +}); + +const tryCatchOver = (tryNodes: FlowNode[], catchNodes: FlowNode[], id = 'guard'): FlowNode => ({ + id, type: 'try_catch', label: 'Guard', + config: { try: { nodes: tryNodes, edges: [] }, catch: { nodes: catchNodes, edges: [] } }, +}); + +const parallelOver = (branches: FlowNode[][], id = 'fan'): FlowNode => ({ + id, type: 'parallel', label: 'Fan out', + config: { branches: branches.map((nodes) => ({ nodes, edges: [] })) }, +}); + +/** Every issue as `[path, message]`, for the assertions below. */ +const issuesOf = (flow: Flow): Array<[string, string]> => { + const result = FlowSchema.safeParse(flow); + if (result.success) return []; + return result.error.issues.map((i) => [i.path.join('.'), i.message]); +}; + +describe('FLOW_PAUSE_CAPABLE_NODE_TYPES — the declared set, and how it was derived', () => { + it('names the four built-in types that park a run on EVERY execution', () => { + // Read off the descriptors, not recalled. SIX shipped executors declare + // `supportsPause: true` — `screen` / `wait` / `subflow` / `map` in + // `service-automation`'s builtins, `approval` / `approval_revise` in + // `plugin-approvals`, the same six the ADR-0044 `resumeAuthority` + // default-flip migration entry names. Four of them pause from this flow's + // own text; those four are the region rule's population. + expect([...FLOW_PAUSE_CAPABLE_NODE_TYPES]).toEqual([ + 'screen', 'wait', 'approval', 'approval_revise', + ]); + }); + + it('⛔ excludes `subflow` and `map`, which are pause-capable but not unconditionally so', () => { + // Ruling D, verbatim: 「`map` and `subflow` are ⛔ not refused by type.」 + // They pause exactly when the child flow `config.flowName` names pauses — + // a different metadata record, unreadable from here. Pinned as a DECISION + // so re-adding either is an edit somebody makes on purpose, against the + // ruling, rather than a tidy-up that looks like completing a list. + expect(FLOW_PAUSE_CAPABLE_NODE_TYPES).not.toContain('subflow'); + expect(FLOW_PAUSE_CAPABLE_NODE_TYPES).not.toContain('map'); + }); + + it('carries the approval node types by their declared constants, so a rename cannot desynchronise the two', () => { + expect(FLOW_PAUSE_CAPABLE_NODE_TYPES).toContain(APPROVAL_NODE_TYPE); + expect(FLOW_PAUSE_CAPABLE_NODE_TYPES).toContain(APPROVAL_REVISE_NODE_TYPE); + }); +}); + +describe('a region body refuses a pause-capable node (#15646)', () => { + it.each(FLOW_PAUSE_CAPABLE_NODE_TYPES)('refuses a `%s` node in a loop body, anchored on its `type`', (type) => { + expect(issuesOf(flowWith([loopOver([pausingNode(type)])]))).toEqual([[ + 'nodes.1.config.body.nodes.0.type', + expect.stringContaining( + `A \`${type}\` node may not sit inside a structured region — \`loop 'sweep' body\` is a region body ` + + `and the \`${type}\` node \`pauser\` is inside it`, + ) as unknown as string, + ]]); + }); + + it('refuses it in a try_catch TRY region', () => { + expect(issuesOf(flowWith([tryCatchOver([pausingNode('wait')], [step('recover')])]))).toEqual([[ + 'nodes.1.config.try.nodes.0.type', + expect.stringContaining("`try_catch 'guard' try` is a region body and the `wait` node `pauser` is inside it") as unknown as string, + ]]); + }); + + it('refuses it in a try_catch CATCH region — the arm route B could never see', () => { + expect(issuesOf(flowWith([tryCatchOver([step('attempt')], [pausingNode('approval')])]))).toEqual([[ + 'nodes.1.config.catch.nodes.0.type', + expect.stringContaining("`try_catch 'guard' catch` is a region body and the `approval` node `pauser` is inside it") as unknown as string, + ]]); + }); + + it('refuses it in a parallel BRANCH — the other arm route B could never see', () => { + expect(issuesOf(flowWith([parallelOver([[step('left')], [pausingNode('screen')]])]))).toEqual([[ + 'nodes.1.config.branches.1.nodes.0.type', + expect.stringContaining("`parallel 'fan' branch 1` is a region body and the `screen` node `pauser` is inside it") as unknown as string, + ]]); + }); + + it('names the CHAINED region path when the regions nest — `loop { try_catch { approval } }`', () => { + // The nesting shape of this card's own reproduction, with a node type the + // parse can judge. The reproduction's own `map` is pinned as still + // declarable in "the declared boundaries" below — that is ruling D, not a + // hole in this assertion. + const issues = issuesOf(flowWith([loopOver([tryCatchOver([pausingNode('approval')], [step('recover')])])])); + expect(issues.map(([path]) => path)).toEqual([ + 'nodes.1.config.body.nodes.0.config.try.nodes.0.type', + ]); + expect(issues[0][1]).toContain("`loop 'sweep' body → try_catch 'guard' try` is a region body"); + }); + + it('says WHY, and names the node and the region — the sentence an author acts on', () => { + const [[, message]] = issuesOf(flowWith([loopOver([pausingNode('wait')])])); + expect(message).toContain('A region body runs synchronously and cannot durably pause'); + expect(message).toContain('parks the run on EVERY execution'); + expect(message).toContain('reads back as progress'); + expect(message).toContain("Move the `wait` node onto the top-level graph and route the region's exit to it"); + // ⛔ The message must not advertise a population the rule does not refuse: + // naming `map` / `subflow` here would send an author hunting for a refusal + // that never fires. + expect(message).not.toContain('subflow'); + expect(message).not.toContain('`map`'); + }); + + it('renders through formatZodError pointing INTO the region', () => { + const result = FlowSchema.safeParse(flowWith([loopOver([pausingNode('wait')])])); + expect(result.success).toBe(false); + if (result.success) return; + expect(formatZodError(result.error)).toContain( + 'nodes.1.config.body.nodes.0.type: A `wait` node may not sit inside a structured region', + ); + }); + + it('defineFlow refuses it with the same anchored issue', () => { + let caught: unknown; + try { + defineFlow(flowWith([loopOver([pausingNode('screen')])])); + } catch (error) { + caught = error; + } + const issues = (caught as { issues?: Array<{ code: string; path: PropertyKey[] }> })?.issues; + expect(issues).toBeDefined(); + expect(issues!.map((i) => [i.code, i.path])).toEqual([ + ['custom', ['nodes', 1, 'config', 'body', 'nodes', 0, 'type']], + ]); + }); +}); + +describe('a region body refuses an `end` node (#18112, absorbed into #15646)', () => { + it.each([ + ['loop body', loopOver([{ id: 'stop', type: 'end', label: 'Stop' }]), 'nodes.1.config.body.nodes.0.type', "loop 'sweep' body"], + ['try region', tryCatchOver([{ id: 'stop', type: 'end', label: 'Stop' }], [step('recover')]), 'nodes.1.config.try.nodes.0.type', "try_catch 'guard' try"], + ['catch region', tryCatchOver([step('attempt')], [{ id: 'stop', type: 'end', label: 'Stop' }]), 'nodes.1.config.catch.nodes.0.type', "try_catch 'guard' catch"], + ['parallel branch', parallelOver([[step('left')], [{ id: 'stop', type: 'end', label: 'Stop' }]]), 'nodes.1.config.branches.1.nodes.0.type', "parallel 'fan' branch 1"], + ])('refuses an `end` in a %s', (_kind, container, path, scope) => { + const issues = issuesOf(flowWith([container])); + expect(issues.map(([p]) => p)).toEqual([path]); + expect(issues[0][1]).toContain( + `An \`end\` node may not sit inside a structured region — \`${scope}\` is a region body and the \`end\` node \`stop\` is inside it`, + ); + }); + + it('gives the ruled prescription verbatim — a region body cannot end the run; put the `end` on the top-level graph', () => { + const [[, message]] = issuesOf(flowWith([loopOver([{ id: 'stop', type: 'end', label: 'Stop' }])])); + expect(message).toContain('A region body cannot END the run'); + expect(message).toContain("Put the `end` on the top-level graph and route the region's exit to it"); + }); + + it('refuses it whatever the `outcome` — a plain terminal is a no-op there, a refusing one is converted to a region error', () => { + for (const config of [undefined, { outcome: 'completed' as const }, { outcome: 'refused' as const, message: 'Refused: {record.name}' }]) { + const issues = issuesOf(flowWith([loopOver([{ id: 'stop', type: 'end', label: 'Stop', config }])])); + expect(issues.map(([p]) => p), JSON.stringify(config)).toEqual(['nodes.1.config.body.nodes.0.type']); + } + }); +}); + +describe('the rule does NOT over-reach', () => { + it('accepts every pause-capable type on the TOP-LEVEL graph — a refusal that over-reaches is worse than the silence it replaces', () => { + for (const type of [...FLOW_PAUSE_CAPABLE_NODE_TYPES, 'subflow', 'map']) { + const flow = flowWith([pausingNode(type)], [{ id: 'e1', source: 'start', target: 'pauser' }]); + expect(FlowSchema.safeParse(flow).success, type).toBe(true); + } + }); + + it('accepts an `end` on the top-level graph, including one inside a flow that also has regions', () => { + const flow = flowWith( + [loopOver([step('work')]), { id: 'stop', type: 'end', label: 'Stop' }], + [{ id: 'e1', source: 'start', target: 'sweep' }, { id: 'e2', source: 'sweep', target: 'stop' }], + ); + const parsed = FlowSchema.safeParse(flow); + expect(parsed.success).toBe(true); + if (!parsed.success) return; + expect(() => validateControlFlow(parsed.data)).not.toThrow(); + }); + + it('leaves every non-pausing node type alone inside a region — the rule is a list, not a mood', () => { + for (const type of ['assignment', 'decision', 'create_record', 'update_record', 'get_record', 'http', 'notify', 'loop']) { + const node: FlowNode = type === 'loop' + ? loopOver([step('inner')], 'inner_loop') + : { id: 'work', type, label: 'Work' }; + expect(FlowSchema.safeParse(flowWith([loopOver([node])])).success, type).toBe(true); + } + }); + + it('judges the node TYPE, not the node id — a node merely NAMED `end` or `wait` in a region still parses', () => { + expect(FlowSchema.safeParse(flowWith([loopOver([step('end'), step('wait')])])).success).toBe(true); + }); +}); + +describe('the declared boundaries — measured, so they move deliberately', () => { + it.each(['map', 'subflow'])('⛔ does NOT refuse a `%s` in a region body — ruling D, letter for letter', (type) => { + // 「`map` and `subflow` are ⛔ not refused by type.」 Their pause lives in + // the child flow `config.flowName` names — a metadata record this parse + // does not hold — so a type-keyed refusal would also refuse + // `loop { map(synchronous child) }`, which runs correctly today and is + // covered by #15616's regression suite in `packages/services`. + expect(FlowSchema.safeParse(flowWith([loopOver([pausingNode(type)])])).success, type).toBe(true); + }); + + it("⛔ leaves this card's own reproduction declarable — `loop { try_catch { map } }` — and that is the ruling, not a hole", () => { + // The exact shape #15646 was filed on. It parses, deliberately: what makes + // it wrong is that the child flow pauses, which only the RUN knows. Ruling + // D's second half is a `domain:services` card that fails such a run with a + // named error instead of reporting `success` with `summary.failed = 0`. + // ⛔ Do not "fix" this pin by widening the parse — that is route A/C, both + // explicitly refused. + const flow = flowWith([loopOver([tryCatchOver([pausingNode('map')], [step('recover')])])]); + expect(FlowSchema.safeParse(flow).success).toBe(true); + }); + + it('a PLUGIN-contributed pausing type is not refused: a parse has no registry (ADR-0018 open namespace)', () => { + // ⚠️ Not an oversight and not a gap to quietly close: `FlowNodeSchema.type` + // is a validated `string`, and a plugin registers pause-capable types at run + // time. The engine's own run-time refusal is what meets this one. Extending + // `FLOW_PAUSE_CAPABLE_NODE_TYPES` is how a first-party type joins the + // rule — and this pin is what makes that an edit somebody makes on purpose. + const flow = flowWith([loopOver([{ id: 'vendor', type: 'vendor_signature_pause', label: 'Sign' }])]); + expect(FlowSchema.safeParse(flow).success).toBe(true); + }); + + it('the seam at MAX_REGION_DEPTH: an unconditionally pausing node at nesting 32 is refused; at nesting 33 the parse does not judge it', () => { + // The walk this rule rides (`collectFlowGraphs`) stops at 32, the ceiling + // `parseFlowNodeRegions` shares — the same measured boundary #16134's + // one-id-space rule hands off at. Past it there is no second spec refusal + // for THIS rule (unlike a duplicate id, which `analyzeRegion` still names), + // so the engine's run-time refusal is the only one left. Stated in the + // docblock and in the changeset rather than discovered by an author. + const nestedTo = (nesting: number): Flow => { + let body: { nodes: FlowNode[]; edges: FlowEdge[] } = { nodes: [pausingNode('wait')], edges: [] }; + for (let k = nesting - 1; k >= 1; k--) { + body = { nodes: [{ id: `l${k}`, type: 'loop', label: `L${k}`, config: { collection: '{items}', body } }], edges: [] }; + } + return JSON.parse(JSON.stringify(flowWith([ + { id: 'sweep', type: 'loop', label: 'Sweep', config: { collection: '{items}', body } }, + ]))) as Flow; + }; + + const atCeiling = FlowSchema.safeParse(nestedTo(32)); + expect(atCeiling.success).toBe(false); + if (atCeiling.success) return; + expect(atCeiling.error.issues).toHaveLength(1); + expect(atCeiling.error.issues[0].message).toContain('A `wait` node may not sit inside a structured region'); + expect(atCeiling.error.issues[0].path.slice(-2)).toEqual([0, 'type']); + + expect(FlowSchema.safeParse(nestedTo(33)).success).toBe(true); + }); +}); diff --git a/packages/spec/src/automation/flow.test.ts b/packages/spec/src/automation/flow.test.ts index e82da586a05..379e7d7df8d 100644 --- a/packages/spec/src/automation/flow.test.ts +++ b/packages/spec/src/automation/flow.test.ts @@ -1464,22 +1464,26 @@ describe('BPMN — Wait Event Configuration', () => { }); /** - * Nested in an ADR-0031 region, the refusal is reached through the REGION - * CONTRACT, not through `FlowSchema` — measured here rather than assumed, - * because the two doors answer differently and only one of them is the door - * a run actually passes through. + * Nested in an ADR-0031 region, `waitEventConfig`'s refusal is reached through + * the REGION CONTRACT (`LoopConfigSchema`) — measured here rather than + * assumed, because the two doors answer differently and only one of them is + * the door a run actually passes through. * * `parseFlowNodeRegions` parses each region slot with `safeParse` and, on a * refusal, leaves the region RAW and continues (its own comment says so: a - * refused region is left for `validateControlFlow` to name). That policy - * predates this change and is not specific to `waitEventConfig` — it is why the - * flow-level parse below is GREEN with a block-less wait sitting in the loop - * body. The contract that refuses it is `LoopConfigSchema`, which is what - * `loop`'s executor parses its config through at execute time - * (`parseNodeConfig` → a guard refusal), so the nested shape still cannot - * run; it is refused one door later and by node id. + * refused region is left for `validateControlFlow` to name). That policy is + * not specific to `waitEventConfig`, and it is why the flow parse never + * carried this key's refusal. + * + * ⚠️ What #15646 changed, and what it did not: the FLOW parse is no longer + * green on the nested fixture, because a `wait` node may not sit inside a + * region body AT ALL now — a region body cannot durably pause, and `wait` + * always does. So the flow parse refuses the fixture for a reason that has + * nothing to do with `waitEventConfig`, and the block's own refusal is still + * the region contract's. Both are asserted below so the two cannot be + * confused for each other again. */ - it('nested in a region: the flow parse leaves it raw, and the REGION contract refuses it by path', () => { + it('nested in a region: the flow parse refuses the `wait` node itself, and the REGION contract still refuses the missing block by path', () => { const loopConfig = (bodyNode: Record) => ({ collection: '{rows}', iteratorVariable: 'row', body: { nodes: [bodyNode], edges: [] }, @@ -1498,20 +1502,34 @@ describe('BPMN — Wait Event Configuration', () => { edges: [{ id: 'e1', source: 'start', target: 'loop' }], }); - // Door 1 — the flow parse: green, and the body node comes back UNPARSED. + // Door 1 — the flow parse: refused on the node TYPE, not on the block. const flowParse = FlowSchema.safeParse(flowWith(bare)); - expect(flowParse.success).toBe(true); + expect(flowParse.success).toBe(false); + if (flowParse.success) return; + expect(flowParse.error.issues.map(i => i.path.join('.'))).toEqual(['nodes.1.config.body.nodes.0.type']); + expect(flowParse.error.issues[0].message).toContain('A `wait` node may not sit inside a structured region'); // Door 2 — the region contract the `loop` executor parses through: refused, - // anchored on the block, at the node's own index inside the body. + // anchored on the block, at the node's own index inside the body. Reached + // directly, because the flow that would carry it no longer parses. const refused = LoopConfigSchema.safeParse(loopConfig(bare)); expect(refused.success).toBe(false); expect(refused.error!.issues.map(i => i.path.join('.'))).toContain('body.nodes.0.waitEventConfig'); - // CONTROL — the declared body node passes both doors, so the refusal above - // is the missing block and not the region fixture. - expect(FlowSchema.safeParse(flowWith(declared)).success).toBe(true); + // CONTROL — the DECLARED body node still passes door 2, so door 2's refusal + // above is the missing block and not the region fixture. It does not pass + // door 1: the region rule judges the type, and a declared block does not + // make a `wait` pausable-in-a-region. expect(LoopConfigSchema.safeParse(loopConfig(declared)).success).toBe(true); + expect(FlowSchema.safeParse(flowWith(declared)).success).toBe(false); + + // CONTROL — the same declared `wait` on the TOP-LEVEL graph passes the flow + // parse, so door 1's refusal is the region and not the node. + expect(FlowSchema.safeParse({ + name: 'top_level', label: 'Top level', type: 'autolaunched', + nodes: [{ id: 'start', type: 'start', label: 'Start' }, declared], + edges: [{ id: 'e1', source: 'start', target: 'pause' }], + }).success).toBe(true); }); }); diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index 553694a19a0..12360e7a511 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -24,6 +24,7 @@ import { retryPolicyShape } from '../shared/retry-policy.zod'; import { strictObject } from '../shared/strict-object'; import { collectFlowGraphs, parseFlowNodeRegions } from './control-flow.zod'; import { EndConfigSchema } from './builtin-node-config.zod'; +import { APPROVAL_NODE_TYPE, APPROVAL_REVISE_NODE_TYPE } from './approval.zod'; export const FlowNodeAction = z.enum([ 'start', // Trigger 'end', // Return/Stop @@ -67,6 +68,51 @@ export const FLOW_BUILTIN_NODE_TYPES: readonly string[] = FlowNodeAction.options */ export const FLOW_STRUCTURAL_NODE_TYPES: readonly string[] = ['start', 'end']; +/** + * The node types a structured region body may not contain — the population the + * `FlowSchema.superRefine` region rule below keys on. + * + * ⚠️ **Read the contents, not the name.** This is NOT every type that can + * pause; it is the four that pause UNCONDITIONALLY, on every execution, decided + * by THIS flow's own text. The name predates that narrowing and is kept + * deliberately: the identifier is in `api-surface/automation.json`, and + * `check:api-surface` grades a removed export breaking — trading a whole-stack + * major for a better name is a ruled decision of its own, ⛔ not a tidy-up to + * make in passing. + * + * ⭐ The unconditional half of the pause-capable population, and that + * distinction is the whole rule. Six shipped executors declare + * `supportsPause: true` — read off the + * `defineActionDescriptor({... supportsPause: true ...})` literals rather than + * recalled: `screen` / `wait` / `subflow` / `map` in `service-automation`'s + * builtins and `approval` / `approval_revise` in `plugin-approvals`, the same + * six the ADR-0044 `resumeAuthority` default-flip migration entry names and + * `check:resume-authority-declared` scans. Four of them pause from THIS flow's + * own text, and those four are listed here. + * + * ⛔ `subflow` and `map` are deliberately NOT listed. They pause exactly when + * the child flow their `config.flowName` names pauses — a DIFFERENT metadata + * record, not in hand while this flow is parsed — so refusing them by type also + * refuses `loop { map(synchronous child) }`, a shape that runs correctly today. + * A parse-time rule refuses what is STATICALLY wrong; a region-contained node + * that actually suspends is a fact only the run holds, and the engine's own + * refusal is what meets it. Adding either type back here is a ruled decision, + * not a fix — maintainer ruling, decision batch #153 item 1, letter D. + * + * ⚠️ **Not closed, and cannot be.** ADR-0018 made the node-type namespace open + * — `FlowNodeSchema.type` is a validated `string` and a plugin registers new + * types, pausing ones included, at run time. A parse has no registry, so a + * plugin-contributed pausing type inside a region is NOT refused here; the + * engine's own run-time refusal is what meets it. Extending this list is how a + * first-party type joins the rule. + */ +export const FLOW_PAUSE_CAPABLE_NODE_TYPES: readonly string[] = [ + 'screen', + 'wait', + APPROVAL_NODE_TYPE, + APPROVAL_REVISE_NODE_TYPE, +]; + /* * ── Unknown-key strictness (#4001, ADR-0078) ──────────────────────────────── * @@ -290,16 +336,23 @@ export const FlowNodeSchema = lazySchema(() => flowNodeObject().transform( * parses each region slot with `safeParse` and, on a refusal, leaves that region * RAW and continues (its own comment says so: a refused region is left for * `validateControlFlow` to name). That policy predates this change and is not - * specific to `waitEventConfig`, and the consequence is measurable: - * `FlowSchema.safeParse` of a flow whose `loop` body holds a block-less `wait` - * answers `success: true`. What refuses the nested node is the REGION contract — + * specific to `waitEventConfig`. What refuses the nested node is the REGION contract — * `LoopConfigSchema` / `ParallelConfigSchema` / `TryCatchConfigSchema` — at * `body.nodes[i].waitEventConfig`, which is the same contract the container * node's executor parses its config through at execute time, so the nested shape * still cannot RUN; it is refused one door later and by node id. Both halves are - * pinned in `flow.test.ts` ("nested in a region: the flow parse leaves it raw, - * and the REGION contract refuses it by path"), and the ADR-0087 entry's - * `acceptanceCriteria` states the same thing for whoever migrates a stack. + * pinned in `flow.test.ts` ("nested in a region: the flow parse refuses the + * `wait` node itself, and the REGION contract still refuses the missing block by + * path"), and the ADR-0087 entry's `acceptanceCriteria` states the same thing + * for whoever migrates a stack. + * + * ⚠️ Since #15646 a `wait` nested in a region body meets an EARLIER refusal than + * either of those, and it is not about this block: a region body cannot durably + * pause, so {@link FLOW_PAUSE_CAPABLE_NODE_TYPES} may not appear in one at all + * and the flow parse says so on the node's `type`. ⛔ Do not read the paragraph + * above as "a nested block-less `wait` parses" — it no longer does, for a + * different reason. The two-door reading it describes still governs every node + * type the region rule leaves alone. * * ⚠️ `boundary_event` gets the contract half ONLY: the platform registers no * executor for that type at all (`NO_EXECUTOR` plus a startup `warn`, measured @@ -1154,6 +1207,87 @@ export const FlowSchema = lazySchema(() => strictObject( }); } + // What a structured region body may NOT contain (#15646, absorbing #18112). + // + // Two refusals, one rule family, because they are one limit read twice: an + // ADR-0031 region body (`loop` / `try_catch` / `parallel`, at every depth the + // walk above reaches) runs SYNCHRONOUSLY inside the enclosing run — it can + // neither park that run on a durable pause nor end it. #3267 ruled that limit + // 禁 rather than a gap to be filled, so this is its authoring-time + // enforcement, ⛔ not an interim. + // + // PAUSE — the engine converts a suspension raised inside a region into an + // error at the region boundary, but the executor has already written its + // progress state into the ENCLOSING scope by then. Contain that error in a + // `try_catch` and the residue is read back as progress by the next entry to + // the same node: measured on `loop { try_catch { map(pausing child) } }`, + // iteration 3 read `started === collection.length`, ran nothing, and + // returned SUCCESS with `summary.failed = 0`. A sweep that reports green + // having done nothing is the worst available failure. + // + // ⛔ Read that measurement for the MECHANISM, not for this rule's reach: + // the shape it was taken on is a `map`, and a `map` is not judged here (see + // below). What the parse removes is the half it can see — the node types + // that pause whatever any other record says. The measured shape itself is + // still declarable, and is met at RUN time; making that run-time refusal + // loud instead of silent is the `domain:services` half of the same ruling. + // + // END — an `end` inside a region is a no-op today whatever its `outcome`, + // and a refusing one is converted into a region error at exactly the + // boundary above (#15788). Neither is what the author wrote, so the shape + // has never once been honoured. + // + // Judged on the node TYPE, and the population is the UNCONDITIONAL one — + // {@link FLOW_PAUSE_CAPABLE_NODE_TYPES}. `map` and `subflow` are + // pause-capable but are ⛔ NOT judged here: whether they pause is decided by + // a DIFFERENT metadata record, so refusing them by type would also refuse + // `loop { map(synchronous child) }`, a shape that runs correctly. Three + // boundaries, declared rather than discovered — a region-contained node that + // durably suspends at RUN time (the `map` / `subflow` case, met by the + // engine at the region boundary), a plugin-registered pausing type (invisible to + // a parse, ADR-0018's open namespace), and a region nested past + // `MAX_REGION_DEPTH`, where this walk stops. + for (const graph of collectFlowGraphs(flow)) { + // The flow's own graph is the one place both are legal — that is the whole + // prescription both messages give, so it must stay true. + if (graph.path.length === 0) continue; + graph.nodes.forEach((node, index) => { + const type: unknown = (node as { type?: unknown } | null)?.type; + if (typeof type !== 'string') return; + const id: unknown = (node as { id?: unknown } | null)?.id; + const named = typeof id === 'string' ? `\`${id}\`` : `at index ${index}`; + if (type === 'end') { + ctx.addIssue({ + code: 'custom', + path: [...graph.path, 'nodes', index, 'type'], + message: + `An \`end\` node may not sit inside a structured region — \`${graph.scope}\` is a region ` + + `body and the \`end\` node ${named} is inside it. A region body cannot END the run: it runs ` + + 'synchronously inside the enclosing run, so an `end` here is a no-op whatever its `outcome`, ' + + 'and a refusing one is converted into a region error rather than terminating anything. Put ' + + "the `end` on the top-level graph and route the region's exit to it.", + }); + return; + } + if (FLOW_PAUSE_CAPABLE_NODE_TYPES.includes(type)) { + ctx.addIssue({ + code: 'custom', + path: [...graph.path, 'nodes', index, 'type'], + message: + `A \`${type}\` node may not sit inside a structured region — \`${graph.scope}\` is a region ` + + `body and the \`${type}\` node ${named} is inside it. A region body runs synchronously and ` + + 'cannot durably pause, while `' + type + '` parks the run on EVERY execution ' + + '(`screen` / `wait` / `approval` / `approval_revise`). The engine ' + + 'refuses such a pause at the region boundary AFTER the node has written its progress state ' + + 'into the enclosing scope, so a contained refusal leaves residue a later entry reads back as ' + + 'progress — the run then reports success having processed nothing. Move the `' + type + '` ' + + "node onto the top-level graph and route the region's exit to it; when the work must repeat " + + 'per item, make the top-level graph the repeating construct rather than nesting the pause.', + }); + } + }); + } + // Edges (#14964): every reader of `edges[].id` assumes the ids are unique — // a designer, a BPMN export, a flow diff, any traversal that dedupes by id — // while nothing enforced it: two edges carrying one id parsed, shipped diff --git a/packages/spec/src/migrations/entries/semantic/18.structured-region-body-pause-and-end-refused.ts b/packages/spec/src/migrations/entries/semantic/18.structured-region-body-pause-and-end-refused.ts new file mode 100644 index 00000000000..ca15d6afb61 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.structured-region-body-pause-and-end-refused.ts @@ -0,0 +1,78 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +export const entry: SemanticMigration = { + id: 'structured-region-body-pause-and-end-refused', + surface: + 'The BODY of every ADR-0031 structured region — `loop.config.body`, each ' + + '`parallel.config.branches[]`, and `try_catch.config.try` / `.catch` — at every depth the ' + + 'flow parse walks. Two node populations become undeclarable there: a node whose TYPE parks ' + + 'the run on EVERY execution (`screen`, `wait`, `approval`, `approval_revise`) and an ' + + '`end` node, whatever its `outcome`. ⛔ `subflow` and `map` are NOT in the population, ' + + 'although their executors also declare `supportsPause: true`: they pause exactly when the ' + + 'child flow their `config.flowName` names pauses, which is a DIFFERENT metadata record and ' + + 'is not in hand while this flow is parsed. Refusing them by type would also refuse ' + + '`loop { map(synchronous child) }`, a shape that runs correctly today, so a region-nested ' + + '`map` or `subflow` still parses and is met at RUN time instead.', + replacement: + 'Move the node onto the TOP-LEVEL graph and route the region\'s exit to it. For an `end`: ' + + 'delete it from the body, give the region a normal exit, and put the terminator (with its ' + + '`outcome` / `message`) on the top-level graph — `loop { body: [ …, end ] }` becomes ' + + '`loop { body: [ … ] } → end`. For a pausing node: hoist it out of the container — ' + + '`loop { body: [ try_catch { try: [ approval ] } ] }` becomes a top-level `approval` with ' + + 'the loop fanning out around it, or the pausing half of the branch is split into a ' + + '`subflow` the top-level graph calls; where the repetition is genuinely needed, make the ' + + 'TOP-LEVEL graph the repeating construct with the pause on it rather than nesting the ' + + 'pause inside a region. A `try_catch` whose only purpose was to contain the region\'s ' + + 'refusal has nothing left to contain and is deleted with it.', + reason: + 'Maintainer ruling, decision batch #145 item 5, verbatim and untranslated: 「同意,其他也同' + + '意」, carrying the presented option C; extended by batch #146 「146 同意」, which attached ' + + 'the `end` half (the absorbed #18112) and recorded that #3267 is ruled 禁 — structured ' + + 'regions do not support durable pause and a region body cannot terminate the run, so this ' + + 'is that limit\'s authoring-time enforcement rather than an interim. The POPULATION was ' + + 'then fixed by decision batch #153 item 1, letter D (maintainer 「其他同意」): 「inside ' + + '`loop` / `parallel` branch / `try_catch` (try and catch) bodies at any depth, the node ' + + 'types `screen`, `wait`, `approval`, `approval_revise` and `end` are refused by ' + + '`FlowSchema.superRefine` … `map` and `subflow` are ⛔ not refused by type.」 A parse-time ' + + 'rule refuses what is STATICALLY wrong; refusing `map` / `subflow` by type would refuse a ' + + 'correct working shape on a guess about another record. The refusal already existed AT RUN ' + + 'TIME and said nothing an author could act on: the engine converts a suspension raised ' + + 'inside a region into an error at the region boundary, AFTER the executor has written its ' + + 'progress state into the enclosing scope, so a `try_catch` that contains that error leaves ' + + 'residue the next entry reads back as progress. Measured on a real `AutomationEngine`, ' + + '`loop { try_catch { map(pausing child) } }` over 3 iterations x 2 items: not one item\'s ' + + 'subflow ever completed, only two of three iterations reached the catch, and iteration 3 ' + + 'read `started === collection.length`, ran nothing, and returned SUCCESS with ' + + '`summary.failed = 0`. ⚠️ Read that measurement for the MECHANISM: the shape it was taken ' + + 'on is a `map`, which this parse rule deliberately does not reach — making the run-time ' + + 'refusal of a region-contained node that durably suspends LOUD is the second half of ' + + 'ruling D and ships as its own `domain:services` change. ⛔ NOT losslessly convertible: ' + + 'hoisting a node out of a region is a GRAPH REWRITE — new edges, a changed exit, sometimes ' + + 'a deleted container — and which of several shapes the author meant is an intent no ' + + 'artifact records, so a transform that picked one would be inventing the design. That ' + + 'leaves D3, a structured TODO naming each node to edit. ⚠️ Two further boundaries this ' + + 'refusal deliberately does NOT reach, because a parse cannot: a pausing node type ' + + 'contributed by a PLUGIN (ADR-0018 left the node-type namespace open and a parse has no ' + + 'registry), and a region nested past `MAX_REGION_DEPTH` (32), where the walk stops. For ' + + 'both, the engine\'s run-time refusal is still the only one — unchanged by this step, not ' + + 'fixed by it.', + acceptanceCriteria: + 'No `screen` / `wait` / `approval` / `approval_revise` node and no `end` node sits inside ' + + 'any `loop` body, `parallel` branch or `try_catch` try/catch region in the stack, at any ' + + 'depth. `FlowSchema.parse` (and therefore `defineFlow`, `registerFlow`, `os validate` and ' + + 'a Studio publish) accepts the stack: a node still nested is refused with the node AND the ' + + 'region named in one message ' + + "(`A \\`approval\\` node may not sit inside a structured region — \\`loop 'sweep' body → " + + "try_catch 'guard' try\\` is a region body …`), anchored at " + + '`nodes[i].config.body.nodes[j].type` so a designer can jump to it. ⛔ A region-nested ' + + '`map` or `subflow` is NOT part of this migration and needs no edit to load — if such a ' + + 'flow reports `success` having processed nothing, that is the run-time half of the same ' + + 'ruling and not a stack edit. Behaviour to re-check after editing, because the fix CHANGES ' + + 'IT deliberately: a region-nested `end` was a no-op, so moving it to the top level makes ' + + 'the run actually TERMINATE there — check that the nodes after the container were not ' + + 'relying on continuing past it. A hoisted `approval` / `wait` / `screen` now parks the run ' + + 'where the enclosing graph can see it, so anything that polled for the sweep to finish ' + + 'sees a suspended run instead of a green-but-empty one.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index b2d7a0d58d1..541ea75d873 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -10980,6 +10980,80 @@ const step18: MigrationStep = { + 'parse-and-refuse accepts and rejects exactly the same sets before and after, ' + 'and no stored metadata or document needs editing.', }, + { + id: 'structured-region-body-pause-and-end-refused', + surface: + 'The BODY of every ADR-0031 structured region — `loop.config.body`, each ' + + '`parallel.config.branches[]`, and `try_catch.config.try` / `.catch` — at every depth the ' + + 'flow parse walks. Two node populations become undeclarable there: a node whose TYPE parks ' + + 'the run on EVERY execution (`screen`, `wait`, `approval`, `approval_revise`) and an ' + + '`end` node, whatever its `outcome`. ⛔ `subflow` and `map` are NOT in the population, ' + + 'although their executors also declare `supportsPause: true`: they pause exactly when the ' + + 'child flow their `config.flowName` names pauses, which is a DIFFERENT metadata record and ' + + 'is not in hand while this flow is parsed. Refusing them by type would also refuse ' + + '`loop { map(synchronous child) }`, a shape that runs correctly today, so a region-nested ' + + '`map` or `subflow` still parses and is met at RUN time instead.', + replacement: + 'Move the node onto the TOP-LEVEL graph and route the region\'s exit to it. For an `end`: ' + + 'delete it from the body, give the region a normal exit, and put the terminator (with its ' + + '`outcome` / `message`) on the top-level graph — `loop { body: [ …, end ] }` becomes ' + + '`loop { body: [ … ] } → end`. For a pausing node: hoist it out of the container — ' + + '`loop { body: [ try_catch { try: [ approval ] } ] }` becomes a top-level `approval` with ' + + 'the loop fanning out around it, or the pausing half of the branch is split into a ' + + '`subflow` the top-level graph calls; where the repetition is genuinely needed, make the ' + + 'TOP-LEVEL graph the repeating construct with the pause on it rather than nesting the ' + + 'pause inside a region. A `try_catch` whose only purpose was to contain the region\'s ' + + 'refusal has nothing left to contain and is deleted with it.', + reason: + 'Maintainer ruling, decision batch #145 item 5, verbatim and untranslated: 「同意,其他也同' + + '意」, carrying the presented option C; extended by batch #146 「146 同意」, which attached ' + + 'the `end` half (the absorbed #18112) and recorded that #3267 is ruled 禁 — structured ' + + 'regions do not support durable pause and a region body cannot terminate the run, so this ' + + 'is that limit\'s authoring-time enforcement rather than an interim. The POPULATION was ' + + 'then fixed by decision batch #153 item 1, letter D (maintainer 「其他同意」): 「inside ' + + '`loop` / `parallel` branch / `try_catch` (try and catch) bodies at any depth, the node ' + + 'types `screen`, `wait`, `approval`, `approval_revise` and `end` are refused by ' + + '`FlowSchema.superRefine` … `map` and `subflow` are ⛔ not refused by type.」 A parse-time ' + + 'rule refuses what is STATICALLY wrong; refusing `map` / `subflow` by type would refuse a ' + + 'correct working shape on a guess about another record. The refusal already existed AT RUN ' + + 'TIME and said nothing an author could act on: the engine converts a suspension raised ' + + 'inside a region into an error at the region boundary, AFTER the executor has written its ' + + 'progress state into the enclosing scope, so a `try_catch` that contains that error leaves ' + + 'residue the next entry reads back as progress. Measured on a real `AutomationEngine`, ' + + '`loop { try_catch { map(pausing child) } }` over 3 iterations x 2 items: not one item\'s ' + + 'subflow ever completed, only two of three iterations reached the catch, and iteration 3 ' + + 'read `started === collection.length`, ran nothing, and returned SUCCESS with ' + + '`summary.failed = 0`. ⚠️ Read that measurement for the MECHANISM: the shape it was taken ' + + 'on is a `map`, which this parse rule deliberately does not reach — making the run-time ' + + 'refusal of a region-contained node that durably suspends LOUD is the second half of ' + + 'ruling D and ships as its own `domain:services` change. ⛔ NOT losslessly convertible: ' + + 'hoisting a node out of a region is a GRAPH REWRITE — new edges, a changed exit, sometimes ' + + 'a deleted container — and which of several shapes the author meant is an intent no ' + + 'artifact records, so a transform that picked one would be inventing the design. That ' + + 'leaves D3, a structured TODO naming each node to edit. ⚠️ Two further boundaries this ' + + 'refusal deliberately does NOT reach, because a parse cannot: a pausing node type ' + + 'contributed by a PLUGIN (ADR-0018 left the node-type namespace open and a parse has no ' + + 'registry), and a region nested past `MAX_REGION_DEPTH` (32), where the walk stops. For ' + + 'both, the engine\'s run-time refusal is still the only one — unchanged by this step, not ' + + 'fixed by it.', + acceptanceCriteria: + 'No `screen` / `wait` / `approval` / `approval_revise` node and no `end` node sits inside ' + + 'any `loop` body, `parallel` branch or `try_catch` try/catch region in the stack, at any ' + + 'depth. `FlowSchema.parse` (and therefore `defineFlow`, `registerFlow`, `os validate` and ' + + 'a Studio publish) accepts the stack: a node still nested is refused with the node AND the ' + + 'region named in one message ' + + "(`A \\`approval\\` node may not sit inside a structured region — \\`loop 'sweep' body → " + + "try_catch 'guard' try\\` is a region body …`), anchored at " + + '`nodes[i].config.body.nodes[j].type` so a designer can jump to it. ⛔ A region-nested ' + + '`map` or `subflow` is NOT part of this migration and needs no edit to load — if such a ' + + 'flow reports `success` having processed nothing, that is the run-time half of the same ' + + 'ruling and not a stack edit. Behaviour to re-check after editing, because the fix CHANGES ' + + 'IT deliberately: a region-nested `end` was a no-op, so moving it to the top level makes ' + + 'the run actually TERMINATE there — check that the nodes after the container were not ' + + 'relying on continuing past it. A hoisted `approval` / `wait` / `screen` now parks the run ' + + 'where the enclosing graph can see it, so anything that polled for the sweep to finish ' + + 'sees a suspended run instead of a green-but-empty one.', + }, { id: 'sys-account-issuer-retired', surface: '`sys_account.issuer` — the column, its `{ fields: [\'issuer\', \'account_id\'], unique: '