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,