From f5da25baa0662190da5cba85a3d44e458b621e05 Mon Sep 17 00:00:00 2001 From: ji233 Date: Tue, 22 Sep 2026 23:07:17 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20=E8=AF=BB=E5=8F=96=E5=9B=BE=E7=89=87?= =?UTF-8?q?=E6=97=B6=E5=B1=95=E7=A4=BA=E9=A2=84=E8=A7=88=E5=B9=B6=E4=BF=9D?= =?UTF-8?q?=E7=95=99=E5=AA=92=E4=BD=93=E7=BB=93=E6=9E=9C=EF=BC=88#192?= =?UTF-8?q?=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- apps/desktop/locales/en.json | 6 + apps/desktop/src/model/tools.rs | 352 ++++++++++++++++++++++++-- apps/desktop/src/view/review.rs | 1 + apps/desktop/src/view/tools.rs | 290 +++++++++++++++++++-- apps/runner/src/application/events.rs | 114 +++++++++ crates/harness-acp/src/lib.rs | 40 ++- crates/harness-core/src/rpc.rs | 8 +- crates/harness-omp/src/lib.rs | 11 + 8 files changed, 781 insertions(+), 41 deletions(-) diff --git a/apps/desktop/locales/en.json b/apps/desktop/locales/en.json index 416c774..2453e78 100644 --- a/apps/desktop/locales/en.json +++ b/apps/desktop/locales/en.json @@ -377,6 +377,12 @@ "此工具事件未提供文件内容或 diff。": "This tool event did not include file contents or a diff.", "输入": "Input", "输出": "Output", + "图片预览": "Image preview", + "媒体预览": "Media preview", + "无法预览此图片。": "This image could not be previewed.", + "暂不支持预览此媒体。": "Preview is not supported for this media yet.", + "放大图片": "Enlarge image", + "缩小图片": "Shrink image", "{path} · 修改片段": "{path} · Changed snippet", "无法读取系统凭据库：{error}": "Cannot read the system credential store: {error}", "正在连接本地 Runner…": "Connecting to the local runner…", diff --git a/apps/desktop/src/model/tools.rs b/apps/desktop/src/model/tools.rs index 2cec2f6..23545bd 100644 --- a/apps/desktop/src/model/tools.rs +++ b/apps/desktop/src/model/tools.rs @@ -3,7 +3,8 @@ use nexus_domain::{Message, MessageKind, MessageRole}; use serde_json::Value; use std::{ collections::{HashMap, HashSet}, - path::Path, + path::{Path, PathBuf}, + sync::Arc, }; use uuid::Uuid; @@ -156,6 +157,12 @@ pub(crate) struct ToolDetail { pub(crate) text: String, pub(crate) language: String, pub(crate) diff: bool, + pub(crate) image: Option>, +} + +pub(crate) enum ToolImage { + Base64 { mime_type: String, data: String }, + Path(PathBuf), } impl<'a> ToolActivity<'a> { @@ -191,7 +198,7 @@ impl<'a> ToolActivity<'a> { .as_str() { "command" | "bash" | "shell" | "exec_command" => ToolCategory::Command, - "read" | "read_file" => ToolCategory::Read, + "read" | "read_file" | "view_image" | "imageview" => ToolCategory::Read, "grep" | "glob" | "search" | "web search" | "websearch" => ToolCategory::Search, "edit" | "multiedit" | "file change" | "apply_patch" => ToolCategory::Edit, "write" | "write_file" | "create_file" => ToolCategory::Create, @@ -214,6 +221,11 @@ impl<'a> ToolActivity<'a> { pub(crate) fn preview(&self, locale: Language) -> String { let input: Value = serde_json::from_str(self.input()).unwrap_or(Value::Null); + let media_output = self.input().is_empty() + && self.result.is_some_and(|result| { + serde_json::from_str::(&result.content) + .is_ok_and(|value| output_blocks(&value).any(is_media_block)) + }); let value = string_field( &input, &["command", "cmd", "file_path", "path", "pattern", "query"], @@ -221,7 +233,11 @@ impl<'a> ToolActivity<'a> { .or_else(|| input.pointer("/changes/0/path").and_then(Value::as_str)) .unwrap_or_else(|| { if self.input().is_empty() { - self.result.map_or("", |result| result.content.as_str()) + if media_output { + locale.text("媒体预览") + } else { + self.result.map_or("", |result| result.content.as_str()) + } } else { self.input() } @@ -245,7 +261,7 @@ impl<'a> ToolActivity<'a> { } } - pub(crate) fn details(&self, locale: Language) -> Vec { + pub(crate) fn details(&self, locale: Language, directory: Option<&Path>) -> Vec { let input: Value = serde_json::from_str(self.input()).unwrap_or(Value::Null); let output = self.result.map(|result| { if self.is_error() { @@ -316,12 +332,29 @@ impl<'a> ToolActivity<'a> { )); } if let Some(output) = output { - details.push(detail( - locale.text("输出"), - "text", - &pretty_payload(output), - false, - )); + if let Some(blocks) = media_output_details(&result, locale) { + details.extend(blocks); + } else if (self.category() == ToolCategory::Read || result["type"] == "imageView") + && !self.is_error() + && let Some(path) = string_field(&input, &["file_path", "path"]) + .or_else(|| string_field(&result, &["path"])) + && (is_image_path(path) || result["type"] == "imageView") + { + let path = Path::new(path); + let resolved = if path.is_absolute() { + Some(path.to_path_buf()) + } else { + directory.map(|directory| directory.join(path)) + }; + details.push(image_detail(resolved.map(ToolImage::Path), locale)); + } else { + details.push(detail( + locale.text("输出"), + "text", + &pretty_payload(output), + false, + )); + } } details } @@ -346,7 +379,101 @@ fn detail(title: &str, language: &str, text: &str, diff: bool) -> ToolDetail { language: language.into(), text: text.into(), diff, + image: None, + } +} + +fn is_image_path(path: &str) -> bool { + matches!( + language_for_path(path).to_ascii_lowercase().as_str(), + "png" + | "jpg" + | "jpeg" + | "gif" + | "webp" + | "svg" + | "bmp" + | "tif" + | "tiff" + | "ico" + | "pnm" + | "ppm" + | "pgm" + | "pbm" + ) +} + +fn image_detail(image: Option, locale: Language) -> ToolDetail { + let mut detail = detail(locale.text("图片预览"), "text", "", false); + detail.image = image.map(Arc::new); + if detail.image.is_none() { + detail.text = locale.text("无法预览此图片。").into(); + } + detail +} + +fn output_blocks(value: &Value) -> impl Iterator + Clone { + let blocks = value + .as_array() + .or_else(|| value.get("content").and_then(Value::as_array)) + .map(Vec::as_slice) + .unwrap_or_else(|| std::slice::from_ref(value)); + // ACP wraps a standard content block in a tool-call content item. + blocks.iter().map(|block| { + if block["type"] == "content" { + &block["content"] + } else { + block + } + }) +} + +fn is_media_block(block: &Value) -> bool { + matches!( + block["type"].as_str(), + Some("image" | "audio" | "video" | "document") + ) +} + +fn media_output_details(value: &Value, locale: Language) -> Option> { + let blocks = output_blocks(value); + if !blocks.clone().any(is_media_block) { + return None; } + Some( + blocks + .map(|block| match block["type"].as_str() { + Some("image") => { + let source = block.get("source").unwrap_or(block); + let image = string_field(source, &["media_type", "mimeType"]) + .zip(string_field(source, &["data"])) + .map(|(mime_type, data)| ToolImage::Base64 { + mime_type: mime_type.into(), + data: data.into(), + }); + image_detail(image, locale) + } + Some("audio" | "video" | "document") => detail( + locale.text("输出"), + "text", + locale.text("暂不支持预览此媒体。"), + false, + ), + Some("text") => detail( + locale.text("输出"), + "text", + block["text"].as_str().unwrap_or_default(), + false, + ), + _ => detail( + locale.text("输出"), + "text", + &pretty_payload(&block.to_string()), + false, + ), + }) + .collect(), + ) } fn append_edit(details: &mut Vec, path: &str, input: &Value, locale: Language) { @@ -477,7 +604,7 @@ mod tests { assert_eq!(last.len(), 2); assert_eq!(last[0].result.unwrap().content, "two"); assert!( - last[1].details(Language::Chinese)[0] + last[1].details(Language::Chinese, None)[0] .text .contains("Legacy output") ); @@ -639,7 +766,7 @@ mod tests { call: &call, result: None, }; - let details = activity.details(Language::Chinese); + let details = activity.details(Language::Chinese, None); assert_eq!(details.len(), 1); assert_eq!(details[0].language, "rs"); assert_eq!(details[0].diff, diff); @@ -650,7 +777,7 @@ mod tests { } else { assert_eq!(details[0].text, code); } - let english = activity.details(Language::English); + let english = activity.details(Language::English, None); assert_eq!(english[0].text, details[0].text); assert_eq!(english[0].language, "rs"); assert_eq!( @@ -683,14 +810,14 @@ mod tests { call: &call, result: Some(&output), } - .details(Language::Chinese); + .details(Language::Chinese, None); assert_eq!(details[0].text, "cargo test"); assert_eq!(details[1].text, code); let english = ToolActivity { call: &call, result: Some(&output), } - .details(Language::English); + .details(Language::English, None); assert_eq!(english[0].title, "Command"); assert_eq!(english[1].title, "Output"); assert_eq!(english[1].text, code); @@ -710,12 +837,203 @@ mod tests { call: &call, result: Some(&output), } - .details(Language::Chinese); + .details(Language::Chinese, None); assert_eq!(details[0].text, "-old\n+new"); assert!(details[0].diff); assert_eq!(details[1].text, "done"); } + #[test] + fn image_results_preserve_text_order_and_preview_each_image_without_dumping_data() { + use serde_json::json; + let run = Uuid::new_v4(); + let call = message( + run, + MessageKind::ToolCall, + Some("read"), + "Read\n{\"file_path\":\"missing.png\"}", + ); + let image = json!({"type": "image", "mimeType": "image/png", "data": "aW1hZ2U="}); + let claude = json!({"type": "image", "source": {"type": "base64", "media_type": "image/png", "data": "aW1hZ2U="}}); + let blocks = json!([ + {"type": "text", "text": "before"}, image, + {"type": "text", "text": "after"}, claude + ]); + let acp: Vec<_> = blocks + .as_array() + .unwrap() + .iter() + .map(|block| json!({"type": "content", "content": block})) + .collect(); + for payload in [blocks.clone(), json!({"content": blocks}), json!(acp)] { + let result = message( + run, + MessageKind::ToolResult, + Some("read"), + &payload.to_string(), + ); + let details = ToolActivity { + call: &call, + result: Some(&result), + } + .details(Language::English, None); + assert_eq!(details.len(), 5); + assert_eq!(details[1].text, "before"); + assert_eq!(details[3].text, "after"); + for index in [2, 4] { + assert_eq!(details[index].title, "Image preview"); + assert!(details[index].text.is_empty()); + assert!( + matches!(details[index].image.as_deref(), Some(ToolImage::Base64 { mime_type, data }) + if mime_type == "image/png" && data == "aW1hZ2U=") + ); + } + assert!( + details + .iter() + .all(|detail| !detail.text.contains("aW1hZ2U=")) + ); + assert_eq!( + result.content, + payload.to_string(), + "stored payload must stay intact" + ); + } + let result = message(run, MessageKind::ToolResult, None, &image.to_string()); + let details = ToolActivity { + call: &result, + result: Some(&result), + } + .details(Language::English, None); + assert_eq!(details.len(), 1); + assert!( + details[0].image.is_some(), + "unpaired legacy results also show previews" + ); + assert_eq!( + ToolActivity { + call: &result, + result: Some(&result) + } + .preview(Language::English), + "Tool output · Media preview" + ); + } + + #[test] + fn image_paths_use_the_task_directory_and_failed_reads_keep_the_diagnostic() { + let run = Uuid::new_v4(); + let directory = tempfile::tempdir().unwrap(); + let mut result = message( + run, + MessageKind::ToolResult, + Some("read"), + "Read image file", + ); + for name in ["Read", "read_file", "functions.view_image", "imageView"] { + let call = message( + run, + MessageKind::ToolCall, + Some("read"), + &format!("{name}\n{{\"path\":\"assets/IMAGE.PNG\"}}"), + ); + let activity = ToolActivity { + call: &call, + result: Some(&result), + }; + let details = activity.details(Language::English, Some(directory.path())); + assert!( + matches!(details[1].image.as_deref(), Some(ToolImage::Path(path)) + if path == &directory.path().join("assets/IMAGE.PNG")) + ); + assert!(details[1].text.is_empty()); + assert!( + activity.details(Language::English, None)[1].image.is_none(), + "never resolve relative paths against the desktop process directory" + ); + assert_eq!( + ToolActivity { + call: &call, + result: None + } + .details(Language::English, Some(directory.path())) + .len(), + 1 + ); + } + let path = directory.path().join("image-without-extension"); + let native = message( + run, + MessageKind::ToolResult, + None, + &serde_json::json!({"type": "imageView", "path": path, "status": "completed"}) + .to_string(), + ); + let details = ToolActivity { + call: &native, + result: Some(&native), + } + .details(Language::English, None); + assert_eq!(details.len(), 1); + assert!( + matches!(details[0].image.as_deref(), Some(ToolImage::Path(resolved)) if resolved == &path) + ); + let call = message( + run, + MessageKind::ToolCall, + Some("read"), + "Read\n{\"file_path\":\"image.png\"}", + ); + result.content = "permission denied".into(); + result.tool.as_mut().unwrap().is_error = true; + let details = ToolActivity { + call: &call, + result: Some(&result), + } + .details(Language::English, Some(directory.path())); + assert!(details[1].image.is_none()); + assert_eq!(details[1].text, "permission denied"); + let call = message( + run, + MessageKind::ToolCall, + Some("read"), + "read_file\n{\"path\":\"main.rs\"}", + ); + result.content = "fn main() {}".into(); + result.tool.as_mut().unwrap().is_error = false; + let details = ToolActivity { + call: &call, + result: Some(&result), + } + .details(Language::English, Some(directory.path())); + assert!(details[1].image.is_none()); + assert_eq!(details[1].text, "fn main() {}"); + } + + #[test] + fn malformed_images_and_unsupported_media_do_not_dump_binary_payloads() { + for payload in [ + serde_json::json!({"type": "image", "data": "opaque binary"}), + serde_json::json!({"type": "audio", "data": "opaque binary", "mimeType": "audio/wav"}), + ] { + let result = message( + Uuid::new_v4(), + MessageKind::ToolResult, + None, + &payload.to_string(), + ); + let details = ToolActivity { + call: &result, + result: Some(&result), + } + .details(Language::English, None); + assert_eq!(details.len(), 1); + assert!(details[0].image.is_none()); + assert!(!details[0].text.is_empty()); + assert!(!details[0].text.contains("opaque binary")); + } + } + #[test] fn file_events_without_diff_report_unavailable_details() { let call = message( @@ -728,7 +1046,7 @@ mod tests { call: &call, result: None, } - .details(Language::Chinese); + .details(Language::Chinese, None); assert_eq!(details[0].title, "main.rs"); assert!(details[0].text.contains("未提供")); assert!(!details[0].diff); diff --git a/apps/desktop/src/view/review.rs b/apps/desktop/src/view/review.rs index 8ec975a..70c713b 100644 --- a/apps/desktop/src/view/review.rs +++ b/apps/desktop/src/view/review.rs @@ -107,6 +107,7 @@ impl ReviewFile { .unwrap_or("text") .into(), diff, + image: None, } } } diff --git a/apps/desktop/src/view/tools.rs b/apps/desktop/src/view/tools.rs index 85928b9..0de9b54 100644 --- a/apps/desktop/src/view/tools.rs +++ b/apps/desktop/src/view/tools.rs @@ -1,9 +1,14 @@ use super::*; -use crate::model::tools::{ToolActivity, ToolCategory, ToolDetail}; -use gpui::{Axis, HighlightStyle}; +use crate::model::tools::{ToolActivity, ToolCategory, ToolDetail, ToolImage}; +use base64::Engine as _; +use gpui::{Axis, HighlightStyle, ObjectFit, StyledImage as _}; use gpui_kit::base::{ScrollableMask, TextView as CodeView, TextViewStyle as CodeStyle}; use gpui_kit::component::scroll::ScrollableElement as _; -use std::{ops::Range, rc::Rc, sync::LazyLock}; +use std::{ + ops::Range, + rc::Rc, + sync::{Arc, LazyLock}, +}; use syntect::{ easy::HighlightLines, highlighting::{FontStyle, ThemeSet}, @@ -58,11 +63,15 @@ impl NexusView { )); } let toggle_id = id.clone(); + let directory = self.presenter.model().working_directory().map(Path::new); let height = if batch.iter().any(|tool| { - tool.category() == ToolCategory::Edit - && self.expanded_messages.contains(&tool.call.id.into()) + self.expanded_messages.contains(&tool.call.id.into()) + && (tool.category() == ToolCategory::Edit + || tool_details(tool, locale, directory, window, cx) + .iter() + .any(|detail| detail.image.is_some())) }) { - // Show the complete diff card and a short result, not a clipped card + // Show the complete diff or image card and a short result, not a clipped card // inside a second scroll viewport. Other tool batches stay compact. 360. } else if batch @@ -179,13 +188,16 @@ impl NexusView { ToolCategory::Edit | ToolCategory::Create => IconName::FileText, ToolCategory::Other => IconName::Asterisk, }; + let result_id = tool.result.map(|result| result.id); let preview = window.use_keyed_state((ElementId::from(id), "preview"), cx, |_, _| { - (locale, tool.preview(locale)) + (result_id, locale, tool.preview(locale)) }); - if preview.read(cx).0 != locale { - preview.update(cx, |state, _| *state = (locale, tool.preview(locale))); + if preview.read(cx).0 != result_id || preview.read(cx).1 != locale { + preview.update(cx, |state, _| { + *state = (result_id, locale, tool.preview(locale)) + }); } - let preview = preview.read(cx).1.clone(); + let preview = preview.read(cx).2.clone(); div() .w_full() .min_w_0() @@ -252,17 +264,8 @@ impl NexusView { })), ) .when(expanded, |element| { - let result_id = tool.result.map(|result| result.id); - let details = - window.use_keyed_state((ElementId::from(id), "details"), cx, |_, _| { - (result_id, locale, Rc::new(tool.details(locale))) - }); - if details.read(cx).0 != result_id || details.read(cx).1 != locale { - details.update(cx, |state, _| { - *state = (result_id, locale, Rc::new(tool.details(locale))) - }); - } - let details = details.read(cx).2.clone(); + let directory = self.presenter.model().working_directory().map(Path::new); + let details = tool_details(tool, locale, directory, window, cx); element.child( div() .pl_6() @@ -296,6 +299,25 @@ impl NexusView { } } +fn tool_details( + tool: &ToolActivity<'_>, + locale: Language, + directory: Option<&Path>, + window: &mut Window, + cx: &mut gpui::App, +) -> Rc> { + let result_id = tool.result.map(|result| result.id); + let details = window.use_keyed_state((ElementId::from(tool.call.id), "details"), cx, |_, _| { + (result_id, locale, Rc::new(tool.details(locale, directory))) + }); + if details.read(cx).0 != result_id || details.read(cx).1 != locale { + details.update(cx, |state, _| { + *state = (result_id, locale, Rc::new(tool.details(locale, directory))) + }); + } + details.read(cx).2.clone() +} + pub(super) fn render_detail( key: SharedString, detail: &ToolDetail, @@ -350,6 +372,9 @@ fn render_detail_content( window: &mut Window, cx: &mut gpui::App, ) -> AnyElement { + if let Some(image) = &detail.image { + return render_image_detail(key, image, locale, window, cx); + } let colors = palette(cx); let dark = cx.global::().dark; let id = ElementId::from(key.clone()); @@ -639,6 +664,122 @@ fn render_detail_content( .into_any_element() } +fn image_source(image: &ToolImage) -> Option { + match image { + ToolImage::Base64 { mime_type, data } => { + let format = gpui::ImageFormat::from_mime_type(mime_type)?; + let bytes = base64::engine::general_purpose::STANDARD + .decode(data) + .ok()?; + Some(Arc::new(gpui::Image::from_bytes(format, bytes)).into()) + } + ToolImage::Path(path) => Some(path.clone().into()), + } +} + +fn render_image_detail( + key: SharedString, + image: &Arc, + locale: Language, + window: &mut Window, + cx: &mut gpui::App, +) -> AnyElement { + let colors = palette(cx); + let id = ElementId::from(key.clone()); + // Decode and hash the payload once per result, not on every scrolling frame. + let source = window.use_keyed_state((id.clone(), "image-source"), cx, |_, _| { + (image.clone(), image_source(image)) + }); + if !Arc::ptr_eq(&source.read(cx).0, image) { + source.update(cx, |state, _| *state = (image.clone(), image_source(image))); + } + let source = source.read(cx).1.clone(); + let expanded = window.use_keyed_state((id.clone(), "image-expanded"), cx, |_, _| false); + let is_expanded = *expanded.read(cx); + let height = px(if is_expanded { 360. } else { 160. }); + let fallback = move || { + div() + .size_full() + .flex() + .items_center() + .justify_center() + .text_size(px(12.)) + .text_color(rgb(colors.muted)) + .child(locale.text("无法预览此图片。")) + .into_any_element() + }; + let preview = source.map(|source| { + gpui::img(source) + .debug_selector({ + let key = key.clone(); + move || format!("{key}-image") + }) + .w_full() + .h(height) + .min_h_0() + .max_h(height) + .flex_none() + .object_fit(ObjectFit::Contain) + .with_fallback(fallback) + .into_any_element() + }); + div() + .w_full() + .min_w_0() + .flex_none() + .flex() + .flex_col() + .rounded(px(CONTROL_RADIUS)) + .border_1() + .border_color(rgb(colors.border)) + .bg(rgb(colors.surface)) + .overflow_hidden() + .child( + div() + .h(px(30.)) + .flex_none() + .px_3() + .flex() + .items_center() + .justify_between() + .text_size(px(12.)) + .text_color(rgb(colors.text_secondary)) + .child(locale.text("图片预览")) + .when(preview.is_some(), |element| { + element.child( + Button::new((id, "expand")) + .debug_selector({ + let key = key.clone(); + move || format!("{key}-expand") + }) + .ghost() + .small() + .h(px(24.)) + .label(locale.text(if is_expanded { + "缩小图片" + } else { + "放大图片" + })) + .on_click(move |_, _, cx| { + expanded.update(cx, |value, cx| { + *value = !*value; + cx.notify(); + }); + }), + ) + }), + ) + .child( + div() + .debug_selector(move || key.to_string()) + .w_full() + .h(height) + .flex_none() + .child(preview.unwrap_or_else(fallback)), + ) + .into_any_element() +} + fn fenced_code(code: &str, language: &str) -> String { let fence = "`".repeat( code.split(|character| character != '`') @@ -861,6 +1002,112 @@ pub(super) fn code_highlights( mod tests { use super::*; + const PNG: &str = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII="; + + #[test] + fn image_sources_reject_invalid_data_and_unsupported_formats() { + for (mime_type, data) in [("image/png", "not base64!"), ("image/unknown", PNG)] { + assert!( + image_source(&ToolImage::Base64 { + mime_type: mime_type.into(), + data: data.into() + }) + .is_none() + ); + } + } + + #[gpui::test] + fn tool_images_arriving_after_expansion_render_and_resize_without_code_controls( + cx: &mut gpui::TestAppContext, + ) { + use crate::presenter::tests::fixture; + use nexus_protocol::Event; + + cx.update(gpui_kit::init); + cx.update(theme::configure_theme); + let (mut presenter, runner, _directory) = fixture(); + assert!(presenter.submit("Read an image", "claude")); + let run_id = presenter.model().active_run.unwrap(); + runner.emit(Event::RunToolStarted { + run_id, + tool_id: "image".into(), + name: "Read".into(), + summary: "{\"file_path\":\"missing.png\"}".into(), + }); + presenter.drain_events(); + let id = presenter.model().messages.last().unwrap().id; + let (view, cx) = cx.add_window_view(|window, cx| { + let mut view = NexusView::new(presenter, window, cx); + view.set_appearance( + AppearanceSettings { + reduced_motion: true, + ..view.presenter.model().appearance + }, + window, + cx, + ); + view + }); + cx.simulate_resize(gpui::size(px(1280.), px(900.))); + cx.run_until_parked(); + let batch: &'static str = format!("tool-batch-{id}").leak(); + let row: &'static str = format!("tool-row-{id}").leak(); + let preview: &'static str = format!("tool-detail-{id}-1").leak(); + let image_selector: &'static str = format!("{preview}-image").leak(); + let expand: &'static str = format!("{preview}-expand").leak(); + let copy: &'static str = format!("{preview}-copy").leak(); + let code: &'static str = format!("{preview}-content").leak(); + for selector in [batch, row] { + let position = cx.debug_bounds(selector).unwrap().center(); + cx.simulate_click(position, Default::default()); + cx.run_until_parked(); + } + assert!(cx.debug_bounds(preview).is_none()); + runner.emit(Event::RunToolCompleted { run_id, tool_id: "image".into(), is_error: false, + output: serde_json::json!([{"type": "image", "source": {"type": "base64", "media_type": "image/png", "data": PNG}}]).to_string() }); + view.update(cx, |view, cx| view.poll_events(Instant::now(), cx)); + cx.run_until_parked(); + assert_eq!( + cx.debug_bounds(image_selector).unwrap().size.height, + px(160.) + ); + assert!(cx.debug_bounds(code).is_none()); + assert!( + cx.debug_bounds(copy).is_none(), + "binary data must not get text copy controls" + ); + let Some(gpui::ImageSource::Image(image)) = image_source(&ToolImage::Base64 { + mime_type: "image/png".into(), + data: PNG.into(), + }) else { + panic!("image source") + }; + cx.update(|window, cx| { + assert!(image.is_asset_cached(cx)); + let rendered = image + .get_render_image(window, cx) + .expect("PNG must actually decode"); + assert_eq!( + rendered.size(0), + gpui::size(gpui::DevicePixels(1), gpui::DevicePixels(1)) + ); + }); + for height in [360., 160.] { + let position = cx.debug_bounds(expand).unwrap().center(); + cx.simulate_click(position, Default::default()); + cx.run_until_parked(); + assert_eq!( + cx.debug_bounds(image_selector).unwrap().size.height, + px(height) + ); + } + let position = cx.debug_bounds(row).unwrap().center(); + cx.simulate_click(position, Default::default()); + cx.run_until_parked(); + assert!(cx.debug_bounds(preview).is_none()); + } + #[test] fn code_and_diff_highlights_preserve_unicode_and_addition_deletion_colors() { let code = "fn main() { println!(\"你好\"); }\n"; @@ -1066,6 +1313,7 @@ mod tests { text: text.clone(), language: "rs".into(), diff: true, + image: None, }, }); let draw = |cx: &mut gpui::VisualTestContext| { diff --git a/apps/runner/src/application/events.rs b/apps/runner/src/application/events.rs index 9c4d020..1474036 100644 --- a/apps/runner/src/application/events.rs +++ b/apps/runner/src/application/events.rs @@ -87,6 +87,14 @@ fn truncate_tool_output(mut output: String) -> String { if output.char_indices().nth(MAX_TOOL_OUTPUT_CHARS).is_none() { return output; } + // Binary payloads are preview data, not display text. Keep their JSON envelope + // valid while applying the text limit to accompanying content blocks. + let mut remaining = MAX_TOOL_OUTPUT_CHARS; + if let Ok(mut value) = serde_json::from_str::(&output) + && trim_media_output(&mut value, &mut remaining) + { + return value.to_string(); + } let retained_chars = MAX_TOOL_OUTPUT_CHARS - TOOL_OUTPUT_TRUNCATED_NOTICE.chars().count(); let boundary = output .char_indices() @@ -97,6 +105,49 @@ fn truncate_tool_output(mut output: String) -> String { output } +fn trim_media_output(value: &mut serde_json::Value, remaining: &mut usize) -> bool { + use serde_json::Value; + match value { + Value::Object(object) => { + if matches!( + object.get("type").and_then(Value::as_str), + Some("image" | "audio" | "video" | "document") + ) { + return true; + } + let mut media = false; + for (key, value) in object { + if key == "text" + && let Some(text) = value.as_str() + { + let count = text.chars().count(); + if count > *remaining { + let notice_len = TOOL_OUTPUT_TRUNCATED_NOTICE.chars().count(); + let retained = remaining.saturating_sub(notice_len); + let mut truncated: String = text.chars().take(retained).collect(); + if *remaining >= notice_len { + truncated.push_str(TOOL_OUTPUT_TRUNCATED_NOTICE); + } + *value = Value::String(truncated); + } + *remaining = remaining.saturating_sub(count); + } else { + media |= trim_media_output(value, remaining); + } + } + media + } + Value::Array(items) => { + let mut media = false; + for item in items { + media |= trim_media_output(item, remaining); + } + media + } + _ => false, + } +} + #[cfg(test)] mod tests { use super::*; @@ -132,6 +183,69 @@ mod tests { ); } + #[tokio::test] + async fn large_images_survive_emission_while_surrounding_text_is_bounded() { + use serde_json::{Value, json}; + let (emitter, mut events) = Emitter::channel(); + let data = "aW1hZ2U=".repeat(MAX_TOOL_OUTPUT_CHARS); + let blocks = json!([ + {"type": "text", "text": "界".repeat(MAX_TOOL_OUTPUT_CHARS + 1)}, + {"type": "image", "mimeType": "image/png", "data": data}, + {"type": "image", "source": {"type": "base64", "media_type": "image/png", "data": data}}, + {"type": "text", "text": "trailing text"} + ]); + let acp: Vec<_> = blocks + .as_array() + .unwrap() + .iter() + .map(|block| json!({"type": "content", "content": block})) + .collect(); + for (payload, wrapped) in [ + (blocks.clone(), false), + (json!({"content": blocks}), false), + (json!(acp), true), + ] { + emit_decoded( + Uuid::new_v4(), + DecodedEvent::ToolCompleted { + id: "image".into(), + output: payload.to_string(), + is_error: false, + }, + &emitter, + ) + .await; + let Event::RunToolCompleted { output, .. } = events.recv().await.unwrap().event else { + panic!("tool result") + }; + let value: Value = serde_json::from_str(&output).expect("media must remain valid JSON"); + let blocks = value + .as_array() + .or_else(|| value["content"].as_array()) + .unwrap(); + let block = |index: usize| { + if wrapped { + &blocks[index]["content"] + } else { + &blocks[index] + } + }; + assert_eq!(block(1)["data"], data); + assert_eq!(block(2)["source"]["data"], data); + assert!( + block(0)["text"] + .as_str() + .unwrap() + .ends_with(TOOL_OUTPUT_TRUNCATED_NOTICE) + ); + assert_eq!( + block(0)["text"].as_str().unwrap().chars().count(), + MAX_TOOL_OUTPUT_CHARS + ); + assert_eq!(block(3)["text"], ""); + } + } + #[tokio::test] async fn decoded_tool_outputs_are_truncated_before_emission() { let (emitter, mut events) = Emitter::channel(); diff --git a/crates/harness-acp/src/lib.rs b/crates/harness-acp/src/lib.rs index 979246c..8b98d2b 100644 --- a/crates/harness-acp/src/lib.rs +++ b/crates/harness-acp/src/lib.rs @@ -258,9 +258,21 @@ impl EventDecoder { if let Some(kind) = update["kind"].as_str() { tool.kind = kind.into(); } - if let Some(output) = update.get("rawOutput").filter(|v| !v.is_null()) { + let content = update.get("content").and_then(Value::as_array); + if content.is_some_and(|items| { + items.iter().any(|item| { + item["type"] == "content" + && matches!( + item["content"]["type"].as_str(), + Some("image" | "audio" | "video" | "document") + ) + }) + }) { + // Display content can carry images even when rawOutput is only a summary. + tool.output = tool_content(&update["content"]); + } else if let Some(output) = update.get("rawOutput").filter(|v| !v.is_null()) { tool.output = tool_content(output); - } else if let Some(content) = update.get("content").and_then(Value::as_array) { + } else if let Some(content) = content { tool.output = content .iter() .map(content_text) @@ -948,6 +960,30 @@ mod tests { } } } + #[test] + fn tool_results_preserve_mixed_image_content() { + let mut decoder = active(HarnessKind::Codebuddy, PermissionMode::Ask); + decode( + &mut decoder, + update(json!({"sessionUpdate": "tool_call", "toolCallId": "image", "title": "Read"})), + ); + let content = json!([ + {"type": "content", "content": {"type": "text", "text": "Read image"}}, + {"type": "content", "content": {"type": "image", "mimeType": "image/png", "data": "aW1hZ2U="}} + ]); + let events = decode( + &mut decoder, + update( + json!({"sessionUpdate": "tool_call_update", "toolCallId": "image", + "status": "completed", "content": content, "rawOutput": "Read image"}), + ), + ); + assert!( + matches!(events.as_slice(), [DecodedEvent::ToolCompleted {output, is_error: false, ..}] + if serde_json::from_str::(output).unwrap() == content) + ); + } + #[test] fn streams_text_tools_and_terminal_failure_without_repeating_completed_tools() { let mut decoder = active(HarnessKind::Codebuddy, PermissionMode::Ask); diff --git a/crates/harness-core/src/rpc.rs b/crates/harness-core/src/rpc.rs index 4b3ce39..f787044 100644 --- a/crates/harness-core/src/rpc.rs +++ b/crates/harness-core/src/rpc.rs @@ -327,7 +327,13 @@ fn decode_message(frame: &Value) -> Vec { fn format_tool_result(result: &Value) -> String { // Edit tools include the actual patch in details (including hashline edits). - if result.get("details").is_some() { + // Mixed content must retain images instead of keeping only the text blocks. + if result.get("details").is_some() + || result + .get("content") + .and_then(Value::as_array) + .is_some_and(|blocks| blocks.iter().any(|block| block["type"] != "text")) + { return tool_content(result); } let text = result diff --git a/crates/harness-omp/src/lib.rs b/crates/harness-omp/src/lib.rs index 0b174fe..b6a460a 100644 --- a/crates/harness-omp/src/lib.rs +++ b/crates/harness-omp/src/lib.rs @@ -1066,6 +1066,17 @@ mod tests { matches!(decoder.decode_line(&plain.to_string()).unwrap().as_slice(), [DecodedEvent::ToolCompleted {output,..}] if output == &code) ); + let media = json!({"content": [ + {"type": "text", "text": "Read image"}, + {"type": "image", "mimeType": "image/png", "data": "aW1hZ2U="} + ]}); + let completed = json!({"type": "tool_execution_end", "toolCallId": "image", + "result": media, "isError": false}); + assert!( + matches!(decoder.decode_line(&completed.to_string()).unwrap().as_slice(), + [DecodedEvent::ToolCompleted { output, .. }] + if serde_json::from_str::(output).unwrap() == media) + ); } #[test]