diff --git a/gui/src/api.ts b/gui/src/api.ts index fd6a777bb..664cde6a9 100644 --- a/gui/src/api.ts +++ b/gui/src/api.ts @@ -1,5 +1,12 @@ let installed = false; +/** Shared 401 refresh gate — concurrent waiters join one prompt / token resolution. */ let promptInFlight: Promise | null = null; +/** + * After the user cancels (or submits blank) once, suppress further prompts for this page + * lifetime so a staggered 401 fan-out does not reopen the dialog N times (#647 / Codex). + * A full reload clears module state and allows prompting again. + */ +let promptCancelled = false; function needsApiAuth(input: RequestInfo | URL): boolean { try { @@ -31,6 +38,11 @@ function clearToken(): void { memoryToken = null; } +/** Clear memory only when it still holds `expected` (avoid wiping a newer concurrent store). */ +function clearTokenIfCurrent(expected: string | null): void { + if (expected != null && readToken() === expected) clearToken(); +} + function clearLegacySessionToken(): void { try { sessionStorage.removeItem(LEGACY_TOKEN_KEY); @@ -46,11 +58,31 @@ function withToken(input: RequestInfo | URL, init: RequestInit | undefined, toke return [input, { ...init, headers }]; } -async function promptForToken(): Promise { +/** + * Resolve a token after a 401. Concurrent callers share one in-flight resolution so a dashboard + * fan-out does not open one window.prompt per /api request (#647). Re-reads memoryToken before + * prompting so waiters that wake after another request already stored a token do not re-prompt. + */ +async function resolveTokenAfter401(failedToken: string | null): Promise { + if (promptCancelled) return null; if (promptInFlight) return promptInFlight; - promptInFlight = Promise.resolve() - .then(() => window.prompt("OpenCodex API token")?.trim() || null) - .finally(() => { promptInFlight = null; }); + + promptInFlight = (async () => { + if (promptCancelled) return null; + const current = readToken(); + if (current && current !== failedToken) return current; + + const prompted = window.prompt("OpenCodex API token")?.trim() || null; + if (prompted) { + storeToken(prompted); + return prompted; + } + promptCancelled = true; + return null; + })().finally(() => { + promptInFlight = null; + }); + return promptInFlight; } @@ -68,14 +100,23 @@ export function installApiAuthFetch(): void { const response = await originalFetch(firstInput, firstInit); if (response.status !== 401) return response; - if (token) clearToken(); - const nextToken = await promptForToken(); + // Another request may have stored a token while this one was in flight (or while prompt blocked). + const refreshed = readToken(); + if (refreshed && refreshed !== token) { + const [retryInput, retryInit] = withToken(input, init, refreshed); + const retry = await originalFetch(retryInput, retryInit); + if (retry.status !== 401) return retry; + clearTokenIfCurrent(refreshed); + } else { + clearTokenIfCurrent(token); + } + + const nextToken = await resolveTokenAfter401(token); if (!nextToken) return response; - storeToken(nextToken); const [retryInput, retryInit] = withToken(input, init, nextToken); const retry = await originalFetch(retryInput, retryInit); - if (retry.status === 401) clearToken(); + if (retry.status === 401) clearTokenIfCurrent(nextToken); return retry; }; } @@ -85,4 +126,5 @@ export function resetApiAuthFetchForTests(): void { installed = false; memoryToken = null; promptInFlight = null; + promptCancelled = false; } diff --git a/gui/tests/api-auth-memory.test.ts b/gui/tests/api-auth-memory.test.ts index 4d27d8600..3bc008d70 100644 --- a/gui/tests/api-auth-memory.test.ts +++ b/gui/tests/api-auth-memory.test.ts @@ -113,6 +113,172 @@ test("cross-origin /api/* requests do not receive the API key or token prompt", expect(promptCalls).toBe(beforeCrossPrompts); }); +test("concurrent 401s share one token prompt and all retry with the stored token", async () => { + // Repro for #647: many /api/* requests start without a token (dashboard fan-out). + // Delivering 401s one-by-one after each auth cycle finishes matches the browser case where + // window.prompt blocks the main thread: each continuation still holds a captured null token + // and must reuse the in-memory token from an earlier request instead of prompting again. + let promptCalls = 0; + const release401: Array<() => void> = []; + const mockFetch = (async (_input: RequestInfo | URL, init?: RequestInit) => { + const headers = new Headers(init?.headers); + if (headers.get("X-OpenCodex-API-Key") === "shared-token") { + return new Response("{}", { status: 200 }); + } + await new Promise((resolve) => { + release401.push(resolve); + }); + return new Response("unauthorized", { status: 401 }); + }) as typeof fetch; + window.prompt = () => { + promptCalls += 1; + return "shared-token"; + }; + await installMockAuthFetch(mockFetch); + + const endpoints = [ + "/api/config", + "/api/providers", + "/api/models", + "/api/selected-models", + "/api/disabled-models", + "/api/effort-caps", + "/api/sidecar-settings", + "/api/injection-model", + "/api/v2", + "/api/keys", + "/api/provider-presets", + "/api/key-providers", + "/api/oauth/providers", + "/api/codex-auth/accounts", + ]; + const pending = endpoints.map((path) => fetch(path).then((r) => r.status)); + // Let every request reach the 401 gate before any response is delivered. + for (let i = 0; i < 20 && release401.length < endpoints.length; i += 1) { + await Promise.resolve(); + } + expect(release401.length).toBe(endpoints.length); + + for (let i = 0; i < endpoints.length; i += 1) { + const done = pending[i]!; + let settled = false; + void done.then(() => { + settled = true; + }); + release401.shift()!(); + for (let spin = 0; spin < 50 && !settled; spin += 1) { + await Promise.resolve(); + } + expect(settled).toBe(true); + } + + const statuses = await Promise.all(pending); + expect(promptCalls).toBe(1); + expect([...new Set(statuses)]).toEqual([200]); +}); + +test("stale concurrent 401 does not clear a token refreshed by another request", async () => { + // Codex/CodeRabbit race: request A prompts and stores T2; request B still holding stale T1 + // must not wipe T2 (clearTokenIfCurrent) before its re-read / shared gate join. + let promptCalls = 0; + let acceptV1 = true; + const release401: Array<() => void> = []; + const mockFetch = (async (_input: RequestInfo | URL, init?: RequestInit) => { + const headers = new Headers(init?.headers); + const key = headers.get("X-OpenCodex-API-Key"); + if (key === "token-v2") return new Response("{}", { status: 200 }); + if (acceptV1 && key === "token-v1") return new Response("{}", { status: 200 }); + if (key === "token-v1") { + await new Promise((resolve) => { + release401.push(resolve); + }); + return new Response("unauthorized", { status: 401 }); + } + return new Response("unauthorized", { status: 401 }); + }) as typeof fetch; + window.prompt = () => { + promptCalls += 1; + return "token-v1"; + }; + await installMockAuthFetch(mockFetch); + expect((await fetch("/api/config")).status).toBe(200); + expect(promptCalls).toBe(1); + + acceptV1 = false; + promptCalls = 0; + window.prompt = () => { + promptCalls += 1; + return "token-v2"; + }; + + const pending = [fetch("/api/config"), fetch("/api/providers")].map((p) => p.then((r) => r.status)); + for (let i = 0; i < 20 && release401.length < 2; i += 1) { + await Promise.resolve(); + } + expect(release401.length).toBe(2); + + for (let i = 0; i < 2; i += 1) { + const done = pending[i]!; + let settled = false; + void done.then(() => { + settled = true; + }); + release401.shift()!(); + for (let spin = 0; spin < 50 && !settled; spin += 1) { + await Promise.resolve(); + } + expect(settled).toBe(true); + } + + const statuses = await Promise.all(pending); + expect(promptCalls).toBe(1); + expect([...new Set(statuses)]).toEqual([200]); +}); + +test("canceling the token prompt once does not reopen it for the rest of the 401 fan-out", async () => { + let promptCalls = 0; + const release401: Array<() => void> = []; + const mockFetch = (async (_input: RequestInfo | URL, init?: RequestInit) => { + const headers = new Headers(init?.headers); + if (headers.get("X-OpenCodex-API-Key")) { + return new Response("{}", { status: 200 }); + } + await new Promise((resolve) => { + release401.push(resolve); + }); + return new Response("unauthorized", { status: 401 }); + }) as typeof fetch; + window.prompt = () => { + promptCalls += 1; + return null; + }; + await installMockAuthFetch(mockFetch); + + const endpoints = ["/api/config", "/api/providers", "/api/models", "/api/keys"]; + const pending = endpoints.map((path) => fetch(path).then((r) => r.status)); + for (let i = 0; i < 20 && release401.length < endpoints.length; i += 1) { + await Promise.resolve(); + } + expect(release401.length).toBe(endpoints.length); + + for (let i = 0; i < endpoints.length; i += 1) { + const done = pending[i]!; + let settled = false; + void done.then(() => { + settled = true; + }); + release401.shift()!(); + for (let spin = 0; spin < 50 && !settled; spin += 1) { + await Promise.resolve(); + } + expect(settled).toBe(true); + } + + const statuses = await Promise.all(pending); + expect(promptCalls).toBe(1); + expect([...new Set(statuses)]).toEqual([401]); +}); + test("cross-origin /v1/* requests do not receive the API key or token prompt", async () => { let promptCalls = 0; let phase: "seed" | "cross" = "seed";