Skip to content

fix(gateway): normalize MSYS drive paths in MEDIA tag parsing on Windows - #73043

Open
Thouser-wu wants to merge 2 commits into
NousResearch:mainfrom
Thouser-wu:fix/windows-msys-media-path-normalization
Open

fix(gateway): normalize MSYS drive paths in MEDIA tag parsing on Windows#73043
Thouser-wu wants to merge 2 commits into
NousResearch:mainfrom
Thouser-wu:fix/windows-msys-media-path-normalization

Conversation

@Thouser-wu

Copy link
Copy Markdown

Summary

On Windows under Git Bash / MSYS, the shell translates native Windows drive paths (e.g. C:/Users/...) into POSIX-style paths (e.g. /C:/Users/... or /C:\Users\...). These paths are emitted in MEDIA: tags by agents running under the gateway.

Native Windows Python Path() cannot parse /C:/... format:

  • Path('/C:/...').is_absolute() returns False
  • Path('/C:/...').resolve(strict=True) raises WinError 123

As a result, validate_media_delivery_path() rejects the path, causing all media attachments (images, videos, files) on Feishu, QQ Bot, and other platforms to fail silently — the MEDIA: tag is sent as plain text instead of a native attachment.

Fix

Strip the leading slash from /X:/... and /X:\... paths in _normalize_media_tag_path() so the shared validate_media_delivery_path() boundary can correctly resolve them.

+    # On Windows running under Git Bash / MSYS, the shell translates
+    # native drive paths like ``C:/...`` into POSIX-style ``/C:/...``.
+    # Strip the leading slash so native Windows ``Path()`` can parse it.
+    if os.name == "nt" and re.match(r"^/[A-Za-z]:[/\\]", path):
+        path = path[1:]

Verification

Input Normalized validate_result
/C:/Users/.../Desktop/小猪跳水.mp4 C:/Users/.../Desktop/小猪跳水.mp4 ✅ OK
/C:\Users\...\pig_diving.mp4 C:\Users\...\pig_diving.mp4 ✅ OK
C:/Users/.../pig_diving.mp4 unchanged ✅ OK

Related

Closes #47767, #31457
Related to #41596 (closed), #31472, #32735

On Windows under Git Bash / MSYS, the shell translates native Windows
drive paths (e.g. C:/Users/...) into POSIX-style paths (e.g. /C:/Users/...
or /C:\Users\...). These paths are emitted in MEDIA: tags by agents
running under the gateway.

Native Windows Python Path() cannot parse /C:/... format:
- Path('/C:/...').is_absolute() returns False
- Path('/C:/...').resolve(strict=True) raises WinError 123

As a result, validate_media_delivery_path() rejects the path, causing
all media attachments (images, videos, files) on Feishu, QQ Bot, and
other platforms to fail silently — the MEDIA: tag is sent as plain text
instead of a native attachment.

Fix: strip the leading slash from /X:/... and /X:\... paths in
_normalize_media_tag_path() so the shared validate_media_delivery_path()
boundary can correctly resolve them.

Related: NousResearch#41643, NousResearch#47767, NousResearch#31457, NousResearch#31472, NousResearch#32735
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/windows Native Windows-specific behavior or breakage sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Jul 28, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #41643 and #31472 cover /c/... and file-URL/validator pathways. This patch handles the distinct /C:/... slash-colon form at _normalize_media_tag_path, so it is not a duplicate.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused Windows MEDIA-path fix. The current extraction flow still calls _normalize_media_tag_path() for every explicit tag (gateway/platforms/base.py:4491-4511), while the helper currently leaves /C:/... unchanged (gateway/platforms/base.py:1799-1803); the downstream validator then constructs a Path from that raw value (gateway/platforms/base.py:1478-1488). The proposed normalization is therefore at the relevant shared seam, and git apply --check confirms it applies cleanly to current main.

Problems

  • No regression test covers either newly handled slash-colon spelling. Existing Windows-path coverage only tests native C:/... and C:\... inputs (tests/gateway/test_extract_local_files.py:183-205).

Suggested changes

  • Add behavioral extraction tests for MEDIA:/C:/... and MEDIA:/C:\... under the Windows branch, asserting that the extracted path reaches the native drive form.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 30, 2026

@GottZ GottZ left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This was generated by AI during triage.

Summary

