fix: separate `codex mcp` into `codex mcp-server` and `codex app-server` #4471

bolinfest · 2025-09-29T23:04:45Z

This is a very large PR with some non-backwards-compatible changes.

Historically, codex mcp (or codex mcp serve) started a JSON-RPC-ish server that had two overlapping responsibilities:

Running an MCP server, providing some basic tool calls.
Running the app server used to power experiences such as the VS Code extension.

This PR aims to separate these into distinct concepts:

codex mcp-server for the MCP server
codex app-server for the "application server"

Note codex mcp still exists because it already has its own subcommands for MCP management (list, add, etc.)

The MCP logic continues to live in codex-rs/mcp-server whereas the refactored app server logic is in the new codex-rs/app-server folder. Note that most of the existing integration tests in codex-rs/mcp-server/tests/suite were actually for the app server, so all the tests have been moved with the exception of codex-rs/mcp-server/tests/suite/mod.rs.

Because this is already a large diff, I tried not to change more than I had to, so codex-rs/app-server/tests/common/mcp_process.rs still uses the name McpProcess for now, but I will do some mechanical renamings to things like AppServer in subsequent PRs.

While mcp-server and app-server share some overlapping functionality (like reading streams of JSONL and dispatching based on message types) and some differences (completely different message types), I ended up doing a bit of copypasta between the two crates, as both have somewhat similar message_processor.rs and outgoing_message.rs files for now, though I expect them to diverge more in the near future.

One material change is that of the initialize handshake for codex app-server, as we no longer use the MCP types for that handshake. Instead, we update codex-rs/protocol/src/mcp_protocol.rs to add an Initialize variant to ClientRequest, which takes the ClientInfo object we need to update the USER_AGENT_SUFFIX in codex-rs/app-server/src/message_processor.rs.

One other material change is in codex-rs/app-server/src/codex_message_processor.rs where I eliminated a use of the send_event_as_notification() method I am generally trying to deprecate (because it blindly maps an EventMsg into a JSONNotification) in favor of send_server_notification(), which takes a ServerNotification, as that is intended to be a custom enum of all notification types supported by the app server. So to make this update, I had to introduce a new variant of ServerNotification, SessionConfigured, which is a non-backwards compatible change with the old codex mcp, and clients will have to be updated after the next release that contains this PR. Note that codex-rs/app-server/tests/suite/list_resume.rs also had to be update to reflect this change.

I introduced codex-rs/utils/json-to-toml/src/lib.rs as a small utility crate to avoid some of the copying between mcp-server and app-server.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ 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 👍.

chatgpt-codex-connector · 2025-09-29T23:11:22Z

