Repository navigation
fix(runner): preserve slow Codex sessions across startup deadlines - #2078
techotaku39 wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Review mode: initial
Requirement — Pass
Preserving slow Codex launches avoids terminating a healthy execution solely because startup confirmation exceeds a deadline.
Evidence
- The PR description identifies slow fresh Codex startup and requires late confirmation of the same tracked session.
- Base cli/src/runner/run.ts:971 removes session tracking on timeout, and line 980 tree-kills the child.
Approach — Pass
Separating deadline expiry from terminal failure is sound. Retained callbacks and deduplication support late readiness, while the longer Hub timeout accommodates startup.
Evidence
- cli/src/runner/sessionStartup.ts:20 transitions to unconfirmed without finishing the startup state machine.
- cli/src/runner/run.ts:948 retains readiness/error callbacks and recovers the deduplicated result on late confirmation.
- cli/src/runner/run.ts:453 prevents sibling webhooks from confirming the requested primary session; hub/src/sync/rpcGateway.ts:280 extends Codex spawn RPCs.
Code — Reviewed
Reviewed the entire fixed-head merge-base diff. Found one failure-cleanup regression.
- [Minor] Clean up the worktree after a late startup failure —
cli/src/runner/run.ts:957
Testing
Not run (automation; PR code execution is prohibited).
- At the fixed head, integration CI passed; test and windows-codex-mcp were still running.
- Added tests cover deadline transitions, late confirmation/failure, deduplication, and RPC timeout selection, but not worktree cleanup after late failure.
- The PR reports additional Windows and task-local acceptance checks. These were not independently verified; no PR code, tests, builds, or scripts were executed.
HAPI Bot
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Preserving slow Codex launches prevents healthy sessions from being terminated solely because startup confirmation exceeds a deadline.
Evidence
- The snapshot PR description requires late readiness to complete the same tracked session while preserving explicit stop.
- Base cli/src/runner/run.ts:971 removes tracking on timeout, and line 980 terminates the child process tree.
Approach — Pass
Separating deadline expiry from terminal failure addresses the problem while retaining process tracking, deduplication, and late confirmation.
Evidence
- cli/src/runner/sessionStartup.ts:32 returns an unconfirmed diagnostic without finishing the startup state machine.
- cli/src/runner/run.ts:948 retains readiness and failure callbacks and recovers the deduplicated result on late readiness.
- cli/src/runner/run.ts:453 prevents sibling-root webhooks from confirming the primary launch; hub/src/sync/rpcGateway.ts:280 gives Codex spawn RPCs 60 seconds.
- cli/src/runner/run.ts:957 schedules failed-startup worktree cleanup after exit, guarded against successful startup.
Code — Reviewed
Reviewed the entire fixed-head merge-base diff and relevant surrounding code. The prior late-failure worktree-cleanup finding is addressed. No additional actionable defects found. Runtime behavior was assessed statically.
No reportable code issues found.
Testing
Not run (automation; PR code execution is prohibited).
- At fixed head a3670ef, integration and windows-codex-mcp CI passed; test remained in progress.
- Added unit coverage exercises startup deadlines, late confirmation and failure, deduplication, exit-gated worktree cleanup, and RPC timeout selection.
- The PR reports additional Windows and task-local acceptance checks; those results were not independently verified, and the acceptance harnesses are absent from the diff.
- No PR code, tests, builds, or scripts were executed. The fixed-commit diff whitespace check passed.
HAPI Bot
Summary
Fresh Codex sessions can exceed Runner's 15-second startup deadline. Timeout cleanup then terminates the process even when a late webhook reports a healthy shared execution.
Validation
Windows, Bun 1.4.0:
bunx vitest run --config vitest.startup.local.config.ts— 97 passed; 109 passed on the existing combined build. Includes 18 new regression tests.bun test hub/src/sync/rpcGateway.test.ts— 13 passed, including the new spawn-budget test.bun run typecheck,bun run build,bun run --cwd cli build:exe:allinone— passed.Invoke-HapiTaskPlaywright.ps1 -Name fix-codex-startup-handshake -Suite Root terminal-wrap-fidelity.spec.ts— 2 passed.Invoke-HapiTaskPlaywright.ps1 -Name startup-handshake-combined-check -Suite Root --config playwright.startup.local.config.ts— 9 passed on combined build 0621c57a with this patch: slow creation with/without early navigation, late confirmation of the same PID, sibling protection, assistant replies and continued activity, archive/reopen, explicit stop during unconfirmed startup, eventual native failure, failed-startup worktree removal, and successful-startup worktree preservation.The acceptance harnesses are task-local. They use native Codex 0.162.1 with a local model fixture. Navigation tests hold delivery of the spawn response until the list click to guarantee the Creating state. All homes, databases and ports are isolated; environments were stopped afterward.
Related Issues
None.
Related PRs: #2035 and #2046.
AI Disclosure
Assisted by OpenAI Codex, using GPT-6.