mirror of
https://github.com/langchain-ai/deepagents.git
synced 2026-07-22 01:35:28 -04:00
feat(sdk): log when grep falls back from ripgrep (#3593)
FilesystemBackend.grep silently degraded to its slow Python fallback whenever ripgrep wasn't on PATH, timed out, or errored, leaving operators unable to diagnose why searches were slow or returning suspicious zero-match results. The change adds targeted logs for each failure mode and stops treating ripgrep hard errors (rc ≥ 2) as "no matches".
This commit is contained in:
Mason Daugherty
@@ -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 @@ from deepagents.backends.utils import (
|
||||
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.
|
||||
|
||||
@@ -556,7 +578,7 @@ class FilesystemBackend(BackendProtocol):
|
||||
matches.append({"path": fpath, "line": int(line_num), "text": line_text})
|
||||
return GrepResult(error=partial_error, 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:
|
||||
@@ -570,7 +592,11 @@ class FilesystemBackend(BackendProtocol):
|
||||
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
|
||||
@@ -596,7 +622,26 @@ class FilesystemBackend(BackendProtocol):
|
||||
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]]] = {}
|
||||
@@ -606,7 +651,14 @@ class FilesystemBackend(BackendProtocol):
|
||||
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")
|
||||
|
||||
@@ -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.tools import ToolRuntime
|
||||
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)
|
||||
|
||||
|
||||
def _install_flaky_rglob(monkeypatch: pytest.MonkeyPatch, exc: Exception, after_yields: int = 1) -> None:
|
||||
"""Replace `Path.rglob` with a generator that yields N entries then raises."""
|
||||
real_rglob = Path.rglob
|
||||
|
||||
Reference in New Issue
Block a user