From d2ce6d8db875ba656ad20dd098b3a2a0601dd755 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 17 Sep 2026 22:57:54 -0700 Subject: [PATCH] fix(accounts): free the queue for a switch, not only for another add MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switching or removing an account shares the mutation queue an abandoned sign-in was holding, so the commonest thing a user does after giving up — pick a different account — still spun for the whole deadline while Add recovered instantly. Both now supersede, as does the Claude side. Every caller is a person: the two IPC handlers and the mobile RPC methods. No poll, sync or CLI path reaches them, and a sign-in that already wrote credentials refuses the cancel, so a switch cannot discard one that succeeded. Also from review: the Cancel button regains the gap its Claude twin has (layout is allowed by the design-system rule; only the colour override was not), and the URL subscription says what it is — registration for the process's lifetime, with no teardown to hand back. --- src/main/claude-accounts/service.ts | 7 +++++-- .../service-abandoned-login-supersede.test.ts | 18 ++++++++++++++++-- src/main/codex-accounts/service.ts | 10 +++++++--- src/main/ipc/codex-accounts.ts | 2 +- .../settings/accounts-pane-codex-section.tsx | 1 + 5 files changed, 30 insertions(+), 8 deletions(-) diff --git a/src/main/claude-accounts/service.ts b/src/main/claude-accounts/service.ts index 7991b258be9..d1c79eaf17d 100644 --- a/src/main/claude-accounts/service.ts +++ b/src/main/claude-accounts/service.ts @@ -93,10 +93,12 @@ export class ClaudeAccountService { } async removeAccount(accountId: string): Promise { + this.supersedePendingLogin() return this.serializeMutation(() => this.selection.remove(accountId)) } async selectAccount(accountId: string | null): Promise { + this.supersedePendingLogin() return this.serializeMutation(() => this.selection.select(accountId)) } @@ -104,6 +106,7 @@ export class ClaudeAccountService { accountId: string | null, target?: ClaudeAccountSelectionTarget ): Promise { + this.supersedePendingLogin() return this.serializeMutation(() => this.selection.select(accountId, target)) } @@ -112,8 +115,8 @@ export class ClaudeAccountService { } // Why before the queue, not inside it: the abandoned login owns the queue slot - // the next add is waiting for. Only add/reauthenticate open a browser, so only - // they supersede — never serializeMutation, which background work also uses. + // every later account action waits for. Called from the four the user drives, + // never from serializeMutation, which background work also uses. private supersedePendingLogin(): void { if (this.cancelPendingLogin()) { console.info( diff --git a/src/main/codex-accounts/service-abandoned-login-supersede.test.ts b/src/main/codex-accounts/service-abandoned-login-supersede.test.ts index b57f18a2751..11743258fe5 100644 --- a/src/main/codex-accounts/service-abandoned-login-supersede.test.ts +++ b/src/main/codex-accounts/service-abandoned-login-supersede.test.ts @@ -58,9 +58,10 @@ function createStubLoginChild(): StubLoginChild { async function createServiceWithHangingLogin(): Promise<{ service: { addAccount: () => Promise<{ accounts: { email: string }[] }> + selectAccount: (accountId: string | null) => Promise cancelPendingLogin: () => boolean getPendingLoginUrl: () => string | null - subscribePendingLoginUrl: (listener: (url: string | null) => void) => void + onPendingLoginUrlChanged: (listener: (url: string | null) => void) => void } children: StubLoginChild[] /** The `CODEX_HOME` each login was spawned against. */ @@ -119,6 +120,19 @@ describe('CodexAccountService abandoned login', () => { await retryRejection }) + it('frees the queue for a plain account switch too, not only for another add', async () => { + const { service, children } = await createServiceWithHangingLogin() + const abandoned = service.addAccount() + const abandonedRejection = expect(abandoned).rejects.toThrow('Codex sign-in was cancelled.') + await vi.waitUntil(() => children.length === 1) + + // Why: switching to the system default is the commonest thing a user does + // after giving up on a sign-in, and it shares the add's mutation queue. + await service.selectAccount(null) + await abandonedRejection + expect(children[0].kill).toHaveBeenCalled() + }) + it('refuses to cancel a sign-in that already wrote credentials, and keeps the account', async () => { const { service, children, loginHomes } = await createServiceWithHangingLogin() const pending = service.addAccount() @@ -156,7 +170,7 @@ describe('CodexAccountService abandoned login', () => { it('publishes the sign-in link codex prints and drops it when the login ends', async () => { const { service, children } = await createServiceWithHangingLogin() const published: (string | null)[] = [] - service.subscribePendingLoginUrl((url) => published.push(url)) + service.onPendingLoginUrlChanged((url) => published.push(url)) const pending = service.addAccount() const rejection = expect(pending).rejects.toThrow('Codex sign-in was cancelled.') diff --git a/src/main/codex-accounts/service.ts b/src/main/codex-accounts/service.ts index 37ede7d1120..67f8ebb218a 100644 --- a/src/main/codex-accounts/service.ts +++ b/src/main/codex-accounts/service.ts @@ -178,7 +178,8 @@ export class CodexAccountService { return this.pendingLoginUrl } - subscribePendingLoginUrl(listener: (url: string | null) => void): void { + /** Registration lasts the process's lifetime; there is no teardown to hand back. */ + onPendingLoginUrlChanged(listener: (url: string | null) => void): void { this.pendingLoginUrlListeners.add(listener) } @@ -195,8 +196,8 @@ export class CodexAccountService { } // Why before the queue, not inside it: the abandoned login owns the queue slot - // the next add is waiting for. Only add/reauthenticate open a browser, so only - // they supersede — never serializeMutation, which background work also uses. + // every later account action waits for. Called from the four the user drives, + // never from serializeMutation, which background reset-credit work also uses. private supersedePendingLogin(): void { if (this.cancelPendingLogin()) { console.info('[codex-accounts] Cancelled a pending Codex login superseded by a new request.') @@ -225,10 +226,12 @@ export class CodexAccountService { } async removeAccount(accountId: string): Promise { + this.supersedePendingLogin() return this.serializeMutation(() => this.selection.remove(accountId)) } async selectAccount(accountId: string | null): Promise { + this.supersedePendingLogin() return this.serializeMutation(() => this.selection.select(accountId)) } @@ -236,6 +239,7 @@ export class CodexAccountService { accountId: string | null, target?: CodexAccountSelectionTarget ): Promise { + this.supersedePendingLogin() return this.serializeMutation(() => this.selection.select(accountId, target)) } diff --git a/src/main/ipc/codex-accounts.ts b/src/main/ipc/codex-accounts.ts index a06f6f9fc3b..4d828ecf82b 100644 --- a/src/main/ipc/codex-accounts.ts +++ b/src/main/ipc/codex-accounts.ts @@ -43,7 +43,7 @@ export function registerCodexAccountHandlers( ipcMain.handle('codexAccounts:pendingLoginUrl', () => codexAccounts.getPendingLoginUrl()) // Why: Settings can open after the login already printed its link, so the // renderer reads the current value on mount and this only carries changes. - codexAccounts.subscribePendingLoginUrl(broadcastCodexPendingLoginUrl) + codexAccounts.onPendingLoginUrlChanged(broadcastCodexPendingLoginUrl) ipcMain.handle( 'codexAccounts:reauthenticate', (_event, args: { accountId: string; activateIfSelectionWasEmpty?: boolean }) => diff --git a/src/renderer/src/components/settings/accounts-pane-codex-section.tsx b/src/renderer/src/components/settings/accounts-pane-codex-section.tsx index 11e887ee741..8e30f563098 100644 --- a/src/renderer/src/components/settings/accounts-pane-codex-section.tsx +++ b/src/renderer/src/components/settings/accounts-pane-codex-section.tsx @@ -187,6 +187,7 @@ export function renderCodexAccountsSection(model: AccountsPaneSectionModel): Rea variant="ghost" size="xs" onClick={() => void window.api.codexAccounts.cancelPendingLogin()} + className="gap-1.5" > {translate('auto.components.settings.AccountsPane.dbb9626ed1', 'Cancel')}