Skip to content
Merged
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
58 changes: 50 additions & 8 deletions gui/src/api.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
let installed = false;
/** Shared 401 refresh gate — concurrent waiters join one prompt / token resolution. */
let promptInFlight: Promise<string | null> | 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 {
Expand Down Expand Up @@ -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);
Expand All @@ -46,11 +58,31 @@ function withToken(input: RequestInfo | URL, init: RequestInit | undefined, toke
return [input, { ...init, headers }];
}

async function promptForToken(): Promise<string | null> {
/**
* 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<string | null> {
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;
}

Expand All @@ -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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Retry with a concurrently corrected token

When concurrent initial 401s retry a token that the user entered incorrectly, one request can clear that token while another request prompts for and stores a corrected token. Any old-token retry that then reaches this line preserves the corrected token but immediately returns its stale 401, leaving part of the dashboard fan-out failed even though authentication has recovered. After a retry returns 401, re-read the current token or rejoin the shared resolution gate before returning, with a bounded retry count to avoid loops.

Useful? React with 👍 / 👎.

return retry;
};
}
Expand All @@ -85,4 +126,5 @@ export function resetApiAuthFetchForTests(): void {
installed = false;
memoryToken = null;
promptInFlight = null;
promptCancelled = false;
}
166 changes: 166 additions & 0 deletions gui/tests/api-auth-memory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>((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<void>((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<void>((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";
Expand Down
Loading