-
Notifications
You must be signed in to change notification settings - Fork 7k
mcp-server: make codex-reply resumable across restarts
#8689
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
mcp-server: make codex-reply resumable across restarts
#8689
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38389bca5e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let config = self.config.as_ref().clone(); | ||
| let auth_manager = self.auth_manager.clone(); | ||
| match self | ||
| .conversation_manager | ||
| .resume_conversation_from_rollout(config, rollout_path, auth_manager) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preserve per-session config when resuming from rollout
Resuming a codex-reply session always clones the server’s global config (self.config) before calling resume_conversation_from_rollout, which silently drops any per-session overrides that were used when the conversation was created (e.g., custom cwd, model, approval/sandbox policy supplied via the initial codex tool call). After an MCP server restart, the resumed session can therefore run in the wrong directory or with different policies/models than the original, which changes tool behavior and can lead to commands acting on the wrong workspace. Consider deriving the config for resume from the rollout’s session metadata (or persisting the original config alongside the rollout) so resumed sessions keep their original settings.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a new commit to restore session settings from rollout TurnContext
Adds logic to restore session cwd, model, approval policy, and sandbox policy from the most recent TurnContextItem in rollout history when resuming a session. Updates tests to verify that these settings are correctly restored from the rollout file.
What
This PR adds persistent resume/recovery to the MCP
codex-replytool-call.Previously,
codex-replyonly succeeded if the conversation was still present in the server’s in-memoryConversationManager. After restartingcodex mcp-server, that map is empty andcodex-replyimmediately fails with “Session not found…”.With this change, when a conversation is not in memory,
codex-replywill rehydrate it from the persisted rollout JSONL on disk (underCODEX_HOME/sessions/...) using the providedconversation_id, then continue the conversation normally.Why
This closes the gap described in:
codex-replycannot resume after restarting the MCP server even though the rollout exists andcodex resume <sessionId>works.Resume parity with CLI: This effectively adds “resume” functionality to the MCP server in the same spirit as
codex resume <sessionId>. External integrations can continue a session after a process restart by providing the conversation/session id, and Codex will load the persisted history from disk.How
codex-replyhandler:conversation_manager.get_conversation(conversation_id)codex_core::find_conversation_path_by_id_str(&config.codex_home, <conversation_id>)conversation_manager.resume_conversation_from_rollout(config.clone(), rollout_path, auth_manager.clone())session_configurednotification (so clients can observe the resume handshake)Tests
Adds an integration test that:
conversation_idcodex-replydirectly (no prior in-memory conversation)session_configuredfor that id and the tool call completes successfullyRun locally:
cargo test -p codex-mcp-server --tests