From 4c1a6f0ee0e79b3ddaa4e20155af817a5fbb25e2 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Fri, 7 Nov 2025 10:11:11 -0800 Subject: [PATCH] Promote shell config tool to model family config (#6351) --- codex-rs/core/src/model_family.rs | 19 ++++++++++--------- codex-rs/core/src/tools/spec.rs | 26 ++++++++++++++++++++------ 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/codex-rs/core/src/model_family.rs b/codex-rs/core/src/model_family.rs index a0444361..136af16b 100644 --- a/codex-rs/core/src/model_family.rs +++ b/codex-rs/core/src/model_family.rs @@ -1,5 +1,6 @@ use crate::config::types::ReasoningSummaryFormat; use crate::tools::handlers::apply_patch::ApplyPatchToolType; +use crate::tools::spec::ConfigShellToolType; /// The `instructions` field in the payload sent to a model should always start /// with this content. @@ -29,12 +30,6 @@ pub struct ModelFamily { // Define if we need a special handling of reasoning summary pub reasoning_summary_format: ReasoningSummaryFormat, - // This should be set to true when the model expects a tool named - // "local_shell" to be provided. Its contract must be understood natively by - // the model such that its description can be omitted. - // See https://platform.openai.com/docs/guides/tools-local-shell - pub uses_local_shell_tool: bool, - /// Whether this model supports parallel tool calls when using the /// Responses API. pub supports_parallel_tool_calls: bool, @@ -57,6 +52,9 @@ pub struct ModelFamily { /// If the model family supports setting the verbosity level when using Responses API. pub support_verbosity: bool, + + /// Preferred shell tool type for this model family when features do not override it. + pub shell_type: ConfigShellToolType, } macro_rules! model_family { @@ -64,19 +62,20 @@ macro_rules! model_family { $slug:expr, $family:expr $(, $key:ident : $value:expr )* $(,)? ) => {{ // defaults + #[allow(unused_mut)] let mut mf = ModelFamily { slug: $slug.to_string(), family: $family.to_string(), needs_special_apply_patch_instructions: false, supports_reasoning_summaries: false, reasoning_summary_format: ReasoningSummaryFormat::None, - uses_local_shell_tool: false, supports_parallel_tool_calls: false, apply_patch_tool_type: None, base_instructions: BASE_INSTRUCTIONS.to_string(), experimental_supported_tools: Vec::new(), effective_context_window_percent: 95, support_verbosity: false, + shell_type: ConfigShellToolType::Default, }; // apply overrides $( @@ -105,8 +104,8 @@ pub fn find_family_for_model(slug: &str) -> Option { model_family!( slug, "codex-mini-latest", supports_reasoning_summaries: true, - uses_local_shell_tool: true, needs_special_apply_patch_instructions: true, + shell_type: ConfigShellToolType::Local, ) } else if slug.starts_with("gpt-4.1") { model_family!( @@ -119,6 +118,8 @@ pub fn find_family_for_model(slug: &str) -> Option { model_family!(slug, "gpt-4o", needs_special_apply_patch_instructions: true) } else if slug.starts_with("gpt-3.5") { model_family!(slug, "gpt-3.5", needs_special_apply_patch_instructions: true) + } else if slug.starts_with("porcupine") { + model_family!(slug, "porcupine", shell_type: ConfigShellToolType::UnifiedExec) } else if slug.starts_with("test-gpt-5-codex") { model_family!( slug, slug, @@ -181,12 +182,12 @@ pub fn derive_default_model_family(model: &str) -> ModelFamily { needs_special_apply_patch_instructions: false, supports_reasoning_summaries: false, reasoning_summary_format: ReasoningSummaryFormat::None, - uses_local_shell_tool: false, supports_parallel_tool_calls: false, apply_patch_tool_type: None, base_instructions: BASE_INSTRUCTIONS.to_string(), experimental_supported_tools: Vec::new(), effective_context_window_percent: 95, support_verbosity: false, + shell_type: ConfigShellToolType::Default, } } diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 8f4e6f35..1e4a0d9e 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -15,7 +15,7 @@ use serde_json::json; use std::collections::BTreeMap; use std::collections::HashMap; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum ConfigShellToolType { Default, Local, @@ -42,17 +42,14 @@ impl ToolsConfig { model_family, features, } = params; - let use_unified_exec_tool = features.enabled(Feature::UnifiedExec); let include_apply_patch_tool = features.enabled(Feature::ApplyPatchFreeform); let include_web_search_request = features.enabled(Feature::WebSearchRequest); let include_view_image_tool = features.enabled(Feature::ViewImageTool); - let shell_type = if use_unified_exec_tool { + let shell_type = if features.enabled(Feature::UnifiedExec) { ConfigShellToolType::UnifiedExec - } else if model_family.uses_local_shell_tool { - ConfigShellToolType::Local } else { - ConfigShellToolType::Default + model_family.shell_type.clone() }; let apply_patch_tool_type = match model_family.apply_patch_tool_type { @@ -1212,6 +1209,23 @@ mod tests { ); } + #[test] + fn test_porcupine_defaults() { + assert_model_tools( + "porcupine", + &Features::with_defaults(), + &[ + "exec_command", + "write_stdin", + "list_mcp_resources", + "list_mcp_resource_templates", + "read_mcp_resource", + "update_plan", + "view_image", + ], + ); + } + #[test] fn test_codex_mini_unified_exec_web_search() { assert_model_tools(