From ee37017e0edfb23110e3eeb674b04a8902c603a0 Mon Sep 17 00:00:00 2001 From: Duc Nguyen <111303583+Buns0705@users.noreply.github.com> Date: Thu, 9 Jul 2026 07:55:02 +0700 Subject: [PATCH] fix: warm Zalo group-approval cache + attach quoted-image content tag (#1402) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: warm Zalo group-approval cache so forwards from non-group origins route correctly send.go picks ThreadTypeGroup vs ThreadTypeUser for a target chat ID by checking BaseChannel's approvedGroups cache (or an explicit "group_id" metadata key). That cache is only ever populated from INBOUND group traffic under the "pairing" group policy — under "allowlist" (this integration's actual default) it stays empty for the whole process lifetime unless something else populates it, and it's wiped on every restart regardless (in-memory sync.Map, no persistence). message.go's cross-target forward only attaches "group_id" metadata when isGroupContext(ctx) is true, which reflects the ORIGIN session's peer kind, not the destination target's. Forwarding from a DM (or any non-group session) into a group therefore sends with no group_id metadata and an empty approval cache, so Send() defaults to ThreadTypeUser — the message goes out addressed as if to a user account, not the group, and is never seen there. No error is returned anywhere in this path, so nothing in the existing (or previously fixed) error-reporting surfaces it. ListGroups now marks every returned group ID as approved via MarkGroupApproved, and Channel.Start kicks off a best-effort ListGroups call right after connecting so the cache is warm from process start, not just after an explicit zalo_list_groups tool call. * fix: expose a media tag for quote-forwarded images, not just vision access extractQuoteMedia downloads the image attached to a quoted message so the model can see it via vision, but the composed content only ever carried the literal "[Quoted image]" placeholder text (from TQuote.Text()) — no tag was ever inserted for it. The later agent-side enrichment (enrichImageIDs/enrichImagePaths) only fills id/path attributes into a bare tag it finds already present in content; with no tag to find, the downloaded file had no path reference the model could hand back to message(MEDIA:) to forward it elsewhere, even though the file was sitting on disk the whole time. Symptom: asked to forward a quoted image to another chat, the agent reported "the image is only a quote, no direct file to attach" despite genuinely having already downloaded it. buildQuoteMediaTag renders the same bare tag a direct (non-quoted) attachment gets, appended after the quote wrapper text in all three call sites (handleDM, handleGroupMessage, extractContentAndMediaWithQuote) — ordered after any of the current message's own attachment tags to match the media slice's append order, so positional id/path enrichment lines up correctly. --- internal/channels/zalo/personal/channel.go | 23 +++++++++++++++ .../channels/zalo/personal/content-media.go | 26 +++++++++++++++++ .../personal/content_media_quote_tag_test.go | 28 +++++++++++++++++++ internal/channels/zalo/personal/handlers.go | 27 +++++++++++++++--- 4 files changed, 100 insertions(+), 4 deletions(-) create mode 100644 internal/channels/zalo/personal/content_media_quote_tag_test.go diff --git a/internal/channels/zalo/personal/channel.go b/internal/channels/zalo/personal/channel.go index 2fa93d55..c0c0ac77 100644 --- a/internal/channels/zalo/personal/channel.go +++ b/internal/channels/zalo/personal/channel.go @@ -96,6 +96,17 @@ func (c *Channel) getListener() *protocol.Listener { // 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. +// +// Also warms the approvedGroups cache (BaseChannel.MarkGroupApproved) for +// every returned group ID. Send() uses that cache to decide whether a +// target is a group (ThreadTypeGroup) or a user (ThreadTypeUser) when the +// outbound message carries no explicit "group_id" metadata — which is +// exactly the case for a message-tool forward whose ORIGIN session isn't +// itself a group (e.g. forwarding from a DM into a group): the cache is +// otherwise only populated by inbound group traffic under the "pairing" +// group policy, so under "allowlist" (this deployment's default) a +// same-session-never-messaged-before group would default to ThreadTypeUser +// and the send would go nowhere, with no error surfaced anywhere. func (c *Channel) ListGroups(ctx context.Context) ([]channels.GroupInfo, error) { sess := c.session() if sess == nil { @@ -108,6 +119,7 @@ func (c *Channel) ListGroups(ctx context.Context) ([]channels.GroupInfo, error) out := make([]channels.GroupInfo, len(groups)) for i, g := range groups { out[i] = channels.GroupInfo{GroupID: g.GroupID, Name: g.Name, TotalMember: g.TotalMember} + c.MarkGroupApproved(g.GroupID) } return out, nil } @@ -144,6 +156,17 @@ func (c *Channel) Start(ctx context.Context) error { c.SetRunning(true) go c.listenLoop(ctx) + // Best-effort: warm the approvedGroups cache so a forward TO a group works + // correctly even if its origin session was never itself a group (see + // ListGroups doc comment). approvedGroups is in-memory only and wiped on + // every restart, so this must run again on every Start, not just once + // per account lifetime. Failure here (e.g. transient API error) is not + // fatal — the cache just stays cold until the next successful listing. + go func() { + if _, err := c.ListGroups(ctx); err != nil { + slog.Warn("zalo_personal: failed to warm group approval cache on start", "error", err) + } + }() slog.Info("zalo_personal listener loop started") return nil diff --git a/internal/channels/zalo/personal/content-media.go b/internal/channels/zalo/personal/content-media.go index 7530b900..f9d66fdd 100644 --- a/internal/channels/zalo/personal/content-media.go +++ b/internal/channels/zalo/personal/content-media.go @@ -186,3 +186,29 @@ func extractQuoteMedia(quote *protocol.TQuote) []string { } return []string{filePath} } + +// buildQuoteMediaTag renders a bare content tag for each image +// downloaded from a quoted message (extractQuoteMedia). +// +// Without this, a quote-forwarded image has NO reference anywhere in the +// text content — it's only reachable through the Media list, which lets the +// model see it via vision but gives it no path to hand back to +// message(MEDIA:) to forward the file elsewhere. The later agent-side +// enrichment (enrichImageIDs/enrichImagePaths in internal/agent/media.go) +// only fills in id/path attributes on a bare tag it finds already present in +// content — it has nothing to enrich if no tag exists at all, which was the +// actual bug (the model reported "no direct file/path" even though the file +// was sitting on disk). Order matters: this must be appended to content +// AFTER any tag from the current message's own attachment, matching the +// order paths are appended in the media slice (own attachment first, quoted +// image second), so positional id/path enrichment lines up correctly. +func buildQuoteMediaTag(paths []string) string { + if len(paths) == 0 { + return "" + } + tags := make([]string, len(paths)) + for i := range paths { + tags[i] = "" + } + return strings.Join(tags, "\n") +} diff --git a/internal/channels/zalo/personal/content_media_quote_tag_test.go b/internal/channels/zalo/personal/content_media_quote_tag_test.go new file mode 100644 index 00000000..3080a363 --- /dev/null +++ b/internal/channels/zalo/personal/content_media_quote_tag_test.go @@ -0,0 +1,28 @@ +package personal + +import "testing" + +// Without a tag in content, the later agent-side enrichment step +// (enrichImageIDs/enrichImagePaths) has nothing to attach a real file path +// to — a downloaded quoted image would be reachable only via vision, with +// no way for the model to reference it for forwarding. buildQuoteMediaTag +// closes that gap by rendering the same bare placeholder a +// direct (non-quoted) attachment gets. +func TestBuildQuoteMediaTag(t *testing.T) { + tests := []struct { + name string + paths []string + want string + }{ + {"no paths", nil, ""}, + {"single image", []string{"/tmp/goclaw_zca_abc123.jpg"}, ""}, + {"multiple images", []string{"/tmp/a.jpg", "/tmp/b.png"}, "\n"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := buildQuoteMediaTag(tt.paths); got != tt.want { + t.Errorf("buildQuoteMediaTag(%v) = %q, want %q", tt.paths, got, tt.want) + } + }) + } +} diff --git a/internal/channels/zalo/personal/handlers.go b/internal/channels/zalo/personal/handlers.go index 6e856a71..c200f2ab 100644 --- a/internal/channels/zalo/personal/handlers.go +++ b/internal/channels/zalo/personal/handlers.go @@ -39,11 +39,18 @@ func (c *Channel) handleDM(msg protocol.UserMessage) { body, media := extractContentAndMedia(msg.Data.Content) // A reply to an image should carry the image itself, not just the // "[Quoted image]" placeholder — download it and attach as media. - media = append(media, extractQuoteMedia(msg.Data.Quote)...) + quoteMedia := extractQuoteMedia(msg.Data.Quote) + media = append(media, quoteMedia...) content := replycontext.Compose(c.buildReplyContext(threadID, msg.Data.Quote), body) if content == "" { return } + // The quote wrapper text above is just the "[Quoted image]" placeholder — + // append a real tag so the model has a path to reference + // (e.g. to forward the file elsewhere), not just vision access to it. + if tag := buildQuoteMediaTag(quoteMedia); tag != "" { + content = content + "\n" + tag + } // Annotate with sender display name so the agent knows who is messaging. senderName := msg.Data.DName @@ -93,11 +100,18 @@ func (c *Channel) handleGroupMessage(msg protocol.GroupMessage) { body, media := extractContentAndMedia(msg.Data.Content) // A reply to an image should carry the image itself, not just the // "[Quoted image]" placeholder — download it and attach as media. - media = append(media, extractQuoteMedia(msg.Data.Quote)...) + quoteMedia := extractQuoteMedia(msg.Data.Quote) + media = append(media, quoteMedia...) content := replycontext.Compose(c.buildReplyContext(threadID, msg.Data.Quote), body) if content == "" { return } + // The quote wrapper text above is just the "[Quoted image]" placeholder — + // append a real tag so the model has a path to reference + // (e.g. to forward the file elsewhere), not just vision access to it. + if tag := buildQuoteMediaTag(quoteMedia); tag != "" { + content = content + "\n" + tag + } c.rememberReplyContext(threadID, senderName, msg.Data.TMessage, cacheBodyForContent(msg.Data.Content)) // Step 2: @mention gating — record non-mentioned messages in history and return. @@ -185,6 +199,11 @@ func (c *Channel) startTyping(threadID string, threadType protocol.ThreadType) { func extractContentAndMediaWithQuote(content protocol.Content, quote *protocol.TQuote) (string, []string) { text, media := extractContentAndMedia(content) - media = append(media, extractQuoteMedia(quote)...) - return replycontext.Compose(formatQuoteContext(quote), text), media + quoteMedia := extractQuoteMedia(quote) + media = append(media, quoteMedia...) + composed := replycontext.Compose(formatQuoteContext(quote), text) + if tag := buildQuoteMediaTag(quoteMedia); tag != "" { + composed = composed + "\n" + tag + } + return composed, media }