mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
enable tool search over dynamic tools (#18263)
## Summary - Normalize deferred MCP and dynamic tools into `ToolSearchEntry` values before constructing `ToolSearchHandler`. - Move the tool-search entry adapter out of `tools/handlers` and into `tools/tool_search_entry.rs` so the handlers directory stays focused on handlers. - Keep `ToolSearchHandler` operating over one generic entry list for BM25 search, namespace grouping, and per-bucket default limits. ## Why Follow-up cleanup for #17849. The dynamic tool-search support made the handler juggle source-specific MCP and dynamic tool lists, index arithmetic, output conversion, and namespace emission. This keeps source adaptation outside the handler so the search loop itself is smaller and source-agnostic. ## Validation - `just fmt` - `cargo test -p codex-core tools::handlers::tool_search::tests` - `git diff --check` - `cargo test -p codex-core` currently fails in unrelated `plugins::manager::tests::list_marketplaces_ignores_installed_roots_missing_from_config`; rerunning that single test fails the same way at `core/src/plugins/manager_tests.rs:1692`. --------- Co-authored-by: pash <pash@openai.com>
This commit is contained in:
@@ -109,12 +109,12 @@ pub use tool_discovery::ToolSearchResultSource;
|
||||
pub use tool_discovery::ToolSearchSource;
|
||||
pub use tool_discovery::ToolSearchSourceInfo;
|
||||
pub use tool_discovery::ToolSuggestEntry;
|
||||
pub use tool_discovery::collect_tool_search_output_tools;
|
||||
pub use tool_discovery::collect_tool_search_source_infos;
|
||||
pub use tool_discovery::collect_tool_suggest_entries;
|
||||
pub use tool_discovery::create_tool_search_tool;
|
||||
pub use tool_discovery::create_tool_suggest_tool;
|
||||
pub use tool_discovery::filter_tool_suggest_discoverable_tools_for_client;
|
||||
pub use tool_discovery::tool_search_result_source_to_output_tool;
|
||||
pub use tool_registry_plan::build_tool_registry_plan;
|
||||
pub use tool_registry_plan_types::ToolHandlerKind;
|
||||
pub use tool_registry_plan_types::ToolHandlerSpec;
|
||||
|
||||
@@ -153,7 +153,7 @@ pub fn create_tool_search_tool(
|
||||
let properties = BTreeMap::from([
|
||||
(
|
||||
"query".to_string(),
|
||||
JsonSchema::string(Some("Search query for MCP tools.".to_string())),
|
||||
JsonSchema::string(Some("Search query for deferred tools.".to_string())),
|
||||
),
|
||||
(
|
||||
"limit".to_string(),
|
||||
@@ -189,7 +189,7 @@ pub fn create_tool_search_tool(
|
||||
};
|
||||
|
||||
let description = format!(
|
||||
"# MCP tool discovery\n\nSearches over MCP tool metadata with BM25 and exposes matching tools for the next model call.\n\nYou have access to tools from the following MCP servers/connectors:\n{source_descriptions}\nSome of the tools may not have been provided to you upfront, and you should use this tool (`{TOOL_SEARCH_TOOL_NAME}`) to search for the required MCP tools. For MCP tool discovery, always use `{TOOL_SEARCH_TOOL_NAME}` instead of `list_mcp_resources` or `list_mcp_resource_templates`."
|
||||
"# Tool discovery\n\nSearches over deferred tool metadata with BM25 and exposes matching tools for the next model call.\n\nYou have access to tools from the following sources:\n{source_descriptions}\nSome of the tools may not have been provided to you upfront, and you should use this tool (`{TOOL_SEARCH_TOOL_NAME}`) to search for the required tools. For MCP tool discovery, always use `{TOOL_SEARCH_TOOL_NAME}` instead of `list_mcp_resources` or `list_mcp_resource_templates`."
|
||||
);
|
||||
|
||||
ToolSpec::ToolSearch {
|
||||
@@ -203,58 +203,38 @@ pub fn create_tool_search_tool(
|
||||
}
|
||||
}
|
||||
|
||||
pub fn collect_tool_search_output_tools<'a>(
|
||||
tool_sources: impl IntoIterator<Item = ToolSearchResultSource<'a>>,
|
||||
) -> Result<Vec<ToolSearchOutputTool>, serde_json::Error> {
|
||||
let mut grouped: Vec<(&'a str, Vec<ToolSearchResultSource<'a>>)> = Vec::new();
|
||||
for tool in tool_sources {
|
||||
if let Some((_, tools)) = grouped
|
||||
.iter_mut()
|
||||
.find(|(tool_namespace, _)| *tool_namespace == tool.tool_namespace)
|
||||
{
|
||||
tools.push(tool);
|
||||
} else {
|
||||
grouped.push((tool.tool_namespace, vec![tool]));
|
||||
}
|
||||
}
|
||||
pub fn tool_search_result_source_to_output_tool(
|
||||
source: ToolSearchResultSource<'_>,
|
||||
) -> Result<ToolSearchOutputTool, serde_json::Error> {
|
||||
Ok(ToolSearchOutputTool::Namespace(ResponsesApiNamespace {
|
||||
name: source.tool_namespace.to_string(),
|
||||
description: tool_search_result_source_namespace_description(source),
|
||||
tools: vec![tool_search_result_source_to_namespace_tool(source)?],
|
||||
}))
|
||||
}
|
||||
|
||||
let mut results = Vec::with_capacity(grouped.len());
|
||||
for (tool_namespace, tools) in grouped {
|
||||
let Some(first_tool) = tools.first() else {
|
||||
continue;
|
||||
};
|
||||
fn tool_search_result_source_namespace_description(source: ToolSearchResultSource<'_>) -> String {
|
||||
source
|
||||
.connector_description
|
||||
.map(str::trim)
|
||||
.filter(|description| !description.is_empty())
|
||||
.map(str::to_string)
|
||||
.or_else(|| {
|
||||
source
|
||||
.connector_name
|
||||
.map(str::trim)
|
||||
.filter(|connector_name| !connector_name.is_empty())
|
||||
.map(|connector_name| format!("Tools for working with {connector_name}."))
|
||||
})
|
||||
.unwrap_or_else(|| default_namespace_description(source.tool_namespace))
|
||||
}
|
||||
|
||||
let description = first_tool
|
||||
.connector_description
|
||||
.map(str::trim)
|
||||
.filter(|description| !description.is_empty())
|
||||
.map(str::to_string)
|
||||
.or_else(|| {
|
||||
first_tool
|
||||
.connector_name
|
||||
.map(str::trim)
|
||||
.filter(|connector_name| !connector_name.is_empty())
|
||||
.map(|connector_name| format!("Tools for working with {connector_name}."))
|
||||
});
|
||||
|
||||
let tools = tools
|
||||
.iter()
|
||||
.map(|tool| {
|
||||
let tool_name = ToolName::namespaced(tool.tool_namespace, tool.tool_name);
|
||||
mcp_tool_to_deferred_responses_api_tool(&tool_name, tool.tool)
|
||||
.map(ResponsesApiNamespaceTool::Function)
|
||||
})
|
||||
.collect::<Result<Vec<_>, _>>()?;
|
||||
|
||||
results.push(ToolSearchOutputTool::Namespace(ResponsesApiNamespace {
|
||||
name: tool_namespace.to_string(),
|
||||
description: description
|
||||
.unwrap_or_else(|| default_namespace_description(tool_namespace)),
|
||||
tools,
|
||||
}));
|
||||
}
|
||||
|
||||
Ok(results)
|
||||
fn tool_search_result_source_to_namespace_tool(
|
||||
source: ToolSearchResultSource<'_>,
|
||||
) -> Result<ResponsesApiNamespaceTool, serde_json::Error> {
|
||||
let tool_name = ToolName::namespaced(source.tool_namespace, source.tool_name);
|
||||
mcp_tool_to_deferred_responses_api_tool(&tool_name, source.tool)
|
||||
.map(ResponsesApiNamespaceTool::Function)
|
||||
}
|
||||
|
||||
pub fn collect_tool_search_source_infos<'a>(
|
||||
|
||||
@@ -2,28 +2,8 @@ use super::*;
|
||||
use crate::JsonSchema;
|
||||
use codex_app_server_protocol::AppInfo;
|
||||
use pretty_assertions::assert_eq;
|
||||
use rmcp::model::JsonObject;
|
||||
use rmcp::model::Tool;
|
||||
use serde_json::json;
|
||||
use std::collections::BTreeMap;
|
||||
use std::sync::Arc;
|
||||
|
||||
fn mcp_tool(name: &str, description: &str) -> Tool {
|
||||
Tool {
|
||||
name: name.to_string().into(),
|
||||
title: None,
|
||||
description: Some(description.to_string().into()),
|
||||
input_schema: Arc::new(JsonObject::from_iter([(
|
||||
"type".to_string(),
|
||||
json!("object"),
|
||||
)])),
|
||||
output_schema: None,
|
||||
annotations: None,
|
||||
execution: None,
|
||||
icons: None,
|
||||
meta: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn create_tool_search_tool_deduplicates_and_renders_enabled_sources() {
|
||||
@@ -50,7 +30,7 @@ fn create_tool_search_tool_deduplicates_and_renders_enabled_sources() {
|
||||
),
|
||||
ToolSpec::ToolSearch {
|
||||
execution: "client".to_string(),
|
||||
description: "# MCP tool discovery\n\nSearches over MCP tool metadata with BM25 and exposes matching tools for the next model call.\n\nYou have access to tools from the following MCP servers/connectors:\n- Google Drive: Use Google Drive as the single entrypoint for Drive, Docs, Sheets, and Slides work.\n- docs\nSome of the tools may not have been provided to you upfront, and you should use this tool (`tool_search`) to search for the required MCP tools. For MCP tool discovery, always use `tool_search` instead of `list_mcp_resources` or `list_mcp_resource_templates`.".to_string(),
|
||||
description: "# Tool discovery\n\nSearches over deferred tool metadata with BM25 and exposes matching tools for the next model call.\n\nYou have access to tools from the following sources:\n- Google Drive: Use Google Drive as the single entrypoint for Drive, Docs, Sheets, and Slides work.\n- docs\nSome of the tools may not have been provided to you upfront, and you should use this tool (`tool_search`) to search for the required tools. For MCP tool discovery, always use `tool_search` instead of `list_mcp_resources` or `list_mcp_resource_templates`.".to_string(),
|
||||
parameters: JsonSchema::object(BTreeMap::from([
|
||||
(
|
||||
"limit".to_string(),
|
||||
@@ -61,7 +41,7 @@ fn create_tool_search_tool_deduplicates_and_renders_enabled_sources() {
|
||||
),
|
||||
(
|
||||
"query".to_string(),
|
||||
JsonSchema::string(Some("Search query for MCP tools.".to_string()),),
|
||||
JsonSchema::string(Some("Search query for deferred tools.".to_string()),),
|
||||
),
|
||||
]), Some(vec!["query".to_string()]), Some(false.into())),
|
||||
}
|
||||
@@ -136,176 +116,6 @@ fn create_tool_suggest_tool_uses_plugin_summary_fallback() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn collect_tool_search_output_tools_preserves_search_order_while_grouping_by_namespace() {
|
||||
let calendar_create_event = mcp_tool("calendar-create-event", "Create a calendar event.");
|
||||
let gmail_read_email = mcp_tool("gmail-read-email", "Read an email.");
|
||||
let gmail_send_email = mcp_tool("gmail-send-email", "Send an email.");
|
||||
let calendar_list_events = mcp_tool("calendar-list-events", "List calendar events.");
|
||||
let docs_search = mcp_tool("search", "Search docs.");
|
||||
|
||||
let tools = collect_tool_search_output_tools([
|
||||
ToolSearchResultSource {
|
||||
server_name: "codex_apps",
|
||||
tool_namespace: "mcp__codex_apps__gmail",
|
||||
tool_name: "_send_email",
|
||||
tool: &gmail_send_email,
|
||||
connector_name: Some("Gmail"),
|
||||
connector_description: Some("Read mail"),
|
||||
},
|
||||
ToolSearchResultSource {
|
||||
server_name: "codex_apps",
|
||||
tool_namespace: "mcp__codex_apps__calendar",
|
||||
tool_name: "_create_event",
|
||||
tool: &calendar_create_event,
|
||||
connector_name: Some("Calendar"),
|
||||
connector_description: Some("Plan events"),
|
||||
},
|
||||
ToolSearchResultSource {
|
||||
server_name: "codex_apps",
|
||||
tool_namespace: "mcp__codex_apps__gmail",
|
||||
tool_name: "_read_email",
|
||||
tool: &gmail_read_email,
|
||||
connector_name: Some("Gmail"),
|
||||
connector_description: Some("Read mail"),
|
||||
},
|
||||
ToolSearchResultSource {
|
||||
server_name: "codex_apps",
|
||||
tool_namespace: "mcp__codex_apps__calendar",
|
||||
tool_name: "_list_events",
|
||||
tool: &calendar_list_events,
|
||||
connector_name: Some("Calendar"),
|
||||
connector_description: Some("Plan events"),
|
||||
},
|
||||
ToolSearchResultSource {
|
||||
server_name: "docs",
|
||||
tool_namespace: "mcp__docs__",
|
||||
tool_name: "search",
|
||||
tool: &docs_search,
|
||||
connector_name: None,
|
||||
connector_description: None,
|
||||
},
|
||||
])
|
||||
.expect("collect tool search output tools");
|
||||
|
||||
assert_eq!(
|
||||
tools,
|
||||
vec![
|
||||
ToolSearchOutputTool::Namespace(ResponsesApiNamespace {
|
||||
name: "mcp__codex_apps__gmail".to_string(),
|
||||
description: "Read mail".to_string(),
|
||||
tools: vec![
|
||||
ResponsesApiNamespaceTool::Function(ResponsesApiTool {
|
||||
name: "_send_email".to_string(),
|
||||
description: "Send an email.".to_string(),
|
||||
strict: false,
|
||||
defer_loading: Some(true),
|
||||
parameters: JsonSchema::object(
|
||||
Default::default(),
|
||||
/*required*/ None,
|
||||
/*additional_properties*/ None
|
||||
),
|
||||
output_schema: None,
|
||||
}),
|
||||
ResponsesApiNamespaceTool::Function(ResponsesApiTool {
|
||||
name: "_read_email".to_string(),
|
||||
description: "Read an email.".to_string(),
|
||||
strict: false,
|
||||
defer_loading: Some(true),
|
||||
parameters: JsonSchema::object(
|
||||
Default::default(),
|
||||
/*required*/ None,
|
||||
/*additional_properties*/ None
|
||||
),
|
||||
output_schema: None,
|
||||
}),
|
||||
],
|
||||
}),
|
||||
ToolSearchOutputTool::Namespace(ResponsesApiNamespace {
|
||||
name: "mcp__codex_apps__calendar".to_string(),
|
||||
description: "Plan events".to_string(),
|
||||
tools: vec![
|
||||
ResponsesApiNamespaceTool::Function(ResponsesApiTool {
|
||||
name: "_create_event".to_string(),
|
||||
description: "Create a calendar event.".to_string(),
|
||||
strict: false,
|
||||
defer_loading: Some(true),
|
||||
parameters: JsonSchema::object(
|
||||
Default::default(),
|
||||
/*required*/ None,
|
||||
/*additional_properties*/ None
|
||||
),
|
||||
output_schema: None,
|
||||
}),
|
||||
ResponsesApiNamespaceTool::Function(ResponsesApiTool {
|
||||
name: "_list_events".to_string(),
|
||||
description: "List calendar events.".to_string(),
|
||||
strict: false,
|
||||
defer_loading: Some(true),
|
||||
parameters: JsonSchema::object(
|
||||
Default::default(),
|
||||
/*required*/ None,
|
||||
/*additional_properties*/ None
|
||||
),
|
||||
output_schema: None,
|
||||
}),
|
||||
],
|
||||
}),
|
||||
ToolSearchOutputTool::Namespace(ResponsesApiNamespace {
|
||||
name: "mcp__docs__".to_string(),
|
||||
description: "Tools in the mcp__docs__ namespace.".to_string(),
|
||||
tools: vec![ResponsesApiNamespaceTool::Function(ResponsesApiTool {
|
||||
name: "search".to_string(),
|
||||
description: "Search docs.".to_string(),
|
||||
strict: false,
|
||||
defer_loading: Some(true),
|
||||
parameters: JsonSchema::object(
|
||||
Default::default(),
|
||||
/*required*/ None,
|
||||
/*additional_properties*/ None
|
||||
),
|
||||
output_schema: None,
|
||||
})],
|
||||
}),
|
||||
],
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn collect_tool_search_output_tools_ignores_blank_connector_description() {
|
||||
let gmail_batch_read_email = mcp_tool("gmail-batch-read-email", "Read multiple emails.");
|
||||
|
||||
let tools = collect_tool_search_output_tools([ToolSearchResultSource {
|
||||
server_name: "codex_apps",
|
||||
tool_namespace: "mcp__codex_apps__gmail",
|
||||
tool_name: "_batch_read_email",
|
||||
tool: &gmail_batch_read_email,
|
||||
connector_name: Some("Gmail"),
|
||||
connector_description: Some(" "),
|
||||
}])
|
||||
.expect("collect tool search output tools");
|
||||
|
||||
assert_eq!(
|
||||
tools,
|
||||
vec![ToolSearchOutputTool::Namespace(ResponsesApiNamespace {
|
||||
name: "mcp__codex_apps__gmail".to_string(),
|
||||
description: "Tools for working with Gmail.".to_string(),
|
||||
tools: vec![ResponsesApiNamespaceTool::Function(ResponsesApiTool {
|
||||
name: "_batch_read_email".to_string(),
|
||||
description: "Read multiple emails.".to_string(),
|
||||
strict: false,
|
||||
defer_loading: Some(true),
|
||||
parameters: JsonSchema::object(
|
||||
Default::default(),
|
||||
/*required*/ None,
|
||||
/*additional_properties*/ None
|
||||
),
|
||||
output_schema: None,
|
||||
})],
|
||||
})],
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn discoverable_tool_enums_use_expected_wire_names() {
|
||||
assert_eq!(
|
||||
|
||||
@@ -11,6 +11,7 @@ use crate::ToolHandlerKind;
|
||||
use crate::ToolRegistryPlan;
|
||||
use crate::ToolRegistryPlanParams;
|
||||
use crate::ToolSearchSource;
|
||||
use crate::ToolSearchSourceInfo;
|
||||
use crate::ToolSpec;
|
||||
use crate::ToolsConfig;
|
||||
use crate::ViewImageToolOptions;
|
||||
@@ -251,17 +252,35 @@ pub fn build_tool_registry_plan(
|
||||
plan.register_handler("request_permissions", ToolHandlerKind::RequestPermissions);
|
||||
}
|
||||
|
||||
let deferred_dynamic_tools = params
|
||||
.dynamic_tools
|
||||
.iter()
|
||||
.filter(|tool| tool.defer_loading)
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
if config.search_tool
|
||||
&& let Some(deferred_mcp_tools) = params.deferred_mcp_tools
|
||||
&& (params.deferred_mcp_tools.is_some() || !deferred_dynamic_tools.is_empty())
|
||||
{
|
||||
let search_source_infos =
|
||||
collect_tool_search_source_infos(deferred_mcp_tools.iter().map(|tool| {
|
||||
ToolSearchSource {
|
||||
server_name: tool.server_name,
|
||||
connector_name: tool.connector_name,
|
||||
connector_description: tool.connector_description,
|
||||
}
|
||||
}));
|
||||
let mut search_source_infos = params
|
||||
.deferred_mcp_tools
|
||||
.map(|deferred_mcp_tools| {
|
||||
collect_tool_search_source_infos(deferred_mcp_tools.iter().map(|tool| {
|
||||
ToolSearchSource {
|
||||
server_name: tool.server_name,
|
||||
connector_name: tool.connector_name,
|
||||
connector_description: tool.connector_description,
|
||||
}
|
||||
}))
|
||||
})
|
||||
.unwrap_or_default();
|
||||
|
||||
if !deferred_dynamic_tools.is_empty() {
|
||||
search_source_infos.push(ToolSearchSourceInfo {
|
||||
name: "Dynamic tools".to_string(),
|
||||
description: Some("Tools provided by the current Codex thread.".to_string()),
|
||||
});
|
||||
}
|
||||
|
||||
plan.push_spec(
|
||||
create_tool_search_tool(&search_source_infos, TOOL_SEARCH_DEFAULT_LIMIT),
|
||||
/*supports_parallel_tool_calls*/ true,
|
||||
@@ -269,8 +288,10 @@ pub fn build_tool_registry_plan(
|
||||
);
|
||||
plan.register_handler(TOOL_SEARCH_TOOL_NAME, ToolHandlerKind::ToolSearch);
|
||||
|
||||
for tool in deferred_mcp_tools {
|
||||
plan.register_handler(tool.name.clone(), ToolHandlerKind::Mcp);
|
||||
if let Some(deferred_mcp_tools) = params.deferred_mcp_tools {
|
||||
for tool in deferred_mcp_tools {
|
||||
plan.register_handler(tool.name.clone(), ToolHandlerKind::Mcp);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1407,6 +1407,57 @@ fn search_tool_requires_model_capability_and_enabled_feature() {
|
||||
assert_contains_tool_names(&tools, &[TOOL_SEARCH_TOOL_NAME]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn search_tool_registers_for_deferred_dynamic_tools() {
|
||||
let model_info = search_capable_model_info();
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::ToolSearch);
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
available_models: &available_models,
|
||||
features: &features,
|
||||
image_generation_tool_auth_allowed: true,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
session_source: SessionSource::Cli,
|
||||
sandbox_policy: &SandboxPolicy::DangerFullAccess,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
});
|
||||
let dynamic_tool = DynamicToolSpec {
|
||||
name: "automation_update".to_string(),
|
||||
description: "Create, update, view, or delete recurring automations.".to_string(),
|
||||
input_schema: json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"mode": { "type": "string" },
|
||||
},
|
||||
}),
|
||||
defer_loading: true,
|
||||
};
|
||||
|
||||
let (tools, handlers) = build_specs(
|
||||
&tools_config,
|
||||
/*mcp_tools*/ None,
|
||||
/*deferred_mcp_tools*/ None,
|
||||
&[dynamic_tool],
|
||||
);
|
||||
|
||||
let search_tool = find_tool(&tools, TOOL_SEARCH_TOOL_NAME);
|
||||
let ToolSpec::ToolSearch { description, .. } = &search_tool.spec else {
|
||||
panic!("expected tool_search tool");
|
||||
};
|
||||
assert!(description.contains("- Dynamic tools: Tools provided by the current Codex thread."));
|
||||
assert_contains_tool_names(&tools, &[TOOL_SEARCH_TOOL_NAME, "automation_update"]);
|
||||
assert!(handlers.contains(&ToolHandlerSpec {
|
||||
name: ToolName::plain(TOOL_SEARCH_TOOL_NAME),
|
||||
kind: ToolHandlerKind::ToolSearch,
|
||||
}));
|
||||
assert!(handlers.contains(&ToolHandlerSpec {
|
||||
name: ToolName::plain("automation_update"),
|
||||
kind: ToolHandlerKind::DynamicTool,
|
||||
}));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_suggest_is_not_registered_without_feature_flag() {
|
||||
let model_info = search_capable_model_info();
|
||||
|
||||
Reference in New Issue
Block a user