Skip to content

Commit 803a735

Browse files
fix: add cancel button to ChatGPT login when OAuth in progress (#172)
## Summary When user starts ChatGPT OAuth login and then closes the browser without completing authorization, the loading state would stay stuck **for up to 5 minutes** because the `waitForCode` promise would not resolve/reject until timeout. During this time, the UI was disabled and user couldn't do anything. Fix this by two improvements: 1. **Add a manual Cancel button** when in loading state - user can immediately reset the UI and try login again without waiting for timeout 2. **Shorten callback timeout** from 5 minutes → 2 minutes for better UX even if user forgets to cancel ## Screenshot Before (stuck forever / 5min): ![image](https://user-images.githubusercontent.com/.../image.png) *(provided by user)* After (user can cancel): The loading state now shows a "Cancel" button next to the in-progress button. ## Testing - Click "Login with ChatGPT Plus" - Browser opens → close browser immediately - Back to app → click "Cancel" - UI resets, you can click login again ## Checklist - [x] Code is typed with TypeScript (`strict: true`) - [x] Pre-push typecheck + lint passes - [x] Fix follows project conventions (no new dependencies, minimal changes) - [x] Fix addresses the root cause (user closes browser → no code arrives → stuck) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Signed-off-by: Sun-sunshine06 <Sun-sunshine06@users.noreply.github.com> Co-authored-by: Sun-sunshine06 <Sun-sunshine06@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 3d7b74e commit 803a735

7 files changed

Lines changed: 129 additions & 17 deletions

File tree

apps/desktop/src/main/codex-oauth-ipc.test.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ describe('codex-oauth:v1:login', () => {
303303
});
304304

305305
await register();
306-
await expect(handlers.get('codex-oauth:v1:login')?.()).rejects.toThrow(/.*/);
306+
await expect(handlers.get('codex-oauth:v1:login')?.()).rejects.toThrow(/ChatGPT.*ID/);
307307
expect(closeMock).toHaveBeenCalledTimes(1);
308308
expect(writeConfigMock).not.toHaveBeenCalled();
309309

@@ -312,6 +312,30 @@ describe('codex-oauth:v1:login', () => {
312312
expect(stored).toBeNull();
313313
expect(fakeCachedConfig).toBeNull();
314314
});
315+
316+
it('aborts an in-flight login when cancel-login is invoked', async () => {
317+
waitForCodeMock.mockImplementation(
318+
async (_expectedState: string, signal?: AbortSignal) =>
319+
await new Promise<never>((_resolve, reject) => {
320+
if (signal?.aborted) {
321+
reject(new Error('Codex OAuth callback aborted'));
322+
return;
323+
}
324+
signal?.addEventListener(
325+
'abort',
326+
() => reject(new Error('Codex OAuth callback aborted')),
327+
{ once: true },
328+
);
329+
}),
330+
);
331+
332+
await register();
333+
const loginPromise = handlers.get('codex-oauth:v1:login')?.() as Promise<unknown>;
334+
await expect(handlers.get('codex-oauth:v1:cancel-login')?.()).resolves.toBe(true);
335+
await expect(loginPromise).rejects.toThrow(/Codex login cancelled/);
336+
expect(closeMock).toHaveBeenCalled();
337+
expect(writeConfigMock).not.toHaveBeenCalled();
338+
});
315339
});
316340

