Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion packages/opencode/src/plugin/codex.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ const log = Log.create({ service: "plugin.codex" })
const CLIENT_ID = "app_EMoamEEZ73f0CkXaXp7hrann"
const ISSUER = "https://auth.openai.com"
const CODEX_API_ENDPOINT = "https://chatgpt.com/backend-api/codex/responses"
const CODEX_REQUEST_TIMEOUT = 30_000
const CODEX_CHUNK_TIMEOUT = 120_000
const OAUTH_PORT = 1455
const OAUTH_POLLING_SAFETY_MARGIN_MS = 3000
const ALLOWED_MODELS = new Set([
Expand Down Expand Up @@ -141,21 +143,35 @@ async function exchangeCodeForTokens(code: string, redirectUri: string, pkce: Pk
}

async function refreshAccessToken(refreshToken: string, issuer = ISSUER): Promise<TokenResponse> {
const timeout = timeoutController(CODEX_REQUEST_TIMEOUT)
const response = await fetch(`${issuer}/oauth/token`, {
method: "POST",
signal: timeout.signal,
headers: { "Content-Type": "application/x-www-form-urlencoded" },
body: new URLSearchParams({
grant_type: "refresh_token",
refresh_token: refreshToken,
client_id: CLIENT_ID,
}).toString(),
})
}).finally(() => timeout.clear())
if (!response.ok) {
throw new Error(`Token refresh failed: ${response.status}`)
}
return response.json()
}

function timeoutController(ms: number) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, I was looking at some of the other plugins (like github-copilot, etc.) and noticed they also make custom fetch calls during their specific OAuth or authentication flows.

Since these custom authentication fetches aren't wrapped by the new main provider timeout, are they still vulnerable to infinite socket hangs if their respective APIs stall during login? Should we consider abstracting this timeoutController logic so it can be easily applied to all custom provider token/auth fetches?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@niStee Good catch. I think you’re right.

The new provider timeout protects the actual provider request path, including custom fetch once it’s invoked by the wrapper, but direct auth/token fetch calls inside plugins are outside that protection. So a stalled OAuth/token endpoint can still hang unless that specific call receives/uses a signal or creates its own timeout.

I’d prefer not to broaden this PR too much beyond the Codex timeout bug, but a shared helper for bounded auth fetches makes sense. I can either extract that helper here, or keep this focused and follow up with a small PR applying it across the custom auth flows.

const ctl = new AbortController()
const id = setTimeout(
() => ctl.abort(new DOMException(`Codex request timed out after ${ms}ms`, "TimeoutError")),
ms,
)
return {
signal: ctl.signal,
clear: () => clearTimeout(id),
}
}

const HTML_SUCCESS = `<!doctype html>
<html>
<head>
Expand Down Expand Up @@ -421,6 +437,8 @@ export async function CodexAuthPlugin(input: PluginInput, options: CodexAuthPlug

return {
apiKey: OAUTH_DUMMY_KEY,
timeout: CODEX_REQUEST_TIMEOUT,
chunkTimeout: CODEX_CHUNK_TIMEOUT,
async fetch(requestInput: RequestInfo | URL, init?: RequestInit) {
// Remove dummy API key authorization header
if (init?.headers) {
Expand Down
20 changes: 17 additions & 3 deletions packages/opencode/src/provider/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { ModelStatus } from "./model-status"
import { RuntimeFlags } from "@/effect/runtime-flags"

const log = Log.create({ service: "provider" })
const PROVIDER_TIMEOUT_DEFAULT = 300_000

function shouldUseCopilotResponsesApi(modelID: string): boolean {
const match = /^gpt-(\d+)/.exec(modelID)
Expand Down Expand Up @@ -85,6 +86,18 @@ function wrapSSE(res: Response, ms: number, ctl: AbortController) {
})
}

function timeoutController(ms: number) {
const ctl = new AbortController()
const id = setTimeout(
() => ctl.abort(new DOMException(`Provider request timed out after ${ms}ms`, "TimeoutError")),
ms,
)
return {
signal: ctl.signal,
clear: () => clearTimeout(id),
}
}

function googleVertexAnthropicBaseURL(project: string | undefined, location: string | undefined) {
if (!project) return
if (location !== "eu" && location !== "us") return
Expand Down Expand Up @@ -1607,12 +1620,13 @@ export const layer = Layer.effect(
const fetchFn = customFetch ?? fetch
const opts = init ?? {}
const chunkAbortCtl = typeof chunkTimeout === "number" && chunkTimeout > 0 ? new AbortController() : undefined
const requestTimeout = options["timeout"] === false ? undefined : (options["timeout"] ?? PROVIDER_TIMEOUT_DEFAULT)
const requestTimeoutCtl = typeof requestTimeout === "number" ? timeoutController(requestTimeout) : undefined
const signals: AbortSignal[] = []

if (opts.signal) signals.push(opts.signal)
if (chunkAbortCtl) signals.push(chunkAbortCtl.signal)
if (options["timeout"] !== undefined && options["timeout"] !== null && options["timeout"] !== false)
signals.push(AbortSignal.timeout(options["timeout"]))
if (requestTimeoutCtl) signals.push(requestTimeoutCtl.signal)

const combined = signals.length === 0 ? null : signals.length === 1 ? signals[0] : AbortSignal.any(signals)
if (combined) opts.signal = combined
Expand All @@ -1639,7 +1653,7 @@ export const layer = Layer.effect(
...opts,
// @ts-ignore see here: https://github.com/oven-sh/bun/issues/16682
timeout: false,
})
}).finally(() => requestTimeoutCtl?.clear())

if (!chunkAbortCtl) return res
return wrapSSE(res, chunkTimeout, chunkAbortCtl)
Expand Down
3 changes: 3 additions & 0 deletions packages/opencode/src/session/retry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ export function retryable(error: Err, provider: string) {
const msg = isRecord(error.data) ? error.data.message : undefined
if (typeof msg === "string") {
const lower = msg.toLowerCase()
if (lower.includes("sse read timed out") || lower.includes("provider request timed out")) {
return { message: msg }
}
if (
lower.includes("rate increased too quickly") ||
lower.includes("rate limit") ||
Expand Down
8 changes: 8 additions & 0 deletions packages/opencode/test/session/retry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,14 @@ describe("session.retry.retryable", () => {
expect(SessionRetry.retryable(error, retryProvider)).toEqual({ message: msg })
})

test("retries transport timeout errors", () => {
const sse = wrap("SSE read timed out")
expect(SessionRetry.retryable(sse, retryProvider)).toEqual({ message: "SSE read timed out" })

const request = wrap("Provider request timed out after 30000ms")
expect(SessionRetry.retryable(request, retryProvider)).toEqual({ message: "Provider request timed out after 30000ms" })
})

test("does not retry context overflow errors", () => {
const error = new MessageV2.ContextOverflowError({
message: "Input exceeds context window of this model",
Expand Down
Loading