Skip to content

fix(search): keep grep fallback root searchable - #83848

Closed
wanquanY wants to merge 1 commit into
NousResearch:mainfrom
wanquanY:codex/fix-grep-fallback-root-exclusion
Closed

fix(search): keep grep fallback root searchable#83848
wanquanY wants to merge 1 commit into
NousResearch:mainfrom
wanquanY:codex/fix-grep-fallback-root-exclusion

Conversation

@wanquanY

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes the search_files grep fallback silently returning zero matches for its default path="." search.

The fallback intentionally passes --exclude-dir='.*' to match ripgrep's hidden-directory behavior. grep also applies that glob to the command-line search root, however, so a relative root such as . (or ./directory) excludes itself before traversal begins.

This change anchors relative grep roots at the terminal backend's live, shell-quoted $PWD. It preserves the existing hidden-directory exclusion, avoids resolving remote/container paths on the host, and keeps the user-provided path component shell-escaped.

Related Issue

N/A — searched open and closed issues/PRs for the error and --exclude-dir behavior; no duplicate found.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • Anchor relative roots passed by ShellFileOperations._search_with_grep() at the backend's live $PWD.
  • Replace false-green handcrafted grep tests with production-API tests that force the real grep fallback.
  • Cover the default ., an explicit ./directory, the .hub prompt-injection cache, and an arbitrary hidden directory.

How to Test

  1. Run python -m pytest tests/tools/test_search_hidden_dirs.py tests/tools/test_search_error_guard.py tests/tools/test_file_operations.py -q (80 passed, 2 skipped on macOS 26.2).
  2. Run the search-focused cross-suite selection (39 passed, 113 deselected).
  3. Run python -m ruff check tools/file_operations.py tests/tools/test_search_hidden_dirs.py and git diff --check.
  4. On Debian 12 with GNU grep, the old relative-root command returns exit 1 with zero lines; the anchored-root command returns the visible match and still excludes both named and arbitrary dot-directories.

The canonical full macOS run reached 29,484 passed / 306 skipped, with 22 unrelated platform/environment failures and one isolated test-file timeout; every touched and search-related suite passed. The PR's Ubuntu CI is the authoritative full-suite gate.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass (see macOS full-run note above; scoped suites are green)
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.2 (BSD grep) and Debian 12 arm64 container (GNU grep)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A; behavior is internal and covered by an inline rationale
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A; public behavior is restored, not changed

Screenshots / Logs

# Regression mutation (old implementation)
1 failed: expected a visible match, got total_count == 0

# Fixed implementation
80 passed, 2 skipped

# Debian 12 / GNU grep
old_exit=1 old_lines=0 new_exit=0 new_lines=1
/work/my-skill/SKILL.md:2:This is a real skill.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists tool/file File tools (read, write, patch, search) labels Aug 11, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #48878 and #34017 address the same grep fallback family via conditional hidden-root handling. This PR instead anchors relative roots at the backend PWD while preserving hidden-descendant exclusion.

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Salvaged via #84780 — your commit was cherry-picked onto current main with your authorship preserved, and merges as soon as checks complete (auto-merge armed, rebase).

Review notes from the salvage: your "$PWD" anchoring was selected over the competing #81205 (cd && pwd -P subprocess approach) — zero extra exec per search, resolution happens in the sandbox shell itself, no rg result-shape change, and the production-API test rewrite is a strict fidelity upgrade over the old handcrafted-grep tests. Verified with a live E2E matrix (./sub/./sub/absolute/space+quote/injection-probe roots) plus a mutation check (reverting the prod file makes your new tests fail).

Thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Medium — degraded but workaround exists tool/file File tools (read, write, patch, search) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants