Skip to content

Resume Cloud Codex chats after app-server restart - #15915

Merged
teamleaderleo merged 2 commits into
mainfrom
fix/codex-chat-resume-after-crash
Sep 30, 2026
Merged

teamleaderleo merged 2 commits into
mainfrom
fix/codex-chat-resume-after-crash

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Cloud Codex sessions silently started new conversations after the shared codex app-server exited. The adapter now retains each persisted thread ID and resumes it on the replacement app-server, including retrying a failed resume without falling back to thread/start.

The regression covers two sessions, process death, resume dispatch, failed resume, and retry.

Changelog

  • Fixed: preserve Cloud Codex conversation context across app-server restarts.

Validation:

  • bun test/codex-recovery.test.ts (pass)
  • bun run build from agent-chat (pass)
  • git diff --check (pass)
  • bun run check from agent-chat is blocked by the existing adapters/lines.ts Bun stream type error.
  • Root bun run biome:check reports existing repository warnings.

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Preserves Cloud Codex conversation context across codex app-server restarts. Sessions previously started new threads silently after the shared app-server exited; the adapter now resumes the saved thread ID on the replacement server and retries a failed resume instead of falling back to thread/start.

  • Adds a regression test that runs the real adapter against a fake Codex executable, covering crash recovery, resume dispatch, failed resume, and retry.
  • Adds a test-only helper to stop the shared server for isolated lifecycle tests.

Written for commit 8c4f943. Summary will update on new commits.

Review in cubic

@cursor

cursor Bot commented Sep 30, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@coderabbitai

coderabbitai Bot commented Sep 30, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 3 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: d8164de4-3765-4a29-9087-e24d96c8d0e2

📥 Commits

Reviewing files that changed from the base of the PR and between 1b06f84 and 8c4f943.

📒 Files selected for processing (3)
  • agent-chat/adapters/codex.ts
  • agent-chat/test/codex-recovery.test.ts
  • agent-chat/test/fake-codex-recovery.ts

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

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

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review: land. The test genuinely drops and re-establishes the connection, which I checked by execution rather than by reading: the shim SIGKILLs a real child, and the per-pid request log shows three distinct app-server pids, with thread/resume for pid-1's thread IDs issued from pid-2 and again from pid-3. Not a simulated tidy reconnect.

In-flight turn across the restart behaves sanely: the session with st.turnActive gets an error event, then done, then idle. The prompt is not replayed automatically, and the user's next send resumes the same thread instead of starting a new one. The failed-resume path is covered too: with refuse-resume, the saved thread ID survives and no blank thread is created.

6 mutants, 2 survived:

  • The single-flight is untested (adapters/codex.ts:77-96). The comment says concurrent first sends and post-crash resumes must share one thread/start or resume; deleting the sharing passes the full suite, because no test issues two concurrent sends on one session. Failure shape: two rapid sends after a crash each issue their own thread/resume and the UI tracks one. Pre-existing gap, same comment and same absent coverage on main.
  • Duplicate meta events are unasserted, so a resume would re-announce the same providerSessionId on every reconnect. Cosmetic.
    — Raindrop g2 🫧 / Run: run_worker_20260930_3fc64ba6

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review: LAND. The best-evidenced PR I have looked at today, and the only one where both the tests and the mutations were executed.

Mechanism. In agent-chat/adapters/codex.ts the exit handler no longer does sess.internal.threadId = undefined, and the send path's gate becomes if (!threadId || srv.sessionsByThread.get(threadId) !== sess). After a crash, ensureServer()'s shared.proc.exitCode === null && !shared.proc.killed check hands back a fresh server whose sessionsByThread is empty, so the gate fires, sees the saved sess.internal.threadId, and issues thread/resume instead of thread/start. Single-flighted through sess.internal.threadStarting.

Mutation test: all four mutations fail the PR's own tests. Baseline bun test/codex-recovery.test.ts passes, then:

  • restore sess.internal.threadId = undefined in the exit handler → fails
  • revert the gate to the old !threadId → fails
  • always thread/start → fails
  • fall back to thread/start when resume is refused → fails

That is real coverage, not decoration, which is a pleasant change from the rest of today's batch.

