From 67f725f8e5ccd5767436a3c3e272d2558f394d2e Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Tue, 26 May 2026 13:20:02 -0400 Subject: [PATCH] feat(sdk): log when grep falls back from ripgrep --- .../deepagents/backends/filesystem.py | 60 ++++++- .../backends/test_filesystem_backend.py | 149 ++++++++++++++++++ 2 files changed, 205 insertions(+), 4 deletions(-) diff --git a/libs/deepagents/deepagents/backends/filesystem.py b/libs/deepagents/deepagents/backends/filesystem.py index 8a7311478d..945f7a3720 100644 --- a/libs/deepagents/deepagents/backends/filesystem.py +++ b/libs/deepagents/deepagents/backends/filesystem.py @@ -2,10 +2,12 @@ import base64 import errno +import functools import json import logging import os import re +import shutil import subprocess from datetime import datetime from pathlib import Path @@ -41,6 +43,26 @@ logger = logging.getLogger(__name__) +@functools.cache +def _resolve_ripgrep_path() -> str | None: + """Locate the `rg` executable on `PATH`, cached for the process lifetime. + + Logs an `INFO`-level message exactly once if ripgrep is not found so + operators can diagnose silent slow-path searches when `rg` is installed + but not visible on the agent's `PATH` (common in sandboxed or + stripped-environment launchers). + + Returns: + Absolute path to `rg`, or `None` if not on `PATH`. + """ + path = shutil.which("rg") + if path is None: + logger.info( + "ripgrep ('rg') not found on PATH; using Python grep fallback. Install ripgrep for faster searches and automatic .gitignore handling." + ) + return path + + class FilesystemBackend(BackendProtocol): """Backend that reads and writes files directly from the filesystem. @@ -555,7 +577,7 @@ def grep( matches.append({"path": fpath, "line": int(line_num), "text": line_text}) return GrepResult(matches=matches) - def _ripgrep_search(self, pattern: str, base_full: Path, include_glob: str | None) -> dict[str, list[tuple[int, str]]] | None: # noqa: C901, PLR0912 # C901: split except clauses for per-clause logging; PLR0912: dir/file cwd branch + containment check + def _ripgrep_search(self, pattern: str, base_full: Path, include_glob: str | None) -> dict[str, list[tuple[int, str]]] | None: # noqa: C901, PLR0912, PLR0915 # except clauses split per-exception for targeted logging (timeout vs exec-race vs ripgrep hard-error) """Search using ripgrep with fixed-string (literal) mode. Args: @@ -569,7 +591,11 @@ def _ripgrep_search(self, pattern: str, base_full: Path, include_glob: str | Non Results whose resolved path lies outside `base_full` are silently filtered regardless of `virtual_mode`. """ - cmd = ["rg", "--json", "-F"] # -F enables fixed-string (literal) mode + rg_path = _resolve_ripgrep_path() + if rg_path is None: + return None + + cmd = [rg_path, "--json", "-F"] # -F enables fixed-string (literal) mode if include_glob: cmd.extend(["--glob", include_glob]) # When rg is given an absolute search path, directory-component @@ -595,7 +621,26 @@ def _ripgrep_search(self, pattern: str, base_full: Path, include_glob: str | Non check=False, cwd=rg_cwd, ) - except (subprocess.TimeoutExpired, FileNotFoundError, PermissionError, NotADirectoryError): + except subprocess.TimeoutExpired: + logger.warning("ripgrep timed out after 30s; using Python grep fallback") + return None + except (FileNotFoundError, PermissionError, NotADirectoryError) as e: + # `rg` resolved at cache time but failed at exec — treat as a + # runtime anomaly (uninstall, permission change, or `which`-vs-exec + # race) rather than a missing-tool config, hence WARNING instead + # of the INFO emitted by `_resolve_ripgrep_path`. Drop the cache + # so the next call re-probes `PATH`. + logger.warning("ripgrep subprocess failed (%s: %s); using Python grep fallback", type(e).__name__, e) + _resolve_ripgrep_path.cache_clear() + return None + + # Ripgrep exits 0 on match, 1 on no-match (both expected), 2+ on a hard + # error (invalid pattern, unreadable directory, malformed glob, etc.). + # Silently parsing stdout on a hard error reports zero matches to the + # agent — exactly the silent failure this resolver is meant to avoid. + if proc.returncode not in (0, 1): + stderr = proc.stderr.strip()[:500] if proc.stderr else "" + logger.warning("ripgrep exited %d (stderr=%r); using Python grep fallback", proc.returncode, stderr) return None results: dict[str, list[tuple[int, str]]] = {} @@ -605,7 +650,14 @@ def _ripgrep_search(self, pattern: str, base_full: Path, include_glob: str | Non data = json.loads(line) except json.JSONDecodeError: continue - if data.get("type") != "match": + data_type = data.get("type") + if data_type == "error": + # Per-file errors in `--json` mode (e.g., non-UTF-8 file + # ripgrep refused to read). Surface at DEBUG so debugging is + # possible without spamming WARNING for every binary file. + logger.debug("ripgrep per-file error frame: %s", data.get("data")) + continue + if data_type != "match": continue pdata = data.get("data", {}) ftext = pdata.get("path", {}).get("text") diff --git a/libs/deepagents/tests/unit_tests/backends/test_filesystem_backend.py b/libs/deepagents/tests/unit_tests/backends/test_filesystem_backend.py index b48202e088..5f7b24f269 100644 --- a/libs/deepagents/tests/unit_tests/backends/test_filesystem_backend.py +++ b/libs/deepagents/tests/unit_tests/backends/test_filesystem_backend.py @@ -1,5 +1,8 @@ +import logging import shutil +import subprocess import warnings +from collections.abc import Iterator from pathlib import Path import pytest @@ -7,6 +10,7 @@ from langchain_core.messages import ToolMessage from deepagents._api.deprecation import LangChainDeprecationWarning +from deepagents.backends import filesystem as fs_module from deepagents.backends.filesystem import FilesystemBackend from deepagents.backends.protocol import EditResult, ReadResult, WriteResult from deepagents.middleware.filesystem import FilesystemMiddleware @@ -699,6 +703,151 @@ def test_grep_containment_check_blocks_escaping_symlink(tmp_path: Path, monkeypa assert not any("secret" in p for p in matched), f"containment check failed — escaping symlink leaked result: {matched}" +_RG_MISSING_PREFIX = "ripgrep ('rg') not found on PATH" + + +@pytest.fixture +def _isolate_rg_cache() -> Iterator[None]: + """Clear the process-wide `rg` resolver cache around each test that touches it.""" + fs_module._resolve_ripgrep_path.cache_clear() + yield + fs_module._resolve_ripgrep_path.cache_clear() + + +class _FakeProc: + def __init__(self, stdout: str = "", stderr: str = "", returncode: int = 0) -> None: + self.stdout = stdout + self.stderr = stderr + self.returncode = returncode + + +@pytest.mark.usefixtures("_isolate_rg_cache") +def test_resolve_ripgrep_logs_once_when_missing(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture) -> None: + """Missing `rg` should log an `INFO` message exactly once per process. + + Operators investigating slow searches need at least one signal that the + Python fallback is in play. + """ + monkeypatch.setattr(fs_module.shutil, "which", lambda _name: None) + + (tmp_path / "a.txt").write_text("hello\n") + be = FilesystemBackend(root_dir=str(tmp_path), virtual_mode=False) + + with caplog.at_level(logging.INFO, logger=fs_module.logger.name): + be.grep("hello", path=str(tmp_path)) + be.grep("hello", path=str(tmp_path)) # second call must not re-log + + matching = [r for r in caplog.records if r.levelno == logging.INFO and r.getMessage().startswith(_RG_MISSING_PREFIX)] + assert len(matching) == 1, f"expected exactly one INFO log, got {len(matching)}: {[r.getMessage() for r in matching]}" + + +@pytest.mark.usefixtures("_isolate_rg_cache") +def test_resolve_ripgrep_uses_resolved_path_in_argv(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """The cached absolute path is exec'd verbatim, alongside the expected flags.""" + fake_rg = "/opt/fake/bin/rg" + monkeypatch.setattr(fs_module.shutil, "which", lambda _name: fake_rg) + + captured: dict[str, list[str]] = {} + + def fake_run(cmd: list[str], **_kwargs: object) -> _FakeProc: + captured["cmd"] = cmd + return _FakeProc() + + monkeypatch.setattr(fs_module.subprocess, "run", fake_run) + + (tmp_path / "a.txt").write_text("hello\n") + be = FilesystemBackend(root_dir=str(tmp_path), virtual_mode=False) + be.grep("hello", path=str(tmp_path)) + + cmd = captured["cmd"] + assert cmd[0] == fake_rg + assert "--json" in cmd + assert "-F" in cmd + assert "hello" in cmd # pattern made it through + + +@pytest.mark.usefixtures("_isolate_rg_cache") +def test_ripgrep_timeout_logs_warning(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture) -> None: + """A `TimeoutExpired` from `subprocess.run` should emit a `WARNING`.""" + monkeypatch.setattr(fs_module.shutil, "which", lambda _name: "/usr/bin/rg") + + def timeout_run(cmd: list[str], **_kwargs: object) -> object: + raise subprocess.TimeoutExpired(cmd, timeout=30) + + monkeypatch.setattr(fs_module.subprocess, "run", timeout_run) + + (tmp_path / "a.txt").write_text("hello\n") + be = FilesystemBackend(root_dir=str(tmp_path), virtual_mode=False) + + with caplog.at_level(logging.WARNING, logger=fs_module.logger.name): + result = be.grep("hello", path=str(tmp_path)) + + assert any("timed out" in r.getMessage() for r in caplog.records), [r.getMessage() for r in caplog.records] + # Python fallback still ran, so the actual match should come through. + assert result.matches and any(m["path"].endswith("a.txt") for m in result.matches) + + +@pytest.mark.usefixtures("_isolate_rg_cache") +def test_ripgrep_exec_race_logs_warning_and_clears_cache(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture) -> None: + """`FileNotFoundError` at exec (post-`which` race) warns with detail and re-probes next call.""" + which_calls = {"n": 0} + + def counting_which(_name: str) -> str | None: + which_calls["n"] += 1 + return "/usr/bin/rg" + + monkeypatch.setattr(fs_module.shutil, "which", counting_which) + + def missing_run(cmd: list[str], **_kwargs: object) -> object: + raise FileNotFoundError(2, "No such file or directory", cmd[0]) + + monkeypatch.setattr(fs_module.subprocess, "run", missing_run) + + (tmp_path / "a.txt").write_text("hello\n") + be = FilesystemBackend(root_dir=str(tmp_path), virtual_mode=False) + + with caplog.at_level(logging.WARNING, logger=fs_module.logger.name): + be.grep("hello", path=str(tmp_path)) + be.grep("hello", path=str(tmp_path)) + + failure_msgs = [r.getMessage() for r in caplog.records if "ripgrep subprocess failed" in r.getMessage()] + assert failure_msgs + assert "FileNotFoundError" in failure_msgs[0] + assert "No such file or directory" in failure_msgs[0] # str(e) carried through + assert which_calls["n"] >= 2, "cache should have been cleared so `which` re-runs" + + +@pytest.mark.usefixtures("_isolate_rg_cache") +def test_ripgrep_nonzero_returncode_falls_back_with_warning( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """A hard ripgrep error (rc=2) must not be silently parsed as 'no matches'. + + Without this guard, malformed globs or unreadable directories produce an + empty result set and the agent confidently reports no matches. + """ + monkeypatch.setattr(fs_module.shutil, "which", lambda _name: "/usr/bin/rg") + + def erroring_run(_cmd: list[str], **_kwargs: object) -> _FakeProc: + return _FakeProc(stdout="", stderr="rg: error parsing glob 'docs/[': unclosed character class", returncode=2) + + monkeypatch.setattr(fs_module.subprocess, "run", erroring_run) + + (tmp_path / "a.txt").write_text("hello\n") + be = FilesystemBackend(root_dir=str(tmp_path), virtual_mode=False) + + with caplog.at_level(logging.WARNING, logger=fs_module.logger.name): + result = be.grep("hello", path=str(tmp_path)) + + msgs = [r.getMessage() for r in caplog.records if "ripgrep exited 2" in r.getMessage()] + assert msgs, [r.getMessage() for r in caplog.records] + assert "error parsing glob" in msgs[0] + # Python fallback ran and still returned the real match. + assert result.matches and any(m["path"].endswith("a.txt") for m in result.matches) + + class TestToVirtualPath: """Tests for FilesystemBackend._to_virtual_path."""