From cd70962bd99e6950038df94c4488bb19c477dae4 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Wed, 11 Mar 2026 09:34:52 -0400 Subject: [PATCH] Add AbortReason to AgentError::Aborted for diagnosable abort messages Previously, AgentError::Aborted was a bare variant that displayed as just "Aborted", making it impossible to tell whether a session was killed by wall clock timeout, user cancellation, or something else. Now it carries an AbortReason (WallClockTimeout or Cancelled) so terminal output like the retro agent failure message includes the cause. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/arc-agent/src/cli.rs | 7 +++ lib/crates/arc-agent/src/error.rs | 38 ++++++++++++--- lib/crates/arc-agent/src/lib.rs | 2 +- lib/crates/arc-agent/src/session.rs | 52 +++++++++++++++++---- lib/crates/arc-workflows/src/cli/backend.rs | 4 +- 5 files changed, 85 insertions(+), 18 deletions(-) diff --git a/lib/crates/arc-agent/src/cli.rs b/lib/crates/arc-agent/src/cli.rs index 2fafa1c04..1893fa171 100644 --- a/lib/crates/arc-agent/src/cli.rs +++ b/lib/crates/arc-agent/src/cli.rs @@ -479,8 +479,15 @@ pub async fn run_with_args_and_client( // SIGINT handler let cancel_token = session.cancel_token(); + let abort_reason = session.abort_reason_handle(); tokio::spawn(async move { tokio::signal::ctrl_c().await.ok(); + { + let mut guard = abort_reason.lock().unwrap_or_else(|e| e.into_inner()); + if guard.is_none() { + *guard = Some(crate::error::AbortReason::Cancelled); + } + } cancel_token.cancel(); }); diff --git a/lib/crates/arc-agent/src/error.rs b/lib/crates/arc-agent/src/error.rs index b03db2181..5ed6f7714 100644 --- a/lib/crates/arc-agent/src/error.rs +++ b/lib/crates/arc-agent/src/error.rs @@ -1,5 +1,22 @@ use arc_llm::error::SdkError; +/// Why a session was aborted. +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum AbortReason { + WallClockTimeout, + Cancelled, +} + +impl std::fmt::Display for AbortReason { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::WallClockTimeout => write!(f, "wall clock timeout"), + Self::Cancelled => write!(f, "cancelled"), + } + } +} + #[derive(Debug, Clone, serde::Serialize, serde::Deserialize, thiserror::Error)] #[serde(tag = "type", content = "data", rename_all = "snake_case")] pub enum AgentError { @@ -15,8 +32,8 @@ pub enum AgentError { #[error("Tool execution error: {0}")] ToolExecution(String), - #[error("Aborted")] - Aborted, + #[error("Aborted: {0}")] + Aborted(AbortReason), } #[cfg(test)] @@ -54,8 +71,14 @@ mod tests { #[test] fn aborted_display() { - let err = AgentError::Aborted; - assert_eq!(err.to_string(), "Aborted"); + let err = AgentError::Aborted(AbortReason::Cancelled); + assert_eq!(err.to_string(), "Aborted: cancelled"); + } + + #[test] + fn aborted_wall_clock_timeout_display() { + let err = AgentError::Aborted(AbortReason::WallClockTimeout); + assert_eq!(err.to_string(), "Aborted: wall clock timeout"); } // --- Serde roundtrip tests --- @@ -114,7 +137,7 @@ mod tests { #[test] fn serde_roundtrip_aborted() { - let err = AgentError::Aborted; + let err = AgentError::Aborted(AbortReason::Cancelled); let json = serde_json::to_string(&err).unwrap(); let deserialized: AgentError = serde_json::from_str(&json).unwrap(); assert_eq!(err.to_string(), deserialized.to_string()); @@ -131,7 +154,7 @@ mod tests { AgentError::SessionClosed, AgentError::InvalidState("reason".into()), AgentError::ToolExecution("reason".into()), - AgentError::Aborted, + AgentError::Aborted(AbortReason::Cancelled), ]; for err in &errors { assert_eq!(err.to_string(), err.clone().to_string()); @@ -176,9 +199,10 @@ mod tests { #[test] fn serde_tag_format_aborted() { - let err = AgentError::Aborted; + let err = AgentError::Aborted(AbortReason::WallClockTimeout); let json = serde_json::to_string(&err).unwrap(); let v: serde_json::Value = serde_json::from_str(&json).unwrap(); assert_eq!(v["type"], "aborted"); + assert_eq!(v["data"], "wall_clock_timeout"); } } diff --git a/lib/crates/arc-agent/src/lib.rs b/lib/crates/arc-agent/src/lib.rs index eeb097729..981bc43d2 100644 --- a/lib/crates/arc-agent/src/lib.rs +++ b/lib/crates/arc-agent/src/lib.rs @@ -30,7 +30,7 @@ pub use arc_mcp::config::McpServerConfig; pub use config::{SessionConfig, ToolApprovalAdapter, ToolHookCallback, ToolHookDecision}; #[cfg(feature = "docker")] pub use docker_sandbox::{DockerSandbox, DockerSandboxConfig}; -pub use error::AgentError; +pub use error::{AbortReason, AgentError}; pub use event::EventEmitter; pub use history::History; pub use local_sandbox::LocalSandbox; diff --git a/lib/crates/arc-agent/src/session.rs b/lib/crates/arc-agent/src/session.rs index 9f02c814a..c4d1de3f5 100644 --- a/lib/crates/arc-agent/src/session.rs +++ b/lib/crates/arc-agent/src/session.rs @@ -1,5 +1,5 @@ use crate::config::SessionConfig; -use crate::error::AgentError; +use crate::error::{AbortReason, AgentError}; use crate::event::EventEmitter; use crate::file_tracker::FileTracker; use crate::history::History; @@ -36,6 +36,7 @@ pub struct Session { steering_queue: Arc>>, followup_queue: Arc>>, cancel_token: CancellationToken, + abort_reason: Arc>>, project_docs: Vec, env_context: EnvContext, skills: Vec, @@ -64,6 +65,7 @@ impl Session { steering_queue: Arc::new(Mutex::new(VecDeque::new())), followup_queue: Arc::new(Mutex::new(VecDeque::new())), cancel_token: CancellationToken::new(), + abort_reason: Arc::new(Mutex::new(None)), project_docs: Vec::new(), env_context: EnvContext::default(), skills: Vec::new(), @@ -360,9 +362,33 @@ impl Session { } pub fn abort(&self) { + self.set_abort_reason(AbortReason::Cancelled); self.cancel_token.cancel(); } + /// Returns a handle that can set the abort reason from another task. + #[must_use] + pub fn abort_reason_handle(&self) -> Arc>> { + self.abort_reason.clone() + } + + fn set_abort_reason(&self, reason: AbortReason) { + let mut guard = self.abort_reason.lock().unwrap_or_else(|e| e.into_inner()); + if guard.is_none() { + *guard = Some(reason); + } + } + + fn aborted_error(&self) -> AgentError { + let reason = self + .abort_reason + .lock() + .unwrap_or_else(|e| e.into_inner()) + .clone() + .unwrap_or(AbortReason::Cancelled); + AgentError::Aborted(reason) + } + #[must_use] pub fn followup_queue_handle(&self) -> Arc>> { self.followup_queue.clone() @@ -422,8 +448,15 @@ impl Session { // Spawn wall-clock timeout task if configured let timer_handle = self.config.wall_clock_timeout.map(|duration| { let token = self.cancel_token.clone(); + let reason_handle = self.abort_reason.clone(); tokio::spawn(async move { tokio::time::sleep(duration).await; + { + let mut guard = reason_handle.lock().unwrap_or_else(|e| e.into_inner()); + if guard.is_none() { + *guard = Some(AbortReason::WallClockTimeout); + } + } token.cancel(); }) }); @@ -528,7 +561,7 @@ impl Session { // Check cancellation if self.cancel_token.is_cancelled() { self.close(); - return Err(AgentError::Aborted); + return Err(self.aborted_error()); } // Build request @@ -630,7 +663,7 @@ impl Session { if self.cancel_token.is_cancelled() { drop(event_stream); self.close(); - return Err(AgentError::Aborted); + return Err(self.aborted_error()); } let response = accumulator.response().cloned().ok_or_else(|| { @@ -743,7 +776,7 @@ impl Session { timestamp: SystemTime::now(), }); self.close(); - return Err(AgentError::Aborted); + return Err(self.aborted_error()); } // Record tool results turn @@ -1156,7 +1189,7 @@ mod tests { let result = session.process_input("Do something").await; // Should return Aborted error and transition to Closed - assert!(matches!(result, Err(AgentError::Aborted))); + assert!(matches!(result, Err(AgentError::Aborted(_)))); assert_eq!(session.state(), SessionState::Closed); // Should have stopped immediately: User turn only, no LLM call @@ -1210,7 +1243,7 @@ mod tests { let result = session.process_input("Do something").await; // Should return Aborted error and transition to Closed - assert!(matches!(result, Err(AgentError::Aborted))); + assert!(matches!(result, Err(AgentError::Aborted(_)))); assert_eq!(session.state(), SessionState::Closed); // Should have processed: User + Assistant(tool_call) + ToolResults = 3 turns @@ -2272,8 +2305,11 @@ mod tests { let result = session.process_input("Do something slow").await; assert!( - matches!(result, Err(AgentError::Aborted)), - "expected Aborted, got {result:?}" + matches!( + result, + Err(AgentError::Aborted(AbortReason::WallClockTimeout)) + ), + "expected Aborted(WallClockTimeout), got {result:?}" ); assert_eq!(session.state(), SessionState::Closed); } diff --git a/lib/crates/arc-workflows/src/cli/backend.rs b/lib/crates/arc-workflows/src/cli/backend.rs index 6b3b9a7c9..cea3245ea 100644 --- a/lib/crates/arc-workflows/src/cli/backend.rs +++ b/lib/crates/arc-workflows/src/cli/backend.rs @@ -529,7 +529,7 @@ impl CodergenBackend for AgentApiBackend { last_err = ArcError::Llm(err); } Err(arc_agent::AgentError::Llm(err)) => return Err(ArcError::Llm(err)), - Err(arc_agent::AgentError::Aborted) => return Err(ArcError::Cancelled), + Err(arc_agent::AgentError::Aborted(_)) => return Err(ArcError::Cancelled), Err(other) => { return Err(ArcError::handler(format!( "Agent session failed: {other}" @@ -545,7 +545,7 @@ impl CodergenBackend for AgentApiBackend { } } Err(arc_agent::AgentError::Llm(sdk_err)) => Err(ArcError::Llm(sdk_err)), - Err(arc_agent::AgentError::Aborted) => Err(ArcError::Cancelled), + Err(arc_agent::AgentError::Aborted(_)) => Err(ArcError::Cancelled), Err(other) => Err(ArcError::handler(format!("Agent session failed: {other}"))), };