diff --git a/.changeset/retry-attempt-pause-suspend-arm.md b/.changeset/retry-attempt-pause-suspend-arm.md new file mode 100644 index 0000000000..485da43f97 --- /dev/null +++ b/.changeset/retry-attempt-pause-suspend-arm.md @@ -0,0 +1,67 @@ +--- +"@objectstack/service-automation": patch +"@objectstack/runtime": patch +--- + +fix(service-automation): a retry attempt that PAUSES is a durable pause, not a failed attempt — `executeWithoutRetry` gets the ADR-0019 suspend arm (#9510) + +`execute()`'s catch tests the suspend signal FIRST, and that arm is what makes +ADR-0019's durable pause work: it snapshots the live variables, calls +`persistSuspendedRun`, records a `paused` log entry and returns +`{ success: true, status: 'paused', runId }`. + +`executeWithoutRetry()` — the method `retryExecution` re-runs the flow through on +**every** retry attempt — had no such arm. A `FlowSuspendSignal` thrown on a +retry attempt fell into the generic failure path, and four things were lost at +once: + +1. `persistSuspendedRun` never ran, so **the continuation was never stored** and + the run could not be resumed by anyone, ever; +2. the run log recorded `failed` for a run that asked to pause; +3. the caller got `status: 'failed'`, with the suspend signal stringified into + `error` (`FlowSuspendSignal` is not an `Error`); +4. `retryExecution` reads only `result.success`, so the pause counted as one more + failed attempt: the loop burned the rest of the budget, and every further + attempt re-entered the pausing node and orphaned another suspension. + +Only a LATER attempt is exposed — `execute()` handles the first one correctly, +and a flow reaches `retryExecution` only after a failure. The reachable shape is +the ordinary one: `errorHandling.strategy: 'retry'` on a flow whose flaky +HTTP/connector call is followed by an `approval` or `screen` node. + +**⚠️ Runs already lost to this defect are NOT recoverable.** Nothing was written +for them — no `sys_automation_run` row, no in-memory suspension — so there is no +continuation to rehydrate and no repair, here or later, can bring one back. The +run log holds a `failed` entry naming the flow and the trigger; those runs have +to be triggered again. What this change fixes is every run from here on. + +**The repair is a restoration of a stated contract on a path that never got it, +not a new capability.** `AutomationResult.status: 'paused'` and ADR-0019 already +describe exactly this behaviour, and `execute()`'s own arm already implements it; +the retry path simply never received it. The alternative — refusing +`strategy: 'retry'` combined with a pausing node at authoring time — was +considered and rejected: it over-refuses (a pausing node can sit on a branch the +retrying path never reaches), under-refuses (a pausing node behind a runtime +condition is not statically decidable), and would ban the one combination authors +most reasonably reach for. + +**The cost, and what was done about it.** Lifting the arm makes `retryExecution` +able to return a NON-TERMINAL result, and both of its readers were taught the +third state explicitly rather than left to a branch that happens to fall through: +the retry loop returns a paused attempt because it PAUSED (tested on `status`, +before the `success` check that means "this attempt succeeded"), and the trigger +route answers it from its own arm. The retry accounting is untouched — a +genuinely failing attempt still consumes one, `maxRetries` still bounds the loop, +and the loop stops only because the attempt did not fail. + +**Both routes give one answer**, pinned as an equality rather than verified in +isolation: a pause on attempt 1 and a pause on attempt 3 produce the same engine +result and the same wire response, so no caller can tell which attempt paused. + +Two adjacent gaps were measured out of this work and filed rather than absorbed: +a retry attempt runs with a smaller variable environment than the first (#9704), +and a flow's declared retry policy stops applying once a run pauses (#9705) — +the latter being the measured answer to "what happens to the retry budget when a +paused run is resumed and then fails": neither inherited nor fresh, because the +resume path has no retry loop at all. Both are pinned as today's behaviour so +neither can change by accident. diff --git a/packages/runtime/src/domains/automation-trigger-paused-run.test.ts b/packages/runtime/src/domains/automation-trigger-paused-run.test.ts new file mode 100644 index 0000000000..f2aa52b319 --- /dev/null +++ b/packages/runtime/src/domains/automation-trigger-paused-run.test.ts @@ -0,0 +1,167 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #9510 — the trigger door answers a PAUSED run as the third state, deliberately. + * + * The engine repair that lifted `execute()`'s ADR-0019 suspend arm into + * `executeWithoutRetry` gave `retryExecution` a NON-TERMINAL result to return: a + * retry attempt that reaches a pausing node now comes back as + * `{ success: true, status: 'paused', runId }` instead of being reported as a + * failed attempt with its continuation dropped. This door is one of the two + * readers that had only ever seen terminal results out of that path. + * + * What is pinned here is the door's READING, driven with scripted + * `AutomationResult`s so every arm is reachable without a real engine (the + * end-to-end sentence — that a real engine's two producers reach this door as + * ONE answer — is `@objectstack/verify`'s + * `automation-trigger-paused-run.test.ts`, and the engine-side equality is + * `service-automation`'s `retry-attempt-pause.test.ts`). + * + * Both spellings of the door are exercised for every arm, from one table, for + * the reason #9378's suite states: they share one context builder and one + * response mapper, and a test covering only the canonical spelling would let the + * legacy one — the one the SDK actually calls — drift unnoticed. + * + * ⚠️ The paused answer is deliberately IDENTICAL to the terminal-success one on + * the wire, so these assertions cannot be satisfied by "some 200". They pin the + * payload a caller resumes with: `status`, `runId`, `screen`, and the absence of + * any refusal envelope. + */ + +import { describe, it, expect, vi } from 'vitest'; + +import { HttpDispatcher } from '../http-dispatcher.js'; +import { classifyFlowRefusal, isPausedRun } from '../flow-dispatch-status.js'; +import type { AutomationResult } from '@objectstack/spec/contracts'; + +const CTX = { request: {}, executionContext: { userId: 'user_1' } } as any; + +/** Both spellings of the same door. `path` takes the flow name. */ +const ROUTES: Array<{ label: string; path: (flow: string) => string }> = [ + { label: 'POST /:name/trigger', path: (f) => `/${f}/trigger` }, + { label: 'legacy POST /trigger/:name', path: (f) => `/trigger/${f}` }, +]; + +function makeDispatcher(result: AutomationResult) { + const flows = new Map([['flaky_approval', { name: 'flaky_approval' }]]); + const execute = vi.fn(async (): Promise => result); + const getFlow = vi.fn(async (name: string) => flows.get(name) ?? null); + const services: Record = { automation: { execute, getFlow } }; + const resolve = (name: string) => services[name]; + const kernel: any = { + getService: resolve, + getServiceAsync: async (name: string) => resolve(name), + context: { getService: resolve }, + }; + return new HttpDispatcher(kernel); +} + +/** + * The engine's paused result, in the shape BOTH producers build it — the arm in + * `execute()`'s catch and the one restored to `executeWithoutRetry`. The two are + * byte-identical apart from the ids, which is the point: this door must not be + * able to tell which attempt paused. + */ +const PAUSED: AutomationResult = { + success: true, + status: 'paused', + runId: 'run_7f0a', + durationMs: 12, + screen: { + title: 'Approve the order', + fields: [{ name: 'verdict', type: 'text', label: 'Verdict' }], + } as AutomationResult['screen'], +}; + +describe('#9510 — a triggered run that PAUSED is answered as the third state', () => { + for (const route of ROUTES) { + it(`${route.label}: answers 200 carrying the runId the caller resumes with`, async () => { + const dispatcher = makeDispatcher(PAUSED); + + const result = await dispatcher.handleAutomation(route.path('flaky_approval'), 'POST', {}, CTX); + + expect(result.handled).toBe(true); + expect(result.response?.status).toBe(200); + expect(result.response?.body?.success).toBe(true); + // The run is ALIVE and parked. Without these two the caller has no + // way to continue it, which is the harm #9510 is about — a 200 that + // merely "looks fine" is not the contract. + expect(result.response?.body?.data?.status).toBe('paused'); + expect(result.response?.body?.data?.runId).toBe('run_7f0a'); + // The screen a screen-flow runner renders travels with it. + expect(result.response?.body?.data?.screen?.title).toBe('Approve the order'); + // …and it is NOT dressed as a refusal: a paused run has no error + // envelope, no code, and nothing for a status-blind caller to read + // as a failure. + expect(result.response?.body?.error).toBeUndefined(); + }); + + it(`${route.label}: a pause with no screen (approval, wait) is still the paused answer`, async () => { + // `screen` is a screen-flow field; an `approval` or `wait` node + // pauses without one, and the body a caller resumes with must be + // the same either way. + // + // ⚠️ Honest about its own reach: this door answers a pause and a + // terminal success IDENTICALLY on the wire — deliberately, since + // both are `200` plus the engine result — so no route-level + // assertion can tell which arm produced the response. What it pins + // is the PAYLOAD (`status`, `runId`, no refusal envelope); that the + // arm reads the lifecycle verdict rather than sniffing `screen` is + // pinned on `isPausedRun` itself, below. + const dispatcher = makeDispatcher({ success: true, status: 'paused', runId: 'run_b21', durationMs: 3 }); + + const result = await dispatcher.handleAutomation(route.path('flaky_approval'), 'POST', {}, CTX); + + expect(result.response?.status).toBe(200); + expect(result.response?.body?.data?.status).toBe('paused'); + expect(result.response?.body?.data?.runId).toBe('run_b21'); + expect(result.response?.body?.error).toBeUndefined(); + }); + } +}); + +describe('#9510 — the shared dispatch table names the non-terminal state', () => { + it('classifies a paused run as no refusal at all', () => { + expect(classifyFlowRefusal('flaky_approval', PAUSED)).toBeUndefined(); + }); + + it('reads the producer\'s lifecycle verdict, never the incidental fields', () => { + // `runId` and `screen` ride along on a pause; neither DEFINES it. + expect(isPausedRun(PAUSED)).toBe(true); + expect(isPausedRun({ success: true, runId: 'run_x' })).toBe(false); + expect(isPausedRun({ success: false, status: 'failed', error: 'boom' })).toBe(false); + expect(isPausedRun(undefined)).toBe(false); + expect(isPausedRun(null)).toBe(false); + + // ⭐ The two cases that actually SEPARATE reading the verdict from + // sniffing a companion field — and the reason they are spelled out: + // every assertion above is satisfied by a predicate that returns + // `!!result.screen`, because a screen happens to accompany the paused + // fixture and to be absent from all the negatives. A pin that cannot + // fail against the tolerant-consumer shape PD #12 forbids is not + // pinning anything, so these two carry the sentence: + // + // - an `approval` or `wait` pause has NO screen and is still a pause; + expect(isPausedRun({ success: true, status: 'paused', runId: 'run_b21' })).toBe(true); + // - a screen on a result that is not parked does NOT make it one. + expect(isPausedRun({ success: true, status: 'completed', screen: PAUSED.screen })).toBe(false); + }); + + it('never promotes a paused run into the FLOW_FAILED row, even against the grain', () => { + // Defensive rather than reachable: no producer stamps this pair today. + // It states which field decides when they disagree — a LIVE suspended + // run, continuation persisted and waiting for a `resume()`, must not be + // reported to its caller as a run that failed. That is #9510's defect + // wearing transport clothing. + const contradictory = { success: false, status: 'paused', runId: 'run_c3' } as AutomationResult; + + expect(classifyFlowRefusal('flaky_approval', contradictory)).toBeUndefined(); + }); + + it('still classifies the terminal refusal rows — the new arm narrows nothing', () => { + expect(classifyFlowRefusal('f', { success: false, status: 'failed', error: 'boom' })?.code) + .toBe('FLOW_FAILED'); + expect(classifyFlowRefusal('f', { success: false, code: 'FLOW_DISABLED' })?.status).toBe(409); + expect(classifyFlowRefusal('f', { success: false, code: 'FLOW_NO_START_NODE' })?.status).toBe(422); + }); +}); diff --git a/packages/runtime/src/domains/automation.ts b/packages/runtime/src/domains/automation.ts index 17c985538f..9ccf12279a 100644 --- a/packages/runtime/src/domains/automation.ts +++ b/packages/runtime/src/domains/automation.ts @@ -27,6 +27,7 @@ import { classifyFlowRefusal, flowIsUnknown, flowNotFoundMessage, + isPausedRun, FLOW_NOT_FOUND_STATUS, } from '../flow-dispatch-status.js'; import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js'; @@ -489,6 +490,14 @@ function flowDefinitionRefusal(err: any): unknown { * | flow disabled | never dispatched | `409` `FLOW_DISABLED` | * | flow has no start node | never dispatched | `422` `FLOW_NO_START_NODE` | * | ran and failed (incl. the retry-strategy exits) | ran, rejected | `400` `FLOW_FAILED` | + * | ran and PAUSED (whichever attempt) | ran, suspended | `200` + `runId` / `screen` | + * + * [#9510] The last row is NON-TERMINAL and is answered by its own arm at the + * bottom of this function. The durable pause is not a refusal, and since the + * suspend arm was restored to the engine's retry path it arrives from two + * producers — `execute()`'s catch and `retryExecution` — which this door must + * NOT be able to tell apart. See that arm for why it is written separately from + * the terminal success it happens to answer identically. * * [#9446] **The table itself now lives in `../flow-dispatch-status.js`** — one * definition, read by this door and by `/actions` (`action-execution.ts`) — @@ -586,6 +595,34 @@ async function respondToFlowTrigger( response: deps.error(refusal.message, refusal.status, { code: refusal.code, ...runDetails }), }; } + // [#9510] THE THIRD STATE, answered deliberately. A run that dispatched and + // then SUSPENDED at a pausing node (ADR-0019) is neither refused nor + // finished: its continuation is persisted, and the `200` here carries the + // `runId` — and the `screen`, for a screen flow — that the caller continues + // it with at `POST /:name/runs/:runId/resume`, the door just below. + // + // The answer is unchanged from what this door has always given a paused + // run, and that IS the requirement rather than an accident of ordering: it + // must be the SAME answer a pause on the first attempt gets, because a + // pause on a retry attempt is the same user-visible situation reached by a + // different route. Two answers for one situation would replace #9510's LOST + // pause with an inconsistent one. Pinned as an equality between the two + // routes — engine-side in `service-automation`'s + // `retry-attempt-pause.test.ts`, and on the wire through a real engine in + // `@objectstack/verify`'s `automation-trigger-paused-run.test.ts`. + // + // ⛔ Its own arm even though it returns what the terminal exit below + // returns. The two are different STATEMENTS about the run — "still running, + // here is how to continue it" versus "it finished" — and collapsing them + // recreates exactly the fall-through this card is about: a non-terminal + // result that no reader on the path ever names is one edit away from being + // classified as a terminal one. + if (isPausedRun(result)) { + return { handled: true, response: deps.success(result) }; + } + // Terminal success: the run reached an `end` node, and `deps.success` serves + // the engine result as the response data (`output`, `successMessage`, + // `summary`). return { handled: true, response: deps.success(result) }; } @@ -606,7 +643,9 @@ async function respondToFlowTrigger( * POST /:name/trigger → execute (legacy: trigger/:name also supported; * unknown name → 404, disabled → 409 `FLOW_DISABLED`, * no start node → 422 `FLOW_NO_START_NODE`, a run that - * ran and failed → 400 `FLOW_FAILED`; #9378 + #9415) + * ran and failed → 400 `FLOW_FAILED`; #9378 + #9415; + * a run that PAUSED → 200 with `runId` / `screen`, + * on whichever attempt it paused — #9510) * POST /:name/toggle → toggleFlow (unknown name → 404, #7535) * GET /:name/runs → listRuns (query: limit, cursor — validated, #7300; * status — validated AND honoured, #7359) diff --git a/packages/runtime/src/flow-dispatch-status.ts b/packages/runtime/src/flow-dispatch-status.ts index 6a72b3bdba..30fd00c5dd 100644 --- a/packages/runtime/src/flow-dispatch-status.ts +++ b/packages/runtime/src/flow-dispatch-status.ts @@ -10,6 +10,25 @@ * | flow disabled | never dispatched | `409` `FLOW_DISABLED` | * | flow has no start node | never dispatched | `422` `FLOW_NO_START_NODE` | * | ran and failed | ran, rejected | `400` `FLOW_FAILED` | + * | ran and PAUSED | ran, suspended | not a refusal — see below | + * + * ## The fifth row is NON-TERMINAL, and it is written down on purpose (#9510) + * + * A run that reaches a pausing node suspends: its continuation is persisted + * (ADR-0019), it answers `{ success: true, status: 'paused', runId }`, and it + * finishes later through `resume()`. It is neither a refusal nor a completed + * run, and it is served as it always has been — the engine result on a `200`, + * carrying the `runId` (and `screen`) a caller continues it with. + * + * It earns a row because it stopped being a single-producer answer. Until #9510 + * only `execute()`'s own catch could pause a triggered run; `executeWithoutRetry` + * had no suspend arm, so a pause on a RETRY attempt was reported as a failed run + * with its continuation silently dropped. With that arm restored the same + * `paused` result now also arrives through `retryExecution` — same shape, second + * producer. A third state reaching a reader that knows two is how one defect + * becomes another, so every reader of this table is TOLD the state exists + * instead of discovering it by falling off the end of + * {@link classifyFlowRefusal}. * * ## Why the table is a module and not a mapper inside one route * @@ -119,9 +138,31 @@ export async function flowIsUnknown(automation: unknown, flowName: string): Prom } /** - * The three result-borne rows: which HTTP answer this engine result declares, - * or `undefined` when the producer classified nothing (see the module note on - * why that is deliberately not a row). + * The table's NON-TERMINAL row: a run that dispatched and then SUSPENDED at a + * pausing node (ADR-0019 durable pause) — `{ success: true, status: 'paused', + * runId }`, with the continuation persisted under that `runId` (#9510). + * + * Read by a door that wants to answer the third state deliberately instead of + * letting it fall through a terminal-shaped branch. It reads the producer's own + * lifecycle verdict — the same field {@link classifyFlowRefusal} reads and the + * same one the engine writes to the run log — never `runId` / `screen` / the + * message, which are incidental to the state. + * + * ⚠️ The ANSWER for a paused run stays each door's own, exactly as the envelope + * is: both trigger doors serve it as today's `200` plus the engine result. What + * is shared, and what this predicate is for, is the QUESTION — so no door can + * read a live pause as a terminal outcome while believing it implements this + * table. + */ +export function isPausedRun(result: AutomationResult | null | undefined): boolean { + return !!result && typeof result === 'object' && result.status === 'paused'; +} + +/** + * The three result-borne REFUSAL rows: which HTTP answer this engine result + * declares, or `undefined` when the producer classified nothing (see the module + * note on why that is deliberately not a row) — and `undefined` for a PAUSED + * run too, which is not a refusal at all (#9510). * * `flowName` is used ONLY to fill a row's default message when the producer * wrote none; it never affects the classification. @@ -130,7 +171,16 @@ export function classifyFlowRefusal( flowName: string, result: AutomationResult | null | undefined, ): FlowRefusal | undefined { - if (!result || typeof result !== 'object' || result.success !== false) return undefined; + if (!result || typeof result !== 'object') return undefined; + // [#9510] A live suspended run is never a refusal, said by naming the state + // rather than by relying on a paused result also being `success: true`. + // ⚠️ Honest about its own weight: this changes no answer today — the + // `success` line below already returns `undefined` for it. What it buys is + // that the non-terminal state is NAMED in the one function every door reads + // the table through, so a future row for unclassified `success: false` + // refusals cannot capture a paused run on its way past. + if (isPausedRun(result)) return undefined; + if (result.success !== false) return undefined; const message = typeof result.error === 'string' && result.error ? result.error : undefined; // ── never dispatched: the producer says WHICH refusal (#9415) ────────── diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index c51fdec4bd..8453289e02 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -3329,7 +3329,23 @@ export class AutomationEngine implements IAutomationService { error: errorMessage, }); - // Error handling strategy + // Error handling strategy. + // + // [#9510] ⚠️ This handoff can now return a NON-TERMINAL result: a + // retry attempt that reaches a pausing node comes back as + // `{ success: true, status: 'paused', runId }` — the same shape the + // suspend arm above returns — and `execute()` forwards it + // unchanged, so its contract is ONE answer per situation rather + // than one per attempt number. Deliberate, not incidental: a caller + // branching on `status` sees no difference between a pause on + // attempt 1 and a pause on attempt 3, which is what + // `AutomationResult` already promises. + // + // ⚠️ The `runId` that comes back is then the PAUSED ATTEMPT's, not + // this failed first attempt's. They are different runs with + // different log rows, and the paused one is the key + // `persistSuspendedRun` stored the continuation under — so it is + // the only id `resume()` can be called with. if (flow.errorHandling?.strategy === 'retry') { return this.retryExecution(flowName, context, startTime, flow.errorHandling, flow.errorMessage); } @@ -6138,6 +6154,17 @@ export class AutomationEngine implements IAutomationService { * Retry execution with exponential backoff, jitter, and recursive protection. * Uses an iterative loop with an internal retry flag to prevent recursive call stacking. * + * ⚠️ [#9510] NOT a terminal-only method. Three results leave it: an attempt + * that succeeded, the exhausted-budget failure at the bottom, and — since + * the ADR-0019 suspend arm was restored to `executeWithoutRetry` — an + * attempt that PAUSED, returned unchanged from inside the loop. The paused + * one is non-terminal: the run is durably suspended and finishes later + * through `resume()`, not here. Its readers are told so explicitly (the + * loop below, `execute()`'s handoff, and the flow-dispatch status table in + * `@objectstack/runtime`) rather than discovering it by falling through a + * `success` check — a non-terminal result reaching a reader that assumes + * terminal is how this defect becomes a different one. + * * Reads the PARSED `errorHandling` block straight — no `??` fallbacks * (#4247). It used to declare every knob optional and re-state a default * for each, and one of those copies disagreed with the schema @@ -6191,6 +6218,29 @@ export class AutomationEngine implements IAutomationService { // Execute directly without recursion into retryExecution again const result = await this.executeWithoutRetry(flowName, context); + // [#9510] THE THIRD STATE, read deliberately and BEFORE `success`. + // Since the ADR-0019 suspend arm was restored to + // `executeWithoutRetry`, an attempt can end NON-TERMINAL: the run + // asked to pause, its continuation is persisted under + // `result.runId`, and it is waiting for a `resume()` that may + // arrive days later. Returning it is the only correct answer — + // re-running the flow would start the paused work a SECOND time + // while the first continuation is still live and resumable, which + // is what the pre-repair loop did on every remaining attempt. + // + // ⛔ Not left to the `result.success` line below, even though a + // paused result is `success: true` and would fall through it today. + // That line means "this attempt SUCCEEDED, stop retrying" — a + // different sentence about a different outcome — and a + // non-terminal result recognised only by a branch that names + // something else is one edit away from being misread again. The + // reason this returns is the PAUSE, so the pause is what it tests. + // + // ⛔ And it weakens no retry accounting: nothing above is skipped, + // reset or shortened, `maxRetries` still bounds the loop, and a + // genuinely failing attempt still consumes one. The loop stops here + // only because this attempt did not fail. + if (result.status === 'paused') return result; if (result.success) return result; lastError = result.error ?? 'Unknown error'; } @@ -6267,6 +6317,13 @@ export class AutomationEngine implements IAutomationService { /** * Execute a flow without triggering retry logic (used by retryExecution to prevent recursion). + * + * [#9510] It exits the way `execute()` exits, and that parity is the + * contract rather than a coincidence: a run that suspends here is a durable + * ADR-0019 pause — continuation persisted, `paused` log row — not a failed + * attempt, because it is the SAME event happening on a later attempt. Two + * answers for one user-visible situation is what this method's missing + * suspend arm produced. */ private async executeWithoutRetry( flowName: string, @@ -6351,6 +6408,87 @@ export class AutomationEngine implements IAutomationService { // work — the same route-dependent shape the fix is removing. return { success: true, output, durationMs, successMessage: flow.successMessage, summary: logged.summary }; } catch (err: unknown) { + // [#9510] A node asked to suspend the run (ADR-0019 durable pause) + // — here, on a RETRY attempt, the only way this method is ever + // reached. Tested FIRST and answered exactly as `execute()`'s own + // catch answers it, because it is the same event: A PAUSE IS NOT A + // FAILURE (`execute()`'s arm says so in those words, and ADR-0019 + // is the contract). A restoration of a stated contract on the one + // path that never got it — not a new capability, and the + // `AutomationResult.status` vocabulary it returns is unchanged. + // + // Without this arm the signal fell through to the generic failure + // path below and four halves of the pause were lost at once: + // + // 1. `persistSuspendedRun` never ran, so THE CONTINUATION WAS + // NEVER STORED and the run could not be resumed by anyone, + // ever — the headline harm, and why the pins for this are + // written against the store and a real `resume()` rather than + // against the status string; + // 2. the run log recorded `failed` for a run that asked to pause; + // 3. the caller got `status: 'failed'`, with the signal + // stringified into `error` (`FlowSuspendSignal` is not an + // `Error`); + // 4. `retryExecution` reads `result.success`, so the pause counted + // as one more failed attempt: the loop burned the rest of the + // budget, and every further attempt re-entered the pausing node + // and orphaned another suspension. + // + // ⛔ Deliberately NOT an authoring-time refusal of + // `errorHandling.strategy: 'retry'` combined with a pausing node. + // That refusal over-refuses (a pausing node can sit on a branch the + // retrying path never reaches), under-refuses (a pausing node + // behind a runtime condition is not statically decidable), and + // would ban the ordinary shape this defect is reached through: a + // flaky HTTP/connector call followed by an `approval` or `screen` + // node — retry is what an author reaches for on the flaky half and + // the approval is what the business needs on the other. + // + // Every field is THIS attempt's own run bookkeeping (`runId`, + // `startedAt`, `steps`, `runContext`, `startTime`), never the + // failed attempt's, so the continuation is keyed by the run id this + // method returns — the only id `resume()` can be called with. + if (isSuspendSignal(err)) { + const durationMs = Date.now() - startTime; + // #7639 — ONE snapshot expression feeding BOTH consumers: the + // continuation the run will resume from, and the `paused` log + // entry run-detail serves. Same object, so what an operator + // reads can never disagree with what the run holds. + const variablesSnapshot = Object.fromEntries(variables); + await this.persistSuspendedRun({ + runId, + flowName, + flowVersion: flow.version, + nodeId: err.nodeId, + nodeType: err.nodeType, + variables: variablesSnapshot, + steps, + context: runContext, + startedAt, + startTime, + correlation: err.correlation, + screen: err.screen, + }); + this.recordLog({ + id: runId, + flowName, + flowVersion: flow.version, + status: 'paused', + startedAt, + durationMs, + trigger: buildRunTrigger(context), + steps, + variables: variablesSnapshot, + }); + return { + success: true, + status: 'paused', + runId, + durationMs, + screen: err.screen, + }; + } + const errorMessage = err instanceof Error ? err.message : String(err); const durationMs = Date.now() - startTime; const logged = this.recordLog({ diff --git a/packages/services/service-automation/src/retry-attempt-pause.test.ts b/packages/services/service-automation/src/retry-attempt-pause.test.ts new file mode 100644 index 0000000000..9a8d6cde45 --- /dev/null +++ b/packages/services/service-automation/src/retry-attempt-pause.test.ts @@ -0,0 +1,346 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #9510 — a retry attempt that PAUSES is a durable pause, not a failed attempt. + * + * `execute()`'s catch tests the suspend signal FIRST, and that arm is what makes + * ADR-0019's durable pause work: it snapshots the live variables, calls + * `persistSuspendedRun`, records a `paused` log entry and returns + * `{ success: true, status: 'paused', runId }`. + * + * `executeWithoutRetry()` — the method `retryExecution` re-runs the flow through + * on every retry attempt — had no such arm, so a `FlowSuspendSignal` thrown on a + * retry attempt fell into the generic failure path and four things were lost at + * once: + * + * 1. `persistSuspendedRun` never ran, so THE CONTINUATION WAS NEVER STORED and + * the run could not be resumed by anyone, ever — the headline harm, and the + * reason the pins below are written against the STORE and a real `resume()` + * rather than against the status string. A repair that flipped `status` to + * `'paused'` and still dropped the continuation would satisfy a + * status-only assertion and leave the defect exactly where it was. + * 2. the run log recorded `failed` for a run that asked to pause; + * 3. the caller got `status: 'failed'` — with the signal stringified into + * `error`, since `FlowSuspendSignal` is not an `Error`; + * 4. `retryExecution` reads `result.success`, so the pause counted as one more + * failed attempt: the loop burned the rest of the budget, and every further + * attempt re-entered the pausing node and orphaned another suspension. + * + * ## Reachability, and why the fixture is shaped the way it is + * + * Only a LATER attempt is exposed — `execute()` handles the first one correctly, + * and a flow reaches `retryExecution` only after a failure. So the fixture is + * the ordinary shape the card names: a flaky call (`flaky`) followed by a + * pausing node (`gate`). `failFirstAttempts` decides which attempt reaches the + * gate, and that single knob is what makes the defect DETERMINISTIC rather than + * timing-dependent: `failFirstAttempts: 1` forces the pause onto attempt 2 every + * run, `failFirstAttempts: 0` puts the identical pause on attempt 1, through + * `execute()`'s own arm. The two are the same user-visible situation reached by + * two routes, which is why they are pinned AGAINST EACH OTHER below rather than + * each in isolation. + * + * The wire half — that both routes reach the trigger door as one answer — is + * pinned end-to-end in `@objectstack/verify`'s + * `automation-trigger-paused-run.test.ts`; the door's own reading of the third + * state is pinned in `@objectstack/runtime`'s + * `automation-trigger-paused-run.test.ts`. + */ + +import { describe, it, expect } from 'vitest'; + +import { AutomationEngine } from './engine.js'; +import { InMemorySuspendedRunStore } from './suspended-run-store.js'; +import { defineActionDescriptor } from '@objectstack/spec/automation'; +import type { AutomationContext, AutomationResult } from '@objectstack/spec/contracts'; + +const silent = { info() {}, warn() {}, error() {}, debug() {}, child: () => silent } as never; + +/** + * `resumeAuthority: 'any'` is required of a pausing fixture since #5561 — these + * tests continue their pause through the public `resume` door. Nothing here is + * about the resume gate (`resume-authority-gate.test.ts` owns that). + */ +const pauser = defineActionDescriptor({ + type: 'gate', + version: '1.0.0', + name: 'gate', + supportsPause: true, + resumeAuthority: 'any', +}); + +interface Harness { + engine: AutomationEngine; + store: InMemorySuspendedRunStore; + /** How many times each node's executor actually ran. */ + calls: { flaky: number; gate: number; after: number }; + trigger(): Promise; +} + +/** + * start -> flaky -> gate (pauses) -> after -> end, under + * `errorHandling.strategy: 'retry'`. + * + * `failFirstAttempts` fails `flaky` on exactly that many leading attempts, so + * the attempt the `gate` is reached on is chosen by the caller, not by timing. + * `afterFails` fails the node BEHIND the pause, which is only ever reached + * through `resume()` — it is how the retry-budget question is measured. + */ +function bootFlow(opts: { + failFirstAttempts: number; + afterFails?: boolean; + maxRetries?: number; +}): Harness { + const store = new InMemorySuspendedRunStore(); + const engine = new AutomationEngine(silent, store); + const calls = { flaky: 0, gate: 0, after: 0 }; + + engine.registerNodeExecutor({ + type: 'flaky', + async execute() { + calls.flaky++; + return calls.flaky <= opts.failFirstAttempts + ? { success: false, error: 'connector 503' } + : { success: true, output: { ok: true } }; + }, + } as never); + + engine.registerNodeExecutor({ + type: 'gate', + descriptor: pauser, + async execute() { + calls.gate++; + return { success: true, suspend: true, correlation: 'approval:req-1' }; + }, + } as never); + + engine.registerNodeExecutor({ + type: 'after', + async execute() { + calls.after++; + return opts.afterFails + ? { success: false, error: 'post-approval write rejected' } + : { success: true, output: { done: true } }; + }, + } as never); + + engine.registerFlow('flaky_approval', { + name: 'flaky_approval', + label: 'flaky_approval', + type: 'autolaunched', + errorHandling: { strategy: 'retry', maxRetries: opts.maxRetries ?? 2, backoffMs: 0 }, + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'flaky', type: 'flaky', label: 'Flaky call' }, + { id: 'gate', type: 'gate', label: 'Approval' }, + { id: 'after', type: 'after', label: 'After approval' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e0', source: 'start', target: 'flaky' }, + { id: 'e1', source: 'flaky', target: 'gate' }, + { id: 'e2', source: 'gate', target: 'after' }, + { id: 'e3', source: 'after', target: 'end' }, + ], + } as never); + + return { + engine, + store, + calls, + trigger: () => engine.execute('flaky_approval', { + event: 'test', + object: 'crm_order', + record: { id: 'ord_9510', amount: 500 }, + } as unknown as AutomationContext), + }; +} + +describe('#9510 — a pause on a RETRY attempt is durable, not a burned attempt', () => { + it('stores the continuation and the run is genuinely resumable — the durable half', async () => { + const h = bootFlow({ failFirstAttempts: 1 }); + + const res = await h.trigger(); + + // Attempt 1 failed at `flaky`; attempt 2 got past it and paused at `gate`. + expect(h.calls.flaky).toBe(2); + expect(h.calls.gate).toBe(1); + expect(res.status).toBe('paused'); + expect(res.runId).toBeDefined(); + + // ⭐ THE assertion this card is about: the continuation EXISTS. Before + // the repair nothing was written here at all, so the run was + // unresumable for good — no status string, message or duration says + // that, which is why the pin is on the store. + const stored = await h.store.load(res.runId as string); + expect(stored).not.toBeNull(); + expect(stored?.runId).toBe(res.runId); + expect(stored?.flowName).toBe('flaky_approval'); + expect(stored?.nodeId).toBe('gate'); + // The resume gate's key (#3801) — a continuation without it is resumable + // only by falling back to the live flow definition. + expect(stored?.nodeType).toBe('gate'); + expect(stored?.correlation).toBe('approval:req-1'); + // The snapshot is the RUN's own state, not an empty husk: what the + // pre-pause node actually produced is in it, under `.`. + expect(stored?.variables?.['flaky.ok']).toBe(true); + expect(stored?.variables?.$record).toEqual({ id: 'ord_9510', amount: 500 }); + + // …and "resumable" asserted by DOING it, not by inspecting a row: the + // run continues from `gate`'s out-edge and reaches `end`. + const resumed = await h.engine.resume(res.runId as string, { output: { verdict: 'approved' } } as never); + expect(resumed.success).toBe(true); + expect(resumed.status).not.toBe('paused'); + expect(h.calls.after).toBe(1); + // The suspension is consumed, exactly as a first-attempt pause's is. + expect(await h.store.load(res.runId as string)).toBeNull(); + }); + + it('records the run log as paused, never failed', async () => { + const h = bootFlow({ failFirstAttempts: 1 }); + + const res = await h.trigger(); + + // A run log that says a paused run failed is a second lie on the same + // event: the run is alive, parked and waiting for a human. + const run = await h.engine.getRun(res.runId as string); + expect(run?.status).toBe('paused'); + expect(run?.status).not.toBe('failed'); + // #7639 — ONE snapshot expression feeds both consumers, on this path + // too: what an operator reads on run-detail is by construction the + // state the continuation will resume from, never a second capture. + const stored = await h.store.load(res.runId as string); + expect(run?.variables).toEqual(stored?.variables); + }); + + it('stops the retry loop as a CONSEQUENCE of the pause — the accounting is untouched', async () => { + const h = bootFlow({ failFirstAttempts: 1, maxRetries: 2 }); + + const res = await h.trigger(); + + expect(res.status).toBe('paused'); + // The budget allowed 1 initial attempt + 2 retries = 3 runs of `flaky`. + // Only 2 happened, and the pausing node was entered exactly ONCE: the + // loop stopped because the attempt did not fail, not because anything + // stopped counting. Re-running the flow here would have started the + // approval a second time while the first continuation was still live — + // the pre-repair behaviour, which orphaned one suspension per attempt. + expect(h.calls.flaky).toBe(2); + expect(h.calls.gate).toBe(1); + }); + + it('still burns the full budget when attempts genuinely FAIL — the loop was not weakened', async () => { + // The guard against "make the loop stop" being implemented by deleting + // retry accounting: with the gate never reached, the count is the #4247 + // contract exactly — maxRetries + 1 attempts. + const h = bootFlow({ failFirstAttempts: 99, maxRetries: 2 }); + + const res = await h.trigger(); + + expect(res.success).toBe(false); + expect(res.status).toBe('failed'); + expect(h.calls.flaky).toBe(3); + expect(h.calls.gate).toBe(0); + }); + + it('answers a pause on attempt 2 exactly as it answers one on attempt 1 — the two producers pinned against each other', async () => { + // Same flow, same pausing node, same trigger. The ONLY difference is + // which attempt reaches the gate — i.e. which engine arm produced the + // pause: `execute()`'s catch, or the one restored to + // `executeWithoutRetry`. One user-visible situation must not have two + // answers, so the two results are compared to EACH OTHER rather than + // each to a hand-written expectation. + const first = bootFlow({ failFirstAttempts: 0 }); + const retry = bootFlow({ failFirstAttempts: 1 }); + + const viaExecute = await first.trigger(); + const viaRetry = await retry.trigger(); + + // Same producer arm reached by two routes: same keys, in the same shape. + expect(Object.keys(viaRetry).sort()).toEqual(Object.keys(viaExecute).sort()); + // …and the same values, except the two that are per-run by nature. + const shape = (r: AutomationResult) => ({ ...r, runId: '', durationMs: 0 }); + expect(shape(viaRetry)).toEqual(shape(viaExecute)); + expect(viaRetry.success).toBe(true); + expect(viaRetry.status).toBe('paused'); + + // The stored continuations match too — a matching return value over a + // missing continuation is the failure this card is about. + const storedViaExecute = await first.store.load(viaExecute.runId as string); + const storedViaRetry = await retry.store.load(viaRetry.runId as string); + expect(storedViaRetry).not.toBeNull(); + expect(Object.keys(storedViaRetry ?? {}).sort()).toEqual(Object.keys(storedViaExecute ?? {}).sort()); + expect(storedViaRetry?.nodeId).toBe(storedViaExecute?.nodeId); + expect(storedViaRetry?.nodeType).toBe(storedViaExecute?.nodeType); + expect(storedViaRetry?.correlation).toBe(storedViaExecute?.correlation); + + // ⚠️ The one thing that does NOT match, asserted rather than skirted. + // `executeWithoutRetry` seeds none of the engine-owned variables + // `execute()` binds (`$runId`, `$flowName`, `$flowLabel`, `record` and + // its flattened fields, `previous`), so a retry attempt has always run + // in a smaller environment than the first — filed as #9704, a divergent + // run environment rather than a lost pause, and out of scope here: it + // afflicts every retry attempt, pausing or not, and predates this card. + // + // It is pinned as TODAY's measured behaviour, deliberately, so #9704 + // cannot be repaired silently. When it is repaired these three + // assertions are the ones that go red, and the correct edit is to + // delete them and add `variables` to the parity block above. + const engineOwned = ['$runId', '$flowName', '$flowLabel', 'previous', 'record']; + for (const name of engineOwned) { + expect(Object.keys(storedViaExecute?.variables ?? {})).toContain(name); + expect(Object.keys(storedViaRetry?.variables ?? {})).not.toContain(name); + } + // What the two DO share: the run's own work. Both snapshots carry the + // pausing node's inputs, so the continuation is a real continuation on + // either route — the half this card is about. + expect(storedViaRetry?.variables?.['flaky.ok']).toEqual(storedViaExecute?.variables?.['flaky.ok']); + expect(storedViaRetry?.variables?.$record).toEqual(storedViaExecute?.variables?.$record); + + // The run LOG agrees on both routes as well. + expect((await retry.engine.getRun(viaRetry.runId as string))?.status) + .toBe((await first.engine.getRun(viaExecute.runId as string))?.status); + }); + + /** + * ⭐ The retry-budget question the ruling required to be ANSWERED rather + * than assumed, measured rather than reasoned: + * + * **A resumed run gets NO retries — on either route.** + * + * Two independent facts produce that answer, both read off `origin/main`: + * + * - `SuspendedRun` (engine.ts) declares no attempt/retry field of any kind, + * so the continuation CANNOT carry attempt state; and + * - `resumeInternal` never reads `flow.errorHandling` and never enters + * `retryExecution`, so a resumed run that fails is terminal. + * + * So the answer is neither "inherits the remaining attempts" nor "starts + * fresh": the retry budget does not survive a pause at all. This is + * PRE-EXISTING behaviour of every paused run, not something this card + * introduces — which is exactly why lifting the arm is safe here: the + * retry-path pause inherits the same answer the execute-path pause has + * always had, and the two stay consistent. + * + * That the resume path ignores a flow's declared retry policy is a real + * gap, but a DIFFERENT one, on a different method; it is filed as #9705 + * rather than absorbed here. What is pinned below is today's measured + * answer, so whatever that card decides is a deliberate change and not an + * accident. + */ + it('answers the retry-budget question: a resumed run does not retry, on either route', async () => { + for (const failFirstAttempts of [0, 1]) { + const h = bootFlow({ failFirstAttempts, afterFails: true, maxRetries: 2 }); + + const paused = await h.trigger(); + expect(paused.status).toBe('paused'); + + const resumed = await h.engine.resume(paused.runId as string, { output: { verdict: 'approved' } } as never); + + // The post-pause node rejected. The run is terminally failed and + // the node ran ONCE: no attempt was inherited and none was granted + // fresh, because `resumeInternal` has no retry path at all. + expect(resumed.success).toBe(false); + expect(h.calls.after).toBe(1); + } + }); +}); diff --git a/packages/verify/src/automation-trigger-paused-run.test.ts b/packages/verify/src/automation-trigger-paused-run.test.ts new file mode 100644 index 0000000000..f3c5020fb4 --- /dev/null +++ b/packages/verify/src/automation-trigger-paused-run.test.ts @@ -0,0 +1,206 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #9510 — a run that pauses on a RETRY attempt reaches the wire as a live, + * resumable pause, and as the SAME answer a first-attempt pause gets. + * + * This is the end-to-end half, and it lives here because `@objectstack/verify` + * is the one package depending on BOTH `@objectstack/runtime` (the trigger and + * resume doors) and `@objectstack/service-automation` (the engine that produces + * the result) — the same reason #9414's `automation-trigger-terminal-messages` + * suite sits beside it. The engine-side pins are in + * `packages/services/service-automation/src/retry-attempt-pause.test.ts`; the + * door-side pins, driven with scripted results, are in + * `packages/runtime/src/domains/automation-trigger-paused-run.test.ts`. Neither + * of those, alone or together, can make the sentence this file makes: the two + * ENGINE PRODUCERS reach the door as one wire answer, and the run a caller is + * handed can actually be continued. + * + * **The requirement is an equality, so it is written as one.** The ruling on + * this card is explicit that the trigger route's answer for a paused retry must + * match what `execute()`'s paused return already produces, *pinned against it + * rather than verified in isolation* — because two paths answering differently + * for one user-visible situation would replace a LOST pause with an inconsistent + * one. So the two responses below are compared to EACH OTHER, and the only + * fields normalised away are the two that are per-run by nature. + * + * **And a matching answer is not the whole contract.** The headline harm was + * that `persistSuspendedRun` never ran, so the continuation was never stored and + * the run could not be resumed by anyone. A repair that returned the right JSON + * over a missing continuation would satisfy an equality and leave that intact — + * so the paused-on-retry run is also CONTINUED here, through the real + * `POST /:name/runs/:runId/resume` door, and the run log is read back through + * the real run-detail door. + * + * ⚠️ This suite resolves both packages through their BUILT `dist/`, as every + * dependent of theirs in this workspace does. Rebuild + * `@objectstack/service-automation` and `@objectstack/runtime` before trusting a + * run of this file — and especially before trusting an ABLATED one, where a + * stale `dist` would run the pre-mutation code and report green over a mutation + * that never reached the artifact. + */ + +import { describe, it, expect } from 'vitest'; + +import { HttpDispatcher } from '@objectstack/runtime'; +import { AutomationEngine, InMemorySuspendedRunStore } from '@objectstack/service-automation'; +import { defineActionDescriptor } from '@objectstack/spec/automation'; + +const CTX = { request: {}, executionContext: { userId: 'user_1' } } as never; + +function createTestLogger(): never { + const logger = { info: () => {}, warn: () => {}, error: () => {}, debug: () => {}, child: () => logger }; + return logger as never; +} + +/** + * `resumeAuthority: 'any'` because this suite continues its pause through the + * PUBLIC resume door; since #5561 a node type declaring none is gated shut. + */ +const gate = defineActionDescriptor({ + type: 'gate', + version: '1.0.0', + name: 'gate', + supportsPause: true, + resumeAuthority: 'any', +}); + +/** + * start -> flaky -> gate (pauses) -> after -> end, under + * `errorHandling.strategy: 'retry'` — the ordinary shape the card names: a + * flaky connector call followed by an approval. + * + * `failFirstAttempts` alone decides WHICH engine arm produces the pause, which + * is what makes the comparison below an apples-to-apples one: `0` pauses on + * attempt 1 through `execute()`'s own catch, `1` pauses on attempt 2 through the + * arm restored to `executeWithoutRetry`. Nothing else differs. + */ +function boot(failFirstAttempts: number): HttpDispatcher { + const engine = new AutomationEngine(createTestLogger(), new InMemorySuspendedRunStore()); + let flakyCalls = 0; + + engine.registerNodeExecutor({ + type: 'flaky', + async execute() { + flakyCalls++; + return flakyCalls <= failFirstAttempts + ? { success: false, error: 'connector 503' } + : { success: true, output: { ok: true } }; + }, + } as never); + engine.registerNodeExecutor({ + type: 'gate', + descriptor: gate, + async execute() { + return { success: true, suspend: true, correlation: 'approval:req-1' }; + }, + } as never); + engine.registerNodeExecutor({ + type: 'after', + async execute() { + return { success: true, output: { booked: true } }; + }, + } as never); + + engine.registerFlow('flaky_approval', { + name: 'flaky_approval', + label: 'flaky_approval', + type: 'autolaunched', + errorHandling: { strategy: 'retry', maxRetries: 2, backoffMs: 0 }, + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'flaky', type: 'flaky', label: 'Flaky call' }, + { id: 'gate', type: 'gate', label: 'Approval' }, + { id: 'after', type: 'after', label: 'After approval' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e0', source: 'start', target: 'flaky' }, + { id: 'e1', source: 'flaky', target: 'gate' }, + { id: 'e2', source: 'gate', target: 'after' }, + { id: 'e3', source: 'after', target: 'end' }, + ], + } as never); + + const services: Record = { automation: engine }; + const resolve = (name: string): unknown => services[name]; + const kernel = { + getService: resolve, + getServiceAsync: async (name: string): Promise => resolve(name), + context: { getService: resolve }, + }; + return new HttpDispatcher(kernel as never); +} + +const trigger = (d: HttpDispatcher) => d.handleAutomation('/flaky_approval/trigger', 'POST', {}, CTX); + +/** The two fields that are per-run by nature and cannot be compared literally. */ +function normalise(body: any) { + return { ...body, data: { ...body?.data, runId: '', durationMs: 0 } }; +} + +describe('#9510 — a paused retry attempt reaches the wire as a live, resumable pause', () => { + it('answers 200 with the runId a caller resumes with, when the pause lands on attempt 2', async () => { + const dispatcher = boot(1); + + const res = await trigger(dispatcher); + + expect(res.response?.status).toBe(200); + expect(res.response?.body?.success).toBe(true); + // Before the repair this same dispatch answered 400 FLOW_FAILED with + // the suspend signal stringified into the message, and no run id at + // all — the pause, and the run, were simply gone. + expect(res.response?.body?.data?.status).toBe('paused'); + expect(typeof res.response?.body?.data?.runId).toBe('string'); + expect(res.response?.body?.error).toBeUndefined(); + }); + + it('answers a pause on attempt 2 EXACTLY as it answers one on attempt 1', async () => { + const viaExecute = await trigger(boot(0)); + const viaRetry = await trigger(boot(1)); + + expect(viaRetry.response?.status).toBe(viaExecute.response?.status); + // The whole body, compared to the other path's body rather than to a + // hand-written expectation: one user-visible situation, one answer, and + // no way for a caller to tell which attempt paused. + expect(normalise(viaRetry.response?.body)).toEqual(normalise(viaExecute.response?.body)); + }); + + it('hands back a run that can really be CONTINUED — the durable half, at the wire', async () => { + const dispatcher = boot(1); + const paused = await trigger(dispatcher); + const runId = paused.response?.body?.data?.runId as string; + + // The real resume door, exactly as a console or the SDK reaches it. + // This is the assertion a status-string-only repair could not pass: + // with no continuation stored, the engine answers RUN_NOT_FOUND and + // this door returns 404. + const resumed = await dispatcher.handleAutomation( + `/flaky_approval/runs/${runId}/resume`, + 'POST', + { output: { verdict: 'approved' } }, + CTX, + ); + + expect(resumed.response?.status).toBe(200); + expect(resumed.response?.body?.success).toBe(true); + // The run continued past the gate and finished, rather than re-pausing + // or reporting a stale suspension. + expect(resumed.response?.body?.data?.status).not.toBe('paused'); + expect(resumed.response?.body?.data?.success).toBe(true); + }); + + it('serves the run log as paused, not failed, on run-detail', async () => { + const dispatcher = boot(1); + const paused = await trigger(dispatcher); + const runId = paused.response?.body?.data?.runId as string; + + const detail = await dispatcher.handleAutomation(`/flaky_approval/runs/${runId}`, 'GET', undefined, CTX); + + expect(detail.response?.status).toBe(200); + // A run log that says a paused run failed is a second lie on the same + // event — the operator surface would show a dead run that is in fact + // parked and waiting for a human. + expect(detail.response?.body?.data?.status).toBe('paused'); + }); +});