diff --git a/docs/decisions/0038-agentrunner-llm-call-boundary.md b/docs/decisions/0038-agentrunner-llm-call-boundary.md index df2dfdfc..9f1bc1a0 100644 --- a/docs/decisions/0038-agentrunner-llm-call-boundary.md +++ b/docs/decisions/0038-agentrunner-llm-call-boundary.md @@ -2,7 +2,14 @@ - **Status**: Accepted - **Date**: 2026-06-14 -- **Related**: [ADR-0006](0006-os-keychain-for-api-keys.md), [ADR-0011](0011-internal-llm-abstraction.md), [ADR-0018](0018-desktop-execution-and-rust-egress.md), [ADR-0019](0019-cli-node-keychain-library.md), [ADR-0024](0024-agent-first-entry-point-agentsession.md), [ADR-0025](0025-agent-surface-refines-desktop-scope.md), [ADR-0026](0026-session-export-to-workflow.md), [ADR-0028](0028-workflow-resource-governance.md), [ADR-0029](0029-tool-policy-hardening.md), [ADR-0030](0030-llm-seam-shape-amendment-reasoning-response-format-provider-executed.md), [ADR-0036](0036-run-loop-substrate-event-bus-and-execution-host.md), [ADR-0037](0037-engine-tool-execution-boundary.md), [ADR-0039](0039-same-provider-reasoning-replay.md), [llm-provider-seam.md](../reference/shared-core/llm-provider-seam.md), [run-plan.md](../reference/shared-core/run-plan.md), [sse-event-schema.md](../reference/contracts/sse-event-schema.md), [error-handling.md](../standards/error-handling.md), [security-review.md](../standards/security-review.md), [shared-core-engine.md](../architecture/shared-core-engine.md) + +> **Amended by [ADR-0040](0040-node-retry-budget-above-the-chain.md) (2026-06-15):** the detail that fed the +> node/agent `retry` (`max` **and** `backoff`) into the **primary `FallbackPlanEntry`** is reversed — `retry` +> is now the engine's *above-chain* node-retry budget, and the primary entry defaults to `maxAttempts: 1` with +> the chain's own default backoff. Everything else in this ADR stands. (Within-chain same-model retry of the +> primary, if wanted, is an optional primary chain-entry `max_attempts`.) + +- **Related**: [ADR-0006](0006-os-keychain-for-api-keys.md), [ADR-0011](0011-internal-llm-abstraction.md), [ADR-0018](0018-desktop-execution-and-rust-egress.md), [ADR-0019](0019-cli-node-keychain-library.md), [ADR-0024](0024-agent-first-entry-point-agentsession.md), [ADR-0025](0025-agent-surface-refines-desktop-scope.md), [ADR-0026](0026-session-export-to-workflow.md), [ADR-0028](0028-workflow-resource-governance.md), [ADR-0029](0029-tool-policy-hardening.md), [ADR-0030](0030-llm-seam-shape-amendment-reasoning-response-format-provider-executed.md), [ADR-0036](0036-run-loop-substrate-event-bus-and-execution-host.md), [ADR-0037](0037-engine-tool-execution-boundary.md), [ADR-0039](0039-same-provider-reasoning-replay.md), [ADR-0040](0040-node-retry-budget-above-the-chain.md), [llm-provider-seam.md](../reference/shared-core/llm-provider-seam.md), [run-plan.md](../reference/shared-core/run-plan.md), [sse-event-schema.md](../reference/contracts/sse-event-schema.md), [error-handling.md](../standards/error-handling.md), [security-review.md](../standards/security-review.md), [shared-core-engine.md](../architecture/shared-core-engine.md) ## Context diff --git a/docs/decisions/0040-node-retry-budget-above-the-chain.md b/docs/decisions/0040-node-retry-budget-above-the-chain.md new file mode 100644 index 00000000..2da40c4f --- /dev/null +++ b/docs/decisions/0040-node-retry-budget-above-the-chain.md @@ -0,0 +1,231 @@ +# ADR-0040: Node-level retry budget above the provider fallback chain + +- **Status**: Accepted +- **Date**: 2026-06-15 +- **Related**: [ADR-0038](0038-agentrunner-llm-call-boundary.md) (amended — see Decision A.2), [ADR-0011](0011-internal-llm-abstraction.md) (the `FallbackChain` owns within-chain policy), [ADR-0036](0036-run-loop-substrate-event-bus-and-execution-host.md) (run loop + the injected one-shot timer), [ADR-0027](0027-expression-sandbox.md) / [ADR-0029](0029-tool-policy-hardening.md) (failure classification), [error-handling.md](../standards/error-handling.md), [run-plan.md](../reference/shared-core/run-plan.md) and [node-types.md](../reference/shared-core/node-types.md) (which already pre-describe this layer), [expression-sandbox-spec.md](../reference/shared-core/expression-sandbox-spec.md) (the `runId + nodeId + retryCount` idempotency key) + +## Context + +Reliability in the engine has **two distinct retry concerns**, and only one is built: + +1. **Within an attempt — provider failover (built, 1.K/1.O).** The `FallbackChain` + ([ADR-0011](0011-internal-llm-abstraction.md)) walks an agent's providers: on a *retryable* `LlmError` + (429/5xx/timeout/transport) it retries a provider per that **chain entry's `max_attempts`** and fails + over to the next. LLM-only, in `@relavium/llm`. +2. **Above the chain — whole-node retry (not built, 1.S).** This layer is **already canonically described**: + [run-plan.md](../reference/shared-core/run-plan.md) (§"Retry is not lifted onto the plan") states *"Node-level + retry above the provider fallback chain is layered by **1.S**, which reads `retry_config` from the authored + node (`config.node`) and re-attempts before a node is considered finally failed,"* and + [node-types.md](../reference/shared-core/node-types.md) already lists the engine `retry_config` field set — + `max_attempts`, `backoff_ms` (base delay), `backoff_strategy` (`linear`/`exponential`, *"from the authored + YAML `retry.backoff`"*), `retry_on[]`. [error-handling.md](../standards/error-handling.md) likewise classifies + `tool_failed` and a sandbox **wall-clock-timeout** as *"retryable **within the node retry budget**."* **None of + it is wired** — non-agent nodes (`transform`, `tool`, `condition`, `fan_in`) have no retry and the run loop + never re-dispatches them; a transient `tool_failed` is terminal on the first trip. + +Two **verified contradictions** block 1.S: + +- **`node.retry` means the wrong thing.** The authored `RetrySchema` is `{ max, backoff }`, doc-commented + *"transient-error retry on the **same** model"* (`agent.ts:33`), and the AgentRunner feeds **both** fields into + the **primary `FallbackPlanEntry`** (`agent-runner.ts:208-214`: `maxAttempts: retry?.max ?? 1` and + `backoff: retry.backoff`) — i.e. `node.retry` currently configures *within-chain* primary retry, not the + above-chain budget run-plan.md/node-types.md describe. +- **The authored config is too thin** to lower to the documented engine form: `retry_on[]` and a base + `backoff_ms` (node-types.md:128) have no authored source in `{ max, backoff }`. + +Getting this wrong silently double-counts (a node retried within the chain *and* re-run above it under one +`max`), or leaves non-agent nodes un-retryable with error-handling.md's "node retry budget" dangling. + +## Decision + +This is two related decisions; **Part A** is the in-run automatic retry budget, **Part B** is the +user-triggered retry-from-node. They share the idempotency key but are otherwise independent. + +### Part A — an engine-level node-retry budget above the chain + +**We will add an engine-level node-retry budget above the provider fallback chain, governed by the node's +extended `retry` config, applied to _every_ node type — re-interpreting `node.retry` as that above-chain +budget.** This realises the layer run-plan.md:62 and node-types.md:128 already specify. + +**A.1 — Two layers, cleanly separated.** +- *Within-attempt (unchanged, 1.K):* the `FallbackChain` fails over across providers and retries a provider per + its **chain-entry `max_attempts`**, on a retryable `LlmError`. LLM-only. +- *Above-chain (new, 1.S):* the **run loop** re-dispatches a **whole node** when its `NodeOutcome` is `failed` + with `retryable: true` (and, if `retry_on` is set, `code ∈ retry_on`), up to `max` **total attempts**, + sleeping `backoff(attempt)` between. Applies to **all** node types. + +**A.2 — `node.retry` IS the above-chain budget (amends ADR-0038).** The AgentRunner stops feeding `node.retry` +into the primary `FallbackPlanEntry` (it defaults to `maxAttempts: 1` and the chain's own default backoff — +exponential / 250 ms base, `fallback-chain.ts`); the engine reads the resolved node's `retry` and applies it +above the chain. This reverses **one detail** of [ADR-0038](0038-agentrunner-llm-call-boundary.md) (both `max` +**and** `backoff` were fed into the primary entry); the rest of ADR-0038 stands. **On acceptance** this lands as +a dated `> Amended by ADR-0040` note on ADR-0038 (not a status flip — [documentation-style.md](../standards/documentation-style.md) §7) plus an update to the `(ADR-0038)` comment at `agent-runner.ts:207`. *Same-model* in-chain +retry of the **primary**, if an author wants it without re-walking the whole chain, is declared as an optional +primary chain-entry `max_attempts` (default 1) — it is no longer implied by `node.retry`. +- *Considered — two configs* (keep `node.retry` within-chain, add a separate above-chain field): rejected — two + retry knobs per node is confusing, and run-plan.md/node-types.md/error-handling.md all frame `retry` as *the* + node budget. One knob, unambiguously above-chain. +- *Considered — no above-chain layer*: rejected — leaves the canonical docs' "node retry budget" dangling and + non-agent nodes un-retryable. + +**A.3 — `RetrySchema` extension + which node schemas carry `retry`.** Authored `retry` keeps `max` (→ engine +`max_attempts`; **total attempts including the first** — `max: 3` ⇒ the initial attempt + up to 2 re-dispatches) +and `backoff` (→ `backoff_strategy`). It **adds**: optional **`backoff_ms?`** (the base delay the strategy +scales — **default 1000 ms**; this is the *above-chain* base, deliberately distinct from and **independent of** +the chain's 250 ms within-chain base) and optional **`retry_on?: ErrorCode[]`**. Existing `{ max, backoff }` +YAML parses unchanged. The lowering (authored friendly names → engine `retry_config`) is the node-types.md:130 +mapping, owned by `@relavium/core`. + +- **The concrete backoff formula** (1-based `attempt` = the index of the retry about to be scheduled, so the + first re-dispatch is `attempt = 1`): `linear` ⇒ `delayMs = backoff_ms * attempt`; `exponential` ⇒ + `delayMs = backoff_ms * 2^(attempt - 1)`. **No jitter** — the run loop is deterministic-replay (backoff is + wall-clock only and never affects event order or outputs; jitter would add non-determinism for no benefit + here). +- **Bounds + the concurrency-slot trade-off.** The computed `delayMs` is **capped at 24 h** + (`MAX_NODE_RETRY_BACKOFF_MS`) so a large schema-valid `max` × `exponential` can never overflow the event + schema's integer range or arm an absurd one-shot timer; a retry that genuinely needs a >24 h wait should be a + scheduled job, not a node budget. `max` itself is **intentionally unbounded** (`positiveInt`, consistent with + `max_attempts` / `window_size` / `max_parallel` — author-controlled, git-committed, code-reviewed config): the + backoff cap, not a `max` ceiling, is the guardrail against an absurd budget. A node holds its `max_parallel` + slot for the **whole** backoff sleep (it stays `running` so the run never idles mid-retry — freeing it would + re-introduce the idle race), so under a tight cap a long `backoff_ms` can serialize otherwise-ready sibling + branches: keep `backoff_ms` modest under a tight cap. +- **Which authored schemas gain `retry`** (a `@relavium/shared` change): it joins `agent` (which already has it) + on the node types that can produce a *retryable* failure — **`condition`, `transform`, `merge`** (their + `merge_fn`/expression runs in the sandbox, whose wall-clock-timeout is retryable). **Excluded:** + `human_gate` (a gate timeout is `run_timeout`, which is **fatal** — re-running a gate is meaningless and its + own `timeout_action` already governs it), and `input` / `output` / `parallel` (purely structural/deterministic + — they cannot produce a transient failure, so a `retry` field would be inert noise). The engine retry layer + itself is **generic** (it acts on any node whose outcome is retryable-and-within-budget); the schema set above + is just which node types may *author* a budget. A.8's "non-agent nodes have only their own `node.retry`" + refers to exactly this set. + +**A.4 — `retry_on` is parse-validated to the retryable subset (reject, not ignore).** A new +`RETRYABLE_ERROR_CODES` constant in `@relavium/shared/constants.ts` (its one canonical home) names the codes a +node budget may retry — exactly these four enum values: **`provider_rate_limit`, `provider_unavailable`, +`tool_failed`, `sandbox_error`** (the wall-clock-timeout arm only; deterministic sandbox errors are +`retryable: false` and excluded by the A.5 gate). There is no fifth code: when the `FallbackChain` exhausts on +real retryable failures the surfaced `ErrorCode` is already `provider_rate_limit` / `provider_unavailable`, and +a chain that exhausts with no real error surfaces `internal` (fatal — never retried). `RetrySchema` +types `retry_on` as `z.array(z.enum(RETRYABLE_ERROR_CODES))` — the subset enum rejects any member outside it at parse time (the +[ADR-0023](0023-strict-authored-yaml-validation.md) strict-reject ethos — a `retry_on: [tool_denied]` is an +authoring error surfaced loudly, never a silent no-op). `retry_on` only **narrows** the already-retryable set; +it can **never** resurrect a fatal failure. Retryability itself stays single-sourced in `NodeFailure.retryable` +per [error-handling.md](../standards/error-handling.md) (fatal: `provider_auth`, `validation`, `tool_denied`, +`cancelled`, `budget_exceeded`, `run_timeout`, `turn_limit`, deterministic `sandbox_error`). + +**A.5 — The engine interceptor + the intermediate-attempt event (the structural change).** Today +`#onOutcome` routes a `failed` outcome straight to `#settleFailed`, which **immediately** sets `#failure`, +marks the vertex `failed`, and calls `#abort.abort()` (killing sibling branches) — none of which may happen +while a retry budget remains. So 1.S inserts a retry decision **before** `#settleFailed`: on a `failed` outcome +that is retryable-and-within-budget-and-`retry_on`-admitted, the engine instead + +- emits a **new, non-terminal `node:retrying` event** `{ nodeId, attemptNumber, error, delayMs }`, where + `error` is the failed attempt's `{ code, message, retryable }` (the `NodeFailure` shape) **without** a + `correlationId` — the correlation id is the anchor of the *terminal* `node:failed`, not of an intermediate + attempt; `delayMs` is the backoff before the next attempt, +- sleeps `delayMs` via the **injected one-shot host timer** (`ExecutionHost.setTimer(delayMs, onFire) => disarm` + — `execution-host.ts`, [ADR-0036](0036-run-loop-substrate-event-bus-and-execution-host.md) Decision 5, the + 1.Q pattern). `setTimer` takes **no** `AbortSignal`; abort-awareness is the engine registering + `signal.addEventListener('abort', disarm)` so a cancel **disarms** the pending retry → **cancel wins** (no + re-dispatch after `run:cancelled`), +- re-dispatches the node as a fresh attempt (incremented `attemptNumber`). + +`#settleFailed` (terminal `node:failed` + `#failure` + `#abort`) runs when the budget is exhausted, the failure +is fatal / excluded by `retry_on`, **or** a cancel/sibling-abort lands while a retry is pending (the engine +settles the last attempt's failure rather than waste a re-dispatch — the run still closes on the cancel / +sibling root cause, by `#settleFailed`'s precedence). `node:failed` therefore **stays terminal — exactly one per +node** (no breaking change to its meaning). `node:started` and `node:failed` **gain an optional `attemptNumber`** +(a second, additive `@relavium/shared` change) so a surface distinguishes "attempt 2 starting" from a replay and +attributes the terminal failure to an attempt. This is the **node-retry** dispatch counter, shared with +`node:completed`/`node:retrying`; it is **distinct from** the within-chain `cost:updated` / `agent:*` counter +(which resets per re-dispatch) — the two do **not** join (sse-event-schema.md §"Two attemptNumber families"). + +**A.6 — Checkpointer (1.R) needs no fold change.** Because `node:retrying` is **non-state-bearing** (folded +like `node:started` — ignored) and `node:failed` is emitted **only** on final exhaustion, a +`node:started → node:retrying → node:started → node:completed` sequence folds to `completed`, and a +`… → node:retrying → node:started → node:failed` sequence folds to `failed` (terminal). `reconstructCheckpointState`'s +existing arms are correct as-is; we add `node:retrying` to its explicitly-ignored set (a one-line comment, no +logic change). A crash mid-retry leaves the node started-but-unfinished → absent → re-run from `pending` (the +1.R trap-b path). **The retry _count_ is not persisted in the Phase-1 checkpoint and resets to 0 on a +crash-resume** — so a node may consume up to `max` *additional* attempts per crash-and-resume cycle. The +idempotency key bounds **side-effect re-application** (a re-run of a non-idempotent step is a no-op at the +target), **not** the attempt count. (A Phase-2 refinement may reconstruct the spent count by folding the +persisted `node:retrying` events' `attemptNumber`; the in-memory reference does not, and that is acceptable — +`max` caps cost *within* a single process, not across crashes.) + +**A.7 — Idempotency + cost.** Each re-dispatch uses the `runId + nodeId + retryCount` key +([expression-sandbox-spec.md](../reference/shared-core/expression-sandbox-spec.md); a 1.R/retry-from-node +concept — **not** ADR-0036). The node-retry `attemptNumber` rides `node:started`/`node:completed`/`node:failed`/ +`node:retrying`; `cost:updated` (and `agent:*`) carries its **own** within-chain attempt counter, which resets +to 1 on each re-dispatch — the two are **distinct** and do not join (sse-event-schema.md §"Two attemptNumber +families"). Cost is still tallied for **every** attempt across both layers and folded run-wide via +`cumulativeCostMicrocents`; to bucket cost *by node-retry attempt*, a surface partitions the ordered stream at +the `node:started`/`node:retrying` boundaries rather than joining on `attemptNumber`. The per-node-execution +`FallbackChain` is rebuilt fresh on each whole-node re-dispatch (ADR-0038) — its per-provider cooldown state does **not** persist across +node retries **by design**: the above-chain `backoff_ms` delay *is* the inter-attempt cooldown, so a rate-limit +that exhausted the chain waits `backoff(attempt)` before the next whole-node attempt rather than re-hitting the +limit immediately. + +**A.8 — Resolution precedence.** The runner resolves `node.retry ?? agent.retry` (a node override wins over the +agent default — unchanged from ADR-0038); both are now the **above-chain** budget. `agent.retry` is the +above-chain budget for an `agent` node; **non-agent nodes have only their own `node.retry`** (no agent fallback). + +**A.9 — Deferred.** Roadmap §1.S's *"optional input adjustment"* on retry (re-running a node with mutated +inputs) is **explicitly deferred** to a follow-up — the budget+backoff+`retry_on` core lands first; input +adjustment needs an authoring surface and a re-resolution story that are out of this ADR's scope. + +### Part B — retry-from-node (user-triggered) + +> **Amended (2026-06-15): Part B is deferred to Phase-2 — NOT implemented with Part A.** Implementing it +> surfaced an irreducible conflict for the Phase-1 in-memory engine: the design below wants the **same +> `runId`** (so the host dedups completed-upstream side effects via `runId+nodeId+retryCount`) **and** a +> single terminal event. But a settled run already holds its one `run:completed`/`run:failed` +> ([ADR-0036](0036-run-loop-substrate-event-bus-and-execution-host.md) exactly-one-terminal); re-running on +> the same `runId` would append a **second** terminal and the 1.R Checkpointer fold would see two. A *new* +> `runId` fixes the terminal/`retryCount` cleanliness but breaks upstream-side-effect dedup (different keys). +> Reconciling "same runId + single terminal + no upstream re-apply" needs the **real persistent store + a +> run-attempt model** (a re-run row referencing the original) — Phase-2, which already owns the surface +> trigger. Part A (the in-run budget) is the landed 1.S deliverable; retry-from-node is tracked in +> [deferred-tasks.md](../roadmap/deferred-tasks.md). The original design intent is preserved below. + +**We will add a `WorkflowEngine` API to re-run a settled/failed run from a chosen node**, reusing the +`runId + nodeId + retryCount` key so completed-upstream side effects are not re-applied. To avoid a key +**collision** with Part A's automatic attempts (a fresh run-from-node must not reuse `retryCount: 0`, which an +automatic first attempt already occupies), retry-from-node **continues** the targeted node's `retryCount` from +where the original run left it (monotonic per `runId + nodeId`), never resetting to 0. The Phase-1 deliverable +is the in-memory engine semantics + this key discipline; the surface trigger (a UI/CLI affordance) is Phase-2. + +## Consequences + +### Positive + +- One coherent retry story matching the canonical docs: within-chain failover (LLM) under whole-node retry + (any node type); the budget error-handling.md/run-plan.md/node-types.md already reference now exists. +- Non-agent nodes recover from a transient failure; `node:failed` stays terminal (exactly one per node), so + the 1.R Checkpointer fold is untouched; `node:retrying` gives per-attempt observability. +- `retry_on` is author-safe by construction (parse-rejected to the retryable subset; cannot resurrect a fatal). +- Cost is tallied for every attempt across both layers and folded run-wide; per-node-retry-attempt bucketing is + by stream order, not an `attemptNumber` join (sse-event-schema.md §"Two attemptNumber families"). A required + node never silently vanishes. + +### Negative / land-time obligations + +- **`@relavium/shared` contract changes (all additive):** (1) `RetrySchema` gains `backoff_ms?` + `retry_on?` + with `retry_on` typed as the `z.enum(RETRYABLE_ERROR_CODES)` subset (reject-at-parse, also rejecting an + empty array via `.min(1)`); (2) a new `RETRYABLE_ERROR_CODES` constant in `constants.ts`; (3) a new + **`node:retrying`** run event — `NodeRetryingEventSchema` in `run-event.ts`, added to `RUN_EVENT_TYPES` + (`constants.ts`) **and** the `RunEventUnionSchema` discriminated union, with its per-variant type export and a + CONTRACT_NAMES count bump in `run-event.test.ts`; (4) optional `attemptNumber` on `node:started` + + `node:failed`; (5) the `retry` field added to the `condition` / `transform` / `merge` authored node schemas + (joining `agent`). `sse-event-schema.md` must document the new event + fields (and that `node:started` may + now repeat per attempt). +- **Behavior change vs ADR-0038:** `node.retry` (both `max` and `backoff`) now drives whole-node re-dispatch, + not primary in-chain retry. Lands as a dated `> Amended by ADR-0040` note on ADR-0038 + the `agent-runner.ts:207` + comment update. A workflow relying on the old meaning moves that intent to a primary chain-entry `max_attempts`. +- **Doc invalidations to fix at land time** (the canonical homes that currently describe the old meaning): + `agent.ts:33` (`RetrySchema` comment), `agent-yaml-spec.md` :37 / :62 / :69 ("same model" retry) and :85 + (the "primary model (with retry) → fallback" resolution order). `run-plan.md:62` and `node-types.md:128` + already describe the new model and need no change. +- A non-idempotent node's re-run is heavier (bounded by the idempotency key); a node with external side effects + must be idempotent or guarded — the same constraint 1.R imposes on crash re-runs. diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 8a9af3cc..b04681fa 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -83,6 +83,7 @@ flowchart TD | 0037 | [Engine-side tool-execution boundary — the `ToolHost` capability seam, policy/mechanism split, bounded results](0037-engine-tool-execution-boundary.md) | Accepted | 2026-06-13 | | 0038 | [AgentRunner LLM-call boundary — host-injected provider resolution, the per-node-execution `FallbackChain`, and the credential discipline](0038-agentrunner-llm-call-boundary.md) | Accepted | 2026-06-14 | | 0039 | [Same-provider signed-reasoning replay — a behavioral amendment to ADR-0030](0039-same-provider-reasoning-replay.md) | Accepted | 2026-06-14 | +| 0040 | [Node-level retry budget above the provider fallback chain (1.S) — amends ADR-0038](0040-node-retry-budget-above-the-chain.md) | Accepted | 2026-06-15 | ## Creating a new ADR diff --git a/docs/reference/contracts/agent-yaml-spec.md b/docs/reference/contracts/agent-yaml-spec.md index b1fbe817..a38dd662 100644 --- a/docs/reference/contracts/agent-yaml-spec.md +++ b/docs/reference/contracts/agent-yaml-spec.md @@ -34,9 +34,11 @@ memory: # optional conversational memory policy type: none | window | summary window_size: number # when type = window -retry: # transient-error retry on the primary model - max: number +retry: # node-retry budget ABOVE the fallback chain (ADR-0040): re-run the whole node on a retryable failure + max: number # total attempts, including the first backoff: linear | exponential + backoff_ms: number # optional base delay (ms) the backoff scales; engine-defaulted when omitted + retry_on: [error-code] # optional; one or more of: provider_rate_limit, provider_unavailable, tool_failed, sandbox_error (a non-retryable code is rejected at parse) fallback_chain: # ordered alternates tried after the primary is exhausted - model: string @@ -59,15 +61,15 @@ fallback_chain: # ordered alternates tried after the primary is exha | `tools` | no | Tool ids — see [../shared-core/built-in-tools.md](../shared-core/built-in-tools.md). | | `mcp_servers` | no | See [../shared-core/mcp-integration.md](../shared-core/mcp-integration.md). | | `memory` | no | `none` (default), `window` (last N turns), or `summary` (rolling summary). | -| `retry` | no | Retry on the *same* model for retryable errors. | -| `fallback_chain` | no | Switch to a *different* model/provider after retries are exhausted. | +| `retry` | no | Node-retry budget **above** the fallback chain — re-runs the whole node on a retryable failure, up to `max` total attempts ([ADR-0040](../../decisions/0040-node-retry-budget-above-the-chain.md)). | +| `fallback_chain` | no | Switch to a *different* model/provider **within an attempt** (the within-chain failover); the node `retry` budget then re-runs the whole chain above it. | ## Retry vs. fallback These are two distinct resilience layers: -- **`retry`** handles transient failures (timeouts, rate limits) by re-attempting the **same** model with the configured `backoff`. -- **`fallback_chain`** handles a model being unavailable or repeatedly failing by moving to the **next** provider/model in the list. Each entry has its own `max_attempts`. The chain is tried in order; the first entry that succeeds produces the node output. This is the mechanism behind the multi-model fallback killer feature. +- **`retry`** is the engine's **above-chain node-retry budget** ([ADR-0040](../../decisions/0040-node-retry-budget-above-the-chain.md)): on a retryable failure (timeouts, rate limits, a transient tool/sandbox error) it re-dispatches the **whole node** up to `max` total attempts with the configured `backoff` (base `backoff_ms`), optionally restricted to specific codes via `retry_on` (one or more of `provider_rate_limit`, `provider_unavailable`, `tool_failed`, `sandbox_error` — a non-retryable code is rejected at parse, never a silent no-op). It applies to non-agent nodes too (`condition`/`transform`/`merge`). *(Historically `retry` meant within-chain same-model retry; ADR-0040 amended that.)* +- **`fallback_chain`** handles a model being unavailable or repeatedly failing by moving to the **next** provider/model in the list **within a single attempt**. Each entry has its own `max_attempts`. The chain is tried in order; the first entry that succeeds produces the node output. This is the mechanism behind the multi-model fallback killer feature. ```yaml retry: @@ -82,7 +84,7 @@ fallback_chain: max_attempts: 1 ``` -The resolution order at run time is: primary `model` (with `retry`) → first `fallback_chain` entry (with its `max_attempts`) → next entry → … . The `model`/`provider` pair and the `fallback_chain` are resolved against Relavium's provider-agnostic `LLMProvider` seam: each entry selects a provider adapter and a canonical model id, and the fallback is executed by the `withFallback` runner that sits *outside* the adapters (the adapters stay dumb). Provider-key resolution and cross-provider tool-schema normalization are likewise handled by `@relavium/llm`. The immovable contract for all of this — the request/result/stream types, the normalization rules, and where `fallback_chain` `max_attempts` is enforced — is [../shared-core/llm-provider-seam.md](../shared-core/llm-provider-seam.md); the rationale and supported-model matrix are in [../../architecture/multi-llm-providers.md](../../architecture/multi-llm-providers.md). +The resolution order **within one attempt** is: primary `model` → first `fallback_chain` entry (with its `max_attempts`) → next entry → … ; the node `retry` budget ([ADR-0040](../../decisions/0040-node-retry-budget-above-the-chain.md)) then re-runs this whole sequence, above the chain, on a retryable failure. The `model`/`provider` pair and the `fallback_chain` are resolved against Relavium's provider-agnostic `LLMProvider` seam: each entry selects a provider adapter and a canonical model id, and the fallback is executed by the `withFallback` runner that sits *outside* the adapters (the adapters stay dumb). Provider-key resolution and cross-provider tool-schema normalization are likewise handled by `@relavium/llm`. The immovable contract for all of this — the request/result/stream types, the normalization rules, and where `fallback_chain` `max_attempts` is enforced — is [../shared-core/llm-provider-seam.md](../shared-core/llm-provider-seam.md); the rationale and supported-model matrix are in [../../architecture/multi-llm-providers.md](../../architecture/multi-llm-providers.md). ## Example diff --git a/docs/reference/contracts/sse-event-schema.md b/docs/reference/contracts/sse-event-schema.md index 96831ed8..c4b7c63c 100644 --- a/docs/reference/contracts/sse-event-schema.md +++ b/docs/reference/contracts/sse-event-schema.md @@ -51,6 +51,7 @@ export type RunEvent = | NodeCompletedEvent | NodeFailedEvent | NodeSkippedEvent + | NodeRetryingEvent | HumanGatePausedEvent | HumanGateResumedEvent | RunCompletedEvent @@ -67,14 +68,15 @@ export type RunEvent = | `type` | Meaning | Key payload fields | | --- | --- | --- | | `run:started` | A run began. | `workflowId` (the `workflows.id` **UUID** FK, not the authored slug — [ADR-0022](../../decisions/0022-run-references-workflow-by-uuid.md)), `inputs` (secret-typed inputs **masked** — see [Security](#security-event-payloads-never-carry-secrets)), `executionMode: 'local' \| 'cloud' \| 'managed'` | -| `node:started` | A node began executing. | `nodeId`, `nodeType` | +| `node:started` | A node began executing. | `nodeId`, `nodeType`, `attemptNumber?` (1-based; absent ⇒ attempt 1, present + >1 ⇒ a node-retry re-dispatch — 1.S) | | `agent:token` | A streaming LLM token from an agent node. | `nodeId`, `token`, `model` | | `agent:tool_call` | An agent invoked a tool. | `nodeId`, `model` (the invoking model — so a tool call is attributable across a failover), `toolId`, `toolInput` (sanitized — no secrets), `attemptNumber?` (1-based, matches `cost:updated`) | | `agent:tool_result` | A tool returned. | `nodeId`, `toolId`, `success`, `outputSummary` (truncated for UI), `attemptNumber?` | | `agent:file_patch_proposed` | An agent proposed a file change (**gated — no write until the user accepts**; e.g. the VS Code inline-diff review). | `nodeId`, `patches: [{ uri, unifiedDiff }]` (≥1 — an empty proposal is meaningless), `attemptNumber?` | -| `cost:updated` | A node's token cost was tallied (drives the cost waterfall). | `nodeId`, `model`, `inputTokens`, `outputTokens`, `costMicrocents`, `cumulativeCostMicrocents` (integer micro-cents — canonical unit in [llm-provider-seam.md](../shared-core/llm-provider-seam.md#6-usage)), `attemptNumber?` (1-based retry attempt this cost belongs to, so per-attempt cost is reconstructable) | -| `node:completed` | A node finished successfully. | `nodeId`, `output`, `tokensUsed: {input, output, model?}` (`model` only for LLM nodes), `durationMs`, `selected?` (a `condition`'s chosen target ids — the authoritative branch record checkpoint/resume restores from, 1.R; **may be an empty array** when the condition routes to no branch, dimming all downstream), `attemptNumber?` | -| `node:failed` | A node failed. | `nodeId`, `error: {code, message, retryable, correlationId?}` (`code` is an [`ErrorCode`](#error-code-taxonomy); `correlationId` is a secret-free id joined to the internal log — ADR-0036) | +| `cost:updated` | A node's token cost was tallied (drives the cost waterfall). | `nodeId`, `model`, `inputTokens`, `outputTokens`, `costMicrocents`, `cumulativeCostMicrocents` (integer micro-cents — canonical unit in [llm-provider-seam.md](../shared-core/llm-provider-seam.md#6-usage)), `attemptNumber?` (1-based **within-chain** FallbackChain attempt — resets per node-retry re-dispatch; **distinct** from `node:*.attemptNumber`, see the [two attemptNumber families](#two-attemptnumber-families) note) | +| `node:completed` | A node finished successfully. | `nodeId`, `output`, `tokensUsed: {input, output, model?}` (`model` only for LLM nodes), `durationMs`, `selected?` (a `condition`'s chosen target ids — the authoritative branch record checkpoint/resume restores from, 1.R; **may be an empty array** when the condition routes to no branch, dimming all downstream), `attemptNumber?` (1-based **node-retry** dispatch attempt — 1.S; absent ⇒ attempt 1) | +| `node:failed` | A node failed (TERMINAL — exactly one per node; emitted when the node-retry budget is exhausted, on a fatal / `retry_on`-excluded failure, **or** when a pending retry is abandoned by a cancel or a sibling abort — see 1.S). | `nodeId`, `error: {code, message, retryable, correlationId?}` (`code` is an [`ErrorCode`](#error-code-taxonomy); `correlationId` is a secret-free id joined to the internal log — ADR-0036), `attemptNumber?` (the last attempt, when a retry budget was spent — 1.S) | +| `node:retrying` | A retryable node attempt failed and the engine will re-dispatch the whole node (1.S, [ADR-0040](../../decisions/0040-node-retry-budget-above-the-chain.md)) — **non-terminal** (the node continues; `node:failed` is the terminal). | `nodeId`, `attemptNumber` (the attempt that just failed, 1-based), `error: {code, message, retryable}` (the `NodeFailure` shape — **no** `correlationId`; that anchors the terminal failure), `delayMs` (backoff before the next attempt) | | `node:skipped` | A node was skip-propagated (never ran). | `nodeId`, `reason: 'branch_not_taken' \| 'upstream_unreachable'` (`branch_not_taken` = a `condition` routed away from it; `upstream_unreachable` = every in-edge is dead because an upstream was skipped/failed). Emitted so the event log is a **complete, replayable** record — checkpoint/resume reconstructs a skipped vertex from it ([run-plan.md](../shared-core/run-plan.md)) and a surface can render the dimmed path instead of the node silently vanishing. | | `human_gate:paused` | Execution suspended at a human gate. | `nodeId`, `gateId`, `gateType: 'approval' \| 'input' \| 'review'`, `message`, `assignee?`, `timeoutMs?`, `timeoutAction?: 'approve' \| 'reject'` (on-timeout policy, present only with `timeoutMs`), `expiresAt?` | | `human_gate:resumed` | A gate decision was applied; execution continues. | `nodeId`, `decision: 'approved' \| 'rejected' \| 'input_provided'`, `decidedBy`, `payload?` | @@ -83,6 +85,15 @@ export type RunEvent = | `run:failed` | The run failed. | `error: {code, message, retryable, nodeId?, correlationId?}` (`code` is an [`ErrorCode`](#error-code-taxonomy); `nodeId` is the root-cause node; `correlationId` joins to the internal log — ADR-0036), `partialOutputs` | | `run:cancelled` | The run was cancelled. | (base only) | +### Two attemptNumber families + +`attemptNumber` appears on two **independent** counter families that must not be conflated (1.S, [ADR-0040](../../decisions/0040-node-retry-budget-above-the-chain.md)): + +- **Node-retry dispatch attempt** — on `node:started` / `node:completed` / `node:failed` / `node:retrying`. The engine's **above-chain** whole-node re-dispatch index. Absent ⇒ attempt 1; present + >1 ⇒ a re-dispatch (distinguishes "attempt N starting" from a replay). +- **Within-chain attempt** — on `cost:updated` / `agent:tool_call` / `agent:tool_result` / `agent:file_patch_proposed`. The **within-chain** `FallbackChain` attempt index inside a *single* node dispatch; it **resets to 1 on every node-retry re-dispatch** (a fresh chain runs each time). + +The two do **not** join: on a node the budget retried, `node:completed.attemptNumber` may be `2` while the accompanying `cost:updated.attemptNumber` is `1`. To attribute cost to a node-retry attempt, **partition the `sequenceNumber`-ordered stream at each `node:started` / `node:retrying` boundary** — do not key by `(nodeId, attemptNumber)` across families. (Run totals are unaffected: `cost:updated.cumulativeCostMicrocents` is the engine's authoritative running total.) + ### Selected definitions > These TypeScript shapes are **illustrative**. The enforced, runtime-validated @@ -107,7 +118,7 @@ export interface CostUpdatedEvent extends BaseEvent { outputTokens: number; costMicrocents: number; // integer micro-cents (canonical unit defined in llm-provider-seam.md); this attempt, from Relavium's pricing table (never the provider) cumulativeCostMicrocents: number; // integer micro-cents running total for the whole run - attemptNumber?: number; // 1-based retry attempt this cost belongs to (per-attempt cost attribution) + attemptNumber?: number; // 1-based WITHIN-CHAIN attempt; resets per node-retry re-dispatch — distinct from node:*.attemptNumber (see "Two attemptNumber families") } export interface NodeCompletedEvent extends BaseEvent { @@ -120,7 +131,7 @@ export interface NodeCompletedEvent extends BaseEvent { tokensUsed: { input: number; output: number; model?: string }; durationMs: number; selected?: string[]; // a `condition` node only: the immediate target ids it routed to (the live branches); MAY be empty when it routes to no branch (all downstream skip-propagated). The authoritative record checkpoint/resume restores `selectedTargets` from (1.R). - attemptNumber?: number; // 1-based retry attempt this completion belongs to (matches cost:updated) + attemptNumber?: number; // 1-based NODE-RETRY dispatch attempt (1.S); absent ⇒ attempt 1 — distinct from cost:updated.attemptNumber (see "Two attemptNumber families") } export interface NodeSkippedEvent extends BaseEvent { @@ -129,6 +140,14 @@ export interface NodeSkippedEvent extends BaseEvent { reason: 'branch_not_taken' | 'upstream_unreachable'; } +export interface NodeRetryingEvent extends BaseEvent { + type: 'node:retrying'; // 1.S — a retryable attempt failed; the engine will re-dispatch the whole node. NON-TERMINAL. + nodeId: string; + attemptNumber: number; // the attempt that just failed (1-based); the next attempt is attemptNumber + 1 + error: { code: ErrorCode; message: string; retryable: boolean }; // the NodeFailure shape — no correlationId (that anchors the terminal node:failed) + delayMs: number; // backoff before the next attempt +} + export interface HumanGatePausedEvent extends BaseEvent { type: 'human_gate:paused'; nodeId: string; diff --git a/docs/reference/contracts/workflow-yaml-spec.md b/docs/reference/contracts/workflow-yaml-spec.md index 47146be2..fbc83298 100644 --- a/docs/reference/contracts/workflow-yaml-spec.md +++ b/docs/reference/contracts/workflow-yaml-spec.md @@ -174,14 +174,17 @@ Each node has an `id` (kebab-case, unique within the workflow) and a `type`. The | `input` | Workflow entry point; emits the resolved inputs. | — | | `agent` | Invoke an agent (LLM call with tools). | `agent_ref`, `prompt_template`, `tools`, `model`, `temperature`, `max_tokens`, `timeout_ms`, `retry` | | `human_gate` | Pause for human approval / input / review. | `gate_type`, `assignee`, `message_template`, `timeout_ms`, `timeout_action` | -| `condition` | Branch on a JS expression over run outputs. | `expression`, `branches[]` (`when`, `target_node`), `default` | -| `transform` | Reshape state without an LLM (JS expression). | `transform` | +| `condition` | Branch on a JS expression over run outputs. | `expression`, `branches[]` (`when`, `target_node`), `default`, `retry` | +| `transform` | Reshape state without an LLM (JS expression). | `transform`, `retry` | | `parallel` | Fan out to several nodes concurrently. | `parallel_of[]` | -| `merge` | Fan in / combine parallel results. | `merge_strategy`, `merge_fn` | +| `merge` | Fan in / combine parallel results. | `merge_strategy`, `merge_fn`, `retry` | | `output` | Terminal node capturing the final result. | `output_format` | > **Canvas vs. engine node taxonomy.** The desktop canvas renders a richer set of node *components* (e.g. `FanOutNode`, `AggregatorNode`, `LoopNode`, `ToolNode`), and the engine's internal node-type enum additionally recognizes `tool`, `loop`, and `subworkflow`. The v1.0 YAML above is the user-authored surface; the full catalog and how the two map is in [../shared-core/node-types.md](../shared-core/node-types.md). +> **`retry` on non-agent nodes.** `condition`, `transform`, and `merge` accept the same optional `retry` +> budget as `agent` — the engine's above-chain node-retry ([ADR-0040](../../decisions/0040-node-retry-budget-above-the-chain.md)): on a retryable failure the whole node is re-dispatched up to `max` total attempts with `backoff`/`backoff_ms`, optionally filtered by `retry_on`. The field shape is owned by [agent-yaml-spec.md](agent-yaml-spec.md#retry-vs-fallback). `input`, `output`, `parallel`, and `human_gate` carry no `retry` (they cannot produce a transient failure; a gate timeout is fatal). + ### `agent` node ```yaml diff --git a/docs/roadmap/deferred-tasks.md b/docs/roadmap/deferred-tasks.md index 50bf548c..305c2f6b 100644 --- a/docs/roadmap/deferred-tasks.md +++ b/docs/roadmap/deferred-tasks.md @@ -339,6 +339,21 @@ Severity is the review's verified rating. Check an item off in the PR that resol `invalid_handle` issue (no existing fixture/spec used one — the spec routes via `branches` + `nodeId:when` handles); pinned by `dag.test.ts` and documented in workflow-yaml-spec.md §edges. +## Node retry (1.S) follow-ups + +> **2026-06-15 1.S implementation (ADR-0040).** The above-chain node-retry budget (Part A — the run loop +> re-dispatches a whole node on a retryable failure, with backoff, bounded by `retry.max`, applied to +> agent/condition/transform/merge nodes) landed. Part B is deferred: + +- [ ] **retry-from-node — re-run a settled run from a chosen node (ADR-0040 Part B) → Phase-2.** Deferred + because the in-memory engine cannot satisfy the design intent simultaneously: re-running on the **same + `runId`** (so the host dedups completed-upstream side effects via `runId+nodeId+retryCount`) would append + a **second terminal event** to a settled run, breaking the exactly-one-terminal invariant (ADR-0036) and + the 1.R Checkpointer fold; a **new `runId`** keeps a single terminal but loses upstream side-effect dedup. + Reconciling both needs the real persistent store + a **run-attempt model** (a re-run row referencing the + original) — Phase-2, which already owns the surface trigger. The in-run budget (Part A) is the landed 1.S + deliverable. *(medium · packages/core/src/engine/engine.ts; ADR-0040 Part B; Phase-2)* + ## Checkpoint/resume + human gate (1.R / 1.Q) follow-ups > **2026-06-15 1.R/1.Q implementation + two pre-merge review passes (PR #22).** The derived `Checkpointer` diff --git a/packages/core/src/engine/agent-runner.e2e.test.ts b/packages/core/src/engine/agent-runner.e2e.test.ts index 2d7f98f7..e03d861b 100644 --- a/packages/core/src/engine/agent-runner.e2e.test.ts +++ b/packages/core/src/engine/agent-runner.e2e.test.ts @@ -309,4 +309,81 @@ workflow: ); expect(tokenNodes).toEqual(new Set(['n1', 'n2'])); }); + + it('retries an agent node via the engine budget resolved from agent.retry (1.S #retryConfig agent fallback, ADR-0040 A.8)', async () => { + // Call 1 errors retryably (chain exhausts → a retryable NodeFailure); call 2 succeeds. The node has NO + // node.retry, so #retryConfig falls back to the AGENT's retry — proving the agent arm + A.8 precedence. + const scripted = scriptedProvider([ + [ + { + type: 'error', + error: { kind: 'overloaded', retryable: true, provider: 'anthropic', message: 'busy' }, + }, + ], + [ + { type: 'text_delta', text: 'recovered' }, + { type: 'stop', stopReason: 'stop', usage: { inputTokens: 1, outputTokens: 1 } }, + ], + ]); + const wf = parseWorkflow( + `schema_version: '1.0' +workflow: + id: e2e-agent-retry + inputs: + - name: text + type: string + agents: + - id: flaky-sum + model: claude-opus-4-8 + provider: anthropic + system_prompt: You summarize. + retry: { max: 2, backoff: linear, backoff_ms: 10 } + nodes: + - id: sum + type: agent + agent_ref: flaky-sum + prompt_template: 'Summarize: {{inputs.text}}' + edges: [] +`, + ); + const host = createInMemoryHost(); + const engine = new WorkflowEngine({ + host, + executor: createAgentNodeExecutor({ + resolveProvider: () => scripted, + registry: noToolRegistry, + tools: [], + keyFor: () => 'k', + sleep: () => Promise.resolve(), + now: () => 1, + }), + }); + const events: RunEvent[] = []; + for await (const event of engine.start({ workflow: wf, inputs: { text: 'x' } }).events) { + events.push(event); + if (event.type === 'node:retrying') { + // Wait for the backoff timer to arm (it is armed in #dispatch's continuation, just after this event), + // then fire it. A bounded guard fails fast instead of hanging if a regression never arms the timer. + let waited = 0; + while (host.armedCount() === 0) { + waited += 1; + if (waited > 1000) { + throw new Error('backoff timer was never armed after node:retrying'); + } + await Promise.resolve(); + } + host.fireTimers(); + } + } + const retrying = events.filter((e) => e.type === 'node:retrying'); + expect(retrying).toHaveLength(1); // one re-dispatch (agent.retry max 2) + expect(retrying[0]?.type === 'node:retrying' ? retrying[0].error.retryable : false).toBe(true); + expect(events.map((e) => e.type).at(-1)).toBe('run:completed'); + expect( + events.some( + (e) => e.type === 'node:completed' && e.nodeId === 'sum' && e.attemptNumber === 2, + ), + ).toBe(true); + assertGapFreeSeq(events); + }); }); diff --git a/packages/core/src/engine/agent-runner.test.ts b/packages/core/src/engine/agent-runner.test.ts index aa3ad707..7630d796 100644 --- a/packages/core/src/engine/agent-runner.test.ts +++ b/packages/core/src/engine/agent-runner.test.ts @@ -264,7 +264,7 @@ describe('createAgentNodeExecutor — output_schema + grant', () => { expect(outcome).toMatchObject({ kind: 'failed', error: { code: 'validation' } }); }); - it('uses node.retry over the agent default for the primary attempt budget', async () => { + it('does NOT use node.retry for within-chain primary retry (it is the engine above-chain budget, ADR-0040)', async () => { let streamCalls = 0; const failing: LlmProvider = { id: 'anthropic', @@ -283,7 +283,9 @@ describe('createAgentNodeExecutor — output_schema + grant', () => { }, }; const exec = createAgentNodeExecutor(deps(failing)); - // AGENT has no retry (default max 1); the node override raises the primary budget to 2. + // node.retry is the engine's ABOVE-chain budget now (ADR-0040) — the AgentRunner no longer feeds it + // into the primary chain entry, so the primary still gets a single within-chain attempt. The + // retryable failure surfaces to the engine, which owns the re-dispatch. const { ctx } = ctxFor( vertexFor({ kind: 'agent', @@ -293,7 +295,10 @@ describe('createAgentNodeExecutor — output_schema + grant', () => { ); const outcome = await exec.execute(ctx); expect(outcome.kind).toBe('failed'); - expect(streamCalls).toBe(2); // node.retry.max, not the agent default of 1 + if (outcome.kind === 'failed') { + expect(outcome.error.retryable).toBe(true); // surfaced as retryable for the engine's node-retry budget + } + expect(streamCalls).toBe(1); // a single primary attempt — node.retry does NOT drive within-chain retry }); it('resolves {{inputs.x}} into a user message, leaving system as the authored prompt', async () => { diff --git a/packages/core/src/engine/agent-runner.ts b/packages/core/src/engine/agent-runner.ts index b0c33484..077a927b 100644 --- a/packages/core/src/engine/agent-runner.ts +++ b/packages/core/src/engine/agent-runner.ts @@ -204,14 +204,16 @@ function buildPlanEntries( if (primary === undefined) { return { ok: false, code: 'internal', message: `no provider wired for '${agent.provider}'` }; } - // The primary entry's retry budget + backoff: a node override wins over the agent default (ADR-0038). - const retry = node.retry ?? agent.retry; + // The primary entry does NOT consume `node.retry` any more: ADR-0040 (amending ADR-0038) makes + // `node.retry` the engine's ABOVE-chain node-retry budget (applied around the whole chain), not the + // primary provider's within-chain same-model retry. The primary defaults to a single attempt + the + // chain's own default backoff; a within-chain primary retry, if ever wanted, is a future primary + // `max_attempts` field (ADR-0040 A.2), not `retry`. const entries: FallbackPlanEntry[] = [ { provider: primary, model: node.model ?? agent.model, - maxAttempts: retry?.max ?? 1, - ...(retry?.backoff === undefined ? {} : { backoff: retry.backoff }), + maxAttempts: 1, }, ]; for (const entry of agent.fallback_chain ?? []) { diff --git a/packages/core/src/engine/checkpoint.test.ts b/packages/core/src/engine/checkpoint.test.ts index 90072daf..331602d1 100644 --- a/packages/core/src/engine/checkpoint.test.ts +++ b/packages/core/src/engine/checkpoint.test.ts @@ -195,6 +195,33 @@ describe('reconstructCheckpointState', () => { expect(state?.cumulativeCostMicrocents).toBe(1600); // the last running total, not a re-sum }); + it('folds node:retrying as non-state-bearing — a retry-then-recover ends `completed` (1.S)', () => { + const state = reconstructCheckpointState([ + started, + { type: 'node:started', ...base(1), nodeId: 'a', nodeType: 'transform' }, + { + type: 'node:retrying', + ...base(2), + nodeId: 'a', + attemptNumber: 1, + error: { code: 'tool_failed', message: 'transient', retryable: true }, + delayMs: 10, + }, + { type: 'node:started', ...base(3), nodeId: 'a', nodeType: 'transform', attemptNumber: 2 }, + { + type: 'node:completed', + ...base(4), + nodeId: 'a', + output: 'A', + tokensUsed: { input: 0, output: 0 }, + durationMs: 1, + attemptNumber: 2, + }, + ]); + // node:retrying is ignored by the fold; the terminal node:completed wins → completed (not failed/limbo). + expect(state?.nodeStates.get('a')).toEqual({ status: 'completed', output: 'A' }); + }); + it('records a failed node with its typed failure', () => { const state = reconstructCheckpointState([ started, diff --git a/packages/core/src/engine/checkpoint.ts b/packages/core/src/engine/checkpoint.ts index a21b0df6..406adaf8 100644 --- a/packages/core/src/engine/checkpoint.ts +++ b/packages/core/src/engine/checkpoint.ts @@ -138,7 +138,9 @@ function applyNodeEvent(acc: ReconAccumulator, event: RunEvent): void { acc.nodeStates.set(event.nodeId, { status: 'skipped' }); break; default: - break; // node:started has no terminal yet → omitted so the rehydrating engine re-runs it + // node:started (running at crash → omit, re-run) and node:retrying (a non-terminal retry attempt, + // 1.S/ADR-0040 — the terminal is a later node:failed/node:completed) are non-state-bearing here. + break; } } diff --git a/packages/core/src/engine/engine.test.ts b/packages/core/src/engine/engine.test.ts index 3ee9fbb5..f2750c16 100644 --- a/packages/core/src/engine/engine.test.ts +++ b/packages/core/src/engine/engine.test.ts @@ -54,6 +54,34 @@ async function drain(handle: RunHandle): Promise { return events; } +/** The node-retry backoff `setTimer` is armed in #dispatch's continuation, just AFTER node:retrying is + * delivered — so yield microtasks until it is armed, then fire it (deterministic; no real wall-clock wait). */ +async function fireBackoff(host: { + armedCount: () => number; + fireTimers: () => void; +}): Promise { + let waited = 0; + while (host.armedCount() === 0) { + waited += 1; + if (waited > 1000) { + throw new Error('backoff timer was never armed after node:retrying'); // fail fast, never hang + } + await Promise.resolve(); + } + host.fireTimers(); +} + +/** A node handler that fails retryably the first `failures` calls, then completes (1.S retry tests). */ +function flaky(failures: number): Handler { + let calls = 0; + return (): NodeOutcome => { + calls += 1; + return calls <= failures + ? { kind: 'failed', error: { code: 'tool_failed', message: 'transient', retryable: true } } + : { kind: 'completed', output: `ok@${calls}` }; + }; +} + const TERMINALS: ReadonlySet = new Set([ 'run:completed', 'run:failed', @@ -1346,6 +1374,285 @@ describe('WorkflowEngine — workflow context (ctx.*) resolution', () => { }); }); +// --- node retry budget (1.S, ADR-0040) ------------------------------------------------------- + +describe('WorkflowEngine — node retry budget above the chain (1.S)', () => { + // A transform node carrying an above-chain retry budget; the stub handler controls the outcome. + const RETRY_WF = ` id: retry-wf + nodes: + - { id: start, type: input } + - { id: work, type: transform, transform: '1', retry: { max: 3, backoff: linear, backoff_ms: 10 } } + - { id: out, type: output } + edges: + - { from: start, to: work } + - { from: work, to: out }`; + + it('retries a transient failure within budget and recovers (node:retrying → re-dispatch → node:completed)', async () => { + const host = createInMemoryHost(); + const engine = engineWith({ work: flaky(1) }, host); // fail once, then succeed + const handle = engine.start({ workflow: workflow(RETRY_WF) }); + const events: RunEvent[] = []; + for await (const event of handle.events) { + events.push(event); + if (event.type === 'node:retrying') { + await fireBackoff(host); // advance past the backoff to the next attempt + } + } + const retrying = events.filter((e) => e.type === 'node:retrying'); + expect(retrying).toHaveLength(1); + if (retrying[0]?.type === 'node:retrying') { + expect(retrying[0].attemptNumber).toBe(1); + expect(retrying[0].error.code).toBe('tool_failed'); + expect(retrying[0].delayMs).toBe(10); // linear, base 10, retry #1 + } + expect( + events.some((e) => e.type === 'node:started' && e.nodeId === 'work' && e.attemptNumber === 2), + ).toBe(true); + const workDone = events.find((e) => e.type === 'node:completed' && e.nodeId === 'work'); + expect(workDone?.type === 'node:completed' ? workDone.attemptNumber : undefined).toBe(2); // recovered on attempt 2 + expect(terminalsIn(events)[0]?.type).toBe('run:completed'); + assertGapFreeSeq(events); + }); + + it('applies exponential backoff and the default base when backoff_ms is omitted', async () => { + const EXP_WF = ` id: exp-wf + nodes: + - { id: start, type: input } + - { id: work, type: transform, transform: '1', retry: { max: 4, backoff: exponential } } + - { id: out, type: output } + edges: + - { from: start, to: work } + - { from: work, to: out }`; + const host = createInMemoryHost(); + const engine = engineWith({ work: flaky(99) }, host); // always fails → exhausts max 4 (3 retries) + const handle = engine.start({ workflow: workflow(EXP_WF) }); + const delays: number[] = []; + for await (const event of handle.events) { + if (event.type === 'node:retrying') { + delays.push(event.delayMs); + await fireBackoff(host); + } + } + // exponential, default base 1000 ms (backoff_ms omitted): base * 2^(retry-1) for retries 1,2,3. + expect(delays).toEqual([1000, 2000, 4000]); + }); + + it('caps the backoff at the 24h ceiling so a large base/budget cannot overflow delayMs', async () => { + // base 50,000,000 ms exponential: retry 1 = 50M (≤ 24h), retry 2 = 100M (> 86.4M) → capped to 86.4M. + // Without the cap a large max would overflow to Infinity and throw at event-stamp time (a zombie run). + const CAP_WF = ` id: cap-wf + nodes: + - { id: start, type: input } + - { id: work, type: transform, transform: '1', retry: { max: 3, backoff: exponential, backoff_ms: 50000000 } } + - { id: out, type: output } + edges: + - { from: start, to: work } + - { from: work, to: out }`; + const host = createInMemoryHost(); + const engine = engineWith({ work: flaky(99) }, host); + const delays: number[] = []; + for await (const event of engine.start({ workflow: workflow(CAP_WF) }).events) { + if (event.type === 'node:retrying') { + delays.push(event.delayMs); + await fireBackoff(host); + } + } + expect(delays).toEqual([50_000_000, 86_400_000]); // the second attempt's delay is clamped to the 24h ceiling + }); + + it('a sibling node failure during the retry backoff abandons the re-dispatch (run:failed, sibling root cause)', async () => { + // Two parallel branches: `flap` retries with a long-ish budget; `boom` fails fatally. boom's failure + // aborts the run while flap is mid-backoff → flap does not re-dispatch; the run fails with boom's cause. + const PAR_RETRY = ` id: par-retry + nodes: + - { id: start, type: input } + - { id: fan, type: parallel, parallel_of: [flap, boom] } + - { id: flap, type: transform, transform: '1', retry: { max: 5, backoff: linear, backoff_ms: 50 } } + - { id: boom, type: transform, transform: '1' } + - { id: join, type: merge, merge_strategy: concat } + - { id: out, type: output } + edges: + - { from: start, to: fan } + - { from: flap, to: join } + - { from: boom, to: join } + - { from: join, to: out }`; + const host = createInMemoryHost(); + const engine = engineWith( + { + flap: (): NodeOutcome => ({ + kind: 'failed', + error: { code: 'tool_failed', message: 'transient', retryable: true }, + }), + boom: (): NodeOutcome => ({ + kind: 'failed', + error: { code: 'validation', message: 'fatal', retryable: false }, + }), + }, + host, + ); + const events: RunEvent[] = []; + for await (const event of engine.start({ workflow: workflow(PAR_RETRY) }).events) { + events.push(event); + // Do NOT fire the backoff timer: boom's fatal failure aborts the run, which abandons flap's pending + // retry without firing it (the abort short-circuits the sleep). + } + const terminal = terminalsIn(events)[0]; + expect(terminal?.type).toBe('run:failed'); + if (terminal?.type === 'run:failed') { + expect(terminal.error.code).toBe('validation'); // boom (the fatal sibling) is the root cause + } + expect(host.armedCount()).toBe(0); // flap's backoff timer was disarmed by the abort + }); + + it('fails the node terminally once the budget is exhausted (node:failed carries the last attemptNumber)', async () => { + const host = createInMemoryHost(); + const engine = engineWith({ work: flaky(99) }, host); // always fails + const handle = engine.start({ workflow: workflow(RETRY_WF) }); // max 3 → 2 retries + const events: RunEvent[] = []; + for await (const event of handle.events) { + events.push(event); + if (event.type === 'node:retrying') { + await fireBackoff(host); + } + } + expect(events.filter((e) => e.type === 'node:retrying')).toHaveLength(2); + const failed = events.find((e) => e.type === 'node:failed' && e.nodeId === 'work'); + expect(failed?.type === 'node:failed' ? failed.attemptNumber : undefined).toBe(3); + expect(terminalsIn(events)[0]?.type).toBe('run:failed'); + assertGapFreeSeq(events); + }); + + it('does not retry a fatal (non-retryable) failure even with a budget', async () => { + const events = await drain( + engineWith({ + work: (): NodeOutcome => ({ + kind: 'failed', + error: { code: 'validation', message: 'bad', retryable: false }, + }), + }).start({ workflow: workflow(RETRY_WF) }), + ); + expect(events.some((e) => e.type === 'node:retrying')).toBe(false); + expect(terminalsIn(events)[0]?.type).toBe('run:failed'); + }); + + it('does not retry a retryable code excluded by retry_on', async () => { + const RETRY_ON_WF = ` id: retry-on-wf + nodes: + - { id: start, type: input } + - { id: work, type: transform, transform: '1', retry: { max: 3, backoff: linear, retry_on: [provider_unavailable] } } + - { id: out, type: output } + edges: + - { from: start, to: work } + - { from: work, to: out }`; + const events = await drain( + engineWith({ + // retryable, but tool_failed is not in retry_on: [provider_unavailable] → no retry + work: (): NodeOutcome => ({ + kind: 'failed', + error: { code: 'tool_failed', message: 'x', retryable: true }, + }), + }).start({ workflow: workflow(RETRY_ON_WF) }), + ); + expect(events.some((e) => e.type === 'node:retrying')).toBe(false); + expect(terminalsIn(events)[0]?.type).toBe('run:failed'); + }); + + it('a cancel during the retry backoff wins — no further attempt, run:cancelled', async () => { + const host = createInMemoryHost(); + const engine = engineWith({ work: flaky(99) }, host); + const handle = engine.start({ workflow: workflow(RETRY_WF) }); + const events: RunEvent[] = []; + for await (const event of handle.events) { + events.push(event); + if (event.type === 'node:retrying') { + // node:retrying is delivered before #abortableSleep arms its timer; cancelling now sets the abort + // first, so the sleep short-circuits on `signal.aborted` and no backoff timer is ever armed. + engine.cancel(handle.runId); + host.fireTimers(); // nothing armed → a no-op (and never will be — the retry was abandoned) + } + } + expect(events.filter((e) => e.type === 'node:retrying')).toHaveLength(1); // only the first attempt's retry + expect(terminalsIn(events)[0]?.type).toBe('run:cancelled'); + expect(host.armedCount()).toBe(0); // no backoff timer was left armed + }); + + it('no node:retrying when the node has no retry budget (a plain transient failure is terminal)', async () => { + // The SEQUENTIAL workflow's `work` transform has no retry field → a retryable failure fails the run. + const events = await drain( + engineWith({ + work: (): NodeOutcome => ({ + kind: 'failed', + error: { code: 'tool_failed', message: 'x', retryable: true }, + }), + }).start({ workflow: workflow(SEQUENTIAL) }), + ); + expect(events.some((e) => e.type === 'node:retrying')).toBe(false); + expect(terminalsIn(events)[0]?.type).toBe('run:failed'); + }); + + it('does not emit a spurious node:retrying when a sibling failure has already doomed the run', async () => { + // A sibling's fatal failure sets #failure + aborts the signal (WITHOUT #cancelling) BEFORE this budgeted + // node's own retryable failure is judged. `flap` resolves a few microtasks later than the synchronous + // `boom`, so by the time flap's outcome is evaluated the run is already doomed — it must settle node:failed + // directly, never promise a non-terminal node:retrying it cannot honour (the willRetry guard, ADR-0040 A.5). + const PAR_SPURIOUS = ` id: par-spurious + nodes: + - { id: start, type: input } + - { id: fan, type: parallel, parallel_of: [flap, boom] } + - { id: flap, type: transform, transform: '1', retry: { max: 5, backoff: linear, backoff_ms: 50 } } + - { id: boom, type: transform, transform: '1' } + - { id: join, type: merge, merge_strategy: concat } + - { id: out, type: output } + edges: + - { from: start, to: fan } + - { from: flap, to: join } + - { from: boom, to: join } + - { from: join, to: out }`; + const host = createInMemoryHost(); + const engine = engineWith( + { + flap: async (): Promise => { + // Yield enough microtasks that the synchronous fatal sibling settles #failure + aborts FIRST. + for (let i = 0; i < 50; i += 1) await Promise.resolve(); + return { + kind: 'failed', + error: { code: 'tool_failed', message: 'transient', retryable: true }, + }; + }, + boom: (): NodeOutcome => ({ + kind: 'failed', + error: { code: 'validation', message: 'fatal', retryable: false }, + }), + }, + host, + ); + const events = await drain(engine.start({ workflow: workflow(PAR_SPURIOUS) })); + expect(events.some((e) => e.type === 'node:retrying')).toBe(false); // no contradicted non-terminal event + const terminal = terminalsIn(events)[0]; + expect(terminal?.type).toBe('run:failed'); + if (terminal?.type === 'run:failed') { + expect(terminal.error.code).toBe('validation'); // boom stays the root cause + } + expect(host.armedCount()).toBe(0); // no backoff timer was ever armed (the retry was never promised) + assertGapFreeSeq(events); + }); + + it('omits attemptNumber on attempt 1 (absent ⇒ attempt 1 — the replay-distinguishing contract)', async () => { + // The first node:started / node:completed must NOT carry attemptNumber: a surface reads "absent ⇒ attempt + // 1", so a stamp of `1` everywhere would silently look like a re-dispatch and break replay-distinguishing. + const events = await drain( + engineWith({ work: flaky(0) }).start({ workflow: workflow(RETRY_WF) }), + ); + const firstStart = events.find((e) => e.type === 'node:started' && e.nodeId === 'work'); + const workDone = events.find((e) => e.type === 'node:completed' && e.nodeId === 'work'); + expect(firstStart).toBeDefined(); + expect(workDone).toBeDefined(); + expect(firstStart?.type === 'node:started' ? firstStart.attemptNumber : 0).toBeUndefined(); + expect(workDone?.type === 'node:completed' ? workDone.attemptNumber : 0).toBeUndefined(); + expect(terminalsIn(events)[0]?.type).toBe('run:completed'); + }); +}); + // --- concurrency cap -------------------------------------------------------------------------- describe('WorkflowEngine — max_parallel concurrency cap', () => { diff --git a/packages/core/src/engine/engine.ts b/packages/core/src/engine/engine.ts index b7cb183b..497ed340 100644 --- a/packages/core/src/engine/engine.ts +++ b/packages/core/src/engine/engine.ts @@ -30,11 +30,13 @@ import { GateDecisionSchema, + RETRYABLE_ERROR_CODES, RunEventSchema, type ExecutionMode, type GateDecision, type MaskedSecret, type NodeSkippedReason, + type Retry, type RunEvent, type RunStatus, type TokensUsed, @@ -90,6 +92,15 @@ const TERMINAL_RUN_STATUSES: ReadonlySet = new Set([ 'cancelled', ]); +/** The above-chain node-retry backoff base (ms) when `retry.backoff_ms` is unset (ADR-0040 A.3, the runner + * constant — distinct from the within-chain FallbackChain base). */ +const DEFAULT_NODE_RETRY_BACKOFF_MS = 1000; + +/** Hard ceiling (ms, 24h) on a computed node-retry backoff — so a large (schema-valid) `retry.max` with + * exponential growth can never overflow `delayMs` past the run-event integer range (a stamp-time throw) or + * arm an absurd timer. A node-retry that needs a >24h wait should be a scheduled job, not a backoff. */ +const MAX_NODE_RETRY_BACKOFF_MS = 86_400_000; + /** The input to {@link WorkflowEngine.start} — a parsed workflow plus its run inputs and mode. */ export interface StartInput { /** The parsed, validated workflow (the host read the file and called `parseWorkflow`). */ @@ -535,7 +546,7 @@ class RunExecution { nodeId: vertex.id, nodeType: vertex.type, }); - void this.#execute(vertex); + void this.#dispatch(vertex, 1); } } @@ -580,9 +591,84 @@ class RunExecution { return claimed; } - async #execute(vertex: PlanVertex): Promise { + /** + * Dispatch a vertex with its above-chain node-retry budget (1.S, ADR-0040). The vertex stays `running` + * across the whole loop — including the backoff sleep — so it never frees its slot or lets the run go + * idle mid-retry. Attempt 1's `node:started` was emitted by `#step`; this loop emits `node:started` for + * each re-dispatch. A retryable failure within budget (and admitted by `retry_on`) emits a non-terminal + * `node:retrying`, sleeps the backoff (abort-aware — cancel wins), and re-runs; any other outcome (or an + * exhausted budget, or a fatal/`retry_on`-excluded failure) settles via `#onOutcome`. + * + * Trade-off: a node waiting out its backoff keeps occupying a `max_parallel` slot (it stays `running`), so + * under a tight cap a long `backoff_ms` can serialize otherwise-ready sibling branches (ADR-0040 A.3 — keep + * `backoff_ms` modest under a tight cap). Freeing the slot mid-backoff would re-introduce the idle race. + */ + async #dispatch(vertex: PlanVertex, firstAttempt: number): Promise { + const retry = this.#retryConfig(vertex); + let attempt = firstAttempt; + // The node holds its slot from the FIRST attempt's node:started; the terminal durationMs measures the + // whole node (all attempts + backoffs), not just the final attempt — consistent with that first start. const startedAtMs = this.#elapsedMs(); - let outcome: Awaited>; + for (;;) { + const outcome = await this.#runAttempt(vertex, attempt); + const willRetry = + outcome.kind === 'failed' && + !this.#settled && + !this.#cancelling && + // …and the run is not already failing/aborting. A sibling node's failure sets `#failure` and aborts + // the signal WITHOUT setting `#cancelling`; without these two guards a doomed node would emit a + // non-terminal `node:retrying` it never honours (the backoff then short-circuits to `node:failed`). + this.#failure === undefined && + !this.#abort.signal.aborted && + this.#shouldRetry(retry, outcome.error, attempt); + if (!willRetry || outcome.kind !== 'failed') { + await this.#onOutcome(vertex, outcome, startedAtMs, attempt); + return; + } + const delayMs = this.#backoffMs(retry, attempt); + await this.#emitDurable({ + type: 'node:retrying', + runId: this.runId, + nodeId: vertex.id, + attemptNumber: attempt, + error: { + code: outcome.error.code, + message: outcome.error.message, + retryable: outcome.error.retryable, + }, + delayMs, + }); + const sleptFully = await this.#abortableSleep(delayMs); + if (this.#settled) { + return; // the run settled (e.g. a sibling failure/cancel) while we waited — drop this re-dispatch + } + if (this.#cancelling || this.#abort.signal.aborted) { + // A cancel / sibling-abort landed on the same tick the timer fully elapsed (so sleptFully was true + // but the run is now ending) — settle this node's last failure rather than waste a re-dispatch. + await this.#onOutcome(vertex, outcome, startedAtMs, attempt); + return; + } + if (!sleptFully) { + // The run's AbortSignal fired during the backoff — a cancel OR a sibling node's failure (which + // aborts to stop other branches). Do not re-dispatch; settle this node's last failure. #settleFailed + // honours precedence: it won't overwrite an already-set #failure (a sibling's root cause) nor set one + // while cancelling — so the run closes as the sibling's run:failed, or run:cancelled, accordingly. + await this.#onOutcome(vertex, outcome, startedAtMs, attempt); + return; + } + attempt += 1; + await this.#emitDurable({ + type: 'node:started', + runId: this.runId, + nodeId: vertex.id, + nodeType: vertex.type, + attemptNumber: attempt, + }); + } + } + + /** Run one attempt of a vertex; returns its outcome (an uncaught handler throw → a single `internal`). */ + async #runAttempt(vertex: PlanVertex, attemptNumber: number): Promise { try { const ctx: NodeExecContext = { vertex, @@ -595,13 +681,13 @@ class RunExecution { this.#nodeEmit(event); }, signal: this.#abort.signal, - attemptNumber: 1, + attemptNumber, }; - outcome = await this.#executor.execute(ctx); + return await this.#executor.execute(ctx); } catch { // The catch-all: any uncaught throw from a node handler maps to a single internal failure // (a tool handler classifies its own failures as tool_failed; a sandbox throw as sandbox_error). - outcome = { + return { kind: 'failed', error: { code: 'internal', @@ -610,10 +696,77 @@ class RunExecution { }, }; } - await this.#onOutcome(vertex, outcome, startedAtMs); } - async #onOutcome(vertex: PlanVertex, outcome: NodeOutcome, startedAtMs: number): Promise { + /** The above-chain retry budget for a vertex (ADR-0040): `node.retry`, defaulting an agent's `agent.retry`; + * `condition`/`transform`/`fan_in` carry their own; other types have none. */ + #retryConfig(vertex: PlanVertex): Retry | undefined { + const config = vertex.config; + switch (config.kind) { + case 'agent': + return config.node.retry ?? config.resolvedAgent?.retry; + case 'condition': + case 'transform': + case 'fan_in': + return config.node.retry; + default: + return undefined; + } + } + + /** Whether to re-dispatch after a failed attempt: a budget exists, the failure is retryable, attempts + * remain (`attempt < max`, where `max` is total attempts incl. the first), and `retry_on` admits the code. */ + #shouldRetry(retry: Retry | undefined, error: NodeFailure, attempt: number): boolean { + if (retry === undefined || !error.retryable || attempt >= retry.max) { + return false; + } + // `retry_on` narrows which codes consume the budget; absent ⇒ the canonical retryable set. Gating the + // absent case on RETRYABLE_ERROR_CODES too is defence-in-depth: the engine never re-dispatches a + // non-transient code even if a future executor mis-sets `retryable: true` on, say, an `internal` failure. + // Widen to `readonly string[]` (a safe widening, no cast) so `.includes` accepts the wider ErrorCode. + const allowed: readonly string[] = retry.retry_on ?? RETRYABLE_ERROR_CODES; + return allowed.includes(error.code); + } + + /** The backoff delay before the retry after `attempt` (1-based retry index = `attempt`): `linear` ⇒ + * `base * attempt`; `exponential` ⇒ `base * 2^(attempt-1)`. No jitter (deterministic replay). Capped at + * {@link MAX_NODE_RETRY_BACKOFF_MS} so a large (schema-valid) `max` can never overflow `delayMs` past the + * event schema's integer range (which would throw at stamp time) or arm an absurd one-shot timer. */ + #backoffMs(retry: Retry | undefined, attempt: number): number { + const base = retry?.backoff_ms ?? DEFAULT_NODE_RETRY_BACKOFF_MS; + const raw = retry?.backoff === 'exponential' ? base * 2 ** (attempt - 1) : base * attempt; + return Math.min(raw, MAX_NODE_RETRY_BACKOFF_MS); + } + + /** Sleep `ms` via the injected one-shot timer; resolves `true` if it elapsed, `false` if the run's + * `AbortSignal` fired first (cancel disarms the pending retry — ADR-0040 A.5). */ + #abortableSleep(ms: number): Promise { + if (this.#abort.signal.aborted) { + return Promise.resolve(false); + } + return new Promise((resolve) => { + const cleanup = (): void => { + this.#abort.signal.removeEventListener('abort', onAbort); + }; + const onAbort = (): void => { + disarm(); + cleanup(); + resolve(false); + }; + const disarm = this.#host.setTimer(ms, () => { + cleanup(); + resolve(true); + }); + this.#abort.signal.addEventListener('abort', onAbort); + }); + } + + async #onOutcome( + vertex: PlanVertex, + outcome: NodeOutcome, + startedAtMs: number, + attemptNumber = 1, + ): Promise { if (this.#settled) { return; // terminal already emitted — ignore a late settle (e.g. an aborted straggler) } @@ -621,10 +774,10 @@ class RunExecution { switch (outcome.kind) { case 'completed': case 'branch': - await this.#settleCompleted(vertex, outcome, startedAtMs); + await this.#settleCompleted(vertex, outcome, startedAtMs, attemptNumber); break; case 'failed': - await this.#settleFailed(vertex, outcome.error); + await this.#settleFailed(vertex, outcome.error, attemptNumber); break; case 'paused': await this.#settlePaused(vertex, outcome.gate); @@ -643,6 +796,7 @@ class RunExecution { vertex: PlanVertex, outcome: Extract, startedAtMs: number, + attemptNumber = 1, ): Promise { // Update status SYNCHRONOUSLY before emitting, so `#countRunning` is consistent the instant this // vertex settles — a concurrent step never sees it as still running. @@ -666,11 +820,14 @@ class RunExecution { durationMs: Math.max(0, this.#elapsedMs() - startedAtMs), // A condition's branch selection — persisted so resume can restore `selectedTargets` (1.R). ...(outcome.kind === 'branch' ? { selected: [...outcome.selected] } : {}), + // Which attempt produced the output, when a node-retry recovered (1.S) — absent ⇒ attempt 1. + ...(attemptNumber > 1 ? { attemptNumber } : {}), }); } - /** A `failed` outcome: record the root cause (cancel wins), then emit `node:failed`. */ - async #settleFailed(vertex: PlanVertex, error: NodeFailure): Promise { + /** A `failed` outcome (terminal — the node-retry budget is exhausted or the failure is fatal): record the + * root cause (cancel wins), then emit the single terminal `node:failed`. */ + async #settleFailed(vertex: PlanVertex, error: NodeFailure, attemptNumber = 1): Promise { const state = this.#states.get(vertex.id); if (state !== undefined) { state.status = 'failed'; @@ -687,6 +844,7 @@ class RunExecution { // Stamp a secret-free correlation id at this single translation point (ADR-0036) so a surface // can quote it and it joins to the structured internal log. error: { ...error, correlationId: this.#host.ids.newId() }, + ...(attemptNumber > 1 ? { attemptNumber } : {}), }); } diff --git a/packages/shared/src/agent.ts b/packages/shared/src/agent.ts index 52eafd7a..26b771f6 100644 --- a/packages/shared/src/agent.ts +++ b/packages/shared/src/agent.ts @@ -9,7 +9,7 @@ import { positiveInt, temperatureSchema, } from './common.js'; -import { LLM_PROVIDERS } from './constants.js'; +import { LLM_PROVIDERS, RETRYABLE_ERROR_CODES } from './constants.js'; /** * Agent schema (agent-yaml-spec.md). An agent is a named, reusable LLM @@ -30,11 +30,27 @@ export const ProviderSchema = z.enum(LLM_PROVIDERS); export const BackoffStrategySchema = z.enum(['linear', 'exponential']); export type BackoffStrategy = z.infer; -/** Transient-error retry on the *same* model. */ +/** + * A node's transient-failure **retry budget**, applied by the engine ABOVE the provider fallback chain + * ([ADR-0040](../decisions/0040-node-retry-budget-above-the-chain.md)): on a retryable failure the *whole node* + * is re-dispatched up to `max` **total attempts** (the initial attempt counts), with `backoff` over a + * `backoff_ms` base. **Not** the within-chain same-model retry it once configured (ADR-0038, amended). On an + * `agent` node this also defaults the agent's value (`node.retry ?? agent.retry`). + */ export const RetrySchema = z .object({ + /** Total attempts, including the first (`max: 3` ⇒ the initial attempt + up to 2 re-dispatches). */ max: positiveInt, + /** How the inter-attempt delay grows: `linear` (`base * n`) or `exponential` (`base * 2^(n-1)`). */ backoff: BackoffStrategySchema, + /** The backoff base delay in ms the strategy scales; the engine defaults it when omitted (ADR-0040 A.3). */ + backoff_ms: positiveInt.optional(), + /** + * Restrict which retryable failures consume the budget — restricted to the retryable subset + * ({@link RETRYABLE_ERROR_CODES}), so a non-retryable code (e.g. `tool_denied`) is rejected at parse + * (ADR-0040 A.4), never a silent no-op. Omitted ⇒ any `retryable` failure is retried. + */ + retry_on: z.array(z.enum(RETRYABLE_ERROR_CODES)).min(1).optional(), }) .strict(); export type Retry = z.infer; diff --git a/packages/shared/src/constants.ts b/packages/shared/src/constants.ts index 22cce702..bda310cd 100644 --- a/packages/shared/src/constants.ts +++ b/packages/shared/src/constants.ts @@ -31,6 +31,7 @@ export const RUN_EVENT_TYPES = [ 'node:completed', 'node:failed', 'node:skipped', + 'node:retrying', 'human_gate:paused', 'human_gate:resumed', 'run:completed', @@ -86,6 +87,24 @@ export const ERROR_CODES = [ ] as const; export type ErrorCode = (typeof ERROR_CODES)[number]; +/** + * The `ErrorCode`s a node-retry budget (1.S, [ADR-0040](../decisions/0040-node-retry-budget-above-the-chain.md)) + * may re-attempt — the **transient** failures of [error-handling.md](../standards/error-handling.md): a + * provider rate-limit / unavailability that exhausted the fallback chain, a transient tool-execution failure, + * and a sandbox **wall-clock-timeout** (`sandbox_error`; the deterministic sandbox failures are + * `retryable: false` and excluded at the engine gate, not here). The single source of which authored + * `retry_on` codes are valid — a `retry_on` member outside this set is rejected at parse (ADR-0040 A.4). + * Retryability of a *runtime* failure is still decided by `NodeFailure.retryable`; this set only bounds the + * authored `retry_on` filter. + */ +export const RETRYABLE_ERROR_CODES = [ + 'provider_rate_limit', + 'provider_unavailable', + 'tool_failed', + 'sandbox_error', +] as const satisfies readonly ErrorCode[]; +export type RetryableErrorCode = (typeof RETRYABLE_ERROR_CODES)[number]; + /** * The five-value LLM **stop reason** vocabulary, used today by `session:turn_completed`. * Intended canonical home: `@relavium/shared`, with the `@relavium/llm` seam re-exporting it diff --git a/packages/shared/src/node.test.ts b/packages/shared/src/node.test.ts index cba39bea..4c1b644b 100644 --- a/packages/shared/src/node.test.ts +++ b/packages/shared/src/node.test.ts @@ -42,6 +42,43 @@ describe('NodeSchema', () => { expect(NodeSchema.safeParse({ id: 'My_Node', type: 'input' }).success).toBe(false); }); + it('accepts an above-chain retry budget on condition / transform / merge (ADR-0040)', () => { + const retry = { max: 3, backoff: 'exponential', backoff_ms: 500, retry_on: ['tool_failed'] }; + const samples: unknown[] = [ + { + id: 'c', + type: 'condition', + expression: 'x', + branches: [{ when: true, target_node: 'a' }], + retry, + }, + { id: 't', type: 'transform', transform: '1', retry }, + { id: 'm', type: 'merge', merge_strategy: 'concat', retry }, + ]; + for (const sample of samples) { + expect(NodeSchema.safeParse(sample).success).toBe(true); + } + }); + + it('rejects a retry_on listing a non-retryable error code (ADR-0040 A.4)', () => { + // `tool_denied` is fatal — retrying it just re-denies; the subset enum rejects it at parse. + const bad = { + id: 't', + type: 'transform', + transform: '1', + retry: { max: 2, backoff: 'linear', retry_on: ['tool_denied'] }, + }; + expect(NodeSchema.safeParse(bad).success).toBe(false); + // …and an empty retry_on (a budget that retries on nothing) is rejected too. + const empty = { + id: 't', + type: 'transform', + transform: '1', + retry: { max: 2, backoff: 'linear', retry_on: [] }, + }; + expect(NodeSchema.safeParse(empty).success).toBe(false); + }); + it('rejects an agent node missing agent_ref', () => { expect(NodeSchema.safeParse({ id: 'a', type: 'agent' }).success).toBe(false); }); diff --git a/packages/shared/src/node.ts b/packages/shared/src/node.ts index e30dcfd8..b12635a2 100644 --- a/packages/shared/src/node.ts +++ b/packages/shared/src/node.ts @@ -104,6 +104,7 @@ export const ConditionNodeSchema = z .array(ConditionBranchSchema) .min(1, 'a condition node must declare at least one branch'), default: kebabIdSchema.optional(), + retry: RetrySchema.optional(), // above-chain node-retry budget (ADR-0040) — the expression runs in the sandbox }) .strict(); @@ -114,6 +115,7 @@ export const TransformNodeSchema = z transform: nonEmptyString, expression_type: ExpressionTypeSchema.optional(), output_schema: OutputSchemaSchema.optional(), + retry: RetrySchema.optional(), // above-chain node-retry budget (ADR-0040) — the expression runs in the sandbox }) .strict(); @@ -134,6 +136,7 @@ export const MergeNodeSchema = z // Required only when merge_strategy = custom; enforced at the workflow level // (a discriminated-union option cannot carry a cross-field refinement). merge_fn: z.string().optional(), + retry: RetrySchema.optional(), // above-chain node-retry budget (ADR-0040) — a custom merge_fn runs in the sandbox }) .strict(); diff --git a/packages/shared/src/run-event.test.ts b/packages/shared/src/run-event.test.ts index 68dfd7e6..6df41c07 100644 --- a/packages/shared/src/run-event.test.ts +++ b/packages/shared/src/run-event.test.ts @@ -80,6 +80,14 @@ const valid: Record> = { nodeId: 'n', reason: 'branch_not_taken', }, + 'node:retrying': { + type: 'node:retrying', + ...env, + nodeId: 'n', + attemptNumber: 1, + error: { code: 'tool_failed', message: 'boom', retryable: true }, + delayMs: 1000, + }, 'human_gate:paused': { type: 'human_gate:paused', ...env, @@ -201,6 +209,13 @@ const reject: Record> = { }, 'node:failed (missing error)': { type: 'node:failed', ...env, nodeId: 'n' }, 'node:skipped (bad reason)': { type: 'node:skipped', ...env, nodeId: 'n', reason: 'because' }, + 'node:retrying (missing delayMs)': { + type: 'node:retrying', + ...env, + nodeId: 'n', + attemptNumber: 1, + error: { code: 'tool_failed', message: 'x', retryable: true }, + }, 'human_gate:paused (bad gateType)': { type: 'human_gate:paused', ...env, @@ -252,7 +267,7 @@ describe('RunEvent union — every variant', () => { expect(RunEventSchema.safeParse(reject[name]).success).toBe(false); }); - it('covers exactly the 19 canonical colon-namespaced names, pinned to a literal list', () => { + it('covers exactly the 20 canonical colon-namespaced names, pinned to a literal list', () => { // A hardcoded contract list — independent of RUN_EVENT_TYPES — so the union and the // constant cannot silently drift together. const CONTRACT_NAMES = [ @@ -266,6 +281,7 @@ describe('RunEvent union — every variant', () => { 'node:completed', 'node:failed', 'node:skipped', + 'node:retrying', 'human_gate:paused', 'human_gate:resumed', 'run:completed', @@ -282,7 +298,7 @@ describe('RunEvent union — every variant', () => { // RunEventSchema wraps the union in the correlation-key refinement; reach the raw union. expect(RunEventSchema.innerType().options).toHaveLength(CONTRACT_NAMES.length); expect(new Set(RUN_EVENT_TYPES)).toEqual(new Set(CONTRACT_NAMES)); - expect(Object.keys(valid)).toEqual(CONTRACT_NAMES); // the matrix covers all 19 + expect(Object.keys(valid)).toEqual(CONTRACT_NAMES); // the matrix covers all 20 }); it('pins the RunEvent discriminant to RunEventType (type-level)', () => { diff --git a/packages/shared/src/run-event.ts b/packages/shared/src/run-event.ts index 86548122..442d3d85 100644 --- a/packages/shared/src/run-event.ts +++ b/packages/shared/src/run-event.ts @@ -141,6 +141,9 @@ export const NodeStartedEventSchema = z.object({ // The engine node type (node-types.md is the canonical taxonomy), not the authored YAML type — // `parallel`/`merge` have already expanded to `fan_out`/`fan_in` by the time the engine runs. nodeType: z.enum(ENGINE_NODE_TYPES), + // 1-based dispatch attempt (1.S, ADR-0040). Absent ⇒ attempt 1; present + >1 ⇒ a node-retry re-dispatch, + // so a surface distinguishes "attempt N starting" from a replay. The node may emit several of these. + attemptNumber: positiveInt.optional(), }); export const AgentTokenEventSchema = z.object({ @@ -158,7 +161,7 @@ export const AgentToolCallEventSchema = z.object({ model: nonEmptyString, // the invoking model — attributable across a failover toolId: nonEmptyString, toolInput: z.unknown(), // sanitized — no secrets - attemptNumber: positiveInt.optional(), // 1-based retry attempt (matches cost:updated) + attemptNumber: positiveInt.optional(), // 1-based within-chain (FallbackChain) attempt — matches cost:updated }); export const AgentToolResultEventSchema = z.object({ @@ -168,7 +171,7 @@ export const AgentToolResultEventSchema = z.object({ toolId: nonEmptyString, success: z.boolean(), outputSummary: z.string(), - attemptNumber: positiveInt.optional(), + attemptNumber: positiveInt.optional(), // 1-based within-chain attempt — matches cost:updated }); export const AgentFilePatchProposedEventSchema = z.object({ @@ -178,7 +181,7 @@ export const AgentFilePatchProposedEventSchema = z.object({ // Gated — no write until the user accepts (e.g. the VS Code inline-diff review). // At least one patch — an empty proposal is meaningless (mirrors run:paused.gateIds). patches: z.array(z.object({ uri: nonEmptyString, unifiedDiff: z.string() })).min(1), - attemptNumber: positiveInt.optional(), + attemptNumber: positiveInt.optional(), // 1-based within-chain attempt — matches cost:updated }); export const CostUpdatedEventSchema = z.object({ @@ -190,7 +193,11 @@ export const CostUpdatedEventSchema = z.object({ outputTokens: nonNegativeInt, costMicrocents: nonNegativeInt, // integer micro-cents (canonical unit); from Relavium's pricing table, never the provider cumulativeCostMicrocents: nonNegativeInt, // integer micro-cents running total for the whole run - attemptNumber: positiveInt.optional(), // 1-based retry attempt this cost belongs to + // 1-based WITHIN-CHAIN (FallbackChain) attempt this cost belongs to; resets to 1 on each node-retry + // re-dispatch. DISTINCT from node:*.attemptNumber (the node-retry dispatch index) — the two do NOT join. + // To attribute cost to a node-retry attempt, partition the sequenceNumber-ordered stream at the + // node:started / node:retrying boundaries; do not key by (nodeId, attemptNumber) across the two families. + attemptNumber: positiveInt.optional(), }); export type CostUpdatedEvent = z.infer; @@ -201,7 +208,10 @@ export const NodeCompletedEventSchema = z.object({ output: z.unknown(), tokensUsed: TokensUsedSchema, durationMs: nonNegativeInt, - attemptNumber: positiveInt.optional(), // 1-based retry attempt (matches cost:updated) + // 1-based NODE-RETRY dispatch attempt (1.S, ADR-0040) — the same counter as node:started/node:failed. + // Absent ⇒ attempt 1. DISTINCT from cost:updated/agent:* attemptNumber (the within-chain FallbackChain + // index, which resets per node re-dispatch); the two counters do NOT join — see cost:updated above. + attemptNumber: positiveInt.optional(), // The immediate downstream ids a `condition` kept live (its branch selection). Present ONLY for a // condition's branch outcome — it is the authoritative record checkpoint/resume (1.R) reconstructs // `selectedTargets` from, so a selected branch that was mid-flight at a crash re-runs (not skipped). @@ -216,6 +226,31 @@ export const NodeFailedEventSchema = z.object({ ...runBase, nodeId: nonEmptyString, error: z.object(eventErrorFields), + // 1-based attempt this terminal failure belongs to (1.S, ADR-0040) — the last attempt when a node-retry + // budget is exhausted. `node:failed` stays the single TERMINAL failure per node; per-attempt failures + // surface as `node:retrying` (below). + attemptNumber: positiveInt.optional(), +}); + +/** + * A retryable node attempt failed and the engine will re-dispatch the whole node (1.S, + * [ADR-0040](../decisions/0040-node-retry-budget-above-the-chain.md)) — **non-terminal**: it does NOT end the + * node (the terminal is `node:failed`, emitted only when the budget is exhausted). Carries the failed attempt's + * error (the `NodeFailure` shape, **no** `correlationId` — that anchors the terminal failure) and the `delayMs` + * backoff before the next attempt. Checkpoint/resume folds it as non-state-bearing (like `node:started`). + */ +export const NodeRetryingEventSchema = z.object({ + type: z.literal('node:retrying'), + ...runBase, + nodeId: nonEmptyString, + /** The attempt that just failed (1-based). The next attempt is `attemptNumber + 1`. */ + attemptNumber: positiveInt, + // Derived from the canonical `eventErrorFields` (single source of truth) minus `correlationId` — which + // anchors only the TERMINAL failure (`node:failed`), never a per-attempt retry. Deriving keeps this in + // lockstep if a field is later added to the shared failure shape, instead of silently drifting. + error: z.object(eventErrorFields).omit({ correlationId: true }), + /** The backoff delay (ms) before the next attempt is dispatched. */ + delayMs: nonNegativeInt, }); /** Why a node was skipped — `branch_not_taken` (a `condition` routed away) or `upstream_unreachable`. */ @@ -328,6 +363,7 @@ const RunEventUnionSchema = z.discriminatedUnion('type', [ NodeCompletedEventSchema, NodeFailedEventSchema, NodeSkippedEventSchema, + NodeRetryingEventSchema, HumanGatePausedEventSchema, HumanGateResumedEventSchema, RunCompletedEventSchema, @@ -435,6 +471,7 @@ export type AgentFilePatchProposedEvent = z.infer; export type NodeFailedEvent = z.infer; export type NodeSkippedEvent = z.infer; +export type NodeRetryingEvent = z.infer; export type RunCompletedEvent = z.infer; export type RunFailedEvent = z.infer; export type RunCancelledEvent = z.infer;