Skip to content

reply: normalize Windows file paths for media dedupe#37425

Closed
shuofengzhang wants to merge 1 commit intoopenclaw:mainfrom
shuofengzhang:fix/reply-media-dedupe-windows-paths
Closed

reply: normalize Windows file paths for media dedupe#37425
shuofengzhang wants to merge 1 commit intoopenclaw:mainfrom
shuofengzhang:fix/reply-media-dedupe-windows-paths

Conversation

@shuofengzhang
Copy link
Copy Markdown
Contributor

What changed

  • Normalized filesystem-style media paths before dedupe matching in filterMessagingToolMediaDuplicates.
  • Added Windows-aware path normalization for:
    • backslash (\\) vs slash (/) separators
    • file:///C:/... vs local C:\... / C:/... forms
    • drive-letter case differences (C: vs c:)
  • Added focused unit tests covering Windows path + file:// URL equivalence.

Why

  • Media dedupe should suppress duplicate sends even when the same file path is represented differently across platforms.
  • Before this change, Windows local paths could fail to match equivalent file:// URLs, causing duplicate media payloads.
  • This keeps dedupe behavior consistent and low-noise for cross-platform usage.

Testing

  • scripts/clone_and_test.sh openclaw/openclaw (pass: 16 passed)
  • npx vitest --run src/auto-reply/reply/reply-payloads.test.ts (pass: 17 passed)

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 6, 2026

Greptile Summary

This PR adds Windows file path normalization to the media deduplication logic in filterMessagingToolMediaDuplicates, ensuring that paths represented as local Windows paths (e.g., C:\tmp\photo.jpg) and their file:// URL equivalents (e.g., file:///C:/tmp/photo%20one.jpg) are treated as the same file during dedupe.

The implementation introduces normalizeFilesystemPathForDedupe, which handles three normalization steps in sequence: replacing backslashes with forward slashes, stripping a leading / before a drive letter (as produced by URL pathname parsing), and lowercasing the drive letter. The existing normalizeMediaForDedupe is updated to apply this normalization for both local paths and decoded file:// URLs.

  • The normalization logic is correct for all described cases: backslash vs. slash separators, file:///C:/... vs. C:\.../C:/... forms, and drive-letter case differences.
  • The two new tests are well-targeted and verify the key Windows-specific equivalence scenarios.
  • The normalizeFilesystemPathForDedupe helper is defined as a nested function inside filterMessagingToolMediaDuplicates, consistent with the pre-existing normalizeMediaForDedupe pattern.
  • UNC file URLs (file://server/share/path) are handled gracefully: the parsed.host branch reconstructs a //server/share/path path, and the drive-letter regexes correctly leave such paths unchanged.
  • No regression risk for existing Unix-style path deduplication — paths not matching the drive-letter patterns pass through unchanged.

Confidence Score: 5/5

  • This PR is safe to merge — it is a focused, additive normalization fix with no behavioural regressions for existing paths.
  • The normalization logic is correctly ordered (backslash → slash, leading-slash strip for URL-parsed pathnames, drive-letter lowercase), the regex conditions are non-overlapping and terminate cleanly, and all existing tests continue to pass. The two new tests directly exercise the new code paths. No security surface is affected, and the change is scoped entirely to internal dedupe comparison logic.
  • No files require special attention.

Last reviewed commit: 98731af

@shuofengzhang shuofengzhang force-pushed the fix/reply-media-dedupe-windows-paths branch from 98731af to cbce322 Compare March 6, 2026 08:24
@openclaw-barnacle openclaw-barnacle bot added app: macos App: macos size: S and removed size: XS labels Mar 6, 2026
@shuofengzhang shuofengzhang force-pushed the fix/reply-media-dedupe-windows-paths branch from 2356f66 to 5256d41 Compare March 6, 2026 09:38
@openclaw-barnacle openclaw-barnacle bot added size: XS and removed app: macos App: macos size: S labels Mar 6, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5256d41c60

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

if (!path) {
return "";
}
let normalized = path.replace(/\\/g, "/");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restrict backslash normalization to Windows-style paths

Replacing every \\ with / changes semantics for valid POSIX file names that contain a literal backslash, so distinct files can be treated as duplicates and dropped. For example on Linux/macOS, a previously sent URL like file:///tmp/a%5Cb.jpg normalizes to /tmp/a/b.jpg, which then matches a different payload path /tmp/a/b.jpg and suppresses a non-duplicate media send. This regression is introduced by applying filesystem normalization unconditionally instead of only for Windows-style paths/URLs.

Useful? React with 👍 / 👎.

@openclaw-barnacle
Copy link
Copy Markdown

Closing this PR because the author has more than 10 active PRs in this repo. Please reduce the active PR queue and reopen or resubmit once it is back under the limit. You can close your own PRs to get back under the limit.

@openclaw-barnacle openclaw-barnacle bot closed this Mar 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants