From 3293d56a066865faec7614f2be7cae3f707d7bbc Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Mon, 20 Jul 2026 18:38:22 +0800 Subject: [PATCH] fix: clarify provider retry delay contract --- .../2026-06-21-bounded-llm-request-recovery.md | 4 ++-- docs/core-data-structures/core.md | 18 +----------------- docs/core-data-structures/llm-streaming.md | 6 ++++-- packages/cordis/tool-cordis/src/api-catalog.ts | 2 +- packages/core/agent-loop/src/loop.ts | 4 +++- .../tests/contract-regressions.spec.ts | 2 +- .../agent-loop/tests/request-recovery.spec.ts | 4 ++-- packages/llm/llm-deepseek/src/adapter.ts | 6 +++--- .../llm/llm-deepseek/tests/adapter.spec.ts | 4 ++-- packages/llm/llm-retry/README.md | 2 +- packages/llm/llm-retry/src/index.ts | 8 +++++--- packages/llm/llm-retry/tests/retry.spec.ts | 4 ++-- packages/llm/llm/src/adapter-failure.ts | 7 ++++--- packages/llm/llm/src/index.ts | 10 +++++----- packages/llm/llm/src/types.ts | 2 +- packages/llm/llm/tests/service.spec.ts | 7 ++++--- scripts/type-equiv.manifest.json | 1 - 17 files changed, 41 insertions(+), 50 deletions(-) diff --git a/.agents/notes/implemented/architecture/2026-06-21-bounded-llm-request-recovery.md b/.agents/notes/implemented/architecture/2026-06-21-bounded-llm-request-recovery.md index ad476b8b35..28de5eb97c 100644 --- a/.agents/notes/implemented/architecture/2026-06-21-bounded-llm-request-recovery.md +++ b/.agents/notes/implemented/architecture/2026-06-21-bounded-llm-request-recovery.md @@ -29,7 +29,7 @@ interface LlmFailure { message: string code: string status?: number - retryAfterMs?: number + providerRetryAfterMs?: number requestId?: ProviderRequestId } ``` @@ -64,7 +64,7 @@ interface Config { The defaults are two transient retries, a 500 millisecond initial delay, a 10 second delay cap, 10 percent jitter, and the four transient codes above. The count and delay bounds match the conservative edge of the inspected implementations: [OpenCode uses two request retries with 500 ms/10 s bounds](https://github.com/anomalyco/opencode/blob/9976269ab1accfc9f9dc98a4a688c516934de422/%70ackages/llm/src/route/executor.ts#L36-L39), [Pi separates three agent-level retries from provider retries and defaults provider retries to zero](https://github.com/earendil-works/pi/blob/3da591ab74ab9ab407e72ed882600b2c851fae21/%70ackages/coding-agent/docs/settings.md#L139-L147), and [Codex uses finite request/stream budgets plus a five-minute idle timeout](https://github.com/openai/codex/blob/0fb559f0f6e231a88ac02ea002d3ecd248e2b515/codex-rs/model-provider-info/src/lib.rs#L25-L33). Ten percent follows [Codex's bounded jitter](https://github.com/openai/codex/blob/0fb559f0f6e231a88ac02ea002d3ecd248e2b515/codex-rs/codex-client/src/retry.rs#L40-L47). Two retries mean at most three provider requests when no other recovery policy applies. `maxTransientRetries` is a non-negative integer, delays are positive finite numbers with `initialDelayMs <= maxDelayMs`, `jitterRatio` is in `[0, 1]`, and codes are non-empty and unique. These are Cordis config fields rather than hidden constants so deployments can choose different cost and latency budgets. -For an eligible failure with budget remaining, the one-based transient retry count uses bounded exponential backoff. A valid provider `retryAfterMs` replaces exponential backoff only when it does not exceed `maxDelayMs`; a longer provider delay causes delegation instead of an earlier retry that violates the provider instruction. Local backoff multiplies by an injected random factor in `[1 - jitterRatio, 1 + jitterRatio]` and clamps the final value to `maxDelayMs`; provider delay is not jittered. +For an eligible failure with budget remaining, the one-based transient retry count uses bounded exponential backoff. A valid `providerRetryAfterMs` replaces exponential backoff only when it does not exceed `maxDelayMs`; a longer provider delay causes delegation instead of an earlier retry that violates the provider instruction. Local backoff multiplies by an injected random factor in `[1 - jitterRatio, 1 + jitterRatio]` and clamps the final value to `maxDelayMs`; provider delay is not jittered. The plugin owns a lifetime `AbortController` and tracks every active backoff callback. Each wait fuses the waterfall's turn signal with that lifetime signal. Effect cleanup first unregisters the listener, then aborts and awaits the active callbacks; a captured callback whose lifetime signal aborts returns `fail` and can neither retry nor enter the rest of its captured waterfall after disposal. This makes HMR disposal quiescent even though Cordis has already captured the listener. diff --git a/docs/core-data-structures/core.md b/docs/core-data-structures/core.md index d08297a751..8db120a963 100644 --- a/docs/core-data-structures/core.md +++ b/docs/core-data-structures/core.md @@ -224,23 +224,7 @@ interface GenerateOptions { } ``` -Why a model response stopped is a merge-extensible reason: - -```ts type-equiv -/** Serializable provider-boundary facts; policy decides whether they are retryable. */ -interface LlmFailure { - /** Human-readable provider or transport failure. */ - readonly message: string - /** Stable provider-neutral machine-routing code. */ - readonly code: string - /** HTTP status observed at the provider boundary, when available. */ - readonly status?: number - /** Provider-requested delay in milliseconds, when valid and available. */ - readonly retryAfterMs?: number - /** Opaque provider-issued request identifier for diagnostics. */ - readonly requestId?: ProviderRequestId -} -``` +Why a model response stopped is a merge-extensible reason. Terminal provider failures carry the streaming contract's [`LlmFailure`](llm-streaming.md#llmfailure): ```ts type-equiv /** diff --git a/docs/core-data-structures/llm-streaming.md b/docs/core-data-structures/llm-streaming.md index 1ecaa00943..674bff9dc8 100644 --- a/docs/core-data-structures/llm-streaming.md +++ b/docs/core-data-structures/llm-streaming.md @@ -31,7 +31,9 @@ type StreamChunk = } ``` -Every thrown or in-band final-adapter failure normalizes to one serializable provider-neutral payload. `retryAfterMs` is a validated positive delay observed at the provider boundary, not a retry decision; `ProviderRequestId` is an opaque branded string for diagnostics. +## `LlmFailure` + +Every thrown or in-band final-adapter failure normalizes to one serializable provider-neutral payload. `providerRetryAfterMs` is a validated positive delay requested by the provider, not a retry decision; `ProviderRequestId` is an opaque branded string for diagnostics. ```ts type-equiv /** Serializable provider-boundary facts; policy decides whether they are retryable. */ @@ -43,7 +45,7 @@ interface LlmFailure { /** HTTP status observed at the provider boundary, when available. */ readonly status?: number /** Provider-requested delay in milliseconds, when valid and available. */ - readonly retryAfterMs?: number + readonly providerRetryAfterMs?: number /** Opaque provider-issued request identifier for diagnostics. */ readonly requestId?: ProviderRequestId } diff --git a/packages/cordis/tool-cordis/src/api-catalog.ts b/packages/cordis/tool-cordis/src/api-catalog.ts index e6c4cf05cb..c0b54d8444 100644 --- a/packages/cordis/tool-cordis/src/api-catalog.ts +++ b/packages/cordis/tool-cordis/src/api-catalog.ts @@ -1186,7 +1186,7 @@ export const TYPE_API: readonly TypeApiEntry[] = [ }, { name: 'LlmFailure', - declaration: 'export interface LlmFailure {\n readonly message: string;\n readonly code: string;\n readonly status?: number;\n readonly retryAfterMs?: number;\n readonly requestId?: ProviderRequestId;\n}', + declaration: 'export interface LlmFailure {\n readonly message: string;\n readonly code: string;\n readonly status?: number;\n readonly providerRetryAfterMs?: number;\n readonly requestId?: ProviderRequestId;\n}', }, { name: 'LlmModelInfo', diff --git a/packages/core/agent-loop/src/loop.ts b/packages/core/agent-loop/src/loop.ts index 926674f866..c237e24fae 100644 --- a/packages/core/agent-loop/src/loop.ts +++ b/packages/core/agent-loop/src/loop.ts @@ -45,7 +45,9 @@ function finishError(finish: FinishReason): { error: RequestError; failure: LlmF const facts = finish.failure const error = new LlmError(facts.message, facts.code, { ...facts.status === undefined ? {} : { status: facts.status }, - ...facts.retryAfterMs === undefined ? {} : { retryAfterMs: facts.retryAfterMs }, + ...facts.providerRetryAfterMs === undefined + ? {} + : { providerRetryAfterMs: facts.providerRetryAfterMs }, ...facts.requestId === undefined ? {} : { requestId: facts.requestId }, }) return { error, failure: error.failure } diff --git a/packages/core/agent-loop/tests/contract-regressions.spec.ts b/packages/core/agent-loop/tests/contract-regressions.spec.ts index f79f917281..e6d269d215 100644 --- a/packages/core/agent-loop/tests/contract-regressions.spec.ts +++ b/packages/core/agent-loop/tests/contract-regressions.spec.ts @@ -929,7 +929,7 @@ describe('a finish-error stream chunk ends the turn as error, not completed', () message: 'provider 401', code: 'AUTH', status: 401, - retryAfterMs: 2_000, + providerRetryAfterMs: 2_000, requestId: ProviderRequestId('finish-request-1'), } const errorStream: StreamChunk[] = [ diff --git a/packages/core/agent-loop/tests/request-recovery.spec.ts b/packages/core/agent-loop/tests/request-recovery.spec.ts index 72688b96bc..039252a6f4 100644 --- a/packages/core/agent-loop/tests/request-recovery.spec.ts +++ b/packages/core/agent-loop/tests/request-recovery.spec.ts @@ -422,7 +422,7 @@ describe('agent post-step and request-error lifecycle', () => { it('passes structured facts beside the original Error and records them on exhaustion', async () => { const original = new LlmError('provider busy', 'RATE_LIMIT', { status: 429, - retryAfterMs: 2_000, + providerRetryAfterMs: 2_000, requestId: ProviderRequestId('req-9'), }) Object.freeze(original) @@ -448,7 +448,7 @@ describe('agent post-step and request-error lifecycle', () => { message: 'provider busy', code: 'RATE_LIMIT', status: 429, - retryAfterMs: 2_000, + providerRetryAfterMs: 2_000, requestId: ProviderRequestId('req-9'), }) expect(seenHistory).toEqual([]) diff --git a/packages/llm/llm-deepseek/src/adapter.ts b/packages/llm/llm-deepseek/src/adapter.ts index b34cc49087..45bc229b7d 100644 --- a/packages/llm/llm-deepseek/src/adapter.ts +++ b/packages/llm/llm-deepseek/src/adapter.ts @@ -42,7 +42,7 @@ export interface DeepSeekAdapterOptions { export const DEFAULT_STREAM_IDLE_TIMEOUT_MS = 300_000 const STREAM_IDLE_TIMEOUT_CODE = 'LLM_STREAM_IDLE_TIMEOUT' -function retryAfterMs(value: string | null): number | undefined { +function providerRetryAfterMs(value: string | null): number | undefined { if (value === null) return undefined if (/^\d+$/.test(value)) { const delay = Number(value) * 1_000 @@ -188,11 +188,11 @@ export class DeepSeekAdapter extends LlmAdapter { // Only swallow error-body parsing: the HTTP status still identifies the // failure, so malformed gateway JSON must not mask it. } - const delay = retryAfterMs(response.headers.get('retry-after')) + const delay = providerRetryAfterMs(response.headers.get('retry-after')) const id = requestId(response.headers) throw new LlmError(message, httpErrorCode(response.status, providerError), { status: response.status, - ...delay === undefined ? {} : { retryAfterMs: delay }, + ...delay === undefined ? {} : { providerRetryAfterMs: delay }, ...id === undefined ? {} : { requestId: id }, }) } diff --git a/packages/llm/llm-deepseek/tests/adapter.spec.ts b/packages/llm/llm-deepseek/tests/adapter.spec.ts index 2e23fd49f7..8cc02891a6 100644 --- a/packages/llm/llm-deepseek/tests/adapter.spec.ts +++ b/packages/llm/llm-deepseek/tests/adapter.spec.ts @@ -232,7 +232,7 @@ describe('DeepSeekAdapter against a mock server', () => { message: 'slow down', code: 'RATE_LIMIT', status: 429, - retryAfterMs: 2_000, + providerRetryAfterMs: 2_000, requestId: ProviderRequestId('req-429'), }) }) @@ -257,7 +257,7 @@ describe('DeepSeekAdapter against a mock server', () => { message: 'come back later', code: 'SERVER', status: 503, - retryAfterMs: 3_000, + providerRetryAfterMs: 3_000, requestId: ProviderRequestId('deepseek-503'), }, }) diff --git a/packages/llm/llm-retry/README.md b/packages/llm/llm-retry/README.md index baebc2d0b3..f84fdba09a 100644 --- a/packages/llm/llm-retry/README.md +++ b/packages/llm/llm-retry/README.md @@ -2,7 +2,7 @@ Function plugin that retries selected transient model-request failures on the agent loop's closed-step recovery seam. It does not wrap `ctx.llm.stream()`: every adapter call remains one provider attempt, and every retry opens a fresh numbered step. -The default policy permits two retries for `RATE_LIMIT`, `SERVER`, `TIMEOUT`, and `TRANSPORT`, using bounded exponential backoff from 500 ms to 10 seconds with 10 percent jitter. Delay bounds must fit Node's supported timer range. A valid provider `retryAfterMs` replaces local backoff when it is within the configured cap; an over-cap instruction delegates to the next recovery policy instead. +The default policy permits two retries for `RATE_LIMIT`, `SERVER`, `TIMEOUT`, and `TRANSPORT`, using bounded exponential backoff from 500 ms to 10 seconds with 10 percent jitter. Delay bounds must fit Node's supported timer range. A valid `providerRetryAfterMs` replaces local backoff when it is within the configured cap; an over-cap instruction delegates to the next recovery policy instead. Before waiting, the plugin appends a non-surface `llm/retry` event with the failure and scheduled delay. Cancellation and plugin disposal abort the wait; disposal drains the plugin's active backoffs, and a callback captured before disposal fails closed if invoked afterward. diff --git a/packages/llm/llm-retry/src/index.ts b/packages/llm/llm-retry/src/index.ts index d4c5b47b3d..4edf22d6f2 100644 --- a/packages/llm/llm-retry/src/index.ts +++ b/packages/llm/llm-retry/src/index.ts @@ -190,9 +190,11 @@ export function apply(ctx: Context, config: Config = {}, internals: RetryInterna const retry = priorTransientFailures + 1 let delayMs: number - if (failure.retryAfterMs !== undefined && Number.isFinite(failure.retryAfterMs) && failure.retryAfterMs > 0) { - if (failure.retryAfterMs > resolved.maxDelayMs) return next() - delayMs = failure.retryAfterMs + if (failure.providerRetryAfterMs !== undefined + && Number.isFinite(failure.providerRetryAfterMs) + && failure.providerRetryAfterMs > 0) { + if (failure.providerRetryAfterMs > resolved.maxDelayMs) return next() + delayMs = failure.providerRetryAfterMs } else { delayMs = localDelay(resolved, retry, random) } diff --git a/packages/llm/llm-retry/tests/retry.spec.ts b/packages/llm/llm-retry/tests/retry.spec.ts index bc10e28922..8e2f086e97 100644 --- a/packages/llm/llm-retry/tests/retry.spec.ts +++ b/packages/llm/llm-retry/tests/retry.spec.ts @@ -235,7 +235,7 @@ describe('bounded transient retry policy', () => { it('uses a bounded provider Retry-After verbatim and delegates an over-cap instruction', async () => { vi.useFakeTimers() const accepted = new ScriptedAdapter([ - new LlmError('wait', 'RATE_LIMIT', { retryAfterMs: 2_000 }), + new LlmError('wait', 'RATE_LIMIT', { providerRetryAfterMs: 2_000 }), textResponse('done'), ]) ;({ ctx: context } = await harness(accepted, { jitterRatio: 1 })) @@ -250,7 +250,7 @@ describe('bounded transient retry policy', () => { await context.fiber.dispose() const rejected = new ScriptedAdapter([ - new LlmError('wait too long', 'RATE_LIMIT', { retryAfterMs: 10_001 }), + new LlmError('wait too long', 'RATE_LIMIT', { providerRetryAfterMs: 10_001 }), ]) ;({ ctx: context } = await harness(rejected)) const rejectedAgent = context.agentLoop.create(SessionId('retry-after-rejected'), { provider: 'mock', model: 'mock' }) diff --git a/packages/llm/llm/src/adapter-failure.ts b/packages/llm/llm/src/adapter-failure.ts index b2189fdaf9..390282327d 100644 --- a/packages/llm/llm/src/adapter-failure.ts +++ b/packages/llm/llm/src/adapter-failure.ts @@ -76,18 +76,19 @@ function failureSnapshot(value: unknown): LlmFailure | undefined { const message = candidate.message const code = candidate.code const status = candidate.status - const retryAfterMs = candidate.retryAfterMs + const providerRetryAfterMs = candidate.providerRetryAfterMs const requestId = candidate.requestId if (typeof message !== 'string' || message.length === 0 || typeof code !== 'string' || code.length === 0 || (status !== undefined && (!Number.isInteger(status) || status < 100 || status > 599)) - || (retryAfterMs !== undefined && (!Number.isFinite(retryAfterMs) || retryAfterMs <= 0)) + || (providerRetryAfterMs !== undefined + && (!Number.isFinite(providerRetryAfterMs) || providerRetryAfterMs <= 0)) || (requestId !== undefined && (typeof requestId !== 'string' || requestId.length === 0))) return undefined return Object.freeze({ message, code, ...status === undefined ? {} : { status }, - ...retryAfterMs === undefined ? {} : { retryAfterMs }, + ...providerRetryAfterMs === undefined ? {} : { providerRetryAfterMs }, ...requestId === undefined ? {} : { requestId }, }) } catch (_sdkFailureGetter) { diff --git a/packages/llm/llm/src/index.ts b/packages/llm/llm/src/index.ts index dfebb059be..fb431a308c 100644 --- a/packages/llm/llm/src/index.ts +++ b/packages/llm/llm/src/index.ts @@ -50,7 +50,7 @@ export interface LlmErrorOptions extends ErrorOptions { /** Valid HTTP status observed at the provider boundary. */ status?: number /** Positive finite provider-requested delay in milliseconds. */ - retryAfterMs?: number + providerRetryAfterMs?: number /** Non-empty opaque provider request id. */ requestId?: ProviderRequestId } @@ -75,9 +75,9 @@ export class LlmError extends HarnessError { && (!Number.isInteger(options.status) || options.status < 100 || options.status > 599)) { throw new Error('LlmError status must be an integer from 100 through 599') } - if (options?.retryAfterMs !== undefined - && (!Number.isFinite(options.retryAfterMs) || options.retryAfterMs <= 0)) { - throw new Error('LlmError retryAfterMs must be a positive finite number') + if (options?.providerRetryAfterMs !== undefined + && (!Number.isFinite(options.providerRetryAfterMs) || options.providerRetryAfterMs <= 0)) { + throw new Error('LlmError providerRetryAfterMs must be a positive finite number') } if (options?.requestId !== undefined && (typeof options.requestId !== 'string' || options.requestId.length === 0)) { @@ -89,7 +89,7 @@ export class LlmError extends HarnessError { message, code, ...options?.status === undefined ? {} : { status: options.status }, - ...options?.retryAfterMs === undefined ? {} : { retryAfterMs: options.retryAfterMs }, + ...options?.providerRetryAfterMs === undefined ? {} : { providerRetryAfterMs: options.providerRetryAfterMs }, ...options?.requestId === undefined ? {} : { requestId: options.requestId }, }) } diff --git a/packages/llm/llm/src/types.ts b/packages/llm/llm/src/types.ts index f8412aee2b..bbe2e09b59 100644 --- a/packages/llm/llm/src/types.ts +++ b/packages/llm/llm/src/types.ts @@ -16,7 +16,7 @@ export interface LlmFailure { /** HTTP status observed at the provider boundary, when available. */ readonly status?: number /** Provider-requested delay in milliseconds, when valid and available. */ - readonly retryAfterMs?: number + readonly providerRetryAfterMs?: number /** Opaque provider-issued request identifier for diagnostics. */ readonly requestId?: ProviderRequestId } diff --git a/packages/llm/llm/tests/service.spec.ts b/packages/llm/llm/tests/service.spec.ts index e491ad2dc8..6994ed5e7a 100644 --- a/packages/llm/llm/tests/service.spec.ts +++ b/packages/llm/llm/tests/service.spec.ts @@ -191,7 +191,7 @@ describe('LlmService', () => { it('keeps structured provider facts beside a frozen third-party Error', async () => { const original = new LlmError('provider busy', 'RATE_LIMIT', { status: 429, - retryAfterMs: 1_500, + providerRetryAfterMs: 1_500, requestId: ProviderRequestId('req-7'), }) Object.freeze(original) @@ -212,7 +212,7 @@ describe('LlmService', () => { message: 'provider busy', code: 'RATE_LIMIT', status: 429, - retryAfterMs: 1_500, + providerRetryAfterMs: 1_500, requestId: ProviderRequestId('req-7'), }) }) @@ -747,7 +747,8 @@ describe('LlmService', () => { it('rejects non-serializable structured failure facts at construction', () => { expect(() => new LlmError('busy', 'RATE_LIMIT', { status: 42 })).toThrow(/status/) - expect(() => new LlmError('busy', 'RATE_LIMIT', { retryAfterMs: Number.NaN })).toThrow(/retryAfterMs/) + expect(() => new LlmError('busy', 'RATE_LIMIT', { providerRetryAfterMs: Number.NaN })) + .toThrow(/providerRetryAfterMs/) expect(() => new LlmError('busy', 'RATE_LIMIT', { requestId: ProviderRequestId('') })).toThrow(/requestId/) expect(() => new LlmError(1 as never, 'RATE_LIMIT')).toThrow(/message/) expect(() => new LlmError('busy', 1 as never)).toThrow(/code/) diff --git a/scripts/type-equiv.manifest.json b/scripts/type-equiv.manifest.json index ccd573dd9f..b9fa1f4874 100644 --- a/scripts/type-equiv.manifest.json +++ b/scripts/type-equiv.manifest.json @@ -6,7 +6,6 @@ { "doc": "docs/core-data-structures/core.md", "symbol": "AssistantProvenance", "source": "packages/llm/llm/src/types.ts" }, { "doc": "docs/core-data-structures/core.md", "symbol": "Message", "source": "packages/llm/llm/src/types.ts" }, { "doc": "docs/core-data-structures/core.md", "symbol": "MessageSourceMap", "source": "packages/llm/llm/src/types.ts" }, - { "doc": "docs/core-data-structures/core.md", "symbol": "LlmFailure", "source": "packages/llm/llm/src/types.ts" }, { "doc": "docs/core-data-structures/core.md", "symbol": "FinishReasonMap", "source": "packages/llm/llm/src/types.ts" }, { "doc": "docs/core-data-structures/core.md", "symbol": "LlmProviderInfo", "source": "packages/llm/llm/src/types.ts" }, { "doc": "docs/core-data-structures/core.md", "symbol": "LlmModelInfo", "source": "packages/llm/llm/src/types.ts" },