vinzenz/hyperhive
vinzenz/hyperhive
Watch
0
0
Fork
You've already forked hyperhive
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

hive-agent: guard the unreadable-mtime login fallback against stale re-triggers

Browse source
This commit is contained in:
damocles 2026-08-11 21:00:37 +02:00 • committed by mara
commit 307c335662
1 changed files with 83 additions and 32 deletions
Download patch file Download diff file Expand all files Collapse all files

115
hive-agent/src/login.rs
View file

@ -9,6 +9,7 @@
//! so a "contains any regular file" check would wrongly report `Online` after
//! a logout + container recreate and burn a turn 401-ing before it reroutes.
use std::collections::HashSet;
use std::path::{Path, PathBuf};
use std::sync::{Arc, Mutex};
use std::time::{Duration, SystemTime};
@ -155,6 +156,11 @@ pub async fn wait_for_login(
// small empty file, so re-entering this function after a transient
// operator action is a no-op for the on-disk state.
bus.emit_status("needs_login_idle");
// Baseline for the unreadable-mtime fallback in `has_fresh_credentials`
// — captured once, here, not recomputed per poll. See that fn's doc
// comment for why a *standing* unreadable-mtime file must not
// re-trigger the fallback on every iteration.
let baseline_unreadable = unreadable_mtime_names(claude_dir);
let probe = Duration::from_millis(poll_ms.max(2000));
loop {
tokio::time::sleep(probe).await;
@ -162,7 +168,7 @@ pub async fn wait_for_login(
// (the race this baseline design closes) is caught here on the
// very first iteration — nothing about the check depends on how
// long we've been polling.
if has_fresh_credentials(claude_dir, since) {
if has_fresh_credentials(claude_dir, since, &baseline_unreadable) {
tracing::info!("claude session refreshed — entering turn loop");
*state.lock().unwrap() = LoginState::Online;
bus.emit_status("online");
@ -171,43 +177,80 @@ pub async fn wait_for_login(
}
}
/// Names of credential files (see [`CRED_FILE_NAMES`]) currently present in
/// `dir` whose mtime is *not* readable (`metadata()`/`modified()` errors —
/// exotic fs, NFS quirks). Used once, at [`wait_for_login`]'s entry, as the
/// baseline its unreadable-mtime fallback compares against.
fn unreadable_mtime_names(dir: &Path) -> HashSet<String> {
let Ok(entries) = std::fs::read_dir(dir) else {
return HashSet::new();
};
entries
.flatten()
.filter(is_cred_file)
.filter(|e| e.metadata().and_then(|m| m.modified()).is_err())
.filter_map(|e| e.file_name().to_str().map(str::to_owned))
.collect()
}
/// True if `dir` holds a credential file (see [`CRED_FILE_NAMES`]) whose
/// mtime is strictly newer than `since`, or — fallback — if credential
/// files exist but *none* of their mtimes are readable at all (exotic fs,
/// NFS quirks where `metadata()`/`modified()` errors on every file). That
/// fallback is the same "don't block forever on a signal we can't read"
/// protection an earlier entry-snapshot-diffing design covered with a raw
/// file-count comparison; reframed here as "no readable mtime, but
/// something is there" since there's no entry snapshot to diff against
/// under a fixed-baseline comparison.
fn has_fresh_credentials(dir: &Path, since: SystemTime) -> bool {
/// mtime is strictly newer than `since`, or — fallback — if a credential
/// file's mtime is unreadable at all (exotic fs, NFS quirks) *and its name
/// isn't in `baseline_unreadable`* — i.e. it's a new occurrence since
/// [`wait_for_login`] started polling, not a standing condition.
///
/// The `baseline_unreadable` guard matters: without it, a *stale* file
/// whose mtime happens to be permanently unreadable would satisfy the
/// fallback on every single poll (it's always "present with no readable
/// mtime"), resuming instantly and reintroducing the exact infinite-401
/// loop this whole mechanism exists to prevent. Requiring the name to be
/// new mirrors what an earlier entry-snapshot-diffing design covered with
/// a raw file-count comparison — "something *changed*", not "something
/// *is present*".
fn has_fresh_credentials(
dir: &Path,
since: SystemTime,
baseline_unreadable: &HashSet<String>,
) -> bool {
let Ok(entries) = std::fs::read_dir(dir) else {
return false;
};
let mut any_file = false;
let mut any_readable_mtime = false;
for entry in entries.flatten() {
if !is_cred_file(&entry) {
continue;
}
any_file = true;
let Ok(meta) = entry.metadata() else { continue };
let Ok(mtime) = meta.modified() else { continue };
any_readable_mtime = true;
if mtime > since {
return true;
match meta.modified() {
Ok(mtime) if mtime > since => return true,
Ok(_) => {}
Err(_) => {
let is_new = entry
.file_name()
.to_str()
.is_some_and(|name| !baseline_unreadable.contains(name));
if is_new {
return true;
}
}
}
}
any_file && !any_readable_mtime
false
}
#[cfg(test)]
mod tests {
use std::collections::HashSet;
use std::fs;
use std::time::{Duration, SystemTime};
use super::{NO_PRIOR_FAILURE, has_fresh_credentials, has_session};
/// Test-only shorthand: most cases don't exercise the unreadable-mtime
/// fallback, so they don't care about its baseline.
fn fresh(dir: &std::path::Path, since: SystemTime) -> bool {
has_fresh_credentials(dir, since, &HashSet::new())
}
#[test]
fn has_session_only_counts_credential_files() {
let dir = tempfile::tempdir().unwrap();
@ -226,7 +269,7 @@ mod tests {
let dir = tempfile::tempdir().unwrap();
fs::write(dir.path().join("history.jsonl"), b"{}").unwrap();
assert!(
!has_fresh_credentials(dir.path(), NO_PRIOR_FAILURE),
!fresh(dir.path(), NO_PRIOR_FAILURE),
"history files must not count as a session"
);
}
@ -240,7 +283,7 @@ mod tests {
.unwrap()
.path()
.join("never-created-subdir");
assert!(!has_fresh_credentials(&missing, NO_PRIOR_FAILURE));
assert!(!fresh(&missing, NO_PRIOR_FAILURE));
}
#[test]
@ -249,9 +292,9 @@ mod tests {
// file appearing is a fresh login by construction — its mtime
// postdates the epoch baseline.
let dir = tempfile::tempdir().unwrap();
assert!(!has_fresh_credentials(dir.path(), NO_PRIOR_FAILURE));
assert!(!fresh(dir.path(), NO_PRIOR_FAILURE));
fs::write(dir.path().join(".credentials.json"), b"{}").unwrap();
assert!(has_fresh_credentials(dir.path(), NO_PRIOR_FAILURE));
assert!(fresh(dir.path(), NO_PRIOR_FAILURE));
}
#[test]
@ -262,7 +305,7 @@ mod tests {
let dir = tempfile::tempdir().unwrap();
fs::write(dir.path().join(".credentials.json"), b"{}").unwrap();
let since = SystemTime::now() + Duration::from_secs(1);
assert!(!has_fresh_credentials(dir.path(), since));
assert!(!fresh(dir.path(), since));
}
#[test]
@ -275,7 +318,7 @@ mod tests {
let since = SystemTime::now();
std::thread::sleep(Duration::from_millis(20));
fs::write(dir.path().join(".credentials.json"), b"{}").unwrap();
assert!(has_fresh_credentials(dir.path(), since));
assert!(fresh(dir.path(), since));
}
#[test]
@ -287,7 +330,7 @@ mod tests {
let since = SystemTime::now();
std::thread::sleep(Duration::from_millis(20));
fs::write(dir.path().join(".credentials.json"), b"{\"v\":2}").unwrap();
assert!(has_fresh_credentials(dir.path(), since));
assert!(fresh(dir.path(), since));
}
#[test]
@ -298,13 +341,21 @@ mod tests {
let dir = tempfile::tempdir().unwrap();
fs::write(dir.path().join(".credentials.json"), b"{}").unwrap();
let since = SystemTime::now() + Duration::from_hours(1);
assert!(!has_fresh_credentials(dir.path(), since));
assert!(!fresh(dir.path(), since));
}
}
// The "credential file exists but its mtime is unreadable at all" fallback
// (see `has_fresh_credentials`'s doc comment) isn't covered by a unit test
// here — forging an unreadable `metadata()`/`modified()` call portably
// needs platform-specific setup (a permission-denied file, an exotic fs)
// this test module doesn't have infrastructure for. The branch exists for
// a real production failure mode (NFS quirks), not a hypothetical.
// The unreadable-mtime fallback and its `baseline_unreadable` guard (see
// `has_fresh_credentials`'s doc comment) aren't covered by a unit test here
// — forging an unreadable `metadata()`/`modified()` call portably needs
// platform-specific setup (a permission-denied file, an exotic fs) this
// test module doesn't have infrastructure for, and a normal file's mtime is
// always readable in a test tempdir, so there's no way to reach the `Err`
// arm (or meaningfully exercise `baseline_unreadable`, which only matters
// inside it) without faking that. The branch exists for a real production
// failure mode (NFS quirks), not a hypothetical — a review pass on this
// module caught that the original version of this fallback ignored `since`
// entirely and could re-trigger the infinite-401 loop this file exists to
// prevent; the current shape (only a *new* unreadable-mtime name resumes)
// is reasoned about in the doc comment above since it can't be asserted
// here.