diff --git a/.changeset/enforce-supports-pause-at-runtime.md b/.changeset/enforce-supports-pause-at-runtime.md new file mode 100644 index 0000000000..5768af21ab --- /dev/null +++ b/.changeset/enforce-supports-pause-at-runtime.md @@ -0,0 +1,24 @@ +--- +'@objectstack/service-automation': patch +--- + +Enforce `ActionDescriptor.supportsPause` at the engine boundary: an executor whose +`execute()` returns `suspend: true` while its descriptor declares `supportsPause: false` +is now refused instead of pausing the run (#6667, from #5703). + +`supportsPause` used to be read only at authoring time — the designer palette, the +registration warning, and the `check:resume-authority-declared` CI gate, all of which key +on `supportsPause: true` and so were silent on exactly this mismatch. The pause it let +through was already broken, just later and elsewhere: a type that declares no pause +declares no `resumeAuthority` either, and since #5561 an unclaimed pause is fail-closed, +so the run parked on a durable continuation that the generic resume route then refused +with `PERMISSION_DENIED` — a message naming `resumeAuthority`, not the `supportsPause` +that actually caused it. The refusal fails the run where the mistake was made, writes no +continuation, and names the one-line fix. + +Behaviour change for third-party executors in that state (no built-in is: all six pausing +built-ins declare `supportsPause: true`). The refusal is guard-class, so a `fault` edge +does not route it — a wrong declaration is not a condition a re-run can fix. Two shapes +are deliberately untouched: declaring `supportsPause: true` and never suspending is legal +(a capability, not an obligation), and an executor that publishes no descriptor at all +declares nothing to enforce — its pauses stay governed by the #5561 resume gate. diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 353e91e838..f41e7d45b6 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -176,7 +176,7 @@ const FLOW_NODE_UNKNOWN_KEY_GUIDANCE: Record> = { }, }; import { runIsUnscopedUserMode, flowTouchesData } from './runtime-identity.js'; -import { isGuardRefusal } from './guard-refusal.js'; +import { isGuardRefusal, refuseNode } from './guard-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 @@ -1461,14 +1461,16 @@ export class AutomationEngine implements IAutomationService { * needs no seal flag (contrast {@link warnIfNodeTypeVocabularyNeverSealed}, * which reports a missing CALL for the same reason). * - * **Blind spot, stated up front:** the trigger is `supportsPause`, itself a - * declaration no execution path enforces (#5703) — a run pauses because - * `execute()` returned `suspend: true`. An executor that suspends while - * leaving `supportsPause` false is therefore silent here, and since step two - * its pauses are refused with no prior warning. The refusal message carries - * the same prescription for exactly that reader (see - * {@link refuseGatedResume}), `check:resume-authority-declared` catches this - * repo's own executors at authoring time, and #5703 tracks the runtime half. + * **Scope, stated up front:** the trigger is `supportsPause`, so a descriptor + * that leaves it false is not asked this question at all. That used to be a + * blind spot — the executor could suspend anyway and nothing said a word + * (#5703) — and it is now closed at the other end instead of here: + * {@link refuseUndeclaredSuspension} refuses the suspension itself at the + * engine boundary (#6667), so the mismatch fails the run that produced it + * rather than parking a continuation this gate never got to warn about. + * A type that suspends therefore reaches this warning by the only route + * left — declaring `supportsPause: true`, which is when the question about + * `resumeAuthority` is worth asking. */ private warnIfResumeAuthorityUndeclared(descriptor: ActionDescriptor): void { if (descriptor.supportsPause !== true) return; @@ -3094,13 +3096,109 @@ export class AutomationEngine implements IAutomationService { * implemented twice and drift. */ private resolveDeclaredResumeAuthority(nodeType: string): ActionDescriptor['resumeAuthority'] { + return this.resolveCanonicalDescriptor(nodeType)?.resumeAuthority; + } + + /** + * The descriptor whose CAPABILITY declarations govern a node type: the one + * registered under that type, or — when that one is a deprecated ADR-0018 + * alias — the canonical descriptor it forwards to. + * + * The alias hop is the whole reason this is a function rather than a map + * lookup, and the reasoning is {@link registerNodeAlias}'s: an alias's + * descriptor is SYNTHESIZED, so it carries the schema defaults for every + * capability (`supportsPause: false`, `resumeAuthority` absent) rather than + * the canonical's real values. Reading it directly would make each capability + * gate answer "no" for the old type name — one rename away from either a hole + * (#5561's, if the gate fails open) or a false refusal (#6667's, if it fails + * closed). Resolving live rather than snapshotting at alias-registration time + * also keeps the answer right whichever order the two register in. No alias + * of a pausing type exists today; this keeps it from becoming a defect the + * day one does. + * + * Extracted at #6667 so the two capability gates that need the hop — + * {@link resolveDeclaredResumeAuthority} (who may resume) and + * {@link refuseUndeclaredSuspension} (may this type pause at all) — share + * ONE walk. Two copies of a four-line loop is exactly how one of them + * acquires a bound the other lacks. + */ + private resolveCanonicalDescriptor(nodeType: string): ActionDescriptor | undefined { let descriptor = this.actionDescriptors.get(nodeType); for (let hop = 0; descriptor?.aliasOf && hop < AutomationEngine.MAX_ALIAS_HOPS; hop++) { const canonical = this.actionDescriptors.get(descriptor.aliasOf); if (!canonical || canonical === descriptor) break; descriptor = canonical; } - return descriptor?.resumeAuthority; + return descriptor; + } + + /** + * Refuse a suspension the node type never declared it could produce — the + * runtime half of `supportsPause` (#6667, from #5703). + * + * Returns a guard refusal when the node type publishes a descriptor whose + * (alias-resolved) `supportsPause` is not `true` and its executor just + * returned `suspend: true`; `null` when there is nothing to refuse. + * + * ## Why refuse rather than pause-and-log + * + * Honouring the pause and logging `error` was the alternative, and it loses + * on consequence. A type that leaves `supportsPause` false is, in the same + * breath, a type `check:resume-authority-declared` does not gate and + * {@link warnIfResumeAuthorityUndeclared} does not warn about — both key on + * `supportsPause: true` — so it almost certainly declares no + * `resumeAuthority` either, and since #5561 step two an undeclared authority + * resolves to `'service'`: the generic resume route REFUSES every pause it + * creates. Honouring the suspension therefore writes a durable continuation + * for a run that nothing can continue, and the `error` line is printed in the + * process that paused — hours or a restart before anyone tries to resume and + * gets a `PERMISSION_DENIED` that names `resumeAuthority`, not the + * `supportsPause` that actually caused it. That is Prime Directive #10 + * exactly: advertising a capability (a resumable pause) the runtime does not + * deliver, discovered by someone who cannot connect it back. + * + * Refusing fails the run at the moment of the mistake, in the process that + * made it, with the failure handed to the run's own caller and NOTHING + * durable written — and the message names the one-line fix. It is the same + * direction #5561 chose for the neighbouring guess: the loud mistake is + * discoverable by the person who made it; the silent one is not. + * + * No log line is emitted here, deliberately. This is AGENTS.md's third legal + * answer under "Degradation log levels" — a failure handed to the CALLER is + * not a degradation, and the run's own `failed` history row already carries + * the message. A `logger.error` on top would fire once per execution of a + * mis-declared node, which is what makes `error` unreadable. + * + * ## What it does NOT judge + * + * - **The inverse.** `supportsPause: true` on a type that never suspends is + * not a mismatch: the declaration is a capability, not an obligation, and + * `wait` legitimately returns without suspending when its condition is + * already met. + * - **Silence.** A node type that publishes NO descriptor declares nothing — + * not even `false` — so there is no declaration for this gate to enforce, + * and `NodeExecutor.descriptor` is optional by contract. Its pauses are + * already fail-closed at the other end (#5561: an absent descriptor means + * an absent `resumeAuthority`, so the generic route refuses them and says + * so). Refusing here as well would delete that behaviour, which + * `resume-authority-gate.test.ts`'s `bare_pause` case pins on purpose. + */ + private refuseUndeclaredSuspension(nodeType: string): NodeExecutionResult | null { + const descriptor = this.resolveCanonicalDescriptor(nodeType); + // No descriptor ⇒ no declaration ⇒ nothing to enforce (see above). + if (!descriptor) return null; + if (descriptor.supportsPause === true) return null; + return refuseNode( + `node type '${nodeType}' suspended the run but its action descriptor declares ` + + `supportsPause: false, so the pause is refused — a run that paused here could not be ` + + `continued on the generic resume route anyway: a type that declares no pause declares no ` + + `resumeAuthority either, and an unclaimed pause is fail-closed since #5561. Declare ` + + `supportsPause: true on the descriptor together with the resumeAuthority the pauses need ` + + `('any' if POST /automation/:name/runs/:runId/resume is the intended door, 'service' if ` + + `resuming is the tail of a decision some service must authorize and record first) — or stop ` + + `returning suspend: true from execute(). This is a metadata defect, not a runtime one, so a ` + + `fault edge does not route it.`, + ); } /** @@ -4862,6 +4960,27 @@ export class AutomationEngine implements IAutomationService { throw execErr; } + // #6667 — declared = enforced for `supportsPause`, at the ONE seam + // every suspension passes through. + // + // Placed here rather than beside the `throw new FlowSuspendSignal` + // below on purpose: converting the mismatch into an ordinary guard + // refusal *before* the success bookkeeping means the run records a + // `failure` step for the offending node (not a `success` step + // followed by an unexplained failed run), sets `$error` like any + // other refusal, and inherits #3863's un-routability — a `fault` + // edge must not be able to swallow a declaration defect, since + // re-running the flow unchanged can never fix one. + // + // Exactly once, and nothing bypasses it: this is the only call site + // of any `executor.execute()` that the engine acts on — the ADR-0018 + // alias path delegates and RETURNS its target's result here rather + // than suspending on its own, `resume()` re-enters through + // {@link executeNode}, and region bodies ({@link runRegion}) do too. + if (result.success && result.suspend === true) { + result = this.refuseUndeclaredSuspension(node.type) ?? result; + } + if (!result.success) { const errMsg = result.error ?? 'Unknown error'; steps.push({ diff --git a/packages/services/service-automation/src/guard-refusal-inventory.test.ts b/packages/services/service-automation/src/guard-refusal-inventory.test.ts index 4d4ef1e680..893ed26879 100644 --- a/packages/services/service-automation/src/guard-refusal-inventory.test.ts +++ b/packages/services/service-automation/src/guard-refusal-inventory.test.ts @@ -1,6 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect, beforeEach } from 'vitest'; +import { defineActionDescriptor } from '@objectstack/spec/automation'; import { AutomationEngine } from './engine.js'; import { registerCrudNodes } from './builtin/crud-nodes.js'; import { registerHttpNodes } from './builtin/http-nodes.js'; @@ -156,6 +157,12 @@ const GUARDS: Array<{ name: string; why: string; node: Record; node: { type: 'connector_action', config: {} }, expect: 'are required', }, + { + name: 'a node that suspends while its descriptor declares supportsPause: false (#6667)', + why: 'a wrong capability declaration — re-running cannot fix it, and the pause asked for would be unresumable', + node: { type: 'mis_declared_pause' }, + expect: 'declares supportsPause: false', + }, ]; describe('#3863 — the guard inventory stays un-routable', () => { @@ -169,6 +176,20 @@ describe('#3863 — the guard inventory stays un-routable', () => { registerSubflowNode(engine, ctx); registerMapNode(engine, ctx); registerConnectorNodes(engine, ctx); + // #6667 — the newest member of the inventory, and the only one that + // needs a fixture: it refuses a DECLARATION mismatch (an executor that + // suspends while its descriptor says it cannot pause), and no shipped + // executor is in that state — all six pausing built-ins declare + // `supportsPause: true`, measured on the #6667 branch. `supports-pause- + // runtime-enforcement.test.ts` owns the behaviour; this row owns its + // classification, which is the one fact this file is about. + engine.registerNodeExecutor({ + type: 'mis_declared_pause', + descriptor: defineActionDescriptor({ + type: 'mis_declared_pause', version: '1.0.0', name: 'Mis-declared Pause', + }), + async execute() { return { success: true, suspend: true }; }, + }); }); it.each(GUARDS.map((g, i) => ({ ...g, i })))( diff --git a/packages/services/service-automation/src/supports-pause-runtime-enforcement.test.ts b/packages/services/service-automation/src/supports-pause-runtime-enforcement.test.ts new file mode 100644 index 0000000000..71dc152080 --- /dev/null +++ b/packages/services/service-automation/src/supports-pause-runtime-enforcement.test.ts @@ -0,0 +1,322 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `supportsPause` is enforced at the engine boundary (#6667, from #5703). + * + * Before this, `supportsPause` was a declaration three authoring-time seams + * read and no execution path did: the designer palette, the registration + * warning (`warnIfResumeAuthorityUndeclared`), and the + * `check:resume-authority-declared` CI gate. A run pauses for one reason only — + * the executor's `execute()` returned `suspend: true` — so an executor could + * suspend while its `ActionDescriptor` said it could not, and every one of those + * three seams stayed silent, by construction: all three key on + * `supportsPause: true`. + * + * ## Why the mismatch is REFUSED rather than honoured-and-logged + * + * A type that leaves `supportsPause` false is exactly the type nothing gates for + * `resumeAuthority` either — the gate and the warning both trigger on + * `supportsPause: true` — so it almost certainly declares no authority, and + * since #5561 step two an undeclared authority resolves to `'service'`. The + * honoured pause is therefore a durable continuation nothing can continue: the + * generic resume route answers `PERMISSION_DENIED`, and it does so at resume + * time, possibly a restart later, naming `resumeAuthority` rather than the + * `supportsPause` that caused it. Refusing fails the run in the process that + * made the mistake, writes nothing durable, and hands back the one-line fix. + * + * That direction is #5561's own: of two possible mistakes, take the one whose + * victim is the person who made it. + * + * ## The three shapes this file separates + * + * - **mismatch** — declares `supportsPause: false` (or omits it, which parses + * to the same) and suspends. Refused, guard-class, nothing persisted. + * - **the inverse** — declares `supportsPause: true` and never suspends. NOT a + * finding: the field is a capability, not an obligation (`wait` legitimately + * returns straight through when its condition is already met). + * - **silence** — publishes no descriptor at all. Out of scope by design: there + * is no declaration to enforce, `NodeExecutor.descriptor` is optional by + * contract, and #5561 already fail-closes the consequence at the resume gate + * (`resume-authority-gate.test.ts`'s `bare_pause`). Pinned here so narrowing + * or widening that boundary is a deliberate edit, not a silent one. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { defineActionDescriptor } from '@objectstack/spec/automation'; +import { AutomationEngine } from './engine.js'; +import { InMemorySuspendedRunStore } from './suspended-run-store.js'; + +function silentLogger(): any { + return { info() {}, warn() {}, error() {}, debug() {}, child() { return silentLogger(); } }; +} + +/** A descriptor that declares the pause it produces, the way the built-ins do. */ +const declaredPauser = (type: string) => defineActionDescriptor({ + type, version: '1.0.0', name: type, + supportsPause: true, resumeAuthority: 'any', +}); + +/** + * The defect this file is about: a descriptor that declares `supportsPause` + * false — explicitly here, and by omission in the sibling below — on an executor + * that suspends anyway. + */ +const explicitlyNonPausing = (type: string) => defineActionDescriptor({ + type, version: '1.0.0', name: type, + supportsPause: false, +}); + +/** The realistic spelling of the same defect: the key is simply never written. */ +const silentlyNonPausing = (type: string) => defineActionDescriptor({ + type, version: '1.0.0', name: type, +}); + +/** `start → pause → after → end`, so "did the run continue?" is observable. */ +function pauseFlow(name: string, pauseType: string) { + return { + name, + label: name, + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'pause', type: pauseType, label: 'Pause' }, + { id: 'after', type: 'after', label: 'After' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'pause' }, + { id: 'e2', source: 'pause', target: 'after' }, + { id: 'e3', source: 'after', target: 'end' }, + ], + }; +} + +describe('#6667 — an executor may not suspend a run its descriptor says it cannot pause', () => { + let engine: AutomationEngine; + let downstream: string[]; + let store: InMemorySuspendedRunStore; + + beforeEach(() => { + downstream = []; + store = new InMemorySuspendedRunStore(); + engine = new AutomationEngine(silentLogger(), store); + engine.registerNodeExecutor({ + type: 'after', + async execute(node) { downstream.push(node.id); return { success: true }; }, + }); + }); + + /** Registers a suspending executor under `type` with the given descriptor. */ + function registerSuspender(type: string, descriptor?: unknown): void { + engine.registerNodeExecutor({ + type, + ...(descriptor ? { descriptor: descriptor as never } : {}), + async execute() { return { success: true, suspend: true, correlation: 'req_1' }; }, + }); + } + + // ── the mismatch ──────────────────────────────────────────────────── + + it('refuses the suspension, fails the run, and names the fix', async () => { + registerSuspender('bad_pause', explicitlyNonPausing('bad_pause')); + engine.registerFlow('bad_flow', pauseFlow('bad_flow', 'bad_pause') as never); + + const result = await engine.execute('bad_flow'); + + expect(result.success).toBe(false); + expect(result.status).not.toBe('paused'); + // The refusal names the declaration that is wrong, the consequence it + // would otherwise have had, and both halves of the one-line fix — a + // reader who sees only `Node 'pause' failed` learns nothing actionable. + expect(result.error ?? '').toContain('declares supportsPause: false'); + expect(result.error ?? '').toContain('Declare supportsPause: true'); + expect(result.error ?? '').toContain('resumeAuthority'); + // Refused BEFORE the pause became a fact: nothing downstream ran, and + // no continuation was written to either the cache or the durable store. + expect(downstream).toEqual([]); + expect(engine.listSuspendedRuns()).toHaveLength(0); + expect(await store.list()).toHaveLength(0); + }); + + it('records the offending node as a FAILED step, not a success followed by a mystery', async () => { + registerSuspender('bad_pause', explicitlyNonPausing('bad_pause')); + engine.registerFlow('bad_flow', pauseFlow('bad_flow', 'bad_pause') as never); + + await engine.execute('bad_flow'); + + const runs = await engine.listRuns('bad_flow'); + expect(runs).toHaveLength(1); + expect(runs[0].status).toBe('failed'); + const step = runs[0].steps.find(s => s.nodeId === 'pause'); + expect(step).toBeDefined(); + expect(step!.status).toBe('failure'); + // The engine's node-failure code (`packages/spec` error-code ledger, + // ADR-0112). A refusal that recorded no code would be indistinguishable + // from the run simply stopping. + expect(step!.error?.code).toBe('NODE_FAILURE'); + expect(step!.error?.message ?? '').toContain('declares supportsPause: false'); + }); + + it('treats an OMITTED supportsPause exactly like an explicit false', async () => { + // The realistic spelling: nobody types `supportsPause: false`, they just + // never think about the key. `ActionDescriptorSchema` defaults it to + // false, so the two are the same declaration — unlike `resumeAuthority`, + // whose omission #5561 deliberately kept observable. + registerSuspender('quiet_pause', silentlyNonPausing('quiet_pause')); + engine.registerFlow('quiet_flow', pauseFlow('quiet_flow', 'quiet_pause') as never); + + const result = await engine.execute('quiet_flow'); + + expect(result.success).toBe(false); + expect(result.error ?? '').toContain('declares supportsPause: false'); + expect(engine.listSuspendedRuns()).toHaveLength(0); + }); + + it('is a GUARD refusal — a fault edge does not route it (#3863)', async () => { + // Re-running the flow unchanged can never fix a wrong declaration, so + // this is the metadata class, not the runtime class. If a fault edge + // could route it, one edge would turn the check off while the run still + // reported success — the shape #3863 exists to prevent. + registerSuspender('bad_pause', explicitlyNonPausing('bad_pause')); + engine.registerNodeExecutor({ + type: 'handler', + async execute(node) { downstream.push(node.id); return { success: true }; }, + }); + engine.registerFlow('fault_flow', { + name: 'fault_flow', + label: 'fault_flow', + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'pause', type: 'bad_pause', label: 'Pause' }, + { id: 'rescue', type: 'handler', label: 'Rescue' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'pause' }, + { id: 'e2', source: 'pause', target: 'end' }, + { id: 'e_fault', source: 'pause', target: 'rescue', type: 'fault' }, + { id: 'e3', source: 'rescue', target: 'end' }, + ], + } as never); + + const result = await engine.execute('fault_flow'); + + expect(result.success).toBe(false); + expect(downstream).toEqual([]); + }); + + it('refuses a re-suspension on the RESUME path too — one seam, every entry', async () => { + // `resume()` re-enters through `executeNode`, so a run that legitimately + // paused and then reaches a mis-declared node downstream is judged by + // the same check. Without this, half of every multi-pause flow would be + // unguarded. + registerSuspender('good_pause', declaredPauser('good_pause')); + registerSuspender('bad_pause', explicitlyNonPausing('bad_pause')); + engine.registerFlow('two_pauses', { + name: 'two_pauses', + label: 'two_pauses', + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'first', type: 'good_pause', label: 'First' }, + { id: 'second', type: 'bad_pause', label: 'Second' }, + { id: 'after', type: 'after', label: 'After' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'first' }, + { id: 'e2', source: 'first', target: 'second' }, + { id: 'e3', source: 'second', target: 'after' }, + { id: 'e4', source: 'after', target: 'end' }, + ], + } as never); + + const paused = await engine.execute('two_pauses'); + expect(paused.status).toBe('paused'); + + const resumed = await engine.resume(paused.runId!); + + expect(resumed.success).toBe(false); + expect(resumed.error ?? '').toContain('declares supportsPause: false'); + expect(downstream).toEqual([]); + // The first suspension was consumed by the resume and the second was + // never created — the run is terminally failed, not half-parked. + expect(engine.listSuspendedRuns()).toHaveLength(0); + expect(await store.list()).toHaveLength(0); + }); + + // ── what still works, unchanged ───────────────────────────────────── + + it('a correctly-declaring executor suspends and resumes exactly as before', async () => { + registerSuspender('good_pause', declaredPauser('good_pause')); + engine.registerFlow('good_flow', pauseFlow('good_flow', 'good_pause') as never); + + const paused = await engine.execute('good_flow'); + expect(paused.success).toBe(true); + expect(paused.status).toBe('paused'); + expect(paused.runId).toBeTruthy(); + expect(engine.listSuspendedRuns()).toHaveLength(1); + + const resumed = await engine.resume(paused.runId!); + expect(resumed.success).toBe(true); + expect(downstream).toEqual(['after']); + }); + + it('does NOT police the inverse: supportsPause: true and never suspending is legal', async () => { + // The declaration is a capability, not an obligation. `wait` returns + // straight through when its condition is already satisfied, and a + // symmetric check would make that a failure. + engine.registerNodeExecutor({ + type: 'may_pause', + descriptor: declaredPauser('may_pause'), + async execute() { return { success: true, output: { went: 'straight through' } }; }, + }); + engine.registerFlow('inverse_flow', pauseFlow('inverse_flow', 'may_pause') as never); + + const result = await engine.execute('inverse_flow'); + + expect(result.success).toBe(true); + expect(result.status).not.toBe('paused'); + expect(downstream).toEqual(['after']); + }); + + it('leaves a descriptor-less executor to the #5561 resume gate — silence is not a declaration', async () => { + // Registering no descriptor declares NOTHING, not `false`. There is no + // declaration for this check to enforce, and the consequence is already + // fail-closed at the other end: the pause is created, and the generic + // resume route refuses it because an absent descriptor carries an absent + // `resumeAuthority`. `resume-authority-gate.test.ts` owns that half; this + // pins that #6667 did not quietly take it over. + registerSuspender('bare_pause'); + engine.registerFlow('bare_flow', pauseFlow('bare_flow', 'bare_pause') as never); + + const paused = await engine.execute('bare_flow'); + + expect(paused.status).toBe('paused'); + expect(engine.listSuspendedRuns()).toHaveLength(1); + + const refused = await engine.resume(paused.runId!); + expect(refused.success).toBe(false); + expect(refused.code).toBe('PERMISSION_DENIED'); + expect(refused.error ?? '').toMatch(/never declares resumeAuthority/); + }); + + it('reads the CANONICAL descriptor through an ADR-0018 alias, not the synthesized one', async () => { + // `registerNodeAlias` synthesizes the alias's descriptor and does not + // copy the canonical's capabilities, so it carries `supportsPause: false` + // by default. Reading it directly would refuse every pause authored under + // the old type name — the mirror image of the hole the same alias hop + // closes for `resumeAuthority`. No alias of a pausing type exists today; + // this keeps the day one does from being a regression. + registerSuspender('new_pause', declaredPauser('new_pause')); + engine.registerNodeAlias('old_pause', 'new_pause'); + engine.registerFlow('alias_flow', pauseFlow('alias_flow', 'old_pause') as never); + + const paused = await engine.execute('alias_flow'); + + expect(paused.success).toBe(true); + expect(paused.status).toBe('paused'); + expect(engine.listSuspendedRuns()).toHaveLength(1); + }); +}); diff --git a/scripts/check-resume-authority-declared.mjs b/scripts/check-resume-authority-declared.mjs index 95d79dfdad..f841d54ab3 100644 --- a/scripts/check-resume-authority-declared.mjs +++ b/scripts/check-resume-authority-declared.mjs @@ -61,15 +61,16 @@ // // ## What it cannot see (stated up front, not discovered later) // -// 1. `supportsPause` is itself a declaration no execution path enforces -// (objectstack#5703): a run pauses because the executor's `execute()` -// returned `suspend: true`. An executor that suspends while leaving -// `supportsPause` false is invisible BOTH here and to the registration -// warning -- and since #5561 step two its pauses are refused with neither -// gate nor warning having said a word first. The refusal message carries the -// same prescription for exactly that reader. Keying on the author's own -// literal is what makes this gate decidable without a call graph; #5703 -// tracks the runtime half. +// 1. An executor that suspends while leaving `supportsPause` false is invisible +// here and to the registration warning alike -- both key on the author's own +// literal, which is what makes this gate decidable without a call graph. +// That used to mean such an executor shipped a fail-open pause (#5703); as +// of objectstack#6667 the ENGINE refuses the suspension at its boundary +// instead, so the mismatch fails the run that produced it and the reader +// hears it from the run that broke rather than from this gate. The division +// of labour is deliberate: this gate judges declarations at authoring time, +// the engine judges behaviour at run time, and neither can see the other's +// subject. // 1b. **Test fixtures are deliberately out of scope.** The subject here is the // SHIPPED node vocabulary — descriptors a real engine registers in a real // deployment. A fixture registers into a throwaway engine and ships to @@ -301,10 +302,10 @@ function report({ list = false, scanRoots = DEFAULT_SCAN_ROOTS } = {}) { if (errors.length) { for (const e of errors) console.error(` x ${e}`); console.error( - '\nNote: supportsPause is itself a declaration nothing enforces at run time (#5703) — a run ' - + 'pauses because execute() returned suspend: true. An executor that suspends while leaving ' - + 'supportsPause false is fail-open and invisible to this gate AND to the engine\'s ' - + 'registration warning; #5703 tracks that half.\n', + '\nNote: a run pauses because execute() returned suspend: true, not because a descriptor said ' + + 'so. An executor that suspends while leaving supportsPause false is invisible to this gate ' + + "AND to the engine's registration warning — since #6667 the engine refuses that suspension " + + 'at its boundary, so the mismatch fails the run instead of parking an unresumable one.\n', ); console.error(`check-resume-authority-declared: ${errors.length} problem(s).\n`); process.exit(1); @@ -320,7 +321,8 @@ function report({ list = false, scanRoots = DEFAULT_SCAN_ROOTS } = {}) { } console.log( `check-resume-authority-declared: OK — every pausing descriptor declares its resume authority ` - + `(${declared}/${pausing}). supportsPause itself stays unenforced at run time (#5703).\n`, + + `(${declared}/${pausing}). A suspension from a type that declares supportsPause: false is ` + + `refused by the engine at run time (#6667), not by this gate.\n`, ); }