From 150c2b3c0120ffa11cca7fe90bbb577fc0c2563c Mon Sep 17 00:00:00 2001 From: Duc Nguyen <111303583+Buns0705@users.noreply.github.com> Date: Sun, 5 Jul 2026 01:57:25 +0700 Subject: [PATCH] fix(zalo): correctly forward inbound reference images to the agent (#1353) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix(zalo): correctly forward inbound reference images to the agent Fixes #1352. Three compounding bugs prevented zalo_personal from using photos as vision/image-generation references: 1. Zalo CDN URLs often lack usable extensions, so downloads landed as .bin and were misclassified as documents (not images). fixDownloadedExt sniffs content and renames correctly. 2. Photo captions (user's actual instruction) were dropped — now appended after the media tag. 3. Quoted photos were never downloaded — extractQuoteMedia + ParseAttachment now fetch the actual image. Tests cover fixDownloadedExt and ParseAttachment helpers. Live-verified against production Zalo traffic. --- .../channels/zalo/personal/content-media.go | 75 ++++++++++++++++++- .../zalo/personal/content_media_ext_test.go | 56 ++++++++++++++ internal/channels/zalo/personal/handlers.go | 7 ++ .../zalo/personal/protocol/message_quote.go | 13 ++++ .../protocol/message_quote_parse_test.go | 46 ++++++++++++ 5 files changed, 196 insertions(+), 1 deletion(-) create mode 100644 internal/channels/zalo/personal/content_media_ext_test.go create mode 100644 internal/channels/zalo/personal/protocol/message_quote_parse_test.go diff --git a/internal/channels/zalo/personal/content-media.go b/internal/channels/zalo/personal/content-media.go index 7049f39c..7530b900 100644 --- a/internal/channels/zalo/personal/content-media.go +++ b/internal/channels/zalo/personal/content-media.go @@ -47,6 +47,17 @@ func extractContentAndMedia(content protocol.Content) (string, []string) { ContentType: mimeType, FileName: att.Title, }}) + // Zalo photo messages carry the user's caption in the attachment title. + // Keep it next to the media tag — dropping it loses the actual request text. + if att.IsImage() { + caption := strings.TrimSpace(att.Title) + if caption == "" { + caption = strings.TrimSpace(att.Description) + } + if caption != "" { + tag = tag + "\n" + caption + } + } return tag, []string{filePath} } @@ -111,5 +122,67 @@ func downloadFile(ctx context.Context, fileURL string) (string, error) { os.Remove(tmpFile.Name()) return "", fmt.Errorf("file too large: %d bytes (max %d)", written, maxMediaBytes) } - return tmpFile.Name(), nil + return fixDownloadedExt(tmpFile.Name()), nil +} + +// fixDownloadedExt renames a downloaded temp file to match its sniffed content +// type when the URL-derived extension misclassifies it. Zalo CDN URLs often +// carry no usable file extension, so downloads land as .bin — downstream media +// classification (persistMedia infers MIME from extension) would then treat an +// image as a document and the agent's vision input would silently skip it. +func fixDownloadedExt(path string) string { + f, err := os.Open(path) + if err != nil { + return path + } + buf := make([]byte, 512) + n, _ := io.ReadFull(f, buf) + f.Close() + if n == 0 { + return path + } + sniffed := http.DetectContentType(buf[:n]) + if !strings.HasPrefix(sniffed, "image/") { + return path + } + if strings.HasPrefix(media.DetectMIMEType(path), "image/") { + return path // extension already says image — nothing to fix + } + var ext string + switch sniffed { + case "image/jpeg": + ext = ".jpg" + case "image/png": + ext = ".png" + case "image/gif": + ext = ".gif" + case "image/webp": + ext = ".webp" + default: + return path + } + newPath := strings.TrimSuffix(path, filepath.Ext(path)) + ext + if err := os.Rename(path, newPath); err != nil { + return path + } + return newPath +} + +// extractQuoteMedia downloads the image attached to a quoted message, so a +// reply like “make a poster from this photo” actually carries the photo. +// Non-image quotes (files, stickers) stay text-only placeholders. +func extractQuoteMedia(quote *protocol.TQuote) []string { + if quote == nil { + return nil + } + att := quote.Attach.ParseAttachment() + if att == nil || !att.IsImage() || att.URL() == "" { + return nil + } + filePath, err := downloadFile(context.Background(), att.URL()) + if err != nil { + slog.Warn("zalo_personal: failed to download quoted attachment", "url", att.URL(), "error", err) + return nil + } + return []string{filePath} } diff --git a/internal/channels/zalo/personal/content_media_ext_test.go b/internal/channels/zalo/personal/content_media_ext_test.go new file mode 100644 index 00000000..6fe4bfcf --- /dev/null +++ b/internal/channels/zalo/personal/content_media_ext_test.go @@ -0,0 +1,56 @@ +package personal + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// Zalo CDN URLs often carry no usable file extension, so downloaded images +// land as ".bin". Downstream media classification infers MIME purely from +// the file extension, so a ".bin" image is silently treated as a document +// and never reaches the model's vision input. fixDownloadedExt must rename +// it based on sniffed content, regardless of the URL-derived extension. +func TestFixDownloadedExtRenamesSniffedImage(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "goclaw_zca_test.bin") + png := append([]byte{0x89, 'P', 'N', 'G', 0x0D, 0x0A, 0x1A, 0x0A}, make([]byte, 64)...) + if err := os.WriteFile(p, png, 0644); err != nil { + t.Fatal(err) + } + got := fixDownloadedExt(p) + if !strings.HasSuffix(got, ".png") { + t.Fatalf("fixDownloadedExt(%q) = %q, want .png suffix", p, got) + } + if _, err := os.Stat(got); err != nil { + t.Fatalf("renamed file missing: %v", err) + } +} + +// Non-image content (documents, etc.) must be left untouched — this fix is +// scoped to unblocking vision input, not reclassifying arbitrary files. +func TestFixDownloadedExtLeavesNonImage(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "goclaw_zca_doc.bin") + if err := os.WriteFile(p, []byte("%PDF-1.4 not an image"), 0644); err != nil { + t.Fatal(err) + } + if got := fixDownloadedExt(p); got != p { + t.Fatalf("fixDownloadedExt(%q) = %q, want unchanged", p, got) + } +} + +// A file whose extension already matches an image type must not be touched, +// even though sniffing would also say "image" — avoids pointless renames. +func TestFixDownloadedExtLeavesCorrectExtension(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "goclaw_zca_test.jpg") + jpg := append([]byte{0xFF, 0xD8, 0xFF, 0xE0}, make([]byte, 64)...) + if err := os.WriteFile(p, jpg, 0644); err != nil { + t.Fatal(err) + } + if got := fixDownloadedExt(p); got != p { + t.Fatalf("fixDownloadedExt(%q) = %q, want unchanged (already .jpg)", p, got) + } +} diff --git a/internal/channels/zalo/personal/handlers.go b/internal/channels/zalo/personal/handlers.go index 3a55ae25..6e856a71 100644 --- a/internal/channels/zalo/personal/handlers.go +++ b/internal/channels/zalo/personal/handlers.go @@ -37,6 +37,9 @@ 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)...) content := replycontext.Compose(c.buildReplyContext(threadID, msg.Data.Quote), body) if content == "" { return @@ -88,6 +91,9 @@ 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)...) content := replycontext.Compose(c.buildReplyContext(threadID, msg.Data.Quote), body) if content == "" { return @@ -179,5 +185,6 @@ 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 } diff --git a/internal/channels/zalo/personal/protocol/message_quote.go b/internal/channels/zalo/personal/protocol/message_quote.go index f0655488..ab09087d 100644 --- a/internal/channels/zalo/personal/protocol/message_quote.go +++ b/internal/channels/zalo/personal/protocol/message_quote.go @@ -101,6 +101,19 @@ func (a QuoteAttachment) MarshalJSON() ([]byte, error) { return a.Raw, nil } +// ParseAttachment decodes the quoted attachment payload into an Attachment. +// Returns nil when there is no payload or it does not decode as an object. +func (a QuoteAttachment) ParseAttachment() *Attachment { + if len(a.Raw) == 0 { + return nil + } + var att Attachment + if err := json.Unmarshal(a.Raw, &att); err != nil { + return nil + } + return &att +} + func (a QuoteAttachment) AttachmentText() string { if len(a.Raw) == 0 { return "" diff --git a/internal/channels/zalo/personal/protocol/message_quote_parse_test.go b/internal/channels/zalo/personal/protocol/message_quote_parse_test.go new file mode 100644 index 00000000..1df2c2ee --- /dev/null +++ b/internal/channels/zalo/personal/protocol/message_quote_parse_test.go @@ -0,0 +1,46 @@ +package protocol + +import ( + "encoding/json" + "testing" +) + +// ParseAttachment must decode the quoted attachment's own JSON payload (not +// just render it as placeholder text) so callers can reach the image URL to +// download it — a reply to a photo needs the actual photo, not "[Quoted image]". +func TestQuoteAttachment_ParseAttachment(t *testing.T) { + t.Run("image quote decodes to Attachment with URL", func(t *testing.T) { + var q TQuote + payload := `{"ownerId":"1","cliMsgId":"2","globalMsgId":"3","msg":"","attach":"{\"type\":\"image\",\"title\":\"photo.jpg\",\"href\":\"https://example.com/photo.jpg\"}","fromD":"Anh"}` + if err := json.Unmarshal([]byte(payload), &q); err != nil { + t.Fatalf("unmarshal TQuote: %v", err) + } + att := q.Attach.ParseAttachment() + if att == nil { + t.Fatal("ParseAttachment() = nil, want decoded Attachment") + } + if !att.IsImage() { + t.Errorf("IsImage() = false, want true for %+v", att) + } + if att.URL() != "https://example.com/photo.jpg" { + t.Errorf("URL() = %q, want https://example.com/photo.jpg", att.URL()) + } + }) + + t.Run("no attachment payload returns nil", func(t *testing.T) { + var a QuoteAttachment + if got := a.ParseAttachment(); got != nil { + t.Errorf("ParseAttachment() = %+v, want nil for empty payload", got) + } + }) + + t.Run("non-object payload (plain quoted text) returns nil", func(t *testing.T) { + var a QuoteAttachment + if err := json.Unmarshal([]byte(`"just a string"`), &a); err != nil { + t.Fatalf("unmarshal QuoteAttachment: %v", err) + } + if got := a.ParseAttachment(); got != nil { + t.Errorf("ParseAttachment() = %+v, want nil for plain-text payload", got) + } + }) +}