diff --git a/.changeset/pause-ends-retry-segment.md b/.changeset/pause-ends-retry-segment.md new file mode 100644 index 0000000000..1486b04e12 --- /dev/null +++ b/.changeset/pause-ends-retry-segment.md @@ -0,0 +1,20 @@ +--- +'@objectstack/spec': patch +--- + +Document the retry/durable-pause boundary on a flow's `errorHandling` block: a durable +pause (`approval`, `screen`, `wait` — ADR-0019) **ends the retry-governed segment**. +`errorHandling.strategy: 'retry'` describes one synchronous dispatch, so a run that pauses +and later resumes gets exactly one attempt for anything that fails after the pause. + +Prose only — no validation change. The accepted flow set is unchanged and every flow that +parsed before parses identically; what changes is that the boundary is now stated where an +author meets it (the `errorHandling` and `strategy` `describe()` text, which is what the +generated reference tables render) instead of having to be inferred from engine behaviour. + +The boundary is deliberate rather than a gap: the retry knobs (`backoffMs`, +`backoffMultiplier`, `jitter`) model an in-process loop, which a pause of arbitrary +duration is not, and the durable continuation carries no attempt counter. To protect the +half of a flow that runs after a pause, give that half its own failure handling in the +flow — a `try_catch` node with its own `retry` around the post-resume work, or a `fault` +edge to a handler node. `content/docs/automation/flows.mdx` carries the recipe. diff --git a/content/docs/automation/approvals.mdx b/content/docs/automation/approvals.mdx index 8dc18227b2..ffe7a2763f 100644 --- a/content/docs/automation/approvals.mdx +++ b/content/docs/automation/approvals.mdx @@ -459,6 +459,18 @@ what `sys_approval_request.status` offers and what `ApprovalStatus` is derived from. If you add a status there, this line is a fourth copy that nothing updates for you. + + **The resumed half is not covered by the flow's `errorHandling.retry`.** A + durable pause ends the retry-governed segment, so `strategy: 'retry'` on the + flow protects the work *before* the approval and gives whatever fails after the + decision exactly one attempt. Wrap the post-decision work in a `try_catch` node + with its own `retry` (or draw a `fault` edge to a handler) — see [A durable + pause ends the retry-governed + segment](/docs/automation/flows#retry-pause-boundary). It is also the shape you + want regardless: a flow-level retry re-runs the flow **from the start**, which + would open a second approval request. + + ### Beyond approve/reject — the full decision surface diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index 09a71bc70e..3ae6dba6de 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -490,6 +490,14 @@ POST /api/v1/automation/{flow}/runs/{runId}/resume | `wait` (timer) | an ISO-8601 duration elapses | **automatically** — a one-shot job resumes the run; after a cold boot the engine re-arms pending timers from the durable store (overdue timers resume immediately) | | `wait` (signal) | a named external event | any caller invoking `resume(runId)` | + + **A pause ends the flow's retry-governed segment.** `errorHandling: { strategy: + 'retry' }` covers one synchronous dispatch, so a failure *after* the run + resumes is not retried — the post-pause half needs its own failure handling in + the flow. See [A durable pause ends the retry-governed + segment](#retry-pause-boundary). + + ### Who may resume — the gate is the suspended node The resume route is generic, so **the node the run is parked on** decides @@ -1159,6 +1167,67 @@ is fine and simply ignored. ambiguous is now rejected instead of guessed. +### A durable pause ends the retry-governed segment [#retry-pause-boundary] + +`strategy: 'retry'` governs **one synchronous dispatch**. When a run parks on a +node that pauses durably — `approval`, `screen`, `wait` +([ADR-0019](#durable-pause--resume-adr-0019)) — the pause **ends the +retry-governed segment**. The continuation that resumes later, possibly days +later and in a different process, is a new segment outside it: whatever fails +after the resume gets exactly **one** attempt. + +So in the ordinary shape — a flaky callout, then an approval, then the work that +follows the decision — the two halves of the flow are not protected alike: + +| Where the failure happens | Under `errorHandling: { strategy: 'retry', maxRetries: 3 }` | +| :--- | :--- | +| before the run pauses | retried — the whole flow re-runs, up to 3 more times | +| after the run resumes | **not retried** — one attempt, then the run fails | + + + This is the contract, not a gap waiting to be filled. The retry knobs describe + an **in-process loop**: `backoffMs` and `backoffMultiplier` are delays the + engine sleeps through and `jitter` spreads a thundering herd — none of which a + pause of arbitrary duration can honestly extend across, and the persisted + continuation carries no attempt counter for the same reason. Having the resume + inherit the remaining budget is a recorded revisit path, not a planned change; + it would need a new field on the durable snapshot and an answer for a flow + republished mid-pause with a different `maxRetries`. + + +#### Protecting the half that runs after the pause + +Flow-level `retry` cannot reach that half, so give it its own failure handling +**inside the flow** — both mechanisms already documented above work after a +resume, because both are per-node and local to the segment that is running: + +- a **`try_catch` node** wrapping the post-resume work, with its own `retry` + block — the direct replacement for the flow-level budget, and the one to reach + for when the post-decision step is a flaky callout; +- a **`fault` edge** from the post-resume node to a handler node, when one + failure has a specific recovery rather than a re-run. + +```typescript +// approve branch: the post-decision callout carries its own retry, so it is +// protected whether the run reached it on attempt 1 or after a two-day approval. +{ + id: 'post_approval', + type: 'try_catch', + label: 'Provision after approval', + config: { + try: { nodes: [{ id: 'provision', type: 'http', label: 'Provision', config: { /* … */ } }], edges: [] }, + catch: { nodes: [{ id: 'flag', type: 'update_record', label: 'Flag failure', config: { /* … */ } }], edges: [] }, + errorVariable: '$error', + retry: { maxRetries: 3, backoffMs: 1000, backoffMultiplier: 2 }, + }, +} +``` + +Note this is not a workaround for the boundary — it is the better shape anyway: +flow-level `retry` replays the flow **from the start**, which after an approval +would mean opening a second approval request. A `try_catch` retries only the +region that failed. + ## Discovery & Registration You almost never call `engine.registerFlow()` directly. The diff --git a/content/docs/references/api/automation-api.mdx b/content/docs/references/api/automation-api.mdx index 1a1bdfbaa7..3726e211b1 100644 --- a/content/docs/references/api/automation-api.mdx +++ b/content/docs/references/api/automation-api.mdx @@ -99,7 +99,7 @@ const result = AutomationApiErrorCode.parse(data); | **edges** | `{ id: string; source: string; target: string; condition?: string \| object; … }[]` | ✅ | Flow connections | | **active** | `never` | optional | [REMOVED] `flow.active` was removed in @objectstack/spec 17.0.0 (#3896 audit close-out) — it never had an effect: the engine arms flows from `status`, and `active: false` did NOT stop a flow (worse, the default read as disabled while the engine treated unset as enabled). Delete the key. Use `status: 'obsolete'` (or 'invalid') to unbind and disable a flow, `status: 'active'` to arm it. Run `os migrate meta --from 16` to list the mechanical edits for existing sources; apply them by hand. | | **runAs** | `Enum<'system' \| 'user'>` | optional (default: `"user"`) | Execution identity for the run: system = elevated (bypasses RLS), user = the triggering user (RLS-respecting). A run with no trigger user has no identity to scope to, so under user its data operations are REFUSED — declare system to make the elevation explicit. This covers schedule/time-relative/api triggers AND any record-change flow fired by a write that carried no user. | -| **errorHandling** | `{ strategy?: Enum<'fail' \| 'retry' \| 'continue'>; maxRetries?: integer; backoffMs?: integer; backoffMultiplier?: number; … }` | optional | Flow-level error handling configuration | +| **errorHandling** | `{ strategy?: Enum<'fail' \| 'retry' \| 'continue'>; maxRetries?: integer; backoffMs?: integer; backoffMultiplier?: number; … }` | optional | Flow-level error handling configuration. A durable pause ends the retry-governed segment: strategy: 'retry' describes one synchronous dispatch, so a run that parks on an approval/screen/wait node and later resumes gets one attempt for anything that fails after the pause. Protect the post-pause half with its own failure handling in the flow — a try_catch node's retry around the post-resume work, or fault edges to a handler node. | | **protection** | `{ lock: Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>; reason: string; docsUrl?: string }` | optional | Package author protection block — lock policy for this flow. | | **_lock** | `Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>` | optional | Item-level lock — controls overlay & delete (ADR-0010). | | **_lockReason** | `string` | optional | Human-readable reason shown when a write is refused by _lock. | diff --git a/content/docs/references/automation/flow.mdx b/content/docs/references/automation/flow.mdx index e6e2b34aa1..f2a9f502e1 100644 --- a/content/docs/references/automation/flow.mdx +++ b/content/docs/references/automation/flow.mdx @@ -53,7 +53,7 @@ const result = FlowSchema.parse(data); | **edges** | `{ id: string; source: string; target: string; condition?: string \| object; … }[]` | ✅ | Flow connections | | **active** | `never` | optional | [REMOVED] `flow.active` was removed in @objectstack/spec 17.0.0 (#3896 audit close-out) — it never had an effect: the engine arms flows from `status`, and `active: false` did NOT stop a flow (worse, the default read as disabled while the engine treated unset as enabled). Delete the key. Use `status: 'obsolete'` (or 'invalid') to unbind and disable a flow, `status: 'active'` to arm it. Run `os migrate meta --from 16` to list the mechanical edits for existing sources; apply them by hand. | | **runAs** | `Enum<'system' \| 'user'>` | optional (default: `"user"`) | Execution identity for the run: system = elevated (bypasses RLS), user = the triggering user (RLS-respecting). A run with no trigger user has no identity to scope to, so under user its data operations are REFUSED — declare system to make the elevation explicit. This covers schedule/time-relative/api triggers AND any record-change flow fired by a write that carried no user. | -| **errorHandling** | `{ strategy?: Enum<'fail' \| 'retry' \| 'continue'>; maxRetries?: integer; backoffMs?: integer; backoffMultiplier?: number; … }` | optional | Flow-level error handling configuration | +| **errorHandling** | `{ strategy?: Enum<'fail' \| 'retry' \| 'continue'>; maxRetries?: integer; backoffMs?: integer; backoffMultiplier?: number; … }` | optional | Flow-level error handling configuration. A durable pause ends the retry-governed segment: strategy: 'retry' describes one synchronous dispatch, so a run that parks on an approval/screen/wait node and later resumes gets one attempt for anything that fails after the pause. Protect the post-pause half with its own failure handling in the flow — a try_catch node's retry around the post-resume work, or fault edges to a handler node. | | **protection** | `{ lock: Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>; reason: string; docsUrl?: string }` | optional | Package author protection block — lock policy for this flow. | | **_lock** | `Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>` | optional | Item-level lock — controls overlay & delete (ADR-0010). | | **_lockReason** | `string` | optional | Human-readable reason shown when a write is refused by _lock. | diff --git a/packages/services/service-automation/src/retry-attempt-pause.test.ts b/packages/services/service-automation/src/retry-attempt-pause.test.ts index 4ad629c8ee..d7e37f7f99 100644 --- a/packages/services/service-automation/src/retry-attempt-pause.test.ts +++ b/packages/services/service-automation/src/retry-attempt-pause.test.ts @@ -329,10 +329,26 @@ describe('#9510 — a pause on a RETRY attempt is durable, not a burned attempt' }); /** - * ⭐ The retry-budget question the ruling required to be ANSWERED rather - * than assumed, measured rather than reasoned: + * ⭐ THE RULED CONTRACT — not merely "current behaviour" (#9705). * - * **A resumed run gets NO retries — on either route.** + * **A durable pause ENDS the retry-governed segment: a resumed run gets + * NO retries — on either route.** + * + * Maintainer ruling recorded 2026-08-18 on #9705 (Option A): + * `errorHandling.strategy: 'retry'` describes ONE synchronous dispatch, and + * a resumed run is a new segment outside it. So the assertions below are a + * CONTRACT PIN, not a snapshot of an accident — **changing them is a + * contract change** and needs its own ruling, not a test fix. The boundary + * is stated for authors on the `errorHandling` block's `describe()` in + * `packages/spec/src/automation/flow.zod.ts` and in + * `content/docs/automation/flows.mdx` ("A durable pause ends the + * retry-governed segment"), with the authoring recipe for protecting the + * post-pause half. Option B (resume inherits the remaining budget) is the + * recorded revisit path only — it needs a `SuspendedRun` field, a store + * migration and a re-published-flow answer, none of which is bought today. + * + * The question was required to be ANSWERED rather than assumed, and it was + * measured rather than reasoned: * * Two independent facts produce that answer, both read off `origin/main`: * @@ -348,13 +364,14 @@ describe('#9510 — a pause on a RETRY attempt is durable, not a burned attempt' * 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. + * That the resume path ignores a flow's declared retry policy was filed as + * a separate card (#9705) rather than absorbed here, and that card is where + * the ruling above landed: the behaviour pinned below is the intended + * contract, and the retry knobs (`backoffMs`, `backoffMultiplier`, + * `jitter`) model an in-process loop that a pause of arbitrary duration + * cannot honestly extend across. */ - it('answers the retry-budget question: a resumed run does not retry, on either route', async () => { + it('pins the RULED contract — a durable pause ends the retry-governed segment: a resumed run does not retry, on either route', async () => { for (const failFirstAttempts of [0, 1]) { const h = bootFlow({ failFirstAttempts, afterFails: true, maxRetries: 2 }); diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index 8c03292995..3d6bfa3e53 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -717,6 +717,29 @@ export const FlowSchema = lazySchema(() => strictObject( * The retry knobs are read **only** under `strategy: 'retry'`; `'fail'` and * `'continue'` ignore them (a fully spelled-out block under `'fail'` is * common and stays legal). + * + * **A durable pause ENDS the retry-governed segment** — ruled deliberate, + * not an omission. `strategy: 'retry'` describes ONE synchronous dispatch; + * when a run parks on an `approval`, `screen` or `wait` node (ADR-0019) the + * continuation is a new segment outside it, and a failure after the resume + * gets exactly one attempt. Measured, both directions: `SuspendedRun` + * carries no attempt state (nothing to inherit across the pause) and the + * resume path never consults this block (nothing would read it if it did). + * The alternative — resume inherits the remaining budget — is the recorded + * revisit path, not the contract: it would need a field on the persisted + * snapshot plus an answer for a flow republished mid-pause with a different + * `maxRetries`, and no deployment has asked for it. It is also what the + * knobs can honestly promise: `backoffMs`/`backoffMultiplier`/`jitter` + * model an in-process loop, which a pause of arbitrary duration is not. + * + * The authoring consequence is the part that belongs to the author, so it + * is stated on the block's own `.describe()` too: to protect the half of a + * flow that runs AFTER the pause, give that half its own failure handling + * in the flow — a `try_catch` node with its own `retry` around the + * post-resume work, or per-node `fault` edges to a handler. The pin for + * this behaviour is + * `packages/services/service-automation/src/retry-attempt-pause.test.ts`; + * changing it is a contract change, not a test fix. */ errorHandling: strictObject({ surface: "this flow's `errorHandling` block", @@ -766,7 +789,7 @@ export const FlowSchema = lazySchema(() => strictObject( 'Since #4964 the retry keys are the converged `RetryPolicySchema` contract, so a spelling ' + 'learned on `job.retryPolicy` or a `try_catch` node\'s `retry` is correct here too.', }, { - strategy: z.enum(['fail', 'retry', 'continue']).default('fail').describe('How to handle node execution errors'), + strategy: z.enum(['fail', 'retry', 'continue']).default('fail').describe("How to handle node execution errors. 'retry' governs ONE synchronous dispatch: a durable pause (approval/screen/wait) ends the retry-governed segment, so a failure after the run resumes is not retried."), // ── The retry policy itself: ONE declaration (#4964) ──────────────── // `maxRetries` / `backoffMs` / `backoffMultiplier` / `maxRetryDelayMs` / @@ -825,7 +848,7 @@ export const FlowSchema = lazySchema(() => strictObject( 'Note a retry re-runs the WHOLE flow, so side-effecting nodes run again.', }); } - }).optional().describe('Flow-level error handling configuration'), + }).optional().describe("Flow-level error handling configuration. A durable pause ends the retry-governed segment: strategy: 'retry' describes one synchronous dispatch, so a run that parks on an approval/screen/wait node and later resumes gets one attempt for anything that fails after the pause. Protect the post-pause half with its own failure handling in the flow — a try_catch node's retry around the post-resume work, or fault edges to a handler node."), /** * ADR-0010 §3.7 — Package-level protection envelope. Package * authors declare lock policy here; the loader translates it