diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index efe12def1d..88d148351e 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -325,10 +325,6 @@ pub struct ToolRegistry { impl ToolRegistry { fn new(tools: HashMap>) -> Self { - let tools = tools - .into_iter() - .map(|(name, tool)| (name.with_default_namespace(), tool)) - .collect(); Self { tools } } @@ -356,8 +352,7 @@ impl ToolRegistry { where T: CoreToolRuntime + 'static, { - let name = handler.tool_name(); - Self::new(HashMap::from([(name, handler as Arc)])) + Self::from_tools([handler as Arc]) } fn tool(&self, name: &ToolName) -> Option> { diff --git a/codex-rs/core/src/tools/registry_tests.rs b/codex-rs/core/src/tools/registry_tests.rs index 7fe7787311..a7c8e2549d 100644 --- a/codex-rs/core/src/tools/registry_tests.rs +++ b/codex-rs/core/src/tools/registry_tests.rs @@ -151,10 +151,8 @@ fn handler_normalizes_only_the_default_namespace() { let namespaced_handler = Arc::new(TestHandler { tool_name: namespaced_name.clone(), }) as Arc; - let registry = ToolRegistry::new(HashMap::from([ - (plain_name.clone(), Arc::clone(&plain_handler)), - (namespaced_name.clone(), Arc::clone(&namespaced_handler)), - ])); + let registry = + ToolRegistry::from_tools([Arc::clone(&plain_handler), Arc::clone(&namespaced_handler)]); let plain = registry.tool(&plain_name); let default_namespaced = registry.tool(&codex_tools::ToolName::function(tool_name)); @@ -164,10 +162,7 @@ fn handler_normalizes_only_the_default_namespace() { tool_name, )); - assert_eq!(plain.is_some(), true); - assert_eq!(default_namespaced.is_some(), true); - assert_eq!(namespaced.is_some(), true); - assert_eq!(missing_namespaced.is_none(), true); + assert!(missing_namespaced.is_none()); assert!( plain .as_ref() @@ -397,10 +392,7 @@ async fn dispatch_notifies_tool_lifecycle_contributors() -> anyhow::Result<()> { tool_name: failing_tool.clone(), result: LifecycleTestResult::Err, }) as Arc; - let registry = ToolRegistry::new(HashMap::from([ - (ok_tool.clone(), ok_handler), - (failing_tool.clone(), failing_handler), - ])); + let registry = ToolRegistry::from_tools([ok_handler, failing_handler]); let session = Arc::new(session); let turn = Arc::new(turn); diff --git a/codex-rs/core/tests/suite/responses_lite.rs b/codex-rs/core/tests/suite/responses_lite.rs index 64c0c76429..947ac6454c 100644 --- a/codex-rs/core/tests/suite/responses_lite.rs +++ b/codex-rs/core/tests/suite/responses_lite.rs @@ -120,13 +120,6 @@ async fn responses_lite_uses_input_items_for_instructions_and_tools() -> Result< Ok(()) } -fn functions_namespace(tools: &[Value]) -> Option<&Value> { - tools.iter().find(|tool| { - tool.get("type").and_then(Value::as_str) == Some("namespace") - && tool.get("name").and_then(Value::as_str) == Some("functions") - }) -} - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn responses_lite_rejects_oversized_additional_tools_item() -> Result<()> { skip_if_no_network!(Ok(())); @@ -279,7 +272,12 @@ async fn responses_lite_uses_standalone_web_search_and_image_generation() -> Res assert!(body.get("tools").is_none()); let tools = additional_tools(&body)?; assert!(!tools.is_empty()); - let function_tools = functions_namespace(tools) + let function_tools = tools + .iter() + .find(|tool| { + tool.get("type").and_then(Value::as_str) == Some("namespace") + && tool.get("name").and_then(Value::as_str) == Some("functions") + }) .and_then(|namespace| namespace.get("tools")) .and_then(Value::as_array) .context("Responses Lite should group default tools under functions")?; diff --git a/codex-rs/core/tests/suite/tool_harness.rs b/codex-rs/core/tests/suite/tool_harness.rs index 0f6de7fd1a..373ae8d4ef 100644 --- a/codex-rs/core/tests/suite/tool_harness.rs +++ b/codex-rs/core/tests/suite/tool_harness.rs @@ -30,7 +30,6 @@ use core_test_support::test_codex::turn_permission_fields; use core_test_support::wait_for_event; use serde_json::Value; use serde_json::json; -use test_case::test_case; fn call_output(req: &ResponsesRequest, call_id: &str) -> (String, Option) { let raw = req.function_call_output(call_id); assert_eq!( @@ -136,11 +135,8 @@ async fn shell_command_tool_executes_command_and_streams_output() -> anyhow::Res Ok(()) } -#[test_case(None; "omitted_namespace")] -#[test_case(Some(""); "empty_namespace")] -#[test_case(Some("functions"); "explicit_functions_namespace")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn update_plan_tool_emits_plan_update_event(namespace: Option<&str>) -> anyhow::Result<()> { +async fn update_plan_tool_emits_plan_update_event() -> anyhow::Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; @@ -163,15 +159,9 @@ async fn update_plan_tool_emits_plan_update_event(namespace: Option<&str>) -> an }) .to_string(); - let function_call = match namespace { - Some(namespace) => { - ev_function_call_with_namespace(call_id, namespace, "update_plan", &plan_args) - } - None => ev_function_call(call_id, "update_plan", &plan_args), - }; let first_response = sse(vec![ ev_response_created("resp-1"), - function_call, + ev_function_call_with_namespace(call_id, "functions", "update_plan", &plan_args), ev_completed("resp-1"), ]); responses::mount_sse_once(&server, first_response).await; diff --git a/codex-rs/protocol/src/tool_name.rs b/codex-rs/protocol/src/tool_name.rs index 34c7849322..b5e13edb8c 100644 --- a/codex-rs/protocol/src/tool_name.rs +++ b/codex-rs/protocol/src/tool_name.rs @@ -48,9 +48,10 @@ impl ToolName { } pub fn is_default_namespace(&self) -> bool { - self.namespace - .as_deref() - .is_none_or(|namespace| namespace.is_empty() || namespace == DEFAULT_FUNCTION_NAMESPACE) + matches!( + self.namespace.as_deref(), + None | Some("") | Some(DEFAULT_FUNCTION_NAMESPACE) + ) } } diff --git a/codex-rs/tools/src/tool_spec.rs b/codex-rs/tools/src/tool_spec.rs index 1c988bf363..19387ddd8f 100644 --- a/codex-rs/tools/src/tool_spec.rs +++ b/codex-rs/tools/src/tool_spec.rs @@ -96,24 +96,31 @@ pub fn create_tools_json_for_responses_api( pub fn create_tools_json_for_responses_lite( tools: &[ToolSpec], ) -> Result, serde_json::Error> { - let mut functions = Vec::new(); - let mut functions_description = None; + let mut functions = ResponsesApiNamespace { + name: DEFAULT_FUNCTION_NAMESPACE.to_string(), + description: default_namespace_description(DEFAULT_FUNCTION_NAMESPACE), + tools: Vec::new(), + }; let mut functions_index = None; let mut tools_json = Vec::new(); for tool in tools { match tool { ToolSpec::Function(tool) => { - functions.push(ResponsesApiNamespaceTool::Function(tool.clone())); + functions + .tools + .push(ResponsesApiNamespaceTool::Function(tool.clone())); } ToolSpec::Freeform(tool) => { - functions.push(ResponsesApiNamespaceTool::Freeform(tool.clone())); + functions + .tools + .push(ResponsesApiNamespaceTool::Freeform(tool.clone())); } ToolSpec::Namespace(namespace) if namespace.name == DEFAULT_FUNCTION_NAMESPACE => { if !namespace.description.trim().is_empty() { - functions_description = Some(namespace.description.clone()); + functions.description = namespace.description.clone(); } - functions.extend(namespace.tools.clone()); + functions.tools.extend(namespace.tools.clone()); } tool => { tools_json.push(serde_json::to_value(tool)?); @@ -124,16 +131,11 @@ pub fn create_tools_json_for_responses_lite( } if let Some(functions_index) = functions_index - && !functions.is_empty() + && !functions.tools.is_empty() { tools_json.insert( functions_index, - serde_json::to_value(ToolSpec::Namespace(ResponsesApiNamespace { - name: DEFAULT_FUNCTION_NAMESPACE.to_string(), - description: functions_description - .unwrap_or_else(|| default_namespace_description(DEFAULT_FUNCTION_NAMESPACE)), - tools: functions, - }))?, + serde_json::to_value(ToolSpec::Namespace(functions))?, ); }