Skip to content

Normalize WSL and Git Bash media paths on Windows - #41596

Closed
IvanSolis1989 wants to merge 1 commit into
NousResearch:mainfrom
IvanSolis1989:codex/windows-media-wsl-gitbash-paths
Closed

Normalize WSL and Git Bash media paths on Windows#41596
IvanSolis1989 wants to merge 1 commit into
NousResearch:mainfrom
IvanSolis1989:codex/windows-media-wsl-gitbash-paths

Conversation

@IvanSolis1989

Copy link
Copy Markdown

Summary

  • normalize WSL-style /mnt/<drive>/... and Git Bash-style /<drive>/... media paths to native Windows drive paths before gateway attachment delivery
  • apply the normalization to explicit MEDIA: tags, bare local-file extraction, and validation
  • keep Unix-style paths unchanged unless running on Windows and the target drive root exists

Tests

  • python -m py_compile gateway/platforms/base.py tests/gateway/test_platform_base.py tests/gateway/test_extract_local_files.py
  • python -m pytest -o addopts='' -p no:timeout tests/gateway/test_platform_base.py::TestExtractMedia tests/gateway/test_extract_local_files.py::TestBasicDetection -q
  • python -m ruff check gateway/platforms/base.py tests/gateway/test_platform_base.py tests/gateway/test_extract_local_files.py

Note: on this Windows runner I disabled the pytest-timeout plugin / project signal timeout addopts for the focused test command because signal.SIGALRM is not available on Windows.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery area/config Config system, migrations, profiles labels Jun 8, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor

Code Review — Positive Verification

Reviewed the full diff. The WSL/Git Bash path normalization is well-implemented:

  1. _normalize_local_media_path() cleanly extracts the quote-stripping and expanduser logic into a reusable function, adding Windows drive-letter normalization for /mnt/x/ and /x/ paths
  2. Regex changes in MEDIA_TAG_CLEANUP_RE and extract_local_files correctly add the new path anchors without breaking existing Unix/Windows paths — the alternation order ensures /mnt/[A-Za-z]/ and /[A-Za-z]/ are tried before bare /
  3. _windows_drive_root_exists() validates the drive letter before path construction, preventing fabricated paths for non-existent drives
  4. Tests cover WSL (/mnt/c/), Git Bash (/c/), and existing Unix/Windows paths with proper monkeypatching of os.name and drive existence

The deduplication of path normalization logic across validate_media_delivery_path, extract_media, and extract_local_files is a clean improvement. No issues found.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for addressing the WSL/Git Bash media-path gap. The premise still holds on current main: explicit MEDIA extraction only expands the POSIX string at gateway/platforms/base.py:3655, bare-path extraction does likewise at gateway/platforms/base.py:3772, and validation has no native-drive normalization at gateway/platforms/base.py:1287.

Problems

  • The added tests verify extraction, but not the validation/filtering behavior the new helper also changes. The production delivery paths call filter_media_delivery_paths() after extraction (gateway/run.py:13241-13252), so a regression test should establish that /mnt/c/... and /c/... resolve through that path while an unavailable drive is rejected.

Suggested changes

  • Rebase the implementation concept onto the current shared media parser, preserving MEDIA_EXTENSIONLESS_TAG_RE / _strip_media_tag_directives at gateway/platforms/base.py:1499-1543, then add direct validation or filter-level Windows-mocked tests.

Automated hermes-sweeper review.

@teknium1 teknium1 added 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 14, 2026
@IvanSolis1989 IvanSolis1989 closed this by deleting the head repository Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists 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-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.

4 participants