fix(search): zero-match probes fall back to grep; native paths for rg on Windows - #77157
fix(search): zero-match probes fall back to grep; native paths for rg on Windows#77157andrexibiza wants to merge 2 commits into
Conversation
|
Verification evidence + merge status (from the contributor's Windows dev box, Aug 2)
Ruff: clean on both changed files. Note for maintainers: this branch currently shows CONFLICTING — upstream |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for carrying the grep-fallback and native-rg fixes. The premise remains partly valid on current main: _zero_match_probe() still returns immediately without rg at tools/file_operations.py:2278, and rg roots still use _escape_shell_arg() at tools/file_operations.py:2447, 2458, and 2542. The rg early-return portion is already implemented by 794d6c434e.
Problems
- Proposed
_escape_native_argis host-OS gated, but command execution is backend-agnostic throughself.env.execute(...)(tools/file_operations.py:876-881). This can rewrite a valid remote/mnt/d/...path on a Windows host. The exact SSH/WSL reproduction and aLocalEnvironment-only guard are documented in #67914.
Suggested changes
- Limit native-rg conversion to the local Windows backend and add local-versus-remote path regression coverage.
- Add one end-to-end forced-grep
search()test that verifies a real zero-match hint is attached.
Automated hermes-sweeper review.
| — POSIX paths are identical either way. | ||
| """ | ||
| from tools.environments.local import _msys_to_windows_path | ||
|
|
There was a problem hiding this comment.
Please scope this conversion to the local Windows backend, not host os.name alone. _exec() forwards commands to arbitrary self.env backends; a Windows-host SSH/container search can legitimately use /mnt/d/..., which this helper would turn into an invalid D:\... remote path. #67914 documents this exact case.
There was a problem hiding this comment.
Addressed in 6d8c422354 — the native-path rewrite is now scoped to the executed backend, not the host OS: _escape_native_arg gates on _local_windows_backend() (tools/file_operations.py:1004/1028), so remote /mnt/d paths never get mangled. Verified at head 6d8c422354: 24/24 tests in tests/tools/test_search_zero_match_and_multipath.py pass.
… on Windows The zero-match steering feature (5797b50/e7aa06c) was dead on the rg path: _search_content returned early after the rg search, so the probe block was unreachable whenever ripgrep was used — the 3 hint tests in test_search_zero_match_and_multipath.py failed on any env with rg. Also: - _zero_match_probe hard-required rg and silently returned None when the main search had fallen back to grep (rg not on PATH). It now picks the same engine as the main search (rg -> grep) with per-engine count flags, so hints appear in grep-only environments too. - Native rg.exe cannot read the MSYS /c/... path form when MSYS argument conversion is disabled (MSYS2_ARG_CONV_EXCL=*, set by the local env to stop flag mangling). New _escape_native_arg passes the C:/... form to rg in the content search, files search, and probes, fixing search on Windows. No-op off Windows. New TestZeroMatchProbeGrepFallback covers the grep-engine hints (case/literal/hidden/zero). Full file: 16 passed, 0 failed locally (was 8 failed on the rg path before this change).
1c014c2 to
2013a58
Compare
|
Rebased onto current main (a6defd4) — conflict in tools/file_operations.py resolved. Main's rework already ran the zero-match probe for both engines (used_rg flag); this branch adds what main still lacks: grep fallback inside the probe itself (main still hard-requires rg there) and Windows native-path handling for rg.exe (main still passes MSYS /c/ paths, which native rg cannot read). Re-verified on rebased commit 2013a58: tests/tools/test_search_zero_match_and_multipath.py = 20 passed / 0 failed, ruff clean. |
Addresses teknium1's hermes-sweeper review on NousResearch#77157 and closes the canonical Windows search bug (NousResearch#63177, dup NousResearch#77036). The native-rg path rewrite (_escape_native_arg) was previously gated on the host OS alone (os.name == 'nt'), but command execution is backend-agnostic through self.env.execute: a Windows host driving a remote SSH/WSL/Docker backend would rewrite valid remote paths such as /mnt/d/... into an invalid D:\... form. The conversion now only applies when the EXECUTED backend is the local Windows environment (_local_windows_backend: isinstance(self.env, LocalEnvironment) and _IS_WINDOWS, mirroring the _lsp_local_only gate). Remote paths pass through unchanged. The grep-fallback probe work (engine picker + per-engine flags) is kept — main's _zero_match_probe still hard-requires rg. Upstream NousResearch#77128 already landed the rg early-return repair; main's structure is preserved (probe runs on both engines, used_rg guard for the newline warning). New tests: TestNativePathBackendGating (local /c/ -> C: conversion, remote /mnt/d/ /mnt/c/ /home/ pass through even with _IS_WINDOWS=True, remote probe command keeps the raw remote path) and test_end_to_end_search_real_hint_via_forced_grep (real search() pipeline with grep forced via _has_command, real probe, asserts the case- insensitive hint in the result warning). 24 passed / 0 failed on the target file; broader-suite failure set byte-identical to pristine origin/main (stash + worktree proven); ruff clean. Independent QA critique: SAFE TO SHIP.
|
Addressed in 6d8c422 — the native-path rewrite is now scoped to the executed backend, not the host OS: |
|
Review receipt — comment from 2026-08-02 on tools/file_operations.py (_escape_shell_arg_native / zero-match path conversion): Addressed in 6d8c422 — the native-path rewrite is now scoped to the LOCAL Windows backend (), not host os.name alone. A Windows host with a remote SSH/container backend keeps POSIX path semantics (/mnt/d/..., /c/... are real paths there — the exact #67914 case); only the local Git Bash env converts /c/Users/x -> C:\Users\x for native rg. Also closes the canonical bug #63177 (dup #77036). Regression coverage added per your ask:
24/24 test_search_zero_match_and_multipath.py tests pass at head 6d8c422. |
|
The exact gate (from tools/file_operations.py:1004): |
|
Decision recorded against the current search architecture. Do not rebase this combined branch as the canonical landing vehicle. Both of its behavioral halves now have narrower owners:
The current #77157 head correctly absorbed the original review finding by gating conversion on the executed backend rather than host Disposition:
Required convergence tests across the two canonical owners: native The decision is complete. The current branch should not be landed independently. |
What changed and why
This PR fixes the native-Windows
search_filespath boundary while preserving the existing MSYS-safe path handling used by shell builtins and remote backends.Hermes intentionally sets
MSYS_NO_PATHCONV=1/MSYS2_ARG_CONV_EXCL=*for local Git Bash commands._escape_shell_arg()therefore correctly produces/c/...for Bash/MSYS consumers, but that same spelling reaches native Win32rg.exeunchanged and cannot be resolved. The PR adds_escape_native_arg()and uses it only for the search-root argument at the native ripgrep boundary:_zero_match_probe(case-insensitive, hidden, and fixed-string probes)_search_files_rg(sorted and fallback commands)_search_with_rgThe conversion is gated to
LocalEnvironmenton Windows. SSH, WSL, Docker, and other remote backends keep their own path namespaces unchanged. Grep and shell builtins retain the existing MSYS-safe escaping.Why this matters to users
Before this change, a Windows user searching an absolute path containing spaces could get an empty result or an
os error 3even thoughread_filecould read the same file. Relative paths appeared to work, which made the failure look inconsistent and could cause the agent to reason from a false empty directory. After this change, native backslash paths, drive-letter paths, MSYS drive paths, and case-varied paths resolve consistently for local Windows search without changing remote-backend behavior.Verification
origin/main2446c8bb6: real Windows 10 nativerg 15.1.0harness.read_filesucceeded for native backslash,C:/,/c/, and case-varied paths containing spaces;search_filesreturned 0 and content search returned native-rgos error 3for each existing path.6d8c4223549c2f1fa5d6a5af3bcaf3536fe337ad: the same real Windows harness returnedread_filesuccess, one content match, and one file match for all four path forms. A nonexistent path with spaces remained a structuredPath not foundresult.scripts/run_tests.sh tests/tools/test_search_zero_match_and_multipath.py tests/tools/test_file_operations.py -k 'native or escape_native or local_windows_backend or remote_backend or search_with_rg_uses or rg_path'— 5 passed.python scripts/check-windows-footguns.pyandgit diff --checkpass on the verified source.Fixes #63177
Related: #67629, #75455, #82400