From 993f2b25dccde9faaac85b5d5011370869018a5e Mon Sep 17 00:00:00 2001 From: Henry Park Date: Fri, 27 Mar 2026 11:23:36 -0700 Subject: [PATCH] Clarify message tool and channel setup guidance --- src/extensions/mod.rs | 3 +- src/llm/reasoning.rs | 38 ++++++++-- src/tools/builtin/extension_tools.rs | 17 ++++- src/tools/builtin/message.rs | 11 ++- tests/e2e_builtin_tool_coverage.rs | 108 ++++++++++++++++++++++++++- tests/multi_tenant_system_prompt.rs | 32 ++++++++ 6 files changed, 197 insertions(+), 12 deletions(-) diff --git a/src/extensions/mod.rs b/src/extensions/mod.rs index 4c32767b..2602884d 100644 --- a/src/extensions/mod.rs +++ b/src/extensions/mod.rs @@ -2,7 +2,8 @@ //! and activation of channels, tools, and MCP servers. //! //! Extensions are the user-facing abstraction that unifies three runtime kinds: -//! - **Channels** (Telegram, Slack, Discord) — messaging integrations (WASM) +//! - **Channels** (Telegram, Slack, Discord) — messaging platform connections +//! and conversation transports (WASM) //! - **Tools** — sandboxed capabilities (WASM) //! - **MCP servers** — external API integrations via Model Context Protocol //! diff --git a/src/llm/reasoning.rs b/src/llm/reasoning.rs index 6e078ac7..abec4a7c 100644 --- a/src/llm/reasoning.rs +++ b/src/llm/reasoning.rs @@ -1017,8 +1017,10 @@ Example: "\n\n## Extensions\n\ You can search, install, and activate extensions to add new capabilities:\n\ - - **Channels** (Telegram, Slack, Discord) — messaging integrations. \ - When users ask about connecting a messaging platform, search for it as a channel.\n\ + - **Channels** (Telegram, Slack, Discord) — connect messaging platforms so users can \ + talk to you there. When users ask about connecting a messaging platform, search for it \ + as a channel. Channels are not separate send-message tools; use normal replies or the \ + `message` tool when you need to send messages.\n\ - **Tools** — sandboxed functions that extend your abilities.\n\ - **MCP servers** — external API integrations via the Model Context Protocol.\n\n\ Use `tool_search` to find extensions by name. Refer to them by their kind \ @@ -1059,15 +1061,16 @@ Example: let message_tool_hint = "\ \n\n## Proactive Messaging\n\ +For ordinary replies in the current conversation, respond normally without calling `message`.\n\ Send messages via Signal, Telegram, Slack, or other connected channels:\n\ - `content` (required): the message text\n\ - `attachments` (optional): array of file paths to send\n\ - `channel` (optional): which channel to use (signal, telegram, slack, etc.)\n\ - `target` (optional): who to send to (phone number, group ID, etc.)\n\ -\nOmit both `channel` and `target` to send to the current conversation.\n\ +\nOmit both `channel` and `target` for a proactive follow-up in the current conversation.\n\ Examples (tool calls use JSON format):\n\ -- Reply here: {\"content\": \"Hi!\"}\n\ -- Send file here: {\"content\": \"Here's the file\", \"attachments\": [\"/path/to/file.txt\"]}\n\ +- Proactive follow-up here: {\"content\": \"Hi again!\"}\n\ +- Send file here proactively: {\"content\": \"Here's the file\", \"attachments\": [\"/path/to/file.txt\"]}\n\ - Message a different user: {\"channel\": \"signal\", \"target\": \"+1234567890\", \"content\": \"Hi!\"}\n\ - Message a different group: {\"channel\": \"signal\", \"target\": \"group:abc123\", \"content\": \"Hi!\"}"; @@ -2452,6 +2455,31 @@ That's my plan."#; ); } + #[test] + fn test_extensions_section_clarifies_channels_are_not_send_tools() { + let reasoning = make_test_reasoning(); + let tool_defs = vec![ToolDefinition { + name: "tool_search".to_string(), + description: "Search extensions".to_string(), + parameters: serde_json::json!({}), + }]; + + let section = reasoning.build_extensions_section_for_tools(&tool_defs); + assert!(section.contains("connect messaging platforms so users can talk to you there")); + assert!(section.contains("Channels are not separate send-message tools")); + assert!(section.contains("`message` tool")); + } + + #[test] + fn test_channel_section_separates_normal_replies_from_message_tool() { + let reasoning = make_test_reasoning().with_channel("telegram"); + + let section = reasoning.build_channel_section(); + assert!(section.contains("respond normally without calling `message`")); + assert!(section.contains("proactive follow-up in the current conversation")); + assert!(section.contains("Proactive follow-up here")); + } + // ---- plan/evaluate bypass clean_response (Bug #564-2) ---- #[test] diff --git a/src/tools/builtin/extension_tools.rs b/src/tools/builtin/extension_tools.rs index fba61613..ea29fae8 100644 --- a/src/tools/builtin/extension_tools.rs +++ b/src/tools/builtin/extension_tools.rs @@ -31,8 +31,10 @@ impl Tool for ToolSearchTool { fn description(&self) -> &str { "Search for available extensions to add new capabilities. Extensions include \ - channels (Telegram, Slack, Discord — for messaging), tools, and MCP servers. \ - Use discover:true to search online if the built-in registry has no results." + channels (Telegram, Slack, Discord — connect messaging platforms so IronClaw can \ + receive and reply there), tools, and MCP servers. Install and activate channels here; \ + use the `message` tool for proactive outbound sends. Use discover:true to search online \ + if the built-in registry has no results." } fn parameters_schema(&self) -> serde_json::Value { @@ -634,6 +636,17 @@ mod tests { assert!(schema["properties"].get("query").is_some()); } + #[test] + fn test_tool_search_description_clarifies_channel_setup_vs_sending() { + let tool = ToolSearchTool { + manager: test_manager_stub(), + }; + + let description = tool.description(); + assert!(description.contains("Install and activate channels here")); + assert!(description.contains("use the `message` tool for proactive outbound sends")); + } + #[test] fn test_tool_install_schema() { use crate::tools::tool::ApprovalRequirement; diff --git a/src/tools/builtin/message.rs b/src/tools/builtin/message.rs index 08029d6f..3868a8de 100644 --- a/src/tools/builtin/message.rs +++ b/src/tools/builtin/message.rs @@ -181,8 +181,10 @@ impl Tool for MessageTool { } fn description(&self) -> &str { - "Send a message to a channel. If channel/target omitted, uses the current conversation's \ - channel and sender/group. Use to proactively message users on any connected channel. \ + "Send a proactive message to a channel. Use normal assistant output to reply in the \ + active conversation; use this tool for proactive notifications, routine/background \ + follow-ups, attachments, or sending to a different channel/recipient. If channel/target \ + are omitted, reuses the current conversation's channel and sender/group when available. \ Supports file attachments: first download the file with the http tool using save_to \ (e.g., http GET https://picsum.photos/800/600 save_to=/tmp/photo.jpg), then pass \ the file path in the attachments array. Images are sent as photos on Telegram. \ @@ -451,7 +453,10 @@ mod tests { #[test] fn message_tool_description() { let tool = MessageTool::new(Arc::new(ChannelManager::new())); - assert!(!tool.description().is_empty()); + let description = tool.description(); + assert!(!description.is_empty()); + assert!(description.contains("Use normal assistant output to reply")); + assert!(description.contains("proactive notifications")); } #[test] diff --git a/tests/e2e_builtin_tool_coverage.rs b/tests/e2e_builtin_tool_coverage.rs index 1c3cc6a2..aae51dde 100644 --- a/tests/e2e_builtin_tool_coverage.rs +++ b/tests/e2e_builtin_tool_coverage.rs @@ -13,7 +13,7 @@ mod tests { use ironclaw::agent::routine::{RoutineAction, Trigger}; use crate::support::test_rig::TestRigBuilder; - use crate::support::trace_llm::LlmTrace; + use crate::support::trace_llm::{LlmTrace, TraceResponse, TraceStep, TraceToolCall, TraceTurn}; // ----------------------------------------------------------------------- // Test 1: time_parse_and_diff @@ -838,4 +838,110 @@ mod tests { rig.shutdown(); } + + #[tokio::test] + async fn tool_info_clarifies_message_and_channel_setup_roles() { + let trace = LlmTrace::new( + "test-tool-info-channel-message-clarity", + vec![TraceTurn { + user_input: "How do message and channels differ?".to_string(), + steps: vec![ + TraceStep { + request_hint: None, + response: TraceResponse::ToolCalls { + tool_calls: vec![TraceToolCall { + id: "call_tool_info_message".to_string(), + name: "tool_info".to_string(), + arguments: serde_json::json!({"name": "message"}), + }], + input_tokens: 100, + output_tokens: 20, + }, + expected_tool_results: Vec::new(), + }, + TraceStep { + request_hint: None, + response: TraceResponse::ToolCalls { + tool_calls: vec![TraceToolCall { + id: "call_tool_info_tool_search".to_string(), + name: "tool_info".to_string(), + arguments: serde_json::json!({"name": "tool_search"}), + }], + input_tokens: 140, + output_tokens: 20, + }, + expected_tool_results: Vec::new(), + }, + TraceStep { + request_hint: None, + response: TraceResponse::Text { + content: "I checked both tool descriptions.".to_string(), + input_tokens: 220, + output_tokens: 30, + }, + expected_tool_results: Vec::new(), + }, + ], + expects: Default::default(), + }], + ); + + let rig = TestRigBuilder::new() + .with_trace(trace.clone()) + .with_auto_approve_tools(true) + .build() + .await; + + rig.send_message("How do message and channels differ?") + .await; + let responses = rig.wait_for_responses(1, Duration::from_secs(15)).await; + + rig.verify_trace_expects(&trace, &responses); + + let results = rig.tool_results(); + let info_results: Vec<_> = results.iter().filter(|(n, _)| n == "tool_info").collect(); + assert_eq!(info_results.len(), 2, "Expected two tool_info results"); + + let info_json: Vec = info_results + .iter() + .map(|(_, preview)| { + serde_json::from_str(preview) + .expect("tool_info result preview should be valid JSON") + }) + .collect(); + + let message_json = info_json + .iter() + .find(|info| info["name"] == "message") + .expect("tool_info result should contain 'message'"); + let message_description = message_json["description"] + .as_str() + .expect("message description should be a string"); + assert!( + message_description.contains("Use normal assistant output to reply"), + "message description should distinguish normal replies: {message_description}" + ); + assert!( + message_description.contains("proactive notifications"), + "message description should describe proactive sends: {message_description}" + ); + + let tool_search_json = info_json + .iter() + .find(|info| info["name"] == "tool_search") + .expect("tool_info result should contain 'tool_search'"); + let tool_search_description = tool_search_json["description"] + .as_str() + .expect("tool_search description should be a string"); + assert!( + tool_search_description.contains("Install and activate channels here"), + "tool_search description should describe setup/activation: {tool_search_description}" + ); + assert!( + tool_search_description.contains("use the `message` tool for proactive outbound sends"), + "tool_search description should point outbound sends to message: {tool_search_description}" + ); + + rig.shutdown(); + } } diff --git a/tests/multi_tenant_system_prompt.rs b/tests/multi_tenant_system_prompt.rs index b89e6cb5..12e5cfdb 100644 --- a/tests/multi_tenant_system_prompt.rs +++ b/tests/multi_tenant_system_prompt.rs @@ -237,4 +237,36 @@ mod tests { rig.shutdown(); } + + #[tokio::test] + async fn telegram_system_prompt_clarifies_reply_vs_proactive_message_tool() { + let trace = simple_trace(1); + let rig = TestRigBuilder::new().with_trace(trace).build().await; + + let msg = IncomingMessage::new("telegram", "telegram-user", "Hello there"); + rig.send_incoming(msg).await; + let _responses = rig.wait_for_responses(1, TIMEOUT).await; + + let requests = rig.captured_llm_requests(); + let system_prompt = + extract_system_prompt(&requests).expect("Expected a system prompt in the LLM request"); + + assert!( + system_prompt.contains("Channels are not separate send-message tools"), + "System prompt should describe channels as setup/integration surfaces.\n\ + Actual system prompt:\n{system_prompt}" + ); + assert!( + system_prompt.contains("respond normally without calling `message`"), + "System prompt should say normal replies do not use the message tool.\n\ + Actual system prompt:\n{system_prompt}" + ); + assert!( + system_prompt.contains("proactive follow-up in the current conversation"), + "System prompt should reserve omitted channel/target for proactive follow-ups.\n\ + Actual system prompt:\n{system_prompt}" + ); + + rig.shutdown(); + } }