Repository navigation
Opening or resuming a Codex chat waits for Codex instead of failing after 30 seconds - #30395
Conversation
…of failing after 30 s The handshake (15 s) and thread/start / thread/resume (30 s) had their own deadlines because the Codex start ignored the host's startup signal. It now honours it, as Claude's start does, so those requests carry no deadline: the host's startup limits, a close, a Stop or quit end a start that hangs, and the abort reaps Codex. Calls after the start keep the 30 s default for now: they run in the session's queue, which a Stop or a close waits behind.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the
Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 38.46% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 24 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly describes the primary behavior change: Codex chat opening and resuming no longer fails after the previous timeout. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description check The description is detailed and covers the required ELI5, changes, rationale, issue context, visual proof, testing, review, scope, and upstream-boundary sections. The linked issue is named but not pro… Full details: Description check
-
Explanation
The description is detailed and covers the required ELI5, changes, rationale, issue context, visual proof, testing, review, scope, and upstream-boundary sections. The linked issue is named but not provided as an actual issue link or
Fixes #reference, and the template checklist and testing checkboxes are omitted. These are minor completeness issues because the description provides substantial equivalent information.
-
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Comment @coderabbitai help to get the list of available commands.
… option as the chat connection
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Observe cancellation during process-identity lookup. · codex-structured-session-acquire.ts:214
src/main/codex/codex-structured-session-acquire.ts:214
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winObserve cancellation during process-identity lookup.
If the signal aborts while
spawnIdentity.read(connection.pid)is pending, acquisition does not observe that abort. If the read completes and the attempt remains current,sessions.setcan publish a session after cancellation. Race this read with the signal, then check the signal before publication.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
e7ac25df-c2c0-4217-83c7-919f27d284a4
📒 Files selected for processing: 11
src/main/codex/codex-app-server-connection-handshake.test.tssrc/main/codex/codex-app-server-connection-types.tssrc/main/codex/codex-app-server-connection.tssrc/main/codex/codex-app-server-handshake.tssrc/main/codex/codex-app-server-record-dispatch.tssrc/main/codex/codex-app-server-session.tssrc/main/codex/codex-structured-session-acquire.tssrc/main/codex/codex-structured-session-options-catalog.test.tssrc/main/codex/codex-structured-session-start-wait.test.tssrc/main/codex/codex-structured-thread-open.tssrc/main/codex/codex-structured-visuals.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
|
Opening or resuming a slow Codex chat could fail while Codex was still working, leaving the first message marked “Your message was not sent.” This PR lets structured chat startup wait for Codex, bounded by the host's ten-minute ceiling, close, Stop and quit. Other requests retain their 30-second default. Two review rounds complete; no open proven source findings. The final main merge preserves typed timeout diagnostics for numeric requests and removes the obsolete injected thread/start deadline test. The direct convenience opener retains its caller-owned 15-second initialize budget, preventing an unowned silent wait; structured chat explicitly disables that request timer. Independent narrow verification found no regression. Focused checks passed 49 tests before test cleanup and 58 afterward. Full PR Checks, including its aggregate, passed on exact head a1da600. GitHub reports CLEAN, out of draft, with all review threads resolved. Real final-head Electron QA passed, admitted by qa-gate on this Mac: real Codex 0.162.1 through the load balancer, isolated homes, separate Codex UI driver. A new chat waited through a 45.001-second hold of only the real thread/start answer, remained Working after 32 seconds, then received its real reply. Normal reply, Stop showing Interrupted, and a real recovery reply in the same chat also passed. All owned processes exited and ports were released. The full four-flow matrix previously passed on 8205c08 (M4Air, real Codex 0.163.0): new start 45s, same-thread resume 45s with history and follow-up, close during a hung start (real child exit 68ms, independently absent 93ms), and normal/Stop/recovery. Those resume/close results retain their earlier commit identity and were not rerun on a1da. The launcher's final-merge QA scope was new-start 45s plus normal/Stop/recovery. A live ten-minute wait, Stop/quit during hung startup, Linux and SSH were not manually exercised; clock-based tests cover the total ceiling. Deferred: post-start request limits remain pending Stop/close queue work tracked in #28051. Failed-start retry and returning an undelivered message with Send are separate lane work. Existing remote attach transport limits remain in the ledger. No new saved field, wire operation, setting or UI element. For Brennan's review
Main was 787e7c0 / Codex 0.159.2 on Windows; final head used Codex 0.162.1 on macOS. Both held only the real thread/start answer for 45s; different machine/CLI versions are disclosed. The PR body also retains the historical same-machine main-versus-branch resume comparison.
Earlier 8205 close proof: hung startup, after tab close. Judgment points: host signal around every startup await, explicit null versus default request timer, direct opener's caller-owned bound. |
…quest-deadline # Conflicts: # src/main/codex/codex-structured-session-acquire.ts
Resolve against #28046's host-awaited Codex open: main's structure for the connection, acquire and thread open, with this PR's change on top (no deadline on initialize and thread start/resume; the start's signal bounds them). The handshake drops its own signal (the open wraps it), the scripted Codex fixture honours a null timeout, and #28046's start-deadline test now pins a slow initialize delivering and the start ceiling ending a silent one. The two tests written for the old acquire-owned open are removed.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
ba016d11-22b8-4599-923a-1432879a1ed8
📒 Files selected for processing: 16
src/main/agent-hooks/server/server-status-application.tssrc/main/agent-hooks/server/server-status-update.tssrc/main/codex/codex-app-server-connection-types.tssrc/main/codex/codex-app-server-connection.tssrc/main/codex/codex-app-server-handshake.tssrc/main/codex/codex-scripted-child.test-fixture.tssrc/main/codex/codex-structured-session-start.tssrc/main/codex/codex-structured-thread-open.tssrc/main/codex/codex-structured-visuals.test.tssrc/main/daemon/pty-subprocess-spawn-file-foreground.test.tssrc/main/daemon/terminal-host-process-inspection.test.tssrc/main/github/github-remote-identity-parsing.test.tssrc/main/providers/local-pty-child-process-verdict.test.tssrc/main/providers/local-pty-provider-foreground-process.test.tssrc/main/runtime/structured-agent-session-codex-start-deadline.test.tssrc/renderer/src/components/terminal-pane/remote-pane-layout-push.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.
…7-30395-merge # Conflicts: # src/main/codex/codex-app-server-connection.ts
1 participant
ELI5
Opening or resuming a Codex chat could fail while Codex was still working. A new chat then showed “Codex couldn't start” and “Your message was not sent.” In STA-11117, Codex took more than 30 seconds to open a conversation. Now Orca waits for Codex during chat startup; closing the chat, Stop, quit and the existing ten-minute total start limit still end the wait.
What Changed
Structured Codex chat startup no longer has separate 15-second initialize or 30-second thread/start and thread/resume request deadlines. Each startup await uses the host's cancellation signal, and the host closes Codex when that signal fires. The connection accepts an explicit
timeoutMs: null; requests without that option retain the 30-second default.The direct convenience opener retains its caller-owned 15-second initialize budget: it cannot expose a connection for cancellation before initialization finishes. One-shot consumers already have whole-session budgets. This exception prevents a silent direct open from waiting forever; it does not apply to structured chat startup.
After merging #28046, the old 60-second startup-silence rule is gone. After merging #30992, its “Codex took too long to start (no response to thread/start after 30s)” message no longer appears for a structured chat open, because that request has no deadline. Typed timeout diagnostics and logging remain for timed requests. The obsolete injected thread/start-timeout test was removed; numeric interrupt timeout metadata, generic failure wording, slow initialize and the host ceiling remain covered.
Why
The incident was caused by an overloaded macOS file-event service, as recorded in #30992: Codex was slow and could still answer. A fixed request deadline turned that delay into a failed chat and an unsent message. The common pattern lets the owner of startup cancel it, rather than treating an individual request's elapsed time as proof of failure. Reusing the host's existing signal also covers close and Stop; raising the request deadline would retain competing owners.
Differences from the common pattern
No new setting, saved field, wire operation or UI element. Codex startup and cancellation remain on the execution host. Older paired servers retain their own behavior; no minimum server version is introduced.
Linked Issue
STA-11117; related diagnosis in #30992.
Testing
Visual Proof
Final head a1da600
A new chat's first thread/start answer was held 45.001 seconds, while initialize answered in 25 ms. At 32 seconds the chat still showed Working with no start/send failure; Codex then delivered the real first reply. A normal new chat replied, Stop interrupted an active generation (real interrupt acknowledged in 11 ms), and another message received a real reply in the same chat.
The baseline is pinned main 787e7c0, Codex 0.159.2 on Windows; final head used Codex 0.162.1 on macOS. Both delayed only the real thread/start answer for 45 seconds. This identifies the different machine/CLI versions; main was not rerun after the diagnostics merge.
Earlier full matrix on 8205c08, before the final diagnostics merge
Real Codex 0.163.0 on the admitted M4Air passed all four flows: first thread/start answer held 45.002 seconds then a reply; the same chat reopened through Session History with thread/resume held 45.003 seconds then a follow-up retaining history; close during a hung start ended the real child 68 ms after click began, independently absent within 93 ms; normal reply, Stop and recovery. The final merge was checked narrowly, and the new-chat/normal/Stop/recovery paths above were rerun on a1da. Resume and hung-start close were not rerun on a1da.
Earlier same-machine Windows resume comparison, main c794225 versus branch ff908c7, real Codex 0.159.2: main failed at 30 seconds; the branch waited beyond 30 seconds and completed after the held answer was released.
Not manually exercised: live ten-minute exhaustion, Stop or quit during a hung startup, Linux or SSH. The total ceiling is covered with a fake clock. The existing completed-turn timer excludes startup; its behavior was traced and left unchanged.
Review
Full-path review completed on a1da600. Review status and evidence.
Agent skill upstream boundary
Author: @BrennanKB5