From 5ae58157383010e839733a56b20ec5ad3ca39910 Mon Sep 17 00:00:00 2001 From: Duy /zuey/ Date: Sun, 24 May 2026 12:48:51 +0700 Subject: [PATCH] fix(telegram): preserve archive upload paths (#58) --- docs/14-skills-runtime.md | 10 +-- internal/agent/loop_input_media.go | 5 +- internal/agent/media_persist_test.go | 8 +++ internal/agent/media_test.go | 28 ++++++++ internal/channels/history.go | 8 ++- internal/channels/telegram/handlers.go | 18 ++---- internal/channels/telegram/media.go | 67 +++++++++++++++++--- internal/channels/telegram/media_test.go | 54 ++++++++++++++++ internal/media/store.go | 6 ++ internal/media/store_test.go | 23 +++++++ internal/tools/read_document_resolve.go | 3 +- internal/tools/read_document_resolve_test.go | 19 +++++- 12 files changed, 217 insertions(+), 32 deletions(-) create mode 100644 internal/media/store_test.go diff --git a/docs/14-skills-runtime.md b/docs/14-skills-runtime.md index ac8fe740..7b91ff02 100644 --- a/docs/14-skills-runtime.md +++ b/docs/14-skills-runtime.md @@ -135,7 +135,7 @@ To install additional packages: pip3 install or npm install -g - Run Python/Node scripts via exec tool - Install packages via `pip3 install` / `npm install -g` -- Access files in `/app/workspace/` including `.media/` subdirectory +- Access files in `/app/workspace/`, including `.uploads/` for current user uploads and `.media/` for legacy media refs - Read skill files from `.goclaw/skills-store/` ### What Agents CANNOT Do @@ -152,16 +152,18 @@ To install additional packages: pip3 install or npm install -g Uploaded files (from web chat, Telegram, Discord, etc.) are persisted to: ``` -/app/workspace/.media/{sessionHash}/{uuid}.{ext} +/app/workspace/.uploads/{safe-original-name}-{8hex}.{ext} ``` +Uploads without a usable original filename fall back to `{uuid}.{ext}`. Legacy media refs may still resolve from `.media/{sessionHash}/{uuid}.{ext}`. + The `enrichDocumentPaths()` function injects the full path into `` tags: ``` - + ``` -Agents can read these files directly via exec — no copy to `/tmp` needed. +Agents can read these files directly via exec — no copy to `/tmp` needed. For archive uploads such as `.zip`, inspect or extract with commands like `unzip -l ""` or `unzip -q "" -d `. --- diff --git a/internal/agent/loop_input_media.go b/internal/agent/loop_input_media.go index fa7989eb..49cb3394 100644 --- a/internal/agent/loop_input_media.go +++ b/internal/agent/loop_input_media.go @@ -27,10 +27,11 @@ func isTextMime(mime string) bool { } // collectRefsByKind gathers MediaRefs of a given kind from message history -// (reverse order) and current-turn refs. Historical first, current last. +// in chronological order, then appends current-turn refs. The last ref is the +// newest document for read_document's omitted media_id fallback. func collectRefsByKind(messages []providers.Message, currentRefs []providers.MediaRef, kind string) []providers.MediaRef { var refs []providers.MediaRef - for i := len(messages) - 1; i >= 0; i-- { + for i := range messages { for _, ref := range messages[i].MediaRefs { if ref.Kind == kind { refs = append(refs, ref) diff --git a/internal/agent/media_persist_test.go b/internal/agent/media_persist_test.go index 297f4a94..b819d208 100644 --- a/internal/agent/media_persist_test.go +++ b/internal/agent/media_persist_test.go @@ -72,6 +72,14 @@ func TestPersistMedia_NamingScheme(t *testing.T) { wantSlug: "", wantExtPat: `\.pdf`, }, + { + name: "zip_archive_preserves_archive_extension", + filename: "codex.zip", + content: "zip bytes", + mime: "application/zip", + wantSlug: "codex", + wantExtPat: `\.zip`, + }, } var loop Loop diff --git a/internal/agent/media_test.go b/internal/agent/media_test.go index cd66173c..f5fd4144 100644 --- a/internal/agent/media_test.go +++ b/internal/agent/media_test.go @@ -73,6 +73,34 @@ func TestEnrichImageIDs_SkipsAlreadyEnriched(t *testing.T) { } } +func TestCollectRefsByKindOrdersOldestToNewestThenCurrent(t *testing.T) { + messages := []providers.Message{ + { + Role: "user", + Content: "old", + MediaRefs: []providers.MediaRef{{ID: "old-doc", Kind: "document"}}, + }, + { + Role: "user", + Content: "latest-history", + MediaRefs: []providers.MediaRef{{ID: "latest-history-doc", Kind: "document"}}, + }, + } + current := []providers.MediaRef{{ID: "current-doc", Kind: "document"}} + + got := collectRefsByKind(messages, current, "document") + want := []string{"old-doc", "latest-history-doc", "current-doc"} + + if len(got) != len(want) { + t.Fatalf("got %d refs, want %d", len(got), len(want)) + } + for i := range want { + if got[i].ID != want[i] { + t.Fatalf("ref[%d] = %q, want %q", i, got[i].ID, want[i]) + } + } +} + // testMediaStore creates a temporary media.Store for tests. func testMediaStore(t *testing.T) *media.Store { t.Helper() diff --git a/internal/channels/history.go b/internal/channels/history.go index 36692857..054d61a4 100644 --- a/internal/channels/history.go +++ b/internal/channels/history.go @@ -36,9 +36,11 @@ const ( // Stored in RAM only (not persisted to DB) — used by channels that defer media // download until the bot is actually mentioned (e.g. Telegram). type MediaRef struct { - Type string // "image", "video", "audio", "voice", "document", "animation" - FileID string // platform-specific file ID for lazy download - FileSize int64 // file size in bytes (0 if unknown) — used to skip large files + Type string // "image", "video", "audio", "voice", "document", "animation" + FileID string // platform-specific file ID for lazy download + FileSize int64 // file size in bytes (0 if unknown) — used to skip large files + FileName string // original filename, if the platform exposes one + ContentType string // MIME type, if the platform exposes one } // HistoryEntry represents a single tracked group message. diff --git a/internal/channels/telegram/handlers.go b/internal/channels/telegram/handlers.go index 7999103c..685d621f 100644 --- a/internal/channels/telegram/handlers.go +++ b/internal/channels/telegram/handlers.go @@ -513,14 +513,8 @@ func (c *Channel) handleMessage(ctx context.Context, update telego.Update) { // Resolve deferred media from history entries (lazy download). if histRefs := c.GroupHistory().CollectMediaRefs(localKey); len(histRefs) > 0 { histMedia, histErrors := c.resolveMediaRefs(ctx, histRefs) - for _, m := range histMedia { - mediaFiles = append(mediaFiles, bus.MediaFile{ - Path: m.FilePath, - MimeType: m.ContentType, - Filename: m.FileName, - }) - } if len(histMedia) > 0 { + mediaFiles = prependMediaInfoFiles(mediaFiles, histMedia) histTags := buildMediaTags(histMedia) annotated = "[Media from recent group messages — only analyze if user asks about them]\n" + histTags + "\n[/Media]\n\n" + annotated } @@ -577,12 +571,12 @@ func (c *Channel) handleMessage(ctx context.Context, update telego.Update) { // user sees typing indicator → first content appears directly. metadata := map[string]string{ - "message_id": fmt.Sprintf("%d", message.MessageID), - "user_id": fmt.Sprintf("%d", user.ID), + "message_id": fmt.Sprintf("%d", message.MessageID), + "user_id": fmt.Sprintf("%d", user.ID), tools.MetaUsername: user.Username, - "first_name": user.FirstName, - "is_group": fmt.Sprintf("%t", isGroup), - "local_key": localKey, + "first_name": user.FirstName, + "is_group": fmt.Sprintf("%t", isGroup), + "local_key": localKey, } if message.Chat.Title != "" { metadata[tools.MetaChatTitle] = message.Chat.Title diff --git a/internal/channels/telegram/media.go b/internal/channels/telegram/media.go index a7d92d02..1a1faa8c 100644 --- a/internal/channels/telegram/media.go +++ b/internal/channels/telegram/media.go @@ -14,6 +14,7 @@ import ( "github.com/mymmrac/telego" + "github.com/nextlevelbuilder/goclaw/internal/bus" "github.com/nextlevelbuilder/goclaw/internal/channels" "github.com/nextlevelbuilder/goclaw/internal/channels/media" "github.com/nextlevelbuilder/goclaw/internal/tools" @@ -359,6 +360,24 @@ func buildMediaTags(mediaList []MediaInfo) string { return media.BuildMediaTags(mediaList) } +func prependMediaInfoFiles(current []bus.MediaFile, history []MediaInfo) []bus.MediaFile { + if len(history) == 0 { + return current + } + ordered := make([]bus.MediaFile, 0, len(history)+len(current)) + for _, m := range history { + if m.FilePath == "" { + continue + } + ordered = append(ordered, bus.MediaFile{ + Path: m.FilePath, + MimeType: m.ContentType, + Filename: m.FileName, + }) + } + return append(ordered, current...) +} + // extractDocumentContent delegates to the shared media package. func extractDocumentContent(filePath, fileName string) (string, error) { return media.ExtractDocumentContent(filePath, fileName) @@ -412,22 +431,51 @@ func extractMediaRefs(msg *telego.Message) []channels.MediaRef { refs = append(refs, channels.MediaRef{Type: "image", FileID: photo.FileID, FileSize: int64(photo.FileSize)}) } if msg.Video != nil { - refs = append(refs, channels.MediaRef{Type: "video", FileID: msg.Video.FileID, FileSize: int64(msg.Video.FileSize)}) + refs = append(refs, channels.MediaRef{ + Type: "video", + FileID: msg.Video.FileID, + FileSize: int64(msg.Video.FileSize), + FileName: msg.Video.FileName, + ContentType: msg.Video.MimeType, + }) } if msg.VideoNote != nil { refs = append(refs, channels.MediaRef{Type: "video", FileID: msg.VideoNote.FileID, FileSize: int64(msg.VideoNote.FileSize)}) } if msg.Animation != nil { - refs = append(refs, channels.MediaRef{Type: "animation", FileID: msg.Animation.FileID, FileSize: int64(msg.Animation.FileSize)}) + refs = append(refs, channels.MediaRef{ + Type: "animation", + FileID: msg.Animation.FileID, + FileSize: int64(msg.Animation.FileSize), + FileName: msg.Animation.FileName, + ContentType: msg.Animation.MimeType, + }) } if msg.Audio != nil { - refs = append(refs, channels.MediaRef{Type: "audio", FileID: msg.Audio.FileID, FileSize: int64(msg.Audio.FileSize)}) + refs = append(refs, channels.MediaRef{ + Type: "audio", + FileID: msg.Audio.FileID, + FileSize: int64(msg.Audio.FileSize), + FileName: msg.Audio.FileName, + ContentType: msg.Audio.MimeType, + }) } if msg.Voice != nil { - refs = append(refs, channels.MediaRef{Type: "voice", FileID: msg.Voice.FileID, FileSize: int64(msg.Voice.FileSize)}) + refs = append(refs, channels.MediaRef{ + Type: "voice", + FileID: msg.Voice.FileID, + FileSize: int64(msg.Voice.FileSize), + ContentType: msg.Voice.MimeType, + }) } if msg.Document != nil { - refs = append(refs, channels.MediaRef{Type: "document", FileID: msg.Document.FileID, FileSize: int64(msg.Document.FileSize)}) + refs = append(refs, channels.MediaRef{ + Type: "document", + FileID: msg.Document.FileID, + FileSize: int64(msg.Document.FileSize), + FileName: msg.Document.FileName, + ContentType: msg.Document.MimeType, + }) } return refs } @@ -492,9 +540,12 @@ func (c *Channel) resolveMediaRefs(ctx context.Context, refs []channels.MediaRef continue } results = append(results, MediaInfo{ - Type: ref.Type, - FilePath: filePath, - FileID: ref.FileID, + Type: ref.Type, + FilePath: filePath, + FileID: ref.FileID, + ContentType: ref.ContentType, + FileName: ref.FileName, + FileSize: ref.FileSize, }) } return results, errs diff --git a/internal/channels/telegram/media_test.go b/internal/channels/telegram/media_test.go index f568db34..a9822c70 100644 --- a/internal/channels/telegram/media_test.go +++ b/internal/channels/telegram/media_test.go @@ -3,6 +3,11 @@ package telegram import ( "strings" "testing" + + "github.com/mymmrac/telego" + + "github.com/nextlevelbuilder/goclaw/internal/bus" + "github.com/nextlevelbuilder/goclaw/internal/channels" ) // --- buildMediaTags tests --- @@ -148,3 +153,52 @@ func TestBuildMediaTags_UnknownType(t *testing.T) { t.Errorf("expected empty string for unknown type, got: %q", got) } } + +func TestExtractMediaRefsPreservesDocumentMetadata(t *testing.T) { + msg := &telego.Message{ + Document: &telego.Document{ + FileID: "telegram-file-id", + FileName: "codex.zip", + MimeType: "application/zip", + FileSize: 1234, + }, + } + + got := extractMediaRefs(msg) + want := []channels.MediaRef{{ + Type: "document", + FileID: "telegram-file-id", + FileSize: 1234, + FileName: "codex.zip", + ContentType: "application/zip", + }} + + if len(got) != len(want) { + t.Fatalf("got %d refs, want %d", len(got), len(want)) + } + if got[0] != want[0] { + t.Fatalf("ref = %+v, want %+v", got[0], want[0]) + } +} + +func TestPrependMediaInfoFilesKeepsHistoryBeforeCurrent(t *testing.T) { + current := []bus.MediaFile{{ + Path: "/workspace/.uploads/current.pdf", + MimeType: "application/pdf", + Filename: "current.pdf", + }} + history := []MediaInfo{{ + Type: "document", + FilePath: "/workspace/.uploads/codex.zip", + ContentType: "application/zip", + FileName: "codex.zip", + }} + + got := prependMediaInfoFiles(current, history) + if len(got) != 2 { + t.Fatalf("got %d files, want 2", len(got)) + } + if got[0].Path != history[0].FilePath || got[1].Path != current[0].Path { + t.Fatalf("order = [%q, %q], want history before current", got[0].Path, got[1].Path) + } +} diff --git a/internal/media/store.go b/internal/media/store.go index 162871b6..f9cada68 100644 --- a/internal/media/store.go +++ b/internal/media/store.go @@ -109,6 +109,12 @@ func ExtFromMime(mime string) string { return ".wav" case strings.HasPrefix(mime, "application/pdf"): return ".pdf" + case strings.HasPrefix(mime, "application/zip"), strings.HasPrefix(mime, "application/x-zip-compressed"): + return ".zip" + case strings.HasPrefix(mime, "application/x-tar"): + return ".tar" + case strings.HasPrefix(mime, "application/gzip"), strings.HasPrefix(mime, "application/x-gzip"): + return ".gz" case mime == "application/vnd.openxmlformats-officedocument.wordprocessingml.document": return ".docx" case mime == "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet": diff --git a/internal/media/store_test.go b/internal/media/store_test.go new file mode 100644 index 00000000..e8d1a12d --- /dev/null +++ b/internal/media/store_test.go @@ -0,0 +1,23 @@ +package media + +import "testing" + +func TestExtFromMimeArchiveTypes(t *testing.T) { + tests := []struct { + mime string + want string + }{ + {mime: "application/zip", want: ".zip"}, + {mime: "application/x-zip-compressed", want: ".zip"}, + {mime: "application/x-tar", want: ".tar"}, + {mime: "application/gzip", want: ".gz"}, + } + + for _, tt := range tests { + t.Run(tt.mime, func(t *testing.T) { + if got := ExtFromMime(tt.mime); got != tt.want { + t.Fatalf("ExtFromMime(%q) = %q, want %q", tt.mime, got, tt.want) + } + }) + } +} diff --git a/internal/tools/read_document_resolve.go b/internal/tools/read_document_resolve.go index 47d72cf9..a84b2870 100644 --- a/internal/tools/read_document_resolve.go +++ b/internal/tools/read_document_resolve.go @@ -43,8 +43,7 @@ func (t *ReadDocumentTool) resolveDocumentFile(ctx context.Context, mediaID, doc } } if ref == nil { - slog.Warn("read_document: media_id not found, falling back to most recent", "media_id", mediaID) - ref = &refs[len(refs)-1] + return "", "", fmt.Errorf("document media_id %q not found in this conversation", mediaID) } } else { // Use the last (most recent) document ref. diff --git a/internal/tools/read_document_resolve_test.go b/internal/tools/read_document_resolve_test.go index bc2ac937..172ac87f 100644 --- a/internal/tools/read_document_resolve_test.go +++ b/internal/tools/read_document_resolve_test.go @@ -66,7 +66,7 @@ func TestResolveDocumentFileMatchesUploadedFilenameAlias(t *testing.T) { } } -func TestResolveDocumentFileInvalidMediaIDFallsBackToMostRecent(t *testing.T) { +func TestResolveDocumentFileInvalidMediaIDReturnsError(t *testing.T) { refs := []providers.MediaRef{ {ID: uuid.NewString(), Kind: "document", Path: "/workspace/.uploads/old.pdf", MimeType: "application/pdf"}, {ID: uuid.NewString(), Kind: "document", Path: "/workspace/.uploads/latest.pdf", MimeType: "application/pdf"}, @@ -75,6 +75,23 @@ func TestResolveDocumentFileInvalidMediaIDFallsBackToMostRecent(t *testing.T) { tool := NewReadDocumentTool(nil, nil) ctx := WithMediaDocRefs(context.Background(), refs) gotPath, _, err := tool.resolveDocumentFile(ctx, "not-a-real-media-id", "") + if err == nil { + t.Fatalf("resolveDocumentFile returned path %q, want explicit media_id error", gotPath) + } + if !strings.Contains(err.Error(), "not-a-real-media-id") { + t.Fatalf("error = %q, want requested media_id", err.Error()) + } +} + +func TestResolveDocumentFileOmittedMediaIDUsesLastRef(t *testing.T) { + refs := []providers.MediaRef{ + {ID: uuid.NewString(), Kind: "document", Path: "/workspace/.uploads/old.pdf", MimeType: "application/pdf"}, + {ID: uuid.NewString(), Kind: "document", Path: "/workspace/.uploads/latest.pdf", MimeType: "application/pdf"}, + } + + tool := NewReadDocumentTool(nil, nil) + ctx := WithMediaDocRefs(context.Background(), refs) + gotPath, _, err := tool.resolveDocumentFile(ctx, "", "") if err != nil { t.Fatalf("resolveDocumentFile returned error: %v", err) }