mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-07 03:00:29 +00:00
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) <noreply@anthropic.com>
This commit is contained in:
parent
83ed0d42a4
commit
cd70962bd9
5 changed files with 85 additions and 18 deletions
|
|
@ -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();
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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<Mutex<VecDeque<String>>>,
|
||||
followup_queue: Arc<Mutex<VecDeque<String>>>,
|
||||
cancel_token: CancellationToken,
|
||||
abort_reason: Arc<Mutex<Option<AbortReason>>>,
|
||||
project_docs: Vec<String>,
|
||||
env_context: EnvContext,
|
||||
skills: Vec<Skill>,
|
||||
|
|
@ -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<Mutex<Option<AbortReason>>> {
|
||||
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<Mutex<VecDeque<String>>> {
|
||||
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);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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}"))),
|
||||
};
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue