Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 18 additions & 9 deletions codex-rs/core/src/shell_snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,13 @@ impl ShellSnapshot {
ShellType::PowerShell => "ps1",
_ => "sh",
};
let path = codex_home
.join(SNAPSHOT_DIR)
.join(format!("{session_id}.{extension}"));
let nonce = SystemTime::now()
.duration_since(SystemTime::UNIX_EPOCH)
.map(|duration| duration.as_nanos())
.unwrap_or(0);
let path = codex_home
.join(SNAPSHOT_DIR)
.join(format!("{session_id}.{nonce}.{extension}"));
let temp_path = codex_home
.join(SNAPSHOT_DIR)
.join(format!("{session_id}.tmp-{nonce}"));
Expand Down Expand Up @@ -510,12 +510,9 @@ pub async fn cleanup_stale_snapshots(codex_home: &Path, active_session_id: Threa

let file_name = entry.file_name();
let file_name = file_name.to_string_lossy();
let (session_id, _) = match file_name.rsplit_once('.') {
Some((stem, ext)) => (stem, ext),
None => {
remove_snapshot_file(&path).await;
continue;
}
let Some(session_id) = snapshot_session_id_from_file_name(&file_name) else {
remove_snapshot_file(&path).await;
continue;
};
if session_id == active_session_id {
continue;
Expand Down Expand Up @@ -556,6 +553,18 @@ async fn remove_snapshot_file(path: &Path) {
}
}

fn snapshot_session_id_from_file_name(file_name: &str) -> Option<&str> {
let (stem, extension) = file_name.rsplit_once('.')?;
match extension {
"sh" | "ps1" => Some(
stem.split_once('.')
.map_or(stem, |(session_id, _generation)| session_id),
),
_ if extension.starts_with("tmp-") => Some(stem),
_ => None,
}
}

#[cfg(test)]
#[path = "shell_snapshot_tests.rs"]
mod tests;
66 changes: 62 additions & 4 deletions codex-rs/core/src/shell_snapshot_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,28 @@ fn strip_snapshot_preamble_requires_marker() {
assert!(result.is_err());
}

#[test]
fn snapshot_file_name_parser_supports_legacy_and_suffixed_names() {
let session_id = "019cf82b-6a62-7700-bbbd-46909794ef89";

assert_eq!(
snapshot_session_id_from_file_name(&format!("{session_id}.sh")),
Some(session_id)
);
assert_eq!(
snapshot_session_id_from_file_name(&format!("{session_id}.123.sh")),
Some(session_id)
);
assert_eq!(
snapshot_session_id_from_file_name(&format!("{session_id}.tmp-123")),
Some(session_id)
);
assert_eq!(
snapshot_session_id_from_file_name("not-a-snapshot.txt"),
None
);
}

#[cfg(unix)]
#[test]
fn bash_snapshot_filters_invalid_exports() -> Result<()> {
Expand Down Expand Up @@ -186,6 +208,42 @@ async fn try_new_creates_and_deletes_snapshot_file() -> Result<()> {
Ok(())
}

#[cfg(unix)]
#[tokio::test]
async fn try_new_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(dir.path(), session_id, dir.path(), &shell)
.await
.expect("initial snapshot should be created");
let refreshed_snapshot = ShellSnapshot::try_new(dir.path(), session_id, dir.path(), &shell)
.await
.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);

drop(initial_snapshot);

assert_eq!(initial_path.exists(), false);
assert_eq!(refreshed_path.exists(), true);

drop(refreshed_snapshot);

assert_eq!(refreshed_path.exists(), false);

Ok(())
}

#[cfg(unix)]
#[tokio::test]
async fn snapshot_shell_does_not_inherit_stdin() -> Result<()> {
Expand Down Expand Up @@ -339,8 +397,8 @@ async fn cleanup_stale_snapshots_removes_orphans_and_keeps_live() -> Result<()>

let live_session = ThreadId::new();
let orphan_session = ThreadId::new();
let live_snapshot = snapshot_dir.join(format!("{live_session}.sh"));
let orphan_snapshot = snapshot_dir.join(format!("{orphan_session}.sh"));
let live_snapshot = snapshot_dir.join(format!("{live_session}.123.sh"));
let orphan_snapshot = snapshot_dir.join(format!("{orphan_session}.456.sh"));
let invalid_snapshot = snapshot_dir.join("not-a-snapshot.txt");

write_rollout_stub(codex_home, live_session).await?;
Expand All @@ -365,7 +423,7 @@ async fn cleanup_stale_snapshots_removes_stale_rollouts() -> Result<()> {
fs::create_dir_all(&snapshot_dir).await?;

let stale_session = ThreadId::new();
let stale_snapshot = snapshot_dir.join(format!("{stale_session}.sh"));
let stale_snapshot = snapshot_dir.join(format!("{stale_session}.123.sh"));
let rollout_path = write_rollout_stub(codex_home, stale_session).await?;
fs::write(&stale_snapshot, "stale").await?;

Expand All @@ -386,7 +444,7 @@ async fn cleanup_stale_snapshots_skips_active_session() -> Result<()> {
fs::create_dir_all(&snapshot_dir).await?;

let active_session = ThreadId::new();
let active_snapshot = snapshot_dir.join(format!("{active_session}.sh"));
let active_snapshot = snapshot_dir.join(format!("{active_session}.123.sh"));
let rollout_path = write_rollout_stub(codex_home, active_session).await?;
fs::write(&active_snapshot, "active").await?;

Expand Down
Loading