diff --git a/src/main/codex-accounts/service.test.ts b/src/main/codex-accounts/service.test.ts index c0617eaea..435a187fe 100644 --- a/src/main/codex-accounts/service.test.ts +++ b/src/main/codex-accounts/service.test.ts @@ -2586,4 +2586,188 @@ describe('CodexAccountService config sync', () => { expect(state.systemDefault?.email).toBe('real@home.dev') }) }) + + // Why: quota probes against a cold per-account CODEX_HOME can take 10–25s + // (RPC + PTY fallback) and queue behind an in-flight global usage refresh; + // account mutations must never block on — or fail because of — that probe. + describe('quota refresh decoupling', () => { + function createAccountOneSettings(): GlobalSettings { + const managedHomePath = createManagedHome( + testState.userDataDir, + 'account-1', + '', + '{"account":"managed"}\n' + ) + return createSettings({ + codexManagedAccounts: [ + { + id: 'account-1', + email: 'user@example.com', + managedHomePath, + providerAccountId: null, + workspaceLabel: null, + workspaceAccountId: null, + createdAt: 1, + updatedAt: 1, + lastAuthenticatedAt: 1 + } + ] + }) + } + + async function expectResolvesPromptly(promise: Promise, label: string): Promise { + let timer: NodeJS.Timeout | undefined + try { + return await Promise.race([ + promise, + new Promise((_, reject) => { + timer = setTimeout( + () => reject(new Error(`${label} blocked on the quota refresh`)), + 2_000 + ) + }) + ]) + } finally { + clearTimeout(timer) + } + } + + function createLoginSpawnMock() { + return vi.fn((_command: string, _args: string[], options: { env: NodeJS.ProcessEnv }) => { + const child = new EventEmitter() as EventEmitter & { + stdout: PassThrough + stderr: PassThrough + kill: () => void + } + child.stdout = new PassThrough() + child.stderr = new PassThrough() + child.kill = vi.fn() + writeFileSync( + join(options.env.CODEX_HOME!, 'auth.json'), + createCodexAuthJson('user@example.com', 'provider-account-1', 'refresh-token'), + 'utf-8' + ) + queueMicrotask(() => child.emit('close', 0)) + return child + }) + } + + it('resolves selectAccount while the quota refresh never settles', async () => { + const store = createStore(createAccountOneSettings()) + const rateLimits = { + refreshForCodexAccountChange: vi.fn(() => new Promise(() => {})), + evictInactiveCodexCache: vi.fn() + } + const runtimeHome = createRuntimeHome() + + const { CodexAccountService } = await import('./service') + const service = new CodexAccountService( + store as never, + rateLimits as never, + runtimeHome as never + ) + + const state = await expectResolvesPromptly( + service.selectAccount('account-1'), + 'selectAccount' + ) + + expect(state.activeAccountId).toBe('account-1') + expect(rateLimits.refreshForCodexAccountChange).toHaveBeenCalledTimes(1) + }) + + it('resolves selectAccount when the quota refresh rejects', async () => { + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + const store = createStore(createAccountOneSettings()) + const rateLimits = { + refreshForCodexAccountChange: vi.fn().mockRejectedValue(new Error('cold probe failed')), + evictInactiveCodexCache: vi.fn() + } + const runtimeHome = createRuntimeHome() + + const { CodexAccountService } = await import('./service') + const service = new CodexAccountService( + store as never, + rateLimits as never, + runtimeHome as never + ) + + const state = await service.selectAccount('account-1') + + expect(state.activeAccountId).toBe('account-1') + await vi.waitFor(() => expect(errorSpy).toHaveBeenCalled()) + errorSpy.mockRestore() + }) + + it('resolves addAccount while the post-login quota refresh never settles', async () => { + vi.resetModules() + writeFileSync( + join(testState.fakeHomeDir, '.codex', 'config.toml'), + 'approval_policy = "never"\n', + 'utf-8' + ) + const spawnMock = createLoginSpawnMock() + vi.doMock('node:child_process', () => ({ execFileSync: vi.fn(), spawn: spawnMock })) + vi.doMock('../codex-cli/command', () => ({ resolveCodexCommand: () => 'codex' })) + + const store = createStore(createSettings()) + const rateLimits = { + refreshForCodexAccountChange: vi.fn(() => new Promise(() => {})), + evictInactiveCodexCache: vi.fn() + } + const runtimeHome = createRuntimeHome() + + const { CodexAccountService } = await import('./service') + const service = new CodexAccountService( + store as never, + rateLimits as never, + runtimeHome as never + ) + + const state = await expectResolvesPromptly(service.addAccount(), 'addAccount') + + expect(state.accounts).toHaveLength(1) + expect(state.accounts[0].email).toBe('user@example.com') + expect(rateLimits.refreshForCodexAccountChange).toHaveBeenCalledTimes(1) + }) + + it('keeps the new account and its managed home when the post-login quota refresh rejects', async () => { + vi.resetModules() + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + writeFileSync( + join(testState.fakeHomeDir, '.codex', 'config.toml'), + 'approval_policy = "never"\n', + 'utf-8' + ) + const spawnMock = createLoginSpawnMock() + vi.doMock('node:child_process', () => ({ execFileSync: vi.fn(), spawn: spawnMock })) + vi.doMock('../codex-cli/command', () => ({ resolveCodexCommand: () => 'codex' })) + + const store = createStore(createSettings()) + const rateLimits = { + refreshForCodexAccountChange: vi.fn().mockRejectedValue(new Error('cold probe failed')), + evictInactiveCodexCache: vi.fn() + } + const runtimeHome = createRuntimeHome() + + const { CodexAccountService } = await import('./service') + const service = new CodexAccountService( + store as never, + rateLimits as never, + runtimeHome as never + ) + + const state = await service.addAccount() + + expect(state.accounts).toHaveLength(1) + const account = store.getSettings().codexManagedAccounts[0] + expect(account.email).toBe('user@example.com') + // The durable mutation must survive a failed usage probe — previously the + // rejection fell into login cleanup and deleted the just-created home. + expect(existsSync(account.managedHomePath)).toBe(true) + expect(existsSync(join(account.managedHomePath, 'auth.json'))).toBe(true) + await vi.waitFor(() => expect(errorSpy).toHaveBeenCalled()) + errorSpy.mockRestore() + }) + }) }) diff --git a/src/main/codex-accounts/service.ts b/src/main/codex-accounts/service.ts index a47337677..a7448e855 100644 --- a/src/main/codex-accounts/service.ts +++ b/src/main/codex-accounts/service.ts @@ -204,6 +204,20 @@ export class CodexAccountService { return this.serializeMutation(() => this.doSelectAccount(accountId, target)) } + // Why: quota probes against a cold per-account CODEX_HOME can take 10–25s + // (RPC + PTY fallback) and queue behind an in-flight global usage refresh. + // The refresh synchronously flips usage to "fetching" before its first await, + // so the switcher updates immediately; the probe itself must never block or + // fail the already-durable account mutation. + private startQuotaRefreshInBackground( + outgoingAccountId: string | null | undefined, + target: CodexAccountSelectionTarget | undefined + ): void { + void this.rateLimits.refreshForCodexAccountChange(outgoingAccountId, target).catch((error) => { + console.error('[codex-accounts] Quota refresh after account change failed:', error) + }) + } + private async doAddAccount(target?: CodexAccountAddTarget): Promise { const accountId = randomUUID() const managedHome = this.createManagedHome(accountId, target) @@ -254,7 +268,7 @@ export class CodexAccountService { // Why: switching activates the new account, so cache the outgoing account's usage for the switcher. const outgoingAccountId = getSelectedCodexAccountIdForTarget(settings, targetSelection) - await this.rateLimits.refreshForCodexAccountChange(outgoingAccountId, targetSelection) + this.startQuotaRefreshInBackground(outgoingAccountId, targetSelection) return this.getSnapshot() } catch (error) { this.safeRemoveManagedHome(managedHomePath, accountId) @@ -310,7 +324,7 @@ export class CodexAccountService { this.runtimeHome.syncForCurrentSelection(accountTarget) // Why: re-auth can change the underlying Codex identity, so force a fresh read to avoid showing stale quota. - await this.rateLimits.refreshForCodexAccountChange(undefined, accountTarget) + this.startQuotaRefreshInBackground(undefined, accountTarget) return this.getSnapshot() } @@ -339,7 +353,7 @@ export class CodexAccountService { // Why: a removed account can no longer appear in the switcher dropdown, // so purge its cached usage to avoid stale entries. this.rateLimits.evictInactiveCodexCache(accountId) - await this.rateLimits.refreshForCodexAccountChange( + this.startQuotaRefreshInBackground( getSelectedCodexAccountIdForTarget(settings, getCodexSelectionTargetForAccount(account)) === accountId ? accountId @@ -388,7 +402,7 @@ export class CodexAccountService { this.lifecycle.onHostSystemDefaultSelected?.() } - await this.rateLimits.refreshForCodexAccountChange(outgoingAccountId, effectiveTarget) + this.startQuotaRefreshInBackground(outgoingAccountId, effectiveTarget) return this.getSnapshot() }