diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 7dd8eeae6..05c300dc8 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -4997,14 +4997,13 @@ async fn built_tools( skills_outcome: Option<&SkillLoadOutcome>, cancellation_token: &CancellationToken, ) -> CodexResult> { - let mut mcp_tools = sess - .services - .mcp_connection_manager - .read() - .await + let mcp_connection_manager = sess.services.mcp_connection_manager.read().await; + let has_mcp_servers = mcp_connection_manager.has_servers(); + let mut mcp_tools = mcp_connection_manager .list_all_tools() .or_cancel(cancellation_token) .await?; + drop(mcp_connection_manager); let mut effective_explicitly_enabled_connectors = explicitly_enabled_connectors.clone(); effective_explicitly_enabled_connectors.extend(sess.get_connector_selection().await); @@ -5050,12 +5049,12 @@ async fn built_tools( Ok(Arc::new(ToolRouter::from_config( &turn_context.tools_config, - Some( + has_mcp_servers.then(|| { mcp_tools .into_iter() .map(|(name, tool)| (name, tool.tool)) - .collect(), - ), + .collect() + }), app_tools, turn_context.dynamic_tools.as_slice(), ))) diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index 5a3c4d6e5..589be7aac 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -353,6 +353,10 @@ pub(crate) struct McpConnectionManager { } impl McpConnectionManager { + pub(crate) fn has_servers(&self) -> bool { + !self.clients.is_empty() + } + pub async fn initialize( &mut self, mcp_servers: &HashMap, diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 817b7d787..16e902304 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -1478,12 +1478,14 @@ pub(crate) fn build_specs( builder.register_handler("shell_command", shell_command_handler); } - builder.push_spec_with_parallel_support(create_list_mcp_resources_tool(), true); - builder.push_spec_with_parallel_support(create_list_mcp_resource_templates_tool(), true); - builder.push_spec_with_parallel_support(create_read_mcp_resource_tool(), true); - builder.register_handler("list_mcp_resources", mcp_resource_handler.clone()); - builder.register_handler("list_mcp_resource_templates", mcp_resource_handler.clone()); - builder.register_handler("read_mcp_resource", mcp_resource_handler); + if mcp_tools.is_some() { + builder.push_spec_with_parallel_support(create_list_mcp_resources_tool(), true); + builder.push_spec_with_parallel_support(create_list_mcp_resource_templates_tool(), true); + builder.push_spec_with_parallel_support(create_read_mcp_resource_tool(), true); + builder.register_handler("list_mcp_resources", mcp_resource_handler.clone()); + builder.register_handler("list_mcp_resource_templates", mcp_resource_handler.clone()); + builder.register_handler("read_mcp_resource", mcp_resource_handler); + } builder.push_spec(PLAN_TOOL.clone()); builder.register_handler("update_plan", plan_handler); @@ -1832,9 +1834,6 @@ mod tests { for spec in [ create_exec_command_tool(true), create_write_stdin_tool(), - create_list_mcp_resources_tool(), - create_list_mcp_resource_templates_tool(), - create_read_mcp_resource_tool(), PLAN_TOOL.clone(), create_request_user_input_tool(), create_apply_patch_freeform_tool(), @@ -2039,7 +2038,7 @@ mod tests { features, web_search_mode, }); - let (tools, _) = build_specs(&tools_config, Some(HashMap::new()), None, &[]).build(); + let (tools, _) = build_specs(&tools_config, None, None, &[]).build(); let tool_names = tools.iter().map(|t| t.spec.name()).collect::>(); assert_eq!(&tool_names, &expected_tools,); } @@ -2106,6 +2105,53 @@ mod tests { ); } + #[test] + fn mcp_resource_tools_are_hidden_without_mcp_servers() { + let config = test_config(); + let model_info = + ModelsManager::construct_model_info_offline_for_tests("gpt-5-codex", &config); + let mut features = Features::with_defaults(); + features.enable(Feature::CollaborationModes); + let tools_config = ToolsConfig::new(&ToolsConfigParams { + model_info: &model_info, + features: &features, + web_search_mode: Some(WebSearchMode::Cached), + }); + let (tools, _) = build_specs(&tools_config, None, None, &[]).build(); + + assert!( + !tools.iter().any(|tool| matches!( + tool.spec.name(), + "list_mcp_resources" | "list_mcp_resource_templates" | "read_mcp_resource" + )), + "MCP resource tools should be omitted when no MCP servers are configured" + ); + } + + #[test] + fn mcp_resource_tools_are_included_when_mcp_servers_are_present() { + let config = test_config(); + let model_info = + ModelsManager::construct_model_info_offline_for_tests("gpt-5-codex", &config); + let mut features = Features::with_defaults(); + features.enable(Feature::CollaborationModes); + let tools_config = ToolsConfig::new(&ToolsConfigParams { + model_info: &model_info, + features: &features, + web_search_mode: Some(WebSearchMode::Cached), + }); + let (tools, _) = build_specs(&tools_config, Some(HashMap::new()), None, &[]).build(); + + assert_contains_tool_names( + &tools, + &[ + "list_mcp_resources", + "list_mcp_resource_templates", + "read_mcp_resource", + ], + ); + } + #[test] fn test_build_specs_gpt5_codex_default() { let mut features = Features::with_defaults(); @@ -2116,9 +2162,6 @@ mod tests { Some(WebSearchMode::Cached), "shell_command", &[ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", @@ -2138,9 +2181,6 @@ mod tests { Some(WebSearchMode::Cached), "shell_command", &[ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", @@ -2162,9 +2202,6 @@ mod tests { &[ "exec_command", "write_stdin", - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", @@ -2186,9 +2223,6 @@ mod tests { &[ "exec_command", "write_stdin", - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", @@ -2208,9 +2242,6 @@ mod tests { Some(WebSearchMode::Cached), "shell_command", &[ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", @@ -2230,9 +2261,6 @@ mod tests { Some(WebSearchMode::Cached), "shell_command", &[ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", @@ -2252,9 +2280,6 @@ mod tests { Some(WebSearchMode::Cached), "shell", &[ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "web_search", @@ -2273,9 +2298,6 @@ mod tests { Some(WebSearchMode::Cached), "shell_command", &[ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", @@ -2297,9 +2319,6 @@ mod tests { &[ "exec_command", "write_stdin", - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", diff --git a/codex-rs/core/tests/suite/model_tools.rs b/codex-rs/core/tests/suite/model_tools.rs index 676b34eb7..836807881 100644 --- a/codex-rs/core/tests/suite/model_tools.rs +++ b/codex-rs/core/tests/suite/model_tools.rs @@ -75,9 +75,6 @@ async fn model_selects_expected_tools() { expected_default_tools( "shell_command", &[ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", @@ -94,9 +91,6 @@ async fn model_selects_expected_tools() { expected_default_tools( "shell_command", &[ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", @@ -113,9 +107,6 @@ async fn model_selects_expected_tools() { expected_default_tools( "shell_command", &[ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", @@ -132,9 +123,6 @@ async fn model_selects_expected_tools() { expected_default_tools( "shell", &[ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "web_search", @@ -150,9 +138,6 @@ async fn model_selects_expected_tools() { expected_default_tools( "shell_command", &[ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", diff --git a/codex-rs/core/tests/suite/prompt_caching.rs b/codex-rs/core/tests/suite/prompt_caching.rs index 97fe9b416..71a565540 100644 --- a/codex-rs/core/tests/suite/prompt_caching.rs +++ b/codex-rs/core/tests/suite/prompt_caching.rs @@ -144,9 +144,6 @@ async fn prompt_tools_are_consistent_across_requests() -> anyhow::Result<()> { vec!["exec_command", "write_stdin"] }; expected_tools_names.extend([ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch",