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 d3e96ed..82f00eb 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 da52c69..27f2e3e 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 f1ad355cd3d7aff2efa11b654aa83216e18bd01f Mon Sep 17 00:00:00 2001 From: marvin-69-jpg Date: Wed, 8 Apr 2026 13:58:08 +0800 Subject: [PATCH 2/2] feat: surface streamed thinking content as a Discord blockquote MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently the only signal Discord users get when an agent enters extended thinking mode is the 🤔 emoji on the bot's message. The actual thinking content delivered via `agent_thought_chunk` is parsed and discarded — `classify_notification` returns `AcpEvent::Thinking` with no payload, and the `Thinking` arm in stream_prompt only flips the reaction emoji. This patch surfaces that content as a blockquote at the top of the streaming Discord message, similar to how the native Claude Code CLI shows thinking blocks above tool calls and the final answer: > 🤔 Thinking > Let me start by reading the README and checking my notes. > > There's a local commit ahead of origin/main, I should reset to > origin/main first before branching. Implementation: * `AcpEvent::Thinking` carries the delta string instead of being a unit variant; `classify_notification` reads `update.content.text` (the same shape `agent_message_chunk` already uses). * `stream_prompt` accumulates thinking deltas into a `thought_buf` alongside the existing `text_buf`, and re-renders the message on each event. * `compose_display` gains a `thought` parameter and emits a blockquote-prefixed section above the tool list when thought is non-empty. claude-agent-acp emits both `thinking` (block start) and `thinking_delta` (continuation) as the same `agent_thought_chunk` shape, with no marker for block boundaries. When the model produces several thinking blocks in a row separated only by tool calls, the deltas concatenate into a wall of text like ".There's a local commit" with no whitespace between sentences. We work around this with a small heuristic in `needs_thinking_separator`: if the previous chunk ends in sentence-terminating punctuation (`. ! ? 。 ！ ？`) and the new chunk starts with a letter (no leading whitespace), insert a paragraph break. This is imperfect but covers the common case without requiring upstream protocol changes. Tested in production for the past day against a multi-user Discord channel with both English and Traditional Chinese conversations; the heuristic correctly inserts breaks between distinct thinking blocks and leaves token-level deltas inside a single block alone. --- src/acp/protocol.rs | 15 ++++++++-- src/discord.rs | 68 +++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 75 insertions(+), 8 deletions(-) diff --git a/src/acp/protocol.rs b/src/acp/protocol.rs index 82f00eb..fbff921 100644 --- a/src/acp/protocol.rs +++ b/src/acp/protocol.rs @@ -59,7 +59,7 @@ impl std::fmt::Display for JsonRpcError { #[derive(Debug)] pub enum AcpEvent { Text(String), - Thinking, + Thinking(String), ToolStart { id: String, title: String }, ToolDone { id: String, title: String, status: String }, Status, @@ -89,7 +89,18 @@ pub fn classify_notification(msg: &JsonRpcMessage) -> Option { Some(AcpEvent::Text(text.to_string())) } "agent_thought_chunk" => { - Some(AcpEvent::Thinking) + // Extract the streamed thinking text. claude-agent-acp emits + // these as `{ content: { type: "text", text: "..." } }` (same + // shape as agent_message_chunk). If the field is missing we + // still emit Thinking with an empty string so the reactions + // controller still flips 🤔. + let text = update + .get("content") + .and_then(|c| c.get("text")) + .and_then(|v| v.as_str()) + .unwrap_or("") + .to_string(); + Some(AcpEvent::Thinking(text)) } "tool_call" => { let title = update.get("title").and_then(|v| v.as_str()).unwrap_or("").to_string(); diff --git a/src/discord.rs b/src/discord.rs index 27f2e3e..201500c 100644 --- a/src/discord.rs +++ b/src/discord.rs @@ -210,6 +210,10 @@ async fn stream_prompt( let (buf_tx, buf_rx) = watch::channel(initial); let mut text_buf = String::new(); + // Accumulated thinking content from agent_thought_chunk events. + // Rendered above tool_lines + text as a blockquote so users can + // see the bot's reasoning before / alongside its actions. + let mut thought_buf = String::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 @@ -274,10 +278,29 @@ async fn stream_prompt( // Reaction: back to thinking after tools } text_buf.push_str(&t); - let _ = buf_tx.send(compose_display(&tool_lines, &text_buf)); + let _ = buf_tx.send(compose_display(&tool_lines, &thought_buf, &text_buf)); } - AcpEvent::Thinking => { + AcpEvent::Thinking(t) => { reactions.set_thinking().await; + if !t.is_empty() { + // claude-agent-acp emits both `thinking` + // (new block) and `thinking_delta` + // (continuation) events as the same + // `agent_thought_chunk` shape, so we can't + // tell block boundaries from the protocol + // alone. Heuristic: if the previous chunk + // ended with sentence-ending punctuation + // and the new one starts with a letter + // (no leading whitespace), it's almost + // certainly a new thinking block — insert + // a paragraph break so they don't visually + // run together as ".There's". + if needs_thinking_separator(&thought_buf, &t) { + thought_buf.push_str("\n\n"); + } + thought_buf.push_str(&t); + let _ = buf_tx.send(compose_display(&tool_lines, &thought_buf, &text_buf)); + } } AcpEvent::ToolStart { id, title } if !title.is_empty() => { reactions.set_tool(&title).await; @@ -299,7 +322,7 @@ async fn stream_prompt( state: ToolState::Running, }); } - let _ = buf_tx.send(compose_display(&tool_lines, &text_buf)); + let _ = buf_tx.send(compose_display(&tool_lines, &thought_buf, &text_buf)); } AcpEvent::ToolDone { id, title, status } => { reactions.set_thinking().await; @@ -327,7 +350,7 @@ async fn stream_prompt( state: new_state, }); } - let _ = buf_tx.send(compose_display(&tool_lines, &text_buf)); + let _ = buf_tx.send(compose_display(&tool_lines, &thought_buf, &text_buf)); } _ => {} } @@ -339,7 +362,7 @@ async fn stream_prompt( let _ = edit_handle.await; // Final edit - let final_content = compose_display(&tool_lines, &text_buf); + let final_content = compose_display(&tool_lines, &thought_buf, &text_buf); let final_content = if final_content.is_empty() { "_(no response)_".to_string() } else { @@ -397,8 +420,41 @@ impl ToolEntry { } } -fn compose_display(tool_lines: &[ToolEntry], text: &str) -> String { +/// Heuristic for detecting a thinking block boundary in a stream of +/// `agent_thought_chunk` deltas. claude-agent-acp doesn't tag the first +/// delta of a new block, so we infer it: if the previous chunk ended in +/// sentence-ending punctuation (`. ! ? 。 ! ?`) and the new chunk starts +/// with a letter (no leading whitespace), they're almost certainly two +/// separate thoughts that need a paragraph break. +fn needs_thinking_separator(prev: &str, new: &str) -> bool { + if prev.is_empty() || new.is_empty() { + return false; + } + let last = match prev.chars().last() { + Some(c) => c, + None => return false, + }; + let first = match new.chars().next() { + Some(c) => c, + None => return false, + }; + let sentence_end = matches!(last, '.' | '!' | '?' | '。' | '！' | '？'); + let starts_word = first.is_alphabetic(); + sentence_end && starts_word +} + +fn compose_display(tool_lines: &[ToolEntry], thought: &str, text: &str) -> String { let mut out = String::new(); + let trimmed_thought = thought.trim(); + if !trimmed_thought.is_empty() { + out.push_str("> 🤔 _Thinking_\n"); + for line in trimmed_thought.lines() { + out.push_str("> "); + out.push_str(line); + out.push('\n'); + } + out.push('\n'); + } if !tool_lines.is_empty() { for entry in tool_lines { out.push_str(&entry.render());