Skip to content

fix(tools): preserve raw paths and regex escapes for ripgrep on Windows - #69183

Closed
rille111 wants to merge 1 commit into
NousResearch:mainfrom
rille111:fix/ripgrep-windows-path-escaping
Closed

fix(tools): preserve raw paths and regex escapes for ripgrep on Windows#69183
rille111 wants to merge 1 commit into
NousResearch:mainfrom
rille111:fix/ripgrep-windows-path-escaping

Conversation

@rille111

Copy link
Copy Markdown
Contributor

Fixes an issue on Windows where search_files with ripgrep failed with rg: /c/...: The system cannot find the path specified. (os error 3).

Root Cause

When searching paths or executing non-path strings across Git Bash MSYS on Windows, _escape_shell_arg previously ran _bash_safe_path() on all arguments. This converted drive roots (C:/..., B:/...) into POSIX paths (/c/..., /b/...) and mangled backslashes in regex search queries (such as \d+ becoming /d+).

While POSIX shell builtins require MSYS path conversion, native Windows binaries like rg.exe expect native drive paths (C:/...) and raw regex strings.

Solution

  1. Added _escape_shell_raw to quote non-path arguments and native CLI tool arguments without running MSYS path translation.
  2. Updated _search_with_rg and _search_files_rg to use _escape_shell_raw for search patterns, python scripts, and ripgrep search paths.
  3. Added unit test coverage for _escape_shell_raw and Windows path preservation in ripgrep search execution.

@alt-glitch alt-glitch added type/bug Something isn't working tool/file File tools (read, write, patch, search) platform/windows Native Windows-specific behavior or breakage P2 Medium — degraded but workaround exists needs-decision Awaiting maintainer decision before any implementation sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Jul 22, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #63458 and #67914 for #63177. The patches overlap on native rg path preservation but differ in scope: this one also changes regex, grep, and Python snippet quoting, while #67914 normalizes native-executable paths. Please choose one cross-command policy.

@rille111

Copy link
Copy Markdown
Contributor Author

This PR intentionally addresses both path preservation for native Windows executables like rg.exe and escaping for regex patterns/python snippets. On MSYS/Git Bash on Windows, passing regex patterns through MSYS path normalization alters backslashes (for example \d turning into /d), which causes search query failures regardless of whether the target path itself is normalized. Standardizing on _escape_shell_raw for non-path strings and native CLI inputs provides a consistent cross-command policy across ripgrep, grep, and inline python invocation.

@monerostar

Copy link
Copy Markdown
Contributor

Native Windows note: live-verified the underlying bug on current main and confirmed #67914 fixes it.

One caution on _escape_shell_raw keeping backslashes in paths (C:\Users\...): under Git Bash those backslashes are still risky even when not MSYS-rewritten (bash escape eating). #67914 normalizes to forward-slash native C:/..., which native rg.exe accepts and bash does not mangle. Prefer that form for path args.

The idea of not MSYS-rewriting regex patterns (\d+) is good and slightly broader than #67914 — worth ensuring that piece is not lost if PRs are collapsed.

@rille111

Copy link
Copy Markdown
Contributor Author

Update for reviewers/triage:

Per the #63177 follow-up and discussion on #67914 (comment), maintainers are consolidating the complementary halves:

That matches @monerostar's earlier note here: path args should use forward-slash native C:/... (not raw backslash form), while regex patterns still need non-path quoting.

I'll close this PR as superseded once #67914 includes the pattern portion (or once it merges). No further changes planned on this branch unless maintainers want the adjacent python -c / _python_delete quoting as a separate follow-up.

Bartok9 added a commit to Bartok9/hermes-agent that referenced this pull request Jul 29, 2026
Add _escape_pattern_arg and use it for rg + grep patterns so backslash
regex escapes (\w, \d, \() are preserved and not mangled into forward
slashes on Windows. Consolidates the pattern portion of NousResearch#69183 per the
NousResearch#63177 split (path handling stays in this PR's _escape_native_exe_arg).

Co-authored-by: rille111 <rille111@users.noreply.github.com>
@alt-glitch alt-glitch added the duplicate This issue or pull request already exists label Jul 29, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #67914 for the Windows rg repair: its current head carries the pattern-escaping work and adds receiver-aware native-path conversion. The remaining python-delete idea can be proposed separately if needed.

@rille111

Copy link
Copy Markdown
Contributor Author

Superseded by #67914.

The receiver-aware path fix already lived there; the narrow pattern portion from this PR (_escape_pattern_arg / quote-only regex escaping for _search_with_rg + _search_with_grep) was consolidated into #67914 in b0123291 with attribution. Path-side _escape_shell_raw from this branch was intentionally not carried — #67914's forward-slash native path dialect is the correct one under Git Bash.

No further work planned here. Thanks @Bartok9 @lifeFedorovAlexey @monerostar for the split.

@rille111 rille111 closed this Jul 29, 2026
@alt-glitch alt-glitch removed the needs-decision Awaiting maintainer decision before any implementation label Jul 29, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Superseded by the open consolidation #67914, which carries the regex/pattern escape work and uses receiver-aware native-path handling for the Windows rg repair.

@Bartok9

Bartok9 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Thanks all — happy to see the pattern-escaping half land in #67914 with the receiver-aware native-path handling. Agreed that #67914's forward-slash native C:/... dialect is the right one under Git Bash, so dropping _escape_shell_raw here makes sense. Glad to defer to that consolidation. If the adjacent python -c / _python_delete quoting is wanted as a separate follow-up, happy to open it.

Dolverin pushed a commit to Dolverin/hermes-agent that referenced this pull request Aug 10, 2026
Add _escape_pattern_arg and use it for rg + grep patterns so backslash
regex escapes (\w, \d, \() are preserved and not mangled into forward
slashes on Windows. Consolidates the pattern portion of NousResearch#69183 per the

Co-authored-by: rille111 <rille111@users.noreply.github.com>
(cherry picked from commit b012329)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage 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.

4 participants