Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions codex-rs/core/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,9 @@
"unified_exec": {
"type": "boolean"
},
"unified_exec_zsh_fork": {
"type": "boolean"
},
"use_legacy_landlock": {
"type": "boolean"
},
Expand Down Expand Up @@ -4737,6 +4740,9 @@
"unified_exec": {
"type": "boolean"
},
"unified_exec_zsh_fork": {
"type": "boolean"
},
"use_legacy_landlock": {
"type": "boolean"
},
Expand Down
1 change: 0 additions & 1 deletion codex-rs/core/src/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 1 addition & 2 deletions codex-rs/core/src/session/review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
4 changes: 1 addition & 3 deletions codex-rs/core/src/session/turn_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
40 changes: 40 additions & 0 deletions codex-rs/core/src/tools/spec_plan_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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| {
Expand Down
12 changes: 12 additions & 0 deletions codex-rs/features/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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",
Expand Down
2 changes: 2 additions & 0 deletions codex-rs/tools/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
70 changes: 57 additions & 13 deletions codex-rs/tools/src/tool_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -41,33 +54,64 @@ 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 => {
ConfigShellToolType::ShellCommand
}
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
}
}
}
}
}

Expand All @@ -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)
Expand Down
51 changes: 49 additions & 2 deletions codex-rs/tools/src/tool_config_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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();
Expand All @@ -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),
Expand All @@ -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),
Expand Down
Loading