From 7701a5d2bb9f466d2a7bb7e508d98af31667119f Mon Sep 17 00:00:00 2001
From: Neil <neil@stably.ai>
Date: Thu, 17 Sep 2026 22:34:38 -0700
Subject: [PATCH] fix(codex): a cancel must not discard a sign-in that already
 succeeded

The Windows post-auth watcher gives a lingering codex login five seconds
to exit after it writes auth.json. A cancel arriving in that window
rejected the login, and the caller's rollback then deleted the managed
home that had just authenticated.

Refuse the cancel once new credential bytes exist: there is nothing left
to cancel, and the close handler already treats that state as success.

Found by review of #21372.
---
 .../codex-accounts/codex-login-session.ts     |  5 ++-
 .../service-abandoned-login-supersede.test.ts | 41 +++++++++++++++++--
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/src/main/codex-accounts/codex-login-session.ts b/src/main/codex-accounts/codex-login-session.ts
index 681f0de20d3..304c971359a 100644
--- a/src/main/codex-accounts/codex-login-session.ts
+++ b/src/main/codex-accounts/codex-login-session.ts
@@ -210,7 +210,10 @@ async function runCodexLoginProcess(
     }
 
     cancellation.setSpawnedCancel(() => {
-      if (settled) {
+      // Why: once codex has written new credential bytes the sign-in already
+      // succeeded, and rejecting here would send the caller's rollback at the
+      // home it just authenticated. Nothing left to cancel — let it settle.
+      if (settled || loginAuthChanged(initialAuthSnapshot, readLoginAuthSnapshot(authJsonPath))) {
         return false
       }
       dependencies.killProcessTree(child, spawnConfig.interactiveLogin)
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 8f9c91c7839..3d2fb675465 100644
--- a/src/main/codex-accounts/service-abandoned-login-supersede.test.ts
+++ b/src/main/codex-accounts/service-abandoned-login-supersede.test.ts
@@ -1,7 +1,10 @@
 import { describe, expect, it, vi } from 'vitest'
 import { EventEmitter } from 'node:events'
+import { existsSync, writeFileSync } from 'node:fs'
+import { join } from 'node:path'
 import { PassThrough } from 'node:stream'
 import {
+  createCodexAuthJson,
   createRateLimits,
   createRuntimeHome,
   createSettings,
@@ -54,16 +57,20 @@ function createStubLoginChild(): StubLoginChild {
 /** A service whose `codex login` never finishes on its own. */
 async function createServiceWithHangingLogin(): Promise<{
   service: {
-    addAccount: () => Promise<unknown>
+    addAccount: () => Promise<{ accounts: { email: string }[] }>
     cancelPendingLogin: () => boolean
   }
   children: StubLoginChild[]
+  /** The `CODEX_HOME` each login was spawned against. */
+  loginHomes: string[]
 }> {
   vi.resetModules()
   const children: StubLoginChild[] = []
+  const loginHomes: string[] = []
   vi.doMock('node:child_process', () => ({
     execFileSync: vi.fn(),
-    spawn: vi.fn(() => {
+    spawn: vi.fn((_command: string, _args: string[], options: { env: NodeJS.ProcessEnv }) => {
+      loginHomes.push(options.env.CODEX_HOME ?? '')
       const child = createStubLoginChild()
       children.push(child)
       return child
@@ -78,7 +85,7 @@ async function createServiceWithHangingLogin(): Promise<{
     asServiceDouble(createRateLimits()),
     asServiceDouble(createRuntimeHome())
   )
-  return { service, children }
+  return { service, children, loginHomes }
 }
 
 describe('CodexAccountService abandoned login', () => {
@@ -110,6 +117,34 @@ describe('CodexAccountService abandoned login', () => {
     }
   })
 
+  it('refuses to cancel a sign-in that already wrote credentials, and keeps the account', async () => {
+    const { service, children, loginHomes } = await createServiceWithHangingLogin()
+    try {
+      const pending = service.addAccount()
+      await vi.waitUntil(() => children.length === 1)
+
+      // The browser half of the OAuth flow finishes while the CLI lingers.
+      writeFileSync(
+        join(loginHomes[0], 'auth.json'),
+        createCodexAuthJson('user@example.com', 'provider-account-1', 'refresh-token'),
+        'utf-8'
+      )
+
+      // Why: cancelling here would send the rollback at a home that just
+      // authenticated. There is nothing left to cancel.
+      expect(service.cancelPendingLogin()).toBe(false)
+      expect(children[0].kill).not.toHaveBeenCalled()
+
+      children[0].emit('close', 0)
+      const accounts = await pending
+      expect(accounts.accounts.map((account) => account.email)).toEqual(['user@example.com'])
+      expect(existsSync(join(loginHomes[0], 'auth.json'))).toBe(true)
+    } finally {
+      vi.doUnmock('node:child_process')
+      vi.doUnmock('../codex-cli/command')
+    }
+  })
+
   it('reports whether a pending login was there to cancel', async () => {
     const { service, children } = await createServiceWithHangingLogin()
     try {