From 49a094ac9ba7615a14f2ee0b8fad10932ffe76f7 Mon Sep 17 00:00:00 2001 From: Ashwin Gopalsamy <47941624+ashwingopalsamy@users.noreply.github.com> Date: Mon, 6 Apr 2026 09:57:01 +0530 Subject: [PATCH 01/39] fix: reset SIGPIPE to default handler to avoid panic --- src/main.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/main.rs b/src/main.rs index 11332034a..9f526264b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1247,6 +1247,15 @@ fn validate_pnpm_filters(filters: &[String], command: &PnpmCommands) -> Option code, Err(e) => { From 03cc790a6a67a033185d45ad85df6641c5b50bee Mon Sep 17 00:00:00 2001 From: Ashwin Gopalsamy <47941624+ashwingopalsamy@users.noreply.github.com> Date: Mon, 6 Apr 2026 10:14:01 +0530 Subject: [PATCH 02/39] test: add ignored test for broken-pipe crash --- src/main.rs | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/main.rs b/src/main.rs index 9f526264b..f9be6380c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2736,4 +2736,38 @@ mod tests { _ => panic!("Expected Pnpm Build command"), } } + + #[test] + #[ignore] // Integration test: requires `cargo build` first + fn test_broken_pipe_does_not_crash() { + let bin_path = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("target") + .join("debug") + .join("rtk"); + assert!( + bin_path.exists(), + "Debug binary not found at {:?} - run `cargo build` first", + bin_path + ); + + let mut child = std::process::Command::new(&bin_path) + .args(["git", "log", "--online", "-50"]) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .expect("Failed to spawn rtk"); + + // Read one byte then drop stdout to close the pipe. + let mut stdout = child.stdout.take().unwrap(); + let mut buf = [0u8; 1]; + let _ = std::io::Read::read(&mut stdout, &mut buf); + + let status = child.wait().expect("Failed to wait for rtk"); + let code = status.code().unwrap_or(-1); + + assert_ne!( + code, 134, + "rtk crashed with SIGABRT (exit 134) on broken pipe - SIGPIPE handler missing" + ); + } } From ccf3a020ea6641482b31a0a841806ca156fe6a50 Mon Sep 17 00:00:00 2001 From: Ashwin Gopalsamy <47941624+ashwingopalsamy@users.noreply.github.com> Date: Mon, 13 Apr 2026 09:37:07 +0530 Subject: [PATCH 03/39] fix: address PR review comments --- src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main.rs b/src/main.rs index f9be6380c..00557d4ce 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1251,7 +1251,7 @@ fn main() { // e.g `rtk git log | head` exits silently instead of panicking. // Rust ignores SIGPIPE by default and with panic="abort" in the // release profile that becomes SIGABRT + coredump. - // #[cfg(unix)] + #[cfg(unix)] unsafe { libc::signal(libc::SIGPIPE, libc::SIG_DFL); } @@ -2751,7 +2751,7 @@ mod tests { ); let mut child = std::process::Command::new(&bin_path) - .args(["git", "log", "--online", "-50"]) + .args(["git", "log", "--oneline", "-50"]) .stdout(std::process::Stdio::piped()) .stderr(std::process::Stdio::piped()) .spawn() From be226d4f1dcdcc0df29ad6f9968be5ab37ed5c19 Mon Sep 17 00:00:00 2001 From: F0rty-Tw0 Date: Sat, 23 May 2026 22:30:31 +0200 Subject: [PATCH 04/39] fix(grep): parse single-file output containing colons Single-file `rtk grep` returned synthetic `[file] N` buckets when a matching line contained `:`. ripgrep emits `:` for single-file searches; the parser used `splitn(3, ':')` length to disambiguate, which fails when content has `:` (e.g. `Foo::bar(...)`). Force `--with-filename` on rg (`-H` on the grep fallback) so output is always `file:line:content`, and replace the inline parser with a regex-based `parse_match_line` that also handles paths with embedded `:` (Windows drive letters). Fixes the parser/rendering portion of #1436. The BRE-translation portion is addressed in a follow-up PR. Signed-off-by: Artiom Tofan --- src/cmds/system/grep_cmd.rs | 110 +++++++++++++++++++++++++++++++----- 1 file changed, 97 insertions(+), 13 deletions(-) diff --git a/src/cmds/system/grep_cmd.rs b/src/cmds/system/grep_cmd.rs index 6a33cf3a4..806c782b4 100644 --- a/src/cmds/system/grep_cmd.rs +++ b/src/cmds/system/grep_cmd.rs @@ -33,7 +33,19 @@ pub fn run( // Without this, rg returns 0 matches for files in .gitignore, causing // false negatives that make AI agents draw wrong conclusions. // Using --no-ignore-vcs (not --no-ignore) so .ignore/.rgignore are still respected. - rg_cmd.args(["-n", "--no-heading", "--no-ignore-vcs", &rg_pattern, path]); + // + // --with-filename: force rg to always include the filename, even for + // single-file invocations. Without it, rg emits `:` for a + // single file, and the parser cannot tell `:` from + // `::` when content itself contains ':' (issue #1436). + rg_cmd.args([ + "-n", + "--no-heading", + "--with-filename", + "--no-ignore-vcs", + &rg_pattern, + path, + ]); if let Some(ft) = file_type { rg_cmd.arg("--type").arg(ft); @@ -50,8 +62,9 @@ pub fn run( let result = exec_capture(&mut rg_cmd) .or_else(|_| { let mut grep_cmd = resolved_command("grep"); - //When we fall back to grep,include all args, not just -rn. - grep_cmd.args(["-rn", pattern, path]).args(extra_args); + // -H: always emit the filename so the parser sees a uniform + // file:line:content shape (parity with rg's --with-filename). + grep_cmd.args(["-rnH", pattern, path]).args(extra_args); exec_capture(&mut grep_cmd) }) .context("grep/rg failed")?; @@ -108,18 +121,9 @@ pub fn run( let mut by_file: HashMap> = HashMap::new(); for line in result.stdout.lines() { - let parts: Vec<&str> = line.splitn(3, ':').collect(); - - let (file, line_num, content) = if parts.len() == 3 { - let ln = parts[1].parse().unwrap_or(0); - (parts[0].to_string(), ln, parts[2]) - } else if parts.len() == 2 { - let ln = parts[0].parse().unwrap_or(0); - (path.to_string(), ln, parts[1]) - } else { + let Some((file, line_num, content)) = parse_match_line(line) else { continue; }; - let cleaned = clean_line(content, max_line_len, context_re.as_ref(), pattern); by_file.entry(file).or_default().push((line_num, cleaned)); } @@ -166,6 +170,29 @@ pub fn run( Ok(exit_code) } +/// Parses a single rg/grep match line of the form `file:line_number:content`. +/// +/// Always returns three colon-separated fields when the underlying command was +/// invoked with `--with-filename` (rg) or `-H` (grep). Uses a greedy regex on +/// the path so: +/// - content with `::` (e.g. `ClassRegistry::init(...)`) does not split into +/// a phantom file bucket (issue #1436); +/// - paths with embedded `:` (Windows drive letters like `C:\foo\file.php`) +/// parse correctly — the LAST `:digits:` before content is the boundary. +/// +/// Returns `None` for lines that do not match the expected shape (e.g. rg +/// `-A`/`-B` context lines that use `-` as separator). +fn parse_match_line(line: &str) -> Option<(String, usize, &str)> { + lazy_static::lazy_static! { + static ref MATCH_LINE_RE: Regex = Regex::new(r"^(.+):(\d+):(.*)$").unwrap(); + } + let caps = MATCH_LINE_RE.captures(line)?; + let file = caps.get(1)?.as_str().to_string(); + let line_num: usize = caps.get(2)?.as_str().parse().ok()?; + let content_start = caps.get(3)?.start(); + Some((file, line_num, &line[content_start..])) +} + fn has_format_flag(extra_args: &[String]) -> bool { extra_args.iter().any(|arg| { matches!( @@ -391,6 +418,63 @@ mod tests { // If rg is not installed, skip gracefully (test still passes) } + // --- issue #1436: parse_match_line robustness --- + + #[test] + fn test_parse_match_line_simple() { + let line = "file.php:10:use Foo\\Bar;"; + let (file, line_num, content) = parse_match_line(line).unwrap(); + assert_eq!(file, "file.php"); + assert_eq!(line_num, 10); + assert_eq!(content, "use Foo\\Bar;"); + } + + // Issue #1436 reproducer: content with `::` must not split into a phantom + // file bucket. Before the fix, rg's single-file output `81: $this->... + // ClassRegistry::init('...')` was parsed into 3 colon-fields, producing + // file="81", line=0, content=":init('...')". + #[test] + fn test_parse_match_line_content_with_double_colon() { + let line = "externalImportShell.class.php:81: $this->queueProcessModel = ClassRegistry::init('Collections.QueueProcess');"; + let (file, line_num, content) = parse_match_line(line).unwrap(); + assert_eq!(file, "externalImportShell.class.php"); + assert_eq!(line_num, 81); + assert_eq!( + content, + " $this->queueProcessModel = ClassRegistry::init('Collections.QueueProcess');" + ); + } + + // Windows abs-path safety: drive letter + backslashes must not break the + // parser. Greedy regex on path picks the LAST `:digits:` boundary. + #[test] + fn test_parse_match_line_windows_path() { + let line = r"C:\src\file.rs:42:fn main() {}"; + let (file, line_num, content) = parse_match_line(line).unwrap(); + assert_eq!(file, r"C:\src\file.rs"); + assert_eq!(line_num, 42); + assert_eq!(content, "fn main() {}"); + } + + #[test] + fn test_parse_match_line_malformed_returns_none() { + // Single token, no colons (e.g. an rg context line written with `-`) + assert!(parse_match_line("not a match line").is_none()); + // Missing line number + assert!(parse_match_line("file.rs:fn foo()").is_none()); + // Empty + assert!(parse_match_line("").is_none()); + } + + #[test] + fn test_parse_match_line_empty_content() { + let line = "file.rs:7:"; + let (file, line_num, content) = parse_match_line(line).unwrap(); + assert_eq!(file, "file.rs"); + assert_eq!(line_num, 7); + assert_eq!(content, ""); + } + #[test] fn test_rg_no_ignore_vcs_flag_accepted() { // Verify rg accepts --no-ignore-vcs (used to match grep -r behavior for .gitignore) From 0e3a6f48305c1302443cbe5206e5b5f7cffb671a Mon Sep 17 00:00:00 2001 From: F0rty-Tw0 Date: Sat, 23 May 2026 22:57:35 +0200 Subject: [PATCH 05/39] fix(grep): NUL-separate file from line:content per review - rg: switch to `-nH --null` for robust file/content separation - grep fallback: switch to `-rnHZ` to match rg's --null contract - Parser regex: `^([^\x00]+)\x00(\d+):(.*)$` so filenames or content containing `:digits:` (e.g. `badly_named:52:file.txt`, `debug: counter is :42: now`) can no longer fool the parser - Use `Captures::extract` for cleaner destructuring Adds tests for the badly-named-filename and digit-colons-in-content cases; updates existing tests to use NUL-separated fixtures. Addresses review on #1554. Signed-off-by: Artiom Tofan --- src/cmds/system/grep_cmd.rs | 92 +++++++++++++++++++++++-------------- 1 file changed, 58 insertions(+), 34 deletions(-) diff --git a/src/cmds/system/grep_cmd.rs b/src/cmds/system/grep_cmd.rs index 806c782b4..dbd61364a 100644 --- a/src/cmds/system/grep_cmd.rs +++ b/src/cmds/system/grep_cmd.rs @@ -34,14 +34,14 @@ pub fn run( // false negatives that make AI agents draw wrong conclusions. // Using --no-ignore-vcs (not --no-ignore) so .ignore/.rgignore are still respected. // - // --with-filename: force rg to always include the filename, even for - // single-file invocations. Without it, rg emits `:` for a - // single file, and the parser cannot tell `:` from - // `::` when content itself contains ':' (issue #1436). + // -H: force rg to always include the filename, even for single-file searches. + // --null: separate file from line:content with a NUL byte so the parser is + // unambiguous even when the filename or content contains `:digits:` + // patterns (issue #1436). rg_cmd.args([ - "-n", + "-nH", "--no-heading", - "--with-filename", + "--null", "--no-ignore-vcs", &rg_pattern, path, @@ -62,9 +62,10 @@ pub fn run( let result = exec_capture(&mut rg_cmd) .or_else(|_| { let mut grep_cmd = resolved_command("grep"); - // -H: always emit the filename so the parser sees a uniform - // file:line:content shape (parity with rg's --with-filename). - grep_cmd.args(["-rnH", pattern, path]).args(extra_args); + // -H: always emit the filename; -Z: NUL-separate filename from + // line:content so the parser can disambiguate even for filenames + // or content containing `:digits:` (parity with rg's --null). + grep_cmd.args(["-rnHZ", pattern, path]).args(extra_args); exec_capture(&mut grep_cmd) }) .context("grep/rg failed")?; @@ -170,27 +171,26 @@ pub fn run( Ok(exit_code) } -/// Parses a single rg/grep match line of the form `file:line_number:content`. +/// Parses a single rg/grep match line of the form `file\0line_number:content`. /// -/// Always returns three colon-separated fields when the underlying command was -/// invoked with `--with-filename` (rg) or `-H` (grep). Uses a greedy regex on -/// the path so: -/// - content with `::` (e.g. `ClassRegistry::init(...)`) does not split into -/// a phantom file bucket (issue #1436); -/// - paths with embedded `:` (Windows drive letters like `C:\foo\file.php`) -/// parse correctly — the LAST `:digits:` before content is the boundary. +/// Requires the underlying command to be invoked with `--null` (rg) or `-Z` +/// (grep) so the filename is NUL-separated from `line:content`. NUL cannot +/// appear in file paths, so the parser is unambiguous regardless of: +/// - content with `:` or `::` (e.g. `ClassRegistry::init(...)`, issue #1436); +/// - paths with embedded `:` (Windows drive letters, weird filenames like +/// `badly_named:52:file.txt`). /// /// Returns `None` for lines that do not match the expected shape (e.g. rg /// `-A`/`-B` context lines that use `-` as separator). fn parse_match_line(line: &str) -> Option<(String, usize, &str)> { lazy_static::lazy_static! { - static ref MATCH_LINE_RE: Regex = Regex::new(r"^(.+):(\d+):(.*)$").unwrap(); + static ref MATCH_LINE_RE: Regex = Regex::new(r"^([^\x00]+)\x00(\d+):(.*)$").unwrap(); } - let caps = MATCH_LINE_RE.captures(line)?; - let file = caps.get(1)?.as_str().to_string(); - let line_num: usize = caps.get(2)?.as_str().parse().ok()?; - let content_start = caps.get(3)?.start(); - Some((file, line_num, &line[content_start..])) + MATCH_LINE_RE.captures(line).and_then(|caps| { + let (_, [file, line_num, content]) = caps.extract(); + let line_num: usize = line_num.parse().ok()?; + Some((file.to_string(), line_num, content)) + }) } fn has_format_flag(extra_args: &[String]) -> bool { @@ -419,10 +419,11 @@ mod tests { } // --- issue #1436: parse_match_line robustness --- + // Input shape is `file\0line:content` (rg --null / grep -Z). #[test] fn test_parse_match_line_simple() { - let line = "file.php:10:use Foo\\Bar;"; + let line = "file.php\x0010:use Foo\\Bar;"; let (file, line_num, content) = parse_match_line(line).unwrap(); assert_eq!(file, "file.php"); assert_eq!(line_num, 10); @@ -430,12 +431,11 @@ mod tests { } // Issue #1436 reproducer: content with `::` must not split into a phantom - // file bucket. Before the fix, rg's single-file output `81: $this->... - // ClassRegistry::init('...')` was parsed into 3 colon-fields, producing - // file="81", line=0, content=":init('...')". + // file bucket. With NUL separation between file and line:content, content + // colons are irrelevant to the parser. #[test] fn test_parse_match_line_content_with_double_colon() { - let line = "externalImportShell.class.php:81: $this->queueProcessModel = ClassRegistry::init('Collections.QueueProcess');"; + let line = "externalImportShell.class.php\x0081: $this->queueProcessModel = ClassRegistry::init('Collections.QueueProcess');"; let (file, line_num, content) = parse_match_line(line).unwrap(); assert_eq!(file, "externalImportShell.class.php"); assert_eq!(line_num, 81); @@ -446,29 +446,53 @@ mod tests { } // Windows abs-path safety: drive letter + backslashes must not break the - // parser. Greedy regex on path picks the LAST `:digits:` boundary. + // parser. The NUL separator makes the file portion unambiguous. #[test] fn test_parse_match_line_windows_path() { - let line = r"C:\src\file.rs:42:fn main() {}"; + let line = "C:\\src\\file.rs\x0042:fn main() {}"; let (file, line_num, content) = parse_match_line(line).unwrap(); assert_eq!(file, r"C:\src\file.rs"); assert_eq!(line_num, 42); assert_eq!(content, "fn main() {}"); } + // Filenames containing `:digits:` (which would fool a greedy `:` parser) + // must still parse correctly under NUL separation. + #[test] + fn test_parse_match_line_filename_with_colons() { + let line = "badly_named:52:file.txt\x001:xxx"; + let (file, line_num, content) = parse_match_line(line).unwrap(); + assert_eq!(file, "badly_named:52:file.txt"); + assert_eq!(line_num, 1); + assert_eq!(content, "xxx"); + } + + // Content that itself contains `:digits:` (e.g. log lines, port numbers, + // line-number-like substrings) must not confuse the parser. + #[test] + fn test_parse_match_line_content_with_digit_colons() { + let line = "log.txt\x007:debug: counter is :42: now"; + let (file, line_num, content) = parse_match_line(line).unwrap(); + assert_eq!(file, "log.txt"); + assert_eq!(line_num, 7); + assert_eq!(content, "debug: counter is :42: now"); + } + #[test] fn test_parse_match_line_malformed_returns_none() { - // Single token, no colons (e.g. an rg context line written with `-`) + // No NUL separator (e.g. rg/grep invoked without --null/-Z, or a + // context line written with `-`). + assert!(parse_match_line("file.rs:1:content").is_none()); assert!(parse_match_line("not a match line").is_none()); - // Missing line number - assert!(parse_match_line("file.rs:fn foo()").is_none()); + // Missing line number after NUL + assert!(parse_match_line("file.rs\x00fn foo()").is_none()); // Empty assert!(parse_match_line("").is_none()); } #[test] fn test_parse_match_line_empty_content() { - let line = "file.rs:7:"; + let line = "file.rs\x007:"; let (file, line_num, content) = parse_match_line(line).unwrap(); assert_eq!(file, "file.rs"); assert_eq!(line_num, 7); From 2543be59126f5b6825b48570a5b5fb99d0112dab Mon Sep 17 00:00:00 2001 From: SAY-5 Date: Tue, 28 Apr 2026 03:29:11 -0700 Subject: [PATCH 06/39] fix(hook): collapse bash line continuations before matching (#1564) Closes #1564. `rtk hook check` rejected commands that began with a bash line continuation (`\`) because the matcher saw the literal `\` as the first token and bailed out: $ rtk hook check $'\\\ngit diff HEAD~1' No rewrite for: \\ngit diff HEAD~1 Bash collapses `\` (and `\`) plus surrounding horizontal whitespace to a single space before dispatching the command, so the matcher should do the same. Internal continuations (e.g. `git diff \HEAD~1`) are equally common when Claude Code formats long invocations. Add a small `collapse_line_continuations` helper that runs a single regex over the input before `cmd.trim()`. The regex eats the trailing whitespace on the previous line and the leading indentation on the next line so the no-double-space invariant holds. The fast path returns `Cow::Borrowed` when no continuation is present, so commands without `\` allocate nothing. Six regression tests pin: leading \\, leading \\, internal \\ between subcommand and args, \\ followed by indentation, the no-op fast path, and the helper-level Cow::Borrowed contract. --- src/discover/registry.rs | 95 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 93 insertions(+), 2 deletions(-) diff --git a/src/discover/registry.rs b/src/discover/registry.rs index bb7b11f2a..ecccc1d6f 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -440,6 +440,27 @@ fn strip_trailing_redirects(cmd: &str) -> (&str, &str) { (cmd_part, redir_part) } +lazy_static! { + /// Matches a bash line-continuation: a backslash immediately + /// followed by `\n` or `\r\n`, *plus* any horizontal whitespace on + /// the line before AND after the break. This is what bash already + /// collapses to a single space before executing the command — rtk's + /// hook matcher needs to do the same so commands authored across + /// multiple lines still hit the rewrite rules. Consuming the + /// trailing whitespace prevents double spaces in cases like + /// `git diff \HEAD~1`. + static ref LINE_CONTINUATION_RE: Regex = + Regex::new(r"[ \t]*\\(?:\r\n|\n)[ \t]*").unwrap(); +} + +/// Replace every bash line continuation with a single space, mirroring +/// what bash does before dispatching the command. Returns a borrowed +/// `&str` when the input contains no continuations, so the common +/// fast path allocates nothing. +fn collapse_line_continuations(s: &str) -> std::borrow::Cow<'_, str> { + LINE_CONTINUATION_RE.replace_all(s, " ") +} + /// Returns `None` if the command is unsupported or ignored (hook should pass through). /// /// Handles compound commands (`&&`, `||`, `;`) by rewriting each segment independently. @@ -449,7 +470,7 @@ fn strip_trailing_redirects(cmd: &str) -> (&str, &str) { /// (`[hooks].transparent_prefixes` in `config.toml`) before routing. /// /// A transparent prefix is a wrapper command that doesn't change *what* is -/// being run, only *how* it's run — e.g. `docker exec mycontainer`, +/// being run, only *how* it's run --- e.g. `docker exec mycontainer`, /// `direnv exec .`, `poetry run`, or `bundle exec`. Stripping it lets the inner /// command match a filter; the prefix is then re-prepended to the rewrite. The /// built-in [`SHELL_PREFIX_BUILTINS`] (`noglob`, `command`, `builtin`, `exec`, @@ -464,7 +485,13 @@ pub fn rewrite_command( excluded: &[String], transparent_prefixes: &[String], ) -> Option { - let trimmed = cmd.trim(); + // Bash line continuations (`\`, `\`) and the leading + // whitespace that follows are syntactically equivalent to a single + // space, but `cmd.trim()` does not unwrap them --- so a leading + // backslash-newline used to defeat the whole matcher. Normalize + // first, then trim. See issue #1564. + let normalized = collapse_line_continuations(cmd); + let trimmed = normalized.trim(); if trimmed.is_empty() { return None; } @@ -3884,4 +3911,68 @@ mod tests { Some("rtk git log | head | tail && rtk git status".into()) ); } + + // --- line-continuation handling (issue #1564) ------------------- + + #[test] + fn test_rewrite_leading_backslash_newline() { + // The exact reproduction from #1564: a leading `\` made + // the matcher see `\` as the command and bail out. + assert_eq!( + rewrite_command("\\\ngit diff HEAD~1", &[]), + Some("rtk git diff HEAD~1".into()) + ); + } + + #[test] + fn test_rewrite_leading_backslash_crlf() { + // CRLF line ending — same shape, Windows shells / Git Bash. + assert_eq!( + rewrite_command("\\\r\ngit diff HEAD~1", &[]), + Some("rtk git diff HEAD~1".into()) + ); + } + + #[test] + fn test_rewrite_internal_backslash_newline() { + // Embedded line continuation between subcommand and args: + // `git diff \HEAD~1` is exactly equivalent to + // `git diff HEAD~1` per bash semantics. + assert_eq!( + rewrite_command("git diff \\\nHEAD~1", &[]), + Some("rtk git diff HEAD~1".into()) + ); + } + + #[test] + fn test_rewrite_backslash_newline_with_indent() { + // Continuation followed by indentation — also collapsed. + assert_eq!( + rewrite_command("git \\\n diff HEAD~1", &[]), + Some("rtk git diff HEAD~1".into()) + ); + } + + #[test] + fn test_rewrite_no_line_continuation_unchanged() { + // Sanity check: a command without any `\` should match + // unchanged. This pins that the normalization step does not + // regress the no-op fast path. + assert_eq!( + rewrite_command("git diff HEAD~1", &[]), + Some("rtk git diff HEAD~1".into()) + ); + } + + #[test] + fn test_collapse_line_continuations_no_op() { + // Helper-level: no continuations → returns Borrowed (no + // allocation). We can only spot-check the equality here, but + // the `Cow::Borrowed` variant is implied by `replace_all` + // when no replacement occurs. + assert_eq!( + collapse_line_continuations("git diff HEAD~1"), + std::borrow::Cow::::Borrowed("git diff HEAD~1"), + ); + } } From 1875945bf32b3cc8f7f4c8da81427e17ded74c7f Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Mon, 25 May 2026 09:42:06 +0200 Subject: [PATCH 07/39] fix(pkg): rtk is Apache 2.0 and no MIT --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 8fc8a9305..0b5b82014 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ version = "0.40.0" edition = "2021" authors = ["Patrick Szymkowiak"] description = "Rust Token Killer - High-performance CLI proxy to minimize LLM token consumption" -license = "MIT" +license = "Apache 2.0" homepage = "https://www.rtk-ai.app" repository = "https://github.com/rtk-ai/rtk" readme = "README.md" From 9bbc7bb79e702ada58f5cfe2499d441715aa6bbd Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Mon, 25 May 2026 12:13:43 +0200 Subject: [PATCH 08/39] fix(docs): replace remaining MIT license references with Apache 2.0 --- Formula/rtk.rb | 2 +- README_es.md | 4 ++-- README_fr.md | 4 ++-- README_ja.md | 4 ++-- README_ko.md | 4 ++-- README_zh.md | 4 ++-- openclaw/README.md | 2 +- openclaw/openclaw.plugin.json | 2 +- openclaw/package.json | 2 +- 9 files changed, 14 insertions(+), 14 deletions(-) diff --git a/Formula/rtk.rb b/Formula/rtk.rb index 25a8b2e8a..34c27f9fb 100644 --- a/Formula/rtk.rb +++ b/Formula/rtk.rb @@ -7,7 +7,7 @@ class Rtk < Formula desc "High-performance CLI proxy to minimize LLM token consumption" homepage "https://www.rtk-ai.app" version "0.1.0" - license "MIT" + license "Apache-2.0" on_macos do on_intel do diff --git a/README_es.md b/README_es.md index 751bd1ba8..10a6a53a5 100644 --- a/README_es.md +++ b/README_es.md @@ -9,7 +9,7 @@

