From 38bb4a36cebd7a365489458f8b215820267640fd Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 1 Jun 2026 08:32:57 -0700 Subject: [PATCH] feat: gate unified exec zsh fork composition --- codex-rs/core/config.schema.json | 6 ++++ codex-rs/core/src/session/mod.rs | 1 - codex-rs/core/src/session/review.rs | 3 +- codex-rs/core/src/session/turn_context.rs | 4 +-- codex-rs/core/src/tools/spec_plan_tests.rs | 40 ++++++++++++++++++++++ codex-rs/features/src/lib.rs | 12 +++++++ codex-rs/tools/src/lib.rs | 1 + codex-rs/tools/src/tool_config.rs | 24 +++++++++++-- codex-rs/tools/src/tool_config_tests.rs | 26 ++++++++++++-- 9 files changed, 106 insertions(+), 11 deletions(-) diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 77d2c97d6e..d3473400d8 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -614,6 +614,9 @@ "unified_exec": { "type": "boolean" }, + "unified_exec_zsh_fork": { + "type": "boolean" + }, "use_legacy_landlock": { "type": "boolean" }, @@ -4731,6 +4734,9 @@ "unified_exec": { "type": "boolean" }, + "unified_exec_zsh_fork": { + "type": "boolean" + }, "use_legacy_landlock": { "type": "boolean" }, diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 1908c46bff..12254f192d 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -362,7 +362,6 @@ use codex_protocol::protocol::WarningEvent; use codex_protocol::user_input::UserInput; use codex_tools::ToolEnvironmentMode; use codex_tools::UnifiedExecShellMode; -use codex_tools::shell_command_backend_for_features; use codex_utils_absolute_path::AbsolutePathBuf; #[cfg(test)] use codex_utils_stream_parser::ProposedPlanSegment; diff --git a/codex-rs/core/src/session/review.rs b/codex-rs/core/src/session/review.rs index 18cacb4d09..38d6f64030 100644 --- a/codex-rs/core/src/session/review.rs +++ b/codex-rs/core/src/session/review.rs @@ -31,9 +31,8 @@ pub(super) async fn spawn_review_thread( .models_manager .list_models(RefreshStrategy::OnlineIfUncached) .await; - let shell_command_backend = shell_command_backend_for_features(review_features.get()); let unified_exec_shell_mode = UnifiedExecShellMode::for_session( - shell_command_backend, + codex_tools::unified_exec_uses_zsh_fork_for_features(review_features.get()), crate::tools::tool_user_shell_type(sess.services.user_shell.as_ref()), sess.services.shell_zsh_path.as_ref(), sess.services.main_execve_wrapper_exe.as_ref(), diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 3bdd2b7683..3a9e3bc867 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -480,10 +480,8 @@ impl Session { let provider_for_context = create_model_provider(provider, auth_manager); let session_telemetry_for_context = session_telemetry; let available_models = models_manager.try_list_models().unwrap_or_default(); - let shell_command_backend = - shell_command_backend_for_features(per_turn_config.features.get()); let unified_exec_shell_mode = UnifiedExecShellMode::for_session( - shell_command_backend, + codex_tools::unified_exec_uses_zsh_fork_for_features(per_turn_config.features.get()), crate::tools::tool_user_shell_type(user_shell), shell_zsh_path, main_execve_wrapper_exe, diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index e2b14b5b14..6e33f45d7c 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -408,6 +408,46 @@ async fn shell_family_registers_visible_unified_exec_and_hidden_legacy_shell() { assert_eq!(plan.exposure("shell_command"), ToolExposure::Hidden); } +#[tokio::test] +async fn shell_zsh_fork_stays_standalone_until_unified_exec_composition_is_enabled() { + let standalone = probe(|turn| { + set_features(turn, &[Feature::ShellTool, Feature::UnifiedExec]); + set_feature(turn, Feature::ShellZshFork, /*enabled*/ true); + set_feature(turn, Feature::UnifiedExecZshFork, /*enabled*/ false); + turn.model_info.shell_type = ConfigShellToolType::ShellCommand; + }) + .await; + + standalone.assert_visible_contains(&["shell_command"]); + standalone.assert_visible_lacks(&["exec_command", "write_stdin"]); + standalone.assert_registered_contains(&["shell_command"]); + standalone.assert_registered_lacks(&["exec_command", "write_stdin"]); + + let composed = probe(|turn| { + set_features( + turn, + &[ + Feature::ShellTool, + Feature::UnifiedExec, + Feature::ShellZshFork, + Feature::UnifiedExecZshFork, + ], + ); + turn.model_info.shell_type = ConfigShellToolType::ShellCommand; + }) + .await; + + if codex_utils_pty::conpty_supported() { + composed.assert_visible_contains(&["exec_command", "write_stdin"]); + composed.assert_visible_lacks(&["shell_command"]); + composed.assert_registered_contains(&["exec_command", "write_stdin", "shell_command"]); + assert_eq!(composed.exposure("shell_command"), ToolExposure::Hidden); + } else { + composed.assert_visible_contains(&["shell_command"]); + composed.assert_visible_lacks(&["exec_command", "write_stdin"]); + } +} + #[tokio::test] async fn environment_count_controls_environment_backed_tools() { let no_environment = probe(|turn| { diff --git a/codex-rs/features/src/lib.rs b/codex-rs/features/src/lib.rs index 116db0a5b5..5a7d5a5714 100644 --- a/codex-rs/features/src/lib.rs +++ b/codex-rs/features/src/lib.rs @@ -90,6 +90,12 @@ pub enum Feature { UnifiedExec, /// Route shell tool execution through the zsh exec bridge. ShellZshFork, + /// Allow unified exec to compose with the zsh exec bridge. + /// + /// This flag is only a composition gate. Enabling it by itself must not turn + /// on either `unified_exec` or `shell_zsh_fork` because those features have + /// separate rollout and enterprise controls. + UnifiedExecZshFork, /// Reflow transcript scrollback when the terminal is resized. TerminalResizeReflow, /// Stream structured progress while apply_patch input is being generated. @@ -739,6 +745,12 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::UnderDevelopment, default_enabled: false, }, + FeatureSpec { + id: Feature::UnifiedExecZshFork, + key: "unified_exec_zsh_fork", + stage: Stage::UnderDevelopment, + default_enabled: false, + }, FeatureSpec { id: Feature::ShellSnapshot, key: "shell_snapshot", diff --git a/codex-rs/tools/src/lib.rs b/codex-rs/tools/src/lib.rs index e09526048d..b44ace0936 100644 --- a/codex-rs/tools/src/lib.rs +++ b/codex-rs/tools/src/lib.rs @@ -76,6 +76,7 @@ pub use tool_config::ZshForkConfig; pub use tool_config::request_user_input_available_modes; pub use tool_config::shell_command_backend_for_features; pub use tool_config::shell_type_for_model_and_features; +pub use tool_config::unified_exec_uses_zsh_fork_for_features; pub use tool_definition::ToolDefinition; pub use tool_discovery::DiscoverablePluginInfo; pub use tool_discovery::DiscoverableTool; diff --git a/codex-rs/tools/src/tool_config.rs b/codex-rs/tools/src/tool_config.rs index 3eaa73f189..7bde6ca2a2 100644 --- a/codex-rs/tools/src/tool_config.rs +++ b/codex-rs/tools/src/tool_config.rs @@ -41,6 +41,22 @@ pub fn shell_command_backend_for_features(features: &Features) -> ShellCommandBa } } +/// Returns whether unified exec should launch local zsh commands through the +/// zsh fork. +/// +/// `unified_exec_zsh_fork` is only a composition gate. It does not enable +/// either underlying shell mode on its own, so disabling `unified_exec` or +/// `shell_zsh_fork` keeps those features independently off. This lets +/// enterprise deployments opt into, or out of, unified exec and zsh-fork +/// behavior separately; otherwise enabling the composition flag would silently +/// activate a shell backend that the configured feature set left disabled. +pub fn unified_exec_uses_zsh_fork_for_features(features: &Features) -> bool { + features.enabled(Feature::ShellTool) + && features.enabled(Feature::UnifiedExec) + && features.enabled(Feature::ShellZshFork) + && features.enabled(Feature::UnifiedExecZshFork) +} + pub fn shell_type_for_model_and_features( model_info: &ModelInfo, features: &Features, @@ -58,7 +74,9 @@ pub fn shell_type_for_model_and_features( if !features.enabled(Feature::ShellTool) { ConfigShellToolType::Disabled - } else if features.enabled(Feature::ShellZshFork) { + } else if features.enabled(Feature::ShellZshFork) + && !unified_exec_uses_zsh_fork_for_features(features) + { ConfigShellToolType::ShellCommand } else if unified_exec_enabled { if codex_utils_pty::conpty_supported() { @@ -85,13 +103,13 @@ pub struct ZshForkConfig { impl UnifiedExecShellMode { pub fn for_session( - shell_command_backend: ShellCommandBackendConfig, + use_zsh_fork: bool, user_shell_type: ToolUserShellType, shell_zsh_path: Option<&PathBuf>, main_execve_wrapper_exe: Option<&PathBuf>, ) -> Self { if cfg!(unix) - && shell_command_backend == ShellCommandBackendConfig::ZshFork + && use_zsh_fork && matches!(user_shell_type, ToolUserShellType::Zsh) && let (Some(shell_zsh_path), Some(main_execve_wrapper_exe)) = (shell_zsh_path, main_execve_wrapper_exe) diff --git a/codex-rs/tools/src/tool_config_tests.rs b/codex-rs/tools/src/tool_config_tests.rs index 2e49c15584..f72dfb5ea9 100644 --- a/codex-rs/tools/src/tool_config_tests.rs +++ b/codex-rs/tools/src/tool_config_tests.rs @@ -82,6 +82,12 @@ fn shell_type_is_derived_from_model_and_feature_gates() { ConfigShellToolType::ShellCommand ); + features.enable(Feature::UnifiedExecZshFork); + assert_eq!( + shell_type_for_model_and_features(&model, &features), + expected_unified_exec + ); + features.disable(Feature::ShellTool); assert_eq!( shell_type_for_model_and_features(&model, &features), @@ -110,6 +116,22 @@ fn shell_command_backend_requires_both_shell_tool_and_zsh_fork() { ); } +#[test] +fn unified_exec_uses_zsh_fork_only_when_composition_and_dependencies_are_enabled() { + let mut features = shell_features(); + assert_eq!(unified_exec_uses_zsh_fork_for_features(&features), false); + + features.enable(Feature::UnifiedExec); + features.enable(Feature::ShellZshFork); + assert_eq!(unified_exec_uses_zsh_fork_for_features(&features), false); + + features.enable(Feature::UnifiedExecZshFork); + assert_eq!(unified_exec_uses_zsh_fork_for_features(&features), true); + + features.disable(Feature::ShellTool); + assert_eq!(unified_exec_uses_zsh_fork_for_features(&features), false); +} + #[test] fn request_user_input_modes_follow_default_mode_feature() { let mut features = Features::with_defaults(); @@ -132,7 +154,7 @@ fn unified_exec_shell_mode_uses_zsh_fork_only_when_all_inputs_match() { let shell = exe.clone(); let mode = UnifiedExecShellMode::for_session( - ShellCommandBackendConfig::ZshFork, + /*use_zsh_fork*/ true, ToolUserShellType::Zsh, Some(&shell), Some(&exe), @@ -145,7 +167,7 @@ fn unified_exec_shell_mode_uses_zsh_fork_only_when_all_inputs_match() { assert_eq!( UnifiedExecShellMode::for_session( - ShellCommandBackendConfig::Classic, + /*use_zsh_fork*/ false, ToolUserShellType::Zsh, Some(&shell), Some(&exe),