turn: snapshot file_count alongside mtime to survive unreadable meta.modified()

hive-ag3nt/src/turn.rs

@@ -492,11 +492,11 @@ pub async fn wait_for_login(
         claude_dir = %claude_dir.display(),
         "no claude session — staying in partial-run mode (web UI only)"
     );
-    let snapshot = newest_file_mtime(claude_dir);
+    let snapshot = snapshot_dir(claude_dir);
     let probe = Duration::from_millis(poll_ms.max(2000));
     loop {
         tokio::time::sleep(probe).await;
-        if session_refreshed_since(claude_dir, snapshot) {
+        if session_refreshed(snapshot, snapshot_dir(claude_dir)) {
             tracing::info!("claude session refreshed — entering turn loop");
             *state.lock().unwrap() = LoginState::Online;
             bus.emit_status("online");
@@ -505,42 +505,57 @@ pub async fn wait_for_login(
     }
 }
 
-/// Newest `mtime` across all regular files in `dir`. `None` when the
+/// Snapshot of the credentials dir at a point in time: number of
-/// dir is empty / unreadable. Used by `wait_for_login` to snapshot
+/// regular files + newest `mtime` across them. The two axes are both
-/// the credentials state at entry so a re-auth that overwrites the
+/// load-bearing for `wait_for_login`'s refresh check (`session_refreshed`):
-/// existing credentials file is detected (closes #542).
+/// mtime catches the common case (re-login overwrites an existing
-fn newest_file_mtime(dir: &Path) -> Option<std::time::SystemTime> {
+/// credentials file in-place), file_count catches the pathological case
-    let entries = std::fs::read_dir(dir).ok()?;
+/// where `meta.modified()` errors on every file (exotic fs, NFS quirks)
-    let mut newest: Option<std::time::SystemTime> = None;
+/// so the mtime axis stays `None` forever but new files still trigger a
+/// resume. Defaults to `{0, None}` on read_dir failure (missing or
+/// unreadable dir) — `wait_for_login` then resumes when files first
+/// appear.
+#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)]
+struct DirSnapshot {
+    file_count: usize,
+    newest_mtime: Option<std::time::SystemTime>,
+}
+
+fn snapshot_dir(dir: &Path) -> DirSnapshot {
+    let Ok(entries) = std::fs::read_dir(dir) else {
+        return DirSnapshot::default();
+    };
+    let mut snap = DirSnapshot::default();
     for entry in entries.flatten() {
         if !entry.file_type().is_ok_and(|t| t.is_file()) {
             continue;
         }
-        let Ok(meta) = entry.metadata() else {
+        snap.file_count += 1;
-            continue;
+        let Ok(meta) = entry.metadata() else { continue };
-        };
+        let Ok(mtime) = meta.modified() else { continue };
-        let Ok(mtime) = meta.modified() else {
+        if snap.newest_mtime.is_none_or(|cur| mtime > cur) {
-            continue;
+            snap.newest_mtime = Some(mtime);
-        };
-        if newest.is_none_or(|cur| mtime > cur) {
-            newest = Some(mtime);
         }
     }
-    newest
+    snap
 }
 
-/// Has the credentials dir been written since `snapshot`? Used as the
+/// Has the credentials dir been written since `prev`? Used as the
 /// exit condition for `wait_for_login`:
 ///
-/// - `snapshot == None` (entry dir was empty) → any file appearing
+/// - file_count changed → something was added or removed, treat as
-///   means a fresh login landed.
+///   refresh (covers the "all files have unreadable mtime" edge case).
-/// - `snapshot == Some(t)` (entry dir had stale creds) → resume only
+/// - newest_mtime advanced → existing file was rewritten in place
-///   once a file with `mtime > t` is present, so a stale 401 doesn't
+///   (the common claude re-login path).
-///   immediately flip us back to `Online`.
+/// - prev had no mtime (empty or all-unreadable) and now has one →
-fn session_refreshed_since(dir: &Path, snapshot: Option<std::time::SystemTime>) -> bool {
+///   first useful signal we've seen, treat as refresh.
-    match (snapshot, newest_file_mtime(dir)) {
+fn session_refreshed(prev: DirSnapshot, now: DirSnapshot) -> bool {
+    if now.file_count != prev.file_count {
+        return true;
+    }
+    match (prev.newest_mtime, now.newest_mtime) {
         (None, Some(_)) => true,
-        (Some(prev), Some(now)) => now > prev,
+        (Some(p), Some(n)) => n > p,
         _ => false,
     }
 }
@@ -795,16 +810,18 @@ mod tests {
     use std::fs;
     use std::time::{Duration, SystemTime};
 
-    use super::{newest_file_mtime, session_refreshed_since};
+    use super::{DirSnapshot, session_refreshed, snapshot_dir};
 
     #[test]
-    fn newest_file_mtime_empty_dir_is_none() {
+    fn snapshot_dir_empty_dir_is_default() {
         let dir = tempfile::tempdir().unwrap();
-        assert!(newest_file_mtime(dir.path()).is_none());
+        let snap = snapshot_dir(dir.path());
+        assert_eq!(snap.file_count, 0);
+        assert!(snap.newest_mtime.is_none());
     }
 
     #[test]
-    fn newest_file_mtime_missing_dir_is_none() {
+    fn snapshot_dir_missing_dir_is_default() {
         // Defensive: a nonexistent dir must NOT panic. Bind mounts that
         // disappear mid-poll (host purge during operator intervention)
         // would otherwise crash the harness.
@@ -812,11 +829,12 @@ mod tests {
             .unwrap()
             .path()
             .join("never-created-subdir");
-        assert!(newest_file_mtime(&missing).is_none());
+        let snap = snapshot_dir(&missing);
+        assert_eq!(snap, DirSnapshot::default());
     }
 
     #[test]
-    fn newest_file_mtime_picks_latest_across_multiple_files() {
+    fn snapshot_dir_picks_latest_mtime_and_counts_files() {
         let dir = tempfile::tempdir().unwrap();
         fs::write(dir.path().join("old.json"), b"{}").unwrap();
         // Sleep so the second file's mtime is strictly greater than
@@ -824,19 +842,21 @@ mod tests {
         std::thread::sleep(Duration::from_millis(20));
         let newer_path = dir.path().join("newer.json");
         fs::write(&newer_path, b"{}").unwrap();
-        let newest = newest_file_mtime(dir.path()).expect("must find a file");
+        let snap = snapshot_dir(dir.path());
+        assert_eq!(snap.file_count, 2);
         let newer_meta = fs::metadata(&newer_path).unwrap().modified().unwrap();
-        assert_eq!(newest, newer_meta);
+        assert_eq!(snap.newest_mtime, Some(newer_meta));
     }
 
     #[test]
     fn session_refreshed_first_login_flips_on_any_file() {
-        // Empty-dir snapshot (None) → any file appearing means a fresh
+        // Empty-dir snapshot → any file appearing means a fresh
         // login landed. Pre-#542 semantics for first-time login.
         let dir = tempfile::tempdir().unwrap();
-        assert!(!session_refreshed_since(dir.path(), None));
+        let snapshot = snapshot_dir(dir.path());
+        assert!(!session_refreshed(snapshot, snapshot_dir(dir.path())));
         fs::write(dir.path().join("credentials.json"), b"{}").unwrap();
-        assert!(session_refreshed_since(dir.path(), None));
+        assert!(session_refreshed(snapshot, snapshot_dir(dir.path())));
     }
 
     #[test]
@@ -846,10 +866,10 @@ mod tests {
         // would loop straight into another 401-failing turn.
         let dir = tempfile::tempdir().unwrap();
         fs::write(dir.path().join("credentials.json"), b"{}").unwrap();
-        let snapshot = newest_file_mtime(dir.path());
+        let snapshot = snapshot_dir(dir.path());
-        assert!(snapshot.is_some());
+        assert_eq!(snapshot.file_count, 1);
         // No change to the file → loop must NOT exit.
-        assert!(!session_refreshed_since(dir.path(), snapshot));
+        assert!(!session_refreshed(snapshot, snapshot_dir(dir.path())));
     }
 
     #[test]
@@ -859,10 +879,10 @@ mod tests {
         // strictly past the snapshot and wait_for_login resumes.
         let dir = tempfile::tempdir().unwrap();
         fs::write(dir.path().join("credentials.json"), b"{}").unwrap();
-        let snapshot = newest_file_mtime(dir.path());
+        let snapshot = snapshot_dir(dir.path());
         std::thread::sleep(Duration::from_millis(20));
         fs::write(dir.path().join("credentials.json"), b"{\"v\":2}").unwrap();
-        assert!(session_refreshed_since(dir.path(), snapshot));
+        assert!(session_refreshed(snapshot, snapshot_dir(dir.path())));
     }
 
     #[test]
@@ -872,7 +892,26 @@ mod tests {
         // file's mtime actually exceeds it, not return on first poll.
         let dir = tempfile::tempdir().unwrap();
         fs::write(dir.path().join("credentials.json"), b"{}").unwrap();
-        let way_future = SystemTime::now() + Duration::from_secs(3600);
+        let snapshot = DirSnapshot {
-        assert!(!session_refreshed_since(dir.path(), Some(way_future)));
+            file_count: 1,
+            newest_mtime: Some(SystemTime::now() + Duration::from_secs(3600)),
+        };
+        assert!(!session_refreshed(snapshot, snapshot_dir(dir.path())));
+    }
+
+    #[test]
+    fn session_refreshed_count_change_flips_when_mtime_unreadable() {
+        // Defensive (argus #545 nit): if all files have unreadable
+        // `meta.modified()` (exotic fs / NFS), newest_mtime stays
+        // `None` forever — but file_count axis still catches new
+        // files appearing. Simulated here by forging a snapshot with
+        // file_count=1 + no mtime, then writing a second file.
+        let dir = tempfile::tempdir().unwrap();
+        fs::write(dir.path().join("a"), b"{}").unwrap();
+        let forged = DirSnapshot { file_count: 1, newest_mtime: None };
+        fs::write(dir.path().join("b"), b"{}").unwrap();
+        // Real snapshot has file_count=2, so refresh fires even
+        // though the mtime axis would be inconclusive.
+        assert!(session_refreshed(forged, snapshot_dir(dir.path())));
     }
 }