From d6748f741a188f5f681c69d26c258c2a3a409f1b Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 1 Jun 2026 13:01:36 -0700 Subject: [PATCH] feat: gate unified exec zsh fork composition (#24979) ## Why `shell_zsh_fork` and unified exec need to remain independently controllable for enterprise rollouts, but we also need a third mode that composes them. That composed mode is intended to preserve unified exec command lifecycle support while letting the zsh fork provide more accurate `execv(2)` interception. Enabling `unified_exec_zsh_fork` by itself is intentionally not sufficient. It is a composition gate, not a dependency-enabling shortcut: - `unified_exec` selects the PTY-backed unified exec tool. - `shell_zsh_fork` opts into the zsh fork backend. - `unified_exec_zsh_fork` only allows those two already-enabled modes to be composed so local zsh unified exec commands can launch through the zsh fork. This separation is deliberate. Enterprises and staged rollouts must be able to enable or disable unified exec and zsh-fork independently. If `unified_exec_zsh_fork` implied either dependency, then enabling one under-development composition flag would silently activate a shell backend that the configured feature set left disabled. This PR introduces only the configuration and planning gate for that composition. Existing `shell_zsh_fork` behavior continues to use the standalone shell tool unless the new composition feature is explicitly enabled alongside both dependencies. ## What Changed - Added the under-development feature flag `unified_exec_zsh_fork`. - Added `UnifiedExecFeatureMode` so the three input feature flags collapse into `Disabled`, `Direct`, or `ZshFork` mode before tool planning. - Updated tool selection so zsh-fork composition requires `unified_exec`, `shell_zsh_fork`, and `unified_exec_zsh_fork`. - Kept the existing standalone zsh-fork shell tool behavior when only `shell_zsh_fork` is enabled. - Updated config schema output for the new feature flag. ## Verification - Added feature and tool-config coverage for the new gate. - Added planner coverage proving `shell_zsh_fork` remains standalone until composition is explicitly enabled. - Ran focused tests for `codex-features`, `codex-tools`, and the affected `codex-core` planner case. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/24979). * #24982 * #24981 * #24980 * __->__ #24979 --- 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 | 2 + codex-rs/tools/src/tool_config.rs | 70 ++++++++++++++++++---- codex-rs/tools/src/tool_config_tests.rs | 51 +++++++++++++++- 9 files changed, 168 insertions(+), 21 deletions(-) diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 43313fbb0..6dd85dffc 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -617,6 +617,9 @@ "unified_exec": { "type": "boolean" }, + "unified_exec_zsh_fork": { + "type": "boolean" + }, "use_legacy_landlock": { "type": "boolean" }, @@ -4737,6 +4740,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 1908c46bf..12254f192 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 18cacb4d0..c47643656 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_feature_mode_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 3bdd2b768..1e0a3f17c 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_feature_mode_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 d82fa55ee..3f832ed0a 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 e2ee60bef..ce1ee6940 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. @@ -741,6 +747,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 e09526048..3adae5923 100644 --- a/codex-rs/tools/src/lib.rs +++ b/codex-rs/tools/src/lib.rs @@ -71,11 +71,13 @@ pub use tool_call::TurnItemEmitter; pub use tool_config::ShellCommandBackendConfig; pub use tool_config::ToolEnvironmentMode; pub use tool_config::ToolUserShellType; +pub use tool_config::UnifiedExecFeatureMode; pub use tool_config::UnifiedExecShellMode; 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_feature_mode_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 3eaa73f18..990d4a3c5 100644 --- a/codex-rs/tools/src/tool_config.rs +++ b/codex-rs/tools/src/tool_config.rs @@ -13,6 +13,19 @@ pub enum ShellCommandBackendConfig { ZshFork, } +#[derive(Debug, Clone, Copy, Eq, PartialEq)] +pub enum UnifiedExecFeatureMode { + /// Unified exec should not be selected by this feature set. + /// + /// This includes standalone `shell_zsh_fork`: until + /// `unified_exec_zsh_fork` is enabled too, `shell_zsh_fork` keeps using + /// the shell command backend instead of silently opting unified exec into + /// zsh-fork interception. + Disabled, + Direct, + ZshFork, +} + #[derive(Debug, Clone, Copy, Eq, PartialEq)] pub enum ToolUserShellType { Zsh, @@ -41,13 +54,39 @@ pub fn shell_command_backend_for_features(features: &Features) -> ShellCommandBa } } +/// Returns the unified-exec mode requested by feature policy, before runtime +/// session inputs such as platform, user shell, and zsh-fork binary paths are +/// resolved. +/// +/// `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_feature_mode_for_features(features: &Features) -> UnifiedExecFeatureMode { + if !features.enabled(Feature::ShellTool) || !features.enabled(Feature::UnifiedExec) { + UnifiedExecFeatureMode::Disabled + } else if features.enabled(Feature::ShellZshFork) { + if features.enabled(Feature::UnifiedExecZshFork) { + UnifiedExecFeatureMode::ZshFork + } else { + UnifiedExecFeatureMode::Disabled + } + } else { + UnifiedExecFeatureMode::Direct + } +} + pub fn shell_type_for_model_and_features( model_info: &ModelInfo, features: &Features, ) -> ConfigShellToolType { - let unified_exec_enabled = features.enabled(Feature::UnifiedExec); + let unified_exec_feature_mode = unified_exec_feature_mode_for_features(features); + let unified_exec_disabled = + matches!(unified_exec_feature_mode, UnifiedExecFeatureMode::Disabled); let model_shell_type = match model_info.shell_type { - ConfigShellToolType::UnifiedExec if !unified_exec_enabled => { + ConfigShellToolType::UnifiedExec if unified_exec_disabled => { ConfigShellToolType::ShellCommand } ConfigShellToolType::Default | ConfigShellToolType::Local => { @@ -55,19 +94,24 @@ pub fn shell_type_for_model_and_features( } other => other, }; + let shell_command_type = match shell_command_backend_for_features(features) { + ShellCommandBackendConfig::Classic => model_shell_type, + ShellCommandBackendConfig::ZshFork => ConfigShellToolType::ShellCommand, + }; if !features.enabled(Feature::ShellTool) { ConfigShellToolType::Disabled - } else if features.enabled(Feature::ShellZshFork) { - ConfigShellToolType::ShellCommand - } else if unified_exec_enabled { - if codex_utils_pty::conpty_supported() { - ConfigShellToolType::UnifiedExec - } else { - ConfigShellToolType::ShellCommand - } } else { - model_shell_type + match unified_exec_feature_mode { + UnifiedExecFeatureMode::Disabled => shell_command_type, + UnifiedExecFeatureMode::Direct | UnifiedExecFeatureMode::ZshFork => { + if codex_utils_pty::conpty_supported() { + ConfigShellToolType::UnifiedExec + } else { + ConfigShellToolType::ShellCommand + } + } + } } } @@ -85,13 +129,13 @@ pub struct ZshForkConfig { impl UnifiedExecShellMode { pub fn for_session( - shell_command_backend: ShellCommandBackendConfig, + feature_mode: UnifiedExecFeatureMode, user_shell_type: ToolUserShellType, shell_zsh_path: Option<&PathBuf>, main_execve_wrapper_exe: Option<&PathBuf>, ) -> Self { if cfg!(unix) - && shell_command_backend == ShellCommandBackendConfig::ZshFork + && matches!(feature_mode, UnifiedExecFeatureMode::ZshFork) && 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 e7fc4d8c1..165d6efa2 100644 --- a/codex-rs/tools/src/tool_config_tests.rs +++ b/codex-rs/tools/src/tool_config_tests.rs @@ -54,6 +54,7 @@ fn shell_features() -> Features { features.enable(Feature::ShellTool); features.disable(Feature::ShellZshFork); features.disable(Feature::UnifiedExec); + features.disable(Feature::UnifiedExecZshFork); features } @@ -83,6 +84,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), @@ -111,6 +118,46 @@ fn shell_command_backend_requires_both_shell_tool_and_zsh_fork() { ); } +#[test] +fn unified_exec_feature_mode_follows_composition_dependencies() { + let mut features = shell_features(); + assert_eq!( + unified_exec_feature_mode_for_features(&features), + UnifiedExecFeatureMode::Disabled + ); + + features.enable(Feature::UnifiedExec); + assert_eq!( + unified_exec_feature_mode_for_features(&features), + UnifiedExecFeatureMode::Direct + ); + + features.enable(Feature::UnifiedExecZshFork); + assert_eq!( + unified_exec_feature_mode_for_features(&features), + UnifiedExecFeatureMode::Direct + ); + + features.enable(Feature::ShellZshFork); + features.disable(Feature::UnifiedExecZshFork); + assert_eq!( + unified_exec_feature_mode_for_features(&features), + UnifiedExecFeatureMode::Disabled + ); + + features.enable(Feature::UnifiedExecZshFork); + assert_eq!( + unified_exec_feature_mode_for_features(&features), + UnifiedExecFeatureMode::ZshFork + ); + + features.disable(Feature::ShellTool); + assert_eq!( + unified_exec_feature_mode_for_features(&features), + UnifiedExecFeatureMode::Disabled + ); +} + #[test] fn request_user_input_modes_follow_default_mode_feature() { let mut features = Features::with_defaults(); @@ -133,7 +180,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, + UnifiedExecFeatureMode::ZshFork, ToolUserShellType::Zsh, Some(&shell), Some(&exe), @@ -146,7 +193,7 @@ fn unified_exec_shell_mode_uses_zsh_fork_only_when_all_inputs_match() { assert_eq!( UnifiedExecShellMode::for_session( - ShellCommandBackendConfig::Classic, + UnifiedExecFeatureMode::Direct, ToolUserShellType::Zsh, Some(&shell), Some(&exe),