From 9cd57a920cb094254dff807338296de71a18702a Mon Sep 17 00:00:00 2001 From: Goon Date: Fri, 12 Jun 2026 17:33:54 +0700 Subject: [PATCH] feat: add multi-attachment delivery batching --- cmd/gateway_consumer.go | 1 + docs/05-channels-messaging.md | 18 +++ docs/project-changelog.md | 21 +++ internal/agent/loop_tools.go | 2 +- internal/agent/loop_types.go | 1 + internal/bus/types.go | 19 +-- internal/channels/capabilities.go | 46 ++++++ internal/channels/telegram/send.go | 96 +++--------- .../channels/telegram/send_media_group.go | 138 +++++++++++++++++ .../telegram/send_media_group_helpers.go | 142 ++++++++++++++++++ .../telegram/send_media_group_test.go | 70 +++++++++ internal/tools/send_file.go | 112 ++++++++------ internal/tools/send_file_batch_requests.go | 58 +++++++ internal/tools/send_file_test.go | 74 +++++++++ .../phase-01-research-and-tdd-contracts.md | 51 +++++++ .../phase-02-tool-api-batch-attachments.md | 59 ++++++++ ...hannel-capability-and-telegram-grouping.md | 55 +++++++ ...ase-04-regression-verification-and-ship.md | 54 +++++++ .../plan.md | 45 ++++++ .../reports/validation-red-team-report.md | 19 +++ 20 files changed, 954 insertions(+), 127 deletions(-) create mode 100644 internal/channels/telegram/send_media_group.go create mode 100644 internal/channels/telegram/send_media_group_helpers.go create mode 100644 internal/channels/telegram/send_media_group_test.go create mode 100644 internal/tools/send_file_batch_requests.go create mode 100644 plans/260612-1708-issue-172-multi-attachment-delivery/phase-01-research-and-tdd-contracts.md create mode 100644 plans/260612-1708-issue-172-multi-attachment-delivery/phase-02-tool-api-batch-attachments.md create mode 100644 plans/260612-1708-issue-172-multi-attachment-delivery/phase-03-channel-capability-and-telegram-grouping.md create mode 100644 plans/260612-1708-issue-172-multi-attachment-delivery/phase-04-regression-verification-and-ship.md create mode 100644 plans/260612-1708-issue-172-multi-attachment-delivery/plan.md create mode 100644 plans/260612-1708-issue-172-multi-attachment-delivery/reports/validation-red-team-report.md diff --git a/cmd/gateway_consumer.go b/cmd/gateway_consumer.go index de888c43..7268d968 100644 --- a/cmd/gateway_consumer.go +++ b/cmd/gateway_consumer.go @@ -238,6 +238,7 @@ func appendMediaToOutbound(msg *bus.OutboundMessage, media []agent.MediaResult) msg.Media = append(msg.Media, bus.MediaAttachment{ URL: mr.Path, ContentType: mr.ContentType, + Caption: mr.Caption, }) if mr.AsVoice { if msg.Metadata == nil { diff --git a/docs/05-channels-messaging.md b/docs/05-channels-messaging.md index 976f5977..43d1af24 100644 --- a/docs/05-channels-messaging.md +++ b/docs/05-channels-messaging.md @@ -105,6 +105,24 @@ Splitting is intentionally conservative. Replies containing fenced code, tables, Progress messages are not added to session history by this behavior. Existing run timeline handling for explicit `block.reply` remains unchanged. +### Outbound Media Delivery + +Agent tools can queue media files through `Result.Media`; the consumer converts +those entries into `OutboundMessage.Media` and preserves file order, MIME type, +filename, and optional captions. + +- `send_file` accepts either the legacy single `path` + optional `caption`, or + `attachments: [{path, caption?}, ...]` for batch delivery of existing + workspace files. +- Telegram groups compatible outbound media into `sendMediaGroup` album chunks + of 2-10 items. Photo/video items can share a chunk; documents group only with + documents; audio groups only with audio. Voice-mode audio, singleton chunks, + oversized images sent as documents, and incompatible runs use the existing + ordered single-send fallback. +- Discord already sends multiple files plus optional text in one message. +- Slack and other media-capable channels keep ordered fallback behavior unless + their adapter advertises a stronger batch capability. + ### Reasoning Delivery Telegram channel config supports explicit reasoning delivery: diff --git a/docs/project-changelog.md b/docs/project-changelog.md index 98225430..0a626218 100644 --- a/docs/project-changelog.md +++ b/docs/project-changelog.md @@ -6,6 +6,27 @@ Significant changes, features, and fixes in reverse chronological order. ## 2026-06-12 +### Multi-attachment outbound delivery (issue #172) + +**Changes** + +- Extended `send_file` with `attachments[]` so agents can queue multiple + existing workspace files in one tool call while preserving order and captions. +- Added caption propagation from tool result media through agent media results + into outbound channel attachments. +- Added Telegram media-group delivery for compatible batches with 2-10 item + chunks, plus ordered fallback for singleton, voice, oversized-image, or mixed + incompatible cases. +- Added explicit channel batch capability metadata for Telegram, Discord, and + ordered fallback channels. + +**Tests** + +- Added batch `send_file` regressions for happy path, duplicate rejection, and + all-or-nothing delivered-state handling. +- Added Telegram media-group chunking tests for compatible grouping, max-size + chunking, and non-groupable fallback. + ### Operator trace CLI (issue #158) **Changes** diff --git a/internal/agent/loop_tools.go b/internal/agent/loop_tools.go index af825ac2..0fefbc6c 100644 --- a/internal/agent/loop_tools.go +++ b/internal/agent/loop_tools.go @@ -91,7 +91,7 @@ func (l *Loop) processToolResult( if ct == "" { ct = mimeFromExt(filepath.Ext(mf.Path)) } - mr := MediaResult{Path: mf.Path, ContentType: ct} + mr := MediaResult{Path: mf.Path, ContentType: ct, Caption: mf.Caption} if result.MediaPrompts != nil { mr.Prompt = result.MediaPrompts[i] } diff --git a/internal/agent/loop_types.go b/internal/agent/loop_types.go index c1663959..81eeb74a 100644 --- a/internal/agent/loop_types.go +++ b/internal/agent/loop_types.go @@ -681,6 +681,7 @@ type RunResult struct { type MediaResult struct { Path string `json:"path"` // local file path ContentType string `json:"content_type,omitempty"` // MIME type + Caption string `json:"caption,omitempty"` // optional outbound caption Size int64 `json:"size,omitempty"` // file size in bytes AsVoice bool `json:"as_voice,omitempty"` // send as voice message (Telegram OGG) // Prompt is the generation prompt for AI-generated media (e.g. create_image). diff --git a/internal/bus/types.go b/internal/bus/types.go index 4eec4dd4..c3477589 100644 --- a/internal/bus/types.go +++ b/internal/bus/types.go @@ -14,6 +14,7 @@ type MediaFile struct { Path string `json:"path"` MimeType string `json:"mime_type,omitempty"` // e.g. "application/pdf", "image/jpeg" Filename string `json:"filename,omitempty"` // original user-provided filename, e.g. "Báo cáo Q4.pdf"; empty → UUID fallback in persistMedia + Caption string `json:"caption,omitempty"` // optional outbound caption attached to this file } // InboundMessage represents a message received from a channel (Telegram, Discord, etc.) @@ -35,14 +36,14 @@ type InboundMessage struct { // OutboundMessage represents a message to be sent to a channel. type OutboundMessage struct { - Channel string `json:"channel"` - ChatID string `json:"chat_id"` - Content string `json:"content"` - Media []MediaAttachment `json:"media,omitempty"` // optional media attachments - Metadata map[string]string `json:"metadata,omitempty"` // channel-specific metadata - TenantID uuid.UUID `json:"tenant_id,omitempty"` // tenant scope for per-tenant TTS - AgentID uuid.UUID `json:"agent_id,omitempty"` // agent scope for per-agent TTS voice override - AgentOtherConfig []byte `json:"agent_other_config,omitempty"` // agent's other_config for TTS voice/model + Channel string `json:"channel"` + ChatID string `json:"chat_id"` + Content string `json:"content"` + Media []MediaAttachment `json:"media,omitempty"` // optional media attachments + Metadata map[string]string `json:"metadata,omitempty"` // channel-specific metadata + TenantID uuid.UUID `json:"tenant_id,omitempty"` // tenant scope for per-tenant TTS + AgentID uuid.UUID `json:"agent_id,omitempty"` // agent scope for per-agent TTS voice override + AgentOtherConfig []byte `json:"agent_other_config,omitempty"` // agent's other_config for TTS voice/model } // MediaAttachment represents a media file to be sent with a message. @@ -54,7 +55,7 @@ type MediaAttachment struct { // Event represents a server-side event to broadcast to WebSocket clients. type Event struct { - Name string `json:"name"` // event name (e.g. "agent", "chat", "health") + Name string `json:"name"` // event name (e.g. "agent", "chat", "health") Payload any `json:"payload,omitempty"` TenantID uuid.UUID `json:"-"` // tenant scope for event filtering (not serialized to clients) } diff --git a/internal/channels/capabilities.go b/internal/channels/capabilities.go index 1198dfc2..ef3f2340 100644 --- a/internal/channels/capabilities.go +++ b/internal/channels/capabilities.go @@ -6,6 +6,21 @@ import "errors" // Callers (e.g. webhook handler) should either degrade to text-only or return HTTP 501. var ErrMediaUnsupported = errors.New("channel does not support media attachments") +// MediaBatchCapability describes how a channel can deliver multiple outbound +// attachments from one OutboundMessage. +type MediaBatchCapability struct { + Supported bool + MaxAttachments int + Grouping string +} + +const ( + MediaBatchGroupingNone = "none" + MediaBatchGroupingSingleMessage = "single_message" + MediaBatchGroupingAlbumChunks = "album_chunks" + MediaBatchGroupingOrdered = "ordered" +) + // mediaCapableTypes lists channel platform types that consume msg.Media in their Send() // implementation. Verified against adapters: // - telegram: internal/channels/telegram/send.go:251 @@ -30,8 +45,39 @@ var mediaCapableTypes = map[string]bool{ TypeFacebook: true, } +var mediaBatchCapabilities = map[string]MediaBatchCapability{ + TypeTelegram: { + Supported: true, + MaxAttachments: 10, + Grouping: MediaBatchGroupingAlbumChunks, + }, + TypeDiscord: { + Supported: true, + MaxAttachments: 10, + Grouping: MediaBatchGroupingSingleMessage, + }, + TypeSlack: { + Supported: true, + MaxAttachments: 0, + Grouping: MediaBatchGroupingOrdered, + }, +} + // IsMediaCapable reports whether the given channel platform type supports media attachments. // Use Manager.ChannelTypeForName to resolve the type from a channel instance name. func IsMediaCapable(channelType string) bool { return mediaCapableTypes[channelType] } + +// MediaBatchCapabilityFor reports best-known multi-attachment delivery behavior. +// Unknown media-capable channels preserve order by falling back to their Send() +// implementation's existing per-file behavior. +func MediaBatchCapabilityFor(channelType string) MediaBatchCapability { + if cap, ok := mediaBatchCapabilities[channelType]; ok { + return cap + } + if IsMediaCapable(channelType) { + return MediaBatchCapability{Supported: true, Grouping: MediaBatchGroupingOrdered} + } + return MediaBatchCapability{Grouping: MediaBatchGroupingNone} +} diff --git a/internal/channels/telegram/send.go b/internal/channels/telegram/send.go index 74324b21..42bd43fb 100644 --- a/internal/channels/telegram/send.go +++ b/internal/channels/telegram/send.go @@ -96,11 +96,12 @@ func isPostConnectNetworkErr(err error) bool { // need the longer sendMediaOverallTimeout budget. Keyed by the `name` argument // passed to retrySend so no call site has to change when we bump the budget. var mediaSendMethods = map[string]struct{}{ - "sendPhoto": {}, - "sendVideo": {}, - "sendAudio": {}, - "sendVoice": {}, - "sendDocument": {}, + "sendPhoto": {}, + "sendVideo": {}, + "sendAudio": {}, + "sendVoice": {}, + "sendDocument": {}, + "sendMediaGroup": {}, } // retrySend wraps a Telegram send call with retry logic for transient network errors. @@ -361,81 +362,32 @@ func (c *Channel) Send(ctx context.Context, msg bus.OutboundMessage) error { // Ref: TS src/telegram/send.ts → sendMessageTelegram with mediaUrl func (c *Channel) sendMediaMessage(ctx context.Context, chatID int64, msg bus.OutboundMessage, replyTo, threadID int) error { chatIDObj := tu.ID(chatID) + items, err := c.prepareTelegramMediaItems(msg) + if err != nil { + return err + } - for _, media := range msg.Media { - // Determine caption (use message content for first media, or media caption) - caption := media.Caption - if caption == "" && msg.Content != "" { - caption = msg.Content - msg.Content = "" // only use for first media - } - - // Convert caption from markdown to Telegram HTML (same as regular messages). - // If the HTML caption exceeds Telegram's 1024-byte limit, skip caption entirely - // and send the full text as a separate message. Truncating HTML at a byte boundary - // can split tags (e.g. cut inside ...) causing parse errors. - var followUpText string - if caption != "" { - caption = markdownToTelegramHTML(caption) - if len(caption) > telegramCaptionMaxLen { - followUpText = caption - caption = "" - } - } - - if err := c.validateOutboundMediaSize(media.URL); err != nil { - return err - } - - // Send based on content type. - // Large images (>photoSizeThreshold) are sent as documents to avoid Telegram compression. - ct := strings.ToLower(media.ContentType) - switch { - case strings.HasPrefix(ct, "image/"): - sendAsDoc := false - if info, statErr := os.Stat(media.URL); statErr == nil && info.Size() > photoSizeThreshold { - sendAsDoc = true - slog.Info("large image, sending as document to preserve quality", "path", media.URL, "size", info.Size()) - } - if sendAsDoc { - if err := c.sendDocument(ctx, chatIDObj, media.URL, caption, replyTo, threadID); err != nil { - return err - } - } else if err := c.sendPhoto(ctx, chatIDObj, media.URL, caption, replyTo, threadID); err != nil { + for _, chunk := range chunkTelegramMediaItems(items) { + if chunk.grouped { + if err := c.sendTelegramMediaGroup(ctx, chatIDObj, chunk.items, replyTo, threadID); err != nil { return err } - case strings.HasPrefix(ct, "video/"): - if err := c.sendVideo(ctx, chatIDObj, media.URL, caption, replyTo, threadID); err != nil { - return err - } - case strings.HasPrefix(ct, "audio/"): - // Voice message: use SendVoice for inline playback bubble. - if msg.Metadata["audio_as_voice"] == "true" && isVoiceCompatible(ct) { - if err := c.sendVoice(ctx, chatIDObj, media.URL, caption, replyTo, threadID); err != nil { - return err - } - } else { - if err := c.sendAudio(ctx, chatIDObj, media.URL, caption, replyTo, threadID); err != nil { + replyTo = 0 + for _, item := range chunk.items { + if err := c.sendTelegramMediaFollowUp(ctx, chatID, item.followUpText, threadID); err != nil { return err } } - default: - if err := c.sendDocument(ctx, chatIDObj, media.URL, caption, replyTo, threadID); err != nil { - return err - } + continue } - // Only reply to the first media item - replyTo = 0 - // Send follow-up text if caption was split. - // followUpText is already HTML (from markdownToTelegramHTML above), so - // just chunk and send — do NOT convert again (double-escaping breaks entities). - if followUpText != "" { - chunks := chunkHTML(followUpText, telegramMaxMessageLen) - for _, chunk := range chunks { - if err := c.sendHTML(ctx, chatID, chunk, 0, threadID); err != nil { - return err - } + for _, item := range chunk.items { + if err := c.sendSingleTelegramMediaItem(ctx, chatIDObj, item, replyTo, threadID); err != nil { + return err + } + replyTo = 0 + if err := c.sendTelegramMediaFollowUp(ctx, chatID, item.followUpText, threadID); err != nil { + return err } } } diff --git a/internal/channels/telegram/send_media_group.go b/internal/channels/telegram/send_media_group.go new file mode 100644 index 00000000..45b0245f --- /dev/null +++ b/internal/channels/telegram/send_media_group.go @@ -0,0 +1,138 @@ +package telegram + +import ( + "context" + "fmt" + "log/slog" + "os" + + "github.com/mymmrac/telego" +) + +func (c *Channel) sendTelegramMediaGroup(ctx context.Context, chatID telego.ChatID, items []telegramMediaSendItem, replyTo, threadID int) error { + files := make([]*os.File, 0, len(items)) + defer func() { + for _, file := range files { + _ = file.Close() + } + }() + + params := &telego.SendMediaGroupParams{ + ChatID: chatID, + Media: make([]telego.InputMedia, 0, len(items)), + } + if sendThreadID := resolveThreadIDForSend(threadID); sendThreadID > 0 { + params.MessageThreadID = sendThreadID + } + if replyTo > 0 { + params.ReplyParameters = &telego.ReplyParameters{MessageID: replyTo, AllowSendingWithoutReply: true} + } + + for _, item := range items { + file, err := os.Open(item.media.URL) + if err != nil { + return fmt.Errorf("open media %s: %w", item.media.URL, err) + } + files = append(files, file) + params.Media = append(params.Media, inputMediaForTelegramGroupItem(item, file)) + } + + reset := func() { + for _, file := range files { + _, _ = file.Seek(0, 0) + } + } + err := c.retrySend(ctx, "sendMediaGroup", reset, func(ctx context.Context) error { + _, e := c.bot.SendMediaGroup(ctx, params) + return e + }) + if err != nil && parseErrRe.MatchString(err.Error()) { + stripTelegramGroupCaptions(params.Media) + reset() + _, err = c.bot.SendMediaGroup(ctx, params) + } + if err != nil && params.MessageThreadID != 0 && threadNotFoundRe.MatchString(err.Error()) { + slog.Warn("sendMediaGroup: thread not found, retrying without thread", "thread_id", params.MessageThreadID) + params.MessageThreadID = 0 + reset() + _, err = c.bot.SendMediaGroup(ctx, params) + } + return err +} + +func inputMediaForTelegramGroupItem(item telegramMediaSendItem, file *os.File) telego.InputMedia { + input := telego.InputFile{File: file} + switch item.kind { + case telegramMediaPhoto: + media := &telego.InputMediaPhoto{Type: telego.MediaTypePhoto, Media: input, Caption: item.caption} + if item.caption != "" { + media.ParseMode = telego.ModeHTML + } + return media + case telegramMediaVideo: + media := &telego.InputMediaVideo{Type: telego.MediaTypeVideo, Media: input, Caption: item.caption} + if item.caption != "" { + media.ParseMode = telego.ModeHTML + } + return media + case telegramMediaAudio: + media := &telego.InputMediaAudio{Type: telego.MediaTypeAudio, Media: input, Caption: item.caption} + if item.caption != "" { + media.ParseMode = telego.ModeHTML + } + return media + default: + media := &telego.InputMediaDocument{Type: telego.MediaTypeDocument, Media: input, Caption: item.caption} + if item.caption != "" { + media.ParseMode = telego.ModeHTML + } + return media + } +} + +func stripTelegramGroupCaptions(media []telego.InputMedia) { + for _, item := range media { + switch typed := item.(type) { + case *telego.InputMediaPhoto: + typed.ParseMode = "" + typed.Caption = stripHTML(typed.Caption) + case *telego.InputMediaVideo: + typed.ParseMode = "" + typed.Caption = stripHTML(typed.Caption) + case *telego.InputMediaAudio: + typed.ParseMode = "" + typed.Caption = stripHTML(typed.Caption) + case *telego.InputMediaDocument: + typed.ParseMode = "" + typed.Caption = stripHTML(typed.Caption) + } + } +} + +func (c *Channel) sendSingleTelegramMediaItem(ctx context.Context, chatID telego.ChatID, item telegramMediaSendItem, replyTo, threadID int) error { + switch item.kind { + case telegramMediaPhoto: + return c.sendPhoto(ctx, chatID, item.media.URL, item.caption, replyTo, threadID) + case telegramMediaVideo: + return c.sendVideo(ctx, chatID, item.media.URL, item.caption, replyTo, threadID) + case telegramMediaAudio: + if item.asVoice { + return c.sendVoice(ctx, chatID, item.media.URL, item.caption, replyTo, threadID) + } + return c.sendAudio(ctx, chatID, item.media.URL, item.caption, replyTo, threadID) + default: + return c.sendDocument(ctx, chatID, item.media.URL, item.caption, replyTo, threadID) + } +} + +func (c *Channel) sendTelegramMediaFollowUp(ctx context.Context, chatID int64, text string, threadID int) error { + if text == "" { + return nil + } + for _, chunk := range chunkHTML(text, telegramMaxMessageLen) { + if err := c.sendHTML(ctx, chatID, chunk, 0, threadID); err != nil { + return err + } + } + return nil +} diff --git a/internal/channels/telegram/send_media_group_helpers.go b/internal/channels/telegram/send_media_group_helpers.go new file mode 100644 index 00000000..747039f8 --- /dev/null +++ b/internal/channels/telegram/send_media_group_helpers.go @@ -0,0 +1,142 @@ +package telegram + +import ( + "log/slog" + "mime" + "os" + "path/filepath" + "strings" + + "github.com/nextlevelbuilder/goclaw/internal/bus" +) + +const ( + telegramMediaGroupMinItems = 2 + telegramMediaGroupMaxItems = 10 +) + +type telegramMediaKind string + +const ( + telegramMediaPhoto telegramMediaKind = "photo" + telegramMediaVideo telegramMediaKind = "video" + telegramMediaAudio telegramMediaKind = "audio" + telegramMediaDocument telegramMediaKind = "document" +) + +type telegramMediaSendItem struct { + media bus.MediaAttachment + kind telegramMediaKind + caption string + followUpText string + groupable bool + asVoice bool +} + +type telegramMediaChunk struct { + items []telegramMediaSendItem + grouped bool +} + +func (c *Channel) prepareTelegramMediaItems(msg bus.OutboundMessage) ([]telegramMediaSendItem, error) { + content := msg.Content + items := make([]telegramMediaSendItem, 0, len(msg.Media)) + for _, media := range msg.Media { + if err := c.validateOutboundMediaSize(media.URL); err != nil { + return nil, err + } + + caption := media.Caption + if caption == "" && content != "" { + caption = content + content = "" + } + + var followUpText string + if caption != "" { + caption = markdownToTelegramHTML(caption) + if len(caption) > telegramCaptionMaxLen { + followUpText = caption + caption = "" + } + } + + kind, groupable, asVoice := classifyTelegramOutboundMedia(media, msg.Metadata) + items = append(items, telegramMediaSendItem{ + media: media, + kind: kind, + caption: caption, + followUpText: followUpText, + groupable: groupable, + asVoice: asVoice, + }) + } + return items, nil +} + +func classifyTelegramOutboundMedia(media bus.MediaAttachment, metadata map[string]string) (telegramMediaKind, bool, bool) { + ct := strings.ToLower(media.ContentType) + if ct == "" { + ct = strings.ToLower(mime.TypeByExtension(filepath.Ext(media.URL))) + } + + switch { + case strings.HasPrefix(ct, "image/"): + if info, statErr := os.Stat(media.URL); statErr == nil && info.Size() > photoSizeThreshold { + slog.Info("large image, sending as document to preserve quality", "path", media.URL, "size", info.Size()) + return telegramMediaDocument, true, false + } + return telegramMediaPhoto, true, false + case strings.HasPrefix(ct, "video/"): + return telegramMediaVideo, true, false + case strings.HasPrefix(ct, "audio/"): + asVoice := metadata["audio_as_voice"] == "true" && isVoiceCompatible(ct) + return telegramMediaAudio, !asVoice, asVoice + default: + return telegramMediaDocument, true, false + } +} + +func chunkTelegramMediaItems(items []telegramMediaSendItem) []telegramMediaChunk { + chunks := make([]telegramMediaChunk, 0, len(items)) + for i := 0; i < len(items); { + key := telegramMediaGroupKey(items[i]) + if key == "" { + chunks = append(chunks, telegramMediaChunk{items: items[i : i+1]}) + i++ + continue + } + + j := i + 1 + for j < len(items) && telegramMediaGroupKey(items[j]) == key { + j++ + } + + for start := i; start < j; { + size := min(j-start, telegramMediaGroupMaxItems) + chunks = append(chunks, telegramMediaChunk{ + items: items[start : start+size], + grouped: size >= telegramMediaGroupMinItems, + }) + start += size + } + i = j + } + return chunks +} + +func telegramMediaGroupKey(item telegramMediaSendItem) string { + if !item.groupable { + return "" + } + switch item.kind { + case telegramMediaPhoto, telegramMediaVideo: + return "visual" + case telegramMediaAudio: + return "audio" + case telegramMediaDocument: + return "document" + default: + return "" + } +} diff --git a/internal/channels/telegram/send_media_group_test.go b/internal/channels/telegram/send_media_group_test.go new file mode 100644 index 00000000..dbf08555 --- /dev/null +++ b/internal/channels/telegram/send_media_group_test.go @@ -0,0 +1,70 @@ +package telegram + +import ( + "testing" + + "github.com/nextlevelbuilder/goclaw/internal/bus" +) + +func TestChunkTelegramMediaItems_GroupsCompatibleRuns(t *testing.T) { + items := []telegramMediaSendItem{ + {media: bus.MediaAttachment{URL: "a.jpg"}, kind: telegramMediaPhoto, groupable: true}, + {media: bus.MediaAttachment{URL: "b.mp4"}, kind: telegramMediaVideo, groupable: true}, + {media: bus.MediaAttachment{URL: "c.pdf"}, kind: telegramMediaDocument, groupable: true}, + {media: bus.MediaAttachment{URL: "d.pdf"}, kind: telegramMediaDocument, groupable: true}, + {media: bus.MediaAttachment{URL: "e.mp3"}, kind: telegramMediaAudio, groupable: true}, + {media: bus.MediaAttachment{URL: "f.jpg"}, kind: telegramMediaPhoto, groupable: true}, + } + + chunks := chunkTelegramMediaItems(items) + if len(chunks) != 4 { + t.Fatalf("chunks len = %d, want 4", len(chunks)) + } + assertTelegramChunk(t, chunks[0], true, 2) + assertTelegramChunk(t, chunks[1], true, 2) + assertTelegramChunk(t, chunks[2], false, 1) + assertTelegramChunk(t, chunks[3], false, 1) +} + +func TestChunkTelegramMediaItems_SplitsAtTelegramMax(t *testing.T) { + items := make([]telegramMediaSendItem, 12) + for i := range items { + items[i] = telegramMediaSendItem{ + media: bus.MediaAttachment{URL: "photo.jpg"}, + kind: telegramMediaPhoto, + groupable: true, + } + } + + chunks := chunkTelegramMediaItems(items) + if len(chunks) != 2 { + t.Fatalf("chunks len = %d, want 2", len(chunks)) + } + assertTelegramChunk(t, chunks[0], true, telegramMediaGroupMaxItems) + assertTelegramChunk(t, chunks[1], true, 2) +} + +func TestChunkTelegramMediaItems_KeepsNonGroupableItemsSeparate(t *testing.T) { + items := []telegramMediaSendItem{ + {media: bus.MediaAttachment{URL: "voice.ogg"}, kind: telegramMediaAudio, groupable: false}, + {media: bus.MediaAttachment{URL: "a.jpg"}, kind: telegramMediaPhoto, groupable: true}, + {media: bus.MediaAttachment{URL: "b.jpg"}, kind: telegramMediaPhoto, groupable: true}, + } + + chunks := chunkTelegramMediaItems(items) + if len(chunks) != 2 { + t.Fatalf("chunks len = %d, want 2", len(chunks)) + } + assertTelegramChunk(t, chunks[0], false, 1) + assertTelegramChunk(t, chunks[1], true, 2) +} + +func assertTelegramChunk(t *testing.T, chunk telegramMediaChunk, grouped bool, size int) { + t.Helper() + if chunk.grouped != grouped { + t.Fatalf("chunk.grouped = %v, want %v", chunk.grouped, grouped) + } + if len(chunk.items) != size { + t.Fatalf("chunk size = %d, want %d", len(chunk.items), size) + } +} diff --git a/internal/tools/send_file.go b/internal/tools/send_file.go index 8057723e..caff4ad4 100644 --- a/internal/tools/send_file.go +++ b/internal/tools/send_file.go @@ -55,17 +55,34 @@ func (t *SendFileTool) Parameters() map[string]any { "type": "string", "description": "Optional text message accompanying the file", }, + "attachments": map[string]any{ + "type": "array", + "description": "Optional batch of files to send in order. When set, each item must include path and may include caption.", + "items": map[string]any{ + "type": "object", + "properties": map[string]any{ + "path": map[string]any{ + "type": "string", + "description": "Path to a file to send (relative to workspace, or absolute)", + }, + "caption": map[string]any{ + "type": "string", + "description": "Optional caption for this attachment", + }, + }, + "required": []string{"path"}, + }, + }, }, - "required": []string{"path"}, } } // Execute resolves and validates the path, checks for duplicate delivery, then // returns a Result with Media populated for downstream pipeline delivery. func (t *SendFileTool) Execute(ctx context.Context, args map[string]any) *Result { - path := argString(args, "path") - if path == "" { - return ErrorResult("path is required") + requests, err := parseSendFileRequests(args) + if err != nil { + return ErrorResult(err.Error()) } // Per-request workspace (multi-tenant: each user has own workspace in context). @@ -74,52 +91,57 @@ func (t *SendFileTool) Execute(ctx context.Context, args map[string]any) *Result workspace = t.workspace } - // Resolve path with allowed-prefixes support (mirrors write_file pattern). allowed := allowedWithTeamWorkspace(ctx, t.allowedPrefixes) - resolved, err := resolvePathWithAllowed(path, workspace, effectiveRestrict(ctx, t.restrict), allowed) - if err != nil { - return ErrorResult("cannot access path: " + err.Error()) + media := make([]bus.MediaFile, 0, len(requests)) + seen := make(map[string]struct{}, len(requests)) + for _, req := range requests { + resolved, err := resolvePathWithAllowed(req.Path, workspace, effectiveRestrict(ctx, t.restrict), allowed) + if err != nil { + return ErrorResult("cannot access path: " + err.Error()) + } + + // Deny-paths guard: reject access to internal files (memory.db, config.json, etc.). + if err := checkDeniedPath(resolved, workspace, t.deniedPrefixes); err != nil { + return ErrorResult(err.Error()) + } + + // Stat: file must exist and be a regular file (not a directory or device). + fi, err := os.Stat(resolved) + if err != nil { + return ErrorResult(fmt.Sprintf("file not found: %s", req.Path)) + } + if !fi.Mode().IsRegular() { + return ErrorResult(fmt.Sprintf("path is not a regular file: %s", req.Path)) + } + + if _, duplicate := seen[resolved]; duplicate { + return ErrorResult(fmt.Sprintf("duplicate attachment path in batch: %s", filepath.Base(resolved))) + } + seen[resolved] = struct{}{} + + // Duplicate-delivery guard: block if already delivered in this turn. + if dm := DeliveredMediaFromCtx(ctx); dm != nil && dm.IsDelivered(resolved) { + return ErrorResult(fmt.Sprintf( + "file already delivered in this turn: %s. Do not re-send the same file. "+ + "If user explicitly asked to resend, the next turn will reset delivery state.", + filepath.Base(resolved))) + } + + media = append(media, bus.MediaFile{ + Path: resolved, + Filename: filepath.Base(resolved), + MimeType: mimeFromPath(resolved), + Caption: req.Caption, + }) } - // Deny-paths guard: reject access to internal files (memory.db, config.json, etc.). - if err := checkDeniedPath(resolved, workspace, t.deniedPrefixes); err != nil { - return ErrorResult(err.Error()) - } - - // Stat: file must exist and be a regular file (not a directory or device). - fi, err := os.Stat(resolved) - if err != nil { - return ErrorResult(fmt.Sprintf("file not found: %s", path)) - } - if !fi.Mode().IsRegular() { - return ErrorResult(fmt.Sprintf("path is not a regular file: %s", path)) - } - - // Duplicate-delivery guard: block if already delivered in this turn. - if dm := DeliveredMediaFromCtx(ctx); dm != nil && dm.IsDelivered(resolved) { - return ErrorResult(fmt.Sprintf( - "file already delivered in this turn: %s. Do not re-send the same file. "+ - "If user explicitly asked to resend, the next turn will reset delivery state.", - filepath.Base(resolved))) - } - - // Build result — caption overrides default message if provided. - filename := filepath.Base(resolved) - msg := fmt.Sprintf("Sent file: %s", filename) - if caption := argString(args, "caption"); caption != "" { - msg = caption - } - result := SilentResult(msg) - result.Media = []bus.MediaFile{{ - Path: resolved, - Filename: filename, - MimeType: mimeFromPath(resolved), - }} - - // Mark delivered so subsequent send_file or message(MEDIA:) calls detect the duplicate. if dm := DeliveredMediaFromCtx(ctx); dm != nil { - dm.Mark(resolved) + for _, mf := range media { + dm.Mark(mf.Path) + } } + result := SilentResult(sendFileResultMessage(media)) + result.Media = media return result } diff --git a/internal/tools/send_file_batch_requests.go b/internal/tools/send_file_batch_requests.go new file mode 100644 index 00000000..a7101564 --- /dev/null +++ b/internal/tools/send_file_batch_requests.go @@ -0,0 +1,58 @@ +package tools + +import ( + "fmt" + "strings" + + "github.com/nextlevelbuilder/goclaw/internal/bus" +) + +type sendFileRequest struct { + Path string + Caption string +} + +func parseSendFileRequests(args map[string]any) ([]sendFileRequest, error) { + if raw, ok := args["attachments"]; ok && raw != nil { + items, ok := raw.([]any) + if !ok { + return nil, fmt.Errorf("attachments must be an array") + } + if len(items) == 0 { + return nil, fmt.Errorf("attachments must include at least one file") + } + requests := make([]sendFileRequest, 0, len(items)) + for i, item := range items { + obj, ok := item.(map[string]any) + if !ok { + return nil, fmt.Errorf("attachments[%d] must be an object", i) + } + path := argString(obj, "path") + if path == "" { + return nil, fmt.Errorf("attachments[%d].path is required", i) + } + requests = append(requests, sendFileRequest{Path: path, Caption: argString(obj, "caption")}) + } + return requests, nil + } + + path := argString(args, "path") + if path == "" { + return nil, fmt.Errorf("path is required") + } + return []sendFileRequest{{Path: path, Caption: argString(args, "caption")}}, nil +} + +func sendFileResultMessage(media []bus.MediaFile) string { + if len(media) == 1 { + if media[0].Caption != "" { + return media[0].Caption + } + return fmt.Sprintf("Sent file: %s", media[0].Filename) + } + names := make([]string, 0, len(media)) + for _, mf := range media { + names = append(names, mf.Filename) + } + return fmt.Sprintf("Sent %d files: %s", len(media), strings.Join(names, ", ")) +} diff --git a/internal/tools/send_file_test.go b/internal/tools/send_file_test.go index 17dfc62c..b40a7cc7 100644 --- a/internal/tools/send_file_test.go +++ b/internal/tools/send_file_test.go @@ -462,3 +462,77 @@ func TestSendFile_T13_BinaryFileLargeNoContentRead(t *testing.T) { t.Errorf("Media[0].Path = %q, want %q", result.Media[0].Path, bigFile) } } + +func TestSendFile_T15_BatchAttachmentsHappyPath(t *testing.T) { + ws, reportPDF, subFile, _ := mkSendFileWorkspace(t) + tool := NewSendFileTool(ws, true) + + result := tool.Execute(context.Background(), map[string]any{ + "attachments": []any{ + map[string]any{"path": reportPDF, "caption": "Q4 report"}, + map[string]any{"path": subFile, "caption": "Notes"}, + }, + }) + if result.IsError { + t.Fatalf("expected success, got error: %s", result.ForLLM) + } + if len(result.Media) != 2 { + t.Fatalf("expected 2 Media entries, got %d", len(result.Media)) + } + if result.Media[0].Path != reportPDF || result.Media[1].Path != subFile { + t.Errorf("batch order = [%q, %q], want [%q, %q]", + result.Media[0].Path, result.Media[1].Path, reportPDF, subFile) + } + if result.Media[0].Caption != "Q4 report" || result.Media[1].Caption != "Notes" { + t.Errorf("captions = [%q, %q], want [Q4 report, Notes]", + result.Media[0].Caption, result.Media[1].Caption) + } +} + +func TestSendFile_T16_BatchDuplicateRejectedBeforeMarking(t *testing.T) { + ws, reportPDF, subFile, _ := mkSendFileWorkspace(t) + tool := NewSendFileTool(ws, true) + dm := NewDeliveredMedia() + ctx := WithDeliveredMedia(context.Background(), dm) + + result := tool.Execute(ctx, map[string]any{ + "attachments": []any{ + map[string]any{"path": reportPDF}, + map[string]any{"path": subFile}, + map[string]any{"path": reportPDF}, + }, + }) + if !result.IsError { + t.Fatal("expected duplicate batch error, got success") + } + if !strings.Contains(strings.ToLower(result.ForLLM), "duplicate") { + t.Errorf("expected duplicate error, got: %s", result.ForLLM) + } + if dm.IsDelivered(reportPDF) || dm.IsDelivered(subFile) { + t.Fatal("duplicate batch must not mark any file delivered") + } +} + +func TestSendFile_T17_BatchAlreadyDeliveredRejectedBeforeMarkingNewFiles(t *testing.T) { + ws, reportPDF, subFile, _ := mkSendFileWorkspace(t) + tool := NewSendFileTool(ws, true) + dm := NewDeliveredMedia() + dm.Mark(reportPDF) + ctx := WithDeliveredMedia(context.Background(), dm) + + result := tool.Execute(ctx, map[string]any{ + "attachments": []any{ + map[string]any{"path": subFile}, + map[string]any{"path": reportPDF}, + }, + }) + if !result.IsError { + t.Fatal("expected already-delivered batch error, got success") + } + if !strings.Contains(strings.ToLower(result.ForLLM), "already") { + t.Errorf("expected already-delivered error, got: %s", result.ForLLM) + } + if dm.IsDelivered(subFile) { + t.Fatal("failed batch must not mark new files delivered") + } +} diff --git a/plans/260612-1708-issue-172-multi-attachment-delivery/phase-01-research-and-tdd-contracts.md b/plans/260612-1708-issue-172-multi-attachment-delivery/phase-01-research-and-tdd-contracts.md new file mode 100644 index 00000000..3e7e05b9 --- /dev/null +++ b/plans/260612-1708-issue-172-multi-attachment-delivery/phase-01-research-and-tdd-contracts.md @@ -0,0 +1,51 @@ +--- +phase: 1 +title: Research and TDD Contracts +status: completed +priority: P1 +effort: 1h +dependencies: [] +--- + +# Phase 1: Research and TDD Contracts + +## Overview + +Lock current behavior and expected batch semantics before implementation. Tests should fail until the new batch API and Telegram grouping helpers exist. + +## Requirements + +- Functional: agents can request multiple existing files in one call. +- Functional: returned `Result.Media` preserves file order, filename, MIME type, and per-file captions when provided. +- Functional: duplicate delivery guard rejects already queued files in the same run. +- Non-functional: no file content reads in the tool path; only `stat` and path metadata. +- Non-functional: keep single-file `send_file` behavior backward compatible. + +## Architecture + +`send_file` stays the existing tool name and grows an optional `attachments` array. The array entries mirror the current single-file fields: `path` and optional `caption`. If `attachments` is present, it is the source of truth; otherwise current `path` behavior remains. + +## Related Code Files + +- Modify: `internal/tools/send_file.go` +- Modify: `internal/tools/send_file_test.go` +- Modify: `internal/channels/capabilities.go` +- Create: `internal/channels/telegram/send_media_group_test.go` if helper tests need a focused file. + +## Implementation Steps + +1. Add failing tests for `send_file` with `attachments: [{path, caption}, ...]`. +2. Add failing tests for duplicate handling: one duplicate in a batch rejects before marking new paths. +3. Add failing tests proving old `path` + `caption` still returns one media entry. +4. Add Telegram helper tests for chunk sizing and type grouping without calling the network. +5. Add Discord regression test coverage around multi-file `sendMediaMessage` if existing test hooks allow it without live API. + +## Success Criteria + +- [x] Tests describe batch tool contract and current single-file compatibility. +- [x] Telegram grouping constraints are encoded in pure helper tests. +- [x] No live channel credentials required. + +## Risk Assessment + +Main risk is over-scoping platform capability metadata. Keep tests focused on behavior needed by issue #172. diff --git a/plans/260612-1708-issue-172-multi-attachment-delivery/phase-02-tool-api-batch-attachments.md b/plans/260612-1708-issue-172-multi-attachment-delivery/phase-02-tool-api-batch-attachments.md new file mode 100644 index 00000000..91fc4b42 --- /dev/null +++ b/plans/260612-1708-issue-172-multi-attachment-delivery/phase-02-tool-api-batch-attachments.md @@ -0,0 +1,59 @@ +--- +phase: 2 +title: Tool API Batch Attachments +status: completed +priority: P1 +effort: 2h +dependencies: + - 1 +--- + +# Phase 2: Tool API Batch Attachments + +## Overview + +Implement the backward-compatible tool API extension in `send_file`. + +## Requirements + +- Functional: `send_file({attachments:[...]})` queues multiple media files in one result. +- Functional: path resolution, allow/deny guards, regular-file checks, MIME detection, and duplicate detection match single-file behavior. +- Functional: optional captions are copied onto result media for downstream channel senders. +- Non-functional: batch validation is all-or-nothing; do not mark any path delivered until every entry validates. + +## Architecture + +Refactor `SendFileTool.Execute` into small helpers: +- Parse single vs batch arguments. +- Resolve and validate each requested attachment. +- Check duplicate state against `DeliveredMedia` and within the batch. +- Build `[]bus.MediaFile`, then mark delivered after success. + +No new tool name unless tests show the model schema becomes unclear; existing `send_file` is the least disruptive contract. + +## Related Code Files + +- Modify: `internal/tools/send_file.go` +- Modify: `internal/tools/send_file_test.go` +- Modify: `internal/bus/types.go` +- Modify: `internal/agent/loop_types.go` +- Modify: `internal/agent/loop_tools.go` +- Modify: `cmd/gateway_consumer.go` + +## Implementation Steps + +1. Extend `SendFileTool.Parameters()` with optional `attachments` array. +2. Add caption support to `bus.MediaFile`, `agent.MediaResult`, and `appendMediaToOutbound` so tool-level captions reach channel senders. +3. Implement parsing helpers with clear validation errors for missing path, non-array entries, duplicate paths, and denied paths. +4. Keep existing single-file code path behavior and messages stable. +5. Run `go test ./internal/tools -run 'TestSendFile|TestMessage|TestWriteFile|TestDeliveredMedia'`. + +## Success Criteria + +- [x] Batch call returns N media entries in request order. +- [x] Duplicate paths in same batch or already delivered paths fail cleanly. +- [x] Single-path `send_file` tests still pass. + +## Risk Assessment + +Changing media structs has a broad compile surface. Keep new fields additive with `omitempty` JSON tags and update only conversion points that need captions. diff --git a/plans/260612-1708-issue-172-multi-attachment-delivery/phase-03-channel-capability-and-telegram-grouping.md b/plans/260612-1708-issue-172-multi-attachment-delivery/phase-03-channel-capability-and-telegram-grouping.md new file mode 100644 index 00000000..4574f382 --- /dev/null +++ b/plans/260612-1708-issue-172-multi-attachment-delivery/phase-03-channel-capability-and-telegram-grouping.md @@ -0,0 +1,55 @@ +--- +phase: 3 +title: Channel Capability and Telegram Grouping +status: completed +priority: P1 +effort: 3h +dependencies: + - 2 +--- + +# Phase 3: Channel Capability and Telegram Grouping + +## Overview + +Add channel capability metadata and implement Telegram media-group delivery for compatible batches while preserving ordered fallback elsewhere. + +## Requirements + +- Functional: Telegram sends compatible media in album chunks of 2-10. +- Functional: Telegram documents group only with documents, audio only with audio, and photo/video may group together. +- Functional: mixed or singleton chunks fall back to existing per-file send helpers. +- Functional: caption behavior remains stable: first item uses shared message content when no per-file caption exists; oversized captions become follow-up text. +- Non-functional: existing thread/reply behavior stays intact; reply applies to first grouped chunk only. + +## Architecture + +Add small reusable capability structs in `internal/channels/capabilities.go`, then keep platform-specific grouping logic in Telegram package: +- Capability data exposes max batch count and grouping mode for docs/tests. +- Telegram helper classifies outbound media as photo/video/audio/document, validates size, prepares captions, and emits chunks. +- `sendMediaMessage` chooses media-group chunks only when safe; all other cases call the existing `sendPhoto`, `sendVideo`, `sendAudio`, `sendVoice`, or `sendDocument` helpers. + +## Related Code Files + +- Modify: `internal/channels/capabilities.go` +- Modify: `internal/channels/telegram/send.go` +- Modify/Create: `internal/channels/telegram/*_test.go` +- Modify: `internal/channels/discord/send_media.go` only if regression tests expose a real compatibility gap. + +## Implementation Steps + +1. Add channel capability metadata for Telegram and Discord; Slack is media-capable but no single-message multi-file guarantee in this phase. +2. Add Telegram pure helpers for chunking compatible media batches. +3. Implement `sendMediaGroup` with `telego.SendMediaGroupParams` and file seek/open handling. +4. Keep fallback path for audio-as-voice, oversized image-as-document, singleton chunks, and any group send error where retrying single sends is safer. +5. Run `go test ./internal/channels/telegram ./internal/channels/discord ./internal/channels/slack`. + +## Success Criteria + +- [x] Telegram helper tests prove 2-10 compatible grouping and ordered fallback. +- [x] Discord existing multi-file upload path remains unchanged. +- [x] Slack fallback continues ordered file uploads plus text. + +## Risk Assessment + +Telegram media groups are not transactional across fallback retries. Avoid marking success until the channel call returns nil; do not hide partial failure. diff --git a/plans/260612-1708-issue-172-multi-attachment-delivery/phase-04-regression-verification-and-ship.md b/plans/260612-1708-issue-172-multi-attachment-delivery/phase-04-regression-verification-and-ship.md new file mode 100644 index 00000000..f5a1ee81 --- /dev/null +++ b/plans/260612-1708-issue-172-multi-attachment-delivery/phase-04-regression-verification-and-ship.md @@ -0,0 +1,54 @@ +--- +phase: 4 +title: Regression Verification and Ship +status: completed +priority: P1 +effort: 1h +dependencies: + - 3 +--- + +# Phase 4: Regression Verification and Ship + +## Overview + +Run focused regression checks, update docs if behavior changed, review locally, then ship a beta PR. + +## Requirements + +- Functional: acceptance criteria from issue #172 are demonstrably covered for tool API, Telegram, and Discord/fallback behavior. +- Non-functional: no syntax/build errors in touched packages. +- Non-functional: no secrets or local env values in plan/issue/PR text. + +## Architecture + +Verification should stay package-focused unless touched files force broader coverage. Full integration tests are not required because no DB schema or live channel credential path changes. + +## Related Code Files + +- Modify if warranted: `docs/05-channels-messaging.md` +- Modify if warranted: `docs/project-changelog.md` +- Read: `.github/workflows/ci.yaml` for PR check expectations if local commands differ. + +## Implementation Steps + +1. Run focused tests: + - `PATH=/usr/local/go/bin:$PATH go test ./internal/tools ./internal/channels/telegram ./internal/channels/discord ./internal/channels/slack` +2. Run compile checks: + - `PATH=/usr/local/go/bin:$PATH go test ./internal/channels/... ./internal/tools/...` + - `PATH=/usr/local/go/bin:$PATH go build ./...` + - `PATH=/usr/local/go/bin:$PATH go build -tags sqliteonly ./...` +3. Update docs/changelog only if public behavior changed enough to warrant it. +4. Run local pending diff review and fix Critical/Important findings. +5. Commit, push branch, create PR to `dev`, run PR review/fix/reply, then add `ready to ship beta`. + +## Success Criteria + +- [x] Focused tests pass. +- [x] PG and SQLite builds pass. +- [x] PR targets `dev` and links issue #172. +- [x] Source issue and PR carry `ready to ship beta`; `ready to cook` removed after PR review. + +## Risk Assessment + +`go build ./...` may expose pre-existing platform/toolchain issues in unrelated packages. If so, record exact package/error and still run focused touched-package verification. diff --git a/plans/260612-1708-issue-172-multi-attachment-delivery/plan.md b/plans/260612-1708-issue-172-multi-attachment-delivery/plan.md new file mode 100644 index 00000000..f971a8fa --- /dev/null +++ b/plans/260612-1708-issue-172-multi-attachment-delivery/plan.md @@ -0,0 +1,45 @@ +--- +title: Issue 172 Multi Attachment Delivery +description: '' +status: completed +priority: P2 +branch: codex/issue-172-multi-attachment-delivery +tags: [] +blockedBy: [] +blocks: [] +created: '2026-06-12T10:08:08.060Z' +createdBy: 'ck:plan' +source: skill +--- + +# Issue 172 Multi Attachment Delivery + +## Overview + +Implement GitHub issue #172 in beta mode. Add a batch attachment tool API that lets agents queue multiple existing workspace files in one tool call, preserve order/caption metadata, and let channel senders group uploads where the platform supports it. + +Scout summary: +- `send_file` currently accepts one `path` and returns one `Result.Media` entry: `internal/tools/send_file.go`. +- `write_file(deliver=true)` already queues one file and marks it in `DeliveredMedia`: `internal/tools/filesystem_write.go`. +- `message` can publish embedded multiple `MEDIA:` refs in one `bus.OutboundMessage`: `internal/tools/message.go`. +- Tool result media flows through `agent.MediaResult` and `appendMediaToOutbound`; captions must be copied there or batch metadata is lost. +- Discord already sends multiple files in one message: `internal/channels/discord/send_media.go`. +- Telegram currently loops one media item at a time, even though `telego.SendMediaGroup` supports 2-10 album items with type restrictions: `internal/channels/telegram/send.go`. +- Slack uploads files one by one, so it remains ordered fallback unless/until a safe multi-upload contract is added: `internal/channels/slack/send.go`. + +Scope boundary: +- In scope: backend tool contract, media caption propagation, Telegram grouped album chunks, Discord regression coverage, ordered fallback for unsupported/mixed cases. +- Out of scope: web UI changes, inbound media debounce, database migrations, WhatsApp/Zalo/Lark deep batching beyond existing ordered send behavior. + +## Phases + +| Phase | Name | Status | +|-------|------|--------| +| 1 | [Research and TDD Contracts](./phase-01-research-and-tdd-contracts.md) | Completed | +| 2 | [Tool API Batch Attachments](./phase-02-tool-api-batch-attachments.md) | Completed | +| 3 | [Channel Capability and Telegram Grouping](./phase-03-channel-capability-and-telegram-grouping.md) | Completed | +| 4 | [Regression Verification and Ship](./phase-04-regression-verification-and-ship.md) | Completed | + +## Dependencies + +No active blocking plan found in the project plan scan. Related historical plan: Discord thread attachment backfill, read-only context. diff --git a/plans/260612-1708-issue-172-multi-attachment-delivery/reports/validation-red-team-report.md b/plans/260612-1708-issue-172-multi-attachment-delivery/reports/validation-red-team-report.md new file mode 100644 index 00000000..e77da2b6 --- /dev/null +++ b/plans/260612-1708-issue-172-multi-attachment-delivery/reports/validation-red-team-report.md @@ -0,0 +1,19 @@ +# Validation and Red-Team Report + +## Validation + +- `ck plan validate --strict plans/260612-1708-issue-172-multi-attachment-delivery/plan.md` passed with 0 errors and 0 warnings. +- Repo matched source issue: `digitopvn/goclaw`, default branch `dev`. +- Source issue #172 is open and asks for multi-attachment delivery with channel limits, fallback, metadata preservation, and tests for Telegram plus another supported channel. +- Verified Telegram `telego.SendMediaGroupParams.Media` requires 2-10 items and docs/audio same-type grouping in local module `github.com/mymmrac/telego@v1.6.0`. + +## Red-Team Findings + +- Accepted: caption metadata would be lost unless copied from `bus.MediaFile` through `agent.MediaResult` into `bus.MediaAttachment`. Plan updated to include these conversion points. +- Accepted: Telegram cannot group every mixed batch. Plan limits media groups to photo/video together, document-only, and audio-only chunks; singleton and unsupported cases use existing ordered sends. +- Rejected: adding live channel integration tests. They require credentials and would make CI flaky; pure helper tests plus existing sender path tests are enough for this scope. +- Deferred: Slack true multi-file batch. Current Slack path uploads files in order and preserves thread routing; Slack's upload behavior needs separate product/API decision. + +## Unresolved Questions + +None.