From f056c595f317bc8be550dbd87c6664ae27a09423 Mon Sep 17 00:00:00 2001 From: Anton Panasenko Date: Wed, 11 Feb 2026 16:00:25 -0800 Subject: [PATCH] Limit search_tool_bm25 to apps and keep non-app MCP tools available --- codex-rs/core/src/codex.rs | 167 ++++++++++++++++-- codex-rs/core/src/features.rs | 2 +- .../src/tools/handlers/search_tool_bm25.rs | 2 + codex-rs/core/src/tools/spec.rs | 4 +- .../search_tool/developer_instructions.md | 18 +- codex-rs/core/tests/suite/search_tool.rs | 68 ++++--- 6 files changed, 196 insertions(+), 65 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 21daadb747..57bcf35fbe 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -4390,19 +4390,31 @@ async fn built_tools( }; if turn_context.config.features.enabled(Feature::SearchTool) { - let mut selected_mcp_tools = + let mut app_mcp_tools = HashMap::new(); + let mut non_app_mcp_tools = HashMap::new(); + + for (name, tool_info) in mcp_tools { + if tool_info.server_name == CODEX_APPS_MCP_SERVER_NAME { + app_mcp_tools.insert(name, tool_info); + } else { + non_app_mcp_tools.insert(name, tool_info); + } + } + + let mut selected_app_mcp_tools = if let Some(selected_tools) = sess.get_mcp_tool_selection().await { - filter_mcp_tools_by_name(mcp_tools.clone(), &selected_tools) + filter_mcp_tools_by_name(app_mcp_tools.clone(), &selected_tools) } else { HashMap::new() }; if let Some(connectors) = connectors_for_tools.as_ref() { - let apps_mcp_tools = filter_codex_apps_mcp_tools_only(mcp_tools, connectors); - selected_mcp_tools.extend(apps_mcp_tools); + let apps_mcp_tools = filter_codex_apps_mcp_tools_only(app_mcp_tools, connectors); + selected_app_mcp_tools.extend(apps_mcp_tools); } - mcp_tools = selected_mcp_tools; + selected_app_mcp_tools.extend(non_app_mcp_tools); + mcp_tools = selected_app_mcp_tools; } else if let Some(connectors) = connectors_for_tools.as_ref() { mcp_tools = filter_codex_apps_mcp_tools(mcp_tools, connectors); } @@ -5355,10 +5367,7 @@ mod tests { #[test] fn search_tool_selection_keeps_codex_apps_tools_without_mentions() { - let selected_tool_names = vec![ - "mcp__codex_apps__calendar_create_event".to_string(), - "mcp__rmcp__echo".to_string(), - ]; + let selected_tool_names = vec!["mcp__codex_apps__calendar_create_event".to_string()]; let mcp_tools = HashMap::from([ ( "mcp__codex_apps__calendar_create_event".to_string(), @@ -5375,8 +5384,6 @@ mod tests { ), ]); - let mut selected_mcp_tools = - filter_mcp_tools_by_name(mcp_tools.clone(), &selected_tool_names); let connectors = connectors::accessible_connectors_from_mcp_tools(&mcp_tools); let explicitly_enabled_connectors = HashSet::new(); let connectors = filter_connectors_for_input( @@ -5385,8 +5392,19 @@ mod tests { &explicitly_enabled_connectors, &HashMap::new(), ); - let apps_mcp_tools = filter_codex_apps_mcp_tools_only(mcp_tools, &connectors); - selected_mcp_tools.extend(apps_mcp_tools); + let mut app_mcp_tools = HashMap::new(); + let mut non_app_mcp_tools = HashMap::new(); + for (name, tool_info) in mcp_tools { + if tool_info.server_name == CODEX_APPS_MCP_SERVER_NAME { + app_mcp_tools.insert(name, tool_info); + } else { + non_app_mcp_tools.insert(name, tool_info); + } + } + let mut selected_mcp_tools = + filter_mcp_tools_by_name(app_mcp_tools.clone(), &selected_tool_names); + selected_mcp_tools.extend(filter_codex_apps_mcp_tools_only(app_mcp_tools, &connectors)); + selected_mcp_tools.extend(non_app_mcp_tools); let mut tool_names: Vec = selected_mcp_tools.into_keys().collect(); tool_names.sort(); @@ -5401,7 +5419,7 @@ mod tests { #[test] fn apps_mentions_add_codex_apps_tools_to_search_selected_set() { - let selected_tool_names = vec!["mcp__rmcp__echo".to_string()]; + let selected_tool_names = Vec::new(); let mcp_tools = HashMap::from([ ( "mcp__codex_apps__calendar_create_event".to_string(), @@ -5418,8 +5436,6 @@ mod tests { ), ]); - let mut selected_mcp_tools = - filter_mcp_tools_by_name(mcp_tools.clone(), &selected_tool_names); let connectors = connectors::accessible_connectors_from_mcp_tools(&mcp_tools); let explicitly_enabled_connectors = HashSet::new(); let connectors = filter_connectors_for_input( @@ -5428,8 +5444,19 @@ mod tests { &explicitly_enabled_connectors, &HashMap::new(), ); - let apps_mcp_tools = filter_codex_apps_mcp_tools_only(mcp_tools, &connectors); - selected_mcp_tools.extend(apps_mcp_tools); + let mut app_mcp_tools = HashMap::new(); + let mut non_app_mcp_tools = HashMap::new(); + for (name, tool_info) in mcp_tools { + if tool_info.server_name == CODEX_APPS_MCP_SERVER_NAME { + app_mcp_tools.insert(name, tool_info); + } else { + non_app_mcp_tools.insert(name, tool_info); + } + } + let mut selected_mcp_tools = + filter_mcp_tools_by_name(app_mcp_tools.clone(), &selected_tool_names); + selected_mcp_tools.extend(filter_codex_apps_mcp_tools_only(app_mcp_tools, &connectors)); + selected_mcp_tools.extend(non_app_mcp_tools); let mut tool_names: Vec = selected_mcp_tools.into_keys().collect(); tool_names.sort(); @@ -5442,6 +5469,110 @@ mod tests { ); } + #[test] + fn search_tool_selection_hides_unselected_codex_apps_tools() { + let selected_tool_names = Vec::new(); + let mcp_tools = HashMap::from([ + ( + "mcp__codex_apps__calendar_create_event".to_string(), + make_mcp_tool( + CODEX_APPS_MCP_SERVER_NAME, + "calendar_create_event", + Some("calendar"), + Some("Calendar"), + ), + ), + ( + "mcp__rmcp__echo".to_string(), + make_mcp_tool("rmcp", "echo", None, None), + ), + ]); + + let connectors = filter_connectors_for_input( + connectors::accessible_connectors_from_mcp_tools(&mcp_tools), + &[user_message("run the selected tools")], + &HashSet::new(), + &HashMap::new(), + ); + let mut app_mcp_tools = HashMap::new(); + let mut non_app_mcp_tools = HashMap::new(); + for (name, tool_info) in mcp_tools { + if tool_info.server_name == CODEX_APPS_MCP_SERVER_NAME { + app_mcp_tools.insert(name, tool_info); + } else { + non_app_mcp_tools.insert(name, tool_info); + } + } + let mut selected_mcp_tools = + filter_mcp_tools_by_name(app_mcp_tools.clone(), &selected_tool_names); + selected_mcp_tools.extend(filter_codex_apps_mcp_tools_only(app_mcp_tools, &connectors)); + selected_mcp_tools.extend(non_app_mcp_tools); + + let mut tool_names: Vec = selected_mcp_tools.into_keys().collect(); + tool_names.sort(); + assert_eq!(tool_names, vec!["mcp__rmcp__echo".to_string()]); + } + + #[test] + fn search_tool_selection_unions_selected_and_mentioned_codex_apps_tools() { + let selected_tool_names = vec!["mcp__codex_apps__calendar_create_event".to_string()]; + let mcp_tools = HashMap::from([ + ( + "mcp__codex_apps__calendar_create_event".to_string(), + make_mcp_tool( + CODEX_APPS_MCP_SERVER_NAME, + "calendar_create_event", + Some("calendar"), + Some("Calendar"), + ), + ), + ( + "mcp__codex_apps__tasks_create_task".to_string(), + make_mcp_tool( + CODEX_APPS_MCP_SERVER_NAME, + "tasks_create_task", + Some("tasks"), + Some("Tasks"), + ), + ), + ( + "mcp__rmcp__echo".to_string(), + make_mcp_tool("rmcp", "echo", None, None), + ), + ]); + + let connectors = filter_connectors_for_input( + connectors::accessible_connectors_from_mcp_tools(&mcp_tools), + &[user_message("use $tasks and then echo the response")], + &HashSet::new(), + &HashMap::new(), + ); + let mut app_mcp_tools = HashMap::new(); + let mut non_app_mcp_tools = HashMap::new(); + for (name, tool_info) in mcp_tools { + if tool_info.server_name == CODEX_APPS_MCP_SERVER_NAME { + app_mcp_tools.insert(name, tool_info); + } else { + non_app_mcp_tools.insert(name, tool_info); + } + } + let mut selected_mcp_tools = + filter_mcp_tools_by_name(app_mcp_tools.clone(), &selected_tool_names); + selected_mcp_tools.extend(filter_codex_apps_mcp_tools_only(app_mcp_tools, &connectors)); + selected_mcp_tools.extend(non_app_mcp_tools); + + let mut tool_names: Vec = selected_mcp_tools.into_keys().collect(); + tool_names.sort(); + assert_eq!( + tool_names, + vec![ + "mcp__codex_apps__calendar_create_event".to_string(), + "mcp__codex_apps__tasks_create_task".to_string(), + "mcp__rmcp__echo".to_string(), + ] + ); + } + #[tokio::test] async fn reconstruct_history_matches_live_compactions() { let (session, turn_context) = make_session_and_context().await; diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index 56aa288133..94b0bd218a 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -89,7 +89,7 @@ pub enum Feature { /// Allow the model to request web searches that fetch cached content. /// Takes precedence over `WebSearchRequest`. WebSearchCached, - /// Allow the model to search MCP tools via BM25 before exposing them. + /// Allow the model to search apps tools via BM25 before exposing app tools. SearchTool, /// Use the bubblewrap-based Linux sandbox pipeline. UseLinuxSandboxBwrap, diff --git a/codex-rs/core/src/tools/handlers/search_tool_bm25.rs b/codex-rs/core/src/tools/handlers/search_tool_bm25.rs index 1da8a3d03f..702442250e 100644 --- a/codex-rs/core/src/tools/handlers/search_tool_bm25.rs +++ b/codex-rs/core/src/tools/handlers/search_tool_bm25.rs @@ -7,6 +7,7 @@ use serde::Deserialize; use serde_json::json; use crate::function_tool::FunctionCallError; +use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; use crate::mcp_connection_manager::ToolInfo; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; @@ -114,6 +115,7 @@ impl ToolHandler for SearchToolBm25Handler { let mut entries: Vec = mcp_tools .into_iter() + .filter(|(_, info)| info.server_name == CODEX_APPS_MCP_SERVER_NAME) .map(|(name, info)| ToolEntry::new(name, info)) .collect(); entries.sort_by(|a, b| a.name.cmp(&b.name)); diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 80fd103b2f..bc0e74df1d 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -874,7 +874,7 @@ fn create_search_tool_bm25_tool() -> ToolSpec { ( "query".to_string(), JsonSchema::String { - description: Some("Search query for MCP tools.".to_string()), + description: Some("Search query for apps tools.".to_string()), }, ), ( @@ -889,7 +889,7 @@ fn create_search_tool_bm25_tool() -> ToolSpec { ToolSpec::Function(ResponsesApiTool { name: "search_tool_bm25".to_string(), - description: "Searches MCP tool metadata with BM25 and exposes matching tools for the next model call.".to_string(), + description: "Searches apps tool metadata with BM25 and exposes matching app tools for the next model call.".to_string(), strict: false, parameters: JsonSchema::Object { properties, diff --git a/codex-rs/core/templates/search_tool/developer_instructions.md b/codex-rs/core/templates/search_tool/developer_instructions.md index 5ff6a83e50..e0ccdb6d37 100644 --- a/codex-rs/core/templates/search_tool/developer_instructions.md +++ b/codex-rs/core/templates/search_tool/developer_instructions.md @@ -1,21 +1,21 @@ -# MCP tool discovery +# Apps tool discovery -When `search_tool_bm25` is available, MCP tools (`mcp__...`) are hidden until you search for them. +When `search_tool_bm25` is available, app tools from `codex_apps` (`mcp__codex_apps__...`) are hidden until you search for them. Follow this workflow: 1. Call `search_tool_bm25` with: - `query` (required): focused terms that describe the capability you need. - `limit` (optional): maximum number of tools to return (default `8`). -2. Use the returned `tools` list to decide which MCP tools are relevant. -3. Matching tools are added to `active_selected_tools`. Only tools in `active_selected_tools` are available for the remainder of the current turn. -4. Repeated searches in the same turn are additive: new matches are unioned into `active_selected_tools`. -5. `active_selected_tools` resets at the start of the next turn. +2. Use the returned `tools` list to decide which app tools are relevant. +3. Matching app tools become available for the remainder of the current turn. +4. Repeated searches in the same turn are additive: new matches are unioned into the available app tools. +5. The available app-tools set resets at the start of the next turn. Notes: -- Core tools remain available without searching. +- Core tools and non-app MCP tools remain available without searching. - If you are unsure, start with `limit` between 5 and 10 to see a broader set of tools. -- `query` is matched against MCP tool metadata fields: +- `query` is matched against app tool metadata fields: - `name` - `tool_name` - `server_name` @@ -24,6 +24,6 @@ Notes: - `connector_name` - `connector_id` - input schema property keys (`input_keys`) -- When the user asks to search/lookup/query any external system (logs, tickets, metrics, Slack, etc.), you must call `search_tool_bm25` first before running any shell command or repo search. +- When the user asks to search/lookup/query any external system (logs, tickets, metrics, Slack, etc.), you must call `search_tool_bm25` first to discover relevant app tools before running any shell command or repo search. - Only use shell commands if (a) MCP tools for that system are not available or not sufficient, and (b) the user explicitly wants a local file/CLI search. - If unsure which system/tool applies, ask a clarifying question after checking MCP tools. diff --git a/codex-rs/core/tests/suite/search_tool.rs b/codex-rs/core/tests/suite/search_tool.rs index fae5956168..8ea9bfec6c 100644 --- a/codex-rs/core/tests/suite/search_tool.rs +++ b/codex-rs/core/tests/suite/search_tool.rs @@ -25,8 +25,8 @@ use serde_json::Value; use serde_json::json; const SEARCH_TOOL_INSTRUCTION_SNIPPETS: [&str; 2] = [ - "MCP tools (`mcp__...`) are hidden until you search for them.", - "Matching tools are added to `active_selected_tools`.", + "app tools from `codex_apps` (`mcp__codex_apps__...`) are hidden until you search for them.", + "Core tools and non-app MCP tools remain available without searching.", ]; fn tool_names(body: &Value) -> Vec { @@ -174,7 +174,7 @@ async fn search_tool_adds_developer_instructions() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn search_tool_hides_mcp_tools_without_search() -> Result<()> { +async fn search_tool_keeps_non_app_mcp_tools_without_search() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; @@ -233,12 +233,8 @@ async fn search_tool_hides_mcp_tools_without_search() -> Result<()> { "tools list should include search_tool_bm25 when enabled: {tools:?}" ); assert!( - !tools.iter().any(|name| name == "mcp__rmcp__echo"), - "tools list should not include MCP tools before search: {tools:?}" - ); - assert!( - !tools.iter().any(|name| name == "mcp__rmcp__image"), - "tools list should not include MCP tools before search: {tools:?}" + tools.iter().any(|name| name == "mcp__rmcp__echo"), + "tools list should include non-app MCP tools without search: {tools:?}" ); Ok(()) @@ -325,18 +321,14 @@ async fn search_tool_selection_persists_within_turn_and_resets_next_turn() -> Re let first_tools = tool_names(&requests[0].body_json()); assert!( - !first_tools.iter().any(|name| name == "mcp__rmcp__echo"), - "first request should not include MCP tools before search: {first_tools:?}" + first_tools.iter().any(|name| name == "mcp__rmcp__echo"), + "first request should include non-app MCP tools without search: {first_tools:?}" ); let second_tools = tool_names(&requests[1].body_json()); assert!( second_tools.iter().any(|name| name == "mcp__rmcp__echo"), - "second request should include selected MCP tool: {second_tools:?}" - ); - assert!( - !second_tools.iter().any(|name| name == "mcp__rmcp__image"), - "second request should only include selected MCP tool: {second_tools:?}" + "second request should include non-app MCP tools: {second_tools:?}" ); let search_output_payload = search_tool_output_payload(&requests[1], call_id); @@ -344,15 +336,27 @@ async fn search_tool_selection_persists_within_turn_and_resets_next_turn() -> Re search_output_payload.get("selected_tools").is_none(), "selected_tools should not be returned: {search_output_payload:?}" ); + assert!( + search_output_payload.get("query").is_some(), + "search_tool_bm25 output should include query: {search_output_payload:?}" + ); + assert!( + search_output_payload.get("total_tools").is_some(), + "search_tool_bm25 output should include total_tools: {search_output_payload:?}" + ); + assert!( + search_output_payload.get("tools").is_some(), + "search_tool_bm25 output should include tools: {search_output_payload:?}" + ); assert_eq!( active_selected_tools(&search_output_payload), - vec!["mcp__rmcp__echo".to_string()], + Vec::::new(), ); let third_tools = tool_names(&requests[2].body_json()); assert!( - !third_tools.iter().any(|name| name == "mcp__rmcp__echo"), - "third request should not include MCP tools after turn reset: {third_tools:?}" + third_tools.iter().any(|name| name == "mcp__rmcp__echo"), + "third request should include non-app MCP tools in the next turn: {third_tools:?}" ); Ok(()) @@ -447,30 +451,27 @@ async fn search_tool_selection_unions_results_within_turn() -> Result<()> { let first_tools = tool_names(&requests[0].body_json()); assert!( - !first_tools.iter().any(|name| name == "mcp__rmcp__echo"), - "first request should not include MCP tools before search: {first_tools:?}" + first_tools.iter().any(|name| name == "mcp__rmcp__echo"), + "first request should include non-app MCP tools without search: {first_tools:?}" ); let second_tools = tool_names(&requests[1].body_json()); assert!( second_tools.iter().any(|name| name == "mcp__rmcp__echo"), - "second request should include echo after first search: {second_tools:?}" - ); - assert!( - !second_tools.iter().any(|name| name == "mcp__rmcp__image"), - "second request should not include image before second search runs: {second_tools:?}" + "second request should include non-app MCP tools: {second_tools:?}" ); let third_tools = tool_names(&requests[2].body_json()); assert!( third_tools.iter().any(|name| name == "mcp__rmcp__echo"), - "third request should still include echo: {third_tools:?}" - ); - assert!( - third_tools.iter().any(|name| name == "mcp__rmcp__image"), - "third request should include image after second search: {third_tools:?}" + "third request should include non-app MCP tools: {third_tools:?}" ); + let first_search_payload = search_tool_output_payload(&requests[1], first_call_id); + assert_eq!( + active_selected_tools(&first_search_payload), + Vec::::new(), + ); let second_search_payload = search_tool_output_payload(&requests[2], second_call_id); assert!( second_search_payload.get("selected_tools").is_none(), @@ -478,10 +479,7 @@ async fn search_tool_selection_unions_results_within_turn() -> Result<()> { ); assert_eq!( active_selected_tools(&second_search_payload), - vec![ - "mcp__rmcp__echo".to_string(), - "mcp__rmcp__image".to_string(), - ], + Vec::::new(), ); Ok(())