From 84fc55c7957ff4f842ee5b487c5c419de9a9fea3 Mon Sep 17 00:00:00 2001 From: marvin-69-jpg Date: Wed, 8 Apr 2026 13:40:49 +0800 Subject: [PATCH 1/2] fix: dedupe tool call display by toolCallId and sanitize titles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit claude-agent-acp emits multiple events for the same tool invocation as the input fields stream in: a `tool_call` with a placeholder title ("Terminal" / "Edit" / etc.) followed by one or more `tool_call_update` events that refine the title to the real command. The current handler pushes a new line on every event without deduping, so the message ends up with orphaned placeholder lines that never get updated: 🔧 `Terminal`... ❌ 🔧 `cd /home/node/work && git status`... ✅ It also passes the raw title through inline-code formatting without flattening newlines or escaping embedded backticks. Discord's single-backtick inline-code spans render on a single line only, so multi-line shell commands (heredocs, &&-chained commands written across lines) appear truncated mid-render with the inline-code span breaking on the first newline. Fix: * Carry the ACP toolCallId through AcpEvent::ToolStart / ToolDone so the renderer can pin updates to the same entry. * Replace `tool_lines: Vec` with `Vec` (id, title, state). ToolStart updates the slot in place if the id is already present; ToolDone preserves the existing title when the Done event omits one (which it routinely does in claude-agent-acp updates). * Add a `sanitize_title` helper that flattens \n to " ; " and rewrites embedded backticks so they can't break the surrounding inline-code span. * Each tool now renders via ToolEntry::render() which picks the icon from the state — no more brittle substring-based line lookups against the placeholder title. Tested in production against a multi-user Discord channel running heavy git/gh workflows with multi-line bash commands. --- src/acp/protocol.rs | 23 +++++++--- src/discord.rs | 100 +++++++++++++++++++++++++++++++++++++++----- 2 files changed, 108 insertions(+), 15 deletions(-) diff --git a/src/acp/protocol.rs b/src/acp/protocol.rs index d3e96ed5..82f00eb8 100644 --- a/src/acp/protocol.rs +++ b/src/acp/protocol.rs @@ -60,8 +60,8 @@ impl std::fmt::Display for JsonRpcError { pub enum AcpEvent { Text(String), Thinking, - ToolStart { title: String }, - ToolDone { title: String, status: String }, + ToolStart { id: String, title: String }, + ToolDone { id: String, title: String, status: String }, Status, } @@ -70,6 +70,19 @@ pub fn classify_notification(msg: &JsonRpcMessage) -> Option { let update = params.get("update")?; let session_update = update.get("sessionUpdate")?.as_str()?; + // toolCallId is the stable identity across tool_call → tool_call_update + // events for the same tool invocation. claude-agent-acp emits the first + // event before the input fields are streamed in (so the title falls back + // to "Terminal" / "Edit" / etc.) and refines them in a later + // tool_call_update; without the id we can't tell those events belong to + // the same call and end up rendering placeholder + refined as two + // separate lines. + let tool_id = update + .get("toolCallId") + .and_then(|v| v.as_str()) + .unwrap_or("") + .to_string(); + match session_update { "agent_message_chunk" => { let text = update.get("content")?.get("text")?.as_str()?; @@ -80,15 +93,15 @@ pub fn classify_notification(msg: &JsonRpcMessage) -> Option { } "tool_call" => { let title = update.get("title").and_then(|v| v.as_str()).unwrap_or("").to_string(); - Some(AcpEvent::ToolStart { title }) + Some(AcpEvent::ToolStart { id: tool_id, title }) } "tool_call_update" => { let title = update.get("title").and_then(|v| v.as_str()).unwrap_or("").to_string(); let status = update.get("status").and_then(|v| v.as_str()).unwrap_or("").to_string(); if status == "completed" || status == "failed" { - Some(AcpEvent::ToolDone { title, status }) + Some(AcpEvent::ToolDone { id: tool_id, title, status }) } else { - Some(AcpEvent::ToolStart { title }) + Some(AcpEvent::ToolStart { id: tool_id, title }) } } "plan" => Some(AcpEvent::Status), diff --git a/src/discord.rs b/src/discord.rs index da52c691..27f2e3ed 100644 --- a/src/discord.rs +++ b/src/discord.rs @@ -210,7 +210,14 @@ async fn stream_prompt( let (buf_tx, buf_rx) = watch::channel(initial); let mut text_buf = String::new(); - let mut tool_lines: Vec = Vec::new(); + // Tool calls indexed by toolCallId. Vec preserves first-seen + // order. We store id + title + state separately so a ToolDone + // event that arrives without a refreshed title (claude-agent-acp's + // update events don't always re-send the title field) can still + // reuse the title we already learned from a prior + // tool_call_update — only the icon flips 🔧 → ✅ / ❌. Rendering + // happens on the fly in compose_display(). + let mut tool_lines: Vec = Vec::new(); let current_msg_id = msg_id; if reset { @@ -272,16 +279,53 @@ async fn stream_prompt( AcpEvent::Thinking => { reactions.set_thinking().await; } - AcpEvent::ToolStart { title, .. } if !title.is_empty() => { + AcpEvent::ToolStart { id, title } if !title.is_empty() => { reactions.set_tool(&title).await; - tool_lines.push(format!("🔧 `{title}`...")); + let title = sanitize_title(&title); + // Dedupe by toolCallId: replace if we've already + // seen this id, otherwise append a new entry. + // claude-agent-acp emits a placeholder title + // ("Terminal", "Edit", etc.) on the first event + // and refines it via tool_call_update; without + // dedup the placeholder and refined version + // appear as two separate orphaned lines. + if let Some(slot) = tool_lines.iter_mut().find(|e| e.id == id) { + slot.title = title; + slot.state = ToolState::Running; + } else { + tool_lines.push(ToolEntry { + id, + title, + state: ToolState::Running, + }); + } let _ = buf_tx.send(compose_display(&tool_lines, &text_buf)); } - AcpEvent::ToolDone { title, status, .. } => { + AcpEvent::ToolDone { id, title, status } => { reactions.set_thinking().await; - let icon = if status == "completed" { "✅" } else { "❌" }; - if let Some(line) = tool_lines.iter_mut().rev().find(|l| l.contains(&title)) { - *line = format!("{icon} `{title}`"); + let new_state = if status == "completed" { + ToolState::Completed + } else { + ToolState::Failed + }; + // Find by id (the title is unreliable — substring + // match against the placeholder "Terminal" would + // never find the refined entry). Preserve the + // existing title if the Done event omits it. + if let Some(slot) = tool_lines.iter_mut().find(|e| e.id == id) { + if !title.is_empty() { + slot.title = sanitize_title(&title); + } + slot.state = new_state; + } else if !title.is_empty() { + // Done arrived without a prior Start (rare + // race) — record it so we still show + // something. + tool_lines.push(ToolEntry { + id, + title: sanitize_title(&title), + state: new_state, + }); } let _ = buf_tx.send(compose_display(&tool_lines, &text_buf)); } @@ -317,11 +361,47 @@ async fn stream_prompt( .await } -fn compose_display(tool_lines: &[String], text: &str) -> String { +/// Flatten a tool-call title into a single line that's safe to render +/// inside Discord inline-code spans. Discord renders single-backtick +/// code on a single line only, so multi-line shell commands (heredocs, +/// `&&`-chained commands split across lines) appear truncated; we +/// collapse newlines to ` ; ` and rewrite embedded backticks so they +/// don't break the wrapping span. +fn sanitize_title(title: &str) -> String { + title.replace('\r', "").replace('\n', " ; ").replace('`', "'") +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ToolState { + Running, + Completed, + Failed, +} + +#[derive(Debug, Clone)] +pub struct ToolEntry { + pub id: String, + pub title: String, + pub state: ToolState, +} + +impl ToolEntry { + fn render(&self) -> String { + let icon = match self.state { + ToolState::Running => "🔧", + ToolState::Completed => "✅", + ToolState::Failed => "❌", + }; + let suffix = if self.state == ToolState::Running { "..." } else { "" }; + format!("{icon} `{}`{}", self.title, suffix) + } +} + +fn compose_display(tool_lines: &[ToolEntry], text: &str) -> String { let mut out = String::new(); if !tool_lines.is_empty() { - for line in tool_lines { - out.push_str(line); + for entry in tool_lines { + out.push_str(&entry.render()); out.push('\n'); } out.push('\n'); From 1331bcff99e97d2cfe5cdcc083c93d45cc7c90ea Mon Sep 17 00:00:00 2001 From: thepagent Date: Sat, 11 Apr 2026 19:45:57 +0000 Subject: [PATCH 2/2] chore: make ToolState/ToolEntry private (review feedback) --- src/discord.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/discord.rs b/src/discord.rs index 27f2e3ed..186cf79b 100644 --- a/src/discord.rs +++ b/src/discord.rs @@ -372,17 +372,17 @@ fn sanitize_title(title: &str) -> String { } #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum ToolState { +enum ToolState { Running, Completed, Failed, } #[derive(Debug, Clone)] -pub struct ToolEntry { - pub id: String, - pub title: String, - pub state: ToolState, +struct ToolEntry { + id: String, + title: String, + state: ToolState, } impl ToolEntry {