From c14c2ce6a0ef76696875fc152d977e708dacc739 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sun, 1 Feb 2026 18:59:12 -0800 Subject: [PATCH] feat: migrate MCP implementation from mcp-types to rmcp --- .github/workflows/rust-ci.yml | 2 - codex-rs/Cargo.lock | 24 +- codex-rs/Cargo.toml | 2 - codex-rs/app-server-test-client/Cargo.lock | 11 - codex-rs/app-server/Cargo.toml | 2 - .../app-server/src/bespoke_event_handling.rs | 15 +- .../core/src/tools/handlers/mcp_resource.rs | 66 +-- codex-rs/core/src/tools/router.rs | 3 +- codex-rs/core/src/tools/spec.rs | 295 +++++----- codex-rs/exec/Cargo.toml | 3 +- .../src/event_processor_with_human_output.rs | 3 +- codex-rs/exec/src/exec_events.rs | 2 +- .../tests/event_processor_with_json_output.rs | 19 +- codex-rs/mcp-server/Cargo.toml | 2 +- codex-rs/mcp-server/src/codex_tool_config.rs | 81 ++- codex-rs/mcp-server/src/codex_tool_runner.rs | 33 +- codex-rs/mcp-server/src/error_code.rs | 2 - codex-rs/mcp-server/src/exec_approval.rs | 35 +- codex-rs/mcp-server/src/lib.rs | 27 +- codex-rs/mcp-server/src/message_processor.rs | 534 ++++++++---------- codex-rs/mcp-server/src/outgoing_message.rs | 125 ++-- codex-rs/mcp-server/src/patch_approval.rs | 30 +- codex-rs/mcp-server/tests/common/Cargo.toml | 2 +- codex-rs/mcp-server/tests/common/lib.rs | 6 +- .../mcp-server/tests/common/mcp_process.rs | 168 +++--- codex-rs/mcp-server/tests/suite/codex_tool.rs | 158 +++--- codex-rs/tui/Cargo.toml | 2 +- .../tui/src/bottom_pane/approval_overlay.rs | 2 +- ...et__tests__binary_size_ideal_response.snap | 2 +- codex-rs/tui/src/history_cell.rs | 219 ++++--- codex-rs/tui/src/lib.rs | 20 + .../tui/tests/suite/no_panic_on_startup.rs | 5 +- 32 files changed, 905 insertions(+), 995 deletions(-) delete mode 100644 codex-rs/mcp-server/src/error_code.rs diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index 7a2ef1f2df..ec6319f8da 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -64,8 +64,6 @@ jobs: components: rustfmt - name: cargo fmt run: cargo fmt -- --config imports_granularity=Item --check - - name: Verify codegen for mcp-types - run: ./mcp-types/check_lib_rs.py cargo_shear: name: cargo shear diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index cbe8aa6937..bf93e45415 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1100,7 +1100,6 @@ dependencies = [ "codex-utils-absolute-path", "codex-utils-json-to-toml", "core_test_support", - "mcp-types", "os_info", "pretty_assertions", "rmcp", @@ -1126,7 +1125,6 @@ dependencies = [ "clap", "codex-protocol", "codex-utils-absolute-path", - "mcp-types", "pretty_assertions", "schemars 0.8.22", "serde", @@ -1423,7 +1421,6 @@ dependencies = [ "landlock", "libc", "maplit", - "mcp-types", "multimap", "once_cell", "openssl-sys", @@ -1435,6 +1432,7 @@ dependencies = [ "regex", "regex-lite", "reqwest", + "rmcp", "schemars 0.8.22", "seccompiler", "serde", @@ -1498,10 +1496,10 @@ dependencies = [ "codex-utils-cargo-bin", "core_test_support", "libc", - "mcp-types", "owo-colors", "predicates", "pretty_assertions", + "rmcp", "serde", "serde_json", "shlex", @@ -1694,10 +1692,10 @@ dependencies = [ "codex-protocol", "codex-utils-json-to-toml", "core_test_support", - "mcp-types", "mcp_test_support", "os_info", "pretty_assertions", + "rmcp", "schemars 0.8.22", "serde", "serde_json", @@ -1806,7 +1804,6 @@ dependencies = [ "icu_decimal", "icu_locale_core", "icu_provider", - "mcp-types", "mime_guess", "pretty_assertions", "schemars 0.8.22", @@ -1850,7 +1847,6 @@ dependencies = [ "codex-utils-home-dir", "futures", "keyring", - "mcp-types", "oauth2", "pretty_assertions", "reqwest", @@ -1942,7 +1938,6 @@ dependencies = [ "itertools 0.14.0", "lazy_static", "libc", - "mcp-types", "pathdiff", "pretty_assertions", "pulldown-cmark", @@ -1951,6 +1946,7 @@ dependencies = [ "ratatui-macros", "regex-lite", "reqwest", + "rmcp", "serde", "serde_json", "serial_test", @@ -4662,16 +4658,6 @@ version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b3eede3bdf92f3b4f9dc04072a9ce5ab557d5ec9038773bf9ffcd5588b3cc05b" -[[package]] -name = "mcp-types" -version = "0.0.0" -dependencies = [ - "schemars 0.8.22", - "serde", - "serde_json", - "ts-rs", -] - [[package]] name = "mcp_test_support" version = "0.0.0" @@ -4681,9 +4667,9 @@ dependencies = [ "codex-mcp-server", "codex-utils-cargo-bin", "core_test_support", - "mcp-types", "os_info", "pretty_assertions", + "rmcp", "serde", "serde_json", "shlex", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 84210838fc..24ad845b17 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -27,7 +27,6 @@ members = [ "lmstudio", "login", "mcp-server", - "mcp-types", "network-proxy", "ollama", "process-hardening", @@ -110,7 +109,6 @@ codex-utils-string = { path = "utils/string" } codex-windows-sandbox = { path = "windows-sandbox-rs" } core_test_support = { path = "core/tests/common" } exec_server_test_support = { path = "exec-server/tests/common" } -mcp-types = { path = "mcp-types" } mcp_test_support = { path = "mcp-server/tests/common" } # External diff --git a/codex-rs/app-server-test-client/Cargo.lock b/codex-rs/app-server-test-client/Cargo.lock index 1720850cd2..c6e4241d2c 100644 --- a/codex-rs/app-server-test-client/Cargo.lock +++ b/codex-rs/app-server-test-client/Cargo.lock @@ -175,7 +175,6 @@ dependencies = [ "base64", "icu_decimal", "icu_locale_core", - "mcp-types", "mime_guess", "serde", "serde_json", @@ -521,16 +520,6 @@ version = "0.4.28" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "34080505efa8e45a4b816c349525ebe327ceaa8559756f0356cba97ef3bf7432" -[[package]] -name = "mcp-types" -version = "0.45.0" -source = "git+https://github.com/openai/codex.git?tag=rust-v0.45.0#a7c7869c23f88f6c468281e6f438ba4a91b81f26" -dependencies = [ - "serde", - "serde_json", - "ts-rs", -] - [[package]] name = "memchr" version = "2.7.6" diff --git a/codex-rs/app-server/Cargo.toml b/codex-rs/app-server/Cargo.toml index 11c57ebbc8..778d57bab0 100644 --- a/codex-rs/app-server/Cargo.toml +++ b/codex-rs/app-server/Cargo.toml @@ -35,7 +35,6 @@ codex-utils-json-to-toml = { workspace = true } chrono = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } -mcp-types = { workspace = true } tempfile = { workspace = true } time = { workspace = true } toml = { workspace = true } @@ -60,7 +59,6 @@ axum = { workspace = true, default-features = false, features = [ base64 = { workspace = true } codex-execpolicy = { workspace = true } core_test_support = { workspace = true } -mcp-types = { workspace = true } os_info = { workspace = true } pretty_assertions = { workspace = true } rmcp = { workspace = true, default-features = false, features = [ diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index bf8fad3a9c..55b185e5a8 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -1841,12 +1841,11 @@ mod tests { use codex_core::protocol::RateLimitWindow; use codex_core::protocol::TokenUsage; use codex_core::protocol::TokenUsageInfo; + use codex_protocol::mcp::CallToolResult; use codex_protocol::plan_tool::PlanItemArg; use codex_protocol::plan_tool::StepStatus; - use mcp_types::CallToolResult; - use mcp_types::ContentBlock; - use mcp_types::TextContent; use pretty_assertions::assert_eq; + use rmcp::model::Content; use serde_json::Value as JsonValue; use std::collections::HashMap; use std::time::Duration; @@ -2374,15 +2373,15 @@ mod tests { #[tokio::test] async fn test_construct_mcp_tool_call_end_notification_success() { - let content = vec![ContentBlock::TextContent(TextContent { - annotations: None, - text: "{\"resources\":[]}".to_string(), - r#type: "text".to_string(), - })]; + let content = vec![ + serde_json::to_value(Content::text("{\"resources\":[]}")) + .expect("content should serialize"), + ]; let result = CallToolResult { content: content.clone(), is_error: Some(false), structured_content: None, + meta: None, }; let end_event = McpToolCallEndEvent { diff --git a/codex-rs/core/src/tools/handlers/mcp_resource.rs b/codex-rs/core/src/tools/handlers/mcp_resource.rs index 62f7a83e1a..df421bd6d3 100644 --- a/codex-rs/core/src/tools/handlers/mcp_resource.rs +++ b/codex-rs/core/src/tools/handlers/mcp_resource.rs @@ -4,17 +4,14 @@ use std::time::Duration; use std::time::Instant; use async_trait::async_trait; -use mcp_types::CallToolResult; -use mcp_types::ContentBlock; -use mcp_types::ListResourceTemplatesRequestParams; -use mcp_types::ListResourceTemplatesResult; -use mcp_types::ListResourcesRequestParams; -use mcp_types::ListResourcesResult; -use mcp_types::ReadResourceRequestParams; -use mcp_types::ReadResourceResult; -use mcp_types::Resource; -use mcp_types::ResourceTemplate; -use mcp_types::TextContent; +use codex_protocol::mcp::CallToolResult; +use rmcp::model::ListResourceTemplatesResult; +use rmcp::model::ListResourcesResult; +use rmcp::model::PaginatedRequestParam; +use rmcp::model::ReadResourceRequestParam; +use rmcp::model::ReadResourceResult; +use rmcp::model::Resource; +use rmcp::model::ResourceTemplate; use serde::Deserialize; use serde::Serialize; use serde::de::DeserializeOwned; @@ -264,7 +261,7 @@ async fn handle_list_resources( let payload_result: Result = async { if let Some(server_name) = server.clone() { - let params = cursor.clone().map(|value| ListResourcesRequestParams { + let params = cursor.clone().map(|value| PaginatedRequestParam { cursor: Some(value), }); let result = session @@ -371,11 +368,9 @@ async fn handle_list_resource_templates( let payload_result: Result = async { if let Some(server_name) = server.clone() { - let params = cursor - .clone() - .map(|value| ListResourceTemplatesRequestParams { - cursor: Some(value), - }); + let params = cursor.clone().map(|value| PaginatedRequestParam { + cursor: Some(value), + }); let result = session .list_resource_templates(&server_name, params) .await @@ -482,7 +477,7 @@ async fn handle_read_resource( let payload_result: Result = async { let result = session - .read_resource(&server, ReadResourceRequestParams { uri: uri.clone() }) + .read_resource(&server, ReadResourceRequestParam { uri: uri.clone() }) .await .map_err(|err| { FunctionCallError::RespondToModel(format!("resources/read failed: {err:#}")) @@ -551,13 +546,10 @@ async fn handle_read_resource( fn call_tool_result_from_content(content: &str, success: Option) -> CallToolResult { CallToolResult { - content: vec![ContentBlock::TextContent(TextContent { - annotations: None, - text: content.to_string(), - r#type: "text".to_string(), - })], - is_error: success.map(|value| !value), + content: vec![serde_json::json!({"type": "text", "text": content})], structured_content: None, + is_error: success.map(|value| !value), + meta: None, } } @@ -678,32 +670,33 @@ where #[cfg(test)] mod tests { use super::*; - use mcp_types::ListResourcesResult; - use mcp_types::ResourceTemplate; use pretty_assertions::assert_eq; + use rmcp::model::AnnotateAble; use serde_json::json; fn resource(uri: &str, name: &str) -> Resource { - Resource { - annotations: None, + rmcp::model::RawResource { + uri: uri.to_string(), + name: name.to_string(), + title: None, description: None, mime_type: None, - name: name.to_string(), size: None, - title: None, - uri: uri.to_string(), + icons: None, + meta: None, } + .no_annotation() } fn template(uri_template: &str, name: &str) -> ResourceTemplate { - ResourceTemplate { - annotations: None, - description: None, - mime_type: None, + rmcp::model::RawResourceTemplate { + uri_template: uri_template.to_string(), name: name.to_string(), title: None, - uri_template: uri_template.to_string(), + description: None, + mime_type: None, } + .no_annotation() } #[test] @@ -719,6 +712,7 @@ mod tests { #[test] fn list_resources_payload_from_single_server_copies_next_cursor() { let result = ListResourcesResult { + meta: None, next_cursor: Some("cursor-1".to_string()), resources: vec![resource("memo://id", "memo")], }; diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index d3ff84a2b0..51328ccc9f 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -15,6 +15,7 @@ use codex_protocol::models::LocalShellAction; use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ResponseItem; use codex_protocol::models::ShellToolCallParams; +use rmcp::model::Tool; use std::collections::HashMap; use std::sync::Arc; use tracing::instrument; @@ -34,7 +35,7 @@ pub struct ToolRouter { impl ToolRouter { pub fn from_config( config: &ToolsConfig, - mcp_tools: Option>, + mcp_tools: Option>, dynamic_tools: &[DynamicToolSpec], ) -> Self { let builder = build_specs(config, mcp_tools, dynamic_tools); diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index a51372b9b3..c08feeef9a 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -1087,20 +1087,26 @@ pub(crate) fn create_tools_json_for_chat_completions_api( pub(crate) fn mcp_tool_to_openai_tool( fully_qualified_name: String, - tool: mcp_types::Tool, + tool: rmcp::model::Tool, ) -> Result { - let mcp_types::Tool { + let rmcp::model::Tool { description, - mut input_schema, + input_schema, .. } = tool; - // OpenAI models mandate the "properties" field in the schema. The Agents - // SDK fixed this by inserting an empty object for "properties" if it is not - // already present https://github.com/openai/openai-agents-python/issues/449 - // so here we do the same. - if input_schema.properties.is_none() { - input_schema.properties = Some(serde_json::Value::Object(serde_json::Map::new())); + let mut serialized_input_schema = serde_json::Value::Object(input_schema.as_ref().clone()); + + // OpenAI models mandate the "properties" field in the schema. Some MCP + // servers omit it (or set it to null), so we insert an empty object to + // match the behavior of the Agents SDK. + if let serde_json::Value::Object(obj) = &mut serialized_input_schema + && obj.get("properties").is_none_or(serde_json::Value::is_null) + { + obj.insert( + "properties".to_string(), + serde_json::Value::Object(serde_json::Map::new()), + ); } // Serialize to a raw JSON value so we can sanitize schemas coming from MCP @@ -1108,13 +1114,12 @@ pub(crate) fn mcp_tool_to_openai_tool( // Schemas (e.g. using enum/anyOf), or use unsupported variants like // `integer`. Our internal JsonSchema is a small subset and requires // `type`, so we coerce/sanitize here for compatibility. - let mut serialized_input_schema = serde_json::to_value(input_schema)?; sanitize_json_schema(&mut serialized_input_schema); let input_schema = serde_json::from_value::(serialized_input_schema)?; Ok(ResponsesApiTool { name: fully_qualified_name, - description: description.unwrap_or_default(), + description: description.map(Into::into).unwrap_or_default(), strict: false, parameters: input_schema, }) @@ -1254,7 +1259,7 @@ fn sanitize_json_schema(value: &mut JsonValue) { /// Builds the tool registry builder while collecting tool specs for later serialization. pub(crate) fn build_specs( config: &ToolsConfig, - mcp_tools: Option>, + mcp_tools: Option>, dynamic_tools: &[DynamicToolSpec], ) -> ToolRegistryBuilder { use crate::tools::handlers::ApplyPatchHandler; @@ -1410,7 +1415,7 @@ pub(crate) fn build_specs( } if let Some(mcp_tools) = mcp_tools { - let mut entries: Vec<(String, mcp_types::Tool)> = mcp_tools.into_iter().collect(); + let mut entries: Vec<(String, rmcp::model::Tool)> = mcp_tools.into_iter().collect(); entries.sort_by(|a, b| a.0.cmp(&b.0)); for (name, tool) in entries.into_iter() { @@ -1452,11 +1457,50 @@ mod tests { use crate::config::test_config; use crate::models_manager::manager::ModelsManager; use crate::tools::registry::ConfiguredToolSpec; - use mcp_types::ToolInputSchema; use pretty_assertions::assert_eq; use super::*; + fn mcp_tool( + name: &str, + description: &str, + input_schema: serde_json::Value, + ) -> rmcp::model::Tool { + rmcp::model::Tool { + name: name.to_string().into(), + title: None, + description: Some(description.to_string().into()), + input_schema: std::sync::Arc::new(rmcp::model::object(input_schema)), + output_schema: None, + annotations: None, + icons: None, + meta: None, + } + } + + #[test] + fn mcp_tool_to_openai_tool_inserts_empty_properties() { + let mut schema = rmcp::model::JsonObject::new(); + schema.insert("type".to_string(), serde_json::json!("object")); + + let tool = rmcp::model::Tool { + name: "no_props".to_string().into(), + title: None, + description: Some("No properties".to_string().into()), + input_schema: std::sync::Arc::new(schema), + output_schema: None, + annotations: None, + icons: None, + meta: None, + }; + + let openai_tool = + mcp_tool_to_openai_tool("server/no_props".to_string(), tool).expect("convert tool"); + let parameters = serde_json::to_value(openai_tool.parameters).expect("serialize schema"); + + assert_eq!(parameters.get("properties"), Some(&serde_json::json!({}))); + } + fn tool_name(tool: &ToolSpec) -> &str { match tool { ToolSpec::Function(ResponsesApiTool { name, .. }) => name, @@ -2026,37 +2070,26 @@ mod tests { &tools_config, Some(HashMap::from([( "test_server/do_something_cool".to_string(), - mcp_types::Tool { - name: "do_something_cool".to_string(), - input_schema: ToolInputSchema { - properties: Some(serde_json::json!({ - "string_argument": { - "type": "string", - }, - "number_argument": { - "type": "number", - }, + mcp_tool( + "do_something_cool", + "Do something cool", + serde_json::json!({ + "type": "object", + "properties": { + "string_argument": { "type": "string" }, + "number_argument": { "type": "number" }, "object_argument": { "type": "object", "properties": { "string_property": { "type": "string" }, "number_property": { "type": "number" }, }, - "required": [ - "string_property", - "number_property", - ], - "additionalProperties": Some(false), + "required": ["string_property", "number_property"], + "additionalProperties": false, }, - })), - required: None, - r#type: "object".to_string(), - }, - output_schema: None, - title: None, - annotations: None, - description: Some("Do something cool".to_string()), - }, + }, + }), + ), )])), &[], ) @@ -2120,51 +2153,18 @@ mod tests { }); // Intentionally construct a map with keys that would sort alphabetically. - let tools_map: HashMap = HashMap::from([ + let tools_map: HashMap = HashMap::from([ ( "test_server/do".to_string(), - mcp_types::Tool { - name: "a".to_string(), - input_schema: ToolInputSchema { - properties: Some(serde_json::json!({})), - required: None, - r#type: "object".to_string(), - }, - output_schema: None, - title: None, - annotations: None, - description: Some("a".to_string()), - }, + mcp_tool("a", "a", serde_json::json!({"type": "object"})), ), ( "test_server/something".to_string(), - mcp_types::Tool { - name: "b".to_string(), - input_schema: ToolInputSchema { - properties: Some(serde_json::json!({})), - required: None, - r#type: "object".to_string(), - }, - output_schema: None, - title: None, - annotations: None, - description: Some("b".to_string()), - }, + mcp_tool("b", "b", serde_json::json!({"type": "object"})), ), ( "test_server/cool".to_string(), - mcp_types::Tool { - name: "c".to_string(), - input_schema: ToolInputSchema { - properties: Some(serde_json::json!({})), - required: None, - r#type: "object".to_string(), - }, - output_schema: None, - title: None, - annotations: None, - description: Some("c".to_string()), - }, + mcp_tool("c", "c", serde_json::json!({"type": "object"})), ), ]); @@ -2200,22 +2200,16 @@ mod tests { &tools_config, Some(HashMap::from([( "dash/search".to_string(), - mcp_types::Tool { - name: "search".to_string(), - input_schema: ToolInputSchema { - properties: Some(serde_json::json!({ - "query": { - "description": "search query" - } - })), - required: None, - r#type: "object".to_string(), - }, - output_schema: None, - title: None, - annotations: None, - description: Some("Search docs".to_string()), - }, + mcp_tool( + "search", + "Search docs", + serde_json::json!({ + "type": "object", + "properties": { + "query": {"description": "search query"} + } + }), + ), )])), &[], ) @@ -2258,20 +2252,14 @@ mod tests { &tools_config, Some(HashMap::from([( "dash/paginate".to_string(), - mcp_types::Tool { - name: "paginate".to_string(), - input_schema: ToolInputSchema { - properties: Some(serde_json::json!({ - "page": { "type": "integer" } - })), - required: None, - r#type: "object".to_string(), - }, - output_schema: None, - title: None, - annotations: None, - description: Some("Pagination".to_string()), - }, + mcp_tool( + "paginate", + "Pagination", + serde_json::json!({ + "type": "object", + "properties": {"page": {"type": "integer"}} + }), + ), )])), &[], ) @@ -2313,20 +2301,14 @@ mod tests { &tools_config, Some(HashMap::from([( "dash/tags".to_string(), - mcp_types::Tool { - name: "tags".to_string(), - input_schema: ToolInputSchema { - properties: Some(serde_json::json!({ - "tags": { "type": "array" } - })), - required: None, - r#type: "object".to_string(), - }, - output_schema: None, - title: None, - annotations: None, - description: Some("Tags".to_string()), - }, + mcp_tool( + "tags", + "Tags", + serde_json::json!({ + "type": "object", + "properties": {"tags": {"type": "array"}} + }), + ), )])), &[], ) @@ -2370,20 +2352,16 @@ mod tests { &tools_config, Some(HashMap::from([( "dash/value".to_string(), - mcp_types::Tool { - name: "value".to_string(), - input_schema: ToolInputSchema { - properties: Some(serde_json::json!({ - "value": { "anyOf": [ { "type": "string" }, { "type": "number" } ] } - })), - required: None, - r#type: "object".to_string(), - }, - output_schema: None, - title: None, - annotations: None, - description: Some("AnyOf Value".to_string()), - }, + mcp_tool( + "value", + "AnyOf Value", + serde_json::json!({ + "type": "object", + "properties": { + "value": {"anyOf": [{"type": "string"}, {"type": "number"}]} + } + }), + ), )])), &[], ) @@ -2482,46 +2460,33 @@ Examples of valid command strings: &tools_config, Some(HashMap::from([( "test_server/do_something_cool".to_string(), - mcp_types::Tool { - name: "do_something_cool".to_string(), - input_schema: ToolInputSchema { - properties: Some(serde_json::json!({ - "string_argument": { - "type": "string", - }, - "number_argument": { - "type": "number", - }, + mcp_tool( + "do_something_cool", + "Do something cool", + serde_json::json!({ + "type": "object", + "properties": { + "string_argument": {"type": "string"}, + "number_argument": {"type": "number"}, "object_argument": { "type": "object", "properties": { - "string_property": { "type": "string" }, - "number_property": { "type": "number" }, + "string_property": {"type": "string"}, + "number_property": {"type": "number"} }, - "required": [ - "string_property", - "number_property", - ], + "required": ["string_property", "number_property"], "additionalProperties": { "type": "object", "properties": { - "addtl_prop": { "type": "string" }, + "addtl_prop": {"type": "string"} }, - "required": [ - "addtl_prop", - ], - "additionalProperties": false, - }, - }, - })), - required: None, - r#type: "object".to_string(), - }, - output_schema: None, - title: None, - annotations: None, - description: Some("Do something cool".to_string()), - }, + "required": ["addtl_prop"], + "additionalProperties": false + } + } + } + }), + ), )])), &[], ) diff --git a/codex-rs/exec/Cargo.toml b/codex-rs/exec/Cargo.toml index 862d8d1694..860f4034be 100644 --- a/codex-rs/exec/Cargo.toml +++ b/codex-rs/exec/Cargo.toml @@ -28,7 +28,6 @@ codex-common = { workspace = true, features = [ codex-core = { workspace = true } codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } -mcp-types = { workspace = true } owo-colors = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } @@ -56,9 +55,9 @@ assert_cmd = { workspace = true } codex-utils-cargo-bin = { workspace = true } core_test_support = { workspace = true } libc = { workspace = true } -mcp-types = { workspace = true } predicates = { workspace = true } pretty_assertions = { workspace = true } +rmcp = { workspace = true } tempfile = { workspace = true } uuid = { workspace = true } walkdir = { workspace = true } diff --git a/codex-rs/exec/src/event_processor_with_human_output.rs b/codex-rs/exec/src/event_processor_with_human_output.rs index 8469dc42ed..3b4a3bf86b 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -375,7 +375,8 @@ impl EventProcessor for EventProcessorWithHumanOutput { ts_msg!(self, "{}", title.style(title_style)); if let Ok(res) = result { - let val: serde_json::Value = res.into(); + let val = serde_json::to_value(res) + .unwrap_or_else(|_| serde_json::Value::String("".to_string())); let pretty = serde_json::to_string_pretty(&val).unwrap_or_else(|_| val.to_string()); diff --git a/codex-rs/exec/src/exec_events.rs b/codex-rs/exec/src/exec_events.rs index 47c67a6dca..3adafa6e14 100644 --- a/codex-rs/exec/src/exec_events.rs +++ b/codex-rs/exec/src/exec_events.rs @@ -1,5 +1,5 @@ use codex_protocol::models::WebSearchAction; -use mcp_types::ContentBlock as McpContentBlock; +type McpContentBlock = serde_json::Value; use serde::Deserialize; use serde::Serialize; use serde_json::Value as JsonValue; diff --git a/codex-rs/exec/tests/event_processor_with_json_output.rs b/codex-rs/exec/tests/event_processor_with_json_output.rs index 47b7a9f331..6a853857da 100644 --- a/codex-rs/exec/tests/event_processor_with_json_output.rs +++ b/codex-rs/exec/tests/event_processor_with_json_output.rs @@ -56,6 +56,7 @@ use codex_exec::exec_events::Usage; use codex_exec::exec_events::WebSearchItem; use codex_protocol::ThreadId; use codex_protocol::config_types::ModeKind; +use codex_protocol::mcp::CallToolResult; use codex_protocol::models::WebSearchAction; use codex_protocol::plan_tool::PlanItemArg; use codex_protocol::plan_tool::StepStatus; @@ -63,10 +64,8 @@ use codex_protocol::plan_tool::UpdatePlanArgs; use codex_protocol::protocol::CodexErrorInfo; use codex_protocol::protocol::ExecCommandOutputDeltaEvent; use codex_protocol::protocol::ExecOutputStream; -use mcp_types::CallToolResult; -use mcp_types::ContentBlock; -use mcp_types::TextContent; use pretty_assertions::assert_eq; +use rmcp::model::Content; use serde_json::json; use std::path::PathBuf; use std::time::Duration; @@ -385,6 +384,7 @@ fn mcp_tool_call_begin_and_end_emit_item_events() { content: Vec::new(), is_error: None, structured_content: None, + meta: None, }), }), ); @@ -499,13 +499,10 @@ fn mcp_tool_call_defaults_arguments_and_preserves_structured_content() { invocation, duration: Duration::from_millis(10), result: Ok(CallToolResult { - content: vec![ContentBlock::TextContent(TextContent { - annotations: None, - text: "done".to_string(), - r#type: "text".to_string(), - })], + content: vec![serde_json::to_value(Content::text("done")).unwrap()], is_error: None, structured_content: Some(json!({ "status": "ok" })), + meta: None, }), }), ); @@ -520,11 +517,7 @@ fn mcp_tool_call_defaults_arguments_and_preserves_structured_content() { tool: "tool_z".to_string(), arguments: serde_json::Value::Null, result: Some(McpToolCallItemResult { - content: vec![ContentBlock::TextContent(TextContent { - annotations: None, - text: "done".to_string(), - r#type: "text".to_string(), - })], + content: vec![serde_json::to_value(Content::text("done")).unwrap()], structured_content: Some(json!({ "status": "ok" })), }), error: None, diff --git a/codex-rs/mcp-server/Cargo.toml b/codex-rs/mcp-server/Cargo.toml index 6236384c95..7a952342de 100644 --- a/codex-rs/mcp-server/Cargo.toml +++ b/codex-rs/mcp-server/Cargo.toml @@ -22,7 +22,7 @@ codex-common = { workspace = true, features = ["cli"] } codex-core = { workspace = true } codex-protocol = { workspace = true } codex-utils-json-to-toml = { workspace = true } -mcp-types = { workspace = true } +rmcp = { workspace = true } schemars = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } diff --git a/codex-rs/mcp-server/src/codex_tool_config.rs b/codex-rs/mcp-server/src/codex_tool_config.rs index 8131d7da52..94bf4369a9 100644 --- a/codex-rs/mcp-server/src/codex_tool_config.rs +++ b/codex-rs/mcp-server/src/codex_tool_config.rs @@ -6,15 +6,15 @@ use codex_core::protocol::AskForApproval; use codex_protocol::ThreadId; use codex_protocol::config_types::SandboxMode; use codex_utils_json_to_toml::json_to_toml; -use mcp_types::Tool; -use mcp_types::ToolInputSchema; -use mcp_types::ToolOutputSchema; +use rmcp::model::JsonObject; +use rmcp::model::Tool; use schemars::JsonSchema; use schemars::r#gen::SchemaSettings; use serde::Deserialize; use serde::Serialize; use std::collections::HashMap; use std::path::PathBuf; +use std::sync::Arc; /// Client-supplied configuration for a `codex` tool-call. #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, Default)] @@ -115,35 +115,35 @@ pub(crate) fn create_tool_for_codex_tool_call_param() -> Tool { .into_generator() .into_root_schema_for::(); - #[expect(clippy::expect_used)] - let schema_value = - serde_json::to_value(&schema).expect("Codex tool schema should serialise to JSON"); - - let tool_input_schema = - serde_json::from_value::(schema_value).unwrap_or_else(|e| { - panic!("failed to create Tool from schema: {e}"); - }); + let input_schema = create_tool_input_schema(schema, "Codex tool schema should serialize"); Tool { - name: "codex".to_string(), + name: "codex".into(), title: Some("Codex".to_string()), - input_schema: tool_input_schema, + input_schema, output_schema: Some(codex_tool_output_schema()), description: Some( - "Run a Codex session. Accepts configuration parameters matching the Codex Config struct.".to_string(), + "Run a Codex session. Accepts configuration parameters matching the Codex Config struct." + .into(), ), annotations: None, + icons: None, + meta: None, } } -fn codex_tool_output_schema() -> ToolOutputSchema { - ToolOutputSchema { - properties: Some(serde_json::json!({ +fn codex_tool_output_schema() -> Arc { + let schema = serde_json::json!({ + "type": "object", + "properties": { "threadId": { "type": "string" }, "content": { "type": "string" } - })), - required: Some(vec!["threadId".to_string(), "content".to_string()]), - r#type: "object".to_string(), + }, + "required": ["threadId", "content"], + }); + match schema { + serde_json::Value::Object(map) => Arc::new(map), + _ => unreachable!("json literal must be an object"), } } @@ -237,27 +237,46 @@ pub(crate) fn create_tool_for_codex_tool_call_reply_param() -> Tool { .into_generator() .into_root_schema_for::(); - #[expect(clippy::expect_used)] - let schema_value = - serde_json::to_value(&schema).expect("Codex reply tool schema should serialise to JSON"); - - let tool_input_schema = - serde_json::from_value::(schema_value).unwrap_or_else(|e| { - panic!("failed to create Tool from schema: {e}"); - }); + let input_schema = create_tool_input_schema(schema, "Codex reply tool schema should serialize"); Tool { - name: "codex-reply".to_string(), + name: "codex-reply".into(), title: Some("Codex Reply".to_string()), - input_schema: tool_input_schema, + input_schema, output_schema: Some(codex_tool_output_schema()), description: Some( - "Continue a Codex conversation by providing the thread id and prompt.".to_string(), + "Continue a Codex conversation by providing the thread id and prompt.".into(), ), annotations: None, + icons: None, + meta: None, } } +fn create_tool_input_schema( + schema: schemars::schema::RootSchema, + panic_message: &str, +) -> Arc { + #[expect(clippy::expect_used)] + let schema_value = serde_json::to_value(&schema).expect(panic_message); + let mut schema_object = match schema_value { + serde_json::Value::Object(object) => object, + _ => panic!("tool schema should serialize to a JSON object"), + }; + + // Prefer keeping the "core" JSON Schema keys while still preserving `$defs` + // in case any `$ref` leaks into the generated schema (even though we try + // to inline subschemas). + let mut input_schema = JsonObject::new(); + for key in ["properties", "required", "type", "$defs", "definitions"] { + if let Some(value) = schema_object.remove(key) { + input_schema.insert(key.to_string(), value); + } + } + + Arc::new(input_schema) +} + #[cfg(test)] mod tests { use super::*; diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index 1dbdbc767b..6273607a22 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -23,15 +23,12 @@ use codex_core::protocol::Submission; use codex_core::protocol::TurnCompleteEvent; use codex_protocol::ThreadId; use codex_protocol::user_input::UserInput; -use mcp_types::CallToolResult; -use mcp_types::ContentBlock; -use mcp_types::RequestId; -use mcp_types::TextContent; +use rmcp::model::CallToolResult; +use rmcp::model::Content; +use rmcp::model::RequestId; use serde_json::json; use tokio::sync::Mutex; -pub(crate) const INVALID_PARAMS_ERROR_CODE: i64 = -32602; - /// To adhere to MCP `tools/call` response format, include the Codex /// `threadId` in the `structured_content` field of the response. /// Some MCP clients ignore `content` when `structuredContent` is present, so @@ -42,11 +39,7 @@ pub(crate) fn create_call_tool_result_with_thread_id( is_error: Option, ) -> CallToolResult { let content_text = text; - let content = vec![ContentBlock::TextContent(TextContent { - r#type: "text".to_string(), - text: content_text.clone(), - annotations: None, - })]; + let content = vec![Content::text(content_text.clone())]; let structured_content = json!({ "threadId": thread_id, "content": content_text, @@ -55,6 +48,7 @@ pub(crate) fn create_call_tool_result_with_thread_id( content, is_error, structured_content: Some(structured_content), + meta: None, } } @@ -78,13 +72,10 @@ pub async fn run_codex_tool_session( Ok(res) => res, Err(e) => { let result = CallToolResult { - content: vec![ContentBlock::TextContent(TextContent { - r#type: "text".to_string(), - text: format!("Failed to start Codex session: {e}"), - annotations: None, - })], + content: vec![Content::text(format!("Failed to start Codex session: {e}"))], is_error: Some(true), structured_content: None, + meta: None, }; outgoing.send_response(id.clone(), result).await; return; @@ -109,10 +100,7 @@ pub async fn run_codex_tool_session( // Use the original MCP request ID as the `sub_id` for the Codex submission so that // any events emitted for this tool-call can be correlated with the // originating `tools/call` request. - let sub_id = match &id { - RequestId::String(s) => s.clone(), - RequestId::Integer(n) => n.to_string(), - }; + let sub_id = id.to_string(); running_requests_id_to_codex_uuid .lock() .await @@ -207,10 +195,7 @@ async fn run_codex_tool_session_inner( request_id: RequestId, running_requests_id_to_codex_uuid: Arc>>, ) { - let request_id_str = match &request_id { - RequestId::String(s) => s.clone(), - RequestId::Integer(n) => n.to_string(), - }; + let request_id_str = request_id.to_string(); // Stream events until the task needs to pause for user interaction or // completes. diff --git a/codex-rs/mcp-server/src/error_code.rs b/codex-rs/mcp-server/src/error_code.rs deleted file mode 100644 index 1ffd889d40..0000000000 --- a/codex-rs/mcp-server/src/error_code.rs +++ /dev/null @@ -1,2 +0,0 @@ -pub(crate) const INVALID_REQUEST_ERROR_CODE: i64 = -32600; -pub(crate) const INTERNAL_ERROR_CODE: i64 = -32603; diff --git a/codex-rs/mcp-server/src/exec_approval.rs b/codex-rs/mcp-server/src/exec_approval.rs index a98099dcf2..c7914d1e60 100644 --- a/codex-rs/mcp-server/src/exec_approval.rs +++ b/codex-rs/mcp-server/src/exec_approval.rs @@ -6,20 +6,16 @@ use codex_core::protocol::Op; use codex_core::protocol::ReviewDecision; use codex_protocol::ThreadId; use codex_protocol::parse_command::ParsedCommand; -use mcp_types::ElicitRequest; -use mcp_types::ElicitRequestParamsRequestedSchema; -use mcp_types::JSONRPCErrorError; -use mcp_types::ModelContextProtocolRequest; -use mcp_types::RequestId; +use rmcp::model::ErrorData; +use rmcp::model::RequestId; use serde::Deserialize; use serde::Serialize; +use serde_json::Value; use serde_json::json; use tracing::error; -use crate::codex_tool_runner::INVALID_PARAMS_ERROR_CODE; - -/// Conforms to [`mcp_types::ElicitRequestParams`] so that it can be used as the -/// `params` field of an [`ElicitRequest`]. +/// Conforms to the MCP elicitation request params shape, so it can be used as +/// the `params` field of an `elicitation/create` request. #[derive(Debug, Deserialize, Serialize)] pub struct ExecApprovalElicitRequestParams { // These fields are required so that `params` @@ -27,7 +23,7 @@ pub struct ExecApprovalElicitRequestParams { pub message: String, #[serde(rename = "requestedSchema")] - pub requested_schema: ElicitRequestParamsRequestedSchema, + pub requested_schema: Value, // These are additional fields the client can use to // correlate the request with the codex tool call. @@ -73,11 +69,7 @@ pub(crate) async fn handle_exec_approval_request( let params = ExecApprovalElicitRequestParams { message, - requested_schema: ElicitRequestParamsRequestedSchema { - r#type: "object".to_string(), - properties: json!({}), - required: None, - }, + requested_schema: json!({"type":"object","properties":{}}), thread_id, codex_elicitation: "exec-approval".to_string(), codex_mcp_tool_call_id: tool_call_id.clone(), @@ -94,14 +86,7 @@ pub(crate) async fn handle_exec_approval_request( error!("{message}"); outgoing - .send_error( - request_id.clone(), - JSONRPCErrorError { - code: INVALID_PARAMS_ERROR_CODE, - message, - data: None, - }, - ) + .send_error(request_id.clone(), ErrorData::invalid_params(message, None)) .await; return; @@ -109,7 +94,7 @@ pub(crate) async fn handle_exec_approval_request( }; let on_response = outgoing - .send_request(ElicitRequest::METHOD, Some(params_json)) + .send_request("elicitation/create", Some(params_json)) .await; // Listen for the response on a separate task so we don't block the main agent loop. @@ -124,7 +109,7 @@ pub(crate) async fn handle_exec_approval_request( async fn on_exec_approval_response( event_id: String, - receiver: tokio::sync::oneshot::Receiver, + receiver: tokio::sync::oneshot::Receiver, codex: Arc, ) { let response = receiver.await; diff --git a/codex-rs/mcp-server/src/lib.rs b/codex-rs/mcp-server/src/lib.rs index dabd7cca0f..eed176c575 100644 --- a/codex-rs/mcp-server/src/lib.rs +++ b/codex-rs/mcp-server/src/lib.rs @@ -8,7 +8,10 @@ use std::path::PathBuf; use codex_common::CliConfigOverrides; use codex_core::config::Config; -use mcp_types::JSONRPCMessage; +use rmcp::model::ClientNotification; +use rmcp::model::ClientRequest; +use rmcp::model::JsonRpcMessage; +use serde_json::Value; use tokio::io::AsyncBufReadExt; use tokio::io::AsyncWriteExt; use tokio::io::BufReader; @@ -21,13 +24,13 @@ use tracing_subscriber::EnvFilter; mod codex_tool_config; mod codex_tool_runner; -mod error_code; mod exec_approval; pub(crate) mod message_processor; mod outgoing_message; mod patch_approval; use crate::message_processor::MessageProcessor; +use crate::outgoing_message::OutgoingJsonRpcMessage; use crate::outgoing_message::OutgoingMessage; use crate::outgoing_message::OutgoingMessageSender; @@ -43,6 +46,8 @@ pub use crate::patch_approval::PatchApprovalResponse; /// plenty for an interactive CLI. const CHANNEL_CAPACITY: usize = 128; +type IncomingMessage = JsonRpcMessage; + pub async fn run_main( codex_linux_sandbox_exe: Option, cli_config_overrides: CliConfigOverrides, @@ -55,7 +60,7 @@ pub async fn run_main( .init(); // Set up channels. - let (incoming_tx, mut incoming_rx) = mpsc::channel::(CHANNEL_CAPACITY); + let (incoming_tx, mut incoming_rx) = mpsc::channel::(CHANNEL_CAPACITY); let (outgoing_tx, mut outgoing_rx) = mpsc::unbounded_channel::(); // Task: read from stdin, push to `incoming_tx`. @@ -66,14 +71,14 @@ pub async fn run_main( let mut lines = reader.lines(); while let Some(line) = lines.next_line().await.unwrap_or_default() { - match serde_json::from_str::(&line) { + match serde_json::from_str::(&line) { Ok(msg) => { if incoming_tx.send(msg).await.is_err() { // Receiver gone – nothing left to do. break; } } - Err(e) => error!("Failed to deserialize JSONRPCMessage: {e}"), + Err(e) => error!("Failed to deserialize JSON-RPC message: {e}"), } } @@ -106,10 +111,10 @@ pub async fn run_main( async move { while let Some(msg) = incoming_rx.recv().await { match msg { - JSONRPCMessage::Request(r) => processor.process_request(r).await, - JSONRPCMessage::Response(r) => processor.process_response(r).await, - JSONRPCMessage::Notification(n) => processor.process_notification(n).await, - JSONRPCMessage::Error(e) => processor.process_error(e), + JsonRpcMessage::Request(r) => processor.process_request(r).await, + JsonRpcMessage::Response(r) => processor.process_response(r).await, + JsonRpcMessage::Notification(n) => processor.process_notification(n).await, + JsonRpcMessage::Error(e) => processor.process_error(e), } } @@ -121,7 +126,7 @@ pub async fn run_main( let stdout_writer_handle = tokio::spawn(async move { let mut stdout = io::stdout(); while let Some(outgoing_message) = outgoing_rx.recv().await { - let msg: JSONRPCMessage = outgoing_message.into(); + let msg: OutgoingJsonRpcMessage = outgoing_message.into(); match serde_json::to_string(&msg) { Ok(json) => { if let Err(e) = stdout.write_all(json.as_bytes()).await { @@ -133,7 +138,7 @@ pub async fn run_main( break; } } - Err(e) => error!("Failed to serialize JSONRPCMessage: {e}"), + Err(e) => error!("Failed to serialize JSON-RPC message: {e}"), } } diff --git a/codex-rs/mcp-server/src/message_processor.rs b/codex-rs/mcp-server/src/message_processor.rs index 9d947cda3f..c0a5249ff2 100644 --- a/codex-rs/mcp-server/src/message_processor.rs +++ b/codex-rs/mcp-server/src/message_processor.rs @@ -1,41 +1,40 @@ use std::collections::HashMap; use std::path::PathBuf; -use crate::codex_tool_config::CodexToolCallParam; -use crate::codex_tool_config::CodexToolCallReplyParam; -use crate::codex_tool_config::create_tool_for_codex_tool_call_param; -use crate::codex_tool_config::create_tool_for_codex_tool_call_reply_param; -use crate::error_code::INVALID_REQUEST_ERROR_CODE; -use crate::outgoing_message::OutgoingMessageSender; -use codex_protocol::ThreadId; -use codex_protocol::protocol::SessionSource; - use codex_core::AuthManager; use codex_core::ThreadManager; use codex_core::config::Config; use codex_core::default_client::USER_AGENT_SUFFIX; use codex_core::default_client::get_codex_user_agent; use codex_core::protocol::Submission; -use mcp_types::CallToolRequestParams; -use mcp_types::CallToolResult; -use mcp_types::ClientRequest as McpClientRequest; -use mcp_types::ContentBlock; -use mcp_types::JSONRPCError; -use mcp_types::JSONRPCErrorError; -use mcp_types::JSONRPCNotification; -use mcp_types::JSONRPCRequest; -use mcp_types::JSONRPCResponse; -use mcp_types::ListToolsResult; -use mcp_types::ModelContextProtocolRequest; -use mcp_types::RequestId; -use mcp_types::ServerCapabilitiesTools; -use mcp_types::ServerNotification; -use mcp_types::TextContent; +use codex_protocol::ThreadId; +use codex_protocol::protocol::SessionSource; +use rmcp::model::CallToolRequestParam; +use rmcp::model::CallToolResult; +use rmcp::model::ClientNotification; +use rmcp::model::ClientRequest; +use rmcp::model::ErrorCode; +use rmcp::model::ErrorData; +use rmcp::model::Implementation; +use rmcp::model::InitializeResult; +use rmcp::model::JsonRpcError; +use rmcp::model::JsonRpcNotification; +use rmcp::model::JsonRpcRequest; +use rmcp::model::JsonRpcResponse; +use rmcp::model::RequestId; +use rmcp::model::ServerCapabilities; +use rmcp::model::ToolsCapability; use serde_json::json; use std::sync::Arc; use tokio::sync::Mutex; use tokio::task; +use crate::codex_tool_config::CodexToolCallParam; +use crate::codex_tool_config::CodexToolCallReplyParam; +use crate::codex_tool_config::create_tool_for_codex_tool_call_param; +use crate::codex_tool_config::create_tool_for_codex_tool_call_reply_param; +use crate::outgoing_message::OutgoingMessageSender; + pub(crate) struct MessageProcessor { outgoing: Arc, initialized: bool, @@ -72,126 +71,113 @@ impl MessageProcessor { } } - pub(crate) async fn process_request(&mut self, request: JSONRPCRequest) { - // Hold on to the ID so we can respond. + pub(crate) async fn process_request(&mut self, request: JsonRpcRequest) { let request_id = request.id.clone(); + let client_request = request.request; - let client_request = match McpClientRequest::try_from(request) { - Ok(client_request) => client_request, - Err(e) => { - tracing::warn!("Failed to convert request: {e}"); - return; - } - }; - - // Dispatch to a dedicated handler for each request type. match client_request { - McpClientRequest::InitializeRequest(params) => { - self.handle_initialize(request_id, params).await; + ClientRequest::InitializeRequest(params) => { + self.handle_initialize(request_id, params.params).await; } - McpClientRequest::PingRequest(params) => { - self.handle_ping(request_id, params).await; + ClientRequest::PingRequest(_params) => { + self.handle_ping(request_id).await; } - McpClientRequest::ListResourcesRequest(params) => { - self.handle_list_resources(params); + ClientRequest::ListResourcesRequest(params) => { + self.handle_list_resources(params.params); } - McpClientRequest::ListResourceTemplatesRequest(params) => { - self.handle_list_resource_templates(params); + ClientRequest::ListResourceTemplatesRequest(params) => { + self.handle_list_resource_templates(params.params); } - McpClientRequest::ReadResourceRequest(params) => { - self.handle_read_resource(params); + ClientRequest::ReadResourceRequest(params) => { + self.handle_read_resource(params.params); } - McpClientRequest::SubscribeRequest(params) => { - self.handle_subscribe(params); + ClientRequest::SubscribeRequest(params) => { + self.handle_subscribe(params.params); } - McpClientRequest::UnsubscribeRequest(params) => { - self.handle_unsubscribe(params); + ClientRequest::UnsubscribeRequest(params) => { + self.handle_unsubscribe(params.params); } - McpClientRequest::ListPromptsRequest(params) => { - self.handle_list_prompts(params); + ClientRequest::ListPromptsRequest(params) => { + self.handle_list_prompts(params.params); } - McpClientRequest::GetPromptRequest(params) => { - self.handle_get_prompt(params); + ClientRequest::GetPromptRequest(params) => { + self.handle_get_prompt(params.params); } - McpClientRequest::ListToolsRequest(params) => { - self.handle_list_tools(request_id, params).await; + ClientRequest::ListToolsRequest(params) => { + self.handle_list_tools(request_id, params.params).await; } - McpClientRequest::CallToolRequest(params) => { - self.handle_call_tool(request_id, params).await; + ClientRequest::CallToolRequest(params) => { + self.handle_call_tool(request_id, params.params).await; } - McpClientRequest::SetLevelRequest(params) => { - self.handle_set_level(params); + ClientRequest::SetLevelRequest(params) => { + self.handle_set_level(params.params); } - McpClientRequest::CompleteRequest(params) => { - self.handle_complete(params); + ClientRequest::CompleteRequest(params) => { + self.handle_complete(params.params); + } + ClientRequest::CustomRequest(custom) => { + let method = custom.method.clone(); + self.outgoing + .send_error( + request_id, + ErrorData::new( + ErrorCode::METHOD_NOT_FOUND, + format!("method not found: {method}"), + Some(json!({ "method": method })), + ), + ) + .await; } } } - /// Handle a standalone JSON-RPC response originating from the peer. - pub(crate) async fn process_response(&mut self, response: JSONRPCResponse) { + pub(crate) async fn process_response(&mut self, response: JsonRpcResponse) { tracing::info!("<- response: {:?}", response); - let JSONRPCResponse { id, result, .. } = response; + let JsonRpcResponse { id, result, .. } = response; self.outgoing.notify_client_response(id, result).await } - /// Handle a fire-and-forget JSON-RPC notification. - pub(crate) async fn process_notification(&mut self, notification: JSONRPCNotification) { - let server_notification = match ServerNotification::try_from(notification) { - Ok(n) => n, - Err(e) => { - tracing::warn!("Failed to convert notification: {e}"); - return; + pub(crate) async fn process_notification( + &mut self, + notification: JsonRpcNotification, + ) { + match notification.notification { + ClientNotification::CancelledNotification(params) => { + self.handle_cancelled_notification(params.params).await; } - }; - - // Similar to requests, route each notification type to its own stub - // handler so additional logic can be implemented incrementally. - match server_notification { - ServerNotification::CancelledNotification(params) => { - self.handle_cancelled_notification(params).await; + ClientNotification::ProgressNotification(params) => { + self.handle_progress_notification(params.params); } - ServerNotification::ProgressNotification(params) => { - self.handle_progress_notification(params); + ClientNotification::RootsListChangedNotification(_params) => { + self.handle_roots_list_changed(); } - ServerNotification::ResourceListChangedNotification(params) => { - self.handle_resource_list_changed(params); + ClientNotification::InitializedNotification(_) => { + self.handle_initialized_notification(); } - ServerNotification::ResourceUpdatedNotification(params) => { - self.handle_resource_updated(params); - } - ServerNotification::PromptListChangedNotification(params) => { - self.handle_prompt_list_changed(params); - } - ServerNotification::ToolListChangedNotification(params) => { - self.handle_tool_list_changed(params); - } - ServerNotification::LoggingMessageNotification(params) => { - self.handle_logging_message(params); + ClientNotification::CustomNotification(_) => { + tracing::warn!("ignoring custom client notification"); } } } - /// Handle an error object received from the peer. - pub(crate) fn process_error(&mut self, err: JSONRPCError) { + pub(crate) fn process_error(&mut self, err: JsonRpcError) { tracing::error!("<- error: {:?}", err); } async fn handle_initialize( &mut self, id: RequestId, - params: ::Params, + params: rmcp::model::InitializeRequestParam, ) { tracing::info!("initialize -> params: {:?}", params); if self.initialized { - // Already initialised: send JSON-RPC error response. - let error = JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: "initialize called more than once".to_string(), - data: None, - }; - self.outgoing.send_error(id, error).await; + self.outgoing + .send_error( + id, + ErrorData::invalid_request("initialize called more than once", None), + ) + .await; return; } @@ -203,109 +189,108 @@ impl MessageProcessor { *suffix = Some(user_agent_suffix); } - self.initialized = true; + let server_info = Implementation { + name: "codex-mcp-server".to_string(), + title: Some("Codex".to_string()), + version: env!("CARGO_PKG_VERSION").to_string(), + icons: None, + website_url: None, + }; - // Build a minimal InitializeResult. Fill with placeholders. - let result = mcp_types::InitializeResult { - capabilities: mcp_types::ServerCapabilities { - completions: None, - experimental: None, - logging: None, - prompts: None, - resources: None, - tools: Some(ServerCapabilitiesTools { + // Preserve Codex's existing non-spec `serverInfo.user_agent` field. + let mut server_info_value = match serde_json::to_value(&server_info) { + Ok(value) => value, + Err(err) => { + self.outgoing + .send_error( + id, + ErrorData::internal_error( + format!("failed to serialize server info: {err}"), + None, + ), + ) + .await; + return; + } + }; + if let serde_json::Value::Object(ref mut obj) = server_info_value { + obj.insert("user_agent".to_string(), json!(get_codex_user_agent())); + } + + let mut result_value = match serde_json::to_value(InitializeResult { + capabilities: ServerCapabilities { + tools: Some(ToolsCapability { list_changed: Some(true), }), + ..Default::default() }, instructions: None, protocol_version: params.protocol_version.clone(), - server_info: mcp_types::Implementation { - name: "codex-mcp-server".to_string(), - version: env!("CARGO_PKG_VERSION").to_string(), - title: Some("Codex".to_string()), - user_agent: Some(get_codex_user_agent()), - }, + server_info, + }) { + Ok(value) => value, + Err(err) => { + self.outgoing + .send_error( + id, + ErrorData::internal_error( + format!("failed to serialize initialize response: {err}"), + None, + ), + ) + .await; + return; + } }; - self.send_response::(id, result) - .await; + if let serde_json::Value::Object(ref mut obj) = result_value { + obj.insert("serverInfo".to_string(), server_info_value); + } + + self.initialized = true; + self.outgoing.send_response(id, result_value).await; } - async fn send_response(&self, id: RequestId, result: T::Result) - where - T: ModelContextProtocolRequest, - { - self.outgoing.send_response(id, result).await; + async fn handle_ping(&self, id: RequestId) { + tracing::info!("ping"); + self.outgoing.send_response(id, json!({})).await; } - async fn handle_ping( - &self, - id: RequestId, - params: ::Params, - ) { - tracing::info!("ping -> params: {:?}", params); - let result = json!({}); - self.send_response::(id, result) - .await; - } - - fn handle_list_resources( - &self, - params: ::Params, - ) { + fn handle_list_resources(&self, params: Option) { tracing::info!("resources/list -> params: {:?}", params); } - fn handle_list_resource_templates( - &self, - params: - ::Params, - ) { + fn handle_list_resource_templates(&self, params: Option) { tracing::info!("resources/templates/list -> params: {:?}", params); } - fn handle_read_resource( - &self, - params: ::Params, - ) { + fn handle_read_resource(&self, params: rmcp::model::ReadResourceRequestParam) { tracing::info!("resources/read -> params: {:?}", params); } - fn handle_subscribe( - &self, - params: ::Params, - ) { + fn handle_subscribe(&self, params: rmcp::model::SubscribeRequestParam) { tracing::info!("resources/subscribe -> params: {:?}", params); } - fn handle_unsubscribe( - &self, - params: ::Params, - ) { + fn handle_unsubscribe(&self, params: rmcp::model::UnsubscribeRequestParam) { tracing::info!("resources/unsubscribe -> params: {:?}", params); } - fn handle_list_prompts( - &self, - params: ::Params, - ) { + fn handle_list_prompts(&self, params: Option) { tracing::info!("prompts/list -> params: {:?}", params); } - fn handle_get_prompt( - &self, - params: ::Params, - ) { + fn handle_get_prompt(&self, params: rmcp::model::GetPromptRequestParam) { tracing::info!("prompts/get -> params: {:?}", params); } async fn handle_list_tools( &self, id: RequestId, - params: ::Params, + _params: Option, ) { - tracing::trace!("tools/list -> {params:?}"); - let result = ListToolsResult { + let result = rmcp::model::ListToolsResult { + meta: None, tools: vec![ create_tool_for_codex_tool_call_param(), create_tool_for_codex_tool_call_reply_param(), @@ -313,19 +298,14 @@ impl MessageProcessor { next_cursor: None, }; - self.send_response::(id, result) - .await; + self.outgoing.send_response(id, result).await; } - async fn handle_call_tool( - &self, - id: RequestId, - params: ::Params, - ) { + async fn handle_call_tool(&self, id: RequestId, params: CallToolRequestParam) { tracing::info!("tools/call -> params: {:?}", params); - let CallToolRequestParams { name, arguments } = params; + let CallToolRequestParam { name, arguments } = params; - match name.as_str() { + match name.as_ref() { "codex" => self.handle_tool_call_codex(id, arguments).await, "codex-reply" => { self.handle_tool_call_codex_session_reply(id, arguments) @@ -333,20 +313,22 @@ impl MessageProcessor { } _ => { let result = CallToolResult { - content: vec![ContentBlock::TextContent(TextContent { - r#type: "text".to_string(), - text: format!("Unknown tool '{name}'"), - annotations: None, - })], - is_error: Some(true), + content: vec![rmcp::model::Content::text(format!("Unknown tool '{name}'"))], structured_content: None, + is_error: Some(true), + meta: None, }; - self.send_response::(id, result) - .await; + self.outgoing.send_response(id, result).await; } } } - async fn handle_tool_call_codex(&self, id: RequestId, arguments: Option) { + + async fn handle_tool_call_codex( + &self, + id: RequestId, + arguments: Option, + ) { + let arguments = arguments.map(serde_json::Value::Object); let (initial_prompt, config): (String, Config) = match arguments { Some(json_val) => match serde_json::from_value::(json_val) { Ok(tool_cfg) => match tool_cfg @@ -356,63 +338,49 @@ impl MessageProcessor { Ok(cfg) => cfg, Err(e) => { let result = CallToolResult { - content: vec![ContentBlock::TextContent(TextContent { - r#type: "text".to_owned(), - text: format!( - "Failed to load Codex configuration from overrides: {e}" - ), - annotations: None, - })], - is_error: Some(true), + content: vec![rmcp::model::Content::text(format!( + "Failed to load Codex configuration from overrides: {e}" + ))], structured_content: None, + is_error: Some(true), + meta: None, }; - self.send_response::(id, result) - .await; + self.outgoing.send_response(id, result).await; return; } }, Err(e) => { let result = CallToolResult { - content: vec![ContentBlock::TextContent(TextContent { - r#type: "text".to_owned(), - text: format!("Failed to parse configuration for Codex tool: {e}"), - annotations: None, - })], - is_error: Some(true), + content: vec![rmcp::model::Content::text(format!( + "Failed to parse configuration for Codex tool: {e}" + ))], structured_content: None, + is_error: Some(true), + meta: None, }; - self.send_response::(id, result) - .await; + self.outgoing.send_response(id, result).await; return; } }, None => { let result = CallToolResult { - content: vec![ContentBlock::TextContent(TextContent { - r#type: "text".to_string(), - text: - "Missing arguments for codex tool-call; the `prompt` field is required." - .to_string(), - annotations: None, - })], - is_error: Some(true), + content: vec![rmcp::model::Content::text( + "Missing arguments for codex tool-call; the `prompt` field is required.", + )], structured_content: None, + is_error: Some(true), + meta: None, }; - self.send_response::(id, result) - .await; + self.outgoing.send_response(id, result).await; return; } }; - // Clone outgoing and server to move into async task. let outgoing = self.outgoing.clone(); let thread_manager = self.thread_manager.clone(); let running_requests_id_to_codex_uuid = self.running_requests_id_to_codex_uuid.clone(); - // Spawn an async task to handle the Codex session so that we do not - // block the synchronous message-processing loop. task::spawn(async move { - // Run the Codex session and stream events back to the client. crate::codex_tool_runner::run_codex_tool_session( id, initial_prompt, @@ -428,27 +396,25 @@ impl MessageProcessor { async fn handle_tool_call_codex_session_reply( &self, request_id: RequestId, - arguments: Option, + arguments: Option, ) { + let arguments = arguments.map(serde_json::Value::Object); tracing::info!("tools/call -> params: {:?}", arguments); - // parse arguments let codex_tool_call_reply_param: CodexToolCallReplyParam = match arguments { Some(json_val) => match serde_json::from_value::(json_val) { Ok(params) => params, Err(e) => { tracing::error!("Failed to parse Codex tool call reply parameters: {e}"); let result = CallToolResult { - content: vec![ContentBlock::TextContent(TextContent { - r#type: "text".to_owned(), - text: format!("Failed to parse configuration for Codex tool: {e}"), - annotations: None, - })], - is_error: Some(true), + content: vec![rmcp::model::Content::text(format!( + "Failed to parse configuration for Codex tool: {e}" + ))], structured_content: None, + is_error: Some(true), + meta: None, }; - self.send_response::(request_id, result) - .await; + self.outgoing.send_response(request_id, result).await; return; } }, @@ -457,16 +423,14 @@ impl MessageProcessor { "Missing arguments for codex-reply tool-call; the `thread_id` and `prompt` fields are required." ); let result = CallToolResult { - content: vec![ContentBlock::TextContent(TextContent { - r#type: "text".to_owned(), - text: "Missing arguments for codex-reply tool-call; the `thread_id` and `prompt` fields are required.".to_owned(), - annotations: None, - })], - is_error: Some(true), + content: vec![rmcp::model::Content::text( + "Missing arguments for codex-reply tool-call; the `thread_id` and `prompt` fields are required.", + )], structured_content: None, + is_error: Some(true), + meta: None, }; - self.send_response::(request_id, result) - .await; + self.outgoing.send_response(request_id, result).await; return; } }; @@ -476,21 +440,18 @@ impl MessageProcessor { Err(e) => { tracing::error!("Failed to parse thread_id: {e}"); let result = CallToolResult { - content: vec![ContentBlock::TextContent(TextContent { - r#type: "text".to_owned(), - text: format!("Failed to parse thread_id: {e}"), - annotations: None, - })], - is_error: Some(true), + content: vec![rmcp::model::Content::text(format!( + "Failed to parse thread_id: {e}" + ))], structured_content: None, + is_error: Some(true), + meta: None, }; - self.send_response::(request_id, result) - .await; + self.outgoing.send_response(request_id, result).await; return; } }; - // Clone outgoing to move into async task. let outgoing = self.outgoing.clone(); let running_requests_id_to_codex_uuid = self.running_requests_id_to_codex_uuid.clone(); @@ -508,7 +469,6 @@ impl MessageProcessor { } }; - // Spawn the long-running reply handler. let prompt = codex_tool_call_reply_param.prompt.clone(); tokio::spawn({ let outgoing = outgoing.clone(); @@ -528,49 +488,30 @@ impl MessageProcessor { }); } - fn handle_set_level( - &self, - params: ::Params, - ) { + fn handle_set_level(&self, params: rmcp::model::SetLevelRequestParam) { tracing::info!("logging/setLevel -> params: {:?}", params); } - fn handle_complete( - &self, - params: ::Params, - ) { + fn handle_complete(&self, params: rmcp::model::CompleteRequestParam) { tracing::info!("completion/complete -> params: {:?}", params); } - // --------------------------------------------------------------------- - // Notification handlers - // --------------------------------------------------------------------- - - async fn handle_cancelled_notification( - &self, - params: ::Params, - ) { + async fn handle_cancelled_notification(&self, params: rmcp::model::CancelledNotificationParam) { let request_id = params.request_id; - // Create a stable string form early for logging and submission id. - let request_id_string = match &request_id { - RequestId::String(s) => s.clone(), - RequestId::Integer(i) => i.to_string(), - }; + let request_id_string = request_id.to_string(); - // Obtain the thread id while holding the first lock, then release. let thread_id = { let map_guard = self.running_requests_id_to_codex_uuid.lock().await; match map_guard.get(&request_id) { Some(id) => *id, None => { - tracing::warn!("Session not found for request_id: {}", request_id_string); + tracing::warn!("Session not found for request_id: {request_id_string}"); return; } } }; tracing::info!("thread_id: {thread_id}"); - // Obtain the Codex thread from the server. let codex_arc = match self.thread_manager.get_thread(thread_id).await { Ok(c) => c, Err(_) => { @@ -579,66 +520,31 @@ impl MessageProcessor { } }; - // Submit interrupt to Codex. - let err = codex_arc + if let Err(e) = codex_arc .submit_with_id(Submission { id: request_id_string, op: codex_core::protocol::Op::Interrupt, }) - .await; - if let Err(e) = err { + .await + { tracing::error!("Failed to submit interrupt to Codex: {e}"); return; } - // unregister the id so we don't keep it in the map self.running_requests_id_to_codex_uuid .lock() .await .remove(&request_id); } - fn handle_progress_notification( - &self, - params: ::Params, - ) { + fn handle_progress_notification(&self, params: rmcp::model::ProgressNotificationParam) { tracing::info!("notifications/progress -> params: {:?}", params); } - fn handle_resource_list_changed( - &self, - params: ::Params, - ) { - tracing::info!( - "notifications/resources/list_changed -> params: {:?}", - params - ); + fn handle_roots_list_changed(&self) { + tracing::info!("notifications/roots/list_changed"); } - fn handle_resource_updated( - &self, - params: ::Params, - ) { - tracing::info!("notifications/resources/updated -> params: {:?}", params); - } - - fn handle_prompt_list_changed( - &self, - params: ::Params, - ) { - tracing::info!("notifications/prompts/list_changed -> params: {:?}", params); - } - - fn handle_tool_list_changed( - &self, - params: ::Params, - ) { - tracing::info!("notifications/tools/list_changed -> params: {:?}", params); - } - - fn handle_logging_message( - &self, - params: ::Params, - ) { - tracing::info!("notifications/message -> params: {:?}", params); + fn handle_initialized_notification(&self) { + tracing::info!("notifications/initialized"); } } diff --git a/codex-rs/mcp-server/src/outgoing_message.rs b/codex-rs/mcp-server/src/outgoing_message.rs index 954e50d9de..e512eedbd7 100644 --- a/codex-rs/mcp-server/src/outgoing_message.rs +++ b/codex-rs/mcp-server/src/outgoing_message.rs @@ -4,28 +4,30 @@ use std::sync::atomic::Ordering; use codex_core::protocol::Event; use codex_protocol::ThreadId; -use mcp_types::JSONRPC_VERSION; -use mcp_types::JSONRPCError; -use mcp_types::JSONRPCErrorError; -use mcp_types::JSONRPCMessage; -use mcp_types::JSONRPCNotification; -use mcp_types::JSONRPCRequest; -use mcp_types::JSONRPCResponse; -use mcp_types::RequestId; -use mcp_types::Result; +use rmcp::model::CustomNotification; +use rmcp::model::CustomRequest; +use rmcp::model::ErrorData; +use rmcp::model::JsonRpcError; +use rmcp::model::JsonRpcMessage; +use rmcp::model::JsonRpcNotification; +use rmcp::model::JsonRpcRequest; +use rmcp::model::JsonRpcResponse; +use rmcp::model::JsonRpcVersion2_0; +use rmcp::model::RequestId; use serde::Serialize; +use serde_json::Value; use tokio::sync::Mutex; use tokio::sync::mpsc; use tokio::sync::oneshot; use tracing::warn; -use crate::error_code::INTERNAL_ERROR_CODE; +pub(crate) type OutgoingJsonRpcMessage = JsonRpcMessage; /// Sends messages to the client and manages request callbacks. pub(crate) struct OutgoingMessageSender { next_request_id: AtomicI64, sender: mpsc::UnboundedSender, - request_id_to_callback: Mutex>>, + request_id_to_callback: Mutex>>, } impl OutgoingMessageSender { @@ -41,8 +43,8 @@ impl OutgoingMessageSender { &self, method: &str, params: Option, - ) -> oneshot::Receiver { - let id = RequestId::Integer(self.next_request_id.fetch_add(1, Ordering::Relaxed)); + ) -> oneshot::Receiver { + let id = RequestId::Number(self.next_request_id.fetch_add(1, Ordering::Relaxed)); let outgoing_message_id = id.clone(); let (tx_approve, rx_approve) = oneshot::channel(); { @@ -59,7 +61,7 @@ impl OutgoingMessageSender { rx_approve } - pub(crate) async fn notify_client_response(&self, id: RequestId, result: Result) { + pub(crate) async fn notify_client_response(&self, id: RequestId, result: Value) { let entry = { let mut request_id_to_callback = self.request_id_to_callback.lock().await; request_id_to_callback.remove_entry(&id) @@ -78,23 +80,20 @@ impl OutgoingMessageSender { } pub(crate) async fn send_response(&self, id: RequestId, response: T) { - match serde_json::to_value(response) { - Ok(result) => { - let outgoing_message = OutgoingMessage::Response(OutgoingResponse { id, result }); - let _ = self.sender.send(outgoing_message); - } + let result = match serde_json::to_value(response) { + Ok(result) => result, Err(err) => { self.send_error( id, - JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: format!("failed to serialize response: {err}"), - data: None, - }, + ErrorData::internal_error(format!("failed to serialize response: {err}"), None), ) .await; + return; } - } + }; + + let outgoing_message = OutgoingMessage::Response(OutgoingResponse { id, result }); + let _ = self.sender.send(outgoing_message); } /// This is used with the MCP server, but not the more general JSON-RPC app @@ -130,7 +129,7 @@ impl OutgoingMessageSender { let _ = self.sender.send(outgoing_message); } - pub(crate) async fn send_error(&self, id: RequestId, error: JSONRPCErrorError) { + pub(crate) async fn send_error(&self, id: RequestId, error: ErrorData) { let outgoing_message = OutgoingMessage::Error(OutgoingError { id, error }); let _ = self.sender.send(outgoing_message); } @@ -144,34 +143,32 @@ pub(crate) enum OutgoingMessage { Error(OutgoingError), } -impl From for JSONRPCMessage { +impl From for OutgoingJsonRpcMessage { fn from(val: OutgoingMessage) -> Self { use OutgoingMessage::*; match val { Request(OutgoingRequest { id, method, params }) => { - JSONRPCMessage::Request(JSONRPCRequest { - jsonrpc: JSONRPC_VERSION.into(), + JsonRpcMessage::Request(JsonRpcRequest { + jsonrpc: JsonRpcVersion2_0, id, - method, - params, + request: CustomRequest::new(method, params), }) } Notification(OutgoingNotification { method, params }) => { - JSONRPCMessage::Notification(JSONRPCNotification { - jsonrpc: JSONRPC_VERSION.into(), - method, - params, + JsonRpcMessage::Notification(JsonRpcNotification { + jsonrpc: JsonRpcVersion2_0, + notification: CustomNotification::new(method, params), }) } Response(OutgoingResponse { id, result }) => { - JSONRPCMessage::Response(JSONRPCResponse { - jsonrpc: JSONRPC_VERSION.into(), + JsonRpcMessage::Response(JsonRpcResponse { + jsonrpc: JsonRpcVersion2_0, id, result, }) } - Error(OutgoingError { id, error }) => JSONRPCMessage::Error(JSONRPCError { - jsonrpc: JSONRPC_VERSION.into(), + Error(OutgoingError { id, error }) => JsonRpcMessage::Error(JsonRpcError { + jsonrpc: JsonRpcVersion2_0, id, error, }), @@ -220,12 +217,12 @@ pub(crate) struct OutgoingNotificationMeta { #[derive(Debug, Clone, PartialEq, Serialize)] pub(crate) struct OutgoingResponse { pub id: RequestId, - pub result: Result, + pub result: Value, } #[derive(Debug, Clone, PartialEq, Serialize)] pub(crate) struct OutgoingError { - pub error: JSONRPCErrorError, + pub error: ErrorData, pub id: RequestId, } @@ -246,6 +243,48 @@ mod tests { use super::*; + #[test] + fn outgoing_request_serializes_as_jsonrpc_request() { + let msg: OutgoingJsonRpcMessage = OutgoingMessage::Request(OutgoingRequest { + id: RequestId::Number(1), + method: "elicitation/create".to_string(), + params: Some(json!({ "k": "v" })), + }) + .into(); + + let value = serde_json::to_value(msg).expect("message should serialize"); + let obj = value.as_object().expect("json object"); + + assert_eq!(obj.get("jsonrpc"), Some(&json!("2.0"))); + assert_eq!(obj.get("id"), Some(&json!(1))); + assert_eq!(obj.get("method"), Some(&json!("elicitation/create"))); + assert_eq!(obj.get("params"), Some(&json!({ "k": "v" }))); + assert!( + obj.get("request").is_none(), + "rmcp request must flatten to JSON-RPC method/params" + ); + } + + #[test] + fn outgoing_notification_serializes_as_jsonrpc_notification() { + let msg: OutgoingJsonRpcMessage = OutgoingMessage::Notification(OutgoingNotification { + method: "notifications/initialized".to_string(), + params: None, + }) + .into(); + + let value = serde_json::to_value(msg).expect("message should serialize"); + let obj = value.as_object().expect("json object"); + + assert_eq!(obj.get("jsonrpc"), Some(&json!("2.0"))); + assert_eq!(obj.get("method"), Some(&json!("notifications/initialized"))); + assert_eq!(obj.get("params"), Some(&serde_json::Value::Null)); + assert!( + obj.get("notification").is_none(), + "rmcp notification must flatten to JSON-RPC method/params" + ); + } + #[tokio::test] async fn test_send_event_as_notification() -> Result<()> { let (outgoing_tx, mut outgoing_rx) = mpsc::unbounded_channel::(); @@ -316,7 +355,7 @@ mod tests { msg: EventMsg::SessionConfigured(session_configured_event.clone()), }; let meta = OutgoingNotificationMeta { - request_id: Some(RequestId::String("123".to_string())), + request_id: Some(RequestId::String("123".into())), thread_id: None, }; @@ -381,7 +420,7 @@ mod tests { msg: EventMsg::SessionConfigured(session_configured_event.clone()), }; let meta = OutgoingNotificationMeta { - request_id: Some(RequestId::String("123".to_string())), + request_id: Some(RequestId::String("123".into())), thread_id: Some(thread_id), }; diff --git a/codex-rs/mcp-server/src/patch_approval.rs b/codex-rs/mcp-server/src/patch_approval.rs index 5c3073959d..55938e257b 100644 --- a/codex-rs/mcp-server/src/patch_approval.rs +++ b/codex-rs/mcp-server/src/patch_approval.rs @@ -7,24 +7,21 @@ use codex_core::protocol::FileChange; use codex_core::protocol::Op; use codex_core::protocol::ReviewDecision; use codex_protocol::ThreadId; -use mcp_types::ElicitRequest; -use mcp_types::ElicitRequestParamsRequestedSchema; -use mcp_types::JSONRPCErrorError; -use mcp_types::ModelContextProtocolRequest; -use mcp_types::RequestId; +use rmcp::model::ErrorData; +use rmcp::model::RequestId; use serde::Deserialize; use serde::Serialize; +use serde_json::Value; use serde_json::json; use tracing::error; -use crate::codex_tool_runner::INVALID_PARAMS_ERROR_CODE; use crate::outgoing_message::OutgoingMessageSender; #[derive(Debug, Deserialize, Serialize)] pub struct PatchApprovalElicitRequestParams { pub message: String, #[serde(rename = "requestedSchema")] - pub requested_schema: ElicitRequestParamsRequestedSchema, + pub requested_schema: Value, #[serde(rename = "threadId")] pub thread_id: ThreadId, pub codex_elicitation: String, @@ -64,11 +61,7 @@ pub(crate) async fn handle_patch_approval_request( let params = PatchApprovalElicitRequestParams { message: message_lines.join("\n"), - requested_schema: ElicitRequestParamsRequestedSchema { - r#type: "object".to_string(), - properties: json!({}), - required: None, - }, + requested_schema: json!({"type":"object","properties":{}}), thread_id, codex_elicitation: "patch-approval".to_string(), codex_mcp_tool_call_id: tool_call_id.clone(), @@ -85,14 +78,7 @@ pub(crate) async fn handle_patch_approval_request( error!("{message}"); outgoing - .send_error( - request_id.clone(), - JSONRPCErrorError { - code: INVALID_PARAMS_ERROR_CODE, - message, - data: None, - }, - ) + .send_error(request_id.clone(), ErrorData::invalid_params(message, None)) .await; return; @@ -100,7 +86,7 @@ pub(crate) async fn handle_patch_approval_request( }; let on_response = outgoing - .send_request(ElicitRequest::METHOD, Some(params_json)) + .send_request("elicitation/create", Some(params_json)) .await; // Listen for the response on a separate task so we don't block the main agent loop. @@ -115,7 +101,7 @@ pub(crate) async fn handle_patch_approval_request( pub(crate) async fn on_patch_approval_response( event_id: String, - receiver: tokio::sync::oneshot::Receiver, + receiver: tokio::sync::oneshot::Receiver, codex: Arc, ) { let response = receiver.await; diff --git a/codex-rs/mcp-server/tests/common/Cargo.toml b/codex-rs/mcp-server/tests/common/Cargo.toml index aba984edab..1dec2d09ac 100644 --- a/codex-rs/mcp-server/tests/common/Cargo.toml +++ b/codex-rs/mcp-server/tests/common/Cargo.toml @@ -12,7 +12,7 @@ anyhow = { workspace = true } codex-core = { workspace = true } codex-mcp-server = { workspace = true } codex-utils-cargo-bin = { workspace = true } -mcp-types = { workspace = true } +rmcp = { workspace = true } os_info = { workspace = true } pretty_assertions = { workspace = true } serde = { workspace = true } diff --git a/codex-rs/mcp-server/tests/common/lib.rs b/codex-rs/mcp-server/tests/common/lib.rs index 364c708651..c2c3757efc 100644 --- a/codex-rs/mcp-server/tests/common/lib.rs +++ b/codex-rs/mcp-server/tests/common/lib.rs @@ -6,14 +6,16 @@ pub use core_test_support::format_with_current_shell; pub use core_test_support::format_with_current_shell_display_non_login; pub use core_test_support::format_with_current_shell_non_login; pub use mcp_process::McpProcess; -use mcp_types::JSONRPCResponse; pub use mock_model_server::create_mock_chat_completions_server; pub use responses::create_apply_patch_sse_response; pub use responses::create_final_assistant_message_sse_response; pub use responses::create_shell_command_sse_response; +use rmcp::model::JsonRpcResponse; use serde::de::DeserializeOwned; -pub fn to_response(response: JSONRPCResponse) -> anyhow::Result { +pub fn to_response( + response: JsonRpcResponse, +) -> anyhow::Result { let value = serde_json::to_value(response.result)?; let codex_response = serde_json::from_value(value)?; Ok(codex_response) diff --git a/codex-rs/mcp-server/tests/common/mcp_process.rs b/codex-rs/mcp-server/tests/common/mcp_process.rs index 9a3f076fb1..c019120282 100644 --- a/codex-rs/mcp-server/tests/common/mcp_process.rs +++ b/codex-rs/mcp-server/tests/common/mcp_process.rs @@ -12,19 +12,21 @@ use tokio::process::ChildStdout; use anyhow::Context; use codex_mcp_server::CodexToolCallParam; -use mcp_types::CallToolRequestParams; -use mcp_types::ClientCapabilities; -use mcp_types::Implementation; -use mcp_types::InitializeRequestParams; -use mcp_types::JSONRPC_VERSION; -use mcp_types::JSONRPCMessage; -use mcp_types::JSONRPCNotification; -use mcp_types::JSONRPCRequest; -use mcp_types::JSONRPCResponse; -use mcp_types::ModelContextProtocolNotification; -use mcp_types::ModelContextProtocolRequest; -use mcp_types::RequestId; use pretty_assertions::assert_eq; +use rmcp::model::CallToolRequestParam; +use rmcp::model::ClientCapabilities; +use rmcp::model::CustomNotification; +use rmcp::model::CustomRequest; +use rmcp::model::ElicitationCapability; +use rmcp::model::Implementation; +use rmcp::model::InitializeRequestParam; +use rmcp::model::JsonRpcMessage; +use rmcp::model::JsonRpcNotification; +use rmcp::model::JsonRpcRequest; +use rmcp::model::JsonRpcResponse; +use rmcp::model::JsonRpcVersion2_0; +use rmcp::model::ProtocolVersion; +use rmcp::model::RequestId; use serde_json::json; use tokio::process::Command; @@ -110,9 +112,11 @@ impl McpProcess { pub async fn initialize(&mut self) -> anyhow::Result<()> { let request_id = self.next_request_id.fetch_add(1, Ordering::Relaxed); - let params = InitializeRequestParams { + let params = InitializeRequestParam { capabilities: ClientCapabilities { - elicitation: Some(json!({})), + elicitation: Some(ElicitationCapability { + schema_validation: None, + }), experimental: None, roots: None, sampling: None, @@ -121,17 +125,17 @@ impl McpProcess { name: "elicitation test".into(), title: Some("Elicitation Test".into()), version: "0.0.0".into(), - user_agent: None, + icons: None, + website_url: None, }, - protocol_version: mcp_types::MCP_SCHEMA_VERSION.into(), + protocol_version: ProtocolVersion::V_2025_03_26, }; let params_value = serde_json::to_value(params)?; - self.send_jsonrpc_message(JSONRPCMessage::Request(JSONRPCRequest { - jsonrpc: JSONRPC_VERSION.into(), - id: RequestId::Integer(request_id), - method: mcp_types::InitializeRequest::METHOD.into(), - params: Some(params_value), + self.send_jsonrpc_message(JsonRpcMessage::Request(JsonRpcRequest { + jsonrpc: JsonRpcVersion2_0, + id: RequestId::Number(request_id), + request: CustomRequest::new("initialize", Some(params_value)), })) .await?; @@ -146,33 +150,38 @@ impl McpProcess { os_info.architecture().unwrap_or("unknown"), codex_core::terminal::user_agent() ); + let JsonRpcMessage::Response(JsonRpcResponse { + jsonrpc, + id, + result, + }) = initialized + else { + anyhow::bail!("expected initialize response message, got: {initialized:?}") + }; + assert_eq!(jsonrpc, JsonRpcVersion2_0); + assert_eq!(id, RequestId::Number(request_id)); assert_eq!( - JSONRPCMessage::Response(JSONRPCResponse { - jsonrpc: JSONRPC_VERSION.into(), - id: RequestId::Integer(request_id), - result: json!({ - "capabilities": { - "tools": { - "listChanged": true - }, + result, + json!({ + "capabilities": { + "tools": { + "listChanged": true }, - "serverInfo": { - "name": "codex-mcp-server", - "title": "Codex", - "version": "0.0.0", - "user_agent": user_agent - }, - "protocolVersion": mcp_types::MCP_SCHEMA_VERSION - }) - }), - initialized + }, + "serverInfo": { + "name": "codex-mcp-server", + "title": "Codex", + "version": "0.0.0", + "user_agent": user_agent + }, + "protocolVersion": ProtocolVersion::V_2025_03_26 + }) ); // Send notifications/initialized to ack the response. - self.send_jsonrpc_message(JSONRPCMessage::Notification(JSONRPCNotification { - jsonrpc: JSONRPC_VERSION.into(), - method: mcp_types::InitializedNotification::METHOD.into(), - params: None, + self.send_jsonrpc_message(JsonRpcMessage::Notification(JsonRpcNotification { + jsonrpc: JsonRpcVersion2_0, + notification: CustomNotification::new("notifications/initialized", None), })) .await?; @@ -185,12 +194,15 @@ impl McpProcess { &mut self, params: CodexToolCallParam, ) -> anyhow::Result { - let codex_tool_call_params = CallToolRequestParams { - name: "codex".to_string(), - arguments: Some(serde_json::to_value(params)?), + let codex_tool_call_params = CallToolRequestParam { + name: "codex".into(), + arguments: Some(match serde_json::to_value(params)? { + serde_json::Value::Object(map) => map, + _ => unreachable!("params serialize to object"), + }), }; self.send_request( - mcp_types::CallToolRequest::METHOD, + "tools/call", Some(serde_json::to_value(codex_tool_call_params)?), ) .await @@ -203,11 +215,10 @@ impl McpProcess { ) -> anyhow::Result { let request_id = self.next_request_id.fetch_add(1, Ordering::Relaxed); - let message = JSONRPCMessage::Request(JSONRPCRequest { - jsonrpc: JSONRPC_VERSION.into(), - id: RequestId::Integer(request_id), - method: method.to_string(), - params, + let message = JsonRpcMessage::Request(JsonRpcRequest { + jsonrpc: JsonRpcVersion2_0, + id: RequestId::Number(request_id), + request: CustomRequest::new(method, params), }); self.send_jsonrpc_message(message).await?; Ok(request_id) @@ -218,15 +229,18 @@ impl McpProcess { id: RequestId, result: serde_json::Value, ) -> anyhow::Result<()> { - self.send_jsonrpc_message(JSONRPCMessage::Response(JSONRPCResponse { - jsonrpc: JSONRPC_VERSION.into(), + self.send_jsonrpc_message(JsonRpcMessage::Response(JsonRpcResponse { + jsonrpc: JsonRpcVersion2_0, id, result, })) .await } - async fn send_jsonrpc_message(&mut self, message: JSONRPCMessage) -> anyhow::Result<()> { + async fn send_jsonrpc_message( + &mut self, + message: JsonRpcMessage, + ) -> anyhow::Result<()> { eprintln!("writing message to stdin: {message:?}"); let payload = serde_json::to_string(&message)?; self.stdin.write_all(payload.as_bytes()).await?; @@ -235,31 +249,37 @@ impl McpProcess { Ok(()) } - async fn read_jsonrpc_message(&mut self) -> anyhow::Result { + async fn read_jsonrpc_message( + &mut self, + ) -> anyhow::Result> { let mut line = String::new(); self.stdout.read_line(&mut line).await?; - let message = serde_json::from_str::(&line)?; + let message = serde_json::from_str::< + JsonRpcMessage, + >(&line)?; eprintln!("read message from stdout: {message:?}"); Ok(message) } - pub async fn read_stream_until_request_message(&mut self) -> anyhow::Result { + pub async fn read_stream_until_request_message( + &mut self, + ) -> anyhow::Result> { eprintln!("in read_stream_until_request_message()"); loop { let message = self.read_jsonrpc_message().await?; match message { - JSONRPCMessage::Notification(_) => { + JsonRpcMessage::Notification(_) => { eprintln!("notification: {message:?}"); } - JSONRPCMessage::Request(jsonrpc_request) => { + JsonRpcMessage::Request(jsonrpc_request) => { return Ok(jsonrpc_request); } - JSONRPCMessage::Error(_) => { + JsonRpcMessage::Error(_) => { anyhow::bail!("unexpected JSONRPCMessage::Error: {message:?}"); } - JSONRPCMessage::Response(_) => { + JsonRpcMessage::Response(_) => { anyhow::bail!("unexpected JSONRPCMessage::Response: {message:?}"); } } @@ -269,22 +289,22 @@ impl McpProcess { pub async fn read_stream_until_response_message( &mut self, request_id: RequestId, - ) -> anyhow::Result { + ) -> anyhow::Result> { eprintln!("in read_stream_until_response_message({request_id:?})"); loop { let message = self.read_jsonrpc_message().await?; match message { - JSONRPCMessage::Notification(_) => { + JsonRpcMessage::Notification(_) => { eprintln!("notification: {message:?}"); } - JSONRPCMessage::Request(_) => { + JsonRpcMessage::Request(_) => { anyhow::bail!("unexpected JSONRPCMessage::Request: {message:?}"); } - JSONRPCMessage::Error(_) => { + JsonRpcMessage::Error(_) => { anyhow::bail!("unexpected JSONRPCMessage::Error: {message:?}"); } - JSONRPCMessage::Response(jsonrpc_response) => { + JsonRpcMessage::Response(jsonrpc_response) => { if jsonrpc_response.id == request_id { return Ok(jsonrpc_response); } @@ -297,15 +317,15 @@ impl McpProcess { /// Method "codex/event" with params.msg.type == "task_complete". pub async fn read_stream_until_legacy_task_complete_notification( &mut self, - ) -> anyhow::Result { + ) -> anyhow::Result> { eprintln!("in read_stream_until_legacy_task_complete_notification()"); loop { let message = self.read_jsonrpc_message().await?; match message { - JSONRPCMessage::Notification(notification) => { - let is_match = if notification.method == "codex/event" { - if let Some(params) = ¬ification.params { + JsonRpcMessage::Notification(notification) => { + let is_match = if notification.notification.method == "codex/event" { + if let Some(params) = ¬ification.notification.params { params .get("msg") .and_then(|m| m.get("type")) @@ -324,13 +344,13 @@ impl McpProcess { eprintln!("ignoring notification: {notification:?}"); } } - JSONRPCMessage::Request(_) => { + JsonRpcMessage::Request(_) => { anyhow::bail!("unexpected JSONRPCMessage::Request: {message:?}"); } - JSONRPCMessage::Error(_) => { + JsonRpcMessage::Error(_) => { anyhow::bail!("unexpected JSONRPCMessage::Error: {message:?}"); } - JSONRPCMessage::Response(_) => { + JsonRpcMessage::Response(_) => { anyhow::bail!("unexpected JSONRPCMessage::Response: {message:?}"); } } diff --git a/codex-rs/mcp-server/tests/suite/codex_tool.rs b/codex-rs/mcp-server/tests/suite/codex_tool.rs index 31c451d24f..15836b9057 100644 --- a/codex-rs/mcp-server/tests/suite/codex_tool.rs +++ b/codex-rs/mcp-server/tests/suite/codex_tool.rs @@ -12,14 +12,10 @@ use codex_mcp_server::ExecApprovalElicitRequestParams; use codex_mcp_server::ExecApprovalResponse; use codex_mcp_server::PatchApprovalElicitRequestParams; use codex_mcp_server::PatchApprovalResponse; -use mcp_types::ElicitRequest; -use mcp_types::ElicitRequestParamsRequestedSchema; -use mcp_types::JSONRPC_VERSION; -use mcp_types::JSONRPCRequest; -use mcp_types::JSONRPCResponse; -use mcp_types::ModelContextProtocolRequest; -use mcp_types::RequestId; use pretty_assertions::assert_eq; +use rmcp::model::JsonRpcResponse; +use rmcp::model::JsonRpcVersion2_0; +use rmcp::model::RequestId; use serde_json::json; use tempfile::TempDir; use tokio::time::timeout; @@ -106,22 +102,27 @@ async fn shell_command_approval_triggers_elicitation() -> anyhow::Result<()> { ) .await??; + assert_eq!(elicitation_request.jsonrpc, JsonRpcVersion2_0); + assert_eq!(elicitation_request.request.method, "elicitation/create"); + let elicitation_request_id = elicitation_request.id.clone(); let params = serde_json::from_value::( elicitation_request + .request .params .clone() .ok_or_else(|| anyhow::anyhow!("elicitation_request.params must be set"))?, )?; - let expected_elicitation_request = create_expected_elicitation_request( - elicitation_request_id.clone(), - expected_shell_command, - workdir_for_shell_function_call.path(), - codex_request_id.to_string(), - params.codex_event_id.clone(), - params.thread_id, - )?; - assert_eq!(expected_elicitation_request, elicitation_request); + assert_eq!( + elicitation_request.request.params, + Some(create_expected_elicitation_request_params( + expected_shell_command, + workdir_for_shell_function_call.path(), + codex_request_id.to_string(), + params.codex_event_id.clone(), + params.thread_id, + )?) + ); // Accept the `git init` request by responding to the elicitation. mcp_process @@ -146,13 +147,13 @@ async fn shell_command_approval_triggers_elicitation() -> anyhow::Result<()> { // Verify the original `codex` tool call completes and that the file was created. let codex_response = timeout( DEFAULT_READ_TIMEOUT, - mcp_process.read_stream_until_response_message(RequestId::Integer(codex_request_id)), + mcp_process.read_stream_until_response_message(RequestId::Number(codex_request_id)), ) .await??; assert_eq!( - JSONRPCResponse { - jsonrpc: JSONRPC_VERSION.into(), - id: RequestId::Integer(codex_request_id), + JsonRpcResponse { + jsonrpc: JsonRpcVersion2_0, + id: RequestId::Number(codex_request_id), result: json!({ "content": [ { @@ -174,41 +175,32 @@ async fn shell_command_approval_triggers_elicitation() -> anyhow::Result<()> { Ok(()) } -fn create_expected_elicitation_request( - elicitation_request_id: RequestId, +fn create_expected_elicitation_request_params( command: Vec, workdir: &Path, codex_mcp_tool_call_id: String, codex_event_id: String, thread_id: codex_protocol::ThreadId, -) -> anyhow::Result { +) -> anyhow::Result { let expected_message = format!( "Allow Codex to run `{}` in `{}`?", shlex::try_join(command.iter().map(std::convert::AsRef::as_ref))?, workdir.to_string_lossy() ); let codex_parsed_cmd = parse_command::parse_command(&command); - Ok(JSONRPCRequest { - jsonrpc: JSONRPC_VERSION.into(), - id: elicitation_request_id, - method: ElicitRequest::METHOD.to_string(), - params: Some(serde_json::to_value(&ExecApprovalElicitRequestParams { - message: expected_message, - requested_schema: ElicitRequestParamsRequestedSchema { - r#type: "object".to_string(), - properties: json!({}), - required: None, - }, - thread_id, - codex_elicitation: "exec-approval".to_string(), - codex_mcp_tool_call_id, - codex_event_id, - codex_command: command, - codex_cwd: workdir.to_path_buf(), - codex_call_id: "call1234".to_string(), - codex_parsed_cmd, - })?), - }) + let params_json = serde_json::to_value(ExecApprovalElicitRequestParams { + message: expected_message, + requested_schema: json!({"type":"object","properties":{}}), + thread_id, + codex_elicitation: "exec-approval".to_string(), + codex_mcp_tool_call_id, + codex_event_id, + codex_command: command, + codex_cwd: workdir.to_path_buf(), + codex_call_id: "call1234".to_string(), + codex_parsed_cmd, + })?; + Ok(params_json) } /// Test that patch approval triggers an elicitation request to the MCP and that @@ -267,9 +259,13 @@ async fn patch_approval_triggers_elicitation() -> anyhow::Result<()> { ) .await??; + assert_eq!(elicitation_request.jsonrpc, JsonRpcVersion2_0); + assert_eq!(elicitation_request.request.method, "elicitation/create"); + let elicitation_request_id = elicitation_request.id.clone(); let params = serde_json::from_value::( elicitation_request + .request .params .clone() .ok_or_else(|| anyhow::anyhow!("elicitation_request.params must be set"))?, @@ -284,16 +280,17 @@ async fn patch_approval_triggers_elicitation() -> anyhow::Result<()> { }, ); - let expected_elicitation_request = create_expected_patch_approval_elicitation_request( - elicitation_request_id.clone(), - expected_changes, - None, // No grant_root expected - None, // No reason expected - codex_request_id.to_string(), - params.codex_event_id.clone(), - params.thread_id, - )?; - assert_eq!(expected_elicitation_request, elicitation_request); + assert_eq!( + elicitation_request.request.params, + Some(create_expected_patch_approval_elicitation_request_params( + expected_changes, + None, // No grant_root expected + None, // No reason expected + codex_request_id.to_string(), + params.codex_event_id.clone(), + params.thread_id, + )?) + ); // Accept the patch approval request by responding to the elicitation mcp_process @@ -308,13 +305,13 @@ async fn patch_approval_triggers_elicitation() -> anyhow::Result<()> { // Verify the original `codex` tool call completes let codex_response = timeout( DEFAULT_READ_TIMEOUT, - mcp_process.read_stream_until_response_message(RequestId::Integer(codex_request_id)), + mcp_process.read_stream_until_response_message(RequestId::Number(codex_request_id)), ) .await??; assert_eq!( - JSONRPCResponse { - jsonrpc: JSONRPC_VERSION.into(), - id: RequestId::Integer(codex_request_id), + JsonRpcResponse { + jsonrpc: JsonRpcVersion2_0, + id: RequestId::Number(codex_request_id), result: json!({ "content": [ { @@ -375,11 +372,11 @@ async fn codex_tool_passes_base_instructions() -> anyhow::Result<()> { let codex_response = timeout( DEFAULT_READ_TIMEOUT, - mcp_process.read_stream_until_response_message(RequestId::Integer(codex_request_id)), + mcp_process.read_stream_until_response_message(RequestId::Number(codex_request_id)), ) .await??; - assert_eq!(codex_response.jsonrpc, JSONRPC_VERSION); - assert_eq!(codex_response.id, RequestId::Integer(codex_request_id)); + assert_eq!(codex_response.jsonrpc, JsonRpcVersion2_0); + assert_eq!(codex_response.id, RequestId::Number(codex_request_id)); assert_eq!( codex_response.result, json!({ @@ -430,42 +427,33 @@ async fn codex_tool_passes_base_instructions() -> anyhow::Result<()> { Ok(()) } -fn create_expected_patch_approval_elicitation_request( - elicitation_request_id: RequestId, +fn create_expected_patch_approval_elicitation_request_params( changes: HashMap, grant_root: Option, reason: Option, codex_mcp_tool_call_id: String, codex_event_id: String, thread_id: codex_protocol::ThreadId, -) -> anyhow::Result { +) -> anyhow::Result { let mut message_lines = Vec::new(); if let Some(r) = &reason { message_lines.push(r.clone()); } message_lines.push("Allow Codex to apply proposed code changes?".to_string()); + let params_json = serde_json::to_value(PatchApprovalElicitRequestParams { + message: message_lines.join("\n"), + requested_schema: json!({"type":"object","properties":{}}), + thread_id, + codex_elicitation: "patch-approval".to_string(), + codex_mcp_tool_call_id, + codex_event_id, + codex_reason: reason, + codex_grant_root: grant_root, + codex_changes: changes, + codex_call_id: "call1234".to_string(), + })?; - Ok(JSONRPCRequest { - jsonrpc: JSONRPC_VERSION.into(), - id: elicitation_request_id, - method: ElicitRequest::METHOD.to_string(), - params: Some(serde_json::to_value(&PatchApprovalElicitRequestParams { - message: message_lines.join("\n"), - requested_schema: ElicitRequestParamsRequestedSchema { - r#type: "object".to_string(), - properties: json!({}), - required: None, - }, - thread_id, - codex_elicitation: "patch-approval".to_string(), - codex_mcp_tool_call_id, - codex_event_id, - codex_reason: reason, - codex_grant_root: grant_root, - codex_changes: changes, - codex_call_id: "call1234".to_string(), - })?), - }) + Ok(params_json) } /// This handle is used to ensure that the MockServer and TempDir are not dropped while diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index d859aa945e..8bdd473220 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -54,7 +54,6 @@ dunce = { workspace = true } image = { workspace = true, features = ["jpeg", "png"] } itertools = { workspace = true } lazy_static = { workspace = true } -mcp-types = { workspace = true } pathdiff = { workspace = true } pulldown-cmark = { workspace = true } rand = { workspace = true } @@ -67,6 +66,7 @@ ratatui = { workspace = true, features = [ ratatui-macros = { workspace = true } regex-lite = { workspace = true } reqwest = { version = "0.12", features = ["json"] } +rmcp = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true, features = ["preserve_order"] } shlex = { workspace = true } diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 0f0445fee8..15f252e86f 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -23,11 +23,11 @@ use codex_core::protocol::ExecPolicyAmendment; use codex_core::protocol::FileChange; use codex_core::protocol::Op; use codex_core::protocol::ReviewDecision; +use codex_protocol::mcp::RequestId; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyEventKind; use crossterm::event::KeyModifiers; -use mcp_types::RequestId; use ratatui::buffer::Buffer; use ratatui::layout::Rect; use ratatui::style::Stylize; diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__binary_size_ideal_response.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__binary_size_ideal_response.snap index 77738439a1..38fb05e28d 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__binary_size_ideal_response.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__binary_size_ideal_response.snap @@ -27,7 +27,7 @@ expression: "lines[start_idx..].join(\"\\n\")" exec, linux-sandbox, tui, login, ollama, and mcp. • Ran for d in ansi-escape apply-patch arg0 cli common core exec execpolicy - │ file-search linux-sandbox login mcp-client mcp-server mcp-types ollama + │ file-search linux-sandbox login mcp-client mcp-server ollama │ tui; do echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; │ … +1 lines └ --- ansi-escape/Cargo.toml diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 934c9506f5..74554dda31 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -46,6 +46,8 @@ use codex_core::protocol::McpInvocation; use codex_core::protocol::SessionConfiguredEvent; use codex_core::web_search::web_search_detail; use codex_otel::RuntimeMetricsSummary; +use codex_protocol::mcp::Resource; +use codex_protocol::mcp::ResourceTemplate; use codex_protocol::models::WebSearchAction; use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig; use codex_protocol::plan_tool::PlanItemArg; @@ -54,10 +56,6 @@ use codex_protocol::plan_tool::UpdatePlanArgs; use codex_protocol::user_input::TextElement; use image::DynamicImage; use image::ImageReader; -use mcp_types::EmbeddedResourceResource; -use mcp_types::Resource; -use mcp_types::ResourceLink; -use mcp_types::ResourceTemplate; use ratatui::prelude::*; use ratatui::style::Color; use ratatui::style::Modifier; @@ -1199,7 +1197,7 @@ pub(crate) struct McpToolCallCell { invocation: McpInvocation, start_time: Instant, duration: Option, - result: Option>, + result: Option>, animations_enabled: bool, } @@ -1226,7 +1224,7 @@ impl McpToolCallCell { pub(crate) fn complete( &mut self, duration: Duration, - result: Result, + result: Result, ) -> Option> { let image_cell = try_new_completed_mcp_tool_call_with_image_output(&result) .map(|cell| Box::new(cell) as Box); @@ -1249,23 +1247,32 @@ impl McpToolCallCell { self.result = Some(Err("interrupted".to_string())); } - fn render_content_block(block: &mcp_types::ContentBlock, width: usize) -> String { - match block { - mcp_types::ContentBlock::TextContent(text) => { + fn render_content_block(block: &serde_json::Value, width: usize) -> String { + let content = match serde_json::from_value::(block.clone()) { + Ok(content) => content, + Err(_) => { + return format_and_truncate_tool_result( + &block.to_string(), + TOOL_CALL_MAX_LINES, + width, + ); + } + }; + + match content.raw { + rmcp::model::RawContent::Text(text) => { format_and_truncate_tool_result(&text.text, TOOL_CALL_MAX_LINES, width) } - mcp_types::ContentBlock::ImageContent(_) => "".to_string(), - mcp_types::ContentBlock::AudioContent(_) => "