diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 74b004a458..da95653b22 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1239,7 +1239,7 @@ async fn submission_loop( cwd: new_cwd.clone(), is_review_mode: false, final_output_json_schema: None, - disabled_tools: disabled_tools.unwrap_or_else(DisabledTool::defaults), + disabled_tools: disabled_tools.unwrap_or_else(|| prev.disabled_tools.clone()), }; // Install the new persistent context for subsequent tasks/turns. @@ -1981,14 +1981,14 @@ async fn run_turn( .get_model_family() .supports_parallel_tool_calls; let parallel_tool_calls = model_supports_parallel; - let allowed_tools = router.allowed_tools(turn_context.disabled_tools.clone()); + let allowed_tools = router.allowed_tools(Some(&turn_context.disabled_tools)); let prompt = Prompt { input, tools: router.specs(), parallel_tool_calls, base_instructions_override: turn_context.base_instructions.clone(), output_schema: turn_context.final_output_json_schema.clone(), - allowed_tools: Some(allowed_tools), + allowed_tools, }; let mut retries = 0; diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index 2367a02d98..228527d6fe 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -49,20 +49,29 @@ impl ToolRouter { .collect() } - pub fn allowed_tools(&self, disabled_tools: Vec) -> Vec { - self.specs - .iter() - .filter_map(|config| { - let name = config.spec.name(); - if disabled_tools - .iter() - .any(|disabled| disabled.matches_tool_name(name)) - { - return None; - } - Some(json!({"type": "function", "name": name})) - }) - .collect() + pub fn allowed_tools( + &self, + disabled_tools: Option<&[DisabledTool]>, + ) -> Option> { + let disabled = disabled_tools.unwrap_or(&[]); + if disabled.is_empty() { + return None; + } + + let mut allowed = Vec::new(); + for config in &self.specs { + let name = config.spec.name(); + if disabled.iter().any(|tool| tool.matches_tool_name(name)) { + continue; + } + allowed.push(json!({"type": "function", "name": name})); + } + + if allowed.len() == self.specs.len() { + None + } else { + Some(allowed) + } } pub fn tool_supports_parallel(&self, tool_name: &str) -> bool { diff --git a/codex-rs/core/tests/suite/compact_resume_fork.rs b/codex-rs/core/tests/suite/compact_resume_fork.rs index 7c1f372d91..86bd7b01c7 100644 --- a/codex-rs/core/tests/suite/compact_resume_fork.rs +++ b/codex-rs/core/tests/suite/compact_resume_fork.rs @@ -123,6 +123,7 @@ async fn compact_resume_and_fork_preserve_model_history_view() { .as_str() .unwrap_or_default() .to_string(); + let tool_calls = requests[0]["tools"].clone(); let tool_choice = requests[0]["tool_choice"].clone(); let prompt_cache_key = requests[0]["prompt_cache_key"] .as_str() @@ -169,6 +170,7 @@ async fn compact_resume_and_fork_preserve_model_history_view() { ] } ], + "tools": tool_calls.clone(), "tool_choice": tool_choice, "parallel_tool_calls": false, "reasoning": { @@ -302,6 +304,7 @@ SUMMARY_ONLY_CONTEXT" ] } ], + "tools": tool_calls.clone(), "tool_choice": tool_choice, "parallel_tool_calls": false, "reasoning": { @@ -386,6 +389,7 @@ SUMMARY_ONLY_CONTEXT" ] } ], + "tools": tool_calls.clone(), "tool_choice": tool_choice, "parallel_tool_calls": false, "reasoning": { @@ -470,6 +474,7 @@ SUMMARY_ONLY_CONTEXT" ] } ], + "tools": tool_calls.clone(), "tool_choice": tool_choice, "parallel_tool_calls": false, "reasoning": { @@ -491,7 +496,6 @@ SUMMARY_ONLY_CONTEXT" ]); normalize_line_endings(&mut expected); assert_eq!(requests.len(), 5); - dbg!(&expected); assert_eq!(json!(requests), expected); } diff --git a/codex-rs/core/tests/suite/prompt_caching.rs b/codex-rs/core/tests/suite/prompt_caching.rs index d6112ef82d..405f941244 100644 --- a/codex-rs/core/tests/suite/prompt_caching.rs +++ b/codex-rs/core/tests/suite/prompt_caching.rs @@ -21,6 +21,7 @@ use core_test_support::load_default_config_for_test; use core_test_support::load_sse_fixture_with_id; use core_test_support::skip_if_no_network; use core_test_support::wait_for_event; +use serde_json::Value; use std::collections::HashMap; use tempfile::TempDir; use wiremock::Mock; @@ -59,12 +60,19 @@ fn sse_completed(id: &str) -> String { } fn assert_tool_names(body: &serde_json::Value, expected_names: &[&str]) { + let tools = body["tools"].as_array().unwrap_or_else(|| { + panic!("tools field missing or not an array: {body:?}"); + }); assert_eq!( - body["tools"] - .as_array() - .unwrap() + tools .iter() - .map(|t| t["name"].as_str().unwrap().to_string()) + .map(|tool| { + tool.get("name") + .or_else(|| tool.get("type")) + .and_then(Value::as_str) + .unwrap_or_else(|| panic!("tool missing name/type: {tool:?}")) + .to_string() + }) .collect::>(), expected_names ); @@ -144,11 +152,25 @@ async fn codex_mini_latest_tools() { .join("\n"); let body0 = requests[0].body_json::().unwrap(); + assert!( + body0 + .get("tools") + .and_then(Value::as_array) + .is_some(), + "first request missing tools field: {body0:?}" + ); assert_eq!( body0["instructions"], serde_json::json!(expected_instructions), ); let body1 = requests[1].body_json::().unwrap(); + assert!( + body1 + .get("tools") + .and_then(Value::as_array) + .is_some(), + "second request missing tools field: {body1:?}" + ); assert_eq!( body1["instructions"], serde_json::json!(expected_instructions), @@ -224,10 +246,13 @@ async fn prompt_tools_are_consistent_across_requests() { // our internal implementation is responsible for keeping tools in sync // with the OpenAI schema, so we just verify the tool presence here let tools_by_model: HashMap<&'static str, Vec<&'static str>> = HashMap::from([ - ("gpt-5", vec!["shell", "update_plan", "view_image"]), + ( + "gpt-5", + vec!["shell", "update_plan", "web_search", "view_image"], + ), ( "gpt-5-codex", - vec!["shell", "update_plan", "apply_patch", "view_image"], + vec!["shell", "update_plan", "apply_patch", "web_search", "view_image"], ), ]); let expected_tools_names = tools_by_model