From 52e0271bd58b6427ed2629a88127f50ca6d675fa Mon Sep 17 00:00:00 2001 From: Giulio V Date: Sat, 21 Mar 2026 15:15:25 +0100 Subject: [PATCH 1/5] feat(tools): add emoji reaction tool for cross-channel reactions Add ReactionTool that exposes Channel::add_reaction and Channel::remove_reaction as an agent-callable tool. Uses a late-binding ChannelMapHandle (Arc>) pattern so the tool can be constructed during tool registry init and populated once channels are available in start_channels. Parameters: channel, message_id, emoji, action (add/remove). Co-Authored-By: Claude Opus 4.6 (1M context) --- src/agent/agent.rs | 2 +- src/agent/loop_.rs | 33 +-- src/channels/mod.rs | 40 ++-- src/gateway/mod.rs | 31 +-- src/tools/mod.rs | 43 +++- src/tools/reaction.rs | 482 ++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 573 insertions(+), 58 deletions(-) create mode 100644 src/tools/reaction.rs diff --git a/src/agent/agent.rs b/src/agent/agent.rs index 763486ab7..11a5f2515 100644 --- a/src/agent/agent.rs +++ b/src/agent/agent.rs @@ -359,7 +359,7 @@ impl Agent { None }; - let (mut tools, delegate_handle) = tools::all_tools_with_runtime( + let (mut tools, delegate_handle, _reaction_handle) = tools::all_tools_with_runtime( Arc::new(config.clone()), &security, runtime, diff --git a/src/agent/loop_.rs b/src/agent/loop_.rs index 7ec005d38..4b67df1a5 100644 --- a/src/agent/loop_.rs +++ b/src/agent/loop_.rs @@ -3525,7 +3525,7 @@ pub async fn run( } else { (None, None) }; - let (mut tools_registry, delegate_handle) = tools::all_tools_with_runtime( + let (mut tools_registry, delegate_handle, _reaction_handle) = tools::all_tools_with_runtime( Arc::new(config.clone()), &security, runtime, @@ -4282,21 +4282,22 @@ pub async fn process_message( } else { (None, None) }; - let (mut tools_registry, delegate_handle_pm) = tools::all_tools_with_runtime( - Arc::new(config.clone()), - &security, - runtime, - mem.clone(), - composio_key, - composio_entity_id, - &config.browser, - &config.http_request, - &config.web_fetch, - &config.workspace_dir, - &config.agents, - config.api_key.as_deref(), - &config, - ); + let (mut tools_registry, delegate_handle_pm, _reaction_handle_pm) = + tools::all_tools_with_runtime( + Arc::new(config.clone()), + &security, + runtime, + mem.clone(), + composio_key, + composio_entity_id, + &config.browser, + &config.http_request, + &config.web_fetch, + &config.workspace_dir, + &config.agents, + config.api_key.as_deref(), + &config, + ); let peripheral_tools: Vec> = crate::peripherals::create_peripheral_tools(&config.peripherals).await?; tools_registry.extend(peripheral_tools); diff --git a/src/channels/mod.rs b/src/channels/mod.rs index 8311b0786..a83e927b4 100644 --- a/src/channels/mod.rs +++ b/src/channels/mod.rs @@ -4264,22 +4264,21 @@ pub async fn start_channels(config: Config) -> Result<()> { }; // Build system prompt from workspace identity files + skills let workspace = config.workspace_dir.clone(); - let (mut built_tools, delegate_handle_ch): (Vec>, _) = - tools::all_tools_with_runtime( - Arc::new(config.clone()), - &security, - runtime, - Arc::clone(&mem), - composio_key, - composio_entity_id, - &config.browser, - &config.http_request, - &config.web_fetch, - &workspace, - &config.agents, - config.api_key.as_deref(), - &config, - ); + let (mut built_tools, delegate_handle_ch, reaction_handle_ch) = tools::all_tools_with_runtime( + Arc::new(config.clone()), + &security, + runtime, + Arc::clone(&mem), + composio_key, + composio_entity_id, + &config.browser, + &config.http_request, + &config.web_fetch, + &workspace, + &config.agents, + config.api_key.as_deref(), + &config, + ); // Wire MCP tools into the registry before freezing — non-fatal. // When `deferred_loading` is enabled, MCP tools are NOT added eagerly. @@ -4551,6 +4550,15 @@ pub async fn start_channels(config: Config) -> Result<()> { .map(|ch| (ch.name().to_string(), Arc::clone(ch))) .collect::>(), ); + + // Populate the reaction tool's channel map now that channels are initialized. + if let Some(ref handle) = reaction_handle_ch { + let mut map = handle.write(); + for (name, ch) in channels_by_name.as_ref() { + map.insert(name.clone(), Arc::clone(ch)); + } + } + let max_in_flight_messages = compute_max_in_flight_messages(channels.len()); println!(" 🚦 In-flight message limit: {max_in_flight_messages}"); diff --git a/src/gateway/mod.rs b/src/gateway/mod.rs index 3feffd2ed..c06eb2584 100644 --- a/src/gateway/mod.rs +++ b/src/gateway/mod.rs @@ -432,21 +432,22 @@ pub async fn run_gateway(host: &str, port: u16, config: Config) -> Result<()> { (None, None) }; - let (mut tools_registry_raw, delegate_handle_gw) = tools::all_tools_with_runtime( - Arc::new(config.clone()), - &security, - runtime, - Arc::clone(&mem), - composio_key, - composio_entity_id, - &config.browser, - &config.http_request, - &config.web_fetch, - &config.workspace_dir, - &config.agents, - config.api_key.as_deref(), - &config, - ); + let (mut tools_registry_raw, delegate_handle_gw, _reaction_handle_gw) = + tools::all_tools_with_runtime( + Arc::new(config.clone()), + &security, + runtime, + Arc::clone(&mem), + composio_key, + composio_entity_id, + &config.browser, + &config.http_request, + &config.web_fetch, + &config.workspace_dir, + &config.agents, + config.api_key.as_deref(), + &config, + ); // ── Wire MCP tools into the gateway tool registry (non-fatal) ─── // Without this, the `/api/tools` endpoint misses MCP tools. diff --git a/src/tools/mod.rs b/src/tools/mod.rs index ed2b51d57..385b6786f 100644 --- a/src/tools/mod.rs +++ b/src/tools/mod.rs @@ -69,6 +69,7 @@ pub mod pdf_read; pub mod project_intel; pub mod proxy_config; pub mod pushover; +pub mod reaction; pub mod read_skill; pub mod report_templates; pub mod schedule; @@ -139,6 +140,7 @@ pub use pdf_read::PdfReadTool; pub use project_intel::ProjectIntelTool; pub use proxy_config::ProxyConfigTool; pub use pushover::PushoverTool; +pub use reaction::{ChannelMapHandle, ReactionTool}; pub use read_skill::ReadSkillTool; pub use schedule::ScheduleTool; #[allow(unused_imports)] @@ -262,7 +264,11 @@ pub fn all_tools( agents: &HashMap, fallback_api_key: Option<&str>, root_config: &crate::config::Config, -) -> (Vec>, Option) { +) -> ( + Vec>, + Option, + Option, +) { all_tools_with_runtime( config, security, @@ -296,7 +302,11 @@ pub fn all_tools_with_runtime( agents: &HashMap, fallback_api_key: Option<&str>, root_config: &crate::config::Config, -) -> (Vec>, Option) { +) -> ( + Vec>, + Option, + Option, +) { let has_shell_access = runtime.has_shell_access(); let sandbox = create_sandbox(&root_config.security); let mut tool_arcs: Vec> = vec![ @@ -566,6 +576,11 @@ pub fn all_tools_with_runtime( } } + // Emoji reaction tool — always registered; channel map populated later by start_channels. + let reaction_tool = ReactionTool::new(security.clone()); + let reaction_handle = reaction_tool.channel_map_handle(); + tool_arcs.push(Arc::new(reaction_tool)); + // Microsoft 365 Graph API integration if root_config.microsoft365.enabled { let ms_cfg = &root_config.microsoft365; @@ -592,7 +607,11 @@ pub fn all_tools_with_runtime( tracing::error!( "microsoft365: client_credentials auth_flow requires a non-empty client_secret" ); - return (boxed_registry_from_arcs(tool_arcs), None); + return ( + boxed_registry_from_arcs(tool_arcs), + None, + Some(reaction_handle), + ); } let resolved = microsoft365::types::Microsoft365ResolvedConfig { @@ -776,7 +795,11 @@ pub fn all_tools_with_runtime( } } - (boxed_registry_from_arcs(tool_arcs), delegate_handle) + ( + boxed_registry_from_arcs(tool_arcs), + delegate_handle, + Some(reaction_handle), + ) } #[cfg(test)] @@ -820,7 +843,7 @@ mod tests { let http = crate::config::HttpRequestConfig::default(); let cfg = test_config(&tmp); - let (tools, _) = all_tools( + let (tools, _, _) = all_tools( Arc::new(Config::default()), &security, mem, @@ -862,7 +885,7 @@ mod tests { let http = crate::config::HttpRequestConfig::default(); let cfg = test_config(&tmp); - let (tools, _) = all_tools( + let (tools, _, _) = all_tools( Arc::new(Config::default()), &security, mem, @@ -1015,7 +1038,7 @@ mod tests { }, ); - let (tools, _) = all_tools( + let (tools, _, _) = all_tools( Arc::new(Config::default()), &security, mem, @@ -1048,7 +1071,7 @@ mod tests { let http = crate::config::HttpRequestConfig::default(); let cfg = test_config(&tmp); - let (tools, _) = all_tools( + let (tools, _, _) = all_tools( Arc::new(Config::default()), &security, mem, @@ -1082,7 +1105,7 @@ mod tests { let mut cfg = test_config(&tmp); cfg.skills.prompt_injection_mode = crate::config::SkillsPromptInjectionMode::Compact; - let (tools, _) = all_tools( + let (tools, _, _) = all_tools( Arc::new(cfg.clone()), &security, mem, @@ -1116,7 +1139,7 @@ mod tests { let mut cfg = test_config(&tmp); cfg.skills.prompt_injection_mode = crate::config::SkillsPromptInjectionMode::Full; - let (tools, _) = all_tools( + let (tools, _, _) = all_tools( Arc::new(cfg.clone()), &security, mem, diff --git a/src/tools/reaction.rs b/src/tools/reaction.rs new file mode 100644 index 000000000..22d3cbd2e --- /dev/null +++ b/src/tools/reaction.rs @@ -0,0 +1,482 @@ +//! Emoji reaction tool for cross-channel message reactions. +//! +//! Exposes `add_reaction` and `remove_reaction` from the [`Channel`] trait as an +//! agent-callable tool. The tool holds a late-binding channel map handle that is +//! populated once channels are initialized (after tool construction). This mirrors +//! the pattern used by [`DelegateTool`] for its parent-tools handle. + +use super::traits::{Tool, ToolResult}; +use crate::channels::traits::Channel; +use crate::security::policy::ToolOperation; +use crate::security::SecurityPolicy; +use async_trait::async_trait; +use parking_lot::RwLock; +use serde_json::json; +use std::collections::HashMap; +use std::sync::Arc; + +/// Shared handle to the channel map. Starts empty; populated once channels boot. +pub type ChannelMapHandle = Arc>>>; + +/// Agent-callable tool for adding or removing emoji reactions on messages. +pub struct ReactionTool { + channels: ChannelMapHandle, + security: Arc, +} + +impl ReactionTool { + /// Create a new reaction tool with an empty channel map. + /// Call [`populate`] or write to the returned [`ChannelMapHandle`] once channels + /// are available. + pub fn new(security: Arc) -> Self { + Self { + channels: Arc::new(RwLock::new(HashMap::new())), + security, + } + } + + /// Return the shared handle so callers can populate it after channel init. + pub fn channel_map_handle(&self) -> ChannelMapHandle { + Arc::clone(&self.channels) + } + + /// Convenience: populate the channel map from a pre-built map. + pub fn populate(&self, map: HashMap>) { + *self.channels.write() = map; + } +} + +#[async_trait] +impl Tool for ReactionTool { + fn name(&self) -> &str { + "reaction" + } + + fn description(&self) -> &str { + "Add or remove an emoji reaction on a message in any active channel. \ + Provide the channel name (e.g. 'discord', 'slack'), the platform message ID, \ + and the emoji (Unicode character or platform shortcode)." + } + + fn parameters_schema(&self) -> serde_json::Value { + json!({ + "type": "object", + "properties": { + "channel": { + "type": "string", + "description": "Name of the channel to react in (e.g. 'discord', 'slack', 'telegram')" + }, + "message_id": { + "type": "string", + "description": "Platform-scoped message identifier to react to" + }, + "emoji": { + "type": "string", + "description": "Emoji to react with (Unicode character or platform shortcode)" + }, + "action": { + "type": "string", + "enum": ["add", "remove"], + "description": "Whether to add or remove the reaction (default: 'add')" + } + }, + "required": ["channel", "message_id", "emoji"] + }) + } + + async fn execute(&self, args: serde_json::Value) -> anyhow::Result { + // Security gate + if let Err(error) = self + .security + .enforce_tool_operation(ToolOperation::Act, "reaction") + { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(error), + }); + } + + let channel_name = args + .get("channel") + .and_then(|v| v.as_str()) + .ok_or_else(|| anyhow::anyhow!("Missing 'channel' parameter"))?; + + let message_id = args + .get("message_id") + .and_then(|v| v.as_str()) + .ok_or_else(|| anyhow::anyhow!("Missing 'message_id' parameter"))?; + + let emoji = args + .get("emoji") + .and_then(|v| v.as_str()) + .ok_or_else(|| anyhow::anyhow!("Missing 'emoji' parameter"))?; + + let action = args.get("action").and_then(|v| v.as_str()).unwrap_or("add"); + + if action != "add" && action != "remove" { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!( + "Invalid action '{action}': must be 'add' or 'remove'" + )), + }); + } + + // Read-lock the channel map to find the target channel. + let channel = { + let map = self.channels.read(); + if map.is_empty() { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("No channels available yet (channels not initialized)".to_string()), + }); + } + match map.get(channel_name) { + Some(ch) => Arc::clone(ch), + None => { + let available: Vec = map.keys().cloned().collect(); + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!( + "Channel '{channel_name}' not found. Available channels: {}", + available.join(", ") + )), + }); + } + } + }; + + let result = if action == "add" { + channel.add_reaction(channel_name, message_id, emoji).await + } else { + channel + .remove_reaction(channel_name, message_id, emoji) + .await + }; + + match result { + Ok(()) => Ok(ToolResult { + success: true, + output: format!( + "Reaction {action}ed: {emoji} on message {message_id} in {channel_name}" + ), + error: None, + }), + Err(e) => Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!("Failed to {action} reaction: {e}")), + }), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::channels::traits::{ChannelMessage, SendMessage}; + use std::sync::atomic::{AtomicBool, Ordering}; + + struct MockChannel { + reaction_added: AtomicBool, + reaction_removed: AtomicBool, + fail_on_add: bool, + } + + impl MockChannel { + fn new() -> Self { + Self { + reaction_added: AtomicBool::new(false), + reaction_removed: AtomicBool::new(false), + fail_on_add: false, + } + } + + fn failing() -> Self { + Self { + reaction_added: AtomicBool::new(false), + reaction_removed: AtomicBool::new(false), + fail_on_add: true, + } + } + } + + #[async_trait] + impl Channel for MockChannel { + fn name(&self) -> &str { + "mock" + } + + async fn send(&self, _message: &SendMessage) -> anyhow::Result<()> { + Ok(()) + } + + async fn listen( + &self, + _tx: tokio::sync::mpsc::Sender, + ) -> anyhow::Result<()> { + Ok(()) + } + + async fn add_reaction( + &self, + _channel_id: &str, + _message_id: &str, + _emoji: &str, + ) -> anyhow::Result<()> { + if self.fail_on_add { + return Err(anyhow::anyhow!("API error: rate limited")); + } + self.reaction_added.store(true, Ordering::SeqCst); + Ok(()) + } + + async fn remove_reaction( + &self, + _channel_id: &str, + _message_id: &str, + _emoji: &str, + ) -> anyhow::Result<()> { + self.reaction_removed.store(true, Ordering::SeqCst); + Ok(()) + } + } + + fn make_tool_with_channels(channels: Vec<(&str, Arc)>) -> ReactionTool { + let tool = ReactionTool::new(Arc::new(SecurityPolicy::default())); + let map: HashMap> = channels + .into_iter() + .map(|(name, ch)| (name.to_string(), ch)) + .collect(); + tool.populate(map); + tool + } + + #[test] + fn tool_metadata() { + let tool = ReactionTool::new(Arc::new(SecurityPolicy::default())); + assert_eq!(tool.name(), "reaction"); + assert!(!tool.description().is_empty()); + let schema = tool.parameters_schema(); + assert_eq!(schema["type"], "object"); + assert!(schema["properties"]["channel"].is_object()); + assert!(schema["properties"]["message_id"].is_object()); + assert!(schema["properties"]["emoji"].is_object()); + assert!(schema["properties"]["action"].is_object()); + let required = schema["required"].as_array().unwrap(); + assert!(required.iter().any(|v| v == "channel")); + assert!(required.iter().any(|v| v == "message_id")); + assert!(required.iter().any(|v| v == "emoji")); + // action is optional (defaults to "add") + assert!(!required.iter().any(|v| v == "action")); + } + + #[tokio::test] + async fn add_reaction_success() { + let mock: Arc = Arc::new(MockChannel::new()); + let tool = make_tool_with_channels(vec![("discord", Arc::clone(&mock))]); + + let result = tool + .execute(json!({ + "channel": "discord", + "message_id": "msg_123", + "emoji": "\u{2705}" + })) + .await + .unwrap(); + + assert!(result.success); + assert!(result.output.contains("added")); + assert!(result.error.is_none()); + } + + #[tokio::test] + async fn remove_reaction_success() { + let mock: Arc = Arc::new(MockChannel::new()); + let tool = make_tool_with_channels(vec![("slack", Arc::clone(&mock))]); + + let result = tool + .execute(json!({ + "channel": "slack", + "message_id": "msg_456", + "emoji": "\u{1F440}", + "action": "remove" + })) + .await + .unwrap(); + + assert!(result.success); + assert!(result.output.contains("removed")); + } + + #[tokio::test] + async fn unknown_channel_returns_error() { + let tool = make_tool_with_channels(vec![( + "discord", + Arc::new(MockChannel::new()) as Arc, + )]); + + let result = tool + .execute(json!({ + "channel": "nonexistent", + "message_id": "msg_1", + "emoji": "\u{2705}" + })) + .await + .unwrap(); + + assert!(!result.success); + let err = result.error.as_deref().unwrap(); + assert!(err.contains("not found")); + assert!(err.contains("discord")); + } + + #[tokio::test] + async fn invalid_action_returns_error() { + let tool = make_tool_with_channels(vec![( + "discord", + Arc::new(MockChannel::new()) as Arc, + )]); + + let result = tool + .execute(json!({ + "channel": "discord", + "message_id": "msg_1", + "emoji": "\u{2705}", + "action": "toggle" + })) + .await + .unwrap(); + + assert!(!result.success); + assert!(result.error.as_deref().unwrap().contains("toggle")); + } + + #[tokio::test] + async fn channel_error_propagated() { + let mock: Arc = Arc::new(MockChannel::failing()); + let tool = make_tool_with_channels(vec![("discord", mock)]); + + let result = tool + .execute(json!({ + "channel": "discord", + "message_id": "msg_1", + "emoji": "\u{2705}" + })) + .await + .unwrap(); + + assert!(!result.success); + assert!(result.error.as_deref().unwrap().contains("rate limited")); + } + + #[tokio::test] + async fn missing_required_params() { + let tool = make_tool_with_channels(vec![( + "test", + Arc::new(MockChannel::new()) as Arc, + )]); + + // Missing channel + let result = tool.execute(json!({"message_id": "1", "emoji": "x"})).await; + assert!(result.is_err()); + + // Missing message_id + let result = tool.execute(json!({"channel": "a", "emoji": "x"})).await; + assert!(result.is_err()); + + // Missing emoji + let result = tool + .execute(json!({"channel": "a", "message_id": "1"})) + .await; + assert!(result.is_err()); + } + + #[tokio::test] + async fn empty_channels_returns_not_initialized() { + let tool = ReactionTool::new(Arc::new(SecurityPolicy::default())); + // No channels populated + + let result = tool + .execute(json!({ + "channel": "discord", + "message_id": "msg_1", + "emoji": "\u{2705}" + })) + .await + .unwrap(); + + assert!(!result.success); + assert!(result.error.as_deref().unwrap().contains("not initialized")); + } + + #[tokio::test] + async fn default_action_is_add() { + let mock = Arc::new(MockChannel::new()); + let mock_ch: Arc = Arc::clone(&mock) as Arc; + let tool = make_tool_with_channels(vec![("test", mock_ch)]); + + let result = tool + .execute(json!({ + "channel": "test", + "message_id": "msg_1", + "emoji": "\u{2705}" + })) + .await + .unwrap(); + + assert!(result.success); + assert!(mock.reaction_added.load(Ordering::SeqCst)); + assert!(!mock.reaction_removed.load(Ordering::SeqCst)); + } + + #[tokio::test] + async fn channel_map_handle_allows_late_binding() { + let tool = ReactionTool::new(Arc::new(SecurityPolicy::default())); + let handle = tool.channel_map_handle(); + + // Initially empty — tool reports not initialized + let result = tool + .execute(json!({ + "channel": "slack", + "message_id": "msg_1", + "emoji": "\u{2705}" + })) + .await + .unwrap(); + assert!(!result.success); + + // Populate via the handle + { + let mut map = handle.write(); + map.insert( + "slack".to_string(), + Arc::new(MockChannel::new()) as Arc, + ); + } + + // Now the tool can route to the channel + let result = tool + .execute(json!({ + "channel": "slack", + "message_id": "msg_1", + "emoji": "\u{2705}" + })) + .await + .unwrap(); + assert!(result.success); + } + + #[test] + fn spec_matches_metadata() { + let tool = ReactionTool::new(Arc::new(SecurityPolicy::default())); + let spec = tool.spec(); + assert_eq!(spec.name, "reaction"); + assert_eq!(spec.description, tool.description()); + assert!(spec.parameters["required"].is_array()); + } +} From 9eac6bafef916bb3bf3921dfb61e66228a299f07 Mon Sep 17 00:00:00 2001 From: Giulio V Date: Sat, 21 Mar 2026 18:09:48 +0100 Subject: [PATCH 2/5] fix(config): fix pre-existing test compilation errors in schema.rs - Remove #[cfg(unix)] gate on `use tempfile::TempDir` import since TempDir is used unconditionally in bootstrap file tests - Add explicit type annotations on tokio::fs::* calls to resolve type inference failures (create_dir_all, write, read_to_string) Co-Authored-By: Claude Opus 4.6 (1M context) --- src/config/schema.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/config/schema.rs b/src/config/schema.rs index d8cb374b9..dc781c5e9 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -9275,7 +9275,6 @@ mod tests { use std::os::unix::fs::PermissionsExt; use std::path::PathBuf; use std::sync::{Arc, Mutex as StdMutex}; - #[cfg(unix)] use tempfile::TempDir; use tokio::sync::{Mutex, MutexGuard}; use tokio::test; @@ -13671,12 +13670,12 @@ require_otp_to_resume = true async fn ensure_bootstrap_files_creates_missing_files() { let tmp = TempDir::new().unwrap(); let ws = tmp.path().join("workspace"); - tokio::fs::create_dir_all(&ws).await.unwrap(); + let _: () = tokio::fs::create_dir_all(&ws).await.unwrap(); ensure_bootstrap_files(&ws).await.unwrap(); - let soul = tokio::fs::read_to_string(ws.join("SOUL.md")).await.unwrap(); - let identity = tokio::fs::read_to_string(ws.join("IDENTITY.md")) + let soul: String = tokio::fs::read_to_string(ws.join("SOUL.md")).await.unwrap(); + let identity: String = tokio::fs::read_to_string(ws.join("IDENTITY.md")) .await .unwrap(); assert!(soul.contains("SOUL.md")); @@ -13687,21 +13686,21 @@ require_otp_to_resume = true async fn ensure_bootstrap_files_does_not_overwrite_existing() { let tmp = TempDir::new().unwrap(); let ws = tmp.path().join("workspace"); - tokio::fs::create_dir_all(&ws).await.unwrap(); + let _: () = tokio::fs::create_dir_all(&ws).await.unwrap(); let custom = "# My custom SOUL"; - tokio::fs::write(ws.join("SOUL.md"), custom).await.unwrap(); + let _: () = tokio::fs::write(ws.join("SOUL.md"), custom).await.unwrap(); ensure_bootstrap_files(&ws).await.unwrap(); - let soul = tokio::fs::read_to_string(ws.join("SOUL.md")).await.unwrap(); + let soul: String = tokio::fs::read_to_string(ws.join("SOUL.md")).await.unwrap(); assert_eq!( soul, custom, "ensure_bootstrap_files must not overwrite existing files" ); // IDENTITY.md should still be created since it was missing - let identity = tokio::fs::read_to_string(ws.join("IDENTITY.md")) + let identity: String = tokio::fs::read_to_string(ws.join("IDENTITY.md")) .await .unwrap(); assert!(identity.contains("IDENTITY.md")); From cdb5ac14718f3df6b0f94f4e04ac0ccb404c1d4a Mon Sep 17 00:00:00 2001 From: Giulio V Date: Sat, 21 Mar 2026 18:49:35 +0100 Subject: [PATCH 3/5] fix(tools): fix remove_reaction_success test The output format used "{action}ed" which produced "removeed" for the remove action. Use explicit past-tense mapping instead. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/tools/reaction.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/tools/reaction.rs b/src/tools/reaction.rs index 22d3cbd2e..59681dea9 100644 --- a/src/tools/reaction.rs +++ b/src/tools/reaction.rs @@ -158,11 +158,17 @@ impl Tool for ReactionTool { .await }; + let past_tense = if action == "remove" { + "removed" + } else { + "added" + }; + match result { Ok(()) => Ok(ToolResult { success: true, output: format!( - "Reaction {action}ed: {emoji} on message {message_id} in {channel_name}" + "Reaction {past_tense}: {emoji} on message {message_id} in {channel_name}" ), error: None, }), From 2ceda31ce21892653ad1cb3148db21937140a86f Mon Sep 17 00:00:00 2001 From: argenis de la rosa Date: Sat, 21 Mar 2026 20:01:22 -0400 Subject: [PATCH 4/5] fix(tools): pass platform channel_id to reaction trait instead of channel name The reaction tool was passing the channel adapter name (e.g. "discord", "slack") as the first argument to Channel::add_reaction() and Channel::remove_reaction(), but the trait signature expects a platform-specific channel_id (e.g. Discord channel snowflake, Slack channel ID like "C0123ABCD"). This would cause all reaction API calls to fail at the platform level. Fixes: - Add required "channel_id" parameter to the tool schema - Extract and pass channel_id (not channel_name) to trait methods - Update tool description to mention the new parameter - Add MockChannel channel_id capture for test verification - Add test asserting channel_id (not name) reaches the trait - Update all existing tests to supply channel_id --- src/tools/reaction.rs | 74 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 64 insertions(+), 10 deletions(-) diff --git a/src/tools/reaction.rs b/src/tools/reaction.rs index 59681dea9..672fd3983 100644 --- a/src/tools/reaction.rs +++ b/src/tools/reaction.rs @@ -54,8 +54,8 @@ impl Tool for ReactionTool { fn description(&self) -> &str { "Add or remove an emoji reaction on a message in any active channel. \ - Provide the channel name (e.g. 'discord', 'slack'), the platform message ID, \ - and the emoji (Unicode character or platform shortcode)." + Provide the channel name (e.g. 'discord', 'slack'), the platform channel ID, \ + the platform message ID, and the emoji (Unicode character or platform shortcode)." } fn parameters_schema(&self) -> serde_json::Value { @@ -66,6 +66,10 @@ impl Tool for ReactionTool { "type": "string", "description": "Name of the channel to react in (e.g. 'discord', 'slack', 'telegram')" }, + "channel_id": { + "type": "string", + "description": "Platform-specific channel/conversation identifier (e.g. Discord channel snowflake, Slack channel ID)" + }, "message_id": { "type": "string", "description": "Platform-scoped message identifier to react to" @@ -80,7 +84,7 @@ impl Tool for ReactionTool { "description": "Whether to add or remove the reaction (default: 'add')" } }, - "required": ["channel", "message_id", "emoji"] + "required": ["channel", "channel_id", "message_id", "emoji"] }) } @@ -102,6 +106,11 @@ impl Tool for ReactionTool { .and_then(|v| v.as_str()) .ok_or_else(|| anyhow::anyhow!("Missing 'channel' parameter"))?; + let channel_id = args + .get("channel_id") + .and_then(|v| v.as_str()) + .ok_or_else(|| anyhow::anyhow!("Missing 'channel_id' parameter"))?; + let message_id = args .get("message_id") .and_then(|v| v.as_str()) @@ -151,10 +160,10 @@ impl Tool for ReactionTool { }; let result = if action == "add" { - channel.add_reaction(channel_name, message_id, emoji).await + channel.add_reaction(channel_id, message_id, emoji).await } else { channel - .remove_reaction(channel_name, message_id, emoji) + .remove_reaction(channel_id, message_id, emoji) .await }; @@ -190,6 +199,7 @@ mod tests { struct MockChannel { reaction_added: AtomicBool, reaction_removed: AtomicBool, + last_channel_id: parking_lot::Mutex>, fail_on_add: bool, } @@ -198,6 +208,7 @@ mod tests { Self { reaction_added: AtomicBool::new(false), reaction_removed: AtomicBool::new(false), + last_channel_id: parking_lot::Mutex::new(None), fail_on_add: false, } } @@ -206,6 +217,7 @@ mod tests { Self { reaction_added: AtomicBool::new(false), reaction_removed: AtomicBool::new(false), + last_channel_id: parking_lot::Mutex::new(None), fail_on_add: true, } } @@ -230,23 +242,25 @@ mod tests { async fn add_reaction( &self, - _channel_id: &str, + channel_id: &str, _message_id: &str, _emoji: &str, ) -> anyhow::Result<()> { if self.fail_on_add { return Err(anyhow::anyhow!("API error: rate limited")); } + *self.last_channel_id.lock() = Some(channel_id.to_string()); self.reaction_added.store(true, Ordering::SeqCst); Ok(()) } async fn remove_reaction( &self, - _channel_id: &str, + channel_id: &str, _message_id: &str, _emoji: &str, ) -> anyhow::Result<()> { + *self.last_channel_id.lock() = Some(channel_id.to_string()); self.reaction_removed.store(true, Ordering::SeqCst); Ok(()) } @@ -270,11 +284,13 @@ mod tests { let schema = tool.parameters_schema(); assert_eq!(schema["type"], "object"); assert!(schema["properties"]["channel"].is_object()); + assert!(schema["properties"]["channel_id"].is_object()); assert!(schema["properties"]["message_id"].is_object()); assert!(schema["properties"]["emoji"].is_object()); assert!(schema["properties"]["action"].is_object()); let required = schema["required"].as_array().unwrap(); assert!(required.iter().any(|v| v == "channel")); + assert!(required.iter().any(|v| v == "channel_id")); assert!(required.iter().any(|v| v == "message_id")); assert!(required.iter().any(|v| v == "emoji")); // action is optional (defaults to "add") @@ -289,6 +305,7 @@ mod tests { let result = tool .execute(json!({ "channel": "discord", + "channel_id": "ch_001", "message_id": "msg_123", "emoji": "\u{2705}" })) @@ -308,6 +325,7 @@ mod tests { let result = tool .execute(json!({ "channel": "slack", + "channel_id": "C0123SLACK", "message_id": "msg_456", "emoji": "\u{1F440}", "action": "remove" @@ -329,6 +347,7 @@ mod tests { let result = tool .execute(json!({ "channel": "nonexistent", + "channel_id": "ch_x", "message_id": "msg_1", "emoji": "\u{2705}" })) @@ -351,6 +370,7 @@ mod tests { let result = tool .execute(json!({ "channel": "discord", + "channel_id": "ch_001", "message_id": "msg_1", "emoji": "\u{2705}", "action": "toggle" @@ -370,6 +390,7 @@ mod tests { let result = tool .execute(json!({ "channel": "discord", + "channel_id": "ch_001", "message_id": "msg_1", "emoji": "\u{2705}" })) @@ -388,16 +409,20 @@ mod tests { )]); // Missing channel - let result = tool.execute(json!({"message_id": "1", "emoji": "x"})).await; + let result = tool.execute(json!({"channel_id": "c1", "message_id": "1", "emoji": "x"})).await; + assert!(result.is_err()); + + // Missing channel_id + let result = tool.execute(json!({"channel": "test", "message_id": "1", "emoji": "x"})).await; assert!(result.is_err()); // Missing message_id - let result = tool.execute(json!({"channel": "a", "emoji": "x"})).await; + let result = tool.execute(json!({"channel": "a", "channel_id": "c1", "emoji": "x"})).await; assert!(result.is_err()); // Missing emoji let result = tool - .execute(json!({"channel": "a", "message_id": "1"})) + .execute(json!({"channel": "a", "channel_id": "c1", "message_id": "1"})) .await; assert!(result.is_err()); } @@ -410,6 +435,7 @@ mod tests { let result = tool .execute(json!({ "channel": "discord", + "channel_id": "ch_001", "message_id": "msg_1", "emoji": "\u{2705}" })) @@ -429,6 +455,7 @@ mod tests { let result = tool .execute(json!({ "channel": "test", + "channel_id": "ch_test", "message_id": "msg_1", "emoji": "\u{2705}" })) @@ -440,6 +467,31 @@ mod tests { assert!(!mock.reaction_removed.load(Ordering::SeqCst)); } + #[tokio::test] + async fn channel_id_passed_to_trait_not_channel_name() { + let mock = Arc::new(MockChannel::new()); + let mock_ch: Arc = Arc::clone(&mock) as Arc; + let tool = make_tool_with_channels(vec![("discord", mock_ch)]); + + let result = tool + .execute(json!({ + "channel": "discord", + "channel_id": "123456789", + "message_id": "msg_1", + "emoji": "\u{2705}" + })) + .await + .unwrap(); + + assert!(result.success); + // The trait must receive the platform channel_id, not the channel name + assert_eq!( + mock.last_channel_id.lock().as_deref(), + Some("123456789"), + "add_reaction must receive channel_id, not channel name" + ); + } + #[tokio::test] async fn channel_map_handle_allows_late_binding() { let tool = ReactionTool::new(Arc::new(SecurityPolicy::default())); @@ -449,6 +501,7 @@ mod tests { let result = tool .execute(json!({ "channel": "slack", + "channel_id": "C0123", "message_id": "msg_1", "emoji": "\u{2705}" })) @@ -469,6 +522,7 @@ mod tests { let result = tool .execute(json!({ "channel": "slack", + "channel_id": "C0123", "message_id": "msg_1", "emoji": "\u{2705}" })) From 691efa4d8c83cf79bbcc63a10eeae445291e3c2e Mon Sep 17 00:00:00 2001 From: argenis de la rosa Date: Sat, 21 Mar 2026 20:38:24 -0400 Subject: [PATCH 5/5] style: fix cargo fmt formatting in reaction tool --- src/tools/reaction.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/tools/reaction.rs b/src/tools/reaction.rs index 672fd3983..78266084a 100644 --- a/src/tools/reaction.rs +++ b/src/tools/reaction.rs @@ -162,9 +162,7 @@ impl Tool for ReactionTool { let result = if action == "add" { channel.add_reaction(channel_id, message_id, emoji).await } else { - channel - .remove_reaction(channel_id, message_id, emoji) - .await + channel.remove_reaction(channel_id, message_id, emoji).await }; let past_tense = if action == "remove" { @@ -409,15 +407,21 @@ mod tests { )]); // Missing channel - let result = tool.execute(json!({"channel_id": "c1", "message_id": "1", "emoji": "x"})).await; + let result = tool + .execute(json!({"channel_id": "c1", "message_id": "1", "emoji": "x"})) + .await; assert!(result.is_err()); // Missing channel_id - let result = tool.execute(json!({"channel": "test", "message_id": "1", "emoji": "x"})).await; + let result = tool + .execute(json!({"channel": "test", "message_id": "1", "emoji": "x"})) + .await; assert!(result.is_err()); // Missing message_id - let result = tool.execute(json!({"channel": "a", "channel_id": "c1", "emoji": "x"})).await; + let result = tool + .execute(json!({"channel": "a", "channel_id": "c1", "emoji": "x"})) + .await; assert!(result.is_err()); // Missing emoji