From 2deed3fb9c00c74dac3d177ea700d6fb7a94539d Mon Sep 17 00:00:00 2001 From: Anton Panasenko Date: Mon, 20 Jul 2026 08:24:32 +0000 Subject: [PATCH] Preserve zsh tied PATH exports in shell snapshots (#34293) ## Why zsh prints tied scalar/array exports such as `PATH` and `path` with `-T` metadata. The snapshot filter rejected that syntax, so commands available only through the configured `PATH` could not be resolved from a restored snapshot. ## What changed - Recognize writable tied exports and retain them in zsh shell snapshots. - Skip readonly exports so restoring a snapshot does not make variables such as `PATH` immutable. ## Testing Added macOS coverage for restoring a tied `PATH`, handling entries with spaces, deduplicating path entries, excluding readonly tied exports, and resolving a command available only through the restored snapshot. GitOrigin-RevId: 297cfae3368230c7f6fe5ef84f0a10b35a2c802c --- codex-rs/core/src/shell_snapshot.rs | 7 ++ codex-rs/core/src/shell_snapshot_tests.rs | 93 +++++++++++++++++++++ codex-rs/core/tests/suite/shell_snapshot.rs | 72 ++++++++++++++++ 3 files changed, 172 insertions(+) diff --git a/codex-rs/core/src/shell_snapshot.rs b/codex-rs/core/src/shell_snapshot.rs index 6a95093003..eed1198580 100644 --- a/codex-rs/core/src/shell_snapshot.rs +++ b/codex-rs/core/src/shell_snapshot.rs @@ -342,6 +342,13 @@ export_lines=$(export -p | awk ' line=$0 name=line sub(/^(export|declare -x|typeset -x) /, "", name) + if (name ~ /^-[A-Za-z]*r[A-Za-z]* /) { + next + } + if (name ~ /^-[A-Za-z]*T[A-Za-z]* /) { + sub(/^-[A-Za-z]*T[A-Za-z]* /, "", name) + sub(/ [A-Za-z_][A-Za-z0-9_]*=.*/, "", name) + } sub(/=.*/, "", name) if (name ~ /^(EXCLUDED_EXPORTS)$/) { next diff --git a/codex-rs/core/src/shell_snapshot_tests.rs b/codex-rs/core/src/shell_snapshot_tests.rs index dcba3e2459..c32a723d74 100644 --- a/codex-rs/core/src/shell_snapshot_tests.rs +++ b/codex-rs/core/src/shell_snapshot_tests.rs @@ -187,6 +187,99 @@ fn bash_snapshot_preserves_multiline_exports() -> Result<()> { Ok(()) } +#[cfg(target_os = "macos")] +#[test] +fn zsh_snapshot_restores_tied_path() -> Result<()> { + let dir = tempdir()?; + let path_with_spaces = dir.path().join("path with spaces").join("bin"); + let plain_path = dir.path().join("plain-path").join("bin"); + let expected_path = format!( + "{}:{}:/usr/bin:/bin", + path_with_spaces.display(), + plain_path.display() + ); + let zshrc = format!( + "export -UT PATH path=('{}' '{}' '{}' /usr/bin /bin)\n", + path_with_spaces.display(), + plain_path.display(), + plain_path.display() + ); + std::fs::write(dir.path().join(".zshrc"), zshrc)?; + + let snapshot = Command::new("/bin/zsh") + .arg("-f") + .arg("-c") + .arg(zsh_snapshot_script()) + .env_clear() + .env("PATH", "/usr/bin:/bin") + .env("ZDOTDIR", dir.path()) + .output()?; + assert!(snapshot.status.success()); + + let snapshot_path = dir.path().join("snapshot.sh"); + std::fs::write(&snapshot_path, &snapshot.stdout)?; + + let restored = Command::new("/bin/zsh") + .arg("-f") + .arg("-c") + .arg("set -e; . \"$1\"; print -r -- \"$PATH\"") + .arg("zsh") + .arg(&snapshot_path) + .env_clear() + .env("PATH", "/usr/bin:/bin") + .output()?; + assert!(restored.status.success()); + assert_eq!( + String::from_utf8(restored.stdout)?.trim_end(), + expected_path + ); + + let snapshot = String::from_utf8(snapshot.stdout)?; + assert!( + snapshot + .lines() + .any(|line| line.starts_with("export -UT PATH path=")), + "snapshot should capture the tied PATH export" + ); + + std::fs::write(dir.path().join(".zshrc"), "readonly PATH\n")?; + let readonly_snapshot = Command::new("/bin/zsh") + .arg("-f") + .arg("-c") + .arg(zsh_snapshot_script()) + .env_clear() + .env("PATH", "/usr/bin:/bin") + .env("ZDOTDIR", dir.path()) + .output()?; + assert!(readonly_snapshot.status.success()); + std::fs::write(&snapshot_path, &readonly_snapshot.stdout)?; + + let readonly_restored = Command::new("/bin/zsh") + .arg("-f") + .arg("-c") + .arg("set -e; . \"$1\"; export PATH='/codex-path':\"$PATH\"; print -r -- \"$PATH\"") + .arg("zsh") + .arg(&snapshot_path) + .env_clear() + .env("PATH", "/usr/bin:/bin") + .output()?; + assert!(readonly_restored.status.success()); + assert_eq!( + String::from_utf8(readonly_restored.stdout)?.trim_end(), + "/codex-path:/usr/bin:/bin" + ); + + let readonly_snapshot = String::from_utf8(readonly_snapshot.stdout)?; + assert!( + !readonly_snapshot + .lines() + .any(|line| line.starts_with("export -rT PATH path=")), + "snapshot should not capture the readonly tied PATH export" + ); + + Ok(()) +} + #[cfg(unix)] #[tokio::test] async fn try_create_creates_and_deletes_snapshot_file() -> Result<()> { diff --git a/codex-rs/core/tests/suite/shell_snapshot.rs b/codex-rs/core/tests/suite/shell_snapshot.rs index 04576f0818..f2f4859940 100644 --- a/codex-rs/core/tests/suite/shell_snapshot.rs +++ b/codex-rs/core/tests/suite/shell_snapshot.rs @@ -1,4 +1,6 @@ use anyhow::Result; +#[cfg(target_os = "macos")] +use codex_core::shell::get_shell_by_model_provided_path; use codex_features::Feature; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; @@ -22,6 +24,8 @@ use core_test_support::wait_for_event_match; use pretty_assertions::assert_eq; use serde_json::json; use std::collections::HashMap; +#[cfg(target_os = "macos")] +use std::os::unix::fs::PermissionsExt; use std::path::Path; use std::path::PathBuf; use tokio::fs; @@ -676,6 +680,74 @@ async fn shell_snapshot_deleted_after_shutdown_with_skills() -> Result<()> { Ok(()) } +#[cfg(target_os = "macos")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn macos_shell_command_resolves_command_from_tied_path_snapshot() -> Result<()> { + let builder = test_codex() + .with_user_shell(get_shell_by_model_provided_path(&PathBuf::from("/bin/zsh"))) + .with_config(|config| { + config + .features + .enable(Feature::ShellSnapshot) + .expect("test config should allow feature update"); + }); + let harness = TestCodexHarness::with_builder(builder).await?; + let command_dir = harness + .test() + .config + .cwd + .join("path with spaces") + .join("bin"); + fs::create_dir_all(&command_dir).await?; + let command_path = command_dir.join("snapshot-only-command"); + fs::write(&command_path, "#!/bin/sh\nprintf tied-path-command").await?; + let mut permissions = fs::metadata(&command_path).await?.permissions(); + permissions.set_mode(0o755); + fs::set_permissions(&command_path, permissions).await?; + + run_tool_turn_on_harness( + &harness, + "warm up the tied PATH shell snapshot", + "shell-snapshot-tied-path-warmup", + "shell_command", + json!({ + "command": "printf warmup", + "timeout_ms": 5_000, + }), + ) + .await?; + let snapshot_path = wait_for_snapshot(harness.test().home.path()).await?; + fs::write( + &snapshot_path, + format!( + "# Snapshot file\nexport -UT PATH path=('{}' /usr/bin /bin)\n", + command_dir.display() + ), + ) + .await?; + + let end = run_tool_turn_on_harness( + &harness, + "resolve a command from the tied PATH snapshot", + "shell-snapshot-tied-path", + "shell_command", + json!({ + "command": "snapshot-only-command", + "timeout_ms": 5_000, + }), + ) + .await?; + + assert_eq!( + end.exit_code, 0, + "tied-path command failed: stderr={:?}", + end.stderr + ); + assert_eq!(normalize_newlines(&end.stdout).trim(), "tied-path-command"); + + Ok(()) +} + #[cfg_attr(not(target_os = "macos"), ignore)] #[cfg_attr( target_os = "macos",