diff --git a/docs/user/tui-and-sessions.md b/docs/user/tui-and-sessions.md index 9620a08..4782682 100644 --- a/docs/user/tui-and-sessions.md +++ b/docs/user/tui-and-sessions.md @@ -101,13 +101,13 @@ An accepted file appears in the editor as `[Image #N]` or `[Audio #N]`. Add surr If you press `Enter` while native clipboard pastes are pending, submission waits for those pastes without blocking the terminal. A failed paste or further editing cancels automatic submission and keeps the draft available. Clearing the prompt or switching sessions discards stale clipboard results so they cannot populate another draft. -Native clipboard reads, clipboard image encoding, and reconstructed-image validation run on a bounded background worker so they do not pause terminal input or redraws. Clipboard files and reconstructed image links use bounded, session-owned temporary storage. On resume, Kit reconstructs local image links from available message content without reading stale paths or downloading URLs. Invalid or unavailable content, or an exhausted temporary-file budget, leaves the placeholder without a local image link. Kit deletes its temporary files on session switch or exit; original files you attached by path are not deleted. +Native clipboard reads, clipboard image encoding, and reconstructed-image validation run on a bounded background worker so they do not pause terminal input or redraws. Clipboard files and reconstructed image links use bounded, session-owned temporary storage. On resume, Kit reconstructs local image links from available message content without reading stale paths or downloading URLs. The stable reconstructed-link cache holds at most 64 files / 64 MiB. If that budget is exhausted but image bytes are still retained, the underlined placeholder remains openable with an ordinary Kit click: a bounded worker validates and materializes it on demand. These overflow placeholders do not support terminal modifier-click / OSC-8 opening. Opened overflow files are reused by image identity and retained until session switch or exit, so opening another image does not invalidate a file already handed to a viewer. This separate, non-evicting pool holds at most 64 files / 32 MiB (matching the retained image-source byte budget), with the unchanged 10 MiB per-image limit. Once the pool is full, Kit shows a notice instead of removing older files; previously opened images remain openable. Starting a new session releases these budgets. Retained base64 image sources are separately capped at 32 MiB; exceeding that limit shows a notice, and labels without either retained bytes or a stable link cannot be opened on demand. Invalid or unavailable content cannot be opened; Kit shows a notice if an on-demand open fails. Kit deletes its temporary files on session switch or exit; original files you attached by path are not deleted. Terminal compatibility: OSC-8 links are understood by current VS Code, Ghostty, Kitty, WezTerm, iTerm2, and many other terminals, subject to their link settings. For older terminals, use Kit's ordinary-click fallback. Multiplexers such as tmux must support or pass through the keyboard, paste, and hyperlink sequences; terminal-native shortcuts can differ from a direct session. Clipboard access and `file://` paths are local to the machine running Kit: over SSH, Kit cannot read your laptop's clipboard or make remote temporary files accessible to your local editor. Transfer an image to the remote host and paste its path instead. The model-facing prompt retains canonical `file://` Markdown links, while Kit also reads and sends the file bytes because remote providers cannot access local files. Image and audio acceptance remains model-dependent. Kit supports these request shapes through OpenRouter and OpenAI subscription; an individual model can still reject a modality it does not support. Video is not supported. -User-attached images render inline as a bounded static first frame when Kit detects Kitty, Sixel, or iTerm2 graphics support. No setting is required. Unsupported terminals, malformed or oversized images, and decode failures retain the safe clickable attachment label. Animated GIF and WebP files currently show only their first frame. Structured assistant and tool images use the same static preview renderer. Audio is not played. Only bounded `file://`, `http://`, and `https://` links are displayed; base64 and `data:` URLs are never copied into terminal text or Markdown links. +User-attached images remain compact attachment labels without inline previews. Structured assistant and tool images use the static preview renderer; animated GIF and WebP previews show only their first frame. Audio is not played. Only bounded `file://`, `http://`, and `https://` links are displayed; base64 and `data:` URLs are never copied into terminal text or Markdown links. ### Interrupt a running turn or quit diff --git a/src/tui/app.rs b/src/tui/app.rs index 5d2dfd8..9fb10bc 100644 --- a/src/tui/app.rs +++ b/src/tui/app.rs @@ -82,6 +82,8 @@ pub enum Update { images: Vec, append: bool, }, + /// A bounded worker request from an internal image-placeholder click. + OpenUserImage(UserImage), /// Agent prose, either appended as a chunk or replaced by an upsert. AgentMessage { id: String, @@ -326,32 +328,50 @@ fn replace_image_uri_on_line( line: usize, source_uri: Option<&str>, local_uri: Option<&str>, -) { - let Some(source_uri) = source_uri else { - return; - }; +) -> Option> { let start = if line == 0 { 0 } else if let Some((index, _)) = text.match_indices('\n').nth(line - 1) { index + 1 } else { - return; + return None; }; let end = text[start..] .find('\n') .map_or(text.len(), |offset| start + offset); + let Some(source_uri) = source_uri else { + // URI-less image blocks get their own generated placeholder line during + // translation. Link only that label, never arbitrary neighboring text. + let label = &text[start..end]; + if let Some(number) = label + .strip_prefix("[Image #") + .and_then(|label| label.strip_suffix(']')) + && !number.is_empty() + && number.bytes().all(|byte| byte.is_ascii_digit()) + { + if let Some(local_uri) = local_uri { + text.insert_str(end, &format!("({local_uri})")); + } + return Some(start..end); + } + return None; + }; let destination = markdown::image_label_link_destinations(&text[start..end]) .into_iter() .find_map(|(range, uri)| (uri == source_uri).then_some(range)); if let Some(destination) = destination { let destination = start + destination.start..start + destination.end; + let label_end = destination.start - 1; + let label_start = text[start..label_end].rfind('[')? + start; if let Some(local_uri) = local_uri { text.replace_range(destination, local_uri); } else { // Remove the complete `](destination)` suffix, leaving the label as plain text. text.replace_range(destination.start - 2..destination.end + 1, "]"); } + return Some(label_start..label_end); } + None } #[derive(Clone, Debug, PartialEq, Eq)] @@ -403,6 +423,7 @@ pub(super) enum ClipboardMode { } pub enum Action { + OpenUserImage(UserImage), Voice(String), None, Redraw, @@ -527,11 +548,18 @@ pub struct UserImage { pub(super) data: String, pub(super) mime_type: String, pub(super) source_uri: Option, - /// Source line after which the fixed image viewport is reserved. + /// Source line carrying this image's clickable placeholder label. pub(super) line: usize, + /// Exact plain-label bytes trusted by translation/rewrite, never a parsed URI. + pub(super) open_label: Option>, } impl UserImage { + /// Internal hit target only: never sent to an OS URI handler or OSC 8. + pub(super) fn open_target(&self) -> String { + format!("kit-image:{}", blake3::Hash::from(self.key).to_hex()) + } + pub(super) fn new(data: String, mime_type: String, line: usize) -> Option { Self::with_source(data, mime_type, line, None) } @@ -572,6 +600,7 @@ impl UserImage { mime_type, source_uri, line, + open_label: None, }) } } @@ -799,6 +828,7 @@ pub struct App { pub(super) transcript_prefixes: Vec, pub(super) transcript_cache_width: usize, retained_image_source_bytes: usize, + image_source_limit_noticed: bool, attachment_cache: SessionAttachmentCache, next_transcript_revision: u64, transcript_focus_index: Option, @@ -1066,6 +1096,7 @@ impl App { transcript_prefixes: vec![0], transcript_cache_width: 0, retained_image_source_bytes: 0, + image_source_limit_noticed: false, attachment_cache: SessionAttachmentCache::default(), next_transcript_revision: 0, transcript_focus_index: None, @@ -1745,19 +1776,34 @@ impl App { self.next_attachment = self.next_attachment.max(self.submitted_attachment); } let mut images = images; - for image in &images { + for index in 0..images.len() { + let image = &images[index]; if image .source_uri .as_deref() - .is_some_and(|uri| uri.starts_with("file:")) + .is_none_or(|uri| uri.starts_with("file:")) { let uri = self.attachment_cache.image_uri(image.key); - replace_image_uri_on_line( + let old_len = text.len(); + let label = replace_image_uri_on_line( &mut text, image.line, image.source_uri.as_deref(), uri.as_deref(), ); + if let Some(label) = label { + // Later rewrites may precede already associated labels (ACP + // image order need not match text order). + for previous in &mut images[..index] { + if let Some(range) = &mut previous.open_label + && range.start >= label.end + { + range.start = range.start + text.len() - old_len; + range.end = range.end + text.len() - old_len; + } + } + images[index].open_label = uri.is_none().then_some(label); + } } } let existing_index = self.message_blocks.get(&id).copied(); @@ -1774,6 +1820,7 @@ impl App { self.retained_image_source_bytes = self.retained_image_source_bytes.saturating_sub(replaced); } + let source_count = images.len(); images.retain(|image| { let retained = self .retained_image_source_bytes @@ -1785,6 +1832,11 @@ impl App { true } }); + // Notify once per session; later messages and resume replays stay quiet. + if images.len() < source_count && !self.image_source_limit_noticed { + self.image_source_limit_noticed = true; + self.toast("image source limit reached; start a new session to retain more images"); + } if let Some(index) = existing_index { let mut changed = false; match (&mut self.blocks[index], role) { @@ -1804,8 +1856,13 @@ impl App { } let line_offset = existing.text.bytes().filter(|&byte| byte == b'\n').count(); + let byte_offset = existing.text.len(); existing.text.push_str(&text); for image in &mut images { + if let Some(range) = &mut image.open_label { + range.start += byte_offset; + range.end += byte_offset; + } image.line += line_offset; } existing.images.extend(std::mem::take(&mut images)); @@ -1997,6 +2054,22 @@ impl App { } pub(super) fn apply_materialized(&mut self, update: Update, images: Vec) { + if matches!(update, Update::OpenUserImage(_)) { + use super::attachment::RetainOpenedError; + let result = images + .into_iter() + .next() + .ok_or(RetainOpenedError::InvalidPath) + .and_then(|image| self.attachment_cache.retain_opened(image)); + match result { + Ok(uri) => open_url(&uri), + Err(RetainOpenedError::Exhausted) => { + self.toast("image open limit reached; start a new session to open more images") + } + Err(RetainOpenedError::InvalidPath) => self.toast("image could not be opened"), + } + return; + } for image in images { self.attachment_cache.admit(image); } @@ -2005,6 +2078,7 @@ impl App { pub fn apply(&mut self, update: Update) { match update { + Update::OpenUserImage(_) => {} Update::VoicePromptAccepted { .. } => {} Update::A2aAddress(address) => self.a2a = address, Update::SessionCatalog(result) => { @@ -2856,6 +2930,7 @@ impl App { self.transcript_prefixes.push(0); self.transcript_cache_width = 0; self.retained_image_source_bytes = 0; + self.image_source_limit_noticed = false; self.attachment_cache.clear(); self.transcript_focus_index = None; self.clear_attachments(); @@ -4438,6 +4513,26 @@ impl App { return Action::None; } if let Some(url) = self.clicked_link(column, offset) { + if url.starts_with("kit-image:") { + let Some(image) = self.blocks.iter().find_map(|block| { + let Block::User(message) = block else { + return None; + }; + message + .images + .iter() + .find(|image| image.open_target() == url) + }) else { + return Action::None; + }; + // Files retained by an earlier open or admission need no worker + // round trip; only misses copy the source and materialize. + if let Some(uri) = self.attachment_cache.image_uri(image.key) { + open_url(&uri); + return Action::None; + } + return Action::OpenUserImage(image.clone()); + } open_url(&url); return Action::None; } @@ -5182,6 +5277,366 @@ mod tests { ); } + #[test] + fn uri_less_user_image_rewrite_only_links_generated_placeholder_lines() { + for label in [ + "[Image #1]", + "[Image #12]", + "prose [Image #1]", + "`[Image #1]`", + "[Image #]", + "[Image #x]", + "[Image #1](https://example.com/image.png)", + ] { + let mut text = format!("before\n{label}\nafter"); + replace_image_uri_on_line(&mut text, 1, None, Some("file:///tmp/local.png")); + let expected = if matches!(label, "[Image #1]" | "[Image #12]") { + format!("before\n{label}(file:///tmp/local.png)\nafter") + } else { + format!("before\n{label}\nafter") + }; + assert_eq!(text, expected); + } + } + + #[tokio::test] + async fn exhausted_image_cache_keeps_placeholders_openable_on_demand() { + use super::super::{BackgroundCompletion, QueuedUpdate, spawn_background_workers}; + use base64::Engine as _; + let (completed, mut completions) = tokio::sync::mpsc::channel(1); + let workers = spawn_background_workers(completed).unwrap(); + let mut app = app(); + let mut first_uri = None; + for color in 0..65u8 { + let mut png = std::io::Cursor::new(Vec::new()); + image::DynamicImage::ImageRgb8(image::RgbImage::from_pixel( + 1, + 1, + image::Rgb([color, 0, 0]), + )) + .write_to(&mut png, image::ImageFormat::Png) + .unwrap(); + let image = UserImage::new( + base64::engine::general_purpose::STANDARD.encode(png.into_inner()), + "image/png".into(), + 0, + ) + .unwrap(); + assert!( + workers + .try_update(QueuedUpdate::for_session( + 0, + Update::UserMessage { + id: color.to_string(), + text: "[Image #1]".into(), + images: vec![image], + append: false, + } + )) + .is_ok() + ); + let BackgroundCompletion::Update { queued, images } = completions.recv().await.unwrap() + else { + panic!("expected update"); + }; + app.apply_materialized(queued.update, images); + if color == 0 { + let Block::User(message) = &app.blocks[0] else { + panic!("user"); + }; + first_uri = app.attachment_cache.image_uri(message.images[0].key); + } + } + let Block::User(last) = app.blocks.last().unwrap() else { + panic!("user"); + }; + assert_eq!(last.text, "[Image #1]"); + let key = last.images[0].key; + assert!(app.attachment_cache.image_uri(key).is_none()); + let mut terminal = + ratatui::Terminal::new(ratatui::backend::TestBackend::new(60, 20)).unwrap(); + let mut runtime = super::super::image::ImageRuntime::with_picker( + ratatui_image::picker::Picker::halfblocks(), + ); + terminal + .draw(|frame| super::super::ui::draw(frame, &mut app, &mut runtime)) + .unwrap(); + let (row, hit) = app + .row_links + .iter() + .enumerate() + .find_map(|(row, hits)| { + hits.iter() + .find(|hit| hit.url.starts_with("kit-image:")) + .map(|hit| (row, hit.clone())) + }) + .unwrap(); + let mouse = |kind| MouseEvent { + kind, + column: (app.transcript_left + hit.start) as u16, + row: (app.transcript_top + row) as u16, + modifiers: KeyModifiers::NONE, + }; + let down = mouse(MouseEventKind::Down(MouseButton::Left)); + let up = mouse(MouseEventKind::Up(MouseButton::Left)); + app.handle_mouse(down); + let Action::OpenUserImage(image) = app.handle_mouse(up) else { + panic!("expected open action"); + }; + assert_eq!(image.key, key); + assert!( + workers + .try_update(QueuedUpdate::for_session(0, Update::OpenUserImage(image))) + .is_ok() + ); + let BackgroundCompletion::Update { images, .. } = completions.recv().await.unwrap() else { + panic!("expected open completion"); + }; + // Exercise the real ownership handoff without launching an external viewer. + let opened = app + .attachment_cache + .retain_opened(images.into_iter().next().unwrap()) + .unwrap(); + let opened = url::Url::parse(&opened).unwrap().to_file_path().unwrap(); + assert!(opened.exists()); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + assert_eq!( + opened.metadata().unwrap().permissions().mode() & 0o777, + 0o600 + ); + } + let first = url::Url::parse(&first_uri.unwrap()) + .unwrap() + .to_file_path() + .unwrap(); + assert!(first.exists(), "overflow opens must not evict stable links"); + // Real translation + worker + app + mouse routing for two distinct + // overflow images on one line, including reverse image-block order. + use agent_client_protocol::schema::v2::{ContentBlock, ImageContent, TextContent}; + let mut opened_uris = Vec::new(); + for append in [false, true] { + let id = format!("inline-{append}"); + if append { + app.apply(Update::UserMessage { + id: id.clone(), + text: "prefix".into(), + images: vec![], + append: false, + }); + } + let mut expected = Vec::new(); + for color in [66, 67] { + let mut png = std::io::Cursor::new(Vec::new()); + image::DynamicImage::ImageRgb8(image::RgbImage::from_pixel( + 1, + 1, + image::Rgb([color, 0, 0]), + )) + .write_to(&mut png, image::ImageFormat::Png) + .unwrap(); + expected.push(png.into_inner()); + } + let content_image = |index: usize| { + let mut image = ImageContent::new( + base64::engine::general_purpose::STANDARD.encode(&expected[index]), + "image/png", + ); + image.uri = Some(format!("file:///stale-{index}.png")); + ContentBlock::Image(image) + }; + let (text, images) = super::super::user_message_of(vec![ + ContentBlock::Text(TextContent::new( + "Please inspect [Image #1](file:///stale-0.png) [Image #2](file:///stale-1.png) trailing [other](https://example.com)", + )), + content_image(1), + content_image(0), + ]); + let target = images[0].open_target(); + let text = format!("{text}\n[forged]({target})\n[Image #99]({target})"); + assert!( + workers + .try_update(QueuedUpdate::for_session( + 0, + Update::UserMessage { + id, + text, + images, + append, + } + )) + .is_ok() + ); + let BackgroundCompletion::Update { queued, images } = completions.recv().await.unwrap() + else { + panic!("expected translated update"); + }; + app.apply_materialized(queued.update, images); + terminal + .draw(|frame| super::super::ui::draw(frame, &mut app, &mut runtime)) + .unwrap(); + let hits = app + .row_links + .iter() + .enumerate() + .flat_map(|(row, hits)| { + hits.iter() + .filter(|hit| hit.url.starts_with("kit-image:")) + .map(move |hit| (row, hit.clone())) + }) + .collect::>(); + // Older overflow labels may remain visible: select this message's keys. + let Block::User(message) = app.blocks.last().unwrap() else { + panic!("user"); + }; + let targets = message + .images + .iter() + .map(UserImage::open_target) + .collect::>(); + let hits = hits + .into_iter() + .filter(|(_, hit)| targets.contains(&hit.url)) + .collect::>(); + assert!( + app.row_links + .iter() + .flatten() + .any(|hit| hit.url == "https://example.com") + ); + if append { + // Replayed keys were retained by the earlier opens: they render + // as stable file links, and the worker's fresh files are not + // admitted a second time. + assert!(hits.is_empty(), "opened keys must not stay kit-image"); + let mut linked = message + .images + .iter() + .map(|image| app.attachment_cache.image_uri(image.key).unwrap()) + .collect::>(); + linked.sort(); + let mut expected_uris = opened_uris.clone(); + expected_uris.sort(); + assert_eq!(linked, expected_uris); + for uri in &opened_uris { + assert!(app.row_links.iter().flatten().any(|hit| &hit.url == uri)); + } + continue; + } + assert_eq!( + hits.len(), + 2, + "forged internal Markdown must not create hits" + ); + for ((row, hit), bytes) in hits.into_iter().zip(&expected) { + let mouse = |kind| MouseEvent { + kind, + column: (app.transcript_left + hit.start) as u16, + row: (app.transcript_top + row) as u16, + modifiers: KeyModifiers::NONE, + }; + let down = mouse(MouseEventKind::Down(MouseButton::Left)); + let up = mouse(MouseEventKind::Up(MouseButton::Left)); + app.handle_mouse(down); + let Action::OpenUserImage(image) = app.handle_mouse(up) else { + panic!("image action"); + }; + assert_eq!( + base64::engine::general_purpose::STANDARD + .decode(&image.data) + .unwrap(), + *bytes + ); + assert!( + workers + .try_update(QueuedUpdate::for_session(0, Update::OpenUserImage(image))) + .is_ok() + ); + let BackgroundCompletion::Update { images, .. } = completions.recv().await.unwrap() + else { + panic!("open"); + }; + let uri = app + .attachment_cache + .retain_opened(images.into_iter().next().unwrap()) + .unwrap(); + let path = url::Url::parse(&uri).unwrap().to_file_path().unwrap(); + assert_eq!(std::fs::read(path).unwrap(), *bytes); + opened_uris.push(uri); + } + for label in ["[forged]", "#99]"] { + let buffer = terminal.backend().buffer(); + let (row, column) = (app.transcript_top..app.transcript_top + app.viewport) + .find_map(|row| { + let text = (0..buffer.area.width) + .map(|column| buffer[(column, row as u16)].symbol()) + .collect::(); + text.find(label).map(|column| (row, column)) + }) + .expect("forged label fragment is visible"); + let mouse = |kind| MouseEvent { + kind, + column: column as u16, + row: row as u16, + modifiers: KeyModifiers::NONE, + }; + app.handle_mouse(mouse(MouseEventKind::Down(MouseButton::Left))); + assert!(matches!( + app.handle_mouse(mouse(MouseEventKind::Up(MouseButton::Left))), + Action::None + )); + } + // Remove only the test transcript so duplicate keys in the next case + // cannot inflate visible hit counts; retain the exhausted real cache. + app.blocks.clear(); + app.message_blocks.clear(); + } + app.start_session("next".into()); + assert!(!opened.exists()); + assert!(!first.exists()); + } + + #[test] + fn exhausted_open_pool_reports_actionable_notice_without_launching() { + use base64::Engine as _; + let mut app = app(); + for color in 0..=64 { + let mut png = std::io::Cursor::new(Vec::new()); + image::DynamicImage::ImageRgb8(image::RgbImage::from_pixel( + 1, + 1, + image::Rgb([color, 0, 0]), + )) + .write_to(&mut png, image::ImageFormat::Png) + .unwrap(); + let image = UserImage::new( + base64::engine::general_purpose::STANDARD.encode(png.into_inner()), + "image/png".into(), + 0, + ) + .unwrap(); + let materialized = super::super::attachment::materialize_image( + image.key, + &image.data, + &image.mime_type, + usize::MAX, + ) + .unwrap(); + if color < 64 { + app.attachment_cache.retain_opened(materialized).unwrap(); + } else { + // Admission failure takes the real completion path, which must + // report the refusal rather than launch an unowned path. + app.apply_materialized(Update::OpenUserImage(image), vec![materialized]); + assert_eq!( + app.toast_text(), + Some("image open limit reached; start a new session to open more images",) + ); + } + } + } + #[test] fn replayed_native_image_links_are_reused_and_live_for_the_session() { use base64::Engine as _; @@ -5282,12 +5737,44 @@ mod tests { }); } + assert_eq!( + app.toast_text(), + Some("image source limit reached; start a new session to retain more images") + ); assert_eq!(app.retained_image_source_bytes, source_bytes * 3); assert!(app.retained_image_source_bytes <= MAX_RETAINED_IMAGE_SOURCE_BYTES); assert!(matches!( app.blocks.last(), Some(Block::User(message)) if message.images.is_empty() )); + + // Later refusals stay quiet so the notice cannot clobber other toasts. + app.toast("copied resume command"); + let image = UserImage::new("A".repeat(source_bytes), "image/png".into(), 0).unwrap(); + app.apply(Update::UserMessage { + id: "image-4".into(), + text: "[Image #4]".into(), + images: vec![image], + append: false, + }); + assert_eq!(app.toast_text(), Some("copied resume command")); + + // A fresh session notifies again. + app.start_session("next".into()); + app.toast = None; + for index in 0..4 { + let image = UserImage::new("A".repeat(source_bytes), "image/png".into(), 0).unwrap(); + app.apply(Update::UserMessage { + id: format!("next-{index}"), + text: format!("[Image #{index}]"), + images: vec![image], + append: false, + }); + } + assert_eq!( + app.toast_text(), + Some("image source limit reached; start a new session to retain more images") + ); } fn compose(app: &mut App, script: &str) { diff --git a/src/tui/attachment.rs b/src/tui/attachment.rs index 27e138e..dd8d542 100644 --- a/src/tui/attachment.rs +++ b/src/tui/attachment.rs @@ -6,6 +6,7 @@ use tempfile::{Builder, TempPath}; const MAX_SESSION_FILES: usize = 64; const MAX_SESSION_BYTES: usize = 64 * 1024 * 1024; +const MAX_OPENED_BYTES: usize = super::app::MAX_RETAINED_IMAGE_SOURCE_BYTES; const MAX_DECODED_ALLOCATION: u64 = 64 * 1024 * 1024; const MAX_DIMENSION: u32 = 8_192; const MAX_IMAGE_BYTES: usize = 10 * 1024 * 1024; @@ -121,17 +122,54 @@ pub(super) fn materialize_image( pub(super) struct SessionAttachmentCache { files: HashMap<[u8; 32], Arc>, bytes: usize, + // Opened overflow files stay alive for external viewers until session cleanup. + opened: HashMap<[u8; 32], Arc>, + opened_bytes: usize, +} + +#[derive(Debug, PartialEq, Eq)] +pub(super) enum RetainOpenedError { + Exhausted, + InvalidPath, } impl SessionAttachmentCache { + /// Retains overflow images without invalidating paths handed to external + /// viewers. Repeated keys reuse their original file, even when the pool is + /// full. The separate pool is bounded by the retained image-source budget. + pub(super) fn retain_opened( + &mut self, + image: MaterializedImage, + ) -> Result { + if let Some(file) = self.file(image.key) { + return url::Url::from_file_path(file.path()) + .map(|uri| uri.to_string()) + .map_err(|()| RetainOpenedError::InvalidPath); + } + if self.opened.len() >= MAX_SESSION_FILES + || image.bytes > MAX_OPENED_BYTES.saturating_sub(self.opened_bytes) + { + return Err(RetainOpenedError::Exhausted); + } + let uri = url::Url::from_file_path(image.file.path()) + .map_err(|()| RetainOpenedError::InvalidPath)? + .to_string(); + self.opened_bytes += image.bytes; + self.opened.insert(image.key, image.file); + Ok(uri) + } + pub(super) fn clear(&mut self) { self.files.clear(); self.bytes = 0; + self.opened.clear(); + self.opened_bytes = 0; } /// Admits a worker-produced file without evicting existing session links. + /// Keys already retained by an open reuse that file instead. pub(super) fn admit(&mut self, image: MaterializedImage) { - if self.files.contains_key(&image.key) + if self.file(image.key).is_some() || self.files.len() >= MAX_SESSION_FILES || image.bytes > MAX_SESSION_BYTES.saturating_sub(self.bytes) { @@ -141,8 +179,13 @@ impl SessionAttachmentCache { self.files.insert(image.key, image.file); } + fn file(&self, key: [u8; 32]) -> Option<&Arc> { + self.files.get(&key).or_else(|| self.opened.get(&key)) + } + + /// Resolves a stable link for keys held by either pool. pub(super) fn image_uri(&self, key: [u8; 32]) -> Option { - let file = self.files.get(&key)?; + let file = self.file(key)?; url::Url::from_file_path(file.path()) .ok() .map(|uri| uri.to_string()) @@ -154,6 +197,162 @@ impl SessionAttachmentCache { mod tests { use super::*; + fn overflow_image(color: u8) -> MaterializedImage { + materialize_image( + [color; 32], + &STANDARD.encode(source(ImageFormat::Png, color)), + "image/png", + MAX_IMAGE_BYTES, + ) + .unwrap() + } + + #[test] + fn overflow_pool_retains_a_and_b_until_clear_or_drop() { + for clear in [false, true] { + let mut cache = SessionAttachmentCache::default(); + let a = overflow_image(1); + let a_path = a.file.path().to_owned(); + cache.retain_opened(a).unwrap(); + let b = overflow_image(2); + let b_path = b.file.path().to_owned(); + cache.retain_opened(b).unwrap(); + assert!(a_path.exists()); + assert!(b_path.exists()); + if clear { + cache.clear(); + assert!(!a_path.exists()); + assert!(!b_path.exists()); + cache.retain_opened(overflow_image(1)).unwrap(); + } + drop(cache); + assert!(!a_path.exists()); + assert!(!b_path.exists()); + } + } + + #[test] + fn admit_and_lookup_reuse_files_retained_by_open() { + let mut cache = SessionAttachmentCache::default(); + let opened = cache.retain_opened(overflow_image(1)).unwrap(); + assert_eq!(cache.image_uri([1; 32]), Some(opened.clone())); + let duplicate = overflow_image(1); + let duplicate_path = duplicate.file.path().to_owned(); + cache.admit(duplicate); + assert!( + !duplicate_path.exists(), + "opened keys must not be admitted twice" + ); + assert!(cache.files.is_empty()); + assert_eq!(cache.bytes, 0); + assert_eq!(cache.image_uri([1; 32]), Some(opened)); + } + + #[test] + fn overflow_pool_reuses_duplicates_and_refuses_new_files_when_full() { + let mut cache = SessionAttachmentCache::default(); + let mut paths = Vec::new(); + let mut first_uri = String::new(); + for color in 0..MAX_SESSION_FILES { + let image = overflow_image(color as u8); + paths.push(image.file.path().to_owned()); + let uri = cache.retain_opened(image).unwrap(); + if color == 0 { + first_uri = uri; + } + } + let duplicate = overflow_image(0); + let duplicate_path = duplicate.file.path().to_owned(); + assert_eq!(cache.retain_opened(duplicate).unwrap(), first_uri); + assert!(!duplicate_path.exists()); + let refused = overflow_image(MAX_SESSION_FILES as u8); + let refused_path = refused.file.path().to_owned(); + assert_eq!( + cache.retain_opened(refused), + Err(RetainOpenedError::Exhausted) + ); + assert!(!refused_path.exists()); + assert!(paths.iter().all(|path| path.exists())); + cache.clear(); + assert!(paths.iter().all(|path| !path.exists())); + cache.retain_opened(overflow_image(0)).unwrap(); + } + + #[test] + fn overflow_pool_refuses_byte_exhaustion_without_evicting() { + let mut cache = SessionAttachmentCache::default(); + let mut paths = Vec::new(); + // Trailing PNG bytes are preserved by materialization, allowing the + // real validation and storage APIs to exercise the byte budget. + for color in 0..3 { + let mut bytes = source(ImageFormat::Png, color); + bytes.resize(MAX_IMAGE_BYTES, 0); + let image = materialize_image( + [color; 32], + &STANDARD.encode(bytes), + "image/png", + MAX_IMAGE_BYTES, + ) + .unwrap(); + paths.push(image.file.path().to_owned()); + cache.retain_opened(image).unwrap(); + } + let mut bytes = source(ImageFormat::Png, 3); + bytes.resize(MAX_IMAGE_BYTES, 0); + let image = materialize_image( + [3; 32], + &STANDARD.encode(bytes), + "image/png", + MAX_IMAGE_BYTES, + ) + .unwrap(); + let refused_path = image.file.path().to_owned(); + assert_eq!( + cache.retain_opened(image), + Err(RetainOpenedError::Exhausted) + ); + assert!(!refused_path.exists()); + assert!(paths.iter().all(|path| path.exists())); + cache.clear(); + assert!(paths.iter().all(|path| !path.exists())); + cache.retain_opened(overflow_image(0)).unwrap(); + } + + #[tokio::test] + async fn stale_on_demand_completion_releases_private_file() { + use super::super::app::{Update, UserImage}; + use super::super::{ + ActiveSessionRoute, BackgroundCompletion, QueuedUpdate, accept_queued_update, + spawn_background_workers, + }; + let (completed, mut completions) = tokio::sync::mpsc::channel(1); + let workers = spawn_background_workers(completed).unwrap(); + let image = UserImage::new( + STANDARD.encode(source(ImageFormat::Png, 1)), + "image/png".into(), + 0, + ) + .unwrap(); + assert!( + workers + .try_update(QueuedUpdate::for_session(1, Update::OpenUserImage(image))) + .is_ok() + ); + let BackgroundCompletion::Update { queued, images } = completions.recv().await.unwrap() + else { + panic!("expected update"); + }; + let path = images[0].file.path().to_owned(); + assert!(path.exists()); + let route = Arc::new(std::sync::Mutex::new(ActiveSessionRoute { + id: "new".into(), + generation: 2, + })); + assert!(accept_queued_update(&route, queued).is_none()); + drop(images); + assert!(!path.exists()); + } + fn source(format: ImageFormat, color: u8) -> Vec { let image = image::RgbImage::from_pixel(1, 1, image::Rgb([color, 40, 60])); let mut bytes = Cursor::new(Vec::new()); diff --git a/src/tui/hyperlinks.rs b/src/tui/hyperlinks.rs index df5ffe0..345fa79 100644 --- a/src/tui/hyperlinks.rs +++ b/src/tui/hyperlinks.rs @@ -73,6 +73,11 @@ impl HyperlinkRenderer { continue; }; for hit in hits { + // Internal on-demand targets require a normal TUI click; + // terminal modifier-click must not dispatch them to the OS. + if hit.url.starts_with("kit-image:") { + continue; + } let Some(start) = transcript_left .checked_add(hit.start) .and_then(|value| u16::try_from(value).ok()) @@ -268,6 +273,22 @@ mod tests { } } + #[test] + fn internal_image_targets_are_not_emitted_as_native_hyperlinks() { + let buffer = Buffer::with_lines([Line::from("[Image #1]")]); + let rows = vec![vec![LinkHit { + start: 0, + end: 10, + url: "kit-image:internal".into(), + }]]; + let mut renderer = HyperlinkRenderer::default(); + let prepared = renderer.prepare(&frame(&buffer), &rows, 0, 0, false); + let capture = Capture::default(); + let mut backend = CrosstermBackend::new(capture.clone()); + renderer.draw(&mut backend, prepared).unwrap(); + assert!(!has_nonempty_open(&capture.bytes())); + } + #[test] fn emits_native_open_and_close_around_linked_cells_and_preserves_cursor() { let buffer = Buffer::with_lines([Line::from("sent Image #1")]); diff --git a/src/tui/markdown.rs b/src/tui/markdown.rs index f7cf3fc..1fd957b 100644 --- a/src/tui/markdown.rs +++ b/src/tui/markdown.rs @@ -733,6 +733,7 @@ pub(super) fn image_label_link_destinations(source: &str) -> Vec<(Range, false, 0, Some(&mut destinations), + &[], ); destinations } @@ -802,6 +803,17 @@ pub(super) fn inline_spans(source: &str, base: Style) -> Vec { inline_with_link_destinations(source, base, false) } +/// Render internal image hits only at caller-associated source ranges. The +/// ordinary Markdown parser still rejects internal URI schemes, including when +/// an attacker copies a real image target into another link. +pub(super) fn inline_spans_with_image_labels( + source: &str, + base: Style, + labels: &[(Range, String)], +) -> Vec { + inline_with_link_destinations_and_ranges(source, base, false, 0, None, labels) +} + /// Splits inline links, emphasis, and code spans out of one line of Markdown. fn inline(source: &str, base: Style) -> Vec { inline_with_link_destinations(source, base, true) @@ -812,7 +824,7 @@ fn inline_with_link_destinations( base: Style, show_link_destinations: bool, ) -> Vec { - inline_with_link_destinations_and_ranges(source, base, show_link_destinations, 0, None) + inline_with_link_destinations_and_ranges(source, base, show_link_destinations, 0, None, &[]) } fn inline_with_link_destinations_and_ranges( @@ -821,6 +833,7 @@ fn inline_with_link_destinations_and_ranges( show_link_destinations: bool, offset: usize, mut destinations: Option<&mut Vec<(Range, String)>>, + labels: &[(Range, String)], ) -> Vec { let mut spans = Vec::new(); let mut plain = String::new(); @@ -831,6 +844,30 @@ fn inline_with_link_destinations_and_ranges( .map(|index| (index, &rest[index..])); let image = image_references(rest).into_iter().next(); let link = next_link(rest); + let consumed = offset + source.len() - rest.len(); + let label = labels + .iter() + .find(|(range, _)| range.start >= consumed && range.end <= consumed + rest.len()); + if let Some((range, target)) = label { + let start = range.start - consumed; + let end = range.end - consumed; + if marker.as_ref().is_none_or(|(index, _)| start < *index) + && image.as_ref().is_none_or(|(range, _)| start <= range.start) + && link.as_ref().is_none_or(|link| start <= link.start) + { + plain.push_str(&rest[..start]); + if !plain.is_empty() { + spans.push(plain_span(std::mem::take(&mut plain), base)); + } + spans.push(link_span( + rest[start..end].to_owned(), + base.add_modifier(Modifier::UNDERLINED), + target, + )); + rest = &rest[end..]; + continue; + } + } if let Some((range, _)) = image && marker .as_ref() @@ -943,6 +980,7 @@ fn inline_with_link_destinations_and_ranges( show_link_destinations, offset + source.len() - body.len(), destinations.as_deref_mut(), + labels, )); } rest = &body[close + delimiter.len()..]; @@ -999,6 +1037,38 @@ mod tests { .collect() } + #[test] + fn trusted_image_labels_preserve_surrounding_emphasis_and_reject_forged_uris() { + let text = + "**Please [Image #1] inspect** [other](https://example.com) [forged](kit-image:one)"; + let start = text.find("[Image #1]").unwrap(); + let spans = super::inline_spans_with_image_labels( + text, + ratatui::style::Style::default(), + &[(start..start + "[Image #1]".len(), "kit-image:one".into())], + ); + let links = spans + .iter() + .filter(|span| span.url.is_some()) + .collect::>(); + assert_eq!(links.len(), 2); + assert_eq!(links[0].span.content, "[Image #1]"); + assert!( + links[0] + .span + .style + .add_modifier + .contains(ratatui::style::Modifier::BOLD) + ); + assert_eq!(links[1].url.as_deref(), Some("https://example.com")); + let visible = spans + .iter() + .map(|span| span.span.content.as_ref()) + .collect::(); + assert!(!visible.contains("**")); + assert!(visible.contains("[forged](kit-image:one)")); + } + #[test] fn renders_aligned_pipe_tables() { let rendered = diff --git a/src/tui/mod.rs b/src/tui/mod.rs index 676fbf5..d2f967a 100644 --- a/src/tui/mod.rs +++ b/src/tui/mod.rs @@ -625,13 +625,17 @@ fn spawn_background_workers( let images = match &queued.update { Update::UserMessage { images, .. } => images .iter() - .take_while(|_| !update_stopping.load(Ordering::Acquire)) + // File-backed placeholders need materialization before they can + // open. Give them priority over URI-less image candidates. .filter(|image| { image .source_uri .as_deref() .is_some_and(|uri| uri.starts_with("file:")) }) + .chain(images.iter().filter(|image| image.source_uri.is_none())) + .take_while(|_| !update_stopping.load(Ordering::Acquire)) + // Bound attempts, including failed decodes, across both groups. .take(64) .filter_map(|image| { let prepared = attachment::materialize_image( @@ -647,6 +651,16 @@ fn spawn_background_workers( Some(prepared) }) .collect(), + // Sources were bounded at ingest; materialization enforces + // its own per-image limit. + Update::OpenUserImage(image) => attachment::materialize_image( + image.key, + &image.data, + &image.mime_type, + usize::MAX, + ) + .into_iter() + .collect(), _ => Vec::new(), }; if !send_background_completion( @@ -2471,6 +2485,22 @@ pub async fn run_with_reasoning_effort_and_openrouter_key( )?; events = EventStream::new(); } + Action::OpenUserImage(image) => { + // Snapshot only; release the guard before queueing work. + // Poison rejects this request rather than selecting another session. + let generation = transition_session.lock().ok().map(|route| route.generation); + if let Some(generation) = generation { + match background_workers.try_update(QueuedUpdate::for_session( + generation, Update::OpenUserImage(image), + )) { + Ok(()) => {} + Err(error) => match *error { + std::sync::mpsc::TrySendError::Full(_) => app.note("image worker is busy; click again to open"), + std::sync::mpsc::TrySendError::Disconnected(_) => app.note("image worker is unavailable"), + }, + } + } + } Action::Copy(text) => { execute!(terminal.backend_mut(), Print(osc52(&text))) .map_err(agent_client_protocol::Error::into_internal_error)?; @@ -3744,7 +3774,9 @@ fn user_message_of(blocks: Vec) -> (String, Vec) { .map(|(line, uri)| (first_line + line, uri)), ); text.push_str(&content); - separate_after_image = false; + if !content.is_empty() { + separate_after_image = false; + } } } } @@ -5234,6 +5266,87 @@ mod tests { } } + #[tokio::test] + async fn background_image_worker_prioritizes_files_and_bounds_decode_attempts() { + use super::app::UserImage; + + let mut png = std::io::Cursor::new(Vec::new()); + image::DynamicImage::new_rgb8(1, 1) + .write_to(&mut png, image::ImageFormat::Png) + .unwrap(); + let encoded = STANDARD.encode(png.into_inner()); + let (completed, mut completions) = tokio::sync::mpsc::channel(1); + let workers = super::spawn_background_workers(completed).unwrap(); + + for invalid_files in [0, 63, 64] { + let mut images: Vec<_> = (0..64) + .map(|_| UserImage::new("not base64!".into(), "image/png".into(), 0).unwrap()) + .collect(); + images.extend((0..invalid_files).map(|_| { + UserImage::with_source( + "not base64!".into(), + "image/png".into(), + 0, + Some("file:///invalid.png".into()), + ) + .unwrap() + })); + images.push( + UserImage::with_source( + encoded.clone(), + "image/png".into(), + 0, + Some("file:///valid.png".into()), + ) + .unwrap(), + ); + assert!( + workers + .try_update(QueuedUpdate::for_session( + 1, + Update::UserMessage { + id: "user".into(), + text: "[Image #1](file:///valid.png)".into(), + images, + append: false, + }, + )) + .is_ok() + ); + let completion = + tokio::time::timeout(std::time::Duration::from_secs(5), completions.recv()) + .await + .unwrap() + .unwrap(); + let BackgroundCompletion::Update { queued, images } = completion else { + panic!("expected image worker completion"); + }; + assert_eq!(images.len(), usize::from(invalid_files < 64)); + let mut app = App::new( + "/tmp/kit".into(), + "provider".into(), + "model".into(), + "a2a".into(), + ); + app.apply_materialized(queued.update, images); + let super::app::Block::User(message) = &app.blocks[0] else { + panic!("expected user"); + }; + if invalid_files < 64 { + let (_, uri) = super::markdown::image_label_links(&message.text) + .pop() + .unwrap(); + let path = url::Url::parse(&uri).unwrap().to_file_path().unwrap(); + assert_eq!( + std::fs::read(path).unwrap(), + STANDARD.decode(&encoded).unwrap() + ); + } else { + assert_eq!(message.text, "[Image #1]"); + } + } + } + #[test] fn assistant_image_translation_defers_decode_to_worker() { let update = agent_content_update( @@ -6707,6 +6820,118 @@ mod tests { assert_eq!(images[0].line, 1); } + #[test] + fn sequential_user_image_chunks_preserve_materialized_links_and_spacing() { + use super::{app::Block, attachment}; + + let mut app = App::new( + PathBuf::from("/tmp"), + "provider".into(), + "model".into(), + "a2a".into(), + ); + let image_block = |value| { + let mut png = std::io::Cursor::new(Vec::new()); + image::DynamicImage::ImageRgb8(image::RgbImage::from_pixel( + 1, + 1, + image::Rgb([value, 0, 0]), + )) + .write_to(&mut png, image::ImageFormat::Png) + .unwrap(); + ContentBlock::Image(wire::ImageContent::new( + STANDARD.encode(png.into_inner()), + "image/png", + )) + }; + for content in [ + ContentBlock::Text(TextContent::new("before")), + image_block(0), + ContentBlock::Text(TextContent::new("")), + ContentBlock::Text(TextContent::new("after")), + image_block(255), + ContentBlock::Text(TextContent::new("end")), + ] { + let notification = UpdateSessionNotification::new( + "session", + SessionUpdate::UserMessageChunk( + serde_json::from_value(json!({ + "messageId": "user", + "content": content, + })) + .unwrap(), + ), + ); + let updates = translate_for_session(notification, "session"); + assert_eq!(updates.len(), 1); + for update in updates { + let Update::UserMessage { + id, images, append, .. + } = &update + else { + panic!("expected a user message chunk"); + }; + assert_eq!(id, "user"); + assert!(*append); + let materialized = images + .iter() + .map(|image| { + assert!(image.source_uri.is_none()); + attachment::materialize_image( + image.key, + &image.data, + &image.mime_type, + usize::MAX, + ) + .unwrap() + }) + .collect(); + app.apply_materialized(update, materialized); + } + } + + let [Block::User(message)] = app.blocks.as_slice() else { + panic!("expected one user message"); + }; + assert_eq!(message.images.len(), 2); + assert_ne!(message.images[0].key, message.images[1].key); + assert_eq!(message.images[0].line, 1); + assert_eq!(message.images[1].line, 3); + let lines: Vec<_> = message.text.lines().collect(); + assert_eq!(lines.len(), 5); + assert_eq!(lines[0], "before"); + assert_eq!(lines[2], "after"); + assert_eq!(lines[4], "end"); + let links = super::markdown::image_label_links(&message.text); + assert_eq!(links.len(), 2); + assert_ne!(links[0].1, links[1].1); + for ((line, uri), image) in links.iter().zip(&message.images) { + assert_eq!(*line, image.line); + assert!(uri.starts_with("file://")); + let path = url::Url::parse(uri).unwrap().to_file_path().unwrap(); + assert_eq!( + std::fs::read(path).unwrap(), + STANDARD.decode(&image.data).unwrap() + ); + } + } + + #[test] + fn user_image_placeholder_separator_survives_empty_text() { + let (text, images) = user_message_of(vec![ + ContentBlock::Image(agent_client_protocol::schema::v2::ImageContent::new( + "AQID", + "image/png", + )), + ContentBlock::Text(TextContent::new("")), + ContentBlock::Text(TextContent::new("after")), + ]); + + assert_eq!(text, "[Image #1]\nafter"); + assert_eq!(images.len(), 1); + assert_eq!(images[0].line, 0); + } + #[test] fn image_deduplication_requires_the_exact_trusted_link() { let actual = "file:///tmp/actual.png"; diff --git a/src/tui/ui.rs b/src/tui/ui.rs index be8aa59..2a9cf35 100644 --- a/src/tui/ui.rs +++ b/src/tui/ui.rs @@ -1144,11 +1144,11 @@ fn draw_transcript(frame: &mut Frame<'_>, app: &mut App, images: &mut ImageRunti } continue; } - let sources = match app.blocks.get(block_index) { - Some(Block::User(message)) => &message.images, - Some(Block::Tool(call)) => &call.images, - _ => continue, + // User images render as clickable labels, never as inline viewports. + let Some(Block::Tool(call)) = app.blocks.get(block_index) else { + continue; }; + let sources = &call.images; let Some(source) = sources.get(source_index) else { continue; }; @@ -1318,47 +1318,37 @@ fn refresh_transcript_cache_with_images(app: &mut App, images: &mut ImageRuntime } } -fn user_block_rows( - message: &UserMessage, - width: usize, - reserve_images: bool, -) -> (Vec, Vec) { +fn user_block_rows(message: &UserMessage, width: usize) -> Vec { let mut rows = Vec::new(); - let mut placements = Vec::new(); + let mut line_start = 0; for (line_index, text) in message.text.split('\n').enumerate() { + let mut line = user_line("", line_index == 0); + let mut labels = message + .images + .iter() + .filter_map(|image| { + let range = image.open_label.as_ref()?; + (range.start >= line_start && range.end <= line_start + text.len()).then(|| { + ( + range.start - line_start..range.end - line_start, + image.open_target(), + ) + }) + }) + .collect::>(); + labels.sort_by_key(|(range, _)| range.start); + line.spans.extend(markdown::inline_spans_with_image_labels( + text, + theme::bold(theme::text_color()), + &labels, + )); + line_start += text.len() + 1; rows.extend(wrap_linked_tagged( - &[( - user_line(text, line_index == 0), - (None, None, Some(line_index)), - )], + &[(line, (None, None, Some(line_index)))], width, )); - if reserve_images { - for (source, _) in message - .images - .iter() - .enumerate() - .filter(|(_, image)| image.line == line_index) - { - let row = rows.len(); - rows.extend((0..RESERVED_ROWS).map(|_| { - ( - Line::default(), - (None, None, None), - Vec::new(), - String::new(), - ) - })); - placements.push(CachedTranscriptImage { - block: None, - source, - row, - destination: None, - }); - } - } } - (rows, placements) + rows } /// Place viewports at complete image boundaries before wrapping following prose. @@ -1564,7 +1554,7 @@ fn single_transcript_block_rows( ) -> (Vec, Vec) { let block = &app.blocks[block_index]; let (block_lines, call) = match block { - Block::User(message) => return user_block_rows(message, width, reserve_images), + Block::User(message) => return (user_block_rows(message, width), Vec::new()), Block::Agent(text) => return agent_block_rows(text, block_index, width, reserve_images), Block::AgentParts(parts) => { return agent_parts_rows(parts, block_index, width, reserve_images); @@ -5752,23 +5742,21 @@ mod tests { } #[test] - fn image_rows_preserve_text_image_text_display_order() { + fn user_image_placeholders_preserve_explicit_text_newlines() { let image = UserImage::new("AQID".into(), "image/png".into(), 1).unwrap(); let message = UserMessage { - text: "before\n[Image #1]\nafter".into(), + text: "before\n[Image #1](file:///tmp/image.png)\n\nafter\n".into(), images: vec![image], }; - let (rows, placements) = user_block_rows(&message, 40, true); + let rows = user_block_rows(&message, 40); - assert_eq!(placements.len(), 1); - let after = &rows[placements[0].row + usize::from(super::RESERVED_ROWS)].0; - assert!( - after - .spans - .iter() - .any(|span| span.content.contains("after")) + assert_eq!( + rows.iter().map(|row| line_text(&row.0)).collect::>(), + ["› before", " Image #1", " ", " after", " "] ); + assert_eq!(rows[1].2.len(), 1); + assert_eq!(rows[1].2[0].url, "file:///tmp/image.png"); } #[test] @@ -5839,7 +5827,7 @@ mod tests { } #[test] - fn image_rows_are_fixed_and_decoding_is_lazy() { + fn user_images_remain_clickable_placeholders_with_image_runtime() { let mut png = std::io::Cursor::new(Vec::new()); image::DynamicImage::new_rgb8(400, 200) .write_to(&mut png, image::ImageFormat::Png) @@ -5862,62 +5850,141 @@ mod tests { })); let mut images = crate::tui::image::ImageRuntime::with_picker(Picker::halfblocks()); - refresh_transcript_cache_with_images(&mut app, &mut images, 12); - assert_eq!(images.cached_entries(), 0, "layout must not decode images"); - let narrow = app.transcript_cache[0].as_ref().unwrap(); - assert_eq!(narrow.images.len(), 1); - assert!(narrow.rows.len() > 1); - let narrow_rows = narrow.rows.len(); - assert_eq!(app.transcript_prefixes.last().copied(), Some(narrow_rows)); - - refresh_transcript_cache_with_images(&mut app, &mut images, 40); - assert_eq!(images.cached_entries(), 0, "width changes stay lazy"); - let wide = app.transcript_cache[0].as_ref().unwrap(); - assert_eq!(wide.images.len(), 1); - assert_eq!(wide.rows.len(), narrow_rows); + for width in [12, 40] { + refresh_transcript_cache_with_images(&mut app, &mut images, width); + let cached = app.transcript_cache[0].as_ref().unwrap(); + assert!(cached.images.is_empty()); + assert_eq!(cached.rows.len(), 1); + assert_eq!(line_text(&cached.rows[0].0), "› Image #1"); + assert_eq!(cached.rows[0].2.len(), 1); + assert_eq!(cached.rows[0].2[0].url, "file:///tmp/image.png"); + assert_eq!(app.transcript_prefixes.last().copied(), Some(1)); + } let mut terminal = Terminal::new(TestBackend::new(40, 20)).unwrap(); terminal .draw(|frame| draw(frame, &mut app, &mut images)) .unwrap(); - assert_eq!(images.cached_entries(), 0, "visible image decode is queued"); - wait_for_image_decode(&mut images); - terminal - .draw(|frame| draw(frame, &mut app, &mut images)) - .unwrap(); - assert_eq!(images.cached_entries(), 1, "visible image is rendered"); - assert!(buffer_contains_black_image_cell( + assert!(!images.pending(), "user images must not queue decoding"); + assert_eq!(images.cached_entries(), 0); + assert!(!buffer_contains_black_image_cell( terminal.backend().buffer() )); - let reserved_rows = app.transcript_cache[0].as_ref().unwrap().rows.len(); - - images.clear(); - assert_eq!( - images.cached_entries(), - 0, - "decoded image cache was evicted" + assert!( + app.row_links + .iter() + .flatten() + .any(|hit| hit.url == "file:///tmp/image.png") ); - terminal - .draw(|frame| draw(frame, &mut app, &mut images)) - .unwrap(); - assert_eq!(images.cached_entries(), 0, "evicted image decode is queued"); - wait_for_image_decode(&mut images); - terminal - .draw(|frame| draw(frame, &mut app, &mut images)) + } + + #[tokio::test] + async fn uri_less_user_images_translate_to_local_placeholder_links_without_preview_gaps() { + use crate::tui::{ + BackgroundCompletion, QueuedUpdate, spawn_background_workers, user_message_of, + }; + use agent_client_protocol::schema::v2::{ContentBlock, ImageContent, TextContent}; + + let mut png = std::io::Cursor::new(Vec::new()); + image::DynamicImage::new_rgb8(2, 1) + .write_to(&mut png, image::ImageFormat::Png) .unwrap(); - assert_eq!( - images.cached_entries(), - 1, - "an evicted visible image is rendered again after decoding" - ); - assert!(buffer_contains_black_image_cell( - terminal.backend().buffer() - )); - assert_eq!( - app.transcript_cache[0].as_ref().unwrap().rows.len(), - reserved_rows, - "cache eviction cannot remove reserved transcript rows" - ); + let bytes = png.into_inner(); + let encoded = base64::engine::general_purpose::STANDARD.encode(&bytes); + let (completed, mut completions) = tokio::sync::mpsc::channel(1); + let workers = spawn_background_workers(completed).unwrap(); + + // Full replay messages and replay chunks share this translation path. + for append in [false, true] { + for payload in [encoded.as_str(), "invalid base64"] { + let (text, sources) = user_message_of(vec![ + ContentBlock::Text(TextContent::new("before")), + ContentBlock::Image(ImageContent::new(payload, "image/png")), + ContentBlock::Text(TextContent::new("after")), + ]); + assert_eq!(text, "before\n[Image #1]\nafter"); + assert!( + workers + .try_update(QueuedUpdate { + generation: None, + update: crate::tui::app::Update::UserMessage { + id: "replayed".into(), + text, + images: sources, + append, + }, + }) + .is_ok() + ); + let BackgroundCompletion::Update { + queued, + images: prepared, + } = completions.recv().await.unwrap() + else { + panic!("expected update") + }; + let mut app = App::new( + PathBuf::from("/tmp/kit"), + "openai-subscription".into(), + "gpt-5.4".into(), + "0:0".into(), + ); + app.apply_materialized(queued.update, prepared); + let mut images = crate::tui::image::ImageRuntime::with_picker(Picker::halfblocks()); + for width in [12, 40] { + refresh_transcript_cache_with_images(&mut app, &mut images, width); + let cached = app.transcript_cache[0].as_ref().unwrap(); + assert!(cached.images.is_empty()); + assert_eq!(cached.rows.len(), 3); + assert_eq!(app.transcript_prefixes.last().copied(), Some(3)); + } + let cached = app.transcript_cache[0].as_ref().unwrap(); + let links = &cached.rows[1].2; + if payload == encoded { + assert_eq!(line_text(&cached.rows[1].0), " Image #1"); + assert_eq!(links.len(), 1); + let path = url::Url::parse(&links[0].url) + .unwrap() + .to_file_path() + .unwrap(); + assert_eq!(std::fs::read(&path).unwrap(), bytes); + app.start_session("replacement".into()); + assert!(!path.exists(), "session switch must release the local file"); + } else { + assert_eq!(links.len(), 1); + assert!( + links[0].url.starts_with("kit-image:"), + "invalid data must not create a dead native file link" + ); + let Block::User(message) = &app.blocks[0] else { + panic!("expected user"); + }; + assert!( + workers + .try_update(QueuedUpdate { + generation: None, + update: crate::tui::app::Update::OpenUserImage( + message.images[0].clone() + ), + }) + .is_ok() + ); + let BackgroundCompletion::Update { queued, images } = + completions.recv().await.unwrap() + else { + panic!("expected open completion"); + }; + assert!( + images.is_empty(), + "invalid data must not create a temporary file" + ); + app.apply_materialized(queued.update, images); + assert_eq!(app.toast_text(), Some("image could not be opened")); + } + assert!(!images.pending()); + assert_eq!(images.cached_entries(), 0); + } + } } #[test]