317341
describe('codex-oauth:v1:logout', () => {

apps/desktop/src/main/codex-oauth-ipc.ts

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,11 @@ export { CHATGPT_CODEX_PROVIDER_ID };
4040

4141
const CHATGPT_CODEX_PROVIDER: ProviderEntry = {
4242
id: CHATGPT_CODEX_PROVIDER_ID,
43-
name: 'ChatGPT 订阅',
43+
name: 'ChatGPT 璁㈤槄',
4444
builtin: false,
4545
wire: 'openai-codex-responses',
4646
// pi-ai's openai-codex-responses wire appends `/codex/responses` itself, so
47-
// we store the bare base. Do not add `/codex` here it'd produce
47+
// we store the bare base. Do not add `/codex` here 鈥?it'd produce
4848
// `/codex/codex/responses`.
4949
baseUrl: 'https://chatgpt.com/backend-api',
5050
defaultModel: 'gpt-5.3-codex',
@@ -67,6 +67,8 @@ const CHATGPT_CODEX_PROVIDER: ProviderEntry = {
6767
};
6868

6969
let tokenStoreSingleton: CodexTokenStore | null = null;
70+
let activeLoginAbortController: AbortController | null = null;
71+
let activeLoginPromise: Promise<CodexOAuthStatus> | null = null;
7072

7173
/**
7274
* Lazily instantiated module-scoped store. Shared with Task 7's generate flow
@@ -81,9 +83,11 @@ export function getCodexTokenStore(): CodexTokenStore {
8183
return tokenStoreSingleton;
8284
}
8385

84-
/** Test-only reset hook vitest resets module state between test cases. */
86+
/** Test-only reset hook 鈥?vitest resets module state between test cases. */
8587
export function __resetCodexTokenStoreForTests(): void {
8688
tokenStoreSingleton = null;
89+
activeLoginAbortController = null;
90+
activeLoginPromise = null;
8791
}
8892

8993
function extractEmail(jwt: string): string | null {
@@ -150,7 +154,7 @@ async function claimActiveProviderIfUnset(): Promise<void> {
150154
setCachedConfig(next);
151155
}
152156

153-
async function runLogin(): Promise<CodexOAuthStatus> {
157+
async function runLoginFlow(abortController: AbortController): Promise<CodexOAuthStatus> {
154158
const pkce = generatePkce();
155159
const state = randomBytes(16).toString('hex');
156160
let server: CallbackServer | null = null;
@@ -163,11 +167,11 @@ async function runLogin(): Promise<CodexOAuthStatus> {
163167
});
164168
await shell.openExternal(authorizeUrl);
165169
logger.info('codex.oauth.login.started', { redirectUri: server.redirectUri });
166-
const { code } = await server.waitForCode(state);
170+
const { code } = await server.waitForCode(state, abortController.signal);
167171
const tokenSet: TokenSet = await exchangeCode(code, pkce.verifier, server.redirectUri);
168172
if (tokenSet.accountId === null) {
169173
throw new CodesignError(
170-
'Codex 登录成功但无法读取 ChatGPT 账户 ID,请重试登录。',
174+
'Codex 鐧诲綍鎴愬姛浣嗘棤娉曡鍙?ChatGPT 璐︽埛 ID锛岃閲嶈瘯鐧诲綍銆?',
171175
ERROR_CODES.PROVIDER_ERROR,
172176
{ cause: null },
173177
);
@@ -192,6 +196,12 @@ async function runLogin(): Promise<CodexOAuthStatus> {
192196
logger.info('codex.oauth.login.ok', { accountId: stored.accountId, hasEmail: email !== null });
193197
return toStatus(stored);
194198
} catch (err) {
199+
if (abortController.signal.aborted) {
200+
logger.info('codex.oauth.login.cancelled');
201+
throw new CodesignError('Codex login cancelled', ERROR_CODES.PROVIDER_ABORTED, {
202+
cause: err,
203+
});
204+
}
195205
logger.error('codex.oauth.login.fail', {
196206
message: err instanceof Error ? err.message : String(err),
197207
});
@@ -206,6 +216,34 @@ async function runLogin(): Promise<CodexOAuthStatus> {
206216
}
207217
}
208218

219+
async function runLogin(): Promise<CodexOAuthStatus> {
220+
if (activeLoginPromise !== null) return activeLoginPromise;
221+
222+
const abortController = new AbortController();
223+
activeLoginAbortController = abortController;
224+
225+
const promise = runLoginFlow(abortController);
226+
const trackedPromise = promise.finally(() => {
227+
if (activeLoginAbortController === abortController) {
228+
activeLoginAbortController = null;
229+
}
230+
if (activeLoginPromise === trackedPromise) {
231+
activeLoginPromise = null;
232+
}
233+
});
234+
235+
activeLoginPromise = trackedPromise;
236+
return trackedPromise;
237+
}
238+
239+
async function runCancelLogin(): Promise<boolean> {
240+
if (activeLoginAbortController === null || activeLoginAbortController.signal.aborted) {
241+
return false;
242+
}
243+
activeLoginAbortController.abort();
244+
return true;
245+
}
246+
209247
async function runLogout(): Promise<CodexOAuthStatus> {
210248
await getCodexTokenStore().clear();
211249
const cfg = getCachedConfig();
@@ -230,7 +268,7 @@ async function runLogout(): Promise<CodexOAuthStatus> {
230268
* `chatgpt-codex` with Phase 1's stale `wire`/`baseUrl`, overwrite with the
231269
* Phase 2 canonical values so the first generate after upgrade works without
232270
* requiring a manual re-login. No-op when the entry is absent or already
233-
* canonical. Safe to call on every boot writes only when state diverges.
271+
* canonical. Safe to call on every boot 鈥?writes only when state diverges.
234272
*
235273
* Phase 1 released the card in "coming soon" disabled mode, so this migration
236274
* only fires for users who ran this feat branch directly; zero writes on
@@ -256,5 +294,6 @@ export async function migrateStaleCodexEntryIfNeeded(): Promise<void> {
256294
export function registerCodexOAuthIpc(): void {
257295
ipcMain.handle('codex-oauth:v1:status', async (): Promise<CodexOAuthStatus> => runStatus());
258296
ipcMain.handle('codex-oauth:v1:login', async (): Promise<CodexOAuthStatus> => runLogin());
297+
ipcMain.handle('codex-oauth:v1:cancel-login', async (): Promise<boolean> => runCancelLogin());
259298
ipcMain.handle('codex-oauth:v1:logout', async (): Promise<CodexOAuthStatus> => runLogout());
260299
}

apps/desktop/src/preload/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,7 @@ const api = {
341341
codexOAuth: {
342342
status: () => ipcRenderer.invoke('codex-oauth:v1:status') as Promise<CodexOAuthStatus>,
343343
login: () => ipcRenderer.invoke('codex-oauth:v1:login') as Promise<CodexOAuthStatus>,
344+
cancelLogin: () => ipcRenderer.invoke('codex-oauth:v1:cancel-login') as Promise<boolean>,
344345
logout: () => ipcRenderer.invoke('codex-oauth:v1:logout') as Promise<CodexOAuthStatus>,
345346
},
346347
connection: {

apps/desktop/src/renderer/src/components/ChatgptLoginCard.test.tsx

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ describe('performLogin', () => {
5757
const api = {
5858
status: vi.fn(),
5959
login: vi.fn().mockResolvedValue(next),
60+
cancelLogin: vi.fn(),
6061
logout: vi.fn(),
6162
};
6263
const setStatus = vi.fn();
@@ -85,6 +86,7 @@ describe('performLogin', () => {
8586
const api = {
8687
status: vi.fn(),
8788
login: vi.fn().mockRejectedValue(new Error('network down')),
89+
cancelLogin: vi.fn(),
8890
logout: vi.fn(),
8991
};
9092
const setStatus = vi.fn();
@@ -99,13 +101,32 @@ describe('performLogin', () => {
99101
expect.objectContaining({ variant: 'error', description: 'network down' }),
100102
);
101103
});
104+
105+
it('silently resets loading when login is cancelled by the user', async () => {
106+
const api = {
107+
status: vi.fn(),
108+
login: vi.fn().mockRejectedValue(new Error('Codex login cancelled')),
109+
cancelLogin: vi.fn(),
110+
logout: vi.fn(),
111+
};
112+
const setStatus = vi.fn();
113+
const setLoading = vi.fn();
114+
const pushToast = vi.fn();
115+
116+
await performLogin({ api, setStatus, setLoading, pushToast, strings: LOGIN_STRINGS });
117+
118+
expect(setStatus).not.toHaveBeenCalled();
119+
expect(pushToast).not.toHaveBeenCalled();
120+
expect(setLoading).toHaveBeenNthCalledWith(2, false);
121+
});
102122
});
103123

104124
describe('performLogout', () => {
105125
it('bails without calling the IPC when the confirm dialog is dismissed', async () => {
106126
const api = {
107127
status: vi.fn(),
108128
login: vi.fn(),
129+
cancelLogin: vi.fn(),
109130
logout: vi.fn().mockResolvedValue(statusLoggedOut()),
110131
};
111132
const setStatus = vi.fn();
@@ -130,6 +151,7 @@ describe('performLogout', () => {
130151
const api = {
131152
status: vi.fn(),
132153
login: vi.fn(),
154+
cancelLogin: vi.fn(),
133155
logout: vi.fn().mockResolvedValue(next),
134156
};
135157
const setStatus = vi.fn();
@@ -157,6 +179,7 @@ describe('performLogout', () => {
157179
const api = {
158180
status: vi.fn(),
159181
login: vi.fn(),
182+
cancelLogin: vi.fn(),
160183
logout: vi.fn().mockRejectedValue(new Error('revoke failed')),
161184
};
162185
const setStatus = vi.fn();
@@ -184,6 +207,7 @@ describe('performFetchStatus', () => {
184207
const api = {
185208
status: vi.fn().mockResolvedValue(next),
186209
login: vi.fn(),
210+
cancelLogin: vi.fn(),
187211
logout: vi.fn(),
188212
};
189213
const setStatus = vi.fn();
@@ -209,6 +233,7 @@ describe('performFetchStatus', () => {
209233
const api = {
210234
status: vi.fn().mockRejectedValue(new Error('IPC backend crashed')),
211235
login: vi.fn(),
236+
cancelLogin: vi.fn(),
212237
logout: vi.fn(),
213238
};
214239
const setStatus = vi.fn();
@@ -236,6 +261,7 @@ describe('performFetchStatus', () => {
236261
const api = {
237262
status: vi.fn().mockResolvedValue(statusLoggedIn()),
238263
login: vi.fn(),
264+
cancelLogin: vi.fn(),
239265
logout: vi.fn(),
240266
};
241267
const setStatus = vi.fn();
@@ -259,6 +285,7 @@ describe('performFetchStatus', () => {
259285
const api = {
260286
status: vi.fn().mockRejectedValue(new Error('boom')),
261287
login: vi.fn(),
288+
cancelLogin: vi.fn(),
262289
logout: vi.fn(),
263290
};
264291
const setStatus = vi.fn();
@@ -280,6 +307,7 @@ describe('performFetchStatus', () => {
280307
const api = {
281308
status: vi.fn().mockRejectedValue('broken string'),
282309
login: vi.fn(),
310+
cancelLogin: vi.fn(),
283311
logout: vi.fn(),
284312
};
285313
const pushToast = vi.fn();

apps/desktop/src/renderer/src/components/ChatgptLoginCard.tsx

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ export function resolveViewState(
2424
interface CodexOAuthApi {
2525
status(): Promise<CodexOAuthStatus>;
2626
login(): Promise<CodexOAuthStatus>;
27+
cancelLogin(): Promise<boolean>;
2728
logout(): Promise<CodexOAuthStatus>;
2829
}
2930

@@ -38,13 +39,19 @@ export interface PerformLoginDeps {
3839
strings: { failedTitle: string; unknownError: string };
3940
}
4041

42+
function isCodexLoginCancelledError(err: unknown): boolean {
43+
if (!(err instanceof Error)) return false;
44+
return /Codex login cancelled|Codex OAuth callback aborted/.test(err.message);
45+
}
46+
4147
export async function performLogin(deps: PerformLoginDeps): Promise<void> {
4248
deps.setLoading(true);
4349
try {
4450
const next = await deps.api.login();
4551
deps.setStatus(next);
4652
await deps.onStatusChange?.();
4753
} catch (err) {
54+
if (isCodexLoginCancelledError(err)) return;
4855
deps.pushToast({
4956
variant: 'error',
5057
title: deps.strings.failedTitle,
@@ -157,6 +164,14 @@ export function ChatgptLoginCard({ onStatusChange }: ChatgptLoginCardProps) {
157164
});
158165
}, [onStatusChange, pushToast, t]);
159166

167+
const handleCancel = useCallback(async () => {
168+
if (!window.codesign) return;
169+
const cancelled = await window.codesign.codexOAuth.cancelLogin();
170+
// When cancellation succeeds, the in-flight login promise will settle
171+
// immediately and clear loading via performLogin's finally block.
172+
if (!cancelled && mountedRef.current) setLoading(false);
173+
}, []);
174+
160175
const handleLogout = useCallback(async () => {
161176
if (!window.codesign) return;
162177
await performLogout({
@@ -211,12 +226,17 @@ export function ChatgptLoginCard({ onStatusChange }: ChatgptLoginCardProps) {
211226
{t('settings.providers.chatgptLogin.description')}
212227
</p>
213228
</div>
214-
<div className="shrink-0">
229+
<div className="shrink-0 flex items-center gap-[var(--space-2)]">
215230
{viewState === 'loading' ? (
216-
<Button variant="primary" size="sm" disabled>
217-
<Loader2 className="w-[var(--size-icon-sm)] h-[var(--size-icon-sm)] animate-spin" />
218-
{t('settings.providers.chatgptLogin.inProgress')}
219-
</Button>
231+
<>
232+
<Button variant="primary" size="sm" disabled>
233+
<Loader2 className="w-[var(--size-icon-sm)] h-[var(--size-icon-sm)] animate-spin" />
234+
{t('settings.providers.chatgptLogin.inProgress')}
235+
</Button>
236+
<Button variant="secondary" size="sm" onClick={() => void handleCancel()}>
237+
{t('common.cancel')}
238+
</Button>
239+
</>
220240
) : (
221241
<Button variant="primary" size="sm" onClick={() => void handleLogin()}>
222242
<Sparkles className="w-[var(--size-icon-sm)] h-[var(--size-icon-sm)]" />

packages/providers/src/codex/oauth-server.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,12 @@ describe('startCallbackServer', () => {
8585
await assertion;
8686
});
8787

88-
it('rejects after the 5-minute timeout', async () => {
88+
it('rejects after the 2-minute timeout', async () => {
8989
vi.useFakeTimers();
9090
const server = await track(await startCallbackServer(0));
9191
const waiter = server.waitForCode('foo');
9292
const assertion = expect(waiter).rejects.toThrow(/timeout/);
93-
await vi.advanceTimersByTimeAsync(5 * 60 * 1000 + 1000);
93+
await vi.advanceTimersByTimeAsync(2 * 60 * 1000 + 1000);
9494
await assertion;
9595
});
9696

packages/providers/src/codex/oauth-server.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ interface PendingWait {
1919
}
2020

2121
const DEFAULT_PORT = 1455;
22-
const CALLBACK_TIMEOUT_MS = 5 * 60 * 1000;
22+
const CALLBACK_TIMEOUT_MS = 2 * 60 * 1000;
2323

2424
function escapeHtml(s: string): string {
2525
return s
@@ -132,7 +132,7 @@ export async function startCallbackServer(preferredPort?: number): Promise<Callb
132132
const timeout = setTimeout(() => {
133133
settle();
134134
pending = null;
135-
reject(new Error('Codex OAuth callback timeout (5 minutes)'));
135+
reject(new Error('Codex OAuth callback timeout (2 minutes)'));
136136
}, CALLBACK_TIMEOUT_MS);
137137

138138
const onAbort = () => {

0 commit comments

Comments
 (0)