Skip to content

Commit 8cd29e5

Browse files
committed
fix: guard invalid RTK hook suggestions
1 parent 40080e5 commit 8cd29e5

3 files changed

Lines changed: 117 additions & 3 deletions

File tree

docs/DEVELOPMENT.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,15 +103,21 @@ Rewrite action order:
103103
- Already preferred RTK commands are left alone, such as `rtk git status --short`.
104104
Invalid `rtk read` forms with invented window flags such as `--line`,
105105
`--lines`, `--range`, `--start-line`, `--start`, `--from`, `--to`, or
106-
`--line-number` are denied with `rtk read --help` so agents do not confuse RTK
107-
parse fallback errors with a missing `rtk read` command.
106+
`--line-number`, plus path range forms like `file.md:35-160`, are denied with
107+
`rtk read --help` so agents do not confuse RTK parse fallback errors with a
108+
missing `rtk read` command.
109+
- Invalid `rtk grep` forms that put ripgrep context flags such as `-C N` or
110+
`--context N` before the pattern are denied with a corrected suggestion that
111+
moves those flags after `--`.
108112
- Mutating PowerShell commands are left alone, including `Remove-Item`,
109113
`Set-Content`, `Add-Content`, `New-Item`, `Move-Item`, and `Copy-Item`.
110114
- Windows/PowerShell and Unix shell reads/searches are handled locally first and
111115
auto-rewritten only when the hook can preserve semantics.
112116
- Generic cross-platform tools are delegated to `rtk rewrite`, such as
113117
`git status --short` to `rtk git status --short` and `ls src` to `rtk ls src`.
114118
Delegated rewrites stay as deny guidance in the first `updatedInput` release.
119+
Raw `git diff -- path...` is left alone because the current RTK git wrapper
120+
does not preserve Git pathspec separator behavior for that form.
115121
- If `rtk rewrite` is unavailable or returns no suggestion, a small local
116122
fallback preserves legacy suggestions for common noisy tools such as `git`,
117123
`cargo`, `npm`, `pytest`, `busted`, `luacheck`, `dotnet`, `pnpm`, `pip`, `go`,

src/rewrite.rs

