Skip to content

fix(file-tools): normalize Windows MSYS task paths - #46995

Closed
LeonSGP43 wants to merge 1 commit into
NousResearch:mainfrom
LeonSGP43:fix-46876-windows-msys-file-tools
Closed

fix(file-tools): normalize Windows MSYS task paths#46995
LeonSGP43 wants to merge 1 commit into
NousResearch:mainfrom
LeonSGP43:fix-46876-windows-msys-file-tools

Conversation

@LeonSGP43

Copy link
Copy Markdown
Contributor

Fixes #46876

Summary

  • normalize MSYS/Cygwin-style drive paths before file tool task-path resolution on Windows
  • keep relative file-tool paths anchored to an MSYS workspace root after normalization
  • add regression coverage for absolute and cwd-relative MSYS path resolution

Proof

  • .venv/bin/pytest tests/tools/test_file_tools.py -q -k WindowsMsysPathResolution
  • .venv/bin/pytest tests/tools/test_windows_native_support.py -q -k GitBashPathNormalization
  • .venv/bin/ruff check tools/file_tools.py tests/tools/test_file_tools.py
  • git diff --check

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists tool/file File tools (read, write, patch, search) area/config Config system, migrations, profiles duplicate This issue or pull request already exists labels Jun 16, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #44735 — both normalize MSYS/Cygwin-style drive paths in tools/file_tools.py for Windows file-tool path resolution using the same approach. #44735 is the earlier-open canonical PR (later twins #44765 and #46887 were also marked duplicate of it). This PR's Fixes #46876 target is linked as related; the fix itself is covered by #44735.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved

Normalizes Windows MSYS task paths in the file tools layer. Handles cross-platform path edge case (2 files, ~4.8KB diff).

Looks Good

  • Platform compatibility fix
  • No behavior change on non-Windows systems
  • No security implications

Reviewed by Hermes Agent (cron batch, 2026-06-16)

@OutThisLife

Copy link
Copy Markdown
Collaborator

Superseded in spirit by #61915 — same MSYS file-tool normalization, rebased onto current main, reusing local._msys_to_windows_path (your relative-cwd + absolute MSYS test cases were adapted there; you're credited via Co-authored-by). Feel free to close this one.

OutThisLife added a commit that referenced this pull request Jul 10, 2026
Git Bash hands file tools paths like /c/Users/... which Path() on native
Windows treats as relative \\c\\Users\\... under the process cwd. Reuse
local._msys_to_windows_path (extended for /cygdrive and /mnt drive forms)
in _resolve_path_for_task / _resolve_base_dir so read/write/search land on
the real drive. Container/WSL Linux paths are left untouched.

Salvages #50488 (drops unrelated desktop artifact commit); tests adapted
from #46995.

Co-authored-by: Jeff Watts <186512915+lEWFkRAD@users.noreply.github.com>
Co-authored-by: LeonSGP43 <cine.dreamer.one@gmail.com>
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
Git Bash hands file tools paths like /c/Users/... which Path() on native
Windows treats as relative \\c\\Users\\... under the process cwd. Reuse
local._msys_to_windows_path (extended for /cygdrive and /mnt drive forms)
in _resolve_path_for_task / _resolve_base_dir so read/write/search land on
the real drive. Container/WSL Linux paths are left untouched.

Salvages NousResearch#50488 (drops unrelated desktop artifact commit); tests adapted
from NousResearch#46995.

Co-authored-by: Jeff Watts <186512915+lEWFkRAD@users.noreply.github.com>
Co-authored-by: LeonSGP43 <cine.dreamer.one@gmail.com>
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the Windows regression coverage.

Automated hermes-sweeper review found that this exact file-tool path-resolution guarantee is already on current main:

Closing as implemented on main.

@teknium1 teknium1 closed this Jul 14, 2026
@teknium1 teknium1 added sweeper:implemented-on-main Sweeper: behavior already present on current main sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 14, 2026
justemu pushed a commit to justemu/hermes-agent that referenced this pull request Jul 18, 2026
Git Bash hands file tools paths like /c/Users/... which Path() on native
Windows treats as relative \\c\\Users\\... under the process cwd. Reuse
local._msys_to_windows_path (extended for /cygdrive and /mnt drive forms)
in _resolve_path_for_task / _resolve_base_dir so read/write/search land on
the real drive. Container/WSL Linux paths are left untouched.

Salvages NousResearch#50488 (drops unrelated desktop artifact commit); tests adapted
from NousResearch#46995.

Co-authored-by: Jeff Watts <186512915+lEWFkRAD@users.noreply.github.com>
Co-authored-by: LeonSGP43 <cine.dreamer.one@gmail.com>
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
Git Bash hands file tools paths like /c/Users/... which Path() on native
Windows treats as relative \\c\\Users\\... under the process cwd. Reuse
local._msys_to_windows_path (extended for /cygdrive and /mnt drive forms)
in _resolve_path_for_task / _resolve_base_dir so read/write/search land on
the real drive. Container/WSL Linux paths are left untouched.

Salvages NousResearch#50488 (drops unrelated desktop artifact commit); tests adapted
from NousResearch#46995.

Co-authored-by: Jeff Watts <186512915+lEWFkRAD@users.noreply.github.com>
Co-authored-by: LeonSGP43 <cine.dreamer.one@gmail.com>
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
Git Bash hands file tools paths like /c/Users/... which Path() on native
Windows treats as relative \\c\\Users\\... under the process cwd. Reuse
local._msys_to_windows_path (extended for /cygdrive and /mnt drive forms)
in _resolve_path_for_task / _resolve_base_dir so read/write/search land on
the real drive. Container/WSL Linux paths are left untouched.

Salvages NousResearch#50488 (drops unrelated desktop artifact commit); tests adapted
from NousResearch#46995.

Co-authored-by: Jeff Watts <186512915+lEWFkRAD@users.noreply.github.com>
Co-authored-by: LeonSGP43 <cine.dreamer.one@gmail.com>
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
Git Bash hands file tools paths like /c/Users/... which Path() on native
Windows treats as relative \\c\\Users\\... under the process cwd. Reuse
local._msys_to_windows_path (extended for /cygdrive and /mnt drive forms)
in _resolve_path_for_task / _resolve_base_dir so read/write/search land on
the real drive. Container/WSL Linux paths are left untouched.

Salvages NousResearch#50488 (drops unrelated desktop artifact commit); tests adapted
from NousResearch#46995.

Co-authored-by: Jeff Watts <186512915+lEWFkRAD@users.noreply.github.com>
Co-authored-by: LeonSGP43 <cine.dreamer.one@gmail.com>
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 duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:implemented-on-main Sweeper: behavior already present on current main 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 tool/file File tools (read, write, patch, search) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Windows MSYS path resolution breaks file writes after Hermes upgrade

5 participants