Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 155 additions & 14 deletions crates/buzz-acp/src/acp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ use crate::usage::{TurnUsage, UsageTracker};
/// Lines exceeding this limit are rejected to prevent OOM from rogue agents.
const MAX_LINE_SIZE: usize = 10_000_000; // 10 MB

/// Maximum serialized JSON detail included in an agent error diagnostic.
const MAX_AGENT_ERROR_DATA_BYTES: usize = 4 * 1024;

/// An MCP server configuration passed to `session/new`.
///
/// Corresponds to the `McpServerStdio` variant in the ACP schema.
Expand Down Expand Up @@ -105,20 +108,48 @@ pub enum AcpError {
Protocol(String),

#[error("Agent reported error (code {code}): {message}")]
AgentError { code: i64, message: String },
AgentError {
code: i64,
message: String,
data: Option<String>,
},
}

/// Build an [`AcpError::AgentError`] from a JSON-RPC error object,
/// preserving the numeric code. When the `message` field is missing or
/// non-string, fall back to the full JSON object so provider-specific
/// detail (e.g. a `data` field) is not lost.
/// preserving the numeric code and bounded provider-specific `data`. When the
/// `message` field is missing or non-string, use a stable fallback message while
/// retaining any `data` detail.
fn agent_error_from_json(error: &serde_json::Value) -> AcpError {
let code = error.get("code").and_then(|c| c.as_i64()).unwrap_or(-32000);
let message = match error.get("message").and_then(|m| m.as_str()) {
Some(m) => m.to_string(),
None => error.to_string(),
let wire_message = error.get("message").and_then(|message| message.as_str());
let message = wire_message.unwrap_or("Unknown agent error").to_string();
Comment thread
brainx marked this conversation as resolved.
let data = match wire_message {
Some(_) => error
.get("data")
.filter(|data| !data.is_null())
.map(bounded_error_data),
None => Some(bounded_error_data(error)),
};
AcpError::AgentError { code, message }
AcpError::AgentError {
code,
message,
data,
}
}

fn bounded_error_data(data: &serde_json::Value) -> String {
let serialized = data.to_string();
if serialized.len() <= MAX_AGENT_ERROR_DATA_BYTES {
return serialized;
}

const TRUNCATION_MARKER: &str = "…";
let mut end = MAX_AGENT_ERROR_DATA_BYTES - TRUNCATION_MARKER.len();
while !serialized.is_char_boundary(end) {
end -= 1;
}

format!("{}{}", &serialized[..end], TRUNCATION_MARKER)
}

fn build_initialize_params() -> serde_json::Value {
Expand Down Expand Up @@ -3438,16 +3469,22 @@ mod tests {
}

#[test]
fn agent_error_from_json_falls_back_to_full_json_when_message_missing() {
fn agent_error_from_json_preserves_data_when_message_missing() {
// Errors without a string `message` field (e.g. only a `data` field) must
// not be silently truncated to "unknown error" — the full JSON is preserved.
// still retain bounded provider-specific detail.
let error = serde_json::json!({"code": -32000, "data": "quota exceeded"});
match super::agent_error_from_json(&error) {
AcpError::AgentError { code, message } => {
AcpError::AgentError {
code,
message,
data,
} => {
assert_eq!(code, -32000);
assert_eq!(message, "Unknown agent error");
let data = data.expect("missing-message response should retain full error JSON");
assert!(
message.contains("quota exceeded"),
"expected full JSON in message, got: {message}"
data.contains("quota exceeded"),
"expected full JSON in owner data, got: {data}"
);
}
other => panic!("expected AgentError, got {other:?}"),
Expand All @@ -3458,9 +3495,113 @@ mod tests {
fn agent_error_from_json_uses_message_field_when_present() {
let error = serde_json::json!({"code": -32001, "message": "auth denied"});
match super::agent_error_from_json(&error) {
AcpError::AgentError { code, message } => {
AcpError::AgentError {
code,
message,
data,
} => {
assert_eq!(code, -32001);
assert_eq!(message, "auth denied");
assert!(data.is_none());
}
other => panic!("expected AgentError, got {other:?}"),
}
}

#[test]
fn agent_error_from_json_preserves_data_when_message_present() {
let error = serde_json::json!({
"code": -32603,
"message": "Internal error",
"data": {
"details": "Claude Code process exited: unknown option '--tools'"
}
});

let agent_error = super::agent_error_from_json(&error);
assert_eq!(
agent_error.to_string(),
"Agent reported error (code -32603): Internal error"
);

match agent_error {
AcpError::AgentError {
code,
message,
data,
} => {
assert_eq!(code, -32603);
assert_eq!(message, "Internal error");
let data = data.expect("structured JSON-RPC data should be retained");
assert!(
data.contains("unknown option '--tools'"),
"expected JSON-RPC error data in owner detail, got: {data}"
);
}
other => panic!("expected AgentError, got {other:?}"),
}
}

#[test]
fn agent_error_from_json_bounds_data_at_utf8_boundary() {
let error = serde_json::json!({
"code": -32603,
"message": "Internal error",
"data": { "details": "é".repeat(4_096) }
});

match super::agent_error_from_json(&error) {
AcpError::AgentError {
data: Some(data), ..
} => {
assert!(data.ends_with('…'));
assert!(
data.len() <= 4_096,
"expected bounded error data, got {} bytes",
data.len()
);
}
other => panic!("expected AgentError, got {other:?}"),
}
}

#[test]
fn agent_error_from_json_bounds_fallback_when_message_missing() {
let error = serde_json::json!({
"code": -32603,
"data": { "details": "é".repeat(4_096) }
});

match super::agent_error_from_json(&error) {
AcpError::AgentError {
message,
data: Some(data),
..
} => {
assert_eq!(message, "Unknown agent error");
assert!(data.ends_with('…'));
assert!(
data.len() <= 4_096,
"expected bounded fallback data, got {} bytes",
data.len()
);
}
other => panic!("expected AgentError, got {other:?}"),
}
}

#[test]
fn agent_error_from_json_ignores_null_data() {
let error = serde_json::json!({
"code": -32603,
"message": "Internal error",
"data": null
});

match super::agent_error_from_json(&error) {
AcpError::AgentError { message, data, .. } => {
assert_eq!(message, "Internal error");
assert!(data.is_none());
}
other => panic!("expected AgentError, got {other:?}"),
}
Expand Down
51 changes: 39 additions & 12 deletions crates/buzz-acp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3109,7 +3109,7 @@ fn handle_prompt_result(
PromptSource::Heartbeat => None,
};
let turn_id = result.turn_id.clone();
let emit_turn_error = |error_msg: &str, error_code: Option<i64>| {
let emit_turn_error = |error_msg: &str, error_code: Option<i64>, error_data: Option<&str>| {
if let Some(ref observer) = observer {
let mut payload = serde_json::json!({
"outcome": outcome_label,
Expand All @@ -3118,6 +3118,9 @@ fn handle_prompt_result(
if let Some(code) = error_code {
payload["code"] = serde_json::json!(code);
}
if let Some(data) = error_data {
payload["data"] = serde_json::json!(data);
}
observer.emit(
"turn_error",
Some(agent_index),
Expand Down Expand Up @@ -3160,7 +3163,7 @@ fn handle_prompt_result(
}
_ => "Agent session timed out due to inactivity".to_string(),
};
emit_turn_error(&death_message, None);
emit_turn_error(&death_message, None, None);

let index = result.agent.index;
let slot_history = &mut crash_history[index];
Expand Down Expand Up @@ -3200,7 +3203,7 @@ fn handle_prompt_result(
let death_message = format!(
"Agent did not stop within {grace:?} after cancellation; the agent process is being replaced."
);
emit_turn_error(&death_message, None);
emit_turn_error(&death_message, None, None);

let index = result.agent.index;
let slot_history = &mut crash_history[index];
Expand Down Expand Up @@ -3252,9 +3255,9 @@ fn handle_prompt_result(
| acp::AcpError::Timeout(_)
| acp::AcpError::Protocol(_)
);
let error_code = match &e {
acp::AcpError::AgentError { code, .. } => Some(*code),
_ => None,
let (error_code, error_data) = match e {
acp::AcpError::AgentError { code, data, .. } => (Some(*code), data.as_deref()),
_ => (None, None),
};
if is_transport_error {
tracing::warn!(
Expand All @@ -3265,7 +3268,7 @@ fn handle_prompt_result(
error = %e,
"transport/protocol error — respawning agent"
);
emit_turn_error(&e.to_string(), error_code);
emit_turn_error(&e.to_string(), error_code, error_data);

let index = result.agent.index;
let slot_history = &mut crash_history[index];
Expand All @@ -3291,7 +3294,7 @@ fn handle_prompt_result(
error = %e,
"agent_returned (application error — pipe intact)"
);
emit_turn_error(&e.to_string(), error_code);
emit_turn_error(&e.to_string(), error_code, error_data);
pool.return_agent(result.agent);
}
}
Expand Down Expand Up @@ -4855,9 +4858,9 @@ mod error_outcome_emission_tests {
}
}

/// Drive one error outcome through `handle_prompt_result` and return how
/// many `turn_error` events it emitted to the observer feed.
async fn turn_errors_emitted_for(outcome: PromptOutcome) -> usize {
/// Drive one error outcome through `handle_prompt_result` and return the
/// `turn_error` events it emitted to the observer feed.
async fn turn_error_events_for(outcome: PromptOutcome) -> Vec<observer::ObserverEvent> {
let agent = dummy_agent(0).await;
let mut pool = AgentPool::from_slots(vec![None]);

Expand Down Expand Up @@ -4924,7 +4927,11 @@ mod error_outcome_emission_tests {
.all(|event| event.turn_id.as_deref() == Some("test-turn-id")),
"turn_error must retain the completed turn id"
);
turn_errors.len()
turn_errors
}

async fn turn_errors_emitted_for(outcome: PromptOutcome) -> usize {
turn_error_events_for(outcome).await.len()
}

#[tokio::test]
Expand Down Expand Up @@ -5757,6 +5764,26 @@ mod error_outcome_emission_tests {
let app = AcpError::IdleTimeout(std::time::Duration::from_secs(1));
assert_eq!(turn_errors_emitted_for(PromptOutcome::Error(app)).await, 1);
}

#[tokio::test]
async fn agent_error_detail_reaches_turn_error_payload() {
let error = AcpError::AgentError {
code: -32603,
message: "Internal error".into(),
data: Some(r#"{"details":"unknown option '--tools'"}"#.into()),
};
let events = turn_error_events_for(PromptOutcome::Error(error)).await;

assert_eq!(events.len(), 1);
assert_eq!(events[0].payload["code"], -32603);
assert_eq!(
events[0].payload["error"],
"Agent reported error (code -32603): Internal error"
);
assert!(events[0].payload["data"]
.as_str()
.is_some_and(|message| message.contains("unknown option '--tools'")));
}
}

#[cfg(test)]
Expand Down