fix(media): persist generated files, prevent duplicate delivery, and improve group media history

.github/workflows/claude-code-review.yml

@@ -21,7 +21,7 @@ jobs:
     runs-on: ubuntu-latest
     permissions:
       contents: read
-      pull-requests: read
+      pull-requests: write
       issues: read
       id-token: write
 

internal/agent/loop.go

@@ -392,6 +392,7 @@ func (l *Loop) runLoop(ctx context.Context, req RunRequest) (*RunResult, error)
 	var deliverables []string      // actual content from tool outputs (for team task results)
 	var blockReplies int           // count of block.reply events emitted (for dedup in consumer)
 	var lastBlockReply string      // last block reply content
+	sentMedia := map[string]bool{} // media paths already sent by message tool (dedup)
 
 	// Mid-loop compaction: summarize in-memory messages when context exceeds threshold.
 	// Uses same config as maybeSummarize (contextWindow * historyShare).
@@ -788,6 +789,18 @@ func (l *Loop) runLoop(ctx context.Context, req RunRequest) (*RunResult, error)
 
 			l.scanWebToolResult(tc.Name, result)
 
+			// Track media sent directly by message tool (dedup with RunResult.Media).
+			if tc.Name == "message" && !result.IsError {
+				if msg, _ := tc.Arguments["message"].(string); strings.HasPrefix(strings.TrimSpace(msg), "MEDIA:") {
+					// message tool sent this media via outbound bus — mark path as sent.
+					mediaPath := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(msg), "MEDIA:"))
+					if nl := strings.IndexByte(mediaPath, '\n'); nl >= 0 {
+						mediaPath = mediaPath[:nl]
+					}
+					sentMedia[filepath.Base(mediaPath)] = true
+				}
+			}
+
 			// Collect MEDIA: paths from tool results.
 			// Prefer result.Media (explicit) over ForLLM MEDIA: prefix (legacy) to avoid duplicates.
 			if len(result.Media) > 0 {
@@ -933,6 +946,17 @@ func (l *Loop) runLoop(ctx context.Context, req RunRequest) (*RunResult, error)
 
 				l.scanWebToolResult(r.tc.Name, r.result)
 
+				// Track media sent directly by message tool (dedup with RunResult.Media).
+				if r.tc.Name == "message" && !r.result.IsError {
+					if msg, _ := r.tc.Arguments["message"].(string); strings.HasPrefix(strings.TrimSpace(msg), "MEDIA:") {
+						mediaPath := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(msg), "MEDIA:"))
+						if nl := strings.IndexByte(mediaPath, '\n'); nl >= 0 {
+							mediaPath = mediaPath[:nl]
+						}
+						sentMedia[filepath.Base(mediaPath)] = true
+					}
+				}
+
 				// Collect MEDIA: paths from tool results.
 				// Prefer result.Media (explicit) over ForLLM MEDIA: prefix (legacy) to avoid duplicates.
 				if len(r.result.Media) > 0 {
@@ -1079,6 +1103,17 @@ func (l *Loop) runLoop(ctx context.Context, req RunRequest) (*RunResult, error)
 	// (e.g. once via ForwardMedia and again when the LLM reads the file).
 	mediaResults = deduplicateMedia(mediaResults)
 
+	// Exclude media already sent by message tool to avoid double delivery.
+	if len(sentMedia) > 0 {
+		filtered := mediaResults[:0]
+		for _, mr := range mediaResults {
+			if !sentMedia[filepath.Base(mr.Path)] {
+				filtered = append(filtered, mr)
+			}
+		}
+		mediaResults = filtered
+	}
+
 	return &RunResult{
 		Content:        finalContent,
 		RunID:          req.RunID,

internal/agent/media_tool_routing.go

@@ -13,6 +13,7 @@ import (
 // hasReadImageProvider checks if the read_image builtin tool has a dedicated provider configured.
 // When true, images should NOT be attached inline to the main LLM — instead the agent
 // uses the read_image tool which routes to the configured vision provider.
+// Supports both legacy flat format {"provider":"X"} and chain format {"providers":[...]}.
 func (l *Loop) hasReadImageProvider() bool {
 	if l.builtinToolSettings == nil {
 		return false
@@ -21,14 +22,31 @@ func (l *Loop) hasReadImageProvider() bool {
 	if !ok || len(raw) == 0 {
 		return false
 	}
-	// Check if provider field is set (non-empty JSON with provider key).
-	var cfg struct {
+
+	// Try chain format first: {"providers":[{"provider":"X",...}]}
+	var chain struct {
+		Providers []struct {
+			Provider string `json:"provider"`
+			Enabled  *bool  `json:"enabled,omitempty"`
+		} `json:"providers"`
+	}
+	if json.Unmarshal(raw, &chain) == nil && len(chain.Providers) > 0 {
+		for _, p := range chain.Providers {
+			if p.Provider != "" && (p.Enabled == nil || *p.Enabled) {
+				return true
+			}
+		}
+	}
+
+	// Fallback: legacy flat format {"provider":"X"}
+	var legacy struct {
 		Provider string `json:"provider"`
 	}
-	if err := json.Unmarshal(raw, &cfg); err != nil || cfg.Provider == "" {
-		return false
+	if json.Unmarshal(raw, &legacy) == nil && legacy.Provider != "" {
+		return true
 	}
-	return true
+
+	return false
 }
 
 // loadHistoricalImagesForTool collects image MediaRefs from historical messages

internal/bus/inbound_debounce.go

@@ -38,7 +38,8 @@ func NewInboundDebouncer(debounceMs time.Duration, flushFn func(InboundMessage))
 }
 
 // Push adds a message to the debounce buffer.
-// If debouncing is disabled or the message should bypass (media), it is flushed immediately.
+// All messages (text and media) are debounced so that a file/image followed by
+// a text caption within the debounce window are merged into a single agent turn.
 func (d *InboundDebouncer) Push(msg InboundMessage) {
 	// Disabled: pass through immediately.
 	if d.debounceMs <= 0 {
@@ -48,13 +49,6 @@ func (d *InboundDebouncer) Push(msg InboundMessage) {
 
 	key := debounceKey(msg)
 
-	// Media messages bypass debounce — flush any buffered text first, then process media.
-	if len(msg.Media) > 0 {
-		d.flushKey(key)
-		d.flushFn(msg)
-		return
-	}
-
 	d.mu.Lock()
 	defer d.mu.Unlock()
 
@@ -79,7 +73,8 @@ func (d *InboundDebouncer) Push(msg InboundMessage) {
 			"key", key, "debounce_ms", d.debounceMs.Milliseconds())
 	} else {
 		slog.Debug("inbound debounce: message appended",
-			"key", key, "buffered", len(buf.messages))
+			"key", key, "buffered", len(buf.messages),
+			"has_media", len(msg.Media) > 0)
 	}
 }
 

internal/channels/discord/handler.go

@@ -139,10 +139,16 @@ func (c *Channel) handleMessage(_ *discordgo.Session, m *discordgo.MessageCreate
 			}
 		}
 		if !mentioned {
+			// Extract file paths to preserve in history for later @mention
+			var mediaPaths []string
+			for _, mf := range mediaFiles {
+				mediaPaths = append(mediaPaths, mf.Path)
+			}
 			c.groupHistory.Record(channelID, channels.HistoryEntry{
 				Sender:    senderName,
 				SenderID:  senderID,
 				Body:      content,
+				Media:     mediaPaths,
 				Timestamp: m.Timestamp,
 				MessageID: m.ID,
 			}, c.historyLimit)
@@ -233,6 +239,14 @@ func (c *Channel) handleMessage(_ *discordgo.Session, m *discordgo.MessageCreate
 		}
 	}
 
+	// Collect media from pending history entries (sent before this @mention).
+	// Must come after BuildContext — CollectMedia nulls out Media fields to prevent double-cleanup.
+	if peerKind == "group" {
+		for _, p := range c.groupHistory.CollectMedia(channelID) {
+			mediaFiles = append(mediaFiles, bus.MediaFile{Path: p})
+		}
+	}
+
 	// Publish directly to bus (to preserve MediaFile MIME types)
 	c.Bus().PublishInbound(bus.InboundMessage{
 		Channel:  c.Name(),

internal/channels/dispatch.go

@@ -6,6 +6,7 @@ import (
 	"log/slog"
 	"net/http"
 	"os"
+	"path/filepath"
 	"regexp"
 	"strings"
 
@@ -70,13 +71,18 @@ func (m *Manager) dispatchOutbound(ctx context.Context) {
 				}
 			}
 
-			// Clean up temp media files only. Workspace-generated files are preserved
-			// so they remain accessible via workspace/web UI after delivery.
+			// Clean up temporary media files after send (success or failure).
+			// Only delete files from OS temp dir — workspace files (generated/)
+			// should persist for user access and potential re-sends.
 			tmpDir := os.TempDir()
 			for _, media := range msg.Media {
-				if media.URL != "" && strings.HasPrefix(media.URL, tmpDir) {
+				if media.URL == "" {
+					continue
+				}
+				abs, _ := filepath.Abs(media.URL)
+				if strings.HasPrefix(abs, tmpDir+string(filepath.Separator)) {
 					if err := os.Remove(media.URL); err != nil {
-						slog.Debug("failed to clean up media file", "path", media.URL, "error", err)
+						slog.Debug("failed to clean up temp media file", "path", media.URL, "error", err)
 					}
 				}
 			}

internal/channels/history.go

@@ -11,6 +11,7 @@ import (
 	"context"
 	"fmt"
 	"log/slog"
+	"os"
 	"strings"
 	"sync"
 	"time"
@@ -34,6 +35,7 @@ type HistoryEntry struct {
 	Sender    string
 	SenderID  string
 	Body      string
+	Media     []string // temp file paths for images/attachments (RAM-only, not persisted to DB)
 	Timestamp time.Time
 	MessageID string
 }
@@ -122,6 +124,8 @@ func (ph *PendingHistory) Record(historyKey string, entry HistoryEntry, limit in
 	existing = append(existing, entry)
 	count = len(existing) // capture pre-trim count so MaybeCompact sees threshold exceeded
 	if len(existing) > limit {
+		trimmed := existing[:len(existing)-limit]
+		go cleanupMedia(trimmed)
 		existing = existing[len(existing)-limit:]
 	}
 	ph.entries[historyKey] = existing
@@ -254,10 +258,14 @@ func (ph *PendingHistory) Clear(historyKey string) {
 	}
 
 	ph.mu.Lock()
+	toClean := ph.entries[historyKey]
 	delete(ph.entries, historyKey)
 	ph.removeFromOrder(historyKey)
 	ph.mu.Unlock()
 
+	// Clean up any remaining media temp files (after CollectMedia took what it needed)
+	go cleanupMedia(toClean)
+
 	if ph.store != nil {
 		// Remove pending flushes for this key
 		ph.removeFromFlushBuf(historyKey)
@@ -285,6 +293,35 @@ func (ph *PendingHistory) evictOldKeys() {
 	for len(ph.order) > maxHistoryKeys {
 		oldest := ph.order[0]
 		ph.order = ph.order[1:]
+		evicted := ph.entries[oldest]
 		delete(ph.entries, oldest)
+		// Clean up media temp files from evicted entries (fire-and-forget)
+		go cleanupMedia(evicted)
+	}
+}
+
+// CollectMedia returns all media file paths from pending entries for a history key
+// and removes them from the entries to prevent double-cleanup by Clear().
+func (ph *PendingHistory) CollectMedia(historyKey string) []string {
+	ph.mu.Lock()
+	defer ph.mu.Unlock()
+
+	entries := ph.entries[historyKey]
+	var paths []string
+	for i := range entries {
+		paths = append(paths, entries[i].Media...)
+		entries[i].Media = nil // prevent double-cleanup
+	}
+	return paths
+}
+
+// cleanupMedia removes temp files from history entries. Best-effort, logs warnings.
+func cleanupMedia(entries []HistoryEntry) {
+	for _, e := range entries {
+		for _, path := range e.Media {
+			if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
+				slog.Warn("pending_history: media cleanup failed", "path", path, "error", err)
+			}
+		}
 	}
 }