From e6939e3969497e61afde00575fc36ac7fba4aac8 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Thu, 14 May 2026 00:37:48 +0200 Subject: [PATCH] feat: namespace in ext (#22556) --- codex-rs/core/src/tools/registry.rs | 13 ----- codex-rs/core/src/tools/router_tests.rs | 54 ++++++++++++--------- codex-rs/core/src/tools/spec_plan.rs | 59 +++++++++++++++++------ codex-rs/ext/memories/Cargo.toml | 2 +- codex-rs/ext/memories/src/lib.rs | 7 +-- codex-rs/ext/memories/src/tests.rs | 23 +++++---- codex-rs/ext/memories/src/tools/list.rs | 7 +-- codex-rs/ext/memories/src/tools/mod.rs | 22 ++++++++- codex-rs/ext/memories/src/tools/read.rs | 7 +-- codex-rs/ext/memories/src/tools/search.rs | 7 +-- 10 files changed, 128 insertions(+), 73 deletions(-) diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 6486e7db71..7e565bd312 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -18,12 +18,10 @@ use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; use crate::tools::flat_tool_name; -use crate::tools::handlers::extension_tools::ExtensionToolHandler; 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_extension_api::ExtensionToolExecutor; use codex_protocol::models::ResponseInputItem; use codex_protocol::protocol::EventMsg; use codex_tools::ToolName; @@ -671,17 +669,6 @@ impl ToolRegistryBuilder { self.handlers.insert(name, handler); } - pub fn register_extension_tool_executor(&mut self, executor: Arc) { - let tool_name = executor.tool_name(); - if self.handlers.contains_key(&tool_name) { - warn!("Skipping extension tool `{tool_name}`: handler already registered"); - return; - } - - let handler: Arc = Arc::new(ExtensionToolHandler::new(executor)); - self.register_tool_internal(handler, /*include_spec*/ true); - } - pub fn build(self) -> (Vec, ToolRegistry) { let registry = ToolRegistry::new(self.handlers); (self.specs, registry) diff --git a/codex-rs/core/src/tools/router_tests.rs b/codex-rs/core/src/tools/router_tests.rs index b154585419..921b52a86e 100644 --- a/codex-rs/core/src/tools/router_tests.rs +++ b/codex-rs/core/src/tools/router_tests.rs @@ -15,9 +15,11 @@ use codex_protocol::dynamic_tools::DynamicToolSpec; use codex_protocol::models::FunctionCallOutputBody; use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ResponseItem; +use codex_tools::ResponsesApiNamespace; use codex_tools::ResponsesApiNamespaceTool; use codex_tools::ToolName; use codex_tools::ToolSpec; +use codex_tools::default_namespace_description; use pretty_assertions::assert_eq; use serde_json::json; use tokio_util::sync::CancellationToken; @@ -44,25 +46,29 @@ struct ExtensionEchoExecutor; impl ExtensionToolExecutor for ExtensionEchoExecutor { fn tool_name(&self) -> ToolName { - ToolName::plain("extension_echo") + ToolName::namespaced("extension/", "echo") } fn spec(&self) -> Option { - Some(ToolSpec::Function(ResponsesApiTool { - name: "extension_echo".to_string(), - description: "Echoes arguments through an extension tool.".to_string(), - strict: true, - parameters: codex_extension_api::parse_tool_input_schema(&json!({ - "type": "object", - "properties": { - "message": { "type": "string" }, - }, - "required": ["message"], - "additionalProperties": false, - })) - .expect("extension schema should parse"), - output_schema: None, - defer_loading: None, + Some(ToolSpec::Namespace(ResponsesApiNamespace { + name: "extension/".to_string(), + description: default_namespace_description("extension/"), + tools: vec![ResponsesApiNamespaceTool::Function(ResponsesApiTool { + name: "echo".to_string(), + description: "Echoes arguments through an extension tool.".to_string(), + strict: true, + parameters: codex_extension_api::parse_tool_input_schema(&json!({ + "type": "object", + "properties": { + "message": { "type": "string" }, + }, + "required": ["message"], + "additionalProperties": false, + })) + .expect("extension schema should parse"), + output_schema: None, + defer_loading: None, + })], })) } @@ -333,17 +339,21 @@ async fn extension_tool_executors_are_model_visible_and_dispatchable() -> anyhow ); assert!( - router - .model_visible_specs() - .iter() - .any(|spec| spec.name() == "extension_echo"), + router.model_visible_specs().iter().any( + |spec| matches!(spec, ToolSpec::Namespace(namespace) + if namespace.name == "extension/" + && namespace.tools.iter().any(|tool| matches!( + tool, + ResponsesApiNamespaceTool::Function(tool) if tool.name == "echo" + ))) + ), "expected extension-provided tool to be visible to the model" ); let call = ToolRouter::build_tool_call(ResponseItem::FunctionCall { id: None, - name: "extension_echo".to_string(), - namespace: None, + name: "echo".to_string(), + namespace: Some("extension/".to_string()), arguments: json!({ "message": "hello" }).to_string(), call_id: "call-extension".to_string(), })? diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index 3d37d58b1b..db530f3477 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -24,6 +24,7 @@ use crate::tools::handlers::ViewImageHandler; use crate::tools::handlers::WriteStdinHandler; use crate::tools::handlers::agent_jobs::ReportAgentJobResultHandler; use crate::tools::handlers::agent_jobs::SpawnAgentsOnCsvHandler; +use crate::tools::handlers::extension_tools::ExtensionToolHandler; use crate::tools::handlers::multi_agents::CloseAgentHandler; use crate::tools::handlers::multi_agents::ResumeAgentHandler; use crate::tools::handlers::multi_agents::SendInputHandler; @@ -49,13 +50,17 @@ use crate::tools::spec_plan_types::agent_type_description; use codex_extension_api::ExtensionToolExecutor; use codex_protocol::openai_models::ConfigShellToolType; use codex_tools::ResponsesApiNamespaceTool; +use codex_tools::TOOL_SEARCH_TOOL_NAME; use codex_tools::ToolEnvironmentMode; +use codex_tools::ToolName; use codex_tools::ToolSpec; use codex_tools::ToolsConfig; use codex_tools::collect_code_mode_exec_prompt_tool_definitions; use codex_tools::default_namespace_description; use std::collections::BTreeMap; +use std::collections::HashSet; use std::sync::Arc; +use tracing::warn; pub fn build_tool_registry_builder( config: &ToolsConfig, @@ -70,7 +75,6 @@ pub fn build_tool_registry_builder( for handler in build_code_mode_handlers( config, &handlers, - params.extension_tool_executors, config.search_tool && deferred_tools_available, ) { builder.register_tool(handler); @@ -130,39 +134,28 @@ pub fn build_tool_registry_builder( builder.register_handler(Arc::new(ToolSearchHandler::new(deferred_search_infos))); } - for executor in params.extension_tool_executors.iter().cloned() { - builder.register_extension_tool_executor(executor); - } - builder } fn build_code_mode_handlers( config: &ToolsConfig, handlers: &[Arc], - extension_tool_executors: &[Arc], deferred_tools_available: bool, ) -> Vec> { if !config.code_mode_enabled { return vec![]; } - let mut code_mode_nested_tool_specs = handlers + let code_mode_nested_tool_specs = handlers .iter() .filter_map(|handler| { if handler.exposure() == ToolExposure::DirectModelOnly { return None; } - let spec = handler.spec()?; - Some(spec) + handler.spec() }) .collect::>(); - code_mode_nested_tool_specs.extend( - extension_tool_executors - .iter() - .filter_map(|executor| executor.spec()), - ); let namespace_descriptions = code_mode_namespace_descriptions(&code_mode_nested_tool_specs); let mut enabled_tools = collect_code_mode_exec_prompt_tool_definitions(code_mode_nested_tool_specs.iter()); @@ -436,9 +429,47 @@ fn collect_handler_tools( handlers.push(handler); } + append_extension_tool_handlers(config, params.extension_tool_executors, &mut handlers); + handlers } +fn append_extension_tool_handlers( + config: &ToolsConfig, + executors: &[Arc], + handlers: &mut Vec>, +) { + if executors.is_empty() { + return; + } + + let mut reserved_tool_names = handlers + .iter() + .map(|handler| handler.tool_name()) + .collect::>(); + if config.code_mode_enabled { + reserved_tool_names.insert(ToolName::plain(codex_code_mode::PUBLIC_TOOL_NAME)); + reserved_tool_names.insert(ToolName::plain(codex_code_mode::WAIT_TOOL_NAME)); + } + if config.search_tool + && config.namespace_tools + && handlers + .iter() + .any(|handler| handler.exposure() == ToolExposure::Deferred) + { + reserved_tool_names.insert(ToolName::plain(TOOL_SEARCH_TOOL_NAME)); + } + + for executor in executors.iter().cloned() { + let tool_name = executor.tool_name(); + if !reserved_tool_names.insert(tool_name.clone()) { + warn!("Skipping extension tool `{tool_name}`: handler already registered"); + continue; + } + handlers.push(Arc::new(ExtensionToolHandler::new(executor))); + } +} + fn multi_agent_v2_handler( handler: impl RegisteredTool + 'static, exposure: ToolExposure, diff --git a/codex-rs/ext/memories/Cargo.toml b/codex-rs/ext/memories/Cargo.toml index 542eaa6bc5..557a25fea8 100644 --- a/codex-rs/ext/memories/Cargo.toml +++ b/codex-rs/ext/memories/Cargo.toml @@ -17,6 +17,7 @@ codex-core = { workspace = true } codex-extension-api = { workspace = true } codex-features = { workspace = true } codex-memories-read = { workspace = true } +codex-tools = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-output-truncation = { workspace = true } schemars = { workspace = true } @@ -26,7 +27,6 @@ thiserror = { workspace = true } tokio = { workspace = true, features = ["fs"] } [dev-dependencies] -codex-tools = { workspace = true } pretty_assertions = { workspace = true } tempfile = { workspace = true } tokio = { workspace = true, features = ["fs", "macros"] } diff --git a/codex-rs/ext/memories/src/lib.rs b/codex-rs/ext/memories/src/lib.rs index 4b6f74c571..bb7b07bc69 100644 --- a/codex-rs/ext/memories/src/lib.rs +++ b/codex-rs/ext/memories/src/lib.rs @@ -12,9 +12,10 @@ pub(crate) const DEFAULT_SEARCH_MAX_RESULTS: usize = 200; pub(crate) const MAX_SEARCH_RESULTS: usize = 200; pub(crate) const DEFAULT_READ_MAX_TOKENS: usize = 20_000; -pub(crate) const LIST_TOOL_NAME: &str = "memory_list"; -pub(crate) const READ_TOOL_NAME: &str = "memory_read"; -pub(crate) const SEARCH_TOOL_NAME: &str = "memory_search"; +pub(crate) const MEMORY_TOOLS_NAMESPACE: &str = "memories/"; +pub(crate) const LIST_TOOL_NAME: &str = "list"; +pub(crate) const READ_TOOL_NAME: &str = "read"; +pub(crate) const SEARCH_TOOL_NAME: &str = "search"; #[cfg(test)] mod tests; diff --git a/codex-rs/ext/memories/src/tests.rs b/codex-rs/ext/memories/src/tests.rs index 3d9cd9a79b..47c58f4686 100644 --- a/codex-rs/ext/memories/src/tests.rs +++ b/codex-rs/ext/memories/src/tests.rs @@ -62,15 +62,15 @@ fn tools_are_contributed_when_enabled() { let tool_names = extension .tools(&ExtensionData::new("session"), &thread_store) .into_iter() - .map(|tool| tool.tool_name().to_string()) + .map(|tool| tool.tool_name()) .collect::>(); assert_eq!( tool_names, vec![ - "memory_list".to_string(), - "memory_read".to_string(), - "memory_search".to_string() + memory_tool_name(crate::LIST_TOOL_NAME), + memory_tool_name(crate::READ_TOOL_NAME), + memory_tool_name(crate::SEARCH_TOOL_NAME), ] ); } @@ -135,7 +135,7 @@ async fn read_tool_reads_memory_file() { let output = tool .handle(ToolCall { call_id: "call-1".to_string(), - tool_name: ToolName::plain(crate::READ_TOOL_NAME), + tool_name: memory_tool_name(crate::READ_TOOL_NAME), payload: payload.clone(), }) .await @@ -177,7 +177,7 @@ async fn search_tool_accepts_multiple_queries() { let output = tool .handle(ToolCall { call_id: "call-1".to_string(), - tool_name: ToolName::plain(crate::SEARCH_TOOL_NAME), + tool_name: memory_tool_name(crate::SEARCH_TOOL_NAME), payload: payload.clone(), }) .await @@ -245,7 +245,7 @@ async fn search_tool_accepts_windowed_all_match_mode() { let output = tool .handle(ToolCall { call_id: "call-1".to_string(), - tool_name: ToolName::plain(crate::SEARCH_TOOL_NAME), + tool_name: memory_tool_name(crate::SEARCH_TOOL_NAME), payload: payload.clone(), }) .await @@ -293,7 +293,7 @@ async fn search_tool_rejects_legacy_single_query() { let err = tool .handle(ToolCall { call_id: "call-1".to_string(), - tool_name: ToolName::plain(crate::SEARCH_TOOL_NAME), + tool_name: memory_tool_name(crate::SEARCH_TOOL_NAME), payload, }) .await @@ -304,8 +304,13 @@ async fn search_tool_rejects_legacy_single_query() { } fn memory_tool(memory_root: &Path, tool_name: &str) -> Arc { + let expected_tool_name = memory_tool_name(tool_name); crate::tools::memory_tools(LocalMemoriesBackend::from_memory_root(memory_root)) .into_iter() - .find(|tool| tool.tool_name().to_string() == tool_name) + .find(|tool| tool.tool_name() == expected_tool_name) .unwrap_or_else(|| panic!("{tool_name} tool should be registered")) } + +fn memory_tool_name(tool_name: &str) -> ToolName { + ToolName::namespaced(crate::MEMORY_TOOLS_NAMESPACE, tool_name) +} diff --git a/codex-rs/ext/memories/src/tools/list.rs b/codex-rs/ext/memories/src/tools/list.rs index bd1599047e..e2bc8304da 100644 --- a/codex-rs/ext/memories/src/tools/list.rs +++ b/codex-rs/ext/memories/src/tools/list.rs @@ -17,7 +17,8 @@ use crate::backend::MemoriesBackend; use super::backend_error_to_function_call; use super::clamp_max_results; -use super::function_tool; +use super::memory_function_tool; +use super::memory_tool_name; use super::parse_args; #[derive(Deserialize, JsonSchema)] @@ -39,11 +40,11 @@ where B: MemoriesBackend, { fn tool_name(&self) -> ToolName { - ToolName::plain(LIST_TOOL_NAME) + memory_tool_name(LIST_TOOL_NAME) } fn spec(&self) -> Option { - Some(function_tool::( + Some(memory_function_tool::( LIST_TOOL_NAME, "List immediate files and directories under a path in the Codex memories store.", )) diff --git a/codex-rs/ext/memories/src/tools/mod.rs b/codex-rs/ext/memories/src/tools/mod.rs index b488cc5e87..f95922b28f 100644 --- a/codex-rs/ext/memories/src/tools/mod.rs +++ b/codex-rs/ext/memories/src/tools/mod.rs @@ -4,12 +4,17 @@ use codex_extension_api::ExtensionToolExecutor; use codex_extension_api::FunctionCallError; use codex_extension_api::ResponsesApiTool; use codex_extension_api::ToolCall; +use codex_extension_api::ToolName; use codex_extension_api::ToolSpec; use codex_extension_api::parse_tool_input_schema; +use codex_tools::ResponsesApiNamespace; +use codex_tools::ResponsesApiNamespaceTool; +use codex_tools::default_namespace_description; use schemars::JsonSchema; use serde::Deserialize; use serde_json::Value; +use crate::MEMORY_TOOLS_NAMESPACE; use crate::backend::MemoriesBackend; use crate::backend::MemoriesBackendError; use crate::schema; @@ -33,8 +38,15 @@ where ] } -fn function_tool(name: &str, description: &str) -> ToolSpec { - ToolSpec::Function(ResponsesApiTool { +pub(super) fn memory_tool_name(name: &str) -> ToolName { + ToolName::namespaced(MEMORY_TOOLS_NAMESPACE, name) +} + +pub(super) fn memory_function_tool( + name: &str, + description: &str, +) -> ToolSpec { + let tool = ResponsesApiTool { name: name.to_string(), description: description.to_string(), strict: false, @@ -42,6 +54,12 @@ fn function_tool(name: &str, description: &str) -> parameters: parse_tool_input_schema(&schema::input_schema_for::()) .unwrap_or_else(|err| panic!("generated input schema for {name} should parse: {err}")), output_schema: Some(schema::output_schema_for::()), + }; + + ToolSpec::Namespace(ResponsesApiNamespace { + name: MEMORY_TOOLS_NAMESPACE.to_string(), + description: default_namespace_description(MEMORY_TOOLS_NAMESPACE), + tools: vec![ResponsesApiNamespaceTool::Function(tool)], }) } diff --git a/codex-rs/ext/memories/src/tools/read.rs b/codex-rs/ext/memories/src/tools/read.rs index ec411a9947..c706c609dc 100644 --- a/codex-rs/ext/memories/src/tools/read.rs +++ b/codex-rs/ext/memories/src/tools/read.rs @@ -15,7 +15,8 @@ use crate::backend::ReadMemoryRequest; use crate::backend::ReadMemoryResponse; use super::backend_error_to_function_call; -use super::function_tool; +use super::memory_function_tool; +use super::memory_tool_name; use super::parse_args; #[derive(Deserialize, JsonSchema)] @@ -38,11 +39,11 @@ where B: MemoriesBackend, { fn tool_name(&self) -> ToolName { - ToolName::plain(READ_TOOL_NAME) + memory_tool_name(READ_TOOL_NAME) } fn spec(&self) -> Option { - Some(function_tool::( + Some(memory_function_tool::( READ_TOOL_NAME, "Read a Codex memory file by relative path, optionally starting at a 1-indexed line offset and limiting the number of lines returned.", )) diff --git a/codex-rs/ext/memories/src/tools/search.rs b/codex-rs/ext/memories/src/tools/search.rs index 38863bcad5..8042a8d45e 100644 --- a/codex-rs/ext/memories/src/tools/search.rs +++ b/codex-rs/ext/memories/src/tools/search.rs @@ -18,7 +18,8 @@ use crate::backend::SearchMemoriesResponse; use super::backend_error_to_function_call; use super::clamp_max_results; -use super::function_tool; +use super::memory_function_tool; +use super::memory_tool_name; use super::parse_args; #[derive(Debug, Deserialize, JsonSchema)] @@ -47,11 +48,11 @@ where B: MemoriesBackend, { fn tool_name(&self) -> ToolName { - ToolName::plain(SEARCH_TOOL_NAME) + memory_tool_name(SEARCH_TOOL_NAME) } fn spec(&self) -> Option { - Some(function_tool::( + Some(memory_function_tool::( SEARCH_TOOL_NAME, "Search Codex memory files for substring matches, optionally normalizing separators or requiring all query substrings on the same line or within a line window.", ))