fix(logging): never delete .log files held by live processes

src/bootstrap/mod.rs

@@ -25,6 +25,8 @@ use worktree_session::{
     print_error_worktree_info, print_resume_info, resolve_resume_for_cwd,
 };
 
+pub(crate) use discovery::is_alive;
+
 pub async fn run() -> anyhow::Result<()> {
     let cli = Cli::parse();
     let cwd = std::env::current_dir()?;

src/log_writer.rs

@@ -13,11 +13,8 @@ const LOG_PREFIX: &str = "loopal-";
 /// Extension for log file names.
 const LOG_EXT: &str = ".log";
 
-/// A file writer that creates a per-run log file and rotates when it exceeds
-/// [`MAX_FILE_SIZE`].
-///
-/// File naming: `loopal-{YYYYMMDD-HHMMSS}-{pid}.log`
-/// On rotation:  `loopal-{YYYYMMDD-HHMMSS}-{pid}.1.log`, `.2.log`, etc.
+/// File naming: `loopal-{YYYYMMDD-HHMMSS}-{pid}.log`; rotation segments
+/// append `.1`, `.2`, etc. before `.log`.
 pub struct RotatingFileWriter {
     state: WriterState,
 }
@@ -36,8 +33,12 @@ impl RotatingFileWriter {
         let pid = std::process::id();
         let base_stem = format!("{LOG_PREFIX}{}-{pid}", now.format("%Y%m%d-%H%M%S"),);
         let path = log_dir.join(format!("{base_stem}{LOG_EXT}"));
+        // reason: open O_RDWR (not O_WRONLY) so the fd survives a subsequent
+        // `cleanup_old_logs` unlink: even when the path disappears, /dev/fd/N
+        // still resolves and external tools can re-open the inode read-side.
         let file = OpenOptions::new()
             .create(true)
+            .read(true)
             .append(true)
             .open(&path)
             .unwrap_or_else(|_| File::open("/dev/null").expect("/dev/null must be openable"));
@@ -73,7 +74,11 @@ impl Write for RotatingFileWriter {
             s.seq += 1;
             let name = format!("{}.{}{LOG_EXT}", s.base_stem, s.seq);
             let path = s.dir.join(name);
-            s.file = OpenOptions::new().create(true).append(true).open(path)?;
+            s.file = OpenOptions::new()
+                .create(true)
+                .read(true)
+                .append(true)
+                .open(path)?;
             s.written = 0;
         }
         let n = s.file.write(buf)?;
@@ -86,43 +91,108 @@ impl Write for RotatingFileWriter {
     }
 }
 
-/// Remove old log files to keep the directory within retention limits.
-///
-/// Strategy: sort by modification time (oldest first), remove until both
-/// file count ≤ [`MAX_LOG_FILES`] and total size ≤ [`MAX_DIR_SIZE`].
 pub fn cleanup_old_logs(log_dir: &Path) {
+    cleanup_with_alive_filter(log_dir, crate::bootstrap::is_alive);
+}
+
+// reason: a long-lived hub fd's path can be unlinked by a short-lived child's
+// cleanup run, stranding KB of logs in an unreadable write-only fd; never
+// delete a .log whose filename PID is still alive.
+pub(crate) fn cleanup_with_alive_filter(log_dir: &Path, is_alive_fn: impl Fn(u32) -> bool) {
     let Ok(entries) = fs::read_dir(log_dir) else {
         return;
     };
-
-    let mut logs: Vec<(PathBuf, u64, std::time::SystemTime)> = entries
+    let mut logs: Vec<(PathBuf, u64, std::time::SystemTime, bool)> = entries
         .filter_map(|e| e.ok())
         .filter(|e| {
-            let name = e.file_name();
-            let s = name.to_string_lossy();
+            let s = e.file_name().to_string_lossy().into_owned();
             s.starts_with(LOG_PREFIX) && s.ends_with(LOG_EXT)
         })
         .filter_map(|e| {
             let meta = e.metadata().ok()?;
-            Some((e.path(), meta.len(), meta.modified().ok()?))
+            let alive =
+                pid_from_log_filename(&e.file_name().to_string_lossy()).is_some_and(&is_alive_fn);
+            Some((e.path(), meta.len(), meta.modified().ok()?, alive))
         })
         .collect();
+    logs.sort_by_key(|(_, _, t, _)| *t);
+    let (total_count, total_size) = (logs.len(), logs.iter().map(|(_, s, _, _)| s).sum::<u64>());
+    let (mut removed_count, mut removed_size) = (0usize, 0u64);
+    for (path, size, _, alive) in &logs {
+        if total_count - removed_count <= MAX_LOG_FILES && total_size - removed_size <= MAX_DIR_SIZE
+        {
+            break;
+        }
+        if *alive {
+            continue;
+        }
+        let _ = fs::remove_file(path);
+        removed_count += 1;
+        removed_size += size;
+    }
+}
 
-    // Sort oldest first
-    logs.sort_by_key(|(_, _, t)| *t);
+pub(crate) fn pid_from_log_filename(name: &str) -> Option<u32> {
+    let stem = name.strip_prefix(LOG_PREFIX)?.strip_suffix(LOG_EXT)?;
+    let head = stem.rsplit_once('.').map(|(h, _)| h).unwrap_or(stem);
+    let (_, pid) = head.rsplit_once('-')?;
+    pid.parse::<u32>().ok()
+}
 
-    let total_count = logs.len();
-    let total_size: u64 = logs.iter().map(|(_, s, _)| s).sum();
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::io::{Read, Seek, SeekFrom};
 
-    let mut removed_size = 0u64;
+    fn scoped_dir() -> PathBuf {
+        let p = std::env::temp_dir().join(format!("loopal_lw_{}", uuid::Uuid::new_v4()));
+        fs::create_dir_all(&p).unwrap();
+        p
+    }
 
-    for (removed_count, (path, size, _)) in logs.iter().enumerate() {
-        let remaining_count = total_count - removed_count;
-        let remaining_size = total_size - removed_size;
-        if remaining_count <= MAX_LOG_FILES && remaining_size <= MAX_DIR_SIZE {
-            break;
+    #[test]
+    fn pid_from_log_filename_handles_known_shapes() {
+        assert_eq!(
+            pid_from_log_filename("loopal-20260512-105154-45582.log"),
+            Some(45582)
+        );
+        assert_eq!(
+            pid_from_log_filename("loopal-20260512-105154-45582.3.log"),
+            Some(45582)
+        );
+        assert_eq!(pid_from_log_filename("sandbox_test"), None);
+        assert_eq!(pid_from_log_filename("loopal-no-pid.log"), None);
+    }
+
+    #[test]
+    fn cleanup_never_deletes_alive_pid_log() {
+        let dir = scoped_dir();
+        let alive = dir.join("loopal-20260512-100000-1001.log");
+        fs::write(&alive, b"x").unwrap();
+        for i in 0..MAX_LOG_FILES + 2 {
+            fs::write(dir.join(format!("loopal-20260512-110000-{i}.log")), b"x").unwrap();
         }
-        let _ = fs::remove_file(path);
-        removed_size += size;
+        cleanup_with_alive_filter(&dir, |pid| pid == 1001);
+        assert!(alive.exists(), "alive PID's log must survive cleanup");
+        let kept = fs::read_dir(&dir).unwrap().count();
+        assert!(
+            kept <= MAX_LOG_FILES + 1,
+            "expected ≤20 dead + 1 alive, got {kept}"
+        );
+        let _ = fs::remove_dir_all(&dir);
+    }
+
+    #[test]
+    fn new_writer_fd_supports_read() {
+        let dir = scoped_dir();
+        let mut w = RotatingFileWriter::new(&dir);
+        w.write_all(b"abc").unwrap();
+        w.flush().unwrap();
+        let mut clone = w.state.file.try_clone().unwrap();
+        clone.seek(SeekFrom::Start(0)).unwrap();
+        let mut buf = String::new();
+        clone.read_to_string(&mut buf).unwrap();
+        assert_eq!(buf, "abc");
+        let _ = fs::remove_dir_all(&dir);
     }
 }