diff --git a/src/openhuman/agent/triage/evaluator.rs b/src/openhuman/agent/triage/evaluator.rs index a96cb3d3b..39c70946b 100644 --- a/src/openhuman/agent/triage/evaluator.rs +++ b/src/openhuman/agent/triage/evaluator.rs @@ -206,10 +206,27 @@ where F: FnOnce() -> Fut, Fut: Future>, { + // Track whether the cloud arm bailed because of user budget so the + // eventual Deferred reason explains *why* we're sitting idle rather + // than the generic "both arms failed" copy. + let mut cloud_budget_exhausted: Option = None; + // ── Cloud arm ────────────────────────────────────────────────── match try_arm(&cloud, envelope, TriageResolutionPath::Cloud).await { Ok(run) => return Ok(TriageOutcome::Decision(run)), Err(ArmError::Fatal(err)) => return Err(err), + Err(ArmError::BudgetExhausted(err)) => { + tracing::warn!( + source = %envelope.source.slug(), + label = %envelope.display_label, + external_id = %envelope.external_id, + path = TriageResolutionPath::Cloud.as_str(), + error = %err, + "[triage::evaluator] cloud rejected for budget; \ + skipping retry and falling back to local arm" + ); + cloud_budget_exhausted = Some(err); + } Err(ArmError::Retryable { retry_after_ms, .. }) => { // Sleep before the cloud retry. Honour Retry-After when // present; otherwise use a short backoff so the second @@ -228,6 +245,18 @@ where match try_arm(&cloud, envelope, TriageResolutionPath::CloudAfterRetry).await { Ok(run) => return Ok(TriageOutcome::Decision(run)), Err(ArmError::Fatal(err)) => return Err(err), + Err(ArmError::BudgetExhausted(err)) => { + tracing::warn!( + source = %envelope.source.slug(), + label = %envelope.display_label, + external_id = %envelope.external_id, + path = TriageResolutionPath::CloudAfterRetry.as_str(), + error = %err, + "[triage::evaluator] cloud rejected for budget on retry; \ + falling back to local arm" + ); + cloud_budget_exhausted = Some(err); + } Err(ArmError::Retryable { .. }) => { // Exhausted cloud budget — fall through to local. tracing::warn!( @@ -244,9 +273,28 @@ where // No local arm available at all (runtime disabled, no model // configured) — the only honest outcome is a deferral so the // next tick retries the whole chain. + // + // `reason` is part of `TriageOutcome::Deferred` and may be + // forwarded into telemetry / UI, so it must stay a stable, + // scrubbed string. Raw upstream error text goes to the debug + // log instead, where it is operator-visible but not surfaced. + let reason = match &cloud_budget_exhausted { + Some(err) => { + tracing::debug!( + target: "[triage::evaluator]", + source = %envelope.source.slug(), + label = %envelope.display_label, + external_id = %envelope.external_id, + error = %err, + "cloud budget exhausted; no local arm — full upstream error" + ); + "cloud budget exhausted; local arm unavailable".to_string() + } + None => "cloud retry exhausted; local arm unavailable".to_string(), + }; return Ok(TriageOutcome::Deferred { defer_until_ms: now_ms().saturating_add(DEFER_WAKEUP_MS), - reason: "cloud retry exhausted; local arm unavailable".to_string(), + reason, }); }; @@ -256,15 +304,34 @@ where match try_arm(&local, envelope, TriageResolutionPath::LocalFallback).await { Ok(run) => Ok(TriageOutcome::Decision(run)), - Err(ArmError::Fatal(err)) | Err(ArmError::Retryable { source: err, .. }) => { + Err(ArmError::Fatal(err)) + | Err(ArmError::BudgetExhausted(err)) + | Err(ArmError::Retryable { source: err, .. }) => { // Local also failed — defer rather than surface a hard // error. Today's "hard fail" is the wrong default for a // transient blocker per #1257. - let reason = format!("cloud + local both failed: {err}"); + // + // `reason` is part of the public Deferred outcome and may + // flow into telemetry / UI, so keep it scrubbed. Raw error + // text from cloud + local lives in the structured warn + // fields below — visible to operators, not callers. + let reason = match cloud_budget_exhausted.as_ref() { + Some(_) => "cloud budget exhausted; local arm also failed".to_string(), + None => "cloud retry exhausted; local arm also failed".to_string(), + }; tracing::warn!( - error = %reason, + target: "[triage::evaluator]", + source = %envelope.source.slug(), + label = %envelope.display_label, + external_id = %envelope.external_id, + local_error = %err, + cloud_error = cloud_budget_exhausted + .as_ref() + .map(|e| e.to_string()) + .unwrap_or_default(), defer_ms = DEFER_WAKEUP_MS, - "[triage::evaluator] both arms failed; deferring" + reason = %reason, + "both arms failed; deferring" ); Ok(TriageOutcome::Deferred { defer_until_ms: now_ms().saturating_add(DEFER_WAKEUP_MS), @@ -287,6 +354,13 @@ enum ArmError { /// Auth failure, missing model, prompt parse error, registry /// missing, etc. — retry / fallback would not change the result. Fatal(anyhow::Error), + /// Cloud upstream rejected the call because the user is out of + /// budget / credits. Retrying the cloud arm would just burn the + /// same wall, but the local arm has no upstream cost — so we + /// skip cloud retry, try local, and defer if local also fails. + /// This is **not** a fatal error: the user takes an explicit + /// action (top up) to fix it, so it must not page Sentry. + BudgetExhausted(anyhow::Error), } /// Run a single arm: dispatch the agent turn through the native bus @@ -438,9 +512,61 @@ fn classify_error(message: String) -> ArmError { source: err, }; } + // Budget-exceeded is technically a 400 (not 5xx/429), so the + // generic transient checks above won't catch it — but it is a + // user-actionable upstream blocker, not a code bug, so we route + // it through `BudgetExhausted` to avoid Sentry pages. + if is_inference_budget_exceeded(&message) { + return ArmError::BudgetExhausted(err); + } ArmError::Fatal(err) } +/// Returns `true` when `message` signals that the upstream rejected the +/// call because the user's inference budget or credit balance is empty — +/// meaning a retry would hit the same wall. +/// +/// The vocabulary matches the OpenHuman backend's error copy and common +/// third-party provider phrasing. It does **not** mirror the +/// *semantics* of `channels/providers/web.rs` (a different code path); +/// it is an independent, conservative allowlist evaluated inline so the +/// triage evaluator carries no cross-domain import. +/// +/// Kept conservative on purpose: a false positive would silently +/// reclassify a real `Fatal` error as `BudgetExhausted`, hiding it from +/// Sentry. +fn is_inference_budget_exceeded(message: &str) -> bool { + // Normalize: lowercase, replace non-alphanumeric with spaces, then + // split into whitespace-separated tokens. This lets us do + // whole-word matching: a raw `contains("top up")` against the + // normalized text would also fire on "stop updating" (which + // contains the substring "top up" across word boundaries). + let normalized: String = message + .trim() + .to_ascii_lowercase() + .chars() + .map(|c| if c.is_ascii_alphanumeric() { c } else { ' ' }) + .collect(); + let words: Vec<&str> = normalized.split_whitespace().collect(); + const NEEDLES: &[&str] = &[ + "budget exceeded", + "budget exceeds", + "top up", + "add credits", + "out of credits", + "no remaining credits", + ]; + NEEDLES.iter().any(|needle| { + let needle_tokens: Vec<&str> = needle.split_whitespace().collect(); + if needle_tokens.is_empty() || words.len() < needle_tokens.len() { + return false; + } + words + .windows(needle_tokens.len()) + .any(|window| window == needle_tokens.as_slice()) + }) +} + /// Heuristic for transient cloud failures the provider stack didn't /// already classify — connection resets, timeouts, generic 5xx text. /// Mirrors the conservative match shape used by `is_upstream_unhealthy`. diff --git a/src/openhuman/agent/triage/evaluator_tests.rs b/src/openhuman/agent/triage/evaluator_tests.rs index 330064a28..f2b54bd2c 100644 --- a/src/openhuman/agent/triage/evaluator_tests.rs +++ b/src/openhuman/agent/triage/evaluator_tests.rs @@ -102,6 +102,75 @@ fn classify_string_treats_auth_failure_as_fatal() { ); } +#[test] +fn classify_string_recognises_budget_exceeded_as_budget_exhausted() { + // Matches the real payload that fired OPENHUMAN-TAURI-X in Sentry. + let err = classify_error( + "OpenHuman API error (400 Bad Request): {\"success\":false,\ + \"error\":\"Budget exceeded — add credits to continue\"}" + .to_string(), + ); + assert!( + matches!(err, ArmError::BudgetExhausted(_)), + "budget-exceeded must classify as BudgetExhausted, not Fatal (which pages Sentry)" + ); +} + +#[test] +fn classify_string_recognises_budget_exceeds_your_limit() { + // Exercises the "budget exceeds" needle added to NEEDLES as a + // grammatically-correct variant of "budget exceeded" (past + // tense vs. present tense) — matches e.g. "Your budget exceeds + // your limit for this billing period." + let err = classify_error("Your budget exceeds your limit for this billing period.".to_string()); + assert!( + matches!(err, ArmError::BudgetExhausted(_)), + "\"budget exceeds\" must classify as BudgetExhausted" + ); +} + +#[test] +fn classify_string_does_not_match_budget_phrases_across_word_boundaries() { + // Regression: a substring-based check would fire BudgetExhausted + // on "stop updating" because the normalized text contains the + // substring "top up" — across the boundary between "stop" and + // "updating". Whole-word (token-window) matching prevents this. + for msg in [ + "please stop updating the row", + "stop updating now", + "topup completed", + ] { + let err = classify_error(msg.to_string()); + assert!( + matches!(err, ArmError::Fatal(_)), + "expected Fatal (no spurious BudgetExhausted) for {msg:?}" + ); + } +} + +#[test] +fn classify_string_recognises_top_up_and_out_of_credits_as_budget_exhausted() { + for msg in [ + "please top up your account", + "you are out of credits, add credits to continue", + "no remaining credits available", + ] { + let err = classify_error(msg.to_string()); + // Match by reference so `err` is only inspected (not moved) — + // lets us reuse it for both the match-check and the failure + // label without a double-move. + let label = match &err { + ArmError::Retryable { .. } => "Retryable", + ArmError::Fatal(_) => "Fatal", + ArmError::BudgetExhausted(_) => "BudgetExhausted", + }; + assert!( + matches!(&err, ArmError::BudgetExhausted(_)), + "expected BudgetExhausted for {msg:?}, got {label}" + ); + } +} + // ── Tiered fallback integration tests ─────────────────────────── // // These drive `run_triage_with_arms` end-to-end through the agent @@ -303,7 +372,7 @@ async fn cloud_then_local_failure_returns_deferred() { "defer_until_ms must be in the future" ); assert!( - reason.to_lowercase().contains("503") || reason.contains("cloud"), + reason.contains("cloud retry exhausted"), "reason should reference the upstream failure: {reason}" ); } @@ -343,6 +412,151 @@ async fn fatal_cloud_error_short_circuits_without_local_attempt() { ); } +#[tokio::test] +async fn cloud_budget_exhausted_skips_retry_and_falls_to_local() { + // Regression for OPENHUMAN-TAURI-X: when the cloud arm returns + // "Budget exceeded — add credits to continue" we must not retry + // the cloud arm (the second call would burn the same wall) and + // we must not surface the error as Fatal (that paged Sentry). + AgentDefinitionRegistry::init_global_builtins().expect("init_global_builtins"); + let counter = StdArc::new(AtomicUsize::new(0)); + let counter_for_stub = StdArc::clone(&counter); + + let _guard = mock_agent_run_turn(move |req| { + let counter = StdArc::clone(&counter_for_stub); + async move { + let n = counter.fetch_add(1, Ordering::SeqCst); + if n == 0 { + assert_eq!(req.provider_name, "stub-cloud", "first call must hit cloud"); + Err("OpenHuman API error (400 Bad Request): \ + {\"success\":false,\"error\":\"Budget exceeded — add credits to continue\"}" + .to_string()) + } else { + // No second cloud call — should jump straight to local. + assert_eq!( + req.provider_name, "stub-local", + "budget-exhausted must skip cloud retry and dispatch to local" + ); + Ok(AgentTurnResponse { + text: VALID_JSON_REPLY.to_string(), + }) + } + } + }) + .await; + + let outcome = run_triage_with_arms_for_test(cloud_arm(), Some(local_arm()), &envelope()) + .await + .expect("budget-exhausted must not surface as Err"); + + let run = outcome.into_decision().expect("decision"); + assert_eq!(run.resolution_path, TriageResolutionPath::LocalFallback); + assert!(run.used_local); + assert_eq!( + counter.load(Ordering::SeqCst), + 2, + "1 cloud (rejected for budget) + 1 local = 2 (no cloud retry)" + ); +} + +#[tokio::test] +async fn cloud_budget_exhausted_on_retry_falls_through_to_local() { + // Variant of OPENHUMAN-TAURI-X: cloud arm trips a transient first + // (so we *do* schedule the cloud retry), but the retry itself + // comes back as Budget exceeded. We must not run a third cloud + // call, and we must fall through to local rather than surface + // the budget error as Fatal. + AgentDefinitionRegistry::init_global_builtins().expect("init_global_builtins"); + let counter = StdArc::new(AtomicUsize::new(0)); + let counter_for_stub = StdArc::clone(&counter); + + let _guard = mock_agent_run_turn(move |req| { + let counter = StdArc::clone(&counter_for_stub); + async move { + let n = counter.fetch_add(1, Ordering::SeqCst); + match n { + 0 => { + assert_eq!(req.provider_name, "stub-cloud", "first call must hit cloud"); + Err("HTTP 503 Service Unavailable".to_string()) + } + 1 => { + assert_eq!( + req.provider_name, "stub-cloud", + "second call must be the cloud retry" + ); + Err("OpenHuman API error (400 Bad Request): \ + {\"success\":false,\"error\":\"Budget exceeded — add credits to continue\"}" + .to_string()) + } + _ => { + // After the retry returned a budget error, we + // must jump straight to local — never a third + // cloud call. + assert_eq!( + req.provider_name, "stub-local", + "post-budget dispatch must land on local" + ); + Ok(AgentTurnResponse { + text: VALID_JSON_REPLY.to_string(), + }) + } + } + } + }) + .await; + + let outcome = run_triage_with_arms_for_test(cloud_arm(), Some(local_arm()), &envelope()) + .await + .expect("budget on retry must not surface as Err"); + + let run = outcome.into_decision().expect("decision"); + assert_eq!(run.resolution_path, TriageResolutionPath::LocalFallback); + assert!(run.used_local); + assert_eq!( + counter.load(Ordering::SeqCst), + 3, + "1 cloud (transient) + 1 cloud retry (budget) + 1 local = 3 (no extra cloud retry)" + ); +} + +#[tokio::test] +async fn cloud_budget_exhausted_without_local_returns_deferred_not_err() { + AgentDefinitionRegistry::init_global_builtins().expect("init_global_builtins"); + let counter = StdArc::new(AtomicUsize::new(0)); + let counter_for_stub = StdArc::clone(&counter); + + let _guard = mock_agent_run_turn(move |_req| { + let counter = StdArc::clone(&counter_for_stub); + async move { + counter.fetch_add(1, Ordering::SeqCst); + Err( + "OpenHuman API error (400 Bad Request): Budget exceeded — add credits to continue" + .to_string(), + ) + } + }) + .await; + + let outcome = run_triage_with_arms_for_test(cloud_arm(), None, &envelope()) + .await + .expect("budget-exhausted with no local must be Deferred, not Err"); + + match outcome { + TriageOutcome::Deferred { reason, .. } => { + assert!( + reason.to_lowercase().contains("budget"), + "deferral reason should name the budget cause: {reason}" + ); + } + TriageOutcome::Decision(_) => panic!("expected Deferred, got Decision"), + } + assert_eq!( + counter.load(Ordering::SeqCst), + 1, + "no retry — budget would block the second cloud call too" + ); +} + #[tokio::test] async fn no_local_arm_returns_deferred_after_cloud_exhaustion() { AgentDefinitionRegistry::init_global_builtins().expect("init_global_builtins");