mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
Clarify message tool and channel setup guidance
This commit is contained in:
@@ -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
|
||||
//!
|
||||
|
||||
+33
-5
@@ -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]
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -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<serde_json::Value> = 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();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user