fix(agent): key session detection off credential filenames; own clear_session in login.rs
This commit is contained in:
parent
d004fdc0eb
commit
d83d03e4c0
2 changed files with 107 additions and 65 deletions
|
|
@ -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<String>,
|
||||
}
|
||||
|
||||
/// 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())));
|
||||
|
|
|
|||
|
|
@ -74,17 +74,9 @@ pub(super) async fn post_login_cancel(State(state): State<AppState>) -> 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<AppState>) -> 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<String> = 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.
|
||||
|
|
|
|||
Loading…
Reference in a new issue