diff --git a/.changeset/18881-region-durable-suspension-refusal.md b/.changeset/18881-region-durable-suspension-refusal.md new file mode 100644 index 00000000000..827602e4f76 --- /dev/null +++ b/.changeset/18881-region-durable-suspension-refusal.md @@ -0,0 +1,29 @@ +--- +"@objectstack/service-automation": patch +--- + +A node that **durably suspends inside a structured region body** now FAILS the run with a named refusal that carries the region node, the suspending node and the sub-flow — instead of being read as an ordinary region failure that a `try_catch` could contain, after which the run reported success over a sweep that had processed nothing (#18881, the runtime half of #15646's ruling D). + +An ADR-0031 region body — a `loop` body, a `parallel` branch, a `try_catch` try or catch region, **at any depth** — runs synchronously inside the enclosing run and cannot park it on a durable pause. #3267 ruled that limit 禁. `runRegion` already converted such a suspension, but into a plain `Error`, which is indistinguishable from a node that simply failed. + +Measured on the card's reproduction, `loop { try_catch { map(pausing child) } }`, before this change: + +``` +result.success true // the catch handler ran and "recovered" +run.status completed +summary.failed 0 // over 0 of 10 child runs +``` + +The `map`'s progress state (`.$mapState`) is written into the **enclosing** scope, so the residue a contained refusal leaves is read back as progress by the next entry to the same node: iteration 2 saw `started === collection.length`, ran nothing, and reported success. A sweep that reports green having done nothing is the worst available failure, and it is the one the run-level `failed` counter (#14456) was built to expose. + +What changed: + +- **`FlowRegionSuspensionRefusalError`** (new internal module `region-suspension-refusal.ts`, ⛔ not exported from the package entry) carries `regionNodeId`, `regionKind`, `suspendedNodeId` and `subFlowName` as fields as well as in its message, so a reader never parses the sentence. It is branded as a #3863 guard refusal, so a `fault` edge on the enclosing container cannot route it either. +- **`try_catch` re-throws it** from both the try-attempt arm and the catch-region arm rather than treating it as a region failure, and ⛔ spends no retry attempt on it — re-entering the region would re-enter the pausing node, and the metadata is what is wrong. **`parallel` re-throws it** rather than folding it into its returned (and therefore routable) branch failure. `loop` already re-threw unchanged. +- **One refusal is one failure.** The region node's own frame records the `EXECUTION_ERROR` step and publishes `{$error}`, exactly as any thrown node failure does; every enclosing container the unwind passes through records nothing, so `summary.failed` counts the fault and ⛔ not the nesting depth. + +⛔ **Nothing changes for a region whose nodes complete synchronously.** `loop { map(synchronous child) }`, `parallel { branch: [map(synchronous child)] }` and #15616's regression suite run exactly as before — pinned as explicit controls beside every refusal case, because without them a reader cannot tell "the durable pause is refused" from "the region path was closed off". + +⛔ **No authoring-time rule is added here**: #18688 landed that half in `packages/spec` and it refuses `screen` / `wait` / `approval` / `approval_revise` / `end` inside a region body by type. `map` and `subflow` are deliberately not refused there — whether they pause is decided by the child flow record their `config.flowName` names — which is exactly why the runtime arm has to exist. + +⛔ **No new `error.code`.** The closed `ERROR_CODE_LEDGER` (ADR-0112) lives in `packages/spec`; the refusal is named by its type and its fields, and the step it produces keeps the `EXECUTION_ERROR` code every thrown node failure has always carried. diff --git a/packages/services/service-automation/src/builtin/parallel-node.ts b/packages/services/service-automation/src/builtin/parallel-node.ts index 85bb8e732d1..831b21c192b 100644 --- a/packages/services/service-automation/src/builtin/parallel-node.ts +++ b/packages/services/service-automation/src/builtin/parallel-node.ts @@ -6,6 +6,7 @@ import type { ParallelConfigParsed } from '@objectstack/spec/automation'; import type { AutomationContext } from '@objectstack/spec/contracts'; import type { AutomationEngine, StepLogEntry } from '../engine.js'; import { parseNodeConfig } from './parse-config.js'; +import { isRegionSuspensionRefusal } from '../region-suspension-refusal.js'; /** * `parallel` built-in node — a **structured parallel block** with an @@ -93,6 +94,14 @@ export function registerParallelNode(engine: AutomationEngine, ctx: PluginContex ), ); } catch (err) { + // [#18881] A durable pause raised inside a BRANCH is refused at the + // region boundary and must reach the run, ⛔ not be folded into this + // node's returned failure. A returned failure is routable by a `fault` + // edge on the `parallel` node, and routing this one would re-open the + // silence the card closes: the run would continue past a region that + // parked nothing and report success. Re-thrown, so the engine's own + // catch path records it once and fails the run. + if (isRegionSuspensionRefusal(err)) throw err; const message = err instanceof Error ? err.message : String(err); return { success: false, error: `parallel '${node.id}': branch failed — ${message}` }; } diff --git a/packages/services/service-automation/src/builtin/try-catch-node.ts b/packages/services/service-automation/src/builtin/try-catch-node.ts index a24ef77771d..c4ed5b30786 100644 --- a/packages/services/service-automation/src/builtin/try-catch-node.ts +++ b/packages/services/service-automation/src/builtin/try-catch-node.ts @@ -7,6 +7,8 @@ import type { AutomationContext } from '@objectstack/spec/contracts'; import type { AutomationEngine, StepLogEntry } from '../engine.js'; import { parseNodeConfig } from './parse-config.js'; import { currentLoopIteration } from './loop-frame.js'; +import { isRegionSuspensionRefusal } from '../region-suspension-refusal.js'; +import { attachPartialSteps } from '../partial-steps.js'; /** * `try_catch` built-in node — **structured try/catch/retry** (ADR-0031 §Decision 3). @@ -214,6 +216,23 @@ export function registerTryCatchNode(engine: AutomationEngine, ctx: PluginContex childSteps: [...failedAttemptSteps, ...trySteps], }; } catch (err) { + // [#18881] ⛔ NOT a try-region failure, and the one arm that decides + // whether this card's defect exists. A durable pause raised inside + // this region is refused at the region boundary; read as a failure it + // would run the catch handler and the node would return SUCCESS — the + // measured shape, `loop { try_catch { map(pausing child) } }` + // reporting `completed` with `summary.failed = 0` over a sweep that + // processed nothing. It is re-thrown so the RUN fails, and ⛔ no + // retry attempt is spent on it: re-entering the region would re-enter + // the pausing node, and the metadata is what is wrong. + // + // The attempt's steps ride out with it (#13803's channel), so the + // rows the region really did write before the pause stay in the run + // log and in the #4354 totals. + if (isRegionSuspensionRefusal(err)) { + attachPartialSteps(err, [...failedAttemptSteps, ...attemptSteps]); + throw err; + } lastError = err instanceof Error ? err.message : String(err); const innerError = variables.get('$error'); // Only a `$error` that actually CHANGED (identity, not content — @@ -284,6 +303,14 @@ export function registerTryCatchNode(engine: AutomationEngine, ctx: PluginContex childSteps: [...failedAttemptSteps, ...catchSteps], }; } catch (catchErr) { + // [#18881] The CATCH region is a region body too — ruling D names + // `try_catch`'s try and catch alike — so a durable pause raised in + // the handler is refused on exactly the same terms and travels out + // rather than becoming this node's returned failure. + if (isRegionSuspensionRefusal(catchErr)) { + attachPartialSteps(catchErr, [...failedAttemptSteps, ...catchAttemptSteps]); + throw catchErr; + } const catchMsg = catchErr instanceof Error ? catchErr.message : String(catchErr); // #14222 — the THIRD returned-failure path, and the last one still // discarding its record. #13803 taught the engine to fold a dying diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index d5d46ca3473..53fee28ca7d 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -211,6 +211,7 @@ const FLOW_NODE_UNKNOWN_KEY_GUIDANCE: Record> = { import { runIsUnscopedUserMode, flowTouchesData } from './runtime-identity.js'; import { isGuardRefusal, refuseNode } from './guard-refusal.js'; import { readPartialSteps } from './partial-steps.js'; +import { isRegionSuspensionRefusal, refuseRegionSuspension } from './region-suspension-refusal.js'; import { summarizeRun, formatRunSummaryLine } from './run-summary.js'; // #5660 — the degrade registration reports a FOREIGN failure (a third-party // provider factory's text), so it renders it as structured `meta` rather than @@ -9507,15 +9508,35 @@ export class AutomationEngine implements IAutomationService { } } catch (execErr: unknown) { const errMsg = execErr instanceof Error ? execErr.message : String(execErr); - steps.push({ - nodeId: node.id, - nodeType: node.type, - status: 'failure', - startedAt: stepStartedAt, - completedAt: new Date().toISOString(), - durationMs: Date.now() - stepStart, - error: { code: 'EXECUTION_ERROR', message: errMsg }, - }); + // [#18881] ONE region refusal is ONE failure. The refusal names + // the region node whose body could not carry the pause, and + // that node's own frame records it exactly as any other thrown + // failure does. Every ENCLOSING container the unwind passes + // through — the `loop` around the `try_catch` in the card's + // reproduction — records nothing: it did not fail, it is the + // frame a failure is travelling out through, and a step for it + // would make `summary.failed` count the NESTING DEPTH rather + // than the fault. `summary.failed` is `Σ nodes[].failures` + // (#14456), so a second step here reads as a second lost row to + // every operator and every #4354 reader. + // + // Keyed on the refusal's OWN `regionNodeId` rather than on a + // mutable "already reported" flag: the identity is decided once + // at the boundary that raised it and cannot drift as the error + // travels. + const enclosingFrameOfRegionRefusal = + isRegionSuspensionRefusal(execErr) && execErr.regionNodeId !== node.id; + if (!enclosingFrameOfRegionRefusal) { + steps.push({ + nodeId: node.id, + nodeType: node.type, + status: 'failure', + startedAt: stepStartedAt, + completedAt: new Date().toISOString(), + durationMs: Date.now() - stepStart, + error: { code: 'EXECUTION_ERROR', message: errMsg }, + }); + } // #13803 — a structured container that DIED mid-body still did // whatever its completed iterations did, and those writes are @@ -9573,8 +9594,17 @@ export class AutomationEngine implements IAutomationService { // untouched and still decides, alone, which failures a `fault` // edge may carry. Nor is the thrown value touched — `execErr` is // rethrown below exactly as caught. - variables.set('$error', { nodeId: node.id, message: errMsg }); - this.setNodeError(variables, node.id, errMsg); + // + // [#18881] …and it is published for the SAME frames that record + // a step, for the same reason: an enclosing container the + // region refusal is travelling out through did not fail, so + // `{$error}` naming it would be a false sentence about which + // node produced the run's failure. The region node's own frame + // publishes, as any failing node does. + if (!enclosingFrameOfRegionRefusal) { + variables.set('$error', { nodeId: node.id, message: errMsg }); + this.setNodeError(variables, node.id, errMsg); + } // #3863 — a guard that THROWS is as un-routable as one that // returns: `UnscopedRunDataAccessError` (ADR-0049/#1888) reports @@ -9997,9 +10027,15 @@ export class AutomationEngine implements IAutomationService { * larger seams than an out-parameter the two callers that want it opt into. * Callers that do not pass a sink (`loop`, `parallel`) are unaffected. * - * Durable pause (`suspend`) inside a region is not supported in this - * iteration — it is converted into a clear error (mirrors the `subflow` - * nested-pause guard). + * [#18881] Durable pause (`suspend`) inside a region is not supported — + * #3267 ruled that limit 禁, and this boundary is where the run meets it. + * The conversion is a NAMED refusal + * ({@link FlowRegionSuspensionRefusalError}) carrying the region node, the + * suspending node and the sub-flow, ⛔ not the plain `Error` it used to + * raise: an enclosing `try_catch` read that one as an ordinary region + * failure, ran its catch handler, and the run reported success over a sweep + * that had processed nothing. The container executors test for the named + * type and re-throw, so no region can contain it. */ async runRegion( region: FlowRegionParsed, @@ -10065,10 +10101,35 @@ export class AutomationEngine implements IAutomationService { // this path's contract is (still) to throw. tag(); partialSteps?.push(...regionSteps); + // [#18881] A refusal raised at an INNER region boundary is already + // the named one, and it is re-thrown untouched. Re-wrapping it here + // would rename the region: for `loop { try_catch { map } }` the + // author's fault is the try region, and the loop is only the frame + // the unwind passes through. Tested before the suspend arm because + // this error is not a suspend signal and must not reach the generic + // rethrow below with an enclosing region's identity stamped on it. + if (isRegionSuspensionRefusal(err)) throw err; + // [#18881] The runtime half of #15646's ruling D: a region body + // cannot carry a durable pause, and the refusal is now NAMED + // (region node, suspending node, sub-flow) instead of a plain + // `Error` that an enclosing `try_catch` read as an ordinary region + // failure and handed to its catch handler. See + // `region-suspension-refusal.ts` for the measurement that is. + // + // The sub-flow is read off the suspending node's own `config` + // (`map` / `subflow` name their child there). The node is looked up + // in THIS region's `nodes` because this is the innermost boundary + // the signal crosses — a deeper suspension was already converted by + // the arm above, so `err.nodeId` always names a node of this body. if (isSuspendSignal(err)) { - throw new Error( - `durable pause inside a structured region (node '${err.nodeId}') is not supported`, - ); + const suspended = region.nodes.find(n => n.id === err.nodeId); + const flowName = (suspended?.config as { flowName?: unknown } | undefined)?.flowName; + throw refuseRegionSuspension({ + regionNodeId: grouping?.parentNodeId ?? entryId, + regionKind: grouping?.regionKind ?? 'region', + suspendedNodeId: err.nodeId, + ...(typeof flowName === 'string' && flowName ? { subFlowName: flowName } : {}), + }); } // [#15788] The refusing `end` node's signal, converted at exactly // the same boundary and for the same reason: a control signal must diff --git a/packages/services/service-automation/src/region-durable-suspension-refusal.test.ts b/packages/services/service-automation/src/region-durable-suspension-refusal.test.ts new file mode 100644 index 00000000000..2c8015186dc --- /dev/null +++ b/packages/services/service-automation/src/region-durable-suspension-refusal.test.ts @@ -0,0 +1,363 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #18881 — the RUNTIME half of #15646's ruling D (director batch #153 item 1). +// +// A node contained in a structured region body (`loop`, a `parallel` branch, +// `try_catch`'s try or catch, at any depth) that DURABLY SUSPENDS must fail the +// run with a named, structured error carrying the region node id, the +// suspending node id and the sub-flow name. #3267 ruled the limit 禁 — +// structured regions do not carry a durable pause — and this is its loud form. +// +// ## What was measured before the fix, on the card's own reproduction +// +// `loop { try_catch { map(pausing child) } }`: `runRegion` converted the +// suspension into a PLAIN `Error`, the enclosing `try_catch` read that as "the +// try region failed" and ran its catch handler, and the run finished +// `completed` with `summary.failed = 0`. The `map`'s progress state +// (`.$mapState`) stayed in the enclosing scope, so the next iteration +// read `started === collection.length`, ran nothing, and reported success. A +// sweep that reports green having processed nothing is the failure this closes. +// +// ⛔ The parse-time half is NOT here and is not re-litigated: #18688 landed it +// in `packages/spec` and it refuses `screen` / `wait` / `approval` / +// `approval_revise` / `end` inside a region body by TYPE. `map` and `subflow` +// are deliberately NOT refused there — whether they pause is decided by the +// child flow record their `config.flowName` names, which no parse holds — so +// every fixture below is still declarable, and that is precisely why the +// runtime arm has to exist. +// +// ⚠️ The CONTROL is not optional (the card says so in those words): the same +// shape over a SYNCHRONOUS child must keep running exactly as it does today. +// Without it a reader cannot tell "the durable pause is refused" from "the +// region path was closed off". + +import { describe, it, expect } from 'vitest'; +import { AutomationEngine } from './engine.js'; +import type { NodeExecutor } from './engine.js'; +import { defineActionDescriptor } from '@objectstack/spec/automation'; +import { InMemorySuspendedRunStore } from './suspended-run-store.js'; +import { registerLoopNode } from './builtin/loop-node.js'; +import { registerMapNode } from './builtin/map-node.js'; +import { registerParallelNode } from './builtin/parallel-node.js'; +import { registerTryCatchNode } from './builtin/try-catch-node.js'; + +function silentLogger(): any { + const l: any = { info() {}, warn() {}, error() {}, debug() {} }; + l.child = () => l; + return l; +} +const pluginCtx = (logger: any) => ({ logger, getService() { throw new Error('none'); } }) as any; + +/** #15616's fixture dimensions, kept identical so the two cards read as one shape. */ +const ROWS = ['r1', 'r2', 'r3', 'r4', 'r5']; +const CELLS = ['a', 'b']; + +interface Harness { + engine: AutomationEngine; + /** One entry per CHILD RUN that actually executed, in order: `row:cell`. */ + ran: string[]; + /** One entry per entry to the `try_catch` node's CATCH handler. */ + recovered: string[]; + /** One entry per loop-body probe pass, downstream of the region. */ + probed: number[]; +} + +/** + * Build the engine, the child flow and the shared executors. + * + * `pausing` decides the ONE thing under test: whether the child flow parks on a + * durable pause. Everything else — the graph, the collection, the executors — + * is byte-identical between the defect fixture and its control. + */ +function base(pausing: boolean): Harness & { registerParent: (flow: unknown) => void } { + const logger = silentLogger(); + const engine = new AutomationEngine(logger); + registerLoopNode(engine, pluginCtx(logger)); + registerMapNode(engine, pluginCtx(logger)); + registerParallelNode(engine, pluginCtx(logger)); + registerTryCatchNode(engine, pluginCtx(logger)); + engine.setSuspendedRunStore(new InMemorySuspendedRunStore()); + + const ran: string[] = []; + const recovered: string[] = []; + const probed: number[] = []; + + engine.registerNodeExecutor({ + type: 'pauser', + descriptor: defineActionDescriptor({ + type: 'pauser', version: '1.0.0', name: 'pauser', + supportsPause: true, resumeAuthority: 'any', + }), + async execute() { return { success: true, suspend: true }; }, + } as NodeExecutor); + engine.registerNodeExecutor({ + type: 'cellmark', + async execute(_node, variables, context) { + const p = (context as any)?.params ?? {}; + ran.push(`${p.row}:${p.cell}`); + variables.set('result', `${p.row}:${p.cell}`); + return { success: true }; + }, + } as NodeExecutor); + engine.registerNodeExecutor({ + type: 'recover', + async execute(_node, variables) { + recovered.push(String(variables.get('row') ?? '?')); + return { success: true }; + }, + } as NodeExecutor); + engine.registerNodeExecutor({ + type: 'probe', + async execute() { probed.push(probed.length); return { success: true }; }, + } as NodeExecutor); + + engine.registerFlow('cell_flow', { + name: 'cell_flow', + label: 'Cell', + type: 'autolaunched', + variables: [{ name: 'result', type: 'text', isOutput: true }], + nodes: [ + { id: 'cs', type: 'start', label: 'Start' }, + ...(pausing ? [{ id: 'cp', type: 'pauser', label: 'Pause' }] : []), + { id: 'cm', type: 'cellmark', label: 'Mark' }, + { id: 'ce', type: 'end', label: 'End' }, + ], + edges: pausing + ? [ + { id: 'c1', source: 'cs', target: 'cp' }, + { id: 'c2', source: 'cp', target: 'cm' }, + { id: 'c3', source: 'cm', target: 'ce' }, + ] + : [ + { id: 'c1', source: 'cs', target: 'cm' }, + { id: 'c2', source: 'cm', target: 'ce' }, + ], + } as never); + + return { engine, ran, recovered, probed, registerParent: (flow) => engine.registerFlow('sweep_flow', flow as never) }; +} + +/** The mapped node, shared by every parent graph below. */ +const MAP_NODE = { + id: 'per_cell', type: 'map', label: 'For each cell', + config: { + flowName: 'cell_flow', + collection: '{cells}', + iteratorVariable: 'cell', + input: { row: '{row}', cell: '{cell}' }, + outputVariable: 'cellResults', + }, +} as const; + +const PARENT_VARIABLES = [ + { name: 'rows', type: 'list', isInput: true }, + { name: 'cells', type: 'list', isInput: true }, +] as const; + +/** #15646's exact reproduction: `loop { try_catch { map } }` — depth 2. */ +function containedSetup(pausing: boolean): Harness { + const h = base(pausing); + h.registerParent({ + name: 'sweep_flow', label: 'Sweep', type: 'autolaunched', + variables: PARENT_VARIABLES, + nodes: [ + { id: 'ss', type: 'start', label: 'Start' }, + { + id: 'sweep', type: 'loop', label: 'For each row', + config: { + collection: '{rows}', iteratorVariable: 'row', + body: { + nodes: [ + { + id: 'guard', type: 'try_catch', label: 'Contain the row', + config: { + try: { nodes: [MAP_NODE], edges: [] }, + catch: { + nodes: [{ id: 'rec', type: 'recover', label: 'Recover' }], + edges: [], + }, + }, + }, + { id: 'probe', type: 'probe', label: 'Probe' }, + ], + edges: [{ id: 'be', source: 'guard', target: 'probe' }], + }, + }, + }, + { id: 'se', type: 'end', label: 'End' }, + ], + edges: [ + { id: 's1', source: 'ss', target: 'sweep' }, + { id: 's2', source: 'sweep', target: 'se' }, + ], + }); + return h; +} + +/** `loop { map }` — depth 1, the region kind a `loop` body is. */ +function loopBodySetup(pausing: boolean): Harness { + const h = base(pausing); + h.registerParent({ + name: 'sweep_flow', label: 'Sweep', type: 'autolaunched', + variables: PARENT_VARIABLES, + nodes: [ + { id: 'ss', type: 'start', label: 'Start' }, + { + id: 'sweep', type: 'loop', label: 'For each row', + config: { + collection: '{rows}', iteratorVariable: 'row', + body: { nodes: [MAP_NODE], edges: [] }, + }, + }, + { id: 'se', type: 'end', label: 'End' }, + ], + edges: [ + { id: 's1', source: 'ss', target: 'sweep' }, + { id: 's2', source: 'sweep', target: 'se' }, + ], + }); + return h; +} + +/** `parallel { branch: [map] }` — the third region kind. */ +function parallelBranchSetup(pausing: boolean): Harness { + const h = base(pausing); + h.registerParent({ + name: 'sweep_flow', label: 'Sweep', type: 'autolaunched', + variables: PARENT_VARIABLES, + nodes: [ + { id: 'ss', type: 'start', label: 'Start' }, + { + id: 'fan', type: 'parallel', label: 'Fan out', + // Two branches, because `validateControlFlow` refuses a + // `parallel` with fewer — the sibling branch is also the + // control for "the region refusal is not simply every branch + // failing": it completes normally either way. + config: { + branches: [ + { nodes: [MAP_NODE], edges: [] }, + { nodes: [{ id: 'sibling', type: 'probe', label: 'Sibling' }], edges: [] }, + ], + }, + }, + { id: 'se', type: 'end', label: 'End' }, + ], + edges: [ + { id: 's1', source: 'ss', target: 'fan' }, + { id: 's2', source: 'fan', target: 'se' }, + ], + }); + return h; +} + +const run = (h: Harness) => + h.engine.execute('sweep_flow', { params: { rows: ROWS, cells: CELLS } }); + +describe('#18881 — a durable suspension inside a structured region FAILS the run', () => { + it("the card's reproduction: `loop { try_catch { map(pausing) } }` fails, and the catch handler never sees it", async () => { + const h = containedSetup(true); + + const result = await run(h); + const runs = await h.engine.listRuns('sweep_flow'); + + // The headline: the run FAILS. Before this card it reported + // `completed` — the `try_catch` swallowed the region's refusal. + expect(result.success).toBe(false); + expect(result.status).toBe('failed'); + expect(runs[0]?.status).toBe('failed'); + // ⛔ Never `success` with nothing run: the catch handler must not be + // handed a region refusal, and the loop must not go on to iteration 2. + expect(h.recovered).toEqual([]); + expect(h.probed).toEqual([]); + }); + + it('the error NAMES the region node, the suspending node and the sub-flow', async () => { + const h = containedSetup(true); + + const result = await run(h); + + // Three names, each independently checkable — an operator who reads + // only the run row can find the region, the node inside it, and the + // child flow whose pause could not be carried. + expect(result.error).toContain('guard'); + expect(result.error).toContain('per_cell'); + expect(result.error).toContain('cell_flow'); + }); + + it('counts the refusal ONCE: `summary.failed = 1`, on the region node', async () => { + const h = containedSetup(true); + + const result = await run(h); + + // The counter #14456 built to expose silently-contained failures now + // sees this one. Exactly 1: the region that could not carry the pause, + // ⛔ not one per enclosing container the unwind passes through. + expect(result.summary?.failed).toBe(1); + const failing = result.summary?.nodes?.filter(n => n.failures > 0) ?? []; + expect(failing.map(n => n.nodeId)).toEqual(['guard']); + }); + + it('CONTROL — the same shape over a SYNCHRONOUS child runs exactly as today', async () => { + const h = containedSetup(false); + + const result = await run(h); + const runs = await h.engine.listRuns('sweep_flow'); + + // #15616's measurement, on this card's graph: 5 iterations x 2 items + // ⇒ 10 child runs, a clean run, nothing caught. + expect(result.success).toBe(true); + expect(runs[0]?.status).toBe('completed'); + expect(result.summary?.failed).toBe(0); + expect(h.ran).toHaveLength(ROWS.length * CELLS.length); + expect(h.recovered).toEqual([]); + expect(h.probed).toHaveLength(ROWS.length); + }); +}); + +describe('#18881 — the same refusal in the other two region kinds', () => { + it('`loop { map(pausing) }` — the region node is the `loop`', async () => { + const h = loopBodySetup(true); + + const result = await run(h); + + expect(result.success).toBe(false); + expect(result.status).toBe('failed'); + expect(result.error).toContain('sweep'); + expect(result.error).toContain('per_cell'); + expect(result.error).toContain('cell_flow'); + expect(result.summary?.failed).toBe(1); + }); + + it('CONTROL — `loop { map(synchronous) }` still runs its collection every iteration', async () => { + const h = loopBodySetup(false); + + const result = await run(h); + + expect(result.success).toBe(true); + expect(result.summary?.failed).toBe(0); + expect(h.ran).toHaveLength(ROWS.length * CELLS.length); + }); + + it('`parallel { branch: [map(pausing)] }` — the region node is the `parallel`', async () => { + const h = parallelBranchSetup(true); + + const result = await run(h); + + expect(result.success).toBe(false); + expect(result.status).toBe('failed'); + expect(result.error).toContain('fan'); + expect(result.error).toContain('per_cell'); + expect(result.error).toContain('cell_flow'); + expect(result.summary?.failed).toBe(1); + }); + + it('CONTROL — `parallel { branch: [map(synchronous)] }` still completes', async () => { + const h = parallelBranchSetup(false); + + const result = await run(h); + + expect(result.success).toBe(true); + expect(result.summary?.failed).toBe(0); + expect(h.ran).toHaveLength(CELLS.length); + }); +}); diff --git a/packages/services/service-automation/src/region-suspension-refusal.ts b/packages/services/service-automation/src/region-suspension-refusal.ts new file mode 100644 index 00000000000..3e1fe005dba --- /dev/null +++ b/packages/services/service-automation/src/region-suspension-refusal.ts @@ -0,0 +1,166 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { markGuardRefusal } from './guard-refusal.js'; + +/** + * The "a structured region body durably suspended" refusal (#18881, ruling D on + * #15646 — director batch #153 item 1). + * + * ## The limit this makes loud + * + * An ADR-0031 region body — a `loop` body, a `parallel` branch, a `try_catch` + * try or catch region, at any depth — runs SYNCHRONOUSLY inside the enclosing + * run. It cannot park that run on a durable pause. #3267 ruled that limit 禁 + * rather than a gap to be filled, and #18688 landed the authoring-time half in + * `packages/spec`: `screen` / `wait` / `approval` / `approval_revise` / `end` + * are refused inside a region body by TYPE. + * + * Two node types are deliberately NOT refused there, and they are the reason + * this module exists. Whether a `map` or a `subflow` pauses is decided by the + * child flow record its `config.flowName` names — a DIFFERENT metadata record, + * not in hand at parse — so refusing them by type would also refuse + * `loop { map(synchronous child) }`, a shape that runs correctly today and that + * #15616's regression suite pins. The pause is therefore knowable only at RUN + * time, and only the run can refuse it. + * + * ## Why a dedicated error and not the plain `Error` this replaces + * + * `runRegion` already converted a region-contained suspension — into a plain + * `Error`, which is indistinguishable from a node that simply failed. Measured + * on the card's own reproduction, `loop { try_catch { map(pausing child) } }`: + * the enclosing `try_catch` read that error as "the try region failed", ran its + * catch handler, and the RUN REPORTED SUCCESS. The `map`'s progress state + * (`.$mapState`) stayed behind in the enclosing scope, so iteration 2 + * read `started === collection.length`, ran nothing, and reported success + * again: `summary.failed = 0` over a sweep that processed nothing. A sweep that + * reports green having done nothing is the worst available failure. + * + * Three properties close that, and each is a property of THIS type rather than + * of the message text: + * + * 1. **It is recognisable.** The container executors (`try_catch`, + * `parallel`) test for it and re-throw instead of handling it, so no region + * can contain it and no run can report success over it. A plain `Error` + * offers nothing to test for short of a regex over its message, which is + * the tolerant-consumer shape Prime Directive #12 forbids. + * 2. **It is un-routable.** It is branded as a #3863 guard refusal, so a + * `fault` edge on the enclosing container cannot route it either — the + * one-edge switch that would otherwise re-open the same silence. + * 3. **It NAMES the three things an operator needs**: the region node that + * could not carry the pause, the node inside it that suspended, and the + * sub-flow whose pause it was. The names are structured FIELDS as well as + * message text, so a reader never has to parse the sentence. + * + * ⛔ It carries no `error.code` of its own. The closed `ERROR_CODE_LEDGER` that + * governs that vocabulary (ADR-0112) lives in `packages/spec`, and the step this + * refusal produces keeps the `EXECUTION_ERROR` code every thrown node failure + * has always carried — the refusal is named by its TYPE and its fields, which + * is what the ruling asks for. + * + * @see `guard-refusal.ts` — the same idiom (its own module, because both ends + * need it and `engine.ts` may not be imported back from a built-in executor). + */ +export interface RegionSuspensionRefusalFacts { + /** The container node whose region body could not carry the pause. */ + readonly regionNodeId: string; + /** Which body it was: `loop-body`, `parallel-branch`, `try`, `catch`. */ + readonly regionKind: string; + /** The node inside that body which asked to suspend. */ + readonly suspendedNodeId: string; + /** + * The child flow whose pause propagated up — a `map` / `subflow` node's + * `config.flowName`. + * + * `undefined` only when the suspending node names no child flow (a + * plugin-registered pausing type, invisible to #18688's parse because + * ADR-0018 leaves the node-type namespace open). Recorded honestly as + * absent rather than filled in with invented text. + */ + readonly subFlowName?: string; +} + +const REGION_SUSPENSION_REFUSAL: unique symbol = Symbol.for( + 'objectstack.automation.regionSuspensionRefusal', +) as never; + +/** + * The refusal itself — an `Error`, so every existing reader of a failed run + * (`err instanceof Error ? err.message : String(err)`, the run log's + * `EXECUTION_ERROR` step, `$error`) keeps working with no arm of its own. + */ +export class FlowRegionSuspensionRefusalError extends Error implements RegionSuspensionRefusalFacts { + readonly regionNodeId: string; + readonly regionKind: string; + readonly suspendedNodeId: string; + readonly subFlowName?: string; + + constructor(facts: RegionSuspensionRefusalFacts) { + super(describeRegionSuspensionRefusal(facts)); + this.name = 'FlowRegionSuspensionRefusalError'; + this.regionNodeId = facts.regionNodeId; + this.regionKind = facts.regionKind; + this.suspendedNodeId = facts.suspendedNodeId; + if (facts.subFlowName !== undefined) this.subFlowName = facts.subFlowName; + Object.defineProperty(this, REGION_SUSPENSION_REFUSAL, { + value: true, + enumerable: false, + configurable: false, + writable: false, + }); + markGuardRefusal(this); + } +} + +/** + * The sentence, built in one place so the three names can never disagree with + * the fields beside them. + * + * It states the limit, then WHY the run is failed rather than continued, then + * the one move that fixes the flow — the same three parts #18688's parse-time + * message gives for the node types a parse CAN see, so an author who meets + * either door reads the same prescription. + */ +function describeRegionSuspensionRefusal(facts: RegionSuspensionRefusalFacts): string { + const child = facts.subFlowName !== undefined + ? `sub-flow '${facts.subFlowName}'` + : 'no sub-flow (the node pauses on its own)'; + return ( + `durable pause inside a structured region: node '${facts.suspendedNodeId}' (${child}) suspended ` + + `inside the '${facts.regionKind}' region of node '${facts.regionNodeId}'. A region body runs ` + + 'synchronously inside the enclosing run and cannot carry a durable pause. The run is FAILED ' + + 'rather than continued: the suspending node has already written its progress state into the ' + + 'enclosing scope, so a contained refusal would leave residue that a later entry reads back as ' + + 'progress — the run would then report success having processed nothing. Move the pausing 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.' + ); +} + +/** + * Refuse a region-contained durable suspension. + * + * The one constructor call site outside this module is `runRegion`, which is + * the single boundary every region body unwinds through. + */ +export function refuseRegionSuspension( + facts: RegionSuspensionRefusalFacts, +): FlowRegionSuspensionRefusalError { + return new FlowRegionSuspensionRefusalError(facts); +} + +/** + * True when `err` is this refusal. + * + * Duck-typed on a registered symbol rather than `instanceof`, matching + * `isGuardRefusal` / `isSuspendSignal`: the package ships ESM and CJS builds + * from one source, and a cross-realm `instanceof` is exactly the check that + * silently answers `false` — which here would mean a container swallowing the + * refusal again. + */ +export function isRegionSuspensionRefusal(err: unknown): err is FlowRegionSuspensionRefusalError { + return ( + !!err + && typeof err === 'object' + && (err as Record)[REGION_SUSPENSION_REFUSAL] === true + ); +}