From 4988f57a9c7e84fb5301119f55158f56df2fe868 Mon Sep 17 00:00:00 2001 From: viettranx Date: Sat, 18 Apr 2026 16:19:23 +0700 Subject: [PATCH] fix(agent): revert path-normalizing dedup and symlink-resolved storage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two pieces of the review-hardening turned out to be risky in production: 1. deduplicateMedia used filepath.Abs on each MediaResult.Path to normalize the dedup key. filepath.Abs depends on the process CWD, which varies across deployment environments. In containers with a non-standard CWD, relative paths from parseMediaResult were being rewritten into keys that could collide with or differ from absolute paths from tools — leading to legitimate media entries being silently eaten. 2. extractMediaFromContent overwrote the stored path with the EvalSymlinks-resolved path. When the workspace is a bind-mount or contains ancestor symlinks, the stored path diverged from what the tool originally wrote and queued into rs.mediaResults, so dedup against tool-origin entries failed and the same file could be delivered twice (or fail altogether if the resolved path wasn't reachable from the downstream reader's FS view). Revert both to exact-string behavior. Keep EvalSymlinks for the containment check only — the symlink-escape P0 remains closed, but the stored path now matches what the tool wrote. --- internal/agent/loop_media.go | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/internal/agent/loop_media.go b/internal/agent/loop_media.go index 42255d42..7ad2118d 100644 --- a/internal/agent/loop_media.go +++ b/internal/agent/loop_media.go @@ -107,6 +107,11 @@ func extractMediaFromContent(content, workspace string) []MediaResult { // Resolve ancestor symlinks THEN check containment. A purely lexical // Rel check would pass "//secret" when symlink-dir // points outside the workspace; EvalSymlinks closes that escape. + // NOTE: resolved is used ONLY for containment — the stored path stays + // `cleaned` so downstream readers (channel senders, history) use the + // same path semantics as the tool that wrote the file. Overwriting + // with the resolved path caused dedup misses in production when + // workspace contains bind-mounts / dir symlinks. resolved, err := filepath.EvalSymlinks(cleaned) if err != nil { continue @@ -115,7 +120,6 @@ func extractMediaFromContent(content, workspace string) []MediaResult { if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { continue } - cleaned = resolved if _, dup := seen[cleaned]; dup { continue } @@ -128,12 +132,12 @@ func extractMediaFromContent(content, workspace string) []MediaResult { return results } -// deduplicateMedia removes duplicate media results, normalizing each path -// so legacy entries from parseMediaResult (raw tool output — may contain -// "./" segments or relative forms) collapse cleanly with entries from -// extractMediaFromContent (already Clean'd + Abs'd). Prevents double-send -// when a tool returns MEDIA:./x.mp3 and the LLM echoes it back in a form -// that resolves to the same file. +// deduplicateMedia removes duplicate media results by path, keeping the first +// occurrence. Exact-string match is the ONLY safe comparison: filepath.Abs +// normalization depends on the process CWD, which varies across deployment +// environments and was observed to drop legitimate entries in production. +// The tiny cost of an occasional aliased-path duplicate (e.g. "./x" vs "/abs/x") +// is preferable to silently eating a real attachment. func deduplicateMedia(media []MediaResult) []MediaResult { if len(media) <= 1 { return media @@ -141,16 +145,10 @@ func deduplicateMedia(media []MediaResult) []MediaResult { seen := make(map[string]bool, len(media)) result := make([]MediaResult, 0, len(media)) for _, m := range media { - key := m.Path - if abs, err := filepath.Abs(key); err == nil { - key = filepath.Clean(abs) - } else { - key = filepath.Clean(key) - } - if seen[key] { + if seen[m.Path] { continue } - seen[key] = true + seen[m.Path] = true result = append(result, m) } return result