Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions cmux-tui/crates/acpmux/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1303,8 +1303,10 @@ records of a failed turn, and so on the session's `lastTurn`. The error's own me
- `errorKind`: `disk.full` (a write hit a full disk or quota), `disk.low` (`daemon status`:
under `diskLowBytes`, 1 GiB by default, free on acpmux's disk), `peer.unreachable` (the peer
did not answer; the request was not sent), `daemon.restarting` (the daemon is
shutting down), `spawn.failed` (the harness's program or folder is missing) or `agent.exited`
(the agent's process ended mid-turn). A client treats a kind it does not know as a plain error.
shutting down), `spawn.failed` (the harness's program or folder is missing), `agent.exited`
(the agent's process ended mid-turn), `auth.signed_out` (the agent said it is not signed in)
or `auth.expired` (it said its sign-in expired). A connection error is never an `auth.*`
kind. A client treats a kind it does not know as a plain error.
- `recovery`: one sentence with the action, e.g. "The disk is full. Free space on the disk that
holds /home/me/.acpmux: 0.9 GB free of 252 GB. acpmux's chats in /home/me/.acpmux/sessions
use 3.0 GB; `acpmux session stop <session> --purge` removes a chat you no longer need."
Expand Down
2 changes: 1 addition & 1 deletion cmux-tui/crates/acpmux/spec/acpmux-protocol.json
Original file line number Diff line number Diff line change
Expand Up @@ -2141,7 +2141,7 @@
"type": "integer"
},
"errorKind": {
"description": "What kind of failure a failed turn was (`recovery.rs`): `disk.full`, `disk.low`,\n`peer.unreachable`, `daemon.restarting`, `spawn.failed` or `agent.exited`. A client\ntreats a kind it does not know as a plain error. The same key is on a request's error\n`data` and on the `turn_error` and `turn_result` records.",
"description": "What kind of failure a failed turn was (`recovery.rs`): `disk.full`, `disk.low`,\n`peer.unreachable`, `daemon.restarting`, `spawn.failed`, `agent.exited`,\n`auth.signed_out` or `auth.expired`. A client treats a kind it does not know as a plain\nerror. The same key is on a request's error `data` and on the `turn_error` and\n`turn_result` records.",
"type": [
"string",
"null"
Expand Down
40 changes: 39 additions & 1 deletion cmux-tui/crates/acpmux/src/claude_auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,11 +240,36 @@ pub fn is_sign_in_failure(message: &str) -> bool {
|| m.contains("authentication_failed")
}

/// Whether `message` says a credential expired: "OAuth token has expired", "the token is
/// expired", "credential expired", "expired API key". One matcher for every place that tells an
/// expired sign-in from a missing one (a failed auth check, a failed turn), so the two never
/// drift. Only a statement that one expired counts: "invalid token expiry", "the credential
/// expires in 30 days" and a bare "session expired" do not.
pub fn is_expired_credential(message: &str) -> bool {
// A constant pattern; the unit test proves it compiles.
static EXPIRED: std::sync::LazyLock<Option<regex::Regex>> = std::sync::LazyLock::new(|| {
regex::Regex::new(
r"(?i)\b(?:tokens?|credentials?|api keys?|logins?|sign-ins?)(?:\s+(?:has|have|is|are|was|were))?\s+expired\b|\bexpired\s+(?:(?:oauth|access|refresh|auth|authentication|session)\s+)?(?:tokens?|credentials?|api keys?|logins?|sign-ins?)\b",
)
.ok()
});
EXPIRED.as_ref().is_some_and(|re| re.is_match(message))
}

/// Whether [`explain`] rewrites `original`: a sign-in failure it has not explained yet, and not
/// one that says the provider was unreachable ("HTTP 503: OAuth token has expired"), which
/// stays as the agent said it, so its turn is retried and typed as the outage it is.
fn explains(original: &str) -> bool {
is_sign_in_failure(original)
&& !crate::hub::turn_errors::says_unreachable(original)
&& !original.starts_with("Claude is not signed in here")
}

/// The explanation for a Claude agent's sign-in failure, or None when this
/// session is not a Claude agent acpmux started (or the text says another
/// thing). `original` is Claude's own text, kept at the end.
pub fn explain(session_id: &str, original: &str) -> Option<String> {
if !is_sign_in_failure(original) || original.starts_with("Claude is not signed in here") {
if !explains(original) {
return None;
}
let agent_env =
Expand Down Expand Up @@ -340,3 +365,16 @@ fn compose(agent: &Found, terminal: &Found, shell: Option<&str>, original: &str)
out.push_str(&format!(" (Claude said: {})", original.trim()));
out
}

#[cfg(test)]
mod tests {
/// A sign-in failure is explained; one that says the provider was unreachable is not.
#[test]
fn an_outage_that_names_a_token_is_not_explained_as_a_sign_in() {
assert!(super::explains("Not logged in · Please run /login"));
assert!(super::explains("API Error: 401 OAuth token has expired"));
assert!(!super::explains("HTTP 503: OAuth token has expired"));
assert!(!super::explains("connection error: Not logged in"));
assert!(!super::explains("Claude is not signed in here. Not logged in"));
}
}
57 changes: 53 additions & 4 deletions cmux-tui/crates/acpmux/src/harness_health/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const LATEST_TTL_MS: u64 = (6 * 3600 - 600) * 1000;
pub enum Auth {
SignedIn,
SignedOut,
Expired,
Unknown,
}

Expand Down Expand Up @@ -296,6 +297,20 @@ pub fn keychain_sign_in(id: &str, profile: &crate::config::HarnessProfile) -> Op
.then_some("Codex's `cli_auth_credentials_store` does not keep its login in a file")
}

/// A failed auth check is expired only when its own output says a credential expired
/// (`claude_auth::is_expired_credential`, the matcher a failed turn uses too), and unknown
/// when it says the provider had no capacity or the connection dropped: a transport error is
/// never a sign-in state (`turn_errors::says_unreachable`, as `recovery::auth_kind`).
fn auth_failure(output: &str) -> Auth {
if crate::hub::turn_errors::says_unreachable(output) {
Auth::Unknown
} else if crate::claude_auth::is_expired_credential(output) {
Auth::Expired
} else {
Auth::SignedOut
}
}

static SIGN_IN: std::sync::LazyLock<regex::Regex> = std::sync::LazyLock::new(|| {
regex::Regex::new(r"(?i)\b(not (logged|signed) in|log ?in|sign ?in|unauthenticated|authentication required|auth required|unauthorized)\b")
.expect("sign-in pattern") // crash-allow: constant pattern
Expand Down Expand Up @@ -480,15 +495,27 @@ fn local(s: Subject, home: &Path) -> HarnessHealth {
let (auth, auth_detail) = match &s.probe_error {
_ if !s.check.is_empty() && found.is_some() => {
match probe::auth_check(&s.check, &s.env, &s.family) {
Ok(true) => (Auth::SignedIn, Some(format!("`{}` passed", s.check.join(" ")))),
Ok(false) => (Auth::SignedOut, Some(format!("`{}` failed", s.check.join(" ")))),
Ok((true, _)) => (Auth::SignedIn, Some(format!("`{}` passed", s.check.join(" ")))),
Ok((false, output)) => {
let auth = auth_failure(&output);
let detail = match auth {
Auth::Expired => "credential expired".to_owned(),
Auth::Unknown => {
format!("`{}` could not reach its provider", s.check.join(" "))
}
_ => format!("`{}` failed", s.check.join(" ")),
};
(auth, Some(detail))
}
Err(e) => (Auth::Unknown, Some(e)),
}
}
_ if builtin.as_ref().is_some_and(|(auth, _)| *auth != Auth::SignedOut) => {
builtin.unwrap_or((Auth::Unknown, None))
}
Some(e) if SIGN_IN.is_match(e) => (Auth::SignedOut, Some(e.clone())),
Some(e) if SIGN_IN.is_match(e) || auth_failure(e) == Auth::Expired => {
(auth_failure(e), Some(e.clone()))
}
_ => builtin.unwrap_or((Auth::Unknown, None)),
};
HarnessHealth {
Expand All @@ -514,10 +541,32 @@ fn local(s: Subject, home: &Path) -> HarnessHealth {

#[cfg(test)]
mod tests {
use super::codex_login_in_file;
use super::{Auth, auth_failure, codex_login_in_file};
use serde_json::json;
use std::collections::BTreeMap;

#[test]
fn expired_login_probe_is_not_just_signed_out() {
assert_eq!(auth_failure("OAuth token has expired"), Auth::Expired);
assert_eq!(auth_failure("Not logged in"), Auth::SignedOut);
assert_eq!(auth_failure("check failed"), Auth::SignedOut);
assert_eq!(auth_failure("HTTP 503"), Auth::Unknown);
assert_eq!(auth_failure("HTTP 502: authentication_failed"), Auth::Unknown);
assert_eq!(auth_failure("Not logged in? HTTP 503: authentication_failed"), Auth::Unknown);
assert_eq!(auth_failure("token expired? connection refused"), Auth::Unknown);
assert_eq!(auth_failure("Your refresh token has expired; run codex login"), Auth::Expired);
assert_eq!(auth_failure("the MCP session expired"), Auth::SignedOut);
assert_eq!(auth_failure("Provided authentication token is expired."), Auth::Expired);
assert_eq!(auth_failure("credential expired"), Auth::Expired);
// Only a statement that a credential expired: not its expiry field, not a future one.
assert_eq!(auth_failure("HTTP 400: invalid token expiry value"), Auth::SignedOut);
assert_eq!(
auth_failure("Not logged in; the credential expires in 30 days"),
Auth::SignedOut
);
assert_eq!(serde_json::to_value(Auth::Expired).unwrap(), json!("expired"));
}

fn home_with(n: usize, config: Option<&str>) -> (std::path::PathBuf, BTreeMap<String, String>) {
let dir = std::env::temp_dir().join(format!("amx-codex-store-{}-{n}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
Expand Down
4 changes: 2 additions & 2 deletions cmux-tui/crates/acpmux/src/harness_health/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ pub fn auth_check(
argv: &[String],
env: &BTreeMap<String, String>,
family: &str,
) -> Result<bool, String> {
run_for(argv, env, TIMEOUT, Some(family)).map(|(ok, _)| ok)
) -> Result<(bool, String), String> {
run_for(argv, env, TIMEOUT, Some(family))
}

/// `codex login status` with the harness's `env`: Some(true) signed in (exit
Expand Down
2 changes: 1 addition & 1 deletion cmux-tui/crates/acpmux/src/hub/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub(crate) mod auto_resume;
mod btw;
mod exit_reason;
pub(crate) mod subagent_retry;
mod turn_errors;
pub(crate) mod turn_errors;
pub(crate) use btw::SIDE_TAG;
pub(crate) use policy_mode::SetRequest;
mod bookmarks;
Expand Down
44 changes: 36 additions & 8 deletions cmux-tui/crates/acpmux/src/hub/turn_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,23 @@ pub fn is_capacity_error(message: &str) -> bool {
/// Does an agent error mean the connection to the model dropped, not that the prompt or the
/// account was at fault? Limit and sign-in errors are excluded: another account solves those.
pub fn is_transport_error(message: &str) -> bool {
if is_limit_error(message) {
return false;
}
!is_limit_error(message) && says_connection_dropped(message)
}

/// Whether an error says the provider had no capacity, a gateway failed (HTTP 502, 504) or
/// the connection dropped ([`is_capacity_error`], [`says_connection_dropped`]), whatever it
/// also says about a sign-in: such an error is never evidence of a sign-in state.
pub fn says_unreachable(message: &str) -> bool {
let m = message.to_lowercase();
is_capacity_error(message)
|| has_status(&m, "502")
|| has_status(&m, "504")
|| says_connection_dropped(message)
}

/// Whether an agent error's words say the connection to the model dropped, whatever else it
/// says ([`is_transport_error`] without its limit and sign-in exclusion).
pub fn says_connection_dropped(message: &str) -> bool {
let m = message.to_lowercase();
const PHRASES: &[&str] = &[
"stream disconnected",
Expand Down Expand Up @@ -119,11 +133,11 @@ pub fn is_refused_request(message: &str) -> bool {

/// Why an agent's error is worth trying again in a moment, if it is: `capacity`
/// ([`is_capacity_error`]) or `transport` ([`is_transport_error`]). A usage limit, a sign-in
/// failure, a bad request, a refused permission ([`is_refused_request`]) and anything else are
/// not: None.
/// failure (`recovery::auth_kind`: not one that says the provider was unreachable), a bad
/// request, a refused permission ([`is_refused_request`]) and anything else are not: None.
pub fn transient_cause(error: &str) -> Option<&'static str> {
if is_limit_error(error)
|| crate::claude_auth::is_sign_in_failure(error)
|| crate::recovery::auth_kind(error).is_some()
|| is_refused_request(error)
{
return None;
Expand All @@ -149,14 +163,15 @@ pub(super) fn continuation_error<'a>(raw: Option<&'a str>, shown: &'a str) -> &'
/// ([`transient_cause`]), or `agent_exited` (its agent process died mid-turn: "agent process
/// closed"). `raw` is the
/// agent's own error, `shown` the turn's error as recorded (with the dead process's last stderr
/// line, or the sign-in explanation). A limit or sign-in failure in either is never continued:
/// line, or the sign-in explanation). A limit or sign-in failure (an expired credential too,
/// unless the error also says the provider was unreachable) in either is never continued:
/// the fallback profile, or the reconciler that reads `lastTurn`, moves that session. Nor is a
/// refused request ([`is_refused_request`]) in either: an agent that died of one would be
/// refused the same way again.
pub fn resume_cause(raw: &str, shown: &str) -> Option<&'static str> {
if is_limit_error(raw)
|| is_limit_error(shown)
|| crate::claude_auth::is_sign_in_failure(shown)
|| crate::recovery::auth_kind(shown).is_some()
|| is_refused_request(raw)
|| is_refused_request(shown)
{
Expand Down Expand Up @@ -222,6 +237,19 @@ mod tests {
Some("agent_exited")
);
assert_eq!(resume_cause(raw, "agent process closed (claude): You've hit your limit"), None);
// An expired credential is a sign-in failure too (`recovery::auth_kind`: auth.expired).
assert_eq!(resume_cause(raw, "agent process closed (claude): credential expired"), None);
// An unreachable provider is still retried, whatever it says of a credential.
let dropped = "token expired? connection refused";
assert_eq!(resume_cause(dropped, dropped), Some("transport"));
assert_eq!(
resume_cause("HTTP 503: credential expired", "HTTP 503: credential expired"),
Some("capacity")
);
for m in ["HTTP 503: OAuth token has expired", "connection error: OAuth token has expired"]
{
assert!(resume_cause(m, m).is_some(), "{m}");
}
assert_eq!(
resume_cause(raw, "agent process closed (fake): Not logged in · Please run /login"),
None
Expand Down
7 changes: 4 additions & 3 deletions cmux-tui/crates/acpmux/src/protocol/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,9 +397,10 @@ pub struct LastTurn {
/// Why a turn ended without an answer, e.g. `outcome_unknown`.
pub detail: Option<String>,
/// What kind of failure a failed turn was (`recovery.rs`): `disk.full`, `disk.low`,
/// `peer.unreachable`, `daemon.restarting`, `spawn.failed` or `agent.exited`. A client
/// treats a kind it does not know as a plain error. The same key is on a request's error
/// `data` and on the `turn_error` and `turn_result` records.
/// `peer.unreachable`, `daemon.restarting`, `spawn.failed`, `agent.exited`,
/// `auth.signed_out` or `auth.expired`. A client treats a kind it does not know as a plain
/// error. The same key is on a request's error `data` and on the `turn_error` and
/// `turn_result` records.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub error_kind: Option<String>,
/// One sentence saying what to do about the failure.
Expand Down
Loading
Loading