fix(session): retry HTTP 408 request timeouts

[Hotragn]

Issue for this PR

Closes #39221

Type of change

• Bug fix

What does this PR do?

retryable() only bypasses the provider SDK's isRetryable flag for status >= 500. An HTTP 408 that the SDK didn't mark retryable therefore falls through and ends the turn, so the user has to resend the prompt by hand. This shows up with OpenAI-compatible proxies that normalize an aborted upstream stream into 408 request_timeout.

408 Request Timeout is the one 4xx that is transient in the same way a 5xx is — the request never completed, so sending it again is the defined behavior for that status. This adds 408 to the existing condition, so it takes exactly the same path as 5xx: same classification, same backoff, same retry-after handling. No new retry mechanism, and other 4xx statuses are unchanged.

How did you verify your code works?

Added retries 408 request timeout errors to test/session/retry.test.ts, built from the payload in the issue (status 408, isRetryable: false, code: request_timeout), asserting it is now classified retryable. Ran the whole retry.test.ts suite so the neighbouring cases still hold — in particular does not retry 4xx errors when isRetryable is false (status 400) still passes, so this doesn't loosen 4xx generally. bun typecheck passes.

Checklist

• I have tested my changes locally
• I have not included unrelated changes in this PR

[github-actions]

The following comment was made by an LLM, it may be inaccurate:

Potential Duplicate Found

PR #38683: fix(session): retry transient transport failures
#38683

This PR appears related because it also addresses retrying transient failures in the session layer. Since the current PR (39413) specifically handles HTTP 408 request timeouts as a transient failure, it's worth verifying that #38683 doesn't already cover this case or if there's overlap in approach.

[Hotragn]

Different layer, and #39221 calls this out explicitly: #38683 (closes #30611) classifies raw/wrapped transport failures by walking the cause chain in MessageV2.fromError and caps transient transport retries. This one is about an API error that already carries an HTTP status — retryable() only bypasses the SDK's isRetryable flag for >= 500, so a 408 falls through. Neither 408 nor statusCode classification appears in #38683's diff.

They do land near each other in retry.ts: #38683 adds a isPermanentTransportCode guard on the line directly above the >= 500 check that I'm editing. Functionally complementary, but whichever merges second may want a trivial rebase — happy to take that side if #38683 goes first.

[fashen97]

Please merge as soon as possible.

[Hotragn]

#39221 was closed as addressed by #39391, which I'd missed — that one adds 408/409 to classifyProviderFailure in packages/ai/src/provider-error.ts.

This PR touches a different spot: retryable() in packages/opencode/src/session/retry.ts, which on dev still gates on status >= 500 only and is still wired in via session/processor.ts (SessionRetry.policy). So a 408 reaching that path is still dropped unless the SDK marked it retryable.

Happy to close this if the v1 session retry path is on its way out and only the packages/ai classification matters going forward — just let me know which you'd prefer.

[fashen97]

I opened follow-up PR #40268 for a separate failure shape observed with CLIProxyAPI: the HTTP response is 200, but the Responses SSE stream contains a top-level code: "request_timeout" event. The existing HTTP statusCode === 408 change in this PR does not see that payload because it is classified in parseStreamError() first.

In local logs, three such failures were each followed by a new stream attempt, and the session completed without a user-visible error after the retry path was fixed. #40268 references #39221 as a follow-up and intentionally does not close it.

[Hotragn]

Thanks @fashen97 — #40268 looks complementary rather than overlapping, and the two cover different classification points:

• this PR: HTTP status 408 on the response, in retryable()
• fix(session): retry top-level stream request timeouts #40268: HTTP 200 with a top-level code: "request_timeout" in the SSE body, in the stream-error parser

Your version sets isRetryable: true on the resulting APIError, so it flows through the existing retry path regardless of the status-based guard here — the two shouldn't interact badly.

On merge order: we both touch test/session/retry.test.ts, but in different blocks — mine adds to describe("session.retry.retryable") around L231, yours to describe("session.message-v2.fromError") at the end of the file — so they should merge cleanly either way.

Worth noting for whoever picks these up: #39221 was closed as addressed by #39391, which fixed 408/409 in packages/ai (v2). Both of these PRs target the v1 packages/opencode/src/session path, which still classifies these as non-retryable on dev.