Lines changed: 94 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ pub fn action(command: &str) -> Option<HookAction> {
2121

2222
invalid_rtk_read_redirect(command)
2323
.map(HookAction::DenySuggestion)
24+
.or_else(|| invalid_rtk_grep_redirect(command).map(HookAction::DenySuggestion))
2425
.or_else(|| {
2526
if starts_with_rtk(command) && preferred_rtk_command(command)
2627
|| is_preferred_pwsh_wrapper(command)
@@ -96,10 +97,91 @@ fn invalid_rtk_read_redirect(command: &str) -> Option<String> {
9697
|| token.text.starts_with("--from=")
9798
|| token.text.starts_with("--to=")
9899
|| token.text.starts_with("--line-number=")
100+
|| looks_like_rtk_read_path_range(&token.text)
99101
})
100102
.then(|| "rtk read --help".to_string())
101103
}
102104

105+
fn looks_like_rtk_read_path_range(value: &str) -> bool {
106+
let Some((path, range)) = value.rsplit_once(':') else {
107+
return false;
108+
};
109+
!path.is_empty()
110+
&& path != "."
111+
&& range.split_once('-').map_or_else(
112+
|| range.chars().all(|ch| ch.is_ascii_digit()),
113+
|(start, end)| {
114+
!start.is_empty()
115+
&& !end.is_empty()
116+
&& start.chars().all(|ch| ch.is_ascii_digit())
117+
&& end.chars().all(|ch| ch.is_ascii_digit())
118+
},
119+
)
120+
}
121+
122+
fn invalid_rtk_grep_redirect(command: &str) -> Option<String> {
123+
let tokens = tokenize(command);
124+
if command_name(&tokens.first()?.text) != "rtk" || command_name(&tokens.get(1)?.text) != "grep"
125+
{
126+
return None;
127+
}
128+
129+
let passthrough_with_values = [
130+
"-C",
131+
"--context",
132+
"-A",
133+
"--after-context",
134+
"-B",
135+
"--before-context",
136+
];
137+
let mut rtk_options = Vec::new();
138+
let mut passthrough = Vec::new();
139+
let mut index = 2;
140+
while index < tokens.len() && tokens[index].text.starts_with('-') && tokens[index].text != "--"
141+
{
142+
let option = tokens[index].text.as_str();
143+
if matches!(option, "-n" | "--line-number") {
144+
rtk_options.push("-n".to_string());
145+
index += 1;
146+
continue;
147+
}
148+
if passthrough_with_values.contains(&option) && index + 1 < tokens.len() {
149+
passthrough.push(tokens[index].text.clone());
150+
index += 1;
151+
passthrough.push(tokens[index].text.clone());
152+
index += 1;
153+
continue;
154+
}
155+
return None;
156+
}
157+
158+
if passthrough.is_empty() || index >= tokens.len() || tokens[index].text == "--" {
159+
return None;
160+
}
161+
162+
let pattern = &tokens[index].text;
163+
let paths = tokens[index + 1..]
164+
.iter()
165+
.take_while(|token| token.text != "--")
166+
.map(|token| token.text.as_str())
167+
.collect::<Vec<_>>();
168+
if paths.iter().any(|path| path.starts_with('-')) {
169+
return None;
170+
}
171+
172+
let mut parts = vec!["rtk".to_string(), "grep".to_string()];
173+
parts.extend(rtk_options);
174+
parts.push(quote_pattern(pattern));
175+
if paths.is_empty() {
176+
parts.push(".".to_string());
177+
} else {
178+
parts.extend(paths.into_iter().map(quote_arg));
179+
}
180+
parts.push("--".to_string());
181+
parts.extend(passthrough.into_iter().map(|arg| quote_arg(&arg)));
182+
Some(parts.join(" "))
183+
}
184+
103185
fn preferred_rtk_command(command: &str) -> bool {
104186
let tokens = tokenize(command);
105187
let second = tokens.get(1).map(|token| command_name(&token.text));
@@ -1087,6 +1169,9 @@ fn is_rg_files_command(command: &str) -> bool {
10871169
fn local_rtk_miss_fallback(command: &str) -> Option<String> {
10881170
let tokens = tokenize(command);
10891171
let first = tokens.first().map(|token| command_name(&token.text));
1172+
if is_git_diff_pathspec_command(command) {
1173+
return None;
1174+
}
10901175
match first.as_deref() {
10911176
Some("python") if python_pytest_args(&tokens).is_some() => {
10921177
let args = python_pytest_args(&tokens)?;
@@ -1105,6 +1190,14 @@ fn local_rtk_miss_fallback(command: &str) -> Option<String> {
11051190
}
11061191
}
11071192

1193+
fn is_git_diff_pathspec_command(command: &str) -> bool {
1194+
let tokens = tokenize(command);
1195+
tokens.len() >= 4
1196+
&& command_name(&tokens[0].text) == "git"
1197+
&& tokens.get(1).is_some_and(|token| token.text == "diff")
1198+
&& tokens.iter().skip(2).any(|token| token.text == "--")
1199+
}
1200+
11081201
fn python_pytest_args(tokens: &[Token]) -> Option<String> {
11091202
if tokens.len() >= 3 && tokens[1].text == "-m" && command_name(&tokens[2].text) == "pytest" {
11101203
Some(join_args(&tokens[3..]))
@@ -1171,7 +1264,7 @@ fn rtk_rewrite(command: &str) -> Option<String> {
11711264
}
11721265

11731266
fn safe_external_rtk_rewrite(command: &str) -> Option<String> {
1174-
if blocks_external_posix_rewrite(command) {
1267+
if blocks_external_posix_rewrite(command) || is_git_diff_pathspec_command(command) {
11751268
return None;
11761269
}
11771270
rtk_rewrite(command)

tests/pretool.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,8 @@ fn invalid_rtk_read_flags_suggest_help() {
246246
"rtk read src/client/source.lua --line 1 --lines 650",
247247
"rtk read src/ui.lua --line 1-120",
248248
"rtk read src/ui.lua --range 130:310",
249+
"rtk read docs/notes.md:35-160",
250+
"rtk read src/ui.lua:130",
249251
"rtk read docs/notes.md --start-line 130 --max-lines 60",
250252
"rtk read docs/notes.md --start-line=130 --max-lines 60",
251253
"rtk read docs/notes.md --start 130 --max-lines 60",
@@ -282,9 +284,22 @@ fn powershell_mutations_are_noops() {
282284
fn generic_rtk_rewrite_fallbacks_apply_to_common_tools() {
283285
assert_deny("git status --short", "rtk git status --short");
284286
assert_deny("ls src", "rtk ls src");
287+
assert_no_output("git diff -- src/rewrite.rs tests/pretool.rs");
285288
assert_no_output("gh pr view --json title");
286289
}
287290

291+
#[test]
292+
fn invalid_rtk_grep_passthrough_flags_are_corrected() {
293+
assert_deny(
294+
r#"rtk grep -n -C 25 "refreshChapterMenu" suwayomi spec"#,
295+
r#"rtk grep -n "refreshChapterMenu" suwayomi spec -- -C 25"#,
296+
);
297+
assert_deny(
298+
r#"rtk grep -n --context 4 "foo|bar" src tests"#,
299+
r#"rtk grep -n "foo|bar" src tests -- --context 4"#,
300+
);
301+
}
302+
288303
#[test]
289304
fn get_content_redirects_to_rtk_read() {
290305
assert_rewrite(

0 commit comments

Comments
 (0)