agent/codex
1
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

[codex] simplify shell snapshot ownership (#27756)

Browse Source 
## 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<ShellSnapshot>` 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`
This commit is contained in:
pakrym-oai
2026-06-15 08:18:13 -07:00
committed by GitHub
Unverified
parent 6d9df687a5
commit 1aa3e432cc
18 changed files with 225 additions and 400 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/agent/control.rs
+7 -1
View File
@@ -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(
codex-rs/core/src/context/environment_context_tests.rs
-1
View File
@@ -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()
}
codex-rs/core/src/session/mod.rs
+10 -13
View File
@@ -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(
codex-rs/core/src/session/session.rs
+17 -23
View File
@@ -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),
codex-rs/core/src/session/tests.rs
+2 -4
View File
@@ -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(
codex-rs/core/src/shell.rs
+1 -30
View File
@@ -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<Option<Arc<ShellSnapshot>>>,
}
impl Shell {
@@ -56,32 +47,13 @@ impl Shell {
}
}
}
/// Return the shell snapshot if existing.
pub fn shell_snapshot(&self) -> Option<Arc<ShellSnapshot>> {
self.shell_snapshot.borrow().clone()
}
}
pub(crate) fn empty_shell_snapshot_receiver() -> watch::Receiver<Option<Arc<ShellSnapshot>>> {
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<DetectedShell> 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(),
})
}
}
codex-rs/core/src/shell_snapshot.rs
+40 -43
View File
@@ -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<Option<std::result::Result<ShellSnapshotFile, &'static str>>>,
}
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<StateDbHandle>,
) -> watch::Sender<Option<Arc<ShellSnapshot>>> {
let (shell_snapshot_tx, shell_snapshot_rx) = watch::channel(None);
shell.shell_snapshot = shell_snapshot_rx;
) -> Arc<Self> {
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<Option<Arc<ShellSnapshot>>>,
session_telemetry: SessionTelemetry,
state_db: Option<StateDbHandle>,
) {
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<AbsolutePathBuf> {
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<Option<Arc<ShellSnapshot>>>,
state_tx: watch::Sender<Option<std::result::Result<ShellSnapshotFile, &'static str>>>,
session_telemetry: SessionTelemetry,
state_db: Option<StateDbHandle>,
) {
@@ -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<StateDbHandle>,
) -> std::result::Result<Self, &'static str> {
) -> std::result::Result<ShellSnapshotFile, &'static str> {
// 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!(
codex-rs/core/src/shell_snapshot_tests.rs
+41 -11
View File
@@ -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(
codex-rs/core/src/shell_tests.rs
-4
View File
@@ -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(),
}
);
}
codex-rs/core/src/state/service.rs
+2 -2
View File
@@ -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<Hooks>,
pub(crate) rollout_thread_trace: ThreadTraceContext,
pub(crate) user_shell: Arc<crate::shell::Shell>,
pub(crate) shell_snapshot_tx: watch::Sender<Option<Arc<crate::shell_snapshot::ShellSnapshot>>>,
pub(crate) shell_snapshot: ArcSwapOption<ShellSnapshot>,
pub(crate) show_raw_agent_reasoning: bool,
pub(crate) exec_policy: Arc<ExecPolicyManager>,
pub(crate) auth_manager: Arc<AuthManager>,
codex-rs/core/src/tasks/user_shell.rs
+11 -7
View File
@@ -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<String, String>,
exec_env_map: &mut HashMap<String, String>,
) -> Vec<String> {
@@ -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<String, String>,
exec_env_map: &mut HashMap<String, String>,
prepend_runtime_path: impl FnOnce(&mut HashMap<String, String>, &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,
codex-rs/core/src/tasks/user_shell_tests.rs
+11 -20
View File
@@ -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| {
codex-rs/core/src/tools/handlers/shell_tests.rs
-13
View File
@@ -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(
codex-rs/core/src/tools/handlers/unified_exec.rs
+4 -5
View File
@@ -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),
codex-rs/core/src/tools/runtimes/mod.rs
+4 -9
View File
@@ -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<String, String>,
env: &HashMap<String, String>,
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]);
codex-rs/core/src/tools/runtimes/mod_tests.rs
+65 -212
View File
@@ -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<NetworkProxy> {
@@ -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(),
codex-rs/core/src/tools/runtimes/shell.rs
+5 -1
View File
@@ -240,6 +240,10 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
ctx: &ToolCtx,
) -> Result<ExecToolCallOutput, ToolError> {
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<ShellRequest, ExecToolCallOutput> 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,
codex-rs/core/src/tools/runtimes/unified_exec.rs
+5 -1
View File
@@ -264,6 +264,10 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
) -> Result<UnifiedExecProcess, ToolError> {
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<UnifiedExecRequest, UnifiedExecProcess> 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,