Skip to content

fix(weixin): convert MSYS / Git-Bash drive paths to native Windows on send - #31472

Open
xxxigm wants to merge 3 commits into
NousResearch:mainfrom
xxxigm:fix/31457-weixin-msys-path-conversion
Open

fix(weixin): convert MSYS / Git-Bash drive paths to native Windows on send#31472
xxxigm wants to merge 3 commits into
NousResearch:mainfrom
xxxigm:fix/31457-weixin-msys-path-conversion

Conversation

@xxxigm

@xxxigm xxxigm commented May 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #31457.

On Windows, when the agent runs under Git Bash / MSYS the gateway
sees MEDIA paths like /c/Users/.../cache/file.jpg (and the matching
file:///c/Users/... URL). Native Windows Python's pathlib.Path
does not understand that drive shape — it joins the leading / to
the current drive's root, so the canonical example resolves to a
non-existent C:\c\Users\... and Path(path).read_bytes() reports
[Errno 2] No such file or directory. The Weixin adapter therefore
silently drops every native attachment, even though Telegram and
Discord deliver the exact same path fine because they upload bytes
through a different code path that doesn't hit the MSYS quirk.

Approach

Two small, well-tested helpers in gateway/platforms/base.py:

  • normalize_msys_path(path) — Windows-only rewrite of
    /c/Users/... and the legacy /cygdrive/c/... shape to
    C:/Users/..., drive letter ASCII-uppercased, no-op on POSIX so a
    real /c filesystem path is never corrupted.
  • strip_file_url_prefix(url) — strip exactly the file://
    scheme delimiter, drop the synthetic third slash before a
    <drive>: segment so callers get C:/foo instead of /C:/foo,
    and route the remainder through normalize_msys_path so the
    file:///c/Users/... shape (lowercase drive, no colon) survives.

Wired in at the validation seam (validate_media_delivery_path),
the bare-path scanner (extract_local_files) and the Weixin
adapter's two file-touching call sites (send_image's old naive
replace("file://", "") and _send_file's Path(path).read_bytes()).

Test plan

  • pytest tests/gateway/test_msys_path_normalization_31457.py
    25 new tests covering both helpers, source-level guards on
    the four wired call sites, and a behavioural smoke test that
    the existing happy-path validation still works.
  • pytest tests/gateway/test_platform_base.py tests/gateway/test_media_extraction.py tests/gateway/test_weixin.py — 153 passed, 2 skipped (no
    regressions in the surrounding adapters).
  • Manual: agent emits MEDIA:/c/Users/.../cache/x.jpg under
    Git Bash on Windows 11 → image is delivered natively to WeChat
    (matching Telegram/Discord behaviour).

Made with Cursor

xxxigm added 3 commits May 24, 2026 18:58
Add ``normalize_msys_path`` and ``strip_file_url_prefix`` helpers and
wire them into ``validate_media_delivery_path`` and the bare-local-path
extraction inside ``extract_local_files``.

Native Windows Python's ``pathlib.Path`` does not understand the
Git-Bash / MSYS drive shape ``/c/Users/...`` — it joins it to the
current drive's root and ends up at ``C:\\c\\Users\\...``, which never
exists.  Agents running under Git Bash routinely emit paths in that
shape, so MEDIA: directives and bare local-file references are
silently dropped on Windows even when the file is right there at
``C:\\Users\\...``.

The helpers are no-ops on non-Windows hosts (``/c`` is a real POSIX
filesystem path that we must never rewrite) and recognize both the
modern ``/<drive>/...`` and legacy ``/cygdrive/<drive>/...`` shapes.

Refs NousResearch#31457
``send_image`` previously stripped the ``file://`` prefix with a naive
``replace`` and trusted the result as a Windows-friendly path.  When
the caller passed ``file:///c/Users/.../cache/file.jpg`` (the shape an
agent running under Git Bash routinely emits on Windows) the result
``/c/Users/...`` failed downstream with ``[Errno 2] No such file or
directory`` because native Windows Python looked for a literal
``C:\\c\\Users\\...``.

Route the URL through ``strip_file_url_prefix`` so the prefix removal
and the MSYS-to-native rewrite happen together, and apply
``normalize_msys_path`` once more inside ``_send_file`` as a defense
against any future caller that bypasses the validator.

Refs NousResearch#31457
…tion

* ``normalize_msys_path`` — Windows-only rewrite of ``/c/Users/...``
  and the legacy ``/cygdrive/c/...`` shape to ``C:/Users/...``,
  ASCII-uppercased drive letter, no-op on POSIX hosts (so a real
  ``/c`` directory at the filesystem root is never corrupted).
* ``strip_file_url_prefix`` — strips ``file://`` once, trims the
  synthetic third slash before a ``<drive>:`` segment so callers get
  a clean ``C:/foo`` rather than ``/C:/foo``, and routes the rest
  through the MSYS rewrite for the ``file:///c/Users/...`` shape the
  agent emits under Git Bash.
* Source-level guards on ``validate_media_delivery_path``,
  ``BasePlatformAdapter.extract_local_files`` and the Weixin
  adapter's ``send_image`` / ``_send_file`` so future refactors
  cannot quietly drop the normalization wiring and re-open NousResearch#31457.
* Behavioural smoke test that the existing happy-path validation
  (real file under an allowlisted root) still works after the new
  helper is threaded through.

Also tightens the prefix regex to strip exactly the two-slash
scheme delimiter rather than greedily eating every slash, which
was masking the MSYS shape on the way to the rewrite step.

Refs NousResearch#31457
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists platform/wecom WeCom / WeChat Work adapter comp/gateway Gateway runner, session dispatch, delivery labels May 24, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing this Windows-specific delivery path. The underlying defect is still present on current main: gateway/platforms/base.py:1295 constructs Path from the unconverted candidate, while gateway/platforms/weixin.py:1991 strips file:// naively before _send_file reaches Path(path).read_bytes().

Problems

  • The new converter duplicates the existing Windows-gated conversion in tools/environments/local.py:23-48, which already handles /c, /cygdrive, and /mnt forms. The PR copy omits /mnt, creating a second, narrower contract.
  • tests/gateway/test_msys_path_normalization_31457.py asserts source text via inspect.getsource() rather than delivery behavior. These assertions would fail after a valid refactor and do not prove the filesystem/send boundary receives a native path.

Suggested changes

  • Consolidate on one low-dependency MSYS normalization utility and use it at the current validation and Weixin file-read seams.
  • Replace source-inspection assertions with behavioral tests for conversion before validation/existence checks and for Weixin forwarding file:///c/... to its local-send path.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
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/wecom WeCom / WeChat Work adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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 sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: MEDIA file delivery broken on Windows with Weixin adapter

3 participants