From c368c6aeea43717e3e05182dbc8a53eb85037ac4 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Thu, 6 Nov 2025 15:46:24 -0800 Subject: [PATCH] Remove shell tool when unified exec is enabled (#6345) Also drop streameable shell that's just an alias for unified exec. --- codex-rs/core/src/codex.rs | 3 +- codex-rs/core/src/config/mod.rs | 13 +- codex-rs/core/src/config/profile.rs | 1 - codex-rs/core/src/features.rs | 14 +-- codex-rs/core/src/features/legacy.rs | 11 -- codex-rs/core/src/tasks/review.rs | 6 +- codex-rs/core/src/tools/spec.rs | 111 +++++++++++++----- .../core/tests/suite/deprecation_notice.rs | 15 ++- codex-rs/core/tests/suite/model_tools.rs | 1 - docs/example-config.md | 4 - 10 files changed, 94 insertions(+), 85 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 70728ba6..fc6b44c6 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1643,8 +1643,7 @@ async fn spawn_review_thread( let mut review_features = config.features.clone(); review_features .disable(crate::features::Feature::WebSearchRequest) - .disable(crate::features::Feature::ViewImageTool) - .disable(crate::features::Feature::StreamableShell); + .disable(crate::features::Feature::ViewImageTool); let tools_config = ToolsConfig::new(&ToolsConfigParams { model_family: &review_model_family, features: &review_features, diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 1ee48d30..0dc9d126 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -241,8 +241,6 @@ pub struct Config { /// When `true`, run a model-based assessment for commands denied by the sandbox. pub experimental_sandbox_command_assessment: bool, - pub use_experimental_streamable_shell_tool: bool, - /// If set to `true`, used only the experimental unified exec tool. pub use_experimental_unified_exec_tool: bool, @@ -655,7 +653,6 @@ pub struct ConfigToml { /// Legacy, now use features pub experimental_instructions_file: Option, pub experimental_compact_prompt_file: Option, - pub experimental_use_exec_command_tool: Option, pub experimental_use_unified_exec_tool: Option, pub experimental_use_rmcp_client: Option, pub experimental_use_freeform_apply_patch: Option, @@ -999,7 +996,6 @@ impl Config { let include_apply_patch_tool_flag = features.enabled(Feature::ApplyPatchFreeform); let tools_web_search_request = features.enabled(Feature::WebSearchRequest); - let use_experimental_streamable_shell_tool = features.enabled(Feature::StreamableShell); let use_experimental_unified_exec_tool = features.enabled(Feature::UnifiedExec); let use_experimental_use_rmcp_client = features.enabled(Feature::RmcpClient); let experimental_sandbox_command_assessment = @@ -1156,7 +1152,6 @@ impl Config { include_apply_patch_tool: include_apply_patch_tool_flag, tools_web_search_request, experimental_sandbox_command_assessment, - use_experimental_streamable_shell_tool, use_experimental_unified_exec_tool, use_experimental_use_rmcp_client, features, @@ -1715,7 +1710,6 @@ trust_level = "trusted" fn legacy_toggles_map_to_features() -> std::io::Result<()> { let codex_home = TempDir::new()?; let cfg = ConfigToml { - experimental_use_exec_command_tool: Some(true), experimental_use_unified_exec_tool: Some(true), experimental_use_rmcp_client: Some(true), experimental_use_freeform_apply_patch: Some(true), @@ -1729,12 +1723,11 @@ trust_level = "trusted" )?; assert!(config.features.enabled(Feature::ApplyPatchFreeform)); - assert!(config.features.enabled(Feature::StreamableShell)); assert!(config.features.enabled(Feature::UnifiedExec)); assert!(config.features.enabled(Feature::RmcpClient)); assert!(config.include_apply_patch_tool); - assert!(config.use_experimental_streamable_shell_tool); + assert!(config.use_experimental_unified_exec_tool); assert!(config.use_experimental_use_rmcp_client); @@ -2902,7 +2895,6 @@ model_verbosity = "high" include_apply_patch_tool: false, tools_web_search_request: false, experimental_sandbox_command_assessment: false, - use_experimental_streamable_shell_tool: false, use_experimental_unified_exec_tool: false, use_experimental_use_rmcp_client: false, features: Features::with_defaults(), @@ -2974,7 +2966,6 @@ model_verbosity = "high" include_apply_patch_tool: false, tools_web_search_request: false, experimental_sandbox_command_assessment: false, - use_experimental_streamable_shell_tool: false, use_experimental_unified_exec_tool: false, use_experimental_use_rmcp_client: false, features: Features::with_defaults(), @@ -3061,7 +3052,6 @@ model_verbosity = "high" include_apply_patch_tool: false, tools_web_search_request: false, experimental_sandbox_command_assessment: false, - use_experimental_streamable_shell_tool: false, use_experimental_unified_exec_tool: false, use_experimental_use_rmcp_client: false, features: Features::with_defaults(), @@ -3134,7 +3124,6 @@ model_verbosity = "high" include_apply_patch_tool: false, tools_web_search_request: false, experimental_sandbox_command_assessment: false, - use_experimental_streamable_shell_tool: false, use_experimental_unified_exec_tool: false, use_experimental_use_rmcp_client: false, features: Features::with_defaults(), diff --git a/codex-rs/core/src/config/profile.rs b/codex-rs/core/src/config/profile.rs index f7eb2dce..6d872546 100644 --- a/codex-rs/core/src/config/profile.rs +++ b/codex-rs/core/src/config/profile.rs @@ -25,7 +25,6 @@ pub struct ConfigProfile { pub experimental_compact_prompt_file: Option, pub include_apply_patch_tool: Option, pub experimental_use_unified_exec_tool: Option, - pub experimental_use_exec_command_tool: Option, pub experimental_use_rmcp_client: Option, pub experimental_use_freeform_apply_patch: Option, pub experimental_sandbox_command_assessment: Option, diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index f6cec4a5..e34128f3 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -29,8 +29,6 @@ pub enum Stage { pub enum Feature { /// Use the single unified PTY-backed exec tool. UnifiedExec, - /// Use the streamable exec-command/write-stdin tool pair. - StreamableShell, /// Enable experimental RMCP features such as OAuth login. RmcpClient, /// Include the freeform apply_patch tool. @@ -118,8 +116,9 @@ impl Features { self.enabled.contains(&f) } - pub fn enable(&mut self, f: Feature) { + pub fn enable(&mut self, f: Feature) -> &mut Self { self.enabled.insert(f); + self } pub fn disable(&mut self, f: Feature) -> &mut Self { @@ -178,7 +177,6 @@ impl Features { let base_legacy = LegacyFeatureToggles { experimental_sandbox_command_assessment: cfg.experimental_sandbox_command_assessment, experimental_use_freeform_apply_patch: cfg.experimental_use_freeform_apply_patch, - experimental_use_exec_command_tool: cfg.experimental_use_exec_command_tool, experimental_use_unified_exec_tool: cfg.experimental_use_unified_exec_tool, experimental_use_rmcp_client: cfg.experimental_use_rmcp_client, tools_web_search: cfg.tools.as_ref().and_then(|t| t.web_search), @@ -197,7 +195,7 @@ impl Features { .experimental_sandbox_command_assessment, experimental_use_freeform_apply_patch: config_profile .experimental_use_freeform_apply_patch, - experimental_use_exec_command_tool: config_profile.experimental_use_exec_command_tool, + experimental_use_unified_exec_tool: config_profile.experimental_use_unified_exec_tool, experimental_use_rmcp_client: config_profile.experimental_use_rmcp_client, tools_web_search: config_profile.tools_web_search, @@ -252,12 +250,6 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::Experimental, default_enabled: false, }, - FeatureSpec { - id: Feature::StreamableShell, - key: "streamable_shell", - stage: Stage::Experimental, - default_enabled: false, - }, FeatureSpec { id: Feature::RmcpClient, key: "rmcp_client", diff --git a/codex-rs/core/src/features/legacy.rs b/codex-rs/core/src/features/legacy.rs index de3c007b..4d59f2a9 100644 --- a/codex-rs/core/src/features/legacy.rs +++ b/codex-rs/core/src/features/legacy.rs @@ -17,10 +17,6 @@ const ALIASES: &[Alias] = &[ legacy_key: "experimental_use_unified_exec_tool", feature: Feature::UnifiedExec, }, - Alias { - legacy_key: "experimental_use_exec_command_tool", - feature: Feature::StreamableShell, - }, Alias { legacy_key: "experimental_use_rmcp_client", feature: Feature::RmcpClient, @@ -54,7 +50,6 @@ pub struct LegacyFeatureToggles { pub include_apply_patch_tool: Option, pub experimental_sandbox_command_assessment: Option, pub experimental_use_freeform_apply_patch: Option, - pub experimental_use_exec_command_tool: Option, pub experimental_use_unified_exec_tool: Option, pub experimental_use_rmcp_client: Option, pub tools_web_search: Option, @@ -81,12 +76,6 @@ impl LegacyFeatureToggles { self.experimental_use_freeform_apply_patch, "experimental_use_freeform_apply_patch", ); - set_if_some( - features, - Feature::StreamableShell, - self.experimental_use_exec_command_tool, - "experimental_use_exec_command_tool", - ); set_if_some( features, Feature::UnifiedExec, diff --git a/codex-rs/core/src/tasks/review.rs b/codex-rs/core/src/tasks/review.rs index 57258f4c..e0bb7d4e 100644 --- a/codex-rs/core/src/tasks/review.rs +++ b/codex-rs/core/src/tasks/review.rs @@ -75,12 +75,12 @@ async fn start_review_conversation( // Avoid loading project docs; reviewer only needs findings sub_agent_config.project_doc_max_bytes = 0; // Carry over review-only feature restrictions so the delegate cannot - // re-enable blocked tools (web search, view image, streamable shell). + // re-enable blocked tools (web search, view image). sub_agent_config .features .disable(crate::features::Feature::WebSearchRequest) - .disable(crate::features::Feature::ViewImageTool) - .disable(crate::features::Feature::StreamableShell); + .disable(crate::features::Feature::ViewImageTool); + // Set explicit review rubric for the sub-agent sub_agent_config.base_instructions = Some(crate::REVIEW_PROMPT.to_string()); (run_codex_conversation_one_shot( diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index eba9fd51..8f4e6f35 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -19,7 +19,7 @@ use std::collections::HashMap; pub enum ConfigShellToolType { Default, Local, - Streamable, + UnifiedExec, } #[derive(Debug, Clone)] @@ -28,7 +28,6 @@ pub(crate) struct ToolsConfig { pub apply_patch_tool_type: Option, pub web_search_request: bool, pub include_view_image_tool: bool, - pub experimental_unified_exec_tool: bool, pub experimental_supported_tools: Vec, } @@ -43,14 +42,13 @@ impl ToolsConfig { model_family, features, } = params; - let use_streamable_shell_tool = features.enabled(Feature::StreamableShell); - let experimental_unified_exec_tool = features.enabled(Feature::UnifiedExec); + 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_streamable_shell_tool { - ConfigShellToolType::Streamable + let shell_type = if use_unified_exec_tool { + ConfigShellToolType::UnifiedExec } else if model_family.uses_local_shell_tool { ConfigShellToolType::Local } else { @@ -74,7 +72,6 @@ impl ToolsConfig { apply_patch_tool_type, web_search_request: include_web_search_request, include_view_image_tool, - experimental_unified_exec_tool, experimental_supported_tools: model_family.experimental_supported_tools.clone(), } } @@ -886,15 +883,6 @@ pub(crate) fn build_specs( let mcp_handler = Arc::new(McpHandler); let mcp_resource_handler = Arc::new(McpResourceHandler); - let use_unified_exec = config.experimental_unified_exec_tool - || matches!(config.shell_type, ConfigShellToolType::Streamable); - - if use_unified_exec { - builder.push_spec(create_exec_command_tool()); - builder.push_spec(create_write_stdin_tool()); - builder.register_handler("exec_command", unified_exec_handler.clone()); - builder.register_handler("write_stdin", unified_exec_handler); - } match &config.shell_type { ConfigShellToolType::Default => { builder.push_spec(create_shell_tool()); @@ -902,8 +890,11 @@ pub(crate) fn build_specs( ConfigShellToolType::Local => { builder.push_spec(ToolSpec::LocalShell {}); } - ConfigShellToolType::Streamable => { - // Already handled by use_unified_exec. + ConfigShellToolType::UnifiedExec => { + builder.push_spec(create_exec_command_tool()); + builder.push_spec(create_write_stdin_tool()); + builder.register_handler("exec_command", unified_exec_handler.clone()); + builder.register_handler("write_stdin", unified_exec_handler); } } @@ -1045,7 +1036,7 @@ mod tests { match config.shell_type { ConfigShellToolType::Default => Some("shell"), ConfigShellToolType::Local => Some("local_shell"), - ConfigShellToolType::Streamable => None, + ConfigShellToolType::UnifiedExec => None, } } @@ -1095,7 +1086,7 @@ mod tests { } #[test] - fn test_full_toolset_specs_for_gpt5_codex() { + fn test_full_toolset_specs_for_gpt5_codex_unified_exec_web_search() { let model_family = find_family_for_model("gpt-5-codex") .expect("gpt-5-codex should be a valid model family"); let mut features = Features::with_defaults(); @@ -1129,7 +1120,6 @@ mod tests { for spec in [ create_exec_command_tool(), create_write_stdin_tool(), - create_shell_tool(), create_list_mcp_resources_tool(), create_list_mcp_resource_templates_tool(), create_read_mcp_resource_tool(), @@ -1156,32 +1146,89 @@ mod tests { } } - #[test] - fn test_build_specs_contains_expected_basics() { - let model_family = find_family_for_model("codex-mini-latest") - .expect("codex-mini-latest should be a valid model family"); - let mut features = Features::with_defaults(); - features.enable(Feature::WebSearchRequest); - features.enable(Feature::UnifiedExec); + fn assert_model_tools(model_family: &str, features: &Features, expected_tools: &[&str]) { + let model_family = find_family_for_model(model_family) + .unwrap_or_else(|| panic!("{model_family} should be a valid model family")); let config = ToolsConfig::new(&ToolsConfigParams { model_family: &model_family, - features: &features, + features, }); let (tools, _) = build_specs(&config, Some(HashMap::new())).build(); let tool_names = tools.iter().map(|t| t.spec.name()).collect::>(); - assert_eq!( - &tool_names, + assert_eq!(&tool_names, &expected_tools,); + } + + #[test] + fn test_build_specs_gpt5_codex_default() { + assert_model_tools( + "gpt-5-codex", + &Features::with_defaults(), + &[ + "shell", + "list_mcp_resources", + "list_mcp_resource_templates", + "read_mcp_resource", + "update_plan", + "apply_patch", + "view_image", + ], + ); + } + + #[test] + fn test_build_specs_gpt5_codex_unified_exec_web_search() { + assert_model_tools( + "gpt-5-codex", + Features::with_defaults() + .enable(Feature::UnifiedExec) + .enable(Feature::WebSearchRequest), &[ "exec_command", "write_stdin", + "list_mcp_resources", + "list_mcp_resource_templates", + "read_mcp_resource", + "update_plan", + "apply_patch", + "web_search", + "view_image", + ], + ); + } + + #[test] + fn test_codex_mini_defaults() { + assert_model_tools( + "codex-mini-latest", + &Features::with_defaults(), + &[ "local_shell", "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( + "codex-mini-latest", + Features::with_defaults() + .enable(Feature::UnifiedExec) + .enable(Feature::WebSearchRequest), + &[ + "exec_command", + "write_stdin", + "list_mcp_resources", + "list_mcp_resource_templates", + "read_mcp_resource", + "update_plan", "web_search", "view_image", - ] + ], ); } diff --git a/codex-rs/core/tests/suite/deprecation_notice.rs b/codex-rs/core/tests/suite/deprecation_notice.rs index 6842bd4d..4e240f0a 100644 --- a/codex-rs/core/tests/suite/deprecation_notice.rs +++ b/codex-rs/core/tests/suite/deprecation_notice.rs @@ -18,12 +18,11 @@ async fn emits_deprecation_notice_for_legacy_feature_flag() -> anyhow::Result<() let server = start_mock_server().await; let mut builder = test_codex().with_config(|config| { - config.features.enable(Feature::StreamableShell); - config.features.record_legacy_usage_force( - "experimental_use_exec_command_tool", - Feature::StreamableShell, - ); - config.use_experimental_streamable_shell_tool = true; + config.features.enable(Feature::UnifiedExec); + config + .features + .record_legacy_usage_force("use_experimental_unified_exec_tool", Feature::UnifiedExec); + config.use_experimental_unified_exec_tool = true; }); let TestCodex { codex, .. } = builder.build(&server).await?; @@ -37,13 +36,13 @@ async fn emits_deprecation_notice_for_legacy_feature_flag() -> anyhow::Result<() let DeprecationNoticeEvent { summary, details } = notice; assert_eq!( summary, - "`experimental_use_exec_command_tool` is deprecated. Use `streamable_shell` instead." + "`use_experimental_unified_exec_tool` is deprecated. Use `unified_exec` instead." .to_string(), ); assert_eq!( details.as_deref(), Some( - "Enable it with `--enable streamable_shell` or `[features].streamable_shell` in config.toml. See https://github.com/openai/codex/blob/main/docs/config.md#feature-flags for details." + "Enable it with `--enable unified_exec` or `[features].unified_exec` in config.toml. See https://github.com/openai/codex/blob/main/docs/config.md#feature-flags for details." ), ); diff --git a/codex-rs/core/tests/suite/model_tools.rs b/codex-rs/core/tests/suite/model_tools.rs index 2a04b88a..c7622f8b 100644 --- a/codex-rs/core/tests/suite/model_tools.rs +++ b/codex-rs/core/tests/suite/model_tools.rs @@ -60,7 +60,6 @@ async fn collect_tool_identifiers_for_model(model: &str) -> Vec { config.features.disable(Feature::ApplyPatchFreeform); config.features.disable(Feature::ViewImageTool); config.features.disable(Feature::WebSearchRequest); - config.features.disable(Feature::StreamableShell); config.features.disable(Feature::UnifiedExec); let conversation_manager = diff --git a/docs/example-config.md b/docs/example-config.md index 531a3856..524cbbe0 100644 --- a/docs/example-config.md +++ b/docs/example-config.md @@ -221,9 +221,6 @@ enable_experimental_windows_sandbox = false # Experimental toggles (legacy; prefer [features]) ################################################################################ -# Use experimental exec command tool (streamable shell). Default: false -experimental_use_exec_command_tool = false - # Use experimental unified exec tool. Default: false experimental_use_unified_exec_tool = false @@ -328,7 +325,6 @@ mcp_oauth_credentials_store = "auto" # experimental_compact_prompt_file = "compact_prompt.txt" # include_apply_patch_tool = false # experimental_use_unified_exec_tool = false -# experimental_use_exec_command_tool = false # experimental_use_rmcp_client = false # experimental_use_freeform_apply_patch = false # experimental_sandbox_command_assessment = false