Skip to content

Commit c794331

Browse files
committed
fix(gui): scope OAuth login generation per provider + abort orphan Codex flows
P2-1: Convert oauthLoginGenerationRef from global counter to provider-keyed Map so canceling/starting provider B does not orphan provider A's in-flight OAuth flow. Generation checks and busy/loginInfo clears are now scoped. P2-2: Add AbortController to AddCodexAccountModal's login start fetch so early close aborts the pending request. Check aliveRef after fetch resolves before installing polling interval/timeout. Guard the done-branch onAdded callback with aliveRef to prevent stale callbacks after unmount.
1 parent 25f3ded commit c794331

7 files changed

Lines changed: 42 additions & 54 deletions

File tree

gui/src/components/AddCodexAccountModal.tsx

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ export default function AddCodexAccountModal({
2121
const pollRef = useRef<ReturnType<typeof setInterval> | null>(null);
2222
const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
2323
const flowRef = useRef<string | null>(null);
24+
const loginAbortRef = useRef<AbortController | null>(null);
2425
/** Ensures reauth auto-start runs once per account id, even if startOAuth identity changes. */
2526
const startedReauthRef = useRef<string | null>(null);
2627
const onAddedRef = useRef(onAdded);
@@ -45,6 +46,8 @@ export default function AddCodexAccountModal({
4546
flowRef.current = null;
4647
setAuthUrl("");
4748
stopPolling();
49+
loginAbortRef.current?.abort();
50+
loginAbortRef.current = null;
4851
if (!flowId) return;
4952
await fetch(`${apiBase}/api/codex-auth/login/cancel`, {
5053
method: "POST",
@@ -55,6 +58,8 @@ export default function AddCodexAccountModal({
5558

5659
useEffect(() => () => {
5760
aliveRef.current = false;
61+
loginAbortRef.current?.abort();
62+
loginAbortRef.current = null;
5863
const flowId = flowRef.current;
5964
flowRef.current = null;
6065
if (pollRef.current) { clearInterval(pollRef.current); pollRef.current = null; }
@@ -75,10 +80,14 @@ export default function AddCodexAccountModal({
7580
}, [step, cancelLogin]);
7681

7782
const startOAuth = useCallback(async (requestedId?: string) => {
83+
const controller = new AbortController();
84+
loginAbortRef.current?.abort();
85+
loginAbortRef.current = controller;
7886
setError("");
7987
try {
8088
const accountId = reauthAccountId ?? requestedId?.trim() ?? "";
8189
const resp = await fetch(`${apiBase}/api/codex-auth/login`, {
90+
signal: controller.signal,
8291
method: "POST",
8392
headers: { "Content-Type": "application/json" },
8493
body: JSON.stringify(
@@ -88,6 +97,7 @@ export default function AddCodexAccountModal({
8897
),
8998
});
9099
const data = await resp.json() as { url?: string; flowId?: string; error?: string; status?: string };
100+
if (!aliveRef.current) return;
91101
if (resp.status === 409) {
92102
setError(t("codexAuth.oauthAlreadyInProgress"));
93103
return;
@@ -108,6 +118,7 @@ export default function AddCodexAccountModal({
108118
if (st.status === "done") {
109119
stopPolling();
110120
flowRef.current = null;
121+
if (!aliveRef.current) return;
111122
onAddedRef.current();
112123
onCloseRef.current();
113124
} else if (st.status === "error" || st.status === "expired") {
@@ -131,7 +142,9 @@ export default function AddCodexAccountModal({
131142
}, 300_000);
132143
}
133144
if (data.error && !data.url) setError(data.error);
134-
} catch (e) { setError(String(e)); }
145+
} catch (e) {
146+
if (aliveRef.current && !(e instanceof Error && e.name === "AbortError")) setError(String(e));
147+
}
135148
}, [apiBase, cancelLogin, reauthAccountId, stopPolling, t]);
136149

137150
useEffect(() => {

gui/src/components/provider-catalog/ProviderCatalog.tsx

Lines changed: 12 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,17 @@
11
/**
22
* ProviderCatalog — the browse surface of the add-provider modal: Accounts /
3-
* Free / Paid tabs, home (top rows) and browse (search) views, and account
4-
* login rows on the Accounts tab. Presentational: presets/usage arrive via
5-
* props; view state (tab, query, view) lives here; selection lifts up.
3+
* Free / Paid tabs over a single searchable scroll list, and account login
4+
* rows on the Accounts tab. Presentational: presets/usage arrive via props;
5+
* view state (tab, query) lives here; selection lifts up.
66
*/
7-
import { useMemo, useRef, useState } from "react";
7+
import { useMemo, useState } from "react";
88
import { useT } from "../../i18n";
99
import {
1010
bucketPresets,
1111
filterPresets,
1212
type CatalogPreset,
1313
} from "./provider-presets";
1414

15-
/** How many rows the home view shows per tab before "browse all". */
16-
const HOME_SLOT_COUNT = 6;
17-
1815
export type AccountLoginStatus = { loggedIn: boolean; email?: string; error?: string };
1916
export type AccountLoginRow = {
2017
id: string;
@@ -55,9 +52,7 @@ export default function ProviderCatalog({
5552
}) {
5653
const t = useT();
5754
const [tier, setTier] = useState<CatalogTier>(initialTier);
58-
const [view, setView] = useState<"home" | "browse">("home");
5955
const [query, setQuery] = useState("");
60-
const searchRef = useRef<HTMLInputElement>(null);
6156

6257
const catalog = useMemo(() => presets.filter(p => p.id !== "custom"), [presets]);
6358

@@ -71,15 +66,7 @@ export default function ProviderCatalog({
7166

7267
const buckets = useMemo(() => bucketPresets(ranked), [ranked]);
7368
const tierList = buckets[tier];
74-
const homeList = tierList.slice(0, HOME_SLOT_COUNT);
75-
const browseList = useMemo(() => filterPresets(tierList, query), [tierList, query]);
76-
const rows = view === "browse" ? browseList : homeList;
77-
78-
const openBrowse = () => {
79-
setView("browse");
80-
setQuery("");
81-
setTimeout(() => searchRef.current?.focus(), 0);
82-
};
69+
const rows = useMemo(() => filterPresets(tierList, query), [tierList, query]);
8370

8471
const badges = (p: CatalogPreset) => {
8572
const auth = p.codexAccountMode === "direct" ? <span className="badge badge-green">{t("modal.badge.direct")}</span>
@@ -106,7 +93,7 @@ export default function ProviderCatalog({
10693
role="tab"
10794
aria-selected={tier === candidate}
10895
className={`provider-catalog-tab${tier === candidate ? " active" : ""}`}
109-
onClick={() => { setTier(candidate); setView("home"); setQuery(""); }}
96+
onClick={() => { setTier(candidate); setQuery(""); }}
11097
>
11198
{t(candidate === "accounts" ? "modal.tab.accounts" : candidate === "free" ? "modal.tab.free" : "modal.tab.paid")}
11299
</button>
@@ -120,15 +107,12 @@ export default function ProviderCatalog({
120107
</div>
121108
)}
122109

123-
{view === "browse" && (
124-
<input
125-
ref={searchRef}
126-
className="input provider-catalog-search"
127-
value={query}
128-
onChange={e => setQuery(e.target.value)}
129-
placeholder={t("modal.search")}
130-
/>
131-
)}
110+
<input
111+
className="input provider-catalog-search"
112+
value={query}
113+
onChange={e => setQuery(e.target.value)}
114+
placeholder={t("modal.search")}
115+
/>
132116

133117
<div className="provider-catalog-rows">
134118
{presetsLoading && rows.length === 0 && (
@@ -175,12 +159,6 @@ export default function ProviderCatalog({
175159
</div>
176160

177161
<div className="provider-catalog-footer">
178-
{view === "home" && tierList.length > HOME_SLOT_COUNT && (
179-
<button className="link-btn" onClick={openBrowse}>{t("modal.browseAll", { count: tierList.length })}</button>
180-
)}
181-
{view === "browse" && (
182-
<button className="link-btn" onClick={() => { setView("home"); setQuery(""); }}>{t("modal.browseBack")}</button>
183-
)}
184162
<div style={{ flex: 1 }} />
185163
{tier !== "accounts" && (
186164
<button className="link-btn" onClick={onSelectCustom}>{t("modal.notListed")}</button>

gui/src/i18n/de.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -652,8 +652,6 @@ export const de = {
652652
"modal.tab.paid": "Bezahlt",
653653
"modal.accountsHint": "Codex-Konto-Provider leiten deinen ChatGPT-Login weiter. Konten verwalten unter",
654654
"modal.accountsCodexAuthLink": "Codex Auth",
655-
"modal.browseAll": "Alle {count} Provider durchsuchen",
656-
"modal.browseBack": "Zurück zu den Top-Providern",
657655
"modal.notListed": "Provider nicht dabei? Eigenen hinzufügen",
658656
"modal.catalogLoading": "Katalog wird geladen…",
659657
"modal.accountLogin": "Anmelden",

gui/src/i18n/en.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -500,8 +500,6 @@ export const en = {
500500
"modal.tab.paid": "Paid",
501501
"modal.accountsHint": "Codex account providers forward your ChatGPT login. Manage accounts in",
502502
"modal.accountsCodexAuthLink": "Codex Auth",
503-
"modal.browseAll": "Browse all {count} providers",
504-
"modal.browseBack": "Back to top providers",
505503
"modal.notListed": "Provider not listed? Add a custom one",
506504
"modal.catalogLoading": "Loading catalog…",
507505
"modal.accountLogin": "Log in",

gui/src/i18n/ko.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -672,8 +672,6 @@ export const ko: Record<TKey, string> = {
672672
"modal.tab.paid": "유료",
673673
"modal.accountsHint": "Codex 계정 프로바이더는 ChatGPT 로그인을 전달합니다. 계정 관리는",
674674
"modal.accountsCodexAuthLink": "Codex 인증",
675-
"modal.browseAll": "프로바이더 {count}개 모두 보기",
676-
"modal.browseBack": "상위 프로바이더로 돌아가기",
677675
"modal.notListed": "찾는 프로바이더가 없나요? 직접 추가",
678676
"modal.catalogLoading": "카탈로그 불러오는 중…",
679677
"modal.accountLogin": "로그인",

gui/src/i18n/zh.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -672,8 +672,6 @@ export const zh: Record<TKey, string> = {
672672
"modal.tab.paid": "付费",
673673
"modal.accountsHint": "Codex 账户提供商会转发你的 ChatGPT 登录。账户管理请前往",
674674
"modal.accountsCodexAuthLink": "Codex 认证",
675-
"modal.browseAll": "浏览全部 {count} 个提供商",
676-
"modal.browseBack": "返回热门提供商",
677675
"modal.notListed": "没有你要的提供商?添加自定义",
678676
"modal.catalogLoading": "正在加载目录…",
679677
"modal.accountLogin": "登录",

gui/src/pages/Providers.tsx

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ export default function Providers({ apiBase }: { apiBase: string }) {
7777
const accountRequestGenerationRef = useRef<Record<string, number>>({});
7878
const switchingAccountRef = useRef<{ provider: string; accountId: string } | null>(null);
7979
const codexReauthGenerationRef = useRef(0);
80-
const oauthLoginGenerationRef = useRef(0);
80+
const oauthLoginGenerationRef = useRef<Map<string, number>>(new Map());
8181

8282
const notify = (msg: string, ok: boolean) => { setStatus(msg); setStatusOk(ok); };
8383

@@ -377,7 +377,8 @@ export default function Providers({ apiBase }: { apiBase: string }) {
377377
};
378378

379379
const cancelLoginOAuth = useCallback(async (provider: string) => {
380-
oauthLoginGenerationRef.current += 1;
380+
const gen = (oauthLoginGenerationRef.current.get(provider) ?? 0) + 1;
381+
oauthLoginGenerationRef.current.set(provider, gen);
381382
try {
382383
await fetch(`${apiBase}/api/oauth/login/cancel`, {
383384
method: "POST",
@@ -386,15 +387,19 @@ export default function Providers({ apiBase }: { apiBase: string }) {
386387
});
387388
} catch { /* ignore */ }
388389
if (!aliveRef.current) return;
389-
setBusy(current => current === provider ? null : current);
390-
setLoginInfo(current => current?.provider === provider ? null : current);
390+
if (oauthLoginGenerationRef.current.get(provider) === gen) {
391+
setBusy(current => current === provider ? null : current);
392+
setLoginInfo(current => current?.provider === provider ? null : current);
393+
}
391394
setManualCode("");
392395
setManualCodeMsg("");
393396
notify(t("prov.loginCancelled", { provider: oauthLabel(provider) }), false);
394397
}, [apiBase, t]);
395398

396399
const loginOAuth = async (provider: string, addAccount = false, accountId?: string) => {
397-
const generation = ++oauthLoginGenerationRef.current;
400+
const nextGen = (oauthLoginGenerationRef.current.get(provider) ?? 0) + 1;
401+
oauthLoginGenerationRef.current.set(provider, nextGen);
402+
const generation = nextGen;
398403
const reauthTargetId = accountId?.trim() || undefined;
399404
setBusy(provider);
400405
setStatus("");
@@ -412,17 +417,17 @@ export default function Providers({ apiBase }: { apiBase: string }) {
412417
}),
413418
});
414419
const data = await res.json();
415-
if (oauthLoginGenerationRef.current !== generation || !aliveRef.current) return;
420+
if (oauthLoginGenerationRef.current.get(provider) !== generation || !aliveRef.current) return;
416421
if (!res.ok) { notify(data.error || t("prov.loginFailStart", { provider: oauthLabel(provider) }), false); return; }
417422
// The server opens the browser itself (popup-safe). Show the URL + paste fallback.
418423
if (data.url || data.instructions) setLoginInfo({ provider, url: data.url, instructions: data.instructions });
419424
const baselineCount = accountSets[provider]?.accounts.length ?? 0;
420425
// Poll until the loopback callback (or device flow / manual paste) completes.
421426
// Prefer s.done so cancel/timeout/error clear "waiting for browser" instead of hanging.
422427
let finished = false;
423-
for (let i = 0; i < 150 && aliveRef.current && oauthLoginGenerationRef.current === generation; i++) {
428+
for (let i = 0; i < 150 && aliveRef.current && oauthLoginGenerationRef.current.get(provider) === generation; i++) {
424429
await new Promise(r => setTimeout(r, 2000));
425-
if (oauthLoginGenerationRef.current !== generation || !aliveRef.current) return;
430+
if (oauthLoginGenerationRef.current.get(provider) !== generation || !aliveRef.current) return;
426431
const s: (OAuthStatus & { accounts?: OAuthAccount[] }) | null = await fetch(`${apiBase}/api/oauth/status?provider=${provider}`).then(r => r.json()).catch(() => null);
427432
if (!s) continue;
428433
if (s.error) {
@@ -471,7 +476,7 @@ export default function Providers({ apiBase }: { apiBase: string }) {
471476
break;
472477
}
473478
}
474-
if (!finished && oauthLoginGenerationRef.current === generation && aliveRef.current) {
479+
if (!finished && oauthLoginGenerationRef.current.get(provider) === generation && aliveRef.current) {
475480
// Browser abandoned / never completed — stop waiting and cancel the server flow.
476481
await fetch(`${apiBase}/api/oauth/login/cancel`, {
477482
method: "POST",
@@ -482,11 +487,11 @@ export default function Providers({ apiBase }: { apiBase: string }) {
482487
setLoginInfo(null);
483488
}
484489
} catch {
485-
if (oauthLoginGenerationRef.current === generation) {
490+
if (oauthLoginGenerationRef.current.get(provider) === generation) {
486491
notify(t("prov.loginRequestFail", { provider: oauthLabel(provider) }), false);
487492
}
488493
} finally {
489-
if (aliveRef.current && oauthLoginGenerationRef.current === generation) setBusy(null);
494+
if (aliveRef.current && oauthLoginGenerationRef.current.get(provider) === generation) setBusy(null);
490495
}
491496
};
492497

0 commit comments

Comments
 (0)