Skip to content

Commit 2d1d4bf

Browse files
authored
fix(gui): carry dashboard token prompt dedupe onto dig2-go (#651)
Carries #651. GUI-only — no Go counterpart.
1 parent 9f2f4ae commit 2d1d4bf

2 files changed

Lines changed: 216 additions & 8 deletions

File tree

gui/src/api.ts

Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
let installed = false;
2+
/** Shared 401 refresh gate — concurrent waiters join one prompt / token resolution. */
23
let promptInFlight: Promise<string | null> | null = null;
4+
/**
5+
* After the user cancels (or submits blank) once, suppress further prompts for this page
6+
* lifetime so a staggered 401 fan-out does not reopen the dialog N times (#647 / Codex).
7+
* A full reload clears module state and allows prompting again.
8+
*/
9+
let promptCancelled = false;
310

411
function needsApiAuth(input: RequestInfo | URL): boolean {
512
try {
@@ -31,6 +38,11 @@ function clearToken(): void {
3138
memoryToken = null;
3239
}
3340

41+
/** Clear memory only when it still holds `expected` (avoid wiping a newer concurrent store). */
42+
function clearTokenIfCurrent(expected: string | null): void {
43+
if (expected != null && readToken() === expected) clearToken();
44+
}
45+
3446
function clearLegacySessionToken(): void {
3547
try {
3648
sessionStorage.removeItem(LEGACY_TOKEN_KEY);
@@ -46,11 +58,31 @@ function withToken(input: RequestInfo | URL, init: RequestInit | undefined, toke
4658
return [input, { ...init, headers }];
4759
}
4860

49-
async function promptForToken(): Promise<string | null> {
61+
/**
62+
* Resolve a token after a 401. Concurrent callers share one in-flight resolution so a dashboard
63+
* fan-out does not open one window.prompt per /api request (#647). Re-reads memoryToken before
64+
* prompting so waiters that wake after another request already stored a token do not re-prompt.
65+
*/
66+
async function resolveTokenAfter401(failedToken: string | null): Promise<string | null> {
67+
if (promptCancelled) return null;
5068
if (promptInFlight) return promptInFlight;
51-
promptInFlight = Promise.resolve()
52-
.then(() => window.prompt("OpenCodex API token")?.trim() || null)
53-
.finally(() => { promptInFlight = null; });
69+
70+
promptInFlight = (async () => {
71+
if (promptCancelled) return null;
72+
const current = readToken();
73+
if (current && current !== failedToken) return current;
74+
75+
const prompted = window.prompt("OpenCodex API token")?.trim() || null;
76+
if (prompted) {
77+
storeToken(prompted);
78+
return prompted;
79+
}
80+
promptCancelled = true;
81+
return null;
82+
})().finally(() => {
83+
promptInFlight = null;
84+
});
85+
5486
return promptInFlight;
5587
}
5688

@@ -68,14 +100,23 @@ export function installApiAuthFetch(): void {
68100
const response = await originalFetch(firstInput, firstInit);
69101
if (response.status !== 401) return response;
70102

71-
if (token) clearToken();
72-
const nextToken = await promptForToken();
103+
// Another request may have stored a token while this one was in flight (or while prompt blocked).
104+
const refreshed = readToken();
105+
if (refreshed && refreshed !== token) {
106+
const [retryInput, retryInit] = withToken(input, init, refreshed);
107+
const retry = await originalFetch(retryInput, retryInit);
108+
if (retry.status !== 401) return retry;
109+
clearTokenIfCurrent(refreshed);
110+
} else {
111+
clearTokenIfCurrent(token);
112+
}
113+
114+
const nextToken = await resolveTokenAfter401(token);
73115
if (!nextToken) return response;
74116

75-
storeToken(nextToken);
76117
const [retryInput, retryInit] = withToken(input, init, nextToken);
77118
const retry = await originalFetch(retryInput, retryInit);
78-
if (retry.status === 401) clearToken();
119+
if (retry.status === 401) clearTokenIfCurrent(nextToken);
79120
return retry;
80121
};
81122
}
@@ -85,4 +126,5 @@ export function resetApiAuthFetchForTests(): void {
85126
installed = false;
86127
memoryToken = null;
87128
promptInFlight = null;
129+
promptCancelled = false;
88130
}

gui/tests/api-auth-memory.test.ts

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,172 @@ test("cross-origin /api/* requests do not receive the API key or token prompt",
113113
expect(promptCalls).toBe(beforeCrossPrompts);
114114
});
115115

116+
test("concurrent 401s share one token prompt and all retry with the stored token", async () => {
117+
// Repro for #647: many /api/* requests start without a token (dashboard fan-out).
118+
// Delivering 401s one-by-one after each auth cycle finishes matches the browser case where
119+
// window.prompt blocks the main thread: each continuation still holds a captured null token
120+
// and must reuse the in-memory token from an earlier request instead of prompting again.
121+
let promptCalls = 0;
122+
const release401: Array<() => void> = [];
123+
const mockFetch = (async (_input: RequestInfo | URL, init?: RequestInit) => {
124+
const headers = new Headers(init?.headers);
125+
if (headers.get("X-OpenCodex-API-Key") === "shared-token") {
126+
return new Response("{}", { status: 200 });
127+
}
128+
await new Promise<void>((resolve) => {
129+
release401.push(resolve);
130+
});
131+
return new Response("unauthorized", { status: 401 });
132+
}) as typeof fetch;
133+
window.prompt = () => {
134+
promptCalls += 1;
135+
return "shared-token";
136+
};
137+
await installMockAuthFetch(mockFetch);
138+
139+
const endpoints = [
140+
"/api/config",
141+
"/api/providers",
142+
"/api/models",
143+
"/api/selected-models",
144+
"/api/disabled-models",
145+
"/api/effort-caps",
146+
"/api/sidecar-settings",
147+
"/api/injection-model",
148+
"/api/v2",
149+
"/api/keys",
150+
"/api/provider-presets",
151+
"/api/key-providers",
152+
"/api/oauth/providers",
153+
"/api/codex-auth/accounts",
154+
];
155+
const pending = endpoints.map((path) => fetch(path).then((r) => r.status));
156+
// Let every request reach the 401 gate before any response is delivered.
157+
for (let i = 0; i < 20 && release401.length < endpoints.length; i += 1) {
158+
await Promise.resolve();
159+
}
160+
expect(release401.length).toBe(endpoints.length);
161+
162+
for (let i = 0; i < endpoints.length; i += 1) {
163+
const done = pending[i]!;
164+
let settled = false;
165+
void done.then(() => {
166+
settled = true;
167+
});
168+
release401.shift()!();
169+
for (let spin = 0; spin < 50 && !settled; spin += 1) {
170+
await Promise.resolve();
171+
}
172+
expect(settled).toBe(true);
173+
}
174+
175+
const statuses = await Promise.all(pending);
176+
expect(promptCalls).toBe(1);
177+
expect([...new Set(statuses)]).toEqual([200]);
178+
});
179+
180+
test("stale concurrent 401 does not clear a token refreshed by another request", async () => {
181+
// Codex/CodeRabbit race: request A prompts and stores T2; request B still holding stale T1
182+
// must not wipe T2 (clearTokenIfCurrent) before its re-read / shared gate join.
183+
let promptCalls = 0;
184+
let acceptV1 = true;
185+
const release401: Array<() => void> = [];
186+
const mockFetch = (async (_input: RequestInfo | URL, init?: RequestInit) => {
187+
const headers = new Headers(init?.headers);
188+
const key = headers.get("X-OpenCodex-API-Key");
189+
if (key === "token-v2") return new Response("{}", { status: 200 });
190+
if (acceptV1 && key === "token-v1") return new Response("{}", { status: 200 });
191+
if (key === "token-v1") {
192+
await new Promise<void>((resolve) => {
193+
release401.push(resolve);
194+
});
195+
return new Response("unauthorized", { status: 401 });
196+
}
197+
return new Response("unauthorized", { status: 401 });
198+
}) as typeof fetch;
199+
window.prompt = () => {
200+
promptCalls += 1;
201+
return "token-v1";
202+
};
203+
await installMockAuthFetch(mockFetch);
204+
expect((await fetch("/api/config")).status).toBe(200);
205+
expect(promptCalls).toBe(1);
206+
207+
acceptV1 = false;
208+
promptCalls = 0;
209+
window.prompt = () => {
210+
promptCalls += 1;
211+
return "token-v2";
212+
};
213+
214+
const pending = [fetch("/api/config"), fetch("/api/providers")].map((p) => p.then((r) => r.status));
215+
for (let i = 0; i < 20 && release401.length < 2; i += 1) {
216+
await Promise.resolve();
217+
}
218+
expect(release401.length).toBe(2);
219+
220+
for (let i = 0; i < 2; i += 1) {
221+
const done = pending[i]!;
222+
let settled = false;
223+
void done.then(() => {
224+
settled = true;
225+
});
226+
release401.shift()!();
227+
for (let spin = 0; spin < 50 && !settled; spin += 1) {
228+
await Promise.resolve();
229+
}
230+
expect(settled).toBe(true);
231+
}
232+
233+
const statuses = await Promise.all(pending);
234+
expect(promptCalls).toBe(1);
235+
expect([...new Set(statuses)]).toEqual([200]);
236+
});
237+
238+
test("canceling the token prompt once does not reopen it for the rest of the 401 fan-out", async () => {
239+
let promptCalls = 0;
240+
const release401: Array<() => void> = [];
241+
const mockFetch = (async (_input: RequestInfo | URL, init?: RequestInit) => {
242+
const headers = new Headers(init?.headers);
243+
if (headers.get("X-OpenCodex-API-Key")) {
244+
return new Response("{}", { status: 200 });
245+
}
246+
await new Promise<void>((resolve) => {
247+
release401.push(resolve);
248+
});
249+
return new Response("unauthorized", { status: 401 });
250+
}) as typeof fetch;
251+
window.prompt = () => {
252+
promptCalls += 1;
253+
return null;
254+
};
255+
await installMockAuthFetch(mockFetch);
256+
257+
const endpoints = ["/api/config", "/api/providers", "/api/models", "/api/keys"];
258+
const pending = endpoints.map((path) => fetch(path).then((r) => r.status));
259+
for (let i = 0; i < 20 && release401.length < endpoints.length; i += 1) {
260+
await Promise.resolve();
261+
}
262+
expect(release401.length).toBe(endpoints.length);
263+
264+
for (let i = 0; i < endpoints.length; i += 1) {
265+
const done = pending[i]!;
266+
let settled = false;
267+
void done.then(() => {
268+
settled = true;
269+
});
270+
release401.shift()!();
271+
for (let spin = 0; spin < 50 && !settled; spin += 1) {
272+
await Promise.resolve();
273+
}
274+
expect(settled).toBe(true);
275+
}
276+
277+
const statuses = await Promise.all(pending);
278+
expect(promptCalls).toBe(1);
279+
expect([...new Set(statuses)]).toEqual([401]);
280+
});
281+
116282
test("cross-origin /v1/* requests do not receive the API key or token prompt", async () => {
117283
let promptCalls = 0;
118284
let phase: "seed" | "cross" = "seed";

0 commit comments

Comments
 (0)