mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-17 02:14:09 +00:00
fix: resolve Zalo group by name + notify origin on failed forward (#1395)
* fix: resolve Zalo group name to real chat ID, notify origin on failed forward Two related root causes behind "forward message to group by name" silently failing while the agent reports success: 1. No tool let the agent resolve a group's display name (e.g. "Ban Dieu Hanh") to the chat ID the message tool actually requires. sessions_list only exposes session keys (numeric group IDs), never human-readable names, so the agent had no reliable way to turn a name into a real target and ended up passing the display name itself as `target`. Adds zalo_list_groups, wrapping the already-used (dashboard picker) protocol.FetchGroups behind the same optional-interface pattern as list_group_members/GroupMemberProvider (GroupListProvider on channels.Manager, gated to zalo_personal via RequiredChannelTypes). 2. When the resulting send fails downstream (e.g. Zalo rejects a bad chat_id), dispatchOutbound only ever retried/notified media failures on the same (already-broken) destination, and dropped text-only failures entirely — even though message.go's own postCrossTargetNotice comment states forwards must never announce a fake delivery. Because the bus publish is fire-and-forget, the tool had already returned "sent" and announced success to the origin chat before the real send was even attempted. message.go now tags cross-target forwards with origin channel/chat in OutboundMessage.Metadata; dispatchOutbound uses it to notify the ORIGIN chat with the real failure instead of silently dropping it or retrying against the same invalid target. * test: cover forward-origin metadata tagging and dispatch failure notice Extracts dispatchOutbound's error branch into handleSendFailure so it can be unit tested without driving the consumer loop/goroutine, and adds coverage for: forward failures notifying the origin chat (not the broken destination), pre-existing non-forward media/text-only behavior staying unchanged, message.go tagging cross-target group forwards with origin metadata alongside group_id, and the new zalo_list_groups tool/Manager delegator.
This commit is contained in:
1 parent
fc7d183961
commit
aa15317d5b
15 files changed
+503
-39
No files matched your search
@@ -728,6 +728,12 @@ func runGateway() {
|
||||
gl.SetGroupMemberLister(channelMgr.ListGroupMembers)
|
||||
}
|
||||
}
|
||||
// Wire group lister on zalo_list_groups tool
|
||||
if t, ok := toolsReg.Get("zalo_list_groups"); ok {
|
||||
if gl, ok := t.(tools.GroupListerAware); ok {
|
||||
gl.SetGroupLister(channelMgr.ListGroups)
|
||||
}
|
||||
}
|
||||
// Wire Telegram manager on telegram_manager tool.
|
||||
for _, toolName := range []string{"telegram_manager", "create_forum_topic"} {
|
||||
if t, ok := toolsReg.Get(toolName); ok {
|
||||
|
||||
@@ -67,6 +67,8 @@ func wireExtraTools(
|
||||
toolsReg.Register(tools.NewSendFileTool(workspace, agentCfg.RestrictToWorkspace))
|
||||
// Group members tool (list members in group chats)
|
||||
toolsReg.Register(tools.NewListGroupMembersTool())
|
||||
// Zalo group list tool (resolve a group's real chat ID from its display name)
|
||||
toolsReg.Register(tools.NewListGroupsTool())
|
||||
// Telegram manager tool (admin/forum/message management; gated by tool policy)
|
||||
// create_forum_topic is kept as a backward-compatible wrapper for topic.create.
|
||||
toolsReg.Register(tools.NewCreateForumTopicTool(nil))
|
||||
|
||||
@@ -217,6 +217,7 @@ var coreToolSummaries = map[string]string{
|
||||
"knowledge_graph_search": "Find people, projects, and their connections — use for relationship questions (who works with whom, project dependencies) that memory_search may miss",
|
||||
"team_tasks": "Team task board — track progress, manage dependencies (spawn auto-creates delegation tasks)",
|
||||
"list_group_members": "List all members of the current group chat (Feishu/Lark only)",
|
||||
"zalo_list_groups": "List all Zalo groups the account belongs to (group_id + name) — resolve a group's real ID before sending/forwarding to it by name (Zalo only)",
|
||||
"create_forum_topic": "Create a forum topic in a Telegram supergroup",
|
||||
"delegate": "Delegate a task to a linked agent (requires agent_links). See ## Delegation Targets for available agents",
|
||||
"memory_expand": "Retrieve full session details from episodic memory results — use after memory_search returns episodic hits",
|
||||
|
||||
@@ -47,6 +47,18 @@ type OutboundMessage struct {
|
||||
AgentOtherConfig []byte `json:"agent_other_config,omitempty"` // agent's other_config for TTS voice/model
|
||||
}
|
||||
|
||||
// Metadata keys on OutboundMessage.Metadata used to track the origin chat of
|
||||
// a cross-target forward (message tool, forward=true). The outbound dispatch
|
||||
// consumer runs async with no path back to the tool call that queued it, so
|
||||
// these let it notify the ORIGIN chat if delivery to the forward target
|
||||
// actually fails — otherwise a bad target (e.g. a display name instead of a
|
||||
// real chat ID) fails silently downstream while the tool already reported
|
||||
// success back to the model.
|
||||
const (
|
||||
MetaForwardOriginChannel = "forward_origin_channel"
|
||||
MetaForwardOriginChatID = "forward_origin_chat_id"
|
||||
)
|
||||
|
||||
// MediaAttachment represents a media file to be sent with a message.
|
||||
type MediaAttachment struct {
|
||||
URL string `json:"url"` // file path or URL
|
||||
|
||||
@@ -724,6 +724,20 @@ type GroupMemberProvider interface {
|
||||
ListGroupMembers(ctx context.Context, chatID string) ([]GroupMember, error)
|
||||
}
|
||||
|
||||
// GroupInfo represents a group/channel chat the account belongs to.
|
||||
type GroupInfo struct {
|
||||
GroupID string `json:"group_id"`
|
||||
Name string `json:"name"`
|
||||
TotalMember int `json:"total_member,omitempty"`
|
||||
}
|
||||
|
||||
// GroupListProvider is optionally implemented by channels that can list the
|
||||
// groups/chats the connected account belongs to — lets the agent resolve a
|
||||
// group's display name to its real chat ID instead of guessing.
|
||||
type GroupListProvider interface {
|
||||
ListGroups(ctx context.Context) ([]GroupInfo, error)
|
||||
}
|
||||
|
||||
// GroupTitleProvider is optionally implemented by channels that can resolve
|
||||
// a platform group/channel ID to a human-readable title.
|
||||
type GroupTitleProvider interface {
|
||||
|
||||
@@ -85,29 +85,7 @@ func (m *Manager) dispatchOutbound(ctx context.Context) {
|
||||
}
|
||||
|
||||
if err := channel.Send(sendCtx, msg); err != nil {
|
||||
slog.Error("error sending message to channel",
|
||||
"channel", msg.Channel,
|
||||
"chat_id", msg.ChatID,
|
||||
"content_len", len(msg.Content),
|
||||
"content_preview", Truncate(msg.Content, 160),
|
||||
"error", err,
|
||||
)
|
||||
// Try to send a text-only error notification back to the chat.
|
||||
// Only for media failures — text-only failures likely mean the chat
|
||||
// is inaccessible (kicked, blocked, etc.) so retrying won't help.
|
||||
if len(msg.Media) > 0 {
|
||||
notifyMsg := bus.OutboundMessage{
|
||||
Channel: msg.Channel,
|
||||
ChatID: msg.ChatID,
|
||||
Content: formatChannelSendError(err),
|
||||
Metadata: sendErrorMeta(msg.Metadata),
|
||||
TenantID: msg.TenantID,
|
||||
}
|
||||
if err2 := channel.Send(sendCtx, notifyMsg); err2 != nil {
|
||||
slog.Warn("failed to send error notification",
|
||||
"channel", msg.Channel, "error", err2)
|
||||
}
|
||||
}
|
||||
m.handleSendFailure(sendCtx, channel, msg, err)
|
||||
}
|
||||
|
||||
// Clean up temp media files only. Workspace-generated files are preserved
|
||||
@@ -124,6 +102,63 @@ func (m *Manager) dispatchOutbound(ctx context.Context) {
|
||||
}
|
||||
}
|
||||
|
||||
// handleSendFailure reports a failed channel.Send from dispatchOutbound.
|
||||
//
|
||||
// Cross-target forwards (message tool, forward=true) are fire-and-forget onto
|
||||
// the bus — the tool already returned "sent" to the model and announced
|
||||
// success to the origin chat before this consumer ever ran. If the
|
||||
// destination itself was bad (e.g. the model passed a display name instead
|
||||
// of a real chat ID), this tells the ORIGIN chat the truth instead of
|
||||
// retrying against the same broken destination or, for text-only forwards,
|
||||
// silently dropping the failure.
|
||||
//
|
||||
// Non-forward sends keep the older behavior: retry-notify the same chat, and
|
||||
// only for media failures (text-only failures on a chat the agent is already
|
||||
// bound to usually mean the chat itself is inaccessible — kicked, blocked —
|
||||
// so retrying won't help).
|
||||
func (m *Manager) handleSendFailure(sendCtx context.Context, channel Channel, msg bus.OutboundMessage, sendErr error) {
|
||||
slog.Error("error sending message to channel",
|
||||
"channel", msg.Channel,
|
||||
"chat_id", msg.ChatID,
|
||||
"content_len", len(msg.Content),
|
||||
"content_preview", Truncate(msg.Content, 160),
|
||||
"error", sendErr,
|
||||
)
|
||||
|
||||
if originCh := msg.Metadata[bus.MetaForwardOriginChannel]; originCh != "" {
|
||||
originChat := msg.Metadata[bus.MetaForwardOriginChatID]
|
||||
m.mu.RLock()
|
||||
origin, originExists := m.channels[originCh]
|
||||
m.mu.RUnlock()
|
||||
if originExists && originChat != "" {
|
||||
notifyMsg := bus.OutboundMessage{
|
||||
Channel: originCh,
|
||||
ChatID: originChat,
|
||||
Content: fmt.Sprintf("⚠️ Không gửi được tin nhắn forward tới %q — kiểm tra lại đích gửi (có thể chưa đúng ID nhóm/chat).", msg.ChatID),
|
||||
}
|
||||
if err2 := origin.Send(sendCtx, notifyMsg); err2 != nil {
|
||||
slog.Warn("failed to send forward-failure notice to origin",
|
||||
"origin_channel", originCh, "origin_chat", originChat, "error", err2)
|
||||
}
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
if len(msg.Media) > 0 {
|
||||
notifyMsg := bus.OutboundMessage{
|
||||
Channel: msg.Channel,
|
||||
ChatID: msg.ChatID,
|
||||
Content: formatChannelSendError(sendErr),
|
||||
Metadata: sendErrorMeta(msg.Metadata),
|
||||
TenantID: msg.TenantID,
|
||||
}
|
||||
if err2 := channel.Send(sendCtx, notifyMsg); err2 != nil {
|
||||
slog.Warn("failed to send error notification",
|
||||
"channel", msg.Channel, "error", err2)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// WebhookHandlers returns all webhook handlers from channels that implement WebhookChannel.
|
||||
// Used to mount webhook routes on the main gateway mux.
|
||||
func (m *Manager) WebhookHandlers() []WebhookRoute {
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
package channels
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"testing"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/bus"
|
||||
)
|
||||
|
||||
// Reproduces the false-success bug: a cross-target forward (message tool,
|
||||
// forward=true) whose destination chat ID is invalid must notify the ORIGIN
|
||||
// chat with the real failure — not retry against the same broken
|
||||
// destination, and not silently drop a text-only failure.
|
||||
func TestHandleSendFailure_ForwardNotifiesOrigin(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
mgr := NewManager(bus.New())
|
||||
origin := newMockChannel("bunny-zalo-personal", TypeZaloPersonal)
|
||||
mgr.channels["bunny-zalo-personal"] = origin
|
||||
|
||||
badTarget := bus.OutboundMessage{
|
||||
Channel: "bunny-zalo-personal",
|
||||
ChatID: "Ban Điều Hành", // display name passed as chat ID — invalid
|
||||
Content: "Anh Tài ơi, xem giúp comment khách hàng nhé.",
|
||||
Metadata: map[string]string{
|
||||
bus.MetaForwardOriginChannel: "bunny-zalo-personal",
|
||||
bus.MetaForwardOriginChatID: "747300108647389888",
|
||||
},
|
||||
}
|
||||
|
||||
mgr.handleSendFailure(context.Background(), origin, badTarget, errors.New("inner error code 114: Tham số không hợp lệ"))
|
||||
|
||||
if origin.lastMsg.ChatID != "747300108647389888" {
|
||||
t.Fatalf("notice went to %q, want the origin chat, not the broken destination %q", origin.lastMsg.ChatID, badTarget.ChatID)
|
||||
}
|
||||
if origin.lastMsg.Content == "" {
|
||||
t.Fatal("expected a non-empty failure notice content")
|
||||
}
|
||||
}
|
||||
|
||||
// Non-forward media failures keep the pre-existing behavior: retry-notify
|
||||
// the SAME chat (no forward metadata present, so there's no separate origin).
|
||||
func TestHandleSendFailure_NonForwardMediaNotifiesSameChat(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
mgr := NewManager(bus.New())
|
||||
ch := newMockChannel("telegram-main", TypeTelegram)
|
||||
mgr.channels["telegram-main"] = ch
|
||||
|
||||
msg := bus.OutboundMessage{
|
||||
Channel: "telegram-main",
|
||||
ChatID: "chat-1",
|
||||
Media: []bus.MediaAttachment{{URL: "/tmp/x.png"}},
|
||||
}
|
||||
|
||||
mgr.handleSendFailure(context.Background(), ch, msg, errors.New("file is too big"))
|
||||
|
||||
if ch.lastMsg.ChatID != "chat-1" {
|
||||
t.Fatalf("notice ChatID = %q, want chat-1 (same chat)", ch.lastMsg.ChatID)
|
||||
}
|
||||
}
|
||||
|
||||
// Non-forward TEXT-ONLY failures are still dropped (pre-existing behavior,
|
||||
// unrelated to the forward bug this file otherwise fixes) — no channel
|
||||
// exists to receive a notice, so Send must not be called again.
|
||||
func TestHandleSendFailure_NonForwardTextOnlyDropped(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
mgr := NewManager(bus.New())
|
||||
ch := newMockChannel("telegram-main", TypeTelegram)
|
||||
mgr.channels["telegram-main"] = ch
|
||||
|
||||
msg := bus.OutboundMessage{
|
||||
Channel: "telegram-main",
|
||||
ChatID: "chat-1",
|
||||
Content: "hello",
|
||||
}
|
||||
|
||||
mgr.handleSendFailure(context.Background(), ch, msg, errors.New("chat not found"))
|
||||
|
||||
if ch.lastMsg.Content != "" {
|
||||
t.Fatalf("expected no notice sent for non-forward text-only failure, got: %+v", ch.lastMsg)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,59 @@
|
||||
package channels
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/bus"
|
||||
)
|
||||
|
||||
type groupListTestChannel struct {
|
||||
*mockChannel
|
||||
groups []GroupInfo
|
||||
}
|
||||
|
||||
func (c *groupListTestChannel) ListGroups(_ context.Context) ([]GroupInfo, error) {
|
||||
return c.groups, nil
|
||||
}
|
||||
|
||||
func TestManagerListGroupsDelegatesToProvider(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
mgr := NewManager(bus.New())
|
||||
ch := &groupListTestChannel{
|
||||
mockChannel: newMockChannel("bunny-zalo-personal", TypeZaloPersonal),
|
||||
groups: []GroupInfo{
|
||||
{GroupID: "747300108647389888", Name: "Ban Điều Hành", TotalMember: 8},
|
||||
},
|
||||
}
|
||||
mgr.RegisterChannel("bunny-zalo-personal", ch)
|
||||
|
||||
groups, err := mgr.ListGroups(context.Background(), "bunny-zalo-personal")
|
||||
if err != nil {
|
||||
t.Fatalf("ListGroups returned error: %v", err)
|
||||
}
|
||||
if len(groups) != 1 || groups[0].Name != "Ban Điều Hành" || groups[0].GroupID != "747300108647389888" {
|
||||
t.Fatalf("unexpected groups: %#v", groups)
|
||||
}
|
||||
}
|
||||
|
||||
func TestManagerListGroupsUnsupportedChannel(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
mgr := NewManager(bus.New())
|
||||
ch := newMockChannel("telegram-main", TypeTelegram) // does not implement GroupListProvider
|
||||
mgr.RegisterChannel("telegram-main", ch)
|
||||
|
||||
if _, err := mgr.ListGroups(context.Background(), "telegram-main"); err == nil {
|
||||
t.Fatal("expected error for channel without GroupListProvider support")
|
||||
}
|
||||
}
|
||||
|
||||
func TestManagerListGroupsUnknownChannel(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
mgr := NewManager(bus.New())
|
||||
if _, err := mgr.ListGroups(context.Background(), "does-not-exist"); err == nil {
|
||||
t.Fatal("expected error for unknown channel")
|
||||
}
|
||||
}
|
||||
@@ -346,6 +346,21 @@ func (m *Manager) ListGroupMembers(ctx context.Context, channelName, chatID stri
|
||||
return gmp.ListGroupMembers(ctx, chatID)
|
||||
}
|
||||
|
||||
// ListGroups delegates to the channel's GroupListProvider if available.
|
||||
func (m *Manager) ListGroups(ctx context.Context, channelName string) ([]GroupInfo, error) {
|
||||
m.mu.RLock()
|
||||
ch, ok := m.channels[channelName]
|
||||
m.mu.RUnlock()
|
||||
if !ok {
|
||||
return nil, fmt.Errorf("channel %q not found", channelName)
|
||||
}
|
||||
glp, ok := ch.(GroupListProvider)
|
||||
if !ok {
|
||||
return nil, fmt.Errorf("channel %q does not support listing groups", channelName)
|
||||
}
|
||||
return glp.ListGroups(ctx)
|
||||
}
|
||||
|
||||
// ResolveGroupTitle delegates to the channel's GroupTitleProvider if available.
|
||||
func (m *Manager) ResolveGroupTitle(ctx context.Context, channelName, chatID string) (string, error) {
|
||||
m.mu.RLock()
|
||||
|
||||
@@ -91,6 +91,27 @@ func (c *Channel) getListener() *protocol.Listener {
|
||||
return c.listener
|
||||
}
|
||||
|
||||
// ListGroups implements channels.GroupListProvider — returns the real Zalo
|
||||
// group ID + display name for every group the authenticated account belongs
|
||||
// to. Lets the agent resolve a group by name (e.g. "Ban Điều Hành") to the
|
||||
// chat ID the message tool actually requires, instead of guessing/passing
|
||||
// the display name as target.
|
||||
func (c *Channel) ListGroups(ctx context.Context) ([]channels.GroupInfo, error) {
|
||||
sess := c.session()
|
||||
if sess == nil {
|
||||
return nil, fmt.Errorf("zalo_personal: not connected")
|
||||
}
|
||||
groups, err := protocol.FetchGroups(ctx, sess)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
out := make([]channels.GroupInfo, len(groups))
|
||||
for i, g := range groups {
|
||||
out[i] = channels.GroupInfo{GroupID: g.GroupID, Name: g.Name, TotalMember: g.TotalMember}
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// Start authenticates and begins listening for Zalo messages.
|
||||
func (c *Channel) Start(ctx context.Context) error {
|
||||
if gh := c.GroupHistory(); gh != nil {
|
||||
|
||||
@@ -0,0 +1,78 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/channels"
|
||||
)
|
||||
|
||||
// GroupLister abstracts listing the groups a channel account belongs to.
|
||||
// Implemented by channels.Manager.ListGroups.
|
||||
type GroupLister func(ctx context.Context, channel string) ([]channels.GroupInfo, error)
|
||||
|
||||
// GroupListerAware tools can receive a group lister function.
|
||||
type GroupListerAware interface {
|
||||
SetGroupLister(GroupLister)
|
||||
}
|
||||
|
||||
// ListGroupsTool lets the agent resolve a group's real chat ID from its
|
||||
// display name before sending/forwarding a message to it. Without this,
|
||||
// the only signal available is sessions_list (which exposes session keys,
|
||||
// not human-readable names), so the agent has no reliable way to turn
|
||||
// "the Ban Điều Hành group" into the ID the message tool actually requires.
|
||||
type ListGroupsTool struct {
|
||||
lister GroupLister
|
||||
}
|
||||
|
||||
func NewListGroupsTool() *ListGroupsTool { return &ListGroupsTool{} }
|
||||
|
||||
func (t *ListGroupsTool) SetGroupLister(l GroupLister) { t.lister = l }
|
||||
|
||||
func (t *ListGroupsTool) RequiredChannelTypes() []string { return []string{"zalo_personal"} }
|
||||
|
||||
func (t *ListGroupsTool) Name() string { return "zalo_list_groups" }
|
||||
func (t *ListGroupsTool) Description() string {
|
||||
return "List all Zalo groups the connected account belongs to (group_id + display name + member count). Use this BEFORE sending/forwarding a message to a group by name — the message tool's `target` requires a real group_id, not a display name; passing the name directly will fail or silently go nowhere."
|
||||
}
|
||||
|
||||
func (t *ListGroupsTool) Parameters() map[string]any {
|
||||
return map[string]any{
|
||||
"type": "object",
|
||||
"properties": map[string]any{},
|
||||
}
|
||||
}
|
||||
|
||||
func (t *ListGroupsTool) Execute(ctx context.Context, args map[string]any) *Result {
|
||||
if t.lister == nil {
|
||||
return ErrorResult("zalo_list_groups: no group lister available")
|
||||
}
|
||||
|
||||
channel := ToolChannelFromCtx(ctx)
|
||||
if channel == "" {
|
||||
return ErrorResult("channel is required (no current channel in context)")
|
||||
}
|
||||
|
||||
groups, err := t.lister(ctx, channel)
|
||||
if err != nil {
|
||||
return ErrorResult(fmt.Sprintf("failed to list groups: %v", err))
|
||||
}
|
||||
|
||||
type groupOut struct {
|
||||
GroupID string `json:"group_id"`
|
||||
Name string `json:"name"`
|
||||
TotalMember int `json:"total_member,omitempty"`
|
||||
}
|
||||
out := make([]groupOut, len(groups))
|
||||
for i, g := range groups {
|
||||
out[i] = groupOut{GroupID: g.GroupID, Name: g.Name, TotalMember: g.TotalMember}
|
||||
}
|
||||
|
||||
data, _ := json.Marshal(map[string]any{
|
||||
"channel": channel,
|
||||
"count": len(out),
|
||||
"groups": out,
|
||||
})
|
||||
return NewResult(string(data))
|
||||
}
|
||||
@@ -0,0 +1,70 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"testing"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/channels"
|
||||
)
|
||||
|
||||
func TestListGroupsTool_Execute(t *testing.T) {
|
||||
tool := NewListGroupsTool()
|
||||
var gotChannel string
|
||||
tool.SetGroupLister(func(_ context.Context, channel string) ([]channels.GroupInfo, error) {
|
||||
gotChannel = channel
|
||||
return []channels.GroupInfo{
|
||||
{GroupID: "747300108647389888", Name: "Ban Điều Hành", TotalMember: 8},
|
||||
{GroupID: "1208490753003346779", Name: "Team Content", TotalMember: 12},
|
||||
}, nil
|
||||
})
|
||||
|
||||
ctx := WithToolChannel(context.Background(), "bunny-zalo-personal")
|
||||
res := tool.Execute(ctx, map[string]any{})
|
||||
if res == nil || res.IsError {
|
||||
t.Fatalf("unexpected error result: %+v", res)
|
||||
}
|
||||
if gotChannel != "bunny-zalo-personal" {
|
||||
t.Fatalf("lister called with channel %q, want bunny-zalo-personal", gotChannel)
|
||||
}
|
||||
|
||||
var out struct {
|
||||
Channel string `json:"channel"`
|
||||
Count int `json:"count"`
|
||||
Groups []struct {
|
||||
GroupID string `json:"group_id"`
|
||||
Name string `json:"name"`
|
||||
} `json:"groups"`
|
||||
}
|
||||
if err := json.Unmarshal([]byte(res.ForLLM), &out); err != nil {
|
||||
t.Fatalf("failed to parse tool output: %v (raw: %s)", err, res.ForLLM)
|
||||
}
|
||||
if out.Count != 2 {
|
||||
t.Fatalf("count = %d, want 2", out.Count)
|
||||
}
|
||||
if out.Groups[0].Name != "Ban Điều Hành" || out.Groups[0].GroupID != "747300108647389888" {
|
||||
t.Fatalf("unexpected first group: %+v", out.Groups[0])
|
||||
}
|
||||
}
|
||||
|
||||
func TestListGroupsTool_NoChannelInContext(t *testing.T) {
|
||||
tool := NewListGroupsTool()
|
||||
tool.SetGroupLister(func(_ context.Context, _ string) ([]channels.GroupInfo, error) {
|
||||
t.Fatal("lister should not be called without a channel in context")
|
||||
return nil, nil
|
||||
})
|
||||
|
||||
res := tool.Execute(context.Background(), map[string]any{})
|
||||
if res == nil || !res.IsError {
|
||||
t.Fatalf("expected error result when channel is missing from context, got: %+v", res)
|
||||
}
|
||||
}
|
||||
|
||||
func TestListGroupsTool_ListerUnset(t *testing.T) {
|
||||
tool := NewListGroupsTool()
|
||||
ctx := WithToolChannel(context.Background(), "bunny-zalo-personal")
|
||||
res := tool.Execute(ctx, map[string]any{})
|
||||
if res == nil || !res.IsError {
|
||||
t.Fatalf("expected error result when lister is unset, got: %+v", res)
|
||||
}
|
||||
}
|
||||
+37
-14
@@ -58,7 +58,7 @@ func (t *MessageTool) Parameters() map[string]any {
|
||||
},
|
||||
"target": map[string]any{
|
||||
"type": "string",
|
||||
"description": "Chat ID to send to (default: current chat from context)",
|
||||
"description": "Chat ID to send to (default: current chat from context). Must be a real chat/group ID, NOT a display name — passing a human-readable name here will fail (or silently go nowhere). If you only know a group by its display name, resolve the real ID first (e.g. zalo_list_groups on Zalo) instead of guessing.",
|
||||
},
|
||||
"message": map[string]any{
|
||||
"type": "string",
|
||||
@@ -191,13 +191,11 @@ func (t *MessageTool) Execute(ctx context.Context, args map[string]any) *Result
|
||||
// If we found embedded media and bus is available, prefer bus path (supports media attachments).
|
||||
if len(embeddedMedia) > 0 && t.msgBus != nil {
|
||||
outMsg := bus.OutboundMessage{
|
||||
Channel: channel,
|
||||
ChatID: target,
|
||||
Content: message,
|
||||
Media: embeddedMedia,
|
||||
}
|
||||
if isGroupContext(ctx) {
|
||||
outMsg.Metadata = map[string]string{"group_id": target}
|
||||
Channel: channel,
|
||||
ChatID: target,
|
||||
Content: message,
|
||||
Media: embeddedMedia,
|
||||
Metadata: t.buildOutboundMetadata(ctx, target, forwardReason),
|
||||
}
|
||||
t.msgBus.PublishOutbound(outMsg)
|
||||
// Mark each embedded media path as delivered.
|
||||
@@ -223,12 +221,10 @@ func (t *MessageTool) Execute(ctx context.Context, args map[string]any) *Result
|
||||
// can distinguish group sends from DMs.
|
||||
if t.msgBus != nil {
|
||||
outMsg := bus.OutboundMessage{
|
||||
Channel: channel,
|
||||
ChatID: target,
|
||||
Content: message,
|
||||
}
|
||||
if isGroupContext(ctx) {
|
||||
outMsg.Metadata = map[string]string{"group_id": target}
|
||||
Channel: channel,
|
||||
ChatID: target,
|
||||
Content: message,
|
||||
Metadata: t.buildOutboundMetadata(ctx, target, forwardReason),
|
||||
}
|
||||
t.msgBus.PublishOutbound(outMsg)
|
||||
return noticeOnSuccess(SilentResult(fmt.Sprintf(`{"status":"sent","channel":"%s","target":"%s"}`, channel, target)))
|
||||
@@ -245,6 +241,33 @@ func (t *MessageTool) Execute(ctx context.Context, args map[string]any) *Result
|
||||
return ErrorResult("no channel sender or message bus available")
|
||||
}
|
||||
|
||||
// buildOutboundMetadata merges group-context routing metadata with
|
||||
// forward-origin tracking. The bus consumer (dispatchOutbound) runs async
|
||||
// with no path back to this call, so when forwardReason is non-empty we
|
||||
// stash the origin channel/chat here — that's the only way a downstream
|
||||
// delivery failure can be reported back to the chat that requested the
|
||||
// forward instead of vanishing silently.
|
||||
func (t *MessageTool) buildOutboundMetadata(ctx context.Context, target, forwardReason string) map[string]string {
|
||||
var meta map[string]string
|
||||
if isGroupContext(ctx) {
|
||||
meta = map[string]string{"group_id": target}
|
||||
}
|
||||
if forwardReason == "" {
|
||||
return meta
|
||||
}
|
||||
origCh := ToolChannelFromCtx(ctx)
|
||||
origChat := ToolChatIDFromCtx(ctx)
|
||||
if origCh == "" || origChat == "" {
|
||||
return meta
|
||||
}
|
||||
if meta == nil {
|
||||
meta = make(map[string]string, 2)
|
||||
}
|
||||
meta[bus.MetaForwardOriginChannel] = origCh
|
||||
meta[bus.MetaForwardOriginChatID] = origChat
|
||||
return meta
|
||||
}
|
||||
|
||||
// validateChannelTenant checks the target channel belongs to the current tenant.
|
||||
// Returns an error Result if the send should be blocked, nil if allowed.
|
||||
func (t *MessageTool) validateChannelTenant(ctx context.Context, channel, target string) *Result {
|
||||
|
||||
@@ -840,6 +840,49 @@ func TestMessageToolCrossTargetGuard_NoNoticeOnSendFailure(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// A cross-target group forward must tag the outbound message with the origin
|
||||
// channel/chat (in addition to group_id) so dispatchOutbound can notify the
|
||||
// origin chat if the downstream send actually fails — otherwise a bad target
|
||||
// (e.g. a display name instead of a real chat ID) fails silently.
|
||||
func TestMessageToolForward_TagsOriginMetadataOnGroupSend(t *testing.T) {
|
||||
tool := NewMessageTool(t.TempDir(), false)
|
||||
mb := bus.New()
|
||||
tool.SetMessageBus(mb)
|
||||
|
||||
ctx := context.Background()
|
||||
ctx = WithToolSessionKey(ctx, "agent:a:zalo:group:747300108647389888")
|
||||
ctx = WithToolChannel(ctx, "bunny-zalo-personal")
|
||||
ctx = WithToolChatID(ctx, "747300108647389888")
|
||||
ctx = WithToolPeerKind(ctx, "group")
|
||||
|
||||
res := tool.Execute(ctx, map[string]any{
|
||||
"action": "send", "channel": "bunny-zalo-personal", "target": "Ban Điều Hành",
|
||||
"forward": true, "forward_reason": "forward to Ban Điều Hành",
|
||||
"message": "Anh Tài ơi, xem giúp comment khách hàng nhé.",
|
||||
})
|
||||
if res != nil && res.IsError {
|
||||
t.Fatalf("unexpected error: %s", res.ForLLM)
|
||||
}
|
||||
|
||||
got := drainBusNow(mb)
|
||||
if len(got) == 0 {
|
||||
t.Fatal("expected at least one outbound message")
|
||||
}
|
||||
forwardMsg := got[0]
|
||||
if forwardMsg.ChatID != "Ban Điều Hành" {
|
||||
t.Fatalf("forward target = %q, want %q", forwardMsg.ChatID, "Ban Điều Hành")
|
||||
}
|
||||
if forwardMsg.Metadata[bus.MetaForwardOriginChannel] != "bunny-zalo-personal" {
|
||||
t.Errorf("MetaForwardOriginChannel = %q, want %q", forwardMsg.Metadata[bus.MetaForwardOriginChannel], "bunny-zalo-personal")
|
||||
}
|
||||
if forwardMsg.Metadata[bus.MetaForwardOriginChatID] != "747300108647389888" {
|
||||
t.Errorf("MetaForwardOriginChatID = %q, want %q", forwardMsg.Metadata[bus.MetaForwardOriginChatID], "747300108647389888")
|
||||
}
|
||||
if forwardMsg.Metadata["group_id"] != "Ban Điều Hành" {
|
||||
t.Errorf("group_id metadata = %q, want %q (must survive alongside forward tracking)", forwardMsg.Metadata["group_id"], "Ban Điều Hành")
|
||||
}
|
||||
}
|
||||
|
||||
func TestMessageTargetEnforced(t *testing.T) {
|
||||
cases := []struct {
|
||||
key string
|
||||
|
||||
@@ -20,7 +20,7 @@ var builtinToolGroups = map[string][]string{
|
||||
"sessions": {"sessions_list", "sessions_history", "sessions_send", "spawn", "session_status"},
|
||||
"ui": {"browser"},
|
||||
"automation": {"cron"},
|
||||
"messaging": {"message", "create_forum_topic", "list_group_members"},
|
||||
"messaging": {"message", "create_forum_topic", "list_group_members", "zalo_list_groups"},
|
||||
"team": {"team_tasks"},
|
||||
"vault": {"vault_search", "vault_read"},
|
||||
// Composite group: all goclaw native tools (excludes MCP/custom plugins).
|
||||
@@ -32,7 +32,7 @@ var builtinToolGroups = map[string][]string{
|
||||
"sessions_list", "sessions_history", "sessions_send", "spawn", "session_status",
|
||||
"delegate",
|
||||
"cron", "datetime", "heartbeat",
|
||||
"message", "create_forum_topic", "list_group_members",
|
||||
"message", "create_forum_topic", "list_group_members", "zalo_list_groups",
|
||||
"read_image", "read_document", "read_audio", "read_video",
|
||||
"create_image", "create_video", "create_audio",
|
||||
"skill_search", "skill_manage", "publish_skill", "use_skill",
|
||||
|
||||
Reference in new issue
Block a user