From 362065ec9ea35eca247ecd03a71615bbf978501b Mon Sep 17 00:00:00 2001 From: Husam Date: Thu, 11 Jun 2026 19:04:07 +0300 Subject: [PATCH 01/66] fix(read): only treat /* at line start as block comment opener MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MinimalFilter used trimmed.contains() to detect block-comment openers, so any code line containing /* as a substring (string literals, globs like "src/*.rs", regex patterns) was dropped — and when no */ followed on the same line, in_block_comment leaked and silently swallowed the rest of the file. Match openers with starts_with() so only genuine comment lines enter block-comment state. Fixes #2385 --- src/core/filter.rs | 74 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/src/core/filter.rs b/src/core/filter.rs index 90f89ade65..e0a95eeeca 100644 --- a/src/core/filter.rs +++ b/src/core/filter.rs @@ -172,8 +172,10 @@ impl FilterStrategy for MinimalFilter { // Handle block comments if let (Some(start), Some(end)) = (patterns.block_start, patterns.block_end) { + // starts_with, not contains: `/*` inside a string literal or + // glob (e.g. "src/*.rs") must not open a comment block (#2385) if !in_docstring - && trimmed.contains(start) + && trimmed.starts_with(start) && !trimmed.starts_with(patterns.doc_block_start.unwrap_or("###")) { in_block_comment = true; @@ -470,6 +472,76 @@ fn main() { assert!(result.contains("fn main()")); } + // --- block comment detection (#2385) --- + + #[test] + fn test_minimal_keeps_code_with_inline_block_marker() { + // `/*` inside a string literal is not a comment opener + let code = "let glob = \"src/*.rs\";\nfn bar() {}\nfn baz() {}"; + let filter = MinimalFilter; + let result = filter.filter(code, &Language::Rust); + assert!( + result.contains("let glob = \"src/*.rs\";"), + "line with /* in string literal must be kept, got:\n{}", + result + ); + assert!( + result.contains("fn bar()") && result.contains("fn baz()"), + "block-comment state must not leak past a non-comment line, got:\n{}", + result + ); + } + + #[test] + fn test_minimal_same_line_block_comment_no_state_leak() { + let code = "/* inline comment */\nfn foo() {}"; + let filter = MinimalFilter; + let result = filter.filter(code, &Language::Rust); + assert!( + !result.contains("inline comment"), + "comment-only line is dropped" + ); + assert!( + result.contains("fn foo()"), + "code after same-line block comment must be kept, got:\n{}", + result + ); + } + + #[test] + fn test_minimal_still_drops_multiline_block_comment() { + let code = "/* start\nstill comment\n*/\nfn after() {}"; + let filter = MinimalFilter; + let result = filter.filter(code, &Language::Rust); + assert!(!result.contains("still comment")); + assert!(result.contains("fn after()")); + } + + #[test] + fn test_minimal_keeps_python_inline_triple_quote_assignment() { + let code = "x = \"\"\"inline\"\"\"\ndef f():\n pass"; + let filter = MinimalFilter; + let result = filter.filter(code, &Language::Python); + assert!( + result.contains("x = "), + "assignment with inline triple-quote must be kept, got:\n{}", + result + ); + assert!(result.contains("def f():")); + } + + #[test] + fn test_minimal_keeps_url_with_trailing_line_comment() { + let code = "const API: &str = \"http://example.com/v1\"; // endpoint\nfn foo() {}"; + let filter = MinimalFilter; + let result = filter.filter(code, &Language::Rust); + assert!( + result.contains("http://example.com/v1"), + "URL line must be kept, got:\n{}", + result + ); + } + // --- truncation accuracy --- #[test] From 143f503b346f5a8a8b0122fd93b54c0fe99e1415 Mon Sep 17 00:00:00 2001 From: elite133713 <26662443+elite133713@users.noreply.github.com> Date: Fri, 19 Jun 2026 18:38:49 -0500 Subject: [PATCH 02/66] fix(init): create Claude config dir for global init --- src/hooks/init.rs | 107 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 28363f4ce6..32c2a4efd3 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -373,6 +373,7 @@ fn write_if_changed(path: &Path, content: &str, name: &str, ctx: InitContext) -> println!("[dry-run] content:\n{}", content); } } else { + ensure_parent_dir(path)?; atomic_write(path, content) .with_context(|| format!("Failed to write {}: {}", name, path.display()))?; if verbose > 0 { @@ -383,6 +384,19 @@ fn write_if_changed(path: &Path, content: &str, name: &str, ctx: InitContext) -> } } +fn ensure_parent_dir(path: &Path) -> Result<()> { + let Some(parent) = path.parent() else { + return Ok(()); + }; + + if parent.as_os_str().is_empty() { + return Ok(()); + } + + fs::create_dir_all(parent) + .with_context(|| format!("Failed to create directory: {}", parent.display())) +} + /// 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) -> PathBuf { @@ -1013,6 +1027,7 @@ fn patch_settings_json_command( } // Atomic write + ensure_parent_dir(&settings_path)?; atomic_write(&settings_path, &serialized)?; println!("\n settings.json: hook added"); @@ -5872,15 +5887,46 @@ mod tests { fn with_claude_dir_override(tmp: &TempDir, f: F) { let _guard = CLAUDE_DIR_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let claude_dir = tmp.path().join(CLAUDE_DIR); + let home_dir = tmp.path().join("home"); fs::create_dir_all(&claude_dir).unwrap(); let orig = std::env::var_os("CLAUDE_CONFIG_DIR"); + let orig_home = std::env::var_os("HOME"); + std::env::set_var("CLAUDE_CONFIG_DIR", &claude_dir); + std::env::set_var("HOME", &home_dir); + f(&claude_dir); + match orig { + Some(v) => std::env::set_var("CLAUDE_CONFIG_DIR", v), + None => std::env::remove_var("CLAUDE_CONFIG_DIR"), + } + match orig_home { + Some(v) => std::env::set_var("HOME", v), + None => std::env::remove_var("HOME"), + } + } + + fn with_missing_claude_dir_override(tmp: &TempDir, f: F) { + let _guard = CLAUDE_DIR_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let claude_dir = tmp.path().join(CLAUDE_DIR); + let home_dir = tmp.path().join("home"); + assert!( + !claude_dir.exists(), + "test precondition: Claude config dir must be missing" + ); + + let orig = std::env::var_os("CLAUDE_CONFIG_DIR"); + let orig_home = std::env::var_os("HOME"); std::env::set_var("CLAUDE_CONFIG_DIR", &claude_dir); + std::env::set_var("HOME", &home_dir); f(&claude_dir); match orig { Some(v) => std::env::set_var("CLAUDE_CONFIG_DIR", v), None => std::env::remove_var("CLAUDE_CONFIG_DIR"), } + match orig_home { + Some(v) => std::env::set_var("HOME", v), + None => std::env::remove_var("HOME"), + } } fn with_pi_dir_override(tmp: &TempDir, f: F) { @@ -5919,6 +5965,28 @@ mod tests { }); } + #[test] + fn test_global_default_mode_creates_missing_claude_dir() { + let tmp = TempDir::new().unwrap(); + with_missing_claude_dir_override(&tmp, |claude_dir| { + run_default_mode(true, PatchMode::Auto, false, InitContext::default()).unwrap(); + + assert!( + claude_dir.exists(), + "missing Claude config dir must be created" + ); + assert!(claude_dir.join(RTK_MD).exists(), "RTK.md must be created"); + assert!( + claude_dir.join(CLAUDE_MD).exists(), + "CLAUDE.md must be created" + ); + assert!( + claude_dir.join(SETTINGS_JSON).exists(), + "settings.json must be created" + ); + }); + } + #[test] fn test_global_uninstall_removes_artifacts() { let tmp = TempDir::new().unwrap(); @@ -6010,6 +6078,28 @@ mod tests { }); } + #[test] + fn test_global_hook_only_mode_creates_missing_claude_dir() { + let tmp = TempDir::new().unwrap(); + with_missing_claude_dir_override(&tmp, |claude_dir| { + run_hook_only_mode(true, PatchMode::Auto, false, InitContext::default()).unwrap(); + + assert!( + claude_dir.exists(), + "missing Claude config dir must be created" + ); + assert!( + !claude_dir.join(RTK_MD).exists(), + "RTK.md must NOT be created in hook-only mode" + ); + let settings = fs::read_to_string(claude_dir.join(SETTINGS_JSON)).unwrap(); + assert!( + settings.contains(CLAUDE_HOOK_COMMAND), + "settings.json must contain hook command" + ); + }); + } + #[test] fn test_run_default_mode_dry_run_writes_nothing() { let tmp = TempDir::new().unwrap(); @@ -6035,6 +6125,23 @@ mod tests { }); } + #[test] + fn test_run_default_mode_dry_run_does_not_create_missing_claude_dir() { + let tmp = TempDir::new().unwrap(); + with_missing_claude_dir_override(&tmp, |claude_dir| { + let dry = InitContext { + dry_run: true, + ..Default::default() + }; + run_default_mode(true, PatchMode::Auto, false, dry).unwrap(); + + assert!( + !claude_dir.exists(), + "dry-run must not create missing Claude config dir" + ); + }); + } + #[test] fn test_uninstall_dry_run_preserves_artifacts() { let tmp = TempDir::new().unwrap(); From 2b08f52beb2fadb3f4e11c768041df49baa2065e Mon Sep 17 00:00:00 2001 From: Husam Date: Fri, 26 Jun 2026 12:00:56 +0300 Subject: [PATCH 03/66] refactor(read): drop redundant test comment --- src/core/filter.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/filter.rs b/src/core/filter.rs index e0a95eeeca..ad04e48c62 100644 --- a/src/core/filter.rs +++ b/src/core/filter.rs @@ -476,7 +476,6 @@ fn main() { #[test] fn test_minimal_keeps_code_with_inline_block_marker() { - // `/*` inside a string literal is not a comment opener let code = "let glob = \"src/*.rs\";\nfn bar() {}\nfn baz() {}"; let filter = MinimalFilter; let result = filter.filter(code, &Language::Rust); From c11b018d003efaff3ee3bdb9ec36580943af91fb Mon Sep 17 00:00:00 2001 From: rNoz Date: Tue, 14 Jul 2026 22:05:29 +0200 Subject: [PATCH 04/66] fix(init): --agent cursor installs Cursor only and creates ~/.cursor rtk init -g --agent cursor fell through to the shared init path with install_claude derived only from !opencode, so it created RTK.md, CLAUDE.md, and patched settings.json under ~/.claude in addition to patching .cursor/hooks.json. On machines without ~/.claude it failed outright when creating the temp file. Two fixes: - Gate install_claude on the absence of a sibling agent target (Cursor, Windsurf, Cline) so only the default or explicit Claude target installs Claude Code files. - Create ~/.cursor before patching hooks.json; atomic_write places its temp file in the target's parent, which failed on a fresh machine where the directory did not yet exist. Closes #2097 --- src/hooks/init.rs | 8 ++++ src/main.rs | 7 ++- tests/init_agent_cursor_test.rs | 79 +++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 tests/init_agent_cursor_test.rs diff --git a/src/hooks/init.rs b/src/hooks/init.rs index c02dc1e116..5af5dfc396 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -3521,6 +3521,14 @@ fn install_cursor_hooks(ctx: InitContext) -> Result<()> { let InitContext { verbose, dry_run } = ctx; let cursor_dir = resolve_cursor_dir()?; + // Ensure ~/.cursor exists before any write: atomic_write creates its temp + // file in the target's parent, which fails on a fresh machine where the + // directory does not yet exist (part of #2097). + if !dry_run { + fs::create_dir_all(&cursor_dir) + .with_context(|| format!("Failed to create {}", cursor_dir.display()))?; + } + // Migrate old hook script if present let old_hook = cursor_dir.join("hooks").join(REWRITE_HOOK_FILE); if old_hook.exists() { diff --git a/src/main.rs b/src/main.rs index 1c85bb55f9..124d62fb65 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2031,10 +2031,15 @@ fn run_cli() -> Result { hooks::init::run_droid_mode(global, ctx)?; } else { let install_opencode = opencode; - let install_claude = !opencode; let install_cursor = agent == Some(AgentTarget::Cursor); let install_windsurf = agent == Some(AgentTarget::Windsurf); let install_cline = agent == Some(AgentTarget::Cline); + // Sibling agents (Cursor, Windsurf, Cline) fall through to this + // shared init path; only the default (no --agent) or an explicit + // Claude target should install Claude Code files. Without this + // guard, `rtk init -g --agent cursor` writes into ~/.claude (#2097). + let install_claude = + !opencode && !install_cursor && !install_windsurf && !install_cline; let patch_mode = if auto_patch { hooks::init::PatchMode::Auto diff --git a/tests/init_agent_cursor_test.rs b/tests/init_agent_cursor_test.rs new file mode 100644 index 0000000000..664dda6f9d --- /dev/null +++ b/tests/init_agent_cursor_test.rs @@ -0,0 +1,79 @@ +#![cfg(unix)] +//! Regression for #2097: `rtk init -g --agent cursor` must only touch `.cursor/` +//! and never create or patch Claude Code files under `~/.claude`. + +use std::process::{Command, Stdio}; + +fn rtk() -> Command { + Command::new(env!("CARGO_BIN_EXE_rtk")) +} + +#[test] +fn agent_cursor_dry_run_never_touches_claude() { + let home = tempfile::tempdir().unwrap(); + let out = rtk() + .args(["init", "-g", "--agent", "cursor", "--dry-run"]) + .env("HOME", home.path()) + .output() + .unwrap(); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + !stdout.contains("/.claude/"), + "cursor init must not touch .claude:\n{stdout}" + ); + assert!( + stdout.contains("/.cursor/"), + "cursor init must patch .cursor hooks.json:\n{stdout}" + ); + // Dry-run must not create anything on disk. + assert!( + !home.path().join(".claude").exists(), + "dry-run must not create ~/.claude" + ); + assert!( + !home.path().join(".cursor").exists(), + "dry-run must not create ~/.cursor" + ); +} + +#[test] +fn agent_cursor_real_write_creates_no_claude_dir() { + // The original #2097 failure was a hard error while creating a temp file + // under a non-existent ~/.claude. Prove a real (non-dry-run) install never + // creates .claude and does write .cursor/hooks.json. + let home = tempfile::tempdir().unwrap(); + let out = rtk() + .args(["init", "-g", "--agent", "cursor", "--auto-patch"]) + .env("HOME", home.path()) + .stdin(Stdio::null()) + .output() + .unwrap(); + assert!( + out.status.success(), + "cursor init failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert!( + !home.path().join(".claude").exists(), + "cursor init must not create ~/.claude" + ); + assert!( + home.path().join(".cursor").join("hooks.json").exists(), + "cursor init must write .cursor/hooks.json" + ); +} + +#[test] +fn default_dry_run_still_installs_claude() { + let home = tempfile::tempdir().unwrap(); + let out = rtk() + .args(["init", "-g", "--dry-run"]) + .env("HOME", home.path()) + .output() + .unwrap(); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + stdout.contains("/.claude/"), + "default init must still install Claude Code files:\n{stdout}" + ); +} From 527b386cd3ea8547356187086eb33ca531a31d7d Mon Sep 17 00:00:00 2001 From: imkp1 Date: Fri, 31 Jul 2026 01:33:35 +0530 Subject: [PATCH 05/66] fix(read): track Python triple-quoted strings in minimal filter Python has no block comments, but the minimal filter treats """ as one. Two things go wrong. A line like `QUERY = """` contains the delimiter without starting with it, so it is taken for a block-comment opener. The same """ then satisfies the closing test on that one line, so the line is dropped and its string body is left behind as loose text. A single-line docstring such as """Module doc.""" toggles the docstring flag once and never back, so the filter stays in docstring mode for the rest of the file and stops stripping comments entirely. Give Python a string-aware path instead. Minimal keeps docstrings, so the only thing to remove is # comments and the only state needed is whether we are inside a triple-quoted string. Delimiter kinds are tracked separately so a ''' inside a """ string does not close it. Other languages keep the existing block-comment walk untouched. Refs #3260 --- src/core/filter.rs | 171 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 171 insertions(+) diff --git a/src/core/filter.rs b/src/core/filter.rs index 4c712814e8..fcd0927ce9 100644 --- a/src/core/filter.rs +++ b/src/core/filter.rs @@ -157,8 +157,85 @@ pub struct MinimalFilter; static MULTIPLE_BLANK_LINES: LazyLock = LazyLock::new(|| Regex::new(r"\n{3,}").unwrap()); +/// Advances triple-quoted string state across one line, returning the delimiter +/// still open at end of line. The two quote kinds are tracked separately so a +/// `'''` inside a `"""` string is text rather than a close. +fn advance_triple_quote(line: &str, open: Option<&'static str>) -> Option<&'static str> { + let bytes = line.as_bytes(); + let mut state = open; + let mut i = 0; + + while i + 3 <= bytes.len() { + let delim = match &bytes[i..i + 3] { + b"\"\"\"" => Some("\"\"\""), + b"'''" => Some("'''"), + _ => None, + }; + + match (state, delim) { + (Some(current), Some(found)) if current == found => { + state = None; + i += 3; + } + (None, Some(found)) => { + state = Some(found); + i += 3; + } + _ => i += 1, + } + } + state +} + +/// Python has no block comments. `"""` opens a *string*, which may be a +/// docstring or an ordinary value, so it cannot be matched with the +/// line-oriented block-comment rules the other languages use: a line such as +/// `QUERY = """` both contains and "closes" the delimiter, and a single-line +/// docstring toggles the state once and never back. +/// +/// Minimal keeps docstrings, so the only thing to remove here is `#` comments, +/// and the only state needed is whether we are inside a triple-quoted string. +fn filter_python_minimal(content: &str) -> String { + let mut result = String::with_capacity(content.len()); + let mut open_string: Option<&'static str> = None; + + for line in content.lines() { + let trimmed = line.trim(); + + // Inside a string every line is literal text, including one that starts + // with `#`. + if open_string.is_some() { + result.push_str(line); + result.push('\n'); + open_string = advance_triple_quote(line, open_string); + continue; + } + + // A comment's contents are not code, so any delimiter in it is not real. + if trimmed.starts_with('#') { + continue; + } + + if trimmed.is_empty() { + result.push('\n'); + continue; + } + + result.push_str(line); + result.push('\n'); + open_string = advance_triple_quote(line, None); + } + + let result = MULTIPLE_BLANK_LINES.replace_all(&result, "\n\n"); + result.trim().to_string() +} + impl FilterStrategy for MinimalFilter { fn filter(&self, content: &str, lang: &Language) -> String { + if *lang == Language::Python { + return filter_python_minimal(content); + } + let patterns = lang.comment_patterns(); let mut result = String::with_capacity(content.len()); let mut in_block_comment = false; @@ -453,6 +530,100 @@ mod tests { ); } + // --- Python triple-quoted strings in minimal mode --- + + #[test] + fn test_minimal_python_keeps_multiline_string_assignment() { + let code = r#"QUERY = """ +SELECT id +FROM users +""" +import os +"#; + let result = MinimalFilter.filter(code, &Language::Python); + assert!( + result.contains(r#"QUERY = """#), + "the line opening the string was dropped, leaving its body as loose text:\n{}", + result + ); + assert!( + result.contains("SELECT id"), + "string body lost:\n{}", + result + ); + assert!( + result.contains("import os"), + "code after string lost:\n{}", + result + ); + } + + #[test] + fn test_minimal_python_strips_comments_after_oneline_docstring() { + let code = r#""""Module doc.""" +import os +# strip me +def go(): + return 1 +"#; + let result = MinimalFilter.filter(code, &Language::Python); + assert!( + !result.contains("# strip me"), + "a one-line docstring left the filter stuck in docstring mode, so no \ + later comment was stripped:\n{}", + result + ); + assert!( + result.contains(r#""""Module doc.""""#), + "docstring lost:\n{}", + result + ); + assert!(result.contains("def go():"), "code lost:\n{}", result); + } + + #[test] + fn test_minimal_python_keeps_hash_inside_docstring() { + let code = r#"""" +# not a comment, it is prose +""" +x = 1 +"#; + let result = MinimalFilter.filter(code, &Language::Python); + assert!( + result.contains("# not a comment, it is prose"), + "text inside a docstring was stripped as a comment:\n{}", + result + ); + assert!(result.contains("x = 1")); + } + + #[test] + fn test_minimal_python_single_quoted_docstring_does_not_close_double() { + let code = r#""""Doc mentioning ''' inline.""" +# strip me +x = 1 +"#; + let result = MinimalFilter.filter(code, &Language::Python); + assert!( + !result.contains("# strip me"), + "a ''' inside a \"\"\" string confused the tracker:\n{}", + result + ); + assert!(result.contains("x = 1")); + } + + #[test] + fn test_minimal_python_still_strips_plain_comments() { + let code = r#"# leading comment +import os +x = 1 # trailing comment kept, matching prior behavior +"#; + let result = MinimalFilter.filter(code, &Language::Python); + assert!(!result.contains("# leading comment")); + assert!(result.contains("import os")); + assert!(result.contains("x = 1")); + } + #[test] fn test_minimal_filter_removes_comments() { let code = r#" From b2b6485ec1ecebc2871c6203c2bf07fc9c1a1d05 Mon Sep 17 00:00:00 2001 From: imkp1 Date: Fri, 31 Jul 2026 01:36:12 +0530 Subject: [PATCH 06/66] docs(core): note why Python skips the block-comment walk --- src/core/README.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/core/README.md b/src/core/README.md index ab3529ef59..abdb4aa184 100644 --- a/src/core/README.md +++ b/src/core/README.md @@ -31,6 +31,28 @@ Three-tier filter lookup (first match wins): 2. `~/.config/rtk/filters.toml` (user-global) 3. Built-in filters concatenated by `build.rs` at compile time +## Source File Comment Stripping + +`src/core/filter.rs` is a separate engine from the TOML DSL: it filters *source +files* (used by `rtk read`) rather than command output. At `-l minimal` it +strips comments using the per-language delimiters in +`Language::comment_patterns()`. + +Python does not use that walk. It has no block comments — `"""` opens a +*string*, which may be a docstring or an ordinary value — so it gets a +string-aware path that removes `#` comments and leaves string contents alone. +Matching `"""` as a block delimiter misread both of these: + +```python +QUERY = """ # contains """ without starting with it +SELECT 1 +""" + +"""Module doc.""" # opens and closes on one line +``` + +Docstrings are kept at `minimal`; `aggressive` is what drops them. + ## Tracking Database Schema ```sql From 2398f8c6b7b2bc39d9b7c1d2463becf86eeaefda Mon Sep 17 00:00:00 2001 From: Jaideep Pyne <25899506+jaideeppyne@users.noreply.github.com> Date: Mon, 3 Aug 2026 16:58:56 +0530 Subject: [PATCH 07/66] fix(discover): distinguish zero-session scans --- src/discover/mod.rs | 1 + src/discover/report.rs | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/src/discover/mod.rs b/src/discover/mod.rs index 98d3eae765..d718661722 100644 --- a/src/discover/mod.rs +++ b/src/discover/mod.rs @@ -255,6 +255,7 @@ pub fn run( let report = DiscoverReport { sessions_scanned: sessions.len(), + all_projects: all, total_commands, already_rtk, since_days, diff --git a/src/discover/report.rs b/src/discover/report.rs index af4dd453d7..c00f875183 100644 --- a/src/discover/report.rs +++ b/src/discover/report.rs @@ -96,6 +96,8 @@ impl AgentIntegrationStatus { #[derive(Debug, Serialize)] pub struct DiscoverReport { pub sessions_scanned: usize, + #[serde(skip)] + pub all_projects: bool, pub total_commands: usize, pub already_rtk: usize, pub since_days: u64, @@ -141,6 +143,18 @@ pub fn format_text(report: &DiscoverReport, limit: usize, verbose: bool) -> Stri } )); + if report.sessions_scanned == 0 { + if report.all_projects { + out.push_str("\nNo sessions found in the selected time range.\n"); + } else { + out.push_str( + "\nNo sessions found for the current project. Try `rtk discover --all` to scan every project.\n", + ); + } + append_agent_notes(&mut out, report.agent_status); + return out; + } + if report.supported.is_empty() && report.unsupported.is_empty() { out.push_str("\nNo missed savings found. RTK usage looks good!\n"); append_agent_notes(&mut out, report.agent_status); @@ -277,6 +291,7 @@ mod tests { fn make_report(total_commands: usize, already_rtk: usize) -> DiscoverReport { DiscoverReport { sessions_scanned: 1, + all_projects: false, total_commands, already_rtk, since_days: 30, @@ -316,6 +331,31 @@ mod tests { assert!(output.contains("0 commands (0.0%)")); } + #[test] + fn test_zero_sessions_does_not_report_success() { + let mut report = make_report(0, 0); + report.sessions_scanned = 0; + + let output = format_text(&report, 10, false); + + assert!(output.contains("No sessions found for the current project")); + assert!(output.contains("rtk discover --all")); + assert!(!output.contains("RTK usage looks good")); + } + + #[test] + fn test_zero_sessions_all_projects_does_not_suggest_all() { + let mut report = make_report(0, 0); + report.sessions_scanned = 0; + report.all_projects = true; + + let output = format_text(&report, 10, false); + + assert!(output.contains("No sessions found in the selected time range")); + assert!(!output.contains("rtk discover --all")); + assert!(!output.contains("RTK usage looks good")); + } + // Full percent: 1000/1000 = 100.0% #[test] fn test_already_rtk_percent_full() { From d2d50540a1e896246aa773d13088fd841a7b3d5f Mon Sep 17 00:00:00 2001 From: Tem Revil Date: Thu, 13 Aug 2026 05:37:25 +0300 Subject: [PATCH 08/66] fix(discover): match the project filter case-insensitively on Windows `rtk discover -p ` matches the filter against project directory names with a case-sensitive `contains`, so on Windows it can come back empty even when the project is there. NTFS is case-insensitive and Claude Code builds the folder name from whatever cwd casing it was launched with, so one project can end up under both `E--proj` and `e--proj` while `std::env::current_dir()` hands back an uppercase drive letter. Compare case-insensitively under `cfg!(windows)`. Unix behavior is unchanged. Refs #2919 --- src/discover/provider.rs | 46 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/src/discover/provider.rs b/src/discover/provider.rs index 57a15ae4d5..8eb4fbb4ec 100644 --- a/src/discover/provider.rs +++ b/src/discover/provider.rs @@ -41,6 +41,27 @@ pub trait SessionProvider { pub struct ClaudeProvider; +/// Substring match of a project filter against a project directory name. +/// +/// On Windows the match is case-insensitive: NTFS paths are case-insensitive +/// and Claude Code encodes the project folder from whatever cwd casing it was +/// launched with, so the same project can produce sibling folders like +/// `E--proj` and `e--proj` while `std::env::current_dir()` reports an +/// uppercase drive letter. A case-sensitive match then finds zero sessions. +fn dir_matches_filter(dir_name: &str, filter: &str) -> bool { + dir_matches_filter_impl(dir_name, filter, cfg!(windows)) +} + +fn dir_matches_filter_impl(dir_name: &str, filter: &str, case_insensitive: bool) -> bool { + if case_insensitive { + dir_name + .to_ascii_lowercase() + .contains(&filter.to_ascii_lowercase()) + } else { + dir_name.contains(filter) + } +} + impl ClaudeProvider { /// Get the base directory for Claude Code projects. fn projects_dir() -> Result { @@ -81,7 +102,7 @@ impl ClaudeProvider { // Apply project filter: substring match on directory name if let Some(filter) = project_filter { let dir_name = path.file_name().and_then(|n| n.to_str()).unwrap_or(""); - if !dir_name.contains(filter) { + if !dir_matches_filter(dir_name, filter) { continue; } } @@ -417,6 +438,29 @@ mod tests { ); } + #[test] + fn test_dir_matches_filter_case_sensitive() { + assert!(dir_matches_filter_impl("-Users-foo-rtk", "rtk", false)); + assert!(!dir_matches_filter_impl("-Users-foo-RTK", "rtk", false)); + } + + #[test] + fn test_dir_matches_filter_case_insensitive_windows() { + // Uppercase cwd drive letter vs lowercase history folder + assert!(dir_matches_filter_impl( + "e--PROJECT-XXX-XXX-xxxx", + "E--PROJECT-XXX-XXX-xxxx", + true + )); + // And the reverse: lowercase cwd vs uppercase folder + assert!(dir_matches_filter_impl( + "E--PROJECT-app", + "e--PROJECT-app", + true + )); + assert!(!dir_matches_filter_impl("e--PROJECT-app", "e--OTHER", true)); + } + #[test] fn test_match_project_filter() { let encoded = ClaudeProvider::encode_project_path("/Users/foo/Sites/rtk"); From 088ceda99e4f95623cda1e7583b5fcf12c1b4c83 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 9 Sep 2026 00:30:30 +0000 Subject: [PATCH 09/66] build(deps): bump actions/setup-go from 5 to 7 Bumps [actions/setup-go](https://github.com/actions/setup-go) from 5 to 7. - [Release notes](https://github.com/actions/setup-go/releases) - [Commits](https://github.com/actions/setup-go/compare/v5...v7) --- updated-dependencies: - dependency-name: actions/setup-go dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 973ab8c4ef..aadeb369c3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -220,7 +220,7 @@ jobs: run: pip install ruff pytest mypy - name: Install Go - uses: actions/setup-go@v5 + uses: actions/setup-go@v7 with: go-version: "stable" From 806c0df361d17df148a44c611a2973b71dd220ab Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 9 Sep 2026 00:30:42 +0000 Subject: [PATCH 10/66] build(deps): bump actions/github-script from 7.1.0 to 9.0.0 Bumps [actions/github-script](https://github.com/actions/github-script) from 7.1.0 to 9.0.0. - [Release notes](https://github.com/actions/github-script/releases) - [Commits](https://github.com/actions/github-script/compare/f28e40c7f34bde8b3046d885e986cb6290c5673b...3a2844b7e9c422d3c10d287c895573f7108da1b3) --- updated-dependencies: - dependency-name: actions/github-script dependency-version: 9.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/pr-target-check.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr-target-check.yml b/.github/workflows/pr-target-check.yml index ac3ec13666..289dda7856 100644 --- a/.github/workflows/pr-target-check.yml +++ b/.github/workflows/pr-target-check.yml @@ -21,7 +21,7 @@ jobs: permission-pull-requests: write - name: Add wrong-base label and comment - uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7 + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: github-token: ${{ steps.app-token.outputs.token }} script: | From a780d4377b8a2e6c8b32a03548b06e23c0a0aefb Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 9 Sep 2026 00:30:44 +0000 Subject: [PATCH 11/66] build(deps): bump actions/stale from 9 to 11 Bumps [actions/stale](https://github.com/actions/stale) from 9 to 11. - [Release notes](https://github.com/actions/stale/releases) - [Changelog](https://github.com/actions/stale/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/stale/compare/v9...v11) --- updated-dependencies: - dependency-name: actions/stale dependency-version: '11' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/stale.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml index f2ca7b2bef..c2708cda2a 100644 --- a/.github/workflows/stale.yml +++ b/.github/workflows/stale.yml @@ -26,7 +26,7 @@ jobs: permission-issues: write permission-pull-requests: write - - uses: actions/stale@v9 + - uses: actions/stale@v11 with: repo-token: ${{ steps.app-token.outputs.token }} # --- Issues & PRs --- From 48d712dfb25f8139ea58641b48abedc5bbcdeede Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 9 Sep 2026 00:31:01 +0000 Subject: [PATCH 12/66] build(deps): bump softprops/action-gh-release from 2 to 3 Bumps [softprops/action-gh-release](https://github.com/softprops/action-gh-release) from 2 to 3. - [Release notes](https://github.com/softprops/action-gh-release/releases) - [Changelog](https://github.com/softprops/action-gh-release/blob/master/CHANGELOG.md) - [Commits](https://github.com/softprops/action-gh-release/compare/v2...v3) --- updated-dependencies: - dependency-name: softprops/action-gh-release dependency-version: '3' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .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 34ec259e55..e61551b145 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -213,7 +213,7 @@ jobs: sha256sum * > checksums.txt - name: Upload Release Assets - uses: softprops/action-gh-release@v2 + uses: softprops/action-gh-release@v3 with: tag_name: ${{ steps.version.outputs.version }} files: release/* From 98f8fffc8e67f5b413a356a8364d7fc78e95af4b Mon Sep 17 00:00:00 2001 From: AxelRay <110029405+axelray-dev@users.noreply.github.com> Date: Sun, 13 Sep 2026 13:20:38 +0800 Subject: [PATCH 13/66] fix(discover): require token boundaries for rewrite rules --- src/discover/registry.rs | 83 +++++++++++++++++++++++++++++++++ src/discover/rules.rs | 42 ++++++++--------- src/filters/pio-run.toml | 2 +- src/filters/quarto-render.toml | 2 +- src/filters/shopify-theme.toml | 2 +- src/filters/terraform-plan.toml | 2 +- src/filters/trunk-build.toml | 2 +- 7 files changed, 109 insertions(+), 26 deletions(-) diff --git a/src/discover/registry.rs b/src/discover/registry.rs index e16da48dca..53720465a8 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -3162,6 +3162,89 @@ mod tests { ); } + #[test] + fn test_subcommand_rules_require_token_boundaries() { + // The pnpm case is covered separately. The sbt rule is included here + // because it is already boundary-safe and guards the full issue family + // against future regressions. + let false_positives = [ + "git branchless status", + "gh prs", + "glab mrs", + "cargo builder", + "prettierish", + "next builder", + "playwrighting", + "prismax", + "docker psql", + "kubectl getall", + "oc status-check", + "ruff checker", + "sqlfluff linting", + "pip installer", + "uv pip installer", + "go vetting", + "sbt tester", + "rake tester", + "rails tester", + "pio runner", + "quarto renderer", + "shopify themepark", + "terraform planner", + "trunk builder", + ]; + + for command in false_positives { + assert!( + matches!( + classify_command(command), + Classification::Unsupported { .. } + ), + "{command} must not classify as a supported command" + ); + assert_eq!( + rewrite_command_no_prefixes(command, &[]), + None, + "{command} must not be rewritten" + ); + } + + let valid_commands = [ + ("git branch status", "rtk git"), + ("gh pr list", "rtk gh"), + ("glab mr list", "rtk glab"), + ("cargo build --release", "rtk cargo"), + ("prettier --check .", "rtk prettier"), + ("next build --turbo", "rtk next"), + ("playwright test", "rtk playwright"), + ("prisma migrate status", "rtk prisma"), + ("docker ps", "rtk docker"), + ("kubectl get pods", "rtk kubectl"), + ("oc status", "rtk oc"), + ("ruff check .", "rtk ruff"), + ("sqlfluff lint .", "rtk sqlfluff"), + ("pip install flask", "rtk pip"), + ("uv pip install flask", "rtk uv"), + ("go test ./...", "rtk go"), + ("sbt test", "rtk sbt"), + ("rake test", "rtk rake"), + ("pio run", "rtk pio"), + ("quarto render docs", "rtk quarto"), + ("shopify theme push", "rtk shopify"), + ("terraform plan", "rtk terraform"), + ("trunk build", "rtk trunk"), + ]; + + for (command, expected_rtk_command) in valid_commands { + match classify_command(command) { + Classification::Supported { rtk_equivalent, .. } => { + assert_eq!(rtk_equivalent, expected_rtk_command, "{command}"); + } + classification => panic!("{command} classified as {classification:?}"), + } + } + } + #[test] fn test_rewrite_playwright() { let commands = vec![ diff --git a/src/discover/rules.rs b/src/discover/rules.rs index a7894c05be..c47c5fa568 100644 --- a/src/discover/rules.rs +++ b/src/discover/rules.rs @@ -56,7 +56,7 @@ impl Default for RtkRule { pub const RULES: &[RtkRule] = &[ RtkRule { - pattern: r"^(?:git|yadm)\s+(?:-[Cc]\s+\S+\s+)*(status|log|diff|show|add|commit|checkout|push|pull|branch|fetch|stash|worktree)", + pattern: r"^(?:git|yadm)\s+(?:-[Cc]\s+\S+\s+)*(status|log|diff|show|add|commit|checkout|push|pull|branch|fetch|stash|worktree)(?:\s|$|[;|&()<>])", rtk_cmd: "rtk git", pipeline_safety: PipelineSafety::ProducerOnly, rewrite_prefixes: &["git", "yadm"], @@ -71,7 +71,7 @@ pub const RULES: &[RtkRule] = &[ ..RtkRule::DEFAULT }, RtkRule { - pattern: r"^gh\s+(pr|issue|run|repo|api|release)", + pattern: r"^gh\s+(pr|issue|run|repo|api|release)(?:\s|$|[;|&()<>])", rtk_cmd: "rtk gh", rewrite_prefixes: &["gh"], category: "GitHub", @@ -80,7 +80,7 @@ pub const RULES: &[RtkRule] = &[ ..RtkRule::DEFAULT }, RtkRule { - pattern: r"^glab\s+(mr|issue|ci|pipeline|api|release)", + pattern: r"^glab\s+(mr|issue|ci|pipeline|api|release)(?:\s|$|[;|&()<>])", rtk_cmd: "rtk glab", rewrite_prefixes: &["glab"], category: "GitLab", @@ -89,7 +89,7 @@ pub const RULES: &[RtkRule] = &[ ..RtkRule::DEFAULT }, RtkRule { - pattern: r"^cargo\s+(build|test|clippy|check|fmt|install)", + pattern: r"^cargo\s+(build|test|clippy|check|fmt|install)(?:\s|$|[;|&()<>])", rtk_cmd: "rtk cargo", pipeline_safety: PipelineSafety::ProducerOnly, rewrite_prefixes: &["cargo"], @@ -241,7 +241,7 @@ pub const RULES: &[RtkRule] = &[ ..RtkRule::DEFAULT }, RtkRule { - pattern: r"^((p?np(m|x)|p?npm\s+(exec|run|run-script)|npm\s+(rum|urn|x)|pnpm\s+dlx)\s+)?prettier", + pattern: r"^((p?np(m|x)|p?npm\s+(exec|run|run-script)|npm\s+(rum|urn|x)|pnpm\s+dlx)\s+)?prettier(?:\s|$|[;|&()<>])", rtk_cmd: "rtk prettier", pipeline_safety: PipelineSafety::ProducerOnly, rewrite_prefixes: &[ @@ -266,7 +266,7 @@ pub const RULES: &[RtkRule] = &[ ..RtkRule::DEFAULT }, RtkRule { - pattern: r"^((p?np(m|x)|p?npm\s+(exec|run|run-script)|npm\s+(rum|urn|x)|pnpm\s+dlx)\s+)?next\s+build", + pattern: r"^((p?np(m|x)|p?npm\s+(exec|run|run-script)|npm\s+(rum|urn|x)|pnpm\s+dlx)\s+)?next\s+build(?:\s|$|[;|&()<>])", rtk_cmd: "rtk next", pipeline_safety: PipelineSafety::ProducerOnly, rewrite_prefixes: &[ @@ -377,7 +377,7 @@ pub const RULES: &[RtkRule] = &[ ..RtkRule::DEFAULT }, RtkRule { - pattern: r"^((p?np(m|x)|p?npm\s+(exec|run|run-script)|npm\s+(rum|urn|x)|pnpm\s+dlx)\s+)?playwright", + pattern: r"^((p?np(m|x)|p?npm\s+(exec|run|run-script)|npm\s+(rum|urn|x)|pnpm\s+dlx)\s+)?playwright(?:\s|$|[;|&()<>])", rtk_cmd: "rtk playwright", rewrite_prefixes: &[ "npm exec playwright", @@ -401,7 +401,7 @@ pub const RULES: &[RtkRule] = &[ ..RtkRule::DEFAULT }, RtkRule { - pattern: r"^((p?np(m|x)|p?npm\s+(exec|run|run-script)|npm\s+(rum|urn|x)|pnpm\s+dlx)\s+)?prisma", + pattern: r"^((p?np(m|x)|p?npm\s+(exec|run|run-script)|npm\s+(rum|urn|x)|pnpm\s+dlx)\s+)?prisma(?:\s|$|[;|&()<>])", rtk_cmd: "rtk prisma", rewrite_prefixes: &[ "npm exec prisma", @@ -425,7 +425,7 @@ pub const RULES: &[RtkRule] = &[ ..RtkRule::DEFAULT }, RtkRule { - pattern: r"^docker\s+(ps|images|logs|run|exec|build|compose\s+(ps|logs|build))", + pattern: r"^docker\s+(ps|images|logs|run|exec|build|compose\s+(ps|logs|build))(?:\s|$|[;|&()<>])", rtk_cmd: "rtk docker", rewrite_prefixes: &["docker"], category: "Infra", @@ -433,7 +433,7 @@ pub const RULES: &[RtkRule] = &[ ..RtkRule::DEFAULT }, RtkRule { - pattern: r"^kubectl\s+(get|logs|describe|apply)", + pattern: r"^kubectl\s+(get|logs|describe|apply)(?:\s|$|[;|&()<>])", rtk_cmd: "rtk kubectl", rewrite_prefixes: &["kubectl"], category: "Infra", @@ -441,7 +441,7 @@ pub const RULES: &[RtkRule] = &[ ..RtkRule::DEFAULT }, RtkRule { - pattern: r"^oc\s+(get|logs|describe|apply|status|adm)", + pattern: r"^oc\s+(get|logs|describe|apply|status|adm)(?:\s|$|[;|&()<>])", rtk_cmd: "rtk oc", rewrite_prefixes: &["oc"], category: "Infra", @@ -491,7 +491,7 @@ pub const RULES: &[RtkRule] = &[ ..RtkRule::DEFAULT }, RtkRule { - pattern: r"^ruff\s+(check|format)", + pattern: r"^ruff\s+(check|format)(?:\s|$|[;|&()<>])", rtk_cmd: "rtk ruff", pipeline_safety: PipelineSafety::ProducerOnly, rewrite_prefixes: &["ruff"], @@ -501,7 +501,7 @@ pub const RULES: &[RtkRule] = &[ ..RtkRule::DEFAULT }, RtkRule { - pattern: r"^sqlfluff\s+lint", + pattern: r"^sqlfluff\s+lint(?:\s|$|[;|&()<>])", rtk_cmd: "rtk sqlfluff", rewrite_prefixes: &["sqlfluff"], category: "Python", @@ -518,7 +518,7 @@ pub const RULES: &[RtkRule] = &[ ..RtkRule::DEFAULT }, RtkRule { - pattern: r"^(pip3?|uv\s+pip)\s+(list|outdated|install|show)", + pattern: r"^(pip3?|uv\s+pip)\s+(list|outdated|install|show)(?:\s|$|[;|&()<>])", rtk_cmd: "rtk pip", pipeline_safety: PipelineSafety::ProducerOnly, rewrite_prefixes: &["pip3", "pip", "uv pip"], @@ -536,7 +536,7 @@ pub const RULES: &[RtkRule] = &[ ..RtkRule::DEFAULT }, RtkRule { - pattern: r"^go\s+(test|build|vet)", + pattern: r"^go\s+(test|build|vet)(?:\s|$|[;|&()<>])", rtk_cmd: "rtk go", pipeline_safety: PipelineSafety::ProducerOnly, rewrite_prefixes: &["go"], @@ -574,7 +574,7 @@ pub const RULES: &[RtkRule] = &[ ..RtkRule::DEFAULT }, RtkRule { - pattern: r"^(?:bundle\s+exec\s+)?(?:bin/)?(?:rake|rails)\s+test", + pattern: r"^(?:bundle\s+exec\s+)?(?:bin/)?(?:rake|rails)\s+test(?:\s|$|[;|&()<>])", rtk_cmd: "rtk rake", pipeline_safety: PipelineSafety::ProducerOnly, rewrite_prefixes: &[ @@ -930,7 +930,7 @@ pub const RULES: &[RtkRule] = &[ ..RtkRule::DEFAULT }, RtkRule { - pattern: r"^pio\s+run", + pattern: r"^pio\s+run(?:\s|$|[;|&()<>])", rtk_cmd: "rtk pio", pipeline_safety: PipelineSafety::ProducerOnly, rewrite_prefixes: &["pio"], @@ -980,7 +980,7 @@ pub const RULES: &[RtkRule] = &[ ..RtkRule::DEFAULT }, RtkRule { - pattern: r"^quarto\s+render", + pattern: r"^quarto\s+render(?:\s|$|[;|&()<>])", rtk_cmd: "rtk quarto", pipeline_safety: PipelineSafety::ProducerOnly, rewrite_prefixes: &["quarto"], @@ -1006,7 +1006,7 @@ pub const RULES: &[RtkRule] = &[ ..RtkRule::DEFAULT }, RtkRule { - pattern: r"^shopify\s+theme\s+(push|pull)", + pattern: r"^shopify\s+theme\s+(push|pull)(?:\s|$|[;|&()<>])", rtk_cmd: "rtk shopify", pipeline_safety: PipelineSafety::ProducerOnly, rewrite_prefixes: &["shopify"], @@ -1041,7 +1041,7 @@ pub const RULES: &[RtkRule] = &[ ..RtkRule::DEFAULT }, RtkRule { - pattern: r"^terraform\s+plan", + pattern: r"^terraform\s+plan(?:\s|$|[;|&()<>])", rtk_cmd: "rtk terraform", pipeline_safety: PipelineSafety::ProducerOnly, rewrite_prefixes: &["terraform"], @@ -1059,7 +1059,7 @@ pub const RULES: &[RtkRule] = &[ ..RtkRule::DEFAULT }, RtkRule { - pattern: r"^trunk\s+build", + pattern: r"^trunk\s+build(?:\s|$|[;|&()<>])", rtk_cmd: "rtk trunk", pipeline_safety: PipelineSafety::ProducerOnly, rewrite_prefixes: &["trunk"], diff --git a/src/filters/pio-run.toml b/src/filters/pio-run.toml index 863df636f5..54d2471ff0 100644 --- a/src/filters/pio-run.toml +++ b/src/filters/pio-run.toml @@ -1,6 +1,6 @@ [filters.pio-run] description = "Compact PlatformIO build output" -match_command = "^pio\\s+run" +match_command = "^pio\\s+run(\\s|$|[;|&()<>])" strip_ansi = true strip_lines_matching = [ "^\\s*$", diff --git a/src/filters/quarto-render.toml b/src/filters/quarto-render.toml index ad59e7a00a..e8520d4b7a 100644 --- a/src/filters/quarto-render.toml +++ b/src/filters/quarto-render.toml @@ -1,6 +1,6 @@ [filters.quarto-render] description = "Compact quarto render output" -match_command = "^quarto\\s+render" +match_command = "^quarto\\s+render(\\s|$|[;|&()<>])" strip_ansi = true strip_lines_matching = [ "^\\s*$", diff --git a/src/filters/shopify-theme.toml b/src/filters/shopify-theme.toml index a6e6cee5a1..1eb016cb69 100644 --- a/src/filters/shopify-theme.toml +++ b/src/filters/shopify-theme.toml @@ -1,6 +1,6 @@ [filters.shopify-theme] description = "Compact shopify theme push/pull output" -match_command = "^shopify\\s+theme\\s+(push|pull)" +match_command = "^shopify\\s+theme\\s+(push|pull)(\\s|$|[;|&()<>])" strip_ansi = true strip_lines_matching = [ "^\\s*$", diff --git a/src/filters/terraform-plan.toml b/src/filters/terraform-plan.toml index 726cd9dd7e..7ee22f35f5 100644 --- a/src/filters/terraform-plan.toml +++ b/src/filters/terraform-plan.toml @@ -1,6 +1,6 @@ [filters.terraform-plan] description = "Compact Terraform plan output" -match_command = "^terraform\\s+plan" +match_command = "^terraform\\s+plan(\\s|$|[;|&()<>])" strip_ansi = true strip_lines_matching = [ "^Refreshing state", diff --git a/src/filters/trunk-build.toml b/src/filters/trunk-build.toml index 21b4a62777..2baf5b4da2 100644 --- a/src/filters/trunk-build.toml +++ b/src/filters/trunk-build.toml @@ -1,6 +1,6 @@ [filters.trunk-build] description = "Compact trunk build output" -match_command = "^trunk\\s+build" +match_command = "^trunk\\s+build(\\s|$|[;|&()<>])" strip_ansi = true strip_lines_matching = [ "^\\s*$", From aefc45cff5e82ddb23e4f9286d40f46321e05ef9 Mon Sep 17 00:00:00 2001 From: Ciaran <31419980+obrienciaran@users.noreply.github.com> Date: Mon, 14 Sep 2026 14:07:24 +0200 Subject: [PATCH 14/66] fix(pip): stop doubling the tool name in pip messages `format!("Failed to run {} pip list", base_cmd)` was written for the `uv pip` branch, so on the plain-pip path it printed "pip pip list". Add `prog_label()` returning "uv pip" or "pip" and use it in all six verbose/error strings. The executed argv was already correct. Fixes #4050 --- src/cmds/python/pip_cmd.rs | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/src/cmds/python/pip_cmd.rs b/src/cmds/python/pip_cmd.rs index bbf5ded3e1..264f96d2fd 100644 --- a/src/cmds/python/pip_cmd.rs +++ b/src/cmds/python/pip_cmd.rs @@ -57,6 +57,12 @@ pub fn run(args: &[String], verbose: u8) -> Result { Ok(exit_code) } +/// Human-readable program name for messages: `uv pip` when routed through +/// uv, plain `pip` otherwise. Keeps error text from reading "pip pip list". +fn prog_label(base_cmd: &str) -> &'static str { + if base_cmd == "uv" { "uv pip" } else { "pip" } +} + fn run_list(base_cmd: &str, args: &[String], verbose: u8) -> Result<(String, String, i32)> { let mut cmd = resolved_command(base_cmd); @@ -71,11 +77,11 @@ fn run_list(base_cmd: &str, args: &[String], verbose: u8) -> Result<(String, Str } if verbose > 0 { - eprintln!("Running: {} pip list --format=json", base_cmd); + eprintln!("Running: {} list --format=json", prog_label(base_cmd)); } - let result = - exec_capture(&mut cmd).with_context(|| format!("Failed to run {} pip list", base_cmd))?; + let result = exec_capture(&mut cmd) + .with_context(|| format!("Failed to run {} list", prog_label(base_cmd)))?; let raw = format!("{}\n{}", result.stdout, result.stderr); @@ -99,11 +105,14 @@ fn run_outdated(base_cmd: &str, args: &[String], verbose: u8) -> Result<(String, } if verbose > 0 { - eprintln!("Running: {} pip list --outdated --format=json", base_cmd); + eprintln!( + "Running: {} list --outdated --format=json", + prog_label(base_cmd) + ); } let result = exec_capture(&mut cmd) - .with_context(|| format!("Failed to run {} pip list --outdated", base_cmd))?; + .with_context(|| format!("Failed to run {} list --outdated", prog_label(base_cmd)))?; let raw = format!("{}\n{}", result.stdout, result.stderr); @@ -125,11 +134,11 @@ fn run_passthrough(base_cmd: &str, args: &[String], verbose: u8) -> Result<(Stri } if verbose > 0 { - eprintln!("Running: {} pip {}", base_cmd, args.join(" ")); + eprintln!("Running: {} {}", prog_label(base_cmd), args.join(" ")); } let result = exec_capture(&mut cmd) - .with_context(|| format!("Failed to run {} pip {}", base_cmd, args.join(" ")))?; + .with_context(|| format!("Failed to run {} {}", prog_label(base_cmd), args.join(" ")))?; let raw = format!("{}\n{}", result.stdout, result.stderr); @@ -232,6 +241,12 @@ fn filter_pip_outdated(output: &str) -> String { mod tests { use super::*; + #[test] + fn test_prog_label_never_doubles_pip() { + assert_eq!(prog_label("pip"), "pip"); + assert_eq!(prog_label("uv"), "uv pip"); + } + #[test] fn test_filter_pip_list() { let output = r#"[ From dc643a87dec9ad4201c46f908f41bb217bccae9a Mon Sep 17 00:00:00 2001 From: Ciaran <31419980+obrienciaran@users.noreply.github.com> Date: Mon, 14 Sep 2026 14:13:08 +0200 Subject: [PATCH 15/66] comments --- src/cmds/python/pip_cmd.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cmds/python/pip_cmd.rs b/src/cmds/python/pip_cmd.rs index 264f96d2fd..76e1c92d87 100644 --- a/src/cmds/python/pip_cmd.rs +++ b/src/cmds/python/pip_cmd.rs @@ -57,8 +57,8 @@ pub fn run(args: &[String], verbose: u8) -> Result { Ok(exit_code) } -/// Human-readable program name for messages: `uv pip` when routed through -/// uv, plain `pip` otherwise. Keeps error text from reading "pip pip list". +/// Human-readable program name for messages. `uv pip` when routed through +/// uv, plain `pip` otherwise. fn prog_label(base_cmd: &str) -> &'static str { if base_cmd == "uv" { "uv pip" } else { "pip" } } From 7520531db2ac801b79fc3fd8a3c0b0825475fd81 Mon Sep 17 00:00:00 2001 From: Ciaran <31419980+obrienciaran@users.noreply.github.com> Date: Mon, 14 Sep 2026 20:39:28 +0200 Subject: [PATCH 16/66] docs(pip): say why prog_label exists, not what it does --- src/cmds/python/pip_cmd.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cmds/python/pip_cmd.rs b/src/cmds/python/pip_cmd.rs index 76e1c92d87..cd1b202b6c 100644 --- a/src/cmds/python/pip_cmd.rs +++ b/src/cmds/python/pip_cmd.rs @@ -57,8 +57,8 @@ pub fn run(args: &[String], verbose: u8) -> Result { Ok(exit_code) } -/// Human-readable program name for messages. `uv pip` when routed through -/// uv, plain `pip` otherwise. +/// `base_cmd` is already "pip" on the plain path, so a hardcoded "pip" after +/// it doubled up as "pip pip list" (#4050). fn prog_label(base_cmd: &str) -> &'static str { if base_cmd == "uv" { "uv pip" } else { "pip" } } From 28627f3b089b9beb160a0f575fd28d991925a1cd Mon Sep 17 00:00:00 2001 From: "Igor A. Zelaya" Date: Tue, 15 Sep 2026 03:25:41 -0600 Subject: [PATCH 17/66] fix(pytest): preserve elapsed duration in summaries --- src/cmds/python/pytest_cmd.rs | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/src/cmds/python/pytest_cmd.rs b/src/cmds/python/pytest_cmd.rs index 58d7dd5c0b..89055334a4 100644 --- a/src/cmds/python/pytest_cmd.rs +++ b/src/cmds/python/pytest_cmd.rs @@ -177,6 +177,7 @@ struct PytestCounts { skipped: usize, xfailed: usize, xpassed: usize, + duration: Option, } fn build_pytest_summary( @@ -192,6 +193,7 @@ fn build_pytest_summary( skipped, xfailed, xpassed, + duration, } = counts; if passed == 0 && failed == 0 && skipped == 0 && xfailed == 0 && xpassed == 0 { @@ -199,9 +201,13 @@ fn build_pytest_summary( } let extras_present = skipped > 0 || xfailed > 0 || xpassed > 0 || !xfail_lines.is_empty(); + let duration_suffix = duration + .as_deref() + .map(|value| format!(" in {value}")) + .unwrap_or_default(); if failed == 0 && passed > 0 && !extras_present { - return format!("Pytest: {} passed", passed); + return format!("Pytest: {passed} passed{duration_suffix}"); } let mut result = String::new(); @@ -215,6 +221,7 @@ fn build_pytest_summary( if xpassed > 0 { result.push_str(&format!(", {} xpassed", xpassed)); } + result.push_str(&duration_suffix); result.push('\n'); // Surface xfail/xpass entries (with their reasons) — XPASS in particular @@ -302,7 +309,10 @@ fn build_pytest_summary( } fn parse_summary_line(summary: &str) -> PytestCounts { - let mut counts = PytestCounts::default(); + let mut counts = PytestCounts { + duration: parse_duration(summary), + ..PytestCounts::default() + }; // Parse lines like "=== 4 passed, 1 failed, 2 xfailed, 1 xpassed in 0.50s ===" for part in summary.split(',') { @@ -332,6 +342,14 @@ fn parse_summary_line(summary: &str) -> PytestCounts { counts } +fn parse_duration(summary: &str) -> Option { + let unwrapped = summary.trim().trim_matches('=').trim(); + let (_, duration) = unwrapped.rsplit_once(" in ")?; + let seconds = duration.strip_suffix('s')?; + seconds.parse::().ok()?; + Some(duration.to_string()) +} + #[cfg(test)] mod tests { use super::*; @@ -349,6 +367,7 @@ tests/test_foo.py ..... [100%] let result = filter_pytest_output(output); assert!(result.contains("Pytest")); assert!(result.contains("5 passed")); + assert_eq!(result, "Pytest: 5 passed in 0.50s"); } #[test] @@ -373,6 +392,7 @@ FAILED tests/test_foo.py::test_something - assert False let result = filter_pytest_output(output); assert!(result.contains("4 passed, 1 failed")); + assert!(result.starts_with("Pytest: 4 passed, 1 failed in 0.50s")); assert!(result.contains("test_something")); assert!(result.contains("assert False")); } @@ -419,15 +439,26 @@ collected 0 items fn test_parse_summary_line() { let c = parse_summary_line("=== 5 passed in 0.50s ==="); assert_eq!((c.passed, c.failed, c.skipped), (5, 0, 0)); + assert_eq!(c.duration.as_deref(), Some("0.50s")); let c = parse_summary_line("=== 4 passed, 1 failed in 0.50s ==="); assert_eq!((c.passed, c.failed, c.skipped), (4, 1, 0)); + assert_eq!(c.duration.as_deref(), Some("0.50s")); let c = parse_summary_line("=== 3 passed, 1 failed, 2 skipped in 1.0s ==="); assert_eq!((c.passed, c.failed, c.skipped), (3, 1, 2)); + assert_eq!(c.duration.as_deref(), Some("1.0s")); let c = parse_summary_line("=== 2 passed, 1 failed, 2 xfailed, 1 xpassed in 1.0s ==="); assert_eq!((c.passed, c.failed, c.xfailed, c.xpassed), (2, 1, 2, 1)); + assert_eq!(c.duration.as_deref(), Some("1.0s")); + } + + #[test] + fn test_filter_pytest_quiet_mode_preserves_duration() { + let output = "2 passed in 3.04s"; + + assert_eq!(filter_pytest_output(output), "Pytest: 2 passed in 3.04s"); } #[test] From 377177c78e6c140de3efbaac9efd15674708730e Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Wed, 2 Sep 2026 09:34:14 +0200 Subject: [PATCH 18/66] test: stop tests depending on the contributor's locale and repo `test_git_status_not_a_repo_exits_nonzero` matched git's English "not a git repository" on stderr, so it failed in any other language -- `git status` is a passthrough and keeps the user's locale by design. It now asserts what RTK actually owns there: a non-zero exit, the message on stderr, nothing on stdout. The quieter half: `!out.contains("Is a directory")` in search_faithful_test passed vacuously wherever grep speaks another language, testing nothing. The engine-spawning helpers pin LC_ALL for both sides of a comparison, so those assertions mean the same thing everywhere. The branch tests ran `git branch` in whatever repo the contributor was sitting in and deleted it afterwards; the broken-pipe test read that repo's log, and outside a repo it stopped exercising the pipe at all. All three take a throwaway repo from the new test_support helpers, which also neutralize GIT_CONFIG_GLOBAL/SYSTEM so local aliases and init.defaultBranch stay out. Verified against fr_FR, de_DE, an empty git config and an injected RIPGREP_CONFIG_PATH. Co-Authored-By: Claude Opus 5 --- .claude/rules/cli-testing.md | 24 +++++++++++ src/cmds/git/git_cmd.rs | 79 ++++++++++++++++++++--------------- src/core/mod.rs | 2 + src/core/test_support.rs | 65 ++++++++++++++++++++++++++++ src/main.rs | 15 +++---- tests/grep_context_test.rs | 7 +++- tests/search_compress_test.rs | 8 +++- tests/search_error_test.rs | 1 + tests/search_faithful_test.rs | 8 +++- 9 files changed, 163 insertions(+), 46 deletions(-) create mode 100644 src/core/test_support.rs diff --git a/.claude/rules/cli-testing.md b/.claude/rules/cli-testing.md index b57266940e..8a0a5f1f82 100644 --- a/.claude/rules/cli-testing.md +++ b/.claude/rules/cli-testing.md @@ -190,6 +190,30 @@ behavior (search/grep compression, guard rails, faithful formatting) rather than filter module, and several of them draw on `tests/fixtures/` (real captured aws/glab/gradlew/ mvn/phpstan/dotnet output) alongside their own inline cases. +### Machine independence (🔴 Critical) + +A test that spawns a real process must not depend on the machine it runs on. Two rules, +both with `src/core/test_support.rs` helpers behind them: + +- **Never assert on a third-party tool's wording.** git, grep and coreutils translate their + messages, so `stderr.contains("not a git repository")` fails outright in a French shell — + and a *negative* assertion (`!out.contains("Is a directory")`) is worse, passing vacuously + while testing nothing. Assert RTK's own output, or the structure (exit code, which stream + the message went to). When a test genuinely needs a tool's English text, spawn it through + `test_support::rtk_command()` / `pin_environment()`, which pin `LC_ALL`/`LANGUAGE` and + neutralize the contributor's git config. +- **Never touch the ambient repository.** Use `test_support::temp_git_repo()` and + `.current_dir(repo.path())`. A test that runs `git branch` in whatever repo the + contributor is sitting in mutates their work, and one that assumes it is inside a repo at + all silently stops exercising anything when it isn't. + +Verify a change here against a hostile environment, not just yours: + +```bash +LC_ALL=de_DE.UTF-8 cargo test --all -- --ignored +GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null cargo test --all -- --ignored +``` + ### Real Command Execution ```rust diff --git a/src/cmds/git/git_cmd.rs b/src/cmds/git/git_cmd.rs index 17c5c4fe49..eaf78b7470 100644 --- a/src/cmds/git/git_cmd.rs +++ b/src/cmds/git/git_cmd.rs @@ -3607,6 +3607,7 @@ pub fn run_passthrough(args: &[OsString], global_args: &[String], verbose: u8) - #[cfg(test)] mod tests { use super::*; + use crate::core::test_support; #[test] fn test_branch_dash_u_links_its_upstream_value_not_a_free_positional() { @@ -6240,12 +6241,23 @@ no changes added to commit (use "git add" and/or "git commit -a") #[test] #[ignore] // Integration test: requires git repo fn test_branch_creation_not_swallowed() { + let repo = test_support::temp_git_repo(); let branch = "test-rtk-create-branch-regression"; - // Create branch via run_branch - run_branch(&[branch.to_string()], 0, &[]).expect("run_branch should succeed"); - // Verify it exists - let output = Command::new("git") + + let created = test_support::rtk_command() + .args(["git", "branch", branch]) + .current_dir(repo.path()) + .output() + .expect("rtk git branch should run"); + assert!( + created.status.success(), + "rtk git branch failed: {created:?}" + ); + + let mut list = std::process::Command::new("git"); + let output = test_support::pin_environment(&mut list) .args(["branch", "--list", branch]) + .current_dir(repo.path()) .output() .expect("git branch --list should work"); let stdout = String::from_utf8_lossy(&output.stdout); @@ -6254,19 +6266,29 @@ no changes added to commit (use "git add" and/or "git commit -a") "Branch '{}' was not created. run_branch silently swallowed the creation.", branch ); - // Cleanup - let _ = Command::new("git").args(["branch", "-d", branch]).output(); } /// Regression test: `git branch ` must create from commit. #[test] #[ignore] // Integration test: requires git repo fn test_branch_creation_from_commit() { + let repo = test_support::temp_git_repo(); let branch = "test-rtk-create-from-commit"; - run_branch(&[branch.to_string(), "HEAD".to_string()], 0, &[]) - .expect("run_branch with start-point should succeed"); - let output = Command::new("git") + + let created = test_support::rtk_command() + .args(["git", "branch", branch, "HEAD"]) + .current_dir(repo.path()) + .output() + .expect("rtk git branch with start-point should run"); + assert!( + created.status.success(), + "rtk git branch failed: {created:?}" + ); + + let mut list = std::process::Command::new("git"); + let output = test_support::pin_environment(&mut list) .args(["branch", "--list", branch]) + .current_dir(repo.path()) .output() .expect("git branch --list should work"); let stdout = String::from_utf8_lossy(&output.stdout); @@ -6275,7 +6297,6 @@ no changes added to commit (use "git add" and/or "git commit -a") "Branch '{}' was not created from commit.", branch ); - let _ = Command::new("git").args(["branch", "-d", branch]).output(); } #[test] @@ -6344,23 +6365,13 @@ no changes added to commit (use "git add" and/or "git commit -a") #[test] #[ignore] // Requires `cargo build` first — run with `cargo test --ignored` fn test_git_status_not_a_repo_exits_nonzero() { - // Run rtk git status in a directory that is not a git repo - let tmp = std::env::temp_dir().join("rtk_test_not_a_repo"); - let _ = std::fs::create_dir_all(&tmp); - - // Build the path to the test binary - 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 output = std::process::Command::new(&bin_path) + // Run rtk git status in a directory that is not a git repo. A fresh temp dir, not a + // fixed /tmp path: two contributors (or two parallel runs) would share the latter. + let tmp = tempfile::tempdir().expect("tempdir"); + + let output = test_support::rtk_command() .args(["git", "status"]) - .current_dir(&tmp) + .current_dir(tmp.path()) .output() .expect("Failed to run rtk"); @@ -6371,17 +6382,19 @@ no changes added to commit (use "git add" and/or "git commit -a") output.status.code() ); - // Message should be on stderr, not stdout + // Message should be on stderr, not stdout. Asserting on git's own wording would only + // hold in an English locale -- `git status` is a passthrough, so its message stays in + // the user's language by design (see git_cmd_c_locale). let stderr = String::from_utf8_lossy(&output.stderr); let stdout = String::from_utf8_lossy(&output.stdout); assert!( - stderr.to_lowercase().contains("not a git repository"), - "Expected 'not a git repository' on stderr, got stderr={:?}, stdout={:?}", - stderr, - stdout + !stderr.trim().is_empty(), + "Expected git's error on stderr, got stderr={stderr:?}, stdout={stdout:?}" + ); + assert!( + stdout.trim().is_empty(), + "Expected nothing on stdout, got stdout={stdout:?}" ); - - let _ = std::fs::remove_dir_all(&tmp); } // --- truncation accuracy --- diff --git a/src/core/mod.rs b/src/core/mod.rs index 0cd1b174ff..6fd65a88e4 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -14,6 +14,8 @@ pub mod tee; pub mod tee_file; pub mod telemetry; pub mod telemetry_cmd; +#[cfg(test)] +pub mod test_support; pub mod toml_filter; pub mod tracking; pub mod truncate; diff --git a/src/core/test_support.rs b/src/core/test_support.rs new file mode 100644 index 0000000000..0aad8cae74 --- /dev/null +++ b/src/core/test_support.rs @@ -0,0 +1,65 @@ +//! Test-only helpers that keep integration tests independent of the machine they run on. +//! +//! Tests that spawn the built binary must not read the contributor's own repository, +//! git config or locale: a test that mutates the working repo is hostile, and one that +//! matches a tool's English wording either fails or silently stops asserting anything in +//! another language. + +use std::path::PathBuf; +use std::process::Command; + +/// Path to the freshly built debug binary. Panics with the fix if it isn't there. +pub fn rtk_bin() -> PathBuf { + let bin = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("target") + .join("debug") + .join("rtk"); + assert!(bin.exists(), "Run `cargo build` first: {}", bin.display()); + bin +} + +/// The binary, with the environment pinned: `LC_ALL`/`LANGUAGE` so a child tool's messages +/// are the ones the assertions were written against, and `GIT_CONFIG_*` so the +/// contributor's own git config (aliases, `init.defaultBranch`, custom pagers) can't reach +/// into the run. +pub fn rtk_command() -> Command { + // Test-only, and the path is this crate's own freshly built binary, not user input. + // nosemgrep: dynamic-command-execution + let mut cmd = Command::new(rtk_bin()); + pin_environment(&mut cmd); + cmd +} + +/// Same pinning for a directly spawned tool, so both sides of a comparison speak one +/// language. +pub fn pin_environment(cmd: &mut Command) -> &mut Command { + cmd.env("LC_ALL", "C") + .env("LANGUAGE", "") + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_SYSTEM", "/dev/null") +} + +/// A throwaway git repo with one commit, its identity set locally so it works with no user +/// git config at all. The `TempDir` deletes it on drop, so keep it alive for the test. +pub fn temp_git_repo() -> tempfile::TempDir { + let dir = tempfile::tempdir().expect("tempdir"); + for args in [ + &["init", "-q", "-b", "main"][..], + &["config", "user.email", "rtk-test@example.invalid"][..], + &["config", "user.name", "rtk test"][..], + &["config", "commit.gpgsign", "false"][..], + &["commit", "-q", "--allow-empty", "-m", "init"][..], + ] { + let ok = Command::new("git") + .args(args) + .current_dir(dir.path()) + .env("LC_ALL", "C") + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_SYSTEM", "/dev/null") + .output() + .map(|out| out.status.success()) + .unwrap_or(false); + assert!(ok, "git setup failed: {args:?}"); + } + dir +} diff --git a/src/main.rs b/src/main.rs index e7d47a6fd3..a0d6b2863e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4091,18 +4091,13 @@ mod tests { #[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 - ); + // A throwaway repo, not whichever one the contributor happens to be sitting in: + // outside a repo `git log` only errors, and the test stops exercising the pipe. + let repo = core::test_support::temp_git_repo(); - let mut child = std::process::Command::new(&bin_path) + let mut child = core::test_support::rtk_command() .args(["git", "log", "--oneline", "-50"]) + .current_dir(repo.path()) .stdout(std::process::Stdio::piped()) .stderr(std::process::Stdio::piped()) .spawn() diff --git a/tests/grep_context_test.rs b/tests/grep_context_test.rs index 80f5525c48..4b2e13ba99 100644 --- a/tests/grep_context_test.rs +++ b/tests/grep_context_test.rs @@ -4,12 +4,17 @@ use std::process::Command; +/// grep/rg messages are localized; pin the locale so assertions on engine +/// text hold in every contributor's shell. fn rtk() -> Command { - Command::new(env!("CARGO_BIN_EXE_rtk")) + let mut cmd = Command::new(env!("CARGO_BIN_EXE_rtk")); + cmd.env("LC_ALL", "C"); + cmd } fn rg_available() -> bool { Command::new("rg") + .env("LC_ALL", "C") .arg("--version") .output() .map(|o| o.status.success()) diff --git a/tests/search_compress_test.rs b/tests/search_compress_test.rs index 702bfd925b..1a8cf0771d 100644 --- a/tests/search_compress_test.rs +++ b/tests/search_compress_test.rs @@ -5,12 +5,17 @@ use std::process::Command; +/// grep/rg messages are localized; pin the locale so assertions on engine +/// text hold in every contributor's shell. fn rtk() -> Command { - Command::new(env!("CARGO_BIN_EXE_rtk")) + let mut cmd = Command::new(env!("CARGO_BIN_EXE_rtk")); + cmd.env("LC_ALL", "C"); + cmd } fn rg_available() -> bool { Command::new("rg") + .env("LC_ALL", "C") .arg("--version") .output() .map(|o| o.status.success()) @@ -253,6 +258,7 @@ fn bulky_grep_yields_token_savings() { let (_dir, path) = write_temp(&content); let raw = Command::new("rg") + .env("LC_ALL", "C") .args(["-nH", "MATCH", path.to_str().unwrap()]) .output() .expect("rg"); diff --git a/tests/search_error_test.rs b/tests/search_error_test.rs index 0d886dcc75..240ef50536 100644 --- a/tests/search_error_test.rs +++ b/tests/search_error_test.rs @@ -15,6 +15,7 @@ fn rtk(args: &[&str]) -> std::process::Output { fn grep_exit(args: &[&str]) -> Option { Command::new("grep") + .env("LC_ALL", "C") .args(args) .output() .expect("grep") diff --git a/tests/search_faithful_test.rs b/tests/search_faithful_test.rs index 08417a42bc..791dd28dd3 100644 --- a/tests/search_faithful_test.rs +++ b/tests/search_faithful_test.rs @@ -8,8 +8,12 @@ use std::process::{Command, Stdio}; use std::sync::mpsc; use std::time::Duration; +/// grep/rg error text (e.g. "Is a directory") is localized; pin it so +/// assertions on that text mean the same thing in every contributor's shell. fn rtk() -> Command { - Command::new(env!("CARGO_BIN_EXE_rtk")) + let mut cmd = Command::new(env!("CARGO_BIN_EXE_rtk")); + cmd.env("LC_ALL", "C"); + cmd } /// Run rtk with `input` fed on stdin; returns (stdout, exit_code). @@ -74,6 +78,7 @@ fn assert_streams_before_stdin_closes(args: &[&str], expected: &str) { fn rg_available() -> bool { Command::new("rg") + .env("LC_ALL", "C") .arg("--version") .output() .map(|o| o.status.success()) @@ -186,6 +191,7 @@ fn bulky_rg_yields_token_savings() { let (_dir, path) = write_temp(&content); let raw = Command::new("rg") + .env("LC_ALL", "C") .args(["-nH", "MATCH", path.to_str().unwrap()]) .output() .expect("rg"); From e5518dcc1335da3f5faa75a4d716a0fcafe8cfb9 Mon Sep 17 00:00:00 2001 From: AxelRay <110029405+axelray-dev@users.noreply.github.com> Date: Sun, 20 Sep 2026 03:05:46 +0800 Subject: [PATCH 19/66] fix(discover): preserve namespaced rake test rewrites --- src/discover/registry.rs | 4 ++++ src/discover/rules.rs | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/discover/registry.rs b/src/discover/registry.rs index 53720465a8..f441fc4f90 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -3228,6 +3228,10 @@ mod tests { ("go test ./...", "rtk go"), ("sbt test", "rtk sbt"), ("rake test", "rtk rake"), + ("rake test:unit", "rtk rake"), + ("rails test:system", "rtk rake"), + ("bundle exec rake test:models", "rtk rake"), + ("bin/rails test:integration", "rtk rake"), ("pio run", "rtk pio"), ("quarto render docs", "rtk quarto"), ("shopify theme push", "rtk shopify"), diff --git a/src/discover/rules.rs b/src/discover/rules.rs index c47c5fa568..1dbc9facc9 100644 --- a/src/discover/rules.rs +++ b/src/discover/rules.rs @@ -574,7 +574,7 @@ pub const RULES: &[RtkRule] = &[ ..RtkRule::DEFAULT }, RtkRule { - pattern: r"^(?:bundle\s+exec\s+)?(?:bin/)?(?:rake|rails)\s+test(?:\s|$|[;|&()<>])", + pattern: r"^(?:bundle\s+exec\s+)?(?:bin/)?(?:rake|rails)\s+test(?:[\s:]|$|[;|&()<>])", rtk_cmd: "rtk rake", pipeline_safety: PipelineSafety::ProducerOnly, rewrite_prefixes: &[ From f6ef00676942533d81282c7112ae335b566f0c96 Mon Sep 17 00:00:00 2001 From: Yijie Xu Date: Mon, 21 Sep 2026 03:01:10 +0800 Subject: [PATCH 20/66] refactor(hooks): consolidate hook config I/O --- src/hooks/hook_cmd.rs | 76 +++- src/hooks/init.rs | 822 ++++++++++++++++++++--------------- tests/hook_config_io_test.rs | 104 +++++ 3 files changed, 641 insertions(+), 361 deletions(-) create mode 100644 tests/hook_config_io_test.rs diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index 5fae3dc8a6..03b00a02eb 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -79,7 +79,7 @@ pub fn run_copilot() -> Result<()> { }; match detect_format(&v) { - HookFormat::VsCode { command } => handle_vscode(&command), + HookFormat::VsCode { command } => handle_vscode(&command, &v), HookFormat::CopilotCli { command, args } => { for path in heal_legacy_copilot_configs() { audit_log("self_heal", &path.display().to_string(), ""); @@ -242,15 +242,19 @@ fn decide_hook_action(cmd: &str, host: permissions::Host) -> HookDecision { decide_from_verdict(cmd, permissions::check_command_for(cmd, host)) } -fn handle_vscode(cmd: &str) -> Result<()> { - if let Some(output) = vscode_response(cmd) { +fn handle_vscode(cmd: &str, input: &Value) -> Result<()> { + if let Some(output) = vscode_response(cmd, input) { let _ = writeln!(io::stdout(), "{output}"); } Ok(()) } -fn vscode_response(cmd: &str) -> Option { - vscode_response_from_decision(decide_hook_action(cmd, permissions::Host::Claude), cmd) +fn vscode_response(cmd: &str, input: &Value) -> Option { + vscode_response_from_decision( + decide_hook_action(cmd, permissions::Host::Claude), + cmd, + input, + ) } /// Build the VS Code Copilot Chat / Copilot CLI (PascalCase compat) hook response. @@ -261,7 +265,11 @@ fn vscode_response(cmd: &str) -> Option { /// the host's own native prompt/allowlist flow in control — see #3037, where /// asserting `"ask"` here made Copilot CLI 1.0.66+ force a blocking dialog with /// no "remember" option on every rewritten command. -fn vscode_response_from_decision(decision: HookDecision, cmd: &str) -> Option { +fn vscode_response_from_decision( + decision: HookDecision, + cmd: &str, + input: &Value, +) -> Option { let (rewritten, allow) = match decision { HookDecision::Deny => { audit_log("deny", cmd, ""); @@ -274,15 +282,11 @@ fn vscode_response_from_decision(decision: HookDecision, cmd: &str) -> Option Result<()> { @@ -1292,11 +1296,34 @@ mod tests { // PascalCase-compat entry (#3037): the same `rtk hook copilot` call // answers both from one JSON schema. + #[test] + fn test_vscode_rewrite_preserves_tool_input_fields() { + let input = json!({ + "tool_name": "run_in_terminal", + "tool_input": { + "command": "git status", + "timeout": 1234, + "description": "Inspect working tree", + "metadata": {"nested": [true, null]} + } + }); + for decision in [ + HookDecision::AllowRewrite("rtk git status".into()), + HookDecision::AskRewrite("rtk git status".into()), + ] { + let response = vscode_response_from_decision(decision, "git status", &input).unwrap(); + let mut expected = input["tool_input"].clone(); + expected["command"] = json!("rtk git status"); + assert_eq!(response["hookSpecificOutput"]["updatedInput"], expected); + } + } + #[test] fn test_vscode_allow_rewrite_sets_permission_allow() { let r = vscode_response_from_decision( HookDecision::AllowRewrite("rtk git status".into()), "git status", + &vscode_input("Bash", "git status"), ) .unwrap(); assert_eq!(r["hookSpecificOutput"]["permissionDecision"], "allow"); @@ -1316,6 +1343,7 @@ mod tests { let r = vscode_response_from_decision( HookDecision::AskRewrite("rtk cargo test".into()), "cargo test", + &vscode_input("Bash", "cargo test"), ) .unwrap(); assert!( @@ -1334,12 +1362,26 @@ mod tests { #[test] fn test_vscode_deny_returns_none() { - assert!(vscode_response_from_decision(HookDecision::Deny, "cargo test").is_none()); + assert!( + vscode_response_from_decision( + HookDecision::Deny, + "cargo test", + &vscode_input("Bash", "cargo test") + ) + .is_none() + ); } #[test] fn test_vscode_defer_returns_none() { - assert!(vscode_response_from_decision(HookDecision::Defer, "cargo test").is_none()); + assert!( + vscode_response_from_decision( + HookDecision::Defer, + "cargo test", + &vscode_input("Bash", "cargo test") + ) + .is_none() + ); } // --- Copilot CLI handler: transparent rewrite via modifiedArgs --- diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 80c0a49462..2cae0d337d 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -785,35 +785,11 @@ fn print_manual_instructions(hook_command: &str, include_opencode: bool) { } fn remove_hook_from_json(root: &mut serde_json::Value) -> bool { - let hooks = match root - .get_mut("hooks") - .and_then(|h| h.get_mut(PRE_TOOL_USE_KEY)) - { - Some(pre_tool_use) => pre_tool_use, - None => return false, - }; - - let pre_tool_use_array = match hooks.as_array_mut() { - Some(arr) => arr, - None => return false, - }; - - let original_len = pre_tool_use_array.len(); - pre_tool_use_array.retain(|entry| { - if let Some(hooks_array) = entry.get("hooks").and_then(|h| h.as_array()) { - for hook in hooks_array { - if let Some(command) = hook.get("command").and_then(|c| c.as_str()) { - // Match both legacy script path and new binary command - if command.contains(REWRITE_HOOK_FILE) || is_claude_hook_command(command) { - return false; - } - } - } - } - true - }); - - pre_tool_use_array.len() < original_len + remove_hook_entries(root, PRE_TOOL_USE_KEY, HookEntries::Grouped, |hook| { + is_command_hook(hook, |cmd| { + is_claude_hook_command(cmd) || cmd.contains(REWRITE_HOOK_FILE) + }) + }) } /// Remove RTK hook from settings.json file @@ -1251,21 +1227,7 @@ fn patch_settings_json_command( let claude_dir = resolve_claude_dir()?; let settings_path = claude_dir.join(SETTINGS_JSON); - // Read or create settings.json - let mut root = if settings_path.exists() { - let content = fs::read_to_string(&settings_path) - .with_context(|| format!("Failed to read {}", settings_path.display()))?; - let content = strip_leading_bom(&content); - - if content.trim().is_empty() { - serde_json::json!({}) - } else { - from_json_str(content) - .with_context(|| format!("Failed to parse {} as JSON", settings_path.display()))? - } - } else { - serde_json::json!({}) - }; + let mut root = read_json_file(&settings_path)?.unwrap_or_else(|| serde_json::json!({})); // Check idempotency if hook_already_present(&root, hook_command) { @@ -1314,19 +1276,12 @@ fn patch_settings_json_command( return Ok(PatchResult::WouldPatch); } - // Backup original - if settings_path.exists() { - let backup_path = settings_path.with_extension("json.bak"); - fs::copy(&settings_path, &backup_path) - .with_context(|| format!("Failed to backup to {}", backup_path.display()))?; - if verbose > 0 { - eprintln!("Backup: {}", backup_path.display()); - } + if let Some(backup_path) = backup_and_atomic_write(&settings_path, &serialized)? + && verbose > 0 + { + eprintln!("Backup: {}", backup_path.display()); } - // Atomic write - atomic_write(&settings_path, &serialized)?; - println!("\n settings.json: hook added"); if settings_path.with_extension("json.bak").exists() { println!( @@ -1373,59 +1328,164 @@ fn clean_double_blanks(content: &str) -> String { result.join("\n") } -/// Deep-merge RTK hook entry into settings.json -/// Creates hooks.PreToolUse structure if missing, preserves existing hooks -fn insert_hook_entry(root: &mut serde_json::Value, hook_command: &str) -> Result<()> { - let root_obj = match root.as_object_mut() { - Some(obj) => obj, - None => { - *root = serde_json::json!({}); - root.as_object_mut().expect("just-created json object") - } - }; +/// Only command hooks belong to RTK; prompt/agent entries are user-owned. +fn is_command_hook(hook: &serde_json::Value, matches: impl Fn(&str) -> bool) -> bool { + hook.get("type").is_none_or(|kind| kind == "command") + && hook + .get("command") + .and_then(serde_json::Value::as_str) + .is_some_and(matches) +} + +/// Codex/Cursor match tool names with regular expressions. +fn group_covers_tool(group: &serde_json::Value, tool: &str) -> bool { + match group.get("matcher") { + None => true, + Some(matcher) => matcher.as_str().is_some_and(|pattern| { + pattern.is_empty() + || pattern == "*" + || regex::Regex::new(pattern).is_ok_and(|regex| regex.is_match(tool)) + }), + } +} + +/// Claude treats simple matchers as exact names/lists, otherwise as regexes. +fn claude_group_covers_bash(group: &serde_json::Value) -> bool { + if let Some(pattern) = group.get("matcher").and_then(serde_json::Value::as_str) + && !pattern.is_empty() + && pattern + .chars() + .all(|c| c.is_ascii_alphanumeric() || "_- ,|".contains(c)) + { + return pattern.split(['|', ',']).any(|name| name.trim() == "Bash"); + } + group_covers_tool(group, "Bash") +} + +#[derive(Clone, Copy)] +enum HookEntries { + Grouped, + Flat, +} - let hooks = root_obj +/// Share traversal, while callers retain their host's matcher and ownership rules. +fn hook_present( + root: &serde_json::Value, + event: &str, + layout: HookEntries, + covers: impl Fn(&serde_json::Value) -> bool, + owns: impl Fn(&serde_json::Value) -> bool, +) -> bool { + root.get("hooks") + .and_then(|hooks| hooks.get(event)) + .and_then(serde_json::Value::as_array) + .is_some_and(|entries| { + entries.iter().any(|entry| { + covers(entry) + && match layout { + HookEntries::Grouped => entry + .get("hooks") + .and_then(serde_json::Value::as_array) + .is_some_and(|hooks| hooks.iter().any(&owns)), + HookEntries::Flat => owns(entry), + } + }) + }) +} + +fn append_hook_entry( + root: &mut serde_json::Value, + event: &str, + entry: serde_json::Value, +) -> Result<()> { + let hooks = root + .as_object_mut() + .context("hook config root is not an object")? .entry("hooks") .or_insert_with(|| serde_json::json!({})) .as_object_mut() .context("hooks value is not an object")?; - - let pre_tool_use = hooks - .entry(PRE_TOOL_USE_KEY) + hooks + .entry(event) .or_insert_with(|| serde_json::json!([])) .as_array_mut() - .context("PreToolUse value is not an array")?; - - pre_tool_use.push(serde_json::json!({ - "matcher": "Bash", - "hooks": [{ - "type": "command", - "command": hook_command - }] - })); + .with_context(|| format!("{event} value is not an array"))? + .push(entry); Ok(()) } +/// Remove only owned entries, pruning a group only when this removal empties it. +fn remove_hook_entries( + root: &mut serde_json::Value, + event: &str, + layout: HookEntries, + owns: impl Fn(&serde_json::Value) -> bool, +) -> bool { + let Some(entries) = root + .get_mut("hooks") + .and_then(|hooks| hooks.get_mut(event)) + .and_then(serde_json::Value::as_array_mut) + else { + return false; + }; + let mut removed = false; + entries.retain_mut(|entry| match layout { + HookEntries::Flat => { + let matched = owns(entry); + removed |= matched; + !matched + } + HookEntries::Grouped => { + let Some(hooks) = entry + .get_mut("hooks") + .and_then(serde_json::Value::as_array_mut) + else { + return true; + }; + let before = hooks.len(); + hooks.retain(|hook| !owns(hook)); + if hooks.len() == before { + return true; + } + removed = true; + !hooks.is_empty() + } + }); + removed +} + +/// Deep-merge RTK hook entry into settings.json +/// Creates hooks.PreToolUse structure if missing, preserves existing hooks +fn insert_hook_entry(root: &mut serde_json::Value, hook_command: &str) -> Result<()> { + if !root.is_object() { + *root = serde_json::json!({}); + } + append_hook_entry( + root, + PRE_TOOL_USE_KEY, + serde_json::json!({ + "matcher": "Bash", + "hooks": [{"type": "command", "command": hook_command}] + }), + ) +} + /// Check if RTK hook is already present in settings.json /// Matches on legacy rtk-rewrite.sh path OR new `rtk hook claude` command fn hook_already_present(root: &serde_json::Value, hook_command: &str) -> bool { - let pre_tool_use_array = match root - .get("hooks") - .and_then(|h| h.get(PRE_TOOL_USE_KEY)) - .and_then(|p| p.as_array()) - { - Some(arr) => arr, - None => return false, - }; - - pre_tool_use_array - .iter() - .filter_map(|entry| entry.get("hooks")?.as_array()) - .flatten() - .filter_map(|hook| hook.get("command")?.as_str()) - .any(|cmd| { - cmd == hook_command || is_claude_hook_command(cmd) || cmd.contains(REWRITE_HOOK_FILE) - }) + hook_present( + root, + PRE_TOOL_USE_KEY, + HookEntries::Grouped, + claude_group_covers_bash, + |hook| { + is_command_hook(hook, |cmd| { + cmd == hook_command + || is_claude_hook_command(cmd) + || cmd.contains(REWRITE_HOOK_FILE) + }) + }, + ) } /// Default mode: hook + slim RTK.md + @RTK.md reference @@ -1654,32 +1714,9 @@ fn remove_legacy_settings_entries(ctx: InitContext) -> Result<()> { /// Returns true if any entries were removed. /// Does NOT remove `rtk hook claude` entries — those are the new format. fn remove_legacy_hook_entries_from_json(root: &mut serde_json::Value) -> bool { - let pre_tool_use_array = match root - .get_mut("hooks") - .and_then(|h| h.get_mut(PRE_TOOL_USE_KEY)) - .and_then(|p| p.as_array_mut()) - { - Some(arr) => arr, - None => return false, - }; - - let original_len = pre_tool_use_array.len(); - pre_tool_use_array.retain(|entry| { - let dominated_by_legacy = entry - .get("hooks") - .and_then(|h| h.as_array()) - .map(|hooks| { - hooks.iter().all(|hook| { - hook.get("command") - .and_then(|c| c.as_str()) - .is_some_and(|cmd| cmd.contains(REWRITE_HOOK_FILE)) - }) - }) - .unwrap_or(false); - !dominated_by_legacy - }); - - pre_tool_use_array.len() < original_len + remove_hook_entries(root, PRE_TOOL_USE_KEY, HookEntries::Grouped, |hook| { + is_command_hook(hook, |cmd| cmd.contains(REWRITE_HOOK_FILE)) + }) } /// Generate .rtk/filters.toml template in the current directory if not present. @@ -2821,14 +2858,13 @@ fn codex_tracking_config(db_path: &Path) -> Result { } fn codex_hook_already_present(root: &serde_json::Value) -> bool { - root.pointer("/hooks/PreToolUse") - .and_then(serde_json::Value::as_array) - .into_iter() - .flatten() - .filter_map(|entry| entry.get("hooks")?.as_array()) - .flatten() - .filter_map(|hook| hook.get("command")?.as_str()) - .any(is_codex_hook_command) + hook_present( + root, + PRE_TOOL_USE_KEY, + HookEntries::Grouped, + |group| group_covers_tool(group, "Bash"), + |hook| is_command_hook(hook, is_codex_hook_command), + ) } fn patch_codex_hooks_json(path: &Path, ctx: InitContext) -> Result { @@ -2870,38 +2906,9 @@ fn patch_codex_hooks_json(path: &Path, ctx: InitContext) -> Result { } fn remove_codex_hook_from_json(root: &mut serde_json::Value) -> bool { - let Some(pre_tool_use) = root - .pointer_mut("/hooks/PreToolUse") - .and_then(serde_json::Value::as_array_mut) - else { - return false; - }; - - let mut removed = false; - for entry in pre_tool_use.iter_mut() { - let Some(hooks) = entry - .get_mut("hooks") - .and_then(serde_json::Value::as_array_mut) - else { - continue; - }; - let before = hooks.len(); - hooks.retain(|hook| { - !hook - .get("command") - .and_then(serde_json::Value::as_str) - .is_some_and(is_codex_hook_command) - }); - removed |= hooks.len() != before; - } - pre_tool_use.retain(|entry| { - entry - .get("hooks") - .and_then(serde_json::Value::as_array) - .is_none_or(|hooks| !hooks.is_empty()) - }); - - removed + remove_hook_entries(root, PRE_TOOL_USE_KEY, HookEntries::Grouped, |hook| { + is_command_hook(hook, is_codex_hook_command) + }) } fn remove_codex_hook_from_file(path: &Path, ctx: InitContext) -> Result { @@ -4862,52 +4869,37 @@ fn patch_cursor_hooks_json(path: &Path, ctx: InitContext) -> Result { /// Check if RTK preToolUse hook is already present in Cursor hooks.json /// Matches on legacy rtk-rewrite.sh path OR new `rtk hook cursor` command fn cursor_hook_already_present(root: &serde_json::Value) -> bool { - let hooks = match root - .get("hooks") - .and_then(|h| h.get("preToolUse")) - .and_then(|p| p.as_array()) - { - Some(arr) => arr, - None => return false, - }; + hook_present( + root, + "preToolUse", + HookEntries::Flat, + |entry| group_covers_tool(entry, "Shell"), + is_cursor_hook_entry, + ) +} - hooks.iter().any(|entry| { - entry - .get("command") - .and_then(|c| c.as_str()) - .is_some_and(|cmd| cmd.contains(REWRITE_HOOK_FILE) || cmd == CURSOR_HOOK_COMMAND) +fn is_cursor_hook_entry(hook: &serde_json::Value) -> bool { + is_command_hook(hook, |cmd| { + cmd.contains(REWRITE_HOOK_FILE) || cmd == CURSOR_HOOK_COMMAND }) } /// Insert RTK preToolUse entry into Cursor hooks.json fn insert_cursor_hook_entry(root: &mut serde_json::Value) -> Result<()> { - let root_obj = match root.as_object_mut() { - Some(obj) => obj, - None => { - *root = serde_json::json!({ "version": 1 }); - root.as_object_mut().expect("just-created json object") - } - }; - - root_obj.entry("version").or_insert(serde_json::json!(1)); - - let hooks = root_obj - .entry("hooks") - .or_insert_with(|| serde_json::json!({})) - .as_object_mut() - .context("hooks value is not an object")?; - - let pre_tool_use = hooks - .entry("preToolUse") - .or_insert_with(|| serde_json::json!([])) - .as_array_mut() - .context("preToolUse value is not an array")?; - - pre_tool_use.push(serde_json::json!({ - "command": CURSOR_HOOK_COMMAND, - "matcher": "Shell" - })); - Ok(()) + if !root.is_object() { + *root = serde_json::json!({}); + } + root.as_object_mut() + .expect("object") + .entry("version") + .or_insert(serde_json::json!(1)); + append_hook_entry( + root, + "preToolUse", + serde_json::json!({ + "command": CURSOR_HOOK_COMMAND, "matcher": "Shell" + }), + ) } /// Remove only legacy `rtk-rewrite.sh` entries from Cursor hooks.json. @@ -4946,24 +4938,9 @@ fn remove_legacy_cursor_hooks_json_entries(path: &Path, ctx: InitContext) -> Res /// Returns true if any entries were removed. /// Does NOT remove `rtk hook cursor` entries — those are the new format. fn remove_legacy_cursor_hook_entries_from_json(root: &mut serde_json::Value) -> bool { - let pre_tool_use = match root - .get_mut("hooks") - .and_then(|h| h.get_mut("preToolUse")) - .and_then(|p| p.as_array_mut()) - { - Some(arr) => arr, - None => return false, - }; - - let original_len = pre_tool_use.len(); - pre_tool_use.retain(|entry| { - !entry - .get("command") - .and_then(|c| c.as_str()) - .is_some_and(|cmd| cmd.contains(REWRITE_HOOK_FILE)) - }); - - pre_tool_use.len() < original_len + remove_hook_entries(root, "preToolUse", HookEntries::Flat, |hook| { + is_command_hook(hook, |cmd| cmd.contains(REWRITE_HOOK_FILE)) + }) } /// Remove Cursor RTK artifacts: hook script + hooks.json entry @@ -5034,24 +5011,7 @@ fn remove_cursor_hooks_at(cursor_dir: &Path, ctx: InitContext) -> Result bool { - let pre_tool_use = match root - .get_mut("hooks") - .and_then(|h| h.get_mut("preToolUse")) - .and_then(|p| p.as_array_mut()) - { - Some(arr) => arr, - None => return false, - }; - - let original_len = pre_tool_use.len(); - pre_tool_use.retain(|entry| { - !entry - .get("command") - .and_then(|c| c.as_str()) - .is_some_and(|cmd| cmd.contains(REWRITE_HOOK_FILE) || cmd == CURSOR_HOOK_COMMAND) - }); - - pre_tool_use.len() < original_len + remove_hook_entries(root, "preToolUse", HookEntries::Flat, is_cursor_hook_entry) } // ─── Trae support ──────────────────────────────────────────────────── @@ -5236,91 +5196,44 @@ fn trae_group_covers_run_command(group: &serde_json::Value) -> bool { /// type we write. A missing `type` is ours too, since a hand-written /// registration commonly omits it; any other explicit type is the user's. fn is_trae_hook_entry(hook: &serde_json::Value) -> bool { - let is_our_command = hook - .get("command") - .and_then(|command| command.as_str()) - .is_some_and(is_trae_hook_command); - let is_command_type = match hook.get("type") { - None => true, - Some(hook_type) => hook_type.as_str() == Some("command"), - }; - is_our_command && is_command_type + is_command_hook(hook, is_trae_hook_command) } fn trae_hook_already_present(root: &serde_json::Value) -> bool { - root.get("hooks") - .and_then(|hooks| hooks.get(PRE_TOOL_USE_KEY)) - .and_then(|groups| groups.as_array()) - .is_some_and(|groups| { - groups.iter().any(|group| { - trae_group_covers_run_command(group) - && group - .get("hooks") - .and_then(|hooks| hooks.as_array()) - .is_some_and(|hooks| hooks.iter().any(is_trae_hook_entry)) - }) - }) + hook_present( + root, + PRE_TOOL_USE_KEY, + HookEntries::Grouped, + trae_group_covers_run_command, + is_trae_hook_entry, + ) } fn insert_trae_hook_entry(root: &mut serde_json::Value) -> Result<()> { validate_trae_hooks_json(root)?; - - let root_object = root.as_object_mut().expect("validated object"); - - root_object.entry("version").or_insert(serde_json::json!(1)); - let hooks = root_object - .entry("hooks") - .or_insert_with(|| serde_json::json!({})) - .as_object_mut() - .context("Trae hooks value is not an object")?; - let pre_tool_use = hooks - .entry(PRE_TOOL_USE_KEY) - .or_insert_with(|| serde_json::json!([])) - .as_array_mut() - .context("Trae PreToolUse value is not an array")?; - - pre_tool_use.push(serde_json::json!({ - "matcher": TRAE_RUN_COMMAND_MATCHER, - "hooks": [{ - "type": "command", - "command": TRAE_HOOK_COMMAND, - "timeout": 30 - }] - })); - Ok(()) + root.as_object_mut() + .expect("validated object") + .entry("version") + .or_insert(serde_json::json!(1)); + append_hook_entry( + root, + PRE_TOOL_USE_KEY, + serde_json::json!({ + "matcher": TRAE_RUN_COMMAND_MATCHER, + "hooks": [{"type": "command", "command": TRAE_HOOK_COMMAND, "timeout": 30}] + }), + ) } /// Remove RTK commands from nested Trae PreToolUse groups. A group is pruned /// only when removing RTK made its nested `hooks` array empty. fn remove_trae_hook_from_json(root: &mut serde_json::Value) -> bool { - let Some(groups) = root - .get_mut("hooks") - .and_then(|hooks| hooks.get_mut(PRE_TOOL_USE_KEY)) - .and_then(|groups| groups.as_array_mut()) - else { - return false; - }; - - let mut removed = false; - groups.retain_mut(|group| { - let Some(hooks) = group - .get_mut("hooks") - .and_then(|hooks| hooks.as_array_mut()) - else { - return true; - }; - - let original_len = hooks.len(); - hooks.retain(|hook| !is_trae_hook_entry(hook)); - if hooks.len() == original_len { - return true; - } - - removed = true; - !hooks.is_empty() - }); - - removed + remove_hook_entries( + root, + PRE_TOOL_USE_KEY, + HookEntries::Grouped, + is_trae_hook_entry, + ) } /// Install Trae's native PreToolUse hook in the project or user configuration. @@ -5747,6 +5660,21 @@ fn show_claude_config() -> Result<()> { Ok(()) } +fn print_codex_hook_status(label: &str, path: &Path) -> Result<()> { + match read_json_file(path) { + Ok(Some(root)) if codex_hook_already_present(&root) => { + println!("[ok] {label} hook: {}", path.display()) + } + Ok(Some(_)) => println!("[--] {label} hooks.json exists but RTK hook is not configured"), + Ok(None) => println!("[--] {label} hook: not found"), + Err(error) if error.downcast_ref::().is_some() => { + println!("[!!] {label} hooks.json is invalid JSON") + } + Err(error) => return Err(error), + } + Ok(()) +} + fn show_codex_config() -> Result<()> { let codex_dir = resolve_codex_dir()?; let global_agents_md = codex_dir.join(AGENTS_MD); @@ -5765,23 +5693,7 @@ fn show_codex_config() -> Result<()> { println!("[--] Global RTK.md: not found"); } - if global_hooks_json.exists() { - let content = fs::read_to_string(&global_hooks_json).with_context(|| { - format!( - "Failed to read global Codex hooks: {}", - global_hooks_json.display() - ) - })?; - match serde_json::from_str::(&content) { - Ok(root) if codex_hook_already_present(&root) => { - println!("[ok] Global hook: {}", global_hooks_json.display()); - } - Ok(_) => println!("[--] Global hooks.json exists but RTK hook is not configured"), - Err(_) => println!("[!!] Global hooks.json is invalid JSON"), - } - } else { - println!("[--] Global hook: not found"); - } + print_codex_hook_status("Global", &global_hooks_json)?; if global_agents_md.exists() { let content = fs::read_to_string(&global_agents_md).with_context(|| { @@ -5807,23 +5719,7 @@ fn show_codex_config() -> Result<()> { println!("[--] Local RTK.md: not found"); } - if local_hooks_json.exists() { - let content = fs::read_to_string(&local_hooks_json).with_context(|| { - format!( - "Failed to read local Codex hooks: {}", - local_hooks_json.display() - ) - })?; - match serde_json::from_str::(&content) { - Ok(root) if codex_hook_already_present(&root) => { - println!("[ok] Local hook: {}", local_hooks_json.display()); - } - Ok(_) => println!("[--] Local hooks.json exists but RTK hook is not configured"), - Err(_) => println!("[!!] Local hooks.json is invalid JSON"), - } - } else { - println!("[--] Local hook: not found"); - } + print_codex_hook_status("Local", &local_hooks_json)?; if local_agents_md.exists() { let content = fs::read_to_string(&local_agents_md).with_context(|| { @@ -9984,6 +9880,244 @@ mod tests { }); } + #[test] + fn test_legacy_migration_preserves_mixed_groups_and_prompt_hooks() { + let legacy = "/home/user/hooks/rtk-rewrite.sh"; + let mut root = serde_json::json!({"hooks": {"PreToolUse": [ + {"hooks": [ + {"command": legacy}, + {"type": "prompt", "command": legacy}, + {"command": "echo user"}, + {"command": CLAUDE_HOOK_COMMAND} + ]}, + {"hooks": []} + ]}}); + assert!(remove_legacy_hook_entries_from_json(&mut root)); + assert!(!remove_legacy_hook_entries_from_json(&mut root)); + assert_eq!( + root["hooks"]["PreToolUse"], + serde_json::json!([ + {"hooks": [ + {"type": "prompt", "command": legacy}, + {"command": "echo user"}, + {"command": CLAUDE_HOOK_COMMAND} + ]}, + {"hooks": []} + ]) + ); + let mut cursor = serde_json::json!({"hooks": {"preToolUse": [ + {"command": legacy}, + {"type": "prompt", "command": legacy}, + {"command": CURSOR_HOOK_COMMAND} + ]}}); + assert!(remove_legacy_cursor_hook_entries_from_json(&mut cursor)); + assert!(!remove_legacy_cursor_hook_entries_from_json(&mut cursor)); + assert_eq!( + cursor["hooks"]["preToolUse"], + serde_json::json!([ + {"type": "prompt", "command": legacy}, + {"command": CURSOR_HOOK_COMMAND} + ]) + ); + } + + #[test] + fn test_hook_presence_respects_host_matcher_forms() { + for matcher in [ + None, + Some(""), + Some("*"), + Some("Bash"), + Some("Read|Bash"), + Some("^Ba"), + ] { + let mut root = serde_json::json!({"hooks": {"PreToolUse": [ + {"hooks": [{"command": CODEX_HOOK_COMMAND}]} + ]}}); + if let Some(matcher) = matcher { + root["hooks"]["PreToolUse"][0]["matcher"] = serde_json::json!(matcher); + } + assert!(codex_hook_already_present(&root), "{matcher:?}"); + root["hooks"]["PreToolUse"][0]["hooks"][0]["command"] = + serde_json::json!(CLAUDE_HOOK_COMMAND); + assert!( + hook_already_present(&root, CLAUDE_HOOK_COMMAND), + "{matcher:?}" + ); + } + for (matcher, expected) in [("Read, Bash", true), ("Ba", false), ("[", false)] { + let root = serde_json::json!({"hooks": {"PreToolUse": [ + {"matcher": matcher, "hooks": [{"command": CLAUDE_HOOK_COMMAND}]} + ]}}); + assert_eq!(hook_already_present(&root, CLAUDE_HOOK_COMMAND), expected); + } + for matcher in [ + None, + Some(""), + Some("*"), + Some("Shell"), + Some("Read|Shell"), + Some("^Sh"), + ] { + let mut root = + serde_json::json!({"hooks": {"preToolUse": [{"command": CURSOR_HOOK_COMMAND}]}}); + if let Some(matcher) = matcher { + root["hooks"]["preToolUse"][0]["matcher"] = serde_json::json!(matcher); + } + assert!(cursor_hook_already_present(&root), "{matcher:?}"); + } + } + + #[test] + fn test_grouped_registration_preserves_user_hooks_and_checks_matcher() { + for (command, tool) in [ + (CLAUDE_HOOK_COMMAND, "Bash"), + (CODEX_HOOK_COMMAND, "Bash"), + (TRAE_HOOK_COMMAND, TRAE_RUN_COMMAND_MATCHER), + ] { + let present = |root: &serde_json::Value| match command { + CLAUDE_HOOK_COMMAND => hook_already_present(root, command), + CODEX_HOOK_COMMAND => codex_hook_already_present(root), + _ => trae_hook_already_present(root), + }; + let mut root = serde_json::json!({"hooks": {"PreToolUse": [ + {"matcher": "Read", "hooks": [{"type": "command", "command": command, "timeout": 99}]}, + {"matcher": tool, "hooks": [{"type": "prompt", "command": command}]}, + {"matcher": tool, "hooks": []} + ], "Stop": [{"command": "echo stop"}]}, "user": true}); + assert!( + !present(&root), + "inactive and prompt hooks do not count: {command}" + ); + match command { + CLAUDE_HOOK_COMMAND | CODEX_HOOK_COMMAND => { + insert_hook_entry(&mut root, command).unwrap() + } + _ => insert_trae_hook_entry(&mut root).unwrap(), + } + assert!(present(&root)); + let groups = root["hooks"]["PreToolUse"].as_array_mut().unwrap(); + groups.last_mut().unwrap()["hooks"] + .as_array_mut() + .unwrap() + .push(serde_json::json!({"type": "command", "command": "echo user"})); + let remove = |root: &mut serde_json::Value| match command { + CLAUDE_HOOK_COMMAND => remove_hook_from_json(root), + CODEX_HOOK_COMMAND => remove_codex_hook_from_json(root), + _ => remove_trae_hook_from_json(root), + }; + assert!(remove(&mut root)); + assert!(!remove(&mut root)); + assert!(!present(&root)); + assert_eq!( + root["hooks"]["PreToolUse"], + serde_json::json!([ + {"matcher": tool, "hooks": [{"type": "prompt", "command": command}]}, + {"matcher": tool, "hooks": []}, + {"matcher": tool, "hooks": [{"type": "command", "command": "echo user"}]} + ]) + ); + assert_eq!( + root["hooks"]["Stop"], + serde_json::json!([{"command": "echo stop"}]) + ); + assert_eq!(root["user"], true); + } + } + + #[test] + fn test_cursor_registration_preserves_prompt_and_unrelated_entries() { + let mut root = serde_json::json!({"hooks": {"preToolUse": [ + {"matcher": "Read", "command": CURSOR_HOOK_COMMAND}, + {"matcher": "Shell", "type": "prompt", "command": CURSOR_HOOK_COMMAND}, + {"matcher": "Shell", "command": "echo user"} + ]}}); + assert!(!cursor_hook_already_present(&root)); + insert_cursor_hook_entry(&mut root).unwrap(); + assert!(cursor_hook_already_present(&root)); + assert!(remove_cursor_hook_from_json(&mut root)); + assert!(!remove_cursor_hook_from_json(&mut root)); + assert_eq!( + root["hooks"]["preToolUse"], + serde_json::json!([ + {"matcher": "Shell", "type": "prompt", "command": CURSOR_HOOK_COMMAND}, + {"matcher": "Shell", "command": "echo user"} + ]) + ); + } + + #[test] + fn test_patch_settings_json_dry_run_idempotency_and_backup() { + let tmp = TempDir::new().unwrap(); + with_claude_dir_override(&tmp, |dir| { + let path = dir.join(SETTINGS_JSON); + let original = "\u{feff}{\"permissions\":{\"allow\":[\"Bash(ls)\"]},\"hooks\":{\"PreToolUse\":[{\"matcher\":\"Bash\",\"hooks\":[{\"type\":\"command\",\"command\":\"echo user\"}]}]}}"; + fs::write(&path, original).unwrap(); + let dry = InitContext { + dry_run: true, + ..Default::default() + }; + assert!(matches!( + patch_settings_json_command(CLAUDE_HOOK_COMMAND, PatchMode::Auto, false, dry) + .unwrap(), + PatchResult::WouldPatch + )); + assert_eq!(fs::read_to_string(&path).unwrap(), original); + assert!(!path.with_extension("json.bak").exists()); + assert!(matches!( + patch_settings_json_command( + CLAUDE_HOOK_COMMAND, + PatchMode::Auto, + false, + InitContext::default() + ) + .unwrap(), + PatchResult::Patched + )); + let installed = fs::read_to_string(&path).unwrap(); + assert!(matches!( + patch_settings_json_command( + CLAUDE_HOOK_COMMAND, + PatchMode::Auto, + false, + InitContext::default() + ) + .unwrap(), + PatchResult::AlreadyPresent + )); + assert_eq!(fs::read_to_string(&path).unwrap(), installed); + assert_eq!( + fs::read_to_string(path.with_extension("json.bak")).unwrap(), + original + ); + let root = read_json_file(&path).unwrap().unwrap(); + assert_eq!(root["permissions"]["allow"][0], "Bash(ls)"); + assert_eq!( + root["hooks"]["PreToolUse"][0]["hooks"][0]["command"], + "echo user" + ); + }); + } + + #[test] + fn test_patch_settings_json_backup_failure_preserves_original() { + let tmp = TempDir::new().unwrap(); + with_claude_dir_override(&tmp, |dir| { + let path = dir.join(SETTINGS_JSON); + fs::write(&path, "{}").unwrap(); + fs::create_dir(path.with_extension("json.bak")).unwrap(); + let error = patch_settings_json_command( + CLAUDE_HOOK_COMMAND, + PatchMode::Auto, + false, + InitContext::default(), + ) + .unwrap_err(); + assert!(format!("{error:#}").contains(&path.display().to_string())); + assert_eq!(fs::read_to_string(&path).unwrap(), "{}"); + }); + } + #[test] fn test_patch_settings_json_tolerates_utf8_bom() { let tmp = TempDir::new().unwrap(); diff --git a/tests/hook_config_io_test.rs b/tests/hook_config_io_test.rs new file mode 100644 index 0000000000..2abd8bc256 --- /dev/null +++ b/tests/hook_config_io_test.rs @@ -0,0 +1,104 @@ +//! Regression coverage for the shared hook configuration I/O. +use std::fs; +use std::io::Write; +use std::process::{Command, Stdio}; +use tempfile::TempDir; + +#[test] +fn copilot_cli_rewrite_preserves_vscode_tool_input() { + let temp = TempDir::new().unwrap(); + let input = serde_json::json!({ + "tool_name": "run_in_terminal", + "tool_input": {"command": "git status", "timeout": 5000, "description": "Inspect changes"} + }); + let mut child = Command::new(env!("CARGO_BIN_EXE_rtk")) + .args(["hook", "copilot"]) + .current_dir(temp.path()) + .env("HOME", temp.path()) + .env("CLAUDE_CONFIG_DIR", temp.path().join("claude")) + .env("XDG_CONFIG_HOME", temp.path().join("config")) + .env("RTK_DB_PATH", temp.path().join("rtk.db")) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + child + .stdin + .take() + .unwrap() + .write_all(input.to_string().as_bytes()) + .unwrap(); + let output = child.wait_with_output().unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let response: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let mut expected = input["tool_input"].clone(); + expected["command"] = serde_json::json!("rtk git status"); + assert_eq!(response["hookSpecificOutput"]["updatedInput"], expected); + assert!( + response["hookSpecificOutput"] + .get("permissionDecision") + .is_none() + ); +} + +#[test] +fn codex_status_handles_global_and_local_empty_bom_and_invalid_files() { + let temp = TempDir::new().unwrap(); + let global = temp.path().join("codex-home"); + let project = temp.path().join("project"); + fs::create_dir_all(&global).unwrap(); + fs::create_dir_all(project.join(".codex")).unwrap(); + let global_path = global.join("hooks.json"); + let local_path = project.join(".codex/hooks.json"); + let show = || { + let output = Command::new(env!("CARGO_BIN_EXE_rtk")) + .args(["init", "--codex", "--show"]) + .current_dir(&project) + .env("HOME", temp.path()) + .env("CODEX_HOME", &global) + .env("XDG_CONFIG_HOME", temp.path().join("config")) + .env("RTK_DB_PATH", temp.path().join("rtk.db")) + .output() + .unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8(output.stdout).unwrap() + }; + let missing = show(); + for label in ["Global", "Local"] { + assert!(missing.contains(&format!("[--] {label} hook: not found"))); + } + for content in ["", " \n", "\u{feff}"] { + fs::write(&global_path, content).unwrap(); + fs::write(&local_path, content).unwrap(); + let status = show(); + for label in ["Global", "Local"] { + assert!(status.contains(&format!( + "[--] {label} hooks.json exists but RTK hook is not configured" + ))); + } + assert!(!status.contains("invalid JSON")); + } + let installed = "\u{feff}{\"hooks\":{\"PreToolUse\":[{\"matcher\":\"Bash\",\"hooks\":[{\"type\":\"command\",\"command\":\"rtk hook codex\"}]}]}}"; + fs::write(&global_path, installed).unwrap(); + fs::write(&local_path, installed).unwrap(); + let status = show(); + assert!(status.contains("[ok] Global hook:")); + assert!(status.contains("[ok] Local hook:")); + fs::write(&global_path, "{broken").unwrap(); + fs::write(&local_path, "{broken").unwrap(); + let status = show(); + for label in ["Global", "Local"] { + assert!(status.contains(&format!("[!!] {label} hooks.json is invalid JSON"))); + } + assert_eq!(fs::read_to_string(&global_path).unwrap(), "{broken"); + assert_eq!(fs::read_to_string(&local_path).unwrap(), "{broken"); +} From bdee7f2046c6a65477c2bf390a12b31a827d28ce Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Mon, 21 Sep 2026 18:23:41 +0900 Subject: [PATCH 21/66] fix(diff): distinguish read failures from file differences --- README.md | 2 +- README_fr.md | 2 +- docs/usage/FEATURES.md | 4 +++ src/cmds/git/diff_cmd.rs | 45 ++++++++++++++++++++++++++++---- src/main.rs | 3 +++ tests/diff_byte_accuracy_test.rs | 37 ++++++++++++++++++++++++++ 6 files changed, 86 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 8bed27497d..018ae5e4c9 100644 --- a/README.md +++ b/README.md @@ -178,7 +178,7 @@ rtk read file.rs -l aggressive # Signatures only (strips bodies) rtk smart file.rs # 2-line heuristic code summary rtk find "*.rs" . # Compact find results rtk grep "pattern" . # Grouped search results -rtk diff file1 file2 # Condensed diff (exit 1 if files differ) +rtk diff file1 file2 # Condensed diff (exit 0: identical, 1: different, 2: read error) ``` ### Git diff --git a/README_fr.md b/README_fr.md index d9004e97fe..70a37f1796 100644 --- a/README_fr.md +++ b/README_fr.md @@ -145,7 +145,7 @@ rtk read file.rs # Lecture intelligente rtk read file.rs -l aggressive # Signatures uniquement rtk find "*.rs" . # Resultats compacts rtk grep "pattern" . # Resultats groupes par fichier -rtk diff file1 file2 # Diff condense +rtk diff file1 file2 # Diff condense (code 0 : identiques, 1 : differents, 2 : erreur de lecture) ``` ### Git diff --git a/docs/usage/FEATURES.md b/docs/usage/FEATURES.md index b5e26f2bd8..633722cafd 100644 --- a/docs/usage/FEATURES.md +++ b/docs/usage/FEATURES.md @@ -267,6 +267,10 @@ rtk diff rtk diff # Stdin comme second fichier ``` +Pour comparer deux fichiers : code de sortie **0** si identiques, **1** si differents, +**2** si un fichier ne peut pas etre lu. Les fichiers non UTF-8 sont compares octet +par octet ; seuls leurs noms sont affiches lorsqu'ils different. + --- ### `rtk wc` -- Comptage compact diff --git a/src/cmds/git/diff_cmd.rs b/src/cmds/git/diff_cmd.rs index 632f3ae24c..1fc3382940 100644 --- a/src/cmds/git/diff_cmd.rs +++ b/src/cmds/git/diff_cmd.rs @@ -33,25 +33,60 @@ static MARKED_PROSE_RE: LazyLock = const IDENTICAL_FILES_MESSAGE: &str = "[ok] Files are identical\n"; /// Ultra-condensed diff - only changed lines, no context. -/// Returns the diff-convention exit code: 0 if identical, 1 if files differ. +/// Returns the diff-convention exit code: 0 if identical, 1 if files differ, +/// 2 if an operand cannot be read. pub fn run(file1: &Path, file2: &Path, verbose: u8) -> Result { let timer = tracking::TimedExecution::start(); + let command = format!("diff {} {}", file1.display(), file2.display()); if verbose > 0 { eprintln!("Comparing: {} vs {}", file1.display(), file2.display()); } - let content1 = fs::read_to_string(file1)?; - let content2 = fs::read_to_string(file2)?; + // Read bytes first so non-UTF-8 data is not mistaken for an I/O failure. + let (bytes1, bytes2) = match fs::read(file1) + .map_err(|error| (file1, error)) + .and_then(|first| { + fs::read(file2) + .map(|second| (first, second)) + .map_err(|error| (file2, error)) + }) { + Ok(contents) => contents, + Err((path, error)) => { + let message = format!("rtk diff: {}: {}", path.display(), error); + eprintln!("{}", message); + timer.track(&command, "rtk diff", &message, &message); + return Ok(2); + } + }; + + let (content1, content2) = match (std::str::from_utf8(&bytes1), std::str::from_utf8(&bytes2)) { + (Ok(first), Ok(second)) => (first, second), + _ => { + let different = bytes1 != bytes2; + let message = if different { + format!( + "Binary files {} and {} differ\n", + file1.display(), + file2.display() + ) + } else { + String::new() + }; + print!("{}", message); + timer.track(&command, "rtk diff", &message, &message); + return Ok(i32::from(different)); + } + }; let both_files = format!("{}\n---\n{}", content1, content2); - let comparison = compare_files(&content1, &content2); + let comparison = compare_files(content1, content2); let fallback = classic_fallback(&comparison); let (rtk, exit_code) = render_diff(file1, file2, &comparison); let shown = select_file_diff_output(&comparison, &fallback, &both_files, &rtk); print!("{}", shown); timer.track( - &format!("diff {} {}", file1.display(), file2.display()), + &command, "rtk diff", tracking_baseline(&fallback, &both_files, shown), shown, diff --git a/src/main.rs b/src/main.rs index 56bd327a8e..1a0877de38 100644 --- a/src/main.rs +++ b/src/main.rs @@ -286,6 +286,9 @@ enum Commands { }, /// Ultra-condensed diff (only changed lines) + /// + /// Comparing two files exits 0 if identical, 1 if different, and 2 on a + /// file-read error. Non-UTF-8 files are compared byte for byte. Diff { /// First file or - for stdin (unified diff) file1: PathBuf, diff --git a/tests/diff_byte_accuracy_test.rs b/tests/diff_byte_accuracy_test.rs index bb999b6842..d46557a50f 100644 --- a/tests/diff_byte_accuracy_test.rs +++ b/tests/diff_byte_accuracy_test.rs @@ -62,3 +62,40 @@ fn one_line_crlf_vs_lf_diff_prints_whitespace_message() { // above raw drops it here. It is shown regardless of size. assert_explains_invisible_difference(ONE_LINE_LF_CONTENT, ONE_LINE_CRLF_CONTENT); } + +#[test] +fn missing_operand_exits_two_and_names_the_failed_path() { + let dir = tempfile::tempdir().unwrap(); + let existing = dir.path().join("existing.txt"); + let missing = dir.path().join("missing.txt"); + fs::write(&existing, "content\n").unwrap(); + + for (left, right) in [(&missing, &existing), (&existing, &missing)] { + let output = run_rtk_diff(left, right); + let stderr = String::from_utf8_lossy(&output.stderr); + assert_eq!(output.status.code(), Some(2), "{stderr}"); + assert!(output.stdout.is_empty(), "read errors belong on stderr"); + assert!(stderr.contains(&*missing.to_string_lossy()), "{stderr}"); + } +} + +#[test] +fn non_utf8_files_are_compared_instead_of_reported_as_io_errors() { + let dir = tempfile::tempdir().unwrap(); + let left = dir.path().join("left.bin"); + let right = dir.path().join("right.bin"); + fs::write(&left, b"\xff").unwrap(); + + // Reading succeeds even though UTF-8 conversion fails. + for (content, expected) in [(b"\xff", 0), (b"\xfe", 1)] { + fs::write(&right, content).unwrap(); + let output = run_rtk_diff(&left, &right); + assert_eq!(output.status.code(), Some(expected)); + assert!(output.stderr.is_empty()); + if expected == 1 { + let stdout = String::from_utf8(output.stdout).unwrap(); + assert!(stdout.contains("Binary files"), "{stdout}"); + assert!(stdout.contains("differ"), "{stdout}"); + } + } +} From 757188f8235a5973e6fb72517faadaa39065b540 Mon Sep 17 00:00:00 2001 From: guy oron Date: Mon, 21 Sep 2026 14:12:45 +0300 Subject: [PATCH 22/66] fix(gradle): remove the unreachable gradle.toml filter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Its match_command, `^(gradle|gradlew|\./)gradlew?\b`, needs a doubled token (`gradlegradle`) or a literal `./gradle…`. Both call sites reduce argv[0] to its basename before the lookup — `run_fallback` in main.rs and `strip_absolute_path` on the hook rewrite path — so the `./` branch is unreachable too. The filter has matched nothing since it landed: the basename logic arrived in adda2537 (2026-03-10), the file in 8fae5b06 (2026-03-18). Deleting it changes no output. `src/discover/rules.rs` already routes the whole gradle family (`gradle`, `gradlew`, `./gradlew`, `gradlew.bat`) to `rtk gradlew` and the Rust gradlew_cmd, and `gradle … bootRun` is filtered by spring-boot.toml. Widening the regex instead would have shadowed spring-boot.toml — first match wins, ordered by filter name — and pulled gradle-profiler and gradle.bat into a head-truncating filter. Making TOML filters reachable from the default passthrough is a separate design question, tracked in #3904. Refs #2708 --- src/core/toml_filter.rs | 10 +++++----- src/filters/gradle.toml | 35 ----------------------------------- 2 files changed, 5 insertions(+), 40 deletions(-) delete mode 100644 src/filters/gradle.toml diff --git a/src/core/toml_filter.rs b/src/core/toml_filter.rs index 290b70fdf5..6850b596cb 100644 --- a/src/core/toml_filter.rs +++ b/src/core/toml_filter.rs @@ -2102,8 +2102,8 @@ match_command = "^make\\b" let filters = make_filters(BUILTIN_TOML); assert_eq!( filters.len(), - 63, - "Expected exactly 63 built-in filters, got {}. \ + 62, + "Expected exactly 62 built-in filters, got {}. \ Update this count when adding/removing filters in src/filters/.", filters.len() ); @@ -2211,11 +2211,11 @@ expected = "output line 1\noutput line 2" let combined = format!("{}\n\n{}", BUILTIN_TOML, new_filter); let filters = make_filters(&combined); - // All 63 existing filters still present + 1 new = 64 + // All 62 existing filters still present + 1 new = 63 assert_eq!( filters.len(), - 64, - "Expected 64 filters after concat (63 built-in + 1 new)" + 63, + "Expected 63 filters after concat (62 built-in + 1 new)" ); // New filter is discoverable diff --git a/src/filters/gradle.toml b/src/filters/gradle.toml deleted file mode 100644 index e6ad28a3ed..0000000000 --- a/src/filters/gradle.toml +++ /dev/null @@ -1,35 +0,0 @@ -[filters.gradle] -description = "Compact Gradle build output — strip progress, keep tasks and errors" -match_command = "^(gradle|gradlew|\\./)gradlew?\\b" -strip_ansi = true -strip_lines_matching = [ - "^\\s*$", - "^> Configuring project", - "^> Resolving dependencies", - "^> Transform ", - "^Download(ing)?\\s+http", - "^\\s*<-+>\\s*$", - "^> Task :.*UP-TO-DATE$", - "^> Task :.*NO-SOURCE$", - "^> Task :.*FROM-CACHE$", - "^Starting a Gradle Daemon", - "^Daemon will be stopped", -] -truncate_lines_at = 150 -max_lines = 50 -on_empty = "gradle: ok" - -[[tests.gradle]] -name = "strips UP-TO-DATE tasks, keeps build result" -input = "> Configuring project :app\n> Task :app:compileJava UP-TO-DATE\n> Task :app:compileKotlin UP-TO-DATE\n> Task :app:test\n\n3 tests completed, 1 failed\n\nBUILD FAILED in 12s" -expected = "> Task :app:test\n3 tests completed, 1 failed\nBUILD FAILED in 12s" - -[[tests.gradle]] -name = "clean build preserved" -input = "BUILD SUCCESSFUL in 8s\n7 actionable tasks: 7 executed" -expected = "BUILD SUCCESSFUL in 8s\n7 actionable tasks: 7 executed" - -[[tests.gradle]] -name = "empty after stripping" -input = "> Configuring project :app\n" -expected = "gradle: ok" From 2b2a662c6842e26805d804e2cb7f0aa19f6434a4 Mon Sep 17 00:00:00 2001 From: Israel Lot Date: Mon, 21 Sep 2026 11:24:22 +0000 Subject: [PATCH 23/66] feat(search): fold shared path prefix in file-list passthrough `rtk grep -l`, `grep -L`, `rg -l`, and `rg --files` print every path in full and record 0/0 savings. Agents search from absolute roots, so each line repeats the same long directory prefix. Fold that prefix into a ` (N files)` header and print each path's tail. The transform is lossless, keeps engine order, cuts only on whole components, and runs under `never_worse`. It applies only to a bare file list: any other shape flag (`-c`, `-Z`, `--null`, `--json`, `-q`) changes the line and leaves the list verbatim. Streaming stdin passthrough is untouched. Track the fold with real sizes so it shows in `rtk gain`; unchanged passthrough keeps recording 0/0. Follow-up to #2630, which targeted the pre-#2641 grep_cmd.rs. --- src/cmds/system/README.md | 2 +- src/cmds/system/search.rs | 256 ++++++++++++++++++++++++++-- tests/grep_faithful_format_test.rs | 40 ++++- tests/search_file_list_fold_test.rs | 224 ++++++++++++++++++++++++ 4 files changed, 497 insertions(+), 25 deletions(-) create mode 100644 tests/search_file_list_fold_test.rs diff --git a/src/cmds/system/README.md b/src/cmds/system/README.md index e11b34073c..4f7ad89361 100644 --- a/src/cmds/system/README.md +++ b/src/cmds/system/README.md @@ -5,7 +5,7 @@ ## Specifics - `read.rs` uses `core/filter` for language-aware code stripping (FilterLevel: none/minimal/aggressive) -- `search.rs` backs both `rtk grep` and `rtk rg`: it runs the invoked engine (never substituting one for the other) and groups its output, reading `core/config` for `limits.grep_max_results` and `limits.grep_max_per_file`. Format-altering flags (`-c`, `-l`, `-L`, `-o`, `-Z`) bypass RTK filtering and run raw. +- `search.rs` backs both `rtk grep` and `rtk rg`: it runs the invoked engine (never substituting one for the other) and groups its output, reading `core/config` for `limits.grep_max_results` and `limits.grep_max_per_file`. Format-altering flags (`-c`, `-l`, `-L`, `-o`, `-Z`) bypass RTK filtering and run raw, except a bare file list (`-l`, `-L`, `--files` with no other shape flag), whose shared directory prefix is folded into a ` (N files)` header under the `never_worse` guard. - `ctest_cmd.rs` takes the run total from the first result line (covering `--stop-on-failure`, disabled tests, and forwarded suites) and validates both result lines and the summary against it, deduplicates retries by test number+name, folds wrapped result lines until their terminator, falls back to the raw `FAILED:` trailer for unparsed failures, keeps the error trailer behind an empty run, labels and caps failure details with tee recovery, and attributes diagnostics safely under `-j`; explicit verbose/show-only/help/version flags and dashboard modes bypass filtering, except `-T Test`, which prints ordinary test output and stays filtered. - `local_llm.rs` (`rtk smart`) uses `core/filter` for heuristic file summarization - `format_cmd.rs` is a cross-ecosystem dispatcher: auto-detects and routes to `prettier_cmd` or `ruff_cmd` (black is handled inline, not as a separate module) diff --git a/src/cmds/system/search.rs b/src/cmds/system/search.rs index 77cc76ccbe..ea7089f2a3 100644 --- a/src/cmds/system/search.rs +++ b/src/cmds/system/search.rs @@ -4,6 +4,7 @@ //! compresses its output by grouping matches by file, capping, and teeing overflow. use crate::core::arg_tokenizer::{self, Dialect, Token, TokenKind, ValueSpec}; +use crate::core::guard::never_worse; use crate::core::stream::{ self, CaptureResult, FilterMode, StdinMode, StreamFilter, exec_capture, exec_capture_stdin, }; @@ -549,12 +550,17 @@ fn run_streaming_search( /// Runs the agent's command verbatim for forms RTK does not group: format/shape /// flags and pattern-less modes (`--files`, `--type-list`). +/// +/// One exception: a bare file list (`-l`/`-L`/`--files`, see [`is_bare_file_list`]) has its +/// shared directory prefix folded into a header when captured. The streaming form reads +/// stdin, where the only "file" is `(standard input)`, so it has nothing to fold. fn passthrough>( timer: &tracking::TimedExecution, engine: Engine, args: &[T], real_cmd: &str, stream_stdin: bool, + fold_file_list: bool, ) -> Result { let mut cmd = resolved_command(engine.bin()); if stream_stdin && !std::io::stdout().is_terminal() { @@ -565,21 +571,103 @@ fn passthrough>( cmd.child_arg(a.as_ref()); } - let exit_code = if stream_stdin { - stream::run_streaming(&mut cmd, StdinMode::Inherit, FilterMode::Passthrough) - .context("search failed")? - .exit_code + if stream_stdin { + let exit_code = + stream::run_streaming(&mut cmd, StdinMode::Inherit, FilterMode::Passthrough) + .context("search failed")? + .exit_code; + timer.track_passthrough(real_cmd, &format!("rtk {} (passthrough)", real_cmd)); + return Ok(exit_code); + } + + let result = exec_capture_stdin(&mut cmd).context("search failed")?; + let cleaned = strip_ansi(&result.stdout); + let folded = if fold_file_list { + fold_path_prefix(&cleaned).filter(|f| never_worse(&cleaned, f) == f) } else { - let result = exec_capture_stdin(&mut cmd).context("search failed")?; - print!("{}", strip_ansi(&result.stdout)); - if !result.stderr.is_empty() { - eprint!("{}", result.stderr); - } - result.exit_code + None }; + match &folded { + Some(folded) => print!("{}", folded), + None => print!("{}", cleaned), + } + if !result.stderr.is_empty() { + eprint!("{}", result.stderr); + } - timer.track_passthrough(real_cmd, &format!("rtk {} (passthrough)", real_cmd)); - Ok(exit_code) + match &folded { + // Real sizes, so the fold shows up in `rtk gain`. + Some(folded) => timer.track( + real_cmd, + &format!("rtk {}", engine.label()), + &cleaned, + folded, + ), + // 0/0 keeps an unchanged passthrough from diluting the savings statistics. + None => timer.track_passthrough(real_cmd, &format!("rtk {} (passthrough)", real_cmd)), + } + Ok(result.exit_code) +} + +/// Folds the directory prefix every line of a file list shares into a one-line header, then +/// emits each path's remaining tail in engine order: +/// +/// ```text +/// /home/u/proj/src/ (3 files) +/// a/foo.rs +/// a/bar.rs +/// b/baz.rs +/// ``` +/// +/// Agents search from absolute roots, so `-l` output restates the same long prefix on every +/// line; that prefix is the whole redundancy of the list. The transform is lossless (each path +/// is `prefix + tail`) and needs no cap or tee. `None` when there is nothing to fold: fewer +/// than two lines, a line that is not a plain path (empty, or NUL-joined under `-Z`), or no +/// shared directory component. The prefix is cut on whole components, never inside one, so +/// `/a/foobar/x` and `/a/foobaz/y` share `/a/`, not `/a/fooba`. +/// +/// KNOWN LIMITATION: only `/` separates components, so Windows-style `\` paths never fold. +fn fold_path_prefix(raw: &str) -> Option { + let paths: Vec<&str> = raw.lines().collect(); + if paths.len() < 2 || paths.iter().any(|p| p.is_empty() || p.contains('\0')) { + return None; + } + + fn dirs(path: &str) -> Vec<&str> { + match path.rsplit_once('/') { + Some((dir, _)) => dir.split('/').collect(), + None => Vec::new(), + } + } + + let first = dirs(paths[0]); + let mut common = first.len(); + for path in &paths[1..] { + let shared = dirs(path) + .iter() + .zip(&first) + .take(common) + .take_while(|(a, b)| a == b) + .count(); + common = shared; + if common == 0 { + return None; + } + } + + // An absolute path splits to a leading empty component, so `/a/b` rejoins with its slash; + // a bare `/` root is the one prefix that saves nothing. + let prefix = format!("{}/", first[..common].join("/")); + if prefix.len() <= 1 { + return None; + } + + let mut out = format!("{} ({} files)\n", prefix, paths.len()); + for path in &paths { + out.push_str(path.strip_prefix(&prefix).unwrap_or(path)); + out.push('\n'); + } + Some(out) } pub fn run( @@ -611,7 +699,7 @@ pub fn run( }); if dangling_value_flag { let real_cmd = format!("{} {}", engine.bin(), args.join(" ")); - return passthrough(&timer, engine, args, &real_cmd, false); + return passthrough(&timer, engine, args, &real_cmd, false, false); } if asks_for_help { @@ -632,7 +720,9 @@ pub fn run( extract_pattern_path(args, engine); if patterns.is_empty() { - return passthrough(&timer, engine, args, &real_cmd, false); + // `rg --files` lists paths without a pattern; fold it like `-l`. + let fold = is_bare_file_list(engine, args); + return passthrough(&timer, engine, args, &real_cmd, false, fold); } let pattern_display = if patterns.len() == 1 { @@ -652,7 +742,8 @@ pub fn run( // format/shape flags (-c/-l/-o/...): already-minimal native output, passthrough. if extra_args_has_format_flag { - return passthrough(&timer, engine, args, &real_cmd, reads_piped_stdin); + let fold = is_bare_file_list(engine, args); + return passthrough(&timer, engine, args, &real_cmd, reads_piped_stdin, fold); } if reads_piped_stdin { @@ -676,7 +767,7 @@ pub fn run( // Unparseable shape re-runs verbatim below (with its own stderr), so handle it // before surfacing this run's stderr (#2333). if unparsed_signal(&raw_output) > 0 { - return passthrough(&timer, engine, args, &real_cmd, false); + return passthrough(&timer, engine, args, &real_cmd, false, false); } if !result.stderr.is_empty() { @@ -895,6 +986,32 @@ fn is_format_flag_token(engine: Engine, kind: TokenKind, text: &str) -> bool { } } +/// True when the command's only shape flag is a file list -- `-l`/`--files-with-matches`, +/// grep's `-L`/`--files-without-match`, or rg's `--files` -- so every stdout line is one +/// plain path and [`fold_path_prefix`] applies. Any other shape flag changes the line +/// (`-c` appends `:count`, `-Z`/`--null` joins with NUL, `--json` wraps it) or removes it +/// (`-q`), so the list is left verbatim. +fn is_bare_file_list>(engine: Engine, args: &[T]) -> bool { + let tokens = tokenize_search_args(args, engine); + let mut file_list = false; + for t in &tokens { + let is_list = match t.kind { + TokenKind::Long => matches!( + t.text, + "files" | "files-with-matches" | "files-without-match" + ), + TokenKind::Short => t.text == "l" || (t.text == "L" && engine == Engine::Grep), + _ => false, + }; + if is_list { + file_list = true; + } else if is_format_flag_token(engine, t.kind, t.text) { + return false; + } + } + file_list +} + /// True for `-H`/`--with-filename`, an explicit request for the filename prefix (same meaning /// for both engines). fn is_show_file_token(kind: TokenKind, text: &str) -> bool { @@ -1149,6 +1266,113 @@ mod tests { assert!(!cleaned.is_empty()); } + // --- fold_path_prefix / is_bare_file_list --- + + #[test] + fn fold_path_prefix_folds_shared_dir_into_header() { + let raw = + "/home/u/proj/src/a/foo.rs\n/home/u/proj/src/a/bar.rs\n/home/u/proj/src/b/baz.rs\n"; + assert_eq!( + fold_path_prefix(raw).as_deref(), + Some("/home/u/proj/src/ (3 files)\na/foo.rs\na/bar.rs\nb/baz.rs\n") + ); + } + + #[test] + fn fold_path_prefix_keeps_engine_order_and_relative_paths() { + let raw = "src/z.rs\nsrc/a.rs\n"; + assert_eq!( + fold_path_prefix(raw).as_deref(), + Some("src/ (2 files)\nz.rs\na.rs\n") + ); + } + + #[test] + fn fold_path_prefix_cuts_on_whole_components() { + // `/a/foobar` and `/a/foobaz` share `/a/`, never the byte run `/a/fooba`. + let raw = "/a/foobar/x\n/a/foobaz/y\n"; + assert_eq!( + fold_path_prefix(raw).as_deref(), + Some("/a/ (2 files)\nfoobar/x\nfoobaz/y\n") + ); + } + + #[test] + fn fold_path_prefix_never_folds_into_a_filename() { + // The shared prefix is a directory only; a file that sits at the prefix root keeps its + // full name. + let raw = "/a/b/c.rs\n/a/b/d/e.rs\n"; + assert_eq!( + fold_path_prefix(raw).as_deref(), + Some("/a/b/ (2 files)\nc.rs\nd/e.rs\n") + ); + } + + #[test] + fn fold_path_prefix_none_when_nothing_to_fold() { + assert_eq!(fold_path_prefix(""), None, "empty"); + assert_eq!(fold_path_prefix("/a/b/c.rs\n"), None, "single line"); + assert_eq!(fold_path_prefix("a.rs\nb.rs\n"), None, "bare filenames"); + assert_eq!( + fold_path_prefix("/x/a.rs\n/y/b.rs\n"), + None, + "only `/` shared" + ); + assert_eq!( + fold_path_prefix("src/a.rs\nlib/b.rs\n"), + None, + "no shared dir" + ); + assert_eq!(fold_path_prefix("/a/b.rs\n\n/a/c.rs\n"), None, "blank line"); + assert_eq!( + fold_path_prefix("/a/b.rs\0/a/c.rs\0"), + None, + "NUL-joined (-Z)" + ); + } + + #[test] + fn fold_path_prefix_is_lossless() { + let raw = "/p/q/a.rs\n/p/q/r/b.rs\n/p/q/c.rs\n"; + let folded = fold_path_prefix(raw).expect("folds"); + let mut lines = folded.lines(); + let header = lines.next().unwrap(); + let prefix = header.strip_suffix(" (3 files)").unwrap(); + let rebuilt: String = lines.map(|tail| format!("{prefix}{tail}\n")).collect(); + assert_eq!(rebuilt, raw); + } + + #[test] + fn bare_file_list_detects_list_flags_per_engine() { + assert!(is_bare_file_list(Engine::Grep, &["-rl", "foo", "."])); + assert!(is_bare_file_list(Engine::Grep, &["-L", "foo", "."])); + assert!(is_bare_file_list( + Engine::Grep, + &["--files-with-matches", "foo"] + )); + assert!(is_bare_file_list(Engine::Rg, &["-l", "foo"])); + assert!(is_bare_file_list(Engine::Rg, &["--files", "src"])); + // rg's -L is --follow, not a list. + assert!(!is_bare_file_list(Engine::Rg, &["-L", "foo"])); + assert!(!is_bare_file_list(Engine::Grep, &["-rn", "foo", "."])); + } + + #[test] + fn bare_file_list_rejects_other_shape_flags() { + // Each of these changes or removes the path line, so the list must stay verbatim. + assert!(!is_bare_file_list(Engine::Grep, &["-lc", "foo", "."])); + assert!(!is_bare_file_list(Engine::Grep, &["-lZ", "foo", "."])); + assert!(!is_bare_file_list( + Engine::Grep, + &["-l", "--null", "foo", "."] + )); + assert!(!is_bare_file_list(Engine::Grep, &["-lq", "foo", "."])); + assert!(!is_bare_file_list(Engine::Rg, &["-l", "--json", "foo"])); + assert!(!is_bare_file_list(Engine::Rg, &["--files", "--null"])); + // `-e -l` is a pattern, not the list flag. + assert!(!is_bare_file_list(Engine::Grep, &["-e", "-l", "."])); + } + // --- extract_pattern_path --- // // parse_cluster/ClusterResult were replaced by arg_tokenizer::tokenize; the diff --git a/tests/grep_faithful_format_test.rs b/tests/grep_faithful_format_test.rs index 1717437d32..49382238f4 100644 --- a/tests/grep_faithful_format_test.rs +++ b/tests/grep_faithful_format_test.rs @@ -44,6 +44,30 @@ fn assert_eq_grep_with_and_without_n(args: &[&str]) { assert_eq_grep(&with_n); } +/// A file list (`-l`/`-L`) is the one passthrough RTK reshapes: when two or more paths share +/// a directory, that prefix is folded into a ` (N files)` header. Rebuild the paths +/// from the header and require the result to equal grep's list, path for path and in order; +/// with fewer than two paths there is no header and the output must be byte-identical. +fn assert_eq_grep_file_list(args: &[&str]) { + let (rtk, rc) = rtk_grep(args); + let (grep, gc) = grep_plain(args); + assert_eq!(rc, gc, "exit code mismatch for {args:?}"); + let unfolded = match rtk.lines().next().and_then(|h| h.strip_suffix(" files)")) { + Some(head) => { + let (prefix, n) = head.rsplit_once(" (").expect("header shape"); + let tails: Vec<&str> = rtk.lines().skip(1).collect(); + assert_eq!( + tails.len(), + n.parse::().unwrap(), + "count for {args:?}" + ); + tails.iter().map(|t| format!("{prefix}{t}\n")).collect() + } + None => rtk.clone(), + }; + assert_eq!(unfolded, grep, "file list mismatch for {args:?}:\n{rtk}"); +} + fn write(dir: &std::path::Path, name: &str, body: &str) -> String { let p = dir.join(name); std::fs::write(&p, body).expect("write"); @@ -212,9 +236,9 @@ fn dash_m_max_count_is_per_file_like_grep_n() { // Regression: the `-l` short used to be bound to RTK's --max-len, so `grep -l PAT` // made clap read PAT as a usize and error out (0% savings via raw fallback). -// `-l` is GNU grep's --files-with-matches; `rtk grep -l` must match `grep -l` -// byte-for-byte (explicit file args => deterministic order). Also covers `-l` -// trailing (arg-order sensitivity) and `-L` (--files-without-match). +// `-l` is GNU grep's --files-with-matches; `rtk grep -l` must list exactly what +// `grep -l` lists (explicit file args => deterministic order), modulo the shared-prefix +// fold. Also covers `-l` trailing (arg-order sensitivity) and `-L` (--files-without-match). #[test] fn dash_l_and_dash_cap_l_match_grep() { let d = tempfile::tempdir().unwrap(); @@ -223,15 +247,15 @@ fn dash_l_and_dash_cap_l_match_grep() { let f3 = write(d.path(), "hit2.txt", "tenant_id again\n"); let f4 = write(d.path(), "port.txt", "listen on 8080\n"); - assert_eq_grep(&["-l", "tenant_id", &f1, &f2, &f3]); // -l leading (the token that broke) - assert_eq_grep(&["tenant_id", &f1, &f2, &f3, "-l"]); // -l trailing - assert_eq_grep(&["-L", "tenant_id", &f1, &f2, &f3]); // -L files-without-match + assert_eq_grep_file_list(&["-l", "tenant_id", &f1, &f2, &f3]); // -l leading (the token that broke) + assert_eq_grep_file_list(&["tenant_id", &f1, &f2, &f3, "-l"]); // -l trailing + assert_eq_grep_file_list(&["-L", "tenant_id", &f1, &f2, &f3]); // -L files-without-match // A numeric pattern is the only form that failed silently: bound to `usize`, // `-l` swallowed it as max_len and read the first path as the pattern, so rtk // printed nothing and exited 1 while grep listed the file. A non-numeric // pattern stops at clap's parse error and falls back to raw grep, which is // byte-identical here and so invisible to the assertions above. - assert_eq_grep(&["-l", "8080", &f4, &f2]); - assert_eq_grep(&["-L", "8080", &f4, &f2]); + assert_eq_grep_file_list(&["-l", "8080", &f4, &f2]); + assert_eq_grep_file_list(&["-L", "8080", &f4, &f2]); } diff --git a/tests/search_file_list_fold_test.rs b/tests/search_file_list_fold_test.rs new file mode 100644 index 0000000000..c562d9e4db --- /dev/null +++ b/tests/search_file_list_fold_test.rs @@ -0,0 +1,224 @@ +//! `-l` / `-L` / `--files` passthrough folds the directory prefix every path shares into a +//! ` (N files)` header. Any other shape flag on top of the list leaves it verbatim. + +use std::path::Path; +use std::process::Command; + +fn rtk() -> Command { + let mut cmd = Command::new(env!("CARGO_BIN_EXE_rtk")); + cmd.env("RTK_NO_TRACK", "1"); + cmd +} + +fn rg_available() -> bool { + Command::new("rg") + .arg("--version") + .output() + .map(|o| o.status.success()) + .unwrap_or(false) +} + +/// `/src/a/foo.rs`, `/src/a/bar.rs`, `/src/b/baz.rs` all contain `needle`; +/// `/src/b/none.rs` does not. +fn fixture() -> tempfile::TempDir { + let dir = tempfile::tempdir().expect("tempdir"); + let src = dir.path().join("src"); + std::fs::create_dir_all(src.join("a")).unwrap(); + std::fs::create_dir_all(src.join("b")).unwrap(); + std::fs::write(src.join("a/foo.rs"), "needle\n").unwrap(); + std::fs::write(src.join("a/bar.rs"), "needle\n").unwrap(); + std::fs::write(src.join("b/baz.rs"), "needle\n").unwrap(); + std::fs::write(src.join("b/none.rs"), "hay\n").unwrap(); + dir +} + +fn src_prefix(root: &Path) -> String { + format!("{}/src/", root.display()) +} + +#[test] +fn grep_l_folds_shared_prefix_into_header() { + let dir = fixture(); + let src = dir.path().join("src"); + let out = rtk() + .args(["grep", "-rl", "needle", src.to_str().unwrap()]) + .output() + .expect("rtk grep"); + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!( + out.status.code(), + Some(0), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + + let prefix = src_prefix(dir.path()); + let mut lines = stdout.lines(); + assert_eq!( + lines.next(), + Some(format!("{prefix} (3 files)").as_str()), + "header must name the shared prefix and the count:\n{stdout}" + ); + let mut tails: Vec<&str> = lines.collect(); + tails.sort_unstable(); + assert_eq!(tails, ["a/bar.rs", "a/foo.rs", "b/baz.rs"], "{stdout}"); + assert!( + !stdout.contains(&format!("{prefix}a/")), + "no path may keep the folded prefix:\n{stdout}" + ); +} + +#[test] +fn grep_big_l_folds_too() { + let dir = fixture(); + let src = dir.path().join("src"); + let out = rtk() + .args(["grep", "-rL", "needle", src.to_str().unwrap()]) + .output() + .expect("rtk grep"); + let stdout = String::from_utf8_lossy(&out.stdout); + // Only one file lacks the needle: a single line has no prefix to fold, so the path + // prints in full, exactly as grep printed it. + assert_eq!( + stdout.trim_end(), + src.join("b/none.rs").to_str().unwrap(), + "{stdout}" + ); + + // Add a second non-matching file and the fold kicks in. + std::fs::write(src.join("a/none.rs"), "hay\n").unwrap(); + let out = rtk() + .args(["grep", "-rL", "needle", src.to_str().unwrap()]) + .output() + .expect("rtk grep"); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + stdout.starts_with(&format!("{} (2 files)\n", src_prefix(dir.path()))), + "{stdout}" + ); +} + +#[test] +fn single_file_list_line_is_verbatim() { + let dir = fixture(); + let only = dir.path().join("src/a/foo.rs"); + let out = rtk() + .args(["grep", "-l", "needle", only.to_str().unwrap()]) + .output() + .expect("rtk grep"); + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!(stdout.trim_end(), only.to_str().unwrap(), "{stdout}"); + assert!( + !stdout.contains("files)"), + "no header for one line:\n{stdout}" + ); +} + +#[test] +fn null_joined_list_is_verbatim() { + let dir = fixture(); + let src = dir.path().join("src"); + let out = rtk() + .args(["grep", "-rlZ", "needle", src.to_str().unwrap()]) + .output() + .expect("rtk grep"); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + stdout.contains('\0'), + "-Z output must stay NUL-joined:\n{stdout:?}" + ); + assert!( + !stdout.contains("files)"), + "no header under -Z:\n{stdout:?}" + ); + assert_eq!( + stdout.matches(&src_prefix(dir.path())).count(), + 3, + "every path keeps its full prefix under -Z:\n{stdout:?}" + ); +} + +#[test] +fn count_with_list_is_verbatim() { + let dir = fixture(); + let src = dir.path().join("src"); + let out = rtk() + .args(["grep", "-rlc", "needle", src.to_str().unwrap()]) + .output() + .expect("rtk grep"); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!(!stdout.contains("files)"), "no header under -c:\n{stdout}"); + assert_eq!( + stdout.matches(&src_prefix(dir.path())).count(), + 3, + "{stdout}" + ); +} + +#[test] +fn no_match_list_exits_1_with_empty_stdout() { + let dir = fixture(); + let src = dir.path().join("src"); + let out = rtk() + .args(["grep", "-rl", "absent", src.to_str().unwrap()]) + .output() + .expect("rtk grep"); + assert_eq!(out.status.code(), Some(1)); + assert!( + out.stdout.is_empty(), + "{:?}", + String::from_utf8_lossy(&out.stdout) + ); +} + +#[test] +fn rg_l_and_files_fold() { + if !rg_available() { + return; + } + let dir = fixture(); + let src = dir.path().join("src"); + let prefix = src_prefix(dir.path()); + + let out = rtk() + .args(["rg", "-l", "needle", src.to_str().unwrap()]) + .output() + .expect("rtk rg"); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + stdout.starts_with(&format!("{prefix} (3 files)\n")), + "{stdout}" + ); + + let out = rtk() + .args(["rg", "--files", src.to_str().unwrap()]) + .output() + .expect("rtk rg"); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + stdout.starts_with(&format!("{prefix} (4 files)\n")), + "{stdout}" + ); + let mut tails: Vec<&str> = stdout.lines().skip(1).collect(); + tails.sort_unstable(); + assert_eq!(tails, ["a/bar.rs", "a/foo.rs", "b/baz.rs", "b/none.rs"]); +} + +#[test] +fn rg_l_from_piped_stdin_is_verbatim() { + if !rg_available() { + return; + } + use std::io::Write; + use std::process::Stdio; + let mut child = rtk() + .args(["rg", "-l", "needle"]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .spawn() + .expect("spawn"); + child.stdin.take().unwrap().write_all(b"needle\n").unwrap(); + let out = child.wait_with_output().unwrap(); + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!(stdout.trim_end(), "", "{stdout}"); +} From 2de82b9d77906da48de4b45175a627e265f87dbe Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Tue, 22 Sep 2026 02:46:08 +0200 Subject: [PATCH 24/66] refactor(diff): name the trouble exit code `Ok(2)` at the read-failure site does not say which of diff's three exit codes it is. The constant states the contract the tests and the docs now depend on: 2 means an operand could not be read, not that the files differ. Co-Authored-By: Claude Opus 5 (1M context) --- src/cmds/git/diff_cmd.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/cmds/git/diff_cmd.rs b/src/cmds/git/diff_cmd.rs index 1fc3382940..0131d1b6de 100644 --- a/src/cmds/git/diff_cmd.rs +++ b/src/cmds/git/diff_cmd.rs @@ -32,6 +32,11 @@ static MARKED_PROSE_RE: LazyLock = const IDENTICAL_FILES_MESSAGE: &str = "[ok] Files are identical\n"; +/// POSIX diff's "trouble" exit code: an operand could not be read at all +/// (missing, permission denied, is-a-directory). Distinct from 1, which says +/// the two files differ — the distinction a caller's `if diff a b` relies on. +const DIFF_EXIT_TROUBLE: i32 = 2; + /// Ultra-condensed diff - only changed lines, no context. /// Returns the diff-convention exit code: 0 if identical, 1 if files differ, /// 2 if an operand cannot be read. @@ -56,7 +61,7 @@ pub fn run(file1: &Path, file2: &Path, verbose: u8) -> Result { let message = format!("rtk diff: {}: {}", path.display(), error); eprintln!("{}", message); timer.track(&command, "rtk diff", &message, &message); - return Ok(2); + return Ok(DIFF_EXIT_TROUBLE); } }; From 5ec8b8c6ace829cfaf835c960f81409067a89513 Mon Sep 17 00:00:00 2001 From: Jonathan Davies Date: Sat, 19 Sep 2026 08:23:45 +0000 Subject: [PATCH 25/66] refactor(hooks): split `init.rs` into per-agent submodules --- .claude/agents/rust-rtk.md | 2 +- .claude/commands/tech/codereview.md | 2 +- .claude/rules/search-strategy.md | 4 +- AWARENESS_CONFIG.md | 6 +- hooks/README.md | 2 +- src/hooks/README.md | 2 +- src/hooks/init.rs | 13626 ------------------------ src/hooks/init/agents_md.rs | 427 + src/hooks/init/claude.rs | 304 + src/hooks/init/codex.rs | 2325 ++++ src/hooks/init/copilot.rs | 837 ++ src/hooks/init/cursor.rs | 544 + src/hooks/init/droid.rs | 822 ++ src/hooks/init/gemini.rs | 365 + src/hooks/init/hermes.rs | 1046 ++ src/hooks/init/instructions_agents.rs | 394 + src/hooks/init/mod.rs | 3196 ++++++ src/hooks/init/omp.rs | 654 ++ src/hooks/init/opencode.rs | 1444 +++ src/hooks/init/trae.rs | 919 ++ src/hooks/init/vibe.rs | 495 + 21 files changed, 13781 insertions(+), 13635 deletions(-) delete mode 100644 src/hooks/init.rs create mode 100644 src/hooks/init/agents_md.rs create mode 100644 src/hooks/init/claude.rs create mode 100644 src/hooks/init/codex.rs create mode 100644 src/hooks/init/copilot.rs create mode 100644 src/hooks/init/cursor.rs create mode 100644 src/hooks/init/droid.rs create mode 100644 src/hooks/init/gemini.rs create mode 100644 src/hooks/init/hermes.rs create mode 100644 src/hooks/init/instructions_agents.rs create mode 100644 src/hooks/init/mod.rs create mode 100644 src/hooks/init/omp.rs create mode 100644 src/hooks/init/opencode.rs create mode 100644 src/hooks/init/trae.rs create mode 100644 src/hooks/init/vibe.rs diff --git a/.claude/agents/rust-rtk.md b/.claude/agents/rust-rtk.md index ec5ed45d6e..bad6c2b2bd 100644 --- a/.claude/agents/rust-rtk.md +++ b/.claude/agents/rust-rtk.md @@ -325,7 +325,7 @@ fn test_real_git_log() { - `src/cmds/system/` - ls.rs, tree.rs, read.rs, grep_cmd.rs, find_cmd.rs, etc. **Hook & analytics** (`src/hooks/`, `src/analytics/`): -- `src/hooks/init.rs` - rtk init command +- `src/hooks/init/` - rtk init command - `src/analytics/gain.rs` - rtk gain command **Tests**: diff --git a/.claude/commands/tech/codereview.md b/.claude/commands/tech/codereview.md index 8dd53b9a09..5167aba1ad 100644 --- a/.claude/commands/tech/codereview.md +++ b/.claude/commands/tech/codereview.md @@ -63,7 +63,7 @@ git diff "$BASE_BRANCH"...HEAD --stat | `src/main.rs` | Command routing + Commands enum | | `src/core/tracking.rs` | SQLite patterns + DB path config | | `src/core/config.rs` | Configuration system | -| `src/hooks/init.rs` | Init patterns + hook installation | +| `src/hooks/init/` | Init patterns + hook installation | | `.github/workflows/` | CI/CD multi-platform build targets | | `tests/` ou `fixtures/` | Testing Strategy (CLAUDE.md) | | `Cargo.toml` | Dependencies + build optimizations | diff --git a/.claude/rules/search-strategy.md b/.claude/rules/search-strategy.md index 05fa29538f..622f27262b 100644 --- a/.claude/rules/search-strategy.md +++ b/.claude/rules/search-strategy.md @@ -32,7 +32,7 @@ src/ │ ├── runner.rs ← Command execution runner │ └── stream.rs ← Streaming output handling ├── hooks/ ← Hook system -│ ├── init.rs ← rtk init command +│ ├── init/ ← rtk init command │ ├── rewrite_cmd.rs ← rtk rewrite command │ ├── hook_cmd.rs ← Gemini/Copilot hook processors │ ├── hook_check.rs ← Hook status detection @@ -146,7 +146,7 @@ Glob pattern="tests/fixtures/*.txt" ### Configuration issues 1. `src/core/config.rs` → `RtkConfig` struct -2. `src/hooks/init.rs` → `rtk init` command +2. `src/hooks/init/` → `rtk init` command 3. Config file: `~/.config/rtk/config.toml` 4. Filter files: `~/.config/rtk/filters/` (global) or `.rtk/filters/` (project) diff --git a/AWARENESS_CONFIG.md b/AWARENESS_CONFIG.md index 46ebfc5289..51059fc275 100644 --- a/AWARENESS_CONFIG.md +++ b/AWARENESS_CONFIG.md @@ -61,7 +61,7 @@ pub struct Config { ``` Unknown value (`level = "max"`) is a TOML parse error, same strictness as `tee.mode`. -Known gap, out of scope here: `init.rs` calls `Config::load().unwrap_or_default()` +Known gap, out of scope here: `src/hooks/init/` calls `Config::load().unwrap_or_default()` (lines ~461/473), so a malformed config silently falls back to defaults during init. Worth a warning on stderr in the same PR, since a typo in `level` would otherwise silently install `default` and the user would not know why. @@ -70,7 +70,7 @@ No CLI flag in phase 1. Config is the source of truth; switching level = edit co `rtk init -g`. `write_if_changed` already rewrites `RTK.md` when content differs. Optional phase 2: `rtk init --awareness high` as a one-run override (no persistence). -## Wiring in `src/hooks/init.rs` +## Wiring in `src/hooks/init/` ```rust const RTK_AWARENESS_DEFAULT: &str = include_str!("../../hooks/rtk-awareness.md"); @@ -211,7 +211,7 @@ tokens; behavior and exit code are unchanged. - `level = "max"` → `Err`. - Round-trip: `Config::default()` serializes `[awareness]\nlevel = "default"` (so `rtk config` shows it). -`src/hooks/init.rs`: +`src/hooks/init/`: - `awareness_content` maps each variant to its constant. - `HIGH.starts_with(DEFAULT.trim_end())` — `high` is `default` plus a tail. - `FULL.contains(DEFAULT.trim_end())` — the output contract is verbatim in `full`. diff --git a/hooks/README.md b/hooks/README.md index b158b37c32..381bc01765 100644 --- a/hooks/README.md +++ b/hooks/README.md @@ -6,7 +6,7 @@ Owns: per-agent hook scripts and configuration files for 12 supported agents (Claude Code, Copilot, Cursor, Cline, Windsurf, Codex, OpenCode, Hermes, Pi, Oh My Pi, Mistral Vibe, Trae). -Does **not** own: hook installation/uninstallation (that's `src/hooks/init.rs`), the rewrite pattern registry (that's `discover/registry`), or integrity verification (that's `src/hooks/integrity.rs`). +Does **not** own: hook installation/uninstallation (that's `src/hooks/init/`), the rewrite pattern registry (that's `discover/registry`), or integrity verification (that's `src/hooks/integrity.rs`). Relationship to `src/hooks/`: that component **creates** these files; this directory **contains** them. diff --git a/src/hooks/README.md b/src/hooks/README.md index 582a0999a3..8d143b3e9f 100644 --- a/src/hooks/README.md +++ b/src/hooks/README.md @@ -107,4 +107,4 @@ Rules are loaded from all Claude Code `settings.json` files (project + global, i Hook processors in `hook_cmd.rs` must return `Ok(())` on every path — success, no-match, parse error, and unexpected input. Returning `Err` propagates to `main()` and exits non-zero, which blocks the agent's command from executing. This violates the non-blocking guarantee documented in `hooks/README.md`. ## Adding New Functionality -To add support for a new AI coding agent: (1) add the hook installation logic to `init.rs` following the existing agent patterns, (2) if the agent requires a custom hook protocol (like Gemini's `BeforeTool` or Vibe's `pre_tool`), add a processor function in `hook_cmd.rs` and a matching `HookCommands::` variant + `AgentTarget::` enum entry in `main.rs`, (3) if the agent has installable permission surfaces (denylist / allowlist), wire them into `permissions.rs::check_command_for` via a new `Host::` variant, and (4) update `integrity.rs` with the expected hash for the new hook file. Note that `hook_check.rs::maybe_warn()` only checks the Claude Code hook — other agents don't have an outdated-hook warning path. Test by running `rtk init` in a fresh environment and verifying the hook rewrites commands correctly in the target agent. +To add support for a new AI coding agent: (1) add the hook installation logic to `src/hooks/init/` following the existing agent patterns, (2) if the agent requires a custom hook protocol (like Gemini's `BeforeTool` or Vibe's `pre_tool`), add a processor function in `hook_cmd.rs` and a matching `HookCommands::` variant + `AgentTarget::` enum entry in `main.rs`, (3) if the agent has installable permission surfaces (denylist / allowlist), wire them into `permissions.rs::check_command_for` via a new `Host::` variant, and (4) update `integrity.rs` with the expected hash for the new hook file. Note that `hook_check.rs::maybe_warn()` only checks the Claude Code hook — other agents don't have an outdated-hook warning path. Test by running `rtk init` in a fresh environment and verifying the hook rewrites commands correctly in the target agent. diff --git a/src/hooks/init.rs b/src/hooks/init.rs deleted file mode 100644 index 431b53c1f5..0000000000 --- a/src/hooks/init.rs +++ /dev/null @@ -1,13626 +0,0 @@ -//! Sets up RTK hooks so AI coding agents automatically route commands through RTK. - -use anyhow::{Context, Result}; -use std::ffi::OsString; -use std::fs; -use std::io::Write; -use std::path::{Component, Path, PathBuf}; -use tempfile::NamedTempFile; - -use crate::core::utils::{from_json_str, strip_leading_bom}; -use crate::hooks::constants::{ - CONFIG_DIR, COPILOT_HOME_ENV, COPILOT_HOOK_FILE, COPILOT_INSTRUCTIONS_FILE, COPILOT_USER_DIR, - CURSOR_DIR, GEMINI_DIR, GITHUB_DIR, OPENCODE_PLUGIN_FILE, OPENCODE_SUBDIR, PLUGIN_SUBDIR, - TRAE_CN_DIR, TRAE_DIR, TRAE_RUN_COMMAND_MATCHER, -}; - -use super::constants::{ - BEFORE_TOOL_KEY, CLAUDE_DIR, CLAUDE_HOOK_COMMAND, CODEX_DIR, CODEX_HOOK_COMMAND, - CURSOR_HOOK_COMMAND, DROID_DIR, DROID_EXECUTE_MATCHER, DROID_HOME_ENV, DROID_HOOK_COMMAND, - DROID_HOOKS_FILE, DROID_HOOKS_SUBDIR, DROID_SETTINGS_FILE, GEMINI_HOOK_FILE, HERMES_DIR, - HERMES_PLUGIN_INIT_FILE, HERMES_PLUGIN_MANIFEST_FILE, HERMES_PLUGIN_NAME, - HERMES_PLUGINS_SUBDIR, HOOKS_JSON, HOOKS_SUBDIR, OMP_DIR, OMP_LOCAL_DIR, PI_AGENT_STATE_FILE, - PI_CODING_AGENT_DIR_ENV, PI_DIR, PI_EXTENSIONS_SUBDIR, PI_LOCAL_DIR, PI_PLUGIN_FILE, - PRE_TOOL_USE_KEY, REWRITE_HOOK_FILE, SETTINGS_JSON, TRAE_HOOK_COMMAND, VIBE_BASH_MATCH, - VIBE_DIR, VIBE_HOOK_COMMAND, VIBE_HOOK_NAME, VIBE_HOOKS_FILE, VIBE_PROMPT_FILE, - VIBE_PROMPTS_SUBDIR, -}; -use super::integrity; -use super::{is_claude_hook_command, is_codex_hook_command, is_trae_hook_command}; -use crate::core::config::AwarenessLevel; - -// Embedded OpenCode plugin (auto-rewrite) -const OPENCODE_PLUGIN: &str = include_str!("../../hooks/opencode/rtk.ts"); - -// Embedded Pi extension (auto-rewrite) -const PI_PLUGIN: &str = include_str!("../../hooks/pi/rtk.ts"); - -// Stable code marker used to recognize a modified RTK extension without -// relying on explanatory comments that users may remove. The marker matches -// both the current `pi.exec` call and older stock revisions that imported -// `exec` locally before invoking it. -const PI_PLUGIN_REWRITE_MARKER: &str = "exec(\"rtk\", [\"rewrite\""; - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum PiCompatibleAgent { - Pi, - Omp, -} - -enum ManagedAgentState { - Absent, - Known(Vec), - Unknown, -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum ExtensionShareStatus { - NotShared, - Shared, - Unknown, -} - -impl PiCompatibleAgent { - fn name(self) -> &'static str { - match self { - Self::Pi => "Pi", - Self::Omp => "OMP", - } - } - - fn state_name(self) -> &'static str { - match self { - Self::Pi => "pi", - Self::Omp => "omp", - } - } - - fn other(self) -> Self { - match self { - Self::Pi => Self::Omp, - Self::Omp => Self::Pi, - } - } - - fn from_state_name(name: &str) -> Option { - match name { - "pi" => Some(Self::Pi), - "omp" => Some(Self::Omp), - _ => None, - } - } -} - -// SHA-256 hashes of stock Pi extension revisions that may exist on user -// machines, including the current embedded file. Keep historical entries -// when changing the extension so an untouched older install can still be -// removed safely. Hashes are computed after normalizing CRLF to LF and -// trimming trailing whitespace, matching the comparisons below. -// The history-based test below verifies that this list remains append-only -// for every revision in the current checkout's ancestor history. -const KNOWN_PI_PLUGIN_HASHES: &[&str] = &[ - "5e80e811e689adc9d5ae5a59d1d5702060ca0c10320fea7cffd83c659026f1c5", - "2cbb2a7a9081275d6eda140d9e375f6772b5c354e7fe931c554c371ad8836c6e", - "94e80d1a5c159ea38ba8913f7c5b9d9b5c89bf7c204f1e583bfac2ed7fc40ab9", - "b63e3f6eeaeec23837df5a7c4024fe16dca1f8a49fb1743f8a877cc136ebc2d9", - "c30d4f4774c59bf25b50b70ab8a7dcb1b8287074592af1598dc09962fa1c7137", - "5ad230679294dc8dce09546fa25101fd3d0949f454cc8b72e04664fa1bd45ed7", - "be251e44747e6d09e5ca56ecaeddd8f4861c35a57500cd8b2bf9c39afe5795e8", - "eb56dd08b8d5f4704906d037d70b357d84d827abe1063135cc7c998efe6cf7f2", - "628308173ae41c488b76bcf90eafbd4c0c72435927645d81cdbec652eac4b107", - "3eb16108f51a29c2a62a453d5c97a6ea2da8aea1061da34c50fdcfaa32dc0ff7", -]; - -// Embedded agent-neutral RTK awareness instructions, one file per `awareness.level`. -const RTK_AWARENESS_DEFAULT: &str = include_str!("../../hooks/rtk-awareness.md"); -const RTK_AWARENESS_HIGH: &str = include_str!("../../hooks/rtk-awareness-high.md"); -const RTK_AWARENESS_FULL: &str = include_str!("../../hooks/rtk-awareness-full.md"); - -/// Template written by `rtk init` when no filters.toml exists yet. -const FILTERS_TEMPLATE: &str = r#"# Project-local RTK filters — commit this file with your repo. -# Filters here override user-global and built-in filters. -# Docs: https://github.com/rtk-ai/rtk#custom-filters -schema_version = 1 - -# Example: suppress build noise from a custom tool -# [filters.my-tool] -# description = "Compact my-tool output" -# match_command = "^my-tool\\s+build" -# strip_ansi = true -# strip_lines_matching = ["^\\s*$", "^Downloading", "^Installing"] -# max_lines = 30 -# on_empty = "my-tool: ok" -"#; - -/// Template for user-global filters (~/.config/rtk/filters.toml). -const FILTERS_GLOBAL_TEMPLATE: &str = r#"# User-global RTK filters — apply to all your projects. -# Project-local .rtk/filters.toml takes precedence over these. -# Docs: https://github.com/rtk-ai/rtk#custom-filters -schema_version = 1 - -# Example: suppress noise from a tool you use everywhere -# [filters.my-global-tool] -# description = "Compact my-global-tool output" -# match_command = "^my-global-tool\\b" -# strip_ansi = true -# strip_lines_matching = ["^\\s*$"] -# max_lines = 40 -"#; - -const RTK_MD: &str = "RTK.md"; -const CLAUDE_MD: &str = "CLAUDE.md"; -const AGENTS_MD: &str = "AGENTS.md"; -const RTK_MD_REF: &str = "@RTK.md"; -const GEMINI_MD: &str = "GEMINI.md"; - -const RTK_BLOCK_START: &str = ""; - -/// Control flow for settings.json patching -#[derive(Debug, Clone, Copy, PartialEq)] -pub enum PatchMode { - Ask, // Default: prompt user [y/N] - Auto, // --auto-patch: no prompt - Skip, // --no-patch: manual instructions -} - -#[derive(Debug, Clone, Copy, PartialEq, Default)] -pub enum FilterTrust { - #[default] - Ask, - Trust, - Skip, -} - -/// Result of settings.json patching operation -#[derive(Debug, Clone, Copy, PartialEq)] -pub enum PatchResult { - Patched, // Hook was added successfully - AlreadyPresent, // Hook was already in settings.json - Declined, // User declined when prompted - Skipped, // --no-patch flag used - WouldPatch, // Dry-run: hook would have been added -} - -/// Shared context threaded through every init/uninstall function. -/// -/// Replaces ad-hoc `verbose: u8, dry_run: bool` parameter pairs to keep -/// signatures compact as more flags are added (mirrors `RunOptions` in -/// `src/core/runner.rs`). -#[derive(Clone, Copy, Default)] -pub struct InitContext { - pub verbose: u8, - pub dry_run: bool, - /// `awareness.level` from config.toml. Agents with a command hook receive the matching - /// awareness file; agents without a hook always receive `full` (see `print_rules_only_awareness_note`). - pub awareness: AwarenessLevel, -} - -fn awareness_content(level: AwarenessLevel) -> &'static str { - match level { - AwarenessLevel::Default => RTK_AWARENESS_DEFAULT, - AwarenessLevel::High => RTK_AWARENESS_HIGH, - AwarenessLevel::Full => RTK_AWARENESS_FULL, - } -} - -/// The line that says an `RTK.md` is RTK's to rewrite and to remove. -/// -/// `--codex` is the one mode whose `RTK.md` sits at the project root, next to the user's own -/// files and under a name RTK does not own, so it has to be able to tell the two apart. It -/// says so in the file rather than by comparing content: the payload changes between releases, -/// and a check against the running build's copy stops recognising RTK's own file the moment it -/// does -- leaving it orphaned on uninstall and backed up on every upgrade. -const RTK_MD_OWNED_MARKER: &str = ""; - -/// The `RTK.md` the Codex mode writes: RTK's ownership line, then the awareness payload. -fn codex_rtk_md_content(level: AwarenessLevel) -> String { - format!("{RTK_MD_OWNED_HEADER}\n\n{}", awareness_content(level)) -} - -/// The heading every `--codex` release up to v0.48.0 wrote at the top of the `RTK.md` it -/// owned, before the ownership line existed. Nothing writes it any more, so it is frozen, and -/// it names both RTK and the mode, so a file starting with it is RTK's. The headings that -/// replaced it (`# RTK`, `# Command output`) are left out on purpose: a user's own notes may -/// legitimately open with either, and taking their file would be the worse mistake. -const RTK_MD_LEGACY_CODEX_HEADING: &str = "# RTK - Rust Token Killer (Codex CLI)"; - -/// SHA-256 of the three awareness payloads RTK wrote as `RTK.md` between the legacy heading -/// and the ownership line -- v0.49.0 and the builds after it, which wrote the shared text with -/// nothing on top claiming it. Their headings (`# RTK`, `# Command output`) name neither RTK -/// nor the mode and a user's notes may open with either, so only the whole file identifies -/// them. Digests rather than the payload constants: these name bytes that shipped, and -/// rewording the awareness text today must not change which files uninstall recognises. -const RTK_MD_UNMARKED_PAYLOAD_DIGESTS: [&str; 3] = [ - "dc37dc6afdf513200c2aae1931496e433d323f313877a49b0d5ba11992c33ac7", - "d124b2926b0cd506680f785ab98ef64c211854549da4a85ec544fb803aaea812", - "278274ef3d08c858d4247cc91419c4d74ef922b95719e987b22e896aef10e1fc", -]; - -/// The digest [`RTK_MD_UNMARKED_PAYLOAD_DIGESTS`] is compared against. Carriage returns are -/// dropped first: a checkout or an editor may have rewritten the line endings of a file RTK -/// wrote, which does not make it the user's. -fn rtk_md_payload_digest(content: &str) -> String { - use sha2::{Digest, Sha256}; - - let mut hasher = Sha256::new(); - hasher.update(content.replace('\r', "").as_bytes()); - format!("{:x}", hasher.finalize()) -} - -/// Whether `content` is an RTK.md that RTK itself wrote. -/// -/// The ownership line and the legacy heading are read on the first non-blank line only: RTK -/// writes its claim at the top of a file it wrote whole, and a user who pasted either -- or -/// an RTK-written block -- somewhere inside their own notes has not handed the rest of the -/// file over with it. A payload from the releases that claimed nothing is matched whole. -fn is_rtk_authored_md(content: &str) -> bool { - // A BOM is what an editor adds, not a change of authorship, and Windows editors add one. - let content = strip_leading_bom(content); - let claimed = content - .lines() - .map(str::trim) - .find(|line| !line.is_empty()) - .is_some_and(|line| { - line.starts_with(RTK_MD_OWNED_MARKER) || line == RTK_MD_LEGACY_CODEX_HEADING - }); - - claimed || RTK_MD_UNMARKED_PAYLOAD_DIGESTS.contains(&rtk_md_payload_digest(content).as_str()) -} - -/// [`is_rtk_authored_md`] for a path. An unreadable file is not provably RTK's, so it is -/// treated as the user's and left alone. -fn rtk_md_is_rtk_authored(path: &Path) -> bool { - fs::read_to_string(path).is_ok_and(|content| is_rtk_authored_md(&content)) -} - -/// Which directory a Codex `RTK.md` sits in, which is what decides who owns it. -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -enum RtkMdScope { - /// `--global`: the Codex home. RTK created the file there, nothing else claims the name, - /// and every release before the ownership marker wrote it there unmarked -- reading the - /// marker in this scope would strand all of those files on uninstall. - CodexHome, - /// Project scope: the project root, shared with the user's own files. Only the marker - /// tells RTK's `RTK.md` from one the user wrote under the same name. - ProjectRoot, -} - -impl RtkMdScope { - fn for_global(global: bool) -> Self { - if global { - Self::CodexHome - } else { - Self::ProjectRoot - } - } - - /// Whether RTK may overwrite or remove the `RTK.md` at `path` without asking. - fn owns(self, path: &Path) -> bool { - match self { - Self::CodexHome => true, - Self::ProjectRoot => rtk_md_is_rtk_authored(path), - } - } -} - -/// How many numbered backups may sit beside a file before RTK refuses to make another. -/// -/// High enough that no ordinary sequence of installs reaches it, and bounded so a file that -/// keeps producing distinct backups cannot quietly fill its directory: at the ceiling RTK -/// stops and says so rather than choosing one to overwrite. -const MAX_BACKUP_ATTEMPTS: usize = 99; - -/// A slot to preserve a file's current content in, and what is known about that content. -/// -/// Reading the source is only ever needed to answer "is this already preserved?", which is -/// why the unreadable case is its own arm rather than a refusal: a caller that copies cannot -/// proceed without that answer, while a caller that renames never needed it, since a rename -/// carries the content whether or not RTK can read it. -#[derive(Debug)] -enum BackupSlot { - /// Free, and the source differs from every backup already present. - Free(PathBuf), - /// Identical content already sits here, so a copy would be redundant. A rename still has - /// the original to move, and moving it here is what discards the duplicate. - AlreadyPreserved(PathBuf), - /// Free, but the source could not be read, so whether it is already preserved is unknown. - SourceUnreadable(PathBuf), -} - -impl BackupSlot { - /// The slot to move the original onto, for a caller that preserves content by renaming. - /// - /// Every arm gives the same answer: a rename carries content RTK could not read, and - /// renaming onto a byte-identical backup discards the duplicate instead of burning a slot. - fn for_rename(self) -> PathBuf { - match self { - BackupSlot::Free(path) - | BackupSlot::AlreadyPreserved(path) - | BackupSlot::SourceUnreadable(path) => path, - } - } -} - -/// `path.bak` for the first slot, `path.bak.N` after that. -/// -/// Built by appending to the `OsString` rather than through `with_extension`, which replaces -/// an extension instead of extending it, and rather than through `display()`, which is lossy: -/// on a path that is not valid UTF-8 the probe and the write would disagree. -fn numbered_backup_path(path: &Path, attempt: usize) -> PathBuf { - let mut name = path.as_os_str().to_os_string(); - name.push(".bak"); - if attempt > 0 { - name.push(format!(".{attempt}")); - } - PathBuf::from(name) -} - -/// Pick a `.bak` sibling for `path`, numbered when earlier backups are still there so a second -/// run cannot overwrite the first one's rescue copy. -fn free_backup_slot(path: &Path) -> Result { - let source = fs::read(path).ok(); - for attempt in 0..=MAX_BACKUP_ATTEMPTS { - let candidate = numbered_backup_path(path, attempt); - match fs::read(&candidate) { - // A provisioning loop that reapplies the same local change would otherwise - // consume a slot on every run. - Ok(existing) if source.as_deref() == Some(existing.as_slice()) => { - return Ok(BackupSlot::AlreadyPreserved(candidate)); - } - Ok(_) => continue, - Err(error) if error.kind() == std::io::ErrorKind::NotFound => { - return Ok(match source { - Some(_) => BackupSlot::Free(candidate), - None => BackupSlot::SourceUnreadable(candidate), - }); - } - // Occupied by something unreadable -- a directory, or a file this user cannot - // read. Its content cannot be compared, so treat the slot as taken rather than - // overwriting it. - Err(_) => continue, - } - } - - anyhow::bail!( - "Cannot back up {}: {} existing backups are already present. \ - Remove or archive them first.", - path.display(), - MAX_BACKUP_ATTEMPTS + 1 - ) -} - -/// Agents without a command hook must prefix `rtk` themselves, which only the `full` level -/// teaches, so they always receive `RTK_AWARENESS_FULL`. This prints the one-line note that -/// tells the user why their configured `awareness.level` was not applied. -fn print_rules_only_awareness_note(agent: &str, ctx: InitContext) { - let prefix = if ctx.dry_run { "[dry-run] " } else { " " }; - let ignored = if ctx.awareness == AwarenessLevel::Full { - String::new() - } else { - format!( - " (config awareness.level = \"{}\" does not apply here)", - ctx.awareness - ) - }; - println!( - "{prefix}Awareness: full — {agent} has no command hook, so the agent is told to prefix rtk itself{ignored}" - ); -} - -/// Wrap an awareness file in the `` markers used by -/// `write_rtk_block`, so it can be upserted into a shared file like AGENTS.md. -fn rtk_block(body: &str) -> String { - format!("{RTK_BLOCK_START} {RTK_BLOCK_VERSION} -->\n{body}{RTK_BLOCK_END}\n") -} - -/// Shared dry-run footer printed at the end of every init sub-mode. -fn print_dry_run_footer() { - println!("\n[dry-run] Nothing written."); -} - -// Legacy full instructions for backward compatibility (--claude-md mode) -const RTK_INSTRUCTIONS: &str = r##" -# RTK (Rust Token Killer) - Token-Optimized Commands - -## Golden Rule - -**Always prefix commands with `rtk`**. If RTK has a dedicated filter, it uses it. If not, it passes through unchanged. This means RTK is always safe to use. - -**Important**: Even in command chains with `&&`, use `rtk`: -```bash -# ❌ Wrong -git add . && git commit -m "msg" && git push - -# ✅ Correct -rtk git add . && rtk git commit -m "msg" && rtk git push -``` - -## RTK Commands by Workflow - -### Build & Compile (80-90% savings) -```bash -rtk cargo build # Cargo build output -rtk cargo check # Cargo check output -rtk cargo clippy # Clippy warnings grouped by file (80%) -rtk tsc # TypeScript errors grouped by file/code (83%) -rtk lint # ESLint/Biome violations grouped (84%) -rtk prettier --check # Files needing format only (70%) -rtk next build # Next.js build with route metrics (87%) -``` - -### Test (60-99% savings) -```bash -rtk cargo test # Cargo test failures only (90%) -rtk go test # Go test failures only (90%) -rtk jest # Jest failures only (99.5%) -rtk vitest # Vitest failures only (99.5%) -rtk playwright test # Playwright failures only (94%) -rtk pytest # Python test failures only (90%) -rtk rake test # Ruby test failures only (90%) -rtk rspec # RSpec test failures only (60%) -rtk test # Generic test wrapper - failures only -``` - -### Git (59-80% savings) -```bash -rtk git status # Compact status -rtk git log # Compact log (works with all git flags) -rtk git diff # Compact diff (80%) -rtk git show # Compact show (80%) -rtk git add # Ultra-compact confirmations (59%) -rtk git commit # Ultra-compact confirmations (59%) -rtk git push # Ultra-compact confirmations -rtk git pull # Ultra-compact confirmations -rtk git branch # Compact branch list -rtk git fetch # Compact fetch -rtk git stash # Compact stash -rtk git worktree # Compact worktree -``` - -Note: Git passthrough works for ALL subcommands, even those not explicitly listed. - -### GitHub (26-87% savings) -```bash -rtk gh pr view # Compact PR view (87%) -rtk gh pr checks # Compact PR checks (79%) -rtk gh run list # Compact workflow runs (82%) -rtk gh issue list # Compact issue list (80%) -rtk gh api # Compact API responses (26%) -``` - -### JavaScript/TypeScript Tooling (70-90% savings) -```bash -rtk pnpm list # Compact dependency tree (70%) -rtk pnpm outdated # Compact outdated packages (80%) -rtk pnpm install # Compact install output (90%) -rtk npm run