fix: grep fallback in search_files — enable ERE and skip --exclude-dir on hidden roots - #48878
fix: grep fallback in search_files — enable ERE and skip --exclude-dir on hidden roots#48878luxles wants to merge 1 commit into
Conversation
alt-glitch
added
type/bug
|
Related: #18473 (hidden-root exclude-dir bug), #18645 and #34017 (open PRs fixing the same hidden-root --exclude-dir issue on a different code path scope). This PR fixes the same --exclude-dir-on-hidden-root problem AND additionally adds the ERE (-E) flag so alternation (|) works in the grep fallback — an orthogonal second bug the other PRs don't address. Verified both bugs are still live on main (_search_with_grep still uses "grep -rnH" with no -E and an unconditional --exclude-dir). Marked related rather than duplicate because of the added ERE layer. |
… search roots Two bugs in _search_with_grep() when ripgrep is unavailable: 1. BRE mode: grep -rnH uses Basic Regular Expression mode, where | is a literal character. ripgrep ships with Rust regex (| = alternation), so patterns with alternation silently fail on the grep fallback. Fix: add -E to enable ERE (Extended Regular Expression) mode. 2. --exclude-dir='.*': GNU grep applies this to all directory components INCLUDING command-line arguments, so a search under ~/.hermes/workspace skips the entire tree because .hermes matches .*. Ripgrep only applies globs to subdirectories, not the root. Fix: only add --exclude-dir when no path component is hidden, mirroring the logic already used by _search_files (merged in NousResearch#16672). Both bugs produce silent false-negatives: search_files returns total_count=0 without any error. Cross-reference NousResearch#18473, NousResearch#18645, NousResearch#34017. Fixes NousResearch#18473
ff71ec2 to
a42764c
Compare
tonydwb
left a comment
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Small, focused fix. The grep fallback now enables ERE (-E) and skips --exclude-dir on hidden root directories, which is the correct behavior for search_files. Well-scoped change, no issues.
Looks Good
- Minimal diff (8 lines, 1 file)
- Targeted fix addressing a specific edge case in grep fallback
- No test changes needed for this type of fix
Reviewed by Hermes Agent
|
PR has been approved by @tonydwb. I do not have write access to this repo — could a maintainer please squash-merge? Thanks! |
|
Thanks for identifying two real grep-fallback defects: current main still uses Problems
Suggested changes
Automated hermes-sweeper review. |
teknium1
added
sweeper:risk-security-boundary
alt-glitch
removed
the
sweeper:risk-security-boundary
4 participants
Summary
Fixes two bugs in
_search_with_grep()(tools/file_operations.py) that causesearch_filesto silently return zero results when ripgrep is unavailable.Bug 1: BRE mode
grep -rnHuses BRE mode where|is a literal character. ripgrep's Rust regex treats|as alternation. Fix: add-Efor ERE.Bug 2:
--exclude-dir='.*'skips hidden search rootsGNU grep applies
--exclude-dirto command-line directories too, so searching~/.hermesskips the entire root. Fix: only add--exclude-dirwhen basename doesn't start with..Verification
Before:
grep -rnH --exclude-dir='.*' --include config.yaml 'write_approval|guard_agent' ~/.hermes→ EXIT=1After:
grep -rnHE --include config.yaml 'write_approval|guard_agent' ~/.hermes→ config.yaml:387-388 matched, EXIT=0See commit message for full details.