From 46dd5d5ccf0e302901f9db135c94eedb9c14303d Mon Sep 17 00:00:00 2001 From: Max Lampert Date: Mon, 24 Aug 2026 22:01:01 -0700 Subject: [PATCH] fix(desktop): degrade to unrotated append when perf-log rotation fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Blind-review round 6 (Sonnet xhigh) finding: a transient lock on the rotated generation (AV/EDR or an editor, chiefly Windows) failed the whole append — and since the live file stays oversized, every later append re-entered the same failing branch, silently dropping every record until the lock cleared. Rotation is now best-effort: on failure the line appends unrotated and the size cap re-applies once a later rotation succeeds. Unix regression locks the directory and asserts the line survives. Co-Authored-By: Claude Fable 5 Signed-off-by: Max Lampert --- desktop/src-tauri/src/commands/perf_log.rs | 59 +++++++++++++++++++--- 1 file changed, 52 insertions(+), 7 deletions(-) diff --git a/desktop/src-tauri/src/commands/perf_log.rs b/desktop/src-tauri/src/commands/perf_log.rs index 6ced5b3467..a2c8ce21a8 100644 --- a/desktop/src-tauri/src/commands/perf_log.rs +++ b/desktop/src-tauri/src/commands/perf_log.rs @@ -102,14 +102,24 @@ fn append_line_rotating(path: &std::path::Path, line: &str, max_bytes: u64) -> R rotated.push(".1"); let rotated = std::path::PathBuf::from(rotated); // Remove the retained generation before renaming over it: on - // Windows, rename does not replace an existing destination, and a - // failed rotation here would silently drop every subsequent trace - // (the frontend deliberately swallows sink errors). Same platform - // rule as managed_agents::storage::start_install_log_session. - if rotated.exists() { - std::fs::remove_file(&rotated).map_err(|e| e.to_string())?; + // Windows, rename does not replace an existing destination. Same + // platform rule as managed_agents::storage::start_install_log_session. + // + // Rotation itself is best-effort: an AV/EDR or editor holding a + // transient lock (again, chiefly Windows) would otherwise fail + // EVERY append until the lock clears — the frontend deliberately + // swallows sink errors, so records would vanish silently. + // Degrade to an unrotated append; the size cap re-applies once + // rotation succeeds on a later write. + let rotation = (|| -> std::io::Result<()> { + if rotated.exists() { + std::fs::remove_file(&rotated)?; + } + std::fs::rename(path, &rotated) + })(); + if let Err(e) = rotation { + eprintln!("buzz-desktop: perf-log rotation failed, appending unrotated: {e}"); } - std::fs::rename(path, &rotated).map_err(|e| e.to_string())?; } } let mut file = std::fs::OpenOptions::new() @@ -316,6 +326,41 @@ mod tests { std::fs::remove_dir_all(&dir).ok(); } + #[cfg(unix)] + #[test] + fn a_failed_rotation_degrades_to_an_unrotated_append() { + use std::os::unix::fs::PermissionsExt; + let dir = std::env::temp_dir().join(format!( + "perf-log-degrade-{}-{:?}", + std::process::id(), + std::thread::current().id() + )); + std::fs::create_dir_all(&dir).expect("tempdir"); + let path = dir.join("switch-perf.jsonl"); + let rotated = dir.join("switch-perf.jsonl.1"); + std::fs::write(&path, "oversized-live\n").expect("seed live"); + std::fs::write(&rotated, "old-generation\n").expect("seed rotated"); + + // A read-only directory makes remove/rename fail, like a transient + // AV/EDR hold would. The append must degrade to the unrotated file — + // dropping every record until an external lock clears would violate + // the sink's no-silent-loss contract. + std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o555)).expect("lock dir"); + let result = append_line_rotating(&path, "must-survive", 8); + std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o755)).expect("unlock dir"); + result.expect("append must survive a failed rotation"); + + assert_eq!( + std::fs::read_to_string(&path).expect("read live"), + "oversized-live\nmust-survive\n" + ); + assert_eq!( + std::fs::read_to_string(&rotated).expect("read rotated"), + "old-generation\n" + ); + std::fs::remove_dir_all(&dir).ok(); + } + #[test] fn concurrent_boundary_appends_lose_no_line_and_rotate_once() { let dir = std::env::temp_dir().join(format!(