diff --git a/packages/opencode/src/session/retry.test.ts b/packages/opencode/src/session/retry.test.ts index bb6d83a56..ba25fb131 100644 --- a/packages/opencode/src/session/retry.test.ts +++ b/packages/opencode/src/session/retry.test.ts @@ -116,6 +116,74 @@ describe("classifyRetry — legacy retryable classifications stay unknown", () = }) }) +describe("classifyRetry — reads providerFailure.kind (slice ④)", () => { + // Terminal kinds are client-side failures retrying cannot fix. Reading the + // canonical kind means they never retry even if the provider SDK wrongly + // marked the error retryable. + for (const kind of ["auth", "invalid_request", "quota_exhausted"] as const) { + test(`terminal kind ${kind} never retries (kind overrides a retryable SDK flag)`, () => { + expect(classifyRetry(makeAPIError({ isRetryable: true, providerFailure: { kind } }))).toBeUndefined() + }) + } + + // Transient kinds always retry, even if isRetryable is false and the status is + // not 5xx — the classification, not the SDK flag, is the source of truth. + for (const kind of ["rate_limit", "server_overload", "transport_disconnect", "decompression"] as const) { + test(`transient kind ${kind} retries even when isRetryable is false and status is not 5xx`, () => { + expect( + classifyRetry(makeAPIError({ isRetryable: false, statusCode: 400, providerFailure: { kind } }))?.kind, + ).toBe("unknown") + }) + } + + test("unknown kind falls back to the legacy isRetryable + 5xx gate", () => { + expect( + classifyRetry(makeAPIError({ isRetryable: true, statusCode: 404, providerFailure: { kind: "unknown" } }))?.kind, + ).toBe("unknown") + expect( + classifyRetry(makeAPIError({ isRetryable: false, statusCode: 404, providerFailure: { kind: "unknown" } })), + ).toBeUndefined() + expect( + classifyRetry(makeAPIError({ isRetryable: false, statusCode: 503, providerFailure: { kind: "unknown" } }))?.kind, + ).toBe("unknown") + }) + + test("absent providerFailure keeps legacy behavior (back-compat with historical rows)", () => { + expect(classifyRetry(makeAPIError({ isRetryable: false, statusCode: 400 }))).toBeUndefined() + expect(classifyRetry(makeAPIError({ isRetryable: true, statusCode: 429 }))?.kind).toBe("unknown") + expect(classifyRetry(makeAPIError({ isRetryable: false, statusCode: 503 }))?.kind).toBe("unknown") + }) + + test("free_quota_exhausted still wins for opencode + marker, regardless of kind", () => { + const error = makeAPIError({ + providerID: ProviderID.opencode, + statusCode: 429, + responseBody: '{"error":{"type":"FreeUsageLimitError"}}', + responseHeaders: { "retry-after": "70" }, + providerFailure: { kind: "rate_limit" }, + }) + expect(classifyRetry(error)?.kind).toBe("free_quota_exhausted") + }) + + test("retry-notice copy is unchanged when reading kind (option A keeps provider message)", () => { + const descriptive = makeAPIError({ + isRetryable: true, + statusCode: 503, + message: "The server is overloaded. Please try again later. (request id req_abc)", + providerFailure: { kind: "server_overload" }, + }) + const c = classifyRetry(descriptive) + expect(c?.kind).toBe("unknown") + if (c?.kind === "unknown") { + expect(c.raw).toBe("The server is overloaded. Please try again later. (request id req_abc)") + } + + const normalized = makeAPIError({ message: "Provider is Overloaded", providerFailure: { kind: "server_overload" } }) + const c2 = classifyRetry(normalized) + if (c2?.kind === "unknown") expect(c2.raw).toBe("Provider is overloaded") + }) +}) + describe("retryAction re-exported from retry.ts", () => { test("free_quota_exhausted maps to stop", () => { expect( diff --git a/packages/opencode/src/session/retry.ts b/packages/opencode/src/session/retry.ts index 1865aa347..6d41f13a1 100644 --- a/packages/opencode/src/session/retry.ts +++ b/packages/opencode/src/session/retry.ts @@ -3,6 +3,7 @@ import { Cause, Clock, Duration, Effect, Schedule } from "effect" import { MessageV2 } from "./message-v2" import { iife } from "@/util/iife" import { ProviderID } from "@/provider/schema" +import type { ProviderFailureKind } from "@/provider/error" import { type RetryClassification, retryAction } from "./retry-classification" export type Err = ReturnType @@ -10,6 +11,19 @@ export type Err = ReturnType export { retryAction } from "./retry-classification" export type { RetryAction } from "./retry-classification" +// providerFailure kinds (slice ②) that are terminal client-side failures — +// retrying cannot fix them, so they never retry. The transient kinds below +// always retry. classifyRetry reads these so the retry decision follows the +// canonical classification instead of re-deriving it from the provider SDK's +// isRetryable flag; `unknown` or absent kinds fall back to that legacy signal. +const RETRY_TERMINAL_KINDS = new Set(["auth", "invalid_request", "quota_exhausted"]) +const RETRY_TRANSIENT_KINDS = new Set([ + "rate_limit", + "server_overload", + "transport_disconnect", + "decompression", +]) + export const RETRY_INITIAL_DELAY = 2000 export const RETRY_BACKOFF_FACTOR = 2 export const RETRY_MAX_DELAY_NO_HEADERS = 30_000 // 30 seconds @@ -59,9 +73,19 @@ export function classifyRetry(error: Err): RetryClassification | undefined { if (MessageV2.ContextOverflowError.isInstance(error)) return undefined if (MessageV2.APIError.isInstance(error)) { const status = error.data.statusCode - // 5xx errors are transient server failures and should always be retried, - // even when the provider SDK doesn't explicitly mark them as retryable. - if (!error.data.isRetryable && !(status !== undefined && status >= 500)) return undefined + const kind = error.data.providerFailure?.kind + // Retry/stop gate. Prefer the canonical providerFailure.kind: terminal kinds + // never retry, transient kinds always do. When the kind is `unknown` or the + // row predates providerFailure, fall back to the legacy signal — isRetryable, + // or a 5xx the provider SDK didn't explicitly mark retryable. The two agree + // for every classified kind today. + const retryable = + kind && RETRY_TERMINAL_KINDS.has(kind) + ? false + : kind && RETRY_TRANSIENT_KINDS.has(kind) + ? true + : error.data.isRetryable || (status !== undefined && status >= 500) + if (!retryable) return undefined // Strict 3-way AND: opencode provider + FreeUsageLimitError marker in body if (