mirror of
https://github.com/openai/codex.git
synced 2026-11-08 20:40:33 +00:00
feat: gate unified exec zsh fork composition
This commit is contained in:
@@ -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"
|
||||
},
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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| {
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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),
|
||||
|
||||
Reference in New Issue
Block a user