From 995138d71ac06b9df5996f049e69dea414ad9764 Mon Sep 17 00:00:00 2001 From: "Adam Perry @ OpenAI" Date: Tue, 29 Sep 2026 20:27:36 +0000 Subject: [PATCH] Carry shell invocation metadata and report executor PATH directories (#49360) ## What changed - Carry the selected shell and login mode through unified exec using `ShellInvocation`. Use this metadata for shell snapshots and credential brokerage instead of inferring it from command arguments. - Add `prependPathDirs` to exec-server environment metadata, populated from the executor's package PATH directory. Default to an empty list for older servers and omit empty lists from serialized output. ## Testing Extend environment metadata round-trip coverage for `prependPathDirs` and the remote exec-server integration test to verify the packaged `codex-path` directory is reported. GitOrigin-RevId: c571c068effcb305d196a701ad9a335ec7a3f5a2 --- codex-rs/Cargo.lock | 1 + codex-rs/cli/tests/exec_server.rs | 11 +++++ codex-rs/core/src/shell.rs | 17 +++++++ .../core/src/tools/handlers/unified_exec.rs | 41 ++++++++--------- .../handlers/unified_exec/exec_command.rs | 3 +- .../src/tools/handlers/unified_exec_tests.rs | 2 +- .../core/src/tools/runtimes/unified_exec.rs | 46 +++++++++++-------- codex-rs/core/src/unified_exec/mod.rs | 4 +- .../core/src/unified_exec/process_manager.rs | 2 +- .../src/unified_exec/process_manager_tests.rs | 11 ++++- .../core/src/unified_exec/shell_snapshot.rs | 11 ++--- codex-rs/exec-server-protocol/Cargo.toml | 1 + codex-rs/exec-server-protocol/src/protocol.rs | 13 ++++++ 13 files changed, 107 insertions(+), 56 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index e8b1096596..c9162751bb 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3250,6 +3250,7 @@ version = "0.0.0" dependencies = [ "base64 0.22.1", "codex-file-system", + "codex-install-context", "codex-mxc-sandbox", "codex-network-proxy", "codex-protocol", diff --git a/codex-rs/cli/tests/exec_server.rs b/codex-rs/cli/tests/exec_server.rs index 75bb8344cf..001a69e619 100644 --- a/codex-rs/cli/tests/exec_server.rs +++ b/codex-rs/cli/tests/exec_server.rs @@ -187,6 +187,8 @@ metrics_exporter = {{ otlp-http = {{ endpoint = "{collector_url}/v1/metrics", pr std::fs::create_dir(&bin_dir)?; let executable = bin_dir.join(format!("codex{}", std::env::consts::EXE_SUFFIX)); copy_executable(&codex_utils_cargo_bin::cargo_bin("codex")?, &executable)?; + let codex_path_dir = package.path().join("codex-path"); + std::fs::create_dir(&codex_path_dir)?; let manifest = package.path().join("codex-package.json"); std::fs::write(&manifest, r#"{"version":"1.2.3-alpha.4"}"#)?; @@ -250,10 +252,19 @@ metrics_exporter = {{ otlp-http = {{ endpoint = "{collector_url}/v1/metrics", pr .context("remote harness did not connect")???; let environment_info = client.environment_info().await?; + assert_eq!( + environment_info + .prepend_path_dirs + .iter() + .map(|path| std::fs::canonicalize(path.inferred_native_path_string())) + .collect::>>()?, + vec![codex_path_dir.canonicalize()?] + ); let expected_info = EnvironmentInfo { executor_version: "1.2.3-alpha.4".to_string(), // The build identity belongs to the spawned CLI, not this test process. provider_id: environment_info.provider_id.clone(), + prepend_path_dirs: environment_info.prepend_path_dirs.clone(), ..EnvironmentInfo::local() }; assert_eq!(environment_info, expected_info); diff --git a/codex-rs/core/src/shell.rs b/codex-rs/core/src/shell.rs index 37a35bfe3f..c253383610 100644 --- a/codex-rs/core/src/shell.rs +++ b/codex-rs/core/src/shell.rs @@ -76,6 +76,23 @@ impl Shell { } } +/// The shell and startup mode Codex chose for an exec command. +#[derive(Debug, Clone)] +pub(crate) struct ShellInvocation { + pub(crate) shell: Shell, + pub(crate) use_login_shell: bool, +} + +impl ShellInvocation { + pub(crate) fn is_posix_login(&self) -> bool { + self.use_login_shell + && matches!( + self.shell.shell_type, + ShellType::Bash | ShellType::Zsh | ShellType::Sh + ) + } +} + #[cfg(all(test, unix))] fn ultimate_fallback_shell() -> Shell { codex_shell_command::shell_detect::ultimate_fallback_shell().into() diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index 9f9f8a8cf1..763fa15576 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -1,5 +1,6 @@ use crate::sandboxing::SandboxPermissions; use crate::shell::Shell; +use crate::shell::ShellInvocation; use crate::shell::ShellType; use crate::shell::get_shell_by_model_provided_path; use crate::tools::context::ToolInvocation; @@ -74,7 +75,7 @@ fn default_tty() -> bool { #[derive(Debug)] pub(crate) struct ResolvedCommand { pub(crate) command: Vec, - pub(crate) shell_type: ShellType, + pub(crate) shell: ShellInvocation, } fn post_unified_exec_tool_use_payload( @@ -112,35 +113,31 @@ pub(crate) fn get_command( None => allow_login_shell, }; - match shell_mode { - UnifiedExecShellMode::Direct => { - let model_shell = args - .shell - .as_ref() - .map(|shell_str| get_shell_by_model_provided_path(&PathBuf::from(shell_str))); - let shell = model_shell.as_ref().unwrap_or(session_shell.as_ref()); - Ok(ResolvedCommand { - command: shell.derive_exec_args(&args.cmd, use_login_shell), - shell_type: shell.shell_type, - }) - } + let shell = match shell_mode { + UnifiedExecShellMode::Direct => args + .shell + .as_ref() + .map(|shell_str| get_shell_by_model_provided_path(&PathBuf::from(shell_str))) + .unwrap_or_else(|| session_shell.as_ref().clone()), UnifiedExecShellMode::ZshFork(zsh_fork_config) => { if args.shell.is_some() { return Err( "`shell` is not supported for local zsh-fork exec; omit `shell` to use zsh-fork, or target a remote environment where `shell` is supported.".to_string(), ); } - - Ok(ResolvedCommand { - command: vec![ - zsh_fork_config.shell_zsh_path.to_string_lossy().to_string(), - if use_login_shell { "-lc" } else { "-c" }.to_string(), - args.cmd.clone(), - ], + Shell { shell_type: ShellType::Zsh, - }) + shell_path: zsh_fork_config.shell_zsh_path.as_path().to_path_buf(), + } } - } + }; + Ok(ResolvedCommand { + command: shell.derive_exec_args(&args.cmd, use_login_shell), + shell: ShellInvocation { + shell, + use_login_shell, + }, + }) } pub(crate) fn shell_mode_for_environment( diff --git a/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs b/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs index bdf6877389..ba5dd79efb 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs @@ -297,7 +297,6 @@ impl ExecCommandHandler { ) .map_err(FunctionCallError::RespondToModel)?; let command = resolved_command.command; - let shell_type = resolved_command.shell_type; let ExecCommandArgs { mut tty, yield_time_ms, @@ -418,7 +417,7 @@ impl ExecCommandHandler { let process_id = manager.allocate_process_id().await; let request = ExecCommandRequest { command, - shell_type, + shell: resolved_command.shell, hook_command: hook_command.clone(), process_id, yield_time_ms, diff --git a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs index 46fa63dbbb..4657b64fbc 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs @@ -130,7 +130,7 @@ fn test_get_command_resolves_powershell_by_type() -> anyhow::Result<()> { resolved.command, expected_shell.derive_exec_args("echo hello", /*use_login_shell*/ true) ); - assert_eq!(resolved.shell_type, expected_shell.shell_type); + assert_eq!(resolved.shell.shell, expected_shell); Ok(()) } diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 4c662e02b3..c6ef29c606 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -14,6 +14,7 @@ use crate::sandboxing::ExecOptions; use crate::sandboxing::ExecServerEnvConfig; use crate::sandboxing::SandboxPermissions; use crate::session::turn_context::TurnEnvironment; +use crate::shell::ShellInvocation; use crate::shell::ShellType; use crate::shell_snapshot::ShellSnapshotSandbox; use crate::shell_snapshot::snapshot_read_permissions; @@ -58,7 +59,6 @@ use codex_tools::UnifiedExecShellMode; use codex_utils_path_uri::PathUri; use std::collections::HashMap; use std::io; -use std::path::PathBuf; use std::time::Duration; use tokio_util::sync::CancellationToken; @@ -74,7 +74,7 @@ const REMOTE_NETWORK_POLICY_DECISION_MARGIN: Duration = Duration::from_secs(10); #[derive(Clone, Debug)] pub struct UnifiedExecRequest { pub command: Vec, - pub shell_type: ShellType, + pub(crate) shell: ShellInvocation, pub hook_command: String, pub process_id: i32, pub cwd: PathUri, @@ -271,6 +271,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt ctx: &ToolCtx, ) -> Result { let base_command = &req.command; + let requested_shell = &req.shell.shell; let windows_sandbox_proxy_settings_mode = ctx.session.windows_sandbox_proxy_settings_mode; let session_shell = ctx.session.user_shell(); let environment_shell = req @@ -306,19 +307,11 @@ impl<'a> ToolRuntime for UnifiedExecRunt "credential brokerage does not yet support shell_zsh_fork; disable shell_zsh_fork to use the broker".to_string(), )); } - let requested_shell = (credential_broker_available - && (req.shell_type != environment_shell.shell_type - || base_command.first().is_some_and(|path| { - path != environment_shell.shell_path.to_string_lossy().as_ref() - }))) - .then(|| { - base_command.first().map(|path| crate::shell::Shell { - shell_type: req.shell_type, - shell_path: PathBuf::from(path), - }) - }) - .flatten(); - let shell = requested_shell.as_ref().unwrap_or(environment_shell); + let shell = if credential_broker_available { + requested_shell + } else { + environment_shell + }; let shell_snapshot = if environment_is_remote || credential_broker_available && launch_sandbox_permissions.requires_escalated_permissions() @@ -348,7 +341,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt let shell_snapshot_location = shell_snapshot.as_ref().map(|snapshot| snapshot.path()); let mut env = exec_env_for_sandbox_permissions(&req.env, launch_sandbox_permissions); let snapshot_credential_context = if let Some(snapshot) = shell_snapshot.as_ref() - && (managed_network.is_some() || base_command.get(1).is_some_and(|flag| flag == "-lc")) + && (managed_network.is_some() || req.shell.is_posix_login()) { Some( snapshot @@ -576,7 +569,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt // preparation is a no-op for PowerShell on other platforms. let command = prepare_powershell_command_for_windows_sandbox( &command, - Some(&req.shell_type), + Some(&requested_shell.shell_type), attempt.sandbox_requested, executor_windows_sandbox_selection( attempt.windows_sandbox_type, @@ -585,7 +578,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt ), environment_is_remote, ); - let command = if matches!(req.shell_type, ShellType::PowerShell) { + let command = if matches!(requested_shell.shell_type, ShellType::PowerShell) { prefix_powershell_script_with_utf8(&command) } else { command @@ -761,6 +754,7 @@ mod tests { use crate::config::PermissionProfileSnapshot; use crate::environment_selection::EnvironmentConfigOrigin; use crate::exec::DEFAULT_EXEC_COMMAND_TIMEOUT_MS; + use crate::shell::Shell; use crate::tools::sandboxing::ToolRuntime; use codex_exec_server::Environment; use codex_exec_server::LOCAL_ENVIRONMENT_ID; @@ -864,7 +858,13 @@ mod tests { let runtime = UnifiedExecRuntime::new(&manager, UnifiedExecShellMode::Direct); let request = UnifiedExecRequest { command: vec!["pwd".to_string()], - shell_type: ShellType::Sh, + shell: ShellInvocation { + shell: Shell { + shell_type: ShellType::Sh, + shell_path: "sh".into(), + }, + use_login_shell: false, + }, hook_command: "pwd".to_string(), process_id: 1000, cwd: cwd.into(), @@ -967,7 +967,13 @@ mod tests { .expect("current dir is absolute"); UnifiedExecRequest { command: vec!["zsh".to_string(), "-c".to_string(), "echo hi".to_string()], - shell_type: ShellType::Zsh, + shell: ShellInvocation { + shell: Shell { + shell_type: ShellType::Zsh, + shell_path: "zsh".into(), + }, + use_login_shell: false, + }, hook_command: "echo hi".to_string(), process_id: 1000, cwd: cwd.clone().into(), diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index 3645306c27..28af9b0229 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -43,7 +43,7 @@ use crate::session::session::Session; use crate::session::step_context::StepContext; use crate::session::turn_context::TurnContext; use crate::session::turn_context::TurnEnvironment; -use crate::shell::ShellType; +use crate::shell::ShellInvocation; use crate::tools::network_approval::DeferredNetworkApproval; use codex_core_plugins::PluginMetricsSidecar; @@ -113,7 +113,7 @@ impl UnifiedExecContext { #[derive(Debug)] pub(crate) struct ExecCommandRequest { pub command: Vec, - pub shell_type: ShellType, + pub shell: ShellInvocation, pub hook_command: String, pub process_id: i32, pub yield_time_ms: u64, diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 3dc3e97d91..565194441f 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -1505,7 +1505,7 @@ impl UnifiedExecProcessManager { .await; let req = UnifiedExecToolRequest { command: request.command.clone(), - shell_type: request.shell_type, + shell: request.shell.clone(), hook_command: request.hook_command.clone(), process_id: request.process_id, cwd, diff --git a/codex-rs/core/src/unified_exec/process_manager_tests.rs b/codex-rs/core/src/unified_exec/process_manager_tests.rs index 7edc743ce4..ce56681e68 100644 --- a/codex-rs/core/src/unified_exec/process_manager_tests.rs +++ b/codex-rs/core/src/unified_exec/process_manager_tests.rs @@ -1,4 +1,7 @@ use super::*; +use crate::shell::Shell; +use crate::shell::ShellInvocation; +use crate::shell::ShellType; use crate::unified_exec::clamp_yield_time; use codex_network_proxy::ManagedNetworkSandboxContext; use core_test_support::assert_regex_match; @@ -459,7 +462,13 @@ async fn failed_initial_end_for_unstored_process_uses_fallback_output() { "-lc".to_string(), "echo before".to_string(), ], - shell_type: crate::shell::ShellType::Sh, + shell: ShellInvocation { + shell: Shell { + shell_type: ShellType::Sh, + shell_path: "sh".into(), + }, + use_login_shell: true, + }, hook_command: "echo before".to_string(), process_id: 123, yield_time_ms: 1000, diff --git a/codex-rs/core/src/unified_exec/shell_snapshot.rs b/codex-rs/core/src/unified_exec/shell_snapshot.rs index dc79ea4619..8deffbaba9 100644 --- a/codex-rs/core/src/unified_exec/shell_snapshot.rs +++ b/codex-rs/core/src/unified_exec/shell_snapshot.rs @@ -179,15 +179,12 @@ pub(super) fn shell_snapshot_request( || !request.turn_environment.shell_snapshot_v2_supported || request.turn_environment.selection.cwd != *cwd || !matches!(request.shell_mode, UnifiedExecShellMode::Direct) - || !matches!( - request.shell_type, - ShellType::Bash | ShellType::Zsh | ShellType::Sh - ) - || request.command.get(1).is_none_or(|flag| flag != "-lc") + || !request.shell.is_posix_login() { return None; } + let shell = &request.shell.shell; Some(ShellSnapshotRequest { scope_id: format!( "{}:{}", @@ -195,8 +192,8 @@ pub(super) fn shell_snapshot_request( request.turn_environment.selection.environment_id ), shell: ShellInfo { - name: request.shell_type.name().to_string(), - path: request.command.first()?.clone(), + name: shell.name().to_string(), + path: shell.shell_path.to_string_lossy().into_owned(), }, }) } diff --git a/codex-rs/exec-server-protocol/Cargo.toml b/codex-rs/exec-server-protocol/Cargo.toml index 62841ed973..eda1add272 100644 --- a/codex-rs/exec-server-protocol/Cargo.toml +++ b/codex-rs/exec-server-protocol/Cargo.toml @@ -15,6 +15,7 @@ workspace = true [dependencies] base64 = { workspace = true } codex-file-system = { workspace = true } +codex-install-context = { workspace = true } codex-network-proxy = { workspace = true } codex-protocol = { workspace = true } codex-shell-command = { workspace = true } diff --git a/codex-rs/exec-server-protocol/src/protocol.rs b/codex-rs/exec-server-protocol/src/protocol.rs index 84b266432f..97402d5e12 100644 --- a/codex-rs/exec-server-protocol/src/protocol.rs +++ b/codex-rs/exec-server-protocol/src/protocol.rs @@ -6,6 +6,7 @@ use codex_file_system::FileSystemSandboxContext; pub use codex_file_system::WalkOptions; pub use codex_file_system::WalkOutcome; use codex_file_system::WireFileSystemSandboxContext; +use codex_install_context::InstallContext; use codex_network_proxy::ManagedNetworkSandboxContext; use codex_network_proxy::RemoteNetworkProxyLaunchConfig; use codex_protocol::ThreadId; @@ -116,6 +117,9 @@ pub struct EnvironmentInfo { /// Operating system reported by the executor; absent for legacy exec-servers. #[serde(default, skip_serializing_if = "Option::is_none")] pub platform_os: Option, + /// Executor directories to prepend to `PATH` when missing, in priority order. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub prepend_path_dirs: Vec, /// Executor-local default directories for resolving `:tmpdir`, when reported. /// On Windows, a command's `TEMP` or `TMP` overrides take precedence. #[serde(default, skip_serializing_if = "Option::is_none")] @@ -252,6 +256,13 @@ impl EnvironmentInfo { cwd: cwd.and_then(|cwd| PathUri::from_host_native_path(cwd).ok()), user_home_dir: PathUri::from_host_native_path("~").ok(), platform_os: Some(std::env::consts::OS.to_string()), + prepend_path_dirs: InstallContext::current() + .package_layout + .as_ref() + .and_then(|layout| layout.path_dir.as_ref()) + .into_iter() + .map(PathUri::from_abs_path) + .collect(), temporary_directories: Some(temporary_directories), temp_dir, capabilities: EnvironmentCapabilities { @@ -1372,6 +1383,7 @@ mod tests { cwd: None, user_home_dir: None, platform_os: None, + prepend_path_dirs: Vec::new(), temporary_directories: None, temp_dir: None, capabilities: EnvironmentCapabilities::default(), @@ -1447,6 +1459,7 @@ mod tests { "cwd": null, "userHomeDir": "file:///C:/Users/remote", "platformOs": "windows", + "prependPathDirs": ["file:///C:/tools/bin", "file:///D:/tools/bin"], "temporaryDirectories": ["file:///C:/Temp", "file:///D:/Temp"], "capabilities": { "networkProxyLaunch": false,