fix(search): strip ripgrep -r/-R so rg --replace no longer corrupts output

[MichelKazi]

Fixes #3161

rtk rg -r/-R silently rewrites match output (ripgrep --replace)

Summary

rtk rg -rn PATTERN FILE corrupts output. Every match is replaced by the string that follows -r. With -rn, that string is n, so output turns into interleaved-n garbage. With -rln PATTERN, the string is ln, so the token tee prints as ln, guarantee prints as guaranln, and so on.

The cause is flag handling in src/cmds/system/search.rs. rtk forwards -r and -R to the engine verbatim, treating them as grep's recursive flag. For ripgrep, -r/-R is --replace (value-taking), not recursive. ripgrep is already recursive by default, so agents that type rg -rn out of grep habit trigger a replace they never asked for.

Version

rtk 0.43.0 (Homebrew). develop branch has the same code path.

Reproduction

printf 'class Api::ApiV3Controller\n' > /tmp/api.rb

rtk rg -n  class /tmp/api.rb     # 1:class Api::ApiV3Controller   (correct)
rtk rg -rn class /tmp/api.rb     # /tmp/api.rb:1:n Api::ApiV3Controller   (corrupt)
rtk proxy rg -rn class /tmp/api.rb   # same corruption: proxy bypasses rtk's
                                     # filter, so this is real ripgrep behavior

Native ripgrep confirms rtk is passing -r through as --replace:

rg -rn class /tmp/api.rb         # n Api::ApiV3Controller

The interleaved-n shape appears because n is the replacement string and ripgrep applies it at every match position across the line.

grep -rn is fine, because grep's -r really is recursive. Only rg is affected.

Root cause

src/cmds/system/search.rs:

• VALUE_FLAGS_SHORT (line 22) is b"ABCMTdfgjmt" and the doc comment on lines 20 and 21 says it lists "all rg short flags that take a value argument except -e and -r (stripped)". The intent is to strip -r.
• parse_cluster (line 84) never strips -r/-R. It accumulates them as boolean flags in raw_prefix and forwards them to the engine. The comment on lines 80 to 83 states r/R "pass through to grep (recursion is the agent's choice, not RTK's)".
• test_extract_cluster_keeps_r and test_parse_cluster_boolean_only pin this behavior, so the tests currently encode the bug.

For grep the pass-through is correct. For rg it is a data-corrupting default, because the same byte means two different things across the two engines rtk claims to run interchangeably.

Suggested fix

Make -r/-R engine-aware. For Engine::Rg, strip -r/-R and their value (they are recursion no-ops for rg and dangerous as replace). For Engine::Grep, keep the current pass-through.

Concretely, parse_cluster needs the engine, or extract_pattern_path needs a post-pass for rg that:

1. drops a standalone -r/-R,
2. drops r/R letters from short clusters,
3. drops --replace and its value.

Then update test_extract_cluster_keeps_r and friends to assert the rg path strips -r while the grep path keeps it.

Impact

Silent. rtk's own stdout looks structured (N matches in M files:), so the corruption reads as real file content. Agents relying on rtk rg -rn (a common grep habit) get wrong file contents with no error. It also lands in the tee recovery files, so the "full output" recovery path is corrupt too.

Workaround (no source change)

A PreToolUse shim wraps rtk hook claude and strips -r/-R from rewritten rtk rg commands before they run. grep is left alone. This restores correct output without disabling rtk's filtering.

[CLAassistant]

All committers have signed the CLA.

[pszymkowiak]

Thanks for tackling this — the diagnosis is right (rg's -r/-R is --replace, not recursive) and strip_rg_replace itself is well-built (handles --replace X, --replace=X, short clusters; correctly scoped to Engine::Rg only, so grep recursion is untouched). rtk rg -rn foo is fixed. ✅

However, tested on a compiled build, the fix is incomplete — any cluster combining -r with a format flag still corrupts output:

rtk rg -rl class file   → "l Foo"        (grep -rl muscle memory — very common)
rtk rg -rc class file   → "c Foo"
rtk rg -ro class file   → "o Foo"
rtk rg -rln tee file    → "guaranln"     ← the exact -rln example from this PR's description

Root cause: the three passthrough(&timer, engine, &args, …) calls (search.rs:446, 463, 474) re-run with the original, unstripped args. strip_rg_replace only touches extra_args (the compression path). So as soon as has_format_flag matches (-rl/-rc/-ro/-rq…), we passthrough with the raw -r… and rg re-applies --replace. has_format_flag's own comment says clusters like -rl should "route through" — but routing through passthrough re-introduces the very bug.

Suggested fix: for Engine::Rg, strip -r/-R/--replace from the args used by passthrough too (e.g. compute the stripped full-arg vector once and pass it to passthrough when the engine is rg), and add end-to-end coverage for -rl, -rc, -ro, -rln.

Grep is unaffected (verified: rtk grep -rn stays recursive). Once the format-flag/passthrough path is covered this is good to go.

[KuSh]

Thanks for the detailed report and fix — the diagnosis is correct, but I'm closing this in favour of #3681, which addresses the same bug a layer lower.

#3681 introduces a shared arg tokenizer and makes -r engine-aware at parse time (is_short_value_flag: "T" | "r" => engine == Engine::Rg), so rg's -r is tokenized as the value-taking --replace it actually is. This PR post-strips flags that were already mis-parsed, which leaves a few cases behind:

• The strip only reaches the reconstructed engine command. passthrough() re-runs the engine with the original args, and has_format_flag is computed on the stripped list — so -rc reduces to -c, routes to passthrough, and rtk rg -rc class still prints api.rb:c Api instead of a count. Same for -rl, -rq, -ro.
• The space-separated form is still mis-parsed: -r stays boolean, so its value lands in the positionals and shifts pattern/paths. rtk rg -r X -n class file.rb errors with class: No such file or directory where real rg prints 1:X Api.
• show_file() reads -r/-R from the stripped list, so with no explicit path rtk rg -rn class prints 1:class Api where rg prints file.rb:1:class Api.

#3681 also removes parse_cluster/ClusterResult and changes extract_pattern_path's signature, so strip_rg_replace and its tests would need rewriting on top of it regardless.

On the behaviour itself: #3681 forwards -r to rg rather than suppressing it, and that's deliberate. There was a time when rtk rewrote grep invocations into rg; that's no longer the case — rtk runs the engine the agent asked for, with the flags the agent passed. An agent typing rg -rn out of grep habit is the agent's mistake, not rtk's, and rtk shouldn't silently drop an explicit flag to paper over it. #3161 will be closed once #3681 lands.

Thanks again for the report — it's a real issue and well documented.