diff --git a/hive-ag3nt/src/login.rs b/hive-ag3nt/src/login.rs index 33a50a43..363c7102 100644 --- a/hive-ag3nt/src/login.rs +++ b/hive-ag3nt/src/login.rs @@ -2,11 +2,12 @@ //! provided by hive-c0re and persists across container destroy/recreate so //! OAuth tokens survive. //! -//! "Has session" today means "the dir contains at least one regular file." -//! That's a heuristic: a fresh bind-mount starts empty, and `claude auth login` -//! writes credentials into the dir. We may refine later (probe for the -//! specific credentials filename, or run a no-op `claude` call) once the -//! exact layout is locked in. +//! "Has session" means the dir contains at least one of the credential files +//! in [`CRED_FILE_NAMES`] — the same set `/logout` (`web_ui::auth`) deletes to +//! force re-login. Keying both off one constant keeps boot detection and +//! logout in agreement: logout deliberately preserves session-history files, +//! 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::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; @@ -24,20 +25,61 @@ pub fn default_dir() -> PathBuf { crate::paths::claude_dir() } -/// Returns `true` if `dir` exists and contains any regular file. Used at -/// startup to decide whether to enter the turn loop (logged in) or stay in -/// the partial-run "needs login" state. +/// The credential files that constitute a logged-in claude session inside +/// [`default_dir`]. A session exists iff at least one is present; a login +/// "refresh" is a change to one of them. `/logout` (`web_ui::auth`) deletes +/// exactly these to force re-login while preserving session-history files — +/// so boot detection ([`has_session`]) and logout agree by construction. +/// Rationale + the previous wholesale-wipe shape we replaced live in +/// [`docs/web-ui/agent.md::Per-agent endpoints`](../../docs/web-ui/agent.md) +/// (the `/api/logout` bullet). +pub const CRED_FILE_NAMES: &[&str] = &[".credentials.json", "mcp-needs-auth-cache.json"]; + +/// Is `entry` a regular file whose name is one of [`CRED_FILE_NAMES`]? +fn is_cred_file(entry: &std::fs::DirEntry) -> bool { + entry.file_type().is_ok_and(|t| t.is_file()) + && entry + .file_name() + .to_str() + .is_some_and(|n| CRED_FILE_NAMES.contains(&n)) +} + +/// Returns `true` if `dir` exists and holds at least one credential file +/// (see [`CRED_FILE_NAMES`]). Used at startup to decide whether to enter the +/// turn loop (logged in) or stay in the partial-run "needs login" state. #[must_use] pub fn has_session(dir: &Path) -> bool { let Ok(entries) = std::fs::read_dir(dir) else { return false; }; - for entry in entries.flatten() { - if entry.file_type().is_ok_and(|t| t.is_file()) { - return true; + entries.flatten().any(|e| is_cred_file(&e)) +} + +/// Outcome of [`clear_session`]: which credential files were removed and any +/// non-fatal per-file errors (e.g. permission denied). A file that was already +/// absent is not reported — deletion is idempotent. +#[derive(Debug, Default)] +pub struct ClearedSession { + pub wiped: Vec<&'static str>, + pub warnings: Vec, +} + +/// Delete the credential files (see [`CRED_FILE_NAMES`]) from `dir`, forcing a +/// re-login on the next turn, while preserving the session-history files +/// alongside them so `claude --continue` keeps working after a fresh login. +/// Idempotent: an already-absent file is skipped, not reported. This is the +/// write-side counterpart to [`has_session`]; `/logout` (`web_ui::auth`) drives +/// it. +pub async fn clear_session(dir: &Path) -> ClearedSession { + let mut cleared = ClearedSession::default(); + for name in CRED_FILE_NAMES { + match tokio::fs::remove_file(dir.join(name)).await { + Ok(()) => cleared.wiped.push(name), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => cleared.warnings.push(format!("{name}: {e}")), } } - false + cleared } /// Login state the harness reports to its web UI. @@ -105,16 +147,16 @@ pub async fn wait_for_login( } } -/// 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. +/// Snapshot of the credential files (see [`CRED_FILE_NAMES`]) in the dir at a +/// point in time: how many are present + 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 a new credential +/// file still triggers a resume. Defaults to `{0, None}` on `read_dir` failure +/// (missing or unreadable dir) — `wait_for_login` then resumes when a +/// credential file first appears. #[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] struct DirSnapshot { file_count: usize, @@ -127,7 +169,7 @@ fn snapshot_dir(dir: &Path) -> DirSnapshot { }; let mut snap = DirSnapshot::default(); for entry in entries.flatten() { - if !entry.file_type().is_ok_and(|t| t.is_file()) { + if !is_cred_file(&entry) { continue; } snap.file_count += 1; @@ -165,7 +207,28 @@ mod tests { use std::fs; use std::time::{Duration, SystemTime}; - use super::{DirSnapshot, session_refreshed, snapshot_dir}; + use super::{DirSnapshot, has_session, session_refreshed, snapshot_dir}; + + #[test] + fn has_session_only_counts_credential_files() { + let dir = tempfile::tempdir().unwrap(); + // Session-history files (what `/logout` preserves) must NOT read as a + // logged-in session — this is the logout+recreate 401 bug. + fs::write(dir.path().join("history.jsonl"), b"{}").unwrap(); + fs::write(dir.path().join("some-project-uuid.json"), b"{}").unwrap(); + assert!(!has_session(dir.path())); + // A real credential file does. + fs::write(dir.path().join(".credentials.json"), b"{}").unwrap(); + assert!(has_session(dir.path())); + } + + #[test] + fn snapshot_dir_ignores_non_credential_files() { + let dir = tempfile::tempdir().unwrap(); + fs::write(dir.path().join("history.jsonl"), b"{}").unwrap(); + let snap = snapshot_dir(dir.path()); + assert_eq!(snap.file_count, 0, "history files must not count as session"); + } #[test] fn snapshot_dir_empty_dir_is_default() { @@ -191,11 +254,11 @@ mod tests { #[test] fn snapshot_dir_picks_latest_mtime_and_counts_files() { let dir = tempfile::tempdir().unwrap(); - fs::write(dir.path().join("old.json"), b"{}").unwrap(); + fs::write(dir.path().join(".credentials.json"), b"{}").unwrap(); // Sleep so the second file's mtime is strictly greater than // the first on filesystems with low timestamp resolution. std::thread::sleep(Duration::from_millis(20)); - let newer_path = dir.path().join("newer.json"); + let newer_path = dir.path().join("mcp-needs-auth-cache.json"); fs::write(&newer_path, b"{}").unwrap(); let snap = snapshot_dir(dir.path()); assert_eq!(snap.file_count, 2); @@ -204,13 +267,13 @@ mod tests { } #[test] - fn session_refreshed_first_login_flips_on_any_file() { - // Empty-dir snapshot → any file appearing means a fresh + fn session_refreshed_first_login_flips_on_cred_file() { + // Empty-dir snapshot → a credential file appearing means a fresh // login landed. First-time login semantics. let dir = tempfile::tempdir().unwrap(); let snapshot = snapshot_dir(dir.path()); assert!(!session_refreshed(snapshot, snapshot_dir(dir.path()))); - fs::write(dir.path().join("credentials.json"), b"{}").unwrap(); + fs::write(dir.path().join(".credentials.json"), b"{}").unwrap(); assert!(session_refreshed(snapshot, snapshot_dir(dir.path()))); } @@ -220,7 +283,7 @@ mod tests { // must NOT immediately return — it would loop straight into // another 401-failing turn. let dir = tempfile::tempdir().unwrap(); - fs::write(dir.path().join("credentials.json"), b"{}").unwrap(); + fs::write(dir.path().join(".credentials.json"), b"{}").unwrap(); let snapshot = snapshot_dir(dir.path()); assert_eq!(snapshot.file_count, 1); // No change to the file → loop must NOT exit. @@ -233,10 +296,10 @@ mod tests { // flow lands a refreshed credentials file — its mtime bumps // strictly past the snapshot and wait_for_login resumes. let dir = tempfile::tempdir().unwrap(); - fs::write(dir.path().join("credentials.json"), b"{}").unwrap(); + fs::write(dir.path().join(".credentials.json"), b"{}").unwrap(); let snapshot = snapshot_dir(dir.path()); std::thread::sleep(Duration::from_millis(20)); - fs::write(dir.path().join("credentials.json"), b"{\"v\":2}").unwrap(); + fs::write(dir.path().join(".credentials.json"), b"{\"v\":2}").unwrap(); assert!(session_refreshed(snapshot, snapshot_dir(dir.path()))); } @@ -246,7 +309,7 @@ mod tests { // skew between snapshot and probe) must keep waiting until a // 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(); + fs::write(dir.path().join(".credentials.json"), b"{}").unwrap(); let snapshot = DirSnapshot { file_count: 1, newest_mtime: Some(SystemTime::now() + Duration::from_hours(1)), @@ -262,12 +325,12 @@ mod tests { // 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(); + fs::write(dir.path().join(".credentials.json"), b"{}").unwrap(); let forged = DirSnapshot { file_count: 1, newest_mtime: None, }; - fs::write(dir.path().join("b"), b"{}").unwrap(); + fs::write(dir.path().join("mcp-needs-auth-cache.json"), 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()))); diff --git a/hive-ag3nt/src/web_ui/auth.rs b/hive-ag3nt/src/web_ui/auth.rs index f3e175e0..65f63a98 100644 --- a/hive-ag3nt/src/web_ui/auth.rs +++ b/hive-ag3nt/src/web_ui/auth.rs @@ -74,17 +74,9 @@ pub(super) async fn post_login_cancel(State(state): State) -> Response (axum::http::StatusCode::OK, "ok").into_response() } -/// OAuth credential filenames inside `paths::claude_dir()`. Wiping -/// only these (and not the rest of `~/.claude/`) preserves session -/// history so `claude --continue` keeps working after a fresh login. -/// Rationale + the previous wholesale-wipe shape we replaced live in -/// [`docs/web-ui/agent.md::Per-agent endpoints`](../../../docs/web-ui/agent.md) -/// (the `/api/logout` bullet). -const CRED_FILE_NAMES: &[&str] = &[".credentials.json", "mcp-needs-auth-cache.json"]; - /// Operator-driven `/logout`: SIGINT claude, delete the credential -/// files in `CRED_FILE_NAMES`, flip `LoginState::NeedsLogin`. The -/// turn loop's next iteration parks into `wait_for_login` which +/// files (via [`crate::login::clear_session`]), flip `LoginState::NeedsLogin`. +/// The turn loop's next iteration parks into `wait_for_login` which /// resumes when a fresh credentials file appears via `/login/code`. /// Always returns 200 with a body describing what happened. See /// [`docs/web-ui/agent.md::Per-agent endpoints`](../../../docs/web-ui/agent.md) @@ -96,32 +88,19 @@ pub(super) async fn post_logout(State(state): State) -> Response { .args(["-INT", "claude"]) .output() .await; - // Step 2: delete OAuth credential files only — preserve session - // history files alongside them. + // Step 2: delete OAuth credential files only — login::clear_session owns + // the file set and preserves session-history files alongside them. let dir = crate::paths::claude_dir(); - let mut warnings: Vec = Vec::new(); - let mut wiped: Vec<&str> = Vec::new(); - for name in CRED_FILE_NAMES { - let path = dir.join(name); - match tokio::fs::remove_file(&path).await { - Ok(()) => wiped.push(name), - Err(e) if e.kind() == std::io::ErrorKind::NotFound => { - // Already gone — operator clicked /logout while - // already logged out, or the file simply didn't exist - // for this agent. Idempotent. - } - Err(e) => warnings.push(format!("{name}: {e}")), - } - } - let wipe_summary = if wiped.is_empty() { + let cleared = crate::login::clear_session(&dir).await; + let wipe_summary = if cleared.wiped.is_empty() { "no credential files present (already logged out)".to_owned() } else { - format!("wiped {}", wiped.join(", ")) + format!("wiped {}", cleared.wiped.join(", ")) }; - let warn_suffix = if warnings.is_empty() { + let warn_suffix = if cleared.warnings.is_empty() { String::new() } else { - format!(" (warnings: {})", warnings.join("; ")) + format!(" (warnings: {})", cleared.warnings.join("; ")) }; // Step 3: flip LoginState + emit Note. Turn loop sees the flip on // its next iteration and parks into wait_for_login.