CI Release - License: MIT + License: Apache 2.0 Discord Homebrew

@@ -158,7 +158,7 @@ Unete a la comunidad en [Discord](https://discord.gg/RySmvNF5kF). ## Licencia -Licencia MIT - ver [LICENSE](LICENSE) para detalles. +Licencia Apache 2.0 - ver [LICENSE](LICENSE) para detalles. ## Descargo de responsabilidad diff --git a/README_fr.md b/README_fr.md index d305feaaf..2de284100 100644 --- a/README_fr.md +++ b/README_fr.md @@ -9,7 +9,7 @@

CI Release - License: MIT + License: Apache 2.0 Discord Homebrew

@@ -195,7 +195,7 @@ Rejoignez la communaute sur [Discord](https://discord.gg/RySmvNF5kF). ## Licence -Licence MIT - voir [LICENSE](LICENSE) pour les details. +Licence Apache 2.0 - voir [LICENSE](LICENSE) pour les details. ## Avertissement diff --git a/README_ja.md b/README_ja.md index 23bf4412f..ce946d628 100644 --- a/README_ja.md +++ b/README_ja.md @@ -9,7 +9,7 @@

CI Release - License: MIT + License: Apache 2.0 Discord Homebrew

@@ -157,7 +157,7 @@ rtk discover # 見逃した節約機会を発見 ## ライセンス -MIT ライセンス - 詳細は [LICENSE](LICENSE) を参照。 +Apache 2.0 ライセンス - 詳細は [LICENSE](LICENSE) を参照。 ## 免責事項 diff --git a/README_ko.md b/README_ko.md index a07a4590a..c969e0ed2 100644 --- a/README_ko.md +++ b/README_ko.md @@ -9,7 +9,7 @@

CI Release - License: MIT + License: Apache 2.0 Discord Homebrew

@@ -157,7 +157,7 @@ rtk discover # 놓친 절약 기회 발견 ## 라이선스 -MIT 라이선스 - 자세한 내용은 [LICENSE](LICENSE)를 참조하세요. +Apache 2.0 라이선스 - 자세한 내용은 [LICENSE](LICENSE)를 참조하세요. ## 면책 조항 diff --git a/README_zh.md b/README_zh.md index 854ca2314..ef8c53b10 100644 --- a/README_zh.md +++ b/README_zh.md @@ -9,7 +9,7 @@

CI Release - License: MIT + License: Apache 2.0 Discord Homebrew

@@ -165,7 +165,7 @@ rtk discover # 发现遗漏的节省机会 ## 许可证 -MIT 许可证 - 详见 [LICENSE](LICENSE)。 +Apache 2.0 许可证 - 详见 [LICENSE](LICENSE)。 ## 免责声明 diff --git a/openclaw/README.md b/openclaw/README.md index 301d7c0fa..480ec856f 100644 --- a/openclaw/README.md +++ b/openclaw/README.md @@ -83,4 +83,4 @@ Handled by `rtk rewrite` guards: ## License -MIT -- same as RTK. +Apache 2.0 -- same as RTK. diff --git a/openclaw/openclaw.plugin.json b/openclaw/openclaw.plugin.json index 3fce418d7..e08d11ba1 100644 --- a/openclaw/openclaw.plugin.json +++ b/openclaw/openclaw.plugin.json @@ -4,7 +4,7 @@ "version": "1.0.0", "description": "Transparently rewrites shell commands to their RTK equivalents for 60-90% LLM token savings", "homepage": "https://github.com/rtk-ai/rtk", - "license": "MIT", + "license": "Apache-2.0", "configSchema": { "type": "object", "additionalProperties": false, diff --git a/openclaw/package.json b/openclaw/package.json index 18d359ff4..aa0b099f6 100644 --- a/openclaw/package.json +++ b/openclaw/package.json @@ -3,7 +3,7 @@ "version": "1.0.0", "description": "RTK plugin for OpenClaw — rewrites shell commands for 60-90% LLM token savings", "main": "index.ts", - "license": "MIT", + "license": "Apache-2.0", "repository": { "type": "git", "url": "https://github.com/rtk-ai/rtk", From 6c4dfc727af0423318bf21a66d53474089d25bf4 Mon Sep 17 00:00:00 2001 From: Ryan A <7192401+ryana-0154@users.noreply.github.com> Date: Mon, 25 May 2026 12:46:23 +0100 Subject: [PATCH 09/39] fix(ls): preserve permission info as octal when -l/-la is passed Closes #1672 Co-Authored-By: Claude Opus 4.7 (1M context) --- src/cmds/system/ls.rs | 224 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 185 insertions(+), 39 deletions(-) diff --git a/src/cmds/system/ls.rs b/src/cmds/system/ls.rs index b96f84876..2c4bc6790 100644 --- a/src/cmds/system/ls.rs +++ b/src/cmds/system/ls.rs @@ -24,6 +24,19 @@ pub fn run(args: &[String], verbose: u8) -> Result { .iter() .any(|a| (a.starts_with('-') && !a.starts_with("--") && a.contains('a')) || a == "--all"); + // Per `man ls`, the long listing is triggered by `-l` and also implied by + // `-g`, `-n`, `-o`, and `--full-time`. In any of those cases we preserve + // permission info as octal. + let show_long = args.iter().any(|a| { + if a == "--full-time" { + return true; + } + if a.starts_with('-') && !a.starts_with("--") { + return a.chars().any(|c| matches!(c, 'l' | 'g' | 'n' | 'o')); + } + false + }); + let flags: Vec<&str> = args .iter() .filter(|a| a.starts_with('-')) @@ -74,7 +87,7 @@ pub fn run(args: &[String], verbose: u8) -> Result { "ls", &format!("-la {}", target_display), |raw| { - let (entries, summary, parsed_count) = compact_ls(raw, show_all); + let (entries, summary, parsed_count) = compact_ls(raw, show_all, show_long); // If no lines were parsed (e.g., unrecognized locale), fall back to raw output. // This is safer than returning "(empty)" for a non-empty directory. @@ -124,14 +137,17 @@ fn human_size(bytes: u64) -> String { } } -/// Parse a single `ls -la` line, returning `(file_type_char, size, name)`. +/// Parse a single `ls -la` line, returning `(file_type_char, perms, size, name)`. +/// +/// `perms` is the raw 10-char string from ls (e.g. `-rw-r--r--`); use +/// [`perms_to_octal`] to render it. /// /// Uses the date field as a stable anchor — the date format in `ls -la` is /// always three tokens (`Mon DD HH:MM` or `Mon DD YYYY`), so we locate it /// with a regex, then extract size (rightmost number before the date) and /// filename (everything after the date). This handles owner/group names that /// contain spaces, which break the old fixed-column approach. -fn parse_ls_line(line: &str) -> Option<(char, u64, String)> { +fn parse_ls_line(line: &str) -> Option<(char, String, u64, String)> { // Skip . and .. entries before date parsing (works for non-English locales too) if is_dotdir(line) { return None; @@ -146,7 +162,7 @@ fn parse_ls_line(line: &str) -> Option<(char, u64, String)> { return None; } - let perms = before_parts[0]; + let perms = before_parts[0].to_string(); let file_type = perms.chars().next()?; // Size is the rightmost parseable number before the date. @@ -160,7 +176,7 @@ fn parse_ls_line(line: &str) -> Option<(char, u64, String)> { } } - Some((file_type, size, name)) + Some((file_type, perms, size, name)) } /// Returns true if the line represents a . or .. directory entry. @@ -173,17 +189,60 @@ fn is_dotdir(line: &str) -> bool { line.trim().ends_with('.') || line.trim().ends_with("..") } -/// Parse ls -la output into compact format: -/// name/ (dirs) -/// name size (files) +/// Convert an `ls`-style permission string (e.g. `-rw-r--r--`, `drwxr-xr-x`, +/// `-rwsr-xr-t`) into octal notation (e.g. `644`, `755`, `4755`). +/// +/// Returns `None` if the input does not look like a permission field. +/// Special bits (setuid/setgid/sticky) are encoded as a leading 4th digit when +/// any are set; otherwise we emit a 3-digit value to stay compact. +fn perms_to_octal(perms: &str) -> Option { + if perms.len() < 10 || !perms.is_ascii() { + return None; + } + let b = perms.as_bytes(); + + fn perm_value(read: bool, write: bool, exec: bool) -> u32 { + ((read as u32) << 2) | ((write as u32) << 1) | (exec as u32) + } + + let owner_x = matches!(b[3], b'x' | b's'); + let group_x = matches!(b[6], b'x' | b's'); + let other_x = matches!(b[9], b'x' | b't'); + + let owner = perm_value(b[1] == b'r', b[2] == b'w', owner_x); + let group = perm_value(b[4] == b'r', b[5] == b'w', group_x); + let other = perm_value(b[7] == b'r', b[8] == b'w', other_x); + + let setuid = matches!(b[3], b's' | b'S'); + let setgid = matches!(b[6], b's' | b'S'); + let sticky = matches!(b[9], b't' | b'T'); + let special = perm_value(setuid, setgid, sticky); + + if special > 0 { + Some(format!("{}{}{}{}", special, owner, group, other)) + } else { + Some(format!("{}{}{}", owner, group, other)) + } +} + +/// Parse ls -la output into compact format. +/// +/// Without `show_long`: +/// name/ (dirs) +/// name size (files) +/// +/// With `show_long` (user passed `-l`): +/// 755 name/ (dirs) +/// 644 name size (files) +/// /// Returns (entries, summary, parsed_count) so caller can suppress summary when piped. /// parsed_count tracks how many non-header lines were successfully parsed. /// If parsed_count == 0 but raw had content, caller should fall back to raw output. -fn compact_ls(raw: &str, show_all: bool) -> (String, String, usize) { +fn compact_ls(raw: &str, show_all: bool, show_long: bool) -> (String, String, usize) { use std::collections::HashMap; - let mut dirs: Vec = Vec::new(); - let mut files: Vec<(String, String)> = Vec::new(); // (name, size) + let mut dirs: Vec<(String, Option)> = Vec::new(); // (name, octal_perms) + let mut files: Vec<(String, String, Option)> = Vec::new(); // (name, size, octal_perms) let mut by_ext: HashMap = HashMap::new(); let mut lines_seen: usize = 0; let mut parsed_count: usize = 0; @@ -195,7 +254,7 @@ fn compact_ls(raw: &str, show_all: bool) -> (String, String, usize) { } lines_seen += 1; - let Some((file_type, size, name)) = parse_ls_line(line) else { + let Some((file_type, perms, size, name)) = parse_ls_line(line) else { if is_dotdir(line) { dotdirs += 1; } @@ -208,8 +267,16 @@ fn compact_ls(raw: &str, show_all: bool) -> (String, String, usize) { continue; } + // Only parse perms when the user actually wants the long listing — + // skip the work otherwise. + let octal = if show_long { + perms_to_octal(&perms) + } else { + None + }; + if file_type == 'd' { - dirs.push(name); + dirs.push((name, octal)); } else { // Regular files, symlinks, character/block devices, pipes, sockets let ext = if let Some(pos) = name.rfind('.') { @@ -218,7 +285,7 @@ fn compact_ls(raw: &str, show_all: bool) -> (String, String, usize) { "no ext".to_string() }; *by_ext.entry(ext).or_insert(0) += 1; - files.push((name, human_size(size))); + files.push((name, human_size(size), octal)); } } @@ -237,13 +304,21 @@ fn compact_ls(raw: &str, show_all: bool) -> (String, String, usize) { let mut entries = String::new(); // Dirs first, compact - for d in &dirs { - entries.push_str(d); + for (name, octal) in &dirs { + if let Some(octal) = octal { + entries.push_str(octal); + entries.push_str(" "); + } + entries.push_str(name); entries.push_str("/\n"); } // Files with size - for (name, size) in &files { + for (name, size, octal) in &files { + if let Some(octal) = octal { + entries.push_str(octal); + entries.push_str(" "); + } entries.push_str(name); entries.push_str(" "); entries.push_str(size); @@ -286,7 +361,7 @@ mod tests { drwxr-xr-x 2 user staff 64 Jan 1 12:00 src\n\ -rw-r--r-- 1 user staff 1234 Jan 1 12:00 Cargo.toml\n\ -rw-r--r-- 1 user staff 5678 Jan 1 12:00 README.md\n"; - let (entries, _summary, _) = compact_ls(input, false); + let (entries, _summary, _parsed) = compact_ls(input, false, false); assert!(entries.contains("src/")); assert!(entries.contains("Cargo.toml")); assert!(entries.contains("README.md")); @@ -307,7 +382,7 @@ mod tests { drwxr-xr-x 2 user staff 64 Jan 1 12:00 target\n\ drwxr-xr-x 2 user staff 64 Jan 1 12:00 src\n\ -rw-r--r-- 1 user staff 100 Jan 1 12:00 main.rs\n"; - let (entries, _summary, _) = compact_ls(input, false); + let (entries, _summary, _parsed) = compact_ls(input, false, false); assert!(!entries.contains("node_modules")); assert!(!entries.contains(".git")); assert!(!entries.contains("target")); @@ -320,7 +395,7 @@ mod tests { let input = "total 8\n\ drwxr-xr-x 2 user staff 64 Jan 1 12:00 .git\n\ drwxr-xr-x 2 user staff 64 Jan 1 12:00 src\n"; - let (entries, _summary, _) = compact_ls(input, true); + let (entries, _summary, _parsed) = compact_ls(input, true, false); assert!(entries.contains(".git/")); assert!(entries.contains("src/")); } @@ -328,7 +403,7 @@ mod tests { #[test] fn test_compact_empty() { let input = "total 0\n"; - let (entries, summary, _) = compact_ls(input, false); + let (entries, summary, _parsed) = compact_ls(input, false, false); assert_eq!(entries, "(empty)\n"); assert!(summary.is_empty()); } @@ -338,7 +413,7 @@ mod tests { let input = "total 8\n\ drwxr-xr-x 2 user user 4096 1月 1 12:00 .\n\ drwxr-xr-x 16 user user 20480 1月 1 12:00 ..\n"; - let (entries, summary, parsed_count) = compact_ls(input, false); + let (entries, summary, parsed_count) = compact_ls(input, false, false); assert_eq!(parsed_count, 0); assert_eq!(entries, "(empty)\n"); assert!(summary.is_empty()); @@ -349,7 +424,7 @@ mod tests { let input = "total 0\n\ drwxr-xr-x 2 lumin wheel 64 Apr 23 00:37 .\n\ drwxr-xr-x 16 root wheel 164576 Apr 23 00:37 ..\n"; - let (entries, summary, parsed_count) = compact_ls(input, false); + let (entries, summary, parsed_count) = compact_ls(input, false, false); assert_eq!(parsed_count, 0); assert_eq!(entries, "(empty)\n"); assert!(summary.is_empty()); @@ -362,7 +437,7 @@ mod tests { -rw-r--r-- 1 user staff 1234 Jan 1 12:00 main.rs\n\ -rw-r--r-- 1 user staff 5678 Jan 1 12:00 lib.rs\n\ -rw-r--r-- 1 user staff 100 Jan 1 12:00 Cargo.toml\n"; - let (_entries, summary, _) = compact_ls(input, false); + let (_entries, summary, _parsed) = compact_ls(input, false, false); assert!(summary.contains("Summary: 3 files, 1 dirs")); assert!(summary.contains(".rs")); assert!(summary.contains(".toml")); @@ -382,7 +457,7 @@ mod tests { fn test_compact_handles_filenames_with_spaces() { let input = "total 8\n\ -rw-r--r-- 1 user staff 1234 Jan 1 12:00 my file.txt\n"; - let (entries, _summary, _) = compact_ls(input, false); + let (entries, _summary, _parsed) = compact_ls(input, false, false); assert!(entries.contains("my file.txt")); } @@ -390,7 +465,7 @@ mod tests { fn test_compact_symlinks() { let input = "total 8\n\ lrwxr-xr-x 1 user staff 10 Jan 1 12:00 link -> target\n"; - let (entries, _summary, _) = compact_ls(input, false); + let (entries, _summary, _parsed) = compact_ls(input, false, false); assert!(entries.contains("link -> target")); } @@ -400,7 +475,7 @@ mod tests { let input = "total 48\n\ drwxr-xr-x 2 user staff 64 Jan 1 12:00 src\n\ -rw-r--r-- 1 user staff 1234 Jan 1 12:00 main.rs\n"; - let (entries, summary, _) = compact_ls(input, false); + let (entries, summary, _parsed) = compact_ls(input, false, false); assert!( !entries.contains("Summary:"), "entries must not contain summary" @@ -419,7 +494,7 @@ mod tests { drwxr-xr-x 2 user staff 64 Jan 1 12:00 src\n\ -rw-r--r-- 1 user staff 1234 Jan 1 12:00 main.rs\n\ -rw-r--r-- 1 user staff 5678 Jan 1 12:00 lib.rs\n"; - let (entries, _summary, _) = compact_ls(input, false); + let (entries, _summary, _parsed) = compact_ls(input, false, false); let line_count = entries.lines().count(); assert_eq!( line_count, 3, @@ -434,7 +509,7 @@ mod tests { let input = "total 8\n\ -rw-r--r-- 1 fjeanne utilisa. du domaine 0 Mar 31 16:18 empty.txt\n\ -rw-r--r-- 1 fjeanne utilisa. du domaine 1234 Mar 31 16:18 data.json\n"; - let (entries, _summary, _) = compact_ls(input, false); + let (entries, _summary, _parsed) = compact_ls(input, false, false); assert!( entries.contains("empty.txt"), "should contain 'empty.txt', got: {entries}" @@ -462,7 +537,7 @@ mod tests { // Some systems show year instead of time for old files let input = "total 8\n\ -rw-r--r-- 1 user staff 5678 Dec 25 2024 archive.tar\n"; - let (entries, _summary, _) = compact_ls(input, false); + let (entries, _summary, _parsed) = compact_ls(input, false, false); assert!( entries.contains("archive.tar"), "should contain filename, got: {entries}" @@ -472,38 +547,42 @@ mod tests { #[test] fn test_parse_ls_line_basic() { - let (ft, size, name) = + let (ft, perms, size, name) = parse_ls_line("-rw-r--r-- 1 user staff 1234 Jan 1 12:00 file.txt").unwrap(); assert_eq!(ft, '-'); + assert_eq!(perms, "-rw-r--r--"); assert_eq!(size, 1234); assert_eq!(name, "file.txt"); } #[test] fn test_parse_ls_line_multiline_group() { - let (ft, size, name) = + let (ft, perms, size, name) = parse_ls_line("-rw-r--r-- 1 fjeanne utilisa. du domaine 0 Mar 31 16:18 empty.txt") .unwrap(); assert_eq!(ft, '-'); + assert_eq!(perms, "-rw-r--r--"); assert_eq!(size, 0); assert_eq!(name, "empty.txt"); } #[test] fn test_parse_ls_line_dir_with_space_in_group() { - let (ft, size, name) = + let (ft, perms, size, name) = parse_ls_line("drwxr-xr-x 2 fjeanne utilisa. du domaine 64 Mar 31 16:18 my dir") .unwrap(); assert_eq!(ft, 'd'); + assert_eq!(perms, "drwxr-xr-x"); assert_eq!(size, 64); assert_eq!(name, "my dir"); } #[test] fn test_parse_ls_line_symlink() { - let (ft, size, name) = + let (ft, perms, size, name) = parse_ls_line("lrwxr-xr-x 1 user staff 10 Jan 1 12:00 link -> target").unwrap(); assert_eq!(ft, 'l'); + assert_eq!(perms, "lrwxr-xr-x"); assert_eq!(size, 10); assert_eq!(name, "link -> target"); } @@ -513,7 +592,7 @@ mod tests { // Regression test for #844: `rtk ls /dev/ttyACM*` returned "(empty)" // because character devices (type 'c') were not handled by compact_ls. let input = "crw-rw---- 1 root dialout 166, 0 Apr 22 09:46 /dev/ttyACM0\n"; - let (entries, _summary, _parsed) = compact_ls(input, false); + let (entries, _summary, _parsed) = compact_ls(input, false, false); assert!( entries.contains("/dev/ttyACM0"), "should contain device file, got: {entries}" @@ -525,7 +604,7 @@ mod tests { fn test_compact_device_files_macos_hex_size() { // macOS shows device major/minor as hex (e.g. 0x2000000) let input = "crw-rw-rw- 1 root wheel 0x2000000 Mar 31 19:25 /dev/tty\n"; - let (entries, _summary, _parsed) = compact_ls(input, false); + let (entries, _summary, _parsed) = compact_ls(input, false, false); assert!( entries.contains("/dev/tty"), "should contain device file, got: {entries}" @@ -535,7 +614,7 @@ mod tests { #[test] fn test_compact_block_device() { let input = "brw-rw---- 1 root disk 8, 0 Apr 22 09:46 /dev/sda\n"; - let (entries, _summary, _parsed) = compact_ls(input, false); + let (entries, _summary, _parsed) = compact_ls(input, false, false); assert!( entries.contains("/dev/sda"), "should contain block device, got: {entries}" @@ -549,19 +628,86 @@ mod tests { #[test] fn test_parse_ls_line_year_format() { - let (ft, size, name) = + let (ft, perms, size, name) = parse_ls_line("-rw-r--r-- 1 user staff 5678 Dec 25 2024 old.tar.gz").unwrap(); assert_eq!(ft, '-'); + assert_eq!(perms, "-rw-r--r--"); assert_eq!(size, 5678); assert_eq!(name, "old.tar.gz"); } + #[test] + fn test_perms_to_octal_common() { + assert_eq!(perms_to_octal("-rw-r--r--").as_deref(), Some("644")); + assert_eq!(perms_to_octal("-rwxr-xr-x").as_deref(), Some("755")); + assert_eq!(perms_to_octal("drwxr-xr-x").as_deref(), Some("755")); + assert_eq!(perms_to_octal("-rw-------").as_deref(), Some("600")); + assert_eq!(perms_to_octal("-rwxrwxrwx").as_deref(), Some("777")); + assert_eq!(perms_to_octal("----------").as_deref(), Some("000")); + assert_eq!(perms_to_octal("lrwxr-xr-x").as_deref(), Some("755")); + } + + #[test] + fn test_perms_to_octal_special_bits() { + // setuid + 755 -> 4755 + assert_eq!(perms_to_octal("-rwsr-xr-x").as_deref(), Some("4755")); + // setuid without execute -> 4644 + assert_eq!(perms_to_octal("-rwSr--r--").as_deref(), Some("4644")); + // setgid + 755 -> 2755 + assert_eq!(perms_to_octal("-rwxr-sr-x").as_deref(), Some("2755")); + // sticky bit on /tmp-style dir -> 1777 + assert_eq!(perms_to_octal("drwxrwxrwt").as_deref(), Some("1777")); + // setuid + setgid + sticky + assert_eq!(perms_to_octal("-rwsrwsrwt").as_deref(), Some("7777")); + } + + #[test] + fn test_perms_to_octal_garbage() { + assert_eq!(perms_to_octal(""), None); + assert_eq!(perms_to_octal("short"), None); + } + + #[test] + fn test_compact_long_format_includes_octal() { + let input = "total 48\n\ + drwxr-xr-x 2 user staff 64 Jan 1 12:00 src\n\ + -rw-r--r-- 1 user staff 1234 Jan 1 12:00 Cargo.toml\n\ + -rwxr-xr-x 1 user staff 500 Jan 1 12:00 build.sh\n"; + let (entries, _summary, _parsed) = compact_ls(input, false, true); + assert!( + entries.contains("755 src/"), + "dir should be prefixed with octal perms, got: {entries}" + ); + assert!( + entries.contains("644 Cargo.toml 1.2K"), + "file should be prefixed with octal perms, got: {entries}" + ); + assert!( + entries.contains("755 build.sh 500B"), + "executable should show 755, got: {entries}" + ); + } + + #[test] + fn test_compact_short_format_omits_octal() { + // Without -l, no octal prefix even though we still parse `ls -la` + // under the hood. + let input = "total 48\n\ + -rw-r--r-- 1 user staff 1234 Jan 1 12:00 Cargo.toml\n"; + let (entries, _summary, _parsed) = compact_ls(input, false, false); + assert!( + !entries.contains("644"), + "short format must not include octal perms, got: {entries}" + ); + assert!(entries.contains("Cargo.toml")); + } + #[test] fn test_compact_chinese_locale_fallback() { let input = "total 8\n\ drwxr-xr-x 2 user staff 64 1月 1 12:00 src\n\ -rw-r--r-- 1 user staff 1234 1月 1 12:00 main.rs\n"; - let (entries, summary, parsed_count) = compact_ls(input, false); + let (entries, summary, parsed_count) = compact_ls(input, false, false); assert_eq!(parsed_count, 0); assert!(entries.is_empty()); assert!(summary.is_empty()); From 53b5e79561fdf98f09db0b5b2006ec838fcd06f5 Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Mon, 25 May 2026 14:12:16 +0200 Subject: [PATCH 10/39] feat: prefer short args and cleanup comments --- src/cmds/system/grep_cmd.rs | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/src/cmds/system/grep_cmd.rs b/src/cmds/system/grep_cmd.rs index dbd61364a..3f7d9a327 100644 --- a/src/cmds/system/grep_cmd.rs +++ b/src/cmds/system/grep_cmd.rs @@ -33,19 +33,10 @@ pub fn run( // Without this, rg returns 0 matches for files in .gitignore, causing // false negatives that make AI agents draw wrong conclusions. // Using --no-ignore-vcs (not --no-ignore) so .ignore/.rgignore are still respected. - // - // -H: force rg to always include the filename, even for single-file searches. - // --null: separate file from line:content with a NUL byte so the parser is - // unambiguous even when the filename or content contains `:digits:` - // patterns (issue #1436). - rg_cmd.args([ - "-nH", - "--no-heading", - "--null", - "--no-ignore-vcs", - &rg_pattern, - path, - ]); + // -H: always emit the filename. + // -0: NUL-separate filename. Allows the parser to disambiguate filenames or + // content containing `:digits:` patterns (issue #1436). + rg_cmd.args(["-nH0", "--no-heading", "--no-ignore-vcs", &rg_pattern, path]); if let Some(ft) = file_type { rg_cmd.arg("--type").arg(ft); @@ -62,9 +53,7 @@ pub fn run( let result = exec_capture(&mut rg_cmd) .or_else(|_| { let mut grep_cmd = resolved_command("grep"); - // -H: always emit the filename; -Z: NUL-separate filename from - // line:content so the parser can disambiguate even for filenames - // or content containing `:digits:` (parity with rg's --null). + // When we fall back to grep, include all args, not just -rnHZ. grep_cmd.args(["-rnHZ", pattern, path]).args(extra_args); exec_capture(&mut grep_cmd) }) @@ -173,9 +162,9 @@ pub fn run( /// Parses a single rg/grep match line of the form `file\0line_number:content`. /// -/// Requires the underlying command to be invoked with `--null` (rg) or `-Z` -/// (grep) so the filename is NUL-separated from `line:content`. NUL cannot -/// appear in file paths, so the parser is unambiguous regardless of: +/// Requires the underlying command to be invoked with `-0` (rg) or `-Z` (grep) +/// so the filename is NUL-separated from `line:content`. NUL cannot appear in +/// file paths, so the parser is unambiguous regardless of: /// - content with `:` or `::` (e.g. `ClassRegistry::init(...)`, issue #1436); /// - paths with embedded `:` (Windows drive letters, weird filenames like /// `badly_named:52:file.txt`). From 8fcc0dc56c0bbcdf0508bbf72897085348884fd5 Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Mon, 25 May 2026 14:55:32 +0200 Subject: [PATCH 11/39] fix: handle GNU `--format=long` and `--format=verbose` long args --- src/cmds/system/ls.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cmds/system/ls.rs b/src/cmds/system/ls.rs index 2c4bc6790..98eb36447 100644 --- a/src/cmds/system/ls.rs +++ b/src/cmds/system/ls.rs @@ -25,10 +25,10 @@ pub fn run(args: &[String], verbose: u8) -> Result { .any(|a| (a.starts_with('-') && !a.starts_with("--") && a.contains('a')) || a == "--all"); // Per `man ls`, the long listing is triggered by `-l` and also implied by - // `-g`, `-n`, `-o`, and `--full-time`. In any of those cases we preserve - // permission info as octal. + // `-g`, `-n`, `-o`, `--full-time` or GNU `--format=long` and `--format=verbose`. + // In any of those cases we preserve permission info as octal. let show_long = args.iter().any(|a| { - if a == "--full-time" { + if a == "--full-time" || a == "--format=long" || a == "--format=verbose" { return true; } if a.starts_with('-') && !a.starts_with("--") { From 42ac86ce2f5bedb011f84532c2112cd5093500ab Mon Sep 17 00:00:00 2001 From: Sai Asish Y Date: Sun, 24 May 2026 19:14:56 -0700 Subject: [PATCH 12/39] fix(hook): use maintainer regex suggestion for line continuation Signed-off-by: Sai Asish Y --- src/discover/registry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/discover/registry.rs b/src/discover/registry.rs index ecccc1d6f..46f28f234 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -450,7 +450,7 @@ lazy_static! { /// trailing whitespace prevents double spaces in cases like /// `git diff \HEAD~1`. static ref LINE_CONTINUATION_RE: Regex = - Regex::new(r"[ \t]*\\(?:\r\n|\n)[ \t]*").unwrap(); + Regex::new(r"(?m)[ \t\x0B\x0C]*\\\r?\n[ \t\x0B\x0C]*").unwrap(); } /// Replace every bash line continuation with a single space, mirroring From 0ec99de8f4ef102c10a827b57636278080e2f3cf Mon Sep 17 00:00:00 2001 From: Sai Asish Y Date: Mon, 25 May 2026 06:16:18 -0700 Subject: [PATCH 13/39] fix(tests): use rewrite_command_no_prefixes in line-continuation tests Signed-off-by: Sai Asish Y --- src/discover/registry.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/discover/registry.rs b/src/discover/registry.rs index 46f28f234..df1d50047 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -3919,7 +3919,7 @@ mod tests { // The exact reproduction from #1564: a leading `\` made // the matcher see `\` as the command and bail out. assert_eq!( - rewrite_command("\\\ngit diff HEAD~1", &[]), + rewrite_command_no_prefixes("\\\ngit diff HEAD~1", &[]), Some("rtk git diff HEAD~1".into()) ); } @@ -3928,7 +3928,7 @@ mod tests { fn test_rewrite_leading_backslash_crlf() { // CRLF line ending — same shape, Windows shells / Git Bash. assert_eq!( - rewrite_command("\\\r\ngit diff HEAD~1", &[]), + rewrite_command_no_prefixes("\\\r\ngit diff HEAD~1", &[]), Some("rtk git diff HEAD~1".into()) ); } @@ -3939,7 +3939,7 @@ mod tests { // `git diff \HEAD~1` is exactly equivalent to // `git diff HEAD~1` per bash semantics. assert_eq!( - rewrite_command("git diff \\\nHEAD~1", &[]), + rewrite_command_no_prefixes("git diff \\\nHEAD~1", &[]), Some("rtk git diff HEAD~1".into()) ); } @@ -3948,7 +3948,7 @@ mod tests { fn test_rewrite_backslash_newline_with_indent() { // Continuation followed by indentation — also collapsed. assert_eq!( - rewrite_command("git \\\n diff HEAD~1", &[]), + rewrite_command_no_prefixes("git \\\n diff HEAD~1", &[]), Some("rtk git diff HEAD~1".into()) ); } @@ -3959,7 +3959,7 @@ mod tests { // unchanged. This pins that the normalization step does not // regress the no-op fast path. assert_eq!( - rewrite_command("git diff HEAD~1", &[]), + rewrite_command_no_prefixes("git diff HEAD~1", &[]), Some("rtk git diff HEAD~1".into()) ); } From 3c356b32416be0815bee9e4364c20c4efa15d829 Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Mon, 25 May 2026 15:21:27 +0200 Subject: [PATCH 14/39] fix: remove remaining noise and rework comments --- src/discover/registry.rs | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/src/discover/registry.rs b/src/discover/registry.rs index df1d50047..4fd716828 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -441,22 +441,20 @@ fn strip_trailing_redirects(cmd: &str) -> (&str, &str) { } lazy_static! { - /// Matches a bash line-continuation: a backslash immediately - /// followed by `\n` or `\r\n`, *plus* any horizontal whitespace on - /// the line before AND after the break. This is what bash already - /// collapses to a single space before executing the command — rtk's - /// hook matcher needs to do the same so commands authored across - /// multiple lines still hit the rewrite rules. Consuming the - /// trailing whitespace prevents double spaces in cases like + /// Matches a bash line-continuation: a backslash immediately followed by + /// `\n` or `\r\n`, *plus* any horizontal whitespace on the line before AND + /// after the break. This is what bash already collapses to a single space + /// before executing the command — rtk's hook matcher needs to do the same + /// so commands authored across multiple lines still hit the rewrite rules. + /// Consuming the trailing whitespace prevents double spaces in cases like /// `git diff \HEAD~1`. static ref LINE_CONTINUATION_RE: Regex = Regex::new(r"(?m)[ \t\x0B\x0C]*\\\r?\n[ \t\x0B\x0C]*").unwrap(); } -/// Replace every bash line continuation with a single space, mirroring -/// what bash does before dispatching the command. Returns a borrowed -/// `&str` when the input contains no continuations, so the common -/// fast path allocates nothing. +/// Replace every bash line continuation with a single space, mirroring what +/// bash does before dispatching the command. Returns a borrowed `&str` when the +/// input contains no continuations, so the common fast path allocates nothing. fn collapse_line_continuations(s: &str) -> std::borrow::Cow<'_, str> { LINE_CONTINUATION_RE.replace_all(s, " ") } @@ -470,7 +468,7 @@ fn collapse_line_continuations(s: &str) -> std::borrow::Cow<'_, str> { /// (`[hooks].transparent_prefixes` in `config.toml`) before routing. /// /// A transparent prefix is a wrapper command that doesn't change *what* is -/// being run, only *how* it's run --- e.g. `docker exec mycontainer`, +/// being run, only *how* it's run — e.g. `docker exec mycontainer`, /// `direnv exec .`, `poetry run`, or `bundle exec`. Stripping it lets the inner /// command match a filter; the prefix is then re-prepended to the rewrite. The /// built-in [`SHELL_PREFIX_BUILTINS`] (`noglob`, `command`, `builtin`, `exec`, @@ -485,11 +483,10 @@ pub fn rewrite_command( excluded: &[String], transparent_prefixes: &[String], ) -> Option { - // Bash line continuations (`\`, `\`) and the leading - // whitespace that follows are syntactically equivalent to a single - // space, but `cmd.trim()` does not unwrap them --- so a leading - // backslash-newline used to defeat the whole matcher. Normalize - // first, then trim. See issue #1564. + // Bash line continuations (`\`, `\`) and the leading whitespace that + // follows are syntactically equivalent to a single space, but `cmd.trim()` does + // not unwrap them so a leading backslash-newline used to defeat the whole matcher. + // Normalize first, then trim. See issue #1564. let normalized = collapse_line_continuations(cmd); let trimmed = normalized.trim(); if trimmed.is_empty() { From e132896d3f3b588813f59790a5c2f7d35e40cb78 Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Mon, 25 May 2026 21:42:07 +0200 Subject: [PATCH 15/39] fix(cicd): MIT to Apache 2.0 --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 06cb022df..d18a3b357 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -291,7 +291,7 @@ jobs: desc "Rust Token Killer - High-performance CLI proxy to minimize LLM token consumption" homepage "https://www.rtk-ai.app" version "VERSION_PLACEHOLDER" - license "MIT" + license "Apache 2.0" if OS.mac? && Hardware::CPU.arm? url "https://github.com/rtk-ai/rtk/releases/download/TAG_PLACEHOLDER/rtk-aarch64-apple-darwin.tar.gz" From 8fb29e2f75a34d9430be74975cccd30213e619aa Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Mon, 25 May 2026 22:15:35 +0200 Subject: [PATCH 16/39] fix(hook): emit Copilot CLI-compatible hook config + strip BOM on Windows --- .../guide/getting-started/supported-agents.md | 6 +- src/hooks/constants.rs | 4 ++ src/hooks/hook_cmd.rs | 23 +++++++- src/hooks/init.rs | 58 +++++++++++++++++-- 4 files changed, 83 insertions(+), 8 deletions(-) diff --git a/docs/guide/getting-started/supported-agents.md b/docs/guide/getting-started/supported-agents.md index b7fb920de..ede38bcac 100644 --- a/docs/guide/getting-started/supported-agents.md +++ b/docs/guide/getting-started/supported-agents.md @@ -66,12 +66,14 @@ rtk init --global --cursor Restart Cursor. The hook uses `preToolUse` with Cursor's `updated_input` format. -### VS Code Copilot Chat +### GitHub Copilot (VS Code Chat + CLI) ```bash -rtk init --global --copilot +rtk init --copilot ``` +Writes one `.github/hooks/rtk-rewrite.json` serving both hosts: VS Code Copilot Chat reads its `PreToolUse` entry (transparent rewrite), GitHub Copilot CLI reads its `preToolUse` entry (deny-with-suggestion; the agent retries with `rtk`). + ### Gemini CLI ```bash diff --git a/src/hooks/constants.rs b/src/hooks/constants.rs index 506e88cdf..f9a692264 100644 --- a/src/hooks/constants.rs +++ b/src/hooks/constants.rs @@ -22,6 +22,10 @@ pub const CURSOR_DIR: &str = ".cursor"; pub const CODEX_DIR: &str = ".codex"; pub const GEMINI_DIR: &str = ".gemini"; +pub const GITHUB_DIR: &str = ".github"; +pub const COPILOT_HOOK_FILE: &str = "rtk-rewrite.json"; +pub const COPILOT_INSTRUCTIONS_FILE: &str = "copilot-instructions.md"; + pub const PI_DIR: &str = ".pi/agent"; pub const PI_LOCAL_DIR: &str = ".pi"; pub const PI_EXTENSIONS_SUBDIR: &str = "extensions"; diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index 36825e3c5..2f29a2d7c 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -42,7 +42,9 @@ enum HookFormat { pub fn run_copilot() -> Result<()> { let input = read_stdin_limited()?; - let input = input.trim(); + // Strip leading BOM(s) before trimming: some Windows hosts prepend UTF-8 + // BOMs to hook stdin (confirmed for Cursor), which serde_json rejects. + let input = strip_leading_bom(&input).trim(); if input.is_empty() { return Ok(()); } @@ -575,6 +577,25 @@ mod tests { assert!(matches!(detect_format(&v), HookFormat::PassThrough)); } + #[test] + fn test_copilot_bom_prefixed_payload_is_recognized() { + // Windows hosts may prepend one or two UTF-8 BOMs to hook stdin + // (confirmed for Cursor). run_copilot strips them before parsing; + // verify both Copilot formats still parse after the same handling. + for raw in [ + format!("\u{feff}{}", copilot_cli_input("git status")), + format!("\u{feff}\u{feff}{}", copilot_cli_input("git status")), + ] { + let cleaned = strip_leading_bom(&raw).trim(); + let v: Value = serde_json::from_str(cleaned).expect("BOM-stripped JSON must parse"); + assert!(matches!(detect_format(&v), HookFormat::CopilotCli { .. })); + } + + let raw = format!("\u{feff}{}", vscode_input("Bash", "git status")); + let v: Value = serde_json::from_str(strip_leading_bom(&raw).trim()).unwrap(); + assert!(matches!(detect_format(&v), HookFormat::VsCode { .. })); + } + #[test] fn test_detect_unknown_is_passthrough() { assert!(matches!(detect_format(&json!({})), HookFormat::PassThrough)); diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 189f5de55..3f57cf6ae 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -8,7 +8,8 @@ use std::path::{Path, PathBuf}; use tempfile::NamedTempFile; use crate::hooks::constants::{ - CONFIG_DIR, CURSOR_DIR, GEMINI_DIR, OPENCODE_PLUGIN_FILE, OPENCODE_SUBDIR, PLUGIN_SUBDIR, + CONFIG_DIR, COPILOT_HOOK_FILE, COPILOT_INSTRUCTIONS_FILE, CURSOR_DIR, GEMINI_DIR, GITHUB_DIR, + OPENCODE_PLUGIN_FILE, OPENCODE_SUBDIR, PLUGIN_SUBDIR, }; use super::constants::{ @@ -3845,7 +3846,9 @@ fn uninstall_gemini(ctx: InitContext) -> Result> { // ── Copilot integration ───────────────────────────────────── +// PreToolUse = VS Code schema, preToolUse = Copilot CLI schema (same file, both hosts). const COPILOT_HOOK_JSON: &str = r#"{ + "version": 1, "hooks": { "PreToolUse": [ { @@ -3854,6 +3857,15 @@ const COPILOT_HOOK_JSON: &str = r#"{ "cwd": ".", "timeout": 5 } + ], + "preToolUse": [ + { + "type": "command", + "bash": "rtk hook copilot", + "powershell": "rtk hook copilot", + "cwd": ".", + "timeoutSec": 5 + } ] } } @@ -3901,8 +3913,8 @@ pub fn run_copilot(ctx: InitContext) -> Result<()> { /// `cargo test`'s default parallel execution). fn run_copilot_at(base: &Path, ctx: InitContext) -> Result<()> { let InitContext { dry_run, .. } = ctx; - let github_dir = base.join(".github"); - let hooks_dir = github_dir.join("hooks"); + let github_dir = base.join(GITHUB_DIR); + let hooks_dir = github_dir.join(HOOKS_SUBDIR); if !dry_run { fs::create_dir_all(&hooks_dir) @@ -3912,7 +3924,7 @@ fn run_copilot_at(base: &Path, ctx: InitContext) -> Result<()> { // 1. Upsert RTK marker block in copilot-instructions.md (preserves user content). // Done BEFORE writing the hook config so a malformed file aborts the install // without leaving a stale hook on disk. - let instructions_path = github_dir.join("copilot-instructions.md"); + let instructions_path = github_dir.join(COPILOT_INSTRUCTIONS_FILE); write_rtk_block( &instructions_path, COPILOT_INSTRUCTIONS, @@ -3922,7 +3934,7 @@ fn run_copilot_at(base: &Path, ctx: InitContext) -> Result<()> { )?; // 2. Write hook config (only reached if the upsert above succeeded). - let hook_path = hooks_dir.join("rtk-rewrite.json"); + let hook_path = hooks_dir.join(COPILOT_HOOK_FILE); write_if_changed(&hook_path, COPILOT_HOOK_JSON, "Copilot hook config", ctx)?; if dry_run { @@ -6254,6 +6266,42 @@ mod tests { assert!(content.contains("rtk cargo test")); } + #[test] + fn test_copilot_hook_json_serves_both_vscode_and_cli_schemas() { + let v: serde_json::Value = serde_json::from_str(COPILOT_HOOK_JSON).unwrap(); + + let vscode = &v["hooks"]["PreToolUse"][0]; + assert_eq!(vscode["command"], "rtk hook copilot"); + assert!(vscode["timeout"].is_number(), "VS Code uses `timeout`"); + + assert_eq!(v["version"], 1, "Copilot CLI requires top-level version"); + let cli = &v["hooks"]["preToolUse"][0]; + assert_eq!(cli["bash"], "rtk hook copilot"); + assert_eq!(cli["powershell"], "rtk hook copilot"); + assert!( + cli["timeoutSec"].is_number(), + "Copilot CLI uses `timeoutSec`" + ); + } + + #[test] + fn test_copilot_init_writes_dual_schema_to_disk() { + let temp = TempDir::new().unwrap(); + run_copilot_at(temp.path(), InitContext::default()).unwrap(); + + let hook_path = temp + .path() + .join(".github") + .join("hooks") + .join("rtk-rewrite.json"); + let v: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&hook_path).unwrap()).unwrap(); + + assert_eq!(v["hooks"]["PreToolUse"][0]["command"], "rtk hook copilot"); + assert_eq!(v["version"], 1); + assert_eq!(v["hooks"]["preToolUse"][0]["bash"], "rtk hook copilot"); + } + #[test] fn test_copilot_init_refuses_malformed_block() { let temp = TempDir::new().unwrap(); From 84b703d7d02d0fa9b25ab9a39ccc467da2eba440 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Mon, 25 May 2026 22:17:27 +0200 Subject: [PATCH 17/39] feat(hook): wire Copilot into uninstall, telemetry, and discover --- .../guide/getting-started/supported-agents.md | 8 + src/core/telemetry.rs | 10 +- src/discover/report.rs | 64 +++++++- src/hooks/init.rs | 142 ++++++++++++++++++ src/main.rs | 2 + 5 files changed, 219 insertions(+), 7 deletions(-) diff --git a/docs/guide/getting-started/supported-agents.md b/docs/guide/getting-started/supported-agents.md index ede38bcac..5007c10be 100644 --- a/docs/guide/getting-started/supported-agents.md +++ b/docs/guide/getting-started/supported-agents.md @@ -74,6 +74,14 @@ rtk init --copilot Writes one `.github/hooks/rtk-rewrite.json` serving both hosts: VS Code Copilot Chat reads its `PreToolUse` entry (transparent rewrite), GitHub Copilot CLI reads its `preToolUse` entry (deny-with-suggestion; the agent retries with `rtk`). +Uninstall (project-scoped): + +```bash +rtk init --uninstall --copilot +``` + +Removes `.github/hooks/rtk-rewrite.json` and the RTK block from `.github/copilot-instructions.md` (your own content is preserved). + ### Gemini CLI ```bash diff --git a/src/core/telemetry.rs b/src/core/telemetry.rs index 97e057be1..426c1e1fc 100644 --- a/src/core/telemetry.rs +++ b/src/core/telemetry.rs @@ -368,11 +368,14 @@ fn detect_hook_type() -> String { } } - // Check project-level hooks + // Check project-level hooks (Claude script + project-scoped Copilot config) if let Ok(cwd) = std::env::current_dir() { if cwd.join(".claude/hooks/rtk-rewrite.sh").exists() { return "claude".to_string(); } + if cwd.join(".github/hooks/rtk-rewrite.json").exists() { + return "copilot".to_string(); + } } "none".to_string() @@ -571,7 +574,7 @@ mod tests { assert!(stats.low_savings_commands.len() <= 5); assert!((0.0..=100.0).contains(&stats.avg_savings_per_command)); assert!( - ["claude", "gemini", "codex", "cursor", "none", "unknown"] + ["claude", "gemini", "codex", "cursor", "copilot", "none", "unknown"] .iter() .any(|&h| stats.hook_type.starts_with(h)), "Unexpected hook type: {}", @@ -583,7 +586,8 @@ mod tests { fn test_detect_hook_type_returns_known() { let ht = detect_hook_type(); assert!( - ["claude", "gemini", "codex", "cursor", "none", "unknown"].contains(&ht.as_str()), + ["claude", "gemini", "codex", "cursor", "copilot", "none", "unknown"] + .contains(&ht.as_str()), "Unexpected hook type: {}", ht ); diff --git a/src/discover/report.rs b/src/discover/report.rs index c2c523e24..af4dd453d 100644 --- a/src/discover/report.rs +++ b/src/discover/report.rs @@ -1,8 +1,8 @@ //! Data types for reporting which commands RTK can and cannot optimize. use crate::hooks::constants::{ - CURSOR_DIR, HERMES_DIR, HERMES_PLUGINS_SUBDIR, HERMES_PLUGIN_MANIFEST_FILE, HERMES_PLUGIN_NAME, - HOOKS_SUBDIR, REWRITE_HOOK_FILE, + COPILOT_HOOK_FILE, CURSOR_DIR, GITHUB_DIR, HERMES_DIR, HERMES_PLUGINS_SUBDIR, + HERMES_PLUGIN_MANIFEST_FILE, HERMES_PLUGIN_NAME, HOOKS_SUBDIR, REWRITE_HOOK_FILE, }; use serde::Serialize; use std::path::Path; @@ -52,13 +52,19 @@ pub struct UnsupportedEntry { pub struct AgentIntegrationStatus { pub cursor_hook_installed: bool, pub hermes_plugin_installed: bool, + pub copilot_hook_installed: bool, } impl AgentIntegrationStatus { pub fn detect() -> Self { - dirs::home_dir() + let mut status = dirs::home_dir() .map(|home| Self::detect_from_home(&home)) - .unwrap_or_default() + .unwrap_or_default(); + // Copilot is project-scoped (.github/hooks/), unlike the home-based agents. + status.copilot_hook_installed = std::env::current_dir() + .map(|cwd| Self::copilot_hook_installed_in(&cwd)) + .unwrap_or(false); + status } fn detect_from_home(home: &Path) -> Self { @@ -74,8 +80,16 @@ impl AgentIntegrationStatus { .join(HERMES_PLUGIN_NAME) .join(HERMES_PLUGIN_MANIFEST_FILE) .is_file(), + copilot_hook_installed: false, } } + + fn copilot_hook_installed_in(dir: &Path) -> bool { + dir.join(GITHUB_DIR) + .join(HOOKS_SUBDIR) + .join(COPILOT_HOOK_FILE) + .exists() + } } /// Full discover report. @@ -221,6 +235,10 @@ fn append_agent_notes(out: &mut String, status: AgentIntegrationStatus) { if status.hermes_plugin_installed { out.push_str("\nNote: Hermes plugin is installed; Hermes sessions are tracked via `rtk gain` (discover scans Claude Code only)\n"); } + + if status.copilot_hook_installed { + out.push_str("\nNote: GitHub Copilot sessions are tracked via `rtk gain` (discover scans Claude Code only)\n"); + } } /// Format report as JSON. @@ -362,6 +380,7 @@ mod tests { report.agent_status = AgentIntegrationStatus { cursor_hook_installed: true, hermes_plugin_installed: true, + copilot_hook_installed: true, }; let output = format_json(&report); @@ -369,5 +388,42 @@ mod tests { assert_eq!(json["agent_status"]["cursor_hook_installed"], true); assert_eq!(json["agent_status"]["hermes_plugin_installed"], true); + assert_eq!(json["agent_status"]["copilot_hook_installed"], true); + } + + #[test] + fn test_agent_status_detects_copilot_hook_in_project() { + let temp = tempfile::tempdir().unwrap(); + let hook = temp + .path() + .join(GITHUB_DIR) + .join(HOOKS_SUBDIR) + .join(COPILOT_HOOK_FILE); + std::fs::create_dir_all(hook.parent().unwrap()).unwrap(); + std::fs::write(&hook, "{}").unwrap(); + + assert!(AgentIntegrationStatus::copilot_hook_installed_in( + temp.path() + )); + assert!(!AgentIntegrationStatus::copilot_hook_installed_in( + tempfile::tempdir().unwrap().path() + )); + } + + #[test] + fn test_format_text_reports_copilot_detected() { + let mut report = make_report(0, 0); + report.agent_status = AgentIntegrationStatus { + copilot_hook_installed: true, + ..AgentIntegrationStatus::default() + }; + + let output = format_text(&report, 10, false); + + assert!( + output.contains("GitHub Copilot sessions are tracked via `rtk gain`"), + "Expected Copilot note in output but got:\n{}", + output + ); } } diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 3f57cf6ae..ac0e4ab57 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -3951,6 +3951,82 @@ fn run_copilot_at(base: &Path, ctx: InitContext) -> Result<()> { Ok(()) } +/// Entry point for `rtk init --uninstall --copilot` (project-scoped, like install). +pub fn uninstall_copilot(ctx: InitContext) -> Result<()> { + let InitContext { dry_run, .. } = ctx; + let removed = uninstall_copilot_at(Path::new("."), ctx)?; + + if removed.is_empty() { + println!("RTK Copilot support was not installed (nothing to remove)"); + } else { + let header = if dry_run { + "[dry-run] would uninstall RTK (GitHub Copilot):" + } else { + "RTK uninstalled (GitHub Copilot):" + }; + println!("{}", header); + for item in &removed { + println!(" - {}", item); + } + if !dry_run { + println!("\nRestart your IDE or Copilot CLI session to apply changes."); + } + } + + if dry_run { + print_dry_run_footer(); + } + Ok(()) +} + +/// Same as [`uninstall_copilot`] but operates relative to an explicit base path. +fn uninstall_copilot_at(base: &Path, ctx: InitContext) -> Result> { + let InitContext { dry_run, .. } = ctx; + let github_dir = base.join(GITHUB_DIR); + let mut removed = Vec::new(); + + let hook_path = github_dir.join(HOOKS_SUBDIR).join(COPILOT_HOOK_FILE); + if hook_path.exists() { + if dry_run { + println!( + "[dry-run] would remove hook config: {}", + hook_path.display() + ); + } else { + fs::remove_file(&hook_path) + .with_context(|| format!("Failed to remove hook: {}", hook_path.display()))?; + } + removed.push(format!("Hook config: {}", hook_path.display())); + } + + let instructions_path = github_dir.join(COPILOT_INSTRUCTIONS_FILE); + if instructions_path.exists() { + let content = fs::read_to_string(&instructions_path) + .with_context(|| format!("Failed to read {}", instructions_path.display()))?; + if content.contains(RTK_BLOCK_START) { + let (cleaned, did_remove) = remove_rtk_block(&content); + if did_remove { + if dry_run { + println!( + "[dry-run] would remove rtk-instructions block from {}", + instructions_path.display() + ); + } else { + atomic_write(&instructions_path, &cleaned).with_context(|| { + format!("Failed to write {}", instructions_path.display()) + })?; + } + removed.push(format!( + "{}: removed rtk-instructions block", + COPILOT_INSTRUCTIONS_FILE + )); + } + } + } + + Ok(removed) +} + #[cfg(test)] mod tests { use super::*; @@ -6302,6 +6378,72 @@ mod tests { assert_eq!(v["hooks"]["preToolUse"][0]["bash"], "rtk hook copilot"); } + #[test] + fn test_copilot_uninstall_removes_hook_and_block() { + let temp = TempDir::new().unwrap(); + run_copilot_at(temp.path(), InitContext::default()).unwrap(); + + let hook_path = temp + .path() + .join(".github") + .join("hooks") + .join("rtk-rewrite.json"); + let instructions_path = temp.path().join(".github").join("copilot-instructions.md"); + assert!(hook_path.exists()); + + let removed = uninstall_copilot_at(temp.path(), InitContext::default()).unwrap(); + + assert!(!removed.is_empty()); + assert!(!hook_path.exists(), "hook config must be removed"); + let instructions = fs::read_to_string(&instructions_path).unwrap(); + assert!( + !instructions.contains(RTK_BLOCK_START), + "RTK block must be removed" + ); + } + + #[test] + fn test_copilot_uninstall_preserves_user_instructions() { + let temp = TempDir::new().unwrap(); + let github_dir = temp.path().join(".github"); + fs::create_dir_all(&github_dir).unwrap(); + let instructions_path = github_dir.join("copilot-instructions.md"); + fs::write(&instructions_path, "# My rules\n\nAlways use pnpm.\n").unwrap(); + + run_copilot_at(temp.path(), InitContext::default()).unwrap(); + uninstall_copilot_at(temp.path(), InitContext::default()).unwrap(); + + let after = fs::read_to_string(&instructions_path).unwrap(); + assert!(after.contains("Always use pnpm."), "user content preserved"); + assert!(!after.contains(RTK_BLOCK_START), "RTK block removed"); + } + + #[test] + fn test_copilot_uninstall_dry_run_keeps_files() { + let temp = TempDir::new().unwrap(); + run_copilot_at(temp.path(), InitContext::default()).unwrap(); + let hook_path = temp + .path() + .join(".github") + .join("hooks") + .join("rtk-rewrite.json"); + + let ctx = InitContext { + verbose: 0, + dry_run: true, + }; + uninstall_copilot_at(temp.path(), ctx).unwrap(); + + assert!(hook_path.exists(), "dry-run must not remove hook config"); + } + + #[test] + fn test_copilot_uninstall_nothing_when_absent() { + let temp = TempDir::new().unwrap(); + let removed = uninstall_copilot_at(temp.path(), InitContext::default()).unwrap(); + assert!(removed.is_empty(), "nothing to remove in a clean project"); + } + #[test] fn test_copilot_init_refuses_malformed_block() { let temp = TempDir::new().unwrap(); diff --git a/src/main.rs b/src/main.rs index 22e6cbca8..f1715186a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1821,6 +1821,8 @@ fn run_cli() -> Result { }; if show { hooks::init::show_config(codex)?; + } else if uninstall && copilot { + hooks::init::uninstall_copilot(ctx)?; } else if uninstall { uninstall_init_dispatch( agent, From 557e57c9f9fe90b090d7bb62024d8bcddd81b732 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Tue, 26 May 2026 13:31:20 +0200 Subject: [PATCH 18/39] fix(hook): semgrep marker + regression tests for Copilot hook isolation --- src/hooks/init.rs | 49 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/hooks/init.rs b/src/hooks/init.rs index ac0e4ab57..7e178cf2d 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -3993,6 +3993,7 @@ fn uninstall_copilot_at(base: &Path, ctx: InitContext) -> Result> { hook_path.display() ); } else { + // nosemgrep: filesystem-deletion -- Copilot uninstall removes only the RTK-managed hook config. fs::remove_file(&hook_path) .with_context(|| format!("Failed to remove hook: {}", hook_path.display()))?; } @@ -6444,6 +6445,54 @@ mod tests { assert!(removed.is_empty(), "nothing to remove in a clean project"); } + #[test] + fn test_copilot_install_does_not_touch_other_hooks() { + let temp = TempDir::new().unwrap(); + let hooks_dir = temp.path().join(".github").join("hooks"); + fs::create_dir_all(&hooks_dir).unwrap(); + let other_hook = hooks_dir.join("user-policy.json"); + let other_content = + r#"{"hooks":{"sessionStart":[{"type":"command","command":"echo hi"}]}}"#; + fs::write(&other_hook, other_content).unwrap(); + + run_copilot_at(temp.path(), InitContext::default()).unwrap(); + + assert!(other_hook.exists(), "third-party hook file must remain"); + assert_eq!( + fs::read_to_string(&other_hook).unwrap(), + other_content, + "third-party hook content must be unchanged by rtk install" + ); + } + + #[test] + fn test_copilot_uninstall_does_not_touch_other_hooks() { + let temp = TempDir::new().unwrap(); + run_copilot_at(temp.path(), InitContext::default()).unwrap(); + + let hooks_dir = temp.path().join(".github").join("hooks"); + let other_hook = hooks_dir.join("user-policy.json"); + let other_content = + r#"{"hooks":{"sessionStart":[{"type":"command","command":"echo hi"}]}}"#; + fs::write(&other_hook, other_content).unwrap(); + + uninstall_copilot_at(temp.path(), InitContext::default()).unwrap(); + + assert!( + other_hook.exists(), + "third-party hook file must survive rtk uninstall" + ); + assert_eq!( + fs::read_to_string(&other_hook).unwrap(), + other_content, + "third-party hook content must be unchanged by rtk uninstall" + ); + assert!( + !hooks_dir.join("rtk-rewrite.json").exists(), + "rtk's own hook must still be removed" + ); + } + #[test] fn test_copilot_init_refuses_malformed_block() { let temp = TempDir::new().unwrap(); From 4b0bbf269b34e953bf08b6d15b2ffc13cf812fb3 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Tue, 26 May 2026 19:56:04 +0200 Subject: [PATCH 19/39] feat(hook): --global Copilot install + transparent rewrite via modifiedArgs --- .../guide/getting-started/supported-agents.md | 12 +- src/hooks/constants.rs | 2 + src/hooks/hook_cmd.rs | 66 +++++-- src/hooks/init.rs | 180 +++++++++++++++++- src/main.rs | 12 +- 5 files changed, 245 insertions(+), 27 deletions(-) diff --git a/docs/guide/getting-started/supported-agents.md b/docs/guide/getting-started/supported-agents.md index 5007c10be..80f22f39e 100644 --- a/docs/guide/getting-started/supported-agents.md +++ b/docs/guide/getting-started/supported-agents.md @@ -30,7 +30,7 @@ Agent runs "cargo test" |-------|-----------------|---------------------------| | Claude Code | Shell hook (`PreToolUse`) | Yes | | VS Code Copilot Chat | Shell hook (`PreToolUse`) | Yes | -| GitHub Copilot CLI | Shell hook (deny-with-suggestion) | No (agent retries) | +| GitHub Copilot CLI | Shell hook (`preToolUse` `modifiedArgs`) | Yes | | Cursor | Shell hook (`preToolUse`) | Yes | | Gemini CLI | Rust binary (`BeforeTool`) | Yes | | OpenCode | TypeScript plugin (`tool.execute.before`) | Yes | @@ -69,18 +69,20 @@ Restart Cursor. The hook uses `preToolUse` with Cursor's `updated_input` format. ### GitHub Copilot (VS Code Chat + CLI) ```bash -rtk init --copilot +rtk init --copilot # project-scoped (.github/hooks/) +rtk init --global --copilot # user-scoped (~/.copilot/hooks/, respects $COPILOT_HOME) ``` -Writes one `.github/hooks/rtk-rewrite.json` serving both hosts: VS Code Copilot Chat reads its `PreToolUse` entry (transparent rewrite), GitHub Copilot CLI reads its `preToolUse` entry (deny-with-suggestion; the agent retries with `rtk`). +Project-scoped writes `.github/hooks/rtk-rewrite.json` (both hosts get transparent rewrite — VS Code Chat via `updatedInput`, Copilot CLI via `modifiedArgs`) plus the RTK block in `.github/copilot-instructions.md`. User-scoped writes only `~/.copilot/hooks/rtk-rewrite.json` — the Copilot CLI docs define no user-level instructions file. -Uninstall (project-scoped): +Uninstall: ```bash rtk init --uninstall --copilot +rtk init --uninstall --global --copilot ``` -Removes `.github/hooks/rtk-rewrite.json` and the RTK block from `.github/copilot-instructions.md` (your own content is preserved). +Removes only RTK's hook file (and, for project, the RTK block in `copilot-instructions.md`). Other files in `.github/hooks/` or `~/.copilot/hooks/` and your own instruction content are untouched. ### Gemini CLI diff --git a/src/hooks/constants.rs b/src/hooks/constants.rs index f9a692264..23d2e1089 100644 --- a/src/hooks/constants.rs +++ b/src/hooks/constants.rs @@ -25,6 +25,8 @@ pub const GEMINI_DIR: &str = ".gemini"; pub const GITHUB_DIR: &str = ".github"; pub const COPILOT_HOOK_FILE: &str = "rtk-rewrite.json"; pub const COPILOT_INSTRUCTIONS_FILE: &str = "copilot-instructions.md"; +pub const COPILOT_USER_DIR: &str = ".copilot"; +pub const COPILOT_HOME_ENV: &str = "COPILOT_HOME"; pub const PI_DIR: &str = ".pi/agent"; pub const PI_LOCAL_DIR: &str = ".pi"; diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index 2f29a2d7c..d5f5c3753 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -31,7 +31,7 @@ fn read_stdin_limited() -> Result { enum HookFormat { /// VS Code Copilot Chat / Claude Code: `tool_name` + `tool_input.command`, supports `updatedInput`. VsCode { command: String }, - /// GitHub Copilot CLI: camelCase `toolName` + `toolArgs` (JSON string), deny-with-suggestion only. + /// GitHub Copilot CLI: camelCase `toolName` + `toolArgs` (JSON string), supports `modifiedArgs` for transparent rewrite. CopilotCli { command: String }, /// Non-bash tool, already uses rtk, or unknown format — pass through silently. PassThrough, @@ -156,27 +156,24 @@ fn handle_vscode(cmd: &str) -> Result<()> { } fn handle_copilot_cli(cmd: &str) -> Result<()> { + if let Some(response) = copilot_cli_response(cmd) { + let _ = writeln!(io::stdout(), "{response}"); + } + Ok(()) +} + +fn copilot_cli_response(cmd: &str) -> Option { if permissions::check_command(cmd) == PermissionVerdict::Deny { audit_log("deny", cmd, ""); - return Ok(()); + return None; } - - let rewritten = match get_rewritten(cmd) { - Some(r) => r, - None => return Ok(()), - }; - + let rewritten = get_rewritten(cmd)?; audit_log("rewrite", cmd, &rewritten); - - let output = json!({ - "permissionDecision": "deny", - "permissionDecisionReason": format!( - "Token savings: use `{}` instead (rtk saves 60-90% tokens)", - rewritten - ) - }); - let _ = writeln!(io::stdout(), "{output}"); - Ok(()) + Some(json!({ + "permissionDecision": "allow", + "permissionDecisionReason": "RTK auto-rewrite", + "modifiedArgs": { "command": rewritten } + })) } // ── Gemini hook ─────────────────────────────────────────────── @@ -621,6 +618,39 @@ mod tests { assert!(get_rewritten("cat <<'EOF'\nhello\nEOF").is_none()); } + // --- Copilot CLI handler: transparent rewrite via modifiedArgs --- + + #[test] + fn test_copilot_cli_transparent_rewrite() { + let r = copilot_cli_response("cargo test").unwrap(); + assert_eq!(r["permissionDecision"], "allow"); + assert_eq!(r["modifiedArgs"]["command"], "rtk cargo test"); + } + + #[test] + fn test_copilot_cli_passthrough_unsupported() { + assert!(copilot_cli_response("htop").is_none()); + } + + #[test] + fn test_copilot_cli_passthrough_already_rtk() { + assert!(copilot_cli_response("rtk cargo test").is_none()); + } + + #[test] + fn test_copilot_cli_passthrough_heredoc() { + assert!(copilot_cli_response("cat < Result> { Ok(removed) } +fn copilot_user_dir() -> Result { + if let Ok(custom) = std::env::var(COPILOT_HOME_ENV) { + return Ok(PathBuf::from(custom)); + } + let home = dirs::home_dir().context("could not determine home directory")?; + Ok(home.join(COPILOT_USER_DIR)) +} + +pub fn run_copilot_global(ctx: InitContext) -> Result<()> { + let copilot_dir = copilot_user_dir()?; + run_copilot_global_at(&copilot_dir, ctx) +} + +fn run_copilot_global_at(copilot_dir: &Path, ctx: InitContext) -> Result<()> { + let InitContext { dry_run, .. } = ctx; + let hooks_dir = copilot_dir.join(HOOKS_SUBDIR); + + if !dry_run { + fs::create_dir_all(&hooks_dir) + .with_context(|| format!("Failed to create {} directory", hooks_dir.display()))?; + } + + let hook_path = hooks_dir.join(COPILOT_HOOK_FILE); + write_if_changed( + &hook_path, + COPILOT_HOOK_JSON, + "Copilot global hook config", + ctx, + )?; + + if dry_run { + print_dry_run_footer(); + } else { + println!("\nGitHub Copilot global integration installed (user-scoped).\n"); + println!(" Hook config: {}", hook_path.display()); + println!("\n Applies to all Copilot CLI sessions on this machine."); + println!(" Restart your Copilot CLI session to activate.\n"); + } + + Ok(()) +} + +pub fn uninstall_copilot_global(ctx: InitContext) -> Result<()> { + let copilot_dir = copilot_user_dir()?; + let InitContext { dry_run, .. } = ctx; + let removed = uninstall_copilot_global_at(&copilot_dir, ctx)?; + + if removed.is_empty() { + println!("RTK global Copilot support was not installed (nothing to remove)"); + } else { + let header = if dry_run { + "[dry-run] would uninstall RTK (global GitHub Copilot):" + } else { + "RTK uninstalled (global GitHub Copilot):" + }; + println!("{}", header); + for item in &removed { + println!(" - {}", item); + } + if !dry_run { + println!("\nRestart your Copilot CLI session to apply changes."); + } + } + + if dry_run { + print_dry_run_footer(); + } + Ok(()) +} + +fn uninstall_copilot_global_at(copilot_dir: &Path, ctx: InitContext) -> Result> { + let InitContext { dry_run, .. } = ctx; + let hook_path = copilot_dir.join(HOOKS_SUBDIR).join(COPILOT_HOOK_FILE); + let mut removed = Vec::new(); + + if hook_path.exists() { + if dry_run { + println!( + "[dry-run] would remove hook config: {}", + hook_path.display() + ); + } else { + // nosemgrep: filesystem-deletion -- Copilot global uninstall removes only the RTK-managed hook config. + fs::remove_file(&hook_path) + .with_context(|| format!("Failed to remove hook: {}", hook_path.display()))?; + } + removed.push(format!("Hook config: {}", hook_path.display())); + } + + Ok(removed) +} + #[cfg(test)] mod tests { use super::*; @@ -6493,6 +6585,90 @@ mod tests { ); } + #[test] + fn test_copilot_global_install_writes_hook() { + let temp = TempDir::new().unwrap(); + run_copilot_global_at(temp.path(), InitContext::default()).unwrap(); + + let hook_path = temp.path().join("hooks").join("rtk-rewrite.json"); + assert!(hook_path.exists()); + let v: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&hook_path).unwrap()).unwrap(); + assert_eq!(v["version"], 1); + assert_eq!(v["hooks"]["PreToolUse"][0]["command"], "rtk hook copilot"); + assert_eq!(v["hooks"]["preToolUse"][0]["bash"], "rtk hook copilot"); + } + + #[test] + fn test_copilot_global_install_does_not_write_instructions() { + let temp = TempDir::new().unwrap(); + run_copilot_global_at(temp.path(), InitContext::default()).unwrap(); + assert!( + !temp.path().join(COPILOT_INSTRUCTIONS_FILE).exists(), + "global install must not create a user-level instructions file (undocumented path)" + ); + } + + #[test] + fn test_copilot_global_uninstall_removes_hook() { + let temp = TempDir::new().unwrap(); + run_copilot_global_at(temp.path(), InitContext::default()).unwrap(); + let hook_path = temp.path().join("hooks").join("rtk-rewrite.json"); + assert!(hook_path.exists()); + + let removed = uninstall_copilot_global_at(temp.path(), InitContext::default()).unwrap(); + assert!(!removed.is_empty()); + assert!(!hook_path.exists()); + } + + #[test] + fn test_copilot_global_uninstall_nothing_when_absent() { + let temp = TempDir::new().unwrap(); + let removed = uninstall_copilot_global_at(temp.path(), InitContext::default()).unwrap(); + assert!(removed.is_empty()); + } + + #[test] + fn test_copilot_global_install_does_not_touch_other_hooks() { + let temp = TempDir::new().unwrap(); + let hooks_dir = temp.path().join("hooks"); + fs::create_dir_all(&hooks_dir).unwrap(); + let other = hooks_dir.join("notification-hooks.json"); + let payload = r#"{"version":1,"hooks":{"agentStop":[{"type":"command","bash":"true"}]}}"#; + fs::write(&other, payload).unwrap(); + + run_copilot_global_at(temp.path(), InitContext::default()).unwrap(); + + assert_eq!(fs::read_to_string(&other).unwrap(), payload); + } + + #[test] + fn test_copilot_global_uninstall_does_not_touch_other_hooks() { + let temp = TempDir::new().unwrap(); + run_copilot_global_at(temp.path(), InitContext::default()).unwrap(); + let hooks_dir = temp.path().join("hooks"); + let other = hooks_dir.join("notification-hooks.json"); + let payload = r#"{"version":1,"hooks":{"agentStop":[{"type":"command","bash":"true"}]}}"#; + fs::write(&other, payload).unwrap(); + + uninstall_copilot_global_at(temp.path(), InitContext::default()).unwrap(); + + assert!(other.exists()); + assert_eq!(fs::read_to_string(&other).unwrap(), payload); + assert!(!hooks_dir.join("rtk-rewrite.json").exists()); + } + + #[test] + fn test_copilot_global_install_dry_run_writes_nothing() { + let temp = TempDir::new().unwrap(); + let ctx = InitContext { + verbose: 0, + dry_run: true, + }; + run_copilot_global_at(temp.path(), ctx).unwrap(); + assert!(!temp.path().join("hooks").join("rtk-rewrite.json").exists()); + } + #[test] fn test_copilot_init_refuses_malformed_block() { let temp = TempDir::new().unwrap(); diff --git a/src/main.rs b/src/main.rs index f1715186a..be4e0a07b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1822,7 +1822,11 @@ fn run_cli() -> Result { if show { hooks::init::show_config(codex)?; } else if uninstall && copilot { - hooks::init::uninstall_copilot(ctx)?; + if global { + hooks::init::uninstall_copilot_global(ctx)?; + } else { + hooks::init::uninstall_copilot(ctx)?; + } } else if uninstall { uninstall_init_dispatch( agent, @@ -1843,7 +1847,11 @@ fn run_cli() -> Result { }; hooks::init::run_gemini(global, hook_only, patch_mode, ctx)?; } else if copilot { - hooks::init::run_copilot(ctx)?; + if global { + hooks::init::run_copilot_global(ctx)?; + } else { + hooks::init::run_copilot(ctx)?; + } } else if agent == Some(AgentTarget::Pi) { hooks::init::run_pi_mode(global, ctx)? } else if agent == Some(AgentTarget::Kilocode) { From 69c10b65887527d5cf95e2c48f8c1b58a1fd3077 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Tue, 26 May 2026 20:41:03 +0200 Subject: [PATCH 20/39] fix(hook): default Copilot CLI rewrite to 'ask' to preserve permission prompts --- src/hooks/hook_cmd.rs | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index d5f5c3753..0f54cf58f 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -163,14 +163,22 @@ fn handle_copilot_cli(cmd: &str) -> Result<()> { } fn copilot_cli_response(cmd: &str) -> Option { - if permissions::check_command(cmd) == PermissionVerdict::Deny { + copilot_cli_response_for_verdict(cmd, permissions::check_command(cmd)) +} + +fn copilot_cli_response_for_verdict(cmd: &str, verdict: PermissionVerdict) -> Option { + if verdict == PermissionVerdict::Deny { audit_log("deny", cmd, ""); return None; } let rewritten = get_rewritten(cmd)?; audit_log("rewrite", cmd, &rewritten); + let decision = match verdict { + PermissionVerdict::Allow => "allow", + _ => "ask", + }; Some(json!({ - "permissionDecision": "allow", + "permissionDecision": decision, "permissionDecisionReason": "RTK auto-rewrite", "modifiedArgs": { "command": rewritten } })) @@ -621,12 +629,27 @@ mod tests { // --- Copilot CLI handler: transparent rewrite via modifiedArgs --- #[test] - fn test_copilot_cli_transparent_rewrite() { - let r = copilot_cli_response("cargo test").unwrap(); + fn test_copilot_cli_default_verdict_returns_ask_with_rewrite() { + let r = copilot_cli_response_for_verdict("cargo test", PermissionVerdict::Default).unwrap(); + assert_eq!( + r["permissionDecision"], "ask", + "Default must be 'ask' so the user is still prompted for the rewritten command" + ); + assert_eq!(r["modifiedArgs"]["command"], "rtk cargo test"); + } + + #[test] + fn test_copilot_cli_explicit_allow_returns_allow_with_rewrite() { + let r = copilot_cli_response_for_verdict("cargo test", PermissionVerdict::Allow).unwrap(); assert_eq!(r["permissionDecision"], "allow"); assert_eq!(r["modifiedArgs"]["command"], "rtk cargo test"); } + #[test] + fn test_copilot_cli_deny_verdict_returns_none() { + assert!(copilot_cli_response_for_verdict("cargo test", PermissionVerdict::Deny).is_none()); + } + #[test] fn test_copilot_cli_passthrough_unsupported() { assert!(copilot_cli_response("htop").is_none()); From 6c5e440d83b30168c61e4c4b0c79847414a7c509 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Tue, 26 May 2026 20:50:06 +0200 Subject: [PATCH 21/39] fix(hook): preserve Copilot CLI toolArgs metadata when rewriting command --- src/hooks/hook_cmd.rs | 86 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 71 insertions(+), 15 deletions(-) diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index 0f54cf58f..72a449ff3 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -32,7 +32,9 @@ enum HookFormat { /// VS Code Copilot Chat / Claude Code: `tool_name` + `tool_input.command`, supports `updatedInput`. VsCode { command: String }, /// GitHub Copilot CLI: camelCase `toolName` + `toolArgs` (JSON string), supports `modifiedArgs` for transparent rewrite. - CopilotCli { command: String }, + /// Carries the full parsed `toolArgs` object so we can rewrite `command` while preserving + /// host-supplied metadata (description, initial_wait, mode, …) the tool requires. + CopilotCli { command: String, args: Value }, /// Non-bash tool, already uses rtk, or unknown format — pass through silently. PassThrough, } @@ -59,7 +61,7 @@ pub fn run_copilot() -> Result<()> { match detect_format(&v) { HookFormat::VsCode { command } => handle_vscode(&command), - HookFormat::CopilotCli { command } => handle_copilot_cli(&command), + HookFormat::CopilotCli { command, args } => handle_copilot_cli(&command, &args), HookFormat::PassThrough => Ok(()), } } @@ -93,6 +95,7 @@ fn detect_format(v: &Value) -> HookFormat { { return HookFormat::CopilotCli { command: cmd.to_string(), + args: tool_args, }; } } @@ -155,18 +158,22 @@ fn handle_vscode(cmd: &str) -> Result<()> { Ok(()) } -fn handle_copilot_cli(cmd: &str) -> Result<()> { - if let Some(response) = copilot_cli_response(cmd) { +fn handle_copilot_cli(cmd: &str, args: &Value) -> Result<()> { + if let Some(response) = copilot_cli_response(cmd, args) { let _ = writeln!(io::stdout(), "{response}"); } Ok(()) } -fn copilot_cli_response(cmd: &str) -> Option { - copilot_cli_response_for_verdict(cmd, permissions::check_command(cmd)) +fn copilot_cli_response(cmd: &str, args: &Value) -> Option { + copilot_cli_response_for_verdict(cmd, args, permissions::check_command(cmd)) } -fn copilot_cli_response_for_verdict(cmd: &str, verdict: PermissionVerdict) -> Option { +fn copilot_cli_response_for_verdict( + cmd: &str, + args: &Value, + verdict: PermissionVerdict, +) -> Option { if verdict == PermissionVerdict::Deny { audit_log("deny", cmd, ""); return None; @@ -177,10 +184,14 @@ fn copilot_cli_response_for_verdict(cmd: &str, verdict: PermissionVerdict) -> Op PermissionVerdict::Allow => "allow", _ => "ask", }; + let mut modified = args.clone(); + if let Some(obj) = modified.as_object_mut() { + obj.insert("command".into(), Value::String(rewritten)); + } Some(json!({ "permissionDecision": decision, "permissionDecisionReason": "RTK auto-rewrite", - "modifiedArgs": { "command": rewritten } + "modifiedArgs": modified })) } @@ -628,9 +639,18 @@ mod tests { // --- Copilot CLI handler: transparent rewrite via modifiedArgs --- + fn cli_args(cmd: &str) -> Value { + json!({ "command": cmd }) + } + #[test] fn test_copilot_cli_default_verdict_returns_ask_with_rewrite() { - let r = copilot_cli_response_for_verdict("cargo test", PermissionVerdict::Default).unwrap(); + let r = copilot_cli_response_for_verdict( + "cargo test", + &cli_args("cargo test"), + PermissionVerdict::Default, + ) + .unwrap(); assert_eq!( r["permissionDecision"], "ask", "Default must be 'ask' so the user is still prompted for the rewritten command" @@ -640,40 +660,76 @@ mod tests { #[test] fn test_copilot_cli_explicit_allow_returns_allow_with_rewrite() { - let r = copilot_cli_response_for_verdict("cargo test", PermissionVerdict::Allow).unwrap(); + let r = copilot_cli_response_for_verdict( + "cargo test", + &cli_args("cargo test"), + PermissionVerdict::Allow, + ) + .unwrap(); assert_eq!(r["permissionDecision"], "allow"); assert_eq!(r["modifiedArgs"]["command"], "rtk cargo test"); } #[test] fn test_copilot_cli_deny_verdict_returns_none() { - assert!(copilot_cli_response_for_verdict("cargo test", PermissionVerdict::Deny).is_none()); + assert!(copilot_cli_response_for_verdict( + "cargo test", + &cli_args("cargo test"), + PermissionVerdict::Deny + ) + .is_none()); } #[test] fn test_copilot_cli_passthrough_unsupported() { - assert!(copilot_cli_response("htop").is_none()); + assert!(copilot_cli_response("htop", &cli_args("htop")).is_none()); } #[test] fn test_copilot_cli_passthrough_already_rtk() { - assert!(copilot_cli_response("rtk cargo test").is_none()); + assert!(copilot_cli_response("rtk cargo test", &cli_args("rtk cargo test")).is_none()); } #[test] fn test_copilot_cli_passthrough_heredoc() { - assert!(copilot_cli_response("cat < Date: Tue, 26 May 2026 21:26:33 +0200 Subject: [PATCH 22/39] fix(hook): omit permissionDecision so Copilot prompts for the rewritten command --- src/hooks/hook_cmd.rs | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index 72a449ff3..8870bf8d7 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -180,19 +180,24 @@ fn copilot_cli_response_for_verdict( } let rewritten = get_rewritten(cmd)?; audit_log("rewrite", cmd, &rewritten); - let decision = match verdict { - PermissionVerdict::Allow => "allow", - _ => "ask", - }; + let mut modified = args.clone(); if let Some(obj) = modified.as_object_mut() { obj.insert("command".into(), Value::String(rewritten)); } - Some(json!({ - "permissionDecision": decision, + + // Copilot CLI v1.0.54 only applies `modifiedArgs` when permissionDecision is + // either "allow" or absent. Omitting permissionDecision lets Copilot's normal + // permission flow run on the rewritten command — `rtk *` is not on its + // auto-allow list, so the user sees a prompt for the rewritten command. + let mut response = json!({ "permissionDecisionReason": "RTK auto-rewrite", - "modifiedArgs": modified - })) + "modifiedArgs": modified, + }); + if verdict == PermissionVerdict::Allow { + response["permissionDecision"] = json!("allow"); + } + Some(response) } // ── Gemini hook ─────────────────────────────────────────────── @@ -644,16 +649,16 @@ mod tests { } #[test] - fn test_copilot_cli_default_verdict_returns_ask_with_rewrite() { + fn test_copilot_cli_default_verdict_omits_permission_decision() { let r = copilot_cli_response_for_verdict( "cargo test", &cli_args("cargo test"), PermissionVerdict::Default, ) .unwrap(); - assert_eq!( - r["permissionDecision"], "ask", - "Default must be 'ask' so the user is still prompted for the rewritten command" + assert!( + r.get("permissionDecision").is_none(), + "Default must NOT set permissionDecision — Copilot then runs its normal prompt flow on the rewritten command" ); assert_eq!(r["modifiedArgs"]["command"], "rtk cargo test"); } From e1bb17f85d387816621a6e77eca44f94458c4e81 Mon Sep 17 00:00:00 2001 From: Benoit Galati Date: Wed, 27 May 2026 17:47:32 +0200 Subject: [PATCH 23/39] fix(gh,glab): don't pre-reject view/checks/run subcommands missing the id gh and glab both accept several subcommands without an explicit id and resolve it from the current branch (gh pr view, gh pr checks, glab mr view) or open an interactive picker (gh run view). RTK was pre-rejecting with "X number required" before the underlying tool ever ran. Six call sites now treat the identifier as optional and forward the request unchanged, letting gh/glab decide: - gh pr view, gh pr checks, gh issue view, gh run view - glab mr view, glab issue view A new `parse_optional_identifier` helper in each file captures the "(Option, Vec)" parse so the no-id branch is unit-testable. format_run_view also drops the dangling `#` from its header when the run id is empty (interactive-picker passthrough). --- src/cmds/git/gh_cmd.rs | 161 +++++++++++++++++++++++++++++---------- src/cmds/git/glab_cmd.rs | 93 +++++++++++++++++----- 2 files changed, 193 insertions(+), 61 deletions(-) diff --git a/src/cmds/git/gh_cmd.rs b/src/cmds/git/gh_cmd.rs index c22c16b40..156b81e2a 100644 --- a/src/cmds/git/gh_cmd.rs +++ b/src/cmds/git/gh_cmd.rs @@ -162,6 +162,16 @@ fn extract_identifier_and_extra_args(args: &[String]) -> Option<(String, Vec (Option, Vec) { + match extract_identifier_and_extra_args(args) { + Some((id, extra)) => (Some(id), extra), + None => (None, args.to_vec()), + } +} + fn run_gh_json(cmd: Command, label: &str, filter_fn: F) -> Result where F: Fn(&Value) -> String, @@ -319,27 +329,32 @@ fn pr_status_json_fields() -> &'static str { } fn view_pr(args: &[String], _verbose: u8, ultra_compact: bool) -> Result { - let (pr_number, extra_args) = match extract_identifier_and_extra_args(args) { - Some(result) => result, - None => return Err(anyhow::anyhow!("PR number required")), - }; + // `gh pr view` without an identifier defaults to the PR for the current branch. + let (pr_number_opt, extra_args) = parse_optional_identifier(args); if should_passthrough_pr_view(&extra_args) { - return run_passthrough_with_extra("gh", &["pr", "view", &pr_number], &extra_args); + let mut base: Vec<&str> = vec!["pr", "view"]; + if let Some(id) = pr_number_opt.as_deref() { + base.push(id); + } + return run_passthrough_with_extra("gh", &base, &extra_args); } let mut cmd = resolved_command("gh"); + cmd.args(["pr", "view"]); + if let Some(id) = pr_number_opt.as_deref() { + cmd.arg(id); + } cmd.args([ - "pr", - "view", - &pr_number, "--json", "number,title,state,author,body,url,mergeable,reviews,statusCheckRollup", ]); for arg in &extra_args { cmd.arg(arg); } - run_gh_json(cmd, &format!("pr view {}", pr_number), |json| { - format_pr_view(json, ultra_compact) - }) + let label = match pr_number_opt.as_deref() { + Some(id) => format!("pr view {}", id), + None => "pr view".to_string(), + }; + run_gh_json(cmd, &label, |json| format_pr_view(json, ultra_compact)) } fn format_pr_view(json: &Value, ultra_compact: bool) -> String { @@ -427,19 +442,24 @@ fn format_pr_view(json: &Value, ultra_compact: bool) -> String { } fn pr_checks(args: &[String], _verbose: u8, _ultra_compact: bool) -> Result { - let (pr_number, extra_args) = match extract_identifier_and_extra_args(args) { - Some(result) => result, - None => return Err(anyhow::anyhow!("PR number required")), - }; + // `gh pr checks` without an identifier defaults to the PR for the current branch. + let (pr_number_opt, extra_args) = parse_optional_identifier(args); let mut cmd = resolved_command("gh"); - cmd.args(["pr", "checks", &pr_number]); + cmd.args(["pr", "checks"]); + if let Some(id) = pr_number_opt.as_deref() { + cmd.arg(id); + } for arg in &extra_args { cmd.arg(arg); } + let label = match pr_number_opt.as_deref() { + Some(id) => format!("pr checks {}", id), + None => "pr checks".to_string(), + }; runner::run_filtered( cmd, "gh", - &format!("pr checks {}", pr_number), + &label, format_pr_checks, RunOptions::stdout_only() .early_exit_on_failure() @@ -623,27 +643,29 @@ fn format_issue_list(json: &Value, ultra_compact: bool) -> String { } fn view_issue(args: &[String], _verbose: u8) -> Result { - let (issue_number, extra_args) = match extract_identifier_and_extra_args(args) { - Some(result) => result, - None => return Err(anyhow::anyhow!("Issue number required")), - }; + // Let gh emit its own error message when the identifier is missing rather than pre-rejecting. + let (issue_number_opt, extra_args) = parse_optional_identifier(args); if should_passthrough_issue_view(&extra_args) { - return run_passthrough_with_extra("gh", &["issue", "view", &issue_number], &extra_args); + let mut base: Vec<&str> = vec!["issue", "view"]; + if let Some(id) = issue_number_opt.as_deref() { + base.push(id); + } + return run_passthrough_with_extra("gh", &base, &extra_args); } let mut cmd = resolved_command("gh"); - cmd.args([ - "issue", - "view", - &issue_number, - "--json", - "number,title,state,author,body,url", - ]); + cmd.args(["issue", "view"]); + if let Some(id) = issue_number_opt.as_deref() { + cmd.arg(id); + } + cmd.args(["--json", "number,title,state,author,body,url"]); for arg in &extra_args { cmd.arg(arg); } - run_gh_json(cmd, &format!("issue view {}", issue_number), |json| { - format_issue_view(json) - }) + let label = match issue_number_opt.as_deref() { + Some(id) => format!("issue view {}", id), + None => "issue view".to_string(), + }; + run_gh_json(cmd, &label, format_issue_view) } fn format_issue_view(json: &Value) -> String { @@ -753,23 +775,32 @@ fn should_passthrough_run_view(extra_args: &[String]) -> bool { } fn view_run(args: &[String], _verbose: u8) -> Result { - let (run_id, extra_args) = match extract_identifier_and_extra_args(args) { - Some(result) => result, - None => return Err(anyhow::anyhow!("Run ID required")), - }; + // `gh run view` without an identifier opens an interactive picker — defer to gh. + let (run_id_opt, extra_args) = parse_optional_identifier(args); if should_passthrough_run_view(&extra_args) { - return run_passthrough_with_extra("gh", &["run", "view", &run_id], &extra_args); + let mut base: Vec<&str> = vec!["run", "view"]; + if let Some(id) = run_id_opt.as_deref() { + base.push(id); + } + return run_passthrough_with_extra("gh", &base, &extra_args); } let mut cmd = resolved_command("gh"); - cmd.args(["run", "view", &run_id]); + cmd.args(["run", "view"]); + if let Some(id) = run_id_opt.as_deref() { + cmd.arg(id); + } for arg in &extra_args { cmd.arg(arg); } - let run_id_owned = run_id.clone(); + let label = match run_id_opt.as_deref() { + Some(id) => format!("run view {}", id), + None => "run view".to_string(), + }; + let run_id_owned = run_id_opt.unwrap_or_default(); runner::run_filtered( cmd, "gh", - &format!("run view {}", run_id), + &label, move |stdout| format_run_view(stdout, &run_id_owned), RunOptions::stdout_only() .early_exit_on_failure() @@ -781,7 +812,11 @@ fn format_run_view(stdout: &str, run_id: &str) -> String { let mut out = String::new(); let mut in_jobs = false; - out.push_str(&format!("Workflow Run #{}\n", run_id)); + if run_id.is_empty() { + out.push_str("Workflow Run\n"); + } else { + out.push_str(&format!("Workflow Run #{}\n", run_id)); + } for line in stdout.lines() { if line.contains("JOBS") { in_jobs = true; @@ -1077,6 +1112,35 @@ mod tests { assert!(extract_identifier_and_extra_args(&args).is_none()); } + // --- parse_optional_identifier tests --- + + #[test] + fn test_parse_optional_identifier_empty_yields_no_id() { + // `gh pr view` (no args) must surface as (None, []) so the caller + // hands the request to gh, which resolves the current branch's PR. + let (id, extra) = parse_optional_identifier(&[]); + assert!(id.is_none()); + assert!(extra.is_empty()); + } + + #[test] + fn test_parse_optional_identifier_only_flags_preserves_flags() { + // Regression: `gh pr view -R rtk-ai/rtk` previously triggered + // "PR number required". Now flags must round-trip into `extra`. + let args: Vec = vec!["-R".into(), "rtk-ai/rtk".into()]; + let (id, extra) = parse_optional_identifier(&args); + assert!(id.is_none()); + assert_eq!(extra, vec!["-R", "rtk-ai/rtk"]); + } + + #[test] + fn test_parse_optional_identifier_with_id_matches_extract() { + let args: Vec = vec!["-R".into(), "rtk-ai/rtk".into(), "42".into()]; + let (id, extra) = parse_optional_identifier(&args); + assert_eq!(id.as_deref(), Some("42")); + assert_eq!(extra, vec!["-R", "rtk-ai/rtk"]); + } + #[test] fn test_extract_identifier_with_web_flag() { let args: Vec = vec!["123".into(), "--web".into()]; @@ -1108,6 +1172,21 @@ mod tests { assert!(!should_passthrough_run_view(&[])); } + #[test] + fn test_format_run_view_with_id() { + let output = format_run_view("", "12345"); + assert!(output.starts_with("Workflow Run #12345\n")); + } + + #[test] + fn test_format_run_view_without_id() { + // `gh run view` with no arg opens an interactive picker — the captured run id + // is empty, so the header must not render an empty `#`. + let output = format_run_view("", ""); + assert!(output.starts_with("Workflow Run\n")); + assert!(!output.contains("#\n")); + } + #[test] fn test_run_view_no_passthrough_other_flags() { assert!(!should_passthrough_run_view(&["--web".into()])); diff --git a/src/cmds/git/glab_cmd.rs b/src/cmds/git/glab_cmd.rs index a0282a221..56885f07c 100644 --- a/src/cmds/git/glab_cmd.rs +++ b/src/cmds/git/glab_cmd.rs @@ -212,6 +212,16 @@ fn extract_identifier_and_extra_args(args: &[String]) -> Option<(String, Vec (Option, Vec) { + match extract_identifier_and_extra_args(args) { + Some((id, extra)) => (Some(id), extra), + None => (None, args.to_vec()), + } +} + /// Check if user explicitly requested JSON/custom output format. /// When present, passthrough to avoid double JSON injection. fn has_output_flag(args: &[String]) -> bool { @@ -409,24 +419,32 @@ fn format_mr_view(json: &Value, ultra_compact: bool) -> String { } fn mr_view(args: &[String], _verbose: u8, ultra_compact: bool) -> Result { - let (mr_number, extra_args) = match extract_identifier_and_extra_args(args) { - Some(pair) => pair, - None => return Err(anyhow::anyhow!("MR number required")), - }; + // `glab mr view` without an identifier defaults to the MR for the current branch. + let (mr_number_opt, extra_args) = parse_optional_identifier(args); // Passthrough for --web, --comments, or explicit output format if should_passthrough_view(&extra_args) { - return run_passthrough_with_extra("glab", &["mr", "view", &mr_number], &extra_args); + let mut base: Vec<&str> = vec!["mr", "view"]; + if let Some(id) = mr_number_opt.as_deref() { + base.push(id); + } + return run_passthrough_with_extra("glab", &base, &extra_args); } let mut cmd = resolved_command("glab"); - cmd.args(["mr", "view", &mr_number, "-F", "json"]); + cmd.args(["mr", "view"]); + if let Some(id) = mr_number_opt.as_deref() { + cmd.arg(id); + } + cmd.args(["-F", "json"]); for arg in &extra_args { cmd.arg(arg); } - run_glab_json(cmd, &format!("mr view {}", mr_number), |json| { - format_mr_view(json, ultra_compact) - }) + let label = match mr_number_opt.as_deref() { + Some(id) => format!("mr view {}", id), + None => "mr view".to_string(), + }; + run_glab_json(cmd, &label, |json| format_mr_view(json, ultra_compact)) } fn mr_create(args: &[String], _verbose: u8) -> Result { @@ -603,25 +621,31 @@ fn format_issue_view(json: &Value) -> String { } fn issue_view(args: &[String], _verbose: u8) -> Result { - let (issue_number, extra_args) = match extract_identifier_and_extra_args(args) { - Some(pair) => pair, - None => return Err(anyhow::anyhow!("Issue number required")), - }; + // Let glab emit its own error message when the identifier is missing rather than pre-rejecting. + let (issue_number_opt, extra_args) = parse_optional_identifier(args); if should_passthrough_view(&extra_args) { - return run_passthrough_with_extra("glab", &["issue", "view", &issue_number], &extra_args); + let mut base: Vec<&str> = vec!["issue", "view"]; + if let Some(id) = issue_number_opt.as_deref() { + base.push(id); + } + return run_passthrough_with_extra("glab", &base, &extra_args); } let mut cmd = resolved_command("glab"); - cmd.args(["issue", "view", &issue_number, "-F", "json"]); + cmd.args(["issue", "view"]); + if let Some(id) = issue_number_opt.as_deref() { + cmd.arg(id); + } + cmd.args(["-F", "json"]); for arg in &extra_args { cmd.arg(arg); } - run_glab_json( - cmd, - &format!("issue view {}", issue_number), - format_issue_view, - ) + let label = match issue_number_opt.as_deref() { + Some(id) => format!("issue view {}", id), + None => "issue view".to_string(), + }; + run_glab_json(cmd, &label, format_issue_view) } // ── CI/Pipeline subcommands ───────────────────────────────────────────── @@ -1235,6 +1259,35 @@ mod tests { assert!(extract_identifier_and_extra_args(&args).is_none()); } + // ── parse_optional_identifier tests ───────────────────────────────── + + #[test] + fn test_parse_optional_identifier_empty_yields_no_id() { + // `glab mr view` (no args) must surface as (None, []) so the caller + // hands the request to glab, which resolves the current branch's MR. + let (id, extra) = parse_optional_identifier(&[]); + assert!(id.is_none()); + assert!(extra.is_empty()); + } + + #[test] + fn test_parse_optional_identifier_only_flags_preserves_flags() { + // Regression: `glab mr view -R group/project` previously triggered + // "MR number required". Now flags must round-trip into `extra`. + let args: Vec = vec!["-R".into(), "group/project".into()]; + let (id, extra) = parse_optional_identifier(&args); + assert!(id.is_none()); + assert_eq!(extra, vec!["-R", "group/project"]); + } + + #[test] + fn test_parse_optional_identifier_with_id_matches_extract() { + let args: Vec = vec!["-R".into(), "group/project".into(), "42".into()]; + let (id, extra) = parse_optional_identifier(&args); + assert_eq!(id.as_deref(), Some("42")); + assert_eq!(extra, vec!["-R", "group/project"]); + } + // ── has_output_flag tests ─────────────────────────────────────────── #[test] From f2a2e01e7a7f95bc6090d25e028fad8164d47936 Mon Sep 17 00:00:00 2001 From: Lara Date: Wed, 27 May 2026 16:00:32 -0300 Subject: [PATCH 24/39] docs(readme): add Portuguese translation --- README.md | 3 +- README_pt.md | 166 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 168 insertions(+), 1 deletion(-) create mode 100644 README_pt.md diff --git a/README.md b/README.md index f8d65efe5..492589ef8 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,8 @@ 中文 • 日本語 • 한국어 • - Espanol + Espanol • + Português

--- diff --git a/README_pt.md b/README_pt.md new file mode 100644 index 000000000..767cd49dc --- /dev/null +++ b/README_pt.md @@ -0,0 +1,166 @@ +

+ RTK - Rust Token Killer +

+ +

+ Proxy CLI de alta performance que reduz o consumo de tokens LLM em 60-90% +

+ +

+ CI + Release + License: Apache 2.0 + Discord + Homebrew +

+ +

+ Site • + Instalar • + Solução de problemas • + Arquitetura • + Discord +

+ +

+ English • + Francais • + 中文 • + 日本語 • + 한국어 • + Espanol • + Português +

+ +--- + +rtk filtra e comprime saídas de comandos antes de chegarem ao contexto do seu LLM. Binário Rust único, zero dependências, overhead inferior a 10ms. + +## Economia de tokens (sessão de 30 min no Claude Code) + +| Operação | Frequência | Padrão | rtk | Economia | +|-----------|------------|----------|-----|--------| +| `ls` / `tree` | 10x | 2,000 | 400 | -80% | +| `cat` / `read` | 20x | 40,000 | 12,000 | -70% | +| `grep` / `rg` | 8x | 16,000 | 3,200 | -80% | +| `git status` | 10x | 3,000 | 600 | -80% | +| `cargo test` / `npm test` | 5x | 25,000 | 2,500 | -90% | +| **Total** | | **~118,000** | **~23,900** | **-80%** | + +## Instalacao + +### Homebrew (recomendado) + +```bash +brew install rtk +``` + +### Instalação rápida (Linux/macOS) + +```bash +curl -fsSL https://raw.githubusercontent.com/rtk-ai/rtk/refs/heads/master/install.sh | sh +``` + +### Cargo + +```bash +cargo install --git https://github.com/rtk-ai/rtk +``` + +### Verificação + +```bash +rtk --version # Deve exibir "rtk 0.28.2" +rtk gain # Deve exibir estatísticas de economia +``` + +## Inicio rapido + +```bash +# 1. Instalar hook para Claude Code (recomendado) +rtk init --global + +# 2. Reiniciar Claude Code, depois testar +git status # Reescrito automaticamente para rtk git status +``` + +## Como funciona + +``` + Sem rtk: Com rtk: + + Claude --git status--> shell --> git Claude --git status--> RTK --> git + ^ | ^ | | + | ~2,000 tokens (bruto) | | ~200 tokens | filtro | + +-----------------------------------+ +------- (filtrado) ---+----------+ +``` + +Quatro estratégias: + +1. **Filtragem inteligente** - Elimina ruído (comentários, espaços, boilerplate) +2. **Agrupamento** - Agrega itens similares (arquivos por diretório, erros por tipo) +3. **Truncamento** - Mantém contexto relevante, elimina redundância +4. **Deduplicação** - Colapsa linhas de log repetidas com contadores + +## Comandos + +### Arquivos +```bash +rtk ls . # Árvore de diretórios otimizada +rtk read file.rs # Leitura inteligente +rtk find "*.rs" . # Resultados compactos +rtk grep "pattern" . # Busca agrupada por arquivo +``` + +### Git +```bash +rtk git status # Status compacto +rtk git log -n 10 # Commits em uma linha +rtk git diff # Diff condensado +rtk git push # -> "ok main" +``` + +### Tests +```bash +rtk jest # Jest compacto +rtk vitest # Vitest compacto +rtk pytest # Tests Python (-90%) +rtk go test # Tests Go (-90%) +rtk cargo test # Tests Rust (-90%) +rtk test # Só falhas (-90%) +``` + +### Build & Lint +```bash +rtk lint # ESLint agrupado por regra +rtk tsc # Erros TypeScript agrupados +rtk cargo build # Build Cargo (-80%) +rtk ruff check # Lint Python (-80%) +``` + +### Análises +```bash +rtk gain # Estatísticas de economia +rtk gain --graph # Gráfico ASCII (30 dias) +rtk discover # Descobrir economias perdidas +``` + +## Documentação + +- **[INSTALL.md](INSTALL.md)** - Guia de instalação detalhado +- **[ARCHITECTURE.md](docs/contributing/ARCHITECTURE.md)** - Arquitetura técnica +- **[CONTRIBUTING.md](CONTRIBUTING.md)** - Guia de contribuição + +## Contribuir + +Contribuições são bem-vindas. Abra uma issue ou PR no [GitHub](https://github.com/rtk-ai/rtk). + +Junte-se à comunidade no [Discord](https://discord.gg/RySmvNF5kF). + +## Licença + +Apache License 2.0 - veja [LICENSE](LICENSE) para detalhes. + +## Aviso Legal + +Veja [DISCLAIMER.md](DISCLAIMER.md). From 207c82a585db4b2b3d343c25c403599cd28fbbe2 Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Thu, 28 May 2026 16:44:06 +0200 Subject: [PATCH 25/39] missing bracket --- src/main.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main.rs b/src/main.rs index 819e57abf..042a78cd5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3194,6 +3194,8 @@ mod tests { code, 134, "rtk crashed with SIGABRT (exit 134) on broken pipe - SIGPIPE handler missing" ); + } + fn test_ultra_compact_long_form_still_works() { let cli = Cli::try_parse_from(["rtk", "--ultra-compact", "git", "status"]).unwrap(); assert!( From b6ecf965a4e829756801da1f19de2e51c3af24c4 Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Thu, 28 May 2026 16:50:18 +0200 Subject: [PATCH 26/39] missing test tag --- src/main.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main.rs b/src/main.rs index 042a78cd5..672a3ec92 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3196,6 +3196,7 @@ mod tests { ); } + #[test] fn test_ultra_compact_long_form_still_works() { let cli = Cli::try_parse_from(["rtk", "--ultra-compact", "git", "status"]).unwrap(); assert!( From e3ad9736177116b637da17faaf38f70312ab6148 Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Fri, 29 May 2026 09:16:22 +0200 Subject: [PATCH 27/39] fix(copilot-cli): use user-level instruction for -g -g was missing user level instructions which are available for copilot --- .../guide/getting-started/supported-agents.md | 2 +- src/hooks/init.rs | 73 +++++++++++++++++-- 2 files changed, 69 insertions(+), 6 deletions(-) diff --git a/docs/guide/getting-started/supported-agents.md b/docs/guide/getting-started/supported-agents.md index 80f22f39e..be70ddbd0 100644 --- a/docs/guide/getting-started/supported-agents.md +++ b/docs/guide/getting-started/supported-agents.md @@ -73,7 +73,7 @@ rtk init --copilot # project-scoped (.github/hooks/) rtk init --global --copilot # user-scoped (~/.copilot/hooks/, respects $COPILOT_HOME) ``` -Project-scoped writes `.github/hooks/rtk-rewrite.json` (both hosts get transparent rewrite — VS Code Chat via `updatedInput`, Copilot CLI via `modifiedArgs`) plus the RTK block in `.github/copilot-instructions.md`. User-scoped writes only `~/.copilot/hooks/rtk-rewrite.json` — the Copilot CLI docs define no user-level instructions file. +Project-scoped writes `.github/hooks/rtk-rewrite.json` (both hosts get transparent rewrite — VS Code Chat via `updatedInput`, Copilot CLI via `modifiedArgs`) plus the RTK block in `.github/copilot-instructions.md`. User-scoped writes the same hook config to `~/.copilot/hooks/rtk-rewrite.json` and the RTK block to `~/.copilot/copilot-instructions.md` (both respect `$COPILOT_HOME` if set). Uninstall: diff --git a/src/hooks/init.rs b/src/hooks/init.rs index d349a9692..2d711073c 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -4050,6 +4050,15 @@ fn run_copilot_global_at(copilot_dir: &Path, ctx: InitContext) -> Result<()> { .with_context(|| format!("Failed to create {} directory", hooks_dir.display()))?; } + let instructions_path = copilot_dir.join(COPILOT_INSTRUCTIONS_FILE); + write_rtk_block( + &instructions_path, + COPILOT_INSTRUCTIONS, + "Copilot user-level instructions", + "rtk init --global --copilot", + ctx, + )?; + let hook_path = hooks_dir.join(COPILOT_HOOK_FILE); write_if_changed( &hook_path, @@ -4063,6 +4072,7 @@ fn run_copilot_global_at(copilot_dir: &Path, ctx: InitContext) -> Result<()> { } else { println!("\nGitHub Copilot global integration installed (user-scoped).\n"); println!(" Hook config: {}", hook_path.display()); + println!(" Instructions: {}", instructions_path.display()); println!("\n Applies to all Copilot CLI sessions on this machine."); println!(" Restart your Copilot CLI session to activate.\n"); } @@ -4117,6 +4127,31 @@ fn uninstall_copilot_global_at(copilot_dir: &Path, ctx: InitContext) -> Result Date: Fri, 29 May 2026 09:26:32 +0200 Subject: [PATCH 28/39] fix: fmt --- src/hooks/init.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 2d711073c..0b1855719 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -6639,7 +6639,10 @@ mod tests { let temp = TempDir::new().unwrap(); run_copilot_global_at(temp.path(), InitContext::default()).unwrap(); let instructions = temp.path().join(COPILOT_INSTRUCTIONS_FILE); - assert!(instructions.exists(), "user-level instructions must be written"); + assert!( + instructions.exists(), + "user-level instructions must be written" + ); let content = fs::read_to_string(&instructions).unwrap(); assert!(content.contains(RTK_BLOCK_START)); assert!(content.contains("rtk cargo test")); @@ -6654,7 +6657,10 @@ mod tests { run_copilot_global_at(temp.path(), InitContext::default()).unwrap(); let content = fs::read_to_string(&instructions).unwrap(); - assert!(content.contains("Always use pnpm."), "user content must be preserved"); + assert!( + content.contains("Always use pnpm."), + "user content must be preserved" + ); assert!(content.contains(RTK_BLOCK_START)); } From 7853819fe0f4285708469766a3c62f3368ac6a5c Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Fri, 29 May 2026 12:51:04 +0200 Subject: [PATCH 29/39] Update main.rs --- src/main.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main.rs b/src/main.rs index 672a3ec92..819d32b71 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1362,6 +1362,7 @@ fn main() { // release profile that becomes SIGABRT + coredump. #[cfg(unix)] unsafe { + // nosemgrep: unsafe-block libc::signal(libc::SIGPIPE, libc::SIG_DFL); } From b11f82751d268a7cc40d9d2aa1d8425d12fbcd03 Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Fri, 29 May 2026 12:57:03 +0200 Subject: [PATCH 30/39] allow unsafe code clippy + semgrep --- src/main.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main.rs b/src/main.rs index 819d32b71..62237dc2f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1361,6 +1361,7 @@ fn main() { // Rust ignores SIGPIPE by default and with panic="abort" in the // release profile that becomes SIGABRT + coredump. #[cfg(unix)] + #[allow(unsafe_code)] unsafe { // nosemgrep: unsafe-block libc::signal(libc::SIGPIPE, libc::SIG_DFL); From dd5362fd546ae07409dbd799f00f524800157f73 Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Fri, 29 May 2026 13:03:02 +0200 Subject: [PATCH 31/39] Update main.rs --- src/main.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main.rs b/src/main.rs index 62237dc2f..5c0f7462b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1362,8 +1362,7 @@ fn main() { // release profile that becomes SIGABRT + coredump. #[cfg(unix)] #[allow(unsafe_code)] - unsafe { - // nosemgrep: unsafe-block + unsafe { // nosemgrep: unsafe-block libc::signal(libc::SIGPIPE, libc::SIG_DFL); } From 26ff547c25a408e40ec2e8b2d6ad10c8c98bcade Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Fri, 29 May 2026 13:15:24 +0200 Subject: [PATCH 32/39] Update main.rs --- src/main.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index 5c0f7462b..7f11a3eb9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1362,7 +1362,8 @@ fn main() { // release profile that becomes SIGABRT + coredump. #[cfg(unix)] #[allow(unsafe_code)] - unsafe { // nosemgrep: unsafe-block + // nosemgrep: unsafe-block + unsafe { libc::signal(libc::SIGPIPE, libc::SIG_DFL); } From 5989ac91b008dece0d36b62c2deb169a90d5063d Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Sat, 30 May 2026 17:19:13 +0200 Subject: [PATCH 33/39] fix(provider): sanatize more chars when encoding claude code project pathes Fixes #2150 --- src/discover/provider.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/discover/provider.rs b/src/discover/provider.rs index ee76c7a16..a9b630a2b 100644 --- a/src/discover/provider.rs +++ b/src/discover/provider.rs @@ -129,7 +129,7 @@ impl ClaudeProvider { /// `/home/chris/2_project` → `-home-chris-2-project` /// `C:\Users\foo\bar` → `C:-Users-foo-bar` pub fn encode_project_path(path: &str) -> String { - const SANITIZED_CHARS: &[char] = &['/', '.', '_', '\\']; + const SANITIZED_CHARS: &[char] = &['/', '.', '_', '\\', ' ', '[', ']']; path.chars() .map(|c| { @@ -421,6 +421,17 @@ mod tests { assert!(encoded.contains("Sites")); } + #[test] + fn test_encode_path_with_spaces() { + // Even if run on Unix, encoding should replace backslashes to match Claude's behavior + assert_eq!( + ClaudeProvider::encode_project_path( + r"/home/user/projects/[QZX-7K42] - Análise Genérica de Exemplo" + ), + "-home-user-projects--QZX-7K42----An-lise-Gen-rica-de-Exemplo" + ); + } + #[test] fn test_discover_sessions_missing_projects_dir_returns_empty() { let temp_home = tempfile::tempdir().unwrap(); From 67a5958459decb784bb0948b921e5ffd9c8ee52c Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Sat, 30 May 2026 18:16:30 +0200 Subject: [PATCH 34/39] doc(init): fix documentation inconsistencies arount `rtk init` Fixes #213 --- README.md | 8 +++++--- docs/guide/getting-started/supported-agents.md | 9 +++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 492589ef8..d91e21b24 100644 --- a/README.md +++ b/README.md @@ -107,10 +107,11 @@ rtk init -g # Claude Code / Copilot (default) rtk init -g --gemini # Gemini CLI rtk init -g --codex # Codex (OpenAI) rtk init -g --agent cursor # Cursor -rtk init --agent windsurf # Windsurf +rtk init -g --agent windsurf # Windsurf rtk init --agent cline # Cline / Roo Code rtk init --agent kilocode # Kilo Code rtk init --agent antigravity # Google Antigravity +rtk init -g --agent pi # Pi rtk init --agent hermes # Hermes # 2. Restart your AI tool, then test @@ -352,7 +353,7 @@ rtk git status ## Supported AI Tools -RTK supports 13 AI coding tools. Each integration rewrites shell commands to `rtk` equivalents for 60-90% token savings where the agent supports command interception. +RTK supports 14 AI coding tools. Each integration rewrites shell commands to `rtk` equivalents for 60-90% token savings where the agent supports command interception. | Tool | Install | Method | |------|---------|--------| @@ -362,10 +363,11 @@ RTK supports 13 AI coding tools. Each integration rewrites shell commands to `rt | **Cursor** | `rtk init -g --agent cursor` | preToolUse hook (hooks.json) | | **Gemini CLI** | `rtk init -g --gemini` | BeforeTool hook | | **Codex** | `rtk init -g --codex` | AGENTS.md + RTK.md instructions | -| **Windsurf** | `rtk init --agent windsurf` | .windsurfrules (project-scoped) | +| **Windsurf** | `rtk init -g --agent windsurf` | .windsurfrules (project-scoped) | | **Cline / Roo Code** | `rtk init --agent cline` | .clinerules (project-scoped) | | **OpenCode** | `rtk init -g --opencode` | Plugin TS (tool.execute.before) | | **OpenClaw** | `openclaw plugins install ./openclaw` | Plugin TS (before_tool_call) | +| **Pi** | `rtk init -g --agent pi` (global) | TypeScript extension (tool_call) | | **Hermes** | `rtk init --agent hermes` | Python plugin adapter (terminal command mutation via `rtk rewrite`) | | **Mistral Vibe** | Planned ([#800](https://github.com/rtk-ai/rtk/issues/800)) | Blocked on upstream | | **Kilo Code** | `rtk init --agent kilocode` | .kilocode/rules/rtk-rules.md (project-scoped) | diff --git a/docs/guide/getting-started/supported-agents.md b/docs/guide/getting-started/supported-agents.md index be70ddbd0..d5a0ad87c 100644 --- a/docs/guide/getting-started/supported-agents.md +++ b/docs/guide/getting-started/supported-agents.md @@ -61,7 +61,7 @@ rtk init --show # shows hook status ### Cursor ```bash -rtk init --global --cursor +rtk init --global --agent cursor ``` Restart Cursor. The hook uses `preToolUse` with Cursor's `updated_input` format. @@ -140,7 +140,7 @@ The plugin fails open. If `rtk` is missing at load time, the hook is not registe ### Cline / Roo Code ```bash -rtk init --cline # creates .clinerules in current project +rtk init --agent cline # creates .clinerules in current project ``` Cline reads `.clinerules` as custom instructions. RTK adds guidance telling Cline to prefer `rtk ` over raw commands. @@ -148,13 +148,14 @@ Cline reads `.clinerules` as custom instructions. RTK adds guidance telling Clin ### Windsurf ```bash -rtk init --windsurf # creates .windsurfrules in current project +rtk init --global --agent windsurf # creates .windsurfrules in current project ``` ### Codex CLI ```bash -rtk init --codex # creates AGENTS.md or patches existing one +rtk init --codex # project-scoped (AGENTS.md) +rtk init --global --codex # user-global (~/.codex/AGENTS.md) ``` ### Kilo Code From 83cd93e7d7ac544e9d0a5783edfdabff04a41226 Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Sun, 10 May 2026 02:10:47 +0200 Subject: [PATCH 35/39] chore(args): introduce a more generic solution for restoring double dashes in args Fixes #1669 --- src/cmds/git/git.rs | 200 ++----------------------------- src/cmds/rust/cargo_cmd.rs | 45 +------ src/core/args_utils.rs | 240 +++++++++++++++++++++++++++++++++++++ src/core/mod.rs | 1 + 4 files changed, 255 insertions(+), 231 deletions(-) create mode 100644 src/core/args_utils.rs diff --git a/src/cmds/git/git.rs b/src/cmds/git/git.rs index eaf8d8b5f..b8ccfe923 100644 --- a/src/cmds/git/git.rs +++ b/src/cmds/git/git.rs @@ -1,5 +1,6 @@ //! Filters git output — log, status, diff, and more — keeping just the essential info. +use crate::core::args_utils; use crate::core::stream::{ self, exec_capture, CaptureResult, FilterMode, LineHandler, LineStreamFilter, StdinMode, }; @@ -102,65 +103,6 @@ pub fn run( } } -/// Re-insert `--` before the first path-like argument when clap has consumed it. -/// -/// clap's `trailing_var_arg = true` silently drops `--` when it appears as the -/// first positional argument (before any other positional). This means: -/// `rtk git diff -- file` → args = ["file"] (clap ate `--`) -/// `rtk git diff HEAD -- file` → args = ["HEAD", "--", "file"] (preserved) -/// -/// Without the `--` separator git may treat an unambiguous path as a revision and -/// emit "fatal: ambiguous argument". We re-insert `--` before the first path-like -/// argument; see `normalize_diff_args_impl` for the detection rules. -fn normalize_diff_args(args: &[String]) -> Vec { - normalize_diff_args_impl(args, |p| std::path::Path::new(p).exists()) -} - -/// Testable core of `normalize_diff_args` — accepts an injectable filesystem existence checker. -/// -/// The path-detection logic is: -/// 1. Explicit path prefixes (`.`, `~`) → always a path, no filesystem check needed. -/// 2. Contains path separator (`/`, `\`) → use `path_exists` to distinguish branch names -/// (e.g. `feature/auth`) from real paths (e.g. `src/main.rs`). -/// 3. Bare word with no separator → never a path (avoids injecting `--` when a file -/// happens to share a name with a branch or ref, e.g. a file named `main`). -fn normalize_diff_args_impl(args: &[String], path_exists: F) -> Vec -where - F: Fn(&str) -> bool, -{ - // Already has `--` — nothing to do - if args.iter().any(|a| a == "--") { - return args.to_vec(); - } - let path_start = args.iter().position(|arg| { - if arg.starts_with('-') { - return false; - } - // Explicit path prefixes — always treat as path regardless of existence - if arg.starts_with('.') || arg.starts_with('~') { - return true; - } - // Contains path separator — use filesystem check to distinguish - // branch names (feature/auth) from real paths (src/main.rs) - if arg.contains('/') || arg.contains('\\') { - return path_exists(arg); - } - // Bare word (no separator, no special prefix) — never inject `--` - // This avoids misidentifying a ref/branch as a path even if a same-named - // file happens to exist on disk. - false - }); - match path_start { - Some(idx) => { - let mut out = args[..idx].to_vec(); - out.push("--".to_string()); - out.extend_from_slice(&args[idx..]); - out - } - None => args.to_vec(), - } -} - fn run_diff( args: &[String], max_lines: Option, @@ -170,7 +112,7 @@ fn run_diff( let timer = tracking::TimedExecution::start(); // Re-insert `--` when clap's trailing_var_arg consumed it (issue #1215) - let args = &normalize_diff_args(args); + let args = &args_utils::restore_double_dash(args); // Check if user wants stat output let wants_stat = args @@ -1911,8 +1853,14 @@ mod tests { assert!(uses_compact_status_path(&["-b".to_string()])); assert!(uses_compact_status_path(&["--branch".to_string()])); assert!(uses_compact_status_path(&["-sb".to_string()])); - assert!(uses_compact_status_path(&["-s".to_string(), "-b".to_string()])); - assert!(uses_compact_status_path(&["--short".to_string(), "--branch".to_string()])); + assert!(uses_compact_status_path(&[ + "-s".to_string(), + "-b".to_string() + ])); + assert!(uses_compact_status_path(&[ + "--short".to_string(), + "--branch".to_string() + ])); assert!(!uses_compact_status_path(&["-s".to_string()])); assert!(!uses_compact_status_path(&["--short".to_string()])); assert!(!uses_compact_status_path(&["--porcelain".to_string()])); @@ -2002,134 +1950,6 @@ mod tests { ); } - // ----- normalize_diff_args (issue #1215 + branch-name fix #1431) ----- - // - // Tests use normalize_diff_args_impl with a mock path-existence checker so - // they don't depend on the real filesystem. - - fn exists_mock<'a>(existing: &'a [&'a str]) -> impl Fn(&str) -> bool + 'a { - move |p| existing.contains(&p) - } - - /// Baseline: `--` already present → no-op, args unchanged. - #[test] - fn test_normalize_diff_args_noop_when_separator_present() { - let args = vec![ - "HEAD".to_string(), - "--".to_string(), - "src/main.rs".to_string(), - ]; - assert_eq!(normalize_diff_args_impl(&args, exists_mock(&[])), args); - } - - /// Core regression (issue #1215): clap ate `--` before a real file path. - /// When the path exists on disk, `--` must be re-inserted. - #[test] - fn test_normalize_diff_args_reinserts_separator_before_existing_path() { - let args = vec!["apps/client/frontend/src/MyComponent.tsx".to_string()]; - let normalized = normalize_diff_args_impl( - &args, - exists_mock(&["apps/client/frontend/src/MyComponent.tsx"]), - ); - assert_eq!( - normalized, - vec![ - "--".to_string(), - "apps/client/frontend/src/MyComponent.tsx".to_string() - ], - "-- must be injected before an existing path" - ); - } - - /// Ref before path: ["HEAD", "src/foo.rs"] where src/foo.rs exists → inject after HEAD. - #[test] - fn test_normalize_diff_args_reinserts_separator_after_ref() { - let args = vec!["HEAD".to_string(), "src/foo.rs".to_string()]; - let normalized = normalize_diff_args_impl(&args, exists_mock(&["src/foo.rs"])); - assert_eq!( - normalized, - vec![ - "HEAD".to_string(), - "--".to_string(), - "src/foo.rs".to_string() - ] - ); - } - - /// Flags before path: ["--cached", "src/foo.rs"] where src/foo.rs exists. - #[test] - fn test_normalize_diff_args_reinserts_separator_after_flag() { - let args = vec!["--cached".to_string(), "src/foo.rs".to_string()]; - let normalized = normalize_diff_args_impl(&args, exists_mock(&["src/foo.rs"])); - assert_eq!( - normalized, - vec![ - "--cached".to_string(), - "--".to_string(), - "src/foo.rs".to_string() - ] - ); - } - - /// Pure flags (no paths) → no injection. - #[test] - fn test_normalize_diff_args_no_injection_for_pure_flags() { - let args = vec!["--stat".to_string(), "--cached".to_string()]; - assert_eq!(normalize_diff_args_impl(&args, exists_mock(&[])), args); - } - - /// Dotfile that exists on disk → inject `--`. - #[test] - fn test_normalize_diff_args_dotfile_is_path() { - let args = vec![".gitignore".to_string()]; - let normalized = normalize_diff_args_impl(&args, exists_mock(&[".gitignore"])); - assert_eq!(normalized, vec!["--".to_string(), ".gitignore".to_string()]); - } - - /// A bare ref (HEAD) that doesn't exist as a file → no injection. - #[test] - fn test_normalize_diff_args_no_injection_for_bare_ref() { - let args = vec!["HEAD".to_string()]; - assert_eq!(normalize_diff_args_impl(&args, exists_mock(&[])), args); - } - - /// Branch name with `/` that does NOT exist as a file → no injection. - /// Regression for issue #1431: `rtk git diff feature/user-auth` must not inject `--`. - #[test] - fn test_normalize_diff_args_no_injection_for_branch_with_slash() { - let args = vec!["feature/user-auth".to_string()]; - assert_eq!( - normalize_diff_args_impl(&args, exists_mock(&[])), - args, - "branch names containing '/' must not trigger -- injection" - ); - } - - /// Range syntax with `/` → no injection. - /// Regression: `rtk git diff main...feature/user-auth` produced no output. - #[test] - fn test_normalize_diff_args_no_injection_for_range_with_slash() { - let args = vec!["main...feature/user-auth".to_string()]; - assert_eq!( - normalize_diff_args_impl(&args, exists_mock(&[])), - args, - "revision ranges like main...feature/user-auth must not trigger -- injection" - ); - } - - /// Bare word that happens to exist as a file on disk → still no injection. - /// A file named "main" must not cause `--` to be injected when the user - /// intends `rtk git diff main` as a branch comparison. - #[test] - fn test_normalize_diff_args_no_injection_for_bare_word_even_if_file_exists() { - let args = vec!["main".to_string()]; - assert_eq!( - normalize_diff_args_impl(&args, exists_mock(&["main"])), - args, - "bare words must never trigger -- injection even when a same-named file exists" - ); - } - #[test] fn test_is_blob_show_arg() { assert!(is_blob_show_arg("develop:modules/pairs_backtest.py")); diff --git a/src/cmds/rust/cargo_cmd.rs b/src/cmds/rust/cargo_cmd.rs index 5b8152ce0..8726de40e 100644 --- a/src/cmds/rust/cargo_cmd.rs +++ b/src/cmds/rust/cargo_cmd.rs @@ -1,5 +1,6 @@ //! Filters cargo output — build errors, test results, clippy warnings. +use crate::core::args_utils; use crate::core::runner; use crate::core::stream::{BlockHandler, BlockStreamFilter, StreamFilter}; use crate::core::truncate::{CAP_ERRORS, CAP_LIST, CAP_WARNINGS}; @@ -31,45 +32,6 @@ pub fn run(cmd: CargoCommand, args: &[String], verbose: u8) -> Result { } } -/// Reconstruct args with `--` separator preserved from the original command line. -/// Clap strips `--` from parsed args, but cargo subcommands need it to separate -/// their own flags from test runner flags (e.g. `cargo test -- --nocapture`). -fn restore_double_dash(args: &[String]) -> Vec { - let raw_args: Vec = std::env::args().collect(); - restore_double_dash_with_raw(args, &raw_args) -} - -/// Testable version that takes raw_args explicitly. -fn restore_double_dash_with_raw(args: &[String], raw_args: &[String]) -> Vec { - if args.is_empty() { - return args.to_vec(); - } - - // If args already contain `--` (Clap preserved it), no restoration needed - if args.iter().any(|a| a == "--") { - return args.to_vec(); - } - - // Find `--` in the original command line - let sep_pos = match raw_args.iter().position(|a| a == "--") { - Some(pos) => pos, - None => return args.to_vec(), - }; - - // Count how many of our parsed args appeared before `--` in the original. - // Args before `--` are positional (e.g. test name), args after are flags. - let args_before_sep = raw_args[..sep_pos] - .iter() - .filter(|a| args.contains(a)) - .count(); - - let mut result = Vec::with_capacity(args.len() + 1); - result.extend_from_slice(&args[..args_before_sep]); - result.push("--".to_string()); - result.extend_from_slice(&args[args_before_sep..]); - result -} - // --- Stream handlers --- struct CargoBuildHandler { @@ -291,7 +253,7 @@ where let mut cmd = resolved_command("cargo"); cmd.arg(subcommand); - let restored_args = restore_double_dash(args); + let restored_args = args_utils::restore_double_dash(args); for arg in &restored_args { cmd.arg(arg); } @@ -318,7 +280,7 @@ fn run_cargo_streamed( let mut cmd = resolved_command("cargo"); cmd.arg(subcommand); - let restored_args = restore_double_dash(args); + let restored_args = args_utils::restore_double_dash(args); for arg in &restored_args { cmd.arg(arg); } @@ -1275,6 +1237,7 @@ pub fn run_passthrough(args: &[OsString], verbose: u8) -> Result { #[cfg(test)] mod tests { use super::*; + use crate::core::args_utils::restore_double_dash_with_raw; #[test] fn test_restore_double_dash_with_separator() { diff --git a/src/core/args_utils.rs b/src/core/args_utils.rs new file mode 100644 index 000000000..f5418b0cc --- /dev/null +++ b/src/core/args_utils.rs @@ -0,0 +1,240 @@ +//! Utility functions for argument handling, particularly for restoring "--" escape +//! arguments that clap consumes during parsing. + +/// Restores "--" escape arguments that clap consumed. +/// Handles: +/// - Single/multiple "--" swallowed by clap (restores each at its original position) +/// - "--" already present in parsed (no change) +/// - No "--" in original command (no injection) +/// - Args appearing before/after "--" in original (preserves order) +/// - Interleaved "--" and args (preserves relative positions, e.g., "-- arg1 -- arg2") +/// - Duplicate args on both sides of "--" +/// +/// Returns parsed_args unchanged if raw has same or fewer "--" than parsed +/// (meaning clap didn't consume any, or preserved them). +pub fn restore_double_dash(parsed_args: &[String]) -> Vec { + let raw_args: Vec = std::env::args().collect(); + restore_double_dash_with_raw(parsed_args, &raw_args) +} + +/// Testable version that takes raw_args explicitly. +pub fn restore_double_dash_with_raw(parsed_args: &[String], raw_args: &[String]) -> Vec { + let raw_dash_count = raw_args.iter().filter(|a| a.as_str() == "--").count(); + let parsed_dash_count = parsed_args.iter().filter(|a| a.as_str() == "--").count(); + + if raw_dash_count <= parsed_dash_count { + return parsed_args.to_vec(); + } + + // Find all positions of "--" in raw_args (skip index 0 = "rtk") + let mut dash_positions: Vec = Vec::new(); + for (i, arg) in raw_args.iter().enumerate().skip(1) { + if arg == "--" { + dash_positions.push(i); + } + } + + if dash_positions.is_empty() { + return parsed_args.to_vec(); + } + + // Build result by inserting "--" at correct positions relative to parsed args + let mut result = Vec::new(); + let mut raw_idx = 1; // start after "rtk" + let mut dash_idx = 0; + + while raw_idx < raw_args.len() { + // Check if current position is a "--" that was swallowed + if dash_idx < dash_positions.len() && raw_idx == dash_positions[dash_idx] { + result.push("--".to_string()); + dash_idx += 1; + } else if parsed_args.contains(&raw_args[raw_idx]) { + // This arg is in parsed_args, add it + result.push(raw_args[raw_idx].clone()); + } + raw_idx += 1; + } + + result +} + +#[cfg(test)] +mod tests { + use super::*; + + fn restore_with_raw(parsed: &[&str], raw: &[&str]) -> Vec { + let parsed: Vec = parsed.iter().map(|s| s.to_string()).collect(); + let raw: Vec = raw.iter().map(|s| s.to_string()).collect(); + restore_double_dash_with_raw(parsed.as_slice(), raw.as_slice()) + } + + // ============ Single "--" swallowed ============ + + #[test] + fn test_single_dash_swallowed() { + // rtk git diff -- file → clap gave ["file"], restore "--" + let raw = vec!["rtk", "git", "diff", "--", "file"]; + let parsed = vec!["file"]; + assert_eq!(restore_with_raw(&parsed, &raw), vec!["--", "file"]); + } + + #[test] + fn test_args_before_dash() { + // rtk cargo test name -- --nocapture → args before "--" stay before + let raw = vec!["rtk", "cargo", "test", "name", "--", "--nocapture"]; + let parsed = vec!["name", "--nocapture"]; + assert_eq!( + restore_with_raw(&parsed, &raw), + vec!["name", "--", "--nocapture"] + ); + } + + // ============ Multiple "--" swallowed ============ + + #[test] + fn test_multiple_dashes_all_swallowed() { + // rtk git diff -- -- -- → all 3 "--" swallowed, consecutive in output + let raw = vec!["rtk", "git", "diff", "--", "--", "--"]; + let parsed: Vec<&str> = vec![]; + assert_eq!(restore_with_raw(&parsed, &raw), vec!["--", "--", "--"]); + } + + #[test] + fn test_dashes_with_args_between() { + // rtk git diff -- arg1 -- arg2 → both "--" consumed, preserve positions + let raw = vec!["rtk", "git", "diff", "--", "arg1", "--", "arg2"]; + let parsed = vec!["arg1", "arg2"]; + // Result: each "--" inserted at its original position relative to args + assert_eq!( + restore_with_raw(&parsed, &raw), + vec!["--", "arg1", "--", "arg2"] + ); + } + + #[test] + fn test_multiple_dashes_some_preserved() { + // rtk git diff -- -- → 2 in raw, 1 preserved in parsed + let raw = vec!["rtk", "git", "diff", "--", "--"]; + let parsed = vec!["--"]; + assert_eq!(restore_with_raw(&parsed, &raw), vec!["--", "--"]); + } + + #[test] + fn test_compound_command_with_dashes() { + // Multiple segments with "--" → restore all + let raw = vec!["rtk", "cmd1", "--", "arg1", "&&", "cmd2", "--", "file"]; + let parsed = vec!["file"]; + assert_eq!(restore_with_raw(&parsed, &raw), vec!["--", "--", "file"]); + } + + // ============ "--" already present (no change needed) ============ + + #[test] + fn test_dash_already_preserved() { + // rtk cargo clippy -p pkg -- -D warnings → clap kept "--" + let raw = vec![ + "rtk", "cargo", "clippy", "-p", "pkg", "--", "-D", "warnings", + ]; + let parsed = vec!["-p", "pkg", "--", "-D", "warnings"]; + assert_eq!( + restore_with_raw(&parsed, &raw), + vec!["-p", "pkg", "--", "-D", "warnings"] + ); + } + + #[test] + fn test_trailing_dash_preserved() { + // rtk git diff file -- → trailing "--" preserved + let raw = vec!["rtk", "git", "diff", "file", "--"]; + let parsed = vec!["file", "--"]; + assert_eq!(restore_with_raw(&parsed, &raw), vec!["file", "--"]); + } + + // ============ No "--" in original (no injection) ============ + + #[test] + fn test_no_dash_in_original() { + // Various cases: branch with /, range, bare word, flags only + // All should return args unchanged (no injection) + let cases = vec![ + ( + vec!["rtk", "git", "diff", "feature/auth"], + vec!["feature/auth"], + ), + ( + vec!["rtk", "git", "diff", "main...feature"], + vec!["main...feature"], + ), + (vec!["rtk", "git", "diff", "main"], vec!["main"]), + ( + vec!["rtk", "git", "diff", "--stat", "--cached"], + vec!["--stat", "--cached"], + ), + ]; + for (raw, parsed) in cases { + assert_eq!(restore_with_raw(&parsed, &raw), parsed); + } + } + + // ============ Edge cases ============ + + #[test] + fn test_duplicate_args_both_sides() { + // -p pkg1 -p pkg2 -- -p pkg3 → restore after last -p + let raw = vec![ + "rtk", "cargo", "clippy", "-p", "p1", "-p", "p2", "--", "-p", "p3", + ]; + let parsed = vec!["-p", "p1", "-p", "p2", "-p", "p3"]; + assert_eq!( + restore_with_raw(&parsed, &raw), + vec!["-p", "p1", "-p", "p2", "--", "-p", "p3"] + ); + } + + #[test] + fn test_empty_args() { + let raw = vec!["rtk", "cargo", "test"]; + let parsed: Vec<&str> = vec![]; + assert_eq!(restore_with_raw(&parsed, &raw), Vec::::new()); + } + + #[test] + fn test_cargo_clippy_missing_dash() { + // No "--" in original → no injection + let raw = vec!["rtk", "cargo", "clippy", "-D", "warnings"]; + let parsed = vec!["-D", "warnings"]; + assert_eq!(restore_with_raw(&parsed, &raw), vec!["-D", "warnings"]); + } + + // ============ Git diff specific cases ============ + + #[test] + fn test_git_diff_ref_before_path() { + // rtk git diff HEAD -- file + let raw = vec!["rtk", "git", "diff", "HEAD", "--", "file"]; + let parsed = vec!["HEAD", "file"]; + assert_eq!(restore_with_raw(&parsed, &raw), vec!["HEAD", "--", "file"]); + } + + #[test] + fn test_git_diff_flags_before_path() { + // rtk git diff --cached -- file + let raw = vec!["rtk", "git", "diff", "--cached", "--", "file"]; + let parsed = vec!["--cached", "file"]; + assert_eq!( + restore_with_raw(&parsed, &raw), + vec!["--cached", "--", "file"] + ); + } + + #[test] + fn test_git_diff_multiple_files() { + // Original issue: multiple files caused "fatal: bad revision" + let raw = vec!["rtk", "git", "diff", "--", "file1", "file2", "file3"]; + let parsed = vec!["file1", "file2", "file3"]; + assert_eq!( + restore_with_raw(&parsed, &raw), + vec!["--", "file1", "file2", "file3"] + ); + } +} diff --git a/src/core/mod.rs b/src/core/mod.rs index fe017d910..d5182bd34 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -1,5 +1,6 @@ //! Building blocks shared across all RTK modules. +pub mod args_utils; pub mod config; pub mod constants; pub mod display_helpers; From 508192dbdcc6a1a753afbd0c87f7255886fad2ea Mon Sep 17 00:00:00 2001 From: "Christian C. Berclaz" Date: Mon, 25 May 2026 15:50:17 +0200 Subject: [PATCH 36/39] fix(init): preserve settings.json symlink during atomic write atomic_write() replaced symlinked settings.json with a regular file. Now resolves symlink targets before the tempfile+rename so the link itself is preserved and the real file gets updated. Covers both absolute and relative symlink targets. Closes #653 --- src/hooks/init.rs | 74 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 70 insertions(+), 4 deletions(-) diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 0b1855719..713ad3187 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -383,13 +383,37 @@ fn write_if_changed(path: &Path, content: &str, name: &str, ctx: InitContext) -> } } +/// Resolve the final write target: if `path` is a symlink, follow it so +/// the atomic rename lands on the real file and the symlink is preserved. +fn resolve_atomic_target(path: &Path) -> Result { + if let Ok(meta) = fs::symlink_metadata(path) { + if meta.file_type().is_symlink() { + let link_target = fs::read_link(path) + .with_context(|| format!("Failed to read symlink target: {}", path.display()))?; + if link_target.is_absolute() { + return Ok(link_target); + } + let parent = path.parent().with_context(|| { + format!( + "Cannot resolve relative symlink for {}: no parent directory", + path.display() + ) + })?; + return Ok(parent.join(link_target)); + } + } + Ok(path.to_path_buf()) +} + /// Atomic write using tempfile + rename /// Prevents corruption on crash/interrupt +/// Follows symlinks so the link itself is preserved. fn atomic_write(path: &Path, content: &str) -> Result<()> { - let parent = path.parent().with_context(|| { + let target = resolve_atomic_target(path)?; + let parent = target.parent().with_context(|| { format!( "Cannot write to {}: path has no parent directory", - path.display() + target.display() ) })?; @@ -403,10 +427,10 @@ fn atomic_write(path: &Path, content: &str) -> Result<()> { .with_context(|| format!("Failed to write {} bytes to temp file", content.len()))?; // Atomic rename - temp_file.persist(path).with_context(|| { + temp_file.persist(&target).with_context(|| { format!( "Failed to atomically replace {} (disk full?)", - path.display() + target.display() ) })?; @@ -5349,6 +5373,48 @@ mod tests { assert_eq!(written, content); } + #[cfg(unix)] + #[test] + fn test_atomic_write_preserves_symlink() { + use std::os::unix::fs::symlink; + + let temp = TempDir::new().unwrap(); + let target_path = temp.path().join("real-settings.json"); + let link_path = temp.path().join("settings.json"); + + fs::write(&target_path, "{}").expect("seed target file"); + symlink(&target_path, &link_path).expect("create symlink"); + + atomic_write(&link_path, "{\"hooks\":{}}").unwrap(); + + let meta = fs::symlink_metadata(&link_path).unwrap(); + assert!(meta.file_type().is_symlink(), "symlink must survive"); + let written = fs::read_to_string(&target_path).unwrap(); + assert_eq!(written, "{\"hooks\":{}}"); + } + + #[cfg(unix)] + #[test] + fn test_atomic_write_preserves_relative_symlink() { + use std::os::unix::fs::symlink; + + let temp = TempDir::new().unwrap(); + let subdir = temp.path().join("real"); + fs::create_dir(&subdir).unwrap(); + let target_path = subdir.join("settings.json"); + let link_path = temp.path().join("settings.json"); + + fs::write(&target_path, "{}").expect("seed target file"); + symlink(Path::new("real/settings.json"), &link_path).expect("create relative symlink"); + + atomic_write(&link_path, "{\"patched\":true}").unwrap(); + + let meta = fs::symlink_metadata(&link_path).unwrap(); + assert!(meta.file_type().is_symlink(), "symlink must survive"); + let written = fs::read_to_string(&target_path).unwrap(); + assert_eq!(written, "{\"patched\":true}"); + } + // Test for preserve_order round-trip #[test] fn test_preserve_order_round_trip() { From f1474b4c12ec3a3d9f8e46b14bfd8b2d70119137 Mon Sep 17 00:00:00 2001 From: "Christian C. Berclaz" Date: Sun, 31 May 2026 11:15:38 +0200 Subject: [PATCH 37/39] fix(init): use fs::canonicalize for symlink resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplify resolve_atomic_target() per review feedback — fs::canonicalize handles chained symlinks, relative paths, and all edge cases. Co-authored-by: Roopesh --- src/hooks/init.rs | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 713ad3187..4c68ec49b 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -385,31 +385,15 @@ fn write_if_changed(path: &Path, content: &str, name: &str, ctx: InitContext) -> /// Resolve the final write target: if `path` is a symlink, follow it so /// the atomic rename lands on the real file and the symlink is preserved. -fn resolve_atomic_target(path: &Path) -> Result { - if let Ok(meta) = fs::symlink_metadata(path) { - if meta.file_type().is_symlink() { - let link_target = fs::read_link(path) - .with_context(|| format!("Failed to read symlink target: {}", path.display()))?; - if link_target.is_absolute() { - return Ok(link_target); - } - let parent = path.parent().with_context(|| { - format!( - "Cannot resolve relative symlink for {}: no parent directory", - path.display() - ) - })?; - return Ok(parent.join(link_target)); - } - } - Ok(path.to_path_buf()) +fn resolve_atomic_target(path: &Path) -> PathBuf { + fs::canonicalize(path).unwrap_or_else(|_| path.to_path_buf()) } /// Atomic write using tempfile + rename /// Prevents corruption on crash/interrupt /// Follows symlinks so the link itself is preserved. fn atomic_write(path: &Path, content: &str) -> Result<()> { - let target = resolve_atomic_target(path)?; + let target = resolve_atomic_target(path); let parent = target.parent().with_context(|| { format!( "Cannot write to {}: path has no parent directory", From 41285725e446706204943d826d89e19608df0c03 Mon Sep 17 00:00:00 2001 From: patrick Date: Wed, 27 May 2026 23:37:15 +0200 Subject: [PATCH 38/39] fix(gh): show fallback note when PR/issue body is filtered to empty When `filter_markdown_body()` strips a PR or issue body to empty (body contained only badges, images, HTML comments, or horizontal rules), `format_pr_view` and `format_issue_view` previously skipped the body section silently. Users had no indication that body content had been present and filtered. Now both views emit a fallback note when the filtered body is empty but the raw body was not: PR view: (body contained only badges/images/comments) Issue view: Description: (body contained only badges/images/comments) The 4 added tests cover: - PR body with only badges/images/comments -> fallback note appears - PR body with real content -> no fallback note (sanity) - PR body empty (raw) -> no fallback note (no signal to give) - Issue body badges-only -> fallback note appears Closes #235 Co-authored-by: polaminggkub-debug --- src/cmds/git/gh_cmd.rs | 87 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/src/cmds/git/gh_cmd.rs b/src/cmds/git/gh_cmd.rs index 156b81e2a..01cf2ac39 100644 --- a/src/cmds/git/gh_cmd.rs +++ b/src/cmds/git/gh_cmd.rs @@ -434,6 +434,8 @@ fn format_pr_view(json: &Value, ultra_compact: bool) -> String { for line in body_filtered.lines() { out.push_str(&format!(" {}\n", line)); } + } else { + out.push_str("\n (body contained only badges/images/comments)\n"); } } } @@ -694,6 +696,8 @@ fn format_issue_view(json: &Value) -> String { for line in body_filtered.lines() { out.push_str(&format!(" {}\n", line)); } + } else { + out.push_str("\n Description: (body contained only badges/images/comments)\n"); } } } @@ -1547,4 +1551,87 @@ ___ assert!(result.contains("## Test Plan")); assert!(result.contains("Filter HTML comments")); } + + #[test] + fn test_format_pr_view_body_badges_only_shows_fallback_note() { + // PR body that filter_markdown_body would strip entirely: + // HTML comments, badge links, image-only lines, horizontal rules. + let body = "\n\ + [![CI](https://shields.io/badge.svg)](https://ci.example.com)\n\ + ![screenshot](https://example.com/img.png)\n\ + ---\n"; + let json = serde_json::json!({ + "number": 42, + "title": "Test PR", + "state": "OPEN", + "author": { "login": "octocat" }, + "url": "https://github.com/foo/bar/pull/42", + "mergeable": "MERGEABLE", + "body": body, + }); + let out = format_pr_view(&json, false); + assert!( + out.contains("(body contained only badges/images/comments)"), + "expected fallback note when body filters to empty, got:\n{}", + out + ); + } + + #[test] + fn test_format_pr_view_body_with_content_no_fallback_note() { + // Sanity check: real content should NOT trigger the fallback note. + let json = serde_json::json!({ + "number": 42, + "title": "Test PR", + "state": "OPEN", + "author": { "login": "octocat" }, + "url": "https://github.com/foo/bar/pull/42", + "mergeable": "MERGEABLE", + "body": "## Summary\nFix the thing.\n", + }); + let out = format_pr_view(&json, false); + assert!( + !out.contains("(body contained only badges/images/comments)"), + "fallback note should not fire when body has real content, got:\n{}", + out + ); + assert!(out.contains("## Summary")); + assert!(out.contains("Fix the thing.")); + } + + #[test] + fn test_format_pr_view_empty_body_no_fallback_note() { + // Empty body should not trigger the note either (nothing to flag). + let json = serde_json::json!({ + "number": 42, + "title": "Test PR", + "state": "OPEN", + "author": { "login": "octocat" }, + "url": "https://github.com/foo/bar/pull/42", + "mergeable": "MERGEABLE", + "body": "", + }); + let out = format_pr_view(&json, false); + assert!(!out.contains("(body contained only badges/images/comments)")); + } + + #[test] + fn test_format_issue_view_body_badges_only_shows_fallback_note() { + let body = "\n\ + [![status](https://shields.io/s.svg)](https://example.com)\n"; + let json = serde_json::json!({ + "number": 99, + "title": "Test Issue", + "state": "OPEN", + "author": { "login": "octocat" }, + "url": "https://github.com/foo/bar/issues/99", + "body": body, + }); + let out = format_issue_view(&json); + assert!( + out.contains("(body contained only badges/images/comments)"), + "expected fallback note when issue body filters to empty, got:\n{}", + out + ); + } } From f69ad6ed6f3f17b66e203613e3015fee517783f9 Mon Sep 17 00:00:00 2001 From: xiaozh Date: Mon, 1 Jun 2026 00:00:43 +0800 Subject: [PATCH 39/39] fix(go): respect build failure exit status Gate go build success output on the child exit code and surface unrecognized non-zero build output instead of reporting success. Closes #2185. --- src/cmds/go/go_cmd.rs | 68 +++++++++++++++++- src/core/runner.rs | 156 ++++++++++++++++++++++++++++-------------- 2 files changed, 168 insertions(+), 56 deletions(-) diff --git a/src/cmds/go/go_cmd.rs b/src/cmds/go/go_cmd.rs index e962e3e1c..a39e8bb1b 100644 --- a/src/cmds/go/go_cmd.rs +++ b/src/cmds/go/go_cmd.rs @@ -92,11 +92,11 @@ pub fn run_build(args: &[String], verbose: u8) -> Result { eprintln!("Running: go build {}", args.join(" ")); } - runner::run_filtered( + runner::run_filtered_with_exit( cmd, "go build", &args.join(" "), - filter_go_build, + filter_go_build_with_exit, crate::core::runner::RunOptions::with_tee("go_build"), ) } @@ -571,6 +571,10 @@ fn is_go_test_failure_line(line: &str) -> bool { /// Filter go build output - show only errors pub(crate) fn filter_go_build(output: &str) -> String { + filter_go_build_with_exit(output, 0) +} + +fn filter_go_build_with_exit(output: &str, exit_code: i32) -> String { let mut errors: Vec = Vec::new(); for line in output.lines() { @@ -581,7 +585,11 @@ pub(crate) fn filter_go_build(output: &str) -> String { } if errors.is_empty() { - return "Go build: Success".to_string(); + return if exit_code == 0 { + "Go build: Success".to_string() + } else { + format_go_build_failure(output, exit_code) + }; } let mut result = String::new(); @@ -604,6 +612,37 @@ pub(crate) fn filter_go_build(output: &str) -> String { result.trim().to_string() } +fn format_go_build_failure(output: &str, exit_code: i32) -> String { + let lines: Vec = output + .lines() + .map(str::trim) + .filter(|line| !line.is_empty()) + .map(str::to_string) + .collect(); + + if lines.is_empty() { + return format!("Go build: failed (exit {})", exit_code); + } + + let mut result = String::new(); + result.push_str(&format!("Go build: failed (exit {})\n", exit_code)); + result.push_str("═══════════════════════════════════════\n"); + + const MAX_GO_BUILD_ERRORS: usize = CAP_ERRORS; + for (i, line) in lines.iter().take(MAX_GO_BUILD_ERRORS).enumerate() { + result.push_str(&format!("{}. {}\n", i + 1, truncate(line, 120))); + } + + if lines.len() > MAX_GO_BUILD_ERRORS { + result.push_str(&format!( + "\n… +{} more output lines\n", + lines.len() - MAX_GO_BUILD_ERRORS + )); + } + + result.trim().to_string() +} + fn is_go_build_error_line(line: &str) -> bool { let trimmed = line.trim(); if trimmed.is_empty() { @@ -646,6 +685,7 @@ fn is_go_build_error_line(line: &str) -> bool { "go: build failed", "go: error ", "error: ", + "pattern ", "go: updates to go.mod needed", "go: inconsistent vendoring", "no go files in ", @@ -991,6 +1031,28 @@ go: cannot load module missing listed in go.work file: open missing/go.mod: no s assert!(result.contains("go: cannot load module missing listed in go.work file")); } + #[test] + fn test_filter_go_build_preserves_package_pattern_errors() { + let output = r#"pattern ./...: directory prefix . does not contain main module or its selected dependencies +pattern ./...: directory prefix . does not contain modules listed in go.work or their selected dependencies"#; + + let result = filter_go_build(output); + assert!(result.contains("2 errors")); + assert!(result.contains("does not contain main module")); + assert!(result.contains("does not contain modules listed in go.work")); + assert!(!result.contains("Success")); + } + + #[test] + fn test_filter_go_build_nonzero_exit_never_reports_success() { + let output = "opaque go build failure from stderr"; + + let result = filter_go_build_with_exit(output, 1); + assert!(result.contains("Go build: failed (exit 1)")); + assert!(result.contains(output)); + assert!(!result.contains("Success")); + } + #[test] fn test_filter_go_vet_no_issues() { let output = ""; diff --git a/src/core/runner.rs b/src/core/runner.rs index 17f0997da..571c7634b 100644 --- a/src/core/runner.rs +++ b/src/core/runner.rs @@ -62,12 +62,79 @@ impl<'a> RunOptions<'a> { } } +pub type CaptureFilter<'a> = Box String + 'a>; +pub type ExitAwareCaptureFilter<'a> = Box String + 'a>; + pub enum RunMode<'a> { - Filtered(Box String + 'a>), + Filtered(CaptureFilter<'a>), + FilteredWithExit(ExitAwareCaptureFilter<'a>), Streamed(Box), Passthrough, } +fn run_captured_filter( + mut cmd: Command, + tool_name: &str, + cmd_label: &str, + filter_fn: F, + opts: RunOptions<'_>, + timer: tracking::TimedExecution, +) -> Result +where + F: Fn(&str, i32) -> String, +{ + let stdin_mode = if opts.inherit_stdin { + StdinMode::Inherit + } else { + StdinMode::Null + }; + let result = stream::run_streaming(&mut cmd, stdin_mode, FilterMode::CaptureOnly) + .with_context(|| format!("Failed to run {}", tool_name))?; + + let exit_code = result.exit_code; + let raw = &result.raw; + let raw_stdout = &result.raw_stdout; + + if opts.skip_filter_on_failure && exit_code != 0 { + if !result.raw_stdout.trim().is_empty() { + print!("{}", result.raw_stdout); + } + if !result.raw_stderr.trim().is_empty() { + eprint!("{}", result.raw_stderr); + } + timer.track(cmd_label, &format!("rtk {}", cmd_label), raw, raw); + return Ok(exit_code); + } + + let text_to_filter = if opts.filter_stdout_only { + raw_stdout + } else { + raw + }; + let filtered = filter_fn(text_to_filter, exit_code); + + if let Some(label) = opts.tee_label { + print_with_hint(&filtered, raw, label, exit_code); + } else if opts.no_trailing_newline { + print!("{}", filtered); + } else { + println!("{}", filtered); + } + + let raw_for_tracking = if opts.filter_stdout_only { + raw_stdout + } else { + raw + }; + timer.track( + cmd_label, + &format!("rtk {}", cmd_label), + raw_for_tracking, + &filtered, + ); + Ok(exit_code) +} + pub fn run( mut cmd: Command, tool_name: &str, @@ -79,58 +146,22 @@ pub fn run( let cmd_label = format!("{} {}", tool_name, args_display); match mode { - RunMode::Filtered(filter_fn) => { - let stdin_mode = if opts.inherit_stdin { - StdinMode::Inherit - } else { - StdinMode::Null - }; - let result = stream::run_streaming(&mut cmd, stdin_mode, FilterMode::CaptureOnly) - .with_context(|| format!("Failed to run {}", tool_name))?; - - let exit_code = result.exit_code; - let raw = &result.raw; - let raw_stdout = &result.raw_stdout; - - if opts.skip_filter_on_failure && exit_code != 0 { - if !result.raw_stdout.trim().is_empty() { - print!("{}", result.raw_stdout); - } - if !result.raw_stderr.trim().is_empty() { - eprint!("{}", result.raw_stderr); - } - timer.track(&cmd_label, &format!("rtk {}", cmd_label), raw, raw); - return Ok(exit_code); - } - - let text_to_filter = if opts.filter_stdout_only { - raw_stdout - } else { - raw - }; - let filtered = filter_fn(text_to_filter); - - if let Some(label) = opts.tee_label { - print_with_hint(&filtered, raw, label, exit_code); - } else if opts.no_trailing_newline { - print!("{}", filtered); - } else { - println!("{}", filtered); - } - - let raw_for_tracking = if opts.filter_stdout_only { - raw_stdout - } else { - raw - }; - timer.track( - &cmd_label, - &format!("rtk {}", cmd_label), - raw_for_tracking, - &filtered, - ); - Ok(exit_code) - } + RunMode::Filtered(filter_fn) => run_captured_filter( + cmd, + tool_name, + &cmd_label, + move |text, _| filter_fn(text), + opts, + timer, + ), + RunMode::FilteredWithExit(filter_fn) => run_captured_filter( + cmd, + tool_name, + &cmd_label, + move |text, exit_code| filter_fn(text, exit_code), + opts, + timer, + ), RunMode::Streamed(filter) => { let result = stream::run_streaming(&mut cmd, StdinMode::Null, FilterMode::Streaming(filter)) @@ -182,6 +213,25 @@ where ) } +pub fn run_filtered_with_exit( + cmd: Command, + tool_name: &str, + args_display: &str, + filter_fn: F, + opts: RunOptions<'_>, +) -> Result +where + F: Fn(&str, i32) -> String, +{ + run( + cmd, + tool_name, + args_display, + RunMode::FilteredWithExit(Box::new(filter_fn)), + opts, + ) +} + pub fn run_passthrough(tool: &str, args: &[std::ffi::OsString], verbose: u8) -> Result { if verbose > 0 { eprintln!("{} passthrough: {:?}", tool, args);