fix(service-automation): a retry attempt that PAUSES is a durable pause, not a failed attempt — executeWithoutRetry gets the ADR-0019 suspend arm (#9510) (#9785)

* fix(service-automation): a retry attempt that PAUSES is a durable pause, not a failed attempt (#9510)

`execute()`'s catch tests the suspend signal first, and that arm is what makes
ADR-0019's durable pause work. `executeWithoutRetry()` — which `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: `persistSuspendedRun` never ran, so the continuation was never stored and
the run could not be resumed by anyone; the run log recorded `failed`; the
caller got `status: 'failed'`; and the retry loop, reading only
`result.success`, counted the pause as one more failed attempt and burned the
rest of the budget re-entering the pausing node.

Lifts the suspend arm into `executeWithoutRetry`, and teaches both readers of
the now non-terminal `retryExecution` result the third state deliberately: the
retry loop returns a paused attempt because it paused, tested on `status`
before the `success` check that means "this attempt succeeded"; the trigger
route answers it from its own arm, off a named predicate on the shared
flow-dispatch table. Retry accounting is untouched — a failing attempt still
consumes one and `maxRetries` still bounds the loop.

Both routes to a pause are pinned as an equality rather than verified in
isolation, engine-side and end-to-end through a real dispatcher, so no caller
can tell which attempt paused. Runs already lost to this defect are not
recoverable: nothing was ever written for them.

Refs #9414 / PR #9514 (the sibling repair on the same three methods),
ADR-0019. Findings filed out of this work: #9704, #9705.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y26DJEHSBhhAQ6wwfsHNza

* test(runtime): make the isPausedRun pin able to fail — separate the verdict from the companion field

The reverse verification caught this: replacing `isPausedRun`'s body with
`!!result.screen` — the tolerant-consumer sniff PD #12 forbids — left the whole
runtime suite green. Every assertion in the pin was satisfied by it, because a
screen happened to accompany the paused fixture and to be absent from all the
negatives, so the test could not fail against the one shape it exists to forbid.

Adds the two cases that separate the readings: an `approval`/`wait` pause with
NO screen is still a pause, and a screen on a result that is not parked does not
make it one. Both go red under that substitution.

Also states plainly, at the route-level case, that this door answers a pause and
a terminal success identically on the wire by design, so no route assertion can
tell which arm produced the response — the discrimination lives on the predicate.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y26DJEHSBhhAQ6wwfsHNza

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-project-manager

.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.

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<AutomationResult> => result);
+    const getFlow = vi.fn(async (name: string) => flows.get(name) ?? null);
+    const services: Record<string, unknown> = { 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);
+    });
+});

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)