From 092b355f90a5ee53a34948888ff6821226fa5c4a Mon Sep 17 00:00:00 2001 From: Royalaid <2439803+royalaid@users.noreply.github.com> Date: Fri, 28 Aug 2026 18:51:01 -0700 Subject: [PATCH] fix(search): scope macOS globs and mark bounded totals --- tests/tools/test_file_operations.py | 8 +- tests/tools/test_macos_protected_search.py | 68 +++++++++++++++++ .../test_search_files_engine_selection.py | 36 +++++++++ tools/file_operations.py | 75 +++++++++++++++---- 4 files changed, 173 insertions(+), 14 deletions(-) diff --git a/tests/tools/test_file_operations.py b/tests/tools/test_file_operations.py index 2dbdc9a2c5..88e839f280 100644 --- a/tests/tools/test_file_operations.py +++ b/tests/tools/test_file_operations.py @@ -156,10 +156,16 @@ class TestSearchResult: assert d["matches"][0]["path"] == "a.py" - def test_truncated_flag(self): + def test_truncated_flag_marks_total_as_lower_bound(self): r = SearchResult(total_count=100, truncated=True) d = r.to_dict() assert d["truncated"] is True + assert d["total_count_is_lower_bound"] is True + + def test_untruncated_total_omits_lower_bound_flag(self): + r = SearchResult(total_count=100) + d = r.to_dict() + assert "total_count_is_lower_bound" not in d class TestSearchResultDensify: diff --git a/tests/tools/test_macos_protected_search.py b/tests/tools/test_macos_protected_search.py index 779fa92ec3..c36ff854ae 100644 --- a/tests/tools/test_macos_protected_search.py +++ b/tests/tools/test_macos_protected_search.py @@ -265,6 +265,74 @@ def test_find_multi_root_keeps_explicit_protected_root_and_reports_actual_skips( assert "Downloads" not in protected_warning +def test_rg_multi_root_scopes_protected_globs_and_restores_absolute_paths(monkeypatch): + env = RecordingEnvironment("/") + ops = ShellFileOperations(env) + monkeypatch.setattr(file_operations, "_HOME", "/Users/alice") + monkeypatch.setattr(file_operations.sys, "platform", "darwin") + + def execute(command, cwd=None, **kwargs): + env.commands.append(command) + if command.startswith("test -e"): + output = "not_found\n" if "'/Users/alice /repo'" in command else "exists\n" + return {"output": output, "returncode": 0} + if command.startswith("command -v rg"): + return {"output": "/usr/bin/rg\n", "returncode": 0} + if "--version" in command: + return {"output": "ripgrep 14.1.1\n", "returncode": 0} + if "--files" in command: + return { + "output": "repo/Downloads/visible.txt\nUsers/alice/safe.txt\n", + "returncode": 0, + } + raise AssertionError(command) + + env.execute = execute + result = ops.search( + "*.txt", path="/Users/alice /repo", target="files", order="modified" + ) + + commands = _rg_files_commands(env.commands) + assert len(commands) == 1 + command = commands[0] + assert command.startswith("set -o pipefail; cd '/' && ") + assert "--sortr=modified" in command + assert "'!Users/alice/Downloads/**'" in command + assert "'!repo/Downloads/**'" not in command + assert "'Users/alice' 'repo'" in command + assert result.files == [ + "/repo/Downloads/visible.txt", + "/Users/alice/safe.txt", + ] + + +def test_rg_scoped_multi_root_handles_dot_spaces_and_overlapping_roots(monkeypatch): + env = RecordingEnvironment("/Users/alice/work space") + ops = ShellFileOperations(env) + monkeypatch.setattr(file_operations, "_HOME", "/Users/alice") + monkeypatch.setattr(file_operations.sys, "platform", "darwin") + + def execute(command, cwd=None, **kwargs): + env.commands.append(command) + if command.startswith("test -e"): + output = "not_found\n" if "'., /Users/alice'" in command else "exists\n" + return {"output": output, "returncode": 0} + if command.startswith("command -v rg"): + return {"output": "/usr/bin/rg\n", "returncode": 0} + if "--files" in command: + return {"output": "work space/local.txt\n", "returncode": 0} + raise AssertionError(command) + + env.execute = execute + result = ops.search("*.txt", path="., /Users/alice", target="files") + + command = _rg_files_commands(env.commands)[0] + assert "cd '/Users/alice' &&" in command + assert "'work space' '.'" in command + assert "'!Downloads/**'" in command + assert result.files == ["/Users/alice/work space/local.txt"] + + def test_real_ripgrep_does_not_descend_into_protected_folder(tmp_path, monkeypatch): home = tmp_path / "Users" / "alice" safe = home / "safe" diff --git a/tests/tools/test_search_files_engine_selection.py b/tests/tools/test_search_files_engine_selection.py index 317f290ee2..72e9100f84 100644 --- a/tests/tools/test_search_files_engine_selection.py +++ b/tests/tools/test_search_files_engine_selection.py @@ -58,6 +58,42 @@ def test_default_file_search_runs_one_bounded_unsorted_rg_command(): assert "head -n 3" in env.rg_commands[0] +@pytest.mark.parametrize("engine", ["rg", "find"]) +def test_bounded_filename_total_is_serialized_as_a_lower_bound(engine, monkeypatch): + conceptual_files = [f"/repo/file-{index:03}.py" for index in range(200)] + env = RecordingEnvironment() + + def execute(command, **kwargs): + env.commands.append(command) + if command.startswith("test -e "): + return {"output": "exists\n", "returncode": 0} + if command.startswith("command -v rg"): + return { + "output": "/usr/bin/rg\n" if engine == "rg" else "", + "returncode": 0 if engine == "rg" else 1, + } + if "--files" in command or command.startswith("set -o pipefail; find "): + fetch_limit = int(re.search(r"head -n (\d+)", command).group(1)) + return { + "output": "\n".join(conceptual_files[:fetch_limit]) + "\n", + "returncode": 0, + } + return {"output": "", "returncode": 1} + + env.execute = execute + ops = ShellFileOperations(env) + if engine == "find": + monkeypatch.setattr(ops, "_has_command", lambda command: command == "find") + + result = ops.search("*.py", path="/repo", target="files", limit=50) + serialized = result.to_dict() + + assert result.total_count == 51 + assert len(result.files) == 50 + assert serialized["truncated"] is True + assert serialized["total_count_is_lower_bound"] is True + + def test_modified_file_search_runs_one_exact_order_rg_command(): env = RecordingEnvironment() ops = ShellFileOperations(env) diff --git a/tools/file_operations.py b/tools/file_operations.py index 56956e6d9a..1d9f8bea18 100644 --- a/tools/file_operations.py +++ b/tools/file_operations.py @@ -28,6 +28,7 @@ Usage: import base64 import binascii import os +import posixpath import re import secrets import sys @@ -372,6 +373,7 @@ class SearchResult: result["counts"] = self.counts if self.truncated: result["truncated"] = True + result["total_count_is_lower_bound"] = True if self.limit_reason: result["limit_reason"] = self.limit_reason if self.warning: @@ -3474,16 +3476,31 @@ class ShellFileOperations(FileOperations): self, roots: List[str] ) -> List[tuple[str, str, str]]: """Return unique exclusions without pruning an explicitly chosen root.""" - explicit_roots = { - os.path.normcase(os.path.abspath(os.path.normpath(root))) + cwd = getattr(self.env, "cwd", None) or self.cwd + use_posix_paths = sys.platform == "darwin" and all( + not re.match(r"^[A-Za-z]:[\\/]", root) and "\\" not in root for root in roots - } + ) + + def normalized(root: str) -> str: + if use_posix_paths: + if not posixpath.isabs(root): + root = posixpath.join(cwd, root) + return posixpath.normpath(root) + return os.path.normcase(os.path.abspath(os.path.normpath(root))) + + normalized_roots = [normalized(root) for root in roots] + explicit_roots = set(normalized_roots) seen = set() effective = [] - for root in roots: + for root, normalized_root in zip(roots, normalized_roots): for relative in self._macos_search_exclusions(root): - absolute = os.path.normpath(os.path.join(root, relative)) - key = os.path.normcase(os.path.abspath(absolute)) + if use_posix_paths: + absolute = posixpath.normpath(posixpath.join(normalized_root, relative)) + key = absolute + else: + absolute = os.path.normpath(os.path.join(root, relative)) + key = os.path.normcase(os.path.abspath(absolute)) if key in explicit_roots or key in seen: continue seen.add(key) @@ -3809,11 +3826,34 @@ class ShellFileOperations(FileOperations): roots = [path] if isinstance(path, str) else path fetch_limit = limit + offset + 1 - exclusion_terms = [ - f"--glob {self._escape_shell_arg(f'!{relative}/**')}" - for _root, relative, _absolute - in self._effective_macos_search_exclusions(roots) - ] + effective_exclusions = self._effective_macos_search_exclusions(roots) + scoped_common = None + command_roots = roots + use_posix_paths = sys.platform == "darwin" and all( + not re.match(r"^[A-Za-z]:[\\/]", root) and "\\" not in root + for root in roots + ) + if len(roots) > 1 and effective_exclusions and use_posix_paths: + cwd = getattr(self.env, "cwd", None) or self.cwd + absolute_roots = [ + posixpath.normpath( + root if posixpath.isabs(root) else posixpath.join(cwd, root) + ) + for root in roots + ] + scoped_common = posixpath.commonpath(absolute_roots) + command_roots = [ + posixpath.relpath(root, scoped_common) for root in absolute_roots + ] + exclusion_terms = [ + f"--glob {self._escape_shell_arg(f'!{posixpath.relpath(absolute, scoped_common)}/**')}" + for _root, _relative, absolute in effective_exclusions + ] + else: + exclusion_terms = [ + f"--glob {self._escape_shell_arg(f'!{relative}/**')}" + for _root, relative, _absolute in effective_exclusions + ] exclusion_globs = " ".join(dict.fromkeys(exclusion_terms)) exclusion_args = f" {exclusion_globs}" if exclusion_globs else "" rg_executable = rg_executable or self._resolve_command("rg") @@ -3825,14 +3865,23 @@ class ShellFileOperations(FileOperations): return SearchResult(error=capability_error) rg = self._quote_executable(rg_executable) sort_arg = " --sortr=modified" if order == "modified" else "" - root_args = " ".join(self._escape_native_tool_arg(root) for root in roots) + root_args = " ".join(self._escape_native_tool_arg(root) for root in command_roots) + cd_prefix = ( + f"cd {self._escape_shell_arg(scoped_common)} && " if scoped_common else "" + ) cmd = ( - f"set -o pipefail; {rg} --files{sort_arg} -g {self._escape_shell_arg(glob_pattern)}" + f"set -o pipefail; {cd_prefix}{rg} --files{sort_arg} -g {self._escape_shell_arg(glob_pattern)}" f"{exclusion_args} {root_args} 2>/dev/null | head -n {fetch_limit}" ) result = self._exec(cmd, timeout=60) stdout, limit_reason = _search_stdout_and_limit(result) all_files = [f for f in stdout.splitlines() if f] + if scoped_common: + all_files = [ + file_path if posixpath.isabs(file_path) + else posixpath.normpath(posixpath.join(scoped_common, file_path)) + for file_path in all_files + ] bounded_sigpipe = result.exit_code == 141 and len(all_files) >= fetch_limit if result.exit_code not in {0, 1, 124} and not bounded_sigpipe: