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

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`.
This commit is contained in:
Michael Bolin
2025-09-30 00:06:18 -07:00
committed by GitHub
parent 2e95e5602d
commit d9dbf48828
49 changed files with 1525 additions and 414 deletions

View File

@@ -0,0 +1,14 @@
[package]
edition.workspace = true
name = "codex-utils-json-to-toml"
version.workspace = true
[dependencies]
serde_json = { workspace = true }
toml = { workspace = true }
[dev-dependencies]
pretty_assertions = { workspace = true }
[lints]
workspace = true

View File

@@ -0,0 +1,83 @@
use serde_json::Value as JsonValue;
use toml::Value as TomlValue;
/// Convert a `serde_json::Value` into a semantically equivalent `toml::Value`.
pub fn json_to_toml(v: JsonValue) -> TomlValue {
match v {
JsonValue::Null => TomlValue::String(String::new()),
JsonValue::Bool(b) => TomlValue::Boolean(b),
JsonValue::Number(n) => {
if let Some(i) = n.as_i64() {
TomlValue::Integer(i)
} else if let Some(f) = n.as_f64() {
TomlValue::Float(f)
} else {
TomlValue::String(n.to_string())
}
}
JsonValue::String(s) => TomlValue::String(s),
JsonValue::Array(arr) => TomlValue::Array(arr.into_iter().map(json_to_toml).collect()),
JsonValue::Object(map) => {
let tbl = map
.into_iter()
.map(|(k, v)| (k, json_to_toml(v)))
.collect::<toml::value::Table>();
TomlValue::Table(tbl)
}
}
}
#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;
use serde_json::json;
#[test]
fn json_number_to_toml() {
let json_value = json!(123);
assert_eq!(TomlValue::Integer(123), json_to_toml(json_value));
}
#[test]
fn json_array_to_toml() {
let json_value = json!([true, 1]);
assert_eq!(
TomlValue::Array(vec![TomlValue::Boolean(true), TomlValue::Integer(1)]),
json_to_toml(json_value)
);
}
#[test]
fn json_bool_to_toml() {
let json_value = json!(false);
assert_eq!(TomlValue::Boolean(false), json_to_toml(json_value));
}
#[test]
fn json_float_to_toml() {
let json_value = json!(1.25);
assert_eq!(TomlValue::Float(1.25), json_to_toml(json_value));
}
#[test]
fn json_null_to_toml() {
let json_value = serde_json::Value::Null;
assert_eq!(TomlValue::String(String::new()), json_to_toml(json_value));
}
#[test]
fn json_object_nested() {
let json_value = json!({ "outer": { "inner": 2 } });
let expected = {
let mut inner = toml::value::Table::new();
inner.insert("inner".into(), TomlValue::Integer(2));
let mut outer = toml::value::Table::new();
outer.insert("outer".into(), TomlValue::Table(inner));
TomlValue::Table(outer)
};
assert_eq!(json_to_toml(json_value), expected);
}
}