From d3ad304f32d08ef32559ee5ff6a70e7f2ef6a64a Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 19 Jul 2026 13:43:17 -0700 Subject: [PATCH] fix(codex): preserve Windows reauthentication login flow --- src/main/codex-accounts/service.test.ts | 72 +++++++++++++++++++++++-- src/main/codex-accounts/service.ts | 32 ++++++++++- 2 files changed, 100 insertions(+), 4 deletions(-) diff --git a/src/main/codex-accounts/service.test.ts b/src/main/codex-accounts/service.test.ts index dd4120eac3d..a488039c7f3 100644 --- a/src/main/codex-accounts/service.test.ts +++ b/src/main/codex-accounts/service.test.ts @@ -2001,6 +2001,74 @@ describe('CodexAccountService config sync', () => { } }) + it('waits for reauthentication to replace existing Windows auth before killing login', async () => { + vi.resetModules() + vi.useFakeTimers() + const originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform')! + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }) + const child = new EventEmitter() as EventEmitter & { + stdout: PassThrough + stderr: PassThrough + kill: () => void + pid: number + exitCode: number | null + signalCode: string | null + } + child.stdout = new PassThrough() + child.stderr = new PassThrough() + child.kill = vi.fn() + child.pid = 4343 + child.exitCode = null + child.signalCode = null + const execFileSyncMock = vi.fn() + vi.doMock('node:child_process', () => ({ + execFileSync: execFileSyncMock, + spawn: vi.fn(() => child) + })) + vi.doMock('../codex-cli/command', () => ({ resolveCodexCommand: () => 'codex' })) + const authPath = join(testState.fakeHomeDir, 'auth.json') + writeFileSync( + authPath, + createCodexAuthJson('user@example.com', 'provider-account-1', 'old-token'), + 'utf-8' + ) + + try { + const { CodexAccountService } = await import('./service') + const service = new CodexAccountService( + createStore(createSettings()) as never, + createRateLimits() as never, + createRuntimeHome() as never + ) + const loginPromise = ( + service as unknown as { runCodexLogin(managedHomePath: string): Promise } + ).runCodexLogin(testState.fakeHomeDir) + + await vi.advanceTimersByTimeAsync(6_000) + expect(execFileSyncMock).not.toHaveBeenCalled() + + writeFileSync( + authPath, + createCodexAuthJson('user@example.com', 'provider-account-1', 'new-token'), + 'utf-8' + ) + await vi.advanceTimersByTimeAsync(6_000) + expect(execFileSyncMock).toHaveBeenCalledWith( + 'taskkill', + ['/pid', '4343', '/t', '/f'], + expect.objectContaining({ windowsHide: true, stdio: 'ignore' }) + ) + + child.emit('close', 1) + await expect(loginPromise).resolves.toBeUndefined() + } finally { + Object.defineProperty(process, 'platform', originalPlatform) + vi.useRealTimers() + vi.doUnmock('node:child_process') + vi.doUnmock('../codex-cli/command') + } + }) + it('removes managed homes with bounded rm retries for transient Windows locks', async () => { vi.resetModules() const actualFs = await vi.importActual('node:fs') // eslint-disable-line @typescript-eslint/consistent-type-imports -- vi.importActual requires inline import() @@ -2059,9 +2127,7 @@ describe('CodexAccountService config sync', () => { const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) try { - managedHomePath = actualFs.realpathSync( - createManagedHome(testState.userDataDir, 'account-1') - ) + managedHomePath = actualFs.realpathSync(createManagedHome(testState.userDataDir, 'account-1')) const store = createStore(createSettings()) const rateLimits = createRateLimits() const runtimeHome = createRuntimeHome() diff --git a/src/main/codex-accounts/service.ts b/src/main/codex-accounts/service.ts index d026d76bb82..27bf692d342 100644 --- a/src/main/codex-accounts/service.ts +++ b/src/main/codex-accounts/service.ts @@ -130,6 +130,30 @@ function killLoginProcessTree(child: ChildProcess): void { child.kill() } +function readLoginAuthSnapshot(authJsonPath: string): string | null | undefined { + try { + return readFileSync(authJsonPath, 'utf-8') + } catch (error) { + const code = (error as NodeJS.ErrnoException).code + if (code === 'ENOENT' || code === 'ENOTDIR') { + return null + } + // Why: codex can atomically replace auth.json while the poll runs; a later + // poll will observe the stable credential. An unreadable initial file must + // disable the shortcut rather than look like a fresh login. + return undefined + } +} + +function loginAuthChanged( + initial: string | null | undefined, + current: string | null | undefined +): boolean { + // Why: metadata-only touches can happen before OAuth finishes. Requiring new + // credential bytes prevents reauthentication from being killed prematurely. + return initial !== undefined && current !== undefined && current !== null && current !== initial +} + export class CodexAccountService { // Why: account mutations read settings, do async work (login, rate-limit // refresh), then write settings. Without serialization, overlapping calls @@ -1009,6 +1033,12 @@ export class CodexAccountService { if (wslInfo) { this.assertWslCodexCliAvailable(wslInfo) } + // Why: reauthentication starts with an existing auth.json. Only new auth + // bytes prove this login completed; existence alone would kill the + // Windows OAuth flow five seconds after it opened. + const initialAuthSnapshot = wslInfo + ? null + : readLoginAuthSnapshot(join(managedHomePath, 'auth.json')) await new Promise((resolvePromise, rejectPromise) => { const spawnConfig = wslInfo @@ -1100,7 +1130,7 @@ export class CodexAccountService { // give the tree a short grace period to exit, then force it down. if (process.platform === 'win32' && !wslInfo) { authWatchInterval = setInterval(() => { - if (!existsSync(authJsonPath)) { + if (!loginAuthChanged(initialAuthSnapshot, readLoginAuthSnapshot(authJsonPath))) { return } if (authWatchInterval) {