diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index e38ff3a562..af5e3007b4 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -72,6 +72,7 @@ use codex_protocol::openai_models::ModelInfo; use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::W3cTraceContext; +use codex_tools::create_tools_json_for_responses_api; use eventsource_stream::Event; use eventsource_stream::EventStreamError; use futures::StreamExt; @@ -107,7 +108,6 @@ use crate::response_debug_context::extract_response_debug_context; use crate::response_debug_context::extract_response_debug_context_from_api_error; use crate::response_debug_context::telemetry_api_error_message; use crate::response_debug_context::telemetry_transport_error_message; -use crate::tools::spec::create_tools_json_for_responses_api; use crate::util::FeedbackRequestTags; use crate::util::emit_feedback_auth_recovery_tags; use crate::util::emit_feedback_request_tags_with_auth_env; diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index aa7f401099..0e77caac72 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -24,6 +24,7 @@ use codex_hooks::HookToolInput; use codex_hooks::HookToolInputLocalShell; use codex_hooks::HookToolKind; use codex_protocol::models::ResponseInputItem; +use codex_tools::ConfiguredToolSpec; use codex_utils_readiness::Readiness; use serde_json::Value; use tracing::warn; @@ -436,21 +437,6 @@ impl ToolRegistry { } } -#[derive(Debug, Clone)] -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 struct ToolRegistryBuilder { handlers: HashMap>, specs: Vec, diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index 345f7ce06f..1145e3a8f2 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -9,7 +9,6 @@ use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; use crate::tools::discoverable::DiscoverableTool; use crate::tools::registry::AnyToolResult; -use crate::tools::registry::ConfiguredToolSpec; use crate::tools::registry::ToolRegistry; use crate::tools::spec::ToolsConfig; use crate::tools::spec::build_specs_with_discoverable_tools; @@ -18,6 +17,7 @@ use codex_protocol::models::LocalShellAction; use codex_protocol::models::ResponseItem; use codex_protocol::models::SearchToolCallParams; use codex_protocol::models::ShellToolCallParams; +use codex_tools::ConfiguredToolSpec; use rmcp::model::Tool; use std::collections::HashMap; use std::sync::Arc; @@ -66,7 +66,7 @@ impl ToolRouter { specs .iter() .filter_map(|configured_tool| { - if !codex_code_mode::is_code_mode_nested_tool(configured_tool.spec.name()) { + if !codex_code_mode::is_code_mode_nested_tool(configured_tool.name()) { Some(configured_tool.spec.clone()) } else { None @@ -101,7 +101,7 @@ impl ToolRouter { pub fn find_spec(&self, tool_name: &str) -> Option { self.specs .iter() - .find(|config| config.spec.name() == tool_name) + .find(|config| config.name() == tool_name) .map(|config| config.spec.clone()) } @@ -109,7 +109,7 @@ impl ToolRouter { self.specs .iter() .filter(|config| config.supports_parallel_tool_calls) - .any(|config| config.spec.name() == tool_name) + .any(|config| config.name() == tool_name) } #[instrument(level = "trace", skip_all, err)] diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 2a0704262c..d5b4b29216 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -2351,22 +2351,6 @@ pub(crate) struct ApplyPatchToolArgs { pub(crate) input: String, } -/// Returns JSON values that are compatible with Function Calling in the -/// Responses API: -/// https://platform.openai.com/docs/guides/function-calling?api-mode=responses -pub fn create_tools_json_for_responses_api( - tools: &[ToolSpec], -) -> crate::error::Result> { - let mut tools_json = Vec::new(); - - for tool in tools { - let json = serde_json::to_value(tool)?; - tools_json.push(json); - } - - Ok(tools_json) -} - fn push_tool_spec( builder: &mut ToolRegistryBuilder, spec: ToolSpec, diff --git a/codex-rs/core/src/tools/spec_tests.rs b/codex-rs/core/src/tools/spec_tests.rs index 60339b7811..2a7644eec5 100644 --- a/codex-rs/core/src/tools/spec_tests.rs +++ b/codex-rs/core/src/tools/spec_tests.rs @@ -4,13 +4,13 @@ use crate::models_manager::model_info::with_config_overrides; use crate::shell::Shell; use crate::shell::ShellType; use crate::tools::ToolRouter; -use crate::tools::registry::ConfiguredToolSpec; use crate::tools::router::ToolRouterParams; use codex_app_server_protocol::AppInfo; use codex_protocol::openai_models::InputModality; use codex_protocol::openai_models::ModelInfo; use codex_protocol::openai_models::ModelsResponse; use codex_tools::AdditionalProperties; +use codex_tools::ConfiguredToolSpec; use codex_tools::FreeformTool; use codex_tools::ResponsesApiWebSearchFilters; use codex_tools::ResponsesApiWebSearchUserLocation; @@ -107,16 +107,12 @@ fn deferred_responses_api_tool_serializes_with_defer_loading() { ); } -fn tool_name(tool: &ToolSpec) -> &str { - tool.name() -} - // Avoid order-based assertions; compare via set containment instead. fn assert_contains_tool_names(tools: &[ConfiguredToolSpec], expected_subset: &[&str]) { use std::collections::HashSet; let mut names = HashSet::new(); let mut duplicates = Vec::new(); - for name in tools.iter().map(|t| tool_name(&t.spec)) { + for name in tools.iter().map(ConfiguredToolSpec::name) { if !names.insert(name) { duplicates.push(name); } @@ -136,7 +132,7 @@ fn assert_contains_tool_names(tools: &[ConfiguredToolSpec], expected_subset: &[& fn assert_lacks_tool_name(tools: &[ConfiguredToolSpec], expected_absent: &str) { let names = tools .iter() - .map(|tool| tool_name(&tool.spec)) + .map(ConfiguredToolSpec::name) .collect::>(); assert!( !names.contains(&expected_absent), @@ -157,7 +153,7 @@ fn shell_tool_name(config: &ToolsConfig) -> Option<&'static str> { fn find_tool<'a>(tools: &'a [ConfiguredToolSpec], expected_name: &str) -> &'a ConfiguredToolSpec { tools .iter() - .find(|tool| tool_name(&tool.spec) == expected_name) + .find(|tool| tool.name() == expected_name) .unwrap_or_else(|| panic!("expected tool {expected_name}")) } @@ -289,7 +285,7 @@ fn test_full_toolset_specs_for_gpt5_codex_unified_exec_web_search() { let mut actual: BTreeMap = BTreeMap::from([]); let mut duplicate_names = Vec::new(); for t in &tools { - let name = tool_name(&t.spec).to_string(); + let name = t.name().to_string(); if actual.insert(name.clone(), t.spec.clone()).is_some() { duplicate_names.push(name); } @@ -318,7 +314,7 @@ fn test_full_toolset_specs_for_gpt5_codex_unified_exec_web_search() { }, create_view_image_tool(config.can_request_original_image_detail), ] { - expected.insert(tool_name(&spec).to_string(), spec); + expected.insert(spec.name().to_string(), spec); } let collab_specs = if config.multi_agent_v2 { vec![ @@ -336,16 +332,16 @@ fn test_full_toolset_specs_for_gpt5_codex_unified_exec_web_search() { ] }; for spec in collab_specs { - expected.insert(tool_name(&spec).to_string(), spec); + expected.insert(spec.name().to_string(), spec); } if !config.multi_agent_v2 { let spec = create_resume_agent_tool(); - expected.insert(tool_name(&spec).to_string(), spec); + expected.insert(spec.name().to_string(), spec); } if config.exec_permission_approvals_enabled { let spec = create_request_permissions_tool(); - expected.insert(tool_name(&spec).to_string(), spec); + expected.insert(spec.name().to_string(), spec); } // Exact name set match — this is the only test allowed to fail when tools change. @@ -1679,11 +1675,7 @@ fn test_test_model_info_includes_sync_tool() { ) .build(); - assert!( - tools - .iter() - .any(|tool| tool_name(&tool.spec) == "test_sync_tool") - ); + assert!(tools.iter().any(|tool| tool.name() == "test_sync_tool")); } #[test] @@ -1817,7 +1809,7 @@ fn test_build_specs_mcp_tools_sorted_by_name() { // Only assert that the MCP tools themselves are sorted by fully-qualified name. let mcp_names: Vec<_> = tools .iter() - .map(|t| tool_name(&t.spec).to_string()) + .map(|t| t.name().to_string()) .filter(|n| n.starts_with("test_server/")) .collect(); let expected = vec![ @@ -2064,7 +2056,7 @@ fn tool_suggest_is_not_registered_without_feature_flag() { assert!( !tools .iter() - .any(|tool| tool_name(&tool.spec) == TOOL_SUGGEST_TOOL_NAME) + .any(|tool| tool.name() == TOOL_SUGGEST_TOOL_NAME) ); } @@ -2155,7 +2147,7 @@ fn tool_suggest_requires_apps_and_plugins_features() { assert!( !tools .iter() - .any(|tool| tool_name(&tool.spec) == TOOL_SUGGEST_TOOL_NAME), + .any(|tool| tool.name() == TOOL_SUGGEST_TOOL_NAME), "tool_suggest should be absent when {disabled_feature:?} is disabled" ); } @@ -3106,38 +3098,3 @@ fn code_mode_exec_description_omits_nested_tool_details_when_not_code_mode_only( assert!(!description.contains("### `update_plan` (`update_plan`)")); assert!(!description.contains("### `view_image` (`view_image`)")); } - -#[test] -fn chat_tools_include_top_level_name() { - let properties = - BTreeMap::from([("foo".to_string(), JsonSchema::String { description: None })]); - let tools = vec![ToolSpec::Function(ResponsesApiTool { - name: "demo".to_string(), - description: "A demo tool".to_string(), - strict: false, - defer_loading: None, - parameters: JsonSchema::Object { - properties, - required: None, - additional_properties: None, - }, - output_schema: None, - })]; - - let responses_json = create_tools_json_for_responses_api(&tools).unwrap(); - assert_eq!( - responses_json, - vec![json!({ - "type": "function", - "name": "demo", - "description": "A demo tool", - "strict": false, - "parameters": { - "type": "object", - "properties": { - "foo": { "type": "string" } - }, - }, - })] - ); -} diff --git a/codex-rs/tools/README.md b/codex-rs/tools/README.md index d8dabdcd72..66d342cdb4 100644 --- a/codex-rs/tools/README.md +++ b/codex-rs/tools/README.md @@ -12,6 +12,7 @@ schema and Responses API tool primitives that no longer need to live in - `AdditionalProperties` - `ToolDefinition` - `ToolSpec` +- `ConfiguredToolSpec` - `ResponsesApiTool` - `FreeformTool` - `FreeformToolFormat` @@ -23,6 +24,7 @@ schema and Responses API tool primitives that no longer need to live in - `parse_tool_input_schema()` - `parse_dynamic_tool()` - `parse_mcp_tool()` +- `create_tools_json_for_responses_api()` - `mcp_call_tool_result_output_schema()` - `tool_definition_to_responses_api_tool()` - `dynamic_tool_to_responses_api_tool()` diff --git a/codex-rs/tools/src/lib.rs b/codex-rs/tools/src/lib.rs index 05b527ba94..527424c276 100644 --- a/codex-rs/tools/src/lib.rs +++ b/codex-rs/tools/src/lib.rs @@ -25,6 +25,8 @@ pub use responses_api::mcp_tool_to_deferred_responses_api_tool; pub use responses_api::mcp_tool_to_responses_api_tool; pub use responses_api::tool_definition_to_responses_api_tool; pub use tool_definition::ToolDefinition; +pub use tool_spec::ConfiguredToolSpec; pub use tool_spec::ResponsesApiWebSearchFilters; pub use tool_spec::ResponsesApiWebSearchUserLocation; pub use tool_spec::ToolSpec; +pub use tool_spec::create_tools_json_for_responses_api; diff --git a/codex-rs/tools/src/tool_spec.rs b/codex-rs/tools/src/tool_spec.rs index fc63223e1b..f537388853 100644 --- a/codex-rs/tools/src/tool_spec.rs +++ b/codex-rs/tools/src/tool_spec.rs @@ -6,6 +6,7 @@ use codex_protocol::config_types::WebSearchFilters as ConfigWebSearchFilters; use codex_protocol::config_types::WebSearchUserLocation as ConfigWebSearchUserLocation; use codex_protocol::config_types::WebSearchUserLocationType; use serde::Serialize; +use serde_json::Value; /// When serialized as JSON, this produces a valid "Tool" in the OpenAI /// Responses API. @@ -60,6 +61,41 @@ impl 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 +pub fn create_tools_json_for_responses_api( + tools: &[ToolSpec], +) -> Result, serde_json::Error> { + let mut tools_json = Vec::new(); + + for tool in tools { + let json = serde_json::to_value(tool)?; + tools_json.push(json); + } + + Ok(tools_json) +} + #[derive(Debug, Clone, Serialize, PartialEq)] pub struct ResponsesApiWebSearchFilters { #[serde(skip_serializing_if = "Option::is_none")] diff --git a/codex-rs/tools/src/tool_spec_tests.rs b/codex-rs/tools/src/tool_spec_tests.rs index 33d31c1401..c84ea94a36 100644 --- a/codex-rs/tools/src/tool_spec_tests.rs +++ b/codex-rs/tools/src/tool_spec_tests.rs @@ -1,3 +1,4 @@ +use super::ConfiguredToolSpec; use super::ResponsesApiWebSearchFilters; use super::ResponsesApiWebSearchUserLocation; use super::ToolSpec; @@ -6,6 +7,7 @@ use crate::FreeformTool; use crate::FreeformToolFormat; use crate::JsonSchema; use crate::ResponsesApiTool; +use crate::create_tools_json_for_responses_api; use codex_protocol::config_types::WebSearchContextSize; use codex_protocol::config_types::WebSearchFilters as ConfigWebSearchFilters; use codex_protocol::config_types::WebSearchUserLocation as ConfigWebSearchUserLocation; @@ -79,6 +81,29 @@ 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 { + properties: 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!( @@ -107,6 +132,40 @@ fn web_search_config_converts_to_responses_api_types() { ); } +#[test] +fn create_tools_json_for_responses_api_includes_top_level_name() { + assert_eq!( + create_tools_json_for_responses_api(&[ToolSpec::Function(ResponsesApiTool { + name: "demo".to_string(), + description: "A demo tool".to_string(), + strict: false, + defer_loading: None, + parameters: JsonSchema::Object { + properties: BTreeMap::from([( + "foo".to_string(), + JsonSchema::String { description: None }, + )]), + required: None, + additional_properties: None, + }, + output_schema: None, + })]) + .expect("serialize tools"), + vec![json!({ + "type": "function", + "name": "demo", + "description": "A demo tool", + "strict": false, + "parameters": { + "type": "object", + "properties": { + "foo": { "type": "string" } + }, + }, + })] + ); +} + #[test] fn web_search_tool_spec_serializes_expected_wire_shape() { assert_eq!(