Skip to content

fix(rewrite): skip recursive grep rewrite to preserve .gitignore handling (fixes #4477) - #4487

Closed
TechWizard9999 wants to merge 15 commits into
rtk-ai:developfrom
TechWizard9999:fix/4477-recursive-grep-rewrite
Closed

TechWizard9999 wants to merge 15 commits into
rtk-ai:developfrom
TechWizard9999:fix/4477-recursive-grep-rewrite

Conversation

@TechWizard9999

Copy link
Copy Markdown
Contributor

Fixes #4477.

Problem: The Claude hook rewrites grep -r (recursive grep) to rtk grep -r, but rtk grep runs OS grep verbatim with no .gitignore handling. This removes the .gitignore protection that the agent's shell already provides (Claude Code's grep is a ugrep shell function with --ignore-files). Result: massive scans of node_modules, .venv, .claude/worktrees/, running 9-12 minutes per call.

Solution: Skip rewrite for recursive grep invocations. Let the shell's own grep handle them (which honors .gitignore). Non-recursive grep still gets rewritten to rtk grep for output compaction.

Changes in src/discover/registry.rs:

  • Added is_recursive_grep() helper that detects recursive flags using shell_split (the rewrite path's tokenizer): handles -r, -R, --recursive, --dereference-recursive, -d recurse, -drecurse, --directories=recurse, and combined short flags like -rn, -Rn, -rA.
  • Guard in rewrite_segment_inner: if rtk_equivalent == "rtk grep" && is_recursive_grep(cmd_part) → return None (passthrough).
  • Added 3 regression tests:
    1. is_recursive_grep_detects_all_spellings - unit tests for the helper
    2. recursive_grep_not_rewritten - recursive variants not rewritten
    3. non_recursive_grep_still_rewritten - non-recursive grep still gets rtk grep

Exit codes unchanged. The fix is in the rewrite path only; rtk grep itself is not modified.

@KuSh — ready for review.

pszymkowiak and others added 14 commits March 16, 2026 14:58
* fix: P1 exit codes, grep regex perf, SQLite concurrency

Exit code propagation (same pattern as existing modules):
- wget_cmd: run() and run_stdout() now exit on failure
- container: docker_logs, kubectl_pods/services/logs now check
  status before parsing JSON (was showing "No pods found" on error)
- pnpm_cmd: replace bail!() with eprint + process::exit in
  run_list and run_install

Performance:
- grep_cmd: compile context regex once before loop instead of
  per-line in clean_line() (was N compilations per grep call)

Data integrity:
- tracking: add PRAGMA journal_mode=WAL and busy_timeout=5000
  to prevent SQLite corruption with concurrent Claude Code instances

Signed-off-by: Patrick <patrick@rtk.ai>
Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>

* fix: address review findings on P1 fixes

- tracking: WAL pragma non-fatal (NFS/read-only compat)
- wget: forward raw stderr on failure, track raw==raw (no fake savings)
- container: remove stderr shadow in docker_logs, add empty-stderr
  guard on all 4 new exit code paths for consistency with prisma pattern

Signed-off-by: Patrick <patrick@rtk.ai>
Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>

---------

Signed-off-by: Patrick <patrick@rtk.ai>
Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>
… (rtk-ai#630)

* fix: raise output caps for grep, git status, and parser fallback (rtk-ai#617, rtk-ai#618, rtk-ai#620)

- grep: per-file match cap 10 → 25, global max 50 → 200
- git status: file list caps 5/5/3 → 15/15/10
- parser fallback: truncate 500 → 2000 chars across all modules

These P0 bugs caused LLM retry loops when RTK returned less signal
than the raw command, making RTK worse than not using it.

Fixes rtk-ai#617, rtk-ai#618, rtk-ai#620

Signed-off-by: Patrick <patrick@rtk.ai>
Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>

* fix: update README example and add truncation tests for modified/untracked

- parser/README.md: update example from 500 → 2000 to match code
- git.rs: add test_format_status_modified_truncation (cap 15)
- git.rs: add test_format_status_untracked_truncation (cap 10)

Signed-off-by: Patrick <patrick@rtk.ai>
Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>

* refactor: extract output caps into [limits] config section

Move hardcoded caps into config.toml so users can tune them:

  [limits]
  grep_max_results = 200      # global grep match limit
  grep_max_per_file = 25      # per-file match limit
  status_max_files = 15       # staged/modified file list cap
  status_max_untracked = 10   # untracked file list cap
  passthrough_max_chars = 2000 # parser fallback truncation

All 8 modules now read from config::limits() instead of hardcoded
values. Defaults unchanged from previous commit.

Signed-off-by: Patrick <patrick@rtk.ai>
Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>

---------

Signed-off-by: Patrick <patrick@rtk.ai>
Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>
…es (rtk-ai#662)

* feat(.claude): add /rtk-triage skill — orchestrated PR+issue cross-analysis

New skill that runs issue-triage + pr-triage in parallel then produces
a cross-analysis layer that neither skill can do individually:

- Double coverage detection: identifies when 2+ PRs target the same issue
  (via body scan + file overlap), recommends which to keep/close
- Security gap detection: for security review issues, maps each finding
  to a PR (or flags it as uncovered)
- P0/P1 bugs without PR: groups by pattern to suggest sprint batching
- Our dirty PRs: identifies probable cause (conflict with sibling PR,
  needs rebase, missing linked issue)

Output is saved automatically to claudedocs/RTK-YYYY-MM-DD.md.

Usage: /rtk-triage           (French, auto-save)
       /rtk-triage en        (English output)

Signed-off-by: Florian Bruniaux <florian@bel-etage.com>
Signed-off-by: Florian BRUNIAUX <florian@bruniaux.com>

* docs(architecture): update module count to 66

Sync ARCHITECTURE.md with current main.rs state.
Previous count (60) was stale since several modules were added
(dotnet_cmd, dotnet_format_report, dotnet_trx, npm_cmd, gt_cmd, etc.).

Signed-off-by: Florian Bruniaux <florian@bel-etage.com>
Signed-off-by: Florian BRUNIAUX <florian@bruniaux.com>

---------

Signed-off-by: Florian Bruniaux <florian@bel-etage.com>
Signed-off-by: Florian BRUNIAUX <florian@bruniaux.com>
…tk-ai#601)

- git stash: pass unknown subcommands (save, branch, clear) through
  instead of silently falling back to git stash push
- git branch: add --show-current, --set-upstream-to, --format, --sort
  to flag detection so they don't get overridden by -a injection
- pip: replace bail!() with passthrough for unknown subcommands
  (freeze, download, wheel, etc.)

Fixes rtk-ai#600

Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>
cargo fmt diffs in config.rs, git.rs, playwright_cmd.rs were failing
the fmt CI check, which cascaded to block clippy/test/security on
PRs rtk-ai#632, rtk-ai#635, rtk-ai#638. Also fixes all clippy warnings: dead code
annotations, iterator simplifications, assert patterns, and
unnecessary allocations.

Signed-off-by: Patrick Szymkowiak <patrick@rtk-ai.app>
Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>
…#163) (rtk-ai#518)

* fix: discover classifies absolute paths like /usr/bin/grep (rtk-ai#485)

Normalize absolute binary paths before classification:
/usr/bin/grep → grep, /bin/ls → ls, /usr/local/bin/git → git

Adds strip_absolute_path() helper + 5 tests.

Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>

* fix: discover and rewrite support git global options -C, --no-pager, etc. (rtk-ai#163)

Strip git global options (-C <path>, -c <key=val>, --git-dir, --work-tree,
--no-pager, --no-optional-locks, --bare, --literal-pathspecs) before
classification so git -C /tmp status is recognized as rtk git.

Rewrite preserves global options: git -C /tmp status → rtk git -C /tmp status

Adds GIT_GLOBAL_OPT lazy_static regex + strip_git_global_opts() helper + 6 tests.

Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>

---------

Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>
…-ai#519)

When running `rtk cargo clippy -p my-crate -- -D warnings`, Clap with
`trailing_var_arg = true` preserves the `--` in parsed args when flags
precede it. `restore_double_dash()` then added a second `--`, producing
`cargo clippy -p my-crate -- -- -D warnings`. This caused rustc to
interpret `-D` as a filename instead of a lint flag.

Fix: skip restoration when args already contain `--` (Clap preserved it).

Fixes rtk-ai#496

Signed-off-by: Ousama Ben Younes <benyounes.ousama@gmail.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
- PR template reminds contributors to target develop
- CI workflow labels PRs targeting master with 'wrong-base' and posts a comment
- Excludes develop→master PRs (maintainer releases)

Signed-off-by: Patrick <patrick@rtk-ai.com>
Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>
Add Language::Data variant for data formats (JSON, YAML, TOML, XML, CSV, etc.)
with empty comment patterns to prevent comment stripping. AggressiveFilter
falls back to MinimalFilter for data files.

Fixes rtk-ai#464

Signed-off-by: Ousama Ben Younes <benyounes.ousama@gmail.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…tk-ai#439) (rtk-ai#563)

rtk find outputs a grouped format incompatible with pipe consumers
like xargs, grep, wc, sort. Skip rewrite when find/fd is followed
by a pipe, preserving native one-per-line output.

Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>
…gh (rtk-ai#427) (rtk-ai#564)

When compact_diff truncates output, append a hint line so Claude knows
how to get the full diff: [full diff: rtk git diff --no-compact]

Also fix --no-compact flag being passed to git (causing usage error)
and remove decorative emoji from compact_diff output.

Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>
rtk-ai#632)

4 P1 bugs where git exit codes were swallowed:
- git diff: failure silently printed empty stat output
- git status (with args): failure was filtered instead of propagated
- git commit: failure printed "FAILED" but returned Ok(()) breaking pre-commit hooks
- git branch (list mode): failure was silently ignored

All now follow the established pattern: eprint stderr, track raw==raw, process::exit(code).

Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>
…tk-ai#635)

* feat: add 5 new TOML built-in filters (ollama, nx, gradle, spring-boot, jira)

New filters for commands not covered by Rust modules:
- ollama: strip ANSI spinners, keep final text response (rtk-ai#624)
- nx: strip Nx monorepo noise, keep build results (rtk-ai#444)
- gradle/gradlew: strip UP-TO-DATE tasks, keep build summary (rtk-ai#147)
- spring-boot: strip banner and verbose logs, keep startup/errors (rtk-ai#147)
- jira: strip blanks, truncate wide columns (rtk-ai#524)

All 5 filters pass inline tests via rtk verify (123/123).
Updated builtin filter count: 47 -> 52.

Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>

* feat: add 5 more TOML filters (turbo, mise, just, task, yadm)

New filters for task runners and git wrapper:
- turbo: strip cache/Tasks/Duration noise, keep task output (rtk-ai#531)
- mise: strip install/download progress, keep task results (rtk-ai#607)
- just: strip blanks and recipe headers, keep output (rtk-ai#607)
- task: strip task headers and up-to-date lines, keep results (rtk-ai#607)
- yadm: strip hint lines, compact git-like output (rtk-ai#567)

All verified with fake binaries through catch-all TOML engine.
137/137 TOML tests pass, 934 Rust tests pass.
Updated builtin filter count: 52 -> 57.

Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>

---------

Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>
…rtk-ai#638)

Git status output used emojis (📌, 📝, ❓, ✅, ⚠️) that confuse
non-Claude LLMs (GPT, etc.) causing retry loops. Replace with plain
text labels (branch:, modified:, staged:, untracked:, conflicts:).

Also add "clean — nothing to commit" when working tree is clean,
so LLMs understand the repo state without ambiguity.

Before: 📌 master
After:  branch: master
        clean — nothing to commit

Fixes rtk-ai#603

Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>
@rtk-wshm-sync-bot

Copy link
Copy Markdown

wshm · Automated triage by AI

📊 Automated PR Analysis

🐛 Type bug-fix
🔴 Risk high

Summary

PR claims to fix the Claude hook's grep-rewrite rule so recursive grep -r invocations are no longer rewritten to rtk grep, preserving the shell's native .gitignore-aware grep behavior; it adds an is_recursive_grep() detector and 3 regression tests in src/discover/registry.rs. However, the actual diff bundles in a large amount of unrelated and unexplained content: a new AGENTS.md mandating exclusive use of a third-party 'graphify' tool instead of grep/read/find, a new OpenCode plugin t…

Review Checklist

  • Tests present
  • Breaking change
  • Docs updated

Linked issues: #4477


Analyzed automatically by wshm · This is an automated analysis, not a human review.

@KuSh

KuSh commented Oct 10, 2026

Copy link
Copy Markdown
Collaborator

Thank you for the fix. #4488 carries the same change for #4477, so I am closing this PR to keep a single branch. Could you port the helper from this PR into #4488? It honours -d skip and stops at --, while the helper in #4488 treats any -d as recursive. Please also drop everything outside src/discover/registry.rs from #4488 (the .opencode graphify plugin, AGENTS.md, check_prs.py, find_candidates.py, the codex init and docs changes), so that it holds only the rewrite guard and its tests. #4488 merges cleanly on the current layout, while this branch is on the old src/ layout and conflicts. If I missed something this PR covers, please comment or reopen.

@KuSh KuSh closed this Oct 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Claude Code hook: recursive grep → rtk grep still ignores .gitignore on 0.51.0 (remaining half of #2606)

5 participants