diff --git a/cmd/gateway.go b/cmd/gateway.go index 34faedc0..5bcdbe27 100644 --- a/cmd/gateway.go +++ b/cmd/gateway.go @@ -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 { diff --git a/cmd/gateway_tools_wiring.go b/cmd/gateway_tools_wiring.go index eaa5f8e2..702856b6 100644 --- a/cmd/gateway_tools_wiring.go +++ b/cmd/gateway_tools_wiring.go @@ -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)) diff --git a/internal/agent/systemprompt.go b/internal/agent/systemprompt.go index 2a15b6df..9dbf1591 100644 --- a/internal/agent/systemprompt.go +++ b/internal/agent/systemprompt.go @@ -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", diff --git a/internal/bus/types.go b/internal/bus/types.go index a37af650..870a736a 100644 --- a/internal/bus/types.go +++ b/internal/bus/types.go @@ -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 diff --git a/internal/channels/channel.go b/internal/channels/channel.go index dde012e3..d5519a0b 100644 --- a/internal/channels/channel.go +++ b/internal/channels/channel.go @@ -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 { diff --git a/internal/channels/dispatch.go b/internal/channels/dispatch.go index 623dd361..6febd4a3 100644 --- a/internal/channels/dispatch.go +++ b/internal/channels/dispatch.go @@ -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 { diff --git a/internal/channels/dispatch_test.go b/internal/channels/dispatch_test.go new file mode 100644 index 00000000..4ba41ecf --- /dev/null +++ b/internal/channels/dispatch_test.go @@ -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) + } +} diff --git a/internal/channels/list_groups_test.go b/internal/channels/list_groups_test.go new file mode 100644 index 00000000..a0be30c1 --- /dev/null +++ b/internal/channels/list_groups_test.go @@ -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") + } +} diff --git a/internal/channels/manager.go b/internal/channels/manager.go index cd2e345f..bcec6766 100644 --- a/internal/channels/manager.go +++ b/internal/channels/manager.go @@ -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() diff --git a/internal/channels/zalo/personal/channel.go b/internal/channels/zalo/personal/channel.go index e31fdec5..2fa93d55 100644 --- a/internal/channels/zalo/personal/channel.go +++ b/internal/channels/zalo/personal/channel.go @@ -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 { diff --git a/internal/tools/list_groups.go b/internal/tools/list_groups.go new file mode 100644 index 00000000..d08bec9c --- /dev/null +++ b/internal/tools/list_groups.go @@ -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)) +} diff --git a/internal/tools/list_groups_test.go b/internal/tools/list_groups_test.go new file mode 100644 index 00000000..a15f44b6 --- /dev/null +++ b/internal/tools/list_groups_test.go @@ -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) + } +} diff --git a/internal/tools/message.go b/internal/tools/message.go index b0a6eca0..ab212613 100644 --- a/internal/tools/message.go +++ b/internal/tools/message.go @@ -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 { diff --git a/internal/tools/message_test.go b/internal/tools/message_test.go index 45af62de..47919796 100644 --- a/internal/tools/message_test.go +++ b/internal/tools/message_test.go @@ -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 diff --git a/internal/tools/policy.go b/internal/tools/policy.go index 4c35ae8c..329525c9 100644 --- a/internal/tools/policy.go +++ b/internal/tools/policy.go @@ -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",