diff --git a/tests/tools/test_search_zero_match_and_multipath.py b/tests/tools/test_search_zero_match_and_multipath.py index 9afccd982cbb..b2e7158b159a 100644 --- a/tests/tools/test_search_zero_match_and_multipath.py +++ b/tests/tools/test_search_zero_match_and_multipath.py @@ -53,6 +53,152 @@ def test_matching_search_unaffected(self, proj): assert "warning" not in r +class TestZeroMatchProbeGrepFallback: + """Zero-match probes must still attach hints when rg is unavailable. + + The main content search falls back to grep when ``rg`` is not on the + executed environment's PATH. The probe used to hard-require rg and + silently return no hint in that case — search worked (via grep) but + the zero-match steering was dead. These tests force the grep engine + and assert the same hints appear. Regression for the CI failure on + ``tests/tools/test_search_zero_match_and_multipath.py``. + """ + + @staticmethod + def _grep_ops(tmp_path): + from tools.environments.local import LocalEnvironment + from tools.file_operations import ShellFileOperations + + d = tmp_path / "proj" + d.mkdir() + env = LocalEnvironment(cwd=str(d.parent)) + ops = ShellFileOperations(env, cwd=str(d.parent)) + # Simulate an environment with grep but no rg. + ops._has_command = lambda cmd: cmd == "grep" + return ops, d + + def test_case_mismatch_hint_via_grep(self, tmp_path): + ops, d = self._grep_ops(tmp_path) + (d / "a.py").write_text("TOKEN_ALPHA = 'x'\n") + hint = ops._zero_match_probe("token_alpha", str(d), None) + assert hint and "case-insensitive" in hint + + def test_literal_hint_via_grep(self, tmp_path): + ops, d = self._grep_ops(tmp_path) + (d / "meta.py").write_text("result = lookup[key+1]\n") + hint = ops._zero_match_probe("lookup[key+1]", str(d), None) + assert hint and "literal match" in hint + + def test_hidden_only_hint_via_grep(self, tmp_path): + ops, d = self._grep_ops(tmp_path) + (d / ".secretdir").mkdir() + (d / ".secretdir" / "conf.cfg").write_text("HIDDEN_ONLY_TOKEN = true\n") + hint = ops._zero_match_probe("HIDDEN_ONLY_TOKEN", str(d), None) + assert hint and "hidden or gitignored" in hint + + def test_true_zero_no_hint_via_grep(self, tmp_path): + ops, d = self._grep_ops(tmp_path) + (d / "a.py").write_text("x = 1\n") + assert ops._zero_match_probe("zzz_absent_zzz", str(d), None) is None + + def test_probe_engine_prefers_rg_when_available(self, tmp_path): + from tools.environments.local import LocalEnvironment + from tools.file_operations import ShellFileOperations + + d = tmp_path / "proj" + d.mkdir() + ops = ShellFileOperations(LocalEnvironment(cwd=str(d.parent)), cwd=str(d.parent)) + ops._has_command = lambda cmd: cmd in ("rg", "grep") + engine, flags = ops._probe_engine() + assert engine == "rg" + assert "count-matches" in flags + + def test_end_to_end_search_real_hint_via_forced_grep(self, tmp_path): + """End-to-end ``search()`` with grep forced attaches a real probe hint. + + The whole pipeline — engine pick, ``_search_with_grep``, zero-count + detection, and the probe itself — runs for real (no sentinel, no + probe stub). Regression for the probe silently dying when rg is + absent while the main search itself falls back to grep. + """ + ops, d = self._grep_ops(tmp_path) + (d / "a.py").write_text("TOKEN_ALPHA = 'x'\n") + r = ops.search("token_alpha", path=str(d), target="content") + assert r.total_count == 0 + assert r.warning and "case-insensitive" in r.warning + + +class TestNativePathBackendGating: + """rg native-path conversion is limited to the local Windows backend. + + Commands run through ``self.env.execute``, so the executed backend — + not the host OS — decides whether the MSYS→native path rewrite + applies. A Windows host driving a remote backend (SSH, WSL, Docker, + ...) must never rewrite valid remote paths like ``/mnt/d/...`` into + ``D:\\...`` (issue #67914). These tests pin the split: conversion on + the local backend only, pass-through on remote backend paths. + """ + + @staticmethod + def _local_ops(tmp_path): + from tools.environments.local import LocalEnvironment + from tools.file_operations import ShellFileOperations + + d = tmp_path / "proj" + d.mkdir() + return ShellFileOperations(LocalEnvironment(cwd=str(d.parent)), cwd=str(d.parent)) + + @staticmethod + def _remote_ops(commands=None): + from tools.file_operations import ShellFileOperations + + class RemoteEnv: + """Minimal POSIX backend (SSH/WSL-like); no MSYS path rewriting.""" + + cwd = "/home/me" + + def execute(self, command, cwd=None, **kwargs): + if commands is not None: + commands.append(command) + return {"output": "", "returncode": 0} + + return ShellFileOperations(RemoteEnv()) + + def test_local_windows_backend_converts_msys_path(self, tmp_path, monkeypatch): + import tools.environments.local as local_mod + + ops = self._local_ops(tmp_path) + monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) + out = ops._escape_native_arg("/c/Users/alice/notes.txt") + assert "C:" in out # native drive form for the native rg.exe + assert "/c/Users" not in out + + def test_remote_backend_keeps_remote_path_unchanged(self, monkeypatch): + import tools.environments.local as local_mod + + ops = self._remote_ops() + # Simulate a Windows host driving a remote backend: conversion + # must still be skipped — the remote POSIX side owns these paths. + monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) + assert ops._escape_native_arg("/mnt/d/data") == "'/mnt/d/data'" + assert ops._escape_native_arg("/mnt/c/Users/alice") == "'/mnt/c/Users/alice'" + assert ops._escape_native_arg("/home/user/proj") == "'/home/user/proj'" + + def test_remote_backend_probe_command_keeps_remote_path(self, monkeypatch): + """The zero-match probe emits the raw remote path in its command.""" + import tools.environments.local as local_mod + + commands = [] + ops = self._remote_ops(commands) + ops._has_command = lambda cmd: cmd == "rg" + monkeypatch.setattr(local_mod, "_IS_WINDOWS", True) # Windows host, remote backend + assert ops._zero_match_probe("needle", "/mnt/d/data", None) is None + assert commands, "probe should have executed shell commands" + for cmd in commands: + assert "/mnt/d/data" in cmd + assert "D:" not in cmd + + class TestMultiPathRecovery: def test_two_existing_paths_merged(self, proj): p = f"{proj / 'proj'} {proj / 'extra'}" diff --git a/tools/file_operations.py b/tools/file_operations.py index f6a2deacaba8..c5605a3ddd5c 100644 --- a/tools/file_operations.py +++ b/tools/file_operations.py @@ -1001,6 +1001,52 @@ def _escape_shell_arg(self, arg: str) -> str: # Use single quotes and escape any single quotes in the string return "'" + arg.replace("'", "'\"'\"'") + "'" + def _escape_native_arg(self, arg: str) -> str: + """Escape *arg* for a native Windows binary (no MSYS path form). + + ``_escape_shell_arg`` rewrites drive paths to the Git Bash + ``/c/Users/x`` form. That is correct for MSYS-aware tools (bash + builtins, grep from Git Bash), but native Windows executables such + as the WinGet ``rg.exe`` cannot read ``/c/...`` when MSYS argument + conversion is disabled (``MSYS2_ARG_CONV_EXCL=*``, which the Hermes + local env sets deliberately to stop flag mangling). Native tools + need the ``C:/Users/x`` form. + + The rewrite is limited to the local Windows backend (see + :meth:`_local_windows_backend`): commands execute through + ``self.env.execute`` on whatever backend is wired in, so the host + OS alone must never trigger it. A Windows host driving a remote + SSH/WSL/Docker backend would otherwise rewrite valid remote paths + such as ``/mnt/d/...`` into ``D:\\...`` and break the remote + search (issue #67914). Remote paths pass through unchanged. + """ + from tools.environments.local import _msys_to_windows_path + + native = _msys_to_windows_path(arg) if self._local_windows_backend() else arg + return "'" + native.replace("'", "'\"'\"'") + "'" + + def _local_windows_backend(self) -> bool: + """Return True iff commands execute on the local Git Bash on a Windows host. + + This gates on the executed *backend*, not the host OS: every + backend (local, SSH, WSL, Docker, Modal, ...) runs commands + through ``self.env.execute``, and only the local environment on a + Windows host speaks Git Bash with MSYS argument conversion + disabled and a native ``rg.exe`` — the one place the MSYS→native + path rewrite in :meth:`_escape_native_arg` is valid. Mirrors the + ``_lsp_local_only`` gate used for the LSP path. + """ + env = getattr(self, "env", None) + if env is None: + # Defensive: some tests construct ShellFileOperations via + # ``__new__`` without going through ``__init__``. + return False + try: + from tools.environments.local import LocalEnvironment, _IS_WINDOWS + except Exception: # noqa: BLE001 + return False + return isinstance(env, LocalEnvironment) and _IS_WINDOWS + def _atomic_write(self, path: str, content: str) -> "ExecuteResult": """Write ``content`` to ``path`` atomically via temp-file + rename. @@ -2272,15 +2318,27 @@ def _zero_match_probe(self, pattern: str, path: str, 13.9% of production content searches return zero matches and give the model nothing to steer by. Run ONE cheap case-insensitive count probe; if it hits, say so. If the pattern contains regex - metacharacters, also probe it as a fixed string. Bounded: two rg + metacharacters, also probe it as a fixed string. Bounded: two invocations max, count-only output. + + Uses whatever engine the main search would have used — rg when + available, grep otherwise — so the probe never silently dies in + an environment where the main search itself fell back to grep + (e.g. a test/sandbox PATH without rg on it). """ - if not self._has_command('rg'): + engine, count_flags = self._probe_engine() + if engine is None: return None - glob_expr = f" --glob {self._escape_shell_arg(file_glob)}" if file_glob else "" + glob_expr = "" + if file_glob: + glob_flag = "--glob" if engine == "rg" else "--include" + glob_expr = f" {glob_flag} {self._escape_shell_arg(file_glob)}" + # Native rg.exe cannot read the MSYS /c/... form when MSYS arg + # conversion is disabled; grep (an MSYS tool) needs that form. + path_arg = self._escape_native_arg(path) if engine == "rg" else self._escape_shell_arg(path) probe = self._exec( - f"rg -i --count-matches{glob_expr} " - f"{self._escape_shell_arg(pattern)} {self._escape_shell_arg(path)} " + f"{engine} {count_flags}{glob_expr} " + f"{self._escape_shell_arg(pattern)} {path_arg} " f"2>/dev/null | head -50", timeout=30, ) @@ -2299,10 +2357,12 @@ def _zero_match_probe(self, pattern: str, path: str, # Hidden/ignored probe: rg skips dotdirs and .gitignore'd files by # default. When the pattern exists only there, say so instead of # returning a bare zero (bench case: match in .hidden/ silently - # missing from results). + # missing from results). Under grep, the case probe carries + # --exclude-dir='.*' (mirroring the main search); this probe + # drops that exclusion so dot-directory matches are found. hidden = self._exec( - f"rg --hidden --no-ignore --count-matches{glob_expr} " - f"{self._escape_shell_arg(pattern)} {self._escape_shell_arg(path)} " + f"{engine} {self._probe_hidden_flags(engine)}{glob_expr} " + f"{self._escape_shell_arg(pattern)} {path_arg} " f"2>/dev/null | head -50", timeout=30, ) @@ -2321,8 +2381,8 @@ def _zero_match_probe(self, pattern: str, path: str, ) if re.search(r"[.\[\](){}?*+^$\\|]", pattern): fixed = self._exec( - f"rg -F --count-matches{glob_expr} " - f"{self._escape_shell_arg(pattern)} {self._escape_shell_arg(path)} " + f"{engine} -F {count_flags}{glob_expr} " + f"{self._escape_shell_arg(pattern)} {path_arg} " f"2>/dev/null | head -50", timeout=30, ) @@ -2339,6 +2399,39 @@ def _zero_match_probe(self, pattern: str, path: str, ) return None + def _probe_engine(self) -> "tuple[str | None, str]": + """Pick the search engine for zero-match probes. + + Returns (engine, count_flags): ``rg`` with ``-i --count-matches`` + when ripgrep is available; ``grep`` with ``-rniHc`` otherwise; + ``(None, "")`` when neither exists. Mirrors the main search's + engine precedence so the probe behaves identically to the search + that produced the zero. + """ + if self._has_command('rg'): + return "rg", "-i --count-matches" + if self._has_command('grep'): + # -c emits per-file counts (path:count), which the probe + # parser expects; -H forces the filename even for a + # single-file search. --exclude-dir mirrors the main grep + # search (and rg's default) so hidden files are probed by + # the dedicated hidden probe, not the case probe. + return "grep", "-rniHc --exclude-dir='.*'" + return None, "" + + def _probe_hidden_flags(self, engine: str) -> str: + """Flags for the hidden/ignored probe, per engine. + + rg: ``--hidden --no-ignore --count-matches`` overrides the + default dotdir and .gitignore exclusion. grep: the case probe + carries ``--exclude-dir='.*'`` (mirroring the main search), so + the hidden probe must drop that exclusion — ``-rniHc`` with no + exclude descends into dot-directories and finds the matches. + """ + if engine == "rg": + return "--hidden --no-ignore --count-matches" + return "-rniHc" + def _search_files(self, pattern: str, path: str, limit: int, offset: int) -> SearchResult: """Search for files by name pattern (glob-like).""" # Auto-prepend **/ for recursive search if not already present @@ -2442,9 +2535,11 @@ def _search_files_rg(self, pattern: str, path: str, limit: int, offset: int) -> fetch_limit = limit + offset # Try mtime-sorted first (rg 13+); fall back to unsorted if not supported. + # rg.exe is a native binary — pass the native drive form on Windows. + native_root = self._escape_native_arg(path) cmd_sorted = ( f"rg --files --sortr=modified -g {self._escape_shell_arg(glob_pattern)} " - f"{self._escape_shell_arg(path)} 2>/dev/null " + f"{native_root} 2>/dev/null " f"| head -n {fetch_limit}" ) result = self._exec(cmd_sorted, timeout=60) @@ -2455,7 +2550,7 @@ def _search_files_rg(self, pattern: str, path: str, limit: int, offset: int) -> # --sortr may have failed on older rg; retry without it. cmd_plain = ( f"rg --files -g {self._escape_shell_arg(glob_pattern)} " - f"{self._escape_shell_arg(path)} 2>/dev/null " + f"{native_root} 2>/dev/null " f"| head -n {fetch_limit}" ) result = self._exec(cmd_plain, timeout=60) @@ -2539,7 +2634,10 @@ def _search_with_rg(self, pattern: str, path: str, file_glob: Optional[str], # Add pattern and path cmd_parts.append(self._escape_shell_arg(pattern)) - cmd_parts.append(self._escape_shell_arg(path)) + # rg.exe on Windows is a native binary: it cannot read the MSYS + # /c/... form when MSYS argument conversion is disabled, so pass + # the native drive form (C:/...) for the search root. + cmd_parts.append(self._escape_native_arg(path)) # Fetch extra rows so we can report the true total before slicing. # For context mode, rg emits separator lines ("--") between groups,