From 1aa3e432ccf9a853cbe536c159f90d79c576314d Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Mon, 15 Jun 2026 08:18:13 -0700 Subject: [PATCH] [codex] simplify shell snapshot ownership (#27756) ## Why Shell snapshot lifecycle state was split between `Shell` and `SessionServices`: `Shell` carried the receiver while session code exposed and forwarded the raw sender. That coupled shell identity to mutable snapshot state and made refresh, inheritance, and file lifetime harder to reason about. ## What changed - make each `Arc` represent one cwd-specific snapshot generation - store the active generation in `SessionServices` with `ArcSwapOption` - have construction start the background build and expose only a cwd-validated snapshot path - use `ShellSnapshotFile` ownership to delete snapshot files automatically - pass snapshot paths explicitly to shell runtimes instead of storing snapshot state on `Shell` - preserve inherited and in-flight generations by pinning their `Arc` while they are in use ## Test plan - `cargo check -p codex-core --lib` - `just test -p codex-core 'shell_snapshot::tests'` - `just test -p codex-core shell_command_snapshot_still_intercepts_apply_patch` - `just test -p codex-core shell_snapshot_deleted_after_shutdown_with_skills` --- codex-rs/core/src/agent/control.rs | 8 +- .../src/context/environment_context_tests.rs | 1 - codex-rs/core/src/session/mod.rs | 23 +- codex-rs/core/src/session/session.rs | 40 ++- codex-rs/core/src/session/tests.rs | 6 +- codex-rs/core/src/shell.rs | 31 +- codex-rs/core/src/shell_snapshot.rs | 83 +++--- codex-rs/core/src/shell_snapshot_tests.rs | 52 +++- codex-rs/core/src/shell_tests.rs | 4 - codex-rs/core/src/state/service.rs | 4 +- codex-rs/core/src/tasks/user_shell.rs | 18 +- codex-rs/core/src/tasks/user_shell_tests.rs | 31 +- .../core/src/tools/handlers/shell_tests.rs | 13 - .../core/src/tools/handlers/unified_exec.rs | 9 +- codex-rs/core/src/tools/runtimes/mod.rs | 13 +- codex-rs/core/src/tools/runtimes/mod_tests.rs | 277 ++++-------------- codex-rs/core/src/tools/runtimes/shell.rs | 6 +- .../core/src/tools/runtimes/unified_exec.rs | 6 +- 18 files changed, 225 insertions(+), 400 deletions(-) diff --git a/codex-rs/core/src/agent/control.rs b/codex-rs/core/src/agent/control.rs index 53a50e8e1..bd982b3c6 100644 --- a/codex-rs/core/src/agent/control.rs +++ b/codex-rs/core/src/agent/control.rs @@ -532,7 +532,13 @@ impl AgentControl { }; let parent_thread = state.get_thread(*parent_thread_id).await.ok()?; - parent_thread.codex.session.user_shell().shell_snapshot() + let snapshot = parent_thread + .codex + .session + .services + .shell_snapshot + .load_full()?; + (!snapshot.is_failed()).then_some(snapshot) } async fn inherited_exec_policy_for_source( diff --git a/codex-rs/core/src/context/environment_context_tests.rs b/codex-rs/core/src/context/environment_context_tests.rs index c8e491b68..5f391ca38 100644 --- a/codex-rs/core/src/context/environment_context_tests.rs +++ b/codex-rs/core/src/context/environment_context_tests.rs @@ -22,7 +22,6 @@ fn fake_shell_name() -> String { let shell = crate::shell::Shell { shell_type: ShellType::Bash, shell_path: PathBuf::from("/bin/bash"), - shell_snapshot: crate::shell::empty_shell_snapshot_receiver(), }; shell.name().to_string() } diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index f22b1fb62..8e6702088 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -1407,10 +1407,6 @@ impl Session { return; } - if !self.features.enabled(Feature::ShellSnapshot) { - return; - } - if matches!( session_source, SessionSource::SubAgent(SubAgentSource::ThreadSpawn { .. }) @@ -1418,15 +1414,16 @@ impl Session { return; } - ShellSnapshot::refresh_snapshot( - codex_home.clone(), - self.thread_id, - next_cwd.clone(), - self.services.user_shell.as_ref().clone(), - self.services.shell_snapshot_tx.clone(), - self.services.session_telemetry.clone(), - self.services.state_db.clone(), - ); + if self.services.shell_snapshot.load().is_some() { + self.services.shell_snapshot.store(Some(ShellSnapshot::new( + codex_home.clone(), + self.thread_id, + next_cwd.clone(), + self.services.user_shell.as_ref(), + self.services.session_telemetry.clone(), + self.services.state_db.clone(), + ))); + } } pub(crate) async fn update_settings( diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 0461925d8..dff94692e 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -810,7 +810,7 @@ impl Session { ); let use_zsh_fork_shell = config.features.enabled(Feature::ShellZshFork); - let mut default_shell = if let Some(user_shell_override) = + let default_shell = if let Some(user_shell_override) = session_configuration.user_shell_override.clone() { user_shell_override @@ -830,27 +830,21 @@ impl Session { } else { shell::default_user_shell() }; - // Create the mutable state for the Session. - let shell_snapshot_tx = if config.features.enabled(Feature::ShellSnapshot) { - if let Some(snapshot) = session_configuration.inherited_shell_snapshot.clone() { - let (tx, rx) = watch::channel(Some(snapshot)); - default_shell.shell_snapshot = rx; - tx - } else { - ShellSnapshot::start_snapshotting( - config.codex_home.clone(), - thread_id, - session_configuration.cwd().clone(), - &mut default_shell, - session_telemetry.clone(), - state_db_ctx.clone(), - ) - } - } else { - let (tx, rx) = watch::channel(None); - default_shell.shell_snapshot = rx; - tx - }; + let shell_snapshot = config.features.enabled(Feature::ShellSnapshot).then(|| { + session_configuration + .inherited_shell_snapshot + .clone() + .unwrap_or_else(|| { + ShellSnapshot::new( + config.codex_home.clone(), + thread_id, + session_configuration.cwd().clone(), + &default_shell, + session_telemetry.clone(), + state_db_ctx.clone(), + ) + }) + }); let thread_name = thread_title_from_thread_store(live_thread_init.as_ref(), &thread_store, thread_id) .instrument(info_span!( @@ -994,7 +988,7 @@ impl Session { hooks: arc_swap::ArcSwap::from_pointee(hooks), rollout_thread_trace, user_shell: Arc::new(default_shell), - shell_snapshot_tx, + shell_snapshot: arc_swap::ArcSwapOption::from(shell_snapshot), show_raw_agent_reasoning: config.show_raw_agent_reasoning, exec_policy, auth_manager: Arc::clone(&auth_manager), diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 460b2b473..810398e97 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -4961,7 +4961,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { })), rollout_thread_trace: codex_rollout_trace::ThreadTraceContext::disabled(), user_shell: Arc::new(default_user_shell()), - shell_snapshot_tx: watch::channel(None).0, + shell_snapshot: arc_swap::ArcSwapOption::empty(), show_raw_agent_reasoning: config.show_raw_agent_reasoning, exec_policy, auth_manager: auth_manager.clone(), @@ -6966,7 +6966,7 @@ where })), rollout_thread_trace: codex_rollout_trace::ThreadTraceContext::disabled(), user_shell: Arc::new(default_user_shell()), - shell_snapshot_tx: watch::channel(None).0, + shell_snapshot: arc_swap::ArcSwapOption::empty(), show_raw_agent_reasoning: config.show_raw_agent_reasoning, exec_policy, auth_manager: Arc::clone(&auth_manager), @@ -7235,7 +7235,6 @@ async fn environment_context_uses_session_shell_when_environment_shell_is_absent session.services.user_shell = Arc::new(crate::shell::Shell { shell_type: crate::shell::ShellType::PowerShell, shell_path: PathBuf::from("powershell"), - shell_snapshot: crate::shell::empty_shell_snapshot_receiver(), }); for environment in &mut turn_context.environments.turn_environments { environment.shell = None; @@ -7260,7 +7259,6 @@ async fn environment_context_uses_session_shell_when_environment_shell_is_absent primary_environment.shell = Some(crate::shell::Shell { shell_type: crate::shell::ShellType::Cmd, shell_path: PathBuf::from("cmd"), - shell_snapshot: crate::shell::empty_shell_snapshot_receiver(), }); let environment_context = crate::context::EnvironmentContext::from_turn_context( diff --git a/codex-rs/core/src/shell.rs b/codex-rs/core/src/shell.rs index b1049ef1d..ccd1ea20d 100644 --- a/codex-rs/core/src/shell.rs +++ b/codex-rs/core/src/shell.rs @@ -1,24 +1,15 @@ -use crate::shell_snapshot::ShellSnapshot; use codex_exec_server::ShellInfo; use codex_shell_command::shell_detect::DetectedShell; use serde::Deserialize; use serde::Serialize; use std::path::PathBuf; -use std::sync::Arc; -use tokio::sync::watch; pub use codex_shell_command::shell_detect::ShellType; -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct Shell { pub(crate) shell_type: ShellType, pub(crate) shell_path: PathBuf, - #[serde( - skip_serializing, - skip_deserializing, - default = "empty_shell_snapshot_receiver" - )] - pub(crate) shell_snapshot: watch::Receiver>>, } impl Shell { @@ -56,32 +47,13 @@ impl Shell { } } } - - /// Return the shell snapshot if existing. - pub fn shell_snapshot(&self) -> Option> { - self.shell_snapshot.borrow().clone() - } } -pub(crate) fn empty_shell_snapshot_receiver() -> watch::Receiver>> { - let (_tx, rx) = watch::channel(None); - rx -} - -impl PartialEq for Shell { - fn eq(&self, other: &Self) -> bool { - self.shell_type == other.shell_type && self.shell_path == other.shell_path - } -} - -impl Eq for Shell {} - impl From for Shell { fn from(detected: DetectedShell) -> Self { Self { shell_type: detected.shell_type, shell_path: detected.shell_path, - shell_snapshot: empty_shell_snapshot_receiver(), } } } @@ -100,7 +72,6 @@ impl Shell { Ok(Self { shell_type, shell_path: PathBuf::from(shell_info.path), - shell_snapshot: empty_shell_snapshot_receiver(), }) } } diff --git a/codex-rs/core/src/shell_snapshot.rs b/codex-rs/core/src/shell_snapshot.rs index 1cbd53866..a2cfbef3d 100644 --- a/codex-rs/core/src/shell_snapshot.rs +++ b/codex-rs/core/src/shell_snapshot.rs @@ -6,6 +6,7 @@ use std::time::Duration; use std::time::SystemTime; use crate::StateDbHandle; +use crate::path_utils; use crate::rollout::list::find_thread_path_by_id_str; use crate::shell::Shell; use crate::shell::ShellType; @@ -24,10 +25,13 @@ use tokio::time::timeout; use tracing::Instrument; use tracing::info_span; -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct ShellSnapshot { - pub path: AbsolutePathBuf, - pub cwd: AbsolutePathBuf, +pub(crate) struct ShellSnapshot { + cwd: AbsolutePathBuf, + state_rx: watch::Receiver>>, +} + +struct ShellSnapshotFile { + path: AbsolutePathBuf, } const SNAPSHOT_TIMEOUT: Duration = Duration::from_secs(10); @@ -36,48 +40,45 @@ const SNAPSHOT_DIR: &str = "shell_snapshots"; const EXCLUDED_EXPORT_VARS: &[&str] = &["PWD", "OLDPWD"]; impl ShellSnapshot { - pub fn start_snapshotting( + pub(crate) fn new( codex_home: AbsolutePathBuf, session_id: ThreadId, session_cwd: AbsolutePathBuf, - shell: &mut Shell, + shell: &Shell, session_telemetry: SessionTelemetry, state_db: Option, - ) -> watch::Sender>> { - let (shell_snapshot_tx, shell_snapshot_rx) = watch::channel(None); - shell.shell_snapshot = shell_snapshot_rx; - + ) -> Arc { + let (state_tx, state_rx) = watch::channel(None); + let snapshot = Arc::new(Self { + cwd: session_cwd.clone(), + state_rx, + }); Self::spawn_snapshot_task( codex_home, session_id, session_cwd, shell.clone(), - shell_snapshot_tx.clone(), + state_tx, session_telemetry, state_db, ); - - shell_snapshot_tx + snapshot } - pub fn refresh_snapshot( - codex_home: AbsolutePathBuf, - session_id: ThreadId, - session_cwd: AbsolutePathBuf, - shell: Shell, - shell_snapshot_tx: watch::Sender>>, - session_telemetry: SessionTelemetry, - state_db: Option, - ) { - Self::spawn_snapshot_task( - codex_home, - session_id, - session_cwd, - shell, - shell_snapshot_tx, - session_telemetry, - state_db, - ); + pub(crate) fn location(&self, cwd: &AbsolutePathBuf) -> Option { + if !path_utils::paths_match_after_normalization(self.cwd.as_path(), cwd) { + return None; + } + + self.state_rx + .borrow() + .as_ref() + .and_then(|snapshot| snapshot.as_ref().ok()) + .map(|snapshot| snapshot.path.clone()) + } + + pub(crate) fn is_failed(&self) -> bool { + matches!(&*self.state_rx.borrow(), Some(Err(_))) } fn spawn_snapshot_task( @@ -85,7 +86,7 @@ impl ShellSnapshot { session_id: ThreadId, session_cwd: AbsolutePathBuf, snapshot_shell: Shell, - shell_snapshot_tx: watch::Sender>>, + state_tx: watch::Sender>>, session_telemetry: SessionTelemetry, state_db: Option, ) { @@ -93,15 +94,14 @@ impl ShellSnapshot { tokio::spawn( async move { let timer = session_telemetry.start_timer("codex.shell_snapshot.duration_ms", &[]); - let snapshot = ShellSnapshot::try_new( + let snapshot = ShellSnapshot::try_create( &codex_home, session_id, &session_cwd, &snapshot_shell, state_db, ) - .await - .map(Arc::new); + .await; let success = snapshot.is_ok(); let success_tag = if success { "true" } else { "false" }; let _ = timer.map(|timer| timer.record(&[("success", success_tag)])); @@ -110,19 +110,19 @@ impl ShellSnapshot { counter_tags.push(("failure_reason", *failure_reason)); } session_telemetry.counter("codex.shell_snapshot", /*inc*/ 1, &counter_tags); - let _ = shell_snapshot_tx.send(snapshot.ok()); + let _ = state_tx.send(Some(snapshot)); } .instrument(snapshot_span), ); } - async fn try_new( + async fn try_create( codex_home: &AbsolutePathBuf, session_id: ThreadId, session_cwd: &AbsolutePathBuf, shell: &Shell, state_db: Option, - ) -> std::result::Result { + ) -> std::result::Result { // File to store the snapshot let extension = match shell.shell_type { ShellType::PowerShell => "ps1", @@ -175,14 +175,11 @@ impl ShellSnapshot { return Err("write_failed"); } - Ok(Self { - path, - cwd: session_cwd.clone(), - }) + Ok(ShellSnapshotFile { path }) } } -impl Drop for ShellSnapshot { +impl Drop for ShellSnapshotFile { fn drop(&mut self) { if let Err(err) = std::fs::remove_file(&self.path) { tracing::warn!( diff --git a/codex-rs/core/src/shell_snapshot_tests.rs b/codex-rs/core/src/shell_snapshot_tests.rs index 0199347b4..de07d9a13 100644 --- a/codex-rs/core/src/shell_snapshot_tests.rs +++ b/codex-rs/core/src/shell_snapshot_tests.rs @@ -189,15 +189,14 @@ fn bash_snapshot_preserves_multiline_exports() -> Result<()> { #[cfg(unix)] #[tokio::test] -async fn try_new_creates_and_deletes_snapshot_file() -> Result<()> { +async fn try_create_creates_and_deletes_snapshot_file() -> Result<()> { let dir = tempdir()?; let shell = Shell { shell_type: ShellType::Bash, shell_path: PathBuf::from("/bin/bash"), - shell_snapshot: crate::shell::empty_shell_snapshot_receiver(), }; - let snapshot = ShellSnapshot::try_new( + let snapshot = ShellSnapshot::try_create( &dir.path().abs(), ThreadId::new(), &dir.path().abs(), @@ -208,7 +207,6 @@ async fn try_new_creates_and_deletes_snapshot_file() -> Result<()> { .expect("snapshot should be created"); let path = snapshot.path.clone(); assert!(path.exists()); - assert_eq!(snapshot.cwd, dir.path().abs()); drop(snapshot); @@ -217,18 +215,53 @@ async fn try_new_creates_and_deletes_snapshot_file() -> Result<()> { Ok(()) } +#[test] +fn location_matches_cwd_and_channel_drop_deletes_file() -> Result<()> { + let dir = tempdir()?; + let path = dir.path().join("snapshot.sh").abs(); + std::fs::write(&path, "# snapshot")?; + let cwd = dir.path().join("worktree-a").abs(); + let other_cwd = dir.path().join("worktree-b").abs(); + let (state_tx, state_rx) = watch::channel(Some(Ok(ShellSnapshotFile { path: path.clone() }))); + let snapshot = ShellSnapshot { + cwd: cwd.clone(), + state_rx, + }; + + assert_eq!(snapshot.location(&cwd), Some(path.clone())); + assert_eq!(snapshot.location(&other_cwd), None); + + drop(state_tx); + drop(snapshot); + assert!(!path.exists()); + + Ok(()) +} + +#[test] +fn snapshot_state_distinguishes_building_from_failed() -> Result<()> { + let cwd = tempdir()?.path().abs(); + let (state_tx, state_rx) = watch::channel(None); + let snapshot = ShellSnapshot { cwd, state_rx }; + + assert!(!snapshot.is_failed()); + state_tx.send(Some(Err("failed")))?; + assert!(snapshot.is_failed()); + + Ok(()) +} + #[cfg(unix)] #[tokio::test] -async fn try_new_uses_distinct_generation_paths() -> Result<()> { +async fn try_create_uses_distinct_generation_paths() -> Result<()> { let dir = tempdir()?; let session_id = ThreadId::new(); let shell = Shell { shell_type: ShellType::Bash, shell_path: PathBuf::from("/bin/bash"), - shell_snapshot: crate::shell::empty_shell_snapshot_receiver(), }; - let initial_snapshot = ShellSnapshot::try_new( + let initial_snapshot = ShellSnapshot::try_create( &dir.path().abs(), session_id, &dir.path().abs(), @@ -237,7 +270,7 @@ async fn try_new_uses_distinct_generation_paths() -> Result<()> { ) .await .expect("initial snapshot should be created"); - let refreshed_snapshot = ShellSnapshot::try_new( + let refreshed_snapshot = ShellSnapshot::try_create( &dir.path().abs(), session_id, &dir.path().abs(), @@ -248,7 +281,6 @@ async fn try_new_uses_distinct_generation_paths() -> Result<()> { .expect("refreshed snapshot should be created"); let initial_path = initial_snapshot.path.clone(); let refreshed_path = refreshed_snapshot.path.clone(); - assert_ne!(initial_path, refreshed_path); assert_eq!(initial_path.exists(), true); assert_eq!(refreshed_path.exists(), true); @@ -282,7 +314,6 @@ async fn snapshot_shell_does_not_inherit_stdin() -> Result<()> { let shell = Shell { shell_type: ShellType::Bash, shell_path: PathBuf::from("/bin/bash"), - shell_snapshot: crate::shell::empty_shell_snapshot_receiver(), }; let home_display = home.display(); @@ -331,7 +362,6 @@ async fn timed_out_snapshot_shell_is_terminated() -> Result<()> { let shell = Shell { shell_type: ShellType::Sh, shell_path: PathBuf::from("/bin/sh"), - shell_snapshot: crate::shell::empty_shell_snapshot_receiver(), }; let err = run_script_with_timeout( diff --git a/codex-rs/core/src/shell_tests.rs b/codex-rs/core/src/shell_tests.rs index f0974900b..dea4a4f68 100644 --- a/codex-rs/core/src/shell_tests.rs +++ b/codex-rs/core/src/shell_tests.rs @@ -106,7 +106,6 @@ fn derive_exec_args() { let test_bash_shell = Shell { shell_type: ShellType::Bash, shell_path: PathBuf::from("/bin/bash"), - shell_snapshot: empty_shell_snapshot_receiver(), }; assert_eq!( test_bash_shell.derive_exec_args("echo hello", /*use_login_shell*/ false), @@ -120,7 +119,6 @@ fn derive_exec_args() { let test_zsh_shell = Shell { shell_type: ShellType::Zsh, shell_path: PathBuf::from("/bin/zsh"), - shell_snapshot: empty_shell_snapshot_receiver(), }; assert_eq!( test_zsh_shell.derive_exec_args("echo hello", /*use_login_shell*/ false), @@ -134,7 +132,6 @@ fn derive_exec_args() { let test_powershell_shell = Shell { shell_type: ShellType::PowerShell, shell_path: PathBuf::from("pwsh.exe"), - shell_snapshot: empty_shell_snapshot_receiver(), }; assert_eq!( test_powershell_shell.derive_exec_args("echo hello", /*use_login_shell*/ false), @@ -161,7 +158,6 @@ async fn test_current_shell_detects_zsh() { Shell { shell_type: ShellType::Zsh, shell_path: PathBuf::from(shell_path), - shell_snapshot: empty_shell_snapshot_receiver(), } ); } diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index 937e84899..2901b2ae5 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -11,6 +11,7 @@ use crate::exec_policy::ExecPolicyManager; use crate::guardian::GuardianRejection; use crate::guardian::GuardianRejectionCircuitBreaker; use crate::mcp::McpManager; +use crate::shell_snapshot::ShellSnapshot; use crate::tools::code_mode::CodeModeService; use crate::tools::network_approval::NetworkApprovalService; use crate::tools::sandboxing::ApprovalStore; @@ -36,7 +37,6 @@ use codex_thread_store::ThreadStore; use std::path::PathBuf; use tokio::runtime::Handle; use tokio::sync::Mutex; -use tokio::sync::watch; use tokio_util::sync::CancellationToken; pub(crate) struct SessionServices { @@ -52,7 +52,7 @@ pub(crate) struct SessionServices { pub(crate) hooks: ArcSwap, pub(crate) rollout_thread_trace: ThreadTraceContext, pub(crate) user_shell: Arc, - pub(crate) shell_snapshot_tx: watch::Sender>>, + pub(crate) shell_snapshot: ArcSwapOption, pub(crate) show_raw_agent_reasoning: bool, pub(crate) exec_policy: Arc, pub(crate) auth_manager: Arc, diff --git a/codex-rs/core/src/tasks/user_shell.rs b/codex-rs/core/src/tasks/user_shell.rs index 6affc3d03..a039dfd67 100644 --- a/codex-rs/core/src/tasks/user_shell.rs +++ b/codex-rs/core/src/tasks/user_shell.rs @@ -129,6 +129,11 @@ pub(crate) async fn execute_user_shell_command( // We do not source rc files or otherwise reformat the script. let use_login_shell = true; let session_shell = session.user_shell(); + let shell_snapshot = session.services.shell_snapshot.load_full(); + #[allow(deprecated)] + let shell_snapshot_location = shell_snapshot + .as_ref() + .and_then(|snapshot| snapshot.location(&turn_context.cwd)); let display_command = session_shell.derive_exec_args(&command, use_login_shell); let mut exec_env_map = create_env( &turn_context.shell_environment_policy, @@ -140,8 +145,7 @@ pub(crate) async fn execute_user_shell_command( let exec_command = prepare_user_shell_exec_command( &display_command, session_shell.as_ref(), - #[allow(deprecated)] - &turn_context.cwd, + shell_snapshot_location.as_ref(), &turn_context.shell_environment_policy.r#set, &mut exec_env_map, ); @@ -337,7 +341,7 @@ pub(crate) async fn execute_user_shell_command( fn prepare_user_shell_exec_command( display_command: &[String], session_shell: &Shell, - cwd: &AbsolutePathBuf, + shell_snapshot: Option<&AbsolutePathBuf>, shell_environment_set: &HashMap, exec_env_map: &mut HashMap, ) -> Vec { @@ -346,7 +350,7 @@ fn prepare_user_shell_exec_command( prepare_user_shell_exec_command_with_path_prepend( display_command, session_shell, - cwd, + shell_snapshot, shell_environment_set, exec_env_map, apply_package_path_prepend, @@ -358,7 +362,7 @@ fn prepare_user_shell_exec_command( maybe_wrap_shell_lc_with_snapshot( display_command, session_shell, - cwd, + shell_snapshot, shell_environment_set, exec_env_map, // On non-Unix targets, arg0 has already prepended the package path @@ -378,7 +382,7 @@ fn prepare_user_shell_exec_command( fn prepare_user_shell_exec_command_with_path_prepend( display_command: &[String], session_shell: &Shell, - cwd: &AbsolutePathBuf, + shell_snapshot: Option<&AbsolutePathBuf>, shell_environment_set: &HashMap, exec_env_map: &mut HashMap, prepend_runtime_path: impl FnOnce(&mut HashMap, &mut RuntimePathPrepends), @@ -389,7 +393,7 @@ fn prepare_user_shell_exec_command_with_path_prepend( maybe_wrap_shell_lc_with_snapshot( display_command, session_shell, - cwd, + shell_snapshot, &explicit_env_overrides, exec_env_map, &runtime_path_prepends, diff --git a/codex-rs/core/src/tasks/user_shell_tests.rs b/codex-rs/core/src/tasks/user_shell_tests.rs index 442c5cc61..b8e50ab73 100644 --- a/codex-rs/core/src/tasks/user_shell_tests.rs +++ b/codex-rs/core/src/tasks/user_shell_tests.rs @@ -1,28 +1,23 @@ use super::*; use crate::shell::Shell; use crate::shell::ShellType; -use crate::shell_snapshot::ShellSnapshot; use core_test_support::PathExt; use pretty_assertions::assert_eq; use std::path::PathBuf; use std::process::Command; -use tokio::sync::watch; fn shell_with_snapshot( shell_type: ShellType, shell_path: &str, snapshot_path: AbsolutePathBuf, - snapshot_cwd: AbsolutePathBuf, -) -> Shell { - let (_tx, shell_snapshot) = watch::channel(Some(Arc::new(ShellSnapshot { - path: snapshot_path, - cwd: snapshot_cwd, - }))); - Shell { - shell_type, - shell_path: PathBuf::from(shell_path), - shell_snapshot, - } +) -> (Shell, AbsolutePathBuf) { + ( + Shell { + shell_type, + shell_path: PathBuf::from(shell_path), + }, + snapshot_path, + ) } #[test] @@ -34,12 +29,8 @@ fn user_shell_snapshot_preserves_package_path_prepend() { "# Snapshot file\nexport PATH='/snapshot/bin'\n", ) .expect("write snapshot"); - let session_shell = shell_with_snapshot( - ShellType::Bash, - "/bin/bash", - snapshot_path.abs(), - dir.path().abs(), - ); + let (session_shell, shell_snapshot) = + shell_with_snapshot(ShellType::Bash, "/bin/bash", snapshot_path.abs()); let command = vec![ "/bin/bash".to_string(), "-lc".to_string(), @@ -50,7 +41,7 @@ fn user_shell_snapshot_preserves_package_path_prepend() { let rewritten = prepare_user_shell_exec_command_with_path_prepend( &command, &session_shell, - &dir.path().abs(), + Some(&shell_snapshot), &HashMap::new(), &mut env, |env, runtime_path_prepends| { diff --git a/codex-rs/core/src/tools/handlers/shell_tests.rs b/codex-rs/core/src/tools/handlers/shell_tests.rs index 792eb2bb1..18dbc2b75 100644 --- a/codex-rs/core/src/tools/handlers/shell_tests.rs +++ b/codex-rs/core/src/tools/handlers/shell_tests.rs @@ -2,8 +2,6 @@ use std::path::PathBuf; use std::sync::Arc; use codex_protocol::models::ShellCommandToolCallParams; -use core_test_support::PathBufExt; -use core_test_support::test_path_buf; use pretty_assertions::assert_eq; use crate::exec_env::create_env; @@ -11,7 +9,6 @@ use crate::sandboxing::SandboxPermissions; use crate::session::tests::make_session_and_context; use crate::shell::Shell; use crate::shell::ShellType; -use crate::shell_snapshot::ShellSnapshot; use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolCallSource; use crate::tools::context::ToolInvocation; @@ -25,7 +22,6 @@ use codex_shell_command::powershell::try_find_powershell_executable_blocking; use codex_shell_command::powershell::try_find_pwsh_executable_blocking; use serde_json::json; use tokio::sync::Mutex; -use tokio::sync::watch; /// The logic for is_known_safe_command() has heuristics for known shells, /// so we must ensure the commands generated by [ShellCommandHandler] can be @@ -35,14 +31,12 @@ fn commands_generated_by_shell_command_handler_can_be_matched_by_is_known_safe_c let bash_shell = Shell { shell_type: ShellType::Bash, shell_path: PathBuf::from("/bin/bash"), - shell_snapshot: crate::shell::empty_shell_snapshot_receiver(), }; assert_safe(&bash_shell, "ls -la"); let zsh_shell = Shell { shell_type: ShellType::Zsh, shell_path: PathBuf::from("/bin/zsh"), - shell_snapshot: crate::shell::empty_shell_snapshot_receiver(), }; assert_safe(&zsh_shell, "ls -la"); @@ -50,7 +44,6 @@ fn commands_generated_by_shell_command_handler_can_be_matched_by_is_known_safe_c let powershell = Shell { shell_type: ShellType::PowerShell, shell_path: path.to_path_buf(), - shell_snapshot: crate::shell::empty_shell_snapshot_receiver(), }; assert_safe(&powershell, "ls -Name"); } @@ -59,7 +52,6 @@ fn commands_generated_by_shell_command_handler_can_be_matched_by_is_known_safe_c let pwsh = Shell { shell_type: ShellType::PowerShell, shell_path: path.to_path_buf(), - shell_snapshot: crate::shell::empty_shell_snapshot_receiver(), }; assert_safe(&pwsh, "ls -Name"); } @@ -128,14 +120,9 @@ async fn shell_command_handler_to_exec_params_uses_session_shell_and_turn_contex #[test] fn shell_command_handler_respects_explicit_login_flag() { - let (_tx, shell_snapshot) = watch::channel(Some(Arc::new(ShellSnapshot { - path: test_path_buf("/tmp/snapshot.sh").abs(), - cwd: test_path_buf("/tmp").abs(), - }))); let shell = Shell { shell_type: ShellType::Bash, shell_path: PathBuf::from("/bin/bash"), - shell_snapshot, }; let login_command = ShellCommandHandler::base_command( diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index ca6b2a4b3..1c6252a1d 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -114,11 +114,10 @@ pub(crate) fn get_command( match shell_mode { UnifiedExecShellMode::Direct => { - let model_shell = args.shell.as_ref().map(|shell_str| { - let mut shell = get_shell_by_model_provided_path(&PathBuf::from(shell_str)); - shell.shell_snapshot = crate::shell::empty_shell_snapshot_receiver(); - shell - }); + 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), diff --git a/codex-rs/core/src/tools/runtimes/mod.rs b/codex-rs/core/src/tools/runtimes/mod.rs index 7940f397e..27f4ee594 100644 --- a/codex-rs/core/src/tools/runtimes/mod.rs +++ b/codex-rs/core/src/tools/runtimes/mod.rs @@ -5,7 +5,6 @@ Concrete ToolRuntime implementations for specific tools. Each runtime stays small and focused and reuses the orchestrator for approvals + sandbox + retry. */ use crate::exec_env::CODEX_THREAD_ID_ENV_VAR; -use crate::path_utils; use crate::sandboxing::SandboxPermissions; use crate::shell::Shell; use crate::shell::ShellType; @@ -251,7 +250,7 @@ pub(crate) fn disable_powershell_profile_for_elevated_windows_sandbox( pub(crate) fn maybe_wrap_shell_lc_with_snapshot( command: &[String], session_shell: &Shell, - cwd: &AbsolutePathBuf, + shell_snapshot: Option<&AbsolutePathBuf>, explicit_env_overrides: &HashMap, env: &HashMap, runtime_path_prepends: &RuntimePathPrepends, @@ -260,15 +259,11 @@ pub(crate) fn maybe_wrap_shell_lc_with_snapshot( return command.to_vec(); } - let Some(snapshot) = session_shell.shell_snapshot() else { + let Some(snapshot) = shell_snapshot else { return command.to_vec(); }; - if !snapshot.path.exists() { - return command.to_vec(); - } - - if !path_utils::paths_match_after_normalization(snapshot.cwd.as_path(), cwd) { + if !snapshot.exists() { return command.to_vec(); } @@ -281,7 +276,7 @@ pub(crate) fn maybe_wrap_shell_lc_with_snapshot( return command.to_vec(); } - let snapshot_path = snapshot.path.to_string_lossy(); + let snapshot_path = snapshot.to_string_lossy(); let shell_path = session_shell.shell_path.to_string_lossy(); let original_shell = shell_single_quote(&command[0]); let original_script = shell_single_quote(&command[2]); diff --git a/codex-rs/core/src/tools/runtimes/mod_tests.rs b/codex-rs/core/src/tools/runtimes/mod_tests.rs index 4a88d014d..7d466fb53 100644 --- a/codex-rs/core/src/tools/runtimes/mod_tests.rs +++ b/codex-rs/core/src/tools/runtimes/mod_tests.rs @@ -3,7 +3,6 @@ use crate::exec::ExecCapturePolicy; use crate::exec::ExecExpiration; use crate::sandboxing::ExecOptions; use crate::shell::ShellType; -use crate::shell_snapshot::ShellSnapshot; use crate::tools::sandboxing::SandboxAttempt; use crate::tools::sandboxing::managed_network_for_sandbox_permissions; #[cfg(target_os = "macos")] @@ -33,7 +32,6 @@ use std::path::PathBuf; use std::process::Command; use std::sync::Arc; use tempfile::tempdir; -use tokio::sync::watch; struct StaticReloader; @@ -55,17 +53,14 @@ fn shell_with_snapshot( shell_type: ShellType, shell_path: &str, snapshot_path: AbsolutePathBuf, - snapshot_cwd: AbsolutePathBuf, -) -> Shell { - let (_tx, shell_snapshot) = watch::channel(Some(Arc::new(ShellSnapshot { - path: snapshot_path, - cwd: snapshot_cwd, - }))); - Shell { - shell_type, - shell_path: PathBuf::from(shell_path), - shell_snapshot, - } +) -> (Shell, AbsolutePathBuf) { + ( + Shell { + shell_type, + shell_path: PathBuf::from(shell_path), + }, + snapshot_path, + ) } async fn test_network_proxy() -> anyhow::Result { @@ -330,12 +325,8 @@ fn maybe_wrap_shell_lc_with_snapshot_bootstraps_in_user_shell() { let dir = tempdir().expect("create temp dir"); let snapshot_path = dir.path().join("snapshot.sh"); std::fs::write(&snapshot_path, "# Snapshot file\n").expect("write snapshot"); - let session_shell = shell_with_snapshot( - ShellType::Zsh, - "/bin/zsh", - snapshot_path.abs(), - dir.path().abs(), - ); + let (session_shell, shell_snapshot) = + shell_with_snapshot(ShellType::Zsh, "/bin/zsh", snapshot_path.abs()); let command = vec![ "/bin/bash".to_string(), "-lc".to_string(), @@ -345,7 +336,7 @@ fn maybe_wrap_shell_lc_with_snapshot_bootstraps_in_user_shell() { let rewritten = maybe_wrap_shell_lc_with_snapshot( &command, &session_shell, - &dir.path().abs(), + Some(&shell_snapshot), &HashMap::new(), &HashMap::new(), &RuntimePathPrepends::default(), @@ -362,12 +353,8 @@ fn maybe_wrap_shell_lc_with_snapshot_escapes_single_quotes() { let dir = tempdir().expect("create temp dir"); let snapshot_path = dir.path().join("snapshot.sh"); std::fs::write(&snapshot_path, "# Snapshot file\n").expect("write snapshot"); - let session_shell = shell_with_snapshot( - ShellType::Zsh, - "/bin/zsh", - snapshot_path.abs(), - dir.path().abs(), - ); + let (session_shell, shell_snapshot) = + shell_with_snapshot(ShellType::Zsh, "/bin/zsh", snapshot_path.abs()); let command = vec![ "/bin/bash".to_string(), "-lc".to_string(), @@ -377,7 +364,7 @@ fn maybe_wrap_shell_lc_with_snapshot_escapes_single_quotes() { let rewritten = maybe_wrap_shell_lc_with_snapshot( &command, &session_shell, - &dir.path().abs(), + Some(&shell_snapshot), &HashMap::new(), &HashMap::new(), &RuntimePathPrepends::default(), @@ -391,12 +378,8 @@ fn maybe_wrap_shell_lc_with_snapshot_uses_bash_bootstrap_shell() { let dir = tempdir().expect("create temp dir"); let snapshot_path = dir.path().join("snapshot.sh"); std::fs::write(&snapshot_path, "# Snapshot file\n").expect("write snapshot"); - let session_shell = shell_with_snapshot( - ShellType::Bash, - "/bin/bash", - snapshot_path.abs(), - dir.path().abs(), - ); + let (session_shell, shell_snapshot) = + shell_with_snapshot(ShellType::Bash, "/bin/bash", snapshot_path.abs()); let command = vec![ "/bin/zsh".to_string(), "-lc".to_string(), @@ -406,7 +389,7 @@ fn maybe_wrap_shell_lc_with_snapshot_uses_bash_bootstrap_shell() { let rewritten = maybe_wrap_shell_lc_with_snapshot( &command, &session_shell, - &dir.path().abs(), + Some(&shell_snapshot), &HashMap::new(), &HashMap::new(), &RuntimePathPrepends::default(), @@ -423,12 +406,8 @@ fn maybe_wrap_shell_lc_with_snapshot_uses_sh_bootstrap_shell() { let dir = tempdir().expect("create temp dir"); let snapshot_path = dir.path().join("snapshot.sh"); std::fs::write(&snapshot_path, "# Snapshot file\n").expect("write snapshot"); - let session_shell = shell_with_snapshot( - ShellType::Sh, - "/bin/sh", - snapshot_path.abs(), - dir.path().abs(), - ); + let (session_shell, shell_snapshot) = + shell_with_snapshot(ShellType::Sh, "/bin/sh", snapshot_path.abs()); let command = vec![ "/bin/bash".to_string(), "-lc".to_string(), @@ -438,7 +417,7 @@ fn maybe_wrap_shell_lc_with_snapshot_uses_sh_bootstrap_shell() { let rewritten = maybe_wrap_shell_lc_with_snapshot( &command, &session_shell, - &dir.path().abs(), + Some(&shell_snapshot), &HashMap::new(), &HashMap::new(), &RuntimePathPrepends::default(), @@ -455,12 +434,8 @@ fn maybe_wrap_shell_lc_with_snapshot_preserves_trailing_args() { let dir = tempdir().expect("create temp dir"); let snapshot_path = dir.path().join("snapshot.sh"); std::fs::write(&snapshot_path, "# Snapshot file\n").expect("write snapshot"); - let session_shell = shell_with_snapshot( - ShellType::Zsh, - "/bin/zsh", - snapshot_path.abs(), - dir.path().abs(), - ); + let (session_shell, shell_snapshot) = + shell_with_snapshot(ShellType::Zsh, "/bin/zsh", snapshot_path.abs()); let command = vec![ "/bin/bash".to_string(), "-lc".to_string(), @@ -472,7 +447,7 @@ fn maybe_wrap_shell_lc_with_snapshot_preserves_trailing_args() { let rewritten = maybe_wrap_shell_lc_with_snapshot( &command, &session_shell, - &dir.path().abs(), + Some(&shell_snapshot), &HashMap::new(), &HashMap::new(), &RuntimePathPrepends::default(), @@ -484,72 +459,6 @@ fn maybe_wrap_shell_lc_with_snapshot_preserves_trailing_args() { ); } -#[test] -fn maybe_wrap_shell_lc_with_snapshot_skips_when_cwd_mismatch() { - let dir = tempdir().expect("create temp dir"); - let snapshot_path = dir.path().join("snapshot.sh"); - std::fs::write(&snapshot_path, "# Snapshot file\n").expect("write snapshot"); - let snapshot_cwd = dir.path().join("worktree-a"); - let command_cwd = dir.path().join("worktree-b"); - std::fs::create_dir_all(&snapshot_cwd).expect("create snapshot cwd"); - std::fs::create_dir_all(&command_cwd).expect("create command cwd"); - let session_shell = shell_with_snapshot( - ShellType::Zsh, - "/bin/zsh", - snapshot_path.abs(), - snapshot_cwd.abs(), - ); - let command = vec![ - "/bin/bash".to_string(), - "-lc".to_string(), - "echo hello".to_string(), - ]; - - let rewritten = maybe_wrap_shell_lc_with_snapshot( - &command, - &session_shell, - &command_cwd.abs(), - &HashMap::new(), - &HashMap::new(), - &RuntimePathPrepends::default(), - ); - - assert_eq!(rewritten, command); -} - -#[test] -fn maybe_wrap_shell_lc_with_snapshot_accepts_dot_alias_cwd() { - let dir = tempdir().expect("create temp dir"); - let snapshot_path = dir.path().join("snapshot.sh"); - std::fs::write(&snapshot_path, "# Snapshot file\n").expect("write snapshot"); - let session_shell = shell_with_snapshot( - ShellType::Zsh, - "/bin/zsh", - snapshot_path.abs(), - dir.path().abs(), - ); - let command = vec![ - "/bin/bash".to_string(), - "-lc".to_string(), - "echo hello".to_string(), - ]; - let command_cwd = dir.path().join("."); - - let rewritten = maybe_wrap_shell_lc_with_snapshot( - &command, - &session_shell, - &command_cwd.abs(), - &HashMap::new(), - &HashMap::new(), - &RuntimePathPrepends::default(), - ); - - assert_eq!(rewritten[0], "/bin/zsh"); - assert_eq!(rewritten[1], "-c"); - assert!(rewritten[2].contains("if . '")); - assert!(rewritten[2].contains("exec '/bin/bash' -c 'echo hello'")); -} - #[test] fn maybe_wrap_shell_lc_with_snapshot_restores_explicit_override_precedence() { let dir = tempdir().expect("create temp dir"); @@ -559,12 +468,8 @@ fn maybe_wrap_shell_lc_with_snapshot_restores_explicit_override_precedence() { "# Snapshot file\nexport TEST_ENV_SNAPSHOT=global\nexport SNAPSHOT_ONLY=from_snapshot\n", ) .expect("write snapshot"); - let session_shell = shell_with_snapshot( - ShellType::Bash, - "/bin/bash", - snapshot_path.abs(), - dir.path().abs(), - ); + let (session_shell, shell_snapshot) = + shell_with_snapshot(ShellType::Bash, "/bin/bash", snapshot_path.abs()); let command = vec![ "/bin/bash".to_string(), "-lc".to_string(), @@ -575,7 +480,7 @@ fn maybe_wrap_shell_lc_with_snapshot_restores_explicit_override_precedence() { let rewritten = maybe_wrap_shell_lc_with_snapshot( &command, &session_shell, - &dir.path().abs(), + Some(&shell_snapshot), &explicit_env_overrides, &HashMap::from([("TEST_ENV_SNAPSHOT".to_string(), "worktree".to_string())]), &RuntimePathPrepends::default(), @@ -602,12 +507,8 @@ fn maybe_wrap_shell_lc_with_snapshot_restores_codex_thread_id_from_env() { "# Snapshot file\nexport CODEX_THREAD_ID='parent-thread'\n", ) .expect("write snapshot"); - let session_shell = shell_with_snapshot( - ShellType::Bash, - "/bin/bash", - snapshot_path.abs(), - dir.path().abs(), - ); + let (session_shell, shell_snapshot) = + shell_with_snapshot(ShellType::Bash, "/bin/bash", snapshot_path.abs()); let command = vec![ "/bin/bash".to_string(), "-lc".to_string(), @@ -616,7 +517,7 @@ fn maybe_wrap_shell_lc_with_snapshot_restores_codex_thread_id_from_env() { let rewritten = maybe_wrap_shell_lc_with_snapshot( &command, &session_shell, - &dir.path().abs(), + Some(&shell_snapshot), &HashMap::new(), &HashMap::from([("CODEX_THREAD_ID".to_string(), "nested-thread".to_string())]), &RuntimePathPrepends::default(), @@ -644,12 +545,8 @@ fn maybe_wrap_shell_lc_with_snapshot_restores_proxy_env_from_process_env() { export GIT_SSH_COMMAND='ssh -o ProxyCommand=stale'\n", ) .expect("write snapshot"); - let session_shell = shell_with_snapshot( - ShellType::Bash, - "/bin/bash", - snapshot_path.abs(), - dir.path().abs(), - ); + let (session_shell, shell_snapshot) = + shell_with_snapshot(ShellType::Bash, "/bin/bash", snapshot_path.abs()); let command = vec![ "/bin/bash".to_string(), "-lc".to_string(), @@ -659,7 +556,7 @@ fn maybe_wrap_shell_lc_with_snapshot_restores_proxy_env_from_process_env() { let rewritten = maybe_wrap_shell_lc_with_snapshot( &command, &session_shell, - &dir.path().abs(), + Some(&shell_snapshot), &HashMap::new(), &HashMap::new(), &RuntimePathPrepends::default(), @@ -703,12 +600,8 @@ fn maybe_wrap_shell_lc_with_snapshot_refreshes_codex_proxy_git_ssh_command() { ), ) .expect("write snapshot"); - let session_shell = shell_with_snapshot( - ShellType::Bash, - "/bin/bash", - snapshot_path.abs(), - dir.path().abs(), - ); + let (session_shell, shell_snapshot) = + shell_with_snapshot(ShellType::Bash, "/bin/bash", snapshot_path.abs()); let command = vec![ "/bin/bash".to_string(), "-lc".to_string(), @@ -717,7 +610,7 @@ fn maybe_wrap_shell_lc_with_snapshot_refreshes_codex_proxy_git_ssh_command() { let rewritten = maybe_wrap_shell_lc_with_snapshot( &command, &session_shell, - &dir.path().abs(), + Some(&shell_snapshot), &HashMap::new(), &HashMap::new(), &RuntimePathPrepends::default(), @@ -749,12 +642,8 @@ fn maybe_wrap_shell_lc_with_snapshot_restores_custom_git_ssh_command() { ), ) .expect("write snapshot"); - let session_shell = shell_with_snapshot( - ShellType::Bash, - "/bin/bash", - snapshot_path.abs(), - dir.path().abs(), - ); + let (session_shell, shell_snapshot) = + shell_with_snapshot(ShellType::Bash, "/bin/bash", snapshot_path.abs()); let command = vec![ "/bin/bash".to_string(), "-lc".to_string(), @@ -763,7 +652,7 @@ fn maybe_wrap_shell_lc_with_snapshot_restores_custom_git_ssh_command() { let rewritten = maybe_wrap_shell_lc_with_snapshot( &command, &session_shell, - &dir.path().abs(), + Some(&shell_snapshot), &HashMap::new(), &HashMap::new(), &RuntimePathPrepends::default(), @@ -794,12 +683,8 @@ fn maybe_wrap_shell_lc_with_snapshot_clears_stale_codex_git_ssh_command_without_ ), ) .expect("write snapshot"); - let session_shell = shell_with_snapshot( - ShellType::Bash, - "/bin/bash", - snapshot_path.abs(), - dir.path().abs(), - ); + let (session_shell, shell_snapshot) = + shell_with_snapshot(ShellType::Bash, "/bin/bash", snapshot_path.abs()); let command = vec![ "/bin/bash".to_string(), "-lc".to_string(), @@ -810,7 +695,7 @@ fn maybe_wrap_shell_lc_with_snapshot_clears_stale_codex_git_ssh_command_without_ let rewritten = maybe_wrap_shell_lc_with_snapshot( &command, &session_shell, - &dir.path().abs(), + Some(&shell_snapshot), &HashMap::new(), &HashMap::new(), &RuntimePathPrepends::default(), @@ -834,12 +719,8 @@ fn maybe_wrap_shell_lc_with_snapshot_keeps_user_proxy_env_when_proxy_inactive() "# Snapshot file\nexport HTTP_PROXY='http://user.proxy:8080'\n", ) .expect("write snapshot"); - let session_shell = shell_with_snapshot( - ShellType::Bash, - "/bin/bash", - snapshot_path.abs(), - dir.path().abs(), - ); + let (session_shell, shell_snapshot) = + shell_with_snapshot(ShellType::Bash, "/bin/bash", snapshot_path.abs()); let command = vec![ "/bin/bash".to_string(), "-lc".to_string(), @@ -848,7 +729,7 @@ fn maybe_wrap_shell_lc_with_snapshot_keeps_user_proxy_env_when_proxy_inactive() let rewritten = maybe_wrap_shell_lc_with_snapshot( &command, &session_shell, - &dir.path().abs(), + Some(&shell_snapshot), &HashMap::new(), &HashMap::new(), &RuntimePathPrepends::default(), @@ -881,12 +762,8 @@ fn maybe_wrap_shell_lc_with_snapshot_restores_live_env_when_snapshot_proxy_activ ), ) .expect("write snapshot"); - let session_shell = shell_with_snapshot( - ShellType::Bash, - "/bin/bash", - snapshot_path.abs(), - dir.path().abs(), - ); + let (session_shell, shell_snapshot) = + shell_with_snapshot(ShellType::Bash, "/bin/bash", snapshot_path.abs()); let command = vec![ "/bin/bash".to_string(), "-lc".to_string(), @@ -899,7 +776,7 @@ fn maybe_wrap_shell_lc_with_snapshot_restores_live_env_when_snapshot_proxy_activ let rewritten = maybe_wrap_shell_lc_with_snapshot( &command, &session_shell, - &dir.path().abs(), + Some(&shell_snapshot), &HashMap::new(), &HashMap::from([( "HTTP_PROXY".to_string(), @@ -931,12 +808,8 @@ fn maybe_wrap_shell_lc_with_snapshot_keeps_snapshot_path_without_override() { "# Snapshot file\nexport PATH='/snapshot/bin'\n", ) .expect("write snapshot"); - let session_shell = shell_with_snapshot( - ShellType::Bash, - "/bin/bash", - snapshot_path.abs(), - dir.path().abs(), - ); + let (session_shell, shell_snapshot) = + shell_with_snapshot(ShellType::Bash, "/bin/bash", snapshot_path.abs()); let command = vec![ "/bin/bash".to_string(), "-lc".to_string(), @@ -945,7 +818,7 @@ fn maybe_wrap_shell_lc_with_snapshot_keeps_snapshot_path_without_override() { let rewritten = maybe_wrap_shell_lc_with_snapshot( &command, &session_shell, - &dir.path().abs(), + Some(&shell_snapshot), &HashMap::new(), &HashMap::new(), &RuntimePathPrepends::default(), @@ -968,12 +841,8 @@ fn maybe_wrap_shell_lc_with_snapshot_applies_explicit_path_override() { "# Snapshot file\nexport PATH='/snapshot/bin'\n", ) .expect("write snapshot"); - let session_shell = shell_with_snapshot( - ShellType::Bash, - "/bin/bash", - snapshot_path.abs(), - dir.path().abs(), - ); + let (session_shell, shell_snapshot) = + shell_with_snapshot(ShellType::Bash, "/bin/bash", snapshot_path.abs()); let command = vec![ "/bin/bash".to_string(), "-lc".to_string(), @@ -983,7 +852,7 @@ fn maybe_wrap_shell_lc_with_snapshot_applies_explicit_path_override() { let rewritten = maybe_wrap_shell_lc_with_snapshot( &command, &session_shell, - &dir.path().abs(), + Some(&shell_snapshot), &explicit_env_overrides, &HashMap::from([("PATH".to_string(), "/worktree/bin".to_string())]), &RuntimePathPrepends::default(), @@ -1038,12 +907,8 @@ fn run_snapshot_path_probe_with_runtime_path_prepend( &snapshot_path, "# Snapshot file\nexport PATH='/snapshot/bin'\n", )?; - let session_shell = shell_with_snapshot( - ShellType::Bash, - "/bin/bash", - snapshot_path.abs(), - dir.path().abs(), - ); + let (session_shell, shell_snapshot) = + shell_with_snapshot(ShellType::Bash, "/bin/bash", snapshot_path.abs()); let command = vec![ "/bin/bash".to_string(), "-lc".to_string(), @@ -1056,7 +921,7 @@ fn run_snapshot_path_probe_with_runtime_path_prepend( let rewritten = maybe_wrap_shell_lc_with_snapshot( &command, &session_shell, - &dir.path().abs(), + Some(&shell_snapshot), &explicit_env_overrides, &env, &runtime_path_prepends, @@ -1086,12 +951,8 @@ fn maybe_wrap_shell_lc_with_snapshot_preserves_zsh_fork_path_prepend() { "# Snapshot file\nexport PATH='/snapshot/bin'\n", ) .expect("write snapshot"); - let session_shell = shell_with_snapshot( - ShellType::Bash, - "/bin/bash", - snapshot_path.abs(), - dir.path().abs(), - ); + let (session_shell, shell_snapshot) = + shell_with_snapshot(ShellType::Bash, "/bin/bash", snapshot_path.abs()); let command = vec![ "/bin/bash".to_string(), "-lc".to_string(), @@ -1111,7 +972,7 @@ fn maybe_wrap_shell_lc_with_snapshot_preserves_zsh_fork_path_prepend() { let rewritten = maybe_wrap_shell_lc_with_snapshot( &command, &session_shell, - &dir.path().abs(), + Some(&shell_snapshot), &explicit_env_overrides, &env, &runtime_path_prepends, @@ -1139,12 +1000,8 @@ fn maybe_wrap_shell_lc_with_snapshot_does_not_embed_override_values_in_argv() { "# Snapshot file\nexport OPENAI_API_KEY='snapshot-value'\n", ) .expect("write snapshot"); - let session_shell = shell_with_snapshot( - ShellType::Bash, - "/bin/bash", - snapshot_path.abs(), - dir.path().abs(), - ); + let (session_shell, shell_snapshot) = + shell_with_snapshot(ShellType::Bash, "/bin/bash", snapshot_path.abs()); let command = vec![ "/bin/bash".to_string(), "-lc".to_string(), @@ -1157,7 +1014,7 @@ fn maybe_wrap_shell_lc_with_snapshot_does_not_embed_override_values_in_argv() { let rewritten = maybe_wrap_shell_lc_with_snapshot( &command, &session_shell, - &dir.path().abs(), + Some(&shell_snapshot), &explicit_env_overrides, &HashMap::from([( "OPENAI_API_KEY".to_string(), @@ -1188,12 +1045,8 @@ fn maybe_wrap_shell_lc_with_snapshot_preserves_unset_override_variables() { "# Snapshot file\nexport CODEX_TEST_UNSET_OVERRIDE='snapshot-value'\n", ) .expect("write snapshot"); - let session_shell = shell_with_snapshot( - ShellType::Bash, - "/bin/bash", - snapshot_path.abs(), - dir.path().abs(), - ); + let (session_shell, shell_snapshot) = + shell_with_snapshot(ShellType::Bash, "/bin/bash", snapshot_path.abs()); let command = vec![ "/bin/bash".to_string(), "-lc".to_string(), @@ -1206,7 +1059,7 @@ fn maybe_wrap_shell_lc_with_snapshot_preserves_unset_override_variables() { let rewritten = maybe_wrap_shell_lc_with_snapshot( &command, &session_shell, - &dir.path().abs(), + Some(&shell_snapshot), &explicit_env_overrides, &HashMap::new(), &RuntimePathPrepends::default(), diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index 612ab8c0f..488a7de9a 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -240,6 +240,10 @@ impl ToolRuntime for ShellRuntime { ctx: &ToolCtx, ) -> Result { let session_shell = ctx.session.user_shell(); + let shell_snapshot = ctx.session.services.shell_snapshot.load_full(); + let shell_snapshot_location = shell_snapshot + .as_ref() + .and_then(|snapshot| snapshot.location(&req.cwd)); let (file_system_sandbox_policy, _) = attempt.permissions.to_runtime_permissions(); let sandbox_permissions = sandbox_permissions_preserving_denied_reads( req.sandbox_permissions, @@ -269,7 +273,7 @@ impl ToolRuntime for ShellRuntime { let command = maybe_wrap_shell_lc_with_snapshot( &req.command, session_shell.as_ref(), - &req.cwd, + shell_snapshot_location.as_ref(), &explicit_env_overrides, &env, &runtime_path_prepends, diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 9aff1ac79..2ec2704eb 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -264,6 +264,10 @@ impl<'a> ToolRuntime for UnifiedExecRunt ) -> Result { let base_command = &req.command; let session_shell = ctx.session.user_shell(); + let shell_snapshot = ctx.session.services.shell_snapshot.load_full(); + let shell_snapshot_location = shell_snapshot + .as_ref() + .and_then(|snapshot| snapshot.location(&req.cwd)); let (file_system_sandbox_policy, _) = attempt.permissions.to_runtime_permissions(); let launch_sandbox_permissions = sandbox_permissions_preserving_denied_reads( req.sandbox_permissions, @@ -305,7 +309,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt maybe_wrap_shell_lc_with_snapshot( base_command, session_shell.as_ref(), - &req.cwd, + shell_snapshot_location.as_ref(), &explicit_env_overrides, &env, &runtime_path_prepends,