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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .changeset/18881-region-durable-suspension-refusal.md
Original file line number Diff line number Diff line change
@@ -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 (`<nodeId>.$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.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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}` };
}
Expand Down
27 changes: 27 additions & 0 deletions packages/services/service-automation/src/builtin/try-catch-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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 —
Expand Down Expand Up @@ -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
Expand Down
95 changes: 78 additions & 17 deletions packages/services/service-automation/src/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ const FLOW_NODE_UNKNOWN_KEY_GUIDANCE: Record<string, Record<string, string>> = {
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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading