From f735d945e6cc5af29bfc46d27490900906908c2c Mon Sep 17 00:00:00 2001 From: Nightt <87569709+nightt5879@users.noreply.github.com> Date: Mon, 29 Jun 2026 10:39:36 +0800 Subject: [PATCH 1/5] fix(tui): defer startup maintenance cleanup Move non-essential interactive startup cleanup for workspace snapshots, tool-output spillover files, and old sessions onto a delayed background maintenance thread so the TUI can reach its first frame without waiting on directory scans or snapshot git pruning. The cleanup behavior remains best-effort and keeps the existing logging; the background pass now records its elapsed time at the startup tracing target for follow-up profiling. Fixes #3757. Verification: - cargo fmt - cargo test -p codewhale-tui --bin codewhale-tui --locked prune_older_than - cargo test -p codewhale-tui --bin codewhale-tui --locked startup - cargo build -p codewhale-tui --bin codewhale-tui --locked - cargo test -p codewhale-tui --bin codewhale-tui --locked (one unrelated timeout flake: core::engine::tests::edit_last_turn_preserves_current_mode; passed when rerun in isolation) --- crates/tui/src/main.rs | 97 ++++++++++++++++++++++++++++-------------- 1 file changed, 65 insertions(+), 32 deletions(-) diff --git a/crates/tui/src/main.rs b/crates/tui/src/main.rs index eb0e70f8fb..546a7df741 100644 --- a/crates/tui/src/main.rs +++ b/crates/tui/src/main.rs @@ -5,7 +5,7 @@ use std::io::{self, IsTerminal, Read, Write}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; -use std::time::Duration; +use std::time::{Duration, Instant}; use anyhow::{Context, Result, anyhow, bail}; use clap::{Args, CommandFactory, Parser, Subcommand, ValueEnum}; @@ -6470,37 +6470,7 @@ async fn run_interactive( logging::warn(format!("Failed to install system skills: {e}")); } - // Prune stale workspace snapshots from prior sessions (7-day default). - // Non-fatal: a flaky disk, missing `git`, or read-only home should - // never block the TUI from starting. - let snapshots = config.snapshots_config(); - if snapshots.enabled { - session_manager::prune_workspace_snapshots(&workspace, snapshots.max_age()); - } - - // Prune stale tool-output spillover files (#422). Non-fatal: home - // missing or directory unreadable just means nothing got pruned; - // we never block startup. Runs unconditionally because the - // spillover store is created lazily on first write — there's no - // user-facing setting to gate. - match crate::tools::truncate::prune_older_than(crate::tools::truncate::SPILLOVER_MAX_AGE) { - Ok(0) => {} - Ok(n) => tracing::debug!( - target: "spillover", - "boot prune removed {n} spillover file(s)" - ), - Err(err) => tracing::warn!( - target: "spillover", - ?err, - "spillover prune skipped on boot" - ), - } - - // v0.8.44: prune managed sessions on boot to prevent unbounded growth. - // Keeps at most MAX_SESSIONS (50) recent sessions; non-fatal on error. - if let Ok(manager) = session_manager::SessionManager::default_location() { - let _ = manager.cleanup_old_sessions(); - } + spawn_interactive_startup_maintenance(workspace.clone(), config.snapshots_config()); // The `deepseek` launcher forwards `--yolo` to this binary via the // DEEPSEEK_YOLO env var (config.yolo), not as a CLI flag. Honour either. @@ -6533,6 +6503,69 @@ async fn run_interactive( .await } +fn spawn_interactive_startup_maintenance( + workspace: PathBuf, + snapshots: crate::config::SnapshotsConfig, +) { + let spawn_result = std::thread::Builder::new() + .name("codewhale-startup-maintenance".to_string()) + .spawn(move || { + // Keep the first interactive frame ahead of optional disk cleanup. + std::thread::sleep(Duration::from_millis(500)); + run_interactive_startup_maintenance(&workspace, &snapshots); + }); + + if let Err(err) = spawn_result { + logging::warn(format!("Startup maintenance skipped: {err}")); + } +} + +fn run_interactive_startup_maintenance( + workspace: &Path, + snapshots: &crate::config::SnapshotsConfig, +) { + let started = Instant::now(); + + // Prune stale workspace snapshots from prior sessions (7-day default). + // Non-fatal: a flaky disk, missing `git`, or read-only home should + // never block the TUI from starting. + if snapshots.enabled { + session_manager::prune_workspace_snapshots(workspace, snapshots.max_age()); + } + + // Prune stale tool-output spillover files (#422). Non-fatal: home + // missing or directory unreadable just means nothing got pruned. + match crate::tools::truncate::prune_older_than(crate::tools::truncate::SPILLOVER_MAX_AGE) { + Ok(0) => {} + Ok(n) => tracing::debug!( + target: "spillover", + "boot prune removed {n} spillover file(s)" + ), + Err(err) => tracing::warn!( + target: "spillover", + ?err, + "spillover prune skipped on boot" + ), + } + + // v0.8.44: prune managed sessions to prevent unbounded growth. + // Keeps at most MAX_SESSIONS (50) recent sessions; non-fatal on error. + match session_manager::SessionManager::default_location() { + Ok(manager) => { + if let Err(err) = manager.cleanup_old_sessions() { + tracing::warn!(target: "session", ?err, "session cleanup skipped on boot"); + } + } + Err(err) => tracing::warn!(target: "session", ?err, "session cleanup skipped on boot"), + } + + tracing::debug!( + target: "startup", + elapsed_ms = started.elapsed().as_millis(), + "startup maintenance finished" + ); +} + #[derive(Debug)] struct CliAutoRoute { provider: crate::config::ApiProvider, From faa70b1457d9ba87f19e708214527a75b08a8234 Mon Sep 17 00:00:00 2001 From: Nightt <87569709+nightt5879@users.noreply.github.com> Date: Mon, 29 Jun 2026 11:47:38 +0800 Subject: [PATCH 2/5] fix(tui): harden deferred startup cleanup Address Codex review feedback on PR #3761 by keeping deferred startup maintenance from racing first-turn state. Serialize side-git snapshot creation and pruning with a repo-local advisory lock, serialize spillover writes with boot pruning while refreshing reused SHA-addressed files, and preserve a resumed session during background session cleanup. Verification: - cargo fmt --all -- --check - cargo clippy --workspace --all-features --locked -- -D warnings -A clippy::uninlined_format_args -A clippy::too_many_arguments -A clippy::unnecessary_map_or -A clippy::collapsible_if -A clippy::assertions_on_constants - cargo test --workspace --locked - cargo build --release -p codewhale-cli -p codewhale-tui --locked --- crates/tui/src/main.rs | 16 ++++- crates/tui/src/session_manager.rs | 52 +++++++++++++- crates/tui/src/snapshot/repo.rs | 27 +++++++- crates/tui/src/tools/truncate.rs | 108 ++++++++++++++++++++++++------ 4 files changed, 177 insertions(+), 26 deletions(-) diff --git a/crates/tui/src/main.rs b/crates/tui/src/main.rs index 546a7df741..a3f9b6e0f5 100644 --- a/crates/tui/src/main.rs +++ b/crates/tui/src/main.rs @@ -6470,7 +6470,11 @@ async fn run_interactive( logging::warn(format!("Failed to install system skills: {e}")); } - spawn_interactive_startup_maintenance(workspace.clone(), config.snapshots_config()); + spawn_interactive_startup_maintenance( + workspace.clone(), + config.snapshots_config(), + resume_session_id.clone(), + ); // The `deepseek` launcher forwards `--yolo` to this binary via the // DEEPSEEK_YOLO env var (config.yolo), not as a CLI flag. Honour either. @@ -6506,13 +6510,18 @@ async fn run_interactive( fn spawn_interactive_startup_maintenance( workspace: PathBuf, snapshots: crate::config::SnapshotsConfig, + protected_session_id: Option, ) { let spawn_result = std::thread::Builder::new() .name("codewhale-startup-maintenance".to_string()) .spawn(move || { // Keep the first interactive frame ahead of optional disk cleanup. std::thread::sleep(Duration::from_millis(500)); - run_interactive_startup_maintenance(&workspace, &snapshots); + run_interactive_startup_maintenance( + &workspace, + &snapshots, + protected_session_id.as_deref(), + ); }); if let Err(err) = spawn_result { @@ -6523,6 +6532,7 @@ fn spawn_interactive_startup_maintenance( fn run_interactive_startup_maintenance( workspace: &Path, snapshots: &crate::config::SnapshotsConfig, + protected_session_id: Option<&str>, ) { let started = Instant::now(); @@ -6552,7 +6562,7 @@ fn run_interactive_startup_maintenance( // Keeps at most MAX_SESSIONS (50) recent sessions; non-fatal on error. match session_manager::SessionManager::default_location() { Ok(manager) => { - if let Err(err) = manager.cleanup_old_sessions() { + if let Err(err) = manager.cleanup_old_sessions_except(protected_session_id) { tracing::warn!(target: "session", ?err, "session cleanup skipped on boot"); } } diff --git a/crates/tui/src/session_manager.rs b/crates/tui/src/session_manager.rs index efeb739d74..54f5b90d1e 100644 --- a/crates/tui/src/session_manager.rs +++ b/crates/tui/src/session_manager.rs @@ -489,11 +489,19 @@ impl SessionManager { /// Clean up old sessions to stay within `MAX_SESSIONS` limit. pub fn cleanup_old_sessions(&self) -> std::io::Result<()> { + self.cleanup_old_sessions_except(None) + } + + /// Clean up old sessions while preserving an active/resumed session. + pub fn cleanup_old_sessions_except(&self, protected_id: Option<&str>) -> std::io::Result<()> { let sessions = self.list_sessions()?; if sessions.len() > MAX_SESSIONS { // Delete oldest sessions for session in sessions.iter().skip(MAX_SESSIONS) { + if protected_id.is_some_and(|id| id == session.id) { + continue; + } let _ = self.delete_session(&session.id); } } @@ -1108,6 +1116,23 @@ mod tests { workspace: &Path, updated_at: DateTime, ) { + let session = make_session_record(id, workspace, updated_at); + manager.save_session(&session).expect("save"); + } + + fn write_session_record_without_cleanup( + manager: &SessionManager, + id: &str, + workspace: &Path, + updated_at: DateTime, + ) { + let session = make_session_record(id, workspace, updated_at); + let path = manager.validated_session_path(id).expect("path"); + let content = serde_json::to_string_pretty(&session).expect("json"); + fs::write(path, content).expect("write session"); + } + + fn make_session_record(id: &str, workspace: &Path, updated_at: DateTime) -> SavedSession { let session = SavedSession { schema_version: CURRENT_SESSION_SCHEMA_VERSION, messages: vec![make_test_message("user", "hi")], @@ -1130,7 +1155,7 @@ mod tests { context_references: Vec::new(), artifacts: Vec::new(), }; - manager.save_session(&session).expect("save"); + session } fn write_empty_session_record( @@ -2184,6 +2209,31 @@ mod tests { assert_eq!(loaded.artifacts, session.artifacts); } + #[test] + fn cleanup_old_sessions_except_preserves_protected_old_session() { + let tmp = tempdir().expect("tempdir"); + let manager = SessionManager::new(tmp.path().join("sessions")).expect("new"); + let now = Utc::now(); + for idx in 0..(MAX_SESSIONS + 2) { + write_session_record_without_cleanup( + &manager, + &format!("session-{idx:02}"), + Path::new("/tmp"), + now - chrono::Duration::minutes(idx as i64), + ); + } + + manager + .cleanup_old_sessions_except(Some("session-51")) + .expect("cleanup"); + let sessions = manager.list_sessions().expect("list"); + let ids: Vec<_> = sessions.iter().map(|session| session.id.as_str()).collect(); + + assert!(ids.contains(&"session-51")); + assert!(!ids.contains(&"session-50")); + assert_eq!(sessions.len(), MAX_SESSIONS + 1); + } + // ---- #406 prune_sessions_older_than ---- // // The helper is a building block for the auto-archive design: it diff --git a/crates/tui/src/snapshot/repo.rs b/crates/tui/src/snapshot/repo.rs index fe016fad3a..9b0deba052 100644 --- a/crates/tui/src/snapshot/repo.rs +++ b/crates/tui/src/snapshot/repo.rs @@ -13,6 +13,7 @@ //! directory". use std::collections::HashSet; +use std::fs::OpenOptions; use std::io; use std::path::{Component, Path, PathBuf}; use std::process::Output; @@ -51,6 +52,7 @@ pub struct SnapshotRepo { } const STALE_TMP_PACK_AGE: Duration = Duration::from_secs(60 * 60); +const SNAPSHOT_LOCK_FILE: &str = "codewhale-snapshot.lock"; /// Maximum total snapshot storage in megabytes before pruning kicks in at /// snapshot time. Keeps the side repo from blowing up the user's disk during @@ -305,6 +307,10 @@ impl SnapshotRepo { /// /// Returns the snapshot's commit SHA. pub fn snapshot(&self, label: &str) -> io::Result { + self.with_repo_write_lock(|| self.snapshot_locked(label)) + } + + fn snapshot_locked(&self, label: &str) -> io::Result { // Guard against disk blowup (#1112): if the snapshot directory has // grown beyond the limit, prune aggressively before adding more. if let Ok(current_mb) = dir_size_mb(&self.git_dir) @@ -320,7 +326,7 @@ impl SnapshotRepo { // we're under the target, or until there's nothing left. let mut age = Duration::from_secs(1); for _ in 0..10 { - let _ = self.prune_older_than(age); + let _ = self.prune_older_than_locked(age); if let Ok(new_size) = dir_size_mb(&self.git_dir) && new_size <= PRUNE_TARGET_MB { @@ -343,7 +349,7 @@ impl SnapshotRepo { target: "snapshot", "snapshot storage still over limit after pruning; wiping history" ); - let _ = self.prune_older_than(Duration::ZERO); + let _ = self.prune_older_than_locked(Duration::ZERO); let _ = self.prune_unreachable_objects(); } } @@ -550,6 +556,10 @@ impl SnapshotRepo { /// `git gc --prune=now` to actually reclaim space. Cheap and avoids /// rewriting history when nothing has aged out. pub fn prune_older_than(&self, max_age: Duration) -> io::Result { + self.with_repo_write_lock(|| self.prune_older_than_locked(max_age)) + } + + fn prune_older_than_locked(&self, max_age: Duration) -> io::Result { let now = SystemTime::now() .duration_since(UNIX_EPOCH) .map_err(|e| io_other(format!("clock error: {e}")))? @@ -712,6 +722,19 @@ impl SnapshotRepo { Ok(removed) } + fn with_repo_write_lock(&self, f: impl FnOnce() -> io::Result) -> io::Result { + let lock_path = self.git_dir.join(SNAPSHOT_LOCK_FILE); + let lock_file = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(lock_path)?; + let mut lock = fd_lock::RwLock::new(lock_file); + let _guard = lock.write()?; + f() + } + /// Drop unreachable loose objects left behind by interrupted or /// orphaned side-repo operations. pub fn prune_unreachable_objects(&self) -> io::Result<()> { diff --git a/crates/tui/src/tools/truncate.rs b/crates/tui/src/tools/truncate.rs index 55306f02b5..9cfad5cf1f 100644 --- a/crates/tui/src/tools/truncate.rs +++ b/crates/tui/src/tools/truncate.rs @@ -33,19 +33,16 @@ //! tool-details pager opens the spillover file when the user //! presses the tool-details shortcut on a spilled tool cell. -use std::fs; +use std::fs::{self, FileTimes, OpenOptions}; use std::io; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::time::{Duration, SystemTime}; use crate::tools::spec::ToolResult; -// `Path` is only referenced from helpers gated to test builds. -#[cfg(test)] -use std::path::Path; - /// Name of the spillover directory under the CodeWhale home. pub const SPILLOVER_DIR_NAME: &str = "tool_outputs"; +const SPILLOVER_LOCK_FILE: &str = ".codewhale-tool-outputs.lock"; /// Default threshold above which a tool result is a candidate for /// spillover. Mirrors the `MAX_MEMORY_SIZE` ceiling we use elsewhere @@ -145,14 +142,23 @@ pub fn write_sha_spillover(sha: &str, content: &str) -> io::Result { "sha must be a 64-char lowercase hex digest", ) })?; - if path.exists() { - return Ok(path); - } - if let Some(parent) = path.parent() { - fs::create_dir_all(parent)?; - } - crate::utils::write_atomic(&path, content.as_bytes())?; - Ok(path) + let parent = path + .parent() + .ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + "could not resolve sha spillover directory", + ) + })? + .to_path_buf(); + with_spillover_write_lock(&parent, || { + if path.exists() { + refresh_modified(&path)?; + return Ok(path.clone()); + } + crate::utils::write_atomic(&path, content.as_bytes())?; + Ok(path.clone()) + }) } /// Write `content` to the spillover file for `id`. Creates the @@ -169,11 +175,19 @@ pub fn write_spillover(id: &str, content: &str) -> io::Result { "could not resolve spillover path (empty/invalid id or missing home directory)", ) })?; - if let Some(parent) = path.parent() { - fs::create_dir_all(parent)?; - } - crate::utils::write_atomic(&path, content.as_bytes())?; - Ok(path) + let parent = path + .parent() + .ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + "could not resolve spillover directory", + ) + })? + .to_path_buf(); + with_spillover_write_lock(&parent, || { + crate::utils::write_atomic(&path, content.as_bytes())?; + Ok(path.clone()) + }) } /// Drop spillover files older than `max_age`. Returns the number of @@ -187,11 +201,15 @@ pub fn prune_older_than(max_age: Duration) -> io::Result { if !root.exists() { return Ok(0); } + with_spillover_write_lock(&root, || prune_older_than_locked(&root, max_age)) +} + +fn prune_older_than_locked(root: &Path, max_age: Duration) -> io::Result { let cutoff = SystemTime::now() .checked_sub(max_age) .unwrap_or(SystemTime::UNIX_EPOCH); let mut pruned = 0usize; - for entry in fs::read_dir(&root)? { + for entry in fs::read_dir(root)? { let entry = match entry { Ok(e) => e, Err(err) => { @@ -200,6 +218,12 @@ pub fn prune_older_than(max_age: Duration) -> io::Result { } }; let path = entry.path(); + if path + .file_name() + .is_some_and(|name| name == SPILLOVER_LOCK_FILE) + { + continue; + } if !path.is_file() { continue; } @@ -221,6 +245,24 @@ pub fn prune_older_than(max_age: Duration) -> io::Result { Ok(pruned) } +fn refresh_modified(path: &Path) -> io::Result<()> { + let file = OpenOptions::new().write(true).open(path)?; + file.set_times(FileTimes::new().set_modified(SystemTime::now())) +} + +fn with_spillover_write_lock(root: &Path, f: impl FnOnce() -> io::Result) -> io::Result { + fs::create_dir_all(root)?; + let lock_file = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(root.join(SPILLOVER_LOCK_FILE))?; + let mut lock = fd_lock::RwLock::new(lock_file); + let _guard = lock.write()?; + f() +} + /// Convenience for the common "too long? spill it." pattern. If /// `content` is at or below `threshold` bytes, returns `None` and the /// caller keeps the inline content. Above the threshold, writes the @@ -628,6 +670,32 @@ mod tests { }); } + #[test] + fn write_sha_spillover_refreshes_reused_file_mtime() { + let _g = setup(); + let tmp = tempdir().unwrap(); + with_test_home(tmp.path(), || { + let sha = "a".repeat(64); + let path = write_sha_spillover(&sha, "large result").expect("write sha"); + let stale_time = SystemTime::now() - Duration::from_secs(30 * 24 * 60 * 60); + let file = OpenOptions::new() + .write(true) + .open(&path) + .expect("open sha"); + file.set_times(FileTimes::new().set_modified(stale_time)) + .expect("backdate sha"); + + let reused = write_sha_spillover(&sha, "large result").expect("reuse sha"); + let refreshed = fs::metadata(&reused) + .expect("metadata") + .modified() + .expect("modified"); + + assert_eq!(reused, path); + assert!(refreshed > stale_time); + }); + } + #[test] fn maybe_spillover_returns_none_below_threshold() { let _g = setup(); From e830f062d6b795d14b1bf49d74ad50140ead413a Mon Sep 17 00:00:00 2001 From: Nightt <87569709+nightt5879@users.noreply.github.com> Date: Mon, 29 Jun 2026 12:10:05 +0800 Subject: [PATCH 3/5] fix(tui): close startup maintenance races Address the second Codex review pass on PR #3761. Put all side-git snapshot mutation entrypoints behind the same repo-local advisory lock, including keep-last pruning and unreachable-object pruning. Also preserve resumed sessions by the same prefix semantics accepted by resume loading, with explicit handling for --resume latest. Verification: - cargo fmt --all -- --check - cargo clippy --workspace --all-features --locked -- -D warnings -A clippy::uninlined_format_args -A clippy::too_many_arguments -A clippy::unnecessary_map_or -A clippy::collapsible_if -A clippy::assertions_on_constants - cargo test -p codewhale-tui --bin codewhale-tui --locked cleanup_old_sessions_except_preserves_protected_old_session - cargo test -p codewhale-tui --bin codewhale-tui --locked prune_keep_last_n - cargo test -p codewhale-tui --bin codewhale-tui --locked startup - cargo test --workspace --locked - cargo build --release -p codewhale-cli -p codewhale-tui --locked --- crates/tui/src/main.rs | 24 +++++++++++++++++++++++- crates/tui/src/session_manager.rs | 11 +++++++---- crates/tui/src/snapshot/repo.rs | 10 +++++++++- 3 files changed, 39 insertions(+), 6 deletions(-) diff --git a/crates/tui/src/main.rs b/crates/tui/src/main.rs index a3f9b6e0f5..b8287153f9 100644 --- a/crates/tui/src/main.rs +++ b/crates/tui/src/main.rs @@ -6470,10 +6470,12 @@ async fn run_interactive( logging::warn(format!("Failed to install system skills: {e}")); } + let protected_session_id = + startup_maintenance_protected_session_id(resume_session_id.as_deref(), &workspace); spawn_interactive_startup_maintenance( workspace.clone(), config.snapshots_config(), - resume_session_id.clone(), + protected_session_id, ); // The `deepseek` launcher forwards `--yolo` to this binary via the @@ -6529,6 +6531,26 @@ fn spawn_interactive_startup_maintenance( } } +fn startup_maintenance_protected_session_id( + resume_session_id: Option<&str>, + workspace: &Path, +) -> Option { + let session_id = resume_session_id?; + if session_id != "latest" { + return Some(session_id.to_string()); + } + + session_manager::SessionManager::default_location() + .ok() + .and_then(|manager| { + manager + .get_latest_session_for_workspace(workspace) + .ok() + .flatten() + }) + .map(|session| session.id) +} + fn run_interactive_startup_maintenance( workspace: &Path, snapshots: &crate::config::SnapshotsConfig, diff --git a/crates/tui/src/session_manager.rs b/crates/tui/src/session_manager.rs index 54f5b90d1e..56b9631a0c 100644 --- a/crates/tui/src/session_manager.rs +++ b/crates/tui/src/session_manager.rs @@ -493,13 +493,16 @@ impl SessionManager { } /// Clean up old sessions while preserving an active/resumed session. + /// + /// Accepts a full id or resume prefix; startup passes the same user-facing + /// resume token that `load_session_by_prefix` accepts. pub fn cleanup_old_sessions_except(&self, protected_id: Option<&str>) -> std::io::Result<()> { let sessions = self.list_sessions()?; if sessions.len() > MAX_SESSIONS { // Delete oldest sessions for session in sessions.iter().skip(MAX_SESSIONS) { - if protected_id.is_some_and(|id| id == session.id) { + if protected_id.is_some_and(|id| session.id == id || session.id.starts_with(id)) { continue; } let _ = self.delete_session(&session.id); @@ -2217,7 +2220,7 @@ mod tests { for idx in 0..(MAX_SESSIONS + 2) { write_session_record_without_cleanup( &manager, - &format!("session-{idx:02}"), + &format!("session-{idx:02}-abcdef"), Path::new("/tmp"), now - chrono::Duration::minutes(idx as i64), ); @@ -2229,8 +2232,8 @@ mod tests { let sessions = manager.list_sessions().expect("list"); let ids: Vec<_> = sessions.iter().map(|session| session.id.as_str()).collect(); - assert!(ids.contains(&"session-51")); - assert!(!ids.contains(&"session-50")); + assert!(ids.contains(&"session-51-abcdef")); + assert!(!ids.contains(&"session-50-abcdef")); assert_eq!(sessions.len(), MAX_SESSIONS + 1); } diff --git a/crates/tui/src/snapshot/repo.rs b/crates/tui/src/snapshot/repo.rs index 9b0deba052..a88a4aaaa5 100644 --- a/crates/tui/src/snapshot/repo.rs +++ b/crates/tui/src/snapshot/repo.rs @@ -350,7 +350,7 @@ impl SnapshotRepo { "snapshot storage still over limit after pruning; wiping history" ); let _ = self.prune_older_than_locked(Duration::ZERO); - let _ = self.prune_unreachable_objects(); + let _ = self.prune_unreachable_objects_locked(); } } // Stage every tracked + untracked path the workspace exposes. @@ -647,6 +647,10 @@ impl SnapshotRepo { /// are preserved — only the parent chain to older snapshots is cut. /// Old objects become unreachable and gc reclaims them. pub fn prune_keep_last_n(&self, max_count: usize) -> io::Result { + self.with_repo_write_lock(|| self.prune_keep_last_n_locked(max_count)) + } + + fn prune_keep_last_n_locked(&self, max_count: usize) -> io::Result { let snapshots = self.list(usize::MAX)?; if snapshots.len() <= max_count { return Ok(0); @@ -738,6 +742,10 @@ impl SnapshotRepo { /// Drop unreachable loose objects left behind by interrupted or /// orphaned side-repo operations. pub fn prune_unreachable_objects(&self) -> io::Result<()> { + self.with_repo_write_lock(|| self.prune_unreachable_objects_locked()) + } + + fn prune_unreachable_objects_locked(&self) -> io::Result<()> { let prune = run_git(&self.git_dir, &self.work_tree, &["prune", "--expire=now"])?; if !prune.status.success() { return Err(io_other(format!( From 90fa8a1f3f13579fdf5618018fb454becd3efc4a Mon Sep 17 00:00:00 2001 From: Nightt <87569709+nightt5879@users.noreply.github.com> Date: Mon, 29 Jun 2026 12:44:36 +0800 Subject: [PATCH 4/5] fix(tui): harden deferred startup maintenance Resolve Codex review feedback on the startup cleanup deferral. Move latest-session protection resolution into the maintenance thread so startup does not scan sessions twice. Use a launch-time cutoff for deferred snapshot pruning, with a conservative same-second boundary for git commit timestamps. Treat existing SHA spillover files as cache hits even when refreshing their mtime fails. Validation: - cargo fmt - cargo fmt --all -- --check - git diff --check - cargo test -p codewhale-tui --bin codewhale-tui --locked prune_with_launch_cutoff_keeps_cutoff_second_snapshots - cargo test -p codewhale-tui --bin codewhale-tui --locked prune_with_existing_repo_zero_age_clears_all - cargo test -p codewhale-tui --bin codewhale-tui --locked write_sha_spillover_reuses_existing_file_when_refresh_fails - cargo test -p codewhale-tui --bin codewhale-tui --locked startup - cargo test -p codewhale-tui --bin codewhale-tui --locked prune_keep_last_n - cargo clippy --workspace --all-features --locked -- -D warnings -A clippy::uninlined_format_args -A clippy::too_many_arguments -A clippy::unnecessary_map_or -A clippy::collapsible_if -A clippy::assertions_on_constants - cargo test --workspace --all-features --locked - cargo build --release -p codewhale-cli -p codewhale-tui --locked --- crates/tui/src/main.rs | 24 ++++++++++++------- crates/tui/src/session_manager.rs | 21 ++++++++++++++++ crates/tui/src/snapshot/mod.rs | 3 ++- crates/tui/src/snapshot/prune.rs | 40 ++++++++++++++++++++++++++++++- crates/tui/src/snapshot/repo.rs | 34 +++++++++++++++++++------- crates/tui/src/tools/truncate.rs | 33 ++++++++++++++++++++++++- 6 files changed, 135 insertions(+), 20 deletions(-) diff --git a/crates/tui/src/main.rs b/crates/tui/src/main.rs index b8287153f9..d49a585a02 100644 --- a/crates/tui/src/main.rs +++ b/crates/tui/src/main.rs @@ -5,7 +5,7 @@ use std::io::{self, IsTerminal, Read, Write}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; -use std::time::{Duration, Instant}; +use std::time::{Duration, Instant, SystemTime}; use anyhow::{Context, Result, anyhow, bail}; use clap::{Args, CommandFactory, Parser, Subcommand, ValueEnum}; @@ -6470,12 +6470,13 @@ async fn run_interactive( logging::warn(format!("Failed to install system skills: {e}")); } - let protected_session_id = - startup_maintenance_protected_session_id(resume_session_id.as_deref(), &workspace); + let startup_maintenance_started_at = SystemTime::now(); + let protected_session_token = resume_session_id.clone(); spawn_interactive_startup_maintenance( workspace.clone(), config.snapshots_config(), - protected_session_id, + protected_session_token, + startup_maintenance_started_at, ); // The `deepseek` launcher forwards `--yolo` to this binary via the @@ -6512,7 +6513,8 @@ async fn run_interactive( fn spawn_interactive_startup_maintenance( workspace: PathBuf, snapshots: crate::config::SnapshotsConfig, - protected_session_id: Option, + protected_session_token: Option, + started_at: SystemTime, ) { let spawn_result = std::thread::Builder::new() .name("codewhale-startup-maintenance".to_string()) @@ -6522,7 +6524,8 @@ fn spawn_interactive_startup_maintenance( run_interactive_startup_maintenance( &workspace, &snapshots, - protected_session_id.as_deref(), + protected_session_token.as_deref(), + started_at, ); }); @@ -6554,7 +6557,8 @@ fn startup_maintenance_protected_session_id( fn run_interactive_startup_maintenance( workspace: &Path, snapshots: &crate::config::SnapshotsConfig, - protected_session_id: Option<&str>, + protected_session_token: Option<&str>, + started_at: SystemTime, ) { let started = Instant::now(); @@ -6562,7 +6566,7 @@ fn run_interactive_startup_maintenance( // Non-fatal: a flaky disk, missing `git`, or read-only home should // never block the TUI from starting. if snapshots.enabled { - session_manager::prune_workspace_snapshots(workspace, snapshots.max_age()); + session_manager::prune_workspace_snapshots_at(workspace, snapshots.max_age(), started_at); } // Prune stale tool-output spillover files (#422). Non-fatal: home @@ -6582,9 +6586,11 @@ fn run_interactive_startup_maintenance( // v0.8.44: prune managed sessions to prevent unbounded growth. // Keeps at most MAX_SESSIONS (50) recent sessions; non-fatal on error. + let protected_session_id = + startup_maintenance_protected_session_id(protected_session_token, workspace); match session_manager::SessionManager::default_location() { Ok(manager) => { - if let Err(err) = manager.cleanup_old_sessions_except(protected_session_id) { + if let Err(err) = manager.cleanup_old_sessions_except(protected_session_id.as_deref()) { tracing::warn!(target: "session", ?err, "session cleanup skipped on boot"); } } diff --git a/crates/tui/src/session_manager.rs b/crates/tui/src/session_manager.rs index 56b9631a0c..26c3a488df 100644 --- a/crates/tui/src/session_manager.rs +++ b/crates/tui/src/session_manager.rs @@ -15,6 +15,7 @@ use serde::{Deserialize, Serialize}; use std::fs; use std::io; use std::path::{Component, Path, PathBuf}; +use std::time::SystemTime; use uuid::Uuid; /// Maximum number of sessions to retain @@ -735,6 +736,26 @@ pub fn prune_workspace_snapshots(workspace: &Path, max_age: std::time::Duration) } } +/// Prune snapshots older than `max_age` as of `now`. +/// +/// Startup maintenance passes the launch timestamp so delayed cleanup does not +/// prune snapshots created by the first interactive turn. +pub fn prune_workspace_snapshots_at( + workspace: &Path, + max_age: std::time::Duration, + now: SystemTime, +) { + match crate::snapshot::prune_older_than_at(workspace, max_age, now) { + Ok(0) => {} + Ok(n) => { + tracing::debug!(target: "snapshot", "boot prune removed {n} snapshot(s)"); + } + Err(e) => { + tracing::warn!(target: "snapshot", "boot prune failed: {e}"); + } + } +} + /// Create a new `SavedSession` from conversation state pub fn create_saved_session( messages: &[Message], diff --git a/crates/tui/src/snapshot/mod.rs b/crates/tui/src/snapshot/mod.rs index 08cf3e0916..a77f5e0a7e 100644 --- a/crates/tui/src/snapshot/mod.rs +++ b/crates/tui/src/snapshot/mod.rs @@ -39,7 +39,8 @@ pub mod repo; #[allow(unused_imports)] pub use paths::{snapshot_dir_for, snapshot_git_dir}; -pub use prune::{DEFAULT_MAX_AGE, prune_older_than}; +#[allow(unused_imports)] +pub use prune::{DEFAULT_MAX_AGE, prune_older_than, prune_older_than_at}; /// Maximum snapshots kept per workspace side-repo. Oldest are pruned /// after each new snapshot to cap disk usage (#1112). diff --git a/crates/tui/src/snapshot/prune.rs b/crates/tui/src/snapshot/prune.rs index 6953941b48..a498f76e53 100644 --- a/crates/tui/src/snapshot/prune.rs +++ b/crates/tui/src/snapshot/prune.rs @@ -6,7 +6,7 @@ use std::io; use std::path::Path; -use std::time::Duration; +use std::time::{Duration, SystemTime}; use super::paths::snapshot_git_dir; use super::repo::SnapshotRepo; @@ -29,11 +29,31 @@ pub fn prune_older_than(workspace: &Path, max_age: Duration) -> io::Result io::Result { + let git_dir = snapshot_git_dir(workspace); + if !git_dir.exists() { + return Ok(0); + } + let repo = SnapshotRepo::open_or_init(workspace)?; + let removed = repo.prune_older_than_at(max_age, now)?; + repo.prune_unreachable_objects()?; + Ok(removed) +} + #[cfg(test)] mod tests { use super::*; use crate::test_support::lock_test_env; use std::sync::MutexGuard; + use std::time::UNIX_EPOCH; use tempfile::tempdir; /// Same guard shape as in `repo::tests` — pins HOME for the lifetime @@ -90,4 +110,22 @@ mod tests { let removed = prune_older_than(&workspace, Duration::from_secs(0)).unwrap(); assert!(removed >= 1); } + + #[test] + fn prune_with_launch_cutoff_keeps_cutoff_second_snapshots() { + let tmp = tempdir().unwrap(); + let _home = scoped_home(tmp.path()); + let workspace = tmp.path().join("ws"); + std::fs::create_dir_all(&workspace).unwrap(); + let repo = SnapshotRepo::open_or_init(&workspace).unwrap(); + + std::fs::write(workspace.join("f.txt"), "x").unwrap(); + repo.snapshot("turn:0").unwrap(); + let snapshot = repo.list(1).unwrap().pop().unwrap(); + let launch_time = UNIX_EPOCH + Duration::from_secs(snapshot.timestamp as u64); + + let removed = prune_older_than_at(&workspace, Duration::from_secs(0), launch_time).unwrap(); + assert_eq!(removed, 0); + assert_eq!(repo.list(10).unwrap().len(), 1); + } } diff --git a/crates/tui/src/snapshot/repo.rs b/crates/tui/src/snapshot/repo.rs index a88a4aaaa5..b4801abd37 100644 --- a/crates/tui/src/snapshot/repo.rs +++ b/crates/tui/src/snapshot/repo.rs @@ -559,8 +559,21 @@ impl SnapshotRepo { self.with_repo_write_lock(|| self.prune_older_than_locked(max_age)) } + pub fn prune_older_than_at(&self, max_age: Duration, now: SystemTime) -> io::Result { + self.with_repo_write_lock(|| self.prune_older_than_locked_at(max_age, now, false)) + } + fn prune_older_than_locked(&self, max_age: Duration) -> io::Result { - let now = SystemTime::now() + self.prune_older_than_locked_at(max_age, SystemTime::now(), true) + } + + fn prune_older_than_locked_at( + &self, + max_age: Duration, + now: SystemTime, + include_cutoff_second: bool, + ) -> io::Result { + let now = now .duration_since(UNIX_EPOCH) .map_err(|e| io_other(format!("clock error: {e}")))? .as_secs() as i64; @@ -571,13 +584,18 @@ impl SnapshotRepo { return Ok(0); } - // Snapshots are newest-first. Find the index of the first one - // at-or-older than the cutoff — every entry from that index - // onward is a candidate for removal. We use `<=` so a 0-second - // retention drops same-second commits (otherwise tests calling - // `prune_older_than(Duration::ZERO)` immediately after creating - // a snapshot would never prune anything). - let cut_index = snapshots.iter().position(|s| s.timestamp <= cutoff); + // Snapshots are newest-first. Find the index of the first prune + // candidate; every entry from that index onward is removed. The + // immediate path includes the cutoff second so zero-age manual prune + // remains aggressive. The fixed-cutoff startup path excludes it + // because git timestamps cannot distinguish subsecond launch order. + let cut_index = snapshots.iter().position(|s| { + if include_cutoff_second { + s.timestamp <= cutoff + } else { + s.timestamp < cutoff + } + }); let Some(cut) = cut_index else { return Ok(0); }; diff --git a/crates/tui/src/tools/truncate.rs b/crates/tui/src/tools/truncate.rs index 9cfad5cf1f..e69d44df47 100644 --- a/crates/tui/src/tools/truncate.rs +++ b/crates/tui/src/tools/truncate.rs @@ -151,9 +151,19 @@ pub fn write_sha_spillover(sha: &str, content: &str) -> io::Result { ) })? .to_path_buf(); + if path.exists() { + let _ = with_spillover_write_lock(&parent, || { + if path.exists() { + let _ = refresh_modified(&path); + } + Ok(()) + }); + return Ok(path); + } + with_spillover_write_lock(&parent, || { if path.exists() { - refresh_modified(&path)?; + let _ = refresh_modified(&path); return Ok(path.clone()); } crate::utils::write_atomic(&path, content.as_bytes())?; @@ -696,6 +706,27 @@ mod tests { }); } + #[test] + fn write_sha_spillover_reuses_existing_file_when_refresh_fails() { + let _g = setup(); + let tmp = tempdir().unwrap(); + with_test_home(tmp.path(), || { + let sha = "b".repeat(64); + let path = write_sha_spillover(&sha, "first").expect("write sha"); + let original_permissions = fs::metadata(&path).expect("metadata").permissions(); + let mut readonly_permissions = original_permissions.clone(); + readonly_permissions.set_readonly(true); + fs::set_permissions(&path, readonly_permissions).expect("make readonly"); + + let reused = write_sha_spillover(&sha, "second"); + fs::set_permissions(&path, original_permissions).expect("restore permissions"); + + let reused = reused.expect("reuse readonly sha"); + assert_eq!(reused, path); + assert_eq!(fs::read_to_string(&path).unwrap(), "first"); + }); + } + #[test] fn maybe_spillover_returns_none_below_threshold() { let _g = setup(); From eba988c99db90d80f8ee87966baf787ca4d0f633 Mon Sep 17 00:00:00 2001 From: Nightt <87569709+nightt5879@users.noreply.github.com> Date: Mon, 29 Jun 2026 13:10:58 +0800 Subject: [PATCH 5/5] fix(tui): keep snapshot pruning before restore commands Resolve Codex review feedback on deferred startup maintenance. Keep snapshot pruning on the pre-TUI path because it rewrites side-repo refs and can run GC after snapshot list/restore commands become available. Keep the deferred thread for spillover and session cleanup, and keep latest resume resolution inside that thread. Also make snapshot list/restore/read checks share the side-repo lock, and make SHA spillover cache hits rewrite the content if the file disappears after the optimistic existence check. Validation: - cargo fmt - cargo fmt --all -- --check - git diff --check - cargo test -p codewhale-tui --bin codewhale-tui --locked startup - cargo test -p codewhale-tui --bin codewhale-tui --locked prune_with_existing_repo_zero_age_clears_all - cargo test -p codewhale-tui --bin codewhale-tui --locked restore_reverts_workspace_files - cargo test -p codewhale-tui --bin codewhale-tui --locked list_respects_limit - cargo test -p codewhale-tui --bin codewhale-tui --locked snapshot_and_restore_do_not_move_user_git_head - cargo test -p codewhale-tui --bin codewhale-tui --locked write_sha_spillover_reuses_existing_file_when_refresh_fails - cargo test -p codewhale-tui --bin codewhale-tui --locked write_sha_spillover_refreshes_reused_file_mtime - cargo clippy --workspace --all-features --locked -- -D warnings -A clippy::uninlined_format_args -A clippy::too_many_arguments -A clippy::unnecessary_map_or -A clippy::collapsible_if -A clippy::assertions_on_constants - cargo test --workspace --all-features --locked - cargo build --release -p codewhale-cli -p codewhale-tui --locked --- crates/tui/src/main.rs | 38 ++++------------ crates/tui/src/session_manager.rs | 21 --------- crates/tui/src/snapshot/mod.rs | 2 +- crates/tui/src/snapshot/prune.rs | 40 +---------------- crates/tui/src/snapshot/repo.rs | 73 +++++++++++++++++-------------- crates/tui/src/tools/truncate.rs | 13 ++++-- 6 files changed, 59 insertions(+), 128 deletions(-) diff --git a/crates/tui/src/main.rs b/crates/tui/src/main.rs index d49a585a02..936fd84480 100644 --- a/crates/tui/src/main.rs +++ b/crates/tui/src/main.rs @@ -5,7 +5,7 @@ use std::io::{self, IsTerminal, Read, Write}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; -use std::time::{Duration, Instant, SystemTime}; +use std::time::{Duration, Instant}; use anyhow::{Context, Result, anyhow, bail}; use clap::{Args, CommandFactory, Parser, Subcommand, ValueEnum}; @@ -6470,14 +6470,13 @@ async fn run_interactive( logging::warn(format!("Failed to install system skills: {e}")); } - let startup_maintenance_started_at = SystemTime::now(); + // Snapshot pruning rewrites side-repo refs and can run GC, so keep it + // before the TUI exposes snapshot list/restore commands. + if config.snapshots_config().enabled { + session_manager::prune_workspace_snapshots(&workspace, config.snapshots_config().max_age()); + } let protected_session_token = resume_session_id.clone(); - spawn_interactive_startup_maintenance( - workspace.clone(), - config.snapshots_config(), - protected_session_token, - startup_maintenance_started_at, - ); + spawn_interactive_startup_maintenance(workspace.clone(), protected_session_token); // The `deepseek` launcher forwards `--yolo` to this binary via the // DEEPSEEK_YOLO env var (config.yolo), not as a CLI flag. Honour either. @@ -6512,21 +6511,14 @@ async fn run_interactive( fn spawn_interactive_startup_maintenance( workspace: PathBuf, - snapshots: crate::config::SnapshotsConfig, protected_session_token: Option, - started_at: SystemTime, ) { let spawn_result = std::thread::Builder::new() .name("codewhale-startup-maintenance".to_string()) .spawn(move || { // Keep the first interactive frame ahead of optional disk cleanup. std::thread::sleep(Duration::from_millis(500)); - run_interactive_startup_maintenance( - &workspace, - &snapshots, - protected_session_token.as_deref(), - started_at, - ); + run_interactive_startup_maintenance(&workspace, protected_session_token.as_deref()); }); if let Err(err) = spawn_result { @@ -6554,21 +6546,9 @@ fn startup_maintenance_protected_session_id( .map(|session| session.id) } -fn run_interactive_startup_maintenance( - workspace: &Path, - snapshots: &crate::config::SnapshotsConfig, - protected_session_token: Option<&str>, - started_at: SystemTime, -) { +fn run_interactive_startup_maintenance(workspace: &Path, protected_session_token: Option<&str>) { let started = Instant::now(); - // Prune stale workspace snapshots from prior sessions (7-day default). - // Non-fatal: a flaky disk, missing `git`, or read-only home should - // never block the TUI from starting. - if snapshots.enabled { - session_manager::prune_workspace_snapshots_at(workspace, snapshots.max_age(), started_at); - } - // Prune stale tool-output spillover files (#422). Non-fatal: home // missing or directory unreadable just means nothing got pruned. match crate::tools::truncate::prune_older_than(crate::tools::truncate::SPILLOVER_MAX_AGE) { diff --git a/crates/tui/src/session_manager.rs b/crates/tui/src/session_manager.rs index 26c3a488df..56b9631a0c 100644 --- a/crates/tui/src/session_manager.rs +++ b/crates/tui/src/session_manager.rs @@ -15,7 +15,6 @@ use serde::{Deserialize, Serialize}; use std::fs; use std::io; use std::path::{Component, Path, PathBuf}; -use std::time::SystemTime; use uuid::Uuid; /// Maximum number of sessions to retain @@ -736,26 +735,6 @@ pub fn prune_workspace_snapshots(workspace: &Path, max_age: std::time::Duration) } } -/// Prune snapshots older than `max_age` as of `now`. -/// -/// Startup maintenance passes the launch timestamp so delayed cleanup does not -/// prune snapshots created by the first interactive turn. -pub fn prune_workspace_snapshots_at( - workspace: &Path, - max_age: std::time::Duration, - now: SystemTime, -) { - match crate::snapshot::prune_older_than_at(workspace, max_age, now) { - Ok(0) => {} - Ok(n) => { - tracing::debug!(target: "snapshot", "boot prune removed {n} snapshot(s)"); - } - Err(e) => { - tracing::warn!(target: "snapshot", "boot prune failed: {e}"); - } - } -} - /// Create a new `SavedSession` from conversation state pub fn create_saved_session( messages: &[Message], diff --git a/crates/tui/src/snapshot/mod.rs b/crates/tui/src/snapshot/mod.rs index a77f5e0a7e..1e68d50b3d 100644 --- a/crates/tui/src/snapshot/mod.rs +++ b/crates/tui/src/snapshot/mod.rs @@ -40,7 +40,7 @@ pub mod repo; #[allow(unused_imports)] pub use paths::{snapshot_dir_for, snapshot_git_dir}; #[allow(unused_imports)] -pub use prune::{DEFAULT_MAX_AGE, prune_older_than, prune_older_than_at}; +pub use prune::{DEFAULT_MAX_AGE, prune_older_than}; /// Maximum snapshots kept per workspace side-repo. Oldest are pruned /// after each new snapshot to cap disk usage (#1112). diff --git a/crates/tui/src/snapshot/prune.rs b/crates/tui/src/snapshot/prune.rs index a498f76e53..6953941b48 100644 --- a/crates/tui/src/snapshot/prune.rs +++ b/crates/tui/src/snapshot/prune.rs @@ -6,7 +6,7 @@ use std::io; use std::path::Path; -use std::time::{Duration, SystemTime}; +use std::time::Duration; use super::paths::snapshot_git_dir; use super::repo::SnapshotRepo; @@ -29,31 +29,11 @@ pub fn prune_older_than(workspace: &Path, max_age: Duration) -> io::Result io::Result { - let git_dir = snapshot_git_dir(workspace); - if !git_dir.exists() { - return Ok(0); - } - let repo = SnapshotRepo::open_or_init(workspace)?; - let removed = repo.prune_older_than_at(max_age, now)?; - repo.prune_unreachable_objects()?; - Ok(removed) -} - #[cfg(test)] mod tests { use super::*; use crate::test_support::lock_test_env; use std::sync::MutexGuard; - use std::time::UNIX_EPOCH; use tempfile::tempdir; /// Same guard shape as in `repo::tests` — pins HOME for the lifetime @@ -110,22 +90,4 @@ mod tests { let removed = prune_older_than(&workspace, Duration::from_secs(0)).unwrap(); assert!(removed >= 1); } - - #[test] - fn prune_with_launch_cutoff_keeps_cutoff_second_snapshots() { - let tmp = tempdir().unwrap(); - let _home = scoped_home(tmp.path()); - let workspace = tmp.path().join("ws"); - std::fs::create_dir_all(&workspace).unwrap(); - let repo = SnapshotRepo::open_or_init(&workspace).unwrap(); - - std::fs::write(workspace.join("f.txt"), "x").unwrap(); - repo.snapshot("turn:0").unwrap(); - let snapshot = repo.list(1).unwrap().pop().unwrap(); - let launch_time = UNIX_EPOCH + Duration::from_secs(snapshot.timestamp as u64); - - let removed = prune_older_than_at(&workspace, Duration::from_secs(0), launch_time).unwrap(); - assert_eq!(removed, 0); - assert_eq!(repo.list(10).unwrap().len(), 1); - } } diff --git a/crates/tui/src/snapshot/repo.rs b/crates/tui/src/snapshot/repo.rs index b4801abd37..3dc3c356e2 100644 --- a/crates/tui/src/snapshot/repo.rs +++ b/crates/tui/src/snapshot/repo.rs @@ -425,6 +425,10 @@ impl SnapshotRepo { /// snapshot tree relative to the workspace root. We do NOT touch the /// user's own `.git` — snapshots only contain working-tree files. pub fn restore(&self, id: &SnapshotId) -> io::Result<()> { + self.with_repo_write_lock(|| self.restore_locked(id)) + } + + fn restore_locked(&self, id: &SnapshotId) -> io::Result<()> { let current_paths = self.tree_paths("HEAD")?; let target_paths = self.tree_paths(id.as_str())?; let checkout = run_git( @@ -452,12 +456,14 @@ impl SnapshotRepo { /// again would be a no-op, so the caller should continue scanning /// older snapshots. pub fn work_tree_matches_snapshot(&self, id: &SnapshotId) -> io::Result { - let diff = run_git( - &self.git_dir, - &self.work_tree, - &["diff", "--quiet", id.as_str(), "--", ":/"], - )?; - Ok(diff.status.success()) + self.with_repo_read_lock(|| { + let diff = run_git( + &self.git_dir, + &self.work_tree, + &["diff", "--quiet", id.as_str(), "--", ":/"], + )?; + Ok(diff.status.success()) + }) } fn tree_paths(&self, treeish: &str) -> io::Result> { @@ -512,6 +518,10 @@ impl SnapshotRepo { /// List up to `limit` most-recent snapshots, newest first. pub fn list(&self, limit: usize) -> io::Result> { + self.with_repo_read_lock(|| self.list_locked(limit)) + } + + fn list_locked(&self, limit: usize) -> io::Result> { // `git log -` is the short form of `--max-count=`; if `limit` // is `usize::MAX` (caller asked for "everything") we pass an empty // count so git defaults to no upper bound. @@ -559,43 +569,25 @@ impl SnapshotRepo { self.with_repo_write_lock(|| self.prune_older_than_locked(max_age)) } - pub fn prune_older_than_at(&self, max_age: Duration, now: SystemTime) -> io::Result { - self.with_repo_write_lock(|| self.prune_older_than_locked_at(max_age, now, false)) - } - fn prune_older_than_locked(&self, max_age: Duration) -> io::Result { - self.prune_older_than_locked_at(max_age, SystemTime::now(), true) - } - - fn prune_older_than_locked_at( - &self, - max_age: Duration, - now: SystemTime, - include_cutoff_second: bool, - ) -> io::Result { - let now = now + let now = SystemTime::now() .duration_since(UNIX_EPOCH) .map_err(|e| io_other(format!("clock error: {e}")))? .as_secs() as i64; let cutoff = now - max_age.as_secs() as i64; - let snapshots = self.list(usize::MAX)?; + let snapshots = self.list_locked(usize::MAX)?; if snapshots.is_empty() { return Ok(0); } - // Snapshots are newest-first. Find the index of the first prune - // candidate; every entry from that index onward is removed. The - // immediate path includes the cutoff second so zero-age manual prune - // remains aggressive. The fixed-cutoff startup path excludes it - // because git timestamps cannot distinguish subsecond launch order. - let cut_index = snapshots.iter().position(|s| { - if include_cutoff_second { - s.timestamp <= cutoff - } else { - s.timestamp < cutoff - } - }); + // Snapshots are newest-first. Find the index of the first one + // at-or-older than the cutoff — every entry from that index + // onward is a candidate for removal. We use `<=` so a 0-second + // retention drops same-second commits (otherwise tests calling + // `prune_older_than(Duration::ZERO)` immediately after creating + // a snapshot would never prune anything). + let cut_index = snapshots.iter().position(|s| s.timestamp <= cutoff); let Some(cut) = cut_index else { return Ok(0); }; @@ -669,7 +661,7 @@ impl SnapshotRepo { } fn prune_keep_last_n_locked(&self, max_count: usize) -> io::Result { - let snapshots = self.list(usize::MAX)?; + let snapshots = self.list_locked(usize::MAX)?; if snapshots.len() <= max_count { return Ok(0); } @@ -757,6 +749,19 @@ impl SnapshotRepo { f() } + fn with_repo_read_lock(&self, f: impl FnOnce() -> io::Result) -> io::Result { + let lock_path = self.git_dir.join(SNAPSHOT_LOCK_FILE); + let lock_file = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(lock_path)?; + let lock = fd_lock::RwLock::new(lock_file); + let _guard = lock.read()?; + f() + } + /// Drop unreachable loose objects left behind by interrupted or /// orphaned side-repo operations. pub fn prune_unreachable_objects(&self) -> io::Result<()> { diff --git a/crates/tui/src/tools/truncate.rs b/crates/tui/src/tools/truncate.rs index e69d44df47..58e8c5a92c 100644 --- a/crates/tui/src/tools/truncate.rs +++ b/crates/tui/src/tools/truncate.rs @@ -152,13 +152,18 @@ pub fn write_sha_spillover(sha: &str, content: &str) -> io::Result { })? .to_path_buf(); if path.exists() { - let _ = with_spillover_write_lock(&parent, || { + return match with_spillover_write_lock(&parent, || { if path.exists() { let _ = refresh_modified(&path); + } else { + crate::utils::write_atomic(&path, content.as_bytes())?; } - Ok(()) - }); - return Ok(path); + Ok(path.clone()) + }) { + Ok(path) => Ok(path), + Err(_) if path.exists() => Ok(path), + Err(err) => Err(err), + }; } with_spillover_write_lock(&parent, || {