Skip to content

Commit c308064

Browse files
fix(service-automation): refuse a suspension the node type declares it cannot produce (#6667) (#6746)
`ActionDescriptor.supportsPause` was read only at authoring time — the designer palette, `warnIfResumeAuthorityUndeclared`, and `check:resume-authority-declared`. A run pauses for one reason only, `execute()` returning `suspend: true`, so an executor could suspend while its descriptor declared `supportsPause: false` and all three seams stayed silent by construction: every one of them keys on `supportsPause: true`. `AutomationEngine.executeNode` now judges the executor's suspend result against the (alias-resolved) declaration at the single seam every suspension passes through, and refuses the mismatch as a guard-class node failure — un-routable by a `fault` edge, since re-running cannot fix a wrong declaration. Refusing rather than pausing-and-logging is decided on consequence: a type that declares no pause declares no `resumeAuthority` either, and since #5561 an unclaimed pause is fail-closed, so the honoured pause would write a durable continuation the generic resume route then refuses — at resume time, naming `resumeAuthority` rather than the `supportsPause` that caused it. Two shapes stay untouched: `supportsPause: true` on a type that never suspends (a capability, not an obligation), and an executor that publishes no descriptor at all (no declaration to enforce; #5561's resume gate already fail-closes it). Measured on this branch: all six shipped pausing executors declare `supportsPause: true`, so no built-in changes behaviour. Claude-Session: https://claude.ai/code/session_01USNUyHEr7uaU6MoEWXitei Co-authored-by: Claude <noreply@anthropic.com>
1 parent bd5fc38 commit c308064

5 files changed

Lines changed: 512 additions & 24 deletions

File tree

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
'@objectstack/service-automation': patch
3+
---
4+
5+
Enforce `ActionDescriptor.supportsPause` at the engine boundary: an executor whose
6+
`execute()` returns `suspend: true` while its descriptor declares `supportsPause: false`
7+
is now refused instead of pausing the run (#6667, from #5703).
8+
9+
`supportsPause` used to be read only at authoring time — the designer palette, the
10+
registration warning, and the `check:resume-authority-declared` CI gate, all of which key
11+
on `supportsPause: true` and so were silent on exactly this mismatch. The pause it let
12+
through was already broken, just later and elsewhere: a type that declares no pause
13+
declares no `resumeAuthority` either, and since #5561 an unclaimed pause is fail-closed,
14+
so the run parked on a durable continuation that the generic resume route then refused
15+
with `PERMISSION_DENIED` — a message naming `resumeAuthority`, not the `supportsPause`
16+
that actually caused it. The refusal fails the run where the mistake was made, writes no
17+
continuation, and names the one-line fix.
18+
19+
Behaviour change for third-party executors in that state (no built-in is: all six pausing
20+
built-ins declare `supportsPause: true`). The refusal is guard-class, so a `fault` edge
21+
does not route it — a wrong declaration is not a condition a re-run can fix. Two shapes
22+
are deliberately untouched: declaring `supportsPause: true` and never suspending is legal
23+
(a capability, not an obligation), and an executor that publishes no descriptor at all
24+
declares nothing to enforce — its pauses stay governed by the #5561 resume gate.

packages/services/service-automation/src/engine.ts

Lines changed: 129 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ const FLOW_NODE_UNKNOWN_KEY_GUIDANCE: Record<string, Record<string, string>> = {
176176
},
177177
};
178178
import { runIsUnscopedUserMode, flowTouchesData } from './runtime-identity.js';
179-
import { isGuardRefusal } from './guard-refusal.js';
179+
import { isGuardRefusal, refuseNode } from './guard-refusal.js';
180180
import { summarizeRun, formatRunSummaryLine } from './run-summary.js';
181181
// #5660 — the degrade registration reports a FOREIGN failure (a third-party
182182
// provider factory's text), so it renders it as structured `meta` rather than
@@ -1461,14 +1461,16 @@ export class AutomationEngine implements IAutomationService {
14611461
* needs no seal flag (contrast {@link warnIfNodeTypeVocabularyNeverSealed},
14621462
* which reports a missing CALL for the same reason).
14631463
*
1464-
* **Blind spot, stated up front:** the trigger is `supportsPause`, itself a
1465-
* declaration no execution path enforces (#5703) — a run pauses because
1466-
* `execute()` returned `suspend: true`. An executor that suspends while
1467-
* leaving `supportsPause` false is therefore silent here, and since step two
1468-
* its pauses are refused with no prior warning. The refusal message carries
1469-
* the same prescription for exactly that reader (see
1470-
* {@link refuseGatedResume}), `check:resume-authority-declared` catches this
1471-
* repo's own executors at authoring time, and #5703 tracks the runtime half.
1464+
* **Scope, stated up front:** the trigger is `supportsPause`, so a descriptor
1465+
* that leaves it false is not asked this question at all. That used to be a
1466+
* blind spot — the executor could suspend anyway and nothing said a word
1467+
* (#5703) — and it is now closed at the other end instead of here:
1468+
* {@link refuseUndeclaredSuspension} refuses the suspension itself at the
1469+
* engine boundary (#6667), so the mismatch fails the run that produced it
1470+
* rather than parking a continuation this gate never got to warn about.
1471+
* A type that suspends therefore reaches this warning by the only route
1472+
* left — declaring `supportsPause: true`, which is when the question about
1473+
* `resumeAuthority` is worth asking.
14721474
*/
14731475
private warnIfResumeAuthorityUndeclared(descriptor: ActionDescriptor): void {
14741476
if (descriptor.supportsPause !== true) return;
@@ -3094,13 +3096,109 @@ export class AutomationEngine implements IAutomationService {
30943096
* implemented twice and drift.
30953097
*/
30963098
private resolveDeclaredResumeAuthority(nodeType: string): ActionDescriptor['resumeAuthority'] {
3099+
return this.resolveCanonicalDescriptor(nodeType)?.resumeAuthority;
3100+
}
3101+
3102+
/**
3103+
* The descriptor whose CAPABILITY declarations govern a node type: the one
3104+
* registered under that type, or — when that one is a deprecated ADR-0018
3105+
* alias — the canonical descriptor it forwards to.
3106+
*
3107+
* The alias hop is the whole reason this is a function rather than a map
3108+
* lookup, and the reasoning is {@link registerNodeAlias}'s: an alias's
3109+
* descriptor is SYNTHESIZED, so it carries the schema defaults for every
3110+
* capability (`supportsPause: false`, `resumeAuthority` absent) rather than
3111+
* the canonical's real values. Reading it directly would make each capability
3112+
* gate answer "no" for the old type name — one rename away from either a hole
3113+
* (#5561's, if the gate fails open) or a false refusal (#6667's, if it fails
3114+
* closed). Resolving live rather than snapshotting at alias-registration time
3115+
* also keeps the answer right whichever order the two register in. No alias
3116+
* of a pausing type exists today; this keeps it from becoming a defect the
3117+
* day one does.
3118+
*
3119+
* Extracted at #6667 so the two capability gates that need the hop —
3120+
* {@link resolveDeclaredResumeAuthority} (who may resume) and
3121+
* {@link refuseUndeclaredSuspension} (may this type pause at all) — share
3122+
* ONE walk. Two copies of a four-line loop is exactly how one of them
3123+
* acquires a bound the other lacks.
3124+
*/
3125+
private resolveCanonicalDescriptor(nodeType: string): ActionDescriptor | undefined {
30973126
let descriptor = this.actionDescriptors.get(nodeType);
30983127
for (let hop = 0; descriptor?.aliasOf && hop < AutomationEngine.MAX_ALIAS_HOPS; hop++) {
30993128
const canonical = this.actionDescriptors.get(descriptor.aliasOf);
31003129
if (!canonical || canonical === descriptor) break;
31013130
descriptor = canonical;
31023131
}
3103-
return descriptor?.resumeAuthority;
3132+
return descriptor;
3133+
}
3134+
3135+
/**
3136+
* Refuse a suspension the node type never declared it could produce — the
3137+
* runtime half of `supportsPause` (#6667, from #5703).
3138+
*
3139+
* Returns a guard refusal when the node type publishes a descriptor whose
3140+
* (alias-resolved) `supportsPause` is not `true` and its executor just
3141+
* returned `suspend: true`; `null` when there is nothing to refuse.
3142+
*
3143+
* ## Why refuse rather than pause-and-log
3144+
*
3145+
* Honouring the pause and logging `error` was the alternative, and it loses
3146+
* on consequence. A type that leaves `supportsPause` false is, in the same
3147+
* breath, a type `check:resume-authority-declared` does not gate and
3148+
* {@link warnIfResumeAuthorityUndeclared} does not warn about — both key on
3149+
* `supportsPause: true` — so it almost certainly declares no
3150+
* `resumeAuthority` either, and since #5561 step two an undeclared authority
3151+
* resolves to `'service'`: the generic resume route REFUSES every pause it
3152+
* creates. Honouring the suspension therefore writes a durable continuation
3153+
* for a run that nothing can continue, and the `error` line is printed in the
3154+
* process that paused — hours or a restart before anyone tries to resume and
3155+
* gets a `PERMISSION_DENIED` that names `resumeAuthority`, not the
3156+
* `supportsPause` that actually caused it. That is Prime Directive #10
3157+
* exactly: advertising a capability (a resumable pause) the runtime does not
3158+
* deliver, discovered by someone who cannot connect it back.
3159+
*
3160+
* Refusing fails the run at the moment of the mistake, in the process that
3161+
* made it, with the failure handed to the run's own caller and NOTHING
3162+
* durable written — and the message names the one-line fix. It is the same
3163+
* direction #5561 chose for the neighbouring guess: the loud mistake is
3164+
* discoverable by the person who made it; the silent one is not.
3165+
*
3166+
* No log line is emitted here, deliberately. This is AGENTS.md's third legal
3167+
* answer under "Degradation log levels" — a failure handed to the CALLER is
3168+
* not a degradation, and the run's own `failed` history row already carries
3169+
* the message. A `logger.error` on top would fire once per execution of a
3170+
* mis-declared node, which is what makes `error` unreadable.
3171+
*
3172+
* ## What it does NOT judge
3173+
*
3174+
* - **The inverse.** `supportsPause: true` on a type that never suspends is
3175+
* not a mismatch: the declaration is a capability, not an obligation, and
3176+
* `wait` legitimately returns without suspending when its condition is
3177+
* already met.
3178+
* - **Silence.** A node type that publishes NO descriptor declares nothing —
3179+
* not even `false` — so there is no declaration for this gate to enforce,
3180+
* and `NodeExecutor.descriptor` is optional by contract. Its pauses are
3181+
* already fail-closed at the other end (#5561: an absent descriptor means
3182+
* an absent `resumeAuthority`, so the generic route refuses them and says
3183+
* so). Refusing here as well would delete that behaviour, which
3184+
* `resume-authority-gate.test.ts`'s `bare_pause` case pins on purpose.
3185+
*/
3186+
private refuseUndeclaredSuspension(nodeType: string): NodeExecutionResult | null {
3187+
const descriptor = this.resolveCanonicalDescriptor(nodeType);
3188+
// No descriptor ⇒ no declaration ⇒ nothing to enforce (see above).
3189+
if (!descriptor) return null;
3190+
if (descriptor.supportsPause === true) return null;
3191+
return refuseNode(
3192+
`node type '${nodeType}' suspended the run but its action descriptor declares ` +
3193+
`supportsPause: false, so the pause is refused — a run that paused here could not be ` +
3194+
`continued on the generic resume route anyway: a type that declares no pause declares no ` +
3195+
`resumeAuthority either, and an unclaimed pause is fail-closed since #5561. Declare ` +
3196+
`supportsPause: true on the descriptor together with the resumeAuthority the pauses need ` +
3197+
`('any' if POST /automation/:name/runs/:runId/resume is the intended door, 'service' if ` +
3198+
`resuming is the tail of a decision some service must authorize and record first) — or stop ` +
3199+
`returning suspend: true from execute(). This is a metadata defect, not a runtime one, so a ` +
3200+
`fault edge does not route it.`,
3201+
);
31043202
}
31053203

31063204
/**
@@ -4862,6 +4960,27 @@ export class AutomationEngine implements IAutomationService {
48624960
throw execErr;
48634961
}
48644962

4963+
// #6667 — declared = enforced for `supportsPause`, at the ONE seam
4964+
// every suspension passes through.
4965+
//
4966+
// Placed here rather than beside the `throw new FlowSuspendSignal`
4967+
// below on purpose: converting the mismatch into an ordinary guard
4968+
// refusal *before* the success bookkeeping means the run records a
4969+
// `failure` step for the offending node (not a `success` step
4970+
// followed by an unexplained failed run), sets `$error` like any
4971+
// other refusal, and inherits #3863's un-routability — a `fault`
4972+
// edge must not be able to swallow a declaration defect, since
4973+
// re-running the flow unchanged can never fix one.
4974+
//
4975+
// Exactly once, and nothing bypasses it: this is the only call site
4976+
// of any `executor.execute()` that the engine acts on — the ADR-0018
4977+
// alias path delegates and RETURNS its target's result here rather
4978+
// than suspending on its own, `resume()` re-enters through
4979+
// {@link executeNode}, and region bodies ({@link runRegion}) do too.
4980+
if (result.success && result.suspend === true) {
4981+
result = this.refuseUndeclaredSuspension(node.type) ?? result;
4982+
}
4983+
48654984
if (!result.success) {
48664985
const errMsg = result.error ?? 'Unknown error';
48674986
steps.push({

packages/services/service-automation/src/guard-refusal-inventory.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
22

33
import { describe, it, expect, beforeEach } from 'vitest';
4+
import { defineActionDescriptor } from '@objectstack/spec/automation';
45
import { AutomationEngine } from './engine.js';
56
import { registerCrudNodes } from './builtin/crud-nodes.js';
67
import { registerHttpNodes } from './builtin/http-nodes.js';
@@ -156,6 +157,12 @@ const GUARDS: Array<{ name: string; why: string; node: Record<string, unknown>;
156157
node: { type: 'connector_action', config: {} },
157158
expect: 'are required',
158159
},
160+
{
161+
name: 'a node that suspends while its descriptor declares supportsPause: false (#6667)',
162+
why: 'a wrong capability declaration — re-running cannot fix it, and the pause asked for would be unresumable',
163+
node: { type: 'mis_declared_pause' },
164+
expect: 'declares supportsPause: false',
165+
},
159166
];
160167

161168
describe('#3863 — the guard inventory stays un-routable', () => {
@@ -169,6 +176,20 @@ describe('#3863 — the guard inventory stays un-routable', () => {
169176
registerSubflowNode(engine, ctx);
170177
registerMapNode(engine, ctx);
171178
registerConnectorNodes(engine, ctx);
179+
// #6667 — the newest member of the inventory, and the only one that
180+
// needs a fixture: it refuses a DECLARATION mismatch (an executor that
181+
// suspends while its descriptor says it cannot pause), and no shipped
182+
// executor is in that state — all six pausing built-ins declare
183+
// `supportsPause: true`, measured on the #6667 branch. `supports-pause-
184+
// runtime-enforcement.test.ts` owns the behaviour; this row owns its
185+
// classification, which is the one fact this file is about.
186+
engine.registerNodeExecutor({
187+
type: 'mis_declared_pause',
188+
descriptor: defineActionDescriptor({
189+
type: 'mis_declared_pause', version: '1.0.0', name: 'Mis-declared Pause',
190+
}),
191+
async execute() { return { success: true, suspend: true }; },
192+
});
172193
});
173194

174195
it.each(GUARDS.map((g, i) => ({ ...g, i })))(

0 commit comments

Comments
 (0)