From a6f30fae5609f8bedf3c10bf0f0c04dd35928455 Mon Sep 17 00:00:00 2001 From: renpengfei <244098063@qq.com> Date: Sat, 19 Sep 2026 22:50:47 +0800 Subject: [PATCH 1/4] fix(acp): preserve file attachments in custom-agent history Recorded prompts contain resource/resource_link blocks, but the history projection only kept top-level text and native images. Session/load replay also discarded binary resources and user images. Use one user-content projection for recorded prompts and replay: preserve file markers, promote image resources, and keep mixed chunks in one user turn without duplicating recorded prompt echoes. No transcript/schema or client protocol changes; existing raw records can be reparsed. Validated with all 20 acp_native parser tests, including four regressions. --- src-tauri/src/parsers/acp_native.rs | 234 +++++++++++++++++++++++----- 1 file changed, 195 insertions(+), 39 deletions(-) diff --git a/src-tauri/src/parsers/acp_native.rs b/src-tauri/src/parsers/acp_native.rs index 2a1d7ceb3b..b94212be96 100644 --- a/src-tauri/src/parsers/acp_native.rs +++ b/src-tauri/src/parsers/acp_native.rs @@ -200,45 +200,88 @@ fn prompt_text(payload: &serde_json::Value) -> String { } /// Blocks for a user turn recorded from a `session/prompt` payload. Text and -/// images are kept; resource links degrade to their text form, which is what -/// the composer serialized them from. +/// images are kept; resources use the same lightweight attachment markers as +/// the live user-message projection. The original bytes stay in the transcript. fn prompt_blocks(payload: &serde_json::Value) -> Vec { let Some(items) = payload.as_array() else { return Vec::new(); }; - let mut blocks = Vec::new(); - for item in items { - match item.get("type").and_then(|t| t.as_str()) { - Some("image") => { - let data = item.get("data").and_then(|d| d.as_str()).unwrap_or_default(); - let mime_type = item - .get("mimeType") - .or_else(|| item.get("mime_type")) - .and_then(|m| m.as_str()) - .unwrap_or("image/png"); - if !data.is_empty() { - blocks.push(ContentBlock::Image { - data: data.to_string(), - mime_type: mime_type.to_string(), - uri: item - .get("uri") - .and_then(|u| u.as_str()) - .map(str::to_string), + items.iter().filter_map(user_content_block).collect() +} + +/// Shared by recorded prompts and session/load replay, whose ACP resources nest +/// their URI/MIME/body under `resource` rather than a top-level `text` field. +fn user_content_block(item: &serde_json::Value) -> Option { + match item.get("type").and_then(|t| t.as_str()) { + Some("image") => { + let data = item + .get("data") + .and_then(|d| d.as_str()) + .unwrap_or_default(); + let mime_type = item + .get("mimeType") + .or_else(|| item.get("mime_type")) + .and_then(|m| m.as_str()) + .unwrap_or("image/png"); + if !data.is_empty() { + return Some(ContentBlock::Image { + data: data.to_string(), + mime_type: mime_type.to_string(), + uri: item.get("uri").and_then(|u| u.as_str()).map(str::to_string), + }); + } + } + Some("resource") => { + let resource = item.get("resource")?; + let uri = resource + .get("uri") + .and_then(|v| v.as_str()) + .unwrap_or_default(); + let mime = resource + .get("mimeType") + .or_else(|| resource.get("mime_type")) + .and_then(|v| v.as_str()); + let blob = resource.get("blob").and_then(|v| v.as_str()); + if let (Some(mime), Some(blob)) = (mime, blob) { + if mime.starts_with("image/") && !blob.is_empty() { + return Some(ContentBlock::Image { + data: blob.to_string(), + mime_type: mime.to_string(), + uri: (!uri.is_empty()).then(|| uri.to_string()), }); } } - _ => { - if let Some(text) = item.get("text").and_then(|t| t.as_str()) { - if !text.is_empty() { - blocks.push(ContentBlock::Text { - text: text.to_string(), - }); - } + if !uri.is_empty() { + return Some(ContentBlock::Text { + text: format!("[{uri}]({uri})"), + }); + } + } + Some("resource_link") => { + let uri = item + .get("uri") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty())?; + let name = item + .get("name") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + .unwrap_or(uri); + return Some(ContentBlock::Text { + text: format!("[{name}]({uri})"), + }); + } + _ => { + if let Some(text) = item.get("text").and_then(|t| t.as_str()) { + if !text.is_empty() { + return Some(ContentBlock::Text { + text: text.to_string(), + }); } } } } - blocks + None } /// Accumulated state of one assistant turn under construction. @@ -516,33 +559,42 @@ fn apply_update( if *prompt_just_recorded { return; } - let text = content_block_text(&chunk.content); - if text.is_empty() { + let Ok(content) = serde_json::to_value(&chunk.content) else { return; - } + }; + let Some(block) = user_content_block(&content) else { + return; + }; + // Resource/image chunks append in order to the same user turn; + // incoming text chunks retain the existing text-coalescing behavior. + let is_text = matches!(chunk.content, sacp::schema::ContentBlock::Text(_)); flush(pending, turns, seq); *turn_start_hint = Some(at_ms); match turns.last_mut() { // Consecutive replay chunks belong to one user message. - Some(last) - if matches!(last.role, TurnRole::User) - && matches!(last.blocks.last(), Some(ContentBlock::Text { .. })) => - { - if let Some(ContentBlock::Text { text: existing }) = last.blocks.last_mut() { - existing.push_str(&text); + Some(last) if matches!(last.role, TurnRole::User) => { + if let ( + true, + Some(ContentBlock::Text { text: existing }), + ContentBlock::Text { text }, + ) = (is_text, last.blocks.last_mut(), &block) + { + existing.push_str(text); + } else { + last.blocks.push(block); } } _ => { turns.push(MessageTurn { id: format!("acp-{seq}"), role: TurnRole::User, - blocks: vec![ContentBlock::Text { text }], + blocks: vec![block], timestamp: epoch_ms_to_utc(at_ms), usage: None, duration_ms: None, model: None, completed_at: None, - agent_message_id: None, + agent_message_id: None, }); *seq += 1; } @@ -847,6 +899,110 @@ mod tests { }) } + fn attachment_prompt() -> serde_json::Value { + serde_json::json!([ + {"type":"text", "text":"Review these files"}, + {"type":"resource_link", "name":"report.pdf", "uri":"file:///tmp/report.pdf", "mimeType":"application/pdf"}, + {"type":"resource", "resource":{"uri":"attachment:///note.txt", "mimeType":"text/plain", "text":"private file contents"}}, + {"type":"resource", "resource":{"uri":"attachment:///data.bin", "mimeType":"application/octet-stream", "blob":"c2VjcmV0"}}, + {"type":"resource", "resource":{"uri":"attachment:///plot.png", "mimeType":"image/png", "blob":"aW1hZ2U="}}, + {"type":"image", "data":"bmF0aXZl", "mimeType":"image/jpeg", "uri":"file:///tmp/photo.jpg"} + ]) + } + + #[test] + fn recorded_prompt_preserves_attachment_markers_and_images() { + let turns = project_turns(&[entry(1, EntryKind::Prompt, attachment_prompt())]); + assert_eq!(turns.len(), 1); + let blocks = &turns[0].blocks; + assert_eq!(blocks.len(), 6); + for (index, expected) in [ + (1, "[report.pdf](file:///tmp/report.pdf)"), + (2, "[attachment:///note.txt](attachment:///note.txt)"), + (3, "[attachment:///data.bin](attachment:///data.bin)"), + ] { + assert!(matches!(&blocks[index], ContentBlock::Text { text } if text == expected)); + } + assert!( + matches!(&blocks[4], ContentBlock::Image { data, mime_type, uri } + if data == "aW1hZ2U=" && mime_type == "image/png" && uri.as_deref() == Some("attachment:///plot.png")) + ); + assert!( + matches!(&blocks[5], ContentBlock::Image { data, mime_type, .. } + if data == "bmF0aXZl" && mime_type == "image/jpeg") + ); + let text = prompt_text(&attachment_prompt()); + assert_eq!(text, "Review these files"); + } + + #[test] + fn replayed_attachments_match_recorded_prompt_and_stay_in_one_user_turn() { + let payload = attachment_prompt(); + let mut entries = vec![]; + for (i, content) in payload.as_array().unwrap().iter().enumerate() { + entries.push(update( + i as u64 + 1, + serde_json::json!({ + "sessionUpdate":"user_message_chunk", "content":content + }), + )); + } + entries.push(update(10, text_chunk("user_message_chunk", "after "))); + entries.push(update(11, text_chunk("user_message_chunk", "images"))); + entries.push(update(12, text_chunk("agent_message_chunk", "done"))); + let turns = project_turns(&entries); + assert_eq!(turns.len(), 2); + assert_eq!(turns[0].blocks.len(), 7); + let expected = prompt_blocks(&payload); + assert_eq!( + serde_json::to_value(&turns[0].blocks[..6]).unwrap(), + serde_json::to_value(expected).unwrap() + ); + assert!( + matches!(&turns[0].blocks[6], ContentBlock::Text { text } if text == "after images") + ); + assert!(matches!(turns[1].role, TurnRole::Assistant)); + } + + #[test] + fn recorded_attachment_echoes_do_not_duplicate_user_turns() { + let payload = attachment_prompt(); + let mut entries = vec![entry(1, EntryKind::Prompt, payload.clone())]; + for content in payload.as_array().unwrap() { + entries.push(update( + 2, + serde_json::json!({"sessionUpdate":"user_message_chunk", "content":content}), + )); + } + entries.push(update(3, text_chunk("agent_message_chunk", "done"))); + let turns = project_turns(&entries); + assert_eq!(turns.len(), 2); + assert_eq!(turns[0].blocks.len(), 6); + } + + #[test] + fn attachment_only_history_is_not_empty_and_malformed_resources_are_ignored() { + let blocks = prompt_blocks(&serde_json::json!([ + {"type":"resource_link", "uri":"file:///tmp/unnamed", "name":""}, + {"type":"resource", "resource":{"uri":"attachment:///empty.png", "mimeType":"image/png", "blob":""}}, + {"type":"image", "data":"legacy", "mime_type":"image/jpeg"}, + {"type":"resource"}, + {"type":"resource", "resource":{"blob":"do not expose"}}, + {"type":"resource_link", "name":"missing uri"}, + {"type":"image", "data":""} + ])); + assert_eq!(blocks.len(), 3); + assert!( + matches!(&blocks[0], ContentBlock::Text { text } if text == "[file:///tmp/unnamed](file:///tmp/unnamed)") + ); + assert!( + matches!(&blocks[1], ContentBlock::Text { text } if text == "[attachment:///empty.png](attachment:///empty.png)") + ); + assert!( + matches!(&blocks[2], ContentBlock::Image { mime_type, .. } if mime_type == "image/jpeg") + ); + } + #[test] fn projects_a_simple_two_turn_conversation() { let entries = vec![ From ace77d6784067d808f6a7331684a8635f163e332 Mon Sep 17 00:00:00 2001 From: xintaofei Date: Sun, 20 Sep 2026 11:37:20 +0800 Subject: [PATCH 2/4] fix(acp): keep replayed attachment markers out of the next sentence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The attachment-preserving history projection turns a non-image resource into a `[uri](uri)` marker block. On the `session/load` replay path the text-coalescing rule then appended the NEXT user chunk onto it, so an agent that stores file context before the prose ("here is the file" + "what does this do?") read back as one run-on paragraph with the prose welded onto the end of a markdown link. The live projection (`user_blocks_from_prompt`) emits one block per prompt block, and this parser exists to match it. Coalesce only onto prose — text a previous `user_message_chunk` streamed — so a marker or an image always closes its block. Adds the two regression cases that the change turns on (prose after a marker; an attachment-only chunk ending the previous assistant turn) and a parity test that pins this projection to `user_blocks_from_prompt` through the real `map_prompt_blocks` wire encoding, so the two implementations of one contract cannot drift silently. Co-Authored-By: Claude Opus 5 (1M context) --- src-tauri/src/acp/connection.rs | 7 +- src-tauri/src/parsers/acp_native.rs | 163 +++++++++++++++++++++++++++- 2 files changed, 167 insertions(+), 3 deletions(-) diff --git a/src-tauri/src/acp/connection.rs b/src-tauri/src/acp/connection.rs index c6307bf70f..b315efd7a7 100644 --- a/src-tauri/src/acp/connection.rs +++ b/src-tauri/src/acp/connection.rs @@ -8439,7 +8439,12 @@ fn normalize_grok_image_blocks(blocks: Vec) -> Vec) -> Vec { +/// `pub(crate)` for one reader beyond this module: the ACP-native history +/// parser's parity test, which needs the EXACT wire bytes `record_prompt` +/// writes in order to assert its projection equals the live one +/// ([`crate::acp::types::user_blocks_from_prompt`]). Rebuilding those bytes by +/// hand in the test would let the two drift without failing anything. +pub(crate) fn map_prompt_blocks(blocks: Vec) -> Vec { blocks .into_iter() .map(|block| match block { diff --git a/src-tauri/src/parsers/acp_native.rs b/src-tauri/src/parsers/acp_native.rs index b94212be96..efbf50a9fe 100644 --- a/src-tauri/src/parsers/acp_native.rs +++ b/src-tauri/src/parsers/acp_native.rs @@ -411,6 +411,13 @@ pub fn project_turns(entries: &[TranscriptEntry]) -> Vec { // time-to-first-token is part of how long the agent took, and this matches // what `turn_timings` records for built-ins. let mut turn_start_hint: Option = None; + // True when the last block of the open user turn is PROSE — text a + // `user_message_chunk` streamed — so the next text chunk may be appended to + // it. False after an attachment marker or an image, which are one block per + // prompt block in the live projection and must stay that way here: gluing + // the next chunk's prose onto `[uri](uri)` would render one run-on + // paragraph where the live path renders two. + let mut user_prose_open = false; let mut seq = 0usize; for entry in entries { @@ -432,6 +439,9 @@ pub fn project_turns(entries: &[TranscriptEntry]) -> Vec { seq += 1; prompt_just_recorded = true; turn_start_hint = Some(entry.t); + // A recorded prompt is a COMPLETE message, never a half-streamed + // one, so nothing may be appended to its trailing block. + user_prose_open = false; } EntryKind::TurnEnd => { if let Some(p) = pending.as_mut() { @@ -441,6 +451,7 @@ pub fn project_turns(entries: &[TranscriptEntry]) -> Vec { flush(&mut pending, &mut turns, &mut seq); prompt_just_recorded = false; turn_start_hint = None; + user_prose_open = false; } EntryKind::Update => { // Deserialized from a BORROWED `&Value`, not a cloned one: this @@ -463,6 +474,7 @@ pub fn project_turns(entries: &[TranscriptEntry]) -> Vec { &mut seq, &mut prompt_just_recorded, &mut turn_start_hint, + &mut user_prose_open, ); } } @@ -542,6 +554,7 @@ fn apply_update( seq: &mut usize, prompt_just_recorded: &mut bool, turn_start_hint: &mut Option, + user_prose_open: &mut bool, ) { // Opening a turn consumes the prompt's timestamp, so the turn's span covers // time-to-first-token; without a recorded prompt (replay) it starts here. @@ -566,8 +579,12 @@ fn apply_update( return; }; // Resource/image chunks append in order to the same user turn; - // incoming text chunks retain the existing text-coalescing behavior. + // incoming text chunks retain the existing text-coalescing behavior, + // but only onto prose — an attachment marker is one block per prompt + // block in the live projection, and appending to it would glue the + // next sentence onto the end of a markdown link. let is_text = matches!(chunk.content, sacp::schema::ContentBlock::Text(_)); + let coalesce = is_text && *user_prose_open; flush(pending, turns, seq); *turn_start_hint = Some(at_ms); match turns.last_mut() { @@ -577,7 +594,7 @@ fn apply_update( true, Some(ContentBlock::Text { text: existing }), ContentBlock::Text { text }, - ) = (is_text, last.blocks.last_mut(), &block) + ) = (coalesce, last.blocks.last_mut(), &block) { existing.push_str(text); } else { @@ -599,6 +616,7 @@ fn apply_update( *seq += 1; } } + *user_prose_open = is_text; } SessionUpdate::AgentMessageChunk(chunk) => { *prompt_just_recorded = false; @@ -980,6 +998,147 @@ mod tests { assert_eq!(turns[0].blocks.len(), 6); } + /// A replay whose user message puts the attachment BEFORE the prose (the + /// order an agent that front-loads file context stores it in). The marker is + /// a whole block, so the sentence after it must not be glued onto the end of + /// its markdown link — the live projection emits one block per prompt block, + /// and the transcript has to read the same way. + #[test] + fn replayed_prose_after_an_attachment_marker_stays_its_own_block() { + let entries = vec![ + update( + 1, + serde_json::json!({ + "sessionUpdate":"user_message_chunk", + "content":{"type":"resource", "resource":{ + "uri":"file:///tmp/note.txt", "mimeType":"text/plain", "text":"body" + }} + }), + ), + update(2, text_chunk("user_message_chunk", "what does ")), + update(3, text_chunk("user_message_chunk", "this do?")), + update(4, text_chunk("agent_message_chunk", "reading it")), + ]; + let turns = project_turns(&entries); + assert_eq!(turns.len(), 2); + assert_eq!(turns[0].blocks.len(), 2); + assert!( + matches!(&turns[0].blocks[0], ContentBlock::Text { text } if text == "[file:///tmp/note.txt](file:///tmp/note.txt)") + ); + // The two prose chunks still coalesce with each other. + assert!( + matches!(&turns[0].blocks[1], ContentBlock::Text { text } if text == "what does this do?") + ); + } + + /// The regression the attachment fix turns on: a user message carrying only + /// an attachment used to project to nothing, so it neither ended the + /// previous assistant turn nor appeared at all — the next reply merged into + /// the previous one. + #[test] + fn an_attachment_only_replayed_chunk_ends_the_previous_assistant_turn() { + let entries = vec![ + update(1, text_chunk("agent_message_chunk", "first reply")), + update( + 2, + serde_json::json!({ + "sessionUpdate":"user_message_chunk", + "content":{"type":"resource", "resource":{ + "uri":"file:///tmp/data.bin", + "mimeType":"application/octet-stream", + "blob":"c2VjcmV0" + }} + }), + ), + update(3, text_chunk("agent_message_chunk", "second reply")), + ]; + let turns = project_turns(&entries); + assert_eq!(turns.len(), 3); + assert!(matches!(turns[0].role, TurnRole::Assistant)); + assert!(matches!(turns[1].role, TurnRole::User)); + assert_eq!(turns[1].blocks.len(), 1); + assert!(matches!(turns[2].role, TurnRole::Assistant)); + assert!( + matches!(&turns[2].blocks[0], ContentBlock::Text { text } if text == "second reply") + ); + } + + /// The live user turn is projected by `acp::types::user_blocks_from_prompt`; + /// this parser re-derives the same mapping from the recorded wire bytes. + /// They are two implementations of one contract — a viewer watching live and + /// a reader after a refresh must see the same message — so pin them + /// together over the block shapes codeg's composer actually sends. + #[test] + fn history_projection_matches_the_live_user_message_projection() { + use crate::acp::types::{user_blocks_from_prompt, PromptInputBlock, UserMessageBlock}; + + let sent = vec![ + PromptInputBlock::Text { + text: "Review these files".into(), + }, + PromptInputBlock::ResourceLink { + uri: "file:///tmp/report.pdf".into(), + name: "report.pdf".into(), + mime_type: Some("application/pdf".into()), + description: None, + }, + // A path-less pasted text file: embedded body, synthetic uri. + PromptInputBlock::Resource { + uri: "clipboard://note.txt-1".into(), + mime_type: Some("text/plain".into()), + text: Some("private file contents".into()), + blob: None, + }, + PromptInputBlock::Resource { + uri: "clipboard://data.bin-2".into(), + mime_type: Some("application/octet-stream".into()), + text: None, + blob: Some("c2VjcmV0".into()), + }, + // How an `image: false` / `embedded_context: true` agent carries an + // image — promoted back to a thumbnail on BOTH sides. + PromptInputBlock::Resource { + uri: "clipboard://plot.png-3".into(), + mime_type: Some("image/png".into()), + text: None, + blob: Some("aW1hZ2U=".into()), + }, + PromptInputBlock::Image { + data: "bmF0aXZl".into(), + mime_type: "image/jpeg".into(), + uri: Some("file:///tmp/photo.jpg".into()), + }, + ]; + // Exactly what `record_prompt` writes: the wire blocks `session/prompt` + // carried, serialized. + let recorded = + serde_json::to_value(crate::acp::connection::map_prompt_blocks(sent.clone())) + .expect("wire blocks serialize"); + + // `uri` is dropped on both sides: the broadcast carries an image by its + // bytes alone, so it is not a projection difference to compare. + let live: Vec<(&str, String, String)> = user_blocks_from_prompt(&sent) + .iter() + .map(|b| match b { + UserMessageBlock::Text { text } => ("text", text.clone(), String::new()), + UserMessageBlock::Image { data, mime_type } => { + ("image", data.clone(), mime_type.clone()) + } + }) + .collect(); + let history: Vec<(&str, String, String)> = prompt_blocks(&recorded) + .iter() + .map(|b| match b { + ContentBlock::Text { text } => ("text", text.clone(), String::new()), + ContentBlock::Image { + data, mime_type, .. + } => ("image", data.clone(), mime_type.clone()), + other => panic!("a user turn may only hold text and images, got {other:?}"), + }) + .collect(); + assert_eq!(history, live); + } + #[test] fn attachment_only_history_is_not_empty_and_malformed_resources_are_ignored() { let blocks = prompt_blocks(&serde_json::json!([ From daaed50c36f648861743c74dd405091c590926d4 Mon Sep 17 00:00:00 2001 From: xintaofei Date: Mon, 21 Sep 2026 22:17:31 +0800 Subject: [PATCH 3/4] refactor(acp): decide a user turn's attachments in one place MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three surfaces showed a user's own prompt back and each decided for itself how an attachment looks: the live broadcast (`user_blocks_from_prompt`), the ACP-native history parser, and the grok history parser. They had already drifted — grok had no `resource_link` case at all, so a plain attached file came back from its history as an empty block, the same defect the ACP-native parser just had fixed. Collapse the rule into `acp::types::project_user_prompt_block`, with `prompt_block_from_wire` to read a recorded ACP block back into the `PromptInputBlock` it was sent as. The live path becomes a thin adapter over it (it only drops an image's `uri`, which its carrier cannot hold), and both parsers go through `parsers::user_turn_block_from_wire`. Three things fall out: - The replay path no longer serializes an already-typed content block back to JSON just to re-read it untyped — a round trip this file's own comments say it avoids, and one that copied every embedded image's base64 twice. It now converts typed, moving the payload. - Attachment markers are escaped the way the composer escapes its own `@`-file links, so `file:///a/b (1).ts` or a Windows `file:///C:\dir\` stays a well-formed link instead of closing early and rendering as raw `[…](…)` source. The frontend already parses that form (`src/lib/reference-link.ts`); a test there pins the two escapers to each other across the language boundary. - Grok's user turns keep their file attachments. Also: a prompt that is nothing but attachments is titled after what was attached. ACP has no title channel, so such a conversation was previously untitled forever. Co-Authored-By: Claude Opus 5 (1M context) --- src-tauri/src/acp/types.rs | 388 +++++++++++++++++++++++++--- src-tauri/src/parsers/acp_native.rs | 257 +++++++++++------- src-tauri/src/parsers/grok.rs | 94 +++---- src-tauri/src/parsers/mod.rs | 32 +++ src/lib/reference-link.test.ts | 43 +++ 5 files changed, 639 insertions(+), 175 deletions(-) diff --git a/src-tauri/src/acp/types.rs b/src-tauri/src/acp/types.rs index 17d30bdcd4..0561ae6e70 100644 --- a/src-tauri/src/acp/types.rs +++ b/src-tauri/src/acp/types.rs @@ -780,7 +780,7 @@ pub enum ConfigStaleKind { /// and stored in the live snapshot. Intentionally narrower than /// [`PromptInputBlock`]: only what a viewer needs to render the user turn. /// Non-image `Resource` / `ResourceLink` prompt blocks are folded into `Text` -/// markdown links by [`user_blocks_from_prompt`]; an image-mime embedded +/// markdown links by [`project_user_prompt_block`]; an image-mime embedded /// `Resource` (how an `image:false` / `embedded_context:true` agent carries a /// pasted image — and still how a format the agent cannot decode travels) is /// promoted to `Image` so the viewer renders a thumbnail, not a link. @@ -793,47 +793,233 @@ pub enum UserMessageBlock { Image { data: String, mime_type: String }, } -/// Project the wire `PromptInputBlock`s the sender submitted into the lean -/// [`UserMessageBlock`]s broadcast to viewers: text and images pass through; an -/// image-mime embedded resource is promoted to an `Image`; other -/// resources/resource-links collapse to a `[label](uri)` markdown line so a +/// One prompt block as it appears in a rendered user turn. +/// +/// THE single projection rule, shared by every surface that shows a user's +/// prompt back to somebody: +/// +/// * the live broadcast, [`user_blocks_from_prompt`] → [`UserMessageBlock`]; +/// * the ACP-native history parser, `parsers::acp_native`, reading codeg's own +/// transcript back off disk; +/// * the grok history parser, `parsers::grok`, reading grok's `updates.jsonl`. +/// +/// They render the same conversation at different times, so the rule has to +/// live in exactly one place — a viewer watching live and a reader after a +/// refresh must see the same message. Only the carrier differs: the broadcast +/// cannot hold an image's `uri` ([`UserMessageBlock::Image`] has no such +/// field), the history parsers keep it because the frontend derives an image's +/// display filename from it. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum UserTurnBlock { + Text { + text: String, + }, + Image { + data: String, + mime_type: String, + uri: Option, + }, +} + +/// Apply that rule to one submitted block: text and images pass through; an +/// image-mime embedded resource is promoted to an `Image`; every other +/// resource / resource-link collapses to a `[label](uri)` markdown line so a /// viewer still sees what was attached without shipping blob bytes twice. /// /// Both image carriages therefore render identically, which is what keeps the /// user turn stable no matter which one the prompt ends up taking. +/// +/// Takes the block BY VALUE so the (potentially megabyte-sized base64) payload +/// moves rather than being copied; callers holding a borrow clone once, which +/// is what they did field-by-field before. +pub fn project_user_prompt_block(block: PromptInputBlock) -> UserTurnBlock { + match block { + PromptInputBlock::Text { text } => UserTurnBlock::Text { text }, + PromptInputBlock::Image { + data, + mime_type, + uri, + } => UserTurnBlock::Image { + data, + mime_type, + uri, + }, + // An image-mime embedded resource carries a pasted image for agents + // that reject native image blocks (an `image:false` + + // `embedded_context:true` agent), and for a format the agent cannot + // decode. Promote it to `Image` so viewers render the thumbnail; + // non-image resources still collapse to a link. + PromptInputBlock::Resource { + uri, + mime_type, + blob, + .. + } => match (mime_type, blob) { + (Some(mt), Some(b)) if mt.starts_with("image/") => UserTurnBlock::Image { + data: b, + mime_type: mt, + // A pasted image has no path; `""` would read as a filename of + // nothing rather than as "unnamed". + uri: (!uri.is_empty()).then_some(uri), + }, + _ => UserTurnBlock::Text { + text: attachment_marker(&uri, &uri), + }, + }, + PromptInputBlock::ResourceLink { uri, name, .. } => UserTurnBlock::Text { + text: attachment_marker(&name, &uri), + }, + } +} + +/// Render an attachment as the inline Markdown link the transcript renders back +/// into a file badge / attachment chip. +/// +/// Escaped exactly like the composer's own `referenceToMarkdown`, whose inverse +/// (`src/lib/reference-link.ts`) is what the frontend parses these with: the +/// label is backslash-escaped, and a destination carrying whitespace, brackets +/// or backslashes is wrapped in `<…>`. Without that a perfectly ordinary +/// attachment — `file:///a/b (1).ts`, or a Windows `file:///C:\dir\x` — closed +/// the link early and rendered as raw `[…](…)` source text. +fn attachment_marker(label: &str, uri: &str) -> String { + format!( + "[{}]({})", + escape_markdown_text(label), + escape_link_destination(uri) + ) +} + +/// Backslash-escape every inline-significant ASCII punctuation char, so a label +/// cannot inject Markdown structure (a nested link, emphasis, a code span…). +/// Mirrors `escapeMarkdownText` in `src/components/chat/composer/reference-text.ts`; +/// GFM does not autolink inside link text, so escaping alone is enough here. +fn escape_markdown_text(text: &str) -> String { + let mut out = String::with_capacity(text.len()); + for ch in text.chars() { + if matches!( + ch, + '\\' | '`' | '*' | '_' | '~' | '[' | ']' | '(' | ')' | '<' | '>' + ) { + out.push('\\'); + } + out.push(ch); + } + out +} + +/// Mirrors `escapeLinkDestination` in the same file: newlines are stripped, and +/// a destination containing whitespace, parentheses, angle brackets or a +/// backslash is wrapped in `<…>` with `\`, `<` and `>` escaped inside. Clean +/// URLs stay bare, so the overwhelmingly common case is byte-identical to what +/// this emitted before. +fn escape_link_destination(uri: &str) -> String { + let cleaned: String = uri.chars().filter(|c| *c != '\r' && *c != '\n').collect(); + if !cleaned + .chars() + .any(|c| c.is_whitespace() || matches!(c, '(' | ')' | '<' | '>' | '\\')) + { + return cleaned; + } + let mut out = String::with_capacity(cleaned.len() + 2); + out.push('<'); + for ch in cleaned.chars() { + if matches!(ch, '\\' | '<' | '>') { + out.push('\\'); + } + out.push(ch); + } + out.push('>'); + out +} + +/// Read ONE recorded ACP content block back into the [`PromptInputBlock`] it +/// was sent as, so a history parser can hand it to +/// [`project_user_prompt_block`] instead of re-deriving the rule. +/// +/// The inverse of `connection::map_prompt_blocks`, and deliberately lenient — +/// it reads bytes written by older builds of codeg and by other agents' stores: +/// +/// * `mimeType` **and** legacy snake_case `mime_type`; +/// * an embedded resource's uri/mime/body nested under `resource` (where ACP +/// puts them) rather than at the top level. +/// +/// Returns `None` for a block that has nothing to show — empty prose, an image +/// with no bytes, a resource with neither bytes nor a uri, a malformed record, +/// or a kind with no visual form (audio). A user turn is assembled from what +/// this returns, so a `None` is a block that is genuinely not renderable, not +/// one that is merely unrecognized. +pub fn prompt_block_from_wire(item: &serde_json::Value) -> Option { + let string = |v: &serde_json::Value, key: &str| { + v.get(key) + .and_then(|x| x.as_str()) + .map(str::to_string) + .filter(|s| !s.is_empty()) + }; + let mime = |v: &serde_json::Value| { + v.get("mimeType") + .or_else(|| v.get("mime_type")) + .and_then(|m| m.as_str()) + .map(str::to_string) + .filter(|s| !s.is_empty()) + }; + match item.get("type").and_then(|t| t.as_str()) { + Some("image") => Some(PromptInputBlock::Image { + data: string(item, "data")?, + // ACP requires `mimeType`; a record missing it is old enough that + // png was the only thing codeg ever pasted. + mime_type: mime(item).unwrap_or_else(|| "image/png".to_string()), + uri: string(item, "uri"), + }), + Some("resource") => { + let resource = item.get("resource")?; + let uri = string(resource, "uri"); + let mime_type = mime(resource); + let blob = string(resource, "blob"); + let is_image = mime_type.as_deref().is_some_and(|m| m.starts_with("image/")); + // Nothing to show: no uri to name it by, and no bytes to draw it + // from. (Kept identical to `acp_native::prompt_block_from_content`, + // the typed reader for the same content off the replay channel.) + if uri.is_none() && !(is_image && blob.is_some()) { + return None; + } + Some(PromptInputBlock::Resource { + uri: uri.unwrap_or_default(), + mime_type, + text: string(resource, "text"), + blob, + }) + } + Some("resource_link") => { + let uri = string(item, "uri")?; + Some(PromptInputBlock::ResourceLink { + name: string(item, "name").unwrap_or_else(|| uri.clone()), + uri, + mime_type: mime(item), + description: string(item, "description"), + }) + } + // `text`, and any kind this build does not know: a future block that + // still carries a top-level `text` shows as that text, which is the ACP + // guidance for unknown content. + _ => Some(PromptInputBlock::Text { + text: string(item, "text")?, + }), + } +} + +/// Project the wire `PromptInputBlock`s the sender submitted into the lean +/// [`UserMessageBlock`]s broadcast to viewers. +/// +/// A thin adapter over [`project_user_prompt_block`] — it only drops the image +/// `uri`, which this carrier has nowhere to put (see [`UserTurnBlock`]). pub fn user_blocks_from_prompt(blocks: &[PromptInputBlock]) -> Vec { blocks .iter() - .map(|b| match b { - PromptInputBlock::Text { text } => UserMessageBlock::Text { text: text.clone() }, - PromptInputBlock::Image { + .map(|b| match project_user_prompt_block(b.clone()) { + UserTurnBlock::Text { text } => UserMessageBlock::Text { text }, + UserTurnBlock::Image { data, mime_type, .. - } => UserMessageBlock::Image { - data: data.clone(), - mime_type: mime_type.clone(), - }, - // An image-mime embedded resource carries a pasted image for agents - // that reject native image blocks (an `image:false` + - // `embedded_context:true` agent), and for a format the agent cannot - // decode. Promote it to `Image` so viewers render the thumbnail; - // non-image resources still collapse to a link. - PromptInputBlock::Resource { - uri, - mime_type, - blob, - .. - } => match (mime_type, blob) { - (Some(mt), Some(b)) if mt.starts_with("image/") => UserMessageBlock::Image { - data: b.clone(), - mime_type: mt.clone(), - }, - _ => UserMessageBlock::Text { - text: format!("[{uri}]({uri})"), - }, - }, - PromptInputBlock::ResourceLink { uri, name, .. } => UserMessageBlock::Text { - text: format!("[{name}]({uri})"), - }, + } => UserMessageBlock::Image { data, mime_type }, }) .collect() } @@ -1686,4 +1872,138 @@ mod envelope_tests { ] ); } + + /// A file name is not a safe Markdown fragment. Unescaped, a space or a + /// `)` closed the link early and the whole marker showed up as raw source + /// text; a crafted one could have opened a second link. The escaping is + /// the exact inverse of `src/lib/reference-link.ts`, which is what parses + /// these back out on the way to the screen. + #[test] + fn attachment_markers_escape_their_label_and_destination() { + let blocks = vec![ + // A space and parentheses in the path — an everyday download. + PromptInputBlock::ResourceLink { + uri: "file:///a/b (1).ts".into(), + name: "b (1).ts".into(), + mime_type: None, + description: None, + }, + // A Windows path: the trailing backslash would escape the `)`. + PromptInputBlock::ResourceLink { + uri: "file:///C:\\dir\\".into(), + name: "dir".into(), + mime_type: None, + description: None, + }, + // A name that tries to inject a second link. + PromptInputBlock::ResourceLink { + uri: "file:///a/x.ts".into(), + name: "](http://evil) [pwn".into(), + mime_type: None, + description: None, + }, + // The label of a bare resource IS its uri, so it is escaped too. + PromptInputBlock::Resource { + uri: "clipboard://a (b).txt".into(), + mime_type: Some("text/plain".into()), + text: Some("x".into()), + blob: None, + }, + ]; + assert_eq!( + user_blocks_from_prompt(&blocks), + vec![ + UserMessageBlock::Text { + text: "[b \\(1\\).ts]()".into(), + }, + UserMessageBlock::Text { + text: "[dir]()".into(), + }, + UserMessageBlock::Text { + text: "[\\]\\(http://evil\\) \\[pwn](file:///a/x.ts)".into(), + }, + UserMessageBlock::Text { + text: "[clipboard://a \\(b\\).txt]()".into(), + }, + ] + ); + } + + /// The history parsers rebuild a `PromptInputBlock` from the ACP bytes on + /// disk so they can reuse [`project_user_prompt_block`] rather than + /// re-deriving it. That only holds if reading back what + /// `connection::map_prompt_blocks` wrote returns the same block. + #[test] + fn wire_blocks_read_back_as_the_prompt_blocks_they_were_sent_as() { + let sent = vec![ + PromptInputBlock::Text { + text: "hi".into(), + }, + PromptInputBlock::Image { + data: "QUJD".into(), + mime_type: "image/jpeg".into(), + uri: Some("file:///a/photo.jpg".into()), + }, + PromptInputBlock::Resource { + uri: "clipboard://notes.txt".into(), + mime_type: Some("text/plain".into()), + text: Some("note".into()), + blob: None, + }, + PromptInputBlock::Resource { + uri: "clipboard://img.png".into(), + mime_type: Some("image/png".into()), + text: None, + blob: Some("QUJD".into()), + }, + PromptInputBlock::ResourceLink { + uri: "file:///a/app.ts".into(), + name: "app.ts".into(), + mime_type: Some("text/x-typescript".into()), + description: None, + }, + ]; + let wire = serde_json::to_value(crate::acp::connection::map_prompt_blocks(sent.clone())) + .expect("wire blocks serialize"); + let read: Vec = wire + .as_array() + .expect("an array of blocks") + .iter() + .map(|b| prompt_block_from_wire(b).expect("every block above is renderable")) + .collect(); + assert_eq!(read, sent); + } + + /// Legacy and hostile records a transcript can hold. A block with nothing + /// to show is dropped rather than rendered as an empty bubble. + #[test] + fn wire_reader_tolerates_legacy_fields_and_drops_unrenderable_blocks() { + let legacy = serde_json::json!({ + "type": "image", "data": "QUJD", "mime_type": "image/jpeg" + }); + assert_eq!( + prompt_block_from_wire(&legacy), + Some(PromptInputBlock::Image { + data: "QUJD".into(), + mime_type: "image/jpeg".into(), + uri: None, + }) + ); + for unrenderable in [ + serde_json::json!({"type": "text", "text": ""}), + serde_json::json!({"type": "image", "data": ""}), + serde_json::json!({"type": "resource"}), + serde_json::json!({"type": "resource", "resource": {"blob": "QUJD"}}), + serde_json::json!({"type": "resource_link", "name": "no uri"}), + serde_json::json!({"type": "audio", "data": "QUJD", "mimeType": "audio/wav"}), + ] { + assert_eq!(prompt_block_from_wire(&unrenderable), None, "{unrenderable}"); + } + // An image resource is renderable on its bytes alone, uri or not. + assert!(prompt_block_from_wire(&serde_json::json!({ + "type": "resource", + "resource": {"blob": "QUJD", "mimeType": "image/png"} + })) + .is_some()); + } } diff --git a/src-tauri/src/parsers/acp_native.rs b/src-tauri/src/parsers/acp_native.rs index efbf50a9fe..5ef72bf633 100644 --- a/src-tauri/src/parsers/acp_native.rs +++ b/src-tauri/src/parsers/acp_native.rs @@ -41,11 +41,12 @@ use crate::acp::connection::{ extract_tool_call_images, json_value_to_text, serialize_tool_call_content, synthesize_edit_input_from_diffs, }; +use crate::acp::types::{prompt_block_from_wire, PromptInputBlock}; use crate::acp_transcript::{self, EntryKind, Transcript, TranscriptEntry}; use crate::models::agent::AgentType; use crate::models::conversation::{ConversationDetail, ConversationSummary, SessionStats}; use crate::models::message::{ContentBlock, ImageData, MessageTurn, TurnRole, TurnUsage}; -use crate::parsers::{AgentParser, ParseError}; +use crate::parsers::{user_turn_block, user_turn_block_from_wire, AgentParser, ParseError}; pub struct AcpNativeParser { agent_type: AgentType, @@ -185,103 +186,133 @@ fn first_prompt_title(entries: &[TranscriptEntry]) -> Option { Some(crate::parsers::truncate_str(trimmed, 80)) } -/// Concatenate the text of a recorded `session/prompt` content-block array. +/// A title for a recorded `session/prompt` payload: its prose, or — when the +/// user sent nothing but attachments — what they attached. +/// +/// ACP has no title channel, so this string is the only name the conversation +/// will ever have (codeg's DB-side auto-title backfill reads it from here). A +/// message that is one dropped-in file used to yield the empty string and leave +/// the row permanently untitled, so the attachment names stand in — the file +/// names, not the `[uri](uri)` markers the turn itself renders, which would be +/// unreadable as a title. fn prompt_text(payload: &serde_json::Value) -> String { - payload - .as_array() - .map(|blocks| { - blocks - .iter() - .filter_map(|b| b.get("text").and_then(|t| t.as_str())) - .collect::>() - .join("") + let Some(items) = payload.as_array() else { + return String::new(); + }; + let prose: String = items + .iter() + .filter_map(|b| b.get("text").and_then(|t| t.as_str())) + .collect(); + if !prose.trim().is_empty() { + return prose; + } + items + .iter() + .filter_map(prompt_block_from_wire) + .filter_map(|b| match b { + PromptInputBlock::ResourceLink { name, .. } => Some(name), + PromptInputBlock::Resource { uri, .. } => Some(attachment_name(&uri)), + PromptInputBlock::Image { uri, .. } => uri.as_deref().map(attachment_name), + PromptInputBlock::Text { .. } => None, }) - .unwrap_or_default() + .collect::>() + .join(", ") +} + +/// The human name of an attachment uri: its last path segment, percent-decoded +/// where that is unambiguous. Falls back to the whole uri when there is no +/// segment to take (`clipboard://`, a bare scheme), which is still better than +/// nothing in a conversation list. +fn attachment_name(uri: &str) -> String { + let trimmed = uri + .split(['?', '#']) + .next() + .unwrap_or(uri) + .trim_end_matches('/'); + let segment = trimmed.rsplit(['/', '\\']).next().unwrap_or(""); + let candidate = if segment.is_empty() { trimmed } else { segment }; + if candidate.is_empty() { + return uri.to_string(); + } + percent_decode(candidate) +} + +/// Decode `%XX` escapes, leaving any malformed escape exactly as written — +/// a name is for reading, so a half-encoded one must not lose characters. +fn percent_decode(s: &str) -> String { + let bytes = s.as_bytes(); + let mut out: Vec = Vec::with_capacity(bytes.len()); + let mut i = 0; + while i < bytes.len() { + if bytes[i] == b'%' && i + 2 < bytes.len() { + let hex = std::str::from_utf8(&bytes[i + 1..i + 3]).ok(); + if let Some(byte) = hex.and_then(|h| u8::from_str_radix(h, 16).ok()) { + out.push(byte); + i += 3; + continue; + } + } + out.push(bytes[i]); + i += 1; + } + String::from_utf8(out).unwrap_or_else(|_| s.to_string()) } -/// Blocks for a user turn recorded from a `session/prompt` payload. Text and -/// images are kept; resources use the same lightweight attachment markers as -/// the live user-message projection. The original bytes stay in the transcript. +/// Blocks for a user turn recorded from a `session/prompt` payload. fn prompt_blocks(payload: &serde_json::Value) -> Vec { let Some(items) = payload.as_array() else { return Vec::new(); }; - items.iter().filter_map(user_content_block).collect() + items.iter().filter_map(user_turn_block_from_wire).collect() } -/// Shared by recorded prompts and session/load replay, whose ACP resources nest -/// their URI/MIME/body under `resource` rather than a top-level `text` field. -fn user_content_block(item: &serde_json::Value) -> Option { - match item.get("type").and_then(|t| t.as_str()) { - Some("image") => { - let data = item - .get("data") - .and_then(|d| d.as_str()) - .unwrap_or_default(); - let mime_type = item - .get("mimeType") - .or_else(|| item.get("mime_type")) - .and_then(|m| m.as_str()) - .unwrap_or("image/png"); - if !data.is_empty() { - return Some(ContentBlock::Image { - data: data.to_string(), - mime_type: mime_type.to_string(), - uri: item.get("uri").and_then(|u| u.as_str()).map(str::to_string), - }); - } - } - Some("resource") => { - let resource = item.get("resource")?; - let uri = resource - .get("uri") - .and_then(|v| v.as_str()) - .unwrap_or_default(); - let mime = resource - .get("mimeType") - .or_else(|| resource.get("mime_type")) - .and_then(|v| v.as_str()); - let blob = resource.get("blob").and_then(|v| v.as_str()); - if let (Some(mime), Some(blob)) = (mime, blob) { - if mime.starts_with("image/") && !blob.is_empty() { - return Some(ContentBlock::Image { - data: blob.to_string(), - mime_type: mime.to_string(), - uri: (!uri.is_empty()).then(|| uri.to_string()), - }); - } - } - if !uri.is_empty() { - return Some(ContentBlock::Text { - text: format!("[{uri}]({uri})"), - }); - } - } - Some("resource_link") => { - let uri = item - .get("uri") - .and_then(|v| v.as_str()) - .filter(|s| !s.is_empty())?; - let name = item - .get("name") - .and_then(|v| v.as_str()) - .filter(|s| !s.is_empty()) - .unwrap_or(uri); - return Some(ContentBlock::Text { - text: format!("[{name}]({uri})"), - }); +/// The replayed counterpart of [`prompt_block_from_wire`]: a `session/load` +/// chunk arrives already deserialized, so it is converted straight across +/// rather than being serialized back to JSON only to be re-read. Consumes the +/// block, so an embedded image's base64 moves instead of being copied twice. +fn prompt_block_from_content(content: sacp::schema::ContentBlock) -> Option { + use sacp::schema::{ContentBlock as Wire, EmbeddedResourceResource as Res}; + let non_empty = |s: String| (!s.is_empty()).then_some(s); + match content { + Wire::Text(t) => Some(PromptInputBlock::Text { + text: non_empty(t.text)?, + }), + Wire::Image(i) => Some(PromptInputBlock::Image { + data: non_empty(i.data)?, + mime_type: i.mime_type, + uri: i.uri.and_then(non_empty), + }), + Wire::ResourceLink(l) => { + let uri = non_empty(l.uri)?; + Some(PromptInputBlock::ResourceLink { + name: non_empty(l.name).unwrap_or_else(|| uri.clone()), + uri, + mime_type: l.mime_type, + description: l.description, + }) } - _ => { - if let Some(text) = item.get("text").and_then(|t| t.as_str()) { - if !text.is_empty() { - return Some(ContentBlock::Text { - text: text.to_string(), - }); - } + Wire::Resource(r) => { + let (uri, mime_type, text, blob) = match r.resource { + Res::TextResourceContents(t) => (t.uri, t.mime_type, non_empty(t.text), None), + Res::BlobResourceContents(b) => (b.uri, b.mime_type, None, non_empty(b.blob)), + // A shape this build does not know: it has no uri to show and + // no bytes this can render. + _ => return None, + }; + let is_image = mime_type.as_deref().is_some_and(|m| m.starts_with("image/")); + if uri.is_empty() && !(is_image && blob.is_some()) { + return None; } + Some(PromptInputBlock::Resource { + uri, + mime_type, + text, + blob, + }) } + // Audio, and any kind a newer schema adds: nothing to render. + _ => None, } - None } /// Accumulated state of one assistant turn under construction. @@ -572,10 +603,7 @@ fn apply_update( if *prompt_just_recorded { return; } - let Ok(content) = serde_json::to_value(&chunk.content) else { - return; - }; - let Some(block) = user_content_block(&content) else { + let Some(input) = prompt_block_from_content(chunk.content) else { return; }; // Resource/image chunks append in order to the same user turn; @@ -583,7 +611,8 @@ fn apply_update( // but only onto prose — an attachment marker is one block per prompt // block in the live projection, and appending to it would glue the // next sentence onto the end of a markdown link. - let is_text = matches!(chunk.content, sacp::schema::ContentBlock::Text(_)); + let is_text = matches!(input, PromptInputBlock::Text { .. }); + let block = user_turn_block(input); let coalesce = is_text && *user_prose_open; flush(pending, turns, seq); *turn_start_hint = Some(at_ms); @@ -998,6 +1027,56 @@ mod tests { assert_eq!(turns[0].blocks.len(), 6); } + /// ACP has no title channel, so a message that is nothing but a dropped-in + /// file used to leave its conversation permanently untitled. The attachment + /// names stand in — readable, unlike the `[uri](uri)` markers the turn + /// itself renders. + #[test] + fn an_attachment_only_prompt_is_titled_after_what_was_attached() { + let named = serde_json::json!([ + {"type":"resource_link", "name":"report.pdf", "uri":"file:///tmp/report.pdf"}, + {"type":"resource", "resource":{ + "uri":"clipboard://my%20notes.txt-9f2", "mimeType":"text/plain", "text":"body" + }} + ]); + let entries = [entry(1, EntryKind::Prompt, named)]; + assert_eq!( + first_prompt_title(&entries).as_deref(), + Some("report.pdf, my notes.txt-9f2") + ); + + // Prose still wins outright, attachments and all. + assert_eq!( + first_prompt_title(&[entry(1, EntryKind::Prompt, attachment_prompt())]).as_deref(), + Some("Review these files") + ); + + // A prompt with neither is still untitled rather than named "". + assert_eq!( + first_prompt_title(&[entry(1, EntryKind::Prompt, serde_json::json!([])),]), + None + ); + } + + /// An everyday file name is not a safe Markdown fragment. The marker is + /// escaped the way the composer escapes its own `@`-file links, so the + /// frontend's reference-link parser recovers the real path instead of + /// showing raw `[…](…)` source with the link broken at the first space. + #[test] + fn markers_for_awkward_paths_stay_well_formed_links() { + let blocks = prompt_blocks(&serde_json::json!([ + {"type":"resource_link", "name":"b (1).ts", "uri":"file:///a/b (1).ts"}, + {"type":"resource", "resource":{"uri":"file:///a/c).ts", "mimeType":"text/plain"}} + ])); + assert_eq!(blocks.len(), 2); + assert!( + matches!(&blocks[0], ContentBlock::Text { text } if text == "[b \\(1\\).ts]()") + ); + assert!( + matches!(&blocks[1], ContentBlock::Text { text } if text == "[file:///a/c\\).ts]()") + ); + } + /// A replay whose user message puts the attachment BEFORE the prose (the /// order an agent that front-loads file context stores it in). The marker is /// a whole block, so the sentence after it must not be glued onto the end of diff --git a/src-tauri/src/parsers/grok.rs b/src-tauri/src/parsers/grok.rs index e2da3cece7..ad69e88cad 100644 --- a/src-tauri/src/parsers/grok.rs +++ b/src-tauri/src/parsers/grok.rs @@ -1025,59 +1025,19 @@ fn update_text(update: &Value) -> String { /// Grok sends prose as `{type:"text"}`. Current codeg prompts send a native /// `{type:"image"}` chunk (so grok's describe sidecar runs). Older transcripts /// still carry the embedded `{type:"resource", resource:{blob, mimeType, uri}}` -/// shape from when we followed grok's `image:false` advertisement. Both -/// image-mime forms become [`ContentBlock::Image`] so they render as a -/// thumbnail; a non-image embedded resource folds to a `[uri](uri)` link -/// (same as the live [`crate::acp::user_blocks_from_prompt`]). Anything else -/// falls back to a (possibly empty) text block. +/// shape from when we followed grok's `image:false` advertisement. +/// +/// All of it goes through [`crate::parsers::user_turn_block_from_wire`], the +/// one projection every surface uses for a user's own message — so both image +/// carriages become a thumbnail, and an attached file becomes the same +/// `[name](uri)` marker a viewer saw live. This parser used to fold resources +/// itself and had no `resource_link` case at all, which silently dropped +/// plain file attachments from a reloaded grok turn. +/// +/// `None` for a chunk with nothing to render (empty prose, a malformed +/// resource): the caller still opens the user turn, it just starts empty. fn user_chunk_to_block(update: &Value) -> Option { - let content = update.get("content")?; - match content.get("type").and_then(Value::as_str).unwrap_or("") { - "resource" => { - let resource = content.get("resource")?; - let mime = resource.get("mimeType").and_then(Value::as_str); - let blob = resource.get("blob").and_then(Value::as_str); - match (mime, blob) { - (Some(mime), Some(blob)) if mime.starts_with("image/") => { - Some(ContentBlock::Image { - data: blob.to_string(), - mime_type: mime.to_string(), - uri: resource - .get("uri") - .and_then(Value::as_str) - .map(str::to_string), - }) - } - _ => { - let uri = resource.get("uri").and_then(Value::as_str).unwrap_or(""); - Some(ContentBlock::Text { - text: format!("[{uri}]({uri})"), - }) - } - } - } - // Native ACP image content — the live send path for every grok that - // decodes the format (see `normalize_grok_image_blocks`). - "image" => { - let data = content.get("data").and_then(Value::as_str)?; - Some(ContentBlock::Image { - data: data.to_string(), - mime_type: content - .get("mimeType") - .and_then(Value::as_str) - .unwrap_or("image/png") - .to_string(), - uri: content - .get("uri") - .and_then(Value::as_str) - .map(str::to_string), - }) - } - // "text" and unknown kinds: existing behavior (reads `/content/text`). - _ => Some(ContentBlock::Text { - text: update_text(update), - }), - } + crate::parsers::user_turn_block_from_wire(update.get("content")?) } // --------------------------------------------------------------------------- @@ -2215,6 +2175,36 @@ mod tests { assert!(matches!(turns[1].role, TurnRole::Assistant)); } + /// A plain (non-image) file attachment. This parser folded resources + /// itself and had no `resource_link` case at all, so an attached file came + /// back from history as an empty block — the same gap `acp_native` had. + /// Both now go through the one projection a viewer saw live. + #[test] + fn plain_file_attachments_survive_a_reload_as_their_markers() { + let updates = concat!( + r#"{"method":"session/update","params":{"sessionId":"s","update":{"sessionUpdate":"user_message_chunk","content":{"type":"text","text":"看看"},"_meta":{"promptIndex":0}}},"timestamp":1783584019}"#, "\n", + r#"{"method":"session/update","params":{"sessionId":"s","update":{"sessionUpdate":"user_message_chunk","content":{"type":"resource_link","name":"report.pdf","uri":"file:///tmp/report.pdf"},"_meta":{"promptIndex":0}}},"timestamp":1783584019}"#, "\n", + r#"{"method":"session/update","params":{"sessionId":"s","update":{"sessionUpdate":"user_message_chunk","content":{"type":"resource","resource":{"text":"body","mimeType":"text/plain","uri":"clipboard://notes.txt-1"}},"_meta":{"promptIndex":0}}},"timestamp":1783584019}"#, "\n", + r#"{"method":"session/update","params":{"sessionId":"s","update":{"sessionUpdate":"agent_message_chunk","content":{"type":"text","text":"读了"}}},"timestamp":1783584024}"#, "\n", + r#"{"method":"session/update","params":{"sessionId":"s","update":{"sessionUpdate":"turn_completed","stop_reason":"end_turn"}},"timestamp":1783584024}"#, "\n", + ); + let (_tmp, sessions) = fixture(SUMMARY, updates); + let parser = GrokParser::with_base_dir(sessions); + let detail = parser + .get_conversation("019f45e3-e1ef-7690-a29f-fe2554382b49") + .unwrap(); + let turns = &detail.turns; + assert_eq!(turns.len(), 2); + assert_eq!(turns[0].blocks.len(), 3); + assert!(matches!(&turns[0].blocks[0], ContentBlock::Text { text } if text == "看看")); + assert!( + matches!(&turns[0].blocks[1], ContentBlock::Text { text } if text == "[report.pdf](file:///tmp/report.pdf)") + ); + assert!( + matches!(&turns[0].blocks[2], ContentBlock::Text { text } if text == "[clipboard://notes.txt-1](clipboard://notes.txt-1)") + ); + } + /// One turn whose stats live where Grok really puts them: model in /// `update._meta.modelId`, occupancy `totalTokens` and timing in the OUTER /// `params._meta`. Shared by the context-ring tests below. diff --git a/src-tauri/src/parsers/mod.rs b/src-tauri/src/parsers/mod.rs index 38d391e1c1..211622d5a5 100644 --- a/src-tauri/src/parsers/mod.rs +++ b/src-tauri/src/parsers/mod.rs @@ -677,6 +677,38 @@ pub fn title_from_user_text(text: &str) -> String { truncate_str(&fold_reference_links(text), 100) } +/// Widen one projected prompt block into a rendered turn's block type. +/// +/// The projection itself is [`crate::acp::types::project_user_prompt_block`] — +/// the SINGLE rule shared with the live broadcast. This only carries the result +/// across into `models::message`, keeping the image `uri` that the live wire +/// type has nowhere to put but the frontend uses for an image's display name. +pub fn user_turn_block(block: crate::acp::types::PromptInputBlock) -> ContentBlock { + match crate::acp::types::project_user_prompt_block(block) { + crate::acp::types::UserTurnBlock::Text { text } => ContentBlock::Text { text }, + crate::acp::types::UserTurnBlock::Image { + data, + mime_type, + uri, + } => ContentBlock::Image { + data, + mime_type, + uri, + }, + } +} + +/// Read one recorded ACP content block off disk and project it the way the +/// live path projects the same prompt. `None` when the block has nothing to +/// render — see [`crate::acp::types::prompt_block_from_wire`]. +/// +/// Every history parser that reconstructs a user turn from raw ACP content +/// goes through here, so "how an attachment appears in a user message" is +/// decided once rather than per agent. +pub fn user_turn_block_from_wire(item: &serde_json::Value) -> Option { + crate::acp::types::prompt_block_from_wire(item).map(user_turn_block) +} + /// Fill in `duration_ms` for assistant turns whose agent reports no timing of /// its own, by *tiling* the conversation timeline: a reply took as long as the /// span between the end of the previous activity and its own completion. diff --git a/src/lib/reference-link.test.ts b/src/lib/reference-link.test.ts index 4a915a7549..5986193fc6 100644 --- a/src/lib/reference-link.test.ts +++ b/src/lib/reference-link.test.ts @@ -262,6 +262,49 @@ describe("foldReferenceLinks", () => { }) }) +// The attachment markers in a user turn are written by Rust +// (`acp::types::attachment_marker`, shared by the live broadcast and every +// history parser) and parsed back here. The two escapers live in different +// languages, so pin the boundary: these are the exact strings the backend +// emits — see `attachment_markers_escape_their_label_and_destination` in +// src-tauri/src/acp/types.rs, which asserts the same values from the other +// side. +describe("backend attachment markers", () => { + const cases: Array<[string, string, string]> = [ + // [emitted by Rust, recovered label, recovered uri] + ["[b \\(1\\).ts]()", "b (1).ts", "file:///a/b (1).ts"], + ["[dir]()", "dir", "file:///C:\\dir\\"], + [ + "[\\]\\(http://evil\\) \\[pwn](file:///a/x.ts)", + "](http://evil) [pwn", + "file:///a/x.ts", + ], + [ + "[clipboard://a \\(b\\).txt]()", + "clipboard://a (b).txt", + "clipboard://a (b).txt", + ], + // The common case is bare and unescaped, byte for byte as before. + ["[app.ts](file:///a/app.ts)", "app.ts", "file:///a/app.ts"], + ] + + it.each(cases)("round-trips %s", (emitted, label, uri) => { + const tokens = tokenizeReferenceLinks(emitted) + expect(tokens).toHaveLength(1) + const token = tokens[0] + if (token.type !== "link") throw new Error("expected one link token") + expect(token.raw).toBe(emitted) + expect(unescapeReferenceLabel(token.label)).toBe(label) + expect(unwrapReferenceDestination(token.destination)).toBe(uri) + }) + + it("folds each marker to the readable name", () => { + expect(cases.map(([emitted]) => foldReferenceLinks(emitted))).toEqual( + cases.map(([, label]) => label) + ) + }) +}) + describe("buildFileUriWithRange", () => { it("returns the plain file uri when no range is given", () => { expect(buildFileUriWithRange("/repo/src/app.ts")).toBe( From da7b3feb31bc0752917eb812013904f7ceb3c522 Mon Sep 17 00:00:00 2001 From: xintaofei Date: Mon, 21 Sep 2026 23:05:02 +0800 Subject: [PATCH 4/4] fix(acp): name an attachment-only chat by seeding, not by overruling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The parsed title is authoritative: `get_folder_conversation_core` hands it to `refresh_auto_title`, which replaces any unlocked title the row holds — including one a custom ACP agent published over `session_info_update`. So returning an attachment filename from the history parse meant "Quarterly report analysis" could be overwritten with "report.pdf" on the next detail load, where before the parse returned nothing and left the agent's name alone. Put the attachment name where it is a fallback rather than a verdict: the first-prompt seed in `acp::manager`, which reaches `seed_auto_title_if_empty` and a fresh row's `title` and nothing else. A row with no title at all gets named after its file; a row that already has one keeps it. The parse goes back to prose-only. Also from the same review pass: - grok latched `first_user_text` off the projected block, so an attachment ahead of the prose would title the chat `[report.pdf](…)`. It now reads the block as sent and latches only on real prose. - `project_user_prompt_block` borrows instead of consuming. Taking it by value made `user_blocks_from_prompt` clone a whole embedded resource — body and all — only to render its uri. It now clones just the fields the projection keeps, which is what it cost before. - The raw wire reader no longer drops an empty `mimeType`, so it agrees with the typed replay reader on that input too (asserted). - `escape_markdown_text` collapses newline runs the way its TypeScript original does, so a label with a line break stays one inline token. Co-Authored-By: Claude Opus 5 (1M context) --- src-tauri/src/acp/manager.rs | 63 ++++++++++-- src-tauri/src/acp/types.rs | 126 +++++++++++++++++++---- src-tauri/src/parsers/acp_native.rs | 153 ++++++++++++---------------- src-tauri/src/parsers/grok.rs | 20 ++-- src-tauri/src/parsers/mod.rs | 4 +- 5 files changed, 240 insertions(+), 126 deletions(-) diff --git a/src-tauri/src/acp/manager.rs b/src-tauri/src/acp/manager.rs index a8c5d65fe2..b643d6532b 100644 --- a/src-tauri/src/acp/manager.rs +++ b/src-tauri/src/acp/manager.rs @@ -250,10 +250,15 @@ fn user_prompt_text_preview(blocks: &[PromptInputBlock]) -> Option { /// delegating prompt's text blocks (the sub-agent's task). Uses the parser's own /// `title_from_user_text` (folds reference links, caps at 100 chars) so the value /// matches what `refresh_auto_title` would later compute from that same first -/// turn — the conditional UPDATE then sees no change and doesn't churn. Returns -/// `None` for a textless prompt, leaving the title unset to be backfilled on -/// first detail load as before. Kept unlocked by the caller so an AI-generated -/// title can still replace it later. +/// turn — the conditional UPDATE then sees no change and doesn't churn. Kept +/// unlocked by the caller so an AI-generated title can still replace it later. +/// +/// A prompt with no prose at all — one dropped-in file and nothing else — is +/// named after what it carries instead. That row would otherwise read +/// "Untitled" forever: for an agent with no store parser, ACP has no title +/// channel and the history parse honestly reports no title. This value only +/// ever reaches `seed_auto_title_if_empty` / a fresh row's `title`, never +/// `refresh_auto_title`, so it cannot displace a name the agent published. fn delegation_child_title_seed(blocks: &[PromptInputBlock]) -> Option { let joined = blocks .iter() @@ -267,11 +272,11 @@ fn delegation_child_title_seed(blocks: &[PromptInputBlock]) -> Option { .collect::>() .join(" "); let trimmed = joined.trim(); - if trimmed.is_empty() { - None - } else { - Some(crate::parsers::title_from_user_text(trimmed)) + if !trimmed.is_empty() { + return Some(crate::parsers::title_from_user_text(trimmed)); } + crate::acp::types::attachment_names_from_prompt(blocks) + .map(|names| crate::parsers::title_from_user_text(&names)) } /// Composite key identifying a logical agent session for spawn-time dedup. @@ -6082,6 +6087,48 @@ mod tests { assert!(delegation_child_title_seed(&img).is_none()); } + /// A message that is one dropped-in file and no prose. The row would read + /// "Untitled" forever otherwise — for an agent with no store parser, ACP + /// has no title channel and the history parse honestly reports none. The + /// value is a SEED: it reaches `seed_auto_title_if_empty` / a fresh row's + /// `title`, never `refresh_auto_title`, so a name the agent publishes over + /// `session_info_update` still wins. + #[test] + fn delegation_child_title_seed_names_an_attachment_only_prompt() { + let blocks = vec![ + PromptInputBlock::ResourceLink { + uri: "file:///tmp/report.pdf".into(), + name: "report.pdf".into(), + mime_type: None, + description: None, + }, + PromptInputBlock::Resource { + uri: "clipboard://my%20notes.txt-9f2".into(), + mime_type: Some("text/plain".into()), + text: Some("body".into()), + blob: None, + }, + ]; + assert_eq!( + delegation_child_title_seed(&blocks).as_deref(), + Some("report.pdf, my notes.txt-9f2") + ); + + // Prose still wins outright, attachments and all — and matches what + // `refresh_auto_title` will later compute, so the row doesn't churn. + let mut with_prose = blocks.clone(); + with_prose.insert( + 0, + PromptInputBlock::Text { + text: "Review these".into(), + }, + ); + assert_eq!( + delegation_child_title_seed(&with_prose), + Some(crate::parsers::title_from_user_text("Review these")) + ); + } + #[test] fn delegation_child_title_seed_caps_long_task_text() { // Mirrors the parser cap (100 chars) so an over-long task doesn't store a diff --git a/src-tauri/src/acp/types.rs b/src-tauri/src/acp/types.rs index 0561ae6e70..93622ec8aa 100644 --- a/src-tauri/src/acp/types.rs +++ b/src-tauri/src/acp/types.rs @@ -829,20 +829,20 @@ pub enum UserTurnBlock { /// Both image carriages therefore render identically, which is what keeps the /// user turn stable no matter which one the prompt ends up taking. /// -/// Takes the block BY VALUE so the (potentially megabyte-sized base64) payload -/// moves rather than being copied; callers holding a borrow clone once, which -/// is what they did field-by-field before. -pub fn project_user_prompt_block(block: PromptInputBlock) -> UserTurnBlock { +/// Borrows, and clones only the fields the projection KEEPS — a non-image +/// embedded resource becomes its uri, so its (potentially megabyte-sized) +/// body is never copied just to be thrown away. +pub fn project_user_prompt_block(block: &PromptInputBlock) -> UserTurnBlock { match block { - PromptInputBlock::Text { text } => UserTurnBlock::Text { text }, + PromptInputBlock::Text { text } => UserTurnBlock::Text { text: text.clone() }, PromptInputBlock::Image { data, mime_type, uri, } => UserTurnBlock::Image { - data, - mime_type, - uri, + data: data.clone(), + mime_type: mime_type.clone(), + uri: uri.clone(), }, // An image-mime embedded resource carries a pasted image for agents // that reject native image blocks (an `image:false` + @@ -856,22 +856,88 @@ pub fn project_user_prompt_block(block: PromptInputBlock) -> UserTurnBlock { .. } => match (mime_type, blob) { (Some(mt), Some(b)) if mt.starts_with("image/") => UserTurnBlock::Image { - data: b, - mime_type: mt, + data: b.clone(), + mime_type: mt.clone(), // A pasted image has no path; `""` would read as a filename of // nothing rather than as "unnamed". - uri: (!uri.is_empty()).then_some(uri), + uri: (!uri.is_empty()).then(|| uri.clone()), }, _ => UserTurnBlock::Text { - text: attachment_marker(&uri, &uri), + text: attachment_marker(uri, uri), }, }, PromptInputBlock::ResourceLink { uri, name, .. } => UserTurnBlock::Text { - text: attachment_marker(&name, &uri), + text: attachment_marker(name, uri), }, } } +/// The human name of an attachment, for places that need to NAME it rather +/// than render it — a conversation whose first message is one dropped-in file +/// is titled after that file. +/// +/// The uri's last path segment, percent-decoded. Falls back to the whole uri +/// when there is no segment to take (a bare scheme), and to the empty string +/// only for an empty uri — a pasted image travels with no path at all and +/// simply has no name to give. +pub fn attachment_display_name(uri: &str) -> String { + let trimmed = uri + .split(['?', '#']) + .next() + .unwrap_or(uri) + .trim_end_matches('/'); + let segment = trimmed.rsplit(['/', '\\']).next().unwrap_or(""); + let candidate = if segment.is_empty() { trimmed } else { segment }; + if candidate.is_empty() { + return uri.to_string(); + } + percent_decode(candidate) +} + +/// Decode `%XX` escapes, leaving any malformed escape exactly as written — a +/// name is for reading, so a half-encoded one must not lose characters. +fn percent_decode(s: &str) -> String { + let bytes = s.as_bytes(); + let mut out: Vec = Vec::with_capacity(bytes.len()); + let mut i = 0; + while i < bytes.len() { + if bytes[i] == b'%' && i + 2 < bytes.len() { + let hex = std::str::from_utf8(&bytes[i + 1..i + 3]).ok(); + if let Some(byte) = hex.and_then(|h| u8::from_str_radix(h, 16).ok()) { + out.push(byte); + i += 3; + continue; + } + } + out.push(bytes[i]); + i += 1; + } + String::from_utf8(out).unwrap_or_else(|_| s.to_string()) +} + +/// Name a prompt after the files it carries, for a message that has no prose +/// to be named after. `None` when it carries nothing nameable. +/// +/// This is a SEED only — a last resort for a row that would otherwise read +/// "Untitled" forever. It must never travel the authoritative +/// `refresh_auto_title` path, where it would overwrite a title the agent +/// itself published (see `acp::lifecycle`'s `NativeSessionTitle` arm). +pub fn attachment_names_from_prompt(blocks: &[PromptInputBlock]) -> Option { + let names: Vec = blocks + .iter() + .filter_map(|b| match b { + PromptInputBlock::ResourceLink { name, .. } => Some(name.clone()), + PromptInputBlock::Resource { uri, .. } => Some(attachment_display_name(uri)), + PromptInputBlock::Image { uri, .. } => { + uri.as_deref().map(attachment_display_name) + } + PromptInputBlock::Text { .. } => None, + }) + .filter(|name| !name.trim().is_empty()) + .collect(); + (!names.is_empty()).then(|| names.join(", ")) +} + /// Render an attachment as the inline Markdown link the transcript renders back /// into a file badge / attachment chip. /// @@ -891,11 +957,28 @@ fn attachment_marker(label: &str, uri: &str) -> String { /// Backslash-escape every inline-significant ASCII punctuation char, so a label /// cannot inject Markdown structure (a nested link, emphasis, a code span…). -/// Mirrors `escapeMarkdownText` in `src/components/chat/composer/reference-text.ts`; -/// GFM does not autolink inside link text, so escaping alone is enough here. +/// Mirrors `collapseNewlines` + `escapeMarkdownText` in +/// `src/components/chat/composer/reference-text.ts`: a newline run collapses to +/// one space first, because a marker has to stay a single inline token. GFM +/// does not autolink inside link text, so escaping alone is enough here. fn escape_markdown_text(text: &str) -> String { let mut out = String::with_capacity(text.len()); - for ch in text.chars() { + let mut chars = text.chars().peekable(); + while let Some(ch) = chars.next() { + // `\s*[\r\n]+\s*` → " ": only a run that CONTAINS a line break + // collapses, so ordinary spaces in a file name survive. + if ch.is_whitespace() { + let mut run = String::from(ch); + while chars.peek().is_some_and(|c| c.is_whitespace()) { + run.push(chars.next().expect("peeked")); + } + if run.contains(['\r', '\n']) { + out.push(' '); + } else { + out.push_str(&run); + } + continue; + } if matches!( ch, '\\' | '`' | '*' | '_' | '~' | '[' | ']' | '(' | ')' | '<' | '>' @@ -955,18 +1038,21 @@ pub fn prompt_block_from_wire(item: &serde_json::Value) -> Option` here + // and the typed reader on the replay side keeps a `""` as `Some("")`, so + // dropping it would make the two readers disagree on the same content. let mime = |v: &serde_json::Value| { v.get("mimeType") .or_else(|| v.get("mime_type")) .and_then(|m| m.as_str()) .map(str::to_string) - .filter(|s| !s.is_empty()) }; match item.get("type").and_then(|t| t.as_str()) { Some("image") => Some(PromptInputBlock::Image { data: string(item, "data")?, - // ACP requires `mimeType`; a record missing it is old enough that - // png was the only thing codeg ever pasted. + // ACP requires `mimeType`; a record MISSING it is old enough that + // png was the only thing codeg ever pasted. A record that carries + // an empty one keeps it, which is what the typed reader sees. mime_type: mime(item).unwrap_or_else(|| "image/png".to_string()), uri: string(item, "uri"), }), @@ -1015,7 +1101,7 @@ pub fn prompt_block_from_wire(item: &serde_json::Value) -> Option Vec { blocks .iter() - .map(|b| match project_user_prompt_block(b.clone()) { + .map(|b| match project_user_prompt_block(b) { UserTurnBlock::Text { text } => UserMessageBlock::Text { text }, UserTurnBlock::Image { data, mime_type, .. diff --git a/src-tauri/src/parsers/acp_native.rs b/src-tauri/src/parsers/acp_native.rs index 5ef72bf633..368f192a1b 100644 --- a/src-tauri/src/parsers/acp_native.rs +++ b/src-tauri/src/parsers/acp_native.rs @@ -41,7 +41,7 @@ use crate::acp::connection::{ extract_tool_call_images, json_value_to_text, serialize_tool_call_content, synthesize_edit_input_from_diffs, }; -use crate::acp::types::{prompt_block_from_wire, PromptInputBlock}; +use crate::acp::types::PromptInputBlock; use crate::acp_transcript::{self, EntryKind, Transcript, TranscriptEntry}; use crate::models::agent::AgentType; use crate::models::conversation::{ConversationDetail, ConversationSummary, SessionStats}; @@ -186,76 +186,26 @@ fn first_prompt_title(entries: &[TranscriptEntry]) -> Option { Some(crate::parsers::truncate_str(trimmed, 80)) } -/// A title for a recorded `session/prompt` payload: its prose, or — when the -/// user sent nothing but attachments — what they attached. +/// Concatenate the text of a recorded `session/prompt` content-block array. /// -/// ACP has no title channel, so this string is the only name the conversation -/// will ever have (codeg's DB-side auto-title backfill reads it from here). A -/// message that is one dropped-in file used to yield the empty string and leave -/// the row permanently untitled, so the attachment names stand in — the file -/// names, not the `[uri](uri)` markers the turn itself renders, which would be -/// unreadable as a title. +/// Prose ONLY. An attachment-only prompt yields nothing here on purpose: this +/// string is the *authoritative* parsed title, and +/// `commands::conversations` writes it over any unlocked title the row +/// already has — including one the agent itself published over +/// `session_info_update`. Naming such a conversation after its attachment is +/// worth doing, but as a seed for a row that has no title at all; that lives +/// in `acp::manager`'s first-prompt seed (`attachment_names_from_prompt`). fn prompt_text(payload: &serde_json::Value) -> String { - let Some(items) = payload.as_array() else { - return String::new(); - }; - let prose: String = items - .iter() - .filter_map(|b| b.get("text").and_then(|t| t.as_str())) - .collect(); - if !prose.trim().is_empty() { - return prose; - } - items - .iter() - .filter_map(prompt_block_from_wire) - .filter_map(|b| match b { - PromptInputBlock::ResourceLink { name, .. } => Some(name), - PromptInputBlock::Resource { uri, .. } => Some(attachment_name(&uri)), - PromptInputBlock::Image { uri, .. } => uri.as_deref().map(attachment_name), - PromptInputBlock::Text { .. } => None, + payload + .as_array() + .map(|blocks| { + blocks + .iter() + .filter_map(|b| b.get("text").and_then(|t| t.as_str())) + .collect::>() + .join("") }) - .collect::>() - .join(", ") -} - -/// The human name of an attachment uri: its last path segment, percent-decoded -/// where that is unambiguous. Falls back to the whole uri when there is no -/// segment to take (`clipboard://`, a bare scheme), which is still better than -/// nothing in a conversation list. -fn attachment_name(uri: &str) -> String { - let trimmed = uri - .split(['?', '#']) - .next() - .unwrap_or(uri) - .trim_end_matches('/'); - let segment = trimmed.rsplit(['/', '\\']).next().unwrap_or(""); - let candidate = if segment.is_empty() { trimmed } else { segment }; - if candidate.is_empty() { - return uri.to_string(); - } - percent_decode(candidate) -} - -/// Decode `%XX` escapes, leaving any malformed escape exactly as written — -/// a name is for reading, so a half-encoded one must not lose characters. -fn percent_decode(s: &str) -> String { - let bytes = s.as_bytes(); - let mut out: Vec = Vec::with_capacity(bytes.len()); - let mut i = 0; - while i < bytes.len() { - if bytes[i] == b'%' && i + 2 < bytes.len() { - let hex = std::str::from_utf8(&bytes[i + 1..i + 3]).ok(); - if let Some(byte) = hex.and_then(|h| u8::from_str_radix(h, 16).ok()) { - out.push(byte); - i += 3; - continue; - } - } - out.push(bytes[i]); - i += 1; - } - String::from_utf8(out).unwrap_or_else(|_| s.to_string()) + .unwrap_or_default() } /// Blocks for a user turn recorded from a `session/prompt` payload. @@ -266,7 +216,8 @@ fn prompt_blocks(payload: &serde_json::Value) -> Vec { items.iter().filter_map(user_turn_block_from_wire).collect() } -/// The replayed counterpart of [`prompt_block_from_wire`]: a `session/load` +/// The replayed counterpart of [`crate::acp::types::prompt_block_from_wire`]: +/// a `session/load` /// chunk arrives already deserialized, so it is converted straight across /// rather than being serialized back to JSON only to be re-read. Consumes the /// block, so an embedded image's base64 moves instead of being copied twice. @@ -612,7 +563,7 @@ fn apply_update( // block in the live projection, and appending to it would glue the // next sentence onto the end of a markdown link. let is_text = matches!(input, PromptInputBlock::Text { .. }); - let block = user_turn_block(input); + let block = user_turn_block(&input); let coalesce = is_text && *user_prose_open; flush(pending, turns, seq); *turn_start_hint = Some(at_ms); @@ -921,6 +872,7 @@ fn session_stats(turns: &[MessageTurn]) -> Option { #[cfg(test)] mod tests { use super::*; + use crate::acp::types::prompt_block_from_wire; use crate::acp_transcript::{TranscriptEntry, TranscriptHeader}; fn entry(t: u64, k: EntryKind, p: serde_json::Value) -> TranscriptEntry { @@ -1027,35 +979,56 @@ mod tests { assert_eq!(turns[0].blocks.len(), 6); } - /// ACP has no title channel, so a message that is nothing but a dropped-in - /// file used to leave its conversation permanently untitled. The attachment - /// names stand in — readable, unlike the `[uri](uri)` markers the turn - /// itself renders. + /// A recorded prompt is read as raw JSON (it must survive bytes written by + /// older builds) while a `session/load` chunk arrives already typed. Two + /// readers, one meaning: the same content has to produce the same block + /// whichever door it comes through, or history would contradict itself + /// depending on whether codeg recorded the turn or the agent replayed it. #[test] - fn an_attachment_only_prompt_is_titled_after_what_was_attached() { - let named = serde_json::json!([ + fn the_typed_and_raw_readers_agree_on_the_same_content() { + for item in attachment_prompt().as_array().expect("an array") { + let typed = sacp::schema::ContentBlock::deserialize(item) + .expect("every block in the fixture is valid ACP"); + assert_eq!( + prompt_block_from_content(typed), + prompt_block_from_wire(item), + "{item}" + ); + } + // And on content neither door should let through. + for junk in [ + serde_json::json!({"type":"text", "text":""}), + serde_json::json!({"type":"image", "data":"", "mimeType":"image/png"}), + serde_json::json!({"type":"resource_link", "uri":"", "name":"x"}), + serde_json::json!({"type":"audio", "data":"QUJD", "mimeType":"audio/wav"}), + ] { + assert_eq!(prompt_block_from_wire(&junk), None, "{junk}"); + if let Ok(typed) = sacp::schema::ContentBlock::deserialize(&junk) { + assert_eq!(prompt_block_from_content(typed), None, "{junk}"); + } + } + } + + /// The parsed title is the AUTHORITATIVE one — `commands::conversations` + /// writes it over any unlocked title the row holds, including one the agent + /// published itself. So it stays prose-only: an attachment-only prompt + /// reports no title here and is named by the first-prompt SEED instead + /// (`acp::manager::delegation_child_title_seed`), which only ever fills a + /// row that has no title at all. + #[test] + fn the_parsed_title_is_prose_only_and_never_an_attachment_name() { + let attachments_only = serde_json::json!([ {"type":"resource_link", "name":"report.pdf", "uri":"file:///tmp/report.pdf"}, - {"type":"resource", "resource":{ - "uri":"clipboard://my%20notes.txt-9f2", "mimeType":"text/plain", "text":"body" - }} + {"type":"image", "data":"aW1n", "mimeType":"image/png"} ]); - let entries = [entry(1, EntryKind::Prompt, named)]; assert_eq!( - first_prompt_title(&entries).as_deref(), - Some("report.pdf, my notes.txt-9f2") + first_prompt_title(&[entry(1, EntryKind::Prompt, attachments_only)]), + None ); - - // Prose still wins outright, attachments and all. assert_eq!( first_prompt_title(&[entry(1, EntryKind::Prompt, attachment_prompt())]).as_deref(), Some("Review these files") ); - - // A prompt with neither is still untitled rather than named "". - assert_eq!( - first_prompt_title(&[entry(1, EntryKind::Prompt, serde_json::json!([])),]), - None - ); } /// An everyday file name is not a safe Markdown fragment. The marker is diff --git a/src-tauri/src/parsers/grok.rs b/src-tauri/src/parsers/grok.rs index ad69e88cad..4983cabeda 100644 --- a/src-tauri/src/parsers/grok.rs +++ b/src-tauri/src/parsers/grok.rs @@ -10,6 +10,7 @@ use crate::models::{ AgentExecutionStats, AgentToolCall, AgentType, ContentBlock, ConversationDetail, ConversationSummary, MessageTurn, TurnRole, TurnUsage, }; +use crate::acp::types::PromptInputBlock; use crate::parsers::claude::BACKGROUND_TASK_MARKER; use crate::parsers::{ backfill_turn_durations, compute_session_stats, folder_name_from_path, @@ -736,11 +737,13 @@ fn parse_updates(path: &Path) -> ParsedUpdates { match kind { "user_message_chunk" => { - let block = user_chunk_to_block(update); + let input = user_chunk_to_input(update); + let block = input.as_ref().map(crate::parsers::user_turn_block); out.content_events += 1; - // Title/first-prompt text comes only from prose chunks; an image - // chunk carries no text and must not overwrite it. - if let Some(ContentBlock::Text { text }) = &block { + // Title/first-prompt text comes only from PROSE chunks: an image + // chunk carries no text, and an attachment projects to a + // `[name](uri)` marker that would make a poor title. + if let Some(PromptInputBlock::Text { text }) = &input { if out.first_user_text.is_none() && !text.trim().is_empty() { out.first_user_text = Some(text.clone()); } @@ -1034,10 +1037,15 @@ fn update_text(update: &Value) -> String { /// itself and had no `resource_link` case at all, which silently dropped /// plain file attachments from a reloaded grok turn. /// +/// Returns the block the chunk was SENT as, so the caller can both render it +/// (via [`crate::parsers::user_turn_block`]) and tell prose from an attachment +/// — an attachment projects to a `Text` marker, and the conversation's title +/// must not latch onto `[report.pdf](…)` when the prose follows it. +/// /// `None` for a chunk with nothing to render (empty prose, a malformed /// resource): the caller still opens the user turn, it just starts empty. -fn user_chunk_to_block(update: &Value) -> Option { - crate::parsers::user_turn_block_from_wire(update.get("content")?) +fn user_chunk_to_input(update: &Value) -> Option { + crate::acp::types::prompt_block_from_wire(update.get("content")?) } // --------------------------------------------------------------------------- diff --git a/src-tauri/src/parsers/mod.rs b/src-tauri/src/parsers/mod.rs index 211622d5a5..4614f0b31a 100644 --- a/src-tauri/src/parsers/mod.rs +++ b/src-tauri/src/parsers/mod.rs @@ -683,7 +683,7 @@ pub fn title_from_user_text(text: &str) -> String { /// the SINGLE rule shared with the live broadcast. This only carries the result /// across into `models::message`, keeping the image `uri` that the live wire /// type has nowhere to put but the frontend uses for an image's display name. -pub fn user_turn_block(block: crate::acp::types::PromptInputBlock) -> ContentBlock { +pub fn user_turn_block(block: &crate::acp::types::PromptInputBlock) -> ContentBlock { match crate::acp::types::project_user_prompt_block(block) { crate::acp::types::UserTurnBlock::Text { text } => ContentBlock::Text { text }, crate::acp::types::UserTurnBlock::Image { @@ -706,7 +706,7 @@ pub fn user_turn_block(block: crate::acp::types::PromptInputBlock) -> ContentBlo /// goes through here, so "how an attachment appears in a user message" is /// decided once rather than per agent. pub fn user_turn_block_from_wire(item: &serde_json::Value) -> Option { - crate::acp::types::prompt_block_from_wire(item).map(user_turn_block) + crate::acp::types::prompt_block_from_wire(item).map(|b| user_turn_block(&b)) } /// Fill in `duration_ms` for assistant turns whose agent reports no timing of