From 5a4a1595c61db8965c9eb1011a9fd141b343c92f Mon Sep 17 00:00:00 2001 From: howie <2318485+howie@users.noreply.github.com> Date: Mon, 18 May 2026 10:42:56 +0800 Subject: [PATCH 1/5] =?UTF-8?q?fix(adapter):=20silence=20handling=20?= =?UTF-8?q?=E2=80=94=20sentinel,=20empty-reply=20config,=20diagnostic=20lo?= =?UTF-8?q?gging=20(#836)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three related fixes, all in the platform-agnostic AdapterRouter layer. All current adapters (Discord, Slack, gateway-relayed Feishu/Telegram/LINE/Teams/WeChat) inherit the fixes automatically — no per-adapter changes needed. **Fix 1 — sentinel** Steering can now instruct agents to output `` to stay silent. AdapterRouter detects the sentinel post-loop (not per-chunk, to avoid partial-buffer false positives), deletes any streaming placeholder, and returns Ok without posting. **Fix 2 — emptyReplyPlaceholder config** New `[reactions] empty_reply_placeholder` field (default true, backward-compatible). When false, empty replies with no error are suppressed instead of posting "_(no response)_". Rendered via Helm configmap template; commented example added to values.yaml. Pairs with the sentinel: both share the same "delete placeholder + early return" path. **Fix 3 — diagnostic logging** Three tracing::warn! lines added at the response_error assignment sites: agent process died, hard timeout exceeded, JSON-RPC error. Zero behaviour change. Each log includes `platform` so entries are filterable across adapter types. Addresses the "two agents returned _(no response)_ with no server-side evidence" case. Also adds docs/steering/silence.md — agent steering template explaining usage and the multi-mention sub-case (Bug 1B in the issue). Co-Authored-By: Claude Opus 4.7 --- charts/openab/templates/configmap.yaml | 1 + charts/openab/values.yaml | 2 + docs/steering/silence.md | 31 +++++++++++++++ src/adapter.rs | 54 +++++++++++++++++++++++++- src/config.rs | 27 +++++++++++++ 5 files changed, 114 insertions(+), 1 deletion(-) create mode 100644 docs/steering/silence.md diff --git a/charts/openab/templates/configmap.yaml b/charts/openab/templates/configmap.yaml index ebbdb758..d9676775 100644 --- a/charts/openab/templates/configmap.yaml +++ b/charts/openab/templates/configmap.yaml @@ -162,6 +162,7 @@ data: [reactions] enabled = {{ if hasKey ($cfg.reactions) "enabled" }}{{ ($cfg.reactions).enabled }}{{ else }}true{{ end }} remove_after_reply = {{ if hasKey ($cfg.reactions) "removeAfterReply" }}{{ ($cfg.reactions).removeAfterReply }}{{ else }}false{{ end }} + empty_reply_placeholder = {{ if hasKey ($cfg.reactions) "emptyReplyPlaceholder" }}{{ ($cfg.reactions).emptyReplyPlaceholder }}{{ else }}true{{ end }} {{- if ($cfg.reactions).toolDisplay }} {{- if not (has ($cfg.reactions).toolDisplay (list "full" "compact" "none")) }} {{- fail (printf "agents.%s.reactions.toolDisplay must be one of: full, compact, none — got: %s" $name ($cfg.reactions).toolDisplay) }} diff --git a/charts/openab/values.yaml b/charts/openab/values.yaml index 8e4f2fa9..156bb006 100644 --- a/charts/openab/values.yaml +++ b/charts/openab/values.yaml @@ -282,6 +282,8 @@ agents: # compact: show count summary (e.g. ✅ 3 · 🔧 1 tool(s)) # full: show complete tool titles (for debugging) # none: hide tool lines entirely + # emptyReplyPlaceholder: true # set to false to silently suppress empty replies + # instead of posting "_(no response)_". Pairs with sentinel steering. stt: enabled: false apiKey: "" diff --git a/docs/steering/silence.md b/docs/steering/silence.md new file mode 100644 index 00000000..7bb826b0 --- /dev/null +++ b/docs/steering/silence.md @@ -0,0 +1,31 @@ +# Silence Steering + +## When to Stay Silent + +You may receive messages addressed to other agents, or batched messages where some +entries are not intended for you. In those cases, staying silent is the correct +behaviour. + +## How to Signal Silence + +**Output exactly `` and nothing else.** + +``` + +``` + +The gateway detects this sentinel and suppresses the message before posting to the +channel. No user-visible output is produced and no placeholder is left behind. + +**Do not** explain or justify staying silent. Do not write things like: +- "I'm staying silent because..." +- "This message wasn't addressed to me." +- "No response needed." + +Any text other than the exact sentinel string will be posted to the channel. + +## Sub-case: Multiple Bot Mentions + +When a message mentions several bots, read the full mention list before deciding +whether your UID is present. Do not infer "I should stay silent" solely because +other bot UIDs appear — check whether your own UID is also in the list. diff --git a/src/adapter.rs b/src/adapter.rs index c8a2be45..139f177a 100644 --- a/src/adapter.rs +++ b/src/adapter.rs @@ -2,7 +2,7 @@ use anyhow::Result; use async_trait::async_trait; use serde::Serialize; use std::sync::Arc; -use tracing::{error, warn}; +use tracing::{debug, error, info, warn}; use crate::acp::{classify_notification, AcpEvent, ContentBlock, SessionPool}; use crate::config::{ReactionsConfig, ToolDisplay}; @@ -462,6 +462,7 @@ impl AdapterRouter { let streaming = adapter.use_streaming(other_bot_present); let table_mode = self.table_mode; let tool_display = self.reactions_config.tool_display; + let empty_reply_placeholder = self.reactions_config.empty_reply_placeholder; let prompt_hard_timeout = self.prompt_hard_timeout; let liveness_check_interval = self.liveness_check_interval; @@ -540,6 +541,7 @@ impl AdapterRouter { _ = tokio::time::sleep(liveness_check_interval) => { if !conn.alive() { response_error = Some("Agent process died".into()); + warn!(platform = %adapter.platform(), "agent process died mid-prompt"); conn.abandon_request(request_id).await; break; } @@ -548,6 +550,11 @@ impl AdapterRouter { "Agent exceeded hard timeout ({}s)", prompt_hard_timeout.as_secs(), )); + warn!( + platform = %adapter.platform(), + elapsed_s = prompt_start.elapsed().as_secs(), + "agent hard timeout exceeded" + ); conn.abandon_request(request_id).await; break; } @@ -565,6 +572,12 @@ impl AdapterRouter { } if let Some(ref err) = notification.error { response_error = Some(format_coded_error(err.code, &err.message)); + warn!( + platform = %adapter.platform(), + code = err.code, + message = %err.message, + "agent JSON-RPC error" + ); } break; } @@ -653,12 +666,29 @@ impl AdapterRouter { let (directives, stripped_text) = parse_output_directives(&text_buf); let text_buf = stripped_text; + // Sentinel: agent explicitly chose silence — suppress reply and clean up placeholder. + // Checked post-loop on the complete buffer; individual streaming chunks may + // transiently contain this substring and must not be filtered mid-stream. + if text_buf.trim() == "" { + info!(platform = %adapter.platform(), "agent emitted sentinel -- suppressing reply"); + if let Some(msg) = placeholder_msg.as_ref() { + let _ = adapter.delete_message(msg).await; + } + return Ok(()); + } + // Build final content let final_content = compose_display(&tool_lines, &text_buf, false, tool_display); let final_content = if final_content.is_empty() { if let Some(err) = response_error { format!("⚠️ {err}") + } else if !empty_reply_placeholder { + debug!(platform = %adapter.platform(), "empty reply suppressed by empty_reply_placeholder=false"); + if let Some(msg) = placeholder_msg.as_ref() { + let _ = adapter.delete_message(msg).await; + } + return Ok(()); } else { "_(no response)_".to_string() } @@ -1195,4 +1225,26 @@ mod directive_tests { assert_eq!(directives.reply_to, Some("456".to_string())); assert_eq!(content, "看看 [[這個]] 怎麼樣"); } + + #[test] + fn silent_sentinel_passes_through_directive_parser() { + // is not a [[key:value]] directive — parse_output_directives must + // return it unchanged so the post-loop sentinel check can match it. + let input = ""; + let (directives, content) = parse_output_directives(input); + assert_eq!(directives.reply_to, None); + assert_eq!(content, ""); + } + + #[test] + fn silent_sentinel_with_whitespace_passes_through() { + // Leading/trailing whitespace and newline variants must survive directive parsing. + for input in &[" ", "\n", "\n"] { + let (_, content) = parse_output_directives(input); + assert!( + content.trim() == "", + "expected trimmed content to equal '' for input {input:?}, got {content:?}" + ); + } + } } diff --git a/src/config.rs b/src/config.rs index 2187f0d7..5e499d6d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -408,6 +408,10 @@ pub struct ReactionsConfig { pub emojis: ReactionEmojis, #[serde(default)] pub timing: ReactionTiming, + /// When false, empty replies (no error) are silently suppressed instead of + /// posting "_(no response)_". Default true preserves existing behaviour. + #[serde(default = "default_true")] + pub empty_reply_placeholder: bool, } #[derive(Debug, Clone, Deserialize)] @@ -922,4 +926,27 @@ echo_transcript = false assert!(cfg.stt.enabled); assert!(!cfg.stt.echo_transcript); } + + #[test] + fn empty_reply_placeholder_defaults_to_true() { + let toml = r#" +[agent] +command = "echo" +"#; + let cfg = parse_config(toml, "test").unwrap(); + assert!(cfg.reactions.empty_reply_placeholder, "default must be true for backward compat"); + } + + #[test] + fn empty_reply_placeholder_can_be_set_to_false() { + let toml = r#" +[agent] +command = "echo" + +[reactions] +empty_reply_placeholder = false +"#; + let cfg = parse_config(toml, "test").unwrap(); + assert!(!cfg.reactions.empty_reply_placeholder); + } } From c38587d86ce1201c54b38b7becde35aefd9f8a11 Mon Sep 17 00:00:00 2001 From: howie <2318485+howie@users.noreply.github.com> Date: Mon, 18 May 2026 11:12:50 +0800 Subject: [PATCH 2/5] refactor(adapter): simplify per /simplify review - Trim verbose sentinel comment to single-line WHY - Fix stringly-typed boolean in debug log message - Spawn placeholder delete to release connection slot immediately (mirrors reactions.clear() spawn pattern at lines 412-416) Co-Authored-By: Claude Opus 4.7 --- src/adapter.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/adapter.rs b/src/adapter.rs index 139f177a..8b8723d8 100644 --- a/src/adapter.rs +++ b/src/adapter.rs @@ -666,13 +666,12 @@ impl AdapterRouter { let (directives, stripped_text) = parse_output_directives(&text_buf); let text_buf = stripped_text; - // Sentinel: agent explicitly chose silence — suppress reply and clean up placeholder. - // Checked post-loop on the complete buffer; individual streaming chunks may - // transiently contain this substring and must not be filtered mid-stream. + // Sentinel: checked post-loop — chunks may transiently match mid-stream. if text_buf.trim() == "" { info!(platform = %adapter.platform(), "agent emitted sentinel -- suppressing reply"); - if let Some(msg) = placeholder_msg.as_ref() { - let _ = adapter.delete_message(msg).await; + if let Some(msg) = placeholder_msg { + let a = adapter.clone(); + tokio::spawn(async move { let _ = a.delete_message(&msg).await; }); } return Ok(()); } @@ -684,9 +683,10 @@ impl AdapterRouter { if let Some(err) = response_error { format!("⚠️ {err}") } else if !empty_reply_placeholder { - debug!(platform = %adapter.platform(), "empty reply suppressed by empty_reply_placeholder=false"); - if let Some(msg) = placeholder_msg.as_ref() { - let _ = adapter.delete_message(msg).await; + debug!(platform = %adapter.platform(), "empty reply suppressed; empty_reply_placeholder disabled"); + if let Some(msg) = placeholder_msg { + let a = adapter.clone(); + tokio::spawn(async move { let _ = a.delete_message(&msg).await; }); } return Ok(()); } else { From a97188c6f4d43b80c3a70743ae5370e14ce63a48 Mon Sep 17 00:00:00 2001 From: howie <2318485+howie@users.noreply.github.com> Date: Mon, 18 May 2026 15:37:25 +0800 Subject: [PATCH 3/5] fix(config): add missing empty_reply_placeholder to ReactionsConfig::Default E0063 compile error: struct update/Default impl was missing the new field. Co-Authored-By: Claude Sonnet 4.6 --- src/config.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/config.rs b/src/config.rs index 5e499d6d..d762c62b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -524,6 +524,7 @@ impl Default for ReactionsConfig { tool_display: ToolDisplay::default(), emojis: ReactionEmojis::default(), timing: ReactionTiming::default(), + empty_reply_placeholder: true, } } } From 3ae80ab742733d62e07676617c5120188e95d6f2 Mon Sep 17 00:00:00 2001 From: howie <2318485+howie@users.noreply.github.com> Date: Mon, 18 May 2026 15:47:37 +0800 Subject: [PATCH 4/5] fix(adapter): address session-reset sentinel, suppress() reactions, Slack delete Four fixes on top of the initial silence-handling commit: 1. Session reset + sentinel (Codex P2): text_buf was pre-populated with the reset prelude before agent output, so text_buf.trim() == "" never matched after a session reset. Track reset_prelude separately and prepend it to streaming updates and final_content after sentinel/suppress checks. 2. reactions.suppress() (Claude Important): early-return suppress paths returned Ok(()), causing dispatch.rs to call reactions.set_done() and add a done emoji to the user message even though no reply was posted. Added suppress() to StatusReactionController (clear + finished=true) so the subsequent set_done() is a no-op. 3. delete_message error logging (Claude Important): tokio::spawn blocks for placeholder deletion were silently discarding errors. Now emit warn! so failures appear in telemetry. 4. Slack delete_message (Codex P2): SlackAdapter lacked a delete_message override, falling back to edit_message with U+200B. Added chat.delete so the placeholder is truly removed rather than left as a blank message. Co-Authored-By: Claude Sonnet 4.6 --- src/adapter.rs | 66 +++++++++++++++++++++++++++++++++--------------- src/reactions.rs | 21 +++++++++++++++ src/slack.rs | 12 +++++++++ 3 files changed, 78 insertions(+), 21 deletions(-) diff --git a/src/adapter.rs b/src/adapter.rs index 8b8723d8..e530b2c1 100644 --- a/src/adapter.rs +++ b/src/adapter.rs @@ -478,10 +478,14 @@ impl AdapterRouter { let mut text_buf = String::new(); let mut tool_lines: Vec = Vec::new(); - - if reset { - text_buf.push_str("⚠️ _Session expired, starting fresh..._\n\n"); - } + // Kept separate from text_buf so the sentinel check ("is the + // agent's actual output ?") is not confused by the + // synthetic prelude. Prepended to final_content before send. + let reset_prelude = if reset { + "⚠️ _Session expired, starting fresh..._\n\n" + } else { + "" + }; // Streaming edit: send placeholder, spawn edit loop let (buf_tx, placeholder_msg) = if streaming { @@ -587,11 +591,14 @@ impl AdapterRouter { AcpEvent::Text(t) => { text_buf.push_str(&t); if let Some(tx) = &buf_tx { - let _ = tx.send(compose_display( - &tool_lines, - &text_buf, - true, - tool_display, + let _ = tx.send(format!( + "{reset_prelude}{}", + compose_display( + &tool_lines, + &text_buf, + true, + tool_display, + ) )); } } @@ -612,11 +619,14 @@ impl AdapterRouter { }); } if let Some(tx) = &buf_tx { - let _ = tx.send(compose_display( - &tool_lines, - &text_buf, - true, - tool_display, + let _ = tx.send(format!( + "{reset_prelude}{}", + compose_display( + &tool_lines, + &text_buf, + true, + tool_display, + ) )); } } @@ -640,11 +650,14 @@ impl AdapterRouter { }); } if let Some(tx) = &buf_tx { - let _ = tx.send(compose_display( - &tool_lines, - &text_buf, - true, - tool_display, + let _ = tx.send(format!( + "{reset_prelude}{}", + compose_display( + &tool_lines, + &text_buf, + true, + tool_display, + ) )); } } @@ -669,9 +682,14 @@ impl AdapterRouter { // Sentinel: checked post-loop — chunks may transiently match mid-stream. if text_buf.trim() == "" { info!(platform = %adapter.platform(), "agent emitted sentinel -- suppressing reply"); + reactions.suppress().await; if let Some(msg) = placeholder_msg { let a = adapter.clone(); - tokio::spawn(async move { let _ = a.delete_message(&msg).await; }); + tokio::spawn(async move { + if let Err(e) = a.delete_message(&msg).await { + warn!(error = ?e, "delete placeholder failed after silent sentinel"); + } + }); } return Ok(()); } @@ -684,9 +702,14 @@ impl AdapterRouter { format!("⚠️ {err}") } else if !empty_reply_placeholder { debug!(platform = %adapter.platform(), "empty reply suppressed; empty_reply_placeholder disabled"); + reactions.suppress().await; if let Some(msg) = placeholder_msg { let a = adapter.clone(); - tokio::spawn(async move { let _ = a.delete_message(&msg).await; }); + tokio::spawn(async move { + if let Err(e) = a.delete_message(&msg).await { + warn!(error = ?e, "delete placeholder failed after empty reply suppression"); + } + }); } return Ok(()); } else { @@ -698,6 +721,7 @@ impl AdapterRouter { final_content }; + let final_content = format!("{reset_prelude}{final_content}"); let final_content = markdown::convert_tables(&final_content, table_mode); let chunks = format::split_message(&final_content, message_limit); if let Some(msg) = placeholder_msg { diff --git a/src/reactions.rs b/src/reactions.rs index 6e68f90b..68ccee34 100644 --- a/src/reactions.rs +++ b/src/reactions.rs @@ -129,6 +129,27 @@ impl StatusReactionController { } } + /// Remove the current reaction, cancel all timers, and mark as finished so + /// subsequent set_done/set_error calls are no-ops. Used when a reply is + /// suppressed (sentinel or empty_reply_placeholder=false) — prevents the + /// done emoji from appearing on a message the user never saw a reply for. + pub async fn suppress(&self) { + if !self.enabled { + return; + } + let mut inner = self.inner.lock().await; + inner.finished = true; + cancel_timers(&mut inner); + let current = inner.current.clone(); + if !current.is_empty() { + let _ = inner + .adapter + .remove_reaction(&inner.message, ¤t) + .await; + inner.current.clear(); + } + } + async fn apply_immediate(&self, emoji: &str) { let mut inner = self.inner.lock().await; if inner.finished || emoji == inner.current { diff --git a/src/slack.rs b/src/slack.rs index 47d4c42d..32ba8e1b 100644 --- a/src/slack.rs +++ b/src/slack.rs @@ -447,6 +447,18 @@ impl ChatAdapter for SlackAdapter { } } + async fn delete_message(&self, msg: &MessageRef) -> Result<()> { + self.api_post( + "chat.delete", + serde_json::json!({ + "channel": msg.channel.channel_id, + "ts": msg.message_id, + }), + ) + .await?; + Ok(()) + } + async fn edit_message(&self, msg: &MessageRef, content: &str) -> Result<()> { let mrkdwn = markdown_to_mrkdwn(content); self.api_post( From 868adcb4ed328cfe193afd0687ea01dda2c80e48 Mon Sep 17 00:00:00 2001 From: howie <2318485+howie@users.noreply.github.com> Date: Mon, 18 May 2026 15:57:15 +0800 Subject: [PATCH 5/5] fix(reactions): block streaming sentinel flash, guard clear() on finished, fix mood face Five fixes found by group review R1: 1. Streaming sentinel flash (Critical): the edit loop fires every 1500ms and would briefly show "" text in the placeholder before delete_message ran. Now skip buf_tx.send() when text_buf.trim() == "". 2. clear() missing finished guard (Important): after suppress() sets finished=true, a subsequent clear() call (e.g. from dispatch's remove_after_reply path) would fire an extra remove_reaction API call on an already-cleared controller. Added if inner.finished { return; } guard. 3. suppress() remove_reaction error logging (Important): silent discard of the remove_reaction failure meant a stuck emoji would leave no trace in telemetry. Now emits tracing::warn! on failure, consistent with delete_message logging. 4. set_done() mood face fires after suppress() (Critical test finding): the mood face addition code ran unconditionally after finish(), which is a no-op when already finished. Added guard: only add face if inner.current == emoji (i.e. finish() actually ran, not already suppressed). 5. reactions tests (Critical gap): added #[cfg(test)] module with MockAdapter recording add_reaction/remove_reaction calls. Two tests verify: - suppress() removes current reaction and blocks subsequent set_done() - clear() is a no-op after suppress() Co-Authored-By: Claude Sonnet 4.6 --- src/adapter.rs | 24 ++++++---- src/reactions.rs | 122 ++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 131 insertions(+), 15 deletions(-) diff --git a/src/adapter.rs b/src/adapter.rs index e530b2c1..cf7a6835 100644 --- a/src/adapter.rs +++ b/src/adapter.rs @@ -590,16 +590,22 @@ impl AdapterRouter { match event { AcpEvent::Text(t) => { text_buf.push_str(&t); + // Don't stream potential-sentinel content to the edit + // loop — if the agent is outputting the + // placeholder should stay as "…" until delete fires, + // not flash the literal sentinel text to users. if let Some(tx) = &buf_tx { - let _ = tx.send(format!( - "{reset_prelude}{}", - compose_display( - &tool_lines, - &text_buf, - true, - tool_display, - ) - )); + if text_buf.trim() != "" { + let _ = tx.send(format!( + "{reset_prelude}{}", + compose_display( + &tool_lines, + &text_buf, + true, + tool_display, + ) + )); + } } } AcpEvent::Thinking => { diff --git a/src/reactions.rs b/src/reactions.rs index 68ccee34..ecde46b8 100644 --- a/src/reactions.rs +++ b/src/reactions.rs @@ -98,11 +98,15 @@ impl StatusReactionController { } let emoji = { self.inner.lock().await.emojis.done.clone() }; self.finish(&emoji).await; - // Add a random mood face + // Add a random mood face — only if finish() actually ran (inner.current == emoji). + // If suppress() was called first, finished=true and current was cleared, so + // current != emoji and we skip the face to avoid reacting on a suppressed reply. let faces = ["😊", "😎", "🫡", "🤓", "😏", "✌️", "💪", "🦾"]; let face = faces[rand::random::() % faces.len()]; let inner = self.inner.lock().await; - let _ = inner.adapter.add_reaction(&inner.message, face).await; + if inner.current == emoji { + let _ = inner.adapter.add_reaction(&inner.message, face).await; + } } pub async fn set_error(&self) { @@ -118,6 +122,9 @@ impl StatusReactionController { return; } let mut inner = self.inner.lock().await; + if inner.finished { + return; + } cancel_timers(&mut inner); let current = inner.current.clone(); if !current.is_empty() { @@ -142,10 +149,9 @@ impl StatusReactionController { cancel_timers(&mut inner); let current = inner.current.clone(); if !current.is_empty() { - let _ = inner - .adapter - .remove_reaction(&inner.message, ¤t) - .await; + if let Err(e) = inner.adapter.remove_reaction(&inner.message, ¤t).await { + tracing::warn!(error = ?e, "suppress: failed to remove reaction"); + } inner.current.clear(); } } @@ -295,3 +301,107 @@ fn cancel_timers(inner: &mut Inner) { h.abort(); } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::adapter::{ChannelRef, MessageRef}; + use async_trait::async_trait; + use std::sync::Arc; + use tokio::sync::Mutex; + + /// Minimal mock adapter that records add_reaction / remove_reaction calls. + struct MockAdapter { + calls: Arc>>, + } + + impl MockAdapter { + fn new() -> (Arc, Arc>>) { + let calls = Arc::new(Mutex::new(Vec::new())); + (Arc::new(Self { calls: calls.clone() }), calls) + } + } + + #[async_trait] + impl ChatAdapter for MockAdapter { + fn platform(&self) -> &'static str { "mock" } + fn message_limit(&self) -> usize { 2000 } + async fn send_message(&self, _ch: &ChannelRef, _content: &str) -> anyhow::Result { + unimplemented!() + } + async fn create_thread(&self, _ch: &ChannelRef, _trigger: &MessageRef, _title: &str) -> anyhow::Result { + unimplemented!() + } + async fn add_reaction(&self, _msg: &MessageRef, emoji: &str) -> anyhow::Result<()> { + self.calls.lock().await.push(format!("add:{emoji}")); + Ok(()) + } + async fn remove_reaction(&self, _msg: &MessageRef, emoji: &str) -> anyhow::Result<()> { + self.calls.lock().await.push(format!("remove:{emoji}")); + Ok(()) + } + fn use_streaming(&self, _other_bot_present: bool) -> bool { false } + } + + fn make_message_ref() -> MessageRef { + MessageRef { + channel: ChannelRef { + platform: "mock".into(), + channel_id: "C1".into(), + thread_id: None, + parent_id: None, + origin_event_id: None, + }, + message_id: "M1".into(), + } + } + + fn make_controller(adapter: Arc) -> StatusReactionController { + StatusReactionController::new( + true, + adapter, + make_message_ref(), + ReactionEmojis::default(), + ReactionTiming::default(), + ) + } + + #[tokio::test] + async fn suppress_removes_current_reaction_and_blocks_set_done() { + let (adapter, calls) = MockAdapter::new(); + let ctrl = make_controller(adapter); + + // set_queued uses apply_immediate (no debounce) so the reaction is + // present synchronously before suppress() is called. + ctrl.set_queued().await; + + ctrl.suppress().await; + + // suppress() must remove the current reaction + let snapshot = calls.lock().await.clone(); + let removed = snapshot.iter().any(|c| c.starts_with("remove:")); + assert!(removed, "suppress() should remove the current reaction; calls: {snapshot:?}"); + + // set_done() must be a no-op after suppress() + ctrl.set_done().await; + let after_done = calls.lock().await.clone(); + let new_adds: Vec<_> = after_done[snapshot.len()..].iter().filter(|c| c.starts_with("add:")).collect(); + assert!(new_adds.is_empty(), "set_done() must be no-op after suppress(); new add calls: {new_adds:?}"); + } + + #[tokio::test] + async fn clear_is_no_op_after_suppress() { + let (adapter, calls) = MockAdapter::new(); + let ctrl = make_controller(adapter); + + ctrl.set_queued().await; + + ctrl.suppress().await; + let after_suppress = calls.lock().await.len(); + + // clear() must not fire additional API calls after suppress() + ctrl.clear().await; + let after_clear = calls.lock().await.len(); + assert_eq!(after_suppress, after_clear, "clear() must be no-op after suppress()"); + } +}