diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 43313fbb08f8..6dd85dffc2b6 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 1908c46bffe3..12254f192d53 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 18cacb4d09d7..c476436560ea 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 3bdd2b76838e..1e0a3f17c549 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 d82fa55ee6c4..3f832ed0acc1 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 e2ee60bef3e4..ce1ee6940eb3 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 e09526048d53..3adae5923a00 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 3eaa73f189ec..990d4a3c58ca 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 2e49c155847f..1f2800138a8c 100644 --- a/codex-rs/tools/src/tool_config_tests.rs +++ b/codex-rs/tools/src/tool_config_tests.rs @@ -53,6 +53,7 @@ fn shell_features() -> Features { features.enable(Feature::ShellTool); features.disable(Feature::ShellZshFork); features.disable(Feature::UnifiedExec); + features.disable(Feature::UnifiedExecZshFork); features } @@ -82,6 +83,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 +117,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(); @@ -132,7 +179,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), @@ -145,7 +192,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),