codex-rs/app-server/src/codex_message_processor.rs

                session_configured,
                ..
            }) => {
-                let event = Event {
-                    id: "".to_string(),
-                    msg: EventMsg::SessionConfigured(session_configured.clone()),
-                };
-                self.outgoing.send_event_as_notification(&event, None).await;
+                self.outgoing
+                    .send_server_notification(ServerNotification::SessionConfigured(
+                        SessionConfiguredNotification {
+                            session_id: session_configured.session_id,
+                            model: session_configured.model.clone(),
+                            reasoning_effort: session_configured.reasoning_effort,
+                            history_log_id: session_configured.history_log_id,
+                            history_entry_count: session_configured.history_entry_count,
+                            initial_messages: session_configured.initial_messages.clone(),
+                            rollout_path: session_configured.rollout_path.clone(),
+                        },


Resumed conversations no longer emit codex/event/session_configured

The new resume_conversation path now sends a ServerNotification::SessionConfigured JSON‑RPC notification (method: "sessionConfigured") instead of reusing send_event_as_notification to emit the standard codex/event/session_configured event. There is no code anywhere in the repo that listens for sessionConfigured (searching for SessionConfiguredNotification or the method name returns nothing outside this file), while numerous consumers handle session initialization via EventMsg::SessionConfigured routed through the generic codex/event handler. This means a client resuming a conversation will no longer receive the expected session_configured event and will miss metadata like history log IDs, breaking any UI logic that depends on that event. Consider continuing to publish the event via the existing codex/event path (possibly in addition to the new notification) or updating clients to handle the new method.

Useful? React with 👍 / 👎.

gpeal

Overall, LGTM. Left a few comments and decently skimmed the whole thing. I'm okay if you want to land this and then iterate.

Since we already have codex mcp, should this be codex mcp start-server?

gpeal · 2025-09-30T06:32:19Z

codex-rs/app-server/tests/common/mcp_process.rs

+        Ok(())
+    }
+
+    /// Send a `newConversation` JSON-RPC request.


These comments seem a bit redundant

Fair point, though this is from the original version:

codex/codex-rs/mcp-server/tests/common/mcp_process.rs

Line 216 in 2e95e56

/// Send a `newConversation` JSON-RPC request.

gpeal · 2025-09-30T06:32:50Z

codex-rs/app-server/tests/common/mcp_process.rs

+use std::process::Command as StdCommand;
+use tokio::process::Command;
+
+pub struct McpProcess {


Are you sure you want to call this McpProcess?

I don't, but from the PR body:

Because this is already a large diff, I tried not to change more than I had to, so codex-rs/app-server/tests/common/mcp_process.rs still uses the name McpProcess for now, but I will do some mechanical renamings to things like AppServer in subsequent PRs.

gpeal · 2025-09-30T06:33:25Z

codex-rs/app-server/tests/common/mcp_process.rs

+    }
+
+    async fn send_jsonrpc_message(&mut self, message: JSONRPCMessage) -> anyhow::Result<()> {
+        eprintln!("writing message to stdin: {message:?}");


Did you mean to leave this in?

Yes, these are in the existing codex-rs/mcp-server/tests/common/mcp_process.rs that this is taken from. Note this is a test utility, so it's not that bad that they're here.

If we want to remove them, we should probably do it from both the app-server and mcp-server versions.

bolinfest · 2025-09-30T06:56:10Z

I don't think we should have codex mcp start-server because I think managing your MCPs when using Codex CLI and running Codex as an MCP server are sufficiently different things so they shouldn't share the codex mcp subcommand.

dylan-hurd-oai · 2025-09-30T19:38:50Z

I don't think we should have codex mcp start-server because I think managing your MCPs when using Codex CLI and running Codex as an MCP server are sufficiently different things so they shouldn't share the codex mcp subcommand.

Agree!

chatgpt-codex-connector bot reviewed Sep 29, 2025

View reviewed changes

bolinfest force-pushed the pr4471 branch 7 times, most recently from 9a7aeeb to 32e0d6e Compare September 30, 2025 05:05

bolinfest requested review from dylan-hurd-oai and gpeal September 30, 2025 05:06

gpeal approved these changes Sep 30, 2025

View reviewed changes

fix: separate codex mcp into codex mcp-server and codex app-server

e8b8343

bolinfest force-pushed the pr4471 branch from 32e0d6e to e8b8343 Compare September 30, 2025 06:55

bolinfest enabled auto-merge (squash) September 30, 2025 07:00

bolinfest merged commit d9dbf48 into main Sep 30, 2025
40 checks passed

bolinfest deleted the pr4471 branch September 30, 2025 07:06

github-actions bot locked and limited conversation to collaborators Sep 30, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: separate `codex mcp` into `codex mcp-server` and `codex app-server` #4471

fix: separate `codex mcp` into `codex mcp-server` and `codex app-server` #4471

bolinfest commented Sep 29, 2025 •

edited

Loading

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Sep 29, 2025

Uh oh!

gpeal left a comment

Uh oh!

gpeal Sep 30, 2025

Uh oh!

bolinfest Sep 30, 2025

Uh oh!

gpeal Sep 30, 2025

Uh oh!

bolinfest Sep 30, 2025

Uh oh!

gpeal Sep 30, 2025

Uh oh!

bolinfest Sep 30, 2025

Uh oh!

bolinfest commented Sep 30, 2025

Uh oh!

Uh oh!

dylan-hurd-oai commented Sep 30, 2025

Uh oh!

Uh oh!

fix: separate codex mcp into codex mcp-server and codex app-server #4471

fix: separate codex mcp into codex mcp-server and codex app-server #4471

Conversation

bolinfest commented Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

gpeal left a comment

Choose a reason for hiding this comment

Uh oh!

gpeal Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

bolinfest Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

gpeal Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

bolinfest Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

gpeal Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

bolinfest Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

bolinfest commented Sep 30, 2025

Uh oh!

Uh oh!

dylan-hurd-oai commented Sep 30, 2025

Uh oh!

Uh oh!

fix: separate `codex mcp` into `codex mcp-server` and `codex app-server` #4471

fix: separate `codex mcp` into `codex mcp-server` and `codex app-server` #4471

bolinfest commented Sep 29, 2025 •

edited

Loading