Four PRs address or reference Issue #47767: #47821, #57696, and #76489 normalize the reported /c/... validator path through different implementations, while #73043 handles the distinct /C:/... and /C:\... extraction forms. None of the four addresses the separate stale-token/CDN-500 report.

Related pull requests

  • #47821 [closed] best fix — (+5/-0) — recorded best fix, closed canonical reference: converts the reported /c/Users/... form to a native Windows drive path immediately before validate_media_delivery_path() performs its absolute-path checks. It remains relevant despite being closed because contributor triage identified it as the earlier canonical implementation, although it has no regression test and does not address CDN token failures.
  • #57696 [closed] best fix — (+28/-0) — recorded best fix, closed duplicate of #47821: normalizes the same /c/... validator input through the existing tools.environments.local._msys_to_windows_path() contract and adds a focused regression test. It remains relevant despite closure because its shared-helper and test approach is technically broader than #47821, but contributors explicitly closed it in favor of the earlier #47821.
  • #73043 related — (+7/-1) — related salvageable fix: normalizes the distinct /C:/... and /C:\... spellings in _normalize_media_tag_path(), so it complements rather than duplicates the /c/... validator fixes. This agrees with the maintainer-bot keep_open verdict; the missing behavioral extraction tests for both slash-colon spellings are still required.
  • #76489 partial — (+81/-0) — partial fix requiring author action: its new local converter fixes the reported /c/... rejection and includes validator-focused tests, but duplicates only the narrow validator goal rather than the established normalization contract. Despite the visible keep_open review on #76489, the diff needs revision before it provides a viable salvage path: the contributor review identifies omitted /cygdrive/c/... and /mnt/c/... forms and an unnormalized bare-path existence check.

Duplicates

#57696 is the tested shared-helper duplicate of the earlier canonical #47821. #76489 overlaps their /c/... validator goal but is a narrower competing implementation rather than an exact duplicate; #73043 is complementary because it handles /C:/... and /C:\... during tag extraction.

Suggested consolidation

Keep #73043 open with a salvage path: preserve its distinct slash-colon normalization and add the behavioral extraction tests requested by the maintainer-bot verdict. For #76489, author action: rebase onto main and replace the new local parser with the existing Windows normalization contract, or split out a shared low-dependency utility applied at both the validator and bare-path existence seams, with coverage for /c/..., /cygdrive/c/..., /mnt/c/..., and bare-path extraction. The recorded best-fix verdicts for closed #47821 and #57696 remain useful historical evidence, but neither should be reopened over the contributor decision that made #47821 canonical and closed #57696 as its duplicate; no additional open PR should be closed as an exact duplicate.

Cross-PR triage: Reviewed 4 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 8 kB of PR diffs, 12 kB of issue/PR text, 5 kB of discussion (8 comments), 5 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

…ction

Adds regression coverage for the /C:/... and /C:\... slash-colon forms
emitted under Git Bash / MSYS on Windows. Previously only native
C:/... and C:\... paths were tested, so the leading-slash MSYS forms
would silently regress (Path().is_absolute() returns False on native
Windows Python, validate_media_delivery_path() rejects the path, and
the attachment is dropped).

Covers:
- MEDIA:/C:/... forward-slash form (Feishu)
- MEDIA:/C:\... backslash form (QQ Bot)
- native Windows paths untouched
- POSIX /tmp paths untouched

Addresses review feedback on NousResearch#73043.
@Thouser-wu

Copy link
Copy Markdown
Author

Thanks for the review! I've added the regression tests you suggested.

New commit 6cb238431 covers both slash-colon spellings in TestExtractMedia:

  • MEDIA:/C:/Users/... (forward-slash form, seen on Feishu) → normalized to C:/Users/...
  • MEDIA:/C:\Users\... (backslash form, seen on QQ Bot) → normalized to C:\Users\...
  • Native C:/... and C:\... paths → assert they remain untouched
  • POSIX /tmp/... paths → assert they remain untouched

Tests assert the extracted path reaches the native drive form via extract_media() (the shared extraction seam that calls _normalize_media_tag_path()), and that the MEDIA: tag is stripped from delivered text.

Run locally:

pytest tests/gateway/test_platform_base.py::TestExtractMedia -q

→ 18 passed (5 new parametrized cases included).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Windows MSYS/Git Bash paths rejected by validate_media_delivery_path(), blocking MEDIA directive sends

4 participants