diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 25f2d11e9a..235cb6fef3 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -25,7 +25,6 @@ use crate::util::error_or_panic; use codex_protocol::models::ResponseInputItem; use codex_protocol::protocol::EventMsg; use codex_tool_api::ToolBundle as ExtensionToolBundle; -use codex_tools::ConfiguredToolSpec; use codex_tools::ToolName; use codex_tools::ToolSpec; use codex_utils_readiness::Readiness; @@ -554,7 +553,7 @@ impl ToolRegistry { pub struct ToolRegistryBuilder { handlers: HashMap>, - specs: Vec, + specs: Vec, code_mode_enabled: bool, } @@ -567,14 +566,13 @@ impl ToolRegistryBuilder { } } - pub(crate) fn push_spec(&mut self, spec: ToolSpec, supports_parallel_tool_calls: bool) { + pub(crate) fn push_spec(&mut self, spec: ToolSpec) { let spec = if self.code_mode_enabled { codex_tools::augment_tool_spec_for_code_mode(spec) } else { spec }; - self.specs - .push(ConfiguredToolSpec::new(spec, supports_parallel_tool_calls)); + self.specs.push(spec); } pub fn register_handler(&mut self, handler: Arc) @@ -588,8 +586,7 @@ impl ToolRegistryBuilder { } if let Some(spec) = handler.spec() { - let supports_parallel_tool_calls = handler.supports_parallel_tool_calls(); - self.push_spec(spec, supports_parallel_tool_calls); + self.push_spec(spec); } let handler: Arc = handler; @@ -612,17 +609,17 @@ impl ToolRegistryBuilder { return; } }; - self.push_spec(spec.clone(), /*supports_parallel_tool_calls*/ false); + self.push_spec(spec.clone()); let handler: Arc = Arc::new(BundledToolHandler::new(bundle, spec)); self.handlers.insert(tool_name, handler); } - pub(crate) fn specs(&self) -> &[ConfiguredToolSpec] { + pub(crate) fn specs(&self) -> &[ToolSpec] { &self.specs } - pub fn build(self) -> (Vec, ToolRegistry) { + pub fn build(self) -> (Vec, ToolRegistry) { let registry = ToolRegistry::new(self.handlers); (self.specs, registry) } diff --git a/codex-rs/core/src/tools/registry_tests.rs b/codex-rs/core/src/tools/registry_tests.rs index be5223a117..d6acb80fb4 100644 --- a/codex-rs/core/src/tools/registry_tests.rs +++ b/codex-rs/core/src/tools/registry_tests.rs @@ -71,7 +71,7 @@ fn register_handler_adds_handler_and_augments_specs_for_code_mode() { assert_eq!(specs.len(), 1); assert_eq!( - specs[0].spec, + specs[0], codex_tools::augment_tool_spec_for_code_mode(create_get_goal_tool()) ); assert!(registry.has_handler(&codex_tools::ToolName::plain(GET_GOAL_TOOL_NAME))); diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index 51fd40a268..ed3f1a720b 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -16,7 +16,6 @@ use codex_protocol::models::ResponseItem; use codex_protocol::models::SearchToolCallParams; use codex_protocol::models::ShellToolCallParams; use codex_tool_api::ToolBundle as ExtensionToolBundle; -use codex_tools::ConfiguredToolSpec; use codex_tools::DiscoverableTool; use codex_tools::ResponsesApiNamespaceTool; use codex_tools::ToolName; @@ -38,7 +37,7 @@ pub struct ToolCall { pub struct ToolRouter { registry: ToolRegistry, - specs: Vec, + specs: Vec, model_visible_specs: Vec, } @@ -78,17 +77,14 @@ impl ToolRouter { .collect::>(); let model_visible_specs = specs .iter() - .filter_map(|configured_tool| { + .filter_map(|spec| { if config.code_mode_only_enabled - && codex_code_mode::is_code_mode_nested_tool(configured_tool.name()) + && codex_code_mode::is_code_mode_nested_tool(spec.name()) { return None; } - filter_deferred_dynamic_tool_spec( - configured_tool.spec.clone(), - &deferred_dynamic_tools, - ) + filter_deferred_dynamic_tool_spec(spec.clone(), &deferred_dynamic_tools) }) .collect(); @@ -100,10 +96,7 @@ impl ToolRouter { } pub fn specs(&self) -> Vec { - self.specs - .iter() - .map(|config| config.spec.clone()) - .collect() + self.specs.clone() } pub fn model_visible_specs(&self) -> Vec { @@ -111,16 +104,16 @@ impl ToolRouter { } pub fn find_spec(&self, tool_name: &ToolName) -> Option { - self.specs.iter().find_map(|config| match &config.spec { + self.specs.iter().find_map(|spec| match spec { ToolSpec::Function(tool) if tool_name.namespace.is_none() && tool.name == tool_name.name => { - Some(config.spec.clone()) + Some(spec.clone()) } ToolSpec::Freeform(tool) if tool_name.namespace.is_none() && tool.name == tool_name.name => { - Some(config.spec.clone()) + Some(spec.clone()) } ToolSpec::Namespace(namespace) => namespace.tools.iter().find_map(|tool| match tool { ResponsesApiNamespaceTool::Function(tool) @@ -142,29 +135,10 @@ impl ToolRouter { self.registry.create_diff_consumer(tool_name) } - fn configured_tool_supports_parallel(&self, tool_name: &ToolName) -> bool { - if tool_name.namespace.is_some() { - return false; - } - - self.specs - .iter() - .filter(|config| config.supports_parallel_tool_calls) - .any(|config| match &config.spec { - ToolSpec::Function(tool) => tool.name == tool_name.name.as_str(), - ToolSpec::Freeform(tool) => tool.name == tool_name.name.as_str(), - ToolSpec::Namespace(_) - | ToolSpec::ToolSearch { .. } - | ToolSpec::LocalShell {} - | ToolSpec::ImageGeneration { .. } - | ToolSpec::WebSearch { .. } => false, - }) - } - pub fn tool_supports_parallel(&self, call: &ToolCall) -> bool { self.registry .supports_parallel_tool_calls(&call.tool_name) - .unwrap_or_else(|| self.configured_tool_supports_parallel(&call.tool_name)) + .unwrap_or(false) } #[instrument(level = "trace", skip_all, err)] diff --git a/codex-rs/core/src/tools/router_tests.rs b/codex-rs/core/src/tools/router_tests.rs index dc06f7ed02..b3fe087fcc 100644 --- a/codex-rs/core/src/tools/router_tests.rs +++ b/codex-rs/core/src/tools/router_tests.rs @@ -157,7 +157,7 @@ async fn build_tool_call_uses_namespace_for_registry_name() -> anyhow::Result<() } #[tokio::test] -async fn mcp_parallel_support_uses_exact_payload_server() -> anyhow::Result<()> { +async fn mcp_parallel_support_uses_handler_data() -> anyhow::Result<()> { let (_, turn) = make_session_and_context().await; let router = ToolRouter::from_config( &turn.tools_config, @@ -184,14 +184,14 @@ async fn mcp_parallel_support_uses_exact_payload_server() -> anyhow::Result<()> }, ); - let deferred_call = ToolCall { + let call = ToolCall { tool_name: ToolName::namespaced("mcp__echo__", "query_with_delay"), - call_id: "call-deferred".to_string(), + call_id: "call-handler".to_string(), payload: ToolPayload::Function { arguments: "{}".to_string(), }, }; - assert!(router.tool_supports_parallel(&deferred_call)); + assert!(router.tool_supports_parallel(&call)); let different_server_call = ToolCall { tool_name: ToolName::namespaced("mcp__hello_echo__", "query_with_delay"), @@ -205,6 +205,32 @@ async fn mcp_parallel_support_uses_exact_payload_server() -> anyhow::Result<()> Ok(()) } +#[tokio::test] +async fn tools_without_handlers_do_not_support_parallel() -> anyhow::Result<()> { + let (_, turn) = make_session_and_context().await; + let router = ToolRouter::from_config( + &turn.tools_config, + ToolRouterParams { + deferred_mcp_tools: None, + mcp_tools: None, + unavailable_called_tools: Vec::new(), + discoverable_tools: None, + extension_tool_bundles: Vec::new(), + dynamic_tools: turn.dynamic_tools.as_slice(), + }, + ); + + assert!(!router.tool_supports_parallel(&ToolCall { + tool_name: ToolName::plain("web_search"), + call_id: "call-web-search".to_string(), + payload: ToolPayload::Function { + arguments: "{}".to_string(), + }, + })); + + Ok(()) +} + #[tokio::test] async fn model_visible_specs_filter_deferred_dynamic_tools() -> anyhow::Result<()> { let (_, turn) = make_session_and_context().await; diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index b07a572ef7..69456a36ef 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -97,12 +97,8 @@ pub fn build_tool_registry_builder( ..params }, ); - let mut enabled_tools = collect_code_mode_exec_prompt_tool_definitions( - nested_builder - .specs() - .iter() - .map(|configured_tool| &configured_tool.spec), - ); + let mut enabled_tools = + collect_code_mode_exec_prompt_tool_definitions(nested_builder.specs().iter()); enabled_tools .sort_by(|left, right| compare_code_mode_tools(left, right, &namespace_descriptions)); builder.register_handler(Arc::new(CodeModeExecuteHandler::new( @@ -277,14 +273,11 @@ pub fn build_tool_registry_builder( web_search_config: config.web_search_config.as_ref(), web_search_tool_type: config.web_search_tool_type, }) { - builder.push_spec(web_search_tool, /*supports_parallel_tool_calls*/ false); + builder.push_spec(web_search_tool); } if config.image_gen_tool { - builder.push_spec( - create_image_generation_tool("png"), - /*supports_parallel_tool_calls*/ false, - ); + builder.push_spec(create_image_generation_tool("png")); } if config.environment_mode.has_environment() { @@ -391,14 +384,11 @@ pub fn build_tool_registry_builder( } if config.namespace_tools && !tools.is_empty() { - builder.push_spec( - ToolSpec::Namespace(ResponsesApiNamespace { - name: namespace, - description, - tools, - }), - /*supports_parallel_tool_calls*/ false, - ); + builder.push_spec(ToolSpec::Namespace(ResponsesApiNamespace { + name: namespace, + description, + tools, + })); } } } @@ -422,7 +412,7 @@ pub fn build_tool_registry_builder( for spec in coalesce_loadable_tool_specs(dynamic_tool_specs) { let spec = spec.into(); if config.namespace_tools || !matches!(spec, ToolSpec::Namespace(_)) { - builder.push_spec(spec, /*supports_parallel_tool_calls*/ false); + builder.push_spec(spec); } } diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index 8079f91da1..a4132391a2 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -47,7 +47,6 @@ use codex_tool_api::ToolBundle as ExtensionToolBundle; use codex_tool_api::ToolExecutor; use codex_tool_api::ToolFuture; use codex_tools::AdditionalProperties; -use codex_tools::ConfiguredToolSpec; use codex_tools::DiscoverablePluginInfo; use codex_tools::DiscoverableTool; use codex_tools::FreeformTool; @@ -132,7 +131,7 @@ fn extension_tools_do_not_replace_builtin_tools() { ); assert_eq!( - find_tool(&tools, "update_plan").spec, + find_tool(&tools, "update_plan").clone(), create_update_plan_tool() ); assert_eq!( @@ -171,7 +170,7 @@ fn test_full_toolset_specs_for_gpt5_codex_unified_exec_web_search() { let mut duplicate_names = Vec::new(); for tool in &tools { let name = tool.name().to_string(); - if actual.insert(name.clone(), tool.spec.clone()).is_some() { + if actual.insert(name.clone(), tool.clone()).is_some() { duplicate_names.push(name); } } @@ -365,7 +364,7 @@ fn test_build_specs_collab_tools_enabled() { assert_lacks_tool_name(&tools, "list_agents"); let spawn_agent = find_tool(&tools, "spawn_agent"); - let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = &spawn_agent.spec else { + let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = spawn_agent else { panic!("spawn_agent should be a function tool"); }; let (properties, _) = expect_object_schema(parameters); @@ -459,7 +458,7 @@ fn test_build_specs_multi_agent_v2_uses_task_names_and_hides_resume() { parameters, output_schema, .. - }) = &spawn_agent.spec + }) = spawn_agent else { panic!("spawn_agent should be a function tool"); }; @@ -483,7 +482,7 @@ fn test_build_specs_multi_agent_v2_uses_task_names_and_hides_resume() { parameters, output_schema, .. - }) = &send_message.spec + }) = send_message else { panic!("send_message should be a function tool"); }; @@ -503,7 +502,7 @@ fn test_build_specs_multi_agent_v2_uses_task_names_and_hides_resume() { parameters, output_schema, .. - }) = &followup_task.spec + }) = followup_task else { panic!("followup_task should be a function tool"); }; @@ -522,7 +521,7 @@ fn test_build_specs_multi_agent_v2_uses_task_names_and_hides_resume() { parameters, output_schema, .. - }) = &wait_agent.spec + }) = wait_agent else { panic!("wait_agent should be a function tool"); }; @@ -543,7 +542,7 @@ fn test_build_specs_multi_agent_v2_uses_task_names_and_hides_resume() { parameters, output_schema, .. - }) = &list_agents.spec + }) = list_agents else { panic!("list_agents should be a function tool"); }; @@ -660,7 +659,7 @@ fn view_image_tool_omits_detail_without_original_detail_support() { &[], ); let view_image = find_tool(&tools, VIEW_IMAGE_TOOL_NAME); - let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = &view_image.spec else { + let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = view_image else { panic!("view_image should be a function tool"); }; let (properties, _) = expect_object_schema(parameters); @@ -690,7 +689,7 @@ fn view_image_tool_includes_detail_with_original_detail_support() { &[], ); let view_image = find_tool(&tools, VIEW_IMAGE_TOOL_NAME); - let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = &view_image.spec else { + let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = view_image else { panic!("view_image should be a function tool"); }; let (properties, _) = expect_object_schema(parameters); @@ -841,7 +840,7 @@ fn request_user_input_description_reflects_default_mode_feature_flag() { ); let request_user_input_tool = find_tool(&tools, REQUEST_USER_INPUT_TOOL_NAME); assert_eq!( - request_user_input_tool.spec, + request_user_input_tool.clone(), request_user_input_tool_spec(&request_user_input_available_modes(&features)) ); @@ -864,7 +863,7 @@ fn request_user_input_description_reflects_default_mode_feature_flag() { ); let request_user_input_tool = find_tool(&tools, REQUEST_USER_INPUT_TOOL_NAME); assert_eq!( - request_user_input_tool.spec, + request_user_input_tool.clone(), request_user_input_tool_spec(&request_user_input_available_modes(&features)) ); } @@ -912,7 +911,7 @@ fn request_permissions_requires_feature_flag() { ); let request_permissions_tool = find_tool(&tools, "request_permissions"); assert_eq!( - request_permissions_tool.spec, + request_permissions_tool.clone(), create_request_permissions_tool(request_permissions_tool_description()) ); } @@ -973,7 +972,7 @@ fn image_generation_tools_require_feature_and_supported_model() { assert!( !default_tools .iter() - .any(|tool| tool.spec.name() == "image_generation"), + .any(|tool| tool.name() == "image_generation"), "image_generation should be disabled when the feature is disabled" ); @@ -996,7 +995,7 @@ fn image_generation_tools_require_feature_and_supported_model() { assert_contains_tool_names(&supported_tools, &["image_generation"]); let image_generation_tool = find_tool(&supported_tools, "image_generation"); assert_eq!( - serde_json::to_value(&image_generation_tool.spec).expect("serialize image tool"), + serde_json::to_value(image_generation_tool).expect("serialize image tool"), serde_json::json!({ "type": "image_generation", "output_format": "png" @@ -1020,9 +1019,7 @@ fn image_generation_tools_require_feature_and_supported_model() { &[], ); assert!( - !tools - .iter() - .any(|tool| tool.spec.name() == "image_generation"), + !tools.iter().any(|tool| tool.name() == "image_generation"), "image_generation should be disabled for unsupported models" ); } @@ -1052,7 +1049,7 @@ fn web_search_mode_cached_sets_external_web_access_false() { let tool = find_tool(&tools, "web_search"); assert_eq!( - tool.spec, + tool.clone(), ToolSpec::WebSearch { external_web_access: Some(false), filters: None, @@ -1088,7 +1085,7 @@ fn web_search_mode_live_sets_external_web_access_true() { let tool = find_tool(&tools, "web_search"); assert_eq!( - tool.spec, + tool.clone(), ToolSpec::WebSearch { external_web_access: Some(true), filters: None, @@ -1138,7 +1135,7 @@ fn web_search_config_is_forwarded_to_tool_spec() { let tool = find_tool(&tools, "web_search"); assert_eq!( - tool.spec, + tool.clone(), ToolSpec::WebSearch { external_web_access: Some(true), filters: web_search_config @@ -1179,7 +1176,7 @@ fn web_search_tool_type_text_and_image_sets_search_content_types() { let tool = find_tool(&tools, "web_search"); assert_eq!( - tool.spec, + tool.clone(), ToolSpec::WebSearch { external_web_access: Some(true), filters: None, @@ -1214,7 +1211,7 @@ fn mcp_resource_tools_are_hidden_without_mcp_servers() { assert!( !tools.iter().any(|tool| matches!( - tool.spec.name(), + tool.name(), "list_mcp_resources" | "list_mcp_resource_templates" | "read_mcp_resource" )), "MCP resource tools should be omitted when no MCP servers are configured" @@ -1277,8 +1274,7 @@ fn test_parallel_support_flags() { &[], ); - assert!(find_tool(&tools, "exec_command").supports_parallel_tool_calls); - assert!(!find_tool(&tools, "write_stdin").supports_parallel_tool_calls); + assert_contains_tool_names(&tools, &["exec_command", "write_stdin"]); } #[test] @@ -1505,7 +1501,7 @@ fn test_build_specs_mcp_namespace_description_falls_back_when_missing() { ); let namespace_tool = find_tool(&tools, "test_server/"); - let ToolSpec::Namespace(namespace) = &namespace_tool.spec else { + let ToolSpec::Namespace(namespace) = namespace_tool else { panic!("expected namespace tool"); }; assert_eq!( @@ -1631,7 +1627,7 @@ fn search_tool_description_lists_each_mcp_source_once() { ); let search_tool = find_tool(&tools, TOOL_SEARCH_TOOL_NAME); - let ToolSpec::ToolSearch { description, .. } = &search_tool.spec else { + let ToolSpec::ToolSearch { description, .. } = search_tool else { panic!("expected tool_search tool"); }; let description = description.as_str(); @@ -1810,7 +1806,7 @@ fn search_tool_registers_for_deferred_dynamic_tools() { ); let search_tool = find_tool(&tools, TOOL_SEARCH_TOOL_NAME); - let ToolSpec::ToolSearch { description, .. } = &search_tool.spec else { + let ToolSpec::ToolSearch { description, .. } = search_tool else { panic!("expected tool_search tool"); }; assert!(description.contains("- Dynamic tools: Tools provided by the current Codex thread.")); @@ -1959,11 +1955,9 @@ fn request_plugin_install_can_be_registered_without_search_tool() { assert_contains_tool_names(&tools, &[REQUEST_PLUGIN_INSTALL_TOOL_NAME]); let request_plugin_install = find_tool(&tools, REQUEST_PLUGIN_INSTALL_TOOL_NAME); - assert!(request_plugin_install.supports_parallel_tool_calls); assert_lacks_tool_name(&tools, TOOL_SEARCH_TOOL_NAME); - let ToolSpec::Function(ResponsesApiTool { description, .. }) = &request_plugin_install.spec - else { + let ToolSpec::Function(ResponsesApiTool { description, .. }) = request_plugin_install else { panic!("expected function tool"); }; assert!(description.contains( @@ -2030,7 +2024,7 @@ fn request_plugin_install_description_lists_discoverable_tools() { description, parameters, .. - }) = &request_plugin_install.spec + }) = request_plugin_install else { panic!("expected function tool"); }; @@ -2247,7 +2241,7 @@ fn code_mode_augments_builtin_tool_descriptions_with_typed_sample() { &[], ); let ToolSpec::Function(ResponsesApiTool { description, .. }) = - &find_tool(&tools, VIEW_IMAGE_TOOL_NAME).spec + find_tool(&tools, VIEW_IMAGE_TOOL_NAME) else { panic!("expected function tool"); }; @@ -2282,8 +2276,7 @@ fn code_mode_only_exec_description_includes_full_nested_tool_details() { /*deferred_mcp_tools*/ None, &[], ); - let ToolSpec::Freeform(FreeformTool { description, .. }) = &find_tool(&tools, "exec").spec - else { + let ToolSpec::Freeform(FreeformTool { description, .. }) = find_tool(&tools, "exec") else { panic!("expected freeform tool"); }; @@ -2326,8 +2319,7 @@ fn code_mode_only_exec_description_includes_extension_tool_details() { &extension_tool_bundles, &[], ); - let ToolSpec::Freeform(FreeformTool { description, .. }) = &find_tool(&tools, "exec").spec - else { + let ToolSpec::Freeform(FreeformTool { description, .. }) = find_tool(&tools, "exec") else { panic!("expected freeform tool"); }; @@ -2358,8 +2350,7 @@ fn code_mode_exec_description_omits_nested_tool_details_when_not_code_mode_only( /*deferred_mcp_tools*/ None, &[], ); - let ToolSpec::Freeform(FreeformTool { description, .. }) = &find_tool(&tools, "exec").spec - else { + let ToolSpec::Freeform(FreeformTool { description, .. }) = find_tool(&tools, "exec") else { panic!("expected freeform tool"); }; @@ -2417,7 +2408,7 @@ fn build_specs( mcp_tools: Option>, deferred_mcp_tools: Option>, dynamic_tools: &[DynamicToolSpec], -) -> (Vec, ToolRegistry) { +) -> (Vec, ToolRegistry) { build_specs_with_discoverable_tools( config, mcp_tools, @@ -2435,7 +2426,7 @@ fn build_specs_with_discoverable_tools( discoverable_tools: Option>, extension_tool_bundles: &[codex_tool_api::ToolBundle], dynamic_tools: &[DynamicToolSpec], -) -> (Vec, ToolRegistry) { +) -> (Vec, ToolRegistry) { build_specs_with_optional_tool_namespaces( config, mcp_tools, @@ -2455,7 +2446,7 @@ fn build_specs_with_optional_tool_namespaces( discoverable_tools: Option>, extension_tool_bundles: &[codex_tool_api::ToolBundle], dynamic_tools: &[DynamicToolSpec], -) -> (Vec, ToolRegistry) { +) -> (Vec, ToolRegistry) { let mcp_tool_inputs = mcp_tools.as_ref().map(|mcp_tools| { mcp_tools .iter() @@ -2636,12 +2627,12 @@ fn deferred_mcp_tool( } } -fn assert_contains_tool_names(tools: &[ConfiguredToolSpec], expected_subset: &[&str]) { +fn assert_contains_tool_names(tools: &[ToolSpec], expected_subset: &[&str]) { use std::collections::HashSet; let mut names = HashSet::new(); let mut duplicates = Vec::new(); - for name in tools.iter().map(ConfiguredToolSpec::name) { + for name in tools.iter().map(ToolSpec::name) { if !names.insert(name) { duplicates.push(name); } @@ -2658,11 +2649,8 @@ fn assert_contains_tool_names(tools: &[ConfiguredToolSpec], expected_subset: &[& } } -fn assert_lacks_tool_name(tools: &[ConfiguredToolSpec], expected_absent: &str) { - let names = tools - .iter() - .map(ConfiguredToolSpec::name) - .collect::>(); +fn assert_lacks_tool_name(tools: &[ToolSpec], expected_absent: &str) { + let names = tools.iter().map(ToolSpec::name).collect::>(); assert!( !names.contains(&expected_absent), "expected tool {expected_absent} to be absent; had: {names:?}" @@ -2692,7 +2680,7 @@ fn wait_agent_timeout_options() -> WaitAgentTimeoutOptions { } } -fn find_tool<'a>(tools: &'a [ConfiguredToolSpec], expected_name: &str) -> &'a ConfiguredToolSpec { +fn find_tool<'a>(tools: &'a [ToolSpec], expected_name: &str) -> &'a ToolSpec { tools .iter() .find(|tool| tool.name() == expected_name) @@ -2700,12 +2688,12 @@ fn find_tool<'a>(tools: &'a [ConfiguredToolSpec], expected_name: &str) -> &'a Co } fn assert_process_tool_environment_id( - tools: &[ConfiguredToolSpec], + tools: &[ToolSpec], expected_name: &str, expected_present: bool, ) { let tool = find_tool(tools, expected_name); - let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = &tool.spec else { + let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = tool else { panic!("expected function tool {expected_name}"); }; let (properties, _) = expect_object_schema(parameters); @@ -2716,9 +2704,9 @@ fn assert_process_tool_environment_id( ); } -fn assert_apply_patch_environment_id(tools: &[ConfiguredToolSpec], expected_present: bool) { +fn assert_apply_patch_environment_id(tools: &[ToolSpec], expected_present: bool) { let tool = find_tool(tools, "apply_patch"); - let ToolSpec::Freeform(FreeformTool { format, .. }) = &tool.spec else { + let ToolSpec::Freeform(FreeformTool { format, .. }) = tool else { panic!("expected freeform apply_patch tool"); }; assert_eq!( @@ -2729,12 +2717,12 @@ fn assert_apply_patch_environment_id(tools: &[ConfiguredToolSpec], expected_pres } fn find_namespace_function_tool<'a>( - tools: &'a [ConfiguredToolSpec], + tools: &'a [ToolSpec], expected_namespace: &str, expected_name: &str, ) -> &'a ResponsesApiTool { let namespace_tool = find_tool(tools, expected_namespace); - let ToolSpec::Namespace(namespace) = &namespace_tool.spec else { + let ToolSpec::Namespace(namespace) = namespace_tool else { panic!("expected namespace tool {expected_namespace}"); }; namespace @@ -2747,9 +2735,9 @@ fn find_namespace_function_tool<'a>( .unwrap_or_else(|| panic!("expected tool {expected_namespace}{expected_name} in namespace")) } -fn namespace_function_names(tools: &[ConfiguredToolSpec], expected_namespace: &str) -> Vec { +fn namespace_function_names(tools: &[ToolSpec], expected_namespace: &str) -> Vec { let namespace_tool = find_tool(tools, expected_namespace); - let ToolSpec::Namespace(namespace) = &namespace_tool.spec else { + let ToolSpec::Namespace(namespace) = namespace_tool else { panic!("expected namespace tool {expected_namespace}"); }; namespace diff --git a/codex-rs/core/src/tools/spec_tests.rs b/codex-rs/core/src/tools/spec_tests.rs index c97d3ccdeb..0086542859 100644 --- a/codex-rs/core/src/tools/spec_tests.rs +++ b/codex-rs/core/src/tools/spec_tests.rs @@ -17,7 +17,6 @@ use codex_protocol::openai_models::ConfigShellToolType; use codex_protocol::openai_models::ModelInfo; use codex_protocol::protocol::SessionSource; use codex_tools::AdditionalProperties; -use codex_tools::ConfiguredToolSpec; use codex_tools::DiscoverableTool; use codex_tools::JsonSchema; use codex_tools::LoadableToolSpec; @@ -163,11 +162,11 @@ fn deferred_responses_api_tool_serializes_with_defer_loading() { } // Avoid order-based assertions; compare via set containment instead. -fn assert_contains_tool_names(tools: &[ConfiguredToolSpec], expected_subset: &[&str]) { +fn assert_contains_tool_names(tools: &[ToolSpec], expected_subset: &[&str]) { use std::collections::HashSet; let mut names = HashSet::new(); let mut duplicates = Vec::new(); - for name in tools.iter().map(ConfiguredToolSpec::name) { + for name in tools.iter().map(ToolSpec::name) { if !names.insert(name) { duplicates.push(name); } @@ -194,7 +193,7 @@ fn shell_tool_name(config: &ToolsConfig) -> Option<&'static str> { } } -fn find_tool<'a>(tools: &'a [ConfiguredToolSpec], expected_name: &str) -> &'a ConfiguredToolSpec { +fn find_tool<'a>(tools: &'a [ToolSpec], expected_name: &str) -> &'a ToolSpec { tools .iter() .find(|tool| tool.name() == expected_name) @@ -202,12 +201,12 @@ fn find_tool<'a>(tools: &'a [ConfiguredToolSpec], expected_name: &str) -> &'a Co } fn find_namespace_function_tool<'a>( - tools: &'a [ConfiguredToolSpec], + tools: &'a [ToolSpec], expected_namespace: &str, expected_name: &str, ) -> &'a ResponsesApiTool { let namespace_tool = find_tool(tools, expected_namespace); - let ToolSpec::Namespace(namespace) = &namespace_tool.spec else { + let ToolSpec::Namespace(namespace) = namespace_tool else { panic!("expected namespace tool {expected_namespace}"); }; namespace @@ -249,7 +248,7 @@ fn multi_agent_v2_spawn_agent_description(tools_config: &ToolsConfig) -> String ) .build(); let spawn_agent = find_tool(&tools, "spawn_agent"); - let ToolSpec::Function(ResponsesApiTool { description, .. }) = &spawn_agent.spec else { + let ToolSpec::Function(ResponsesApiTool { description, .. }) = spawn_agent else { panic!("spawn_agent should be a function tool"); }; description.clone() @@ -326,7 +325,7 @@ async fn get_memory_requires_feature_flag() { ) .build(); assert!( - !tools.iter().any(|t| t.spec.name() == "get_memory"), + !tools.iter().any(|t| t.name() == "get_memory"), "get_memory should be disabled when memory_tool feature is off" ); } @@ -778,7 +777,7 @@ async fn multi_agent_v2_wait_agent_schema_uses_configured_min_timeout() { ) .build(); let wait_agent = find_tool(&tools, "wait_agent"); - let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = &wait_agent.spec else { + let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = wait_agent else { panic!("wait_agent should be a function tool"); }; let timeout_description = parameters @@ -867,7 +866,7 @@ async fn search_tool_description_handles_no_enabled_mcp_tools() { ) .build(); let search_tool = find_tool(&tools, TOOL_SEARCH_TOOL_NAME); - let ToolSpec::ToolSearch { description, .. } = &search_tool.spec else { + let ToolSpec::ToolSearch { description, .. } = search_tool else { panic!("expected tool_search tool"); }; @@ -916,7 +915,7 @@ async fn search_tool_description_falls_back_to_connector_name_without_descriptio ) .build(); let search_tool = find_tool(&tools, TOOL_SEARCH_TOOL_NAME); - let ToolSpec::ToolSearch { description, .. } = &search_tool.spec else { + let ToolSpec::ToolSearch { description, .. } = search_tool else { panic!("expected tool_search tool"); }; @@ -1123,7 +1122,7 @@ async fn unavailable_mcp_tools_are_exposed_as_dummy_function_tools() { description, parameters, .. - }) = &tool.spec + }) = tool else { panic!("unavailable MCP tool should be exposed as a function tool"); }; diff --git a/codex-rs/tools/src/lib.rs b/codex-rs/tools/src/lib.rs index d0a1794cbc..8c63e6e631 100644 --- a/codex-rs/tools/src/lib.rs +++ b/codex-rs/tools/src/lib.rs @@ -76,7 +76,6 @@ 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::ConfiguredToolSpec; pub use tool_spec::ResponsesApiWebSearchFilters; pub use tool_spec::ResponsesApiWebSearchUserLocation; pub use tool_spec::ToolSpec; diff --git a/codex-rs/tools/src/tool_spec.rs b/codex-rs/tools/src/tool_spec.rs index be8d00d08a..2a0183a905 100644 --- a/codex-rs/tools/src/tool_spec.rs +++ b/codex-rs/tools/src/tool_spec.rs @@ -75,25 +75,6 @@ impl From for ToolSpec { } } -#[derive(Debug, Clone, PartialEq)] -pub struct ConfiguredToolSpec { - pub spec: ToolSpec, - pub supports_parallel_tool_calls: bool, -} - -impl ConfiguredToolSpec { - pub fn new(spec: ToolSpec, supports_parallel_tool_calls: bool) -> Self { - Self { - spec, - supports_parallel_tool_calls, - } - } - - pub fn name(&self) -> &str { - self.spec.name() - } -} - /// Returns JSON values that are compatible with Function Calling in the /// Responses API: /// https://platform.openai.com/docs/guides/function-calling?api-mode=responses diff --git a/codex-rs/tools/src/tool_spec_tests.rs b/codex-rs/tools/src/tool_spec_tests.rs index 0b98625626..6d2017e553 100644 --- a/codex-rs/tools/src/tool_spec_tests.rs +++ b/codex-rs/tools/src/tool_spec_tests.rs @@ -1,4 +1,3 @@ -use super::ConfiguredToolSpec; use super::ResponsesApiNamespace; use super::ResponsesApiWebSearchFilters; use super::ResponsesApiWebSearchUserLocation; @@ -92,29 +91,6 @@ fn tool_spec_name_covers_all_variants() { ); } -#[test] -fn configured_tool_spec_name_delegates_to_tool_spec() { - assert_eq!( - ConfiguredToolSpec::new( - ToolSpec::Function(ResponsesApiTool { - name: "lookup_order".to_string(), - description: "Look up an order".to_string(), - strict: false, - defer_loading: None, - parameters: JsonSchema::object( - BTreeMap::new(), - /*required*/ None, - /*additional_properties*/ None - ), - output_schema: None, - }), - /*supports_parallel_tool_calls*/ true, - ) - .name(), - "lookup_order" - ); -} - #[test] fn web_search_config_converts_to_responses_api_types() { assert_eq!(