diff --git a/src/bin/harness_subagent_audit.rs b/src/bin/harness_subagent_audit.rs index 890b6dfbb2..1e48159ac0 100644 --- a/src/bin/harness_subagent_audit.rs +++ b/src/bin/harness_subagent_audit.rs @@ -419,6 +419,7 @@ async fn drain_progress( output_chars, elapsed_ms, iteration, + .. } => { eprintln!( "[harness_subagent_audit] progress turn={} parent_tool_completed tool={} call_id={} success={} output_chars={} elapsed_ms={} iteration={}", diff --git a/src/core/socketio.rs b/src/core/socketio.rs index 31020bf454..1c9203a84a 100644 --- a/src/core/socketio.rs +++ b/src/core/socketio.rs @@ -185,6 +185,12 @@ pub struct WebChannelEvent { /// `tool_result` events. #[serde(skip_serializing_if = "Option::is_none")] pub tool_call_id: Option, + /// Structured, user-facing classification of a failed tool call (class, + /// category, plain-language cause + next action). Present on `tool_result` + /// events when the tool failed; the chat "View processing" timeline renders + /// the "why / what to do next" pair. `None` on success. + #[serde(skip_serializing_if = "Option::is_none")] + pub failure: Option, /// Optional citations attached to `chat_done` payloads. #[serde(skip_serializing_if = "Option::is_none")] pub citations: Option, diff --git a/src/openhuman/agent/harness/session/tool_progress.rs b/src/openhuman/agent/harness/session/tool_progress.rs index 252ecadc70..83861fa091 100644 --- a/src/openhuman/agent/harness/session/tool_progress.rs +++ b/src/openhuman/agent/harness/session/tool_progress.rs @@ -178,6 +178,7 @@ impl ProgressReporter for TurnProgress { output_chars: output.chars().count(), elapsed_ms, iteration, + failure: None, }, ); } diff --git a/src/openhuman/agent/progress.rs b/src/openhuman/agent/progress.rs index 99e47aceb8..ec7b33b2ff 100644 --- a/src/openhuman/agent/progress.rs +++ b/src/openhuman/agent/progress.rs @@ -57,6 +57,11 @@ pub enum AgentProgress { elapsed_ms: u64, /// 1-based iteration index. iteration: u32, + /// Present when `success` is false: a user-facing classification of the + /// failure (class, category, plain-language cause + next action) that + /// the chat "View processing" timeline renders. `None` on success and + /// on legacy snapshots. See `crate::openhuman::tool_status`. + failure: Option, }, /// A sub-agent was spawned during tool execution. diff --git a/src/openhuman/agent/progress_tracing/tests.rs b/src/openhuman/agent/progress_tracing/tests.rs index ecceaad94e..9049da8505 100644 --- a/src/openhuman/agent/progress_tracing/tests.rs +++ b/src/openhuman/agent/progress_tracing/tests.rs @@ -89,6 +89,7 @@ fn tool_completed( output_chars: chars, elapsed_ms: elapsed, iteration: 1, + failure: None, } } diff --git a/src/openhuman/channels/proactive.rs b/src/openhuman/channels/proactive.rs index f8d7c56f8a..596ec9b032 100644 --- a/src/openhuman/channels/proactive.rs +++ b/src/openhuman/channels/proactive.rs @@ -210,6 +210,7 @@ impl EventHandler for ProactiveMessageSubscriber { delta: None, delta_kind: None, tool_call_id: None, + failure: None, citations: None, subagent: None, task_board: None, diff --git a/src/openhuman/channels/providers/presentation.rs b/src/openhuman/channels/providers/presentation.rs index beb6c9a1dc..d4ee9bd154 100644 --- a/src/openhuman/channels/providers/presentation.rs +++ b/src/openhuman/channels/providers/presentation.rs @@ -99,6 +99,7 @@ pub(crate) async fn deliver_response( delta: None, delta_kind: None, tool_call_id: None, + failure: None, subagent: None, task_board: None, tool_display_label: None, @@ -148,6 +149,7 @@ pub(crate) async fn deliver_response( delta: None, delta_kind: None, tool_call_id: None, + failure: None, subagent: None, task_board: None, tool_display_label: None, @@ -188,6 +190,7 @@ pub(crate) async fn deliver_response( delta: None, delta_kind: None, tool_call_id: None, + failure: None, subagent: None, task_board: None, tool_display_label: None, diff --git a/src/openhuman/channels/providers/web/progress_bridge.rs b/src/openhuman/channels/providers/web/progress_bridge.rs index 7c3baf5dca..7ba8e21eb5 100644 --- a/src/openhuman/channels/providers/web/progress_bridge.rs +++ b/src/openhuman/channels/providers/web/progress_bridge.rs @@ -410,7 +410,10 @@ pub(crate) fn spawn_progress_bridge( output_chars, elapsed_ms, iteration, + failure, } => { + // Serialize the classified failure (if any) for the UI + ledger. + let failure_json = failure.as_ref().and_then(|f| serde_json::to_value(f).ok()); ledger_append_event( &config, RunEventAppend { @@ -422,7 +425,8 @@ pub(crate) fn spawn_progress_bridge( "success": success, "outputChars": output_chars, "elapsedMs": elapsed_ms, - "iteration": iteration + "iteration": iteration, + "failure": failure_json, }), }, ); @@ -440,6 +444,7 @@ pub(crate) fn spawn_progress_bridge( success: Some(success), round: Some(iteration), tool_call_id: Some(call_id), + failure: failure_json, ..Default::default() }); } diff --git a/src/openhuman/mod.rs b/src/openhuman/mod.rs index 5daa41d896..983f2d9fe8 100644 --- a/src/openhuman/mod.rs +++ b/src/openhuman/mod.rs @@ -130,6 +130,7 @@ pub mod tls; pub mod todos; pub mod tokenjuice; pub mod tool_registry; +pub mod tool_status; pub mod tool_timeout; pub mod tools; pub mod update; diff --git a/src/openhuman/threads/turn_state/mirror_tests.rs b/src/openhuman/threads/turn_state/mirror_tests.rs index 55bf22cbdb..363e9b3225 100644 --- a/src/openhuman/threads/turn_state/mirror_tests.rs +++ b/src/openhuman/threads/turn_state/mirror_tests.rs @@ -130,6 +130,7 @@ fn tool_call_start_and_complete_track_timeline() { output_chars: 12, elapsed_ms: 50, iteration: 1, + failure: None, }); let s = m.snapshot(); assert_eq!(s.tool_timeline[0].status, ToolTimelineStatus::Success); @@ -203,6 +204,7 @@ fn tool_call_started_reuses_args_delta_placeholder_for_same_call_id() { output_chars: 1, elapsed_ms: 5, iteration: 1, + failure: None, }); assert_eq!(m.snapshot().tool_timeline.len(), 1); assert_eq!( diff --git a/src/openhuman/tinyagents/middleware.rs b/src/openhuman/tinyagents/middleware.rs index 05874fc25d..2568a834eb 100644 --- a/src/openhuman/tinyagents/middleware.rs +++ b/src/openhuman/tinyagents/middleware.rs @@ -1360,11 +1360,18 @@ impl ToolMiddleware<()> for ToolPolicyMiddleware { /// chain so it records the final (summarized/capped) content the transcript keeps. pub(crate) struct ToolOutcomeCaptureMiddleware { sink: super::ToolOutcomeSink, + /// `call_id → (success, classified failure)` side-channel read by the event + /// bridge when projecting `ToolCallCompleted` (the crate event lacks the + /// success/error the failure UI needs). + failure_map: super::observability::ToolFailureMap, } impl ToolOutcomeCaptureMiddleware { - pub(crate) fn new(sink: super::ToolOutcomeSink) -> Self { - Self { sink } + pub(crate) fn new( + sink: super::ToolOutcomeSink, + failure_map: super::observability::ToolFailureMap, + ) -> Self { + Self { sink, failure_map } } } @@ -1380,11 +1387,35 @@ impl Middleware<()> for ToolOutcomeCaptureMiddleware { _state: &(), result: &mut TaToolResult, ) -> TaResult<()> { + let success = result.error.is_none(); + // Classify the failure so the live `ToolCallCompleted` event and the + // persisted timeline can explain it in plain language. A hard + // policy/permission denial is its own class; otherwise heuristics over + // the error text (`timed_out` detected from the timeout branch's phrase). + let failure = if success { + None + } else { + let text = result.error.as_deref().unwrap_or(result.content.as_str()); + if result + .content + .contains(crate::openhuman::security::POLICY_BLOCKED_MARKER) + { + Some(crate::openhuman::tool_status::describe( + crate::openhuman::tool_status::ToolFailureClass::BlockedByPolicy, + )) + } else { + let timed_out = result.content.contains("timed out"); + Some(crate::openhuman::tool_status::classify(text, timed_out)) + } + }; + if let Ok(mut map) = self.failure_map.lock() { + map.insert(result.call_id.clone(), (success, failure)); + } if let Ok(mut sink) = self.sink.lock() { sink.push(super::ToolCallOutcome { call_id: result.call_id.clone(), name: result.name.clone(), - success: result.error.is_none(), + success, content: result.content.clone(), }); } diff --git a/src/openhuman/tinyagents/mod.rs b/src/openhuman/tinyagents/mod.rs index 4f0fed46c0..f5e6c79814 100644 --- a/src/openhuman/tinyagents/mod.rs +++ b/src/openhuman/tinyagents/mod.rs @@ -71,7 +71,9 @@ pub(crate) use embeddings::ProviderEmbeddingModel; pub(crate) use middleware::{HandoffConfig, SuperContextConfig, TurnContextMiddleware}; use model::ProviderModel; pub(crate) use observability::SubagentScope; -use observability::{CapPauser, IterationCursor, OpenhumanEventBridge, ToolNameMap}; +use observability::{ + CapPauser, IterationCursor, OpenhumanEventBridge, ToolFailureMap, ToolNameMap, +}; pub(crate) use run_cancellation_context::{current_run_cancellation, with_run_cancellation}; #[cfg(test)] use tools::ToolAdapter; @@ -396,6 +398,7 @@ pub(crate) async fn run_turn_via_tinyagents_shared( harness, cursor, tool_names, + failure_map, error_slot, halt_summary, tool_outcome_sink, @@ -540,6 +543,7 @@ pub(crate) async fn run_turn_via_tinyagents_shared( subagent_scope.clone(), cursor.clone(), tool_names.clone(), + failure_map.clone(), ); events.subscribe(bridge.clone()); Some(bridge) @@ -858,6 +862,10 @@ struct AssembledTurnHarness { /// writes it on tool-call start; the event bridge reads it to label the /// tool-argument fragments it now projects off the crate stream. tool_names: ToolNameMap, + /// Shared `call_id → (success, failure)` side-channel: the tool-outcome + /// capture middleware classifies each outcome; the event bridge reads it to + /// project real success + a user-facing failure onto `ToolCallCompleted`. + failure_map: ToolFailureMap, /// Recovers the original (downcastable) provider error on run failure. error_slot: crate::openhuman::tinyagents::model::ProviderErrorSlot, /// Root-cause summary recorded by the repeated-tool-failure breaker. @@ -1352,8 +1360,10 @@ fn assemble_turn_harness( // result into a `Message::tool` that drops the failure flag, so the turn can // build honest per-call `ToolCallRecord`s (post-turn hooks + cap checkpoint). let tool_outcome_sink: ToolOutcomeSink = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let failure_map: ToolFailureMap = Arc::default(); harness.push_middleware(Arc::new(middleware::ToolOutcomeCaptureMiddleware::new( tool_outcome_sink.clone(), + failure_map.clone(), ))); // Builder-configured tool policy (`.tool_policy()`), enforced at the tool @@ -1382,6 +1392,7 @@ fn assemble_turn_harness( harness, cursor, tool_names, + failure_map, error_slot, halt_summary, tool_outcome_sink, diff --git a/src/openhuman/tinyagents/observability.rs b/src/openhuman/tinyagents/observability.rs index 212d72ce0d..20651274d1 100644 --- a/src/openhuman/tinyagents/observability.rs +++ b/src/openhuman/tinyagents/observability.rs @@ -49,6 +49,25 @@ pub(crate) type IterationCursor = Arc; /// `tool_name` contract without the forwarder emitting those fragments itself. pub(crate) type ToolNameMap = Arc>>; +/// Shared `call_id → (success, classified failure)` side-channel. The crate's +/// `AgentEvent::ToolCompleted` carries only `call_id` + `tool_name` (no +/// success/error), so `ToolOutcomeCaptureMiddleware::after_tool` — which does +/// see the `ToolResult` — classifies each outcome and writes it here; the bridge +/// reads it when projecting the live `ToolCallCompleted` event, so a failed tool +/// surfaces real `success: false` + a user-facing `failure`. Absent entry (event +/// projected before the middleware ran) falls back to `(true, None)`. +pub(crate) type ToolFailureMap = Arc< + Mutex< + std::collections::HashMap< + String, + ( + bool, + Option, + ), + >, + >, +>; + /// An [`EventListener`] that pauses the run once `cap` model calls have /// completed, so the loop stops gracefully at the iteration budget (returning /// the partial transcript) instead of erroring with `LimitExceeded`. The harness @@ -120,6 +139,9 @@ pub(crate) struct OpenhumanEventBridge { /// `ThinkingForwarder` on tool-call start; read here to label the /// incremental tool-argument fragments projected off the crate stream. tool_names: ToolNameMap, + /// Shared `call_id → (success, failure)` side-channel written by + /// `ToolOutcomeCaptureMiddleware`; read when projecting `ToolCallCompleted`. + failure_map: ToolFailureMap, state: Mutex, } @@ -137,6 +159,7 @@ impl OpenhumanEventBridge { None, Arc::default(), Arc::default(), + Arc::default(), ) } @@ -150,6 +173,7 @@ impl OpenhumanEventBridge { scope: Option, cursor: IterationCursor, tool_names: ToolNameMap, + failure_map: ToolFailureMap, ) -> Arc { Arc::new(Self { on_progress, @@ -158,6 +182,7 @@ impl OpenhumanEventBridge { scope, cursor, tool_names, + failure_map, state: Mutex::new(BridgeState::default()), }) } @@ -489,21 +514,35 @@ impl EventListener for OpenhumanEventBridge { } AgentEvent::ToolCompleted { call_id, tool_name } => { let iteration = self.iteration(); + // The crate event carries no success/error, so read what the + // outcome-capture middleware classified for this call. Absent → + // the event was projected before the middleware ran; assume + // success (never worse than the previous hardcoded `true`). + let outcome = self + .failure_map + .lock() + .ok() + .and_then(|mut m| m.remove(call_id.as_str())); + let success = outcome.as_ref().map(|(ok, _)| *ok).unwrap_or(true); match &self.scope { - None => self.send(AgentProgress::ToolCallCompleted { - call_id: call_id.as_str().to_string(), - tool_name: tool_name.clone(), - success: true, - output_chars: 0, - elapsed_ms: 0, - iteration, - }), + None => { + let failure = outcome.and_then(|(_, f)| f); + self.send(AgentProgress::ToolCallCompleted { + call_id: call_id.as_str().to_string(), + tool_name: tool_name.clone(), + success, + output_chars: 0, + elapsed_ms: 0, + iteration, + failure, + }) + } Some(s) => self.send(AgentProgress::SubagentToolCallCompleted { agent_id: s.agent_id.clone(), task_id: s.task_id.clone(), call_id: call_id.as_str().to_string(), tool_name: tool_name.clone(), - success: true, + success, output_chars: 0, output: String::new(), elapsed_ms: 0, diff --git a/src/openhuman/tool_status/mod.rs b/src/openhuman/tool_status/mod.rs new file mode 100644 index 0000000000..58908e8474 --- /dev/null +++ b/src/openhuman/tool_status/mod.rs @@ -0,0 +1,20 @@ +//! Tool-call lifecycle state and human-readable failure classification. +//! +//! Foundation for the "Visible tool status, failure diagnosis, and safe +//! recovery flows" epic (#4254). This module owns the shared vocabulary that +//! later phases (status panel, bounded retry, in-app diagnostics) build on: +//! +//! - [`ToolLifecycleState`] — where a call is (queued/running/…/needs-input). +//! - [`ToolFailureClass`] / [`FailureCategory`] — what kind of failure it was. +//! - [`ClassifiedFailure`] — a class plus plain-language cause + next action. +//! - [`classify`] — the pure heuristic mapping raw tool error text → the above. +//! +//! It owns no agent tools, no persistence, and no event subscribers; it is a +//! pure data + logic module consumed by the agent tool executor and surfaced +//! over the `tool` event-bus domain. + +mod ops; +mod types; + +pub use ops::{classify, describe}; +pub use types::{ClassifiedFailure, FailureCategory, ToolFailureClass, ToolLifecycleState}; diff --git a/src/openhuman/tool_status/ops.rs b/src/openhuman/tool_status/ops.rs new file mode 100644 index 0000000000..1787f7128d --- /dev/null +++ b/src/openhuman/tool_status/ops.rs @@ -0,0 +1,430 @@ +//! Pure failure classification: raw tool error text → [`ClassifiedFailure`]. +//! +//! This is deliberately a heuristic, keyword-driven mapping over the error +//! string the executor already produces (`agent_tool_exec` collapses a tool +//! outcome to a message + `success` flag). It touches no global state and does +//! no I/O, so every branch is unit-testable and stays cheap to call on the hot +//! tool-execution path. +//! +//! Precedence matters: the first matching class wins, so the checks are ordered +//! most-specific → least-specific. Anything unmatched falls through to +//! [`ToolFailureClass::Unknown`] (treated as recoverable so a later retry phase +//! can give it one bounded attempt rather than surfacing a dead end). + +use super::types::{ClassifiedFailure, FailureCategory, ToolFailureClass}; + +/// Classify a failed tool call into a user-facing [`ClassifiedFailure`]. +/// +/// * `error_text` — the raw error/output message from the tool. Matched +/// case-insensitively; never surfaced verbatim to the user. +/// * `timed_out` — set by the executor when the failure was a deadline stop +/// (the message alone is not always reliable), which short-circuits to +/// [`ToolFailureClass::Timeout`]. +pub fn classify(error_text: &str, timed_out: bool) -> ClassifiedFailure { + let class = classify_class(error_text, timed_out); + describe(class) +} + +/// The class-only half of [`classify`], split out so the ordering heuristics can +/// be tested independently of the copy. +fn classify_class(error_text: &str, timed_out: bool) -> ToolFailureClass { + let text = error_text.to_lowercase(); + + // 1. Timeout — the executor's explicit signal wins over any text sniffing. + if timed_out || contains_any(&text, &["timed out", "timeout", "deadline exceeded"]) { + return ToolFailureClass::Timeout; + } + + // 2. Blocked by policy — the OpenHuman security/autonomy gate or a forbidden + // path. Checked *before* credentials so the OpenHuman-specific + // `forbidden path` marker wins over the bare `forbidden` that a plain + // external 403 body carries (routed to credentials below). Reserved for + // OpenHuman policy phrasing only — a hard policy block is normally tagged + // upstream with `POLICY_BLOCKED_MARKER` and never reaches this heuristic; + // bare HTTP `403`/`Forbidden` is an external authz failure, not our gate. + // `channel allows` is the tail of the tool-policy PermissionDenied render. + if contains_any( + &text, + &[ + "blocked by policy", + "security policy", + "channel allows", + "not allowed by", + "forbidden path", + "autonomy", + "policy denied", + ], + ) { + return ToolFailureClass::BlockedByPolicy; + } + + // 3. Bad credentials — auth-token problems and external authz failures + // (401/403). A bare HTTP 403/Forbidden or an `insufficient scopes` body + // means the connected account lacks the grant, so the user should + // reconnect / re-authorize — not toggle OpenHuman's Agent-access policy. + // Numeric codes go through `contains_code` so `401`/`403` never match + // inside a longer digit run (a port, byte count, or `14033`). + if contains_any( + &text, + &[ + "unauthorized", + "invalid api key", + "invalid_api_key", + "authentication failed", + "invalid credentials", + "bad credentials", + "token expired", + "invalid_grant", + "not signed in", + "sign in again", + "forbidden", + "insufficient authentication scopes", + "insufficient scopes", + "insufficient_scope", + ], + ) || contains_code(&text, "401") + || contains_code(&text, "403") + { + return ToolFailureClass::BadCredentials; + } + + // 4. Missing OS/tool permission — access denied at the filesystem/OS layer. + if contains_any( + &text, + &[ + "permission denied", + "os error 13", + "eacces", + "operation not permitted", + "access is denied", + "not permitted", + ], + ) { + return ToolFailureClass::MissingPermission; + } + + // 5. Missing app/command — the thing we tried to invoke isn't there. + if contains_any( + &text, + &[ + "command not found", + "not installed", + "no such application", + "could not find application", + "executable not found", + "is not recognized as", + "no such file or directory (os error 2)", + ], + ) { + return ToolFailureClass::MissingApp; + } + + // 6. Model / provider connectivity — before generic service errors so a + // provider outage is named specifically. + if contains_any( + &text, + &[ + "provider error", + "could not reach the model", + "could not reach model", + "ollama", + "llm provider", + "inference failed", + "model endpoint", + "no route to host", + ], + ) { + return ToolFailureClass::ModelConnection; + } + + // 7. Service unavailable — generic transient upstream/network failure. + if contains_any( + &text, + &[ + "connection refused", + "econnrefused", + "service unavailable", + "temporarily unavailable", + "could not connect", + "connection reset", + "network is unreachable", + ], + ) || contains_code(&text, "502") + || contains_code(&text, "503") + || contains_code(&text, "504") + { + return ToolFailureClass::ServiceUnavailable; + } + + ToolFailureClass::Unknown +} + +/// Attach the category + plain-language copy for a known class. Use this at the +/// call sites that already know the class for certain (e.g. the policy gate, +/// which knows a refusal is [`ToolFailureClass::BlockedByPolicy`]) rather than +/// round-tripping a synthetic message through [`classify`]. The copy is the +/// user-facing English source string; the UI localizes by class, so keep these +/// stable and jargon-free. +pub fn describe(class: ToolFailureClass) -> ClassifiedFailure { + let category = class.category(); + let (cause_plain, next_action) = match class { + ToolFailureClass::MissingPermission => ( + "OpenHuman doesn't have permission to do this yet.", + "Grant the permission it needs, then try again.", + ), + ToolFailureClass::MissingApp => ( + "The app or program needed for this action isn't available.", + "Install or open the app, then try again.", + ), + ToolFailureClass::ServiceUnavailable => ( + "A service OpenHuman needs is temporarily unavailable.", + "OpenHuman will try again shortly — no action needed.", + ), + ToolFailureClass::BadCredentials => ( + "The saved sign-in details are missing or no longer valid.", + "Sign in again or update the credentials, then try again.", + ), + ToolFailureClass::BlockedByPolicy => ( + "This action is blocked by your safety settings.", + "Allow it in Settings → Agent access if you want it to run.", + ), + ToolFailureClass::ModelConnection => ( + "OpenHuman couldn't reach the AI model.", + "Check your connection or model settings; OpenHuman will retry.", + ), + ToolFailureClass::Timeout => ( + "The action took too long and was stopped.", + "OpenHuman will try again, or you can retry it manually.", + ), + ToolFailureClass::Unknown => ( + "Something went wrong with this action.", + "Try again; if it keeps failing, run diagnostics from Settings.", + ), + }; + ClassifiedFailure { + class, + category, + cause_plain: cause_plain.to_string(), + next_action: next_action.to_string(), + recoverable: category.is_recoverable(), + } +} + +/// Case-insensitive: does `haystack` (already lowercased) contain any needle? +fn contains_any(haystack: &str, needles: &[&str]) -> bool { + needles.iter().any(|n| haystack.contains(n)) +} + +/// Does `haystack` contain `code` (an all-ASCII-digit HTTP status like `"403"`) +/// as a standalone number — i.e. not embedded in a longer digit run? Guards the +/// numeric needles against false positives such as `403` inside `14033`, a port, +/// a byte count, or a timestamp. Matches when the char on each side of the hit +/// is a non-digit (or a string boundary). +fn contains_code(haystack: &str, code: &str) -> bool { + let bytes = haystack.as_bytes(); + let mut from = 0; + while let Some(rel) = haystack[from..].find(code) { + let start = from + rel; + let end = start + code.len(); + let left_ok = start == 0 || !bytes[start - 1].is_ascii_digit(); + let right_ok = end >= bytes.len() || !bytes[end].is_ascii_digit(); + if left_ok && right_ok { + return true; + } + from = start + 1; + } + false +} + +#[cfg(test)] +mod tests { + use super::*; + + fn class_of(text: &str) -> ToolFailureClass { + classify(text, false).class + } + + #[test] + fn timeout_flag_wins_regardless_of_text() { + assert_eq!( + classify("anything at all", true).class, + ToolFailureClass::Timeout + ); + } + + #[test] + fn timeout_detected_from_text() { + assert_eq!( + class_of("tool 'shell' timed out after 120 seconds"), + ToolFailureClass::Timeout + ); + } + + #[test] + fn missing_permission_from_os_error() { + assert_eq!( + class_of("Error executing file_write: Permission denied (os error 13)"), + ToolFailureClass::MissingPermission + ); + assert_eq!( + class_of("EACCES: operation not permitted"), + ToolFailureClass::MissingPermission + ); + } + + #[test] + fn missing_app_from_command_not_found() { + assert_eq!( + class_of("bash: gh: command not found"), + ToolFailureClass::MissingApp + ); + assert_eq!( + class_of("ffmpeg is not installed on this system"), + ToolFailureClass::MissingApp + ); + } + + #[test] + fn service_unavailable_from_connection_errors() { + assert_eq!( + class_of("connection refused (ECONNREFUSED)"), + ToolFailureClass::ServiceUnavailable + ); + assert_eq!( + class_of("upstream returned 503 Service Unavailable"), + ToolFailureClass::ServiceUnavailable + ); + } + + #[test] + fn bad_credentials_from_auth_errors() { + assert_eq!( + class_of("HTTP 401 Unauthorized"), + ToolFailureClass::BadCredentials + ); + assert_eq!( + class_of("invalid api key provided"), + ToolFailureClass::BadCredentials + ); + assert_eq!( + class_of("auth token expired, please sign in again"), + ToolFailureClass::BadCredentials + ); + } + + #[test] + fn blocked_by_policy_from_gate_and_forbidden() { + assert_eq!( + class_of( + "Permission denied for tool 'shell': requires Execute, channel allows ReadOnly" + ), + ToolFailureClass::BlockedByPolicy + ); + assert_eq!( + class_of("blocked by policy: destructive command"), + ToolFailureClass::BlockedByPolicy + ); + // OpenHuman's own path guard stays policy... + assert_eq!( + class_of("write rejected: forbidden path outside action_dir"), + ToolFailureClass::BlockedByPolicy + ); + } + + #[test] + fn external_403_is_credentials_not_policy() { + // A bare external authz failure must route to credentials (reconnect / + // grant scopes), NOT OpenHuman's Agent-access policy. + assert_eq!( + class_of("HTTP 403 Forbidden"), + ToolFailureClass::BadCredentials + ); + assert_eq!( + class_of("Gmail API error: 403 insufficient authentication scopes"), + ToolFailureClass::BadCredentials + ); + assert_eq!( + class_of("401 Unauthorized"), + ToolFailureClass::BadCredentials + ); + } + + #[test] + fn numeric_status_codes_need_word_boundaries() { + // `403`/`503` embedded in a longer digit run must NOT trip the code + // needles — these fall through to Unknown. + assert_eq!( + class_of("processed 14033 records before aborting"), + ToolFailureClass::Unknown + ); + assert_eq!( + class_of("listening on port 15032 failed unexpectedly"), + ToolFailureClass::Unknown + ); + // ...but a standalone 503 is still a service outage. + assert_eq!( + class_of("upstream returned 503"), + ToolFailureClass::ServiceUnavailable + ); + } + + #[test] + fn model_connection_from_provider_errors() { + assert_eq!( + class_of("Provider error (retryable=true): boom"), + ToolFailureClass::ModelConnection + ); + assert_eq!( + class_of("could not reach the model endpoint"), + ToolFailureClass::ModelConnection + ); + assert_eq!( + class_of("ollama daemon not responding"), + ToolFailureClass::ModelConnection + ); + } + + #[test] + fn unknown_when_nothing_matches() { + assert_eq!( + class_of("some totally novel failure mode"), + ToolFailureClass::Unknown + ); + } + + #[test] + fn credentials_precedence_over_service_when_both_present() { + // A 401 that also mentions a connection should read as credentials, not + // a transient service blip — the ordering guarantees this. + assert_eq!( + class_of("could not connect: 401 unauthorized"), + ToolFailureClass::BadCredentials + ); + } + + #[test] + fn every_class_produces_nonempty_user_copy() { + for class in [ + ToolFailureClass::MissingPermission, + ToolFailureClass::MissingApp, + ToolFailureClass::ServiceUnavailable, + ToolFailureClass::BadCredentials, + ToolFailureClass::BlockedByPolicy, + ToolFailureClass::ModelConnection, + ToolFailureClass::Timeout, + ToolFailureClass::Unknown, + ] { + let f = describe(class); + assert!(!f.cause_plain.is_empty(), "empty cause for {class:?}"); + assert!(!f.next_action.is_empty(), "empty next_action for {class:?}"); + assert_eq!(f.recoverable, f.category.is_recoverable()); + } + } + + #[test] + fn recoverable_flag_matches_category() { + assert!(classify("503 service unavailable", false).recoverable); + assert!(!classify("permission denied (os error 13)", false).recoverable); + assert!(!classify("blocked by policy", false).recoverable); + } +} diff --git a/src/openhuman/tool_status/types.rs b/src/openhuman/tool_status/types.rs new file mode 100644 index 0000000000..14561772bb --- /dev/null +++ b/src/openhuman/tool_status/types.rs @@ -0,0 +1,170 @@ +//! Domain types for tool-call lifecycle state and failure classification. +//! +//! These types are the shared vocabulary for the "Visible tool status, failure +//! diagnosis, and safe recovery flows" epic (#4254). They are intentionally +//! transport-agnostic serde types: the same values are carried on the +//! `tool` event-bus domain, returned over JSON-RPC, and rendered in the UI. +//! +//! Nothing here performs classification — see [`super::classify`] for the pure +//! mapping from a raw tool error into a [`ClassifiedFailure`]. Keeping the data +//! model and the heuristics in separate files lets the heuristics be unit +//! tested without any runtime state. + +use serde::{Deserialize, Serialize}; + +/// Where a single tool call is in its lifecycle. +/// +/// Rendered 1:1 in the UI status surfaces (chat timeline today, dedicated +/// status panel in a later phase). `Queued`/`Retrying`/`NeedsUserInput` are not +/// yet emitted by the executor in P1 — they are modelled here so downstream +/// phases (retry, status panel) do not need to reshape the enum. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub enum ToolLifecycleState { + /// Accepted but not yet started (e.g. waiting on a concurrency slot). + Queued, + /// Executing right now. + Running, + /// Finished successfully. + Succeeded, + /// Finished with a failure. Pair with a [`ClassifiedFailure`] for the cause. + Failed, + /// Refused before execution by the security policy / permission gate. + Blocked, + /// A bounded automatic retry is in progress. + Retrying, + /// Parked awaiting the user (approval or additional input). + NeedsUserInput, +} + +/// The kind of failure, one of the classes called out in #4254 plus a catch-all. +/// +/// The set is deliberately small and user-facing — it maps to the plain-language +/// explanations in [`ClassifiedFailure`], not to internal error enums. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub enum ToolFailureClass { + /// OpenHuman lacks an OS/tool permission it needs (e.g. file access). + MissingPermission, + /// A required application or command is not installed / not available. + MissingApp, + /// A dependency service is temporarily unavailable / unreachable. + ServiceUnavailable, + /// Saved credentials are missing, expired, or invalid. + BadCredentials, + /// The action was refused by the user's safety / autonomy policy. + BlockedByPolicy, + /// The AI model / provider could not be reached. + ModelConnection, + /// The action ran past its deadline and was stopped. + Timeout, + /// Could not be classified into any of the above. + Unknown, +} + +/// The three top-level states the UI separates, per the #4254 acceptance +/// criterion "clear separation between recoverable failure, blocked-by-policy, +/// and action-needs-user-confirmation states". +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub enum FailureCategory { + /// Transient — safe for OpenHuman to retry on its own (bounded, in a later + /// phase). `recoverable` on [`ClassifiedFailure`] is true iff this variant. + Recoverable, + /// Refused by policy. Not retried; the user must change settings to allow it. + BlockedByPolicy, + /// Needs the user to act (grant permission, install an app, sign in) before + /// the action can succeed. + NeedsUserConfirmation, +} + +/// A tool failure rendered for a non-technical user: what class it is, which +/// top-level category it falls in, a plain-language cause, and a next action. +/// +/// Constructed only via [`super::classify`]; the string fields are stable, +/// user-facing copy (English source; localized in the UI layer). +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ClassifiedFailure { + /// The specific failure class. + pub class: ToolFailureClass, + /// The top-level category the UI separates on. + pub category: FailureCategory, + /// Plain-language description of what went wrong (no jargon, no stack). + pub cause_plain: String, + /// Plain-language description of what to do next. + pub next_action: String, + /// Whether OpenHuman may retry automatically. Always equal to + /// `category == FailureCategory::Recoverable` — carried explicitly so + /// serialized consumers don't have to re-derive it. + pub recoverable: bool, +} + +impl FailureCategory { + /// Whether a failure in this category is safe to retry automatically. + pub fn is_recoverable(self) -> bool { + matches!(self, FailureCategory::Recoverable) + } +} + +impl ToolFailureClass { + /// The category this class belongs to. Single source of truth for the + /// class→category mapping used by [`super::classify`]. + pub fn category(self) -> FailureCategory { + match self { + ToolFailureClass::ServiceUnavailable + | ToolFailureClass::ModelConnection + | ToolFailureClass::Timeout + | ToolFailureClass::Unknown => FailureCategory::Recoverable, + ToolFailureClass::BlockedByPolicy => FailureCategory::BlockedByPolicy, + ToolFailureClass::MissingPermission + | ToolFailureClass::MissingApp + | ToolFailureClass::BadCredentials => FailureCategory::NeedsUserConfirmation, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn recoverable_iff_recoverable_category() { + assert!(FailureCategory::Recoverable.is_recoverable()); + assert!(!FailureCategory::BlockedByPolicy.is_recoverable()); + assert!(!FailureCategory::NeedsUserConfirmation.is_recoverable()); + } + + #[test] + fn every_class_maps_to_expected_category() { + use FailureCategory::*; + use ToolFailureClass::*; + assert_eq!(ServiceUnavailable.category(), Recoverable); + assert_eq!(ModelConnection.category(), Recoverable); + assert_eq!(Timeout.category(), Recoverable); + assert_eq!(Unknown.category(), Recoverable); + assert_eq!( + ToolFailureClass::BlockedByPolicy.category(), + FailureCategory::BlockedByPolicy + ); + assert_eq!(MissingPermission.category(), NeedsUserConfirmation); + assert_eq!(MissingApp.category(), NeedsUserConfirmation); + assert_eq!(BadCredentials.category(), NeedsUserConfirmation); + } + + #[test] + fn types_round_trip_through_json() { + let f = ClassifiedFailure { + class: ToolFailureClass::Timeout, + category: FailureCategory::Recoverable, + cause_plain: "took too long".into(), + next_action: "try again".into(), + recoverable: true, + }; + let json = serde_json::to_string(&f).unwrap(); + let back: ClassifiedFailure = serde_json::from_str(&json).unwrap(); + assert_eq!(f, back); + } + + #[test] + fn lifecycle_state_serializes_to_variant_name() { + let json = serde_json::to_string(&ToolLifecycleState::NeedsUserInput).unwrap(); + assert_eq!(json, "\"NeedsUserInput\""); + } +} diff --git a/tests/memory_threads_raw_coverage_e2e.rs b/tests/memory_threads_raw_coverage_e2e.rs index dc71def4c5..239739febb 100644 --- a/tests/memory_threads_raw_coverage_e2e.rs +++ b/tests/memory_threads_raw_coverage_e2e.rs @@ -3159,6 +3159,7 @@ fn turn_state_mirror_persists_progress_edges_from_public_events() { output_chars: 0, elapsed_ms: 11, iteration: 2, + failure: None, })); assert!(!mirror.observe(&AgentProgress::TurnCostUpdated { model: "coverage-model".into(),