fix(codex): a Codex native chat that never sent a message reopens after restart - #22639
Conversation
A structured Codex chat records its thread at create time, but Codex writes no rollout until the first input. After a restart, launch resumed that thread, Codex answered "no rollout found for thread id", and the chat could never run again. When the head of the handle chain is the session's own creation and Codex answers that exact error for that exact thread, start a new thread instead. The new link supersedes the unsaved creation in place and names it, so the chain keeps one live identity and does not grow across restarts. A thread a resume, fork or adoption proved is never superseded, and no other resume error starts fresh.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughThe launch resolver enables supersession only for a resumed thread with a created provider handle origin. If the resume request returns the exact no-rollout error for that thread, the opener starts a replacement thread. The session acquisition path records the replaced thread in the new provider handle link. The provider handle chain validates supersession and replaces the unsaved created link. Added tests cover eligibility, error matching, handle validation, and persistence across store restarts. Merge Risk: 🔵 Low · up to A narrowly shaped resume error could start a replacement thread when the error should instead be reported. Tighten the message check before merging, or accept this bounded risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run reviewed the full PR: how a Codex chat whose thread was never saved is restarted after an Orca restart, and how that replacement is recorded in the durable provider-handle chain.
- Codex's own no-rollout answer gates the restart —
openCodexThreadstarts a freshthread/startonly whensupersedeIfUnsavedis set andthread/resumefailed withmethod === 'thread/resume',code === -32600, and the exact messagecodex app-server thread/resume failed: no rollout found for thread id <id>; any other failure is rethrown unchanged. - Only a creation is supersedable —
createCodexStructuredLaunchResolversetssupersedeIfUnsavedonly when the chain head's origin iscreated;resumed,forked, andadoptedheads keep their failure visible. - Supersession replaces in place —
appendAgentSessionProviderHandleLinkroutes acreatedlink carryingsupersedesKeytosupersedeUnsavedCreation, which validates that the head iscreated, the key names exactly the head, the root differs, the linkId is new, and the fence does not decrease, then returns a one-link chain. - The record store stays consistent — the acquisition reports
resumed: falseplussupersededThreadId, so the new link is acreatedlink withsupersedesKey;proveAgentSessionOwnerrepointsprovenHandleLinkIdat the new head, and the load-time validator accepts the one-link form while rejecting a persisted two-link[created, created+supersedes].
I traced the error plumbing (the record dispatcher composes the exact wrapper message the matcher expects, matching the existing -32600 / no rollout found for thread id fixture), confirmed resolvePinnedCodexRolloutProof returns null rather than throwing for a missing rollout, and checked every production consumer of providerHandleChain (the origin === 'adopted' first-link check, the mintedAtFence === runtimeFence owner probe, restart reconcile/rollback, and the ownership index). A specialist independently probed for a reachable non-idempotent re-prove, a dangling provenHandleLinkId, or an invariant violation and found none. The new tests pin exact behavior rather than loose assertions, including the ablations the PR describes.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
…e real connection The fallback matched Orca's own error-wrapper prefix too, and every test built that string itself, so rewording the wrapper would have disabled the fallback with the suite green. Match the method, code -32600 and Codex's exact detail as the message suffix, and drive Codex's raw error frame through the real connection in a test. The link builder now refuses, at the type level, a supersession on an adopted or resumed link, which the chain would reject downstream anyway.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This incremental run reviewed the delta since the prior pullfrog review, commit 6c91609bf5 ("match only Codex's own no-rollout text, pinned through the real connection").
- Match Codex's no-rollout proof by suffix —
isCodexNoRolloutErrornow testserror.message.endsWith('no rollout found for thread id <id>')instead of the fully wrapped string, so a reworded Orca error prefix can no longer hide the proof;method === 'thread/resume'andcode === -32600still gate it, and the trailing thread id (with its preceding space) keeps a longer, different id from matching. - Pin the proof to Codex's raw frame — a new test drives Codex's raw JSON-RPC error frame through the real
openCodexAppServerConnection, so the match cannot silently drift from Codex's wording. - Restrict supersession to a creation at the type level —
codexProviderHandleLinknow takes a discriminated input union that rejectssupersedesThreadIdon anadoptedorresumedlink (checked by@ts-expect-errorpins), withcodex-structured-session-acquirepassingresumed: falsealongsidesupersededThreadId.
Verified in this run: the four touched test files pass (70 tests, including the raw-frame case) and pnpm tc:node exits 0, so the type pins are live.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
![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 · Require exact error-message equality before replacement. · codex-structured-thread-open.ts:105-122
src/main/codex/codex-structured-thread-open.ts:105-122
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRequire exact error-message equality before replacement.
endsWithalso matches messages such ascontext: no rollout found for thread id ${threadId}. The reachablethread/resumeerror path then callsthread/startand replaces the thread instead of propagating the unrelated error. Compare the complete message with the exact fallback text.Suggested fix
- return error.message.endsWith(`no rollout found for thread id ${threadId}`) + return error.message === `no rollout found for thread id ${threadId}`
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: af1dfc0e-241e-4e08-b0c9-9a7c2a8117c3
📒 Files selected for processing (1)
src/main/codex/codex-structured-thread-open.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/codex/codex-structured-thread-open.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
brennanb2025
changed the title
Electron QA (independent validation agent)Independent live run of the scenarios in the QA spec, on this branch at
Results: scenario 1 PASS (chat A answered Chain table from
Nothing unexpected: model and options unchanged across restarts, exactly the 2 created chats (no duplicates), no error toasts, no stale thread ids shown, no refusal at any send. |
Review status: ready to mergeHead reviewed: Review loop. One independent review pass came back clean. It checked:
Architecture. The approach holds up. Whether Codex can resume a thread is decided by Codex's own answer about its own storage, and nothing in Orca stores a second copy of that fact. The fallback needs the exact error, for the exact thread, on a head Orca created. The main alternative is to start the Codex thread at the first send instead of at create, which removes this class of bug. It is a larger change, because the model picker and the lease both need a live session, so it stays a follow-up, as the description says. Description corrections (no code change):
Live QA. An independent validation agent ran it live; see the comment above. The run used a hidden-window dev build of this head, an isolated profile, and the real Codex login.
Readiness checks at the start and end of this review both passed:
Remaining gaps (not blocking):
|
1 participant
Scope
Codex native chat only. Every behaviour change is in the Codex native chat launch path (
src/main/codex/codex-structured-*). Out of scope, and unchanged:The one shared file,
src/shared/agent-session-provider-handle.ts, gains a way to mark an unused Codex thread as replaced. Only a chat's own never-used created handle may use it.ELI5
If you opened a Codex native chat and never sent anything, the chat broke after an Orca restart. Orca tried to reopen the Codex conversation it had started, but Codex only saves a conversation after the first message. Codex answered "no rollout found", and every message after that was refused. Now, when Codex itself says it never saved that conversation, Orca starts a new one in its place and the chat works. A chat that did have a conversation still reopens that conversation, with its history.
What Changed
Before. At create time, Orca starts a Codex thread and records it as the chat's
createdprovider handle (codex-structured-session-acquire.ts). Every later launch treated any recorded handle as resumable and sentthread/resume(codex-structured-launch-resolution.ts). Codex writes no rollout until the first input, so after a restart the resume of a chat that never ran a turn failed withno rollout found for thread id <id>. The failed attach released the lease, and the next send was refused with "The session has no live owner to accept writes."After.
Codex's own answer decides.
openCodexThread(codex-structured-thread-open.ts) starts a new thread only when all of these hold:thread/resumefailed with code-32600.no rollout found for thread id <that thread id>.Codex's app-server matches that exact text itself. Orca's own error-wrapper prefix is deliberately not part of the match, and a test drives Codex's raw JSON-RPC error frame through the real connection so the two cannot drift apart unnoticed. Any other resume failure is thrown as before, including a generic "not found", another thread id, another code, or a dead process. A chat with a real conversation is never silently restarted without its history.
Only a creation can be replaced.
createCodexStructuredLaunchResolversetssupersedeIfUnsavedonly when the chain head's origin iscreated. Aresumedhead (Codex already resumed it once, so a rollout existed), aforkedhead or anadoptedhead (an imported conversation) never gets the fallback. There, a failed resume stays a visible failure.The handle chain models the replacement as a supersession. A link gains an optional
supersedesKey, the key of the handle it replaced, alongside the existingforkedFromKey. The new thread's link isorigin: 'created'withsupersedesKey. It replaces the unsaved creation in place (appendAgentSessionProviderHandleLink→supersedeUnsavedCreation), so the chain holds one live identity and does not grow across restarts. Before this change, the chain refused a secondcreatedlink outright (agent_session_provider_handle_invalid).Handle-chain invariants (all kept; the last one is new)
A chain starts with exactly one
createdoradoptedlink, and no later link iscreatedoradopted.Every link has the same provider.
mintedAtFencenever decreases along the chain.A
resumedlink keeps the identity root. Landing on another root is a fork, and is refused as one.A
forkedlink has a new root and names the head it was seeded from (forkedFromKey).Re-proving the same handle at the same fence is a retry and adds no link.
Link ids are unique, because the lease names its proof by link id.
At most 256 links.
New: a
createdlink withsupersedesKeymay replace the head only when all of these hold:created, which by (1) means it is the only link.supersedesKeyequals the head's key.The result is a one-link chain.
supersedesKeyis only valid on acreatedlink. A persisted chain of[created, created+supersedes]is invalid, so the replacement cannot be stored as a second live identity.The lease keeps its current meaning.
proveOwnersetsprovenHandleLinkIdto the new head, andisAgentSessionRecord's live-lease check (head link id equals the proven id, at the lease's fence) holds unchanged.Why
CODEX_HOME/sessionsto passpath, and that scan stays. But Codex is the authority on whether it can resume a thread. Its own lookup covers layouts Orca's scan does not. A scan that missed a real rollout, for example after a storage-layout change, would start a new thread over a real conversation. That is the silent history loss this fix must never cause. With Codex's answer, a missed scan only means Codex looks the thread up itself. A change to Codex's message text makes the fallback stop matching, so the failure stays visible.created, every turn went through Orca's own chat path, because a terminal handoff records aresumedlink and the fallback stops applying after that. It would still be a second, stored copy of a fact Codex already owns, and the two can disagree. Codex is the only authority on whether it can resume its own thread. Checked beside Codex's answer, the journal fact would change the outcome in one case only: a saved conversation deleted outside Orca before the chat's first restart (see Known limit), where Codex has nothing left to resume either way.path, and Codex resumes by path. Supersession therefore needs two independent misses: Orca's scan finds no rollout, and Codex's own lookup by id finds none either. Codex uses the same "no rollout found" text for an archived thread read active-only, but its resume reads archived threads too and answers "is archived" instead. So on the resume path the text means no rollout exists at all, and an archived thread stays a visible failure.supersedesKeynames only the thread it directly replaced. Across repeated restarts of a never-used chat, only the most recent superseded thread id is kept; earlier discarded ids survive only in the operation ledger, until the ledger evicts them (24 hours, 512 operations per client, 4096 overall). None of them ever held a conversation.--session-idwhen Claude wrote no transcript, decided from Claude's own storage. Codex mints its own thread id and cannot reuse the old one, so it needs the chain supersession. The rule is the same in both: resumability comes from whether the provider actually saved the conversation, not from Orca's record of the handle.The send refusal ("no live owner")
That refusal comes from the shared, provider-agnostic admission path:
refuseUnlessWriterAdmittedinsrc/shared/agent-session-mutation-envelope.ts, reached from the host's mutation admission for both Claude and Codex. #22364 replaces that path for both providers:ensureProviderChild).agent_session_owner_restart_failed, which names the cause, for example "Codex couldn't restart: codex app-server thread/resume failed: …". It also writes the same status row into the chat.Adding a second cause-carrying refusal here would put a parallel mechanism beside that one on the same shared path, and they would conflict. So this PR leaves the refusal to #22364. After this PR, the unrun-chat case no longer fails at all. A real Codex resume failure gets its cause shown through #22364's path. #22364 is now on
main, so that path is live.Linked Issue
None — part of the structured chat status/orchestration program.
Visual Proof
Hidden-window Electron validation (
ORCA_BACKGROUND_LAUNCH=1, CDP screenshots), real Codex app-server, real~/.codex.Before (
main122b8c2): a Codex chat that never ran a turn, after restart, then a send.After (this branch): a new Codex chat, nothing sent, app restarted, then a send. Codex answers.
Control (this branch): a Codex chat that ran one turn before the restart resumes the same thread, and Codex recalls its earlier reply.
After a second restart: the formerly unrun chat now has a saved conversation. It resumes normally and recalls its reply.
Testing
Live run (macOS, dev build of this branch, isolated profile). Durable records read from
agent-sessions.jsonafter each step:codex_5c9ec790codex_ebbd5c65[created 01a0d2f8-8ad7…]; no rollout file for that id. The positive control, other rollouts from today, is present.[created 01a0d2f8-f5a8…]; one turn sent, rollout writtenlivefence 3,[created 01a0d2fb-375d… supersedes 01a0d2f8-8ad7…]. A send is answered and a rollout is written for the new thread.livefence 3,[created, resumed]on the same thread; recalls its earlier replylivefence 5,[created 01a0d2fb… (supersedes …), resumed 01a0d2fb…]; recalls its earlier replyA Codex chat created on
main,codex_b97206fa, was also never sent to. It superseded its unsaved thread on each of both restarts, and its chain stayed one link long.Automated.
agent-session-provider-handle.test.ts(supersession rules)codex-structured-thread-open.test.ts(exact-proof fallback, and no fallback for any other failure)codex-structured-launch-resolution.test.ts(only acreatedhead is supersedable)codex-structured-session-adapter.test.ts(the link names what it replaced)agent-session-unsaved-creation-supersession.test.ts. This runs the real record store: prove, restart, supersede, restart, read back. It also checks that the store refuses a supersession over a resumed conversation.src/main/codex,src/shared/agent-session*,src/main/runtime/agent-session*,src/main/runtime/structured-agent-session*,src/main/native-chat/agent-session-wire. That is 344 files and 3429 tests, all passing.pnpm tc:nodepasses, and fullpnpm exec oxlintexits 0.Ablations. The fix was committed first, then each part was removed and the tests re-run:
mainagent_session_provider_handle_invalid, which is the chain bug. The adapter and thread-open tests fail withno rollout found for thread id thread-unsaved.createdheadReview follow-up (
6c91609bf5). The match no longer includes Orca's own wrapper prefix, and a new test sends Codex's raw error frame through the real connection. The link builder now rejects a supersession on an adopted or resumed link at the type level. Ablations of that commit (67 tests across the thread-open, adapter and connection test files):pnpm tc:nodefails on both@ts-expect-errorpinsArms A–G ran at
9b44721706. The follow-up commite3903103cfonly rebuilt the launch-resolution test's chains without a type assertion. Arm D was re-run at that head and still fails.Review
executionHostId !== localor a WSL distro), so the resolver and the app-server always share one execution host. A paired remote Orca server runs this same code in its own runtime. Nothing crosses the wire. The handle chain lives only in the host's record store, and no RPC or stream frame changed.createdchain, because older validators ignoresupersedesKey. No record written by an older build changes meaning.Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Merge order
codex-structured-session-state.tsis an import and an event-type rename, not the launch type changed here. A test merge with currentmainis clean, and the related suites pass on it.Notes
Not verified:
codex resume <thread>in an agent terminal) still passes the recorded thread. It is not exercised for a chat that never ran a turn and is not changed here.journalIdentityFortakes the chain head when the attach begins, and that attach lasts for the whole run. This predates this PR: a fork is stamped the same way. The stamp is informational, and no reader keys on it today. Follow-up: re-derive the journal identity from the proven link once the owner is proved, for supersessions and forks alike.Future direction: starting the provider thread with the first turn, instead of eagerly at create, would remove this class of bug for both providers. That changes when models and options are published, so it is out of scope here.
Known limit: a
createdthread whose rollout the user deleted outside Orca, before the chat's first restart, is also answered "no rollout found". It starts a new thread. After the first restart the head isresumed, and the fallback no longer applies.