diff --git a/pkg/agent/context.go b/pkg/agent/context.go index 665496781..0d44d451c 100644 --- a/pkg/agent/context.go +++ b/pkg/agent/context.go @@ -463,6 +463,7 @@ func buildReplyRoutingContext(channel string, replyCtx *ReplyContextInfo) string "- `[[reply:parent]]` replies to the parent/replied-to message when there is one\n"+ "- `[[reply:message_id=123]]` replies to a specific known message ID\n\n"+ "After the directive, add a blank line and then the user-visible message.\n"+ + "Do not use the `message` tool for the normal reply in this chat; use the final answer plus a directive when you need reply routing.\n"+ "If you do not need special routing, answer normally without a directive.\n"+ "Never mention the directive in the visible message body.", replyCtx.CurrentMessageID, diff --git a/pkg/agent/context_cache_test.go b/pkg/agent/context_cache_test.go index d2e605ca0..5a15efa9a 100644 --- a/pkg/agent/context_cache_test.go +++ b/pkg/agent/context_cache_test.go @@ -156,6 +156,9 @@ func TestBuildMessages_TelegramReplyRoutingContext(t *testing.T) { if !strings.Contains(sys, "[[reply:current]]") { t.Fatal("system prompt missing final reply directive guidance") } + if !strings.Contains(sys, "Do not use the `message` tool for the normal reply in this chat") { + t.Fatal("system prompt missing guidance to avoid message tool for normal replies") + } } // TestMtimeAutoInvalidation verifies that the cache detects source file changes diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index f2b9c1131..da41ca095 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -377,15 +377,21 @@ func (al *AgentLoop) Run(ctx context.Context) error { al.bus.PublishOutbound(ctx, response.outboundMessage(msg.Channel, msg.ChatID)) logger.InfoCF("agent", "Published outbound response", map[string]any{ - "channel": msg.Channel, - "chat_id": msg.ChatID, - "content_len": len(response.Content), + "channel": msg.Channel, + "chat_id": msg.ChatID, + "content_len": len(response.Content), + "reply_to_message_id": response.ReplyToMessageID, }) } else { logger.DebugCF( "agent", "Skipped outbound (message tool already sent)", - map[string]any{"channel": msg.Channel}, + map[string]any{ + "channel": msg.Channel, + "chat_id": msg.ChatID, + "content_len": len(response.Content), + "reply_to_message_id": response.ReplyToMessageID, + }, ) } } @@ -882,6 +888,42 @@ func resolveFinalResponse( rawContent string, ) agentResponse { content, replyToMessageID := parseFinalReplyDirective(channel, replyCtx, rawContent) + if channel == "telegram" { + firstLine, _, _ := strings.Cut(rawContent, "\n") + directive := strings.TrimSpace(firstLine) + hasDirective := strings.HasPrefix(directive, "[[reply:") && strings.HasSuffix(directive, "]]") + directiveMode := "" + if hasDirective { + directiveMode = strings.TrimSpace(strings.TrimSuffix(strings.TrimPrefix(directive, "[[reply:"), "]]")) + } + directiveStatus := "none" + switch { + case !hasDirective: + directiveStatus = "none" + case directiveMode == "chat": + directiveStatus = "applied_chat" + case replyToMessageID != "": + directiveStatus = "applied_reply" + default: + directiveStatus = "dropped" + } + + fields := map[string]any{ + "directive_status": directiveStatus, + "reply_to_message_id": replyToMessageID, + "raw_content_len": len(rawContent), + "final_content_len": len(content), + } + if hasDirective { + fields["directive"] = directive + fields["directive_mode"] = directiveMode + } + if replyCtx != nil { + fields["current_message_id"] = strings.TrimSpace(replyCtx.CurrentMessageID) + fields["parent_message_id"] = strings.TrimSpace(replyCtx.ParentMessageID) + } + logger.DebugCF("agent", "Resolved final reply routing", fields) + } return agentResponse{ Content: content, ReplyToMessageID: replyToMessageID, diff --git a/pkg/tools/message.go b/pkg/tools/message.go index 6f896fa90..98da6ba06 100644 --- a/pkg/tools/message.go +++ b/pkg/tools/message.go @@ -7,6 +7,7 @@ import ( "sync/atomic" "github.com/sipeed/picoclaw/pkg/bus" + "github.com/sipeed/picoclaw/pkg/logger" ) type SendCallback func(msg bus.OutboundMessage) error @@ -31,7 +32,7 @@ func (t *MessageTool) Name() string { } func (t *MessageTool) Description() string { - return "Send a message to the user on a chat channel. Use this when you want to communicate something or explicitly control reply threading." + return "Send an out-of-band message to a chat channel. Do not use this for the normal final reply in the current conversation." } func (t *MessageTool) Parameters() map[string]any { @@ -50,15 +51,6 @@ func (t *MessageTool) Parameters() map[string]any { "type": "string", "description": "Optional: target chat/user ID", }, - "reply_mode": map[string]any{ - "type": "string", - "enum": []string{replyModeChat, replyModeCurrent, replyModeParent}, - "description": "Optional: threading mode. chat sends a normal message, current replies to the current inbound message, parent replies to the parent/replied-to inbound message.", - }, - "reply_to_message_id": map[string]any{ - "type": "string", - "description": "Optional: explicit platform message ID to reply to. Overrides reply_mode when provided.", - }, }, "required": []string{"content"}, } @@ -99,6 +91,32 @@ func (t *MessageTool) Execute(ctx context.Context, args map[string]any) *ToolRes return &ToolResult{ForLLM: "No target channel/chat specified", IsError: true} } + currentChannel := ToolChannel(ctx) + currentChatID := ToolChatID(ctx) + replyMode, _ := args["reply_mode"].(string) + replyMode = strings.ToLower(strings.TrimSpace(replyMode)) + explicitReplyTo, _ := args["reply_to_message_id"].(string) + explicitReplyTo = strings.TrimSpace(explicitReplyTo) + + if replyMode != "" || explicitReplyTo != "" { + logger.WarnCF("tool", "Message tool received deprecated reply routing args", map[string]any{ + "channel": channel, + "chat_id": chatID, + "reply_mode": replyMode, + "reply_to_message_id": explicitReplyTo, + }) + } + if currentChannel != "" && currentChatID != "" && channel == currentChannel && chatID == currentChatID { + logger.InfoCF("tool", "Message tool targeting current conversation", map[string]any{ + "channel": channel, + "chat_id": chatID, + "content_len": len(content), + "reply_mode": replyMode, + "same_target": true, + "session_key": ToolSessionKey(ctx), + }) + } + replyToMessageID, err := resolveReplyTarget(ctx, args) if err != nil { return &ToolResult{ @@ -127,6 +145,14 @@ func (t *MessageTool) Execute(ctx context.Context, args map[string]any) *ToolRes } t.sentInRound.Store(true) + logger.InfoCF("tool", "Message tool sent outbound message", map[string]any{ + "channel": channel, + "chat_id": chatID, + "content_len": len(content), + "reply_to_message_id": replyToMessageID, + "same_target": currentChannel != "" && currentChatID != "" && channel == currentChannel && chatID == currentChatID, + }) + // Silent: user already received the message directly status := fmt.Sprintf("Message sent to %s:%s", channel, chatID) if replyToMessageID != "" { diff --git a/pkg/tools/message_test.go b/pkg/tools/message_test.go index ccae1e518..6e19f3345 100644 --- a/pkg/tools/message_test.go +++ b/pkg/tools/message_test.go @@ -284,6 +284,9 @@ func TestMessageTool_Description(t *testing.T) { if desc == "" { t.Error("Description should not be empty") } + if desc == "Send a message to the user on a chat channel. Use this when you want to communicate something or explicitly control reply threading." { + t.Fatal("description still advertises reply threading") + } } func TestMessageTool_Parameters(t *testing.T) { @@ -334,19 +337,10 @@ func TestMessageTool_Parameters(t *testing.T) { t.Error("Expected chat_id type to be 'string'") } - replyModeProp, ok := props["reply_mode"].(map[string]any) - if !ok { - t.Error("Expected 'reply_mode' property") + if _, ok := props["reply_mode"]; ok { + t.Error("Did not expect 'reply_mode' property in advertised schema") } - if replyModeProp["type"] != "string" { - t.Error("Expected reply_mode type to be 'string'") - } - - replyToProp, ok := props["reply_to_message_id"].(map[string]any) - if !ok { - t.Error("Expected 'reply_to_message_id' property") - } - if replyToProp["type"] != "string" { - t.Error("Expected reply_to_message_id type to be 'string'") + if _, ok := props["reply_to_message_id"]; ok { + t.Error("Did not expect 'reply_to_message_id' property in advertised schema") } }