From 701ec6b5a3f3423846b0ece3827313c72dd312ec Mon Sep 17 00:00:00 2001 From: brainx <12695242+brainx@users.noreply.github.com> Date: Thu, 23 Jul 2026 01:40:45 +0200 Subject: [PATCH 1/2] fix(acp): preserve JSON-RPC error data Signed-off-by: brainx <12695242+brainx@users.noreply.github.com> --- crates/buzz-acp/src/acp.rs | 169 ++++++++++++++++++++++++++++++++++--- crates/buzz-acp/src/lib.rs | 51 ++++++++--- 2 files changed, 194 insertions(+), 26 deletions(-) diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index b553adaabb..adef8f09b5 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -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. @@ -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, + }, } /// 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(); + 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 { @@ -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:?}"), @@ -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:?}"), } diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 862732f478..b3c138ad11 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -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| { + let emit_turn_error = |error_msg: &str, error_code: Option, error_data: Option<&str>| { if let Some(ref observer) = observer { let mut payload = serde_json::json!({ "outcome": outcome_label, @@ -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), @@ -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]; @@ -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]; @@ -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!( @@ -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]; @@ -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); } } @@ -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 { let agent = dummy_agent(0).await; let mut pool = AgentPool::from_slots(vec![None]); @@ -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] @@ -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)] From d835077c426185f4bb55eb6e38e26803e02df91d Mon Sep 17 00:00:00 2001 From: brainx <12695242+brainx@users.noreply.github.com> Date: Thu, 23 Jul 2026 23:48:46 +0200 Subject: [PATCH 2/2] fix(desktop): surface ACP error diagnostics Signed-off-by: brainx <12695242+brainx@users.noreply.github.com> --- .../agents/lib/friendlyAgentLastError.ts | 23 ++++++++---- .../agents/ui/agentSessionTranscript.test.mjs | 36 +++++++++++++++++++ .../agents/ui/agentSessionTranscript.ts | 6 +++- 3 files changed, 57 insertions(+), 8 deletions(-) diff --git a/desktop/src/features/agents/lib/friendlyAgentLastError.ts b/desktop/src/features/agents/lib/friendlyAgentLastError.ts index 60c77bb04c..7c20327778 100644 --- a/desktop/src/features/agents/lib/friendlyAgentLastError.ts +++ b/desktop/src/features/agents/lib/friendlyAgentLastError.ts @@ -6,9 +6,10 @@ * JSON-RPC codes (`-32001` auth, `-32002` model-not-found, * `-32000` generic), defined in `crates/buzz-agent/src/types.rs`. * buzz-acp — preserves the code structurally in - * `AcpError::AgentError { code, message }`, whose Display is - * `"Agent reported error (code N): message"`, and includes - * `code` in `turn_error` observer events. + * `AcpError::AgentError { code, message, data }`, whose Display + * is `"Agent reported error (code N): message"`, and includes + * `code` plus bounded diagnostics in `turn_error` observer + * events. * desktop supervisor — on nonzero exit, recovers `{ message, code }` from * the log tail (`managed_agents/storage.rs`) into * `ManagedAgent.lastError` / `lastErrorCode`. @@ -118,11 +119,19 @@ export function friendlyAgentLastError( /** * Convenience for `turn_error` / `agent_panic` observer payloads: coerce the - * payload's untyped `code` JSON value and return the display copy, falling - * back to the raw error text when no classification applies. + * payload's untyped `code` JSON value, include owner-scoped diagnostic data, + * and return the display copy. General ACP logs and channel notices do not use + * this helper, so diagnostic data remains confined to the observer transcript. */ -export function friendlyTurnErrorCopy(raw: string, code: unknown): string { +export function friendlyTurnErrorCopy( + raw: string, + code: unknown, + data?: unknown, +): string { + const detail = typeof data === "string" ? data.trim() : ""; + const diagnostic = + detail.length > 0 && !raw.includes(detail) ? `${raw}\n${detail}` : raw; const numeric = code == null ? null : Number(code); const safe = Number.isFinite(numeric) ? (numeric as number) : null; - return friendlyAgentLastError(raw, safe)?.copy ?? raw; + return friendlyAgentLastError(diagnostic, safe)?.copy ?? diagnostic; } diff --git a/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs b/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs index cc6f0467d6..9f5811b285 100644 --- a/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs +++ b/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs @@ -720,6 +720,42 @@ test("buildTranscript separates repeated lifecycle text", () => { assert.equal(item.text, "recovered: first\nrecovered: second"); }); +test("buildTranscript surfaces structured turn error detail to the owner", () => { + const [dataOnlyError] = buildTranscript([ + { + ...baseEvent, + kind: "turn_error", + payload: { + outcome: "error", + error: "Agent reported error (code -32000): Unknown agent error", + code: -32000, + data: '{"code":-32000,"data":"quota exceeded"}', + }, + }, + ]); + assert.equal( + dataOnlyError.text, + 'error: Agent reported error (code -32000): Unknown agent error\n{"code":-32000,"data":"quota exceeded"}', + ); + + const [internalError] = buildTranscript([ + { + ...baseEvent, + kind: "turn_error", + payload: { + outcome: "error", + error: "Agent reported error (code -32603): Internal error", + code: -32603, + data: '{"details":"unknown option \'--tools\'"}', + }, + }, + ]); + assert.equal( + internalError.text, + 'error: Internal error\n{"details":"unknown option \'--tools\'"}', + ); +}); + // --- permission outcome (Fix #3) --- function makePermissionRequest(seq, requestId, turnId = "turn-1") { diff --git a/desktop/src/features/agents/ui/agentSessionTranscript.ts b/desktop/src/features/agents/ui/agentSessionTranscript.ts index 962290c6ca..3c0e0f373b 100644 --- a/desktop/src/features/agents/ui/agentSessionTranscript.ts +++ b/desktop/src/features/agents/ui/agentSessionTranscript.ts @@ -773,7 +773,11 @@ export function processTranscriptEvent( const payload = asRecord(event.payload); const outcome = asString(payload.outcome) ?? "error"; const error = asString(payload.error) ?? "Unknown error"; - const displayError = friendlyTurnErrorCopy(error, payload.code); + const displayError = friendlyTurnErrorCopy( + error, + payload.code, + payload.data, + ); const title = event.kind === "agent_panic" ? "Agent error (crash)" : "Turn error"; upsertTextItem(