fix: dedupe tool call display by toolCallId and sanitize titles (#138)

fix: dedupe tool call display by toolCallId and sanitize titles

Author: marvin-69-jpg

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<AcpEvent> {
     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<AcpEvent> {
         }
         "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),

src/discord.rs

@@ -407,7 +407,14 @@ async fn stream_prompt(
             let (buf_tx, buf_rx) = watch::channel(initial);
 
             let mut text_buf = String::new();
-            let mut tool_lines: Vec<String> = 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<ToolEntry> = Vec::new();
             let current_msg_id = msg_id;
 
             if reset {
@@ -474,16 +481,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));
                         }
@@ -529,11 +573,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)]
+enum ToolState {
+    Running,
+    Completed,
+    Failed,
+}
+
+#[derive(Debug, Clone)]
+struct ToolEntry {
+    id: String,
+    title: String,
+    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');