fix(codex): stop a surviving descendant wedging the Codex home process lock forever (#12410)

* fix(codex): stop a surviving descendant wedging the Codex home process lock

The per-Codex-home process lock was released only by the child's 'close'
event. 'close' fires when the child AND every process that inherited its
stdio have exited, so a grandchild that outlives codex (an MCP server, a
helper, an `sh -c` wrapper) keeps the pipe open and 'close' never arrives.
killProcessTree only SIGKILLs the direct child on POSIX, so Stop and the
60s generation timeout both leave the lock held. Every later AI commit
message, PR field, branch name, and quota probe for that home then queued
forever with no error and no recovery until the app restarted.

Release the hold on 'exit' as well as 'close': once the codex process
itself is gone it can no longer rotate that home's auth.json, which is the
race the lock exists to prevent, so waiting on descendants bought nothing.
As a backstop for a child that never reports exit at all (a kill that does
not land), bound the hold: while another run is queued behind it, a hold
that outlives CODEX_HOME_PROCESS_LOCK_MAX_HOLD_MS releases the queue. The
cap is armed only when someone is waiting and only counts time the entry
actually holds the lock, so uncontended runs arm no timer and queueing
behind a slow predecessor never shortens a run's own budget.

* fix(codex): preserve home lock exclusivity

---------

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
(cherry picked from commit 1ef83986a1)

src/main/codex-cli/codex-home-process-lock.test.ts

@@ -1,6 +1,6 @@
 import { join } from 'node:path'
 import { homedir } from 'node:os'
-import { describe, expect, it } from 'vitest'
+import { describe, expect, it, vi } from 'vitest'
 import {
   resolveCodexHomeProcessLockKey,
   resolveCodexHomeProcessLockKeyForSpawnEnv,
@@ -70,6 +70,30 @@ describe('withCodexHomeProcessLock', () => {
     await expect(withCodexHomeProcessLock('home-a', async () => 'after')).resolves.toBe('after')
   })
 
+  it('does not release a running lock based on elapsed time', async () => {
+    vi.useFakeTimers()
+    try {
+      const events: string[] = []
+      const gate = deferred()
+      const first = withCodexHomeProcessLock('home-long-running', async () => {
+        events.push('first:start')
+        await gate.promise
+      })
+      const second = withCodexHomeProcessLock('home-long-running', async () => {
+        events.push('second:start')
+      })
+
+      await vi.advanceTimersByTimeAsync(60 * 60_000)
+      expect(events).toEqual(['first:start'])
+
+      gate.resolve()
+      await Promise.all([first, second])
+      expect(events).toEqual(['first:start', 'second:start'])
+    } finally {
+      vi.useRealTimers()
+    }
+  })
+
   it('keys explicit and default host homes consistently', () => {
     const previousCodexHome = process.env.CODEX_HOME
     delete process.env.CODEX_HOME

src/main/text-generation/commit-message-text-generation.test.ts

@@ -540,6 +540,38 @@ describe('discoverCommitMessageModelsLocal', () => {
     }
   })
 
+  it('releases the Codex home after a discovery timeout once the child exits', async () => {
+    vi.useFakeTimers()
+    const firstChild = createMockDiscoveryChild()
+    const secondChild = createMockDiscoveryChild()
+    spawnMock.mockReturnValueOnce(firstChild as never).mockReturnValueOnce(secondChild as never)
+    const env = { CODEX_HOME: '/managed/codex-discovery-descendant-home' }
+
+    try {
+      const first = discoverCommitMessageModelsLocal('codex', env)
+      await vi.advanceTimersByTimeAsync(0)
+      const second = discoverCommitMessageModelsLocal('codex', env)
+      await vi.advanceTimersByTimeAsync(60_000)
+      await expect(first).resolves.toMatchObject({ success: false })
+      expect(spawnMock).toHaveBeenCalledTimes(1)
+
+      // A grandchild kept the inherited stdout open, so the killed child reports
+      // 'exit' and 'close' never arrives.
+      firstChild.emit('exit', null, 'SIGKILL')
+      await vi.advanceTimersByTimeAsync(0)
+      expect(spawnMock).toHaveBeenCalledTimes(2)
+
+      secondChild.stdout.emit(
+        'data',
+        Buffer.from(JSON.stringify({ models: [{ slug: 'gpt-5.5', display_name: 'GPT-5.5' }] }))
+      )
+      secondChild.emit('close', 0)
+      await expect(second).resolves.toMatchObject({ success: true, defaultModelId: 'gpt-5.5' })
+    } finally {
+      vi.useRealTimers()
+    }
+  })
+
   it('settles and detaches model discovery when output exceeds the limit', async () => {
     const child = createMockDiscoveryChild()
     spawnMock.mockReturnValue(child as never)
@@ -1692,6 +1724,39 @@ describe('generateCommitMessageFromContext', () => {
     await expect(second).resolves.toMatchObject({ success: true, message: 'Update README' })
   })
 
+  it('releases the Codex home lock when the child exits with a descendant holding its stdio', async () => {
+    const firstChild = createMockDiscoveryChild()
+    const secondChild = createMockDiscoveryChild()
+    spawnMock.mockReturnValueOnce(firstChild as never).mockReturnValueOnce(secondChild as never)
+    const env = { CODEX_HOME: '/managed/codex-descendant-home' }
+    const context = { branch: 'main', stagedSummary: 'M\tREADME.md', stagedPatch: '+hello' }
+    const params = { agentId: 'codex' as const, model: 'gpt-5.5' }
+
+    const first = generateCommitMessageFromContext(context, params, {
+      kind: 'local',
+      cwd: '/descendant-repo',
+      env
+    })
+    await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledTimes(1))
+    cancelGenerateCommitMessageLocal('/descendant-repo')
+    await expect(first).resolves.toMatchObject({ canceled: true })
+    expectChildTerminated(firstChild)
+
+    // SIGKILL reaches the codex process but not a grandchild that inherited its
+    // stdout, so 'exit' arrives and 'close' never does.
+    firstChild.emit('exit', null, 'SIGKILL')
+
+    const second = generateCommitMessageFromContext(context, params, {
+      kind: 'local',
+      cwd: '/descendant-repo-2',
+      env
+    })
+    await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledTimes(2))
+    secondChild.stdout.emit('data', Buffer.from('Update README\n'))
+    secondChild.emit('close', 0)
+    await expect(second).resolves.toMatchObject({ success: true, message: 'Update README' })
+  })
+
   it('holds the Codex home lock until Windows tree termination and wrapper close', async () => {
     const originalPlatform = process.platform
     Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })

src/main/text-generation/commit-message-text-generation.ts

@@ -452,6 +452,10 @@ export async function discoverCommitMessageModelsLocal(
       if (agentId === 'codex') {
         // Result publication stays prompt while the home lock follows the
         // process lifetime after asynchronous timeout/output-limit kills.
+        // Why: 'close' also waits on descendants that inherited this child's
+        // stdio, so a surviving MCP helper would hold the home forever; at
+        // 'exit' the codex process is gone and can no longer rotate auth.json.
+        child.once('exit', markClosedAfterTermination)
         child.once('close', markClosedAfterTermination)
       }
       child.on('error', onError)
@@ -604,7 +608,7 @@ function runLocalPlan(
   emptyResultName = 'message',
   operation: TextGenerationOperation = 'commit-message',
   wslDistro?: string,
-  holdHomeLockUntilClose = false
+  holdHomeLockUntilExit = false
 ): LocalProcessExecution<InternalTextGenerationResult> {
   const { binary, args, stdinPayload, label } = plan
   let markProcessClosed!: () => void
@@ -685,7 +689,7 @@ function runLocalPlan(
       if (cancelToken && cancelTokensByLane.get(laneKey) === cancelToken) {
         cancelTokensByLane.delete(laneKey)
       }
-      if (!holdHomeLockUntilClose) {
+      if (!holdHomeLockUntilExit) {
         markProcessClosed()
       }
       resolve(result)
@@ -769,7 +773,11 @@ function runLocalPlan(
     }
     child.stdout?.on('data', onStdoutData)
     child.stderr?.on('data', onStderrData)
-    if (holdHomeLockUntilClose) {
+    if (holdHomeLockUntilExit) {
+      // Why: 'close' also waits on descendants that inherited this child's
+      // stdio, so a surviving MCP helper would hold the home forever; at 'exit'
+      // the codex process is gone and can no longer rotate auth.json.
+      child.once('exit', markClosedAfterTermination)
       child.once('close', markClosedAfterTermination)
     }
     child.on('error', onError)
@@ -801,7 +809,7 @@ function runLocalPlanForAgent(
   operation: TextGenerationOperation
 ): Promise<InternalTextGenerationResult> {
   const start = (
-    holdHomeLockUntilClose = false
+    holdHomeLockUntilExit = false
   ): LocalProcessExecution<InternalTextGenerationResult> =>
     runLocalPlan(
       plan,
@@ -810,7 +818,7 @@ function runLocalPlanForAgent(
       emptyResultName,
       operation,
       target.wslDistro,
-      holdHomeLockUntilClose
+      holdHomeLockUntilExit
     )
   if (agentId !== 'codex') {
     // Why: no extra promise hops here — cancellation timing for non-codex