feat(agent): classify tool-call failures + surface on web event — rework of #4413 (backend) (#4445)

This commit is contained in:
Mega Mind
2026-07-03 11:20:28 -07:00
committed by GitHub
parent 19bf3aa583
commit b5837ed0d1
17 changed files with 742 additions and 14 deletions
+1
View File
@@ -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={}",
+6
View File
@@ -185,6 +185,12 @@ pub struct WebChannelEvent {
/// `tool_result` events.
#[serde(skip_serializing_if = "Option::is_none")]
pub tool_call_id: Option<String>,
/// 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<serde_json::Value>,
/// Optional citations attached to `chat_done` payloads.
#[serde(skip_serializing_if = "Option::is_none")]
pub citations: Option<serde_json::Value>,
@@ -178,6 +178,7 @@ impl ProgressReporter for TurnProgress {
output_chars: output.chars().count(),
elapsed_ms,
iteration,
failure: None,
},
);
}
+5
View File
@@ -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<crate::openhuman::tool_status::ClassifiedFailure>,
},
/// A sub-agent was spawned during tool execution.
@@ -90,6 +90,7 @@ fn tool_completed(
output_chars: chars,
elapsed_ms: elapsed,
iteration: 1,
failure: None,
}
}
+1
View File
@@ -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,
@@ -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,
@@ -414,7 +414,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 {
@@ -426,7 +429,8 @@ pub(crate) fn spawn_progress_bridge(
"success": success,
"outputChars": output_chars,
"elapsedMs": elapsed_ms,
"iteration": iteration
"iteration": iteration,
"failure": failure_json,
}),
},
);
@@ -444,6 +448,7 @@ pub(crate) fn spawn_progress_bridge(
success: Some(success),
round: Some(iteration),
tool_call_id: Some(call_id),
failure: failure_json,
..Default::default()
});
}
+1
View File
@@ -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;
@@ -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!(
+34 -3
View File
@@ -1376,11 +1376,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 }
}
}
@@ -1396,11 +1403,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(),
});
}
+12 -1
View File
@@ -72,7 +72,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;
@@ -397,6 +399,7 @@ pub(crate) async fn run_turn_via_tinyagents_shared(
harness,
cursor,
tool_names,
failure_map,
error_slot,
halt_summary,
tool_outcome_sink,
@@ -541,6 +544,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)
@@ -859,6 +863,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.
@@ -1359,8 +1367,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
@@ -1389,6 +1399,7 @@ fn assemble_turn_harness(
harness,
cursor,
tool_names,
failure_map,
error_slot,
halt_summary,
tool_outcome_sink,
+48 -9
View File
@@ -49,6 +49,25 @@ pub(crate) type IterationCursor = Arc<AtomicU32>;
/// `tool_name` contract without the forwarder emitting those fragments itself.
pub(crate) type ToolNameMap = Arc<Mutex<std::collections::HashMap<String, String>>>;
/// 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<crate::openhuman::tool_status::ClassifiedFailure>,
),
>,
>,
>;
/// 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<BridgeState>,
}
@@ -137,6 +159,7 @@ impl OpenhumanEventBridge {
None,
Arc::default(),
Arc::default(),
Arc::default(),
)
}
@@ -150,6 +173,7 @@ impl OpenhumanEventBridge {
scope: Option<SubagentScope>,
cursor: IterationCursor,
tool_names: ToolNameMap,
failure_map: ToolFailureMap,
) -> Arc<Self> {
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,
+20
View File
@@ -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};
+430
View File
@@ -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);
}
}
+170
View File
@@ -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\"");
}
}
+1
View File
@@ -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(),