diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 8419848ec..f78f27ffa 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -4275,6 +4275,7 @@ dependencies = [ "reqwest", "serde_json", "shlex", + "similar", "tempfile", "tokio", "tokio-tungstenite", diff --git a/codex-rs/codex-mcp/src/lib.rs b/codex-rs/codex-mcp/src/lib.rs index 54631cd1c..2080a5c31 100644 --- a/codex-rs/codex-mcp/src/lib.rs +++ b/codex-rs/codex-mcp/src/lib.rs @@ -32,7 +32,6 @@ pub use mcp::with_codex_apps_mcp; pub use mcp::McpServerStatusSnapshot; pub use mcp::McpSnapshotDetail; pub use mcp::collect_mcp_server_status_snapshot_with_detail; -pub use mcp::collect_mcp_snapshot_from_manager; pub use mcp::read_mcp_resource; pub use mcp::McpAuthStatusEntry; diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index 958677b9e..03a5d1a06 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -31,7 +31,6 @@ use codex_protocol::mcp::Tool; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::McpAuthStatus; -use codex_protocol::protocol::McpListToolsResponseEvent; use rmcp::model::ReadResourceRequestParams; use rmcp::model::ReadResourceResult; use serde_json::Value; @@ -347,18 +346,6 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( snapshot } -pub async fn collect_mcp_snapshot_from_manager( - mcp_connection_manager: &McpConnectionManager, - auth_status_entries: HashMap, -) -> McpListToolsResponseEvent { - collect_mcp_snapshot_from_manager_with_detail( - mcp_connection_manager, - auth_status_entries, - McpSnapshotDetail::Full, - ) - .await -} - pub(crate) fn codex_apps_mcp_url(config: &McpConfig) -> String { codex_apps_mcp_url_for_base_url( &config.chatgpt_base_url, @@ -595,44 +582,6 @@ async fn collect_mcp_server_status_snapshot_from_manager( } } -async fn collect_mcp_snapshot_from_manager_with_detail( - mcp_connection_manager: &McpConnectionManager, - auth_status_entries: HashMap, - detail: McpSnapshotDetail, -) -> McpListToolsResponseEvent { - let (tools, resources, resource_templates) = tokio::join!( - mcp_connection_manager.list_all_tools(), - async { - if detail.include_resources() { - mcp_connection_manager.list_all_resources().await - } else { - HashMap::new() - } - }, - async { - if detail.include_resources() { - mcp_connection_manager.list_all_resource_templates().await - } else { - HashMap::new() - } - }, - ); - - let tools = tools - .into_iter() - .filter_map(|(name, tool)| { - protocol_tool_from_rmcp_tool(&name, &tool.tool).map(|tool| (name, tool)) - }) - .collect::>(); - - McpListToolsResponseEvent { - tools, - resources: convert_mcp_resources(resources), - resource_templates: convert_mcp_resource_templates(resource_templates), - auth_statuses: auth_statuses_from_entries(&auth_status_entries), - } -} - #[cfg(test)] #[path = "mod_tests.rs"] mod tests; diff --git a/codex-rs/core/src/session/handlers.rs b/codex-rs/core/src/session/handlers.rs index b57798e5c..1dc68a69f 100644 --- a/codex-rs/core/src/session/handlers.rs +++ b/codex-rs/core/src/session/handlers.rs @@ -24,8 +24,6 @@ use crate::tasks::CompactTask; use crate::tasks::UserShellCommandMode; use crate::tasks::UserShellCommandTask; use crate::tasks::execute_user_shell_command; -use codex_mcp::collect_mcp_snapshot_from_manager; -use codex_mcp::compute_auth_statuses; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseInputItem; use codex_protocol::protocol::CodexErrorInfo; @@ -469,35 +467,6 @@ pub async fn reload_user_config(sess: &Arc) { sess.reload_user_config_layer().await; } -#[expect( - clippy::await_holding_invalid_type, - reason = "MCP tool listing reads through the session-owned manager guard" -)] -pub async fn list_mcp_tools(sess: &Session, config: &Arc, sub_id: String) { - let mcp_connection_manager = sess.services.mcp_connection_manager.read().await; - let auth = sess.services.auth_manager.auth().await; - let mcp_servers = sess - .services - .mcp_manager - .effective_servers(config, auth.as_ref()) - .await; - let snapshot = collect_mcp_snapshot_from_manager( - &mcp_connection_manager, - compute_auth_statuses( - mcp_servers.iter(), - config.mcp_oauth_credentials_store_mode, - auth.as_ref(), - ) - .await, - ) - .await; - let event = Event { - id: sub_id, - msg: EventMsg::McpListToolsResponse(snapshot), - }; - sess.send_event_raw(event).await; -} - pub async fn compact(sess: &Arc, sub_id: String) { let turn_context = sess.new_default_turn_with_sub_id(sub_id).await; @@ -863,10 +832,6 @@ pub(super) async fn submission_loop( dynamic_tool_response(&sess, id, response).await; false } - Op::ListMcpTools => { - list_mcp_tools(&sess, &config, sub.id.clone()).await; - false - } Op::RefreshMcpServers { config } => { refresh_mcp_servers(&sess, config).await; false diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index fb2d9caeb..7aa64aae5 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -1509,7 +1509,6 @@ pub(super) fn realtime_text_for_event(msg: &EventMsg) -> Option { | EventMsg::DeprecationNotice(_) | EventMsg::StreamError(_) | EventMsg::TurnDiff(_) - | EventMsg::McpListToolsResponse(_) | EventMsg::RealtimeConversationListVoicesResponse(_) | EventMsg::SkillsUpdateAvailable | EventMsg::PlanUpdate(_) diff --git a/codex-rs/core/tests/common/Cargo.toml b/codex-rs/core/tests/common/Cargo.toml index c59edf57a..8c1a79f02 100644 --- a/codex-rs/core/tests/common/Cargo.toml +++ b/codex-rs/core/tests/common/Cargo.toml @@ -33,6 +33,7 @@ opentelemetry = { workspace = true } opentelemetry_sdk = { workspace = true } regex-lite = { workspace = true } serde_json = { workspace = true } +similar = { workspace = true } tempfile = { workspace = true } tokio = { workspace = true, features = ["net", "time"] } tokio-tungstenite = { workspace = true } diff --git a/codex-rs/core/tests/suite/pending_input.rs b/codex-rs/core/tests/suite/pending_input.rs index b582a2abc..62851c515 100644 --- a/codex-rs/core/tests/suite/pending_input.rs +++ b/codex-rs/core/tests/suite/pending_input.rs @@ -164,11 +164,11 @@ async fn submit_queue_only_agent_mail(codex: &CodexThread, text: &str) { .await .unwrap_or_else(|err| panic!("submit queue-only agent mail: {err}")); codex - .submit(Op::ListMcpTools) + .submit(Op::RealtimeConversationListVoices) .await - .unwrap_or_else(|err| panic!("submit list-mcp-tools barrier: {err}")); + .unwrap_or_else(|err| panic!("submit list-voices barrier: {err}")); wait_for_event(codex, |event| { - matches!(event, EventMsg::McpListToolsResponse(_)) + matches!(event, EventMsg::RealtimeConversationListVoicesResponse(_)) }) .await; } diff --git a/codex-rs/core/tests/suite/plugins.rs b/codex-rs/core/tests/suite/plugins.rs index df042029a..7b10db025 100644 --- a/codex-rs/core/tests/suite/plugins.rs +++ b/codex-rs/core/tests/suite/plugins.rs @@ -99,20 +99,6 @@ fn write_plugin_app_plugin(home: &TempDir) { .expect("write plugin app config"); } -async fn build_plugin_test_codex( - server: &MockServer, - codex_home: Arc, -) -> Result> { - let mut builder = test_codex() - .with_home(codex_home) - .with_auth(CodexAuth::from_api_key("Test API Key")); - Ok(builder - .build(server) - .await - .expect("create new conversation") - .codex) -} - async fn build_analytics_plugin_test_codex( server: &MockServer, codex_home: Arc, @@ -448,34 +434,3 @@ async fn explicit_plugin_mentions_track_plugin_used_analytics() -> Result<()> { Ok(()) } - -#[tokio::test(flavor = "multi_thread", worker_threads = 1)] -async fn plugin_mcp_tools_are_listed() -> Result<()> { - skip_if_no_network!(Ok(())); - let server = start_mock_server().await; - let codex_home = Arc::new(TempDir::new()?); - let rmcp_test_server_bin = stdio_server_bin()?; - write_plugin_mcp_plugin(codex_home.as_ref(), &rmcp_test_server_bin); - let codex = build_plugin_test_codex(&server, codex_home).await?; - wait_for_sample_mcp_ready(&codex).await?; - - codex.submit(Op::ListMcpTools).await?; - let list_event = wait_for_event_with_timeout( - &codex, - |ev| matches!(ev, EventMsg::McpListToolsResponse(_)), - Duration::from_secs(10), - ) - .await; - let EventMsg::McpListToolsResponse(tool_list) = list_event else { - unreachable!("event guard guarantees McpListToolsResponse"); - }; - let mut available_tools: Vec<&str> = tool_list.tools.keys().map(String::as_str).collect(); - available_tools.sort_unstable(); - assert!( - tool_list.tools.contains_key("mcp__sample__echo") - && tool_list.tools.contains_key("mcp__sample__image"), - "expected plugin MCP tools to be listed; discovered tools: {available_tools:?}" - ); - - Ok(()) -} diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index 8b7b4211c..1a7062bdb 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -242,31 +242,42 @@ fn copy_binary_to_remote_env( Ok(remote_path) } -async fn wait_for_mcp_tool(fixture: &TestCodex, tool_name: &str) -> anyhow::Result<()> { - let tools_ready_deadline = Instant::now() + Duration::from_secs(30); - loop { - fixture.codex.submit(Op::ListMcpTools).await?; - let list_event = wait_for_event_with_timeout( - &fixture.codex, - |ev| matches!(ev, EventMsg::McpListToolsResponse(_)), - Duration::from_secs(10), - ) - .await; - let EventMsg::McpListToolsResponse(tool_list) = list_event else { - unreachable!("event guard guarantees McpListToolsResponse"); - }; - if tool_list.tools.contains_key(tool_name) { - return Ok(()); - } - - let available_tools: Vec<&str> = tool_list.tools.keys().map(String::as_str).collect(); - if Instant::now() >= tools_ready_deadline { - panic!( - "timed out waiting for MCP tool {tool_name} to become available; discovered tools: {available_tools:?}" - ); - } - sleep(Duration::from_millis(200)).await; +async fn wait_for_mcp_server(fixture: &TestCodex, server_name: &str) -> anyhow::Result<()> { + let startup_event = wait_for_event_with_timeout( + &fixture.codex, + |ev| match ev { + EventMsg::McpStartupComplete(summary) => { + summary.ready.iter().any(|server| server == server_name) + || summary + .failed + .iter() + .any(|failure| failure.server == server_name) + || summary.cancelled.iter().any(|server| server == server_name) + } + _ => false, + }, + Duration::from_secs(70), + ) + .await; + let EventMsg::McpStartupComplete(summary) = startup_event else { + unreachable!("event guard guarantees McpStartupComplete"); + }; + if let Some(failure) = summary + .failed + .iter() + .find(|failure| failure.server == server_name) + { + let error = &failure.error; + anyhow::bail!("MCP server {server_name} failed to start: {error}"); } + if summary.cancelled.iter().any(|server| server == server_name) { + anyhow::bail!("MCP server {server_name} startup was cancelled"); + } + ensure!( + summary.ready.iter().any(|server| server == server_name), + "expected MCP server {server_name} to be ready; startup summary: {summary:?}" + ); + Ok(()) } #[derive(Default)] @@ -731,7 +742,6 @@ async fn stdio_mcp_tool_call_includes_sandbox_state_meta() -> anyhow::Result<()> let call_id = "sandbox-meta-call"; let server_name = "rmcp"; let namespace = format!("mcp__{server_name}__"); - let tool_name = format!("{namespace}sandbox_meta"); let call_mock = mount_sse_once( &server, @@ -767,30 +777,7 @@ async fn stdio_mcp_tool_call_includes_sandbox_state_meta() -> anyhow::Result<()> .build_remote_aware(&server) .await?; - let tools_ready_deadline = Instant::now() + Duration::from_secs(30); - loop { - fixture.codex.submit(Op::ListMcpTools).await?; - let list_event = wait_for_event_with_timeout( - &fixture.codex, - |ev| matches!(ev, EventMsg::McpListToolsResponse(_)), - Duration::from_secs(10), - ) - .await; - let EventMsg::McpListToolsResponse(tool_list) = list_event else { - unreachable!("event guard guarantees McpListToolsResponse"); - }; - if tool_list.tools.contains_key(&tool_name) { - break; - } - - let available_tools: Vec<&str> = tool_list.tools.keys().map(String::as_str).collect(); - if Instant::now() >= tools_ready_deadline { - panic!( - "timed out waiting for MCP tool {tool_name} to become available; discovered tools: {available_tools:?}" - ); - } - sleep(Duration::from_millis(200)).await; - } + wait_for_mcp_server(&fixture, server_name).await?; fixture .submit_turn_with_permission_profile( @@ -1039,7 +1026,6 @@ async fn stdio_image_responses_round_trip() -> anyhow::Result<()> { let call_id = "img-1"; let server_name = "rmcp"; - let tool_name = format!("mcp__{server_name}__image"); let namespace = format!("mcp__{server_name}__"); // First stream: model decides to call the image tool. @@ -1086,7 +1072,7 @@ async fn stdio_image_responses_round_trip() -> anyhow::Result<()> { }) .build_remote_aware(&server) .await?; - wait_for_mcp_tool(&fixture, &tool_name).await?; + wait_for_mcp_server(&fixture, server_name).await?; fixture .codex @@ -1176,7 +1162,6 @@ async fn stdio_image_responses_preserve_original_detail_metadata() -> anyhow::Re let call_id = "img-original-detail-1"; let server_name = "rmcp"; - let tool_name = format!("mcp__{server_name}__image_scenario"); let namespace = format!("mcp__{server_name}__"); mount_sse_once( @@ -1219,7 +1204,7 @@ async fn stdio_image_responses_preserve_original_detail_metadata() -> anyhow::Re }) .build_remote_aware(&server) .await?; - wait_for_mcp_tool(&fixture, &tool_name).await?; + wait_for_mcp_server(&fixture, server_name).await?; fixture .codex @@ -1955,7 +1940,6 @@ async fn streamable_http_with_oauth_round_trip_impl() -> anyhow::Result<()> { let call_id = "call-789"; let server_name = "rmcp_http_oauth"; - let tool_name = format!("mcp__{server_name}__echo"); let namespace = format!("mcp__{server_name}__"); mount_sse_once( @@ -2036,9 +2020,9 @@ async fn streamable_http_with_oauth_round_trip_impl() -> anyhow::Result<()> { }) .build_remote_aware(&server) .await?; - // Phase 5: wait for MCP discovery to publish the expected tool before the - // turn is submitted, which keeps failures tied to server startup/discovery. - wait_for_mcp_tool(&fixture, &tool_name).await?; + // Phase 5: wait for MCP startup before the turn is submitted, which keeps + // failures tied to server startup/discovery. + wait_for_mcp_server(&fixture, server_name).await?; // Phase 6: submit the user turn that should invoke the OAuth-backed tool. fixture diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index 8917ebf82..b462022db 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -334,7 +334,6 @@ async fn run_codex_tool_session_inner( | EventMsg::AgentReasoningSectionBreak(_) | EventMsg::McpToolCallBegin(_) | EventMsg::McpToolCallEnd(_) - | EventMsg::McpListToolsResponse(_) | EventMsg::RealtimeConversationListVoicesResponse(_) | EventMsg::ExecCommandBegin(_) | EventMsg::TerminalInteraction(_) diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index efa4554eb..4eb1b8804 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -30,9 +30,6 @@ use crate::dynamic_tools::DynamicToolSpec; use crate::items::TurnItem; use crate::mcp::CallToolResult; use crate::mcp::RequestId; -use crate::mcp::Resource as McpResource; -use crate::mcp::ResourceTemplate as McpResourceTemplate; -use crate::mcp::Tool as McpTool; use crate::memory_citation::MemoryCitation; use crate::models::ActivePermissionProfile; use crate::models::BaseInstructions; @@ -722,10 +719,6 @@ pub enum Op { response: DynamicToolResponse, }, - /// Request the list of MCP tools available across all configured servers. - /// Reply is delivered via `EventMsg::McpListToolsResponse`. - ListMcpTools, - /// Request MCP servers to reinitialize and refresh cached tool lists. RefreshMcpServers { config: McpServerRefreshConfig }, @@ -862,7 +855,6 @@ impl Op { Self::UserInputAnswer { .. } => "user_input_answer", Self::RequestPermissionsResponse { .. } => "request_permissions_response", Self::DynamicToolResponse { .. } => "dynamic_tool_response", - Self::ListMcpTools => "list_mcp_tools", Self::RefreshMcpServers { .. } => "refresh_mcp_servers", Self::ReloadUserConfig => "reload_user_config", Self::Compact => "compact", @@ -1406,9 +1398,6 @@ pub enum EventMsg { TurnDiff(TurnDiffEvent), - /// List of MCP tools available to the agent. - McpListToolsResponse(McpListToolsResponseEvent), - /// List of voices supported by realtime conversation streams. RealtimeConversationListVoicesResponse(RealtimeConversationListVoicesResponseEvent), @@ -3247,18 +3236,6 @@ pub struct TurnDiffEvent { pub unified_diff: String, } -#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] -pub struct McpListToolsResponseEvent { - /// Fully qualified tool name -> tool definition. - pub tools: std::collections::HashMap, - /// Known resources grouped by server name. - pub resources: std::collections::HashMap>, - /// Known resource templates grouped by server name. - pub resource_templates: std::collections::HashMap>, - /// Authentication status for each configured MCP server. - pub auth_statuses: std::collections::HashMap, -} - #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] pub struct McpStartupUpdateEvent { /// Server name being started. diff --git a/codex-rs/rollout-trace/src/protocol_event.rs b/codex-rs/rollout-trace/src/protocol_event.rs index 91ee3f09a..3d52798b8 100644 --- a/codex-rs/rollout-trace/src/protocol_event.rs +++ b/codex-rs/rollout-trace/src/protocol_event.rs @@ -259,7 +259,6 @@ pub(crate) fn tool_runtime_trace_event(event: &EventMsg) -> Option Option<&'static s | EventMsg::PatchApplyUpdated(_) | EventMsg::PatchApplyEnd(_) | EventMsg::TurnDiff(_) - | EventMsg::McpListToolsResponse(_) | EventMsg::RealtimeConversationListVoicesResponse(_) | EventMsg::SkillsUpdateAvailable | EventMsg::PlanUpdate(_) diff --git a/codex-rs/rollout/src/policy.rs b/codex-rs/rollout/src/policy.rs index 39c865a83..558c3fef9 100644 --- a/codex-rs/rollout/src/policy.rs +++ b/codex-rs/rollout/src/policy.rs @@ -154,7 +154,6 @@ fn event_msg_persistence_mode(ev: &EventMsg) -> Option { | EventMsg::PatchApplyBegin(_) | EventMsg::PatchApplyUpdated(_) | EventMsg::TurnDiff(_) - | EventMsg::McpListToolsResponse(_) | EventMsg::RealtimeConversationListVoicesResponse(_) | EventMsg::McpStartupUpdate(_) | EventMsg::McpStartupComplete(_)