hive-agent: don't claim 'already logged out' when a credential file is stuck undeletable
Closes #2831. post_logout's wipe_summary only checked whether cleared.wiped was empty, so a real .credentials.json blocked by EPERM (nothing made it into wiped) got reported as 'no credential files present (already logged out)' in the same breath as a warning saying the delete failed -- self contradictory and misleading, since the file is still there un-deleted. Extracted the branch into a pure wipe_summary() helper (wiped file(s) named > warnings-only failure > genuinely nothing found) with unit tests covering the exact permission-error shape from the issue.
This commit is contained in:
parent
f457d9ce7b
commit
c5fe61777e
1 changed files with 66 additions and 5 deletions
|
|
@ -89,11 +89,7 @@ pub(super) async fn post_logout(State(state): State<AppState>) -> Response {
|
|||
// the file set and preserves session-history files alongside them.
|
||||
let dir = crate::paths::claude_dir();
|
||||
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 {}", cleared.wiped.join(", "))
|
||||
};
|
||||
let wipe_summary = wipe_summary(&cleared);
|
||||
let warn_suffix = if cleared.warnings.is_empty() {
|
||||
String::new()
|
||||
} else {
|
||||
|
|
@ -115,3 +111,68 @@ pub(super) async fn post_logout(State(state): State<AppState>) -> Response {
|
|||
)
|
||||
.into_response()
|
||||
}
|
||||
|
||||
/// Human-readable summary of a [`crate::login::ClearedSession`] outcome for
|
||||
/// the `/api/logout` response + note. Deliberately distinguishes "nothing to
|
||||
/// delete" from "something's there but we couldn't delete it" — both leave
|
||||
/// `wiped` empty, but only the former is actually "already logged out".
|
||||
/// These two used to be conflated, so a stuck undeletable `.credentials.json`
|
||||
/// got reported as "already logged out" in the same breath as a warning
|
||||
/// saying the delete failed.
|
||||
fn wipe_summary(cleared: &crate::login::ClearedSession) -> String {
|
||||
if !cleared.wiped.is_empty() {
|
||||
format!("wiped {}", cleared.wiped.join(", "))
|
||||
} else if cleared.warnings.is_empty() {
|
||||
"no credential files present (already logged out)".to_owned()
|
||||
} else {
|
||||
"failed to delete existing credential file(s), NOT logged out".to_owned()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::wipe_summary;
|
||||
use crate::login::ClearedSession;
|
||||
|
||||
#[test]
|
||||
fn nothing_found_reports_already_logged_out() {
|
||||
let cleared = ClearedSession::default();
|
||||
assert_eq!(
|
||||
wipe_summary(&cleared),
|
||||
"no credential files present (already logged out)"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn wiped_files_are_named() {
|
||||
let cleared = ClearedSession {
|
||||
wiped: vec![".credentials.json"],
|
||||
warnings: vec![],
|
||||
};
|
||||
assert_eq!(wipe_summary(&cleared), "wiped .credentials.json");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn permission_error_does_not_claim_already_logged_out() {
|
||||
// A real file blocked by EPERM must not be reported the same way
|
||||
// as a genuinely absent one.
|
||||
let cleared = ClearedSession {
|
||||
wiped: vec![],
|
||||
warnings: vec![".credentials.json: Permission denied (os error 13)".to_owned()],
|
||||
};
|
||||
let summary = wipe_summary(&cleared);
|
||||
assert!(!summary.contains("already logged out"));
|
||||
assert!(summary.contains("failed to delete"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn partial_wipe_with_warnings_still_reports_wiped_files() {
|
||||
// A mix of successes and failures should favor telling the operator
|
||||
// what actually happened over the negative "nothing" framing.
|
||||
let cleared = ClearedSession {
|
||||
wiped: vec![".credentials.json"],
|
||||
warnings: vec!["settings.json: Permission denied (os error 13)".to_owned()],
|
||||
};
|
||||
assert_eq!(wipe_summary(&cleared), "wiped .credentials.json");
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue