From d6252740ed47f067c96138b643cf6d0e82c596b2 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Tue, 31 Mar 2026 16:15:48 +1100 Subject: [PATCH 1/2] feat(staged): forward tool call raw_input params and improve display Plumb raw_input from ACP ToolCall/ToolCallUpdate through the full pipeline (LiveAction enum, MessageWriter trait, DB writer) so tool calls are stored as JSON {"name", "input"} when raw_input is present. Handle raw_input-only ToolCallUpdate messages that previously were silently dropped when no title was present, by caching the last-known title per tool-call row. Improve formatToolDisplay to extract just the tool name from ACP titles, show only the primary arg per tool type, and fall through to plain-text for unrecognized tool names to avoid duplicated content. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src-tauri/examples/acp_stream_probe.rs | 15 +- apps/staged/src-tauri/src/agent/writer.rs | 139 +++++++++++++++--- .../features/sessions/sessionModalHelpers.ts | 72 ++++++++- crates/acp-client/src/driver.rs | 65 ++++++-- 4 files changed, 249 insertions(+), 42 deletions(-) diff --git a/apps/staged/src-tauri/examples/acp_stream_probe.rs b/apps/staged/src-tauri/examples/acp_stream_probe.rs index 814d4f2c..8c5f0bbe 100644 --- a/apps/staged/src-tauri/examples/acp_stream_probe.rs +++ b/apps/staged/src-tauri/examples/acp_stream_probe.rs @@ -68,7 +68,12 @@ impl MessageWriter for ProbeWriter { println!("[probe] finalize"); } - async fn record_tool_call(&self, tool_call_id: &str, title: &str) { + async fn record_tool_call( + &self, + tool_call_id: &str, + title: &str, + _raw_input: Option<&serde_json::Value>, + ) { let mut state = self.state.lock().expect("probe state lock poisoned"); state.total_tool_calls += 1; let count_for_id = *state @@ -85,9 +90,15 @@ impl MessageWriter for ProbeWriter { ); } - async fn update_tool_call_title(&self, tool_call_id: &str, title: &str) { + async fn update_tool_call_title( + &self, + tool_call_id: &str, + title: Option<&str>, + _raw_input: Option<&serde_json::Value>, + ) { let mut state = self.state.lock().expect("probe state lock poisoned"); state.total_tool_title_updates += 1; + let title = title.unwrap_or(""); println!( "[probe] tool_call_update #{} id={} title={}", state.total_tool_title_updates, diff --git a/apps/staged/src-tauri/src/agent/writer.rs b/apps/staged/src-tauri/src/agent/writer.rs index e890a5eb..7cfddb09 100644 --- a/apps/staged/src-tauri/src/agent/writer.rs +++ b/apps/staged/src-tauri/src/agent/writer.rs @@ -39,8 +39,8 @@ pub struct MessageWriter { current_text: Mutex, /// When we last wrote to the DB — used to throttle flush frequency. last_flush_at: Mutex, - /// Maps external tool-call IDs → DB row IDs. - tool_call_rows: Mutex>, + /// Maps external tool-call IDs → (DB row ID, last-known title). + tool_call_rows: Mutex>, /// DB row id of the currently streaming tool result. /// /// ACP can send multiple content updates for one tool call; we update @@ -53,6 +53,17 @@ fn sanitize_title(title: &str) -> String { title.replace('`', "") } +/// Format a tool call for storage. When `raw_input` is present, produces a JSON +/// object `{"name": title, "input": raw_input}` that the frontend can parse to +/// display structured tool call info. Without raw_input, falls back to the plain +/// title string. +fn format_tool_call_content(title: &str, raw_input: Option<&serde_json::Value>) -> String { + match raw_input { + Some(input) => serde_json::json!({ "name": title, "input": input }).to_string(), + None => title.to_string(), + } +} + impl MessageWriter { pub fn new(session_id: String, store: Arc) -> Self { Self { @@ -98,39 +109,60 @@ impl MessageWriter { /// Record a tool call. Finalizes any in-progress assistant text first /// to maintain correct message ordering. - pub async fn record_tool_call(&self, tool_call_id: &str, title: &str) { + pub async fn record_tool_call( + &self, + tool_call_id: &str, + title: &str, + raw_input: Option<&serde_json::Value>, + ) { self.finalize().await; *self.current_tool_result_msg_id.lock().await = None; let title = sanitize_title(title); + let content = format_tool_call_content(&title, raw_input); // Some providers may resend ToolCall for the same ID while streaming. // Treat those as updates to the existing row. - if let Some(&row_id) = self.tool_call_rows.lock().await.get(tool_call_id) { - let _ = self.store.update_message_content(row_id, &title); + let mut rows = self.tool_call_rows.lock().await; + if let Some((row_id, stored_title)) = rows.get_mut(tool_call_id) { + *stored_title = title.clone(); + let _ = self.store.update_message_content(*row_id, &content); return; } match self .store - .add_session_message(&self.session_id, MessageRole::ToolCall, &title) + .add_session_message(&self.session_id, MessageRole::ToolCall, &content) { Ok(id) => { - self.tool_call_rows - .lock() - .await - .insert(tool_call_id.to_string(), id); + rows.insert(tool_call_id.to_string(), (id, title)); } Err(e) => log::error!("Failed to insert tool_call message: {e}"), } } - /// Update a previously recorded tool call's title. - pub async fn update_tool_call_title(&self, tool_call_id: &str, title: &str) { - let title = sanitize_title(title); - let rows = self.tool_call_rows.lock().await; - if let Some(&row_id) = rows.get(tool_call_id) { - let _ = self.store.update_message_content(row_id, &title); + /// Update a previously recorded tool call's title and/or raw input. + /// + /// When `title` is `None`, the last-known title stored at recording time + /// is reused so that a `raw_input`-only update doesn't blank the name. + pub async fn update_tool_call_title( + &self, + tool_call_id: &str, + title: Option<&str>, + raw_input: Option<&serde_json::Value>, + ) { + let mut rows = self.tool_call_rows.lock().await; + if let Some((row_id, stored_title)) = rows.get_mut(tool_call_id) { + let effective_title = match title { + Some(t) => { + let sanitized = sanitize_title(t); + *stored_title = sanitized.clone(); + sanitized + } + None => stored_title.clone(), + }; + let content = format_tool_call_content(&effective_title, raw_input); + let _ = self.store.update_message_content(*row_id, &content); } } @@ -208,12 +240,23 @@ impl acp_client::MessageWriter for MessageWriter { self.finalize().await } - async fn record_tool_call(&self, tool_call_id: &str, title: &str) { - self.record_tool_call(tool_call_id, title).await + async fn record_tool_call( + &self, + tool_call_id: &str, + title: &str, + raw_input: Option<&serde_json::Value>, + ) { + self.record_tool_call(tool_call_id, title, raw_input).await } - async fn update_tool_call_title(&self, tool_call_id: &str, title: &str) { - self.update_tool_call_title(tool_call_id, title).await + async fn update_tool_call_title( + &self, + tool_call_id: &str, + title: Option<&str>, + raw_input: Option<&serde_json::Value>, + ) { + self.update_tool_call_title(tool_call_id, title, raw_input) + .await } async fn record_tool_result(&self, content: &str) { @@ -241,7 +284,9 @@ mod tests { async fn record_tool_result_updates_existing_row_for_streaming_updates() { let (store, session_id, writer) = setup_writer(); - writer.record_tool_call("tc-1", "Run echo hello").await; + writer + .record_tool_call("tc-1", "Run echo hello", None) + .await; writer.record_tool_result("first chunk").await; writer.record_tool_result("second chunk").await; @@ -258,8 +303,12 @@ mod tests { async fn record_tool_call_same_id_updates_instead_of_inserting() { let (store, session_id, writer) = setup_writer(); - writer.record_tool_call("tc-dup", "Run first title").await; - writer.record_tool_call("tc-dup", "Run updated title").await; + writer + .record_tool_call("tc-dup", "Run first title", None) + .await; + writer + .record_tool_call("tc-dup", "Run updated title", None) + .await; let messages = store .get_session_messages(&session_id) @@ -268,4 +317,48 @@ mod tests { assert_eq!(messages[0].role, MessageRole::ToolCall); assert_eq!(messages[0].content, "Run updated title"); } + + #[tokio::test] + async fn record_tool_call_with_raw_input_stores_json() { + let (store, session_id, writer) = setup_writer(); + + let raw_input = serde_json::json!({"path": "foo.rs"}); + writer + .record_tool_call("tc-json", "Read file", Some(&raw_input)) + .await; + + let messages = store + .get_session_messages(&session_id) + .expect("query messages"); + assert_eq!(messages.len(), 1); + assert_eq!(messages[0].role, MessageRole::ToolCall); + + let parsed: serde_json::Value = + serde_json::from_str(&messages[0].content).expect("content should be valid JSON"); + assert_eq!(parsed["name"], "Read file"); + assert_eq!(parsed["input"]["path"], "foo.rs"); + } + + #[tokio::test] + async fn update_tool_call_raw_input_without_title_preserves_title() { + let (store, session_id, writer) = setup_writer(); + + writer.record_tool_call("tc-ri", "Read file", None).await; + + // Update with raw_input only (no title). + let raw_input = serde_json::json!({"path": "bar.rs"}); + writer + .update_tool_call_title("tc-ri", None, Some(&raw_input)) + .await; + + let messages = store + .get_session_messages(&session_id) + .expect("query messages"); + assert_eq!(messages.len(), 1); + + let parsed: serde_json::Value = + serde_json::from_str(&messages[0].content).expect("content should be valid JSON"); + assert_eq!(parsed["name"], "Read file"); + assert_eq!(parsed["input"]["path"], "bar.rs"); + } } diff --git a/apps/staged/src/lib/features/sessions/sessionModalHelpers.ts b/apps/staged/src/lib/features/sessions/sessionModalHelpers.ts index 86c58336..2fff85cf 100644 --- a/apps/staged/src/lib/features/sessions/sessionModalHelpers.ts +++ b/apps/staged/src/lib/features/sessions/sessionModalHelpers.ts @@ -70,6 +70,36 @@ const TOOL_VERBS: Record = { SemanticSearch: ['Searched', 'Searching'], }; +/** Pick the single most useful display value from structured tool args. */ +function primaryArg(toolName: string, args: Record): string { + const str = (key: string) => { + const v = args[key]; + return typeof v === 'string' ? v : undefined; + }; + switch (toolName) { + case 'Read': + case 'ReadFile': + case 'Write': + case 'WriteFile': + case 'Delete': + case 'EditNotebook': + case 'StrReplace': + return str('file_path') || str('path') || ''; + case 'Run': + case 'Shell': + case 'Bash': + return str('command') || str('cmd') || ''; + case 'Grep': + case 'Search': + case 'SemanticSearch': + return str('pattern') || str('query') || ''; + case 'Glob': + return str('pattern') || str('glob') || ''; + default: + return formatArgs(args); + } +} + const TITLE_VERBS = new Set([ 'Add', 'Analyze', @@ -211,19 +241,51 @@ export function formatToolDisplay( const tenseIdx = pending ? 1 : 0; const parsed = parseToolCall(content); if (parsed) { - const entry = TOOL_VERBS[parsed.name]; - const verb = entry ? entry[tenseIdx] : parsed.name; - return { verb, detail: makePathsRelative(formatArgs(parsed.args), repoDir) }; + // parsed.name may be a bare tool name ("Read") or a full ACP title + // ("Read /path/to/file"). Try exact match first, then first-word match. + let toolName = parsed.name; + let entry = TOOL_VERBS[toolName]; + if (!entry) { + const spaceIdx = parsed.name.indexOf(' '); + if (spaceIdx > 0) { + const firstWord = parsed.name.slice(0, spaceIdx); + if (TOOL_VERBS[firstWord]) { + toolName = firstWord; + entry = TOOL_VERBS[firstWord]; + } + } + } + if (entry) { + const verb = entry[tenseIdx]; + const detail = makePathsRelative(primaryArg(toolName, parsed.args), repoDir); + return { verb, detail }; + } + // Unrecognized tool name — fall through to treat parsed.name as plain text + content = parsed.name; } + // Plain-text content: check TOOL_VERBS first (handles "Shell", "Bash ls", etc.) const spaceIdx = content.indexOf(' '); if (spaceIdx > 0) { const firstWord = content.slice(0, spaceIdx); + const tvEntry = TOOL_VERBS[firstWord]; + if (tvEntry) { + return { + verb: tvEntry[tenseIdx], + detail: makePathsRelative(content.slice(spaceIdx + 1), repoDir), + }; + } if (TITLE_VERBS.has(firstWord)) { return { verb: firstWord, detail: makePathsRelative(content.slice(spaceIdx + 1), repoDir) }; } - } else if (TITLE_VERBS.has(content)) { - return { verb: content, detail: '' }; + } else { + const tvEntry = TOOL_VERBS[content]; + if (tvEntry) { + return { verb: tvEntry[tenseIdx], detail: '' }; + } + if (TITLE_VERBS.has(content)) { + return { verb: content, detail: '' }; + } } return { verb: pending ? 'Running' : 'Ran', detail: makePathsRelative(content, repoDir) }; diff --git a/crates/acp-client/src/driver.rs b/crates/acp-client/src/driver.rs index 5683d068..cb432b24 100644 --- a/crates/acp-client/src/driver.rs +++ b/crates/acp-client/src/driver.rs @@ -50,11 +50,24 @@ pub trait MessageWriter: Send + Sync { /// Flush all buffered text and close the current message block. async fn finalize(&self); - /// Record a tool call with its ID and title. - async fn record_tool_call(&self, tool_call_id: &str, title: &str); + /// Record a tool call with its ID, title, and optional raw input parameters. + async fn record_tool_call( + &self, + tool_call_id: &str, + title: &str, + raw_input: Option<&serde_json::Value>, + ); - /// Update a previously recorded tool call's title. - async fn update_tool_call_title(&self, tool_call_id: &str, title: &str); + /// Update a previously recorded tool call's title and/or raw input. + /// + /// When `title` is `None`, the implementation should preserve the + /// existing title while updating only `raw_input`. + async fn update_tool_call_title( + &self, + tool_call_id: &str, + title: Option<&str>, + raw_input: Option<&serde_json::Value>, + ); /// Record the result/output of a tool call. async fn record_tool_result(&self, content: &str); @@ -841,10 +854,12 @@ impl agent_client_protocol::Client for AcpNotificationHandler { RecordToolCall { id: String, title: String, + raw_input: Option, }, ToolCallUpdate { id: String, title: Option, + raw_input: Option, result: Option, }, Ignore, @@ -950,19 +965,22 @@ impl agent_client_protocol::Client for AcpNotificationHandler { SessionUpdate::ToolCall(tool_call) => LiveAction::RecordToolCall { id: tool_call.tool_call_id.0.to_string(), title: tool_call.title.clone(), + raw_input: tool_call.raw_input.clone(), }, SessionUpdate::ToolCallUpdate(update) => { let tc_id = update.tool_call_id.0.to_string(); let title = update.fields.title.clone(); + let raw_input = update.fields.raw_input.clone(); let result = update .fields .content .as_ref() .and_then(|c| extract_content_preview(c)); - if title.is_some() || result.is_some() { + if title.is_some() || raw_input.is_some() || result.is_some() { LiveAction::ToolCallUpdate { id: tc_id, title, + raw_input, result, } } else { @@ -981,12 +999,25 @@ impl agent_client_protocol::Client for AcpNotificationHandler { LiveAction::AppendText(text) => { self.writer.append_text(&text).await; } - LiveAction::RecordToolCall { id, title } => { - self.writer.record_tool_call(&id, &title).await; + LiveAction::RecordToolCall { + id, + title, + raw_input, + } => { + self.writer + .record_tool_call(&id, &title, raw_input.as_ref()) + .await; } - LiveAction::ToolCallUpdate { id, title, result } => { - if let Some(title) = title { - self.writer.update_tool_call_title(&id, &title).await; + LiveAction::ToolCallUpdate { + id, + title, + raw_input, + result, + } => { + if title.is_some() || raw_input.is_some() { + self.writer + .update_tool_call_title(&id, title.as_deref(), raw_input.as_ref()) + .await; } if let Some(preview) = result { self.writer.record_tool_result(&preview).await; @@ -1255,12 +1286,22 @@ impl MessageWriter for BasicMessageWriter { // Nothing to do for basic implementation } - async fn record_tool_call(&self, _tool_call_id: &str, title: &str) { + async fn record_tool_call( + &self, + _tool_call_id: &str, + title: &str, + _raw_input: Option<&serde_json::Value>, + ) { let mut current = self.text.lock().await; current.push_str(&format!("\n[Tool: {}]\n", title)); } - async fn update_tool_call_title(&self, _tool_call_id: &str, _title: &str) { + async fn update_tool_call_title( + &self, + _tool_call_id: &str, + _title: Option<&str>, + _raw_input: Option<&serde_json::Value>, + ) { // Nothing to do for basic implementation } From 0adf3f2c0def9ede60fa045fe9fc7cc262180ace Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Tue, 31 Mar 2026 16:21:09 +1100 Subject: [PATCH 2/2] fix(staged): add Edit tool to display helpers and truncate long args Address code review feedback: add Edit tool to TOOL_VERBS and primaryArg so it shows file_path like other file-based tools, and truncate formatArgs output at 200 chars in the default case to prevent very long detail strings from tools with large inputs. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/lib/features/sessions/sessionModalHelpers.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/apps/staged/src/lib/features/sessions/sessionModalHelpers.ts b/apps/staged/src/lib/features/sessions/sessionModalHelpers.ts index 2fff85cf..8710ee3d 100644 --- a/apps/staged/src/lib/features/sessions/sessionModalHelpers.ts +++ b/apps/staged/src/lib/features/sessions/sessionModalHelpers.ts @@ -64,6 +64,7 @@ const TOOL_VERBS: Record = { Grep: ['Searched', 'Searching'], Search: ['Searched', 'Searching'], Glob: ['Listed', 'Listing'], + Edit: ['Edited', 'Editing'], StrReplace: ['Edited', 'Editing'], Delete: ['Deleted', 'Deleting'], EditNotebook: ['Edited', 'Editing'], @@ -81,6 +82,7 @@ function primaryArg(toolName: string, args: Record): string { case 'ReadFile': case 'Write': case 'WriteFile': + case 'Edit': case 'Delete': case 'EditNotebook': case 'StrReplace': @@ -95,8 +97,10 @@ function primaryArg(toolName: string, args: Record): string { return str('pattern') || str('query') || ''; case 'Glob': return str('pattern') || str('glob') || ''; - default: - return formatArgs(args); + default: { + const formatted = formatArgs(args); + return formatted.length > 200 ? formatted.slice(0, 200) + '…' : formatted; + } } }