mirror of
https://github.com/openai/codex.git
synced 2026-11-06 20:20:34 +00:00
Limit search_tool_bm25 to apps and keep non-app MCP tools available
This commit is contained in:
+150
-19
@@ -4400,20 +4400,32 @@ async fn built_tools(
|
||||
None
|
||||
};
|
||||
|
||||
if turn_context.config.features.enabled(Feature::Apps) {
|
||||
let mut selected_mcp_tools =
|
||||
if turn_context.config.features.enabled(Feature::SearchTool) {
|
||||
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);
|
||||
}
|
||||
@@ -5366,10 +5378,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(),
|
||||
@@ -5386,8 +5395,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(
|
||||
@@ -5396,8 +5403,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<String> = selected_mcp_tools.into_keys().collect();
|
||||
tool_names.sort();
|
||||
@@ -5412,7 +5430,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(),
|
||||
@@ -5429,8 +5447,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(
|
||||
@@ -5439,8 +5455,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<String> = selected_mcp_tools.into_keys().collect();
|
||||
tool_names.sort();
|
||||
@@ -5453,6 +5480,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<String> = 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<String> = 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;
|
||||
|
||||
@@ -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<ToolEntry> = 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));
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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<String> {
|
||||
@@ -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::<String>::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::<String>::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::<String>::new(),
|
||||
);
|
||||
|
||||
Ok(())
|
||||
|
||||
Reference in New Issue
Block a user