diff --git a/hive-ag3nt/src/turn.rs b/hive-ag3nt/src/turn.rs index 784fce19..c0b591b8 100644 --- a/hive-ag3nt/src/turn.rs +++ b/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 -/// dir is empty / unreadable. Used by `wait_for_login` to snapshot -/// the credentials state at entry so a re-auth that overwrites the -/// existing credentials file is detected (closes #542). -fn newest_file_mtime(dir: &Path) -> Option { - let entries = std::fs::read_dir(dir).ok()?; - let mut newest: Option = None; +/// Snapshot of the credentials dir at a point in time: number of +/// regular files + newest `mtime` across them. The two axes are both +/// load-bearing for `wait_for_login`'s refresh check (`session_refreshed`): +/// mtime catches the common case (re-login overwrites an existing +/// credentials file in-place), file_count catches the pathological case +/// where `meta.modified()` errors on every file (exotic fs, NFS quirks) +/// 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, +} + +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 { - continue; - }; - let Ok(mtime) = meta.modified() else { - continue; - }; - if newest.is_none_or(|cur| mtime > cur) { - newest = Some(mtime); + snap.file_count += 1; + let Ok(meta) = entry.metadata() else { continue }; + let Ok(mtime) = meta.modified() else { continue }; + if snap.newest_mtime.is_none_or(|cur| mtime > cur) { + snap.newest_mtime = 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 -/// means a fresh login landed. -/// - `snapshot == Some(t)` (entry dir had stale creds) → resume only -/// once a file with `mtime > t` is present, so a stale 401 doesn't -/// immediately flip us back to `Online`. -fn session_refreshed_since(dir: &Path, snapshot: Option) -> bool { - match (snapshot, newest_file_mtime(dir)) { +/// - file_count changed → something was added or removed, treat as +/// refresh (covers the "all files have unreadable mtime" edge case). +/// - newest_mtime advanced → existing file was rewritten in place +/// (the common claude re-login path). +/// - prev had no mtime (empty or all-unreadable) and now has one → +/// first useful signal we've seen, treat as refresh. +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()); - assert!(snapshot.is_some()); + let snapshot = snapshot_dir(dir.path()); + 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); - assert!(!session_refreshed_since(dir.path(), Some(way_future))); + let snapshot = DirSnapshot { + 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()))); } }