From c074ec50da641241e79867cf520cea93e45a919d Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 25 Oct 2026 09:07:34 -0700 Subject: [PATCH] fix(codex-accounts): keep the old login until re-auth finishes, and relabel swapped homes (#33568) Codex account re-auth no longer signs the account out on cancel/failure: `codex login` revokes and deletes existing auth before it starts, so re-auth now logs in inside a staging CODEX_HOME and copies auth.json in only on success. Listing accounts relabels a host account whose auth.json identity no longer matches its saved record. Fixes #10795 Fixes #22284 Supersedes #11852 Co-authored-by: Rod Boev <106971+rodboev@users.noreply.github.com> --- .../codex-account-registration.ts | 45 ++++++- .../codex-accounts/codex-account-selection.ts | 44 +++++++ .../codex-accounts/codex-auth-identity.ts | 24 ++-- .../service-account-add-login.test.ts | 6 +- .../service-managed-home-identity.test.ts | 113 ++++++++++++++++++ .../service-wsl-accounts.test.ts | 26 +++- src/main/codex-accounts/service.ts | 1 + 7 files changed, 240 insertions(+), 19 deletions(-) create mode 100644 src/main/codex-accounts/service-managed-home-identity.test.ts diff --git a/src/main/codex-accounts/codex-account-registration.ts b/src/main/codex-accounts/codex-account-registration.ts index fa7f058964f..8ce06a76a06 100644 --- a/src/main/codex-accounts/codex-account-registration.ts +++ b/src/main/codex-accounts/codex-account-registration.ts @@ -1,4 +1,11 @@ import { randomUUID } from 'node:crypto' +import { copyFileSync, mkdirSync, rmSync } from 'node:fs' +import { join } from 'node:path' +import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence' +import { + WINDOWS_RM_MAX_RETRIES, + WINDOWS_RM_RETRY_DELAY_MS +} from '../../shared/windows-transient-lock-removal' import type { CodexManagedAccount, CodexRateLimitAccountsState @@ -39,6 +46,17 @@ type CodexAccountRegistrationDependencies = { login: (managedHomePath: string) => Promise } +const REAUTH_STAGING_DIR_NAME = '.orca-reauth-login' + +function removeStagingHome(stagingHomePath: string): void { + rmSync(stagingHomePath, { + recursive: true, + force: true, + maxRetries: WINDOWS_RM_MAX_RETRIES, + retryDelay: WINDOWS_RM_RETRY_DELAY_MS + }) +} + export class CodexAccountRegistration { constructor(private readonly dependencies: CodexAccountRegistrationDependencies) {} @@ -93,7 +111,7 @@ export class CodexAccountRegistration { options?.activateIfSelectionWasEmpty === true && selectedAccountId === null this.dependencies.configMirror.safeSyncIntoManagedHome(managedHomePath, undefined, account.id) - await this.dependencies.login(managedHomePath) + await this.loginThroughStagingHome(managedHomePath, account.id) const identity = this.dependencies.readIdentityFromHome(managedHomePath, account.id) if (!identity.email) { throw new Error('Codex login completed, but Orca could not resolve the account email.') @@ -134,6 +152,31 @@ export class CodexAccountRegistration { return this.dependencies.selection.snapshot() } + // `codex login` revokes and deletes the home's current auth first, so only a finished login may replace it (#10795). + private async loginThroughStagingHome(managedHomePath: string, accountId: string): Promise { + const stagingHomePath = join(managedHomePath, REAUTH_STAGING_DIR_NAME) + removeStagingHome(stagingHomePath) + mkdirSync(stagingHomePath, { recursive: true }) + try { + // Carry forced-workspace and credential-store settings into the login. + copyFileSync(join(managedHomePath, 'config.toml'), join(stagingHomePath, 'config.toml')) + } catch (error) { + if (!isDefinitiveAbsence(error)) { + throw error + } + } + try { + await this.dependencies.login(stagingHomePath) + this.dependencies.managedHomes.importAuthFromHome(stagingHomePath, managedHomePath, accountId) + } finally { + try { + removeStagingHome(stagingHomePath) + } catch (error) { + console.warn('[codex-accounts] Failed to remove reauthentication staging home:', error) + } + } + } + private prepareManagedHomeForLogin(managedHomePath: string, accountId: string): void { const canonicalConfig = this.dependencies.configMirror.readForManagedHome(managedHomePath) this.dependencies.configMirror.assertOAuthAccountAddAllowed(canonicalConfig) diff --git a/src/main/codex-accounts/codex-account-selection.ts b/src/main/codex-accounts/codex-account-selection.ts index 24785e79dc1..c3b0a5338f0 100644 --- a/src/main/codex-accounts/codex-account-selection.ts +++ b/src/main/codex-accounts/codex-account-selection.ts @@ -3,10 +3,13 @@ import type { CodexRateLimitAccountsState, CodexSystemDefaultIdentity } from '../../shared/managed-account-types' +import { parseWslUncPath } from '../../shared/wsl-paths' import type { Store } from '../persistence' import type { RateLimitService } from '../rate-limits/service' import type { CodexRuntimeHomeService } from './runtime-home-service' import type { CodexConfigMirror } from './codex-config-mirror' +import type { ResolvedCodexIdentity } from './codex-account-identity' +import { codexAuthIdentityContradictsAccount } from './codex-auth-identity' import type { CodexAccountServiceLifecycle } from './codex-account-service-types' import { toCodexManagedAccountSummary } from './codex-account-service-types' import { @@ -27,6 +30,10 @@ type CodexAccountSelectionDependencies = { configMirror: CodexConfigMirror lifecycle: CodexAccountServiceLifecycle resolveSystemDefault: () => CodexSystemDefaultIdentity + readIdentityFromHome: ( + managedHomePath: string, + expectedAccountId: string + ) => ResolvedCodexIdentity removeManagedHome: (candidatePath: string, expectedAccountId: string) => void discardResetAttempts: (accountId: string) => Promise } @@ -36,6 +43,7 @@ export class CodexAccountSelection { list(): CodexRateLimitAccountsState { this.normalizeActiveSelection() + this.adoptManagedHomeIdentities() return this.snapshot() } @@ -144,6 +152,42 @@ export class CodexAccountSelection { return this.snapshot() } + // A `codex login` run inside a managed home swaps its account; label the row with what actually runs (#22284). + private adoptManagedHomeIdentities(): void { + const settings = this.dependencies.store.getSettings() + let changed = false + const accounts = settings.codexManagedAccounts.map((account) => { + // WSL homes are read over UNC, which can block when the distro is stopped. + if (account.managedHomeRuntime === 'wsl' || parseWslUncPath(account.managedHomePath)) { + return account + } + let identity: ResolvedCodexIdentity + try { + identity = this.dependencies.readIdentityFromHome(account.managedHomePath, account.id) + } catch { + return account + } + if (!identity.email || !codexAuthIdentityContradictsAccount(identity, account)) { + return account + } + console.warn( + '[codex-accounts] Managed home holds a different Codex login; updating its label' + ) + changed = true + return { + ...account, + email: identity.email, + providerAccountId: identity.providerAccountId, + workspaceLabel: identity.workspaceLabel, + workspaceAccountId: identity.workspaceAccountId, + updatedAt: Date.now() + } + }) + if (changed) { + this.dependencies.store.updateSettings({ codexManagedAccounts: accounts }) + } + } + private normalizeActiveSelection(): void { const settings = this.dependencies.store.getSettings() const selection = normalizeCodexRuntimeSelection(settings) diff --git a/src/main/codex-accounts/codex-auth-identity.ts b/src/main/codex-accounts/codex-auth-identity.ts index 6b8457ffb71..1b2af49544d 100644 --- a/src/main/codex-accounts/codex-auth-identity.ts +++ b/src/main/codex-accounts/codex-auth-identity.ts @@ -65,16 +65,20 @@ export function codexAuthCouldBelongToManagedAccount( account: CodexManagedAccount ): boolean { const identity = readCodexAuthIdentity(runtimeAuthContents) - return ( - !identity || - !identityContradictsSelection( - { - email: normalizeField(account.email), - providerAccountId: normalizeField(account.providerAccountId), - workspaceAccountId: normalizeField(account.workspaceAccountId) - }, - identity - ) + return !identity || !codexAuthIdentityContradictsAccount(identity, account) +} + +export function codexAuthIdentityContradictsAccount( + identity: CodexAuthIdentity, + account: CodexManagedAccount +): boolean { + return identityContradictsSelection( + { + email: normalizeField(account.email), + providerAccountId: normalizeField(account.providerAccountId), + workspaceAccountId: normalizeField(account.workspaceAccountId) + }, + identity ) } diff --git a/src/main/codex-accounts/service-account-add-login.test.ts b/src/main/codex-accounts/service-account-add-login.test.ts index 374e1626fd6..d0fae399e04 100644 --- a/src/main/codex-accounts/service-account-add-login.test.ts +++ b/src/main/codex-accounts/service-account-add-login.test.ts @@ -234,8 +234,10 @@ describe('CodexAccountService config sync', () => { const spawnMock = vi.fn( (_command: string, _args: string[], options: { env: NodeJS.ProcessEnv }) => { const loginHome = options.env.CODEX_HOME - expect(loginHome).toBeTruthy() - expect(readFileSync(join(loginHome!, '.orca-managed-home'), 'utf-8')).toBe('account-1\n') + expect(loginHome).toMatch(/[\\/]account-1[\\/]home[\\/]\.orca-reauth-login$/) + expect(readFileSync(join(managedHomePath, '.orca-managed-home'), 'utf-8')).toBe( + 'account-1\n' + ) expect(readFileSync(join(loginHome!, 'config.toml'), 'utf-8')).toBe(canonicalConfig) const child = new EventEmitter() as EventEmitter & { diff --git a/src/main/codex-accounts/service-managed-home-identity.test.ts b/src/main/codex-accounts/service-managed-home-identity.test.ts new file mode 100644 index 00000000000..9cb15cf3f83 --- /dev/null +++ b/src/main/codex-accounts/service-managed-home-identity.test.ts @@ -0,0 +1,113 @@ +import { describe, expect, it, vi, beforeEach } from 'vitest' +import { installFakeAppEnvironment } from '../../../config/scripts/vitest-host-ports-setup' +import { EventEmitter } from 'node:events' +import { readFileSync, rmSync, writeFileSync } from 'node:fs' +import { join } from 'node:path' +import { PassThrough } from 'node:stream' +import { + createCodexAuthJson, + createManagedHome, + createRateLimits, + createRuntimeHome, + createSettings, + createStore, + registerCodexAccountsTestHomes, + testState +} from './service-test-harness' + +vi.mock('electron', () => ({ + app: { + getPath: () => testState.userDataDir + } +})) +beforeEach(() => installFakeAppEnvironment({ getPath: () => testState.userDataDir })) + +vi.mock(import('node:os'), async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + homedir: () => testState.fakeHomeDir + } +}) + +const accountIAuth = createCodexAuthJson('i@example.com', 'provider-i', 'refresh-i') + +function createAccountI() { + return { + id: 'account-i', + email: 'i@example.com', + managedHomePath: createManagedHome(testState.userDataDir, 'account-i', '', accountIAuth), + providerAccountId: 'provider-i', + workspaceLabel: null, + workspaceAccountId: 'provider-i', + createdAt: 1, + updatedAt: 1, + lastAuthenticatedAt: 1 + } +} + +async function createService(settings: ReturnType) { + const store = createStore(settings) + const { CodexAccountService } = await import('./service') + const service = new CodexAccountService( + store as never, + createRateLimits() as never, + createRuntimeHome() as never + ) + return { service, store } +} + +describe('Codex managed home identity', () => { + registerCodexAccountsTestHomes() + + it('keeps the existing login when a reauthentication is abandoned', async () => { + vi.resetModules() + // Mirrors `codex login`, which deletes the home's auth before its login server starts. + const spawnMock = vi.fn( + (_command: string, _args: string[], options: { env: NodeJS.ProcessEnv }) => { + rmSync(join(options.env.CODEX_HOME!, 'auth.json'), { force: true }) + const child = Object.assign(new EventEmitter(), { + stdout: new PassThrough(), + stderr: new PassThrough(), + exitCode: null, + signalCode: null, + kill: vi.fn(() => true) + }) + return child + } + ) + vi.doMock('node:child_process', () => ({ execFileSync: vi.fn(), spawn: spawnMock })) + vi.doMock('../codex-cli/command', () => ({ resolveCodexCommand: () => 'codex' })) + const account = createAccountI() + const { service, store } = await createService( + createSettings({ + codexManagedAccounts: [account], + activeCodexManagedAccountId: 'account-i', + activeCodexManagedAccountIdsByRuntime: { host: 'account-i', wsl: {} } + }) + ) + + const reauth = service.reauthenticateAccount('account-i') + await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledOnce()) + expect(service.cancelPendingLogin()).toBe(true) + + await expect(reauth).rejects.toThrow() + expect(readFileSync(join(account.managedHomePath, 'auth.json'), 'utf-8')).toBe(accountIAuth) + expect(store.getSettings().activeCodexManagedAccountId).toBe('account-i') + }) + + it('relabels a managed account whose home now holds a different login', async () => { + const account = createAccountI() + writeFileSync( + join(account.managedHomePath, 'auth.json'), + createCodexAuthJson('c@example.com', 'provider-c', 'refresh-c'), + 'utf-8' + ) + const { service } = await createService(createSettings({ codexManagedAccounts: [account] })) + + const listed = service.listAccounts().accounts.find((entry) => entry.id === 'account-i') + + expect(listed).toMatchObject({ email: 'c@example.com', providerAccountId: 'provider-c' }) + expect(listed?.updatedAt).toBeGreaterThan(account.updatedAt) + }) +}) diff --git a/src/main/codex-accounts/service-wsl-accounts.test.ts b/src/main/codex-accounts/service-wsl-accounts.test.ts index b2e237e01e7..6b610982214 100644 --- a/src/main/codex-accounts/service-wsl-accounts.test.ts +++ b/src/main/codex-accounts/service-wsl-accounts.test.ts @@ -514,7 +514,9 @@ describe('CodexAccountService config sync', () => { let clearSelectionDuringLogin = (): void => {} const spawnMock = vi.fn((command: string, args: string[]) => { expect(command).toBe('wsl.exe') - expect(args).toEqual(buildWslCodexLoginArgs('Ubuntu', wslLinuxHomePath)) + expect(args).toEqual( + buildWslCodexLoginArgs('Ubuntu', `${wslLinuxHomePath}/.orca-reauth-login`) + ) const child = new EventEmitter() as EventEmitter & { stdout: PassThrough stderr: PassThrough @@ -525,7 +527,7 @@ describe('CodexAccountService config sync', () => { child.kill = vi.fn() clearSelectionDuringLogin() writeFileSync( - join(wslManagedHomePath, 'auth.json'), + join(wslManagedHomePath, '.orca-reauth-login', 'auth.json'), JSON.stringify({ tokens: { id_token: `header.${Buffer.from(JSON.stringify({ email: 'new@example.com' })).toString( @@ -546,7 +548,12 @@ describe('CodexAccountService config sync', () => { vi.doMock('../wsl/wsl-runner', () => ({ runWslProcess: runWslProcessMock })) vi.doMock('../../shared/wsl-paths', () => ({ parseWslUncPath: (path: string) => - path === wslManagedHomePath ? { distro: 'Ubuntu', linuxPath: wslLinuxHomePath } : null + path.startsWith(wslManagedHomePath) + ? { + distro: 'Ubuntu', + linuxPath: `${wslLinuxHomePath}${path.slice(wslManagedHomePath.length).replaceAll('\\', '/')}` + } + : null })) vi.doMock('../wsl', () => ({ toWindowsWslPath: () => wslManagedHomePath @@ -654,7 +661,9 @@ describe('CodexAccountService config sync', () => { }) const spawnMock = vi.fn((command: string, args: string[]) => { expect(command).toBe('wsl.exe') - expect(args).toEqual(buildWslCodexLoginArgs('Ubuntu', wslLinuxHomePath)) + expect(args).toEqual( + buildWslCodexLoginArgs('Ubuntu', `${wslLinuxHomePath}/.orca-reauth-login`) + ) expect(readFileSync(join(wslManagedHomePath, '.orca-managed-home'), 'utf-8')).toBe( 'account-1\n' ) @@ -667,7 +676,7 @@ describe('CodexAccountService config sync', () => { child.stderr = new PassThrough() child.kill = vi.fn() writeFileSync( - join(wslManagedHomePath, 'auth.json'), + join(wslManagedHomePath, '.orca-reauth-login', 'auth.json'), createCodexAuthJson('new-wsl@example.com', 'provider-wsl-1', 'refresh-token'), 'utf-8' ) @@ -682,7 +691,12 @@ describe('CodexAccountService config sync', () => { vi.doMock('../wsl/wsl-runner', () => ({ runWslProcess: runWslProcessMock })) vi.doMock('../../shared/wsl-paths', () => ({ parseWslUncPath: (path: string) => - path === wslManagedHomePath ? { distro: 'Ubuntu', linuxPath: wslLinuxHomePath } : null + path.startsWith(wslManagedHomePath) + ? { + distro: 'Ubuntu', + linuxPath: `${wslLinuxHomePath}${path.slice(wslManagedHomePath.length).replaceAll('\\', '/')}` + } + : null })) vi.doMock('../wsl', () => ({ toWindowsWslPath: () => wslManagedHomePath diff --git a/src/main/codex-accounts/service.ts b/src/main/codex-accounts/service.ts index 95719d613fd..2800fdbf5df 100644 --- a/src/main/codex-accounts/service.ts +++ b/src/main/codex-accounts/service.ts @@ -127,6 +127,7 @@ export class CodexAccountService { configMirror: this.configMirror, lifecycle, resolveSystemDefault: () => this.resolveSystemDefaultIdentity(), + readIdentityFromHome: (path, accountId) => this.readIdentityFromHome(path, accountId), removeManagedHome: (path, accountId) => this.safeRemoveManagedHome(path, accountId), discardResetAttempts: (accountId) => this.resetCredits.discardForRemovedAccount(accountId) })