Repository navigation
Fix Codex chat creation after its own model listing fails - #25754
Conversation
7aad63a to
aa8dea1
Compare
aa8dea1 to
9dc8d61
Compare
…rives - A live model, effort or Fast pick with no stored model list is kept as the next turn's intent instead of refused; Codex judges the model on turn/start and Fast without a known tier still sends Standard. - Restoring saved Fast checks the saved model the turn will send, not the resumed thread's old model; the thread's effort is only reused for its own model. - turn/start reads the exact Fast tier from the account's stored catalog at send time, so a tier any writer stored applies; the per-session copy is gone. - Revert the delivery-loop extraction; that file matches main again.
…2025/codex-acquisition-catalog-followup
…ach tests Merged main (#25159) requires agents on attach and an agent definition for catalog access; the PR's new acquisition attach tests and Fast-mode tests now pass them.
|
Problem. A Codex chat could show "Chat could not be started." with Retry even though Codex had already opened its thread. Orca waited for that chat's own model list while confirming it owned the chat, so a failed list made a working chat look broken and a stalled list held chat start until the request timed out. This hit both new-tab Codex chats and Resolve with AI. What changes for the user. The chat opens as soon as Codex's thread is ready. A failed or stalled model list no longer holds a message, Stop, a follow-up sent into a running turn, or a model, effort, or Fast choice. A choice made before the model list arrives (for example while a first chat is starting) is saved and used by the next message instead of being refused; if Codex rejects that model, the message fails with Codex's reason and the chat stays usable. Fast mode. With Fast saved and the exact Fast tier already known, the first message runs Fast. If the tier is not known yet, the first message runs Standard right away and later messages run Fast once the list arrives; the saved Fast choice stays on. This is a deliberate trade: Orca does not guess a paid tier Codex has not named for that model. Fixed during review. Earlier review rounds removed every model-list wait from the chat's action queue: a send-time Fast lookup that could hold Stop and follow-ups, a picker read and option validation that could queue sends behind a stalled list, and a restore that overwrote the saved next-turn model. The final round fixed a refused choice made during chat launch, a reopened chat switching saved Fast off by checking the wrong model, a reopened chat reusing another model's effort, and made each message read the exact Fast tier from the shared catalog. It also reverted an unneeded code move and fixed two tests broken by a newer main change. Deferred follow-ups (low impact). While a chat's model list is still loading, the model button stays disabled (up to about 30 s); typing and Send are never blocked, and this is existing behaviour that is now reachable because the chat opens. After a model list fails, nothing asks again until the next message starts, so the picker keeps Orca's built-in model list and saved Fast stays Standard for one extra message; the follow-up is to re-read when the picker opens after a failed read. A later list saying the saved model has no Fast can turn Fast off for the live chat while the saved choice stays on until the next start. A cold picker and the background refresh can send two bounded model-list requests. The picker's own listing has no overall time cap beyond per-page limits. Skipping the live model read at chat start is opted into per agent (Codex here), so a future agent could bring the startup wait back; making every agent start from the options it already knows is a follow-up. Verified. Local hidden Mac app run with a stub Codex on exactly 2fb9d47: with the chat's own model list rejected, Resolve with AI and new-tab Codex both start and answer, and still answer after 8+ idle minutes; with the list stalled, the chat starts in under 2 s, a model pick is kept and used by the next message, and steer, Stop, and a send after Stop do not wait; saved Fast with a known tier runs Fast on both turns; saved Fast after a restart with the catalog deleted and the list stalled sends Standard immediately and returns to Fast once a list arrives. On main, the same rejected or stalled list shows the start failure for both entry points. Focused stub tests (219 across the 16 touched files, plus catalog store tests), scoped Node typecheck, and the changed-code quality gate pass. CI on 2fb9d47 is fully green, including all five test shards and cross-version wire compatibility; neither of main's two known failures (the file-explorer bounded-scan test, with a separate fix in progress on main, and cross-version wire / streamparser, #25796) appeared on this head. Not verified. Real Codex CLI, remote machines, Linux, Windows, and mobile UI. Structured Codex chats run only on the local host, and this change adds no wire field or platform-specific path. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughCodex model and Fast-mode option handling now uses stored catalog data and supports background catalog refresh. Catalog requests can use a shared deadline. Session adapters add prepared live option reads and acquisition-time option reporting. The session wire applies prepared results only when the child remains unchanged. Tests cover catalog refresh, Fast-tier dispatch, option selection, acquisition restoration, and live option reads. Priority: ➖ Normal Merge Risk: 🔵 Low · up to The catalog refresh does not appear to update a closed or replaced chat. The prepared-read concern still warrants owner follow-up, but the established risk is bounded. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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:
9599d81e-db9b-44b6-a699-2cef0542e53b
📒 Files selected for processing (30)
src/main/codex/codex-close-turn-end.test.tssrc/main/codex/codex-configured-launch-defaults.tssrc/main/codex/codex-requested-close-turn-timing.test.tssrc/main/codex/codex-structured-acquire-catalog.tssrc/main/codex/codex-structured-acquisition-notification.tssrc/main/codex/codex-structured-background-catalog.tssrc/main/codex/codex-structured-catalog-entry.tssrc/main/codex/codex-structured-fast-mode.test.tssrc/main/codex/codex-structured-fast-mode.tssrc/main/codex/codex-structured-model-catalog.tssrc/main/codex/codex-structured-session-acquire.tssrc/main/codex/codex-structured-session-adapter.test.tssrc/main/codex/codex-structured-session-adapter.tssrc/main/codex/codex-structured-session-close.test.tssrc/main/codex/codex-structured-session-options-catalog.test.tssrc/main/codex/codex-structured-session-options.test.tssrc/main/codex/codex-structured-session-options.tssrc/main/codex/codex-structured-session-prompts.test.tssrc/main/codex/codex-structured-session-state.tssrc/main/codex/codex-structured-turn-start.tssrc/main/codex/codex-structured-turn-steer.test.tssrc/main/native-chat/agent-model-catalog/agent-model-catalog-store.tssrc/main/native-chat/agent-session-wire/structured-agent-session-acquisition-options.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-acquisition.tssrc/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.tssrc/main/native-chat/agent-session-wire/structured-agent-session-adapter.tssrc/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-options-read.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-options-read.ts
💤 Files with no reviewable changes (6)
- src/main/codex/codex-structured-session-state.ts
- src/main/codex/codex-structured-session-close.test.ts
- src/main/codex/codex-close-turn-end.test.ts
- src/main/codex/codex-structured-fast-mode.ts
- src/main/codex/codex-requested-close-turn-timing.test.ts
- src/main/native-chat/agent-model-catalog/agent-model-catalog-store.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
| context.serialize(sessionId, async () => { | ||
| const child = (await context.openConversation(sessionId))?.child | ||
| return child === started.child ? apply() : null | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Map the second openConversation failure to a read refusal.
The first serialized section wraps openConversation failures with journalOpenReadRefusal. The apply section at Line 140 calls openConversation again without that wrapper. The conversation can fail to reopen while the prepared listing is pending. In that case the picker gets a raw error instead of an AgentSessionRefusalError. The raw error also skips the logging that journalOpenReadRefusal does. Use the same mapping in both sections.
🐛 Proposed fix
context.serialize(sessionId, async () => {
- const child = (await context.openConversation(sessionId))?.child
+ const reopened = await context.openConversation(sessionId).catch((error: unknown) => {
+ throw journalOpenReadRefusal(error, context.deps.logger, sessionId)
+ })
+ const child = reopened?.child
return child === started.child ? apply() : null
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| context.serialize(sessionId, async () => { | |
| const child = (await context.openConversation(sessionId))?.child | |
| return child === started.child ? apply() : null | |
| }) | |
| context.serialize(sessionId, async () => { | |
| const reopened = await context.openConversation(sessionId).catch((error: unknown) => { | |
| throw journalOpenReadRefusal(error, context.deps.logger, sessionId) | |
| }) | |
| const child = reopened?.child | |
| return child === started.child ? apply() : null | |
| }) |
…quisition-catalog-followup # Conflicts: # src/main/codex/codex-structured-session-acquire.ts
1 participant
ELI5
A Codex chat could show “Chat could not be started” and Retry after Codex had already opened its thread. Orca still waited for that chat's model list during startup, so a failed or stalled list made a usable chat look broken.
The chat now opens when its Codex thread is ready. A failed or stalled model list does not hold a message, Stop, a follow-up sent to a running turn, or a model, effort, or Fast choice.
With saved Fast mode and an already known exact Fast tier, the first message still runs Fast. If no exact tier is known yet, the first message runs Standard immediately; a later message runs Fast once the model list arrives. The saved Fast choice stays on.
What Changed
Why
A successful Codex thread start proves the chat is ready; model discovery is separate work. Waiting for discovery at startup or inside the chat's action queue made a usable chat fail, or made Stop, later messages, and model choices wait behind a stalled request.
Orca saves Fast as on or off. The exact tier value Codex expects comes from its model list and can differ by model, so when that value is not known yet Orca sends Standard explicitly rather than guess a paid tier or rely on a shorthand Codex marks for removal. Sending Standard explicitly also means a resumed thread does not silently keep a paid tier.
Linked Issue
Related: #23831, which is now on the main branch. That change prevents a separate probe from holding chat startup and protects a newer catalog answer from an older probe; this PR removes the remaining wait on the chat's own model list.
Visual Proof
Local hidden Mac app run with a stub Codex whose own model list is rejected or stalled.
Before (main): a new-tab Codex chat whose thread opened still shows "Chat could not be started." with Retry when its own model list is rejected.
After (this PR, same rejected list): the chat opens and answers, including after an idle period.
After (this PR, model list stalled): a model picked with no list loaded is kept (GPT-5.6 Luna) and used by the next message; a long turn is stopped and a message sent after Stop answers right away.
Testing
Manual rows on this head: rejected list (Resolve with AI and new-tab Codex both start and answer; still answers after 8+ idle minutes); stalled list (chat starts in under 2 s; a model pick is kept and used; steer, Stop, and a send after Stop do not wait on the list); saved Fast with a known tier (both turns Fast); saved Fast with the catalog deleted and the list stalled after a restart (first message Standard immediately, Fast again once a list arrives). On main, both entry points show "Chat could not be started." for the rejected and stalled rows. No real Codex CLI was used.
On macOS against current main, 219 tests passed across the 16 focused Vitest files this PR touches, plus the catalog store tests; scoped Node typecheck and the changed-code quality gate passed; CI is green. Linux, Windows, remote machines, and mobile were not tested; structured Codex chats run only on the local host, and the change adds no platform-specific path, shortcut, or wire field.
Review
Reviewed over five rounds; the final summary, fixes, and deferred follow-ups are in the PR comment.
Agent skill upstream boundary
Checklist