From 4a692d51d82c39b81dd71ed3814d1237ee016879 Mon Sep 17 00:00:00 2001 From: viettranx Date: Wed, 25 Mar 2026 12:37:49 +0700 Subject: [PATCH] fix(files): heal legacy media URLs and deduplicate file references SignFileURLs now heals legacy session data at delivery time: - Dedup double /v1/files/v1/files/ prefix from old mutation bug - Strip stale ?ft= from markdown link display text - Re-sign all URLs with fresh tokens (never skip) File serving: use workspace root for ft= auth fallback search (no tenant context available with signed tokens). Deduplicate ContentSuffix media by basename to prevent duplicate file links when agent text already references the same file. Frontend dedup as defense-in-depth for legacy data. --- internal/agent/loop_finalize.go | 5 +- internal/agent/loop_media_dedup.go | 68 +++++++++++++++++++++ internal/http/file_token.go | 17 +++++- internal/http/files.go | 6 +- ui/web/src/components/chat/rich-content.tsx | 51 +++++++++++++++- 5 files changed, 141 insertions(+), 6 deletions(-) create mode 100644 internal/agent/loop_media_dedup.go diff --git a/internal/agent/loop_finalize.go b/internal/agent/loop_finalize.go index edadd2f3..bf2c53da 100644 --- a/internal/agent/loop_finalize.go +++ b/internal/agent/loop_finalize.go @@ -48,8 +48,9 @@ func (l *Loop) finalizeRun( } // Append content suffix (e.g. image markdown for WS) before saving to session. - if req.ContentSuffix != "" && !strings.Contains(rs.finalContent, req.ContentSuffix) { - rs.finalContent += req.ContentSuffix + // Dedup by basename: skip suffix lines whose file already appears in the agent's text. + if req.ContentSuffix != "" { + rs.finalContent += deduplicateMediaSuffix(rs.finalContent, req.ContentSuffix) } // Collect forwarded media + dedup + populate sizes BEFORE saving to session, diff --git a/internal/agent/loop_media_dedup.go b/internal/agent/loop_media_dedup.go new file mode 100644 index 00000000..531f3262 --- /dev/null +++ b/internal/agent/loop_media_dedup.go @@ -0,0 +1,68 @@ +package agent + +import ( + "path/filepath" + "strings" +) + +// deduplicateMediaSuffix filters ContentSuffix lines, removing any whose file +// basename already appears in the main content. This prevents duplicate file +// references when the agent's own text already includes the same media. +// +// ContentSuffix format (from mediaToMarkdown): +// +// \n\n![image](/v1/files/path/to/file.png) +// [filename.md](/v1/files/path/to/filename.md) +func deduplicateMediaSuffix(content, suffix string) string { + lines := strings.Split(suffix, "\n") + var kept []string + for _, line := range lines { + if line == "" { + kept = append(kept, line) + continue + } + // Extract basename from the URL in markdown link/image: ](url) or ](url) + base := extractLinkBasename(line) + if base != "" && containsMediaRef(content, base) { + continue // already referenced in content + } + kept = append(kept, line) + } + result := strings.Join(kept, "\n") + // If only empty lines remain, return empty string. + if strings.TrimSpace(result) == "" { + return "" + } + return result +} + +// containsMediaRef checks if content contains a URL reference to a file by basename. +// Requires URL context (slash before, paren or query after) to avoid false positives +// matching prose text like "I saved output.png". +func containsMediaRef(content, basename string) bool { + return strings.Contains(content, "/"+basename+")") || + strings.Contains(content, "/"+basename+"?") || + strings.Contains(content, "/"+basename+"\n") || + strings.HasSuffix(content, "/"+basename) +} + +// extractLinkBasename extracts the filename (basename) from a markdown link or +// image line. Handles: ![alt](url), [text](url). +func extractLinkBasename(line string) string { + // Find the URL between ]( and ) + idx := strings.Index(line, "](") + if idx < 0 { + return "" + } + urlStart := idx + 2 + urlEnd := strings.LastIndex(line, ")") + if urlEnd <= urlStart { + return "" + } + url := line[urlStart:urlEnd] + // Strip query params before taking basename. + if qIdx := strings.Index(url, "?"); qIdx > 0 { + url = url[:qIdx] + } + return filepath.Base(url) +} diff --git a/internal/http/file_token.go b/internal/http/file_token.go index 207cd44d..3878d752 100644 --- a/internal/http/file_token.go +++ b/internal/http/file_token.go @@ -70,12 +70,21 @@ var fileURLRe = regexp.MustCompile(`(/v1/(?:files|media)/[^\s)"'<>]+)`) // SignFileURLs finds all /v1/files/ and /v1/media/ URLs in content and appends // a signed ?ft= token. Used at delivery time (WS events, HTTP responses) to avoid -// persisting tokens in session messages. Strips any stale ?ft= tokens from legacy -// data before re-signing with a fresh token. +// persisting tokens in session messages. Also heals legacy data issues: +// - Strips stale ?ft= tokens and re-signs with fresh tokens +// - Fixes double /v1/files/v1/files/ prefix from old media mutation bug +// - Cleans ?ft= from markdown link display text [name?ft=xxx](url) → [name](url) func SignFileURLs(content, secret string) string { if secret == "" || !strings.Contains(content, "/v1/") { return content } + // Heal legacy: deduplicate /v1/files/v1/files/ → /v1/files/ and /v1/media/v1/media/ → /v1/media/ + content = strings.ReplaceAll(content, "/v1/files/v1/files/", "/v1/files/") + content = strings.ReplaceAll(content, "/v1/files/v1/media/", "/v1/media/") + content = strings.ReplaceAll(content, "/v1/media/v1/media/", "/v1/media/") + // Heal legacy: clean ?ft=... from markdown link display text [name?ft=xxx](...) → [name](...) + content = linkTextFtRe.ReplaceAllString(content, "[$1]") + // Sign URLs return fileURLRe.ReplaceAllStringFunc(content, func(url string) string { // Strip stale ft= token if present (legacy data may have persisted tokens). cleanURL := stripFileToken(url) @@ -88,6 +97,10 @@ func SignFileURLs(content, secret string) string { }) } +// linkTextFtRe matches markdown link text containing ?ft=... e.g. [filename.md?ft=xxx](...) +// Captures the clean name before ?ft= for replacement. +var linkTextFtRe = regexp.MustCompile(`\[([^\]\s?]+)\?ft=[^\]]*\]`) + // staleTokenRe matches ?ft=... or &ft=... query parameter (greedy to end of URL segment). var staleTokenRe = regexp.MustCompile(`[?&]ft=[^\s)"'<>&]*`) diff --git a/internal/http/files.go b/internal/http/files.go index e2764e52..614e5fe4 100644 --- a/internal/http/files.go +++ b/internal/http/files.go @@ -110,8 +110,12 @@ func (h *FilesHandler) handleServe(w http.ResponseWriter, r *http.Request) { if err != nil || info.IsDir() { // Fallback: search workspace for file by basename (handles LLM-hallucinated paths). // Generated filenames (goclaw_gen_*) include nanosecond timestamps and are globally unique. - // Workspace scoped to tenant to prevent cross-tenant file discovery. + // For ft= signed requests, search from workspace root (no tenant context available); + // for bearer requests, scope to tenant workspace. ws := h.tenantWorkspace(r) + if r.URL.Query().Get("ft") != "" { + ws = h.workspace // ft= auth has no tenant context; path is cryptographically bound + } if resolved := h.findInWorkspace(ws, filepath.Base(absPath)); resolved != "" { absPath = resolved info, _ = os.Stat(absPath) diff --git a/ui/web/src/components/chat/rich-content.tsx b/ui/web/src/components/chat/rich-content.tsx index 3a70ec4e..a6a41b94 100644 --- a/ui/web/src/components/chat/rich-content.tsx +++ b/ui/web/src/components/chat/rich-content.tsx @@ -251,6 +251,55 @@ function VideoNoticeBadge({ content }: { content: string }) { ); } +// --- Media link dedup --- + +/** Extract basename from a markdown link/image URL, stripping query params. */ +function linkBasename(line: string): string | null { + const m = line.match(/\]\(([^)]+)\)/); + if (!m?.[1]) return null; + const url = m[1].split("?")[0] ?? ""; + const slash = url.lastIndexOf("/"); + return slash >= 0 ? url.slice(slash + 1) : url; +} + +/** + * Remove duplicate media links from content. When the same file (by basename) + * appears in both the agent's body text and an appended ContentSuffix block, + * keep the first occurrence and drop later duplicates. + */ +function deduplicateMediaLinks(content: string): string { + const lines = content.split("\n"); + // Pass 1: collect first occurrence index of each media basename (inline or standalone). + const firstSeen = new Map(); + for (let i = 0; i < lines.length; i++) { + const line = lines[i] ?? ""; + if (!line.includes("](/")) continue; + const base = linkBasename(line); + if (base && !firstSeen.has(base)) firstSeen.set(base, i); + } + // Pass 2: drop STANDALONE link/image lines whose basename appeared in an earlier line. + const seen = new Set(); + const result: string[] = []; + for (let i = 0; i < lines.length; i++) { + const line = lines[i] ?? ""; + const trimmed = line.trim(); + const isStandaloneLink = (trimmed.startsWith("![") || trimmed.startsWith("[")) && + trimmed.includes("](/") && trimmed.endsWith(")"); + if (isStandaloneLink) { + const base = linkBasename(trimmed); + if (base) { + if (seen.has(base)) continue; // duplicate standalone — drop + seen.add(base); + // Also skip if this basename appeared inline in an earlier line + const first = firstSeen.get(base); + if (first !== undefined && first < i) continue; + } + } + result.push(line); + } + return result.join("\n"); +} + // --- Main component --- interface RichContentProps { @@ -259,7 +308,7 @@ interface RichContentProps { } export function RichContent({ content, role }: RichContentProps) { - const blocks = parseRichContent(content); + const blocks = parseRichContent(deduplicateMediaLinks(content)); // If no special blocks found, render as plain markdown (fast path) const first = blocks[0];