From fb94f9f05a6aff7fa71eb50667a780de62e58d8b Mon Sep 17 00:00:00 2001 From: daniel Date: Tue, 8 Sep 2026 16:50:24 +0100 Subject: [PATCH 1/4] feat(tui): unify image presentation across message origins --- Cargo.lock | 18 + Cargo.toml | 1 + docs/plans/compose-managed-files.md | 6 + docs/user/tui-and-sessions.md | 18 + src/managed_files.rs | 32 +- src/managed_files/tests.rs | 41 ++ src/tui/app.rs | 354 +++++++++++--- src/tui/image.rs | 564 ++++++++++++++++------ src/tui/image_source.rs | 709 ++++++++++++++++++++++++++++ src/tui/markdown.rs | 299 +++++++++++- src/tui/markdown_images.rs | 288 +++++++++++ src/tui/mod.rs | 227 +++++++-- src/tui/ui.rs | 570 +++++++++++++++++++--- 13 files changed, 2832 insertions(+), 295 deletions(-) create mode 100644 src/tui/image_source.rs create mode 100644 src/tui/markdown_images.rs diff --git a/Cargo.lock b/Cargo.lock index 9370d3a..b9d93b0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2670,6 +2670,7 @@ dependencies = [ "opentelemetry 0.32.0", "opentelemetry-otlp", "opentelemetry_sdk", + "pulldown-cmark", "ratatui", "ratatui-image", "reqwest", @@ -3394,6 +3395,17 @@ dependencies = [ "prost", ] +[[package]] +name = "pulldown-cmark" +version = "0.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e8bbe1a966bd2f362681a44f6edce3c2310ac21e4d5067a6e7ec396297a6ea0" +dependencies = [ + "bitflags 2.13.1", + "memchr", + "unicase", +] + [[package]] name = "pxfm" version = "0.1.30" @@ -5040,6 +5052,12 @@ version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "eaea85b334db583fe3274d12b4cd1880032beab409c0d774be044d4480ab9a94" +[[package]] +name = "unicase" +version = "2.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dbc4bc3a9f746d862c45cb89d705aa10f187bb96c76001afab07a0d35ce60142" + [[package]] name = "unicode-general-category" version = "1.1.0" diff --git a/Cargo.toml b/Cargo.toml index 97d2763..e6e456c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -49,6 +49,7 @@ neo_frizbee = "=0.11.0" opentelemetry = { version = "=0.32.0", default-features = false, features = ["trace"] } opentelemetry-otlp = { version = "=0.32.0", default-features = false, features = ["grpc-tonic", "http-proto", "http-json", "reqwest-client", "tls-webpki-roots", "trace"] } opentelemetry_sdk = { version = "=0.32.1", default-features = false, features = ["experimental_trace_batch_span_processor_with_async_runtime", "rt-tokio", "trace"] } +pulldown-cmark = { version = "=0.13.0", default-features = false } ratatui = { version = "=0.30.2", default-features = false, features = ["crossterm", "layout-cache"] } ratatui-image = { version = "=11.0.8", default-features = false, features = ["crossterm"] } reqwest = { version = "=0.13.4", default-features = false, features = ["blocking", "form", "rustls", "stream"] } diff --git a/docs/plans/compose-managed-files.md b/docs/plans/compose-managed-files.md index f7704f1..95a5898 100644 --- a/docs/plans/compose-managed-files.md +++ b/docs/plans/compose-managed-files.md @@ -105,6 +105,12 @@ Generalize user-image rendering into reusable media presentation for user attach Markdown image nodes such as `![Edited image](kit-file://file_opaque_id)` resolve through the managed file resolver. Local paths follow filesystem permission policy. Remote images require explicit network policy, asynchronous bounded fetch/decode, redirect/address validation, and no implicit credentials. Do not fetch arbitrary model-supplied URLs without the applicable authorization. Parse real image nodes, not regexes scanning code fences. Retain alt text, source links, placeholders, and terminal fallback. Ordinary links remain links unless explicitly previewed. TUI display never automatically attaches pixels to model context. +### Phase 4 presentation contract + +The TUI retains one presentation-only message/image representation for native user, assistant, and tool media. CommonMark image nodes use source-aware layout without splitting Markdown documents. Managed references resolve under current-session authority; external sources are temporary snapshots and are re-resolved after cache reset or replay. Duplicate typed/Markdown viewports require independently authorized, exact resolved content identity within the same message; repeated Markdown nodes remain visible. No display path imports files, grants access, or adds model-context attachments. + +Remote loading is default-denied and requires the startup `KIT_TUI_IMAGE_ORIGINS` exact-origin HTTPS policy. Redirects are rejected, every resolved address must be public, and DNS results are pinned while TLS hostname checks remain active. The dedicated client has no ambient proxy or credential configuration. Acquisition and the common decode/protocol renderer have separate bounded worker admission; blocking DNS/file/decode operations retain their permits even after cancellation. The [TUI guide](../user/tui-and-sessions.md#images-in-the-transcript) specifies fallback, cache, replay, and local-file semantics. + ## Milestones and PR stack | Phase | Branch / PR base | Owner | Scope and completion milestone | diff --git a/docs/user/tui-and-sessions.md b/docs/user/tui-and-sessions.md index 874b2f2..de0f281 100644 --- a/docs/user/tui-and-sessions.md +++ b/docs/user/tui-and-sessions.md @@ -23,6 +23,24 @@ The catalog requires an existing directory and is workspace-filtered and newest- A session ID must be 1–128 ASCII letters, digits, `-`, or `_`. `kit prompt` uses the same durable sessions: it prints `session_id: ` after its answer, and that ID can be continued by either `kit prompt --resume ` or `kit tui --resume `. +## Images in the transcript + +User attachments, native assistant-generated images, typed tool results, and Markdown image nodes use the same terminal image renderer. Image loading and decoding are asynchronous. Loading or unavailable images retain text placeholders; unsupported terminal graphics retain readable text and source links. Image viewports keep a fixed height and refit to the terminal width after resize. Display is presentation-only: it never attaches pixels to a prompt, grants File access, or changes provider requests. + +Markdown images use CommonMark image syntax, including reference-style images. Ordinary links, escaped image syntax, and images inside code do not load. An incomplete streamed image node remains text until it parses as an image. Explicit repeated image nodes remain separate occurrences. A typed attachment and a Markdown image are deduplicated only after the resolved bytes identify the same image; an inaccessible Markdown source does not hide the typed attachment. + +- `![Edited image](kit-file://file_<64-hex-digits>)` resolves through the current session's managed-file authority. An ID alone cannot grant access to another session's image. +- Relative paths resolve from the session project root. Absolute paths and local `file:` URLs use OS process permissions, including symlink targets; only bounded regular files are read. This is not a workspace filesystem sandbox. +- Remote images are **disabled by default**. To authorize anonymous HTTPS image loads from an exact origin, start Kit with an explicit environment policy, for example: + + ```sh + KIT_TUI_IMAGE_ORIGINS=https://images.example.com kit tui --root /path/to/project + ``` + + Multiple origins are comma-separated. Authorization is for the entire origin, not an individual path. URLs with credentials, nondefault ports, nonpublic destination addresses, or redirects are rejected. Each DNS address must pass validation and the connection is pinned to the validated addresses while retaining TLS hostname verification. The dedicated client does not inherit proxies, cookies, or authorization headers. Reads, connection time, response size, decode work, and concurrent jobs are bounded. An ordinary HTTPS link is still only a link. + +External Markdown images are temporary presentation snapshots, not managed imports. Cache reset or session replay re-resolves local files and remote URLs; changes outside Kit can therefore change those images. Managed and typed images retain their original snapshot semantics. Session changes invalidate cached authorization and prevent stale work from publishing into the new session. Already running blocking work can finish before its bounded worker slot is released. Missing, denied, and corrupt sources are negatively cached rather than retried on every redraw. Restart the TUI to change the remote-origin policy. + ## Recovering from full storage Kit routes its internal persistence through a shared filesystem service. If a write fails because storage is full or a quota is exceeded, the service retains the pending change in a bounded memory overlay. Internal reads and session listings use the same view, so finishing a turn or closing a session handle does not discard accepted changes. An existing session can be reopened in the same running process while persistence is pending. diff --git a/src/managed_files.rs b/src/managed_files.rs index 788826b..760e300 100644 --- a/src/managed_files.rs +++ b/src/managed_files.rs @@ -450,11 +450,34 @@ impl FileStore { pub(crate) fn resolve(&self, session: &str, selected: &FileReference) -> Result> { selected.validate()?; + self.resolve_stored(session, &selected.id, Some(selected)) + .map(|(bytes, _)| bytes) + } + + /// Resolve an existing session-owned object without importing or granting it. + pub(crate) fn resolve_id(&self, session: &str, id: &str) -> Result<(Vec, String)> { + if !id.strip_prefix("file_").is_some_and(|hash| { + hash.len() == 64 + && hash + .bytes() + .all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b)) + }) { + return Err("invalid managed file ID".into()); + } + self.resolve_stored(session, id, None) + } + + fn resolve_stored( + &self, + session: &str, + id: &str, + selected: Option<&FileReference>, + ) -> Result<(Vec, String)> { let directory = self.session_directory(session); // A reference must never resolve an import still retained only in the // resilient filesystem's volatile write-back layer. - fs::require_disk(directory.join(&selected.id)).map_err(display)?; - let mut file = fs::open_beneath(&directory, Path::new(&selected.id)).map_err(|error| { + fs::require_disk(directory.join(id)).map_err(display)?; + let mut file = fs::open_beneath(&directory, Path::new(id)).map_err(|error| { format!("managed file is missing or inaccessible in this session: {error}") })?; let length = file.metadata().map_err(display)?.len(); @@ -475,9 +498,10 @@ impl FileStore { file.read_exact(&mut header).map_err(display)?; let header: Header = serde_json::from_slice(&header).map_err(display)?; header.file.validate()?; - if &header.file != selected { + if header.file.id != id || selected.is_some_and(|selected| &header.file != selected) { return Err("selected file metadata does not match its stored object".into()); } + let selected = &header.file; if length != 12 + header_length as u64 + selected.size_bytes { return Err("managed file envelope length does not match its payload".into()); } @@ -492,7 +516,7 @@ impl FileStore { } // Digest and metadata bind the already validated immutable import. No // repeated pixel decode is necessary for each selection or replay. - Ok(bytes) + Ok((bytes, header.file.mime_type)) } pub(crate) fn selected_parts( diff --git a/src/managed_files/tests.rs b/src/managed_files/tests.rs index 50e3707..614b4dd 100644 --- a/src/managed_files/tests.rs +++ b/src/managed_files/tests.rs @@ -42,6 +42,47 @@ impl Fixture { self.store.selected_parts("session", value, None) } } +#[test] +fn id_lookup_uses_session_authority_and_validates_stored_envelope() { + let f = Fixture::new(); + let reference = f.import("image.png"); + let expected = f.store.resolve("session", &reference).unwrap(); + assert_eq!( + f.store.resolve_id("session", &reference.id).unwrap(), + (expected, "image/png".into()) + ); + assert!(f.store.resolve_id("other-session", &reference.id).is_err()); + for invalid in [ + format!("{}/", reference.id), + format!("{}?q=1", reference.id), + format!("{}#x", reference.id), + "../image".into(), + format!("file_{}", "A".repeat(64)), + format!("file_{}", "0".repeat(64)), + ] { + assert!(f.store.resolve_id("session", &invalid).is_err()); + } + let path = f.object(&reference); + let mut bytes = disk::read(&path).unwrap(); + let last = bytes.len() - 1; + bytes[last] ^= 1; + disk::write(&path, bytes).unwrap(); + assert!(f.store.resolve_id("session", &reference.id).is_err()); +} + +#[test] +fn id_lookup_rejects_stored_descriptor_for_another_id() { + let f = Fixture::new(); + let reference = f.import("image.png"); + let alternate = format!("file_{}", "f".repeat(64)); + disk::copy( + f.object(&reference), + f.store.session_directory("session").join(&alternate), + ) + .unwrap(); + assert!(f.store.resolve_id("session", &alternate).is_err()); +} + #[test] fn png_and_jpeg_snapshots_survive_source_deletion_and_reopen() { for (format, name, mime) in [ diff --git a/src/tui/app.rs b/src/tui/app.rs index f0d4f30..a228adc 100644 --- a/src/tui/app.rs +++ b/src/tui/app.rs @@ -27,6 +27,9 @@ use crate::file_search::FileMatch; const MAX_TOOL_OUTPUT_LINES: usize = 5_000; pub(super) const MAX_TOOL_IMAGES: usize = 32; +/// Typed occurrence metadata is bounded independently of encoded source bytes. +pub(super) const MAX_RETAINED_IMAGES: usize = 256; +pub(super) const MAX_MESSAGE_IMAGES: usize = 64; const MAX_IMAGE_BASE64_BYTES: usize = 14 * 1024 * 1024; const MAX_IMAGE_SOURCE_BYTES: usize = 10 * 1024 * 1024; pub(super) const MAX_RETAINED_IMAGE_SOURCE_BYTES: usize = 32 * 1024 * 1024; @@ -69,13 +72,14 @@ pub enum Update { UserMessage { id: String, text: String, - images: Vec, + images: Vec, append: bool, }, /// Agent prose, either appended as a chunk or replaced by an upsert. AgentMessage { id: String, text: String, + images: Vec, append: bool, }, /// Agent reasoning, either appended as a chunk or replaced by an upsert. @@ -100,7 +104,7 @@ pub enum Update { status: Option, script: Option, output: Option>, - images: Option>, + images: Option>, append_output: bool, intent: Option>, backgrounded: bool, @@ -364,8 +368,8 @@ pub struct ToolCall { pub children: Vec, /// Raw tool output, kept whole but folded away until asked for. pub output: Vec, - /// Typed tool-result images sharing the user-image retention and decode budgets. - pub images: Vec, + /// Typed tool-result images sharing transcript retention and decode budgets. + pub images: Vec, /// User-facing summary supplied by a compose caller. pub intent: Option, pub expanded: bool, @@ -457,21 +461,27 @@ impl ToolCall { } } -/// One entry in the transcript. -#[derive(Clone, Debug)] -pub struct UserImage { +/// A typed image source shared by user, assistant, and tool messages. +#[derive(Clone)] +pub struct MediaImage { pub(super) key: [u8; 32], - pub(super) data: String, + pub(super) data: std::sync::Arc, pub(super) mime_type: String, /// Source line after which the fixed image viewport is reserved. pub(super) line: usize, + /// Original ACP URI; consumers must validate it before deduplication or use. + pub(super) source_uri: Option, } -impl UserImage { +impl MediaImage { pub(super) fn new(data: String, mime_type: String, line: usize) -> Option { // Check the encoded and maximum decoded lengths before hashing or retaining // attacker-controlled ACP payloads. The exact decode stays lazy. - if data.len() > MAX_IMAGE_BASE64_BYTES { + if data.is_empty() + || data.len() > MAX_IMAGE_BASE64_BYTES + || mime_type.len() > 128 + || mime_type.chars().any(char::is_control) + { return None; } let padding = data @@ -495,20 +505,46 @@ impl UserImage { hasher.update(data.as_bytes()); Some(Self { key: *hasher.finalize().as_bytes(), - data, + data: data.into(), mime_type, line, + source_uri: None, }) } } +impl MediaImage { + pub(super) fn with_uri(mut self, uri: Option) -> Self { + self.source_uri = uri.filter(|uri| uri.len() <= 4096 && !uri.chars().any(char::is_control)); + self + } +} + +impl std::fmt::Debug for MediaImage { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("MediaImage") + .field("source_bytes", &self.data.len()) + .field("line", &self.line) + .finish_non_exhaustive() + } +} + +#[cfg(test)] +pub type UserImage = MediaImage; + #[derive(Clone, Debug)] -pub struct UserMessage { +pub struct Message { pub(super) text: String, - pub(super) images: Vec, + pub(super) images: Vec, +} + +impl From<&str> for Message { + fn from(text: &str) -> Self { + text.to_owned().into() + } } -impl From for UserMessage { +impl From for Message { fn from(text: String) -> Self { Self { text, @@ -517,9 +553,11 @@ impl From for UserMessage { } } +pub type UserMessage = Message; + pub enum Block { - User(UserMessage), - Agent(String), + User(Message), + Agent(Message), Thought { text: String, started: Instant, @@ -531,8 +569,14 @@ pub enum Block { Error(String), } +#[derive(Clone, Debug, PartialEq, Eq)] +pub(super) enum TranscriptImageSource { + Typed(usize), + Markdown(String), +} + pub(super) struct CachedTranscriptImage { - pub source: usize, + pub source: TranscriptImageSource, pub row: usize, } @@ -627,6 +671,7 @@ pub struct App { pub(super) transcript_prefixes: Vec, pub(super) transcript_cache_width: usize, retained_image_source_bytes: usize, + retained_images: usize, next_transcript_revision: u64, transcript_focus_index: Option, pub editor: Editor, @@ -877,6 +922,7 @@ impl App { transcript_prefixes: vec![0], transcript_cache_width: 0, retained_image_source_bytes: 0, + retained_images: 0, next_transcript_revision: 0, transcript_focus_index: None, editor: Editor::default(), @@ -1049,6 +1095,29 @@ impl App { } } + pub(super) fn invalidate_image_layout(&mut self, sources: &std::collections::HashSet) { + if sources.is_empty() { + return; + } + let affected: Vec<_> = self + .transcript_cache + .iter() + .enumerate() + .filter(|(_, block)| { + block.as_ref().is_some_and(|block| { + block + .images + .iter() + .any(|image| matches!(&image.source, TranscriptImageSource::Markdown(source) if sources.contains(source))) + }) + }) + .map(|(index, _)| index) + .collect(); + for index in affected { + self.mark_block_dirty(index); + } + } + /// Aligns cache bookkeeping for tests and other direct transcript setup. pub(super) fn sync_transcript_cache(&mut self) { self.transcript_cache.truncate(self.blocks.len()); @@ -1401,7 +1470,7 @@ impl App { &mut self, id: String, text: String, - images: Vec, + images: Vec, append: bool, role: MessageRole, ) { @@ -1409,11 +1478,17 @@ impl App { self.collapse_last_tool_output(); } let mut images = images; - let existing_index = self.message_blocks.get(&id).copied(); + let existing_index = self.message_blocks.get(&id).copied().filter(|&index| { + matches!( + (&self.blocks[index], role), + (Block::User(_), MessageRole::User) + | (Block::Agent(_), MessageRole::Agent) + | (Block::Thought { .. }, MessageRole::Thought) + ) + }); if !append - && matches!(role, MessageRole::User) && let Some(index) = existing_index - && let Block::User(existing) = &self.blocks[index] + && let Block::User(existing) | Block::Agent(existing) = &self.blocks[index] { let replaced = existing .images @@ -1422,22 +1497,31 @@ impl App { .sum::(); self.retained_image_source_bytes = self.retained_image_source_bytes.saturating_sub(replaced); + self.retained_images = self.retained_images.saturating_sub(existing.images.len()); } images.retain(|image| { let retained = self .retained_image_source_bytes .saturating_add(image.data.len()); - if retained > MAX_RETAINED_IMAGE_SOURCE_BYTES { + if retained > MAX_RETAINED_IMAGE_SOURCE_BYTES + || self.retained_images >= MAX_RETAINED_IMAGES + { false } else { self.retained_image_source_bytes = retained; + self.retained_images += 1; true } }); if let Some(index) = existing_index { let mut changed = false; match (&mut self.blocks[index], role) { - (Block::User(existing), MessageRole::User) => { + (Block::User(existing), MessageRole::User) + | (Block::Agent(existing), MessageRole::Agent) => { + let old_len = existing.text.len(); + let replaces_latest = !append + && matches!(role, MessageRole::Agent) + && self.latest_agent_source.ends_with(&existing.text); if append { let last_line = existing.text.bytes().filter(|&byte| byte == b'\n').count(); let follows_image = @@ -1462,21 +1546,16 @@ impl App { existing.text = text.clone(); existing.images = std::mem::take(&mut images); } - changed = true; - } - (Block::Agent(existing), MessageRole::Agent) => { - if append { - existing.push_str(&text); - self.latest_agent_source.push_str(&text); - } else { - if self.latest_agent_source.ends_with(existing.as_str()) { + if matches!(role, MessageRole::Agent) { + if append { + self.latest_agent_source.push_str(&existing.text[old_len..]); + } else if replaces_latest { self.latest_agent_source - .truncate(self.latest_agent_source.len() - existing.len()); - self.latest_agent_source.push_str(&text); + .truncate(self.latest_agent_source.len() - old_len); + self.latest_agent_source.push_str(&existing.text); } else { - self.latest_agent_source = text.clone(); + self.latest_agent_source = existing.text.clone(); } - *existing = text.clone(); } changed = true; } @@ -1510,7 +1589,7 @@ impl App { self.latest_agent_source.push_str(&text); } self.agent_stream_sealed = false; - self.push_block(Block::Agent(text)); + self.push_block(Block::Agent(Message { text, images })); } MessageRole::Thought => self.push_block(Block::Thought { text, @@ -1695,8 +1774,13 @@ impl App { self.remove_pending_steer(&id); self.apply_message(id, text, images, append, MessageRole::User); } - Update::AgentMessage { id, text, append } => { - self.apply_message(id, text, Vec::new(), append, MessageRole::Agent); + Update::AgentMessage { + id, + text, + images, + append, + } => { + self.apply_message(id, text, images, append, MessageRole::Agent); } Update::AgentThought { id, text, append } => { self.apply_message(id, text, Vec::new(), append, MessageRole::Thought); @@ -1763,10 +1847,14 @@ impl App { .sum::(); self.retained_image_source_bytes = self.retained_image_source_bytes.saturating_sub(replaced); + self.retained_images = + self.retained_images.saturating_sub(call.images.len()); call.images.clear(); } for image in images { - if call.images.len() >= MAX_TOOL_IMAGES { + if call.images.len() >= MAX_TOOL_IMAGES + || self.retained_images >= MAX_RETAINED_IMAGES + { break; } if call.images.iter().any(|existing| existing.key == image.key) { @@ -1777,6 +1865,7 @@ impl App { .saturating_add(image.data.len()); if retained <= MAX_RETAINED_IMAGE_SOURCE_BYTES { self.retained_image_source_bytes = retained; + self.retained_images += 1; call.images.push(image); } } @@ -2212,6 +2301,7 @@ impl App { self.transcript_prefixes.push(0); self.transcript_cache_width = 0; self.retained_image_source_bytes = 0; + self.retained_images = 0; self.transcript_focus_index = None; self.clear_attachments(); self.latest_agent_source.clear(); @@ -3619,7 +3709,7 @@ impl App { .and_then(Option::as_ref) .and_then(|hit| self.blocks.get(hit.block).map(|block| (block, hit))) .and_then(|(block, hit)| match block { - Block::Agent(source) => source.get(hit.range.clone()), + Block::Agent(source) => source.text.get(hit.range.clone()), _ => None, }) .map(str::to_string); @@ -3799,6 +3889,7 @@ mod test_support { impl Update { pub(in crate::tui) fn test_text(text: String) -> Self { Self::AgentMessage { + images: Vec::new(), id: "test-agent".into(), text, append: true, @@ -3863,7 +3954,7 @@ mod tests { use super::{ Action, App, AttachmentKind, Block, MAX_IMAGE_BASE64_BYTES, MAX_IMAGE_SOURCE_BYTES, - MAX_RETAINED_IMAGE_SOURCE_BYTES, Phase, Update, UserImage, + MAX_RETAINED_IMAGE_SOURCE_BYTES, MediaImage, Phase, Update, }; use crate::{events::RuntimeEvent, file_search::FileMatch, tui::wrap::LinkHit}; @@ -4230,13 +4321,146 @@ mod tests { assert!(working.file_picker.is_none()); } + #[test] + fn assistant_images_append_replace_and_clear_without_payload_previews() { + let mut app = app(); + let image = MediaImage::new("c2VjcmV0".into(), "image/png".into(), 0) + .unwrap() + .with_uri(Some("data:image/png;base64,c2VjcmV0".into())); + assert!(!format!("{image:?}").contains("c2VjcmV0")); + let patch = |text: &str, images, append| Update::AgentMessage { + id: "assistant".into(), + text: text.into(), + images, + append, + }; + app.apply(patch("before", Vec::new(), true)); + app.apply(patch("[Image #1]", vec![image.clone()], true)); + app.apply(patch("after", Vec::new(), true)); + let Block::Agent(message) = &app.blocks[0] else { + panic!("assistant"); + }; + assert_eq!(message.text, "before\n[Image #1]\nafter"); + assert_eq!(message.images[0].line, 1); + assert_eq!(app.retained_image_source_bytes, image.data.len()); + assert_eq!(app.latest_agent_text().unwrap(), message.text); + app.apply(patch("replacement", vec![image.clone()], false)); + assert_eq!(app.retained_image_source_bytes, image.data.len()); + app.apply(patch("", Vec::new(), false)); + assert_eq!(app.retained_image_source_bytes, 0); + let Block::Agent(message) = &app.blocks[0] else { + panic!("assistant"); + }; + assert!(message.text.is_empty() && message.images.is_empty()); + app.apply(patch("[Image #1]", vec![image], false)); + app.start_session("next".into()); + assert_eq!(app.retained_image_source_bytes, 0); + assert!(app.blocks.is_empty()); + assert!(app.latest_agent_text().is_none()); + } + + #[test] + fn typed_occurrence_budget_is_shared_and_recovers_on_replacement_and_reset() { + let mut app = app(); + let image = MediaImage::new("AQID".into(), "image/png".into(), 0).unwrap(); + let patch = |index: usize, images: Vec, append| match index % 3 { + 0 => Update::UserMessage { + id: format!("user-{index}"), + text: "[Image]".into(), + images, + append, + }, + 1 => Update::AgentMessage { + id: format!("agent-{index}"), + text: "[Image]".into(), + images, + append, + }, + _ => Update::ToolPatched { + id: format!("tool-{index}"), + title: None, + kind: None, + status: None, + script: None, + output: Some(vec!["[Image]".into()]), + images: Some(images), + append_output: append, + intent: None, + backgrounded: false, + }, + }; + for index in 0..super::MAX_RETAINED_IMAGES { + app.apply(patch(index, vec![image.clone()], true)); + } + let retained = |app: &App| { + app.blocks + .iter() + .map(|block| match block { + Block::User(message) | Block::Agent(message) => message.images.len(), + Block::Tool(call) => call.images.len(), + _ => 0, + }) + .sum::() + }; + assert_eq!(retained(&app), super::MAX_RETAINED_IMAGES); + assert_eq!(app.retained_images, retained(&app)); + for index in 0..3 { + app.apply(patch(index, vec![image.clone()], true)); + } + assert_eq!(retained(&app), super::MAX_RETAINED_IMAGES); + assert_eq!( + app.retained_image_source_bytes, + retained(&app) * image.data.len() + ); + // Each role releases its own slot before admitting a replacement. + for index in 0..3 { + app.apply(patch(index, Vec::new(), false)); + assert_eq!(retained(&app), super::MAX_RETAINED_IMAGES - 1); + app.apply(patch(index, vec![image.clone()], false)); + assert_eq!(retained(&app), super::MAX_RETAINED_IMAGES); + assert_eq!(app.retained_images, retained(&app)); + } + app.start_session("next".into()); + assert_eq!(app.retained_images, 0); + assert_eq!(app.retained_image_source_bytes, 0); + app.apply(patch(1, vec![image], true)); + assert_eq!(retained(&app), 1); + } + + #[test] + fn tiny_assistant_append_chunks_cannot_exceed_occurrence_budget() { + let mut app = app(); + let image = MediaImage::new("AQID".into(), "image/png".into(), 0).unwrap(); + for _ in 0..super::MAX_RETAINED_IMAGES + 10 { + app.apply(Update::AgentMessage { + id: "chunks".into(), + text: "[Image]".into(), + images: vec![image.clone()], + append: true, + }); + } + let Block::Agent(message) = &app.blocks[0] else { + panic!("assistant"); + }; + assert_eq!(message.images.len(), super::MAX_RETAINED_IMAGES); + assert!(message.text.matches("[Image]").count() > message.images.len()); + app.apply(Update::AgentMessage { + id: "chunks".into(), + text: "replacement".into(), + images: vec![image], + append: false, + }); + assert_eq!(app.retained_images, 1); + assert!(MediaImage::new(String::new(), "image/png".into(), 0).is_none()); + } + #[test] fn oversized_user_image_payload_is_rejected_before_retention() { let encoded_too_large = "A".repeat(MAX_IMAGE_BASE64_BYTES + 1); - assert!(UserImage::new(encoded_too_large, "image/png".into(), 0).is_none()); + assert!(MediaImage::new(encoded_too_large, "image/png".into(), 0).is_none()); let decoded_too_large = "A".repeat((MAX_IMAGE_SOURCE_BYTES + 1).div_ceil(3) * 4); - assert!(UserImage::new(decoded_too_large, "image/png".into(), 0).is_none()); + assert!(MediaImage::new(decoded_too_large, "image/png".into(), 0).is_none()); } #[test] @@ -4244,7 +4468,7 @@ mod tests { let mut app = app(); let bytes = 9 * 1024 * 1024; let image = |byte: char| { - UserImage::new(byte.to_string().repeat(bytes), "image/png".into(), 0).unwrap() + MediaImage::new(byte.to_string().repeat(bytes), "image/png".into(), 0).unwrap() }; app.apply(Update::UserMessage { id: "user".into(), @@ -4279,25 +4503,35 @@ mod tests { } #[test] - fn retained_user_image_sources_have_an_aggregate_bound() { + fn retained_user_and_assistant_image_sources_have_an_aggregate_bound() { let source_bytes = 9 * 1024 * 1024; let mut app = app(); for index in 0..4 { - let image = UserImage::new("A".repeat(source_bytes), "image/png".into(), 0) + let image = MediaImage::new("A".repeat(source_bytes), "image/png".into(), 0) .expect("source is within the per-image limit"); - app.apply(Update::UserMessage { - id: format!("image-{index}"), - text: format!("[Image #{index}]"), - images: vec![image], - append: false, - }); + let update = if index == 0 { + Update::UserMessage { + id: format!("image-{index}"), + text: format!("[Image #{index}]"), + images: vec![image], + append: false, + } + } else { + Update::AgentMessage { + id: format!("image-{index}"), + text: format!("[Image #{index}]"), + images: vec![image], + append: false, + } + }; + app.apply(update); } 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() + Some(Block::Agent(message)) if message.images.is_empty() )); } @@ -4619,6 +4853,7 @@ mod tests { }); compose(&mut app, "return 1"); app.apply(Update::AgentMessage { + images: Vec::new(), id: "agent".into(), text: "hello".into(), append: false, @@ -4648,6 +4883,7 @@ mod tests { IdleStateUpdate::new().stop_reason(StopReason::EndTurn), ))); app.apply(Update::AgentMessage { + images: Vec::new(), id: "late".into(), text: "background result".into(), append: false, @@ -6028,6 +6264,7 @@ mod tests { app.apply(Update::test_text("# Heading\n\tindented ".into())); compose(&mut app, "value = tool({})"); app.apply(Update::AgentMessage { + images: Vec::new(), id: "post-tool-agent".into(), text: "\n\n- item".into(), append: true, @@ -6049,6 +6286,7 @@ mod tests { ))); app.push_user("next".into()); app.apply(Update::AgentMessage { + images: Vec::new(), id: "next-agent".into(), text: "new".into(), append: true, @@ -6066,7 +6304,7 @@ mod tests { let Some(Block::Agent(text)) = app.blocks.last() else { panic!("expected an agent block"); }; - assert_eq!(text, "hello"); + assert_eq!(text.text, "hello"); } #[test] @@ -6077,11 +6315,13 @@ mod tests { IdleStateUpdate::new().stop_reason(StopReason::EndTurn), ))); app.apply(Update::AgentMessage { + images: Vec::new(), id: "autonomous".into(), text: "RAVENS_".into(), append: true, }); app.apply(Update::AgentMessage { + images: Vec::new(), id: "autonomous".into(), text: "HARBOR_INEVITABLE".into(), append: true, @@ -6091,7 +6331,7 @@ mod tests { .blocks .iter() .filter_map(|block| match block { - Block::Agent(text) => Some(text.as_str()), + Block::Agent(text) => Some(text.text.as_str()), _ => None, }) .collect::>(); @@ -6126,11 +6366,13 @@ mod tests { backgrounded: false, }); app.apply(Update::AgentMessage { + images: Vec::new(), id: "second".into(), text: "second completion".into(), append: true, }); app.apply(Update::AgentMessage { + images: Vec::new(), id: "second".into(), text: " continued".into(), append: true, @@ -6140,7 +6382,7 @@ mod tests { .blocks .iter() .filter_map(|block| match block { - Block::Agent(text) => Some(text.as_str()), + Block::Agent(text) => Some(text.text.as_str()), _ => None, }) .collect::>(); diff --git a/src/tui/image.rs b/src/tui/image.rs index 4174940..9cb46ac 100644 --- a/src/tui/image.rs +++ b/src/tui/image.rs @@ -1,7 +1,6 @@ -use std::{collections::HashMap, io::Cursor, time::Duration}; - +use super::app::MediaImage; use base64::{Engine as _, engine::general_purpose::STANDARD}; -use image::{ImageReader, Limits}; +use image::{ImageDecoder, ImageReader, Limits}; use ratatui::{ Frame, layout::{Rect, Size}, @@ -11,109 +10,294 @@ use ratatui_image::{ picker::{Picker, ProtocolType, cap_parser::QueryStdioOptions}, sliced::{SignedPosition, SlicedImage, SlicedProtocol}, }; - -use super::app::UserImage; +use std::{collections::HashMap, io::Cursor, sync::Arc, time::Duration}; +use tokio::sync::{Semaphore, oneshot}; const TERMINAL_QUERY_TIMEOUT: Duration = Duration::from_millis(150); +// Preserve the existing user attachment limit; external/managed acquisition +// remains stricter (8 MiB) while sharing this decoder and pixel budgets. +const MAX_SOURCE_BYTES: usize = 10 * 1024 * 1024; +const MAX_BASE64_BYTES: usize = MAX_SOURCE_BYTES.div_ceil(3) * 4; +const MAX_QUEUED_SOURCE_BYTES: usize = 32 * 1024 * 1024; const MAX_DECODED_ALLOCATION: u64 = 64 * 1024 * 1024; const MAX_DECODED_BACKING_BYTES: u64 = 128 * 1024 * 1024; const MAX_CACHE_ENTRIES: usize = 16; const MAX_DIMENSION: u32 = 8_192; +const MAX_PIXELS: u64 = 16 * 1024 * 1024; +// Bound protocol input, including cell padding, to 256 KiB RGBA. Retain full +// decoded resolution separately: resize never resamples an earlier thumbnail. +// At most 16 protocols plus two in-flight replacements live. Reserve 8 MiB +// each for encoder scratch, strings and render copies (32 bytes per pixel), +// rather than counting only RGBA input. Codec/library overhead is additional. +const MAX_PROTOCOL_PIXELS: u32 = 65_536; +const MAX_JOBS: usize = 2; +// Permits belong to blocking closures, NOT join handles or cache entries. +// Clear, suspend/resume and dropped receivers cannot free running capacity. +// Admission uses try_acquire: there are no spawn_blocking permit waiters. +static WORKERS: Semaphore = Semaphore::const_new(MAX_JOBS); pub(super) const RESERVED_ROWS: u16 = 12; #[derive(Clone, Copy)] pub(super) struct PreparedImage { pub key: [u8; 32], } - +type Decoded = Arc; +type JobResult = Result<(Decoded, SlicedProtocol), &'static str>; struct CacheEntry { - decoded: Option, - decoded_backing_bytes: u64, + source: Option<(Arc, String)>, + decoded: Option, protocol: Option<(u16, SlicedProtocol)>, + width: u16, + running: bool, + error: Option<&'static str>, last_used: u64, } - +struct Job { + key: [u8; 32], + generation: u64, + width: u16, + receiver: oneshot::Receiver, +} pub(super) struct ImageRuntime { + pub(super) markdown: super::markdown_images::MarkdownImages, picker: Option, cache: HashMap<[u8; 32], CacheEntry>, - decoded_backing_bytes: u64, + jobs: Vec, + generation: u64, clock: u64, } - impl ImageRuntime { - pub fn detect() -> Self { - let picker = Picker::from_query_stdio_with_options(QueryStdioOptions { - timeout: TERMINAL_QUERY_TIMEOUT, - ..QueryStdioOptions::default() - }) - .ok() - .filter(|picker| picker.protocol_type() != ProtocolType::Halfblocks); + fn new(picker: Option) -> Self { Self { + markdown: super::markdown_images::MarkdownImages::new(), picker, cache: HashMap::new(), - decoded_backing_bytes: 0, + jobs: Vec::new(), + generation: 0, clock: 0, } } - + pub fn detect() -> Self { + Self::new( + Picker::from_query_stdio_with_options(QueryStdioOptions { + timeout: TERMINAL_QUERY_TIMEOUT, + ..QueryStdioOptions::default() + }) + .ok() + .filter(|picker| picker.protocol_type() != ProtocolType::Halfblocks), + ) + } pub fn enabled(&self) -> bool { self.picker.is_some() } - pub fn clear(&mut self) { + self.markdown.clear(); self.cache.clear(); - self.decoded_backing_bytes = 0; + self.generation = self.generation.wrapping_add(1); + // Stale results retain their job slot and reservation until completion. + } + pub fn status(&self, key: &[u8; 32]) -> &'static str { + if !self.enabled() { + return "image display unavailable"; + } + match self.cache.get(key) { + Some(entry) if entry.error.is_some() => entry.error.unwrap_or("image unavailable"), + Some(entry) if entry.protocol.is_some() && !entry.running => "image ready", + _ => "image loading", + } + } + /// Drain completions on the TUI tick; redraw when true. Never waits. + pub fn pending(&self) -> bool { + self.markdown.pending() + || !self.jobs.is_empty() + || self.cache.values().any(|entry| { + entry.error.is_none() + && (entry.source.is_some() + || entry + .protocol + .as_ref() + .is_none_or(|(width, _)| *width != entry.width)) + }) } - pub fn prepare(&mut self, image: &UserImage, width: u16) -> Option { - let picker = self.picker.clone()?; - if width == 0 { + pub fn poll(&mut self) -> bool { + let mut changed = self.markdown.poll(); + let mut index = 0; + while index < self.jobs.len() { + let result = match self.jobs[index].receiver.try_recv() { + Ok(result) => result, + Err(oneshot::error::TryRecvError::Empty) => { + index += 1; + continue; + } + Err(oneshot::error::TryRecvError::Closed) => Err("image worker failed"), + }; + let job = self.jobs.swap_remove(index); + if job.generation != self.generation { + continue; + } + let Some(entry) = self.cache.get_mut(&job.key) else { + continue; + }; + entry.running = false; + changed = true; + match result { + Ok((decoded, protocol)) => { + entry.decoded = Some(decoded); + if entry.width == job.width { + entry.protocol = Some((job.width, protocol)); + } + } + Err(error) => entry.error = Some(error), + } + } + // Cache backing is separate from the two 64 MiB job reservations. + // Completed outputs move from those reservations into the cache. + // Trim even when there is no queued work left to schedule. + while self + .cache + .values() + .filter_map(|entry| entry.decoded.as_ref()) + .map(|decoded| decoded.as_bytes().len() as u64) + .sum::() + > MAX_DECODED_BACKING_BYTES + { + let victim = self + .cache + .iter() + .filter(|(_, entry)| !entry.running && entry.decoded.is_some()) + .min_by_key(|(_, entry)| entry.last_used) + .map(|(key, _)| *key); + let Some(victim) = victim else { + break; + }; + self.cache.remove(&victim); + } + self.schedule(); + changed + } + pub fn prepare(&mut self, image: &MediaImage, width: u16) -> Option { + if !self.enabled() || width == 0 { return None; } self.clock = self.clock.wrapping_add(1); if !self.cache.contains_key(&image.key) { - let decoded = decode(image); - // This only accounts for the decoded image backing buffer. Protocol - // encoders can allocate additional implementation-defined memory. - let decoded_backing_bytes = decoded - .as_ref() - .map_or(0, |image| image.as_bytes().len() as u64); - if decoded_backing_bytes > MAX_DECODED_BACKING_BYTES { + if image.data.len() > MAX_BASE64_BYTES || image.mime_type.len() > 256 { return None; } - self.evict_for(decoded_backing_bytes); - self.decoded_backing_bytes += decoded_backing_bytes; + while self.cache.len() >= MAX_CACHE_ENTRIES + || self.queued_bytes().saturating_add(image.data.len()) > MAX_QUEUED_SOURCE_BYTES + { + if !self.evict() { + return None; + } + } self.cache.insert( image.key, CacheEntry { - decoded, - decoded_backing_bytes, + source: Some((image.data.clone(), image.mime_type.clone())), + decoded: None, protocol: None, + width, + running: false, + error: None, last_used: self.clock, }, ); } let entry = self.cache.get_mut(&image.key)?; entry.last_used = self.clock; + entry.width = width; if entry .protocol .as_ref() - .is_none_or(|(cached_width, _)| *cached_width != width) + .is_some_and(|(cached, _)| *cached != width) { - let target = Size::new(width, RESERVED_ROWS); - let protocol = SlicedProtocol::new_with_resize( - &picker, - entry.decoded.as_ref()?.clone(), - target, - Resize::Fit(None), - ) - .ok()?; - entry.protocol = Some((width, protocol)); + entry.protocol = None; + } + let ready = entry.protocol.is_some(); + self.schedule(); + ready.then_some(PreparedImage { key: image.key }) + } + fn queued_bytes(&self) -> usize { + self.cache + .values() + .filter_map(|entry| entry.source.as_ref()) + .map(|(data, _)| data.len()) + .sum() + } + fn evict(&mut self) -> bool { + let key = self + .cache + .iter() + .filter(|(_, entry)| !entry.running) + .min_by_key(|(_, entry)| entry.last_used) + .map(|(key, _)| *key); + key.is_some_and(|key| self.cache.remove(&key).is_some()) + } + fn schedule(&mut self) { + let Some(picker) = self.picker.as_ref() else { + return; + }; + let picker = picker.clone(); + let Ok(handle) = tokio::runtime::Handle::try_current() else { + for entry in self.cache.values_mut() { + entry.error = Some("image runtime unavailable"); + entry.source = None; + } + return; + }; + while self.jobs.len() < MAX_JOBS { + let Some(key) = self + .cache + .iter() + .filter(|(_, entry)| { + !entry.running && entry.error.is_none() && entry.protocol.is_none() + }) + .max_by_key(|(_, entry)| entry.last_used) + .map(|(key, _)| *key) + else { + break; + }; + let Ok(permit) = WORKERS.try_acquire() else { + break; + }; + let Some(entry) = self.cache.get_mut(&key) else { + break; + }; + let source = entry.source.take(); + let decoded = entry.decoded.clone(); + let width = entry.width; + entry.running = true; + let (sender, receiver) = oneshot::channel(); + self.jobs.push(Job { + key, + generation: self.generation, + width, + receiver, + }); + let picker = picker.clone(); + handle.spawn_blocking(move || { + let _permit = permit; + let result = (|| { + let decoded = match decoded { + Some(decoded) => decoded, + None => { + let (data, mime) = source.ok_or("image source unavailable")?; + Arc::new( + decode(&data, &mime) + .ok_or("image decode failed or exceeds limits")?, + ) + } + }; + let protocol = protocol(&picker, &decoded, width) + .ok_or("image protocol unavailable or exceeds limits")?; + Ok((decoded, protocol)) + })(); + let _ = sender.send(result); + }); } - entry.protocol.as_ref()?; - Some(PreparedImage { key: image.key }) } - pub fn render(&mut self, frame: &mut Frame<'_>, image: PreparedImage, area: Rect, y: i16) { self.clock = self.clock.wrapping_add(1); let Some(entry) = self.cache.get_mut(&image.key) else { @@ -128,36 +312,17 @@ impl ImageRuntime { area, ); } - - fn evict_for(&mut self, incoming: u64) { - while !self.cache.is_empty() - && (self.cache.len() >= MAX_CACHE_ENTRIES - || self.decoded_backing_bytes.saturating_add(incoming) > MAX_DECODED_BACKING_BYTES) - { - let Some(key) = self - .cache - .iter() - .min_by_key(|(_, entry)| entry.last_used) - .map(|(key, _)| *key) - else { - break; - }; - if let Some(entry) = self.cache.remove(&key) { - self.decoded_backing_bytes = self - .decoded_backing_bytes - .saturating_sub(entry.decoded_backing_bytes); - } - } - } } - -fn decode(source: &UserImage) -> Option { - let bytes = STANDARD.decode(source.data.as_bytes()).ok()?; - if bytes.len() as u64 > MAX_DECODED_ALLOCATION { +fn decode(data: &str, mime: &str) -> Option { + if data.len() > MAX_BASE64_BYTES { + return None; + } + let bytes = STANDARD.decode(data.as_bytes()).ok()?; + if bytes.len() > MAX_SOURCE_BYTES { return None; } let mut reader = ImageReader::new(Cursor::new(bytes)); - if let Some(format) = image::ImageFormat::from_mime_type(&source.mime_type) { + if let Some(format) = image::ImageFormat::from_mime_type(mime) { reader.set_format(format); } else { reader = reader.with_guessed_format().ok()?; @@ -167,40 +332,58 @@ fn decode(source: &UserImage) -> Option { limits.max_image_height = Some(MAX_DIMENSION); limits.max_alloc = Some(MAX_DECODED_ALLOCATION); reader.limits(limits); - reader.decode().ok() + let mut decoder = reader.into_decoder().ok()?; + let (width, height) = decoder.dimensions(); + if u64::from(width) * u64::from(height) > MAX_PIXELS + || decoder.total_bytes() > MAX_DECODED_ALLOCATION + { + return None; + } + let orientation = decoder.orientation().ok()?; + let decoded = image::DynamicImage::from_decoder(decoder).ok()?; + // Conversion/orientation transiently retain two 64 MiB buffers per worker. + // Two workers also hold at most 2*(14 MiB base64 + 10 MiB encoded source). + let mut decoded = image::DynamicImage::ImageRgba8(decoded.into_rgba8()); + decoded.apply_orientation(orientation); + Some(decoded) +} +fn protocol(picker: &Picker, decoded: &image::DynamicImage, width: u16) -> Option { + let font = picker.font_size(); + let height = u32::from(font.height).checked_mul(u32::from(RESERVED_ROWS))?; + let cell_pixels = u32::from(font.width).checked_mul(height)?; + if cell_pixels == 0 || height > u32::from(u16::MAX) { + return None; + } + let columns = u32::from(width) + .min(512) + .min(MAX_PROTOCOL_PIXELS / cell_pixels); + if columns == 0 { + return None; + } + let pixels = columns * u32::from(font.width); + if pixels > u32::from(u16::MAX) { + return None; + } + // No full-resolution clone or full-image floating-point filter scratch. + let fitted = decoded.resize(pixels, height, image::imageops::FilterType::Nearest); + SlicedProtocol::new_with_resize( + picker, + fitted, + Size::new(columns as u16, RESERVED_ROWS), + Resize::Fit(Some(image::imageops::FilterType::Nearest)), + ) + .ok() } - #[cfg(test)] -#[allow( - clippy::unwrap_used, - clippy::expect_used, - clippy::panic, - clippy::unreachable, - clippy::disallowed_methods, - clippy::disallowed_macros -)] mod test_support { use super::*; - impl ImageRuntime { pub fn disabled() -> Self { - Self { - picker: None, - cache: HashMap::new(), - decoded_backing_bytes: 0, - clock: 0, - } + Self::new(None) } - pub fn with_picker(picker: Picker) -> Self { - Self { - picker: Some(picker), - cache: HashMap::new(), - decoded_backing_bytes: 0, - clock: 0, - } + Self::new(Some(picker)) } - pub fn cached_entries(&self) -> usize { self.cache.len() } @@ -212,67 +395,180 @@ mod test_support { clippy::unwrap_used, clippy::expect_used, clippy::panic, - clippy::unreachable, clippy::disallowed_methods, clippy::disallowed_macros )] mod tests { use super::*; + use ratatui::{Terminal, backend::TestBackend}; + + fn source(seed: u8) -> MediaImage { + let mut png = Cursor::new(Vec::new()); + image::DynamicImage::ImageRgba8(image::RgbaImage::from_pixel( + 200, + 100, + image::Rgba([seed, 80, 120, 255]), + )) + .write_to(&mut png, image::ImageFormat::Png) + .unwrap(); + MediaImage::new(STANDARD.encode(png.into_inner()), "image/png".into(), 0).unwrap() + } - fn image(data: &str) -> UserImage { - UserImage::new(data.into(), "image/png".into(), 0).unwrap() + async fn settle(runtime: &mut ImageRuntime, source: &MediaImage) { + tokio::time::timeout(Duration::from_secs(10), async { + while runtime.status(&source.key) == "image loading" { + runtime.poll(); + tokio::time::sleep(Duration::from_millis(1)).await; + } + }) + .await + .unwrap(); } #[test] fn disabled_runtime_uses_text_fallback() { + assert!(ImageRuntime::disabled().prepare(&source(0), 40).is_none()); + } + + #[tokio::test] + async fn completion_renders_and_resize_retains_full_resolution() { + let source = source(1); + let mut runtime = ImageRuntime::with_picker(Picker::halfblocks()); + assert!(runtime.prepare(&source, 20).is_none()); + settle(&mut runtime, &source).await; + assert_eq!(runtime.status(&source.key), "image ready"); + let decoded = runtime.cache[&source.key].decoded.as_ref().unwrap().clone(); + assert_eq!((decoded.width(), decoded.height()), (200, 100)); + let mut terminal = Terminal::new(TestBackend::new(40, RESERVED_ROWS)).unwrap(); + let prepared = runtime.prepare(&source, 20).unwrap(); + terminal + .draw(|frame| runtime.render(frame, prepared, frame.area(), 0)) + .unwrap(); assert!( - ImageRuntime::disabled() - .prepare(&image("invalid"), 40) - .is_none() + terminal + .backend() + .buffer() + .content + .iter() + .any(|cell| cell.symbol() != " " || cell.bg != ratatui::style::Color::Reset) ); + assert!(runtime.prepare(&source, 40).is_none()); + settle(&mut runtime, &source).await; + assert!(runtime.prepare(&source, 40).is_some()); + assert!(Arc::ptr_eq( + &decoded, + runtime.cache[&source.key].decoded.as_ref().unwrap() + )); + assert_eq!(runtime.cached_entries(), 1); } - #[test] - fn decoded_image_is_reused_when_width_changes() { - let mut png = Cursor::new(Vec::new()); - image::DynamicImage::new_rgb8(200, 100) - .write_to(&mut png, image::ImageFormat::Png) + #[tokio::test] + async fn common_decoder_handles_supported_formats_and_corrupt_signatures() { + for format in [ + image::ImageFormat::Png, + image::ImageFormat::Jpeg, + image::ImageFormat::Gif, + image::ImageFormat::WebP, + ] { + let mut bytes = Cursor::new(Vec::new()); + image::DynamicImage::new_rgb8(3, 2) + .write_to(&mut bytes, format) + .unwrap(); + let source = MediaImage::new( + STANDARD.encode(bytes.into_inner()), + format.to_mime_type().into(), + 0, + ) .unwrap(); - let source = image(&STANDARD.encode(png.into_inner())); + let mut runtime = ImageRuntime::with_picker(Picker::halfblocks()); + assert!(runtime.prepare(&source, 40).is_none()); + settle(&mut runtime, &source).await; + assert_eq!(runtime.status(&source.key), "image ready"); + } + let source = + MediaImage::new(STANDARD.encode(b"\x89PNG\r\n\x1a\n"), "image/png".into(), 0).unwrap(); + let mut runtime = ImageRuntime::with_picker(Picker::halfblocks()); + runtime.prepare(&source, 40); + settle(&mut runtime, &source).await; + assert_eq!( + runtime.status(&source.key), + "image decode failed or exceeds limits" + ); + } + + #[tokio::test] + async fn failures_are_negative_cached() { + let source = MediaImage::new("aW52YWxpZA==".into(), "image/png".into(), 0).unwrap(); let mut runtime = ImageRuntime::with_picker(Picker::halfblocks()); + assert!(runtime.prepare(&source, 40).is_none()); + settle(&mut runtime, &source).await; + assert_eq!( + runtime.status(&source.key), + "image decode failed or exceeds limits" + ); + assert!(runtime.prepare(&source, 20).is_none()); + assert!(!runtime.poll()); + assert_eq!(runtime.cached_entries(), 1); + } - assert!(runtime.prepare(&source, 20).is_some()); - let decoded_backing_bytes = runtime.decoded_backing_bytes; - assert_eq!(decoded_backing_bytes, 200 * 100 * 3); + #[tokio::test] + async fn clear_discards_old_completions_and_allows_same_source_again() { + let source = source(2); + let mut runtime = ImageRuntime::with_picker(Picker::halfblocks()); + runtime.prepare(&source, 20); + runtime.clear(); + assert_eq!(runtime.cached_entries(), 0); + runtime.prepare(&source, 40); + settle(&mut runtime, &source).await; assert!(runtime.prepare(&source, 40).is_some()); - assert_eq!(runtime.cache.len(), 1); - assert_eq!(runtime.decoded_backing_bytes, decoded_backing_bytes); assert_eq!(runtime.cache[&source.key].protocol.as_ref().unwrap().0, 40); + runtime.clear(); + assert_eq!(runtime.cached_entries(), 0); } - #[test] - fn failed_decodes_are_cached() { + #[tokio::test] + async fn queue_and_cache_remain_bounded() { let mut runtime = ImageRuntime::with_picker(Picker::halfblocks()); - let source = image("aW52YWxpZA=="); - assert!(runtime.prepare(&source, 40).is_none()); - assert!(runtime.prepare(&source, 20).is_none()); - assert_eq!(runtime.cache.len(), 1); - assert!(runtime.cache[&source.key].decoded.is_none()); + for seed in 0..40 { + runtime.prepare(&source(seed), 40); + assert!(runtime.cached_entries() <= MAX_CACHE_ENTRIES); + assert!(runtime.queued_bytes() <= MAX_QUEUED_SOURCE_BYTES); + } + let final_source = source(41); + runtime.prepare(&final_source, 40); + settle(&mut runtime, &final_source).await; + assert!(runtime.prepare(&final_source, 40).is_some()); + } + + #[tokio::test] + async fn exif_orientation_is_applied_before_protocol_geometry() { + use image::ImageEncoder as _; + let mut exif = b"II\x2a\0\x08\0\0\0\x01\0\x12\x01\x03\0\x01\0\0\0".to_vec(); + exif.extend(6_u16.to_le_bytes()); + exif.extend([0; 6]); + let pixels = image::RgbaImage::from_pixel(3, 2, image::Rgba([120, 80, 40, 255])); + let mut png = Vec::new(); + let mut encoder = image::codecs::png::PngEncoder::new(&mut png); + encoder.set_exif_metadata(exif).unwrap(); + encoder + .write_image(pixels.as_raw(), 3, 2, image::ExtendedColorType::Rgba8) + .unwrap(); + let source = MediaImage::new(STANDARD.encode(png), "image/png".into(), 0).unwrap(); + let mut runtime = ImageRuntime::with_picker(Picker::halfblocks()); + runtime.prepare(&source, 20); + settle(&mut runtime, &source).await; + let decoded = runtime.cache[&source.key].decoded.as_ref().unwrap(); + assert_eq!((decoded.width(), decoded.height()), (2, 3)); } #[test] - fn clear_drops_all_decoded_backing_bytes() { + fn rejects_source_and_pixel_budgets() { + assert!(decode(&"A".repeat(MAX_BASE64_BYTES + 1), "image/png").is_none()); + // A compact PNG can describe more than 16 MP while remaining below 8 MiB. let mut png = Cursor::new(Vec::new()); - image::DynamicImage::new_rgb8(20, 10) + image::DynamicImage::new_luma8(8192, 2049) .write_to(&mut png, image::ImageFormat::Png) .unwrap(); - let source = image(&STANDARD.encode(png.into_inner())); - let mut runtime = ImageRuntime::with_picker(Picker::halfblocks()); - assert!(runtime.prepare(&source, 20).is_some()); - - runtime.clear(); - - assert!(runtime.cache.is_empty()); - assert_eq!(runtime.decoded_backing_bytes, 0); + assert!(decode(&STANDARD.encode(png.into_inner()), "image/png").is_none()); } } diff --git a/src/tui/image_source.rs b/src/tui/image_source.rs new file mode 100644 index 0000000..3d58f80 --- /dev/null +++ b/src/tui/image_source.rs @@ -0,0 +1,709 @@ +//! Presentation-only image loading. This never grants managed-file authority. +use std::{ + collections::HashSet, + io::Read as _, + net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}, + path::{Path, PathBuf}, + sync::Arc, + time::Duration, +}; + +use url::Url; + +// The permit is also owned by every blocking stage. Dropping a presentation +// runtime cannot release admission while its file read or DNS lookup still runs. +static SOURCE_WORKERS: tokio::sync::Semaphore = tokio::sync::Semaphore::const_new(2); +type SourcePermit = Arc>; +const MAX_BYTES: u64 = 8 * 1024 * 1024; +const TOTAL_TIMEOUT: Duration = Duration::from_secs(20); +const IO_TIMEOUT: Duration = Duration::from_secs(5); +type Result = std::result::Result; + +#[derive(Clone, Debug, Default)] +pub(super) struct ImagePolicy { + origins: HashSet, +} + +impl ImagePolicy { + pub(super) fn from_environment() -> Self { + Self::parse(&std::env::var("KIT_TUI_IMAGE_ORIGINS").unwrap_or_default()) + } + + fn parse(value: &str) -> Self { + Self { + origins: value + .split(',') + .take(64) + .filter_map(|entry| { + let entry = entry.trim(); + if entry.len() > 2048 { + return None; + } + let authority = entry.strip_prefix("https://")?; + let authority = authority.strip_suffix('/').unwrap_or(authority); + if authority.is_empty() + || authority.contains(['/', '?', '#', '@', '\\']) + || authority.chars().any(char::is_whitespace) + { + return None; + } + let url = Url::parse(entry).ok()?; + (https_url(&url) + && url.path() == "/" + && url.query().is_none() + && url.fragment().is_none()) + .then(|| url.origin().ascii_serialization()) + }) + .collect(), + } + } + + fn permits(&self, url: &Url) -> bool { + https_url(url) && self.origins.contains(&url.origin().ascii_serialization()) + } +} + +fn https_url(url: &Url) -> bool { + url.scheme() == "https" + && url.host_str().is_some() + && url.username().is_empty() + && url.password().is_none() + && url.port_or_known_default() == Some(443) + && url.fragment().is_none() +} + +/// Caller bounds concurrent workers and discards cancelled/stale publications. +pub(super) async fn resolve( + source: &str, + root: &Path, + session: &str, + policy: &ImagePolicy, +) -> Result<(Vec, String)> { + if source.len() > 4096 || source.starts_with("//") || source.starts_with("\\\\") { + return Err("invalid image source".into()); + } + let permit = Arc::new( + tokio::time::timeout(IO_TIMEOUT, SOURCE_WORKERS.acquire()) + .await + .map_err(|_| "image workers busy".to_owned())? + .map_err(|_| "image workers unavailable".to_owned())?, + ); + if let Some(id) = source.strip_prefix("kit-file://") { + let store = crate::managed_files::FileStore::new(root); + let session = session.to_owned(); + let id = id.to_owned(); + return tokio::task::spawn_blocking(move || { + let _permit = permit; + store + .resolve_id(&session, &id) + .map_err(|_| "managed image unavailable".to_owned()) + }) + .await + .map_err(|_| "image worker unavailable".to_owned())?; + } + if source.split_once("://").is_some_and(|(_, rest)| { + rest.split(['/', '?', '#']) + .next() + .is_some_and(|authority| authority.contains('@')) + || source.contains('\\') + || source.chars().any(char::is_control) + }) { + return Err("invalid image URL".into()); + } + let bytes = if Path::new(source).is_absolute() { + local(PathBuf::from(source), permit.clone()).await? + } else if let Ok(url) = Url::parse(source) { + match url.scheme() { + "https" => { + if !policy.permits(&url) { + return Err("image origin not allowed".into()); + } + tokio::time::timeout(TOTAL_TIMEOUT, remote(url, permit.clone())) + .await + .map_err(|_| "image request timed out".to_owned())?? + } + "file" => { + if !url.username().is_empty() + || url.password().is_some() + || url.port().is_some() + || url.query().is_some() + || url.fragment().is_some() + || !matches!(url.host_str(), None | Some("localhost")) + { + return Err("invalid local image URL".into()); + } + local( + url.to_file_path() + .map_err(|_| "invalid local image path".to_owned())?, + permit.clone(), + ) + .await? + } + _ => return Err("unsupported image source".into()), + } + } else { + local(root.join(source), permit.clone()).await? + }; + // Sniff only; all origins share the runtime's single bounded decoder. + let format = image::guess_format(&bytes).map_err(|_| "invalid image format".to_owned())?; + if !matches!( + format, + image::ImageFormat::Png + | image::ImageFormat::Jpeg + | image::ImageFormat::Gif + | image::ImageFormat::WebP + ) { + return Err("unsupported image format".into()); + } + Ok((bytes, format.to_mime_type().to_owned())) +} + +async fn local(path: PathBuf, permit: SourcePermit) -> Result> { + tokio::task::spawn_blocking(move || { + let _permit = permit; + let mut options = std::fs::OpenOptions::new(); + options.read(true); + // Open first, then inspect the actual handle. Symlinks are allowed, but + // a FIFO must not block this worker before its type can be checked. + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt as _; + options.custom_flags(libc::O_NONBLOCK); + } + let file = options + .open(path) + .map_err(|_| "local image unavailable".to_owned())?; + let metadata = file + .metadata() + .map_err(|_| "local image unavailable".to_owned())?; + if !metadata.is_file() || metadata.len() > MAX_BYTES { + return Err("invalid or oversized local image".into()); + } + let mut bytes = Vec::new(); + file.take(MAX_BYTES + 1) + .read_to_end(&mut bytes) + .map_err(|_| "local image read failed".to_owned())?; + if bytes.len() as u64 > MAX_BYTES { + return Err("image too large".into()); + } + Ok(bytes) + }) + .await + .map_err(|_| "image worker unavailable".to_owned())? +} + +async fn remote(url: Url, permit: SourcePermit) -> Result> { + let host = url + .host_str() + .ok_or_else(|| "invalid image host".to_owned())?; + let addresses: Vec = match url.host() { + Some(url::Host::Ipv4(ip)) => vec![SocketAddr::new(ip.into(), 443)], + Some(url::Host::Ipv6(ip)) => vec![SocketAddr::new(ip.into(), 443)], + _ => { + let host = host.to_owned(); + // DNS is blocking and cannot be cancelled. Its actual closure owns + // admission until completion/unwind, even if either timeout fires + // or the awaiting presentation task is dropped. + let lookup = tokio::task::spawn_blocking(move || { + use std::net::ToSocketAddrs as _; + let _permit = permit; + (host.as_str(), 443) + .to_socket_addrs() + .map(|addresses| addresses.take(17).collect::>()) + }); + tokio::time::timeout(IO_TIMEOUT, lookup) + .await + .map_err(|_| "image DNS timed out".to_owned())? + .map_err(|_| "image DNS worker unavailable".to_owned())? + .map_err(|_| "image DNS failed".to_owned())? + } + }; + if addresses.is_empty() + || addresses.len() > 16 + || addresses.iter().any(|addr| !public_address(addr.ip())) + { + return Err("image address not public".into()); + } + // A fresh client has no cookie jar or credential defaults. Disabling proxy + // discovery and redirects prevents bypasses of the checked, pinned DNS set. + let client = reqwest::Client::builder() + .no_proxy() + .redirect(reqwest::redirect::Policy::none()) + .connect_timeout(IO_TIMEOUT) + .read_timeout(IO_TIMEOUT) + .timeout(TOTAL_TIMEOUT) + .resolve_to_addrs(host, &addresses) + .build() + .map_err(|_| "image client unavailable".to_owned())?; + let response = client + .get(url) + .send() + .await + .map_err(|_| "image request failed".to_owned())?; + read_response(response).await +} + +async fn read_response(mut response: reqwest::Response) -> Result> { + if response.status().is_redirection() { + return Err("image redirect rejected".into()); + } + if !response.status().is_success() { + return Err("image request rejected".into()); + } + if response + .content_length() + .is_some_and(|size| size > MAX_BYTES) + { + return Err("image too large".into()); + } + let mut bytes = Vec::new(); + while let Some(chunk) = response + .chunk() + .await + .map_err(|_| "image read failed".to_owned())? + { + if chunk.len() as u64 > MAX_BYTES - bytes.len() as u64 { + return Err("image too large".into()); + } + bytes.extend_from_slice(&chunk); + } + Ok(bytes) +} + +fn public_address(ip: IpAddr) -> bool { + match ip { + IpAddr::V4(ip) => public_v4(ip), + IpAddr::V6(ip) => public_v6(ip), + } +} + +fn public_v4(ip: Ipv4Addr) -> bool { + let [a, b, c, _] = ip.octets(); + // Fail closed on special-purpose blocks, including shared space, protocol + // assignments, documentation, benchmarking, multicast and future use. + !(a == 0 + || a == 10 + || a == 127 + || a >= 224 + || (a == 100 && (64..=127).contains(&b)) + || (a == 169 && b == 254) + || (a == 172 && (16..=31).contains(&b)) + || (a == 192 && (b == 0 || (b == 88 && c == 99) || b == 168)) + || (a == 198 && (b == 18 || b == 19 || (b == 51 && c == 100))) + || (a == 203 && b == 0 && c == 113)) +} + +fn public_v6(ip: Ipv6Addr) -> bool { + let s = ip.segments(); + // Only ordinary global unicast, excluding IETF special assignments, + // documentation, 6to4 and deprecated 6bone. This also rejects all mapped, + // translated, local, multicast and unspecified address representations. + (s[0] & 0xe000) == 0x2000 + && !(s[0] == 0x2001 && (s[1] <= 0x01ff || s[1] == 0x0db8)) + && s[0] != 0x2002 + && s[0] != 0x3ffe + && !(s[0] == 0x3fff && s[1] <= 0x0fff) +} + +#[cfg(test)] +#[allow( + clippy::unwrap_used, + clippy::expect_used, + clippy::disallowed_methods, + clippy::disallowed_macros +)] +mod tests { + use super::*; + use std::io::Cursor; + + fn png() -> Vec { + let mut bytes = Cursor::new(Vec::new()); + image::DynamicImage::new_rgb8(2, 3) + .write_to(&mut bytes, image::ImageFormat::Png) + .unwrap(); + bytes.into_inner() + } + + #[tokio::test] + #[ignore = "explicit external HTTPS smoke; requires public Internet and httpbin.org"] + async fn authorized_https_load_and_redirect_rejection() { + let root = tempfile::tempdir().unwrap(); + let policy = ImagePolicy::parse("https://httpbin.org"); + let (bytes, mime) = resolve( + "https://httpbin.org/image/png", + root.path(), + "smoke", + &policy, + ) + .await + .unwrap(); + assert_eq!(mime, "image/png"); + assert!(image::load_from_memory(&bytes).is_ok()); + let error = resolve( + "https://httpbin.org/redirect-to?url=https%3A%2F%2Fhttpbin.org%2Fimage%2Fpng", + root.path(), + "smoke", + &policy, + ) + .await + .unwrap_err(); + assert_eq!(error, "image redirect rejected"); + } + + #[test] + fn origins_are_exact_and_fail_closed() { + let policy = ImagePolicy::parse("https://example.com, https://other.example:443/"); + assert!(policy.permits(&Url::parse("https://example.com/image?q=1").unwrap())); + for url in [ + "http://example.com/a", + "https://sub.example.com/a", + "https://example.com:444/a", + "https://user@example.com/a", + "https://example.com/a#fragment", + ] { + assert!(!policy.permits(&Url::parse(url).unwrap()), "{url}"); + } + for origin in [ + "", + "https://example.com/path", + "https://example.com/path/..", + "https://example.com?", + "https://example.com#", + "https://@example.com", + "https://example.com:444", + "http://example.com", + "https://example.com\\", + ] { + assert!(ImagePolicy::parse(origin).origins.is_empty(), "{origin}"); + } + } + + #[test] + fn origin_count_and_length_are_capped() { + let origins = (0..65) + .map(|index| format!("https://host{index}.example")) + .collect::>(); + let policy = ImagePolicy::parse(&origins.join(",")); + assert_eq!(policy.origins.len(), 64); + assert!(policy.permits(&Url::parse(&origins[63]).unwrap())); + assert!(!policy.permits(&Url::parse(&origins[64]).unwrap())); + let overlong = format!("https://{}", "a".repeat(2041)); + assert_eq!(overlong.len(), 2049); + assert!(ImagePolicy::parse(&overlong).origins.is_empty()); + // A rejected entry does not grant authority or hide a later valid one. + let policy = ImagePolicy::parse(&format!("{overlong},https://allowed.example")); + assert_eq!(policy.origins.len(), 1); + assert!(policy.permits(&Url::parse("https://allowed.example/image").unwrap())); + } + + #[tokio::test] + async fn rejects_overlong_unc_and_nonlocal_file_sources() { + let dir = tempfile::tempdir().unwrap(); + let policy = ImagePolicy::default(); + for source in [ + "a".repeat(4097), + "//server/share/image.png".into(), + "\\\\server\\share\\image.png".into(), + ] { + assert_eq!( + resolve(&source, dir.path(), "s", &policy) + .await + .unwrap_err(), + "invalid image source" + ); + } + for source in [ + "file://server/share/image.png", + "file://127.0.0.1/image.png", + "file://localhost/image.png?secret=1", + "file://localhost/image.png#fragment", + ] { + assert_eq!( + resolve(source, dir.path(), "s", &policy).await.unwrap_err(), + "invalid local image URL" + ); + } + let path = dir.path().join("image.png"); + let bytes = png(); + std::fs::write(&path, &bytes).unwrap(); + let url = Url::from_file_path(&path).unwrap(); + let localhost = format!("file://localhost{}", url.path()); + assert_eq!( + resolve(&localhost, dir.path(), "s", &policy).await.unwrap(), + (bytes, "image/png".into()) + ); + } + + #[tokio::test] + async fn acquisition_sniffs_magic_without_claiming_pixel_validation() { + let dir = tempfile::tempdir().unwrap(); + let bytes = b"\x89PNG\r\n\x1a\n".to_vec(); + std::fs::write(dir.path().join("truncated.png"), &bytes).unwrap(); + // Acquisition accepts supported magic only. The common runtime decoder + // must reject this missing image header before publishing pixels. + assert_eq!( + resolve("truncated.png", dir.path(), "s", &ImagePolicy::default()) + .await + .unwrap(), + (bytes, "image/png".into()) + ); + } + + #[test] + fn rejects_special_addresses_in_both_families() { + for ip in [ + "0.0.0.0", + "10.1.2.3", + "100.64.0.1", + "127.0.0.1", + "169.254.169.254", + "172.16.0.1", + "192.168.1.1", + "192.0.2.1", + "192.88.99.1", + "198.18.0.1", + "198.51.100.1", + "203.0.113.1", + "224.0.0.1", + "255.255.255.255", + "::", + "::1", + "::ffff:8.8.8.8", + "64:ff9b::808:808", + "fc00::1", + "fe80::1", + "ff02::1", + "2001:db8::1", + "2001:20::1", + "2002:808:808::1", + "3fff::1", + ] { + assert!(!public_address(ip.parse().unwrap()), "{ip}"); + } + for ip in [ + "8.8.8.8", + "1.1.1.1", + "2606:4700:4700::1111", + "2001:4860:4860::8888", + ] { + assert!(public_address(ip.parse().unwrap()), "{ip}"); + } + } + + #[tokio::test] + async fn allowed_origins_still_cannot_fetch_nonpublic_hosts() { + for origin in [ + "https://127.0.0.1", + "https://[::1]", + "https://[::ffff:8.8.8.8]", + "https://localhost", + ] { + let policy = ImagePolicy::parse(origin); + assert_eq!( + resolve(origin, Path::new("."), "s", &policy) + .await + .unwrap_err(), + "image address not public" + ); + } + } + + #[tokio::test] + async fn local_bytes_are_bounded_and_sniffed() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("picture.png"); + let bytes = png(); + std::fs::write(&path, &bytes).unwrap(); + let policy = ImagePolicy::default(); + for source in [ + "picture.png".to_owned(), + path.to_str().unwrap().to_owned(), + Url::from_file_path(&path).unwrap().to_string(), + ] { + assert_eq!( + resolve(&source, dir.path(), "session", &policy) + .await + .unwrap(), + (bytes.clone(), "image/png".into()) + ); + } + assert!(resolve(".", dir.path(), "s", &policy).await.is_err()); + std::fs::write(&path, b"not an image").unwrap(); + assert_eq!( + resolve("picture.png", dir.path(), "s", &policy) + .await + .unwrap_err(), + "invalid image format" + ); + std::fs::File::create(&path) + .unwrap() + .set_len(MAX_BYTES + 1) + .unwrap(); + assert!( + resolve("picture.png", dir.path(), "s", &policy) + .await + .is_err() + ); + assert!( + resolve( + "https://example.com/private?token=secret", + dir.path(), + "s", + &policy + ) + .await + .unwrap_err() + == "image origin not allowed" + ); + } + + #[tokio::test] + async fn managed_source_preserves_session_isolation_without_reimport() { + let dir = tempfile::tempdir().unwrap(); + let source = dir.path().join("image.png"); + let bytes = png(); + std::fs::write(&source, &bytes).unwrap(); + let store = crate::managed_files::FileStore::new(dir.path()); + let reference = store.import("owner", &source, None).unwrap(); + let descriptor = serde_json::to_value(reference).unwrap(); + let id = descriptor["id"].as_str().unwrap(); + let uri = format!("kit-file://{id}"); + std::fs::remove_file(&source).unwrap(); + assert_eq!( + resolve(&uri, dir.path(), "owner", &ImagePolicy::default()) + .await + .unwrap(), + (bytes, "image/png".into()) + ); + assert_eq!( + resolve(&uri, dir.path(), "other", &ImagePolicy::default()) + .await + .unwrap_err(), + "managed image unavailable" + ); + for suffix in ["/", "?x=1", "#fragment"] { + assert!( + resolve( + &format!("{uri}{suffix}"), + dir.path(), + "owner", + &ImagePolicy::default() + ) + .await + .is_err() + ); + } + } + + #[tokio::test] + async fn actual_streamed_body_is_bounded_without_content_length() { + use tokio::io::{AsyncReadExt as _, AsyncWriteExt as _}; + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let server = tokio::spawn(async move { + let (mut stream, _) = listener.accept().await.unwrap(); + let mut buffer = [0; 4096]; + let _ = stream.read(&mut buffer).await.unwrap(); + stream + .write_all( + b"HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\nConnection: close\r\n\r\n", + ) + .await + .unwrap(); + let chunk = vec![0; 1024 * 1024]; + for _ in 0..9 { + if stream.write_all(b"100000\r\n").await.is_err() { + return; + } + if stream.write_all(&chunk).await.is_err() { + return; + } + if stream.write_all(b"\r\n").await.is_err() { + return; + } + } + let _ = stream.write_all(b"0\r\n\r\n").await; + }); + let response = reqwest::Client::builder() + .no_proxy() + .build() + .unwrap() + .get(format!("http://{address}/")) + .send() + .await + .unwrap(); + assert_eq!(response.content_length(), None); + assert_eq!( + read_response(response).await.unwrap_err(), + "image too large" + ); + server.await.unwrap(); + } + + #[cfg(unix)] + #[tokio::test] + async fn symlinks_allowed_fifo_and_permissions_rejected() { + use std::os::unix::fs::{PermissionsExt, symlink}; + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("image"); + std::fs::write(&path, png()).unwrap(); + symlink(&path, dir.path().join("link")).unwrap(); + assert!( + resolve("link", dir.path(), "s", &ImagePolicy::default()) + .await + .is_ok() + ); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o0)).unwrap(); + // Root can read mode-000 files; assert only when the OS denies access. + if std::fs::File::open(&path).is_err() { + assert!( + resolve("image", dir.path(), "s", &ImagePolicy::default()) + .await + .is_err() + ); + } + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600)).unwrap(); + let fifo = dir.path().join("fifo"); + assert!( + std::process::Command::new("mkfifo") + .arg(&fifo) + .status() + .unwrap() + .success() + ); + assert!( + resolve("fifo", dir.path(), "s", &ImagePolicy::default()) + .await + .is_err() + ); + } + + #[tokio::test] + async fn actual_http_redirect_response_is_rejected_without_following() { + use tokio::io::{AsyncReadExt as _, AsyncWriteExt as _}; + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let server = tokio::spawn(async move { + let (mut stream, _) = listener.accept().await.unwrap(); + let mut buffer = [0; 4096]; + let _ = stream.read(&mut buffer).await.unwrap(); + stream.write_all(b"HTTP/1.1 302 Found\r\nLocation: http://127.0.0.1/private\r\nContent-Length: 0\r\nConnection: close\r\n\r\n").await.unwrap(); + }); + // Exercise the real HTTP response boundary; production still only + // permits HTTPS/public DNS, with no test bypass in its network path. + let response = reqwest::Client::builder() + .no_proxy() + .redirect(reqwest::redirect::Policy::none()) + .build() + .unwrap() + .get(format!("http://{address}/")) + .send() + .await + .unwrap(); + assert_eq!( + read_response(response).await.unwrap_err(), + "image redirect rejected" + ); + server.await.unwrap(); + } +} diff --git a/src/tui/markdown.rs b/src/tui/markdown.rs index 05398b4..40831b4 100644 --- a/src/tui/markdown.rs +++ b/src/tui/markdown.rs @@ -7,6 +7,69 @@ use std::ops::Range; +/// A completed CommonMark image occurrence. Ordinary links and code never load media. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(super) struct ImageNode { + pub range: Range, + pub line: usize, + pub destination: String, + pub alt: String, +} + +pub(super) fn image_nodes(source: &str) -> Vec { + use pulldown_cmark::{Event, Parser, Tag, TagEnd}; + + // Bound parser work and occurrence retention independently of image bytes. + if source.len() > 1024 * 1024 { + return Vec::new(); + } + let mut nodes = Vec::new(); + let mut current: Option = None; + let mut depth = 0usize; + for (event, range) in Parser::new(source).into_offset_iter() { + match event { + Event::Start(Tag::Image { dest_url, .. }) => { + if depth == 0 { + current = Some(ImageNode { + line: source[..range.start] + .bytes() + .filter(|byte| *byte == b'\n') + .count(), + range, + destination: dest_url.into_string(), + alt: String::new(), + }); + } + depth += 1; + } + Event::End(TagEnd::Image) => { + depth = depth.saturating_sub(1); + if depth == 0 + && let Some(mut node) = current.take() + { + node.range.end = range.end; + nodes.push(node); + if nodes.len() == 64 { + break; + } + } + } + Event::Text(text) | Event::Code(text) if depth > 0 => { + if let Some(node) = &mut current { + node.alt.push_str(&text); + } + } + Event::SoftBreak | Event::HardBreak if depth > 0 => { + if let Some(node) = &mut current { + node.alt.push(' '); + } + } + _ => {} + } + } + nodes +} + use super::{ theme, wrap::{LinkedLine, LinkedSpan}, @@ -17,10 +80,25 @@ use ratatui::{ }; use unicode_width::UnicodeWidthStr; +#[cfg(test)] pub(super) fn render_copyable_at_width( source: &str, max_width: Option, ) -> Vec<(LinkedLine, Option>)> { + render_copyable_with_source_at_width(source, max_width) + .into_iter() + .map(|(line, code, _)| (line, code)) + .collect() +} + +/// Render the complete document, retaining code-copy ranges and byte coverage. +/// Coverage includes raw line endings; synthetic frames have empty ranges. +/// A source row expanded into several table lines is covered by its last line, +/// so media insertion cannot interrupt that row before its contents are shown. +pub(super) fn render_copyable_with_source_at_width( + source: &str, + max_width: Option, +) -> Vec<(LinkedLine, Option>, Range)> { let mut next_offset = 0; let raw_lines: Vec<(usize, &str)> = source .split('\n') @@ -37,6 +115,7 @@ pub(super) fn render_copyable_at_width( if index < table_end { continue; } + let coverage = offset..(offset + raw.len() + 1).min(source.len()); let trimmed = raw.trim_start(); let candidate = fence_line(raw); if let Some((language, marker, length, content)) = fence.as_ref() { @@ -51,10 +130,11 @@ pub(super) fn render_copyable_at_width( } )), Some(content.clone()), + coverage, )); fence = None; } else { - lines.push((code_line(raw), Some(content.clone()))); + lines.push((code_line(raw), Some(content.clone()), coverage)); } continue; } @@ -85,19 +165,28 @@ pub(super) fn render_copyable_at_width( } )), Some(content.clone()), + coverage, )); fence = Some((language, marker, length, content)); continue; } - if let Some((end, table)) = table_at(&raw_lines, index, max_width) { - lines.extend(table.into_iter().map(|line| (line, None))); + if let Some((end, table)) = table_at(&raw_lines, index, max_width, source.len()) { + lines.extend( + table + .into_iter() + .map(|(line, coverage)| (line, None, coverage)), + ); table_end = end; continue; } - lines.push((block_line(raw, trimmed), None)); + lines.push((block_line(raw, trimmed), None, coverage)); } if let Some((_, _, _, content)) = fence { - lines.push((code_frame("└─ code"), Some(content))); + lines.push(( + code_frame("└─ code"), + Some(content), + source.len()..source.len(), + )); } lines } @@ -114,11 +203,14 @@ struct TableCell { width: usize, } +type CoveredLine = (LinkedLine, Range); + fn table_at( lines: &[(usize, &str)], start: usize, max_width: Option, -) -> Option<(usize, Vec)> { + source_len: usize, +) -> Option<(usize, Vec)> { let header_line = lines.get(start)?.1; if !table_row_allowed(header_line) { return None; @@ -172,20 +264,51 @@ fn table_at( .map(|column| rows.iter().map(|row| row[column].width).max().unwrap_or(0)) .collect::>(); + let coverage = |index: usize| { + let (offset, raw) = lines[index]; + offset..(offset + raw.len() + 1).min(source_len) + }; let table_width = 1 + widths.iter().map(|width| width + 3).sum::(); if max_width.is_some_and(|max_width| table_width > max_width) { - return Some((end, stacked_table(&rows))); + let rendered = stacked_table(&rows) + .into_iter() + .enumerate() + .map(|(index, line)| { + let row = index / columns; + let last_column = index % columns == columns - 1; + let raw_row = if rows.len() == 1 { + start + 1 + } else { + start + 2 + row + }; + let range_start = if row == 0 { + lines[start].0 + } else { + lines[raw_row].0 + }; + let range_end = if last_column { + coverage(raw_row).end + } else { + range_start + }; + (line, range_start..range_end) + }) + .collect(); + return Some((end, rendered)); } - let mut rendered = vec![table_rule(&widths, '┌', '┬', '┐')]; - rendered.push(table_row(&rows[0], &widths, &alignments)); - rendered.push(table_rule(&widths, '├', '┼', '┤')); - rendered.extend( - rows[1..] - .iter() - .map(|row| table_row(row, &widths, &alignments)), - ); - rendered.push(table_rule(&widths, '└', '┴', '┘')); + let table_start = lines[start].0; + let mut rendered = vec![(table_rule(&widths, '┌', '┬', '┐'), table_start..table_start)]; + rendered.push((table_row(&rows[0], &widths, &alignments), coverage(start))); + rendered.push((table_rule(&widths, '├', '┼', '┤'), coverage(start + 1))); + rendered.extend(rows[1..].iter().enumerate().map(|(row, cells)| { + ( + table_row(cells, &widths, &alignments), + coverage(start + 2 + row), + ) + })); + let table_end = coverage(end - 1).end; + rendered.push((table_rule(&widths, '└', '┴', '┘'), table_end..table_end)); Some((end, rendered)) } @@ -694,6 +817,125 @@ mod tests { .collect() } + #[test] + fn source_coverage_uses_raw_unicode_and_crlf_byte_offsets() { + let source = "# café\r\n雪\r\n"; + let rendered = render_copyable_with_source_at_width(source, None); + let covered: Vec<_> = rendered + .iter() + .map(|(_, _, range)| &source[range.clone()]) + .collect(); + assert_eq!(covered, ["# café\r\n", "雪\r\n", ""]); + assert_eq!(rendered.last().unwrap().2, source.len()..source.len()); + assert!(rendered.iter().all(|(_, code, _)| code.is_none())); + } + + #[test] + fn fenced_source_coverage_is_separate_from_complete_code_copy_range() { + let source = "前\r\n```rust\r\n雪\r\n```\r\n後"; + let rendered = render_copyable_with_source_at_width(source, None); + let covered: Vec<_> = rendered + .iter() + .map(|(_, _, range)| &source[range.clone()]) + .collect(); + assert_eq!( + covered, + ["前\r\n", "```rust\r\n", "雪\r\n", "```\r\n", "後"] + ); + for (_, code, _) in &rendered[1..4] { + assert_eq!(&source[code.clone().unwrap()], "雪"); + } + let unclosed = "~~~\r\n雪"; + let rendered = render_copyable_with_source_at_width(unclosed, None); + assert_eq!(rendered[0].2, 0..5); + assert_eq!(rendered[1].2, 5..unclosed.len()); + assert_eq!(rendered[2].2, unclosed.len()..unclosed.len()); + for (_, code, _) in rendered { + assert_eq!(&unclosed[code.unwrap()], "雪"); + } + } + + #[test] + fn table_source_coverage_places_images_after_their_rendered_rows() { + let source = "| 頭 | B |\r\n| --- | --- |\r\n| ![雪](image.png) | x |\r\n| last | y |"; + let rendered = render_copyable_with_source_at_width(source, None); + let covered: Vec<_> = rendered + .iter() + .map(|(_, _, range)| &source[range.clone()]) + .collect(); + assert_eq!( + covered, + [ + "", + "| 頭 | B |\r\n", + "| --- | --- |\r\n", + "| ![雪](image.png) | x |\r\n", + "| last | y |", + "" + ] + ); + let node = image_nodes(source).remove(0); + assert_eq!( + rendered + .iter() + .position(|(_, _, range)| range.end >= node.range.end), + Some(3) + ); + let stacked = render_copyable_with_source_at_width(source, Some(1)); + assert_eq!(stacked.len(), 4); + assert_eq!(stacked[0].2, 0..0); + assert_eq!(stacked[1].2.end, rendered[3].2.end); + assert_eq!( + stacked + .iter() + .position(|(_, _, range)| range.end >= node.range.end), + Some(1) + ); + assert_eq!(stacked[2].2.start, stacked[2].2.end); + assert_eq!(stacked[3].2.end, source.len()); + + let header_only = "| ![頭](header.png) | B |\n| --- | --- |"; + let node = image_nodes(header_only).remove(0); + for width in [None, Some(1)] { + let rows = render_copyable_with_source_at_width(header_only, width); + assert_eq!( + rows.iter() + .position(|(_, _, range)| range.end >= node.range.end), + Some(1) + ); + } + } + + #[test] + fn compatibility_wrapper_preserves_styles_links_gutters_and_code_ranges() { + for source in [ + "# café\n- **bold** [link](https://example.com)\n> quote", + "before\r\n```rust\r\nlet 雪 = 1;\r\n```\r\nafter", + "~~~\n雪", + "| **A** | B |\n| --- | --- |\n| [link](https://example.com) | 雪 |", + "| A | B |\n| --- | --- |", + "", + ] { + for width in [None, Some(1), Some(80)] { + let original = render_copyable_at_width(source, width); + let covered = render_copyable_with_source_at_width(source, width); + assert_eq!(original.len(), covered.len()); + for ((line, code), (with_source, source_code, range)) in + original.iter().zip(&covered) + { + assert_eq!(format!("{line:?}"), format!("{with_source:?}")); + assert_eq!(code, source_code); + assert!(source.get(range.clone()).is_some()); + } + assert!( + covered + .windows(2) + .all(|rows| rows[0].2.end <= rows[1].2.end) + ); + } + } + } + #[test] fn renders_aligned_pipe_tables() { let rendered = @@ -989,6 +1231,31 @@ mod tests { ); } + #[test] + fn image_nodes_are_completed_commonmark_not_links_or_code() { + let source = "![a *bold* image](kit-file://example)\n![again][pic]\n\n[pic]: /tmp/image.png\n\n`![code](bad.png)`\n```md\n![fence](bad.png)\n```\n\\![escaped](bad.png)\n[ordinary](bad.png)\n![partial]("; + let nodes = super::image_nodes(source); + assert_eq!(nodes.len(), 2); + assert_eq!(nodes[0].alt, "a bold image"); + assert_eq!(nodes[0].destination, "kit-file://example"); + assert_eq!( + &source[nodes[0].range.clone()], + "![a *bold* image](kit-file://example)" + ); + assert_eq!(nodes[1].destination, "/tmp/image.png"); + assert_eq!(nodes[1].line, 1); + } + + #[test] + fn image_nodes_preserve_repeated_occurrences_and_stream_completion() { + assert!(super::image_nodes("![alt](https://example.com/a").is_empty()); + let nodes = + super::image_nodes("![alt](https://example.com/a) ![alt](https://example.com/a)"); + assert_eq!(nodes.len(), 2); + assert_ne!(nodes[0].range, nodes[1].range); + assert_eq!(nodes[0].destination, nodes[1].destination); + } + #[test] fn bare_links_keep_balanced_parentheses_and_trim_sentence_punctuation() { assert_eq!( diff --git a/src/tui/markdown_images.rs b/src/tui/markdown_images.rs new file mode 100644 index 0000000..ae09fa9 --- /dev/null +++ b/src/tui/markdown_images.rs @@ -0,0 +1,288 @@ +//! Presentation-only external image snapshots. Never publishes files or model attachments. +use std::{ + collections::{HashMap, HashSet}, + path::{Path, PathBuf}, +}; + +use base64::{Engine as _, engine::general_purpose::STANDARD}; +use tokio::task::JoinSet; + +use super::{ + app::MediaImage, + image_source::{self, ImagePolicy}, +}; + +const MAX_ENTRIES: usize = 32; +const MAX_SOURCE_BYTES: usize = 32 * 1024 * 1024; +const MAX_JOBS: usize = 2; + +enum State { + Waiting, + Loading(tokio::task::Id), + Ready(MediaImage), + Failed(String), +} + +struct Entry { + state: State, + used: u64, +} + +struct Completion { + generation: u64, + source: String, + image: Result, +} + +pub(super) struct MarkdownImages { + root: PathBuf, + session: String, + generation: u64, + policy: ImagePolicy, + entries: HashMap, + changed_sources: HashSet, + jobs: JoinSet, + clock: u64, + retained: usize, +} + +impl MarkdownImages { + pub fn new() -> Self { + Self { + root: PathBuf::new(), + session: String::new(), + generation: 0, + policy: ImagePolicy::from_environment(), + entries: HashMap::new(), + changed_sources: HashSet::new(), + jobs: JoinSet::new(), + clock: 0, + retained: 0, + } + } + + pub fn context(&mut self, root: &Path, session: Option<&str>) { + let session = session.unwrap_or_default(); + if self.root != root || self.session != session { + self.clear(); + self.root = root.to_path_buf(); + self.session = session.to_owned(); + } + } + + pub fn clear(&mut self) { + self.generation = self.generation.wrapping_add(1); + self.changed_sources.extend(self.entries.keys().cloned()); + self.entries.clear(); + self.retained = 0; + // Stop old async stages, including requests following DNS. Actual blocking + // closures retain global admission until completion even after this abort. + self.jobs.abort_all(); + } + + pub fn take_changed_sources(&mut self) -> HashSet { + std::mem::take(&mut self.changed_sources) + } + + pub fn pending(&self) -> bool { + !self.jobs.is_empty() + } + + pub fn poll(&mut self) -> bool { + let mut changed = false; + while let Some(result) = self.jobs.try_join_next() { + let done = match result { + Ok(done) => done, + Err(error) => { + for entry in self.entries.values_mut() { + if matches!(entry.state, State::Loading(id) if id == error.id()) { + entry.state = State::Failed("image worker failed".into()); + changed = true; + } + } + continue; + } + }; + if done.generation != self.generation { + continue; + } + let Some(entry) = self.entries.get_mut(&done.source) else { + continue; + }; + entry.state = match done.image { + Ok(image) if self.retained.saturating_add(image.data.len()) <= MAX_SOURCE_BYTES => { + self.retained += image.data.len(); + self.changed_sources.insert(done.source); + State::Ready(image) + } + Ok(_) => State::Failed("image source budget exceeded".into()), + Err(error) => State::Failed(error), + }; + changed = true; + } + changed + } + + pub fn request(&mut self, source: &str) { + if source.len() > 4096 { + return; + } + self.clock = self.clock.wrapping_add(1); + if !self.entries.contains_key(source) { + if self.entries.len() == MAX_ENTRIES { + let oldest = self + .entries + .iter() + .filter(|(_, entry)| !matches!(entry.state, State::Loading(_))) + .min_by_key(|(_, entry)| entry.used) + .map(|(source, _)| source.clone()); + let Some(oldest) = oldest else { return }; + if let Some(Entry { + state: State::Ready(image), + .. + }) = self.entries.remove(&oldest) + { + self.retained = self.retained.saturating_sub(image.data.len()); + self.changed_sources.insert(oldest); + } + } + self.entries.insert( + source.to_owned(), + Entry { + state: State::Waiting, + used: self.clock, + }, + ); + } + let Some(entry) = self.entries.get_mut(source) else { + return; + }; + entry.used = self.clock; + if !matches!(entry.state, State::Waiting) + || self.jobs.len() >= MAX_JOBS + || self.session.is_empty() + { + return; + } + let Ok(handle) = tokio::runtime::Handle::try_current() else { + return; + }; + let source = source.to_owned(); + let root = self.root.clone(); + let session = self.session.clone(); + let policy = self.policy.clone(); + let generation = self.generation; + let job = self.jobs.spawn_on( + async move { + let image = image_source::resolve(&source, &root, &session, &policy) + .await + .and_then(|(bytes, mime)| { + MediaImage::new(STANDARD.encode(bytes), mime, 0) + .ok_or_else(|| "image source budget exceeded".to_owned()) + }); + Completion { + generation, + source, + image, + } + }, + &handle, + ); + entry.state = State::Loading(job.id()); + } + + pub fn image(&self, source: &str) -> Option<&MediaImage> { + match &self.entries.get(source)?.state { + State::Ready(image) => Some(image), + _ => None, + } + } + + pub fn status(&self, source: &str) -> &str { + if source.len() > 4096 { + return "image source too long"; + } + match self.entries.get(source).map(|entry| &entry.state) { + Some(State::Ready(_)) => "image", + Some(State::Failed(error)) => error, + _ => "image loading", + } + } +} + +#[cfg(test)] +#[allow( + clippy::unwrap_used, + clippy::expect_used, + clippy::disallowed_methods, + clippy::disallowed_macros +)] +mod tests { + use super::*; + use std::{io::Cursor, time::Duration}; + + fn write_png(root: &Path, name: &str) { + let mut bytes = Cursor::new(Vec::new()); + image::DynamicImage::new_rgb8(2, 3) + .write_to(&mut bytes, image::ImageFormat::Png) + .unwrap(); + std::fs::write(root.join(name), bytes.into_inner()).unwrap(); + } + + async fn settle(images: &mut MarkdownImages) { + tokio::time::timeout(Duration::from_secs(5), async { + while images.pending() { + images.poll(); + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + } + + #[tokio::test] + async fn local_snapshot_and_negative_cache_reset_on_session_change() { + let root = tempfile::tempdir().unwrap(); + write_png(root.path(), "image.png"); + let mut images = MarkdownImages::new(); + images.context(root.path(), Some("first")); + images.request("image.png"); + assert_eq!(images.status("image.png"), "image loading"); + settle(&mut images).await; + assert!(images.image("image.png").is_some()); + std::fs::remove_file(root.path().join("image.png")).unwrap(); + images.request("image.png"); + assert!(!images.pending()); + assert!(images.image("image.png").is_some()); + images.context(root.path(), Some("second")); + images.request("image.png"); + settle(&mut images).await; + assert!(images.image("image.png").is_none()); + assert_eq!(images.status("image.png"), "local image unavailable"); + write_png(root.path(), "image.png"); + images.request("image.png"); + assert!(!images.pending()); + images.clear(); + images.request("image.png"); + settle(&mut images).await; + assert!(images.image("image.png").is_some()); + } + + #[tokio::test] + async fn session_reset_discards_inflight_results() { + let root = tempfile::tempdir().unwrap(); + write_png(root.path(), "image.png"); + let mut images = MarkdownImages::new(); + images.context(root.path(), Some("first")); + images.request("image.png"); + images.context(root.path(), Some("second")); + // No yield occurred after spawn: reset cancels the actual pending future, + // not merely publication of a completed read into another session. + let cancelled = images.jobs.join_next().await.unwrap().err().unwrap(); + assert!(cancelled.is_cancelled()); + assert!(images.image("image.png").is_none()); + images.request("image.png"); + settle(&mut images).await; + assert!(images.image("image.png").is_some()); + } +} diff --git a/src/tui/mod.rs b/src/tui/mod.rs index 6d76308..8888bb0 100644 --- a/src/tui/mod.rs +++ b/src/tui/mod.rs @@ -10,7 +10,9 @@ mod app; mod command; mod editor; mod image; +mod image_source; mod markdown; +mod markdown_images; mod progress; mod theme; mod ui; @@ -71,8 +73,8 @@ use crate::{ }; use app::{ - Action, App, Attachment, AttachmentKind, EffortChoice, ModelChoice, SubmittedPrompt, Update, - UserImage, + Action, App, Attachment, AttachmentKind, EffortChoice, MediaImage, ModelChoice, + SubmittedPrompt, Update, }; struct ModelSwitchCompletion { @@ -1158,7 +1160,7 @@ pub async fn run_with_reasoning_effort_and_openrouter_key( return Err(agent_client_protocol::Error::into_internal_error(error)); } let event = { - let redraw = app.needs_redraw_tick(); + let redraw = app.needs_redraw_tick() || images.pending(); let mut stopped = pin!(stop.requested()); // Rotate the first eligible source after every winner. // If none is ready, poll every eligible source with the @@ -1347,7 +1349,7 @@ pub async fn run_with_reasoning_effort_and_openrouter_key( .draw(|frame| ui::draw(frame, &mut app, &mut images)) .map_err(agent_client_protocol::Error::into_internal_error)?; let event = { - let redraw = app.needs_redraw_tick(); + let redraw = app.needs_redraw_tick() || images.pending(); let mut stopped = pin!(stop.requested()); let mut shutdown = pin!(storage_shutdown.cancelled()); // A local round-robin race keeps hot input/update queues @@ -2547,14 +2549,15 @@ fn translate(notification: UpdateSessionNotification) -> (String, Vec) { append: true, }] } - SessionUpdate::AgentMessageChunk(chunk) => message_of(chunk.content) - .map(|text| Update::AgentMessage { + SessionUpdate::AgentMessageChunk(chunk) => { + let (text, images) = user_message_of(vec![chunk.content]); + vec![Update::AgentMessage { id: chunk.message_id.to_string(), text, + images, append: true, - }) - .into_iter() - .collect(), + }] + } SessionUpdate::AgentThoughtChunk(chunk) => match chunk.content { ContentBlock::Text(text) => vec![Update::AgentThought { id: chunk.message_id.to_string(), @@ -2718,11 +2721,15 @@ fn message_patch( append: false, }]; } - MessageKind::Agent => |id, text| Update::AgentMessage { - id, - text, - append: false, - }, + MessageKind::Agent => { + let (text, images) = user_message_of(blocks); + return vec![Update::AgentMessage { + id, + text, + images, + append: false, + }]; + } MessageKind::Thought => |id, text| Update::AgentThought { id, text, @@ -2737,7 +2744,7 @@ fn message_patch( vec![patch(id, text)] } -fn user_message_of(blocks: Vec) -> (String, Vec) { +fn user_message_of(blocks: Vec) -> (String, Vec) { let mut text = String::new(); let mut images = Vec::new(); let mut image_ordinal = 0; @@ -2747,7 +2754,8 @@ fn user_message_of(blocks: Vec) -> (String, Vec) { match block { ContentBlock::Image(image) => { image_ordinal += 1; - let uri = image.uri.filter(|uri| safe_media_uri(uri)); + let source_uri = image.uri; + let uri = source_uri.clone().filter(|uri| safe_media_uri(uri)); let existing_line = uri .as_deref() .and_then(|uri| markdown::line_with_link(&text, uri)); @@ -2765,8 +2773,11 @@ fn user_message_of(blocks: Vec) -> (String, Vec) { separate_after_image = true; line }); - if let Some(image) = UserImage::new(image.data, image.mime_type.to_string(), line) { - images.push(image); + if images.len() < app::MAX_MESSAGE_IMAGES + && let Some(image) = + MediaImage::new(image.data, image.mime_type.to_string(), line) + { + images.push(image.with_uri(source_uri)); } } block => { @@ -2808,7 +2819,12 @@ fn message_of(content: ContentBlock) -> Option { fn safe_media_uri(uri: &str) -> bool { uri.len() <= 2_048 - && url::Url::parse(uri).is_ok_and(|uri| matches!(uri.scheme(), "file" | "http" | "https")) + && !uri.chars().any(char::is_control) + && url::Url::parse(uri).is_ok_and(|uri| { + uri.username().is_empty() + && uri.password().is_none() + && matches!(uri.scheme(), "file" | "http" | "https" | "kit-file") + }) } /// The Runlet program inside a `compose` call's input, when there is one. @@ -2950,8 +2966,8 @@ fn redact_image_data_urls(text: &str) -> String { // Keep pixels separate from text previews. Live chunks and replay snapshots // use this path; never fetch a model-supplied URI. -fn tool_images_of(content: &[ToolCallContent]) -> Vec { - let mut images: Vec = Vec::new(); +fn tool_images_of(content: &[ToolCallContent]) -> Vec { + let mut images: Vec = Vec::new(); let mut retained = 0usize; for entry in content { if images.len() >= app::MAX_TOOL_IMAGES { @@ -2966,7 +2982,8 @@ fn tool_images_of(content: &[ToolCallContent]) -> Vec { if retained.saturating_add(image.data.len()) > app::MAX_RETAINED_IMAGE_SOURCE_BYTES { continue; } - if let Some(image) = UserImage::new(image.data.clone(), image.mime_type.to_string(), 0) + if let Some(image) = MediaImage::new(image.data.clone(), image.mime_type.to_string(), 0) + .map(|source| source.with_uri(image.uri.clone())) && !images.iter().any(|existing| existing.key == image.key) { retained += image.data.len(); @@ -3856,7 +3873,7 @@ mod tests { ); assert!(matches!( translate_for_session(message, "session").as_slice(), - [Update::AgentMessage { id, text, append: false }] + [Update::AgentMessage { id, text, append: false, .. }] if id == "message" && text.is_empty() )); @@ -3866,7 +3883,7 @@ mod tests { ); assert!(matches!( translate_for_session(thought, "session").as_slice(), - [Update::AgentThought { id, text, append: false }] + [Update::AgentThought { id, text, append: false, .. }] if id == "thought" && text.is_empty() )); } @@ -4062,10 +4079,10 @@ mod tests { let links = "inspect these[file:///tmp/image.png](file:///tmp/image.png)[https://example.com/result.png](https://example.com/result.png)"; assert_eq!(user.text, format!("{links}\n[Image #1]")); assert_eq!(user.images.len(), 1); - assert_eq!(user.images[0].data, "c2VjcmV0"); - assert_eq!(summary, "summary"); + assert_eq!(user.images[0].data.as_ref(), "c2VjcmV0"); + assert_eq!(summary.text, "summary"); assert_eq!(tool.images.len(), 1); - assert_eq!(tool.images[0].data, "c2VjcmV0"); + assert_eq!(tool.images[0].data.as_ref(), "c2VjcmV0"); assert_eq!(tool.status, wire::ToolCallStatus::Completed); assert_eq!( tool.output, @@ -4109,8 +4126,158 @@ mod tests { ); assert!( matches!(translate_for_session(update, "session").as_slice(), - [Update::AgentMessage { text, .. }] if text == "[Image]") + [Update::AgentMessage { text, .. }] if text == "[Image #1]") + ); + } + } + + #[test] + fn assistant_images_survive_live_and_replay_with_source_uri() { + let image = ContentBlock::Image( + wire::ImageContent::new("c2VjcmV0", "image/png") + .uri(Some("file:///tmp/native.png".into())), + ); + let updates = [ + SessionUpdate::AgentMessageChunk(wire::ContentChunk::new(image.clone(), "native")), + SessionUpdate::AgentMessage(AgentMessage::new("native").content(vec![image])), + ]; + for (index, update) in updates.into_iter().enumerate() { + let translated = + translate_for_session(UpdateSessionNotification::new("session", update), "session"); + let [ + Update::AgentMessage { + text, + images, + append, + .. + }, + ] = translated.as_slice() + else { + panic!("expected typed assistant message"); + }; + assert_eq!(*append, index == 0); + assert_eq!(images.len(), 1); + assert_eq!( + images[0].source_uri.as_deref(), + Some("file:///tmp/native.png") + ); + assert!(!text.contains("c2VjcmV0")); + assert!(!format!("{translated:?}").contains("c2VjcmV0")); + } + } + + #[test] + fn typed_message_snapshots_bound_occurrences_and_keep_overflow_text() { + for user in [true, false] { + for payload in ["", "AQID"] { + let content = (0..super::app::MAX_MESSAGE_IMAGES + 100) + .map(|_| ContentBlock::Image(wire::ImageContent::new(payload, "image/png"))) + .collect::>(); + let update = if user { + SessionUpdate::UserMessage(UserMessage::new("snapshot").content(content)) + } else { + SessionUpdate::AgentMessage(AgentMessage::new("snapshot").content(content)) + }; + let updates = translate_for_session( + UpdateSessionNotification::new("session", update), + "session", + ); + let [ + Update::UserMessage { text, images, .. } + | Update::AgentMessage { text, images, .. }, + ] = updates.as_slice() + else { + panic!("typed snapshot"); + }; + assert_eq!( + images.len(), + if payload.is_empty() { + 0 + } else { + super::app::MAX_MESSAGE_IMAGES + } + ); + assert!(text.contains(&format!( + "[Image #{}]", + super::app::MAX_MESSAGE_IMAGES + 100 + ))); + } + } + } + + #[test] + fn empty_typed_live_chunks_preserve_fallback_without_images() { + for user in [true, false] { + let content = ContentBlock::Image(wire::ImageContent::new("", "image/png")); + let chunk = wire::ContentChunk::new(content, "empty"); + let update = if user { + SessionUpdate::UserMessageChunk(chunk) + } else { + SessionUpdate::AgentMessageChunk(chunk) + }; + let updates = + translate_for_session(UpdateSessionNotification::new("session", update), "session"); + let [ + Update::UserMessage { text, images, .. } + | Update::AgentMessage { text, images, .. }, + ] = updates.as_slice() + else { + panic!("typed chunk"); + }; + assert_eq!(text, "[Image #1]"); + assert!(images.is_empty()); + } + } + + #[test] + fn tool_snapshots_bound_tiny_occurrences_and_empty_replacement_releases_them() { + let mut app = App::new( + PathBuf::from("/tmp"), + "provider".into(), + "model".into(), + "a2a".into(), + ); + for empty in [false, true, false] { + let content = (0..super::app::MAX_TOOL_IMAGES + 100) + .map(|index| { + let payload = if empty { + String::new() + } else { + format!("{index:04}") + }; + wire::ToolCallContent::Content(Box::new(wire::Content::new( + ContentBlock::Image(wire::ImageContent::new(payload, "image/png")), + ))) + }) + .collect::>(); + let notification = UpdateSessionNotification::new( + "session", + SessionUpdate::ToolCallUpdate(wire::ToolCallUpdate::new("tool").content(content)), ); + let updates = translate_for_session(notification, "session"); + let [ + Update::ToolPatched { + images: Some(images), + .. + }, + ] = updates.as_slice() + else { + panic!("typed tool patch"); + }; + let expected = if empty { + 0 + } else { + super::app::MAX_TOOL_IMAGES + }; + assert_eq!(images.len(), expected); + for update in updates { + app.apply(update); + } + let super::app::Block::Tool(call) = &app.blocks[0] else { + panic!("tool"); + }; + assert_eq!(call.images.len(), expected); + assert!(call.output.iter().any(|line| line.contains("[Image]"))); } } @@ -4233,7 +4400,7 @@ mod tests { panic!("expected tool") }; assert_eq!(tool.images.len(), 1); - assert_eq!(tool.images[0].data, "c2VjcmV0"); + assert_eq!(tool.images[0].data.as_ref(), "c2VjcmV0"); let text = tool.output.join("\n"); assert!(!text.contains("c2VjcmV0"), "{text}"); assert!(!text.contains("231"), "{text}"); @@ -4871,7 +5038,7 @@ mod tests { assert_eq!(text, format!("describe [Image #1]({uri})")); assert_eq!(images.len(), 1); - assert_eq!(images[0].data, "AQID"); + assert_eq!(images[0].data.as_ref(), "AQID"); assert_eq!(images[0].mime_type, "image/png"); } diff --git a/src/tui/ui.rs b/src/tui/ui.rs index 64117d5..ab45e67 100644 --- a/src/tui/ui.rs +++ b/src/tui/ui.rs @@ -22,7 +22,8 @@ use super::{ app::{ AgentTreeRow, App, Block, CachedTranscriptBlock, CachedTranscriptImage, CachedTranscriptRow, Child, CodeHit, ComposeView, EffortDialog, FilePickerDialog, - FilePickerStatus, ModelDialog, Phase, SessionRename, ToolCall, UserMessage, + FilePickerStatus, ModelDialog, Phase, SessionRename, ToolCall, TranscriptImageSource, + UserMessage, }, command, image::{ImageRuntime, RESERVED_ROWS}, @@ -47,6 +48,11 @@ type TranscriptTag = (Option, Option, Option); type TaggedTranscriptLine = (LinkedLine, TranscriptTag); pub fn draw(frame: &mut Frame<'_>, app: &mut App, images: &mut ImageRuntime) { + images + .markdown + .context(&app.root, app.session_id.as_deref()); + images.poll(); + app.invalidate_image_layout(&images.markdown.take_changed_sources()); // Two border columns plus the `›` gutter; the prompt grows as the wrapped // text needs more rows, up to the cap. let start_width = frame @@ -933,7 +939,7 @@ fn draw_transcript(frame: &mut Frame<'_>, app: &mut App, images: &mut ImageRunti Vec::new(), String::new(), ); - let mut visible_images: Vec<(usize, usize, i16)> = Vec::new(); + let mut visible_images: Vec<(usize, TranscriptImageSource, i16)> = Vec::new(); let mut materialize = |row: &crate::tui::app::CachedTranscriptRow| { visible.push(row.0.clone()); app.row_calls.push(row.1.0.clone()); @@ -972,7 +978,7 @@ fn draw_transcript(frame: &mut Frame<'_>, app: &mut App, images: &mut ImageRunti let y = image_start as isize - offset as isize; visible_images.push(( block_index, - placement.source, + placement.source.clone(), y.clamp(i16::MIN as isize, i16::MAX as isize) as i16, )); } @@ -998,19 +1004,42 @@ fn draw_transcript(frame: &mut Frame<'_>, app: &mut App, images: &mut ImageRunti let row_widths: Vec = visible.iter().map(ratatui::text::Line::width).collect(); frame.render_widget(Paragraph::new(visible), inner); draw_selection(frame, app, inner, offset, &row_widths); - for (block_index, source_index, y) in visible_images { - let sources = match app.blocks.get(block_index) { - Some(Block::User(message)) => &message.images, - Some(Block::Tool(call)) => &call.images, - _ => continue, - }; - let Some(source) = sources.get(source_index) else { - continue; + for (block_index, source, y) in visible_images { + let (prepared, status) = match source { + TranscriptImageSource::Typed(index) => { + let sources = match app.blocks.get(block_index) { + Some(Block::User(message) | Block::Agent(message)) => &message.images, + Some(Block::Tool(call)) => &call.images, + _ => continue, + }; + let Some(source) = sources.get(index) else { + continue; + }; + let prepared = images.prepare(source, inner.width.max(1)); + (prepared, images.status(&source.key).to_owned()) + } + TranscriptImageSource::Markdown(destination) => { + if !images.enabled() { + continue; + } + // Requests start only for viewports intersecting the screen. Layout + // and replay never initiate IO for offscreen transcript history. + images.markdown.request(&destination); + if let Some(source) = images.markdown.image(&destination).cloned() { + let prepared = images.prepare(&source, inner.width.max(1)); + (prepared, images.status(&source.key).to_owned()) + } else { + (None, images.markdown.status(&destination).to_owned()) + } + } }; - if let Some(image) = images.prepare(source, inner.width.max(1)) { + if let Some(image) = prepared { images.render(frame, image, inner, y); + } else { + draw_image_status(frame, inner, y, &status); } } + if total > height { let mut state = ScrollbarState::new(bottom).position(offset); frame.render_stateful_widget( @@ -1027,6 +1056,26 @@ fn draw_transcript(frame: &mut Frame<'_>, app: &mut App, images: &mut ImageRunti /// Restyles the cells a drag selected. The highlight hugs each row's text /// instead of running to the margin, so it shows exactly what a copy takes. +fn draw_image_status(frame: &mut Frame<'_>, area: Rect, y: i16, status: &str) { + let start = i32::from(y).max(0).min(i32::from(area.height)) as u16; + let end = (i32::from(y) + i32::from(RESERVED_ROWS)) + .max(0) + .min(i32::from(area.height)) as u16; + if start < end { + frame.render_widget( + Paragraph::new(Line::from(Span::styled( + format!("[Image: {status}]"), + theme::dim(), + ))), + Rect { + y: area.y + start, + height: end - start, + ..area + }, + ); + } +} + fn draw_selection( frame: &mut Frame<'_>, app: &App, @@ -1132,7 +1181,7 @@ fn refresh_transcript_cache_with_images(app: &mut App, images: &mut ImageRuntime .as_ref() .map_or(0, |cached| cached.rows.len()); let (rows, cached_images) = - transcript_block_rows(app, block_index, width, images.enabled()); + transcript_block_rows(app, block_index, width, images.enabled(), images); if missing || rows.len() != old_count { first_changed_count = first_changed_count.min(block_index); layout_changed |= !missing; @@ -1160,38 +1209,128 @@ fn refresh_transcript_cache_with_images(app: &mut App, images: &mut ImageRuntime } } -fn user_block_rows( +fn reserve_image_rows( + rows: &mut Vec, + placements: &mut Vec, + source: TranscriptImageSource, + call: Option, +) { + let row = rows.len(); + rows.extend((0..RESERVED_ROWS).map(|_| { + ( + Line::default(), + (call.clone(), None, None), + Vec::new(), + String::new(), + ) + })); + placements.push(CachedTranscriptImage { source, row }); +} + +/// Render the whole document first, then insert image occurrences using source +/// ranges. Fences, tables, and code-copy byte offsets never see sliced Markdown. +fn message_block_rows( message: &UserMessage, + block_index: usize, width: usize, reserve_images: bool, + user: bool, + images: &ImageRuntime, ) -> (Vec, Vec) { + let nodes = markdown::image_nodes(&message.text); + let rendered = if user { + let mut offset = 0; + message + .text + .split('\n') + .enumerate() + .map(|(index, text)| { + let start = offset; + offset += text.len() + 1; + ( + user_line(text, index == 0), + None, + start..offset.min(message.text.len()), + ) + }) + .collect::>() + } else { + markdown::render_copyable_with_source_at_width(&message.text, Some(width)) + }; + let mut occurrences = vec![Vec::new(); rendered.len()]; + for (index, node) in nodes.iter().enumerate() { + if let Some(row) = rendered + .iter() + .position(|(_, _, range)| range.end >= node.range.end) + { + occurrences[row].push(( + TranscriptImageSource::Markdown(node.destination.clone()), + Some(index), + )); + } + } + let line_ends: Vec<_> = message + .text + .split('\n') + .scan(0, |offset, line| { + let end = *offset + line.len(); + *offset = end + 1; + Some(end) + }) + .collect(); + for (index, image) in message.images.iter().enumerate() { + let duplicate = reserve_images + && nodes.iter().any(|node| { + images + .markdown + .image(&node.destination) + .is_some_and(|loaded| loaded.key == image.key) + }); + if duplicate { + continue; + } + let end = line_ends + .get(image.line) + .copied() + .unwrap_or(message.text.len()); + if let Some(row) = rendered.iter().position(|(_, _, range)| range.end >= end) { + occurrences[row].push((TranscriptImageSource::Typed(index), None)); + } + } let mut rows = Vec::new(); let mut placements = Vec::new(); - for (line_index, text) in message.text.split('\n').enumerate() { + for (line_index, ((line, code, _), occurrences)) in + rendered.into_iter().zip(occurrences).enumerate() + { + let code = code.map(|range| CodeHit { + block: block_index, + range, + }); rows.extend(wrap_linked_tagged( - &[( - user_line(text, line_index == 0), - (None, None, Some(line_index)), - )], + &[(line, (None, code, 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 { source, row }); + for (source, node) in occurrences { + if let Some(node) = node { + let node = &nodes[node]; + let safe = super::safe_media_uri(&node.destination); + let relative = !node.destination.contains(':') + && node.destination.len() <= 2048 + && !node.destination.chars().any(char::is_control); + let destination = if safe || relative { + node.destination.as_str() + } else { + "unsupported source" + }; + let label = format!("[Image: {} — {destination}]", node.alt); + let line = LinkedLine::new(vec![LinkedSpan { + span: Span::styled(label, theme::dim()), + url: safe.then(|| node.destination.clone()), + }]); + rows.extend(wrap_linked_tagged(&[(line, (None, None, None))], width)); + } + if reserve_images { + reserve_image_rows(&mut rows, &mut placements, source, None); } } } @@ -1203,11 +1342,16 @@ fn transcript_block_rows( block_index: usize, width: usize, reserve_images: bool, + images: &ImageRuntime, ) -> (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::Agent(text) => (markdown::render_copyable_at_width(text, Some(width)), None), + Block::User(message) => { + return message_block_rows(message, block_index, width, reserve_images, true, images); + } + Block::Agent(message) => { + return message_block_rows(message, block_index, width, reserve_images, false, images); + } Block::Thought { text, started, @@ -1282,16 +1426,12 @@ fn transcript_block_rows( if !reserve_images { continue; } - let row = rows.len(); - rows.extend((0..RESERVED_ROWS).map(|_| { - ( - Line::default(), - (Some(call.id.clone()), None, None), - Vec::new(), - String::new(), - ) - })); - placements.push(CachedTranscriptImage { source, row }); + reserve_image_rows( + &mut rows, + &mut placements, + TranscriptImageSource::Typed(source), + Some(call.id.clone()), + ); } } (rows, placements) @@ -2236,7 +2376,7 @@ mod tests { use super::{ ImageRuntime, MAX_PROMPT_ROWS, ModelDialogRow, agent_lines, body_layout, draw, draw_agents, model_dialog_rows, model_dialog_viewport, prompt_lines, - refresh_transcript_cache_with_images, truncate_to_width, user_block_rows, user_line, + refresh_transcript_cache_with_images, truncate_to_width, user_line, }; use crate::{ events::{GenerationOutcome, RuntimeEvent, SubagentStatus}, @@ -3491,6 +3631,7 @@ mod tests { }); app.apply(Update::AgentMessage { + images: Vec::new(), id: "after-compose".into(), text: "Moving on.".into(), append: true, @@ -3933,7 +4074,7 @@ mod tests { ); let code = "one\n\n abcdefghijklmnopqrstuvwxyz0123456789\ntwo"; app.blocks - .push(Block::Agent(format!("```text\n{code}\n```"))); + .push(Block::Agent(format!("```text\n{code}\n```").into())); let frame = render(&mut app, 24, 24); let first = frame .lines() @@ -4067,8 +4208,10 @@ mod tests { ); let first = "https://first.example/a/very/long/path"; let second = "https://second.example/a/very/long/path"; - app.blocks.push(Block::Agent(format!("[same]({first})"))); - app.blocks.push(Block::Agent(format!("[same]({second})"))); + app.blocks + .push(Block::Agent(format!("[same]({first})").into())); + app.blocks + .push(Block::Agent(format!("[same]({second})").into())); let _ = render(&mut app, 28, 24); let all_urls: Vec<_> = app @@ -4256,7 +4399,8 @@ mod tests { "0:0".into(), ); for index in 0..99 { - app.blocks.push(Block::Agent(format!("history {index}"))); + app.blocks + .push(Block::Agent(format!("history {index}").into())); } app.apply(Update::test_text("history 99".into())); refresh_transcript_cache(&mut app, 12); @@ -4312,8 +4456,27 @@ mod tests { images: vec![image], }; - let (rows, placements) = user_block_rows(&message, 40, true); + let (rows, placements) = + super::message_block_rows(&message, 0, 40, true, true, &ImageRuntime::disabled()); + + 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")) + ); + } + #[test] + fn assistant_images_preserve_placement_copy_and_search_text() { + let message = UserMessage { + text: "before\n[Image #1]\nafter\n```text\ncopy me\n```".into(), + images: vec![UserImage::new("c2VjcmV0".into(), "image/png".into(), 1).unwrap()], + }; + let (rows, placements) = + super::message_block_rows(&message, 7, 40, true, false, &ImageRuntime::disabled()); assert_eq!(placements.len(), 1); let after = &rows[placements[0].row + usize::from(super::RESERVED_ROWS)].0; assert!( @@ -4322,6 +4485,303 @@ mod tests { .iter() .any(|span| span.content.contains("after")) ); + let code = rows.iter().find_map(|row| row.1.1.as_ref()).unwrap(); + assert_eq!(&message.text[code.range.clone()], "copy me"); + assert_eq!(code.block, 7); + assert!(rows.iter().all(|row| !row.3.contains("c2VjcmV0"))); + let (fallback, placements) = + super::message_block_rows(&message, 7, 40, false, false, &ImageRuntime::disabled()); + assert!(placements.is_empty()); + assert!(fallback.iter().any(|row| { + row.0 + .spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + .contains("Image #1") + })); + } + + #[test] + fn markdown_occurrences_keep_repeats_and_do_not_treat_links_or_code_as_images() { + let text = "![first](file:///tmp/a.png) ![second](file:///tmp/a.png)\n[ordinary](file:///tmp/b.png)\n```text\n![code](file:///tmp/c.png)\n```"; + for user in [true, false] { + let message = UserMessage::from(text); + let images = ImageRuntime::disabled(); + let (rows, placements) = + super::message_block_rows(&message, 0, 100, true, user, &images); + assert_eq!(placements.len(), 2); + assert!( + placements + .iter() + .all(|placement| matches!(&placement.source, + super::TranscriptImageSource::Markdown(uri) if uri == "file:///tmp/a.png")) + ); + if !user { + let code = rows.iter().find_map(|row| row.1.1.as_ref()).unwrap(); + assert_eq!(&text[code.range.clone()], "![code](file:///tmp/c.png)"); + } + let (fallback, placements) = + super::message_block_rows(&message, 0, 100, false, user, &images); + assert!(placements.is_empty()); + let text = fallback + .iter() + .map(|row| line_text(&row.0)) + .collect::(); + assert!(text.contains("first") && text.contains("file:///tmp/a.png")); + assert!(!images.markdown.pending()); + } + } + + #[test] + fn source_completion_invalidates_only_dependent_markdown_layout() { + let mut app = sample(); + app.start_session("owner".into()); + for (id, source) in [("first", "first.png"), ("second", "second.png")] { + app.apply(Update::AgentMessage { + id: id.into(), + text: format!("![image]({source})"), + images: Vec::new(), + append: false, + }); + } + let mut images = ImageRuntime::with_picker(Picker::halfblocks()); + refresh_transcript_cache_with_images(&mut app, &mut images, 80); + let revisions = app.transcript_revisions.clone(); + app.invalidate_image_layout(&std::collections::HashSet::from(["first.png".into()])); + assert_ne!(app.transcript_revisions[0], revisions[0]); + assert_eq!(app.transcript_revisions[1], revisions[1]); + assert_eq!( + app.transcript_cache[1].as_ref().unwrap().revision, + revisions[1] + ); + let after = app.transcript_revisions.clone(); + app.invalidate_image_layout(&std::collections::HashSet::new()); + assert_eq!(app.transcript_revisions, after); + } + + #[tokio::test] + async fn markdown_user_and_assistant_load_local_and_managed_live_replay_and_replace() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("image.png"); + let mut bytes = std::io::Cursor::new(Vec::new()); + image::DynamicImage::new_rgb8(4, 2) + .write_to(&mut bytes, image::ImageFormat::Png) + .unwrap(); + std::fs::write(&path, bytes.get_ref()).unwrap(); + let local = url::Url::from_file_path(&path).unwrap().to_string(); + let store = crate::managed_files::FileStore::new(directory.path()); + let reference = store.import("owner", &path, None).unwrap(); + let descriptor = serde_json::to_value(reference).unwrap(); + let managed = format!("kit-file://{}", descriptor["id"].as_str().unwrap()); + for user in [true, false] { + for uri in [&local, &managed] { + let mut app = App::new( + directory.path().into(), + "provider".into(), + "model".into(), + "a2a".into(), + ); + app.start_session("owner".into()); + let patch = |text: String, append| { + if user { + Update::UserMessage { + id: "message".into(), + text, + images: Vec::new(), + append, + } + } else { + Update::AgentMessage { + id: "message".into(), + text, + images: Vec::new(), + append, + } + } + }; + app.apply(patch(format!("![visible alt]({uri})"), true)); + let mut images = ImageRuntime::with_picker(Picker::halfblocks()); + let mut terminal = Terminal::new(TestBackend::new(100, 45)).unwrap(); + terminal + .draw(|frame| draw(frame, &mut app, &mut images)) + .unwrap(); + assert!(images.markdown.pending()); + let visible = terminal + .backend() + .buffer() + .content + .iter() + .map(|cell| cell.symbol()) + .collect::(); + assert!(visible.contains("visible alt") && visible.contains("image loading")); + tokio::time::timeout(std::time::Duration::from_secs(5), async { + loop { + terminal + .draw(|frame| draw(frame, &mut app, &mut images)) + .unwrap(); + if images.markdown.image(uri).is_some() { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(5)).await; + } + }) + .await + .unwrap(); + assert_eq!(app.transcript_cache[0].as_ref().unwrap().images.len(), 1); + app.apply(patch("replacement without images".into(), false)); + terminal + .draw(|frame| draw(frame, &mut app, &mut images)) + .unwrap(); + assert!(app.transcript_cache[0].as_ref().unwrap().images.is_empty()); + app.start_session("owner".into()); + app.apply(patch(format!("![replayed alt]({uri})"), false)); + terminal + .draw(|frame| draw(frame, &mut app, &mut images)) + .unwrap(); + assert_eq!(app.transcript_cache[0].as_ref().unwrap().images.len(), 1); + } + } + } + + #[tokio::test] + async fn markdown_dedup_requires_loaded_matching_identity_and_preserves_repeats() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("same.png"); + let mut bytes = std::io::Cursor::new(Vec::new()); + image::DynamicImage::new_rgb8(4, 2) + .write_to(&mut bytes, image::ImageFormat::Png) + .unwrap(); + std::fs::write(&path, bytes.get_ref()).unwrap(); + let uri = url::Url::from_file_path(&path).unwrap().to_string(); + let typed = UserImage::new( + base64::engine::general_purpose::STANDARD.encode(bytes.get_ref()), + "image/png".into(), + 0, + ) + .unwrap() + .with_uri(Some(uri.clone())); + let message = UserMessage { + text: format!("![one]({uri}) ![two]({uri})"), + images: vec![typed], + }; + let mut images = ImageRuntime::with_picker(Picker::halfblocks()); + images.markdown.context(directory.path(), Some("owner")); + assert_eq!( + super::message_block_rows(&message, 0, 100, true, false, &images) + .1 + .len(), + 3 + ); + images.markdown.request(&uri); + tokio::time::timeout(std::time::Duration::from_secs(5), async { + while images.markdown.image(&uri).is_none() { + images.markdown.poll(); + tokio::time::sleep(std::time::Duration::from_millis(5)).await; + } + }) + .await + .unwrap(); + for user in [true, false] { + let placements = super::message_block_rows(&message, 0, 100, true, user, &images).1; + assert_eq!(placements.len(), 2); + assert!(placements.iter().all(|placement| matches!( + placement.source, + super::TranscriptImageSource::Markdown(_) + ))); + } + std::fs::write(directory.path().join("other.png"), bytes.get_ref()).unwrap(); + // Independently authorized aliases and distinct files with exact bytes + // suppress the redundant typed viewport, not explicit Markdown nodes. + for destination in ["same.png", "other.png"] { + images.markdown.request(destination); + tokio::time::timeout(std::time::Duration::from_secs(5), async { + while images.markdown.pending() { + images.poll(); + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + let mut alias = message.clone(); + alias.text = format!("![one]({destination}) ![two]({destination})"); + assert_eq!( + super::message_block_rows(&alias, 0, 100, true, false, &images) + .1 + .len(), + 2 + ); + } + let mut mismatch = message.clone(); + mismatch.images[0] = UserImage::new("AQID".into(), "image/png".into(), 0) + .unwrap() + .with_uri(Some(uri)); + assert_eq!( + super::message_block_rows(&mismatch, 0, 100, true, false, &images) + .1 + .len(), + 3 + ); + } + + #[tokio::test] + async fn markdown_missing_sources_keep_typed_pixels_and_offscreen_sources_do_not_load() { + let directory = tempfile::tempdir().unwrap(); + let uri = url::Url::from_file_path(directory.path().join("missing.png")) + .unwrap() + .to_string(); + let mut app = App::new( + directory.path().into(), + "provider".into(), + "model".into(), + "a2a".into(), + ); + app.start_session("owner".into()); + app.apply(Update::AgentMessage { + id: "old".into(), + text: format!("![missing alt]({uri})"), + images: vec![ + UserImage::new("AQID".into(), "image/png".into(), 0) + .unwrap() + .with_uri(Some(uri.clone())), + ], + append: false, + }); + app.apply(Update::AgentMessage { + id: "tail".into(), + text: "tail\n".repeat(100), + images: Vec::new(), + append: false, + }); + let mut images = ImageRuntime::with_picker(Picker::halfblocks()); + let mut terminal = Terminal::new(TestBackend::new(100, 40)).unwrap(); + terminal + .draw(|frame| draw(frame, &mut app, &mut images)) + .unwrap(); + assert!(!images.markdown.pending()); + app.scroll_by(-1000); + terminal + .draw(|frame| draw(frame, &mut app, &mut images)) + .unwrap(); + assert!(images.markdown.pending()); + tokio::time::timeout(std::time::Duration::from_secs(5), async { + while images.markdown.pending() { + tokio::time::sleep(std::time::Duration::from_millis(5)).await; + terminal + .draw(|frame| draw(frame, &mut app, &mut images)) + .unwrap(); + } + }) + .await + .unwrap(); + assert!(images.markdown.image(&uri).is_none()); + assert_ne!(images.markdown.status(&uri), "image loading"); + assert_eq!(app.transcript_cache[0].as_ref().unwrap().images.len(), 2); + let mut disabled = ImageRuntime::disabled(); + terminal + .draw(|frame| draw(frame, &mut app, &mut disabled)) + .unwrap(); + assert!(!disabled.markdown.pending()); } #[test] @@ -4378,7 +4838,7 @@ mod tests { if let Block::Tool(call) = &mut app.blocks[0] { call.expanded = false; } - let (_, placements) = super::transcript_block_rows(&app, 0, 40, true); + let (_, placements) = super::transcript_block_rows(&app, 0, 40, true, &images); assert!(placements.is_empty()); } From ee4d01c879c85d949cef79119498dbf148f3cb71 Mon Sep 17 00:00:00 2001 From: daniel Date: Tue, 8 Sep 2026 17:10:31 +0100 Subject: [PATCH 2/4] fix(tui): decode image paths and reclaim snapshot capacity --- src/tui/image_source.rs | 153 ++++++++++++++++++++++++++++++- src/tui/markdown_images.rs | 182 ++++++++++++++++++++++++++++++++----- 2 files changed, 309 insertions(+), 26 deletions(-) diff --git a/src/tui/image_source.rs b/src/tui/image_source.rs index 3d58f80..854dd2c 100644 --- a/src/tui/image_source.rs +++ b/src/tui/image_source.rs @@ -111,7 +111,7 @@ pub(super) async fn resolve( return Err("invalid image URL".into()); } let bytes = if Path::new(source).is_absolute() { - local(PathBuf::from(source), permit.clone()).await? + local(native_path(source)?, permit.clone()).await? } else if let Ok(url) = Url::parse(source) { match url.scheme() { "https" => { @@ -142,7 +142,7 @@ pub(super) async fn resolve( _ => return Err("unsupported image source".into()), } } else { - local(root.join(source), permit.clone()).await? + local(root.join(native_path(source)?), permit.clone()).await? }; // Sniff only; all origins share the runtime's single bounded decoder. let format = image::guess_format(&bytes).map_err(|_| "invalid image format".to_owned())?; @@ -158,7 +158,45 @@ pub(super) async fn resolve( Ok((bytes, format.to_mime_type().to_owned())) } +// Markdown destinations retain URL escapes. Decode only native destinations; +// Url::to_file_path already decodes file URLs. The caller caps input at 4096 bytes. +fn native_path(source: &str) -> Result { + let mut decoded = Vec::with_capacity(source.len()); + let mut bytes = source.bytes(); + while let Some(byte) = bytes.next() { + decoded.push(if byte == b'%' { + let high = bytes.next().and_then(|b| char::from(b).to_digit(16)); + let low = bytes.next().and_then(|b| char::from(b).to_digit(16)); + match (high, low) { + (Some(high), Some(low)) => (high * 16 + low) as u8, + _ => return Err("invalid local image path".into()), + } + } else { + byte + }); + } + // Reject malformed UTF-8 rather than opening a lossy replacement filename. + let decoded = String::from_utf8(decoded).map_err(|_| "invalid local image path".to_owned())?; + let path = PathBuf::from(decoded); + validate_local_path(&path)?; + Ok(path) +} + +fn validate_local_path(path: &Path) -> Result<()> { + let bytes = path.as_os_str().as_encoded_bytes(); + // Check after decoding as well as before it, including mixed separators. + // Do not reinterpret native paths as URLs or change their OS path semantics. + if bytes.contains(&0) { + return Err("invalid local image path".into()); + } + if bytes.len() >= 2 && matches!(bytes[0], b'/' | b'\\') && matches!(bytes[1], b'/' | b'\\') { + return Err("invalid image source".into()); + } + Ok(()) +} + async fn local(path: PathBuf, permit: SourcePermit) -> Result> { + validate_local_path(&path)?; tokio::task::spawn_blocking(move || { let _permit = permit; let mut options = std::fs::OpenOptions::new(); @@ -557,6 +595,117 @@ mod tests { ); } + #[tokio::test] + async fn native_markdown_destinations_decode_exactly_once() { + let dir = tempfile::tempdir().unwrap(); + std::fs::create_dir(dir.path().join("screenshots")).unwrap(); + let policy = ImagePolicy::default(); + let fixtures = [ + ("run%201.png", "run 1.png"), + ("run%25201.png", "run%201.png"), + ("literal%25.png", "literal%.png"), + ("caf%C3%A9.png", "café.png"), + ]; + // Distinct valid images prove we open the decoded file, not a literal + // %20 filename or a twice-decoded space filename that also exists. + for (index, (_, filename)) in fixtures.iter().enumerate() { + let mut bytes = png(); + bytes.push(index as u8); + std::fs::write(dir.path().join("screenshots").join(filename), bytes).unwrap(); + } + for (index, (destination, filename)) in fixtures.iter().enumerate() { + let mut expected = png(); + expected.push(index as u8); + let relative = format!("screenshots/{destination}"); + let absolute = format!( + "{}/{relative}", + dir.path() + .to_str() + .unwrap() + .replace('%', "%25") + .replace(' ', "%20") + ); + let file_url = Url::from_file_path(dir.path().join("screenshots").join(filename)) + .unwrap() + .to_string(); + for source in [relative, absolute, file_url] { + assert_eq!( + resolve(&source, dir.path(), "s", &policy).await.unwrap(), + (expected.clone(), "image/png".into()), + "{source}" + ); + } + } + } + + #[tokio::test] + async fn malformed_native_escapes_fail_without_lossy_or_literal_fallback() { + let dir = tempfile::tempdir().unwrap(); + let policy = ImagePolicy::default(); + for filename in [ + "bad%.png", + "bad%2.png", + "bad%GG.png", + "bad%FF.png", + "bad%C3%28.png", + "bad%00.png", + ] { + std::fs::write(dir.path().join(filename), png()).unwrap(); + for source in [ + filename.to_owned(), + dir.path().join(filename).to_str().unwrap().to_owned(), + ] { + assert_eq!( + resolve(&source, dir.path(), "s", &policy) + .await + .unwrap_err(), + "invalid local image path", + "{source}" + ); + } + } + std::fs::write(dir.path().join("bad�.png"), png()).unwrap(); + assert_eq!( + resolve("bad%EF%BF.png", dir.path(), "s", &policy) + .await + .unwrap_err(), + "invalid local image path" + ); + } + + #[tokio::test] + async fn decoded_local_destinations_cannot_introduce_unc_paths() { + let dir = tempfile::tempdir().unwrap(); + let policy = ImagePolicy::default(); + for source in [ + "%2f%2fserver/share/image.png", + "/%2Fserver/share/image.png", + "%5c%5cserver%5cshare%5cimage.png", + "%2f%5cserver/share/image.png", + "%5c%2fserver/share/image.png", + "file:///%2Fserver/share/image.png", + "file://localhost/%2Fserver/share/image.png", + ] { + let error = resolve(source, dir.path(), "s", &policy).await.unwrap_err(); + assert!( + matches!( + error.as_str(), + "invalid image source" | "invalid local image path" + ), + "{source}: {error}" + ); + } + for source in [ + "file://%73erver/share/image.png", + "file://%31%32%37.0.0.1/image.png", + ] { + assert_eq!( + resolve(source, dir.path(), "s", &policy).await.unwrap_err(), + "invalid local image URL" + ); + } + } + #[tokio::test] async fn managed_source_preserves_session_isolation_without_reimport() { let dir = tempfile::tempdir().unwrap(); diff --git a/src/tui/markdown_images.rs b/src/tui/markdown_images.rs index ae09fa9..7df5aff 100644 --- a/src/tui/markdown_images.rs +++ b/src/tui/markdown_images.rs @@ -106,45 +106,72 @@ impl MarkdownImages { if done.generation != self.generation { continue; } - let Some(entry) = self.entries.get_mut(&done.source) else { + if !self.entries.contains_key(&done.source) { continue; - }; - entry.state = match done.image { - Ok(image) if self.retained.saturating_add(image.data.len()) <= MAX_SOURCE_BYTES => { - self.retained += image.data.len(); - self.changed_sources.insert(done.source); - State::Ready(image) + } + let state = match done.image { + Ok(image) => { + let bytes = image.data.len(); + // Reclaim snapshots, never active acquisitions. An image that + // cannot fit stays retryable instead of caching capacity as + // an intrinsic source failure. + if bytes <= MAX_SOURCE_BYTES { + while self.retained.saturating_add(bytes) > MAX_SOURCE_BYTES { + if !self.evict_oldest(true) { + break; + } + } + } + if self.retained.saturating_add(bytes) <= MAX_SOURCE_BYTES { + self.retained += bytes; + self.changed_sources.insert(done.source.clone()); + State::Ready(image) + } else { + State::Waiting + } } - Ok(_) => State::Failed("image source budget exceeded".into()), Err(error) => State::Failed(error), }; + if let Some(entry) = self.entries.get_mut(&done.source) { + entry.state = state; + } changed = true; } changed } + /// Byte pressure reclaims only ready snapshots; entry pressure can also + /// reclaim idle placeholders and failures. Neither may remove active jobs. + fn evict_oldest(&mut self, ready_only: bool) -> bool { + let oldest = self + .entries + .iter() + .filter(|(_, entry)| { + matches!(entry.state, State::Ready(_)) + || (!ready_only && !matches!(entry.state, State::Loading(_))) + }) + .min_by_key(|(_, entry)| entry.used) + .map(|(source, _)| source.clone()); + let Some(oldest) = oldest else { return false }; + if let Some(Entry { + state: State::Ready(image), + .. + }) = self.entries.remove(&oldest) + { + self.retained = self.retained.saturating_sub(image.data.len()); + self.changed_sources.insert(oldest); + } + true + } + pub fn request(&mut self, source: &str) { if source.len() > 4096 { return; } self.clock = self.clock.wrapping_add(1); if !self.entries.contains_key(source) { - if self.entries.len() == MAX_ENTRIES { - let oldest = self - .entries - .iter() - .filter(|(_, entry)| !matches!(entry.state, State::Loading(_))) - .min_by_key(|(_, entry)| entry.used) - .map(|(source, _)| source.clone()); - let Some(oldest) = oldest else { return }; - if let Some(Entry { - state: State::Ready(image), - .. - }) = self.entries.remove(&oldest) - { - self.retained = self.retained.saturating_sub(image.data.len()); - self.changed_sources.insert(oldest); - } + if self.entries.len() == MAX_ENTRIES && !self.evict_oldest(false) { + return; } self.entries.insert( source.to_owned(), @@ -268,6 +295,113 @@ mod tests { assert!(images.image("image.png").is_some()); } + #[tokio::test] + async fn source_byte_pressure_evicts_lru_and_revisited_images_recover() { + use image::{ + ImageEncoder as _, + codecs::png::{CompressionType, FilterType, PngEncoder}, + }; + + let root = tempfile::tempdir().unwrap(); + let sources = ["first.png", "second.png", "third.png", "fourth.png"]; + for (index, source) in sources.iter().enumerate() { + // Real, individually valid ~7 MiB PNGs, with distinct pixel content. + let pixels = vec![index as u8; 1792 * 1366 * 3]; + let mut bytes = Vec::new(); + PngEncoder::new_with_quality( + &mut bytes, + CompressionType::Level(0), + FilterType::NoFilter, + ) + .write_image(&pixels, 1792, 1366, image::ExtendedColorType::Rgb8) + .unwrap(); + assert!((7 * 1024 * 1024..8 * 1024 * 1024).contains(&bytes.len())); + std::fs::write(root.path().join(source), bytes).unwrap(); + } + let mut images = MarkdownImages::new(); + images.context(root.path(), Some("session")); + let mut encoded_bytes = 0; + for source in &sources[..3] { + images.request(source); + settle(&mut images).await; + encoded_bytes += images.image(source).unwrap().data.len(); + assert_eq!(images.retained, encoded_bytes); + assert!(images.retained <= MAX_SOURCE_BYTES); + assert_eq!( + images.take_changed_sources(), + HashSet::from([source.to_string()]) + ); + } + + // Visiting the first again makes the second the least recently used. + images.request(sources[0]); + images.request(sources[3]); + settle(&mut images).await; + assert!(images.image(sources[3]).is_some()); + assert!(images.image(sources[0]).is_some()); + assert!(images.image(sources[1]).is_none()); + assert!(images.image(sources[2]).is_some()); + assert_eq!( + images.take_changed_sources(), + HashSet::from([sources[1].to_owned(), sources[3].to_owned()]) + ); + assert_eq!( + images.retained, + sources + .iter() + .filter_map(|source| images.image(source)) + .map(|image| image.data.len()) + .sum::() + ); + assert!(images.retained <= MAX_SOURCE_BYTES); + + // Scroll back without clearing the cache: reacquire the evicted source + // and invalidate both the newly ready and newly evicted source layouts. + images.request(sources[1]); + settle(&mut images).await; + assert!(images.image(sources[1]).is_some()); + assert!(images.image(sources[2]).is_none()); + assert_eq!( + images.take_changed_sources(), + HashSet::from([sources[1].to_owned(), sources[2].to_owned()]) + ); + assert_eq!( + images.retained, + sources + .iter() + .filter_map(|source| images.image(source)) + .map(|image| image.data.len()) + .sum::() + ); + assert!(images.retained <= MAX_SOURCE_BYTES); + } + + #[tokio::test] + async fn entry_pressure_preserves_active_acquisitions() { + let root = tempfile::tempdir().unwrap(); + write_png(root.path(), "first.png"); + write_png(root.path(), "second.png"); + let mut images = MarkdownImages::new(); + images.context(root.path(), Some("session")); + images.request("first.png"); + images.request("second.png"); + // Without polling, these entries remain Loading even if a worker has + // finished. Fill the remaining entry slots, then request one more. + for index in 0..MAX_ENTRIES - 1 { + images.request(&format!("waiting-{index}.png")); + } + assert_eq!(images.entries.len(), MAX_ENTRIES); + assert!(!images.entries.contains_key("waiting-0.png")); + assert!(images.take_changed_sources().is_empty()); + settle(&mut images).await; + assert!(images.image("first.png").is_some()); + assert!(images.image("second.png").is_some()); + assert_eq!( + images.take_changed_sources(), + HashSet::from(["first.png".to_owned(), "second.png".to_owned(),]) + ); + } + #[tokio::test] async fn session_reset_discards_inflight_results() { let root = tempfile::tempdir().unwrap(); From 9541e99d2506c3903df8b08eee1929f3bc8bbec5 Mon Sep 17 00:00:00 2001 From: daniel Date: Tue, 8 Sep 2026 17:59:24 +0100 Subject: [PATCH 3/4] fix(tui): stabilize visible image cache admission --- docs/user/tui-and-sessions.md | 2 +- src/tui/app.rs | 8 +- src/tui/image.rs | 224 +++++++++++++++-- src/tui/markdown_images.rs | 205 +++++++++++++-- src/tui/mod.rs | 4 +- src/tui/ui.rs | 457 +++++++++++++++++++++++++++++++++- 6 files changed, 835 insertions(+), 65 deletions(-) diff --git a/docs/user/tui-and-sessions.md b/docs/user/tui-and-sessions.md index de0f281..a069376 100644 --- a/docs/user/tui-and-sessions.md +++ b/docs/user/tui-and-sessions.md @@ -25,7 +25,7 @@ A session ID must be 1–128 ASCII letters, digits, `-`, or `_`. `kit prompt` us ## Images in the transcript -User attachments, native assistant-generated images, typed tool results, and Markdown image nodes use the same terminal image renderer. Image loading and decoding are asynchronous. Loading or unavailable images retain text placeholders; unsupported terminal graphics retain readable text and source links. Image viewports keep a fixed height and refit to the terminal width after resize. Display is presentation-only: it never attaches pixels to a prompt, grants File access, or changes provider requests. +User attachments, native assistant-generated images, typed tool results, and Markdown image nodes use the same terminal image renderer. Image loading and decoding are asynchronous. Loading or unavailable images retain text placeholders; unsupported terminal graphics retain readable text and source links. Image viewports keep a fixed height and refit to the terminal width after resize. When the visible image set exceeds cache capacity, Kit keeps a stable admitted subset and shows a capacity-deferred placeholder for the rest; scrolling to a smaller set permits recovery without repeated background downloads or decoding. Display is presentation-only: it never attaches pixels to a prompt, grants File access, or changes provider requests. Markdown images use CommonMark image syntax, including reference-style images. Ordinary links, escaped image syntax, and images inside code do not load. An incomplete streamed image node remains text until it parses as an image. Explicit repeated image nodes remain separate occurrences. A typed attachment and a Markdown image are deduplicated only after the resolved bytes identify the same image; an inaccessible Markdown source does not hide the typed attachment. diff --git a/src/tui/app.rs b/src/tui/app.rs index a228adc..32b0ebb 100644 --- a/src/tui/app.rs +++ b/src/tui/app.rs @@ -664,6 +664,7 @@ pub struct App { runtime_session_id: Option, pub blocks: Vec, pub(super) transcript_cache: Vec>, + /// Block content/presentation revisions; source-cache invalidations only mark dirty. pub(super) transcript_revisions: Vec, pub(super) transcript_dirty: BTreeSet, pub(super) transcript_dynamic: BTreeSet, @@ -1113,9 +1114,10 @@ impl App { }) .map(|(index, _)| index) .collect(); - for index in affected { - self.mark_block_dirty(index); - } + // Source readiness changes layout, not message content. Preserve the + // revision so refresh can distinguish this from an overlapping message + // replacement without storing or hashing another copy of the content. + self.transcript_dirty.extend(affected); } /// Aligns cache bookkeeping for tests and other direct transcript setup. diff --git a/src/tui/image.rs b/src/tui/image.rs index 9cb46ac..710e433 100644 --- a/src/tui/image.rs +++ b/src/tui/image.rs @@ -10,7 +10,12 @@ use ratatui_image::{ picker::{Picker, ProtocolType, cap_parser::QueryStdioOptions}, sliced::{SignedPosition, SlicedImage, SlicedProtocol}, }; -use std::{collections::HashMap, io::Cursor, sync::Arc, time::Duration}; +use std::{ + collections::{HashMap, HashSet}, + io::Cursor, + sync::Arc, + time::Duration, +}; use tokio::sync::{Semaphore, oneshot}; const TERMINAL_QUERY_TIMEOUT: Duration = Duration::from_millis(150); @@ -50,6 +55,7 @@ struct CacheEntry { width: u16, running: bool, error: Option<&'static str>, + deferred_at: Option, last_used: u64, } struct Job { @@ -62,6 +68,8 @@ pub(super) struct ImageRuntime { pub(super) markdown: super::markdown_images::MarkdownImages, picker: Option, cache: HashMap<[u8; 32], CacheEntry>, + visible: HashSet<[u8; 32]>, + visibility_generation: u64, jobs: Vec, generation: u64, clock: u64, @@ -72,6 +80,8 @@ impl ImageRuntime { markdown: super::markdown_images::MarkdownImages::new(), picker, cache: HashMap::new(), + visible: HashSet::new(), + visibility_generation: 0, jobs: Vec::new(), generation: 0, clock: 0, @@ -90,7 +100,19 @@ impl ImageRuntime { pub fn enabled(&self) -> bool { self.picker.is_some() } + pub fn set_visible_keys(&mut self, keys: impl IntoIterator) { + // The caller supplies actual visible occurrences, including pending and + // deferred sources. Decode-cache residency must not define visibility. + let visible = keys.into_iter().collect(); + if self.visible != visible { + self.visible = visible; + self.visibility_generation = self.visibility_generation.wrapping_add(1); + } + } + pub fn clear(&mut self) { + self.visible.clear(); + self.visibility_generation = self.visibility_generation.wrapping_add(1); self.markdown.clear(); self.cache.clear(); self.generation = self.generation.wrapping_add(1); @@ -102,6 +124,10 @@ impl ImageRuntime { } match self.cache.get(key) { Some(entry) if entry.error.is_some() => entry.error.unwrap_or("image unavailable"), + Some(entry) if entry.deferred_at.is_some() => "image deferred (visible image budget)", + None if self.cache.len() >= MAX_CACHE_ENTRIES => { + "image deferred (visible image budget)" + } Some(entry) if entry.protocol.is_some() && !entry.running => "image ready", _ => "image loading", } @@ -112,6 +138,7 @@ impl ImageRuntime { || !self.jobs.is_empty() || self.cache.values().any(|entry| { entry.error.is_none() + && entry.deferred_at.is_none() && (entry.source.is_some() || entry .protocol @@ -121,7 +148,7 @@ impl ImageRuntime { } pub fn poll(&mut self) -> bool { - let mut changed = self.markdown.poll(); + let mut changed = false; let mut index = 0; while index < self.jobs.len() { let result = match self.jobs[index].receiver.try_recv() { @@ -136,51 +163,87 @@ impl ImageRuntime { if job.generation != self.generation { continue; } + if !self.cache.contains_key(&job.key) { + continue; + } + let admitted = match &result { + Ok((decoded, _)) => self.reserve_decoded(&job.key, decoded.as_bytes().len() as u64), + Err(_) => true, + }; let Some(entry) = self.cache.get_mut(&job.key) else { continue; }; entry.running = false; changed = true; match result { - Ok((decoded, protocol)) => { + Ok((decoded, protocol)) if admitted => { entry.decoded = Some(decoded); if entry.width == job.width { entry.protocol = Some((job.width, protocol)); } } + Ok(_) => { + // Drop the unadmittable result, not another visible image. + // Only a meaningful visible-set change permits a new decode. + entry.decoded = None; + entry.protocol = None; + entry.deferred_at = Some(self.visibility_generation); + } Err(error) => entry.error = Some(error), } } - // Cache backing is separate from the two 64 MiB job reservations. - // Completed outputs move from those reservations into the cache. - // Trim even when there is no queued work left to schedule. - while self - .cache - .values() - .filter_map(|entry| entry.decoded.as_ref()) - .map(|decoded| decoded.as_bytes().len() as u64) - .sum::() - > MAX_DECODED_BACKING_BYTES - { + self.schedule(); + changed + } + fn reserve_decoded(&mut self, key: &[u8; 32], bytes: u64) -> bool { + loop { + let retained = self + .cache + .iter() + .filter(|(cached, _)| *cached != key) + .filter_map(|(_, entry)| entry.decoded.as_ref()) + .map(|decoded| decoded.as_bytes().len() as u64) + .sum::(); + if retained.saturating_add(bytes) <= MAX_DECODED_BACKING_BYTES { + return true; + } let victim = self .cache .iter() - .filter(|(_, entry)| !entry.running && entry.decoded.is_some()) + .filter(|(cached, entry)| { + *cached != key + && !self.visible.contains(*cached) + && !entry.running + && entry.decoded.is_some() + }) .min_by_key(|(_, entry)| entry.last_used) .map(|(key, _)| *key); - let Some(victim) = victim else { - break; - }; + let Some(victim) = victim else { return false }; self.cache.remove(&victim); } - self.schedule(); - changed } + pub fn prepare(&mut self, image: &MediaImage, width: u16) -> Option { if !self.enabled() || width == 0 { return None; } self.clock = self.clock.wrapping_add(1); + if let Some(entry) = self.cache.get(&image.key) + && let Some(generation) = entry.deferred_at + { + if generation == self.visibility_generation { + return None; + } + while self.queued_bytes().saturating_add(image.data.len()) > MAX_QUEUED_SOURCE_BYTES { + if !self.evict() { + return None; + } + } + if let Some(entry) = self.cache.get_mut(&image.key) { + entry.deferred_at = None; + entry.source = Some((image.data.clone(), image.mime_type.clone())); + } + } if !self.cache.contains_key(&image.key) { if image.data.len() > MAX_BASE64_BYTES || image.mime_type.len() > 256 { return None; @@ -201,6 +264,7 @@ impl ImageRuntime { width, running: false, error: None, + deferred_at: None, last_used: self.clock, }, ); @@ -230,7 +294,7 @@ impl ImageRuntime { let key = self .cache .iter() - .filter(|(_, entry)| !entry.running) + .filter(|(key, entry)| !entry.running && !self.visible.contains(*key)) .min_by_key(|(_, entry)| entry.last_used) .map(|(key, _)| *key); key.is_some_and(|key| self.cache.remove(&key).is_some()) @@ -252,7 +316,10 @@ impl ImageRuntime { .cache .iter() .filter(|(_, entry)| { - !entry.running && entry.error.is_none() && entry.protocol.is_none() + !entry.running + && entry.error.is_none() + && entry.deferred_at.is_none() + && entry.protocol.is_none() }) .max_by_key(|(_, entry)| entry.last_used) .map(|(key, _)| *key) @@ -425,6 +492,119 @@ mod tests { .unwrap(); } + async fn settle_work(runtime: &mut ImageRuntime) { + tokio::time::timeout(Duration::from_secs(15), async { + while runtime.pending() { + runtime.poll(); + tokio::time::sleep(Duration::from_millis(1)).await; + } + }) + .await + .unwrap(); + } + + #[tokio::test] + async fn visible_decoded_pressure_settles_and_recovers_after_scroll() { + let sources: Vec<_> = (0..3) + .map(|seed| { + let mut bytes = Cursor::new(Vec::new()); + image::DynamicImage::ImageRgb8(image::RgbImage::from_pixel( + 4096, + 4096, + image::Rgb([seed, 80, 120]), + )) + .write_to(&mut bytes, image::ImageFormat::Png) + .unwrap(); + MediaImage::new(STANDARD.encode(bytes.into_inner()), "image/png".into(), 0).unwrap() + }) + .collect(); + let keys: Vec<_> = sources.iter().map(|source| source.key).collect(); + let mut runtime = ImageRuntime::with_picker(Picker::halfblocks()); + runtime.set_visible_keys(keys.clone()); + for source in &sources { + runtime.prepare(source, 40); + } + settle_work(&mut runtime).await; + let deferred = sources + .iter() + .find(|source| runtime.status(&source.key).contains("deferred")) + .unwrap(); + assert!( + sources + .iter() + .any(|source| runtime.status(&source.key) == "image ready") + ); + for _ in 0..3 { + runtime.set_visible_keys(keys.clone()); + for source in &sources { + runtime.prepare(source, 40); + } + assert!(!runtime.pending()); + } + // Resize rebuilds only admitted protocols; it is not more decode capacity. + let retained = runtime + .cache + .iter() + .filter_map(|(key, entry)| entry.decoded.clone().map(|decoded| (*key, decoded))) + .collect::>(); + for source in &sources { + runtime.prepare(source, 60); + } + settle_work(&mut runtime).await; + assert!(runtime.status(&deferred.key).contains("deferred")); + for (key, decoded) in retained { + assert!(Arc::ptr_eq( + &decoded, + runtime.cache[&key].decoded.as_ref().unwrap() + )); + } + runtime.set_visible_keys([deferred.key]); + runtime.prepare(deferred, 60); + settle_work(&mut runtime).await; + assert_eq!(runtime.status(&deferred.key), "image ready"); + assert!( + runtime + .cache + .values() + .filter_map(|entry| entry.decoded.as_ref()) + .map(|image| image.as_bytes().len() as u64) + .sum::() + <= MAX_DECODED_BACKING_BYTES + ); + runtime.clear(); + runtime.set_visible_keys([sources[0].key]); + runtime.prepare(&sources[0], 40); + settle_work(&mut runtime).await; + assert_eq!(runtime.status(&sources[0].key), "image ready"); + } + + #[tokio::test] + async fn visible_entry_pressure_does_not_cycle_decoding() { + let sources: Vec<_> = (0..MAX_CACHE_ENTRIES + 2) + .map(|seed| source(seed as u8)) + .collect(); + let keys: Vec<_> = sources.iter().map(|source| source.key).collect(); + let mut runtime = ImageRuntime::with_picker(Picker::halfblocks()); + runtime.set_visible_keys(keys.clone()); + for source in &sources { + runtime.prepare(source, 40); + } + settle_work(&mut runtime).await; + for _ in 0..3 { + runtime.set_visible_keys(keys.clone()); + for source in &sources { + runtime.prepare(source, 40); + } + assert!(!runtime.pending()); + } + let last = sources.last().unwrap(); + assert!(runtime.status(&last.key).contains("deferred")); + runtime.set_visible_keys([last.key]); + runtime.prepare(last, 40); + settle_work(&mut runtime).await; + assert_eq!(runtime.status(&last.key), "image ready"); + } + #[test] fn disabled_runtime_uses_text_fallback() { assert!(ImageRuntime::disabled().prepare(&source(0), 40).is_none()); diff --git a/src/tui/markdown_images.rs b/src/tui/markdown_images.rs index 7df5aff..085af05 100644 --- a/src/tui/markdown_images.rs +++ b/src/tui/markdown_images.rs @@ -18,6 +18,7 @@ const MAX_JOBS: usize = 2; enum State { Waiting, + Deferred, Loading(tokio::task::Id), Ready(MediaImage), Failed(String), @@ -26,6 +27,7 @@ enum State { struct Entry { state: State, used: u64, + key: Option<[u8; 32]>, } struct Completion { @@ -40,6 +42,7 @@ pub(super) struct MarkdownImages { generation: u64, policy: ImagePolicy, entries: HashMap, + visible: HashSet, changed_sources: HashSet, jobs: JoinSet, clock: u64, @@ -54,6 +57,7 @@ impl MarkdownImages { generation: 0, policy: ImagePolicy::from_environment(), entries: HashMap::new(), + visible: HashSet::new(), changed_sources: HashSet::new(), jobs: JoinSet::new(), clock: 0, @@ -74,12 +78,46 @@ impl MarkdownImages { self.generation = self.generation.wrapping_add(1); self.changed_sources.extend(self.entries.keys().cloned()); self.entries.clear(); + self.visible.clear(); self.retained = 0; // Stop old async stages, including requests following DNS. Actual blocking // closures retain global admission until completion even after this abort. self.jobs.abort_all(); } + /// Register the complete viewport before polling or requesting sources. The + /// first unique destinations form a stable, bounded admission set. + pub fn set_visible<'a>(&mut self, sources: impl IntoIterator) { + let mut visible = HashSet::new(); + for source in sources { + if source.len() > 4096 { + continue; + } + visible.insert(source.to_owned()); + if visible.len() == MAX_ENTRIES { + break; + } + } + if visible == self.visible { + return; + } + self.visible = visible; + for entry in self.entries.values_mut() { + if matches!(entry.state, State::Deferred) { + entry.state = State::Waiting; + } + } + } + + pub fn is_visible(&self, source: &str) -> bool { + self.visible.contains(source) + } + + /// Authorized content identity survives byte-pressure deferral. + pub fn key(&self, source: &str) -> Option<[u8; 32]> { + self.entries.get(source)?.key + } + pub fn take_changed_sources(&mut self) -> HashSet { std::mem::take(&mut self.changed_sources) } @@ -111,10 +149,12 @@ impl MarkdownImages { } let state = match done.image { Ok(image) => { + if let Some(entry) = self.entries.get_mut(&done.source) { + entry.key = Some(image.key); + } let bytes = image.data.len(); - // Reclaim snapshots, never active acquisitions. An image that - // cannot fit stays retryable instead of caching capacity as - // an intrinsic source failure. + // Reclaim offscreen snapshots, never the visible working set. + // Capacity deferrals retry only when that set changes. if bytes <= MAX_SOURCE_BYTES { while self.retained.saturating_add(bytes) > MAX_SOURCE_BYTES { if !self.evict_oldest(true) { @@ -127,7 +167,7 @@ impl MarkdownImages { self.changed_sources.insert(done.source.clone()); State::Ready(image) } else { - State::Waiting + State::Deferred } } Err(error) => State::Failed(error), @@ -146,9 +186,10 @@ impl MarkdownImages { let oldest = self .entries .iter() - .filter(|(_, entry)| { - matches!(entry.state, State::Ready(_)) - || (!ready_only && !matches!(entry.state, State::Loading(_))) + .filter(|(source, entry)| { + !self.visible.contains(*source) + && (matches!(entry.state, State::Ready(_)) + || (!ready_only && !matches!(entry.state, State::Loading(_)))) }) .min_by_key(|(_, entry)| entry.used) .map(|(source, _)| source.clone()); @@ -165,7 +206,7 @@ impl MarkdownImages { } pub fn request(&mut self, source: &str) { - if source.len() > 4096 { + if source.len() > 4096 || (!self.visible.is_empty() && !self.is_visible(source)) { return; } self.clock = self.clock.wrapping_add(1); @@ -178,6 +219,7 @@ impl MarkdownImages { Entry { state: State::Waiting, used: self.clock, + key: None, }, ); } @@ -232,6 +274,7 @@ impl MarkdownImages { match self.entries.get(source).map(|entry| &entry.state) { Some(State::Ready(_)) => "image", Some(State::Failed(error)) => error, + Some(State::Deferred) => "image deferred (visible image budget)", _ => "image loading", } } @@ -256,6 +299,20 @@ mod tests { std::fs::write(root.join(name), bytes.into_inner()).unwrap(); } + fn write_large_png(root: &Path, name: &str, value: u8) { + use image::{ + ImageEncoder as _, + codecs::png::{CompressionType, FilterType, PngEncoder}, + }; + let pixels = vec![value; 1792 * 1366 * 3]; + let mut bytes = Vec::new(); + PngEncoder::new_with_quality(&mut bytes, CompressionType::Level(0), FilterType::NoFilter) + .write_image(&pixels, 1792, 1366, image::ExtendedColorType::Rgb8) + .unwrap(); + assert!((7 * 1024 * 1024..8 * 1024 * 1024).contains(&bytes.len())); + std::fs::write(root.join(name), bytes).unwrap(); + } + async fn settle(images: &mut MarkdownImages) { tokio::time::timeout(Duration::from_secs(5), async { while images.pending() { @@ -297,26 +354,11 @@ mod tests { #[tokio::test] async fn source_byte_pressure_evicts_lru_and_revisited_images_recover() { - use image::{ - ImageEncoder as _, - codecs::png::{CompressionType, FilterType, PngEncoder}, - }; - let root = tempfile::tempdir().unwrap(); let sources = ["first.png", "second.png", "third.png", "fourth.png"]; for (index, source) in sources.iter().enumerate() { // Real, individually valid ~7 MiB PNGs, with distinct pixel content. - let pixels = vec![index as u8; 1792 * 1366 * 3]; - let mut bytes = Vec::new(); - PngEncoder::new_with_quality( - &mut bytes, - CompressionType::Level(0), - FilterType::NoFilter, - ) - .write_image(&pixels, 1792, 1366, image::ExtendedColorType::Rgb8) - .unwrap(); - assert!((7 * 1024 * 1024..8 * 1024 * 1024).contains(&bytes.len())); - std::fs::write(root.path().join(source), bytes).unwrap(); + write_large_png(root.path(), source, index as u8); } let mut images = MarkdownImages::new(); images.context(root.path(), Some("session")); @@ -376,6 +418,121 @@ mod tests { assert!(images.retained <= MAX_SOURCE_BYTES); } + async fn settle_viewport(images: &mut MarkdownImages, sources: &[&str]) { + tokio::time::timeout(Duration::from_secs(10), async { + loop { + images.set_visible(sources.iter().copied()); + images.poll(); + for source in sources { + images.request(source); + } + if !images.pending() { + break; + } + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + } + + #[tokio::test] + async fn visible_source_byte_pressure_settles_and_recovers_on_viewport_change() { + let root = tempfile::tempdir().unwrap(); + let sources = ["first.png", "second.png", "third.png", "fourth.png"]; + for (index, source) in sources.iter().enumerate() { + write_large_png(root.path(), source, index as u8); + } + let mut images = MarkdownImages::new(); + images.context(root.path(), Some("session")); + settle_viewport(&mut images, &sources).await; + let ready: Vec<_> = sources + .iter() + .copied() + .filter(|source| images.image(source).is_some()) + .collect(); + let deferred: Vec<_> = sources + .iter() + .copied() + .filter(|source| images.status(source) == "image deferred (visible image budget)") + .collect(); + assert_eq!(ready.len(), 3); + assert_eq!(deferred.len(), 1); + let key = images.key(deferred[0]).unwrap(); + images.take_changed_sources(); + for _ in 0..8 { + images.set_visible(sources); + assert!(!images.poll()); + for source in sources { + images.request(source); + } + assert!(!images.pending()); + assert!(ready.iter().all(|source| images.image(source).is_some())); + assert_eq!(images.key(deferred[0]), Some(key)); + assert!(images.take_changed_sources().is_empty()); + } + // Shrinking the viewport (scroll or resize) releases offscreen snapshots. + settle_viewport(&mut images, &deferred).await; + assert!(images.image(deferred[0]).is_some()); + assert_eq!(images.key(deferred[0]), Some(key)); + settle_viewport(&mut images, &sources).await; + for source in sources { + images.request(source); + } + assert!(!images.pending()); + assert!(images.retained <= MAX_SOURCE_BYTES); + images.context(root.path(), Some("replacement")); + assert!(sources.iter().all(|source| images.key(source).is_none())); + settle_viewport(&mut images, &sources).await; + assert_eq!( + sources + .iter() + .filter(|source| images.image(source).is_some()) + .count(), + 3 + ); + assert!(!images.pending()); + } + + #[tokio::test] + async fn visible_entry_overflow_keeps_first_unique_sources_and_scroll_recovers() { + let root = tempfile::tempdir().unwrap(); + let sources: Vec<_> = (0..MAX_ENTRIES + 2) + .map(|index| format!("{index}.png")) + .collect(); + for source in &sources { + write_png(root.path(), source); + } + let viewport: Vec<_> = sources.iter().map(String::as_str).collect(); + let mut images = MarkdownImages::new(); + images.context(root.path(), Some("session")); + settle_viewport(&mut images, &viewport).await; + for _ in 0..8 { + // Repeated occurrences do not consume extra admission slots. + images.set_visible(viewport.iter().flat_map(|source| [*source, *source])); + assert!(!images.poll()); + for (index, source) in sources.iter().enumerate() { + assert_eq!(images.is_visible(source), index < MAX_ENTRIES); + images.request(source); + assert_eq!(images.image(source).is_some(), index < MAX_ENTRIES); + } + assert!(!images.pending()); + } + settle_viewport(&mut images, &viewport[2..]).await; + assert!( + viewport[2..] + .iter() + .all(|source| images.image(source).is_some()) + ); + settle_viewport(&mut images, &viewport[..MAX_ENTRIES]).await; + assert!( + viewport[..MAX_ENTRIES] + .iter() + .all(|source| images.image(source).is_some()) + ); + assert!(images.retained <= MAX_SOURCE_BYTES); + } + #[tokio::test] async fn entry_pressure_preserves_active_acquisitions() { let root = tempfile::tempdir().unwrap(); diff --git a/src/tui/mod.rs b/src/tui/mod.rs index 8888bb0..17565d0 100644 --- a/src/tui/mod.rs +++ b/src/tui/mod.rs @@ -1160,7 +1160,7 @@ pub async fn run_with_reasoning_effort_and_openrouter_key( return Err(agent_client_protocol::Error::into_internal_error(error)); } let event = { - let redraw = app.needs_redraw_tick() || images.pending(); + let redraw = app.needs_redraw_tick() || images.pending() || !app.transcript_dirty.is_empty(); let mut stopped = pin!(stop.requested()); // Rotate the first eligible source after every winner. // If none is ready, poll every eligible source with the @@ -1349,7 +1349,7 @@ pub async fn run_with_reasoning_effort_and_openrouter_key( .draw(|frame| ui::draw(frame, &mut app, &mut images)) .map_err(agent_client_protocol::Error::into_internal_error)?; let event = { - let redraw = app.needs_redraw_tick() || images.pending(); + let redraw = app.needs_redraw_tick() || images.pending() || !app.transcript_dirty.is_empty(); let mut stopped = pin!(stop.requested()); let mut shutdown = pin!(storage_shutdown.cancelled()); // A local round-robin race keeps hot input/update queues diff --git a/src/tui/ui.rs b/src/tui/ui.rs index ab45e67..530ebd5 100644 --- a/src/tui/ui.rs +++ b/src/tui/ui.rs @@ -51,8 +51,13 @@ pub fn draw(frame: &mut Frame<'_>, app: &mut App, images: &mut ImageRuntime) { images .markdown .context(&app.root, app.session_id.as_deref()); - images.poll(); - app.invalidate_image_layout(&images.markdown.take_changed_sources()); + if app.blocks.is_empty() { + images.markdown.set_visible(std::iter::empty()); + images.markdown.poll(); + images.set_visible_keys(std::iter::empty()); + images.poll(); + app.invalidate_image_layout(&images.markdown.take_changed_sources()); + } // Two border columns plus the `›` gutter; the prompt grows as the wrapped // text needs more rows, up to the cap. let start_width = frame @@ -1004,6 +1009,39 @@ fn draw_transcript(frame: &mut Frame<'_>, app: &mut App, images: &mut ImageRunti let row_widths: Vec = visible.iter().map(ratatui::text::Line::width).collect(); frame.render_widget(Paragraph::new(visible), inner); draw_selection(frame, app, inner, offset, &row_widths); + // Register the whole viewport before any completion can evict another + // visible image. Known identities include source snapshots deferred by the + // source byte budget, not just images already in the decoder cache. + images.markdown.set_visible( + visible_images + .iter() + .filter_map(|(_, source, _)| match source { + TranscriptImageSource::Markdown(destination) => Some(destination.as_str()), + TranscriptImageSource::Typed(_) => None, + }), + ); + images.markdown.poll(); + let visible_keys = visible_images + .iter() + .filter_map(|(block_index, source, _)| match source { + TranscriptImageSource::Typed(index) => { + let sources = match app.blocks.get(*block_index) { + Some(Block::User(message) | Block::Agent(message)) => &message.images, + Some(Block::Tool(call)) => &call.images, + _ => return None, + }; + sources.get(*index).map(|source| source.key) + } + TranscriptImageSource::Markdown(destination) => images + .markdown + .is_visible(destination) + .then(|| images.markdown.key(destination)) + .flatten(), + }) + .collect::>(); + images.set_visible_keys(visible_keys); + images.poll(); + app.invalidate_image_layout(&images.markdown.take_changed_sources()); for (block_index, source, y) in visible_images { let (prepared, status) = match source { TranscriptImageSource::Typed(index) => { @@ -1024,6 +1062,10 @@ fn draw_transcript(frame: &mut Frame<'_>, app: &mut App, images: &mut ImageRunti } // Requests start only for viewports intersecting the screen. Layout // and replay never initiate IO for offscreen transcript history. + if !images.markdown.is_visible(&destination) { + draw_image_status(frame, inner, y, "image deferred (visible image budget)"); + continue; + } images.markdown.request(&destination); if let Some(source) = images.markdown.image(&destination).cloned() { let prepared = images.prepare(&source, inner.width.max(1)); @@ -1142,6 +1184,48 @@ fn welcome_logo() -> Paragraph<'static> { Paragraph::new(lines).alignment(Alignment::Center) } +/// Keep the same logical row when cache-dependent dedup inserts or removes +/// reserved image rows. Image anchors include their occurrence so repeated +/// Markdown destinations do not jump to the first occurrence. +fn remap_image_layout_row( + old: &[CachedTranscriptImage], + new: &[CachedTranscriptImage], + row: usize, +) -> usize { + let reserved = usize::from(RESERVED_ROWS); + let mut text_row = row; + for (index, image) in old.iter().enumerate() { + if image.row > row { + break; + } + if row < image.row + reserved { + let occurrence = old[..index] + .iter() + .filter(|previous| previous.source == image.source) + .count(); + if let Some(current) = new + .iter() + .filter(|current| current.source == image.source) + .nth(occurrence) + { + return current.row + row - image.row; + } + // A removed image falls back to the following logical text row. + text_row -= row - image.row; + break; + } + text_row -= reserved; + } + let mut mapped = text_row; + for image in new { + if image.row > mapped { + break; + } + mapped += reserved; + } + mapped +} + /// Renders the transcript, tagging each line with the tool call it belongs to /// so a click on a card can be traced back to it. fn refresh_transcript_cache_with_images(app: &mut App, images: &mut ImageRuntime, width: usize) { @@ -1152,6 +1236,42 @@ fn refresh_transcript_cache_with_images(app: &mut App, images: &mut ImageRuntime app.sync_transcript_cache(); } let width_changed = app.transcript_cache_width != width; + // A fixed numeric scroll offset is not a viewport anchor: source readiness + // can remove a typed duplicate above it, change the visible working set, + // and trigger eviction/reacquisition forever. Preserve the top block and + // its logical row only for an unchanged User/Agent message, the only blocks + // with cache-dependent Markdown/typed-image dedup. Tools and thoughts keep + // their existing scroll behavior. Earlier blocks may change: their rebuilt + // prefixes locate the same message. + // A replacement of the anchored block makes old image indices and logical + // row numbers unsafe, even at the same width/block count. + let mut anchor = if !app.follow && !structure_changed && !width_changed { + let block = app + .transcript_prefixes + .partition_point(|prefix| *prefix <= app.scroll) + .saturating_sub(1); + app.transcript_cache + .get(block) + .and_then(Option::as_ref) + .filter(|cached| { + matches!( + app.blocks.get(block), + Some(Block::User(_) | Block::Agent(_)) + ) && cached.revision == app.transcript_revisions[block] + }) + .map(|_| { + let start = app.transcript_prefixes[block]; + let separator = usize::from(start > 0); + let on_separator = separator > 0 && app.scroll == start; + ( + block, + app.scroll.saturating_sub(start + separator), + on_separator, + ) + }) + } else { + None + }; let mut layout_changed = structure_changed || width_changed; if width_changed { app.transcript_cache_width = width; @@ -1168,14 +1288,8 @@ fn refresh_transcript_cache_with_images(app: &mut App, images: &mut ImageRuntime _ => false, }; let revision = app.transcript_revisions[block_index]; - if !width_changed - && !dynamic - && app.transcript_cache[block_index] - .as_ref() - .is_some_and(|cached| cached.revision == revision) - { - continue; - } + // Dirty includes source-cache-only invalidations, whose content + // revision intentionally matches the cache. Every dirty block rebuilds. let missing = app.transcript_cache[block_index].is_none(); let old_count = app.transcript_cache[block_index] .as_ref() @@ -1186,6 +1300,13 @@ fn refresh_transcript_cache_with_images(app: &mut App, images: &mut ImageRuntime first_changed_count = first_changed_count.min(block_index); layout_changed |= !missing; } + if let Some((block, row, false)) = &mut anchor + && *block == block_index + && let Some(old) = &app.transcript_cache[block_index] + { + *row = remap_image_layout_row(&old.images, &cached_images, *row) + .min(rows.len().saturating_sub(1)); + } app.transcript_cache[block_index] = Some(CachedTranscriptBlock { revision, rows, @@ -1204,6 +1325,15 @@ fn refresh_transcript_cache_with_images(app: &mut App, images: &mut ImageRuntime app.transcript_prefixes[index + 1] = app.transcript_prefixes[index] + rows + usize::from(app.transcript_prefixes[index] > 0); } + if let Some((block, row, on_separator)) = anchor { + let start = app.transcript_prefixes[block]; + app.scroll = start + + if on_separator { + 0 + } else { + usize::from(start > 0) + row + }; + } if layout_changed { app.clear_transcript_interaction(); } @@ -4549,8 +4679,8 @@ mod tests { refresh_transcript_cache_with_images(&mut app, &mut images, 80); let revisions = app.transcript_revisions.clone(); app.invalidate_image_layout(&std::collections::HashSet::from(["first.png".into()])); - assert_ne!(app.transcript_revisions[0], revisions[0]); - assert_eq!(app.transcript_revisions[1], revisions[1]); + assert_eq!(app.transcript_revisions, revisions); + assert_eq!(app.transcript_dirty, std::collections::BTreeSet::from([0])); assert_eq!( app.transcript_cache[1].as_ref().unwrap().revision, revisions[1] @@ -4644,6 +4774,307 @@ mod tests { } } + #[test] + fn message_replacements_do_not_reuse_image_layout_anchors() { + fn typed_image(value: u8, line: usize) -> UserImage { + let mut bytes = std::io::Cursor::new(Vec::new()); + image::DynamicImage::ImageRgb8(image::RgbImage::from_pixel( + 2, + 3, + image::Rgb([value; 3]), + )) + .write_to(&mut bytes, image::ImageFormat::Png) + .unwrap(); + UserImage::new( + base64::engine::general_purpose::STANDARD.encode(bytes.into_inner()), + "image/png".into(), + line, + ) + .unwrap() + } + let tail = "tail\n".repeat(80); + let initial = format!("typed\n{tail}![A](a.png)\n{tail}"); + let replacement = format!( + "{}typed\n{tail}![A](a.png)\n{tail}", + "inserted\n".repeat(20) + ); + // Exercise both orders of a source completion overlapping replacement. + for invalidate_first in [false, true] { + for change_pixels in [false, true] { + let mut app = sample(); + app.start_session("owner".into()); + let update = |text: String, image| Update::UserMessage { + id: "message".into(), + text, + images: vec![image], + append: false, + }; + app.apply(update(initial.clone(), typed_image(0, 0))); + let mut images = ImageRuntime::with_picker(Picker::halfblocks()); + let mut terminal = Terminal::new(TestBackend::new(80, 38)).unwrap(); + terminal + .draw(|frame| draw(frame, &mut app, &mut images)) + .unwrap(); + let scroll = if change_pixels { 5 } else { 14 }; + app.scroll_by(scroll as isize - app.scroll as isize); + terminal + .draw(|frame| draw(frame, &mut app, &mut images)) + .unwrap(); + assert_eq!(app.scroll, scroll); + assert_eq!(app.transcript_cache[0].as_ref().unwrap().images[0].row, 1); + let changed = std::collections::HashSet::from(["a.png".to_owned()]); + if invalidate_first { + app.invalidate_image_layout(&changed); + } + app.apply(update( + replacement.clone(), + typed_image(u8::from(change_pixels), 20), + )); + if !invalidate_first { + app.invalidate_image_layout(&changed); + } + terminal + .draw(|frame| draw(frame, &mut app, &mut images)) + .unwrap(); + assert_eq!(app.blocks.len(), 1); + assert_eq!(app.transcript_cache[0].as_ref().unwrap().images[0].row, 21); + // Row 14 must not become row 2; a new image at typed index 0 + // must not inherit the old image's partially clipped anchor. + assert_eq!(app.scroll, scroll); + } + } + + let mut app = sample(); + app.start_session("owner".into()); + let update = |text| Update::UserMessage { + id: "message".into(), + text, + images: Vec::new(), + append: false, + }; + app.apply(update(format!("![A](a.png)\n![A](a.png)\n{tail}"))); + let mut images = ImageRuntime::with_picker(Picker::halfblocks()); + let mut terminal = Terminal::new(TestBackend::new(80, 38)).unwrap(); + terminal + .draw(|frame| draw(frame, &mut app, &mut images)) + .unwrap(); + let scroll = app.transcript_cache[0].as_ref().unwrap().images[1].row + 1; + app.scroll_by(scroll as isize - app.scroll as isize); + terminal + .draw(|frame| draw(frame, &mut app, &mut images)) + .unwrap(); + app.apply(update(format!("![A](a.png)\n{tail}"))); + app.invalidate_image_layout(&std::collections::HashSet::from(["a.png".into()])); + terminal + .draw(|frame| draw(frame, &mut app, &mut images)) + .unwrap(); + assert_eq!(app.transcript_cache[0].as_ref().unwrap().images.len(), 1); + assert_eq!(app.scroll, scroll); + } + + #[test] + fn image_layout_anchor_preserves_text_and_repeated_image_occurrences() { + use crate::tui::app::{CachedTranscriptImage, TranscriptImageSource}; + let typed = CachedTranscriptImage { + source: TranscriptImageSource::Typed(0), + row: 1, + }; + let markdown = |row| CachedTranscriptImage { + source: TranscriptImageSource::Markdown("a.png".into()), + row, + }; + let old = [typed, markdown(14), markdown(28)]; + let new = [markdown(2), markdown(16)]; + // Text before, between, and after reservations retains its logical row. + assert_eq!(super::remap_image_layout_row(&old, &new, 0), 0); + assert_eq!(super::remap_image_layout_row(&old, &new, 27), 15); + assert_eq!(super::remap_image_layout_row(&old, &new, 40), 28); + // The second repeated occurrence stays the second, with the same clip. + assert_eq!(super::remap_image_layout_row(&old, &new, 30), 18); + assert_eq!(super::remap_image_layout_row(&new, &old, 18), 30); + // If the anchored image itself vanishes, use the next logical text row. + assert_eq!(super::remap_image_layout_row(&old, &new, 5), 1); + } + + #[tokio::test] + async fn markdown_dedup_keeps_scrolled_viewport_stable_under_source_pressure() { + use crate::tui::app::TranscriptImageSource; + use image::{ + ImageEncoder as _, + codecs::png::{CompressionType, FilterType, PngEncoder}, + }; + + fn image_row(app: &App, destination: &str) -> usize { + app.transcript_cache + .iter() + .enumerate() + .find_map(|(index, block)| { + block + .as_ref()? + .images + .iter() + .find(|image| { + image.source == TranscriptImageSource::Markdown(destination.into()) + }) + .map(|image| { + let start = app.transcript_prefixes[index]; + start + usize::from(start > 0) + image.row + }) + }) + .unwrap() + } + + async fn settle_draws( + terminal: &mut Terminal, + app: &mut App, + images: &mut ImageRuntime, + ) { + tokio::time::timeout(std::time::Duration::from_secs(20), async { + loop { + app.tick(); + terminal.draw(|frame| draw(frame, app, images)).unwrap(); + if !images.pending() + && app + .transcript_dirty + .iter() + .all(|index| app.transcript_dynamic.contains(index)) + { + break; + } + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + } + + let directory = tempfile::tempdir().unwrap(); + let sources = ["a.png", "c.png", "d.png", "b.png"]; + let mut typed = None; + let mut encoded_bytes = 0; + for (index, source) in sources.iter().enumerate() { + // Valid ~7 MiB PNGs: three encoded snapshots fit, four do not. + let pixels = vec![index as u8; 1792 * 1366 * 3]; + let mut bytes = Vec::new(); + PngEncoder::new_with_quality( + &mut bytes, + CompressionType::Level(0), + FilterType::NoFilter, + ) + .write_image(&pixels, 1792, 1366, image::ExtendedColorType::Rgb8) + .unwrap(); + std::fs::write(directory.path().join(source), &bytes).unwrap(); + let data = base64::engine::general_purpose::STANDARD.encode(bytes); + encoded_bytes += data.len(); + if index == 0 { + typed = UserImage::new(data, "image/png".into(), 0); + } + } + assert!(encoded_bytes > 32 * 1024 * 1024); + assert!(encoded_bytes / 4 * 3 < 32 * 1024 * 1024); + for running_prefix in [false, true] { + let mut app = App::new( + directory.path().into(), + "provider".into(), + "model".into(), + "a2a".into(), + ); + app.start_session("owner".into()); + if running_prefix { + app.apply(Update::ToolStarted { + id: "running-prefix".into(), + title: "Working".into(), + kind: ToolKind::Other, + script: None, + backgrounded: false, + }); + } + app.apply(Update::UserMessage { + id: "message".into(), + text: format!( + "typed duplicate\n{}![A](a.png)\n![C](c.png)\n![D](d.png)\n![B](b.png)\n{}", + "before\n".repeat(20), + "after\n".repeat(80) + ), + images: vec![typed.clone().unwrap()], + append: false, + }); + let mut images = ImageRuntime::with_picker(Picker::halfblocks()); + let mut terminal = Terminal::new(TestBackend::new(80, 38)).unwrap(); + // Start below the image viewports, then load B by genuinely scrolling. + settle_draws(&mut terminal, &mut app, &mut images).await; + let b = image_row(&app, "b.png"); + app.scroll_by(b as isize - app.scroll as isize + 1); + settle_draws(&mut terminal, &mut app, &mut images).await; + assert!(images.markdown.image("b.png").is_some()); + assert!(images.markdown.image("a.png").is_none()); + + let a = image_row(&app, "a.png"); + app.scroll_by(a as isize - app.scroll as isize + 1); + terminal + .draw(|frame| draw(frame, &mut app, &mut images)) + .unwrap(); + // Typed A is above the viewport. Markdown A and D straddle its edges; + // B would enter if dedup removed twelve rows without moving the anchor. + assert!(app.scroll > usize::from(super::RESERVED_ROWS)); + assert_eq!(app.scroll, image_row(&app, "a.png") + 1); + assert!(image_row(&app, "d.png") < app.scroll + app.viewport); + assert!( + image_row(&app, "d.png") + usize::from(super::RESERVED_ROWS) + > app.scroll + app.viewport + ); + assert!(image_row(&app, "b.png") >= app.scroll + app.viewport); + assert!( + image_row(&app, "b.png") - usize::from(super::RESERVED_ROWS) + < app.scroll + app.viewport + ); + settle_draws(&mut terminal, &mut app, &mut images).await; + assert_eq!(app.scroll, a + 1 - usize::from(super::RESERVED_ROWS)); + assert_eq!(app.scroll, image_row(&app, "a.png") + 1); + assert!( + sources[..3] + .iter() + .all(|source| images.markdown.image(source).is_some()) + ); + assert!(images.markdown.image("b.png").is_none()); + let stable_scroll = app.scroll; + for _ in 0..16 { + app.tick(); + terminal + .draw(|frame| draw(frame, &mut app, &mut images)) + .unwrap(); + assert_eq!(app.scroll, stable_scroll); + assert!(!images.pending()); + assert_eq!(app.transcript_dynamic.contains(&0), running_prefix); + assert!( + sources[..3] + .iter() + .all(|source| images.markdown.image(source).is_some()) + ); + assert!(!images.markdown.is_visible("b.png")); + } + + // Scroll to the evicted B and back. Evicting A restores its typed + // duplicate above the viewport; the inverse layout change also anchors. + let b = image_row(&app, "b.png"); + app.scroll_by(b as isize - app.scroll as isize + 1); + settle_draws(&mut terminal, &mut app, &mut images).await; + assert!(images.markdown.image("b.png").is_some()); + assert!(images.markdown.image("a.png").is_none()); + assert_eq!(app.scroll, image_row(&app, "b.png") + 1); + let a = image_row(&app, "a.png"); + app.scroll_by(a as isize - app.scroll as isize + 1); + settle_draws(&mut terminal, &mut app, &mut images).await; + assert_eq!(app.scroll, image_row(&app, "a.png") + 1); + assert!( + sources[..3] + .iter() + .all(|source| images.markdown.image(source).is_some()) + ); + assert!(!images.pending()); + } + } + #[tokio::test] async fn markdown_dedup_requires_loaded_matching_identity_and_preserves_repeats() { let directory = tempfile::tempdir().unwrap(); @@ -4697,7 +5128,7 @@ mod tests { images.markdown.request(destination); tokio::time::timeout(std::time::Duration::from_secs(5), async { while images.markdown.pending() { - images.poll(); + images.markdown.poll(); tokio::task::yield_now().await; } }) From 49986665cac2d17fb553838e5fa69727b727e37c Mon Sep 17 00:00:00 2001 From: daniel Date: Tue, 8 Sep 2026 18:09:27 +0100 Subject: [PATCH 4/4] docs: remove completed image implementation plan --- docs/plans/compose-managed-files.md | 152 ---------------------------- 1 file changed, 152 deletions(-) delete mode 100644 docs/plans/compose-managed-files.md diff --git a/docs/plans/compose-managed-files.md b/docs/plans/compose-managed-files.md deleted file mode 100644 index 95a5898..0000000 --- a/docs/plans/compose-managed-files.md +++ /dev/null @@ -1,152 +0,0 @@ -# Compose-managed files and multimodal tools - -## Goal and non-negotiable constraints - -Keep **compose as the only model-exposed tool**. Build composable file references for image reads, image transformations, multimodal subagents, and consistent TUI rendering. Do not add a directly exposed read-image tool. - -Implement as four stacked PRs, each owned by a distinct subagent in a distinct worktree created using `wt`. Phase 1 carries this plan. Start each later phase only after its predecessor passes the review gate. Never merge, enable auto-merge, enqueue, deploy, or change release versions. - -Base: current main (`origin/main` at setup: `0a9291d7d3ccdd6837903d0521ab2b040ab54044`). Phase-1 worktree was created using `wt switch --create feat/compose-files-phase-1 --base main --no-hooks --format json`, then fast-forwarded to fetched origin/main. The user's original dirty worktree must remain untouched. - -## Architecture - -### Managed File value - -Tools exchange bounded ordinary JSON descriptors, not base64 strings or raw filesystem paths. Illustrative descriptor (final schema requires design review): - -```json -{ - "$kit": "file", - "version": 1, - "id": "file_opaque_id", - "name": "my_image.png", - "mime_type": "image/png", - "size_bytes": 184230, - "image": { "width": 1024, "height": 768 } -} -``` - -Bytes are immutable snapshots in Kit-managed storage. Descriptors survive session resume independently of the original path. Resolution requires session access; metadata is validated against the stored object. A marker is not authorization. Forged IDs, cross-session references, and traversal must fail safely. Future fork/subagent grants must be explicit. Use the existing artifact/resilient filesystem infrastructure where appropriate, but do not stretch the UTF-8 artifact reader into a binary contract. Define version handling, retention, cleanup, restart behavior, and storage-failure semantics. - -### Compose delivery boundary - -```text -read_file / image transform / subagent -> managed bytes + JSON File descriptor - -> Runlet ordinary JSON -> final returned JSON - -> Kit reference validation and resolution - -> ToolOutput::Parts(text/structured result + typed image parts) - -> canonical tool result -> provider adapter -> model - -> ACP -> TUI -``` - -Only references reachable from the final returned value deliver content to the parent model. Intermediate images remain private to the operations consuming them. Resolve nested references with bounded depth/count, deterministic order, identity deduplication, and labels identifying return-value positions. Invalid/inaccessible/unsupported selected references must never appear as successful text-only image delivery. - -Integrate resolution with compose result finalization in `BackgroundableCompose`, sharing behavior across invoke, invoke_outcome, foreground, and background completion. JSON markers should permit the first implementation without changing Runlet or agentkit-tool-compose. Re-check current main before relying on earlier traced line numbers. - -Extract references before text spilling. Apply the 8 KiB spill policy only to the text/JSON portion; preserve selected typed images and enforce separate image budgets. Define interruption, retry/replay, cancellation, and ownership lifetime. - -### Provider contract - -The canonical transcript retains typed tool-result images. **Phase 1 MUST include the non-native fallback**, not defer it to a later phase. Use native multimodal tool output on verified routes; otherwise project text/metadata tool results with a pointer to an immediately following user-role image message through the existing user attachment encoder. Place that message after the full parallel tool-result batch, never between unanswered results. This is provider-request-only: never persist fake user turns, rerun compose, duplicate delivery, or stringify pixels. Preserve original result text, labels, diagnostics, call/result pairing, normalization budgets, background completion, and replay/continuations. Lack of native image tool-output support is not a fatal gate when ordinary image input transport is available; model vision capability remains a provider/model constraint, not an arbitrary Kit allowlist. Verify actual private Responses and Completions/OpenRouter request encoders. - -The acceptance criterion is an actual image block in the next outgoing provider request, not a base64 string or marker. Replay must use snapshotted content after the original file changes or disappears. - -### Hidden tool suite - -Initial read_file imports local content and returns a managed File. Image operations consume references and create new immutable references. Use flat callable names compatible with current Runlet. Export is explicit, not an implicit overwrite. Define format sniffing, bounded read/decode/allocation, cancellation, decoded-pixel/output budgets, animation policy, orientation, and metadata handling. No HTTP/SVG/PDF/OCR support is needed for the initial reader. - -Proposed end-state example (new APIs, not existing syntax contracts): - -```text -image = read_file({ path: "my_image.png" }) -rotated = image_rotate({ image, degrees: 90 }) -cropped = image_crop({ - image: rotated, - aspect_ratio: { width: 1, height: 1 }, - anchor: "center" -}) -stickered = subagent({ - model: "", - prompt: "Add a Hello Kitty sticker to the attached image.", - attachments: [cropped], - output_schema: { - type: "object", - properties: { result: { "$ref": "kit://schemas/file/v1", "x-kit-image-index": 0 } }, - required: ["result"], - additionalProperties: false - } -}) -return stickered.output.result -``` - -Explicit attachment arguments, not concatenating a reference into prompt text, determine image input. A model capable of images behind a text-only harness is still unsupported. - -### Phase 2 implemented contract - -The chosen explicit export name is `export_file({ file, path })`; its path/status receipt has no File marker. Hidden `image_rotate({ image, degrees })`, `image_crop({ image, aspect_ratio, anchor })`, and `image_resize({ image, width, height, fit })` use the unchanged version-1 session-authorized File descriptor and immutable disk publication. Geometry, all nine anchors, contain/cover/stretch rounding, format/orientation/metadata policy, per-stage allocation limits, and disk-only no-clobber export semantics are specified in [the user guide](../user/compose-and-local-tools.md#transform-images-in-one-compose-program). - -The pipeline remains one compose invocation with final-return-only delivery, using the existing foreground/background/replay finalizer and native/user-role provider transport. Export uses OS process permissions rather than inventing a filesystem sandbox; its explicit create-new/partial-output cancellation contract does not overwrite or roll back user paths. No dependency or persistent envelope schema change is intended. Phase 3 subagent contracts and phase 4 presentation remain separate milestones. - -### Multimodal subagents - -Extend the currently text-oriented ACP child prompt/output path to retain typed attachments and assistant media. Import native generated media into managed storage. Bind actual emitted media to a file-aware output contract; models must not invent file IDs. Specify single-image binding and reject ambiguous multiple output. Validate shape AND reference existence/access, with explicit contract failure rather than silent string fallback for the new file-aware contract. Parent outputs must survive child close; grants and promotion must preserve session isolation. - -### Phase 3 implemented contract - -The subagent tool layer accepts optional typed `attachments` on `subagent`, `prompt`, and `fork`, resolving authority from `ToolRequest.session_id`. Native ACP image inputs and generated outputs use managed storage rather than model-created identities. Attachment grants cross working-directory stores explicitly; generated-image parent publication and final access validation complete before the existing success transition. Errors use the existing create cleanup, continuation retry-handle, and fork cleanup paths without adding shared-state instrumentation. - -File-aware `output_schema` uses the locally resolved `kit://schemas/file/v1` reference. The supported binding is exactly one root File or one required fixed nested object-property path. By default, Kit requires exactly one distinct native assistant image; an optional caller-fixed `x-kit-image-index` integer 0–7 beside the exact File `$ref` instead selects a distinct image in first-emission order. Kit rejects attempted model binding, validates surrounding JSON and the completed schema, and only constructs omitted surrounding objects when the complete result is valid. Arrays, unions, conditionals, indirect references and multiple bindings are unsupported. Every native image occurrence is independently validated and charged against occurrence, encoded/decoded byte, and aggregate pixel budgets before deduplication. Only equal MIME types and byte-identical image payloads from the current output collapse; visually identical images with different bytes remain distinct and require an explicit index to select one. Every occurrence, including unselected images, must validate and satisfy budgets before selection. Out-of-range indices fail without fallback, and only the selected image is imported/published. Kit does not strip signed metadata or deduplicate perceptually. No model identity, input image, or previous turn participates in that comparison. Ordinary text-only schema fallback is preserved. Native images without a file-aware schema have the explicit `output: { value, files }` surface, not base64 diagnostic updates. The [user guide](../user/compose-and-local-tools.md#attach-files-to-subagents-and-return-native-images) specifies the contract and final-return-only behavior. - -The complete read/rotate/crop → built-in ACP subagent → managed output → child close → `return output.result` pipeline has been verified against the canonical OpenRouter `google/gemini-3-pro-image` route. The caller explicitly selected distinct native output index 0; this does not claim that the backend emits only one image. The selected bytes were visually verified to contain the requested Hello Kitty sticker. Eligibility comes from exact-model and concrete-endpoint capability discovery, not a model allowlist. Other harness/provider routes require their own native-output support. Shared TUI presentation remains Phase 4 work. - -### Shared TUI presentation - -Generalize user-image rendering into reusable media presentation for user attachments, tool results, assistant-generated media, and Markdown image nodes. Share decode/cache/terminal protocols and budgets, and align live updates with history replay. Keep media out of text-only search/previews/logs. - -Markdown image nodes such as `![Edited image](kit-file://file_opaque_id)` resolve through the managed file resolver. Local paths follow filesystem permission policy. Remote images require explicit network policy, asynchronous bounded fetch/decode, redirect/address validation, and no implicit credentials. Do not fetch arbitrary model-supplied URLs without the applicable authorization. Parse real image nodes, not regexes scanning code fences. Retain alt text, source links, placeholders, and terminal fallback. Ordinary links remain links unless explicitly previewed. TUI display never automatically attaches pixels to model context. - -### Phase 4 presentation contract - -The TUI retains one presentation-only message/image representation for native user, assistant, and tool media. CommonMark image nodes use source-aware layout without splitting Markdown documents. Managed references resolve under current-session authority; external sources are temporary snapshots and are re-resolved after cache reset or replay. Duplicate typed/Markdown viewports require independently authorized, exact resolved content identity within the same message; repeated Markdown nodes remain visible. No display path imports files, grants access, or adds model-context attachments. - -Remote loading is default-denied and requires the startup `KIT_TUI_IMAGE_ORIGINS` exact-origin HTTPS policy. Redirects are rejected, every resolved address must be public, and DNS results are pinned while TLS hostname checks remain active. The dedicated client has no ambient proxy or credential configuration. Acquisition and the common decode/protocol renderer have separate bounded worker admission; blocking DNS/file/decode operations retain their permits even after cancellation. The [TUI guide](../user/tui-and-sessions.md#images-in-the-transcript) specifies fallback, cache, replay, and local-file semantics. - -## Milestones and PR stack - -| Phase | Branch / PR base | Owner | Scope and completion milestone | -| --- | --- | --- | --- | -| 1 | `feat/compose-files-phase-1` -> `main` | Dedicated phase-1 subagent | Versioned managed storage/reference contract; hidden read_file; compose finalizer; media-aware spill; native provider delivery plus mandatory user-image transport fallback; basic tool-result TUI rendering. `return read_file({ path: "screenshot.png" })` delivers real pixels to model and TUI. | -| 2 | `feat/compose-files-phase-2` -> phase-1 branch | New dedicated phase-2 subagent | Rotate/crop/resize/export, immutable references, bounded transforms, docs/tests. A pipeline works in one compose invocation; only returned files reach parent model. | -| 3 | `feat/compose-files-phase-3` -> phase-2 branch | New dedicated phase-3 subagent | Typed subagent attachments/output, file-aware schema/binding, capability checks, parent/child grants and output promotion. Sticker-editing pipeline is supported end to end. | -| 4 | `feat/compose-files-phase-4` -> phase-3 branch | New dedicated phase-4 subagent | Unified user/tool/assistant/Markdown rendering, resolver policy, live/replay consistency, caching and terminal fallback. All image origins render safely. | - -Each successor worktree is created with `wt` from its predecessor's CURRENT review-clean tip, invoked from the phase-1 worktree; do not fork all phases independently from main. Verify `git merge-base --is-ancestor HEAD`, and set PR base to the parent branch. The plan is inherited through the stack. Changes to a published parent require pausing descendants and asking for restack/history-rewrite approval. - -## Per-phase owner instructions and review gate - -1. Read this plan, current repository instructions, and relevant skills. Reinspect code on current main/parent; the original investigation was on a different dirty worktree. -2. Get independent design consensus before implementation; retain an explicit reviewer session ID in the orchestration ledger, not committed docs. Resolve material disagreements with the same reviewer. -3. Load shared-state skill for lock/shared-state changes, updating-artifact-schema for persistent schema changes, secure-rust-dependency-changes for dependency surface changes, and pr for PR creation. Apply ship-issue/shepherd with stack/no-merge overrides; this is not a Linear task. -4. Implement only your phase, preserving later extensibility. Keep production free of test-only instrumentation. Run smallest useful checks during iteration, then repository-required checks. No release version changes. -5. Self-review for reuse, quality, efficiency; obtain independent current-head review. Open a Conventional Commit PR with correct base, plan link, testing and limitations. -6. KEEP GOING through CI/reviewer findings. Fix valid findings, rerun tests, push follow-up commits, respond/resolve threads, and obtain fresh review evidence for the latest head. Never self-approve or misrepresent absent approval. -7. Stop successfully ONLY when local checks pass, CI passes with no pending/unknown required check, reviewers have approved the current head with no unresolved threads, and GitHub reports conflict-free/mergeable against its current base. A skipped stacked-base workflow is not a pass without repository-approved equivalent current-head evidence. -8. Do not merge, enable auto-merge, enqueue, deploy, or launch a successor yourself. Return the gate evidence to the parent orchestrator. If external approval/auth/infra genuinely blocks progress, report the blocker honestly; parent does not launch next phase. - -The parent launches each successor only after verifying the previous gate. Keep an uncommitted ledger outside tracked plan content with phase, worktree, branch/base/tip, agent/reviewer session IDs, PR URL, CI/check evidence, approval SHA, unresolved threads, mergeability, and blockers. - -## Acceptance test matrix - -- Real provider request contains image input after compose return, natively or in the mandatory request-only user-image fallback after the full tool-result batch. No descriptor/base64 text substituted for pixels. -- Nested returns, multiple references, deduplication, deterministic labels, and intermediate non-delivery. -- Text spill does not hide or corrupt image delivery; bounded traversal and independent byte/pixel budgets. -- Malformed/forged/stale/cross-session references, unsupported versions, missing files, corrupt formats, directories, permissions, oversized/decompression-bomb input. -- Foreground/background completion, interruption/resume, cancellation, replay, storage errors. -- Session resume/fork and child close preserve authorized output independently of original path. -- Transform geometry, format/animation/metadata policies, immutable source, explicit export. -- Subagent capability/contract failures, native output import, ambiguous binding, grant isolation. -- Live and replayed TUI tool/assistant/user images, Markdown policy, async placeholders/fallback, no credentialed arbitrary fetches. - -## Initial status - -Plan persisted before implementation. Phase 1 is next; phases 2–4 are blocked on predecessor review gates. Operational status and agent IDs live in the uncommitted orchestration ledger.