From 1d9436350d5d28a378911253fa39d1d9d03d112c Mon Sep 17 00:00:00 2001 From: Duy /zuey/ Date: Sun, 31 May 2026 12:33:15 +0700 Subject: [PATCH] fix(discord): backfill thread history attachments Squash merge PR #106 after resolving dev changelog conflict. PR CI run 26704244687 passed release-versioning, go, and web. --- docs/05-channels-messaging.md | 1 + docs/project-changelog.md | 10 + internal/agent/media_persist_test.go | 38 +++ internal/channels/discord/handler.go | 17 +- internal/channels/discord/media.go | 11 +- .../discord/thread-history-backfill_test.go | 276 ++++++++++++++++++ .../discord/thread_history_backfill.go | 160 ++++++++++ .../phase-01-characterization-tests.md | 51 ++++ ...phase-02-thread-backfill-implementation.md | 69 +++++ .../phase-03-media-tool-integration-tests.md | 51 ++++ .../phase-04-docs-and-verification.md | 52 ++++ .../plan.md | 52 ++++ 12 files changed, 785 insertions(+), 3 deletions(-) create mode 100644 internal/channels/discord/thread-history-backfill_test.go create mode 100644 internal/channels/discord/thread_history_backfill.go create mode 100644 plans/260528-1747-discord-thread-history-attachment-backfill/phase-01-characterization-tests.md create mode 100644 plans/260528-1747-discord-thread-history-attachment-backfill/phase-02-thread-backfill-implementation.md create mode 100644 plans/260528-1747-discord-thread-history-attachment-backfill/phase-03-media-tool-integration-tests.md create mode 100644 plans/260528-1747-discord-thread-history-attachment-backfill/phase-04-docs-and-verification.md create mode 100644 plans/260528-1747-discord-thread-history-attachment-backfill/plan.md diff --git a/docs/05-channels-messaging.md b/docs/05-channels-messaging.md index 657a6d3c..0b82ddfc 100644 --- a/docs/05-channels-messaging.md +++ b/docs/05-channels-messaging.md @@ -517,6 +517,7 @@ The Discord channel uses the `discordgo` library to connect via the Discord Gate - **Bot identity**: Fetches `@me` on startup to detect and ignore own messages - **Typing indicator**: 9-second keepalive while agent processes - **Group history**: Pending message buffer for context when mentioned +- **Thread backfill**: When the bot is mentioned inside a Discord thread, the channel fetches up to 25 prior thread messages before the triggering message through Discord REST, prepends their text as context, and downloads up to 15 prior attachments for the same inbound media pipeline. This is thread-only, bounded to 5 MB per backfilled file with a 30-second timeout, and gracefully falls back to the current message when Discord lacks `READ_MESSAGE_HISTORY` or the REST request fails. --- diff --git a/docs/project-changelog.md b/docs/project-changelog.md index 3dd60edc..029f674c 100644 --- a/docs/project-changelog.md +++ b/docs/project-changelog.md @@ -6,6 +6,16 @@ Significant changes, features, and fixes in reverse chronological order. ## 2026-05-29 +### Discord thread history and attachment backfill (issue #69) + +**Fixes** + +- Discord thread mentions now fetch recent thread messages before the triggering mention and prepend them as context for the agent run. +- Prior thread image and document attachments are downloaded immediately and passed through the existing inbound media pipeline, so `read_image` and `read_document` can use files posted earlier in the thread. +- Backfill is limited to addressed Discord threads, 25 prior messages, 15 attachments, 5 MB per backfilled file, and a 30-second timeout. Missing Discord history permission or REST failures fall back to the current message without crashing. + +--- + ### Telegram voice transcription and read_audio fallback (issue #85) **Fixes** diff --git a/internal/agent/media_persist_test.go b/internal/agent/media_persist_test.go index b819d208..537d6fcc 100644 --- a/internal/agent/media_persist_test.go +++ b/internal/agent/media_persist_test.go @@ -116,3 +116,41 @@ func TestPersistMedia_NamingScheme(t *testing.T) { }) } } + +func TestPersistMedia_BackfilledThreadAttachmentsCreateToolRefs(t *testing.T) { + workspace := t.TempDir() + imagePath := filepath.Join(t.TempDir(), "diagram.png") + if err := os.WriteFile(imagePath, []byte("not a real png"), 0644); err != nil { + t.Fatalf("write image: %v", err) + } + docPath := filepath.Join(t.TempDir(), "brief.pdf") + if err := os.WriteFile(docPath, []byte("%PDF-1.4\n%fake"), 0644); err != nil { + t.Fatalf("write doc: %v", err) + } + + var loop Loop + refs := loop.persistMedia("discord-thread-session", []bus.MediaFile{ + {Path: imagePath, MimeType: "image/png", Filename: "diagram.png"}, + {Path: docPath, MimeType: "application/pdf", Filename: "brief.pdf"}, + }, workspace) + + if len(refs) != 2 { + t.Fatalf("refs = %d, want 2: %#v", len(refs), refs) + } + gotKinds := map[string]bool{} + for _, ref := range refs { + gotKinds[ref.Kind] = true + if ref.Path == "" { + t.Fatalf("ref missing persisted path: %#v", ref) + } + if _, err := os.Stat(ref.Path); err != nil { + t.Fatalf("persisted file missing for %s: %v", ref.Kind, err) + } + } + if !gotKinds["image"] { + t.Fatalf("missing image ref: %#v", refs) + } + if !gotKinds["document"] { + t.Fatalf("missing document ref: %#v", refs) + } +} diff --git a/internal/channels/discord/handler.go b/internal/channels/discord/handler.go index 3482acef..e0a7f710 100644 --- a/internal/channels/discord/handler.go +++ b/internal/channels/discord/handler.go @@ -253,11 +253,23 @@ func (c *Channel) handleMessage(_ *discordgo.Session, m *discordgo.MessageCreate content = strings.ReplaceAll(content, "<@"+c.botUserID+">", "") content = strings.TrimSpace(content) + threadBackfill := threadBackfillResult{} + if peerKind == "group" && mentioned { + threadBackfill = c.backfillThreadHistory(ctx, m, maxBytes) + if len(threadBackfill.Media) > 0 { + mediaFiles = append(threadBackfill.Media, mediaFiles...) + } + } + hasThreadBackfill := threadBackfill.Context != "" || len(threadBackfill.Media) > 0 + // Build final content with group context. finalContent := content if peerKind == "group" { annotated := fmt.Sprintf("[From: %s (<@%s>)]\n%s", senderName, senderID, content) - if c.HistoryLimit() > 0 { + if threadBackfill.Context != "" { + annotated = threadBackfill.Context + "\n\n" + annotated + } + if c.HistoryLimit() > 0 && !hasThreadBackfill { finalContent = c.GroupHistory().BuildContext(channelID, annotated, c.HistoryLimit()) } else { finalContent = annotated @@ -265,7 +277,8 @@ func (c *Channel) handleMessage(_ *discordgo.Session, m *discordgo.MessageCreate // Collect media from pending history entries (sent before this @mention). // Original filename not retained by CollectMedia; use disk basename so // persistMedia's sanitizer gets a meaningful stem instead of UUID fallback. - if histMediaPaths := c.GroupHistory().CollectMedia(channelID); len(histMediaPaths) > 0 { + if !hasThreadBackfill { + histMediaPaths := c.GroupHistory().CollectMedia(channelID) for _, p := range histMediaPaths { mediaFiles = append(mediaFiles, bus.MediaFile{Path: p, Filename: filepath.Base(p)}) } diff --git a/internal/channels/discord/media.go b/internal/channels/discord/media.go index 4babe3e2..26c19811 100644 --- a/internal/channels/discord/media.go +++ b/internal/channels/discord/media.go @@ -1,6 +1,7 @@ package discord import ( + "context" "fmt" "io" "log/slog" @@ -73,11 +74,19 @@ func resolveMedia(attachments []*discordgo.MessageAttachment, maxBytes int64) [] // downloadFromURL downloads a file from a URL with retry logic and saves to a temp file. func downloadFromURL(url string, maxBytes int64) (string, error) { + return downloadFromURLContext(context.Background(), url, maxBytes) +} + +func downloadFromURLContext(ctx context.Context, url string, maxBytes int64) (string, error) { var resp *http.Response var err error for attempt := 1; attempt <= downloadMaxRetries; attempt++ { - resp, err = http.Get(url) //nolint:gosec // Discord CDN URLs are trusted + req, reqErr := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if reqErr != nil { + return "", reqErr + } + resp, err = http.DefaultClient.Do(req) //nolint:gosec // Discord CDN URLs are trusted if err == nil { break } diff --git a/internal/channels/discord/thread-history-backfill_test.go b/internal/channels/discord/thread-history-backfill_test.go new file mode 100644 index 00000000..371c1c50 --- /dev/null +++ b/internal/channels/discord/thread-history-backfill_test.go @@ -0,0 +1,276 @@ +package discord + +import ( + "context" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/bwmarrin/discordgo" + + "github.com/nextlevelbuilder/goclaw/internal/bus" + "github.com/nextlevelbuilder/goclaw/internal/channels" + "github.com/nextlevelbuilder/goclaw/internal/config" +) + +func TestDiscordThreadBackfillPrependsPriorMessages(t *testing.T) { + server := newDiscordThreadBackfillServer(t, discordThreadBackfillFixture{ + historyJSON: `[ + {"id":"m2","channel_id":"thread-1","content":"second prior","author":{"id":"user-2","username":"Bob"}}, + {"id":"m1","channel_id":"thread-1","content":"first prior","author":{"id":"user-1","username":"Alice"}} + ]`, + }) + defer server.Close() + ch, mb := newThreadBackfillTestChannel(t, server) + + ch.handleMessage(ch.session, mentionedThreadMessage("current-1", "current request")) + + msg := consumeThreadBackfillInbound(t, mb) + assertContainsInOrder(t, msg.Content, + "[Discord thread messages before this mention - for context]", + "Alice: first prior", + "Bob: second prior", + "current request", + ) +} + +func TestDiscordThreadBackfillIncludesPriorAttachments(t *testing.T) { + server := newDiscordThreadBackfillServer(t, discordThreadBackfillFixture{ + historyJSON: `[ + {"id":"m1","channel_id":"thread-1","content":"see attached","author":{"id":"user-1","username":"Alice"},"attachments":[ + {"id":"att-1","filename":"diagram.png","content_type":"image/png","size":4,"url":"__SERVER__/cdn/diagram.png"}, + {"id":"att-2","filename":"brief.pdf","content_type":"application/pdf","size":5,"url":"__SERVER__/cdn/brief.pdf"} + ]} + ]`, + media: map[string]string{ + "/cdn/diagram.png": "png!", + "/cdn/brief.pdf": "%PDF-", + }, + }) + defer server.Close() + ch, mb := newThreadBackfillTestChannel(t, server) + + ch.handleMessage(ch.session, mentionedThreadMessage("current-1", "use earlier attachments")) + + msg := consumeThreadBackfillInbound(t, mb) + if len(msg.Media) != 2 { + t.Fatalf("media count = %d, want 2: %#v", len(msg.Media), msg.Media) + } + assertContainsInOrder(t, msg.Content, + "see attached", + "", + "use earlier attachments", + ) + want := map[string]string{ + "diagram.png": "image/png", + "brief.pdf": "application/pdf", + } + for _, file := range msg.Media { + if got, ok := want[file.Filename]; !ok || got != file.MimeType { + t.Fatalf("unexpected media file: %#v", file) + } + } +} + +func TestDiscordThreadBackfillFallsBackWhenHistoryUnavailable(t *testing.T) { + server := newDiscordThreadBackfillServer(t, discordThreadBackfillFixture{ + historyStatus: http.StatusForbidden, + historyJSON: `{"message":"Missing Access"}`, + }) + defer server.Close() + ch, mb := newThreadBackfillTestChannel(t, server) + + ch.handleMessage(ch.session, mentionedThreadMessage("current-1", "current only")) + + msg := consumeThreadBackfillInbound(t, mb) + if !strings.Contains(msg.Content, "current only") { + t.Fatalf("content = %q, want current message", msg.Content) + } + if strings.Contains(msg.Content, "Discord thread messages before") { + t.Fatalf("unexpected backfill context after history error: %q", msg.Content) + } +} + +func TestDiscordThreadBackfillSkipsNonThreadChannels(t *testing.T) { + server := newDiscordThreadBackfillServer(t, discordThreadBackfillFixture{ + channelJSON: `{"id":"thread-1","guild_id":"guild-1","type":0}`, + }) + defer server.Close() + ch, mb := newThreadBackfillTestChannel(t, server) + + ch.handleMessage(ch.session, mentionedThreadMessage("current-1", "regular channel request")) + + msg := consumeThreadBackfillInbound(t, mb) + if !strings.Contains(msg.Content, "regular channel request") { + t.Fatalf("content = %q, want current message", msg.Content) + } + if strings.Contains(msg.Content, "Discord thread messages before") { + t.Fatalf("unexpected backfill context for non-thread channel: %q", msg.Content) + } + if len(msg.Media) != 0 { + t.Fatalf("media count = %d, want 0: %#v", len(msg.Media), msg.Media) + } +} + +func TestDiscordThreadBackfillDoesNotDuplicatePendingThreadHistory(t *testing.T) { + server := newDiscordThreadBackfillServer(t, discordThreadBackfillFixture{ + historyJSON: `[ + {"id":"prior-1","channel_id":"thread-1","content":"same prior context","author":{"id":"user-1","username":"Alice"},"attachments":[ + {"id":"att-1","filename":"diagram.png","content_type":"image/png","size":4,"url":"__SERVER__/cdn/diagram.png"} + ]} + ]`, + media: map[string]string{ + "/cdn/diagram.png": "png!", + }, + }) + defer server.Close() + ch, mb := newThreadBackfillTestChannel(t, server) + + ch.handleMessage(ch.session, &discordgo.MessageCreate{Message: &discordgo.Message{ + ID: "prior-1", + ChannelID: "thread-1", + GuildID: "guild-1", + Content: "same prior context", + Author: &discordgo.User{ID: "user-1", Username: "Alice"}, + Attachments: []*discordgo.MessageAttachment{{ + ID: "att-1", + Filename: "diagram.png", + ContentType: "image/png", + Size: 4, + URL: server.URL + "/cdn/diagram.png", + }}, + Timestamp: time.Now(), + }}) + ch.handleMessage(ch.session, mentionedThreadMessage("current-1", "current request")) + + msg := consumeThreadBackfillInbound(t, mb) + if got := strings.Count(msg.Content, "same prior context"); got != 1 { + t.Fatalf("prior context count = %d, want 1:\n%s", got, msg.Content) + } + if got := len(msg.Media); got != 1 { + t.Fatalf("media count = %d, want 1: %#v", got, msg.Media) + } +} + +type discordThreadBackfillFixture struct { + channelJSON string + historyStatus int + historyJSON string + media map[string]string +} + +func newDiscordThreadBackfillServer(t *testing.T, fixture discordThreadBackfillFixture) *httptest.Server { + t.Helper() + server := httptest.NewServer(nil) + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = io.Copy(io.Discard, r.Body) + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && r.URL.Path == "/channels/thread-1": + body := fixture.channelJSON + if body == "" { + body = `{"id":"thread-1","guild_id":"guild-1","type":11,"parent_id":"parent-1"}` + } + _, _ = fmt.Fprint(w, body) + case r.Method == http.MethodGet && r.URL.Path == "/channels/thread-1/messages": + if got := r.URL.Query().Get("before"); got != "current-1" { + t.Fatalf("before query = %q, want current-1", got) + } + if got := r.URL.Query().Get("limit"); got != "25" { + t.Fatalf("limit query = %q, want 25", got) + } + status := fixture.historyStatus + if status == 0 { + status = http.StatusOK + } + w.WriteHeader(status) + body := strings.ReplaceAll(fixture.historyJSON, "__SERVER__", server.URL) + _, _ = fmt.Fprint(w, body) + case r.Method == http.MethodPost && r.URL.Path == "/channels/thread-1/typing": + w.WriteHeader(http.StatusNoContent) + case r.Method == http.MethodPost && r.URL.Path == "/channels/thread-1/messages": + _, _ = fmt.Fprint(w, `{"id":"placeholder-1","channel_id":"thread-1","content":"Thinking..."}`) + case r.Method == http.MethodGet && fixture.media != nil: + if body, ok := fixture.media[r.URL.Path]; ok { + if strings.HasSuffix(r.URL.Path, ".png") { + w.Header().Set("Content-Type", "image/png") + } + _, _ = fmt.Fprint(w, body) + return + } + http.NotFound(w, r) + default: + t.Fatalf("unexpected Discord test request: %s %s", r.Method, r.URL.String()) + } + }) + server.Config.Handler = handler + return server +} + +func newThreadBackfillTestChannel(t *testing.T, server *httptest.Server) (*Channel, *bus.MessageBus) { + t.Helper() + prevEndpointChannels := discordgo.EndpointChannels + discordgo.EndpointChannels = server.URL + "/channels/" + t.Cleanup(func() { + discordgo.EndpointChannels = prevEndpointChannels + }) + + session, err := discordgo.New("Bot test-token") + if err != nil { + t.Fatalf("discordgo.New() error = %v", err) + } + session.Client = server.Client() + + mb := bus.New() + ch := &Channel{ + BaseChannel: channels.NewBaseChannel(channels.TypeDiscord, mb, nil), + session: session, + botUserID: "bot-1", + config: config.DiscordConfig{GroupPolicy: "open", MediaMaxBytes: 8 * 1024 * 1024}, + } + ch.SetRequireMention(true) + ch.SetGroupHistory(channels.MakeHistory(channels.TypeDiscord, nil, ch.TenantID())) + ch.SetHistoryLimit(channels.DefaultGroupHistoryLimit) + ch.SetRunning(true) + return ch, mb +} + +func mentionedThreadMessage(id, content string) *discordgo.MessageCreate { + return &discordgo.MessageCreate{Message: &discordgo.Message{ + ID: id, + ChannelID: "thread-1", + GuildID: "guild-1", + Content: "<@bot-1> " + content, + Author: &discordgo.User{ID: "current-user", Username: "Current"}, + Mentions: []*discordgo.User{{ID: "bot-1"}}, + Timestamp: time.Now(), + }} +} + +func consumeThreadBackfillInbound(t *testing.T, mb *bus.MessageBus) bus.InboundMessage { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + msg, ok := mb.ConsumeInbound(ctx) + if !ok { + t.Fatal("timed out waiting for inbound message") + } + return msg +} + +func assertContainsInOrder(t *testing.T, text string, parts ...string) { + t.Helper() + offset := 0 + for _, part := range parts { + idx := strings.Index(text[offset:], part) + if idx < 0 { + t.Fatalf("content missing %q after offset %d:\n%s", part, offset, text) + } + offset += idx + len(part) + } +} diff --git a/internal/channels/discord/thread_history_backfill.go b/internal/channels/discord/thread_history_backfill.go new file mode 100644 index 00000000..97e61a05 --- /dev/null +++ b/internal/channels/discord/thread_history_backfill.go @@ -0,0 +1,160 @@ +package discord + +import ( + "context" + "fmt" + "log/slog" + "strings" + "time" + + "github.com/bwmarrin/discordgo" + + "github.com/nextlevelbuilder/goclaw/internal/bus" + "github.com/nextlevelbuilder/goclaw/internal/channels" + "github.com/nextlevelbuilder/goclaw/internal/channels/media" +) + +const ( + discordThreadHistoryLimit = 25 + discordThreadHistoryMaxBytes = 5 * 1024 * 1024 + discordThreadHistoryMaxFiles = 15 + discordThreadHistoryTimeout = 30 * time.Second + discordThreadHistoryContextHead = "[Discord thread messages before this mention - for context]" +) + +type threadBackfillResult struct { + Context string + Media []bus.MediaFile +} + +type threadHistoryAttachment struct { + File bus.MediaFile + Tag string +} + +func (c *Channel) backfillThreadHistory(ctx context.Context, m *discordgo.MessageCreate, maxBytes int64) threadBackfillResult { + if m == nil || m.Message == nil || m.ChannelID == "" || m.ID == "" { + return threadBackfillResult{} + } + if !c.isThreadChannel(ctx, m.ChannelID) { + return threadBackfillResult{} + } + ctx, cancel := context.WithTimeout(ctx, discordThreadHistoryTimeout) + defer cancel() + + messages, err := c.session.ChannelMessages( + m.ChannelID, + discordThreadHistoryLimit, + m.ID, + "", + "", + discordgo.WithContext(ctx), + ) + if err != nil { + slog.Warn("discord: thread history backfill failed", "channel_id", m.ChannelID, "message_id", m.ID, "error", err) + return threadBackfillResult{} + } + if len(messages) == 0 { + return threadBackfillResult{} + } + if maxBytes <= 0 || maxBytes > discordThreadHistoryMaxBytes { + maxBytes = discordThreadHistoryMaxBytes + } + + lines := []string{discordThreadHistoryContextHead} + var mediaFiles []bus.MediaFile + for i := len(messages) - 1; i >= 0; i-- { + hm := messages[i] + if hm == nil || hm.ID == m.ID || hm.Author == nil || hm.Author.ID == c.botUserID { + continue + } + body := strings.TrimSpace(hm.Content) + if body != "" { + lines = append(lines, fmt.Sprintf("%s: %s", discordHistoryAuthorName(hm), channels.Truncate(body, 1000))) + } + for _, attachment := range c.resolveThreadHistoryAttachments(ctx, hm.Attachments, maxBytes, discordThreadHistoryMaxFiles-len(mediaFiles)) { + if attachment.Tag != "" { + lines = append(lines, attachment.Tag) + } + mediaFiles = append(mediaFiles, attachment.File) + } + } + if len(lines) == 1 && len(mediaFiles) == 0 { + return threadBackfillResult{} + } + var contextBlock string + if len(lines) > 1 { + contextBlock = strings.Join(lines, "\n") + } + return threadBackfillResult{Context: contextBlock, Media: mediaFiles} +} + +func (c *Channel) isThreadChannel(ctx context.Context, channelID string) bool { + if c.session == nil { + return false + } + if c.session.State != nil { + if ch, err := c.session.State.Channel(channelID); err == nil && ch != nil { + return ch.IsThread() + } + } + ch, err := c.session.Channel(channelID, discordgo.WithContext(ctx)) + if err != nil { + slog.Warn("discord: thread channel lookup failed", "channel_id", channelID, "error", err) + return false + } + return ch != nil && ch.IsThread() +} + +func (c *Channel) resolveThreadHistoryAttachments(ctx context.Context, attachments []*discordgo.MessageAttachment, maxBytes int64, remaining int) []threadHistoryAttachment { + if len(attachments) == 0 || remaining <= 0 { + return nil + } + var mediaFiles []threadHistoryAttachment + for _, att := range attachments { + if att == nil || att.URL == "" || remaining <= 0 { + continue + } + if int64(att.Size) > maxBytes { + slog.Debug("discord: thread history attachment too large, skipping", "filename", att.Filename, "size", att.Size, "max", maxBytes) + continue + } + filePath, err := downloadFromURLContext(ctx, att.URL, maxBytes) + if err != nil { + slog.Warn("discord: thread history attachment download failed", "filename", att.Filename, "error", err) + continue + } + ct := att.ContentType + if ct == "" { + ct = media.DetectMIMEType(att.Filename) + } + info := media.MediaInfo{ + Type: classifyMediaType(ct, att.Filename), + FilePath: filePath, + SourceURL: att.URL, + ContentType: ct, + FileName: att.Filename, + FileSize: int64(att.Size), + } + mediaFiles = append(mediaFiles, threadHistoryAttachment{ + File: bus.MediaFile{ + Path: filePath, + MimeType: ct, + Filename: att.Filename, + }, + Tag: media.BuildMediaTags([]media.MediaInfo{info}), + }) + remaining-- + } + return mediaFiles +} + +func discordHistoryAuthorName(m *discordgo.Message) string { + if m == nil || m.Author == nil { + return "unknown" + } + if m.Author.GlobalName != "" { + return m.Author.GlobalName + } + return m.Author.Username +} diff --git a/plans/260528-1747-discord-thread-history-attachment-backfill/phase-01-characterization-tests.md b/plans/260528-1747-discord-thread-history-attachment-backfill/phase-01-characterization-tests.md new file mode 100644 index 00000000..4ca51680 --- /dev/null +++ b/plans/260528-1747-discord-thread-history-attachment-backfill/phase-01-characterization-tests.md @@ -0,0 +1,51 @@ +--- +phase: 1 +title: "Characterization Tests" +status: complete +priority: P1 +effort: "2h" +dependencies: [] +--- + +# Phase 1: Characterization Tests + +## Overview + +Write failing tests that lock the expected Discord thread behavior before implementation. Tests must prove current behavior does not backfill thread history from Discord REST and does not expose prior attachments to the inbound media pipeline when runtime has no pending history. + +## Requirements + +- Functional: when a mentioned Discord thread message arrives, prior thread messages before that message are fetched and represented as context. +- Functional: prior thread attachments are downloaded and included in published inbound media. +- Non-functional: tests run without live Discord by using `httptest` around discordgo REST calls or a narrow test seam. + +## Architecture + +Target the handler boundary. Build a `discordgo.Session` pointed at a local test server, return `/channels/{threadID}/messages?before={currentID}&limit=N`, and inspect the published `bus.InboundMessage`. + +## Related Code Files + +- Modify: `internal/channels/discord/handler_test.go` +- Modify or create focused test file: `internal/channels/discord/thread-history-backfill_test.go` +- Read: `internal/channels/discord/handler.go` +- Read: `internal/channels/discord/media.go` +- Read: `internal/bus/types.go` + +## Implementation Steps + +1. Add a test channel fixture that can publish inbound messages to a real `bus.MessageBus` and use a local discordgo REST endpoint. +2. Add a failing test for a public/private Discord thread: current message mentions bot; REST history returns one earlier user message; published content includes the earlier message before current message. +3. Add a failing test where earlier history message has image and document attachments; test server serves attachment bytes; published inbound `Media` includes those files with useful filenames/MIME types. +4. Add a failing test for missing/empty history response: handler still publishes current message and logs/continues without panic. +5. Keep tests deterministic: no real network, no live Discord token, no sleeps. + +## Success Criteria + +- [x] Tests fail on current code for missing thread backfill. +- [x] Tests assert order: historical messages oldest-to-newest, then current message. +- [x] Tests assert media count and filenames/MIME where available. +- [x] No live credentials or real Discord calls. + +## Risk Assessment + +Risk: handler is large and hard to test. Mitigation: keep the test seam focused on Discord REST and bus output; do not refactor unrelated handler behavior in this phase. diff --git a/plans/260528-1747-discord-thread-history-attachment-backfill/phase-02-thread-backfill-implementation.md b/plans/260528-1747-discord-thread-history-attachment-backfill/phase-02-thread-backfill-implementation.md new file mode 100644 index 00000000..f16d13c7 --- /dev/null +++ b/plans/260528-1747-discord-thread-history-attachment-backfill/phase-02-thread-backfill-implementation.md @@ -0,0 +1,69 @@ +--- +phase: 2 +title: "Thread Backfill Implementation" +status: complete +priority: P1 +effort: "3h" +dependencies: [1] +--- + +# Phase 2: Thread Backfill Implementation + +## Overview + +Implement the narrow Discord thread backfill that makes Phase 1 tests pass. The backfill runs only when the inbound message is in a Discord thread and the bot is addressed. + +## Requirements + +- Functional: detect Discord thread channels reliably. +- Functional: call `ChannelMessages(threadID, limit, before=currentMessageID, after="", around="")`. +- Functional: merge prior text and prior attachments into the same inbound run. +- Non-functional: bounded message count, bounded attachment count, bounded file size, bounded total timeout. + +## Architecture + +Add focused helper functions instead of growing `handler.go` further. `handleMessage` should ask a helper for `threadBackfillResult`, then prepend its context and merge its media before publishing inbound. + +Suggested helper shape: + +```go +type threadBackfillResult struct { + Context string + Media []bus.MediaFile +} +``` + +Use Discord REST only for thread messages before the triggering message. Reverse REST result because Discord returns newest-to-oldest. + +## Related Code Files + +- Modify: `internal/channels/discord/handler.go` +- Modify: `internal/channels/discord/media.go` +- Create focused helper if needed: `internal/channels/discord/thread-history-backfill.go` +- Read: `internal/channels/history.go` +- Read: `internal/channels/media/media_tags.go` + +## Implementation Steps + +1. Add constants: history message limit (start 25), history media max bytes (5 MB), history media max refs (15), total timeout (30s). +2. Add `isDiscordThread` helper using session state first and REST `Channel` fallback when state misses. +3. Add `fetchThreadHistoryBefore(ctx, threadID, currentMessageID)` using `ChannelMessages`. +4. Normalize messages: skip current/bot messages, skip empty content unless attachments exist, resolve author display name, keep timestamps. +5. Resolve prior attachments immediately using existing download/classify path with stricter history caps. +6. Build a clear context block: `[Discord thread messages before this mention - for context]`. +7. Merge backfill into current flow before `PublishInbound`: current user message remains the active request; prior history is context only. +8. On REST errors or no permissions, log `slog.Warn("discord: thread history backfill failed", ...)` and continue current message. +9. Keep pending `GroupHistory()` behavior unchanged for normal group mention gating. + +## Success Criteria + +- [x] Phase 1 tests pass. +- [x] Backfill is invoked only for `Channel.IsThread()` and mentioned/reply-to-bot triggers. +- [x] Attachment download failures skip that attachment, not the whole run. +- [x] No schema/config migration required. + +## Risk Assessment + +Risk: Discord REST permissions vary by server. Mitigation: graceful fallback and explicit docs note. + +Risk: rate limits if called too often. Mitigation: only on thread mention, low default limit, no pagination in v1. diff --git a/plans/260528-1747-discord-thread-history-attachment-backfill/phase-03-media-tool-integration-tests.md b/plans/260528-1747-discord-thread-history-attachment-backfill/phase-03-media-tool-integration-tests.md new file mode 100644 index 00000000..0ffda729 --- /dev/null +++ b/plans/260528-1747-discord-thread-history-attachment-backfill/phase-03-media-tool-integration-tests.md @@ -0,0 +1,51 @@ +--- +phase: 3 +title: "Media Tool Integration Tests" +status: complete +priority: P1 +effort: "3h" +dependencies: [2] +--- + +# Phase 3: Media Tool Integration Tests + +## Overview + +Prove the backfilled attachments are not just shown as text tags; they must flow into persisted media refs so `read_image` and `read_document` can access them in the agent run. + +## Requirements + +- Functional: backfilled image attachments become image refs available to `read_image`. +- Functional: backfilled document attachments become document refs available to `read_document`. +- Non-functional: reuse existing media persistence and tool context logic, do not create a Discord-specific media store. + +## Architecture + +The Discord handler publishes `bus.MediaFile`. Agent loop persists those via `persistMedia`, adds `MediaRefs` to the current user message, and then tools resolve refs from context/history. Tests should validate at the lowest reliable layer without live LLM calls. + +## Related Code Files + +- Modify: `internal/channels/discord/thread-history-backfill_test.go` +- Modify or add targeted agent test: `internal/agent/loop_input_media_test.go` or existing media tests +- Read: `internal/agent/loop_input_media.go` +- Read: `internal/agent/media_tool_routing.go` +- Read: `internal/tools/read_image.go` +- Read: `internal/tools/read_document.go` + +## Implementation Steps + +1. Add a focused test that feeds backfilled `bus.MediaFile` image/doc into `enrichInputMedia` and asserts resulting `MediaRefs` contain image/document kinds. +2. For image path mode, assert `loadHistoricalImagesForTool` can include persisted image refs in tool context. +3. For document path mode, assert `tools.MediaDocRefsFromCtx(ctx)` contains backfilled document refs. +4. Ensure content tags include media IDs/paths where existing enrichment functions already provide them. +5. Avoid actual provider calls; do not invoke LLM/vision/document providers. + +## Success Criteria + +- [x] Tests prove `read_image` has image context or path access for prior thread image. +- [x] Tests prove `read_document` can resolve prior thread document ref. +- [x] No fake string-only acceptance; actual files are persisted/resolved. + +## Risk Assessment + +Risk: tool integration test may overreach into private loop internals. Mitigation: prefer existing agent media tests and helper-level assertions over full LLM runs. diff --git a/plans/260528-1747-discord-thread-history-attachment-backfill/phase-04-docs-and-verification.md b/plans/260528-1747-discord-thread-history-attachment-backfill/phase-04-docs-and-verification.md new file mode 100644 index 00000000..b7b163e6 --- /dev/null +++ b/plans/260528-1747-discord-thread-history-attachment-backfill/phase-04-docs-and-verification.md @@ -0,0 +1,52 @@ +--- +phase: 4 +title: "Docs and Verification" +status: complete +priority: P2 +effort: "2h" +dependencies: [3] +--- + +# Phase 4: Docs and Verification + +## Overview + +Document the new Discord thread behavior and run focused verification for compile and regression safety. + +## Requirements + +- Functional: docs explain that Discord thread mention triggers bounded REST history backfill. +- Non-functional: no misleading promise for channels/groups, no claim that history works without Discord `READ_MESSAGE_HISTORY`. + +## Architecture + +Docs live in existing channel docs. Verification follows repo checklist but scoped to files touched unless a broader compile catches integration issues. + +## Related Code Files + +- Modify: `docs/05-channels-messaging.md` +- Optional: `docs/project-changelog.md` if implementation is considered significant user-facing fix +- Run: `go test ./internal/channels/discord ./internal/agent` +- Run: `go build ./...` +- Run: `go build -tags sqliteonly ./...` + +## Implementation Steps + +1. Update Discord section with thread history backfill behavior, bounds, and permission caveat. +2. Add changelog entry if implementation changes user-facing Discord behavior. +3. Run focused tests for Discord and agent media packages. +4. Run Go compile checks for PG and SQLite builds. +5. If tests need live Discord, do not add them; keep live verification as manual note. +6. Reply to GitHub issue #69 with implemented behavior and verification commands after code is done. + +## Success Criteria + +- [x] Docs mention Discord thread-only scope. +- [x] Focused tests pass. +- [x] `go build ./...` passes. +- [x] `go build -tags sqliteonly ./...` passes. +- [x] GitHub issue reply includes summary and verification. + +## Risk Assessment + +Risk: full integration tests require local Postgres/pgvector and may be unavailable. Mitigation: run compile + focused unit tests; list skipped integration tests explicitly. diff --git a/plans/260528-1747-discord-thread-history-attachment-backfill/plan.md b/plans/260528-1747-discord-thread-history-attachment-backfill/plan.md new file mode 100644 index 00000000..60900d98 --- /dev/null +++ b/plans/260528-1747-discord-thread-history-attachment-backfill/plan.md @@ -0,0 +1,52 @@ +--- +title: "Discord Thread History Attachment Backfill" +description: "TDD plan for Discord thread REST backfill so a newly mentioned bot can see prior thread messages and attachments." +status: complete +priority: P2 +branch: "codex/issue-69-discord-thread-history-attachments" +tags: [discord, threads, media, tdd, issue-69] +blockedBy: [] +blocks: [] +created: "2026-05-28T10:47:50.194Z" +createdBy: "ck:plan" +source: skill +--- + +# Discord Thread History Attachment Backfill + +## Overview + +Fix GitHub issue #69 with a narrow Discord-thread-only backfill. When the bot is mentioned in a Discord thread, fetch recent thread messages before the triggering message via Discord REST, convert prior text into context, download bounded prior attachments, and pass those files through the existing inbound media pipeline so `read_image` and `read_document` can access them. + +Non-goals: no global Discord group/channel history backfill, no schema migration, no full-thread archive export, no always-on polling. If Discord permissions do not allow history, log and continue with current message. + +## Phases + +| Phase | Name | Status | +|-------|------|--------| +| 1 | [Characterization Tests](./phase-01-characterization-tests.md) | Complete | +| 2 | [Thread Backfill Implementation](./phase-02-thread-backfill-implementation.md) | Complete | +| 3 | [Media Tool Integration Tests](./phase-03-media-tool-integration-tests.md) | Complete | +| 4 | [Docs and Verification](./phase-04-docs-and-verification.md) | Complete | + +## Dependencies + +- GitHub issue: https://github.com/digitopvn/goclaw/issues/69 +- Discord docs: `GET /channels/{channel.id}/messages` returns newest-to-oldest messages, `limit` 1-100, with `before/after/around`; guild channels need `VIEW_CHANNEL`, and missing `READ_MESSAGE_HISTORY` returns no messages. +- Discord attachment docs: fetched message payload attachment URLs are signed CDN URLs valid when received, so history attachments must be downloaded immediately. +- Existing Discord handler: `internal/channels/discord/handler.go` +- Existing Discord media download/classification: `internal/channels/discord/media.go` +- Existing media tool pipeline: `internal/agent/loop_input_media.go`, `internal/agent/media_tool_routing.go`, `internal/tools/read_image.go`, `internal/tools/read_document.go` + +## Success Criteria + +- [x] In a Discord thread, message A before bot mention with image/doc attachment becomes available to the run triggered by message B. +- [x] Agent receives prior text context and prior attachment files. +- [x] `read_image` can analyze prior image; `read_document` can resolve prior document. +- [x] Backfill runs only for Discord threads when the bot is addressed. +- [x] Backfill is bounded by message count, attachment count, file size, and timeout. +- [x] Missing Discord history permission is graceful: no crash, current message still processed. + +## Implementation Boundary + +Use TDD. Do not implement a broad channel history system. Keep this as a Discord thread adapter feature around current `handleMessage` flow and existing media pipeline.