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

[OrcaWin]

Defect

The per-Codex-home process lock (withCodexHomeProcessLock) is held until LocalProcessExecution.processClosed resolves, and that promise was resolved only from the child's close event (child.once('close', markClosedAfterTermination) in both runLocalPlan and discoverCommitMessageModelsLocal).

close fires only when the child and every process that inherited its stdio have exited. killProcessTree sends SIGKILL to the direct child on POSIX, which does not reach a grandchild that inherited stdout — an MCP server, a helper process, or an sh -c wrapper. The codex process dies, the descendant keeps the pipe open, close never fires, and the lock is never released.

Failure scenario

1. User hits "Generate commit message" with Codex; the codex CLI spawns an MCP server that inherits stdout.
2. User hits Stop (or the 60s GENERATION_TIMEOUT_MS fires). killProcessTree SIGKILLs codex; the MCP grandchild survives.
3. Codex emits exit; close never arrives.
4. Every later AI commit message, PR description, branch name, and Codex quota probe for that CODEX_HOME queues behind the never-settling hold. No error, no spinner end, no recovery — the feature is dead until the app restarts.

The same wedge existed on the model-discovery path, which holds the same lock.

Fix

• 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 — the exact race the lock exists to prevent — so waiting on unrelated descendants bought no safety. Renamed holdHomeLockUntilClose to holdHomeLockUntilExit to match.
• Keep close as a fallback and preserve the existing Windows tree-termination wait, so wrapper/process-tree cleanup still completes before the next run starts.
• Preserve strict mutual exclusion for a process that has not exited; elapsed time alone never releases the lock or allows a second Codex process to overlap it.

Verification

New tests, each confirmed to fail with the source change reverted and pass with it restored:

• commit-message-text-generation.test.ts — "releases the Codex home lock when the child exits with a descendant holding its stdio": cancel a Codex run, emit only exit, and assert the next Codex run for the same home still spawns. Without the fix it never spawns.
• commit-message-text-generation.test.ts — "releases the Codex home after a discovery timeout once the child exits": same for the model-discovery lane.
• codex-home-process-lock.test.ts — "does not release a running lock based on elapsed time": advance an hour while another run is queued and assert strict serialization remains intact.

Run: npx vitest run --config config/vitest.config.ts src/main/codex-cli/ src/main/rate-limits/ src/main/text-generation/ — 36 files, 531 tests pass. npx tsc --noEmit -p config/tsconfig.node.json, oxlint, oxfmt, reliability gates, and the max-lines ratchet are clean.

[coderabbitai]

📝 Walkthrough

Walkthrough

The change updates Codex process-home lock tracking to recognize child-process exit events in addition to close. It renames lock-control options to reflect exit-based handling and propagates the new names through local execution and wrapper functions. Tests cover long-running locks, timed-out model discovery, and cancelled commit-message generation when descendants retain stdio.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	⚠️ Warning	The description explains the defect, fix, failure scenario, and verification, but it omits required template sections for screenshots, AI review, and security audit.	Add the required Screenshots, AI Review Report, Security Audit, and Notes sections, and include the prescribed testing checklist.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the fix for surviving descendants that keep the Codex home process lock held.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[brennanb2025]

Electron QA — live app, this branch

Ran a dev build off this branch (cabezon @ fix/codex-home-lock-timeout, verified via app.getIdentity()), isolated ORCA_DEV_USER_DATA_PATH, scratch git repo, Codex set as the commit-message agent.

Cancel → regenerate on the same CODEX_HOME, through the real IPC:

cancelledAfterMs: 2502
first:  { success: false, error: "Generation canceled.", canceled: true }
secondAfterMs: 7209
second: { success: true, message: "Add QA test fixture file and README note", agentLabel: "Codex" }

The run cancelled mid-flight (so killProcessTree fired), and the next Codex run for the same home acquired the lock and completed. A UI-level generation through the Source Control button also succeeds end to end.

Generation in flight (stop control, staged changes):

Generated message landed in the commit box:

What this does and does not prove

Being explicit, because it matters for how much weight to put on the screenshots: on macOS a plain cancel usually leaves no surviving descendant holding the child's stdio, so close fires anyway and this same sequence would likely pass on main too. The live run is a no-regression check on the normal path, not a reproduction of the wedge.

The discriminating evidence is the unit tests. Reverting only the two source files to origin/main and keeping the tests:

Tests  8 failed | 68 passed (76)     Duration 92.70s  (tests 152.35s)

restoring them:

Tests  76 passed (76)                Duration 1.49s

Worth noting the failure shape: it is not confined to the new cases. Because lockTails is module-global, one wedged hold contaminates later tests in the same file — several unrelated cases (e.g. the Windows cmd.exe routing test) time out at 30s each. That is the same blast radius the fix describes in production: one wedge kills that home for the rest of the session.

Note on a lane the description does not mention

codex-fetcher.ts:1259 and :1297 are a third holder of this same lock. They are not affected: both self-bound via RPC_TIMEOUT_MS (10s), WSL_RPC_TIMEOUT_MS (25s), the init timeouts (30s/40s), and PTY_TIMEOUT_MS (15s), so they settle far below the new 5-minute cap and were never at wedge risk.

Not covered here: Windows and WSL process teardown, and the actual surviving-MCP-descendant scenario against a real Codex binary.

[brennanb2025]

Electron QA

Validated the current PR head (5aee22e980) in a fresh isolated macOS profile through Electron/CDP.

• App launched successfully from the updated branch.
• Selected Codex and advanced through the onboarding flow; the UI remained responsive.
• This PR changes main-process child lifecycle/locking with no new visual state, so the deterministic lock/process tests are the behavior proof. The screenshots provide app-level smoke evidence.
• The notification permission notice in the second screenshot is expected for the disposable profile.