Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 146 additions & 0 deletions tests/tools/test_search_zero_match_and_multipath.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'}"
Expand Down
124 changes: 111 additions & 13 deletions tools/file_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

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.

Expand Down Expand Up @@ -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,
)
Expand All @@ -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,
)
Expand All @@ -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,
)
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand Down
Loading