diff --git a/codex-rs/core/src/tools/handlers/dynamic.rs b/codex-rs/core/src/tools/handlers/dynamic.rs index 9ecdf2340d..4637aac917 100644 --- a/codex-rs/core/src/tools/handlers/dynamic.rs +++ b/codex-rs/core/src/tools/handlers/dynamic.rs @@ -6,6 +6,7 @@ use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; use crate::tools::handlers::parse_arguments; use crate::tools::registry::ToolHandler; +use crate::tools::tool_search_entry::ToolSearchInfo; use crate::turn_timing::now_unix_timestamp_ms; use codex_protocol::dynamic_tools::DynamicToolCallRequest; use codex_protocol::dynamic_tools::DynamicToolResponse; @@ -13,9 +14,13 @@ use codex_protocol::dynamic_tools::DynamicToolSpec; use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::protocol::DynamicToolCallResponseEvent; use codex_protocol::protocol::EventMsg; +use codex_tools::ResponsesApiNamespace; +use codex_tools::ResponsesApiNamespaceTool; use codex_tools::ToolName; +use codex_tools::ToolSearchSourceInfo; use codex_tools::ToolSpec; -use codex_tools::dynamic_tool_to_loadable_tool_spec; +use codex_tools::default_namespace_description; +use codex_tools::dynamic_tool_to_responses_api_tool; use serde_json::Value; use std::time::Instant; use tokio::sync::oneshot; @@ -24,15 +29,25 @@ use tracing::warn; pub struct DynamicToolHandler { tool_name: ToolName, spec: Option, + search_text: String, } impl DynamicToolHandler { pub fn new(tool: &DynamicToolSpec) -> Option { let tool_name = ToolName::new(tool.namespace.clone(), tool.name.clone()); - let spec = dynamic_tool_to_loadable_tool_spec(tool).ok()?.into(); + let output_tool = dynamic_tool_to_responses_api_tool(tool).ok()?; + let spec = match tool.namespace.as_ref() { + Some(namespace) => ToolSpec::Namespace(ResponsesApiNamespace { + name: namespace.clone(), + description: default_namespace_description(namespace), + tools: vec![ResponsesApiNamespaceTool::Function(output_tool)], + }), + None => ToolSpec::Function(output_tool), + }; Some(Self { tool_name, spec: Some(spec), + search_text: build_dynamic_search_text(tool), }) } } @@ -48,6 +63,17 @@ impl ToolHandler for DynamicToolHandler { self.spec.clone() } + fn search_info(&self) -> Option { + ToolSearchInfo::from_spec( + self.search_text.clone(), + self.spec()?, + Some(ToolSearchSourceInfo { + name: "Dynamic tools".to_string(), + description: Some("Tools provided by the current Codex thread.".to_string()), + }), + ) + } + async fn handle(&self, invocation: ToolInvocation) -> Result { let ToolInvocation { session, @@ -166,3 +192,27 @@ async fn request_dynamic_tool( response } + +fn build_dynamic_search_text(tool: &DynamicToolSpec) -> String { + let mut schema_properties = tool + .input_schema + .get("properties") + .and_then(serde_json::Value::as_object) + .map(|map| map.keys().cloned().collect::>()) + .unwrap_or_default(); + schema_properties.sort(); + let mut parts = vec![ + tool.name.clone(), + tool.name.replace('_', " "), + tool.description.clone(), + ]; + if let Some(namespace) = &tool.namespace { + parts.push(namespace.clone()); + } + parts.extend(schema_properties); + parts.join(" ") +} + +#[cfg(test)] +#[path = "dynamic_tests.rs"] +mod tests; diff --git a/codex-rs/core/src/tools/handlers/dynamic_tests.rs b/codex-rs/core/src/tools/handlers/dynamic_tests.rs new file mode 100644 index 0000000000..201ef45855 --- /dev/null +++ b/codex-rs/core/src/tools/handlers/dynamic_tests.rs @@ -0,0 +1,36 @@ +use super::*; +use codex_tools::ToolSearchSourceInfo; +use pretty_assertions::assert_eq; +use serde_json::json; + +#[test] +fn search_info_uses_dynamic_tool_metadata_and_parameter_names() { + let handler = DynamicToolHandler::new(&DynamicToolSpec { + namespace: Some("codex_app".to_string()), + name: "automation_update".to_string(), + description: "Create or update automations.".to_string(), + input_schema: json!({ + "type": "object", + "properties": { + "timezone": { "type": "string" }, + "mode": { "type": "string" } + } + }), + defer_loading: true, + }) + .expect("dynamic handler should be created"); + + let search_info = handler.search_info().expect("dynamic search info"); + + assert_eq!( + search_info.entry.search_text, + "automation_update automation update Create or update automations. codex_app mode timezone" + ); + assert_eq!( + search_info.source_info, + Some(ToolSearchSourceInfo { + name: "Dynamic tools".to_string(), + description: Some("Tools provided by the current Codex thread.".to_string()), + }) + ); +} diff --git a/codex-rs/core/src/tools/handlers/mcp.rs b/codex-rs/core/src/tools/handlers/mcp.rs index 89666c80aa..6e7d01f9e7 100644 --- a/codex-rs/core/src/tools/handlers/mcp.rs +++ b/codex-rs/core/src/tools/handlers/mcp.rs @@ -8,15 +8,18 @@ use crate::tools::context::McpToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::flat_tool_name; use crate::tools::hook_names::HookToolName; use crate::tools::registry::PostToolUsePayload; use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolTelemetryTags; +use crate::tools::tool_search_entry::ToolSearchInfo; use codex_mcp::ToolInfo; use codex_tools::ResponsesApiNamespace; use codex_tools::ResponsesApiNamespaceTool; use codex_tools::ToolName; +use codex_tools::ToolSearchSourceInfo; use codex_tools::ToolSpec; use codex_tools::mcp_tool_to_responses_api_tool; use serde_json::Map; @@ -50,6 +53,14 @@ impl ToolHandler for McpHandler { .map(str::trim) .filter(|description| !description.is_empty()) .map(str::to_string) + .or_else(|| { + self.tool_info + .connector_name + .as_deref() + .map(str::trim) + .filter(|connector_name| !connector_name.is_empty()) + .map(|connector_name| format!("Tools for working with {connector_name}.")) + }) .unwrap_or_default(); Some(ToolSpec::Namespace(ResponsesApiNamespace { @@ -59,6 +70,32 @@ impl ToolHandler for McpHandler { })) } + fn search_info(&self) -> Option { + let source_name = self + .tool_info + .connector_name + .as_deref() + .map(str::trim) + .filter(|connector_name| !connector_name.is_empty()) + .unwrap_or_else(|| self.tool_info.server_name.trim()); + let source_info = (!source_name.is_empty()).then(|| ToolSearchSourceInfo { + name: source_name.to_string(), + description: self + .tool_info + .namespace_description + .as_deref() + .map(str::trim) + .filter(|description| !description.is_empty()) + .map(str::to_string), + }); + + ToolSearchInfo::from_spec( + build_mcp_search_text(&self.tool_info), + self.spec()?, + source_info, + ) + } + fn supports_parallel_tool_calls(&self) -> bool { self.tool_info.supports_parallel_tool_calls } @@ -171,6 +208,58 @@ fn mcp_hook_tool_input(raw_arguments: &str) -> Value { serde_json::from_str(raw_arguments).unwrap_or_else(|_| Value::String(raw_arguments.to_string())) } +fn build_mcp_search_text(info: &ToolInfo) -> String { + let tool_name = info.canonical_tool_name(); + let mut schema_properties = info + .tool + .input_schema + .get("properties") + .and_then(serde_json::Value::as_object) + .map(|map| map.keys().cloned().collect::>()) + .unwrap_or_default(); + schema_properties.sort(); + let mut parts = vec![ + flat_tool_name(&tool_name).into_owned(), + info.callable_name.clone(), + info.tool.name.to_string(), + info.server_name.clone(), + ]; + if let Some(title) = info.tool.title.as_deref().map(str::trim) + && !title.is_empty() + { + parts.push(title.to_string()); + } + if let Some(description) = info.tool.description.as_deref().map(str::trim) + && !description.is_empty() + { + parts.push(description.to_string()); + } + if let Some(connector_name) = info.connector_name.as_deref().map(str::trim) + && !connector_name.is_empty() + { + parts.push(connector_name.to_string()); + } + if let Some(namespace_description) = info.namespace_description.as_deref().map(str::trim) + && !namespace_description.is_empty() + { + parts.push(namespace_description.to_string()); + } + parts.extend( + info.plugin_display_names + .iter() + .map(String::as_str) + .map(str::trim) + .filter(|display_name| !display_name.is_empty()) + .map(str::to_string), + ); + parts.extend(schema_properties); + parts.join(" ") +} + +#[cfg(test)] +#[path = "mcp_search_tests.rs"] +mod search_tests; + #[cfg(test)] mod tests { use super::*; diff --git a/codex-rs/core/src/tools/handlers/mcp_search_tests.rs b/codex-rs/core/src/tools/handlers/mcp_search_tests.rs new file mode 100644 index 0000000000..b9467bad4e --- /dev/null +++ b/codex-rs/core/src/tools/handlers/mcp_search_tests.rs @@ -0,0 +1,75 @@ +use super::*; +use codex_tools::LoadableToolSpec; +use codex_tools::ToolSearchSourceInfo; +use pretty_assertions::assert_eq; +use serde_json::json; + +#[test] +fn search_info_uses_mcp_tool_metadata_and_parameter_names() { + let handler = McpHandler::new(tool_info()); + let search_info = handler.search_info().expect("MCP search info"); + + assert_eq!( + search_info.entry.search_text, + "mcp__calendar___create_event _create_event createEvent codex-apps Create event Create a calendar event. Calendar Plan events. Calendar plugin attendees start_time" + ); + assert_eq!( + search_info.source_info, + Some(ToolSearchSourceInfo { + name: "Calendar".to_string(), + description: Some("Plan events.".to_string()), + }) + ); +} + +#[test] +fn search_info_uses_connector_name_for_output_namespace_description() { + let mut tool_info = tool_info(); + tool_info.namespace_description = None; + let handler = McpHandler::new(tool_info); + let search_info = handler.search_info().expect("MCP search info"); + + let LoadableToolSpec::Namespace(namespace) = search_info.entry.output else { + panic!("expected namespace search output"); + }; + assert_eq!(namespace.description, "Tools for working with Calendar."); + assert_eq!( + search_info.source_info, + Some(ToolSearchSourceInfo { + name: "Calendar".to_string(), + description: None, + }) + ); +} + +fn tool_info() -> ToolInfo { + ToolInfo { + server_name: "codex-apps".to_string(), + supports_parallel_tool_calls: false, + server_origin: None, + callable_name: "_create_event".to_string(), + callable_namespace: "mcp__calendar__".to_string(), + namespace_description: Some("Plan events.".to_string()), + tool: rmcp::model::Tool { + name: "createEvent".to_string().into(), + title: Some("Create event".to_string()), + description: Some("Create a calendar event.".to_string().into()), + input_schema: Arc::new(rmcp::model::object(json!({ + "type": "object", + "properties": { + "start_time": { "type": "string" }, + "attendees": { "type": "string" } + }, + "additionalProperties": false + }))), + output_schema: None, + annotations: None, + execution: None, + icons: None, + meta: None, + }, + connector_id: None, + connector_name: Some("Calendar".to_string()), + plugin_display_names: vec![" Calendar plugin ".to_string(), " ".to_string()], + } +} diff --git a/codex-rs/core/src/tools/handlers/tool_search.rs b/codex-rs/core/src/tools/handlers/tool_search.rs index 844612d3ef..a857257213 100644 --- a/codex-rs/core/src/tools/handlers/tool_search.rs +++ b/codex-rs/core/src/tools/handlers/tool_search.rs @@ -5,6 +5,7 @@ use crate::tools::context::ToolSearchOutput; use crate::tools::handlers::tool_search_spec::create_tool_search_tool; use crate::tools::registry::ToolHandler; use crate::tools::tool_search_entry::ToolSearchEntry; +use crate::tools::tool_search_entry::ToolSearchInfo; use bm25::Document; use bm25::Language; use bm25::SearchEngine; @@ -24,10 +25,15 @@ pub struct ToolSearchHandler { } impl ToolSearchHandler { - pub(crate) fn new( - entries: Vec, - search_source_infos: Vec, - ) -> Self { + pub(crate) fn new(search_infos: Vec) -> Self { + let mut entries = Vec::with_capacity(search_infos.len()); + let mut search_source_infos = Vec::new(); + for search_info in search_infos { + entries.push(search_info.entry); + if let Some(source_info) = search_info.source_info { + search_source_infos.push(source_info); + } + } let documents: Vec> = entries .iter() .map(|entry| entry.search_text.clone()) @@ -130,24 +136,20 @@ impl ToolSearchHandler { #[cfg(test)] mod tests { use super::*; - use crate::session::tests::make_session_and_context; - use crate::tools::context::ToolCallSource; - use crate::tools::tool_search_entry::build_tool_search_entries; - use crate::turn_diff_tracker::TurnDiffTracker; + use crate::tools::handlers::DynamicToolHandler; + use crate::tools::handlers::McpHandler; use codex_mcp::ToolInfo; use codex_protocol::dynamic_tools::DynamicToolSpec; - use codex_protocol::models::SearchToolCallParams; use codex_tools::ResponsesApiNamespace; use codex_tools::ResponsesApiNamespaceTool; use codex_tools::ResponsesApiTool; use pretty_assertions::assert_eq; use rmcp::model::Tool; use std::sync::Arc; - use tokio::sync::Mutex; #[test] fn mixed_search_results_coalesce_mcp_namespaces() { - let dynamic_tools = vec![DynamicToolSpec { + let dynamic_tools = [DynamicToolSpec { namespace: Some("codex_app".to_string()), name: "automation_update".to_string(), description: "Create, update, view, or delete recurring automations.".to_string(), @@ -161,11 +163,25 @@ mod tests { }), defer_loading: true, }]; - let mcp_tools = vec![ + let mcp_tools = [ tool_info("calendar", "create_event", "Create events"), tool_info("calendar", "list_events", "List events"), ]; - let handler = handler_from_tools(Some(&mcp_tools), &dynamic_tools); + let mut search_infos = mcp_tools + .iter() + .map(|tool| { + McpHandler::new(tool.clone()) + .search_info() + .expect("MCP handler should return search info") + }) + .collect::>(); + search_infos.extend(dynamic_tools.iter().map(|tool| { + DynamicToolHandler::new(tool) + .expect("dynamic tool should convert") + .search_info() + .expect("dynamic handler should return search info") + })); + let handler = ToolSearchHandler::new(search_infos); let results = [ &handler.entries[0], &handler.entries[2], @@ -233,70 +249,6 @@ mod tests { ); } - #[tokio::test] - async fn omitted_limit_uses_default_tool_search_result_limit() { - let tool_count = TOOL_SEARCH_DEFAULT_LIMIT + 5; - let dynamic_tools = numbered_dynamic_tools(tool_count); - let handler = handler_from_tools(/*mcp_tools*/ None, &dynamic_tools); - - let output = tool_search_output(&handler, /*limit*/ None).await; - - assert_eq!(output.tools.len(), TOOL_SEARCH_DEFAULT_LIMIT); - } - - #[tokio::test] - async fn explicit_limit_controls_tool_search_result_count() { - let explicit_limit = 3; - let tool_count = TOOL_SEARCH_DEFAULT_LIMIT + explicit_limit; - let dynamic_tools = numbered_dynamic_tools(tool_count); - let handler = handler_from_tools(/*mcp_tools*/ None, &dynamic_tools); - - let output = tool_search_output(&handler, Some(explicit_limit)).await; - - assert_eq!(output.tools.len(), explicit_limit); - } - - async fn tool_search_output( - handler: &ToolSearchHandler, - limit: Option, - ) -> ToolSearchOutput { - let (session, turn) = make_session_and_context().await; - handler - .handle(ToolInvocation { - session: Arc::new(session), - turn: Arc::new(turn), - cancellation_token: tokio_util::sync::CancellationToken::new(), - tracker: Arc::new(Mutex::new(TurnDiffTracker::new())), - call_id: "call-tool-search".to_string(), - tool_name: ToolName::plain(TOOL_SEARCH_TOOL_NAME), - source: ToolCallSource::Direct, - payload: ToolPayload::ToolSearch { - arguments: SearchToolCallParams { - query: "calendar".to_string(), - limit, - }, - }, - }) - .await - .expect("tool_search should succeed") - } - - fn numbered_dynamic_tools(count: usize) -> Vec { - (0..count) - .map(|index| DynamicToolSpec { - namespace: None, - name: format!("calendar_tool_{index:03}"), - description: "Calendar search helper.".to_string(), - input_schema: serde_json::json!({ - "type": "object", - "properties": {}, - "additionalProperties": false, - }), - defer_loading: true, - }) - .collect() - } - fn tool_info(server_name: &str, tool_name: &str, description_prefix: &str) -> ToolInfo { ToolInfo { server_name: server_name.to_string(), @@ -325,14 +277,4 @@ mod tests { plugin_display_names: Vec::new(), } } - - fn handler_from_tools( - mcp_tools: Option<&[ToolInfo]>, - dynamic_tools: &[DynamicToolSpec], - ) -> ToolSearchHandler { - ToolSearchHandler::new( - build_tool_search_entries(mcp_tools, dynamic_tools), - Vec::new(), - ) - } } diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 65dcc863a9..c981e0059f 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -21,6 +21,7 @@ use crate::tools::handlers::extension_tools::BundledToolHandler; use crate::tools::handlers::extension_tools::extension_tool_spec; use crate::tools::hook_names::HookToolName; use crate::tools::tool_dispatch_trace::ToolDispatchTrace; +use crate::tools::tool_search_entry::ToolSearchInfo; use crate::util::error_or_panic; use codex_protocol::models::ResponseInputItem; use codex_protocol::protocol::EventMsg; @@ -43,6 +44,10 @@ pub trait ToolHandler: Send + Sync { None } + fn search_info(&self) -> Option { + None + } + fn supports_parallel_tool_calls(&self) -> bool { false } @@ -173,6 +178,8 @@ pub(crate) trait AnyToolHandler: Send + Sync { fn spec(&self) -> Option; + fn search_info(&self) -> Option; + fn supports_parallel_tool_calls(&self) -> bool; fn matches_kind(&self, payload: &ToolPayload) -> bool; @@ -209,6 +216,10 @@ where ToolHandler::spec(self) } + fn search_info(&self) -> Option { + ToolHandler::search_info(self) + } + fn supports_parallel_tool_calls(&self) -> bool { ToolHandler::supports_parallel_tool_calls(self) } diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 812997ae84..7bf3a095d9 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -42,7 +42,6 @@ pub(crate) fn build_specs_with_discoverable_tools( ) -> ToolRegistryBuilder { use crate::tools::handlers::UnavailableToolHandler; use crate::tools::handlers::unavailable_tool_message; - use crate::tools::tool_search_entry::build_tool_search_entries_for_config; let default_agent_type_description = crate::agent::role::spawn_tool_spec::build(&std::collections::BTreeMap::new()); @@ -56,16 +55,6 @@ pub(crate) fn build_specs_with_discoverable_tools( }; let default_wait_timeout_ms = DEFAULT_WAIT_TIMEOUT_MS.clamp(min_wait_timeout_ms, MAX_WAIT_TIMEOUT_MS); - let deferred_dynamic_tools = dynamic_tools - .iter() - .filter(|tool| tool.defer_loading) - .cloned() - .collect::>(); - let tool_search_entries = build_tool_search_entries_for_config( - config, - deferred_mcp_tools.as_deref(), - &deferred_dynamic_tools, - ); let mut builder = build_tool_registry_builder( config, ToolRegistryBuildParams { @@ -80,7 +69,6 @@ pub(crate) fn build_specs_with_discoverable_tools( min_timeout_ms: min_wait_timeout_ms, max_timeout_ms: MAX_WAIT_TIMEOUT_MS, }, - tool_search_entries: &tool_search_entries, }, ); let mut existing_spec_names = builder diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index 4b2eb3ed24..6087cd6539 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -53,12 +53,9 @@ use codex_protocol::openai_models::ConfigShellToolType; use codex_tools::ResponsesApiNamespaceTool; use codex_tools::ToolEnvironmentMode; use codex_tools::ToolName; -use codex_tools::ToolSearchSource; -use codex_tools::ToolSearchSourceInfo; use codex_tools::ToolSpec; use codex_tools::ToolsConfig; use codex_tools::collect_code_mode_exec_prompt_tool_definitions; -use codex_tools::collect_tool_search_source_infos; use codex_tools::default_namespace_description; use std::collections::BTreeMap; use std::collections::HashSet; @@ -94,11 +91,14 @@ pub fn build_tool_registry_builder( } let mut non_deferred_specs = Vec::new(); + let mut deferred_search_infos = Vec::new(); for handler in &handlers { let tool_name = handler.tool_name(); - if !all_deferred_tools.contains(&tool_name) - && let Some(spec) = handler.spec() - { + if all_deferred_tools.contains(&tool_name) { + if let Some(search_info) = handler.search_info() { + deferred_search_infos.push(search_info); + } + } else if let Some(spec) = handler.spec() { non_deferred_specs.push(spec); } } @@ -130,31 +130,8 @@ pub fn build_tool_registry_builder( builder.register_any_handler_without_spec(handler); } - if config.search_tool && config.namespace_tools && !all_deferred_tools.is_empty() { - let mut search_source_infos = params - .deferred_mcp_tools - .map(|mcp_tools| { - collect_tool_search_source_infos(mcp_tools.iter().map(|tool| ToolSearchSource { - server_name: tool.server_name.as_str(), - connector_name: tool.connector_name.as_deref(), - description: tool.namespace_description.as_deref(), - })) - }) - .unwrap_or_default(); - - if params.dynamic_tools.iter().any(|tool| { - all_deferred_tools.contains(&ToolName::new(tool.namespace.clone(), tool.name.clone())) - }) { - search_source_infos.push(ToolSearchSourceInfo { - name: "Dynamic tools".to_string(), - description: Some("Tools provided by the current Codex thread.".to_string()), - }); - } - - builder.register_handler(Arc::new(ToolSearchHandler::new( - params.tool_search_entries.to_vec(), - search_source_infos, - ))); + if config.search_tool && config.namespace_tools && !deferred_search_infos.is_empty() { + builder.register_handler(Arc::new(ToolSearchHandler::new(deferred_search_infos))); } for bundle in params.extension_tool_bundles.iter().cloned() { diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index b028c2a2a9..41159c6309 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -1709,7 +1709,7 @@ fn search_tool_requires_model_capability_and_enabled_feature() { } #[test] -fn search_tool_is_hidden_when_only_deferred_namespace_tools_are_available() { +fn no_search_tool_when_namespaces_disabled() { let model_info = search_capable_model_info(); let mut features = Features::with_defaults(); features.enable(Feature::ToolSearch); @@ -2415,7 +2415,6 @@ fn build_specs_with_discoverable_tools( dynamic_tools, default_agent_type_description: DEFAULT_AGENT_TYPE_DESCRIPTION, wait_agent_timeouts: wait_agent_timeout_options(), - tool_search_entries: &[], }, ); builder.build() diff --git a/codex-rs/core/src/tools/spec_plan_types.rs b/codex-rs/core/src/tools/spec_plan_types.rs index 53a53dca46..30cae6096d 100644 --- a/codex-rs/core/src/tools/spec_plan_types.rs +++ b/codex-rs/core/src/tools/spec_plan_types.rs @@ -14,7 +14,6 @@ pub struct ToolRegistryBuildParams<'a> { pub dynamic_tools: &'a [DynamicToolSpec], pub default_agent_type_description: &'a str, pub wait_agent_timeouts: WaitAgentTimeoutOptions, - pub tool_search_entries: &'a [crate::tools::tool_search_entry::ToolSearchEntry], } pub(crate) fn agent_type_description( diff --git a/codex-rs/core/src/tools/spec_tests.rs b/codex-rs/core/src/tools/spec_tests.rs index 52f92b281d..b4ef8f994d 100644 --- a/codex-rs/core/src/tools/spec_tests.rs +++ b/codex-rs/core/src/tools/spec_tests.rs @@ -19,7 +19,6 @@ use codex_protocol::protocol::SessionSource; use codex_tools::AdditionalProperties; use codex_tools::DiscoverableTool; use codex_tools::JsonSchema; -use codex_tools::LoadableToolSpec; use codex_tools::REQUEST_PLUGIN_INSTALL_TOOL_NAME; use codex_tools::ResponsesApiNamespaceTool; use codex_tools::ResponsesApiTool; @@ -40,7 +39,6 @@ use std::collections::BTreeMap; use std::path::PathBuf; use super::*; -use crate::tools::tool_search_entry::build_tool_search_entries_for_config; fn mcp_tool(name: &str, description: &str, input_schema: serde_json::Value) -> rmcp::model::Tool { rmcp::model::Tool { @@ -1000,63 +998,6 @@ async fn search_tool_registers_namespaced_mcp_tool_aliases() { assert!(registry.has_handler(&mcp_alias)); } -#[tokio::test] -async fn tool_search_entries_skip_mcp_outputs_when_namespace_tools_are_disabled() { - let model_info = search_capable_model_info().await; - let mut features = Features::with_defaults(); - features.enable(Feature::ToolSearch); - let available_models = Vec::new(); - let mut 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, - permission_profile: &PermissionProfile::Disabled, - windows_sandbox_level: WindowsSandboxLevel::Disabled, - }); - tools_config.namespace_tools = false; - let mcp_tools = vec![mcp_tool_info(mcp_tool( - "echo", - "Echo", - serde_json::json!({"type": "object"}), - ))]; - let dynamic_tools = vec![ - DynamicToolSpec { - namespace: Some("codex_app".to_string()), - name: "automation_update".to_string(), - description: "Create or update automations.".to_string(), - input_schema: serde_json::json!({"type": "object", "properties": {}}), - defer_loading: true, - }, - DynamicToolSpec { - namespace: None, - name: "plain_dynamic".to_string(), - description: "Plain dynamic tool.".to_string(), - input_schema: serde_json::json!({"type": "object", "properties": {}}), - defer_loading: true, - }, - ]; - - let entries = - build_tool_search_entries_for_config(&tools_config, Some(&mcp_tools), &dynamic_tools); - let outputs = entries - .into_iter() - .map(|entry| entry.output) - .collect::>(); - - assert_eq!(outputs.len(), 2); - assert!(outputs.iter().any(|output| match output { - LoadableToolSpec::Namespace(namespace) => namespace.name == "codex_app", - LoadableToolSpec::Function(_) => false, - })); - assert!(outputs.iter().any(|output| match output { - LoadableToolSpec::Function(tool) => tool.name == "plain_dynamic", - LoadableToolSpec::Namespace(_) => false, - })); -} - #[tokio::test] async fn direct_mcp_tools_register_namespaced_handlers() { let config = test_config().await; diff --git a/codex-rs/core/src/tools/tool_search_entry.rs b/codex-rs/core/src/tools/tool_search_entry.rs index 1467b51c65..7ecaf91a4d 100644 --- a/codex-rs/core/src/tools/tool_search_entry.rs +++ b/codex-rs/core/src/tools/tool_search_entry.rs @@ -1,11 +1,8 @@ -use crate::tools::flat_tool_name; -use codex_mcp::ToolInfo; -use codex_protocol::dynamic_tools::DynamicToolSpec; use codex_tools::LoadableToolSpec; -use codex_tools::ToolSearchResultSource; -use codex_tools::ToolsConfig; -use codex_tools::dynamic_tool_to_loadable_tool_spec; -use codex_tools::tool_search_result_source_to_loadable_tool_spec; +use codex_tools::ResponsesApiNamespaceTool; +use codex_tools::ToolSearchSourceInfo; +use codex_tools::ToolSpec; +use codex_tools::default_namespace_description; #[derive(Clone)] pub(crate) struct ToolSearchEntry { @@ -13,152 +10,48 @@ pub(crate) struct ToolSearchEntry { pub(crate) output: LoadableToolSpec, } -pub(crate) fn build_tool_search_entries( - mcp_tools: Option<&[ToolInfo]>, - dynamic_tools: &[DynamicToolSpec], -) -> Vec { - let mut entries = Vec::new(); +#[derive(Clone)] +pub(crate) struct ToolSearchInfo { + pub(crate) entry: ToolSearchEntry, + pub(crate) source_info: Option, +} - let mut mcp_tools = mcp_tools - .map(|tools| tools.iter().collect::>()) - .unwrap_or_default(); - mcp_tools.sort_by_key(|info| info.canonical_tool_name()); - for info in mcp_tools { - match mcp_tool_search_entry(info) { - Ok(entry) => entries.push(entry), - Err(error) => { - let tool_name = info.canonical_tool_name(); - tracing::error!( - "Failed to convert deferred MCP tool `{tool_name}` to OpenAI tool: {error:?}" - ); +impl ToolSearchInfo { + pub(crate) fn from_spec( + search_text: String, + spec: ToolSpec, + source_info: Option, + ) -> Option { + let output = match spec { + ToolSpec::Function(mut tool) => { + tool.defer_loading = Some(true); + tool.output_schema = None; + LoadableToolSpec::Function(tool) } - } - } - - let mut dynamic_tools = dynamic_tools.iter().collect::>(); - dynamic_tools.sort_by(|a, b| a.namespace.cmp(&b.namespace).then(a.name.cmp(&b.name))); - for tool in dynamic_tools { - match dynamic_tool_search_entry(tool) { - Ok(entry) => entries.push(entry), - Err(error) => { - tracing::error!( - "Failed to convert deferred dynamic tool {:?} to OpenAI tool: {error:?}", - tool.name - ); + ToolSpec::Namespace(mut namespace) => { + if namespace.description.trim().is_empty() { + namespace.description = default_namespace_description(&namespace.name); + } + for tool in &mut namespace.tools { + let ResponsesApiNamespaceTool::Function(tool) = tool; + tool.defer_loading = Some(true); + tool.output_schema = None; + } + LoadableToolSpec::Namespace(namespace) } - } + ToolSpec::ToolSearch { .. } + | ToolSpec::LocalShell {} + | ToolSpec::ImageGeneration { .. } + | ToolSpec::WebSearch { .. } + | ToolSpec::Freeform(_) => return None, + }; + + Some(Self { + entry: ToolSearchEntry { + search_text, + output, + }, + source_info, + }) } - - entries -} - -pub(crate) fn build_tool_search_entries_for_config( - config: &ToolsConfig, - mcp_tools: Option<&[ToolInfo]>, - dynamic_tools: &[DynamicToolSpec], -) -> Vec { - let mcp_tools = if config.namespace_tools { - mcp_tools - } else { - None - }; - let dynamic_tools = dynamic_tools.to_vec(); - build_tool_search_entries(mcp_tools, &dynamic_tools) -} - -fn mcp_tool_search_entry(info: &ToolInfo) -> Result { - Ok(ToolSearchEntry { - search_text: build_mcp_search_text(info), - output: tool_search_result_source_to_loadable_tool_spec(ToolSearchResultSource { - server_name: info.server_name.as_str(), - tool_namespace: info.callable_namespace.as_str(), - tool_name: info.callable_name.as_str(), - tool: &info.tool, - connector_name: info.connector_name.as_deref(), - description: info.namespace_description.as_deref(), - })?, - }) -} - -fn dynamic_tool_search_entry(tool: &DynamicToolSpec) -> Result { - Ok(ToolSearchEntry { - search_text: build_dynamic_search_text(tool), - output: dynamic_tool_to_loadable_tool_spec(tool)?, - }) -} - -fn build_mcp_search_text(info: &ToolInfo) -> String { - let tool_name = info.canonical_tool_name(); - let mut parts = vec![ - flat_tool_name(&tool_name).into_owned(), - info.callable_name.clone(), - info.tool.name.to_string(), - info.server_name.clone(), - ]; - - if let Some(title) = info.tool.title.as_deref() - && !title.trim().is_empty() - { - parts.push(title.to_string()); - } - - if let Some(description) = info.tool.description.as_deref() - && !description.trim().is_empty() - { - parts.push(description.to_string()); - } - - if let Some(connector_name) = info.connector_name.as_deref() - && !connector_name.trim().is_empty() - { - parts.push(connector_name.to_string()); - } - - if let Some(description) = info.namespace_description.as_deref() - && !description.trim().is_empty() - { - parts.push(description.to_string()); - } - - parts.extend( - info.plugin_display_names - .iter() - .map(String::as_str) - .map(str::trim) - .filter(|name| !name.is_empty()) - .map(str::to_string), - ); - - parts.extend( - info.tool - .input_schema - .get("properties") - .and_then(serde_json::Value::as_object) - .map(|map| map.keys().cloned().collect::>()) - .unwrap_or_default(), - ); - - parts.join(" ") -} - -fn build_dynamic_search_text(tool: &DynamicToolSpec) -> String { - let mut parts = vec![ - tool.name.clone(), - tool.name.replace('_', " "), - tool.description.clone(), - ]; - - if let Some(namespace) = &tool.namespace { - parts.push(namespace.clone()); - } - - parts.extend( - tool.input_schema - .get("properties") - .and_then(serde_json::Value::as_object) - .map(|map| map.keys().cloned().collect::>()) - .unwrap_or_default(), - ); - - parts.join(" ") } diff --git a/codex-rs/tools/src/lib.rs b/codex-rs/tools/src/lib.rs index 8c63e6e631..8722796aa5 100644 --- a/codex-rs/tools/src/lib.rs +++ b/codex-rs/tools/src/lib.rs @@ -47,7 +47,6 @@ pub use responses_api::ResponsesApiNamespaceTool; pub use responses_api::ResponsesApiTool; pub use responses_api::coalesce_loadable_tool_specs; pub use responses_api::default_namespace_description; -pub use responses_api::dynamic_tool_to_loadable_tool_spec; pub use responses_api::dynamic_tool_to_responses_api_tool; pub use responses_api::mcp_tool_to_deferred_responses_api_tool; pub use responses_api::mcp_tool_to_responses_api_tool; @@ -69,13 +68,9 @@ pub use tool_discovery::REQUEST_PLUGIN_INSTALL_TOOL_NAME; pub use tool_discovery::RequestPluginInstallEntry; pub use tool_discovery::TOOL_SEARCH_DEFAULT_LIMIT; pub use tool_discovery::TOOL_SEARCH_TOOL_NAME; -pub use tool_discovery::ToolSearchResultSource; -pub use tool_discovery::ToolSearchSource; pub use tool_discovery::ToolSearchSourceInfo; pub use tool_discovery::collect_request_plugin_install_entries; -pub use tool_discovery::collect_tool_search_source_infos; pub use tool_discovery::filter_request_plugin_install_discoverable_tools_for_client; -pub use tool_discovery::tool_search_result_source_to_loadable_tool_spec; pub use tool_spec::ResponsesApiWebSearchFilters; pub use tool_spec::ResponsesApiWebSearchUserLocation; pub use tool_spec::ToolSpec; diff --git a/codex-rs/tools/src/responses_api.rs b/codex-rs/tools/src/responses_api.rs index a5b26abae4..013c491cf6 100644 --- a/codex-rs/tools/src/responses_api.rs +++ b/codex-rs/tools/src/responses_api.rs @@ -74,21 +74,6 @@ pub fn dynamic_tool_to_responses_api_tool( )?)) } -pub fn dynamic_tool_to_loadable_tool_spec( - tool: &DynamicToolSpec, -) -> Result { - let output_tool = dynamic_tool_to_responses_api_tool(tool)?; - Ok(match tool.namespace.as_ref() { - Some(namespace) => LoadableToolSpec::Namespace(ResponsesApiNamespace { - name: namespace.clone(), - // the user doesn't provide a description for dynamic tools, so we use the default - description: default_namespace_description(namespace), - tools: vec![ResponsesApiNamespaceTool::Function(output_tool)], - }), - None => LoadableToolSpec::Function(output_tool), - }) -} - pub fn coalesce_loadable_tool_specs( specs: impl IntoIterator, ) -> Vec { diff --git a/codex-rs/tools/src/tool_discovery.rs b/codex-rs/tools/src/tool_discovery.rs index d95b9f7e32..a5beb768a2 100644 --- a/codex-rs/tools/src/tool_discovery.rs +++ b/codex-rs/tools/src/tool_discovery.rs @@ -1,9 +1,3 @@ -use crate::LoadableToolSpec; -use crate::ResponsesApiNamespace; -use crate::ResponsesApiNamespaceTool; -use crate::ToolName; -use crate::default_namespace_description; -use crate::mcp_tool_to_deferred_responses_api_tool; use codex_app_server_protocol::AppInfo; use serde::Deserialize; use serde::Serialize; @@ -19,23 +13,6 @@ pub struct ToolSearchSourceInfo { pub description: Option, } -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub struct ToolSearchSource<'a> { - pub server_name: &'a str, - pub connector_name: Option<&'a str>, - pub description: Option<&'a str>, -} - -#[derive(Clone, Copy, Debug, PartialEq)] -pub struct ToolSearchResultSource<'a> { - pub server_name: &'a str, - pub tool_namespace: &'a str, - pub tool_name: &'a str, - pub tool: &'a rmcp::model::Tool, - pub connector_name: Option<&'a str>, - pub description: Option<&'a str>, -} - #[derive(Clone, Copy, Debug, Deserialize, Serialize, PartialEq, Eq)] #[serde(rename_all = "snake_case")] pub enum DiscoverableToolType { @@ -133,78 +110,6 @@ pub struct RequestPluginInstallEntry { pub app_connector_ids: Vec, } -pub fn tool_search_result_source_to_loadable_tool_spec( - source: ToolSearchResultSource<'_>, -) -> Result { - Ok(LoadableToolSpec::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)?], - })) -} - -fn tool_search_result_source_namespace_description(source: ToolSearchResultSource<'_>) -> String { - source - .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)) -} - -fn tool_search_result_source_to_namespace_tool( - source: ToolSearchResultSource<'_>, -) -> Result { - 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>( - searchable_tools: impl IntoIterator>, -) -> Vec { - searchable_tools - .into_iter() - .filter_map(|tool| { - if let Some(name) = tool - .connector_name - .map(str::trim) - .filter(|connector_name| !connector_name.is_empty()) - { - return Some(ToolSearchSourceInfo { - name: name.to_string(), - description: tool - .description - .map(str::trim) - .filter(|description| !description.is_empty()) - .map(str::to_string), - }); - } - - let name = tool.server_name.trim(); - if name.is_empty() { - return None; - } - - Some(ToolSearchSourceInfo { - name: name.to_string(), - description: tool - .description - .map(str::trim) - .filter(|description| !description.is_empty()) - .map(str::to_string), - }) - }) - .collect() -} - pub fn collect_request_plugin_install_entries( discoverable_tools: &[DiscoverableTool], ) -> Vec {