Four targeted questions, answered by executed probes:

  • Triple restart: 3 SIGKILL cycles preserve one thread id, exactly 1 thread/start, 0 errors.
  • Resume racing a live session: an in-flight turn takes turn/steer, thread/resume delta 0, thread id stable. No spurious resume.
  • Unbounded retry: none. The single-flight promise clears in .finally(), and 3 concurrent post-restart sends issue exactly 1 thread/resume and 0 thread/start.
  • Replaying work already done: not settled. The fake app-server's thread/resume returns only {thread:{id}}. Whether the real codex app-server replays historical items as notifications on resume cannot be checked from here, and handleServerMessage has no per-item dedup, so if it does replay, the transcript duplicates. One question for you rather than a blocker I can substantiate: does resume replay items?

Wiring confirmed at run level, not inferred: run-tests.ts auto-discovers via new Glob("*.test.ts").scanSync(.../test), and the "Run agent-chat unit tests" step in ci-guards.yml under matrix.group == 'preflight' succeeded on the head (job 109831685656). This is not the tests/test-execution.toml case, so there is no registry entry to be missing.

Minor, not blocking: sessionsByThread entries and sess.internal.threadId are never cleaned up, so both grow for the process lifetime. Bounded by session count, so a leak rather than a bug.

Fixed: nothing needed. Left: the resume-replay question and the unbounded map.

Merging on green. Fix, so no team review.

— Raindrop g2 🫧 / Run: run_worker_20260930_3fc64ba6

@teamleaderleo
teamleaderleo enabled auto-merge (squash) September 30, 2026 15:25
@teamleaderleo
teamleaderleo merged commit 7ef6d3a into main Sep 30, 2026
59 of 60 checks passed
@teamleaderleo
teamleaderleo deleted the fix/codex-chat-resume-after-crash branch September 30, 2026 15:27
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for 8c4f943985: every check was green at merge (12 verified; 16 skipped by policy). Full suite runs on main after merge.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 30, 2026
e709b69 fix(cloud): stop reconciling panes a Cloud workspace already shows (manaflow-ai#16025)
d13dde3 Diff viewer: viewed state, file filter, generated and large diffs collapsed (manaflow-ai#15536)
e0d5c5e test: pay the Pi fixtures' first exec before timing them (manaflow-ai#16028)
e2e0b61 ci: disable unstable UI test dispatch lane (manaflow-ai#16075)
15996b0 ci: sweep side lanes instead of rescuing workflow runs (manaflow-ai#16076)
3dcf462 Recover terminal chat when transcript files are replaced (manaflow-ai#16045)
272d069 fix(agent-chat): let Stop cancel a queued or starting ACP turn (manaflow-ai#15925)
30bd116 test: cover invalid unquoted Xcode extension paths (manaflow-ai#16054)
a24a1b5 Make GitHub references in the agent chat transcript clickable (manaflow-ai#15916)
86d1cfc Reap failed Codex app-server startups before retrying (manaflow-ai#15977)
890cd1e fix(sidebar): expose workspace close button to accessibility (manaflow-ai#15965)
faf4c8f docs: define agent fan-out and reusable Cloud work environments (manaflow-ai#15836)
ab20b79 ci: cut cmux-tui Testbox warmup hold time (manaflow-ai#15557)
31fb228 Promote devbox images with cmux-tui 7d17754 (VT replay blank-cell fix) (manaflow-ai#16072)
e0da0a6 feat(acp): cmux as a read-only ACP host, phase 1 (manaflow-ai#15976)
3ed1d77 Reap failed ACP startups and temporary catalog probes (manaflow-ai#15979)
f5c3567 Add a Focus TextBox Input item to the View menu (manaflow-ai#15730)
b3a1ca1 Document the 32 CLI verbs the contract table was missing, and guard it (manaflow-ai#15993)
3bba04e Say which app-host result file could not be read (manaflow-ai#15997)
7ef6d3a Resume Cloud Codex chats after app-server restart (manaflow-ai#15915)
a803f36 fix: surface simulator process output reader failures (manaflow-ai#15880)
f6a0163 Keep terminal approval notices from moving the composer (manaflow-ai#15886)
b8ab767 test: isolate feature flag defaults between runs (manaflow-ai#15587)
5150a9b Keep unsent cloud prompts recoverable (manaflow-ai#15902)
233bd6d Restore terminal attention when transcript chat reconnects (manaflow-ai#15891)
573f998 Resolve a dogfood menu path against the direct children of each open menu (manaflow-ai#15923)
7b7a1b2 test(ci): assert the registry guard's exit code, and handle merge_group (manaflow-ai#16017)

# Conflicts:
#	.github/workflows/ci-guards.yml
#	.github/workflows/ci-owned-pool-rescue.yml
#	.github/workflows/ci-ui-tests.yml
#	.github/workflows/ci.yml
#	.github/workflows/cmux-tui-testbox-warmup.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant