From 258951b6ae3b10a0e5664781e068ec4c68536198 Mon Sep 17 00:00:00 2001 From: Sayan Sisodiya Date: Mon, 27 Apr 2026 14:20:52 -0700 Subject: [PATCH] rm McpToolExposure wrapper --- codex-rs/core/src/mcp_tool_exposure.rs | 34 +++++++++----------- codex-rs/core/src/mcp_tool_exposure_test.rs | 35 ++++++++++++--------- codex-rs/core/src/session/turn.rs | 4 +-- 3 files changed, 37 insertions(+), 36 deletions(-) diff --git a/codex-rs/core/src/mcp_tool_exposure.rs b/codex-rs/core/src/mcp_tool_exposure.rs index f65d68d994..71d27591fb 100644 --- a/codex-rs/core/src/mcp_tool_exposure.rs +++ b/codex-rs/core/src/mcp_tool_exposure.rs @@ -13,17 +13,13 @@ use crate::tools::mcp_tool_input::McpToolInput; pub(crate) const DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD: usize = 100; -pub(crate) struct McpToolExposure { - pub(crate) tools: HashMap, -} - pub(crate) fn build_mcp_tool_exposure( all_mcp_tools: &HashMap, connectors: Option<&[connectors::AppInfo]>, explicitly_enabled_connectors: &[connectors::AppInfo], config: &Config, tools_config: &ToolsConfig, -) -> McpToolExposure { +) -> HashMap { let mut candidate_tools = filter_non_codex_apps_mcp_tools_only(all_mcp_tools); if let Some(connectors) = connectors { candidate_tools.extend(filter_codex_apps_mcp_tools( @@ -40,20 +36,18 @@ pub(crate) fn build_mcp_tool_exposure( || candidate_tools.len() >= DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD); if !should_defer { - return McpToolExposure { - tools: candidate_tools - .into_iter() - .map(|(name, tool_info)| { - ( - name, - McpToolInput { - tool_info, - defer_loading: false, - }, - ) - }) - .collect(), - }; + return candidate_tools + .into_iter() + .map(|(name, tool_info)| { + ( + name, + McpToolInput { + tool_info, + defer_loading: false, + }, + ) + }) + .collect(); } let direct_tools = @@ -79,7 +73,7 @@ pub(crate) fn build_mcp_tool_exposure( ); } - McpToolExposure { tools } + tools } fn filter_codex_apps_mcp_tools( diff --git a/codex-rs/core/src/mcp_tool_exposure_test.rs b/codex-rs/core/src/mcp_tool_exposure_test.rs index 01f38c1d84..f77281af07 100644 --- a/codex-rs/core/src/mcp_tool_exposure_test.rs +++ b/codex-rs/core/src/mcp_tool_exposure_test.rs @@ -91,9 +91,11 @@ fn numbered_mcp_tools(count: usize) -> HashMap { .collect() } -fn tool_names_with_defer_loading(exposure: &McpToolExposure, defer_loading: bool) -> Vec { - let mut names = exposure - .tools +fn tool_names_with_defer_loading( + exposed_tools: &HashMap, + defer_loading: bool, +) -> Vec { + let mut names = exposed_tools .iter() .filter_map(|(name, tool)| (tool.defer_loading == defer_loading).then_some(name.clone())) .collect::>(); @@ -127,7 +129,7 @@ async fn directly_exposes_small_candidate_tool_sets() { let tools_config = tools_config_for_mcp_tool_exposure(/*search_tool*/ true).await; let mcp_tools = numbered_mcp_tools(DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD - 1); - let exposure = build_mcp_tool_exposure( + let exposed_tools = build_mcp_tool_exposure( &mcp_tools, /*connectors*/ None, &[], @@ -135,11 +137,12 @@ async fn directly_exposes_small_candidate_tool_sets() { &tools_config, ); - let direct_tool_names = tool_names_with_defer_loading(&exposure, /*defer_loading*/ false); + let direct_tool_names = + tool_names_with_defer_loading(&exposed_tools, /*defer_loading*/ false); let mut expected_tool_names: Vec<_> = mcp_tools.keys().cloned().collect(); expected_tool_names.sort(); assert_eq!(direct_tool_names, expected_tool_names); - assert!(exposure.tools.values().all(|tool| !tool.defer_loading)); + assert!(exposed_tools.values().all(|tool| !tool.defer_loading)); } #[tokio::test] @@ -148,7 +151,7 @@ async fn searches_large_candidate_tool_sets() { let tools_config = tools_config_for_mcp_tool_exposure(/*search_tool*/ true).await; let mcp_tools = numbered_mcp_tools(DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD); - let exposure = build_mcp_tool_exposure( + let exposed_tools = build_mcp_tool_exposure( &mcp_tools, /*connectors*/ None, &[], @@ -156,7 +159,8 @@ async fn searches_large_candidate_tool_sets() { &tools_config, ); - let deferred_tool_names = tool_names_with_defer_loading(&exposure, /*defer_loading*/ true); + let deferred_tool_names = + tool_names_with_defer_loading(&exposed_tools, /*defer_loading*/ true); let mut expected_tool_names: Vec<_> = mcp_tools.keys().cloned().collect(); expected_tool_names.sort(); assert_eq!(deferred_tool_names, expected_tool_names); @@ -178,7 +182,7 @@ async fn directly_exposes_explicit_apps_without_deferred_overlap() { )]); let connectors = vec![make_connector("calendar", "Calendar")]; - let exposure = build_mcp_tool_exposure( + let exposed_tools = build_mcp_tool_exposure( &mcp_tools, Some(connectors.as_slice()), connectors.as_slice(), @@ -186,12 +190,13 @@ async fn directly_exposes_explicit_apps_without_deferred_overlap() { &tools_config, ); - let tool_names = tool_names_with_defer_loading(&exposure, /*defer_loading*/ false); + let tool_names = tool_names_with_defer_loading(&exposed_tools, /*defer_loading*/ false); assert_eq!( tool_names, vec!["mcp__codex_apps__calendar_create_event".to_string()] ); - let deferred_tool_names = tool_names_with_defer_loading(&exposure, /*defer_loading*/ true); + let deferred_tool_names = + tool_names_with_defer_loading(&exposed_tools, /*defer_loading*/ true); assert_eq!( deferred_tool_names.len(), DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD - 1 @@ -233,7 +238,7 @@ async fn always_defer_feature_preserves_explicit_apps() { ]); let connectors = vec![make_connector("calendar", "Calendar")]; - let exposure = build_mcp_tool_exposure( + let exposed_tools = build_mcp_tool_exposure( &mcp_tools, Some(connectors.as_slice()), connectors.as_slice(), @@ -241,12 +246,14 @@ async fn always_defer_feature_preserves_explicit_apps() { &tools_config, ); - let direct_tool_names = tool_names_with_defer_loading(&exposure, /*defer_loading*/ false); + let direct_tool_names = + tool_names_with_defer_loading(&exposed_tools, /*defer_loading*/ false); assert_eq!( direct_tool_names, vec!["mcp__codex_apps__calendar_create_event".to_string()] ); - let deferred_tool_names = tool_names_with_defer_loading(&exposure, /*defer_loading*/ true); + let deferred_tool_names = + tool_names_with_defer_loading(&exposed_tools, /*defer_loading*/ true); assert!(deferred_tool_names.contains(&"mcp__rmcp__tool".to_string())); assert!(!deferred_tool_names.contains(&"mcp__codex_apps__calendar_create_event".to_string())); } diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 18cb6d1d07..6ca2e4acbb 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -1196,14 +1196,14 @@ pub(crate) async fn built_tools( } else { Vec::new() }; - let mcp_tool_exposure = build_mcp_tool_exposure( + let exposed_mcp_tools = build_mcp_tool_exposure( &all_mcp_tools, connectors.as_deref(), explicitly_enabled.as_slice(), &turn_context.config, &turn_context.tools_config, ); - let mcp_tools = has_mcp_servers.then_some(mcp_tool_exposure.tools); + let mcp_tools = has_mcp_servers.then_some(exposed_mcp_tools); let unavailable_called_tools = if turn_context .config .features