Skip to content

fix(grep): match every repeated -e pattern - #447

Open
breken-ai wants to merge 2 commits into
activeloopai:mainfrom
breken-ai:fix/grep-repeated-patterns
Open

breken-ai wants to merge 2 commits into
activeloopai:mainfrom
breken-ai:fix/grep-repeated-patterns

Conversation

@breken-ai

@breken-ai breken-ai commented Sep 29, 2026 •

Copy link
Copy Markdown

Summary

grep -e apple -e banana /notes should print lines that match either pattern. The Claude Code grep fast path only prints lines matching the first one.

parseBashGrep (src/hooks/grep-direct.ts) collects every -e / --regexp value into explicitPatterns, but then passes only explicitPatterns[0] on to handleGrepDirect. Every later expression is silently dropped.

Reproduction on main (e054fb16), through the same parseBashGrep → handleGrepDirect path the pre-tool-use hook uses (lexical mode):

stored: /notes/a.md = "apple pie\nbanana bread"
grep -e apple -e banana /notes
  before: "apple pie"
  after:  "apple pie\nbanana bread"

stored: /notes/a.md = "a.b\naxb\nbanana"
grep -F -e a.b -e banana /notes
  before: "a.b"
  after:  "a.b\nbanana"

Fix:

  • When there is more than one explicit pattern, parseBashGrep joins them into one alternation (apple|banana). With -F, each expression is regex-escaped on its own first, so a.b still only matches a literal dot.
  • extractRegexAlternationPrefilters (src/shell/grep-core.ts) used to drop alternation branches that had no safe literal anchor and keep the rest. For an OR, that narrows the SQL prefilter: apple|\d+ became ILIKE '%apple%' and lost every row that only matched \d+. It now returns null (no content prefilter) when any branch has no anchor. When every branch has one, all of them still reach the memory and session prefilters.

A single -e or a positional pattern takes the same path as before.

Version Bump

Not bumped. This is a patch-level bug fix, so I've left the release decision to you.

Test plan

  • Tests pass locally (npm test, after npm run build): 5832 passed. The 17 failures are the same on the untouched main checkout and come from the environment: cowork-queue-leak, install-cowork, skillify-state, plugin-cache-gc-bundle, and two graph suites.
  • Relevant new tests added: parseBashGrep: repeated patterns (-e apple -e banana returns both lines and puts both literals in the memory and session SQL prefilters; -F -e a.b -e banana stays literal) and returns null when any branch has no safe literal anchor. All three fail on main (expected 'apple pie' to be 'apple pie\nbanana bread', expected 'a.b' to be 'a.b\nbanana', expected [ 'apple' ] to be null) and pass on this branch. grep-core + grep-direct: 204/204.
  • npm run typecheck, npm run dup and git diff --check pass.
  • Version bumped in package.json, or no release needed for this change

I didn't find an existing issue or PR for this. #446 also touches grep-direct.ts, but it changes semantic-mode eligibility (patternIsSemanticFriendly), not pattern parsing or the lexical prefilter.

An AI agent (breken-ai) found this and wrote the fix and tests. I reviewed and ran everything above before opening the PR.

Summary by CodeRabbit

  • New Features

    • Grep now supports multiple -e patterns, matching lines that contain any of the specified expressions.
    • Repeated fixed-string patterns remain literal, including characters that would otherwise have special meaning in regular expressions.
  • Bug Fixes

    • Grep results are no longer incorrectly filtered when a regular-expression alternative lacks a safe literal match.

@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: activeloopai/hivemind/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 1124b897-83eb-4a71-8f44-9304ee21661c

📥 Commits

Reviewing files that changed from the base of the PR and between e054fb1 and 40197e1.

📒 Files selected for processing (4)
  • src/hooks/grep-direct.ts
  • src/shell/grep-core.ts
  • tests/claude-code/grep-core.test.ts
  • tests/claude-code/grep-direct.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

Repeated -e expressions are combined into regex alternatives, including escaped alternatives for repeated fixed-string expressions. Alternation prefilters now return null when any branch lacks a safe literal anchor.

Changes

Grep pattern handling

Layer / File(s) Summary
Combine repeated grep expressions
src/hooks/grep-direct.ts, tests/claude-code/grep-direct.test.ts
parseBashGrep combines multiple explicit patterns as alternatives. In fixed-string mode, it escapes regex metacharacters before combining them. Tests cover matching either expression, SQL generation for both expressions, and literal matching for repeated -F expressions.
Require safe anchors for alternations
src/shell/grep-core.ts, tests/claude-code/grep-core.test.ts
extractRegexAlternationPrefilters returns null if any branch lacks a safe literal anchor. The test checks this behavior for `apple

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: efenocchi

Merge Risk: ⚪ Minimal · up to 40197

Repeated -e patterns in grep now match every supplied pattern, and prefiltering no longer drops rows that match unanchored branches. No merge-blocking risk was identified.

Architecture Summary

Architecture risk: 🔵 Low · up to 40197

The change affects 2 systems.

Changed systems: src, tests

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — src (service) was modified; 2 changed files map to changed impact.
  • observed — tests (service) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in src/hooks/grep-direct.ts: parseBashGrep now uses a mutable pattern and, when multiple explicit patterns are supplied, joins them as regex alternatives. In fixed-string mode, it first escapes regex metacharacters in each pattern; it then disables fixed-string mode for the combined pattern.
  • observed — Modified behavior in src/shell/grep-core.ts: extractRegexAlternationPrefilters now returns null if any alternation branch has no safe literal anchor. Previously, it retained only branches with anchors, so the resulting prefilter could exclude matches from unanchored branches; when all branches are anchored, it still deduplicates the anchors.
  • observed — Modified behavior in tests/claude-code/grep-core.test.ts: Added a test expecting extractRegexAlternationPrefilters("apple|\\d+") to return null when an alternation branch has no safe literal anchor.
  • observed — Modified behavior in tests/claude-code/grep-direct.test.ts: Adds a test asserting that repeated -e expressions match lines containing either pattern and that SQL includes both patterns for summary and message fields.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main fix: repeated -e patterns now match correctly.
Description check ✅ Passed The description includes the required Summary, Version Bump, and Test plan sections. It explains the bug, implementation, tests, and known environment failures. The version-bump decision remains unche…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant