From 443f6b831e47a91e17a824beea493854f4df269c Mon Sep 17 00:00:00 2001 From: Abhinav Date: Fri, 1 May 2026 14:16:22 -0700 Subject: [PATCH] Use the 2025-06-18 elicitation capability shape (#20562) # Why Codex currently negotiates MCP `2025-06-18`, where the client elicitation capability is represented as an empty object. We were still serializing `capabilities.elicitation.form`, which belongs to the later capability shape and can cause strict `2025-06-18` servers to reject `initialize` with an unrecognized-field error. This keeps the handshake aligned with the protocol version Codex actually negotiates and fixes the compatibility regression tracked in #17492. # What - Serialize the client elicitation capability as `elicitation: {}` for `2025-06-18`. - Keep elicitation advertised for both Codex Apps and custom MCP servers. - Tighten regression coverage so the unit test asserts both the Rust value and the serialized wire shape. - Add an app-server integration test that round-trips a form elicitation from a custom MCP server; the existing connector round-trip continues to cover the connector path. # Verification - `cargo test -p codex-mcp` - `cargo test -p codex-app-server mcp_server_elicitation_round_trip` - `cargo test -p codex-app-server mcp_server_tool_call_round_trips_elicitation` # Next steps - Decide whether `tool_call_mcp_elicitation=false` should also suppress capability advertisement during `initialize`. - Revisit `form` / `url` capability advertisement when Codex is ready to negotiate MCP `2025-11-25`, which defines that newer shape. --- .../app-server/tests/suite/v2/mcp_tool.rs | 153 ++++++++++++++++++ .../codex-mcp/src/connection_manager_tests.rs | 17 +- codex-rs/codex-mcp/src/rmcp_client.rs | 8 +- 3 files changed, 160 insertions(+), 18 deletions(-) diff --git a/codex-rs/app-server/tests/suite/v2/mcp_tool.rs b/codex-rs/app-server/tests/suite/v2/mcp_tool.rs index 03f3db95f..141761e88 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_tool.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_tool.rs @@ -13,10 +13,16 @@ use axum::Router; use codex_app_server_protocol::ItemCompletedNotification; use codex_app_server_protocol::JSONRPCError; use codex_app_server_protocol::JSONRPCResponse; +use codex_app_server_protocol::McpElicitationSchema; +use codex_app_server_protocol::McpServerElicitationAction; +use codex_app_server_protocol::McpServerElicitationRequest; +use codex_app_server_protocol::McpServerElicitationRequestParams; +use codex_app_server_protocol::McpServerElicitationRequestResponse; use codex_app_server_protocol::McpServerToolCallParams; use codex_app_server_protocol::McpServerToolCallResponse; use codex_app_server_protocol::McpToolCallStatus; use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::ServerRequest; use codex_app_server_protocol::ThreadItem; use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; @@ -27,12 +33,17 @@ use codex_utils_pty::DEFAULT_OUTPUT_BYTES_CAP; use core_test_support::responses; use pretty_assertions::assert_eq; use rmcp::handler::server::ServerHandler; +use rmcp::model::BooleanSchema; use rmcp::model::CallToolRequestParams; use rmcp::model::CallToolResult; use rmcp::model::Content; +use rmcp::model::CreateElicitationRequestParams; +use rmcp::model::ElicitationAction; +use rmcp::model::ElicitationSchema; use rmcp::model::JsonObject; use rmcp::model::ListToolsResult; use rmcp::model::Meta; +use rmcp::model::PrimitiveSchema; use rmcp::model::ServerCapabilities; use rmcp::model::ServerInfo; use rmcp::model::Tool; @@ -52,6 +63,8 @@ const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(10); const TEST_SERVER_NAME: &str = "tool_server"; const TEST_TOOL_NAME: &str = "echo_tool"; const LARGE_RESPONSE_MESSAGE: &str = "large"; +const ELICITATION_TRIGGER_MESSAGE: &str = "confirm"; +const ELICITATION_MESSAGE: &str = "Allow this request?"; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn mcp_server_tool_call_returns_tool_result() -> Result<()> { @@ -171,6 +184,116 @@ async fn mcp_server_tool_call_returns_error_for_unknown_thread() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn mcp_server_tool_call_round_trips_elicitation() -> Result<()> { + let responses_server = responses::start_mock_server().await; + let (mcp_server_url, mcp_server_handle) = start_mcp_server().await?; + let codex_home = TempDir::new()?; + write_mock_responses_config_toml( + codex_home.path(), + &responses_server.uri(), + &BTreeMap::new(), + /*auto_compact_limit*/ 1024, + /*requires_openai_auth*/ None, + "mock_provider", + "compact", + )?; + + let config_path = codex_home.path().join("config.toml"); + let mut config_toml = std::fs::read_to_string(&config_path)?; + config_toml.push_str(&format!( + r#" +[mcp_servers.{TEST_SERVER_NAME}] +url = "{mcp_server_url}/mcp" +"# + )); + std::fs::write(config_path, config_toml)?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let thread_start_id = mcp + .send_thread_start_request(ThreadStartParams { + model: Some("mock-model".to_string()), + approval_policy: Some(codex_app_server_protocol::AskForApproval::UnlessTrusted), + ..Default::default() + }) + .await?; + let thread_start_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(thread_start_id)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response(thread_start_resp)?; + + let tool_call_request_id = mcp + .send_mcp_server_tool_call_request(McpServerToolCallParams { + thread_id: thread.id.clone(), + server: TEST_SERVER_NAME.to_string(), + tool: TEST_TOOL_NAME.to_string(), + arguments: Some(json!({ + "message": ELICITATION_TRIGGER_MESSAGE, + })), + meta: None, + }) + .await?; + + let server_req = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_request_message(), + ) + .await??; + let ServerRequest::McpServerElicitationRequest { request_id, params } = server_req else { + panic!("expected McpServerElicitationRequest request, got: {server_req:?}"); + }; + let requested_schema: McpElicitationSchema = serde_json::from_value(serde_json::to_value( + ElicitationSchema::builder() + .required_property("confirmed", PrimitiveSchema::Boolean(BooleanSchema::new())) + .build() + .map_err(anyhow::Error::msg)?, + )?)?; + assert_eq!( + params, + McpServerElicitationRequestParams { + thread_id: thread.id, + turn_id: None, + server_name: TEST_SERVER_NAME.to_string(), + request: McpServerElicitationRequest::Form { + meta: None, + message: ELICITATION_MESSAGE.to_string(), + requested_schema, + }, + } + ); + + mcp.send_response( + request_id, + serde_json::to_value(McpServerElicitationRequestResponse { + action: McpServerElicitationAction::Accept, + content: Some(json!({ + "confirmed": true, + })), + meta: None, + })?, + ) + .await?; + + let tool_call_response: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(tool_call_request_id)), + ) + .await??; + let response: McpServerToolCallResponse = to_response(tool_call_response)?; + assert_eq!(response.content.len(), 1); + assert_eq!(response.content[0].get("type"), Some(&json!("text"))); + assert_eq!(response.content[0].get("text"), Some(&json!("accepted"))); + + mcp_server_handle.abort(); + let _ = mcp_server_handle.await; + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn mcp_tool_call_completion_notification_contains_truncated_large_result() -> Result<()> { let call_id = "call-large-mcp"; @@ -375,6 +498,36 @@ impl ServerHandler for ToolAppsMcpServer { return Ok(result); } + if message == ELICITATION_TRIGGER_MESSAGE { + let requested_schema = ElicitationSchema::builder() + .required_property("confirmed", PrimitiveSchema::Boolean(BooleanSchema::new())) + .build() + .map_err(|err| rmcp::ErrorData::internal_error(err.to_string(), None))?; + let result = context + .peer + .create_elicitation(CreateElicitationRequestParams::FormElicitationParams { + meta: None, + message: ELICITATION_MESSAGE.to_string(), + requested_schema, + }) + .await + .map_err(|err| rmcp::ErrorData::internal_error(err.to_string(), None))?; + let output = match result.action { + ElicitationAction::Accept => { + assert_eq!( + result.content, + Some(json!({ + "confirmed": true, + })) + ); + "accepted" + } + ElicitationAction::Decline => "declined", + ElicitationAction::Cancel => "cancelled", + }; + return Ok(CallToolResult::success(vec![Content::text(output)])); + } + let mut result = CallToolResult::structured(json!({ "echoed": message, "threadId": thread_id, diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index 3fcef0c06..01b1161b7 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -26,7 +26,6 @@ use pretty_assertions::assert_eq; use rmcp::model::CreateElicitationRequestParams; use rmcp::model::ElicitationAction; use rmcp::model::ElicitationCapability; -use rmcp::model::FormElicitationCapability; use rmcp::model::JsonObject; use rmcp::model::Meta; use rmcp::model::NumberOrString; @@ -801,18 +800,14 @@ async fn list_all_tools_uses_startup_snapshot_when_client_startup_fails() { } #[test] -fn elicitation_capability_enabled_for_custom_servers() { +fn elicitation_capability_uses_2025_06_18_shape_for_all_servers() { for server_name in [CODEX_APPS_MCP_SERVER_NAME, "custom_mcp"] { let capability = elicitation_capability_for_server(server_name); - assert!(matches!( - capability, - Some(ElicitationCapability { - form: Some(FormElicitationCapability { - schema_validation: None - }), - url: None, - }) - )); + assert_eq!(capability, Some(ElicitationCapability::default())); + assert_eq!( + serde_json::to_value(capability).expect("serialize elicitation capability"), + serde_json::json!({}) + ); } } diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index b88942c4e..38ef96919 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -55,7 +55,6 @@ use futures::future::FutureExt; use futures::future::Shared; use rmcp::model::ClientCapabilities; use rmcp::model::ElicitationCapability; -use rmcp::model::FormElicitationCapability; use rmcp::model::Implementation; use rmcp::model::InitializeRequestParams; use rmcp::model::ProtocolVersion; @@ -323,12 +322,7 @@ pub(crate) fn elicitation_capability_for_server( ) -> Option { // https://modelcontextprotocol.io/specification/2025-06-18/client/elicitation#capabilities // indicates this should be an empty object. - Some(ElicitationCapability { - form: Some(FormElicitationCapability { - schema_validation: None, - }), - url: None, - }) + Some(ElicitationCapability::default()) } pub(crate) async fn list_tools_for_client_uncached(