feat: add suffix to shell snapshot name (#14938)
https://github.com/openai/codex/issues/14906
This commit is contained in:
parent
f26ad3c92c
commit
d484bb57d9
2 changed files with 80 additions and 13 deletions
|
|
@ -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}"));
|
||||
|
|
@ -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;
|
||||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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<()> {
|
||||
|
|
@ -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<()> {
|
||||
|
|
@ -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?;
|
||||
|
|
@ -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?;
|
||||
|
||||
|
|
@ -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?;
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue