From 8a7eb8d3d71cd6ecc988963b36ff5777715f33ef Mon Sep 17 00:00:00 2001 From: Tyler <109685178+tlongwell-block@users.noreply.github.com> Date: Wed, 5 Aug 2026 11:51:05 -0400 Subject: [PATCH] fix(agent): recover from unsupported image input instead of poisoning the turn (#4896) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem `buzz-dev-mcp` advertises `view_image` to every agent regardless of whether the session's model accepts images. When a text-only model (e.g. DeepSeek V4 Flash) takes the bait, the image lands in session history and every subsequent LLM request 404s with `No endpoints found that support image input`. The error was classified as `LlmModelNotFound` and propagated fatally out of the turn loop — history stays poisoned, buzz-acp retries the batch with exponential backoff, and the session burns its entire clock doing no work. In a recent trial run, **all 57 trials that called `view_image` on a text-only model died this way; none recovered.** ## Fix Capability-gating the advertised tool isn't reliable — there is no image-capability metadata at the agent layer across providers. Instead, recover at the turn loop: - **Typed error**: new `AgentError::UnsupportedImageInput`, classified narrowly on the exact provider phrase `No endpoints found that support image input` on both the generic 404 path and OpenRouter's 404 path. Unknown-model 404s and OpenRouter parameter-routing 404s keep their existing classifications. No deterministic retry. - **In-turn recovery**: on this error, `RunCtx::run` strips every image block from history — keeping the tool result (and therefore tool-call/result pairing) intact — marks the result `is_error`, appends actionable model-facing guidance ("The current model does not support image input. The image was removed from conversation history so this turn can continue. Use a text-based inspection tool…"), and continues the same turn. Base64 never replays again. - **Loop guard**: recovery only fires when at least one image was removed; if the provider says "image" and history has none, the error propagates as before. ## Tests - Unit: phrase classification (typed, not retried; unknown-model 404 unaffected), idempotent image-to-error history mutation preserving call IDs and text. - End-to-end (`fake_llm.rs` + `fake_mcp.rs`): tool call → MCP image result → 404 unsupported-image → same-turn recovery. Captured requests prove round 2 carried the image, round 3 replays no image, carries the guidance text, preserves pairing, and ends `end_turn`. - Loop guard: typed unsupported-image error with **no** image in history fails after exactly one provider request instead of spinning — mutation-testing showed deleting the `removed == 0` guard survived the suite, and `max_rounds` defaults to unlimited in production, so this branch needed direct coverage. Verified at `a210305019b33d5f56677b4c82bab79e4ac52d24`: `cargo test -p buzz-agent` (full package, 381 unit + all integration suites) green; `clippy --all-targets -D warnings` green; `fmt --check` green; pre-push hooks (rust-tests, desktop-tauri-checks, branch-skew) green. **Scope of the classification guarantee**: the classifier runs in the shared `post()` (which Anthropic and OpenAI paths route through) and in `openrouter_post()` — i.e., every 404 path in `llm.rs`. It only runs on 404 responses; providers that reject images with a different status (e.g. a 400) are out of scope for this PR — see the review-comment discussion for why broadening the phrase list alone would not cover them. Authored by Wren, loop-guard test by Sami, reviewed by Eva. --------- Signed-off-by: Wren <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@buzz.block.builderlab.xyz> Signed-off-by: Sami Co-authored-by: Wren <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@buzz.block.builderlab.xyz> Co-authored-by: Sami --- crates/buzz-agent/src/agent.rs | 89 ++++++++++- crates/buzz-agent/src/llm.rs | 45 +++++- crates/buzz-agent/src/types.rs | 5 + crates/buzz-agent/tests/bin/fake_mcp.rs | 11 +- crates/buzz-agent/tests/fake_llm.rs | 198 +++++++++++++++++++++++- 5 files changed, 335 insertions(+), 13 deletions(-) diff --git a/crates/buzz-agent/src/agent.rs b/crates/buzz-agent/src/agent.rs index ff87a33a1..b3dfdd3aa 100644 --- a/crates/buzz-agent/src/agent.rs +++ b/crates/buzz-agent/src/agent.rs @@ -21,6 +21,34 @@ use crate::wire::{self, WireSender}; const ERROR_REFLECTION_SUFFIX: &str = "\n\n[Reflect] Before retrying, identify the cause and change your approach."; +const UNSUPPORTED_IMAGE_TOOL_MESSAGE: &str = "The current model does not support image input. The image was removed from conversation history so this turn can continue. Use a text-based inspection tool or ask the user for a textual description instead."; + +/// Remove image blocks that the provider has explicitly rejected while keeping +/// their surrounding tool result (and therefore the tool-call/result pairing) +/// intact. Returns the number of images removed; zero means the provider error +/// cannot be safely recovered by mutating history. +fn replace_unsupported_images(history: &mut [HistoryItem]) -> usize { + let mut replaced = 0; + for item in history { + let HistoryItem::ToolResult(result) = item else { + continue; + }; + let before = result.content.len(); + result + .content + .retain(|content| !matches!(content, ToolResultContent::Image { .. })); + let removed = before - result.content.len(); + if removed > 0 { + replaced += removed; + result.is_error = true; + result.content.push(ToolResultContent::Text( + UNSUPPORTED_IMAGE_TOOL_MESSAGE.to_string(), + )); + } + } + replaced +} + /// Maximum reply reminders emitted per prompt when `require_reply` is on. /// /// After this many, the turn is allowed to end whether or not anything was @@ -249,10 +277,10 @@ impl RunCtx<'_> { tools.push(builtin::load_skill_def()); } round = round.saturating_add(1); - let response = tokio::select! { + let response_result = tokio::select! { biased; _ = self.cancel.changed() => return Ok(StopReason::Cancelled), - r = self.llm.complete(self.cfg, self.system_prompt, self.history, &tools, self.effective_model) => r?, + r = self.llm.complete(self.cfg, self.system_prompt, self.history, &tools, self.effective_model) => r, _ = async { // Keepalive ticker: emit a lightweight session update every 30s // while waiting on the LLM provider. This resets the ACP harness @@ -275,6 +303,22 @@ impl RunCtx<'_> { } } => unreachable!(), }; + let response = match response_result { + Ok(response) => response, + Err(AgentError::UnsupportedImageInput(detail)) => { + let removed = replace_unsupported_images(self.history); + if removed == 0 { + return Err(AgentError::UnsupportedImageInput(detail)); + } + tracing::warn!( + model = self.effective_model, + removed_images = removed, + "provider rejected image input; removed images from history and continuing turn" + ); + continue; + } + Err(error) => return Err(error), + }; // Record provider-reported input usage so the next loop iteration's // handoff gate can compare it against the token budget. We capture @@ -1075,6 +1119,47 @@ mod tests { assert!(total_after <= max_bytes); } + #[test] + fn unsupported_images_become_recoverable_tool_errors() { + let mut history = vec![ + HistoryItem::Assistant { + text: String::new(), + tool_calls: vec![ToolCall { + provider_id: "call-image".into(), + name: "dev__view_image".into(), + arguments: json!({ "source": "spec.png" }), + provider_extra: Default::default(), + }], + reasoning_details: None, + }, + HistoryItem::ToolResult(ToolResult { + provider_id: "call-image".into(), + content: vec![ + ToolResultContent::Text("10x10 image from spec.png".into()), + ToolResultContent::Image { + data: "aW1n".into(), + mime_type: "image/png".into(), + }, + ], + is_error: false, + }), + ]; + + assert_eq!(replace_unsupported_images(&mut history), 1); + let HistoryItem::ToolResult(result) = &history[1] else { + panic!("tool result must stay paired with the assistant tool call"); + }; + assert_eq!(result.provider_id, "call-image"); + assert!(result.is_error); + assert!(result + .content + .iter() + .all(|content| !matches!(content, ToolResultContent::Image { .. }))); + assert!(result.text().contains("does not support image input")); + assert!(result.text().contains("10x10 image from spec.png")); + assert_eq!(replace_unsupported_images(&mut history), 0); + } + #[test] fn truncate_history_noop_when_under_budget() { let mut history = vec![ diff --git a/crates/buzz-agent/src/llm.rs b/crates/buzz-agent/src/llm.rs index 220d99f9a..99e74c6d6 100644 --- a/crates/buzz-agent/src/llm.rs +++ b/crates/buzz-agent/src/llm.rs @@ -1706,6 +1706,11 @@ fn is_retryable_transport_error(e: &reqwest::Error) -> bool { e.is_timeout() || e.is_connect() || e.is_request() } +fn is_unsupported_image_input_error(body: &str) -> bool { + body.to_ascii_lowercase() + .contains("no endpoints found that support image input") +} + /// Build the terminal `AgentError::Llm` for a `post()` exit that has given up /// retrying — persistent retryable status, transport failure, or a body-read /// break. `detail` carries the specific cause (status/body, or the transport @@ -1864,9 +1869,14 @@ where // upstream capacity — no retry was attempted, so cumulative duration // would be misleading. if status == 404 { + let error_body = read_error_body(resp).await; + if is_unsupported_image_input_error(&error_body) { + return Err(PostError::Agent(AgentError::UnsupportedImageInput( + error_body, + ))); + } return Err(PostError::Agent(AgentError::LlmModelNotFound(format!( - "{status}: {}", - read_error_body(resp).await + "{status}: {error_body}" )))); } if !status.is_success() { @@ -2117,6 +2127,9 @@ async fn openrouter_post( // about the model, and reporting a parameter problem as // `LlmModelNotFound` (or vice versa) sends the user to the wrong fix. let error_body = read_error_body(resp).await; + if is_unsupported_image_input_error(&error_body) { + return Err(AgentError::UnsupportedImageInput(error_body)); + } if error_body.contains("No endpoints found that can handle the requested parameters") { return Err(openrouter_parameter_routing_error(&error_body)); } @@ -6217,6 +6230,34 @@ mod tests { ); } + /// A provider's explicit image-capability rejection is a recoverable typed + /// error, not a missing model. The agent loop uses this signal to remove the + /// image from history before retrying the next LLM round. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn openrouter_post_404_unsupported_image_is_typed_and_not_retried() { + let (url, _captured, attempts) = spawn_openrouter_stub(vec![CannedResponse::new( + 404, + r#"{"error":{"message":"No endpoints found that support image input"}}"#, + )]) + .await; + let http = Client::builder() + .timeout(Duration::from_secs(5)) + .build() + .unwrap(); + let err = openrouter_post(&http, &format!("{url}/x"), &json!({}), "key") + .await + .unwrap_err(); + assert!( + matches!(&err, AgentError::UnsupportedImageInput(s) if s.contains("support image input")), + "image rejection must reach the history-recovery path: got {err:?}" + ); + assert_eq!( + attempts.load(std::sync::atomic::Ordering::SeqCst), + 1, + "a deterministic capability rejection must not be retried" + ); + } + /// Every other 404 still maps to `LlmModelNotFound`, including one that /// shares the `No endpoints found` prefix but is about the model rather than /// the parameters — the discriminator is narrow enough that a genuinely diff --git a/crates/buzz-agent/src/types.rs b/crates/buzz-agent/src/types.rs index e38642198..fce705d69 100644 --- a/crates/buzz-agent/src/types.rs +++ b/crates/buzz-agent/src/types.rs @@ -388,6 +388,10 @@ pub enum AgentError { Llm(String), LlmAuth(String), LlmModelNotFound(String), + /// The provider explicitly rejected image content for the selected model. + /// Kept distinct so the agent loop can remove the unsupported image from + /// replayed history and give the model a recoverable tool error. + UnsupportedImageInput(String), Mcp(String), Cancelled, } @@ -399,6 +403,7 @@ impl std::fmt::Display for AgentError { Self::Llm(s) => write!(f, "llm: {s}"), Self::LlmAuth(s) => write!(f, "llm auth: {s}"), Self::LlmModelNotFound(s) => write!(f, "llm model not found: {s}"), + Self::UnsupportedImageInput(s) => write!(f, "llm image input unsupported: {s}"), Self::Mcp(s) => write!(f, "mcp: {s}"), Self::Cancelled => write!(f, "cancelled"), } diff --git a/crates/buzz-agent/tests/bin/fake_mcp.rs b/crates/buzz-agent/tests/bin/fake_mcp.rs index 5b660da48..1b7f34616 100644 --- a/crates/buzz-agent/tests/bin/fake_mcp.rs +++ b/crates/buzz-agent/tests/bin/fake_mcp.rs @@ -12,6 +12,7 @@ //! (use a large value, e.g. 999, to simulate hang) //! FAKE_MCP_RESULT_SIZE=N — `tools/call` returns an N-byte text result //! (default: the literal "ok"); grows history +//! FAKE_MCP_IMAGE_RESULT=1 — `tools/call` returns text plus a PNG image block //! FAKE_MCP_PID_FILE=path — write the child PID to `path` on startup //! (for tests that want to verify the child died) //! FAKE_MCP_SPAWN_GRANDCHILD=1 @@ -300,10 +301,18 @@ fn main() { } else { "ok".to_owned() }; + let content = if env_flag("FAKE_MCP_IMAGE_RESULT") { + json!([ + { "type": "text", "text": result_text }, + { "type": "image", "data": "aW1n", "mimeType": "image/png" }, + ]) + } else { + json!([{ "type": "text", "text": result_text }]) + }; write_response( id, json!({ - "content": [{ "type": "text", "text": result_text }], + "content": content, "isError": false, }), ); diff --git a/crates/buzz-agent/tests/fake_llm.rs b/crates/buzz-agent/tests/fake_llm.rs index ef6f9d2d8..4253ef329 100644 --- a/crates/buzz-agent/tests/fake_llm.rs +++ b/crates/buzz-agent/tests/fake_llm.rs @@ -57,9 +57,26 @@ async fn spawn_fake_llm(responses: Vec) -> String { url } +struct CannedResponse { + status: u16, + body: Value, +} + /// Like `spawn_fake_llm` but also captures the full JSON request body from each /// incoming HTTP request. Returns (url, captured_requests). async fn spawn_capturing_fake_llm(responses: Vec) -> (String, Arc>>) { + spawn_capturing_fake_llm_with_statuses( + responses + .into_iter() + .map(|body| CannedResponse { status: 200, body }) + .collect(), + ) + .await +} + +async fn spawn_capturing_fake_llm_with_statuses( + responses: Vec, +) -> (String, Arc>>) { let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); let url = format!("http://{}", listener.local_addr().unwrap()); let queue = Arc::new(Mutex::new(VecDeque::from(responses))); @@ -122,15 +139,22 @@ async fn spawn_capturing_fake_llm(responses: Vec) -> (String, Arc