From 0fcd6ad670597451194e97777d065009675f57a1 Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Sat, 25 Jul 2026 23:51:00 +0200 Subject: [PATCH 01/22] fix(hooks): stop Copilot from ever asserting a permissionDecision on AskRewrite handle_vscode (rtk hook copilot's PascalCase path, shared by VS Code Copilot Chat and Copilot CLI's Claude-compat entry) always set permissionDecision to "allow" or "ask", diverging from process_claude_payload which only asserts "allow" for an explicit user-configured Allow rule and stays silent otherwise. Asserting "ask" is what caused #3037: Copilot CLI 1.0.66+ treats it as authoritative and forces a blocking dialog with no "remember" option on every rewritten command. 84aa4d6/0df6929 patched the wrong path (the camelCase native handler, which never had this problem) with an auto-allow heuristic gated on an `explicit` flag, instead of fixing handle_vscode itself. Replace both with the same rule Claude's own hook and Droid's hook already use: never assert a decision for AskRewrite, regardless of whether the verdict was Default or an explicit Ask rule. This removes the Copilot-specific heuristic (and the now-unused `explicit` field) in favor of one behavior shared across all hosts. Also extracts vscode_response_from_decision as a pure, testable function (mirroring copilot_cli_response_from_decision), closing a prior gap where handle_vscode's decision output had no direct unit test coverage. --- src/hooks/hook_cmd.rs | 156 +++++++++++++++++++++++++++--------------- 1 file changed, 100 insertions(+), 56 deletions(-) diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index 29812c79b9..43fb673515 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -138,7 +138,7 @@ fn get_rewritten(cmd: &str) -> Option { enum HookDecision { AllowRewrite(String), - AskRewrite { rewritten: String, explicit: bool }, + AskRewrite(String), Defer, Deny, } @@ -152,10 +152,7 @@ fn decide_from_verdict(cmd: &str, verdict: PermissionVerdict) -> HookDecision { } match get_rewritten(cmd) { Some(r) if verdict == PermissionVerdict::Allow => HookDecision::AllowRewrite(r), - Some(r) => HookDecision::AskRewrite { - rewritten: r, - explicit: verdict == PermissionVerdict::Ask, - }, + Some(r) => HookDecision::AskRewrite(r), None => HookDecision::Defer, } } @@ -165,28 +162,46 @@ fn decide_hook_action(cmd: &str, host: permissions::Host) -> HookDecision { } fn handle_vscode(cmd: &str) -> Result<()> { - let (decision, rewritten) = match decide_hook_action(cmd, permissions::Host::Claude) { + if let Some(output) = vscode_response(cmd) { + let _ = writeln!(io::stdout(), "{output}"); + } + Ok(()) +} + +fn vscode_response(cmd: &str) -> Option { + vscode_response_from_decision(decide_hook_action(cmd, permissions::Host::Claude), cmd) +} + +/// Build the VS Code Copilot Chat / Copilot CLI (PascalCase compat) hook response. +/// +/// Mirrors `process_claude_payload`: `permissionDecision: "allow"` is only ever +/// asserted for an explicit, user-configured Allow rule. Every other rewrite +/// (Default verdict or an explicit Ask rule) omits the field entirely, leaving +/// 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 { + let (rewritten, allow) = match decision { HookDecision::Deny => { audit_log("deny", cmd, ""); - return Ok(()); + return None; } - HookDecision::Defer => return Ok(()), - HookDecision::AllowRewrite(r) => ("allow", r), - HookDecision::AskRewrite { rewritten: r, .. } => ("ask", r), + HookDecision::Defer => return None, + HookDecision::AllowRewrite(r) => (r, true), + HookDecision::AskRewrite(r) => (r, false), }; audit_log("rewrite", cmd, &rewritten); - let output = json!({ - "hookSpecificOutput": { - "hookEventName": PRE_TOOL_USE_KEY, - "permissionDecision": decision, - "permissionDecisionReason": "RTK auto-rewrite", - "updatedInput": { "command": rewritten } - } + let mut hook_output = json!({ + "hookEventName": PRE_TOOL_USE_KEY, + "permissionDecisionReason": "RTK auto-rewrite", + "updatedInput": { "command": rewritten } }); - let _ = writeln!(io::stdout(), "{output}"); - Ok(()) + if allow { + hook_output["permissionDecision"] = json!("allow"); + } + Some(json!({ "hookSpecificOutput": hook_output })) } fn handle_copilot_cli(cmd: &str, args: &Value) -> Result<()> { @@ -244,13 +259,7 @@ fn copilot_cli_response_from_decision( } HookDecision::Defer => return None, HookDecision::AllowRewrite(r) => (r, true), - HookDecision::AskRewrite { - rewritten: r, - explicit, - } => { - let is_simple = crate::discover::lexer::split_for_permissions(cmd).len() <= 1; - (r, !explicit && is_simple) - } + HookDecision::AskRewrite(r) => (r, false), }; audit_log("rewrite", cmd, &rewritten); @@ -306,7 +315,7 @@ pub fn run_gemini() -> Result<()> { audit_log("rewrite", cmd, rewritten); print_gemini("allow", Some(rewritten)); } - HookDecision::AskRewrite { ref rewritten, .. } => { + HookDecision::AskRewrite(ref rewritten) => { audit_log("ask", cmd, rewritten); print_gemini("ask_user", Some(rewritten)); } @@ -411,7 +420,7 @@ fn process_claude_payload(v: &Value) -> PayloadAction { } } HookDecision::AllowRewrite(r) => (r, true), - HookDecision::AskRewrite { rewritten: r, .. } => (r, false), + HookDecision::AskRewrite(r) => (r, false), }; let updated_input = { @@ -535,7 +544,7 @@ pub fn run_cursor() -> Result<()> { audit_log("rewrite", &cmd, &rewritten); cursor_allow(&rewritten) } - HookDecision::AskRewrite { rewritten, .. } => { + HookDecision::AskRewrite(rewritten) => { audit_log("ask", &cmd, &rewritten); cursor_ask(&rewritten) } @@ -598,7 +607,7 @@ fn run_cursor_inner_with_rules( let verdict = permissions::check_command_with_rules(&cmd, deny_rules, ask_rules, allow_rules); match decide_from_verdict(&cmd, verdict) { HookDecision::AllowRewrite(rewritten) => cursor_allow(&rewritten), - HookDecision::AskRewrite { rewritten, .. } => cursor_ask(&rewritten), + HookDecision::AskRewrite(rewritten) => cursor_ask(&rewritten), _ => "{}".to_string(), } } @@ -644,7 +653,7 @@ fn droid_response_from_decision(v: &Value, cmd: &str, decision: HookDecision) -> return None; } HookDecision::Defer => return None, - HookDecision::AllowRewrite(r) | HookDecision::AskRewrite { rewritten: r, .. } => r, + HookDecision::AllowRewrite(r) | HookDecision::AskRewrite(r) => r, }; audit_log("rewrite", cmd, &rewritten); @@ -834,44 +843,82 @@ mod tests { assert!(get_rewritten("cat <<'EOF'\nhello\nEOF").is_none()); } - // --- Copilot CLI handler: transparent rewrite via modifiedArgs --- + // --- VS Code Copilot Chat / Copilot CLI (PascalCase) handler --- + // Serves both VS Code Copilot Chat's PreToolUse hook and Copilot CLI's + // PascalCase-compat entry (#3037): the same `rtk hook copilot` call + // answers both from one JSON schema. - fn cli_args(cmd: &str) -> Value { - json!({ "command": cmd }) + #[test] + fn test_vscode_allow_rewrite_sets_permission_allow() { + let r = vscode_response_from_decision( + HookDecision::AllowRewrite("rtk git status".into()), + "git status", + ) + .unwrap(); + assert_eq!(r["hookSpecificOutput"]["permissionDecision"], "allow"); + assert_eq!( + r["hookSpecificOutput"]["updatedInput"]["command"], + "rtk git status" + ); } #[test] - fn test_copilot_cli_default_ask_rewrite_sets_permission_allow() { - let r = copilot_cli_response_from_decision( - &cli_args("cargo test"), - HookDecision::AskRewrite { - rewritten: "rtk cargo test".into(), - explicit: false, - }, + fn test_vscode_ask_rewrite_omits_permission_decision() { + // Default (unconfigured) and explicit-Ask verdicts both land here as + // AskRewrite — neither must assert a decision, matching Claude's own + // hook (process_claude_payload). Asserting "ask" is what caused #3037: + // Copilot CLI 1.0.66+ treats it as authoritative and forces a blocking + // dialog with no "remember" option on every rewritten command. + let r = vscode_response_from_decision( + HookDecision::AskRewrite("rtk cargo test".into()), "cargo test", ) .unwrap(); + assert!( + r["hookSpecificOutput"] + .as_object() + .unwrap() + .get("permissionDecision") + .is_none(), + "AskRewrite must NOT set permissionDecision" + ); assert_eq!( - r["permissionDecision"], "allow", - "Default AskRewrite must set permissionDecision to allow — Copilot CLI 1.0.66+ prompts on every command without it" + r["hookSpecificOutput"]["updatedInput"]["command"], + "rtk cargo test" ); - assert_eq!(r["modifiedArgs"]["command"], "rtk cargo test"); } #[test] - fn test_copilot_cli_explicit_ask_rewrite_omits_permission_decision() { + fn test_vscode_deny_returns_none() { + assert!(vscode_response_from_decision(HookDecision::Deny, "cargo test").is_none()); + } + + #[test] + fn test_vscode_defer_returns_none() { + assert!(vscode_response_from_decision(HookDecision::Defer, "cargo test").is_none()); + } + + // --- Copilot CLI handler: transparent rewrite via modifiedArgs --- + + fn cli_args(cmd: &str) -> Value { + json!({ "command": cmd }) + } + + #[test] + fn test_copilot_cli_ask_rewrite_omits_permission_decision() { + // Whether the Ask verdict came from an explicit rule or the Default + // (unconfigured) fallback, RTK must never assert a decision here — + // matches Claude's own hook (process_claude_payload) and avoids the + // Copilot CLI 1.0.66+ forced-prompt bug from #3037. let r = copilot_cli_response_from_decision( &cli_args("cargo test"), - HookDecision::AskRewrite { - rewritten: "rtk cargo test".into(), - explicit: true, - }, + HookDecision::AskRewrite("rtk cargo test".into()), "cargo test", ) .unwrap(); assert!( r.get("permissionDecision").is_none(), - "Explicit AskRewrite must NOT auto-allow — user deliberately configured ask for this command" + "AskRewrite must NOT set permissionDecision — the host's native prompt/allowlist stays in control" ); assert_eq!(r["modifiedArgs"]["command"], "rtk cargo test"); } @@ -1000,10 +1047,7 @@ mod tests { }); let r = copilot_cli_response_from_decision( &args, - HookDecision::AskRewrite { - rewritten: "rtk cargo install ripgrep".into(), - explicit: false, - }, + HookDecision::AskRewrite("rtk cargo install ripgrep".into()), "cargo install ripgrep", ) .unwrap(); @@ -1577,7 +1621,7 @@ mod tests { fn test_decide_ask_for_default_verdict() { assert!(matches!( decide_with_rules("git status", &[], &[], &[]), - HookDecision::AskRewrite { .. } + HookDecision::AskRewrite(_) )); } @@ -1635,7 +1679,7 @@ mod tests { r#"{"decision":"deny","reason":"Blocked by RTK permission rule"}"#.to_string() } HookDecision::AllowRewrite(r) => gemini_json("allow", Some(&r)), - HookDecision::AskRewrite { rewritten: r, .. } => gemini_json("ask_user", Some(&r)), + HookDecision::AskRewrite(r) => gemini_json("ask_user", Some(&r)), HookDecision::Defer => gemini_json("ask_user", None), } } From b754b850098fcbcf89c3d31be025943d69e5dfca Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Sun, 26 Jul 2026 02:18:12 +0200 Subject: [PATCH 02/22] fix(hooks): drop redundant camelCase preToolUse entry from Copilot hook config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rtk init --copilot registered both a PascalCase PreToolUse entry and a camelCase preToolUse entry in the same rtk-rewrite.json, on the assumption that VS Code Copilot Chat needs the former and Copilot CLI needs the latter. Live testing showed Copilot CLI treats PreToolUse/preToolUse as two independent, sequentially-run hooks — a redundant second `rtk hook copilot` process spawn per tool call, chaining the first hook's rewrite into the second's input (confirmed via raw stdin capture, and confirmed independent of declaration order in the file). Also confirmed Copilot CLI honors the PascalCase-only schema perfectly well on its own, receiving the same tool_name/tool_input.command shape either way — so the camelCase entry buys nothing for Copilot CLI, while adding process overhead and an extra, harder-to-reason-about execution path. Drop the camelCase preToolUse entry, keeping the single PascalCase PreToolUse entry shared by both hosts. Existing installs are not upgraded automatically — re-running `rtk init --copilot` / `rtk init --global --copilot` overwrites the old dual-schema file with the new one (write_if_changed overwrites unconditionally on content diff), verified by test_copilot_init_upgrades_old_dual_schema_install and test_copilot_global_install_upgrades_old_dual_schema_install, which seed the old dual-schema content and assert it gets replaced. --- docs/contributing/TECHNICAL.md | 4 +- .../guide/getting-started/supported-agents.md | 6 +- src/hooks/init.rs | 105 ++++++++++++++---- 3 files changed, 91 insertions(+), 24 deletions(-) diff --git a/docs/contributing/TECHNICAL.md b/docs/contributing/TECHNICAL.md index d9c8f993c0..e465c2563c 100644 --- a/docs/contributing/TECHNICAL.md +++ b/docs/contributing/TECHNICAL.md @@ -314,7 +314,7 @@ Start here, then drill down into each README for file-level details. |-----------|-------|-------------------------------| | [`hooks/`](../hooks/README.md) | _(parent)_ | **All JSON formats**, rewrite registry overview, exit code contract, override controls | | [`claude/`](../hooks/claude/README.md) | Claude Code | Shell hook mechanism, `PreToolUse` JSON, test script | -| [`copilot/`](../hooks/copilot/README.md) | GitHub Copilot | Rust binary hook, VS Code Chat vs Copilot CLI dual format | +| [`copilot/`](../hooks/copilot/README.md) | GitHub Copilot | Rust binary hook, single `PreToolUse` schema shared by VS Code Chat and Copilot CLI | | [`cursor/`](../hooks/cursor/README.md) | Cursor IDE | Shell hook, empty JSON response requirement | | [`cline/`](../hooks/cline/README.md) | Cline / Roo Code | Rules file (prompt-level, no programmatic hook) | | [`windsurf/`](../hooks/windsurf/README.md) | Windsurf / Cascade | Rules file (workspace-scoped) | @@ -331,7 +331,7 @@ RTK supports the following LLM agents through hook integrations: |-------|-----------|-----------|---------------------| | Claude Code | Shell hook | `PreToolUse` in `settings.json` | Yes (`updatedInput`) | | GitHub Copilot (VS Code) | Rust binary | `rtk hook copilot` reads JSON | Yes (`updatedInput`) | -| GitHub Copilot CLI | Rust binary | `rtk hook copilot` reads JSON | No (deny + suggestion) | +| GitHub Copilot CLI | Rust binary | `rtk hook copilot` reads JSON | Yes (`updatedInput`) | | Cursor | Rust binary | `rtk hook cursor` reads JSON | Yes (`updated_input`) | | Gemini CLI | Rust binary | `rtk hook gemini` reads JSON | Yes (`hookSpecificOutput`) | | Cline/Roo Code | Rules file | Prompt-level guidance | N/A (prompt) | diff --git a/docs/guide/getting-started/supported-agents.md b/docs/guide/getting-started/supported-agents.md index adb0e8bd17..3906e364cb 100644 --- a/docs/guide/getting-started/supported-agents.md +++ b/docs/guide/getting-started/supported-agents.md @@ -30,7 +30,7 @@ Agent runs "cargo test" |-------|-----------------|---------------------------| | Claude Code | Shell hook (`PreToolUse`) | Yes | | VS Code Copilot Chat | Shell hook (`PreToolUse`) | Yes | -| GitHub Copilot CLI | Shell hook (`preToolUse` `modifiedArgs`) | Yes | +| GitHub Copilot CLI | Shell hook (`PreToolUse`) | Yes | | Cursor | Shell hook (`preToolUse`) | Yes | | Gemini CLI | Rust binary (`BeforeTool`) | Yes | | OpenCode | TypeScript plugin (`tool.execute.before`) | Yes | @@ -74,7 +74,9 @@ rtk init --copilot # project-scoped (.github/hooks/) rtk init --global --copilot # user-scoped (~/.copilot/hooks/, respects $COPILOT_HOME) ``` -Project-scoped writes `.github/hooks/rtk-rewrite.json` (both hosts get transparent rewrite — VS Code Chat via `updatedInput`, Copilot CLI via `modifiedArgs`) plus the RTK block in `.github/copilot-instructions.md`. User-scoped writes the same hook config to `~/.copilot/hooks/rtk-rewrite.json` and the RTK block to `~/.copilot/copilot-instructions.md` (both respect `$COPILOT_HOME` if set). +Project-scoped writes `.github/hooks/rtk-rewrite.json` — a single `PreToolUse` entry shared by both hosts, each getting transparent rewrite via `updatedInput` — plus the RTK block in `.github/copilot-instructions.md`. User-scoped writes the same hook config to `~/.copilot/hooks/rtk-rewrite.json` and the RTK block to `~/.copilot/copilot-instructions.md` (both respect `$COPILOT_HOME` if set). + +Earlier `rtk` versions also registered a second, camelCase `preToolUse` entry for Copilot CLI's native schema. Copilot CLI treats `PreToolUse`/`preToolUse` as independent hooks and runs both sequentially for the same tool call — a redundant process spawn with no behavioral benefit, since Copilot CLI honors the single `PreToolUse` schema on its own. Re-run `rtk init --copilot` (or `--global --copilot`) to upgrade an existing install to the single-hook config. Uninstall: diff --git a/src/hooks/init.rs b/src/hooks/init.rs index b71c6288c7..439dbfefa6 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -4448,7 +4448,14 @@ fn uninstall_gemini(ctx: InitContext) -> Result> { // ── Copilot integration ───────────────────────────────────── -// PreToolUse = VS Code schema, preToolUse = Copilot CLI schema (same file, both hosts). +// Single PascalCase `PreToolUse` entry, shared by VS Code Copilot Chat and +// Copilot CLI. Previously this file also declared a camelCase `preToolUse` +// entry for Copilot CLI's native schema, but Copilot CLI registers BOTH keys +// as independent hooks and runs them sequentially, chaining the camelCase +// hook's rewrite into the PascalCase hook's input — a redundant second +// process spawn per tool call for no behavioral benefit (confirmed live: +// Copilot CLI honors the PascalCase-only schema on its own, receiving the +// same `tool_name`/`tool_input.command` shape either way). const COPILOT_HOOK_JSON: &str = r#"{ "version": 1, "hooks": { @@ -4459,15 +4466,6 @@ const COPILOT_HOOK_JSON: &str = r#"{ "cwd": ".", "timeout": 5 } - ], - "preToolUse": [ - { - "type": "command", - "bash": "rtk hook copilot", - "powershell": "rtk hook copilot", - "cwd": ".", - "timeoutSec": 5 - } ] } } @@ -7534,25 +7532,23 @@ mod tests { } #[test] - fn test_copilot_hook_json_serves_both_vscode_and_cli_schemas() { + fn test_copilot_hook_json_serves_single_pascalcase_schema() { let v: serde_json::Value = serde_json::from_str(COPILOT_HOOK_JSON).unwrap(); let vscode = &v["hooks"]["PreToolUse"][0]; assert_eq!(vscode["command"], "rtk hook copilot"); assert!(vscode["timeout"].is_number(), "VS Code uses `timeout`"); + assert_eq!(v["version"], 1); - assert_eq!(v["version"], 1, "Copilot CLI requires top-level version"); - let cli = &v["hooks"]["preToolUse"][0]; - assert_eq!(cli["bash"], "rtk hook copilot"); - assert_eq!(cli["powershell"], "rtk hook copilot"); assert!( - cli["timeoutSec"].is_number(), - "Copilot CLI uses `timeoutSec`" + v["hooks"].get("preToolUse").is_none(), + "must not register a second, redundant camelCase hook — Copilot CLI treats \ + PreToolUse and preToolUse as independent hooks and runs both sequentially" ); } #[test] - fn test_copilot_init_writes_dual_schema_to_disk() { + fn test_copilot_init_writes_single_schema_to_disk() { let temp = TempDir::new().unwrap(); run_copilot_at(temp.path(), InitContext::default()).unwrap(); @@ -7566,7 +7562,44 @@ mod tests { assert_eq!(v["hooks"]["PreToolUse"][0]["command"], "rtk hook copilot"); assert_eq!(v["version"], 1); - assert_eq!(v["hooks"]["preToolUse"][0]["bash"], "rtk hook copilot"); + assert!(v["hooks"].get("preToolUse").is_none()); + } + + #[test] + fn test_copilot_init_upgrades_old_dual_schema_install() { + // Simulates a pre-existing install from before this fix, which wrote + // both a PascalCase PreToolUse and a camelCase preToolUse entry. + // Re-running `rtk init --copilot` must overwrite it with the current + // single-schema config, not leave the stale camelCase entry in place. + let old_dual_schema_json = r#"{ + "version": 1, + "hooks": { + "PreToolUse": [ + { "type": "command", "command": "rtk hook copilot", "cwd": ".", "timeout": 5 } + ], + "preToolUse": [ + { "type": "command", "bash": "rtk hook copilot", "powershell": "rtk hook copilot", "cwd": ".", "timeoutSec": 5 } + ] + } +} +"#; + + let temp = TempDir::new().unwrap(); + let hooks_dir = temp.path().join(".github").join("hooks"); + fs::create_dir_all(&hooks_dir).unwrap(); + let hook_path = hooks_dir.join("rtk-rewrite.json"); + fs::write(&hook_path, old_dual_schema_json).unwrap(); + + run_copilot_at(temp.path(), InitContext::default()).unwrap(); + + let v: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&hook_path).unwrap()).unwrap(); + assert_eq!(v["hooks"]["PreToolUse"][0]["command"], "rtk hook copilot"); + assert!( + v["hooks"].get("preToolUse").is_none(), + "re-running init must upgrade an old dual-schema install, dropping the \ + redundant camelCase preToolUse entry" + ); } #[test] @@ -7694,7 +7727,39 @@ mod tests { serde_json::from_str(&fs::read_to_string(&hook_path).unwrap()).unwrap(); assert_eq!(v["version"], 1); assert_eq!(v["hooks"]["PreToolUse"][0]["command"], "rtk hook copilot"); - assert_eq!(v["hooks"]["preToolUse"][0]["bash"], "rtk hook copilot"); + assert!(v["hooks"].get("preToolUse").is_none()); + } + + #[test] + fn test_copilot_global_install_upgrades_old_dual_schema_install() { + let old_dual_schema_json = r#"{ + "version": 1, + "hooks": { + "PreToolUse": [ + { "type": "command", "command": "rtk hook copilot", "cwd": ".", "timeout": 5 } + ], + "preToolUse": [ + { "type": "command", "bash": "rtk hook copilot", "powershell": "rtk hook copilot", "cwd": ".", "timeoutSec": 5 } + ] + } +} +"#; + + let temp = TempDir::new().unwrap(); + let hooks_dir = temp.path().join("hooks"); + fs::create_dir_all(&hooks_dir).unwrap(); + let hook_path = hooks_dir.join("rtk-rewrite.json"); + fs::write(&hook_path, old_dual_schema_json).unwrap(); + + run_copilot_global_at(temp.path(), InitContext::default()).unwrap(); + + let v: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&hook_path).unwrap()).unwrap(); + assert_eq!(v["hooks"]["PreToolUse"][0]["command"], "rtk hook copilot"); + assert!( + v["hooks"].get("preToolUse").is_none(), + "re-running global init must upgrade an old dual-schema install" + ); } #[test] From 1ebc2710d1d024420d2460693d020bbe50911564 Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Sun, 26 Jul 2026 02:25:05 +0200 Subject: [PATCH 03/22] fix(hooks): recognize VS Code Copilot Chat's run_in_terminal tool name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit detect_format only matched tool_name values "runTerminalCommand", "Bash", and "bash" for the VsCode hook format. Live payload capture from a real VS Code Copilot Chat session (agent mode, GitHub.copilot-chat) showed it actually sends "run_in_terminal" — none of the previously recognized values — so detect_format fell through to PassThrough and the hook never fired at all for VS Code Copilot Chat: no rewrite, no permissionDecision, nothing. Add "run_in_terminal" to the recognized tool_name set. Verified against the real captured payload end-to-end: a compound "cd && git status --short --branch" command now correctly rewrites only the git segment to "rtk git status --short --branch", leaving "cd" untouched, with no permissionDecision asserted (consistent with the Default-verdict behavior fixed in 042aeaf). --- src/hooks/hook_cmd.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index 43fb673515..33859405d6 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -71,9 +71,15 @@ pub fn run_copilot() -> Result<()> { } fn detect_format(v: &Value) -> HookFormat { - // VS Code Copilot Chat / Claude Code: snake_case keys + // VS Code Copilot Chat / Claude Code: snake_case keys. + // "run_in_terminal" is VS Code Copilot Chat's actual terminal tool name + // (confirmed via live payload capture) — without it, detect_format falls + // through to PassThrough and the hook never fires for VS Code Copilot Chat. if let Some(tool_name) = v.get("tool_name").and_then(|t| t.as_str()) { - if matches!(tool_name, "runTerminalCommand" | "Bash" | "bash") { + if matches!( + tool_name, + "runTerminalCommand" | "run_in_terminal" | "Bash" | "bash" + ) { if let Some(cmd) = v .pointer("/tool_input/command") .and_then(|c| c.as_str()) @@ -765,6 +771,16 @@ mod tests { )); } + #[test] + fn test_detect_vscode_run_in_terminal() { + // VS Code Copilot Chat's actual terminal tool name, confirmed via + // live payload capture — distinct from "runTerminalCommand". + assert!(matches!( + detect_format(&vscode_input("run_in_terminal", "cargo test")), + HookFormat::VsCode { .. } + )); + } + #[test] fn test_detect_copilot_cli_bash() { assert!(matches!( From 1253d151b3d95f2f7bb36bac3668a36786639f1c Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Sun, 26 Jul 2026 02:48:03 +0200 Subject: [PATCH 04/22] docs(hooks): clarify CopilotCli schema is now legacy/host-specific, not generated The doc comments described the camelCase toolName/toolArgs schema as "GitHub Copilot CLI"'s format, which was accurate when rtk init --copilot registered it. Now that only the PascalCase schema is generated, this path is reachable only via not-yet-upgraded installs' leftover registration, or hosts that use this schema under a different toolName value on their own (JetBrains/ IntelliJ's Copilot plugin sends "run_in_terminal", tracked in #2443/#3093). Note both call sites accordingly so the code isn't mistaken for dead weight. --- src/hooks/hook_cmd.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index 33859405d6..b47d83d26b 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -31,7 +31,13 @@ fn read_stdin_limited() -> Result { enum HookFormat { /// VS Code Copilot Chat / Claude Code: `tool_name` + `tool_input.command`, supports `updatedInput`. VsCode { command: String }, - /// GitHub Copilot CLI: camelCase `toolName` + `toolArgs` (JSON string), supports `modifiedArgs` for transparent rewrite. + /// GitHub Copilot CLI's native schema: camelCase `toolName` + `toolArgs` (JSON string), + /// supports `modifiedArgs` for transparent rewrite. `rtk init --copilot` no longer + /// registers this schema (Copilot CLI honors the PascalCase `VsCode` schema on its + /// own — registering both caused a redundant second hook invocation per tool call, + /// see git history). Kept for installs that haven't re-run `rtk init --copilot` since + /// upgrading, and as the schema JetBrains/IntelliJ's Copilot plugin uses under a + /// different `toolName` value (`run_in_terminal`, not `bash` — see #2443/#3093). /// Carries the full parsed `toolArgs` object so we can rewrite `command` while preserving /// host-supplied metadata (description, initial_wait, mode, …) the tool requires. CopilotCli { command: String, args: Value }, @@ -93,8 +99,12 @@ fn detect_format(v: &Value) -> HookFormat { return HookFormat::PassThrough; } - // Copilot CLI: camelCase keys, toolArgs is a JSON-encoded string. + // Copilot's native camelCase schema: toolName + toolArgs (JSON-encoded string). // The shell tool is "bash" on Unix and "powershell" on Windows. + // Only reachable today via a not-yet-upgraded install's leftover camelCase + // preToolUse registration (see the CopilotCli variant doc) or a host that + // registers this schema itself, like JetBrains/IntelliJ's Copilot plugin + // (toolName "run_in_terminal", tracked separately in #2443/#3093). if let Some(tool_name) = v.get("toolName").and_then(|t| t.as_str()) { if matches!(tool_name, "bash" | "powershell" | "run_in_terminal") { if let Some(tool_args_str) = v.get("toolArgs").and_then(|t| t.as_str()) { From 2cb3908eebb8f5d23d2100140acab47fefa049db Mon Sep 17 00:00:00 2001 From: Nicolas Le Cam Date: Mon, 27 Jul 2026 14:09:32 +0200 Subject: [PATCH 05/22] docs(hooks): confirm PascalCase Bash mapping already covers Windows powershell MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Contributor testing on Windows 11 with Copilot CLI 1.0.73 (#3179) confirmed Copilot CLI remaps its native bash/powershell shell tool to tool_name: "Bash" under the PascalCase PreToolUse schema, and that its updatedInput is honored end-to-end there. Since rtk init --copilot now only registers that schema, Windows already works standalone through it — the camelCase toolName "powershell" case (#3178/#3179) becomes legacy-only, relevant solely to un-upgraded installs. Document that on both HookFormat variants. Also fixes a HookDecision::AskRewrite struct-pattern leftover in copilot_ide_response_from_decision (added by #3093, merged into develop after this branch's own struct-to-tuple AskRewrite refactor), surfaced as a compile error by rebasing onto develop to pick up #3179. --- src/hooks/hook_cmd.rs | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index b47d83d26b..864e3bdeb4 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -30,6 +30,10 @@ fn read_stdin_limited() -> Result { /// Format detected from the preToolUse JSON input. enum HookFormat { /// VS Code Copilot Chat / Claude Code: `tool_name` + `tool_input.command`, supports `updatedInput`. + /// If using the PreToolUse pascal case form, Copilot CLI also remaps its native `bash`/`powershell` + /// runtime tool to `tool_name: "Bash"` for this schema and honors its `updatedInput`, live-verified + /// on Linux+Windows 11 with Copilot CLI 1.0.73+ by rewriting a marker command end-to-end + /// see . VsCode { command: String }, /// GitHub Copilot CLI's native schema: camelCase `toolName` + `toolArgs` (JSON string), /// supports `modifiedArgs` for transparent rewrite. `rtk init --copilot` no longer @@ -38,6 +42,10 @@ enum HookFormat { /// see git history). Kept for installs that haven't re-run `rtk init --copilot` since /// upgrading, and as the schema JetBrains/IntelliJ's Copilot plugin uses under a /// different `toolName` value (`run_in_terminal`, not `bash` — see #2443/#3093). + /// On Windows, Copilot CLI reports this schema's `toolName` as the unmapped runtime + /// name `"powershell"` (#3178/#3179) — but since the `VsCode` schema above already + /// works standalone there, that arm is legacy-only: relevant for un-upgraded installs, + /// not exercised by a fresh `rtk init --copilot` on any platform. /// Carries the full parsed `toolArgs` object so we can rewrite `command` while preserving /// host-supplied metadata (description, initial_wait, mode, …) the tool requires. CopilotCli { command: String, args: Value }, @@ -81,6 +89,9 @@ fn detect_format(v: &Value) -> HookFormat { // "run_in_terminal" is VS Code Copilot Chat's actual terminal tool name // (confirmed via live payload capture) — without it, detect_format falls // through to PassThrough and the hook never fires for VS Code Copilot Chat. + // No separate Windows/"powershell" case is needed: Copilot CLI remaps both + // `bash` and `powershell` to `tool_name: "Bash"` for this schema — already + // handled below, live-confirmed (see the VsCode variant doc). if let Some(tool_name) = v.get("tool_name").and_then(|t| t.as_str()) { if matches!( tool_name, @@ -99,12 +110,12 @@ fn detect_format(v: &Value) -> HookFormat { return HookFormat::PassThrough; } - // Copilot's native camelCase schema: toolName + toolArgs (JSON-encoded string). + // Copilot CLI's native camelCase schema: toolName + toolArgs (JSON-encoded string). // The shell tool is "bash" on Unix and "powershell" on Windows. // Only reachable today via a not-yet-upgraded install's leftover camelCase // preToolUse registration (see the CopilotCli variant doc) or a host that // registers this schema itself, like JetBrains/IntelliJ's Copilot plugin - // (toolName "run_in_terminal", tracked separately in #2443/#3093). + // (toolName "run_in_terminal"). if let Some(tool_name) = v.get("toolName").and_then(|t| t.as_str()) { if matches!(tool_name, "bash" | "powershell" | "run_in_terminal") { if let Some(tool_args_str) = v.get("toolArgs").and_then(|t| t.as_str()) { @@ -243,7 +254,7 @@ fn copilot_ide_response_from_decision(decision: HookDecision, cmd: &str) -> Opti audit_log("deny", cmd, ""); "Blocked by RTK permission rule".to_string() } - HookDecision::AllowRewrite(rewritten) | HookDecision::AskRewrite { rewritten, .. } => { + HookDecision::AllowRewrite(rewritten) | HookDecision::AskRewrite(rewritten) => { audit_log("rewrite", cmd, &rewritten); format!("RTK token optimization: re-run this command as `{rewritten}` instead.") } @@ -986,10 +997,7 @@ mod tests { #[test] fn test_copilot_ide_rewrite_returns_deny_with_suggestion() { let response = copilot_ide_response_from_decision( - HookDecision::AskRewrite { - rewritten: "rtk git status".into(), - explicit: false, - }, + HookDecision::AskRewrite("rtk git status".into()), "git status", ) .unwrap(); From fc8054eb0b357d32cb0457094d142de52c3db0e7 Mon Sep 17 00:00:00 2001 From: revopsrocks Date: Fri, 24 Jul 2026 15:37:23 +0930 Subject: [PATCH 06/22] feat(rewrite): rewrite each line of multi-line Bash blocks Coding agents routinely emit multi-line Bash blocks of sequential commands. The rewriter tokenized newlines as plain whitespace, so a block was treated as one command: the rtk prefix landed on the first line and every following line ran raw and unfiltered. Split multi-line input at the newline tokens the quote-aware lexer emits (newlines inside quoted strings are never split points) and rewrite each line through the existing single-line path. Blank lines, comment lines, indentation, and CRLF separators are preserved verbatim. Per-line rewriting only applies when every line is an independent command. The whole block passes through untouched when: - a shell keyword opens control flow (for/if/while/case/...) - a list or pipeline continues across the line break (&&, ||, |, |& at a line edge) - a subshell or group spans lines - the block contains a heredoc (existing gate) Ref #1243 --- src/discover/lexer.rs | 20 ++++ src/discover/registry.rs | 237 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 255 insertions(+), 2 deletions(-) diff --git a/src/discover/lexer.rs b/src/discover/lexer.rs index 6cca2a39e1..9a3805599d 100644 --- a/src/discover/lexer.rs +++ b/src/discover/lexer.rs @@ -26,6 +26,13 @@ pub fn tokenize(input: &str) -> Vec { tokenize_inner(input, false) } +/// Like [`tokenize`] but emits a `\n` operator token for each newline that +/// sits outside quotes. Newlines inside quoted strings stay part of their +/// argument, so callers can use the emitted offsets as safe line-split points. +pub fn tokenize_with_newlines(input: &str) -> Vec { + tokenize_inner(input, true) +} + fn tokenize_inner(input: &str, emit_newline: bool) -> Vec { let mut tokens = Vec::new(); let mut current = String::new(); @@ -1342,4 +1349,17 @@ mod tests { assert!(split_for_permissions("").is_empty()); assert!(split_for_permissions(" ").is_empty()); } + + #[test] + fn test_tokenize_with_newlines_emits_operator_outside_quotes_only() { + let newline_ops = |input: &str| { + tokenize_with_newlines(input) + .iter() + .filter(|t| t.kind == TokenKind::Operator && t.value == "\n") + .count() + }; + assert_eq!(newline_ops("git status\ngit log"), 1); + assert_eq!(newline_ops("echo 'line1\nline2'"), 0); + assert_eq!(newline_ops("git status\r\ngit log"), 2); + } } diff --git a/src/discover/registry.rs b/src/discover/registry.rs index 81b170785a..b7827482b1 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -5,7 +5,10 @@ use regex::{Regex, RegexSet}; use std::path::Path; use std::sync::LazyLock; -use super::lexer::{shell_split, split_on_operators, tokenize, ParsedToken, PipeKind, TokenKind}; +use super::lexer::{ + shell_split, split_on_operators, tokenize, tokenize_with_newlines, ParsedToken, PipeKind, + TokenKind, +}; use super::rules::{IGNORED_EXACT, IGNORED_PREFIXES, RULES}; const PHP_TOOL_NAMES: [&str; 6] = ["phpunit", "phpstan", "ecs", "pest", "paratest", "pint"]; @@ -583,6 +586,19 @@ pub fn rewrite_command( let compiled = compile_exclude_patterns(excluded); let normalized_prefixes = normalize_transparent_prefixes(transparent_prefixes); + if trimmed.contains('\n') { + return rewrite_multiline_block(trimmed, &compiled, &normalized_prefixes); + } + + rewrite_single(trimmed, &compiled, &normalized_prefixes) +} + +/// Rewrite one logical command line (no unquoted newlines). +fn rewrite_single( + trimmed: &str, + excluded: &[ExcludePattern], + transparent_prefixes: &[String], +) -> Option { // Simple (non-compound) already-RTK command — return as-is. // For compound commands that start with "rtk" (e.g. "rtk git add . && cargo test"), // fall through to rewrite_compound so the remaining segments get rewritten. @@ -595,7 +611,103 @@ pub fn rewrite_command( return Some(trimmed.to_string()); } - rewrite_compound(trimmed, &compiled, &normalized_prefixes) + rewrite_compound(trimmed, excluded, transparent_prefixes) +} + +/// Shell keywords that open or close a multi-line construct. A line inside a +/// loop, conditional, case arm, function body, or group is not an independent +/// command, so the whole block passes through untouched. +const BLOCK_KEYWORDS: &[&str] = &[ + "for", "while", "until", "if", "then", "else", "elif", "fi", "do", "done", "case", "esac", + "select", "function", "coproc", "{", "}", "(", ")", +]; + +/// A list or pipeline that continues across a line break makes adjacent lines +/// one logical command; grouping chars at a line edge mean a construct spans +/// lines. Either way, per-line rewriting is unsafe. +fn line_breaks_independence(line: &str) -> bool { + let first = line.split_whitespace().next().unwrap_or(""); + if BLOCK_KEYWORDS.contains(&first) { + return true; + } + ["&&", "||", "|&", "|"] + .iter() + .any(|op| line.ends_with(op) || line.starts_with(op)) + || line.ends_with('(') + || line.ends_with('{') + || line.starts_with(')') + || line.starts_with('}') +} + +/// Rewrite each line of a multi-line block independently (issue #1243). +/// +/// Split points are the newline tokens the quote-aware lexer emits, so a +/// newline inside a quoted string (e.g. a multi-line commit message) never +/// becomes a boundary. The whole block passes through unchanged unless every +/// line is an independent command (see [`line_breaks_independence`]); blank +/// lines and comment lines are preserved verbatim, as is indentation and the +/// original separator bytes (`\n` vs `\r\n`). +fn rewrite_multiline_block( + cmd: &str, + excluded: &[ExcludePattern], + transparent_prefixes: &[String], +) -> Option { + let newline_offsets: Vec = tokenize_with_newlines(cmd) + .iter() + .filter(|t| t.kind == TokenKind::Operator && t.value == "\n") + .map(|t| t.offset) + .collect(); + + // Newlines only inside quotes — a single logical command. + if newline_offsets.is_empty() { + return rewrite_single(cmd, excluded, transparent_prefixes); + } + + let mut segments = Vec::with_capacity(newline_offsets.len() + 1); + let mut start = 0; + for &off in &newline_offsets { + segments.push(&cmd[start..off]); + start = off + 1; + } + segments.push(&cmd[start..]); + + if segments + .iter() + .map(|seg| seg.trim()) + .filter(|line| !line.is_empty() && !line.starts_with('#')) + .any(line_breaks_independence) + { + return None; + } + + let mut any_changed = false; + let mut result = String::with_capacity(cmd.len() + 32); + for (i, seg) in segments.iter().enumerate() { + if i > 0 { + let off = newline_offsets[i - 1]; + result.push_str(&cmd[off..off + 1]); + } + let line = seg.trim(); + if line.is_empty() || line.starts_with('#') { + result.push_str(seg); + continue; + } + match rewrite_single(line, excluded, transparent_prefixes) { + Some(rewritten) if rewritten != line => { + any_changed = true; + let indent = &seg[..seg.len() - seg.trim_start().len()]; + result.push_str(indent); + result.push_str(&rewritten); + } + _ => result.push_str(seg), + } + } + + if any_changed { + Some(result) + } else { + None + } } /// Pipeline boundaries used to rewrite its final stage. @@ -1159,6 +1271,127 @@ mod tests { super::rewrite_command(cmd, excluded, &[]) } + mod multiline_blocks { + use super::rewrite_command_no_prefixes; + + #[test] + fn test_rewrites_each_line() { + assert_eq!( + rewrite_command_no_prefixes("git status\ngit log --oneline -3", &[]), + Some("rtk git status\nrtk git log --oneline -3".into()) + ); + } + + #[test] + fn test_preserves_blank_lines_comments_and_indentation() { + assert_eq!( + rewrite_command_no_prefixes("git status\n\n# check history\n git log -3", &[]), + Some("rtk git status\n\n# check history\n rtk git log -3".into()) + ); + } + + #[test] + fn test_compound_line_inside_block() { + assert_eq!( + rewrite_command_no_prefixes("cd /tmp && git status\ngrep -rn foo src", &[]), + Some("cd /tmp && rtk git status\nrtk grep -rn foo src".into()) + ); + } + + #[test] + fn test_crlf_separators_preserved() { + assert_eq!( + rewrite_command_no_prefixes("git status\r\ngit log -3", &[]), + Some("rtk git status\r\nrtk git log -3".into()) + ); + } + + #[test] + fn test_newline_inside_quotes_is_not_a_split_point() { + // The quoted body must never be treated as a command line of its own. + let result = + rewrite_command_no_prefixes("git commit -m \"subject\ngit status in body\"", &[]); + if let Some(rewritten) = result { + assert!(!rewritten.contains("rtk git status")); + } + } + + #[test] + fn test_no_rewritable_line_passes_through() { + assert_eq!(rewrite_command_no_prefixes("echo one\necho two", &[]), None); + } + + #[test] + fn test_already_rtk_lines_count_as_unchanged() { + assert_eq!( + rewrite_command_no_prefixes("rtk git status\necho done", &[]), + None + ); + } + + #[test] + fn test_mixed_rtk_and_rewritable_line() { + assert_eq!( + rewrite_command_no_prefixes("rtk git status\ngit log -3", &[]), + Some("rtk git status\nrtk git log -3".into()) + ); + } + + #[test] + fn test_for_loop_block_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("for f in a b; do\n grep -n foo $f\ndone", &[]), + None + ); + } + + #[test] + fn test_if_block_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("if [ -d src ]; then\n git status\nfi", &[]), + None + ); + } + + #[test] + fn test_cross_line_and_list_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("git status &&\ngit log -3", &[]), + None + ); + } + + #[test] + fn test_cross_line_pipeline_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("git log |\ngrep feat", &[]), + None + ); + assert_eq!( + rewrite_command_no_prefixes("cargo test |&\ngrep FAILED", &[]), + None + ); + } + + #[test] + fn test_subshell_spanning_lines_passes_through() { + assert_eq!(rewrite_command_no_prefixes("(\n git status\n)", &[]), None); + } + + #[test] + fn test_group_spanning_lines_passes_through() { + assert_eq!(rewrite_command_no_prefixes("{\n git status\n}", &[]), None); + } + + #[test] + fn test_heredoc_block_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("git status\ncat < PipelineAnalysis { let tokens = tokenize(cmd); let first_pipe_offset = tokens From ac5febe3bb1c57195d3bdbe73d029c6aad34a64d Mon Sep 17 00:00:00 2001 From: revopsrocks Date: Fri, 24 Jul 2026 16:14:42 +0930 Subject: [PATCH 07/22] fix(rewrite): bail on swallowed newlines and cross-line arithmetic Hardening from adversarial review of the multi-line rewrite: - If any newline byte was swallowed by quote state (raw \n/\r count vs emitted newline tokens), pass the whole block through. The lexer has no comment awareness, so an apostrophe in a # comment opens quote state and hides subsequent lines; rewriting such a block would act on lines no permission verdict accounted for. Passthrough hands the original command to native permission handling. Quoted multi-line strings (commit messages) forgo their rewrite as the safe trade. - Bail when (( or )) sits at a line edge: arithmetic spanning lines must not get a command prefix spliced into arithmetic context. --- src/discover/registry.rs | 73 ++++++++++++++++++++++++++++++++++------ 1 file changed, 63 insertions(+), 10 deletions(-) diff --git a/src/discover/registry.rs b/src/discover/registry.rs index b7827482b1..78597cb048 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -637,6 +637,9 @@ fn line_breaks_independence(line: &str) -> bool { || line.ends_with('{') || line.starts_with(')') || line.starts_with('}') + || line.starts_with("((") + || line.ends_with("))") + || line.ends_with("((") } /// Rewrite each line of a multi-line block independently (issue #1243). @@ -647,6 +650,14 @@ fn line_breaks_independence(line: &str) -> bool { /// line is an independent command (see [`line_breaks_independence`]); blank /// lines and comment lines are preserved verbatim, as is indentation and the /// original separator bytes (`\n` vs `\r\n`). +/// +/// If any newline byte was swallowed by quote state, the block passes through +/// untouched. The lexer has no comment awareness, so an apostrophe in a `#` +/// comment opens quote state and hides the rest of the block — rewriting (or +/// prefixing) such a block would act on lines no permission verdict was +/// computed for. Passthrough hands the original command to the agent's native +/// permission handling instead. Genuine quoted newlines (multi-line commit +/// messages) also land here; forgoing that rewrite is the safe trade. fn rewrite_multiline_block( cmd: &str, excluded: &[ExcludePattern], @@ -658,9 +669,9 @@ fn rewrite_multiline_block( .map(|t| t.offset) .collect(); - // Newlines only inside quotes — a single logical command. - if newline_offsets.is_empty() { - return rewrite_single(cmd, excluded, transparent_prefixes); + let raw_breaks = cmd.chars().filter(|c| matches!(c, '\n' | '\r')).count(); + if raw_breaks != newline_offsets.len() { + return None; } let mut segments = Vec::with_capacity(newline_offsets.len() + 1); @@ -1307,13 +1318,55 @@ mod tests { } #[test] - fn test_newline_inside_quotes_is_not_a_split_point() { - // The quoted body must never be treated as a command line of its own. - let result = - rewrite_command_no_prefixes("git commit -m \"subject\ngit status in body\"", &[]); - if let Some(rewritten) = result { - assert!(!rewritten.contains("rtk git status")); - } + fn test_newline_inside_quotes_passes_through() { + // A swallowed newline means the block can't be split safely — + // the quoted body must never be treated as a command line of its own. + assert_eq!( + rewrite_command_no_prefixes("git commit -m \"subject\ngit status in body\"", &[]), + None + ); + } + + #[test] + fn test_comment_apostrophe_swallowing_newline_passes_through() { + // The lexer has no comment state: the apostrophe in `don't` opens + // a quote that swallows the newline and hides the next line. The + // block must pass through so native permission handling sees the + // original command — never a partially rewritten one. + assert_eq!( + rewrite_command_no_prefixes("git status # don't\nrm -rf /tmp/x", &[]), + None + ); + } + + #[test] + fn test_comment_apostrophe_hidden_in_later_segment_passes_through() { + // Same hazard when a clean split point precedes the contaminated + // line: the swallowed-newline check is global, not per-segment. + assert_eq!( + rewrite_command_no_prefixes("git log -3\ngit status # don't\nrm -rf /tmp/x", &[]), + None + ); + } + + #[test] + fn test_comment_with_balanced_quotes_still_rewrites() { + // Both apostrophes close before the newline, so the split is safe + // and the trailing comment rides along untouched. + assert_eq!( + rewrite_command_no_prefixes( + "git status # isn't it what's expected\ngit log -3", + &[] + ), + Some("rtk git status # isn't it what's expected\nrtk git log -3".into()) + ); + } + + #[test] + fn test_arithmetic_spanning_lines_passes_through() { + // `(( x = ls ))` is arithmetic evaluation; injecting `rtk` before + // `ls` would splice a command into arithmetic context. + assert_eq!(rewrite_command_no_prefixes("(( x =\nls ))", &[]), None); } #[test] From 93aab9a29c940c7adcbf98b25f28d9b60020302d Mon Sep 17 00:00:00 2001 From: revopsrocks Date: Fri, 24 Jul 2026 20:55:12 +0930 Subject: [PATCH 08/22] fix(rewrite): harden line-independence checks from second review round MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Strip trailing unquoted comments before the independence checks: 'git log | # keep pipeline' continues the pipeline across the newline even though the line ends in comment text, and rewriting the next line would rebuild a pipeline whose final stage the pipeline-safety guard may specifically reject (e.g. grep -f). - Bail when a line's unquoted ()/{} don't balance: array literals (arr=(one), function bodies (foo() {), and groups span lines, so surrounding lines are not independent commands. Subsumes the previous single-char edge checks. - Bail on any block containing $'...': inside ANSI-C quoting bash treats \' as a literal quote that does not close the string, but the lexer thinks it does — emitting a split point bash would never honor, the inverse of the swallowed-newline case the count check catches. --- src/discover/registry.rs | 149 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 138 insertions(+), 11 deletions(-) diff --git a/src/discover/registry.rs b/src/discover/registry.rs index 78597cb048..29c2c9c03c 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -622,24 +622,81 @@ const BLOCK_KEYWORDS: &[&str] = &[ "select", "function", "coproc", "{", "}", "(", ")", ]; +/// Byte offset where an unquoted `#` at the start of a word begins a trailing +/// comment, if any. The lexer has no comment state, so the independence checks +/// must ignore comment text themselves: `git log | # keep pipeline` continues +/// the pipeline across the newline even though the line ends in comment text. +fn comment_start(line: &str) -> Option { + let bytes = line.as_bytes(); + let mut in_single = false; + let mut in_double = false; + let mut i = 0; + while i < bytes.len() { + match bytes[i] { + b'\\' if !in_single => { + i += 2; + continue; + } + b'\'' if !in_double => in_single = !in_single, + b'"' if !in_single => in_double = !in_double, + b'#' if !in_single && !in_double && (i == 0 || bytes[i - 1].is_ascii_whitespace()) => { + return Some(i) + } + _ => {} + } + i += 1; + } + None +} + +/// Unquoted `(`/`)` or `{`/`}` that don't balance within the line: an array +/// literal (`arr=(one`), function body (`foo() {`), or group spans lines, so +/// the lines around it are not independent commands. +fn line_has_unbalanced_grouping(code: &str) -> bool { + let bytes = code.as_bytes(); + let mut in_single = false; + let mut in_double = false; + let mut paren = 0i32; + let mut brace = 0i32; + let mut i = 0; + while i < bytes.len() { + match bytes[i] { + b'\\' if !in_single => { + i += 2; + continue; + } + b'\'' if !in_double => in_single = !in_single, + b'"' if !in_single => in_double = !in_double, + b'(' if !in_single && !in_double => paren += 1, + b')' if !in_single && !in_double => paren -= 1, + b'{' if !in_single && !in_double => brace += 1, + b'}' if !in_single && !in_double => brace -= 1, + _ => {} + } + if paren < 0 || brace < 0 { + return true; + } + i += 1; + } + paren != 0 || brace != 0 +} + /// A list or pipeline that continues across a line break makes adjacent lines -/// one logical command; grouping chars at a line edge mean a construct spans -/// lines. Either way, per-line rewriting is unsafe. +/// one logical command; grouping that spans lines means no line stands alone. +/// Either way, per-line rewriting is unsafe. All checks run against the line +/// with any trailing comment stripped. fn line_breaks_independence(line: &str) -> bool { - let first = line.split_whitespace().next().unwrap_or(""); + let code = comment_start(line).map_or(line, |i| line[..i].trim_end()); + let first = code.split_whitespace().next().unwrap_or(""); if BLOCK_KEYWORDS.contains(&first) { return true; } ["&&", "||", "|&", "|"] .iter() - .any(|op| line.ends_with(op) || line.starts_with(op)) - || line.ends_with('(') - || line.ends_with('{') - || line.starts_with(')') - || line.starts_with('}') - || line.starts_with("((") - || line.ends_with("))") - || line.ends_with("((") + .any(|op| code.ends_with(op) || code.starts_with(op)) + || code.starts_with("((") + || code.ends_with("))") + || line_has_unbalanced_grouping(code) } /// Rewrite each line of a multi-line block independently (issue #1243). @@ -669,6 +726,14 @@ fn rewrite_multiline_block( .map(|t| t.offset) .collect(); + // ANSI-C quoting is the inverse hazard: inside $'...' bash treats \' as a + // literal quote that does NOT close the string, but the lexer thinks it + // does — emitting a split point bash would never honor, which the + // swallowed-newline count check below cannot see. Forgo the rewrite. + if cmd.contains("$'") { + return None; + } + let raw_breaks = cmd.chars().filter(|c| matches!(c, '\n' | '\r')).count(); if raw_breaks != newline_offsets.len() { return None; @@ -1369,6 +1434,68 @@ mod tests { assert_eq!(rewrite_command_no_prefixes("(( x =\nls ))", &[]), None); } + #[test] + fn test_array_assignment_spanning_lines_passes_through() { + // The inner line is an array element, not a command; rewriting it + // would mutate the array's contents. + assert_eq!( + rewrite_command_no_prefixes("arr=(one\ngit status\ntwo)", &[]), + None + ); + } + + #[test] + fn test_function_definition_spanning_lines_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("foo() {\n git status\n}", &[]), + None + ); + } + + #[test] + fn test_continuation_operator_behind_comment_passes_through() { + // Bash continues the pipeline across the newline even though the + // line ends in comment text; the next line is a pipeline stage, + // not an independent command. + assert_eq!( + rewrite_command_no_prefixes("git log | # keep pipeline\ngrep -f patterns.txt", &[]), + None + ); + assert_eq!( + rewrite_command_no_prefixes("git status && # continue\ngit log -3", &[]), + None + ); + } + + #[test] + fn test_ansi_c_quoting_passes_through() { + // Inside $'...' bash treats \' as a literal quote that does not + // close the string, so the second line is string content — the + // lexer can't see that, so any $' in the block forgoes the rewrite. + assert_eq!( + rewrite_command_no_prefixes("x=$'foo\\'\ngit status\n'", &[]), + None + ); + assert_eq!( + rewrite_command_no_prefixes("echo $'a\\tb'\ngit status", &[]), + None + ); + } + + #[test] + fn test_balanced_grouping_within_a_line_still_rewrites() { + // `${HOME}` braces (quoted or not) must not trip the + // unbalanced-grouping bail. + assert_eq!( + rewrite_command_no_prefixes("echo ${HOME}\ngit status", &[]), + Some("echo ${HOME}\nrtk git status".into()) + ); + assert_eq!( + rewrite_command_no_prefixes("echo \"${HOME}\"\ngit status", &[]), + Some("echo \"${HOME}\"\nrtk git status".into()) + ); + } + #[test] fn test_no_rewritable_line_passes_through() { assert_eq!(rewrite_command_no_prefixes("echo one\necho two", &[]), None); From d1714494914b003d6456c1dae0afa579d2bef4c4 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Thu, 30 Jul 2026 14:13:44 +0200 Subject: [PATCH 09/22] fix(rewrite): narrow multiline passthrough bails from #3188 review --- src/discover/registry.rs | 238 ++++++++++++++++++++++++++++++++------- 1 file changed, 200 insertions(+), 38 deletions(-) diff --git a/src/discover/registry.rs b/src/discover/registry.rs index 29c2c9c03c..a48610ae03 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -538,6 +538,8 @@ fn strip_trailing_redirects(cmd: &str) -> (&str, &str) { static LINE_CONTINUATION_RE: LazyLock = LazyLock::new(|| Regex::new(r"(?m)[ \t\x0B\x0C]*\\\r?\n[ \t\x0B\x0C]*").unwrap()); +static BASH_JOIN_RE: LazyLock = LazyLock::new(|| Regex::new(r"\\\r?\n").unwrap()); + /// Replace every bash line continuation with a single space, mirroring what /// bash does before dispatching the command. Returns a borrowed `&str` when the /// input contains no continuations, so the common fast path allocates nothing. @@ -569,6 +571,15 @@ pub fn rewrite_command( excluded: &[String], transparent_prefixes: &[String], ) -> Option { + // Bash joins `\` with nothing, so `<<` or `$((` can arrive split across + // a continuation; the space-join below would erase them (#3188 review). + if cmd.contains('\\') { + let joined = BASH_JOIN_RE.replace_all(cmd, ""); + if has_heredoc(&joined) || joined.contains("$((") { + return None; + } + } + // Bash line continuations (`\`, `\`) and the leading whitespace that // follows are syntactically equivalent to a single space, but `cmd.trim()` does // not unwrap them so a leading backslash-newline used to defeat the whole matcher. @@ -639,7 +650,14 @@ fn comment_start(line: &str) -> Option { } b'\'' if !in_double => in_single = !in_single, b'"' if !in_single => in_double = !in_double, - b'#' if !in_single && !in_double && (i == 0 || bytes[i - 1].is_ascii_whitespace()) => { + // `#` starts a comment at any word start, incl. after an operator + // byte — but not after `{`: `${#var}` is an expansion (#3188 review). + b'#' if !in_single + && !in_double + && (i == 0 + || bytes[i - 1].is_ascii_whitespace() + || matches!(bytes[i - 1], b'|' | b'&' | b';' | b'(' | b')')) => + { return Some(i) } _ => {} @@ -681,32 +699,92 @@ fn line_has_unbalanced_grouping(code: &str) -> bool { paren != 0 || brace != 0 } -/// A list or pipeline that continues across a line break makes adjacent lines -/// one logical command; grouping that spans lines means no line stands alone. -/// Either way, per-line rewriting is unsafe. All checks run against the line -/// with any trailing comment stripped. -fn line_breaks_independence(line: &str) -> bool { - let code = comment_start(line).map_or(line, |i| line[..i].trim_end()); +// Only `\'` inside `$'…'` diverges: bash keeps the string open, the lexer +// closes it — an extra split point the newline-count check can't see (#3188). +fn ansi_c_quote_defeats_lexer(cmd: &str) -> bool { + let bytes = cmd.as_bytes(); + let mut in_single = false; + let mut in_double = false; + let mut i = 0; + while i < bytes.len() { + match bytes[i] { + b'\\' if !in_single => { + i += 2; + continue; + } + b'\'' if !in_double => in_single = !in_single, + b'"' if !in_single => in_double = !in_double, + b'$' if !in_single && !in_double && bytes.get(i + 1) == Some(&b'\'') => { + i += 2; + while i < bytes.len() { + match bytes[i] { + b'\\' => { + if bytes.get(i + 1) == Some(&b'\'') { + return true; + } + i += 2; + continue; + } + b'\'' => break, + _ => {} + } + i += 1; + } + } + _ => {} + } + i += 1; + } + false +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum LineRole { + Passive, + Independent, + ContinuesNext, + Unsafe, +} + +fn classify_line(line: &str) -> LineRole { + if line.is_empty() || line.starts_with('#') { + return LineRole::Passive; + } + let comment = comment_start(line); + let code = comment.map_or(line, |i| line[..i].trim_end()); let first = code.split_whitespace().next().unwrap_or(""); if BLOCK_KEYWORDS.contains(&first) { - return true; + return LineRole::Unsafe; } - ["&&", "||", "|&", "|"] - .iter() - .any(|op| code.ends_with(op) || code.starts_with(op)) + const CONTINUATION_OPS: [&str; 4] = ["&&", "||", "|&", "|"]; + if CONTINUATION_OPS.iter().any(|op| code.starts_with(op)) || code.starts_with("((") || code.ends_with("))") || line_has_unbalanced_grouping(code) + { + return LineRole::Unsafe; + } + if CONTINUATION_OPS.iter().any(|op| code.ends_with(op)) { + // An operator behind a trailing comment can't be joined textually: + // the comment-blind tokenizer would read the comment as command words. + return if comment.is_some() { + LineRole::Unsafe + } else { + LineRole::ContinuesNext + }; + } + LineRole::Independent } /// Rewrite each line of a multi-line block independently (issue #1243). /// /// Split points are the newline tokens the quote-aware lexer emits, so a /// newline inside a quoted string (e.g. a multi-line commit message) never -/// becomes a boundary. The whole block passes through unchanged unless every -/// line is an independent command (see [`line_breaks_independence`]); blank -/// lines and comment lines are preserved verbatim, as is indentation and the -/// original separator bytes (`\n` vs `\r\n`). +/// becomes a boundary. Lines continued by a trailing `&&`/`||`/`|`/`|&` are +/// joined and rewritten as one logical command through the single-line path; +/// any line [`classify_line`] marks unsafe passes the whole block through. +/// Blank lines and comment lines are preserved verbatim, as is indentation +/// and the original separator bytes (`\n` vs `\r\n`). /// /// If any newline byte was swallowed by quote state, the block passes through /// untouched. The lexer has no comment awareness, so an apostrophe in a `#` @@ -726,11 +804,7 @@ fn rewrite_multiline_block( .map(|t| t.offset) .collect(); - // ANSI-C quoting is the inverse hazard: inside $'...' bash treats \' as a - // literal quote that does NOT close the string, but the lexer thinks it - // does — emitting a split point bash would never honor, which the - // swallowed-newline count check below cannot see. Forgo the rewrite. - if cmd.contains("$'") { + if ansi_c_quote_defeats_lexer(cmd) { return None; } @@ -742,32 +816,59 @@ fn rewrite_multiline_block( let mut segments = Vec::with_capacity(newline_offsets.len() + 1); let mut start = 0; for &off in &newline_offsets { - segments.push(&cmd[start..off]); + segments.push((start, &cmd[start..off])); start = off + 1; } - segments.push(&cmd[start..]); + segments.push((start, &cmd[start..])); - if segments + let roles: Vec = segments .iter() - .map(|seg| seg.trim()) - .filter(|line| !line.is_empty() && !line.starts_with('#')) - .any(line_breaks_independence) - { + .map(|(_, seg)| classify_line(seg.trim())) + .collect(); + if roles.contains(&LineRole::Unsafe) { return None; } let mut any_changed = false; let mut result = String::with_capacity(cmd.len() + 32); - for (i, seg) in segments.iter().enumerate() { + let mut i = 0; + while i < segments.len() { if i > 0 { let off = newline_offsets[i - 1]; result.push_str(&cmd[off..off + 1]); } - let line = seg.trim(); - if line.is_empty() || line.starts_with('#') { + let (seg_off, seg) = segments[i]; + + if roles[i] == LineRole::Passive { result.push_str(seg); + i += 1; continue; } + + let mut end = i; + while roles[end] == LineRole::ContinuesNext { + let mut next = end + 1; + while next < segments.len() && segments[next].1.trim().is_empty() { + next += 1; + } + if next >= segments.len() { + break; + } + if roles[next] == LineRole::Passive { + // Comment line inside a continuation: the comment-blind + // tokenizer would join it as command words (#3188 review). + return None; + } + end = next; + } + + let unit = if end == i { + seg + } else { + let (last_off, last_seg) = segments[end]; + &cmd[seg_off..last_off + last_seg.len()] + }; + let line = unit.trim(); match rewrite_single(line, excluded, transparent_prefixes) { Some(rewritten) if rewritten != line => { any_changed = true; @@ -775,8 +876,9 @@ fn rewrite_multiline_block( result.push_str(indent); result.push_str(&rewritten); } - _ => result.push_str(seg), + _ => result.push_str(unit), } + i = end + 1; } if any_changed { @@ -1468,17 +1570,21 @@ mod tests { } #[test] - fn test_ansi_c_quoting_passes_through() { + fn test_ansi_c_escaped_quote_passes_through() { // Inside $'...' bash treats \' as a literal quote that does not // close the string, so the second line is string content — the - // lexer can't see that, so any $' in the block forgoes the rewrite. + // lexer can't see that, so the block forgoes the rewrite. assert_eq!( rewrite_command_no_prefixes("x=$'foo\\'\ngit status\n'", &[]), None ); + } + + #[test] + fn test_ansi_c_without_escaped_quote_still_rewrites() { assert_eq!( rewrite_command_no_prefixes("echo $'a\\tb'\ngit status", &[]), - None + Some("echo $'a\\tb'\nrtk git status".into()) ); } @@ -1534,18 +1640,18 @@ mod tests { } #[test] - fn test_cross_line_and_list_passes_through() { + fn test_cross_line_and_list_joins_and_rewrites() { assert_eq!( rewrite_command_no_prefixes("git status &&\ngit log -3", &[]), - None + Some("rtk git status && rtk git log -3".into()) ); } #[test] - fn test_cross_line_pipeline_passes_through() { + fn test_cross_line_pipeline_joins_and_rewrites() { assert_eq!( rewrite_command_no_prefixes("git log |\ngrep feat", &[]), - None + Some("git log | rtk grep feat".into()) ); assert_eq!( rewrite_command_no_prefixes("cargo test |&\ngrep FAILED", &[]), @@ -1553,6 +1659,46 @@ mod tests { ); } + #[test] + fn test_cross_line_pipeline_unsafe_final_stage_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("git log |\ngrep -f patterns.txt", &[]), + None + ); + } + + #[test] + fn test_mixed_independent_and_continued_lines() { + assert_eq!( + rewrite_command_no_prefixes("grep -rn foo src\ngit status &&\ngit log -3", &[]), + Some("rtk grep -rn foo src\nrtk git status && rtk git log -3".into()) + ); + } + + #[test] + fn test_blank_line_inside_continuation_joins() { + assert_eq!( + rewrite_command_no_prefixes("git status &&\n\ngit log -3", &[]), + Some("rtk git status && rtk git log -3".into()) + ); + } + + #[test] + fn test_comment_line_inside_continuation_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("git status &&\n# note\ngit log -3", &[]), + None + ); + } + + #[test] + fn test_comment_directly_after_operator_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("git log |# keep pipeline\ngrep -f patterns.txt", &[]), + None + ); + } + #[test] fn test_subshell_spanning_lines_passes_through() { assert_eq!(rewrite_command_no_prefixes("(\n git status\n)", &[]), None); @@ -1570,6 +1716,22 @@ mod tests { None ); } + + #[test] + fn test_heredoc_split_by_line_continuation_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("cat <\\\n PipelineAnalysis { From 3c8892f5c08d23d857643ff2c8dc4ee2dfeee549 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Thu, 30 Jul 2026 14:54:55 +0200 Subject: [PATCH 10/22] fix(rewrite): keep rewriting quoted multi-line strings as one command --- src/discover/registry.rs | 44 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/src/discover/registry.rs b/src/discover/registry.rs index a48610ae03..0030a30f25 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -738,6 +738,26 @@ fn ansi_c_quote_defeats_lexer(cmd: &str) -> bool { false } +fn quotes_balanced(cmd: &str) -> bool { + let bytes = cmd.as_bytes(); + let mut in_single = false; + let mut in_double = false; + let mut i = 0; + while i < bytes.len() { + match bytes[i] { + b'\\' if !in_single => { + i += 2; + continue; + } + b'\'' if !in_double => in_single = !in_single, + b'"' if !in_single => in_double = !in_double, + _ => {} + } + i += 1; + } + !in_single && !in_double +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum LineRole { Passive, @@ -810,6 +830,12 @@ fn rewrite_multiline_block( let raw_breaks = cmd.chars().filter(|c| matches!(c, '\n' | '\r')).count(); if raw_breaks != newline_offsets.len() { + // Every newline swallowed by quote state with quotes balanced at EOF + // is one logical command (a multi-line commit message), not a hidden + // extra line; rewrite it whole, as develop always did (#3319 fuzz). + if newline_offsets.is_empty() && quotes_balanced(cmd) { + return rewrite_single(cmd, excluded, transparent_prefixes); + } return None; } @@ -1485,11 +1511,23 @@ mod tests { } #[test] - fn test_newline_inside_quotes_passes_through() { - // A swallowed newline means the block can't be split safely — - // the quoted body must never be treated as a command line of its own. + fn test_newline_inside_quotes_rewrites_as_one_command() { + // The quoted body is never treated as a command line of its own; + // the whole thing is one logical command and gets one prefix. assert_eq!( rewrite_command_no_prefixes("git commit -m \"subject\ngit status in body\"", &[]), + Some("rtk git commit -m \"subject\ngit status in body\"".into()) + ); + assert_eq!( + rewrite_command_no_prefixes("git commit -m 'multi\nline\nmessage'", &[]), + Some("rtk git commit -m 'multi\nline\nmessage'".into()) + ); + } + + #[test] + fn test_unbalanced_swallowed_newline_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("git commit -m \"subject\ngit status", &[]), None ); } From b09da2d0f804cdb61e9f7f1a452803d028212408 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Sat, 1 Aug 2026 14:31:54 +0200 Subject: [PATCH 11/22] fix(rewrite): bail on cross-line [[ ]], share quote-state walker --- src/discover/registry.rs | 226 ++++++++++++++++++++++++--------------- 1 file changed, 137 insertions(+), 89 deletions(-) diff --git a/src/discover/registry.rs b/src/discover/registry.rs index 0030a30f25..6469b178d8 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -633,129 +633,150 @@ const BLOCK_KEYWORDS: &[&str] = &[ "select", "function", "coproc", "{", "}", "(", ")", ]; +/// Shared quote-state byte walker used by all line scanners. Yields +/// `(offset, byte, in_single_before, in_double_before)`, skipping backslash +/// escape pairs outside single quotes and toggling quote state — the same +/// model the lexer applies. +struct QuoteScan<'a> { + bytes: &'a [u8], + i: usize, + in_single: bool, + in_double: bool, +} + +impl<'a> QuoteScan<'a> { + fn new(s: &'a str) -> Self { + Self { + bytes: s.as_bytes(), + i: 0, + in_single: false, + in_double: false, + } + } + + fn balanced(&self) -> bool { + !self.in_single && !self.in_double + } +} + +impl Iterator for QuoteScan<'_> { + type Item = (usize, u8, bool, bool); + + fn next(&mut self) -> Option { + while self.i < self.bytes.len() { + let i = self.i; + let b = self.bytes[i]; + if b == b'\\' && !self.in_single { + self.i += 2; + continue; + } + let item = (i, b, self.in_single, self.in_double); + match b { + b'\'' if !self.in_double => self.in_single = !self.in_single, + b'"' if !self.in_single => self.in_double = !self.in_double, + _ => {} + } + self.i += 1; + return Some(item); + } + None + } +} + /// Byte offset where an unquoted `#` at the start of a word begins a trailing /// comment, if any. The lexer has no comment state, so the independence checks /// must ignore comment text themselves: `git log | # keep pipeline` continues /// the pipeline across the newline even though the line ends in comment text. fn comment_start(line: &str) -> Option { let bytes = line.as_bytes(); - let mut in_single = false; - let mut in_double = false; - let mut i = 0; - while i < bytes.len() { - match bytes[i] { - b'\\' if !in_single => { - i += 2; - continue; - } - b'\'' if !in_double => in_single = !in_single, - b'"' if !in_single => in_double = !in_double, - // `#` starts a comment at any word start, incl. after an operator - // byte — but not after `{`: `${#var}` is an expansion (#3188 review). - b'#' if !in_single - && !in_double - && (i == 0 - || bytes[i - 1].is_ascii_whitespace() - || matches!(bytes[i - 1], b'|' | b'&' | b';' | b'(' | b')')) => - { - return Some(i) - } - _ => {} - } - i += 1; - } - None + // `#` starts a comment at any word start, incl. after an operator + // byte — but not after `{`: `${#var}` is an expansion (#3188 review). + QuoteScan::new(line).find_map(|(i, b, in_single, in_double)| { + (b == b'#' + && !in_single + && !in_double + && (i == 0 + || bytes[i - 1].is_ascii_whitespace() + || matches!(bytes[i - 1], b'|' | b'&' | b';' | b'(' | b')'))) + .then_some(i) + }) } /// Unquoted `(`/`)` or `{`/`}` that don't balance within the line: an array /// literal (`arr=(one`), function body (`foo() {`), or group spans lines, so /// the lines around it are not independent commands. fn line_has_unbalanced_grouping(code: &str) -> bool { - let bytes = code.as_bytes(); - let mut in_single = false; - let mut in_double = false; let mut paren = 0i32; let mut brace = 0i32; - let mut i = 0; - while i < bytes.len() { - match bytes[i] { - b'\\' if !in_single => { - i += 2; - continue; - } - b'\'' if !in_double => in_single = !in_single, - b'"' if !in_single => in_double = !in_double, - b'(' if !in_single && !in_double => paren += 1, - b')' if !in_single && !in_double => paren -= 1, - b'{' if !in_single && !in_double => brace += 1, - b'}' if !in_single && !in_double => brace -= 1, + for (_, b, in_single, in_double) in QuoteScan::new(code) { + if in_single || in_double { + continue; + } + match b { + b'(' => paren += 1, + b')' => paren -= 1, + b'{' => brace += 1, + b'}' => brace -= 1, _ => {} } if paren < 0 || brace < 0 { return true; } - i += 1; } paren != 0 || brace != 0 } +/// Unquoted `[[` / `]]` words that don't balance within the line: bash allows +/// a conditional expression to span lines (`[[ -f a &&` / `-f b ]]`), so the +/// surrounding lines are not independent commands. +fn line_has_unbalanced_test_brackets(code: &str) -> bool { + let bytes = code.as_bytes(); + let mut depth = 0i32; + for (i, b, in_single, in_double) in QuoteScan::new(code) { + if in_single || in_double || !matches!(b, b'[' | b']') { + continue; + } + let word_start = i == 0 || bytes[i - 1].is_ascii_whitespace(); + let word_end = bytes.get(i + 2).is_none_or(|c| c.is_ascii_whitespace()); + if bytes.get(i + 1) == Some(&b) && word_start && word_end { + depth += if b == b'[' { 1 } else { -1 }; + if depth < 0 { + return true; + } + } + } + depth != 0 +} + // Only `\'` inside `$'…'` diverges: bash keeps the string open, the lexer // closes it — an extra split point the newline-count check can't see (#3188). fn ansi_c_quote_defeats_lexer(cmd: &str) -> bool { let bytes = cmd.as_bytes(); - let mut in_single = false; - let mut in_double = false; - let mut i = 0; - while i < bytes.len() { - match bytes[i] { - b'\\' if !in_single => { - i += 2; - continue; + let mut ansi_span = false; + let mut backslash_run = 0u32; + for (i, b, in_single, in_double) in QuoteScan::new(cmd) { + if b == b'\'' && !in_double { + if !in_single { + ansi_span = i > 0 && bytes[i - 1] == b'$'; + backslash_run = 0; + } else if ansi_span && backslash_run % 2 == 1 { + return true; } - b'\'' if !in_double => in_single = !in_single, - b'"' if !in_single => in_double = !in_double, - b'$' if !in_single && !in_double && bytes.get(i + 1) == Some(&b'\'') => { - i += 2; - while i < bytes.len() { - match bytes[i] { - b'\\' => { - if bytes.get(i + 1) == Some(&b'\'') { - return true; - } - i += 2; - continue; - } - b'\'' => break, - _ => {} - } - i += 1; - } + } else if in_single { + if b == b'\\' { + backslash_run += 1; + } else { + backslash_run = 0; } - _ => {} } - i += 1; } false } fn quotes_balanced(cmd: &str) -> bool { - let bytes = cmd.as_bytes(); - let mut in_single = false; - let mut in_double = false; - let mut i = 0; - while i < bytes.len() { - match bytes[i] { - b'\\' if !in_single => { - i += 2; - continue; - } - b'\'' if !in_double => in_single = !in_single, - b'"' if !in_single => in_double = !in_double, - _ => {} - } - i += 1; - } - !in_single && !in_double + let mut scan = QuoteScan::new(cmd); + scan.by_ref().for_each(drop); + scan.balanced() } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -781,6 +802,7 @@ fn classify_line(line: &str) -> LineRole { || code.starts_with("((") || code.ends_with("))") || line_has_unbalanced_grouping(code) + || line_has_unbalanced_test_brackets(code) { return LineRole::Unsafe; } @@ -801,7 +823,9 @@ fn classify_line(line: &str) -> LineRole { /// Split points are the newline tokens the quote-aware lexer emits, so a /// newline inside a quoted string (e.g. a multi-line commit message) never /// becomes a boundary. Lines continued by a trailing `&&`/`||`/`|`/`|&` are -/// joined and rewritten as one logical command through the single-line path; +/// joined and rewritten as one logical command through the single-line path — +/// joining is not byte-preserving: separators inside a joined unit collapse +/// to single spaces (see `test_blank_line_inside_continuation_joins`); /// any line [`classify_line`] marks unsafe passes the whole block through. /// Blank lines and comment lines are preserved verbatim, as is indentation /// and the original separator bytes (`\n` vs `\r\n`). @@ -828,6 +852,8 @@ fn rewrite_multiline_block( return None; } + // The lexer emits one newline token per `\r` and per `\n` (CRLF = two + // tokens), so the parity check must count both bytes individually. let raw_breaks = cmd.chars().filter(|c| matches!(c, '\n' | '\r')).count(); if raw_breaks != newline_offsets.len() { // Every newline swallowed by quote state with quotes balanced at EOF @@ -888,6 +914,8 @@ fn rewrite_multiline_block( end = next; } + // A joined unit is rebuilt through the single-line path: interior + // newlines and blank lines collapse to single spaces, not preserved. let unit = if end == i { seg } else { @@ -1737,6 +1765,26 @@ mod tests { ); } + #[test] + fn test_conditional_expression_spanning_lines_passes_through() { + assert_eq!( + rewrite_command_no_prefixes("[[ -f a &&\n-f b ]]\ngit status", &[]), + None + ); + assert_eq!( + rewrite_command_no_prefixes("git status\n[[\n-f a ]]", &[]), + None + ); + } + + #[test] + fn test_balanced_conditional_line_still_rewrites() { + assert_eq!( + rewrite_command_no_prefixes("[[ -x foo ]] &&\ngit status", &[]), + Some("[[ -x foo ]] && rtk git status".into()) + ); + } + #[test] fn test_subshell_spanning_lines_passes_through() { assert_eq!(rewrite_command_no_prefixes("(\n git status\n)", &[]), None); From d480f1ec481fbd30bce16269a31d5d063bb96023 Mon Sep 17 00:00:00 2001 From: Xavier Pestel Date: Tue, 4 Aug 2026 16:51:24 +0200 Subject: [PATCH 12/22] feat(hooks): add transparent hook support for Mistral Vibe CLI Add `rtk init -g --agent vibe` and `rtk hook vibe` to route bash tool calls through the RTK proxy via Vibe's newly-shipped pre_tool hook. Implementation follows the Gemini / Droid pattern: - Native binary hook (`rtk hook vibe`), no shell script dependency. - Global-only install (`~/.vibe/hooks.toml`); user-scope only. - Idempotent install: detects existing `name = "rtk-rewrite"` entry. - Uninstall is surgical: strips only the RTK `[[hooks]]` block and the `~/.vibe/prompts/rtk.md` prompt file, preserving any other user hooks byte-for-byte. Removes hooks.toml only when it becomes empty. - Hook response uses Vibe's documented `hook_specific_output.tool_input` rewrite contract with a `system_message` for UI visibility. Vibe hook API reference: https://docs.mistral.ai/vibe/code/cli/hooks Closes #800. --- README.md | 2 +- src/hooks/constants.rs | 9 + src/hooks/hook_cmd.rs | 56 +++++ src/hooks/init.rs | 439 ++++++++++++++++++++++++++++++++++++++- src/hooks/permissions.rs | 2 + src/main.rs | 19 ++ 6 files changed, 525 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index e019e80e82..a2bb93d7bb 100644 --- a/README.md +++ b/README.md @@ -397,7 +397,7 @@ RTK supports 15 AI coding tools. Each integration rewrites shell commands to `rt | **OpenClaw** | `openclaw plugins install ./openclaw` | Plugin TS (before_tool_call) | | **Pi** | `rtk init -g --agent pi` (global) | TypeScript extension (tool_call) | | **Hermes** | `rtk init --agent hermes` | Python plugin adapter (terminal command mutation via `rtk rewrite`) | -| **Mistral Vibe** | Planned ([#800](https://github.com/rtk-ai/rtk/issues/800)) | Blocked on upstream | +| **Mistral Vibe** | `rtk init -g --agent vibe` | `pre_tool` hook (hooks.toml) | | **Kilo Code** | `rtk init --agent kilocode` | .kilocode/rules/rtk-rules.md (project-scoped) | | **Google Antigravity** | `rtk init --agent antigravity` | .agents/rules/antigravity-rtk-rules.md (project-scoped) | | **Kimi AI** | `rtk init --agent kimi` | AGENTS.md (project-scoped) | diff --git a/src/hooks/constants.rs b/src/hooks/constants.rs index 4caaf94473..700e9798f4 100644 --- a/src/hooks/constants.rs +++ b/src/hooks/constants.rs @@ -14,6 +14,8 @@ pub const CLAUDE_HOOK_COMMAND: &str = "rtk hook claude"; pub const CURSOR_HOOK_COMMAND: &str = "rtk hook cursor"; /// Native Rust hook command for Factory Droid. pub const DROID_HOOK_COMMAND: &str = "rtk hook droid"; +/// Native Rust hook command for Mistral Vibe. +pub const VIBE_HOOK_COMMAND: &str = "rtk hook vibe"; pub const CONFIG_DIR: &str = ".config"; pub const OPENCODE_SUBDIR: &str = "opencode"; @@ -58,3 +60,10 @@ pub const HERMES_PLUGINS_SUBDIR: &str = "plugins"; pub const HERMES_PLUGIN_NAME: &str = "rtk-rewrite"; pub const HERMES_PLUGIN_INIT_FILE: &str = "__init__.py"; pub const HERMES_PLUGIN_MANIFEST_FILE: &str = "plugin.yaml"; + +pub const VIBE_DIR: &str = ".vibe"; +pub const VIBE_HOOKS_FILE: &str = "hooks.toml"; +pub const VIBE_PROMPTS_SUBDIR: &str = "prompts"; +pub const VIBE_PROMPT_FILE: &str = "rtk.md"; +pub const VIBE_HOOK_NAME: &str = "rtk-rewrite"; +pub const VIBE_BASH_MATCH: &str = "bash"; diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index e9d186d844..aa6b214eed 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -352,6 +352,62 @@ pub fn run_gemini() -> Result<()> { Ok(()) } +// ── Vibe hook ───────────────────────────────────────────────── + +/// Run the Mistral Vibe CLI pre_tool hook. +/// +/// Vibe hook contract (https://docs.mistral.ai/vibe/code/cli/hooks): +/// - stdin: JSON with `tool_name`, `tool_input`, `hook_event_name`, etc. +/// - Passthrough: exit 0 with empty stdout. +/// - Rewrite: emit `{"hook_specific_output": {"tool_input": {"command": "..."}}}`. +/// - Deny: emit `{"decision": "deny", "reason": "..."}`. +pub fn run_vibe() -> Result<()> { + let input = read_stdin_limited()?; + + let json: Value = serde_json::from_str(&input).context("Failed to parse hook input as JSON")?; + + let tool_name = json.get("tool_name").and_then(|v| v.as_str()).unwrap_or(""); + + if tool_name != "bash" { + return Ok(()); + } + + let cmd = json + .pointer("/tool_input/command") + .and_then(|v| v.as_str()) + .unwrap_or(""); + + if cmd.is_empty() { + return Ok(()); + } + + match decide_hook_action(cmd, permissions::Host::Vibe) { + HookDecision::Deny => { + let _ = writeln!( + io::stdout(), + r#"{{"decision":"deny","reason":"Blocked by RTK permission rule"}}"# + ); + } + HookDecision::AllowRewrite(ref rewritten) | HookDecision::AskRewrite(ref rewritten) => { + audit_log("rewrite", cmd, rewritten); + let _ = writeln!(io::stdout(), "{}", vibe_rewrite_json(rewritten)); + } + HookDecision::Defer => {} + } + + Ok(()) +} + +fn vibe_rewrite_json(rewritten: &str) -> String { + serde_json::json!({ + "hook_specific_output": { + "tool_input": { "command": rewritten } + }, + "system_message": format!("rtk: rewrote to `{}`", rewritten), + }) + .to_string() +} + fn print_allow() { let _ = writeln!(io::stdout(), r#"{{"decision":"allow"}}"#); } diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 439dbfefa6..6930e81a8e 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -18,7 +18,8 @@ use super::constants::{ DROID_HOOK_COMMAND, DROID_SETTINGS_FILE, GEMINI_HOOK_FILE, HERMES_DIR, HERMES_PLUGINS_SUBDIR, HERMES_PLUGIN_INIT_FILE, HERMES_PLUGIN_MANIFEST_FILE, HERMES_PLUGIN_NAME, HOOKS_JSON, HOOKS_SUBDIR, 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, + PI_PLUGIN_FILE, PRE_TOOL_USE_KEY, REWRITE_HOOK_FILE, SETTINGS_JSON, VIBE_BASH_MATCH, VIBE_DIR, + VIBE_HOOKS_FILE, VIBE_HOOK_COMMAND, VIBE_HOOK_NAME, VIBE_PROMPTS_SUBDIR, VIBE_PROMPT_FILE, }; use super::integrity; use super::is_claude_hook_command; @@ -4446,6 +4447,311 @@ fn uninstall_gemini(ctx: InitContext) -> Result> { Ok(removed) } +// ── Vibe integration ──────────────────────────────────────── + +fn resolve_vibe_dir() -> Result { + resolve_home_subdir(VIBE_DIR) +} + +/// Entry point for `rtk init -g --agent vibe`. +/// +/// Installs a `pre_tool` hook into `~/.vibe/hooks.toml` (Vibe CLI's hook +/// registry, see https://docs.mistral.ai/vibe/code/cli/hooks) that routes +/// bash tool calls through the native `rtk hook vibe` binary. When not +/// `hook_only`, also drops an `~/.vibe/prompts/rtk.md` system prompt file +/// as a belt-and-suspenders fallback if the hook is disabled. +pub fn run_vibe_mode( + global: bool, + hook_only: bool, + patch_mode: PatchMode, + ctx: InitContext, +) -> Result<()> { + if !global { + anyhow::bail!("Vibe support is global-only. Use: rtk init -g --agent vibe"); + } + let vibe_dir = resolve_vibe_dir()?; + run_vibe_mode_at(&vibe_dir, hook_only, patch_mode, ctx) +} + +fn run_vibe_mode_at( + vibe_dir: &Path, + hook_only: bool, + patch_mode: PatchMode, + ctx: InitContext, +) -> Result<()> { + let InitContext { dry_run, .. } = ctx; + if !dry_run { + fs::create_dir_all(vibe_dir) + .with_context(|| format!("Failed to create Vibe config dir: {}", vibe_dir.display()))?; + } + + let hooks_path = vibe_dir.join(VIBE_HOOKS_FILE); + patch_vibe_hooks_toml(&hooks_path, patch_mode, ctx)?; + + if !hook_only { + let prompts_dir = vibe_dir.join(VIBE_PROMPTS_SUBDIR); + if !dry_run { + fs::create_dir_all(&prompts_dir).with_context(|| { + format!("Failed to create prompts dir: {}", prompts_dir.display()) + })?; + } + let prompt_path = prompts_dir.join(VIBE_PROMPT_FILE); + write_if_changed(&prompt_path, RTK_SLIM, VIBE_PROMPT_FILE, ctx)?; + } + + if dry_run { + print_dry_run_footer(); + } else { + println!("\nMistral Vibe CLI hook installed (global).\n"); + println!(" Hook registry: {}", hooks_path.display()); + if !hook_only { + println!( + " Prompt: {}", + vibe_dir + .join(VIBE_PROMPTS_SUBDIR) + .join(VIBE_PROMPT_FILE) + .display() + ); + } + println!(" Restart Vibe. Test with: git status\n"); + } + Ok(()) +} + +/// Append the RTK `[[hooks]]` entry to `~/.vibe/hooks.toml` if not already present. +/// +/// Uses append-based patching (string level) rather than parse-serialize round-trip +/// to preserve any user comments and formatting in the file. +fn patch_vibe_hooks_toml(hooks_path: &Path, patch_mode: PatchMode, ctx: InitContext) -> Result<()> { + let InitContext { verbose, dry_run } = ctx; + + let existing = if hooks_path.exists() { + fs::read_to_string(hooks_path) + .with_context(|| format!("Failed to read {}", hooks_path.display()))? + } else { + String::new() + }; + + if vibe_hooks_toml_has_rtk(&existing) { + if verbose > 0 { + eprintln!("Vibe hooks.toml already has RTK hook"); + } + return Ok(()); + } + + if patch_mode == PatchMode::Skip { + println!( + "\nManual setup needed: add RTK hook to {}\n\ + See: https://github.com/rtk-ai/rtk#mistral-vibe", + hooks_path.display() + ); + return Ok(()); + } + + if patch_mode == PatchMode::Ask { + if dry_run { + println!( + "[dry-run] would prompt before patching {}", + hooks_path.display() + ); + } else { + print!("Patch {} with RTK hook? [y/N] ", hooks_path.display()); + std::io::stdout().flush().ok(); + let mut answer = String::new(); + std::io::stdin().read_line(&mut answer).ok(); + if !matches!(answer.trim().to_lowercase().as_str(), "y" | "yes") { + println!( + "Skipped. Re-run with --auto-patch, or add the hook manually to {}", + hooks_path.display() + ); + return Ok(()); + } + } + } + + let entry = vibe_hook_entry(); + let new_content = if existing.is_empty() { + entry.clone() + } else if existing.ends_with("\n\n") { + format!("{existing}{entry}") + } else if existing.ends_with('\n') { + format!("{existing}\n{entry}") + } else { + format!("{existing}\n\n{entry}") + }; + + if dry_run { + println!( + "[dry-run] would patch Vibe hooks.toml: {}", + hooks_path.display() + ); + if verbose > 0 { + println!("[dry-run] appended entry:\n{entry}"); + } + } else { + atomic_write(hooks_path, &new_content) + .with_context(|| format!("Failed to write {}", hooks_path.display()))?; + } + Ok(()) +} + +/// TOML entry emitted for the Vibe pre_tool hook. Mirrors the shape documented +/// at https://docs.mistral.ai/vibe/code/cli/hooks. +fn vibe_hook_entry() -> String { + format!( + r#"[[hooks]] +name = "{name}" +type = "pre_tool" +match = "{match_glob}" +command = "{command}" +timeout = 10.0 +strict = false +description = "Rewrite bash commands through the rtk proxy to save tokens." +"#, + name = VIBE_HOOK_NAME, + match_glob = VIBE_BASH_MATCH, + command = VIBE_HOOK_COMMAND, + ) +} + +/// Detect an existing RTK entry by looking for the hook `name` field. Scanning +/// the raw string is enough because `name` is required by Vibe and must be +/// unique, so a substring match is both necessary and sufficient. +fn vibe_hooks_toml_has_rtk(content: &str) -> bool { + let needle = format!(r#"name = "{VIBE_HOOK_NAME}""#); + content.contains(&needle) +} + +/// Public entry point for `rtk init -g --agent vibe --uninstall`. +pub fn uninstall_vibe(ctx: InitContext) -> Result<()> { + let InitContext { dry_run, .. } = ctx; + let vibe_dir = match resolve_vibe_dir() { + Ok(d) => d, + Err(_) => return Ok(()), + }; + let removed = uninstall_vibe_at(&vibe_dir, ctx)?; + + if removed.is_empty() { + println!("RTK Vibe support was not installed (nothing to remove)"); + } else { + let header = if dry_run { + "[dry-run] would uninstall RTK for Mistral Vibe CLI:" + } else { + "RTK uninstalled for Mistral Vibe CLI:" + }; + println!("{}", header); + for item in removed { + println!(" - {}", item); + } + if !dry_run { + println!("\nRestart Vibe CLI to apply changes."); + } + } + + if dry_run { + print_dry_run_footer(); + } + Ok(()) +} + +/// Remove the RTK hook entry (and, when non-empty, the surrounding blank +/// lines) from `~/.vibe/hooks.toml` and the sibling `~/.vibe/prompts/rtk.md` +/// prompt file. Leaves any other user-declared hooks intact. +fn uninstall_vibe_at(vibe_dir: &Path, ctx: InitContext) -> Result> { + let InitContext { verbose, dry_run } = ctx; + let mut removed = Vec::new(); + + let prompt_path = vibe_dir.join(VIBE_PROMPTS_SUBDIR).join(VIBE_PROMPT_FILE); + if prompt_path.exists() { + if dry_run { + println!( + "[dry-run] would remove Vibe RTK prompt: {}", + prompt_path.display() + ); + } else { + fs::remove_file(&prompt_path) + .with_context(|| format!("Failed to remove {}", prompt_path.display()))?; + } + removed.push(format!("Vibe prompt: {}", prompt_path.display())); + } + + let hooks_path = vibe_dir.join(VIBE_HOOKS_FILE); + if hooks_path.exists() { + let content = fs::read_to_string(&hooks_path) + .with_context(|| format!("Failed to read {}", hooks_path.display()))?; + if let Some(new_content) = strip_vibe_rtk_entry(&content) { + if dry_run { + println!( + "[dry-run] would remove RTK hook from Vibe hooks.toml: {}", + hooks_path.display() + ); + } else if new_content.trim().is_empty() { + fs::remove_file(&hooks_path) + .with_context(|| format!("Failed to remove {}", hooks_path.display()))?; + } else { + atomic_write(&hooks_path, &new_content) + .with_context(|| format!("Failed to write {}", hooks_path.display()))?; + } + removed.push(format!( + "Vibe hooks.toml: removed RTK entry ({})", + hooks_path.display() + )); + } + } + + if verbose > 0 && !removed.is_empty() { + eprintln!("Vibe artifacts removed"); + } + + Ok(removed) +} + +/// Extract and drop the `[[hooks]]` block whose `name = "rtk-rewrite"` field +/// is set. Returns `None` when the entry is absent, `Some(new_content)` after +/// removal (with surrounding blank lines collapsed). The scan walks `[[hooks]]` +/// section boundaries — anything else in the file is preserved verbatim. +fn strip_vibe_rtk_entry(content: &str) -> Option { + let needle = format!(r#"name = "{VIBE_HOOK_NAME}""#); + if !content.contains(&needle) { + return None; + } + + let lines: Vec<&str> = content.lines().collect(); + let mut sections: Vec<(usize, usize)> = Vec::new(); + let mut current_start: Option = None; + for (i, line) in lines.iter().enumerate() { + let trimmed = line.trim_start(); + if trimmed.starts_with("[[hooks]]") || trimmed.starts_with('[') { + if let Some(start) = current_start.take() { + sections.push((start, i)); + } + if trimmed.starts_with("[[hooks]]") { + current_start = Some(i); + } + } + } + if let Some(start) = current_start { + sections.push((start, lines.len())); + } + + let target = sections + .iter() + .find(|(start, end)| lines[*start..*end].iter().any(|l| l.contains(&needle)))?; + + let mut kept: Vec<&str> = Vec::with_capacity(lines.len()); + kept.extend(&lines[..target.0]); + kept.extend(&lines[target.1..]); + + let mut out = kept.join("\n"); + while out.contains("\n\n\n") { + out = out.replace("\n\n\n", "\n\n"); + } + if !out.is_empty() && !out.ends_with('\n') { + out.push('\n'); + } + Some(out) +} + // ── Copilot integration ───────────────────────────────────── // Single PascalCase `PreToolUse` entry, shared by VS Code Copilot Chat and @@ -7912,4 +8218,135 @@ mod tests { hook_path.display() ); } + + // ── Vibe tests ──────────────────────────────────────────── + + #[test] + fn test_vibe_detects_rtk_entry_by_name_field() { + assert!(!vibe_hooks_toml_has_rtk("")); + assert!(!vibe_hooks_toml_has_rtk("[[hooks]]\nname = \"other\"\n")); + assert!(vibe_hooks_toml_has_rtk( + "[[hooks]]\nname = \"rtk-rewrite\"\n" + )); + } + + #[test] + fn test_vibe_hook_entry_shape_matches_docs() { + let entry = vibe_hook_entry(); + assert!(entry.contains("[[hooks]]")); + assert!(entry.contains(r#"name = "rtk-rewrite""#)); + assert!(entry.contains(r#"type = "pre_tool""#)); + assert!(entry.contains(r#"match = "bash""#)); + assert!(entry.contains(r#"command = "rtk hook vibe""#)); + assert!(entry.contains("strict = false")); + } + + #[test] + fn test_vibe_strip_returns_none_when_entry_absent() { + let content = "[[hooks]]\nname = \"other\"\ntype = \"post_tool\"\n"; + assert!(strip_vibe_rtk_entry(content).is_none()); + } + + #[test] + fn test_vibe_strip_removes_only_rtk_entry() { + let content = "[[hooks]]\nname = \"user-audit\"\ntype = \"post_tool\"\nmatch = \"*\"\ncommand = \"audit.py\"\n\n[[hooks]]\nname = \"rtk-rewrite\"\ntype = \"pre_tool\"\nmatch = \"bash\"\ncommand = \"rtk hook vibe\"\n"; + let stripped = strip_vibe_rtk_entry(content).expect("expected removal"); + assert!(stripped.contains(r#"name = "user-audit""#)); + assert!(!stripped.contains(r#"name = "rtk-rewrite""#)); + assert!(!stripped.contains("rtk hook vibe")); + } + + #[test] + fn test_vibe_install_creates_hook_and_prompt() { + let temp = TempDir::new().unwrap(); + let vibe_dir = temp.path().join(".vibe"); + run_vibe_mode_at(&vibe_dir, false, PatchMode::Auto, InitContext::default()).unwrap(); + + let hooks_content = fs::read_to_string(vibe_dir.join(VIBE_HOOKS_FILE)).unwrap(); + assert!(hooks_content.contains(r#"name = "rtk-rewrite""#)); + assert!(hooks_content.contains(r#"command = "rtk hook vibe""#)); + + let prompt_path = vibe_dir.join(VIBE_PROMPTS_SUBDIR).join(VIBE_PROMPT_FILE); + assert!(prompt_path.exists()); + } + + #[test] + fn test_vibe_install_is_idempotent() { + let temp = TempDir::new().unwrap(); + let vibe_dir = temp.path().join(".vibe"); + run_vibe_mode_at(&vibe_dir, false, PatchMode::Auto, InitContext::default()).unwrap(); + run_vibe_mode_at(&vibe_dir, false, PatchMode::Auto, InitContext::default()).unwrap(); + + let hooks_content = fs::read_to_string(vibe_dir.join(VIBE_HOOKS_FILE)).unwrap(); + assert_eq!(hooks_content.matches("rtk-rewrite").count(), 1); + } + + #[test] + fn test_vibe_install_preserves_existing_user_hook() { + let temp = TempDir::new().unwrap(); + let vibe_dir = temp.path().join(".vibe"); + fs::create_dir_all(&vibe_dir).unwrap(); + let user_hook = "[[hooks]]\nname = \"user-audit\"\ntype = \"post_tool\"\nmatch = \"*\"\ncommand = \"audit.py\"\n"; + fs::write(vibe_dir.join(VIBE_HOOKS_FILE), user_hook).unwrap(); + + run_vibe_mode_at(&vibe_dir, false, PatchMode::Auto, InitContext::default()).unwrap(); + + let hooks_content = fs::read_to_string(vibe_dir.join(VIBE_HOOKS_FILE)).unwrap(); + assert!(hooks_content.contains(r#"name = "user-audit""#)); + assert!(hooks_content.contains(r#"name = "rtk-rewrite""#)); + } + + #[test] + fn test_vibe_hook_only_skips_prompt_file() { + let temp = TempDir::new().unwrap(); + let vibe_dir = temp.path().join(".vibe"); + run_vibe_mode_at(&vibe_dir, true, PatchMode::Auto, InitContext::default()).unwrap(); + + assert!(vibe_dir.join(VIBE_HOOKS_FILE).exists()); + assert!(!vibe_dir + .join(VIBE_PROMPTS_SUBDIR) + .join(VIBE_PROMPT_FILE) + .exists()); + } + + #[test] + fn test_vibe_uninstall_removes_only_rtk_entry_and_prompt() { + let temp = TempDir::new().unwrap(); + let vibe_dir = temp.path().join(".vibe"); + fs::create_dir_all(&vibe_dir).unwrap(); + let user_hook = "[[hooks]]\nname = \"user-audit\"\ntype = \"post_tool\"\nmatch = \"*\"\ncommand = \"audit.py\"\n"; + fs::write(vibe_dir.join(VIBE_HOOKS_FILE), user_hook).unwrap(); + + run_vibe_mode_at(&vibe_dir, false, PatchMode::Auto, InitContext::default()).unwrap(); + assert!(vibe_dir + .join(VIBE_PROMPTS_SUBDIR) + .join(VIBE_PROMPT_FILE) + .exists()); + + let removed_first = uninstall_vibe_at(&vibe_dir, InitContext::default()).unwrap(); + let removed_second = uninstall_vibe_at(&vibe_dir, InitContext::default()).unwrap(); + + assert_eq!(removed_first.len(), 2); + assert!(removed_second.is_empty()); + assert!(!vibe_dir + .join(VIBE_PROMPTS_SUBDIR) + .join(VIBE_PROMPT_FILE) + .exists()); + + let remaining = fs::read_to_string(vibe_dir.join(VIBE_HOOKS_FILE)).unwrap(); + assert!(remaining.contains(r#"name = "user-audit""#)); + assert!(!remaining.contains(r#"name = "rtk-rewrite""#)); + } + + #[test] + fn test_vibe_uninstall_removes_hooks_file_when_no_other_hooks() { + let temp = TempDir::new().unwrap(); + let vibe_dir = temp.path().join(".vibe"); + run_vibe_mode_at(&vibe_dir, false, PatchMode::Auto, InitContext::default()).unwrap(); + assert!(vibe_dir.join(VIBE_HOOKS_FILE).exists()); + + uninstall_vibe_at(&vibe_dir, InitContext::default()).unwrap(); + + assert!(!vibe_dir.join(VIBE_HOOKS_FILE).exists()); + } } diff --git a/src/hooks/permissions.rs b/src/hooks/permissions.rs index 516952bab0..a23fd2d307 100644 --- a/src/hooks/permissions.rs +++ b/src/hooks/permissions.rs @@ -36,6 +36,7 @@ pub enum Host { Cursor, Gemini, Droid, + Vibe, } pub fn check_command_for(cmd: &str, host: Host) -> PermissionVerdict { @@ -44,6 +45,7 @@ pub fn check_command_for(cmd: &str, host: Host) -> PermissionVerdict { Host::Cursor => load_cursor_rules(), Host::Gemini => load_gemini_rules(), Host::Droid => load_droid_rules(), + Host::Vibe => (Vec::new(), Vec::new(), Vec::new()), }; check_command_with_rules(cmd, &deny_rules, &ask_rules, &allow_rules) } diff --git a/src/main.rs b/src/main.rs index d1e0269f5a..b29cf0769b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -55,6 +55,8 @@ pub enum AgentTarget { Hermes, /// Factory Droid CLI Droid, + /// Mistral Vibe CLI + Vibe, } #[derive(Parser)] @@ -866,6 +868,8 @@ enum HookCommands { Copilot, /// Process Factory Droid PreToolUse hook (reads JSON from stdin) Droid, + /// Process Mistral Vibe CLI pre_tool hook (reads JSON from stdin) + Vibe, /// Check how a command would be rewritten by the hook engine (dry-run) Check { /// Target agent @@ -1566,6 +1570,8 @@ where uninstall_hermes(ctx) } else if agent == Some(AgentTarget::Droid) { hooks::init::uninstall_droid(global, ctx) + } else if agent == Some(AgentTarget::Vibe) { + hooks::init::uninstall_vibe(ctx) } else { let cursor = agent == Some(AgentTarget::Cursor); let pi = agent == Some(AgentTarget::Pi); @@ -2068,6 +2074,15 @@ fn run_cli() -> Result { hooks::init::run_hermes_mode(ctx)?; } else if agent == Some(AgentTarget::Droid) { hooks::init::run_droid_mode(global, ctx)?; + } else if agent == Some(AgentTarget::Vibe) { + let patch_mode = if auto_patch { + hooks::init::PatchMode::Auto + } else if no_patch { + hooks::init::PatchMode::Skip + } else { + hooks::init::PatchMode::Ask + }; + hooks::init::run_vibe_mode(global, hook_only, patch_mode, ctx)?; } else { let install_opencode = opencode; let install_claude = !opencode; @@ -2438,6 +2453,10 @@ fn run_cli() -> Result { hooks::hook_cmd::run_droid()?; 0 } + HookCommands::Vibe => { + hooks::hook_cmd::run_vibe()?; + 0 + } HookCommands::Check { agent: _, command } => { use crate::discover::registry::rewrite_command; let raw = command.join(" "); From 1847b07f7a87fecba7fe0e39ade3d360c897dd66 Mon Sep 17 00:00:00 2001 From: Xavier Pestel Date: Wed, 5 Aug 2026 13:55:09 +0200 Subject: [PATCH 13/22] =?UTF-8?q?fix(vibe):=20address=20PR=20review=20?= =?UTF-8?q?=E2=80=94=20exit=20code=20contract,=20tests,=20telemetry,=20doc?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses @aeppling's review on #3391: Blocking fixes: - run_vibe now returns Ok(()) on malformed JSON (matches run_droid / run_copilot / run_cursor pattern). Prior code violated the exit-code contract documented at src/hooks/README.md:100 — a bad payload exited non-zero and blocked the agent's command. Fixed via a match on serde_json::from_str with a stderr warning fallback. - Extract run_vibe_inner(input: &str) -> Option from run_vibe so the hook contract is unit-testable (mirrors run_droid_inner). Public run_vibe becomes a thin stdin/stdout wrapper. - Add 6 runtime tests exercising the hook contract: bash rewrite happy path, non-bash tool passthrough, empty command passthrough, malformed JSON returns None, unknown binary passthrough, substitution defers. Should-fix: - Telemetry agent detection: add ~/.vibe/hooks.toml to detect_hook_type() checks in src/core/telemetry.rs, plus the two test enum arrays so Vibe sessions no longer report as 'unknown' in rtk gain history. - Dead deny arm: add a comment on Host::Vibe in permissions.rs documenting that the empty-rules branch is defensive scaffolding for when Vibe ships native denylist/allowlist config we can honor. - Broken link: patch_vibe_hooks_toml skip-message now points at https://www.rtk-ai.app/guide/getting-started/supported-agents#mistral-vibe instead of a fragment that doesn't resolve. Nits addressed: - Install summary no longer prints 'hook installed' when the user chose PatchMode::Skip or declined the interactive prompt. patch_vibe_hooks_toml now returns a VibeHookPatchOutcome enum (Installed / AlreadyPresent / Skipped) and the caller gates the summary on it. - Document the string-spacing tradeoff on vibe_hooks_toml_has_rtk: a reformatted 'name="rtk-rewrite"' would defeat idempotency, acceptable because we control the writer and toml_edit round-trip would clobber user comments. - Fix stale line in src/hooks/README.md 'Adding New Functionality': hook_check.rs::maybe_warn() only checks the Claude Code hook now, not every agent. Documentation: - docs/guide/getting-started/supported-agents.md: frontmatter now lists Mistral Vibe, drop 'planned' from the intro, tier table row flipped from 'Planned (#800)' to 'Rust binary (pre_tool) / Yes', replace the ### Mistral Vibe (planned) placeholder with a full user-facing section modeled on Factory Droid (install/uninstall commands, hook mechanism, permission semantics, idempotency contract). - hooks/README.md: agent count 9 -> 10, add Vibe entry to Directory Structure list, add Vibe row to Supported Agents table, add '### Mistral Vibe (Rust Binary)' entry to the JSON Formats section showing the pre_tool input shape and rewrite response shape. - src/hooks/README.md: agent count 5 -> 6, add Vibe row to per-host ask-support table. - README.md: '15 AI coding tools' -> '16'. No behavior change for existing agents. --- README.md | 2 +- .../guide/getting-started/supported-agents.md | 29 +++++- hooks/README.md | 30 +++++- src/core/telemetry.rs | 5 +- src/hooks/README.md | 5 +- src/hooks/hook_cmd.rs | 91 ++++++++++++++++--- src/hooks/init.rs | 43 +++++++-- src/hooks/permissions.rs | 5 + 8 files changed, 177 insertions(+), 33 deletions(-) diff --git a/README.md b/README.md index a2bb93d7bb..420a237c2e 100644 --- a/README.md +++ b/README.md @@ -381,7 +381,7 @@ rtk init -g ## Supported AI Tools -RTK supports 15 AI coding tools. Each integration rewrites shell commands to `rtk` equivalents, reducing the bash output the agent reads where the agent supports command interception. +RTK supports 16 AI coding tools. Each integration rewrites shell commands to `rtk` equivalents, reducing the bash output the agent reads where the agent supports command interception. | Tool | Install | Method | |------|---------|--------| diff --git a/docs/guide/getting-started/supported-agents.md b/docs/guide/getting-started/supported-agents.md index 3906e364cb..6af4995e51 100644 --- a/docs/guide/getting-started/supported-agents.md +++ b/docs/guide/getting-started/supported-agents.md @@ -1,13 +1,13 @@ --- title: Supported Agents -description: How to integrate RTK with Claude Code, Cursor, Copilot, Cline, Windsurf, Codex, OpenCode, Hermes, Kilo Code, Antigravity, and Factory Droid +description: How to integrate RTK with Claude Code, Cursor, Copilot, Cline, Windsurf, Codex, OpenCode, Hermes, Kilo Code, Antigravity, Factory Droid, and Mistral Vibe sidebar: order: 3 --- # Supported Agents -RTK supports all major AI coding agents across 3 integration tiers. Mistral Vibe support is planned. +RTK supports all major AI coding agents across 3 integration tiers. ## How it works @@ -43,7 +43,7 @@ Agent runs "cargo test" | Codex CLI | AGENTS.md instructions | N/A | | Kilo Code | Rules file (prompt-level) | N/A | | Google Antigravity | Rules file (prompt-level) | N/A | -| Mistral Vibe | Planned ([#800](https://github.com/rtk-ai/rtk/issues/800)) | Pending upstream | +| Mistral Vibe | Rust binary (`pre_tool`) | Yes | ## Installation by agent @@ -197,9 +197,28 @@ rtk init --agent antigravity # creates .agents/rules/antigravity-rtk-rules.md Antigravity reads `.agents/rules/` as custom instructions. RTK adds guidance telling Antigravity to prefer `rtk ` over raw commands. -### Mistral Vibe (planned) +### Mistral Vibe -Support is blocked on upstream `BeforeToolCallback` ([mistral-vibe#531](https://github.com/mistralai/mistral-vibe/issues/531)). Tracked in [#800](https://github.com/rtk-ai/rtk/issues/800). +```bash +rtk init -g --agent vibe # user-scoped (~/.vibe/hooks.toml) +rtk init -g --agent vibe --hook-only # skip the ~/.vibe/prompts/rtk.md prompt file +``` + +Installs a `pre_tool` hook entry (`match = "bash"`, `command = "rtk hook vibe"`, `strict = false`) into `~/.vibe/hooks.toml`, following the contract at [docs.mistral.ai/vibe/code/cli/hooks](https://docs.mistral.ai/vibe/code/cli/hooks). Vibe invokes the native `rtk hook vibe` binary before every bash tool call; RTK reads Vibe's stdin JSON payload and emits `{"hook_specific_output": {"tool_input": {"command": "rtk ..."}}}` to rewrite the command in place. The Vibe UI surfaces `[rtk-rewrite] rtk: rewrote to \`…\`` via RTK's `system_message` field so the rewrite is visible. + +Unlike Droid, Vibe does not yet expose a denylist / allowlist surface in `hooks.toml` for RTK to honor. RTK therefore rewrites every bash command it knows how to compress and defers to Vibe's own permission prompt on the rewritten command; commands RTK doesn't handle pass through unchanged. `strict = false` ensures a hook crash degrades to a warning rather than blocking the tool call. + +Alongside the hook, RTK drops a system prompt at `~/.vibe/prompts/rtk.md` describing the RTK conventions to Vibe as a belt-and-suspenders fallback. Use `--hook-only` to skip it. + +Install is global-only (Vibe's hook registry is user-scoped). Re-running the installer is a no-op; the RTK entry is detected by its `name = "rtk-rewrite"` field and never duplicated. + +Uninstall: + +```bash +rtk init -g --agent vibe --uninstall +``` + +Strips only RTK's `[[hooks]]` block and the `~/.vibe/prompts/rtk.md` file. Any other user-declared hooks in `hooks.toml` are preserved byte-for-byte. `hooks.toml` is removed only when the RTK entry was the sole content. ## Integration tiers explained diff --git a/hooks/README.md b/hooks/README.md index a79f64e17d..77bd3f9a63 100644 --- a/hooks/README.md +++ b/hooks/README.md @@ -4,7 +4,7 @@ **Deployed hook artifacts** — the actual files installed on user machines by `rtk init`. These are shell scripts, TypeScript plugins, and rules files that run outside the Rust binary. They are **thin delegates**: parse agent-specific JSON, call `rtk rewrite` as a subprocess, format agent-specific response. Zero filtering logic lives here. -Owns: per-agent hook scripts and configuration files for 9 supported agents (Claude Code, Copilot, Cursor, Cline, Windsurf, Codex, OpenCode, Hermes, Pi). +Owns: per-agent hook scripts and configuration files for 10 supported agents (Claude Code, Copilot, Cursor, Cline, Windsurf, Codex, OpenCode, Hermes, Pi, Mistral Vibe). 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`). @@ -42,6 +42,7 @@ Each agent subdirectory has its own README with hook-specific details: - **[`opencode/`](opencode/README.md)** — TypeScript plugin, `zx` library, `tool.execute.before` event, in-place mutation - **[`pi/`](pi/README.md)** — TypeScript extension, `tool_call` event, `isToolCallEventType` guard, in-place mutation, `~/.pi/agent/extensions/` - **[`hermes/`](hermes/README.md)** — Python plugin, `pre_tool_call` hook, in-place terminal command mutation +- **Mistral Vibe** — Native Rust binary (`rtk hook vibe`), `pre_tool` hook in `~/.vibe/hooks.toml`, `hook_specific_output.tool_input` rewrite (no dedicated subdirectory — the hook is a subcommand of the RTK binary itself, installed via `src/hooks/init.rs::run_vibe_mode`) ## Supported Agents @@ -58,6 +59,7 @@ Each agent subdirectory has its own README with hook-specific details: | OpenCode | TypeScript plugin (`tool.execute.before`) | In-place mutation | Yes | | Pi | TypeScript extension (`tool_call` event) | In-place mutation | Yes | | Hermes | Python plugin (`pre_tool_call`) | In-place mutation | Yes | +| Mistral Vibe | Rust binary (`rtk hook vibe`) | Transparent rewrite | Yes (`hook_specific_output.tool_input`) | ## JSON Formats by Agent @@ -157,6 +159,32 @@ Returns `{}` when no rewrite (Cursor requires JSON for all paths). **No rewrite**: `{"decision": "allow"}` +### Mistral Vibe (Rust Binary) + +**Input** (stdin): + +```json +{ + "tool_name": "bash", + "tool_input": { "command": "git status" }, + "hook_event_name": "pre_tool", + "session_id": "..." +} +``` + +**Output** (when rewritten): + +```json +{ + "hook_specific_output": { + "tool_input": { "command": "rtk git status" } + }, + "system_message": "rtk: rewrote to `rtk git status`" +} +``` + +**No rewrite**: exit 0 with empty stdout (Vibe's contract for "no opinion" from a `pre_tool` hook). + ### OpenCode (TypeScript Plugin) Mutates `args.command` in-place via the zx library: diff --git a/src/core/telemetry.rs b/src/core/telemetry.rs index acf3e10493..08776c7b35 100644 --- a/src/core/telemetry.rs +++ b/src/core/telemetry.rs @@ -362,6 +362,7 @@ fn detect_hook_type() -> String { (home.join(".gemini/hooks/rtk-hook.sh"), "gemini"), (home.join(".codex/AGENTS.md"), "codex"), (home.join(".cursor/hooks/rtk-rewrite.json"), "cursor"), + (home.join(".vibe/hooks.toml"), "vibe"), ]; for (path, name) in &checks { @@ -576,7 +577,7 @@ mod tests { assert!(stats.low_savings_commands.len() <= 5); assert!((0.0..=100.0).contains(&stats.avg_savings_per_command)); assert!( - ["claude", "gemini", "codex", "cursor", "copilot", "none", "unknown"] + ["claude", "gemini", "codex", "cursor", "copilot", "vibe", "none", "unknown"] .iter() .any(|&h| stats.hook_type.starts_with(h)), "Unexpected hook type: {}", @@ -588,7 +589,7 @@ mod tests { fn test_detect_hook_type_returns_known() { let ht = detect_hook_type(); assert!( - ["claude", "gemini", "codex", "cursor", "copilot", "none", "unknown"] + ["claude", "gemini", "codex", "cursor", "copilot", "vibe", "none", "unknown"] .contains(&ht.as_str()), "Unexpected hook type: {}", ht diff --git a/src/hooks/README.md b/src/hooks/README.md index 67a0bf3cc0..586105c8ae 100644 --- a/src/hooks/README.md +++ b/src/hooks/README.md @@ -6,7 +6,7 @@ The **lifecycle management** layer for LLM agent hooks: install, uninstall, verify integrity, audit usage, and manage trust. This component creates and maintains the hook artifacts that live in `hooks/` (root), but does **not** execute rewrite logic itself — that lives in `discover/registry`. -Owns: `rtk init` installation flows (5 agents via `AgentTarget` enum + 3 special modes: Gemini, Codex, OpenCode), SHA-256 integrity verification, hook version checking, audit log analysis, `rtk rewrite` CLI entry point, and TOML filter trust management. +Owns: `rtk init` installation flows (6 agents via `AgentTarget` enum, now including Mistral Vibe + 3 special modes: Gemini, Codex, OpenCode), SHA-256 integrity verification, hook version checking, audit log analysis, `rtk rewrite` CLI entry point, and TOML filter trust management. Does **not** own: the deployed hook scripts themselves (that's `hooks/`), the rewrite pattern registry (that's `discover/`), or command filtering (that's `cmds/`). @@ -90,6 +90,7 @@ Rules are loaded from all Claude Code `settings.json` files (project + global, i | Gemini CLI (rtk hook gemini) | No (allow/deny only) | allow (limitation — no ask mode in Gemini) | | Copilot CLI (rtk hook copilot) | No updatedInput | deny-with-suggestion (unchanged) | | Codex | ask parsed but no-op | allow (limitation — fails open) | +| Mistral Vibe (rtk hook vibe) | No native ask surface | passthrough — Vibe's own approval prompt fires on the rewritten command | ### Implementation @@ -102,4 +103,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`), add a processor function in `hook_cmd.rs`, (3) add the agent's hook file path to `hook_check.rs` for validation, and (4) update `integrity.rs` with the expected hash for the new hook file. 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 `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. diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index aa6b214eed..c0ae051ab4 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -363,39 +363,45 @@ pub fn run_gemini() -> Result<()> { /// - Deny: emit `{"decision": "deny", "reason": "..."}`. pub fn run_vibe() -> Result<()> { let input = read_stdin_limited()?; + if let Some(output) = run_vibe_inner(&input) { + let _ = writeln!(io::stdout(), "{output}"); + } + Ok(()) +} - let json: Value = serde_json::from_str(&input).context("Failed to parse hook input as JSON")?; +fn run_vibe_inner(input: &str) -> Option { + let json: Value = match serde_json::from_str(input) { + Ok(v) => v, + Err(e) => { + let _ = writeln!(io::stderr(), "[rtk hook] Failed to parse JSON input: {e}"); + return None; + } + }; let tool_name = json.get("tool_name").and_then(|v| v.as_str()).unwrap_or(""); - if tool_name != "bash" { - return Ok(()); + return None; } let cmd = json .pointer("/tool_input/command") .and_then(|v| v.as_str()) .unwrap_or(""); - if cmd.is_empty() { - return Ok(()); + return None; } match decide_hook_action(cmd, permissions::Host::Vibe) { HookDecision::Deny => { - let _ = writeln!( - io::stdout(), - r#"{{"decision":"deny","reason":"Blocked by RTK permission rule"}}"# - ); + audit_log("deny", cmd, ""); + Some(r#"{"decision":"deny","reason":"Blocked by RTK permission rule"}"#.to_string()) } HookDecision::AllowRewrite(ref rewritten) | HookDecision::AskRewrite(ref rewritten) => { audit_log("rewrite", cmd, rewritten); - let _ = writeln!(io::stdout(), "{}", vibe_rewrite_json(rewritten)); + Some(vibe_rewrite_json(rewritten)) } - HookDecision::Defer => {} + HookDecision::Defer => None, } - - Ok(()) } fn vibe_rewrite_json(rewritten: &str) -> String { @@ -2029,4 +2035,63 @@ mod tests { let input = droid_input("Execute", "definitely-not-a-real-binary --foo"); assert!(run_droid_inner(&input).is_none()); } + + fn vibe_input(tool: &str, cmd: &str) -> String { + json!({ + "session_id": "abc123", + "hook_event_name": "pre_tool", + "tool_name": tool, + "tool_input": { "command": cmd } + }) + .to_string() + } + + #[test] + fn test_vibe_rewrites_bash_command() { + let input = vibe_input("bash", "git status"); + let out = run_vibe_inner(&input).expect("rewrite expected"); + let v: Value = serde_json::from_str(&out).unwrap(); + let rewritten = v + .pointer("/hook_specific_output/tool_input/command") + .and_then(|c| c.as_str()) + .unwrap_or(""); + assert!( + rewritten.starts_with("rtk "), + "expected rtk-prefixed rewrite, got `{rewritten}`" + ); + assert!( + v.get("system_message").is_some(), + "expected system_message for UI visibility" + ); + } + + #[test] + fn test_vibe_ignores_non_bash_tool() { + let input = vibe_input("read_file", "irrelevant"); + assert!(run_vibe_inner(&input).is_none()); + } + + #[test] + fn test_vibe_empty_command_passthrough() { + let input = vibe_input("bash", ""); + assert!(run_vibe_inner(&input).is_none()); + } + + #[test] + fn test_vibe_malformed_json_returns_none() { + assert!(run_vibe_inner("not json at all").is_none()); + assert!(run_vibe_inner("{ unterminated").is_none()); + } + + #[test] + fn test_vibe_unknown_binary_passthrough() { + let input = vibe_input("bash", "definitely-not-a-real-binary --foo"); + assert!(run_vibe_inner(&input).is_none()); + } + + #[test] + fn test_vibe_substitution_defers() { + let input = vibe_input("bash", "echo $(rm -rf /)"); + assert!(run_vibe_inner(&input).is_none()); + } } diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 6930e81a8e..89024219dc 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -4486,7 +4486,7 @@ fn run_vibe_mode_at( } let hooks_path = vibe_dir.join(VIBE_HOOKS_FILE); - patch_vibe_hooks_toml(&hooks_path, patch_mode, ctx)?; + let hook_outcome = patch_vibe_hooks_toml(&hooks_path, patch_mode, ctx)?; if !hook_only { let prompts_dir = vibe_dir.join(VIBE_PROMPTS_SUBDIR); @@ -4501,8 +4501,13 @@ fn run_vibe_mode_at( if dry_run { print_dry_run_footer(); - } else { - println!("\nMistral Vibe CLI hook installed (global).\n"); + } else if hook_outcome != VibeHookPatchOutcome::Skipped { + let summary_verb = match hook_outcome { + VibeHookPatchOutcome::Installed => "installed", + VibeHookPatchOutcome::AlreadyPresent => "already present", + VibeHookPatchOutcome::Skipped => unreachable!(), + }; + println!("\nMistral Vibe CLI hook {summary_verb} (global).\n"); println!(" Hook registry: {}", hooks_path.display()); if !hook_only { println!( @@ -4518,11 +4523,24 @@ fn run_vibe_mode_at( Ok(()) } +/// Outcome of `patch_vibe_hooks_toml`. Distinguishes installed / already-present / +/// skipped so the caller can decide whether the "installed" summary is truthful. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum VibeHookPatchOutcome { + Installed, + AlreadyPresent, + Skipped, +} + /// Append the RTK `[[hooks]]` entry to `~/.vibe/hooks.toml` if not already present. /// /// Uses append-based patching (string level) rather than parse-serialize round-trip /// to preserve any user comments and formatting in the file. -fn patch_vibe_hooks_toml(hooks_path: &Path, patch_mode: PatchMode, ctx: InitContext) -> Result<()> { +fn patch_vibe_hooks_toml( + hooks_path: &Path, + patch_mode: PatchMode, + ctx: InitContext, +) -> Result { let InitContext { verbose, dry_run } = ctx; let existing = if hooks_path.exists() { @@ -4536,16 +4554,16 @@ fn patch_vibe_hooks_toml(hooks_path: &Path, patch_mode: PatchMode, ctx: InitCont if verbose > 0 { eprintln!("Vibe hooks.toml already has RTK hook"); } - return Ok(()); + return Ok(VibeHookPatchOutcome::AlreadyPresent); } if patch_mode == PatchMode::Skip { println!( "\nManual setup needed: add RTK hook to {}\n\ - See: https://github.com/rtk-ai/rtk#mistral-vibe", + See: https://www.rtk-ai.app/guide/getting-started/supported-agents#mistral-vibe", hooks_path.display() ); - return Ok(()); + return Ok(VibeHookPatchOutcome::Skipped); } if patch_mode == PatchMode::Ask { @@ -4564,7 +4582,7 @@ fn patch_vibe_hooks_toml(hooks_path: &Path, patch_mode: PatchMode, ctx: InitCont "Skipped. Re-run with --auto-patch, or add the hook manually to {}", hooks_path.display() ); - return Ok(()); + return Ok(VibeHookPatchOutcome::Skipped); } } } @@ -4592,7 +4610,7 @@ fn patch_vibe_hooks_toml(hooks_path: &Path, patch_mode: PatchMode, ctx: InitCont atomic_write(hooks_path, &new_content) .with_context(|| format!("Failed to write {}", hooks_path.display()))?; } - Ok(()) + Ok(VibeHookPatchOutcome::Installed) } /// TOML entry emitted for the Vibe pre_tool hook. Mirrors the shape documented @@ -4617,6 +4635,13 @@ description = "Rewrite bash commands through the rtk proxy to save tokens." /// Detect an existing RTK entry by looking for the hook `name` field. Scanning /// the raw string is enough because `name` is required by Vibe and must be /// unique, so a substring match is both necessary and sufficient. +/// +/// Tradeoff: matches the exact spacing `name = "rtk-rewrite"`. A reformatted +/// file (`name="rtk-rewrite"` or extra whitespace) would defeat idempotency +/// and cause a duplicate append on re-install. Acceptable because our own +/// installer only ever writes the canonical spacing, and the alternative +/// (parse-serialize round-trip via toml_edit) would clobber user comments +/// and formatting in the file. fn vibe_hooks_toml_has_rtk(content: &str) -> bool { let needle = format!(r#"name = "{VIBE_HOOK_NAME}""#); content.contains(&needle) diff --git a/src/hooks/permissions.rs b/src/hooks/permissions.rs index a23fd2d307..a95d3ecc7f 100644 --- a/src/hooks/permissions.rs +++ b/src/hooks/permissions.rs @@ -45,6 +45,11 @@ pub fn check_command_for(cmd: &str, host: Host) -> PermissionVerdict { Host::Cursor => load_cursor_rules(), Host::Gemini => load_gemini_rules(), Host::Droid => load_droid_rules(), + // Vibe stores hooks in ~/.vibe/hooks.toml and has no denylist / allowlist + // surface at the time of writing. Empty rules mean check_command_with_rules + // returns Default (treated as Ask by callers), and the Deny arm in + // hook_cmd::run_vibe is dead code kept as defensive scaffolding for when + // Vibe ships native permission config we can honor here. Host::Vibe => (Vec::new(), Vec::new(), Vec::new()), }; check_command_with_rules(cmd, &deny_rules, &ask_rules, &allow_rules) From 94ae76b2dacbdde0516e5d14b6a7a0361ab766bd Mon Sep 17 00:00:00 2001 From: Xavier Pestel Date: Wed, 5 Aug 2026 15:37:02 +0200 Subject: [PATCH 14/22] docs(vibe): add hooks/vibe/README.md and link from Directory Structure Every other agent with a dedicated hook implementation carries a hooks//README.md (see antigravity/cline/opencode/copilot/hermes for the shape). The initial Vibe commit skipped this, leaving Vibe as the odd one out in the hooks/ layout. - Add hooks/vibe/README.md following the Copilot template (Rust binary hook, no shell dependency). Documents the pre_tool hook location, input JSON shape, rewrite response, passthrough / deny behavior, and the belt-and-suspenders prompt fallback. - Fix hooks/README.md Directory Structure entry to point at vibe/README.md (previously claimed 'no dedicated subdirectory'). --- hooks/README.md | 2 +- hooks/vibe/README.md | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 hooks/vibe/README.md diff --git a/hooks/README.md b/hooks/README.md index 77bd3f9a63..6e9cd01a2a 100644 --- a/hooks/README.md +++ b/hooks/README.md @@ -42,7 +42,7 @@ Each agent subdirectory has its own README with hook-specific details: - **[`opencode/`](opencode/README.md)** — TypeScript plugin, `zx` library, `tool.execute.before` event, in-place mutation - **[`pi/`](pi/README.md)** — TypeScript extension, `tool_call` event, `isToolCallEventType` guard, in-place mutation, `~/.pi/agent/extensions/` - **[`hermes/`](hermes/README.md)** — Python plugin, `pre_tool_call` hook, in-place terminal command mutation -- **Mistral Vibe** — Native Rust binary (`rtk hook vibe`), `pre_tool` hook in `~/.vibe/hooks.toml`, `hook_specific_output.tool_input` rewrite (no dedicated subdirectory — the hook is a subcommand of the RTK binary itself, installed via `src/hooks/init.rs::run_vibe_mode`) +- **[`vibe/`](vibe/README.md)** — Rust binary hook (`rtk hook vibe`), `pre_tool` entry in `~/.vibe/hooks.toml`, `hook_specific_output.tool_input` rewrite plus `system_message` for UI visibility ## Supported Agents diff --git a/hooks/vibe/README.md b/hooks/vibe/README.md new file mode 100644 index 0000000000..d38cb73326 --- /dev/null +++ b/hooks/vibe/README.md @@ -0,0 +1,19 @@ +# Mistral Vibe Hooks + +> Part of [`hooks/`](../README.md) — see also [`src/hooks/`](../../src/hooks/README.md) for installation code + +## Specifics + +- Uses the `rtk hook vibe` Rust binary (not a shell script) -- no `jq` dependency +- `pre_tool` hook declared in `~/.vibe/hooks.toml` (user-scoped) with `match = "bash"` and `strict = false` +- Reads Vibe's stdin JSON payload (`tool_name`, `tool_input.command`, `hook_event_name`, `session_id`) +- Returns `hook_specific_output.tool_input.command` for transparent rewrite plus a `system_message` for UI visibility +- Non-bash tool / empty command / malformed JSON / RTK-unknown command → passthrough (exit 0, empty stdout) +- RTK permission deny → `{"decision":"deny","reason":"..."}` +- Alongside the hook, a system prompt at `~/.vibe/prompts/rtk.md` is installed as a belt-and-suspenders fallback (skip with `--hook-only`) +- Installed globally via `rtk init -g --agent vibe`; there is no project-scoped variant + +## Notes + +- This directory intentionally holds only this README — the hook is a subcommand of the RTK binary (`rtk hook vibe`), not a standalone script or plugin file, so nothing is deployed here +- Vibe hook contract reference: https://docs.mistral.ai/vibe/code/cli/hooks From 0430df48ab1321c9154b04b246a09f412deca567 Mon Sep 17 00:00:00 2001 From: Xavier Pestel Date: Wed, 5 Aug 2026 15:38:06 +0200 Subject: [PATCH 15/22] refactor(vibe): drop defensive-only comment on Host::Vibe arm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment was previously added in response to review point #5. Removed per follow-up feedback — the arm itself is self-explanatory in context alongside the other Host variants. --- src/hooks/permissions.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/hooks/permissions.rs b/src/hooks/permissions.rs index a95d3ecc7f..a23fd2d307 100644 --- a/src/hooks/permissions.rs +++ b/src/hooks/permissions.rs @@ -45,11 +45,6 @@ pub fn check_command_for(cmd: &str, host: Host) -> PermissionVerdict { Host::Cursor => load_cursor_rules(), Host::Gemini => load_gemini_rules(), Host::Droid => load_droid_rules(), - // Vibe stores hooks in ~/.vibe/hooks.toml and has no denylist / allowlist - // surface at the time of writing. Empty rules mean check_command_with_rules - // returns Default (treated as Ask by callers), and the Deny arm in - // hook_cmd::run_vibe is dead code kept as defensive scaffolding for when - // Vibe ships native permission config we can honor here. Host::Vibe => (Vec::new(), Vec::new(), Vec::new()), }; check_command_with_rules(cmd, &deny_rules, &ask_rules, &allow_rules) From d1f71398fde6e071c416cb2b6dbe9665b2bfb488 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Wed, 5 Aug 2026 15:57:03 +0200 Subject: [PATCH 16/22] fix(hooks): self-heal stale dual-schema Copilot hook config Legacy camelCase invocation rewrites the old stock config to the single-schema form, so upgrades need no manual rtk init --copilot. --- src/hooks/hook_cmd.rs | 178 +++++++++++++++++++++++++++++++++++++++++- src/hooks/init.rs | 4 +- 2 files changed, 179 insertions(+), 3 deletions(-) diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index e9d186d844..47ff8271b1 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -78,7 +78,12 @@ pub fn run_copilot() -> Result<()> { match detect_format(&v) { HookFormat::VsCode { command } => handle_vscode(&command), - HookFormat::CopilotCli { command, args } => handle_copilot_cli(&command, &args), + HookFormat::CopilotCli { command, args } => { + for path in heal_legacy_copilot_configs() { + audit_log("self_heal", &path.display().to_string(), ""); + } + handle_copilot_cli(&command, &args) + } HookFormat::CopilotIde { command } => handle_copilot_ide(&command), HookFormat::PassThrough => Ok(()), } @@ -145,6 +150,73 @@ fn detect_format(v: &Value) -> HookFormat { HookFormat::PassThrough } +// Stale dual-schema config written by pre-b754b85 `rtk init --copilot`; its +// camelCase entry is what routes invocations into the CopilotCli arm above. +const COPILOT_LEGACY_HOOK_JSON: &str = r#"{ + "version": 1, + "hooks": { + "PreToolUse": [ + { + "type": "command", + "command": "rtk hook copilot", + "cwd": ".", + "timeout": 5 + } + ], + "preToolUse": [ + { + "type": "command", + "bash": "rtk hook copilot", + "powershell": "rtk hook copilot", + "cwd": ".", + "timeoutSec": 5 + } + ] + } +} +"#; + +fn heal_legacy_copilot_configs() -> Vec { + use super::constants::{COPILOT_HOOK_FILE, GITHUB_DIR, HOOKS_SUBDIR}; + + let mut healed = Vec::new(); + let project = std::path::Path::new(GITHUB_DIR) + .join(HOOKS_SUBDIR) + .join(COPILOT_HOOK_FILE); + if heal_legacy_hook_file(&project) { + healed.push(project); + } + if let Ok(dir) = super::init::copilot_user_dir() { + let global = dir.join(HOOKS_SUBDIR).join(COPILOT_HOOK_FILE); + if heal_legacy_hook_file(&global) { + healed.push(global); + } + } + healed +} + +fn heal_legacy_hook_file(path: &std::path::Path) -> bool { + let Ok(raw) = std::fs::read_to_string(path) else { + return false; + }; + let Ok(current) = serde_json::from_str::(&raw) else { + return false; + }; + let Ok(legacy) = serde_json::from_str::(COPILOT_LEGACY_HOOK_JSON) else { + return false; + }; + if current != legacy { + return false; + } + let tmp = path.with_extension(format!("heal.{}", std::process::id())); + std::fs::write(&tmp, super::init::COPILOT_HOOK_JSON) + .and_then(|()| std::fs::rename(&tmp, path)) + .map_err(|_| { + let _ = std::fs::remove_file(&tmp); + }) + .is_ok() +} + fn get_rewritten(cmd: &str) -> Option { if has_heredoc(cmd) { return None; @@ -752,6 +824,110 @@ mod tests { crate::discover::registry::rewrite_command(cmd, excluded, &[]) } + // --- Copilot legacy config self-heal --- + + fn heal_input(dir: &tempfile::TempDir, content: &str) -> std::path::PathBuf { + let path = dir.path().join("rtk-rewrite.json"); + std::fs::write(&path, content).expect("write test config"); + path + } + + #[test] + fn heal_rewrites_exact_legacy_stock_to_current_stock() { + let dir = tempfile::TempDir::new().expect("tempdir"); + let path = heal_input(&dir, COPILOT_LEGACY_HOOK_JSON); + + assert!(heal_legacy_hook_file(&path)); + assert_eq!( + std::fs::read_to_string(&path).expect("read"), + crate::hooks::init::COPILOT_HOOK_JSON + ); + } + + #[test] + fn heal_is_key_order_insensitive() { + let reordered = r#"{ + "hooks": { + "preToolUse": [ + { "timeoutSec": 5, "cwd": ".", "powershell": "rtk hook copilot", "bash": "rtk hook copilot", "type": "command" } + ], + "PreToolUse": [ + { "timeout": 5, "cwd": ".", "command": "rtk hook copilot", "type": "command" } + ] + }, + "version": 1 +}"#; + let dir = tempfile::TempDir::new().expect("tempdir"); + let path = heal_input(&dir, reordered); + assert!(heal_legacy_hook_file(&path)); + } + + #[test] + fn heal_second_run_is_a_noop() { + let dir = tempfile::TempDir::new().expect("tempdir"); + let path = heal_input(&dir, COPILOT_LEGACY_HOOK_JSON); + assert!(heal_legacy_hook_file(&path)); + + assert!(!heal_legacy_hook_file(&path)); + assert_eq!( + std::fs::read_to_string(&path).expect("read"), + crate::hooks::init::COPILOT_HOOK_JSON + ); + } + + #[test] + fn heal_refuses_any_deviation_from_legacy_stock() { + let customized = COPILOT_LEGACY_HOOK_JSON.replace( + r#""bash": "rtk hook copilot""#, + r#""bash": "my-wrapper.sh""#, + ); + let extra_key = + COPILOT_LEGACY_HOOK_JSON.replace(r#""version": 1,"#, r#""version": 1, "extra": true,"#); + let missing_pascal = COPILOT_LEGACY_HOOK_JSON.replace( + r#""PreToolUse": [ + { + "type": "command", + "command": "rtk hook copilot", + "cwd": ".", + "timeout": 5 + } + ], + "#, + "", + ); + for content in [ + customized.as_str(), + extra_key.as_str(), + missing_pascal.as_str(), + crate::hooks::init::COPILOT_HOOK_JSON, + "{ not json", + "{}", + "", + ] { + let dir = tempfile::TempDir::new().expect("tempdir"); + let path = heal_input(&dir, content); + assert!(!heal_legacy_hook_file(&path), "input: {content:?}"); + assert_eq!( + std::fs::read_to_string(&path).expect("read"), + content, + "input: {content:?}" + ); + } + } + + #[test] + fn heal_missing_file_is_a_noop() { + let dir = tempfile::TempDir::new().expect("tempdir"); + let path = dir.path().join("rtk-rewrite.json"); + assert!(!heal_legacy_hook_file(&path)); + assert!(!path.exists()); + assert_eq!( + std::fs::read_dir(dir.path()).expect("readdir").count(), + 0, + "heal must not create files" + ); + } + // --- Copilot format detection --- fn vscode_input(tool: &str, cmd: &str) -> Value { diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 439dbfefa6..975642f054 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -4456,7 +4456,7 @@ fn uninstall_gemini(ctx: InitContext) -> Result> { // process spawn per tool call for no behavioral benefit (confirmed live: // Copilot CLI honors the PascalCase-only schema on its own, receiving the // same `tool_name`/`tool_input.command` shape either way). -const COPILOT_HOOK_JSON: &str = r#"{ +pub(crate) const COPILOT_HOOK_JSON: &str = r#"{ "version": 1, "hooks": { "PreToolUse": [ @@ -4628,7 +4628,7 @@ fn uninstall_copilot_at(base: &Path, ctx: InitContext) -> Result> { Ok(removed) } -fn copilot_user_dir() -> Result { +pub(crate) fn copilot_user_dir() -> Result { if let Ok(custom) = std::env::var(COPILOT_HOME_ENV) { return Ok(PathBuf::from(custom)); } From db31da9af4d46ece27f996f350ecbaf6b724e208 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Wed, 5 Aug 2026 16:09:31 +0200 Subject: [PATCH 17/22] fix(hooks): heal only rtk's own legacy camelCase entry, keep user config Match the exact stock camelCase entry instead of the whole file, so extended configs (extra hooks/keys) heal too; anything non-stock stays untouched. --- src/hooks/hook_cmd.rs | 169 ++++++++++++++++++++++++++++++------------ 1 file changed, 123 insertions(+), 46 deletions(-) diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index 47ff8271b1..0dbe5ca644 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -150,32 +150,6 @@ fn detect_format(v: &Value) -> HookFormat { HookFormat::PassThrough } -// Stale dual-schema config written by pre-b754b85 `rtk init --copilot`; its -// camelCase entry is what routes invocations into the CopilotCli arm above. -const COPILOT_LEGACY_HOOK_JSON: &str = r#"{ - "version": 1, - "hooks": { - "PreToolUse": [ - { - "type": "command", - "command": "rtk hook copilot", - "cwd": ".", - "timeout": 5 - } - ], - "preToolUse": [ - { - "type": "command", - "bash": "rtk hook copilot", - "powershell": "rtk hook copilot", - "cwd": ".", - "timeoutSec": 5 - } - ] - } -} -"#; - fn heal_legacy_copilot_configs() -> Vec { use super::constants::{COPILOT_HOOK_FILE, GITHUB_DIR, HOOKS_SUBDIR}; @@ -195,21 +169,60 @@ fn heal_legacy_copilot_configs() -> Vec { healed } +// Exact camelCase entry written by pre-b754b85 `rtk init --copilot`; that +// stale registration is the only thing routing invocations into the +// CopilotCli arm above. Only this entry is removed — user additions stay. +fn legacy_camelcase_entry() -> Value { + json!([{ + "type": "command", + "bash": "rtk hook copilot", + "powershell": "rtk hook copilot", + "cwd": ".", + "timeoutSec": 5 + }]) +} + fn heal_legacy_hook_file(path: &std::path::Path) -> bool { let Ok(raw) = std::fs::read_to_string(path) else { return false; }; - let Ok(current) = serde_json::from_str::(&raw) else { + let Ok(mut config) = serde_json::from_str::(&raw) else { return false; }; - let Ok(legacy) = serde_json::from_str::(COPILOT_LEGACY_HOOK_JSON) else { + let Some(hooks) = config.get("hooks").and_then(|h| h.as_object()) else { return false; }; - if current != legacy { + if hooks.get("preToolUse") != Some(&legacy_camelcase_entry()) { return false; } + let pascalcase_still_registered = hooks + .get("PreToolUse") + .and_then(|p| p.as_array()) + .is_some_and(|entries| { + entries + .iter() + .any(|e| e.get("command").and_then(|c| c.as_str()) == Some("rtk hook copilot")) + }); + if !pascalcase_still_registered { + return false; + } + let Some(hooks) = config.get_mut("hooks").and_then(|h| h.as_object_mut()) else { + return false; + }; + hooks.shift_remove("preToolUse"); + + let stock = serde_json::from_str::(super::init::COPILOT_HOOK_JSON).ok(); + let content = if stock.is_some_and(|s| s == config) { + super::init::COPILOT_HOOK_JSON.to_string() + } else { + let Ok(mut pretty) = serde_json::to_string_pretty(&config) else { + return false; + }; + pretty.push('\n'); + pretty + }; let tmp = path.with_extension(format!("heal.{}", std::process::id())); - std::fs::write(&tmp, super::init::COPILOT_HOOK_JSON) + std::fs::write(&tmp, content) .and_then(|()| std::fs::rename(&tmp, path)) .map_err(|_| { let _ = std::fs::remove_file(&tmp); @@ -826,6 +839,19 @@ mod tests { // --- Copilot legacy config self-heal --- + const LEGACY_STOCK: &str = r#"{ + "version": 1, + "hooks": { + "PreToolUse": [ + { "type": "command", "command": "rtk hook copilot", "cwd": ".", "timeout": 5 } + ], + "preToolUse": [ + { "type": "command", "bash": "rtk hook copilot", "powershell": "rtk hook copilot", "cwd": ".", "timeoutSec": 5 } + ] + } +} +"#; + fn heal_input(dir: &tempfile::TempDir, content: &str) -> std::path::PathBuf { let path = dir.path().join("rtk-rewrite.json"); std::fs::write(&path, content).expect("write test config"); @@ -833,9 +859,9 @@ mod tests { } #[test] - fn heal_rewrites_exact_legacy_stock_to_current_stock() { + fn heal_rewrites_legacy_stock_to_current_stock_bytes() { let dir = tempfile::TempDir::new().expect("tempdir"); - let path = heal_input(&dir, COPILOT_LEGACY_HOOK_JSON); + let path = heal_input(&dir, LEGACY_STOCK); assert!(heal_legacy_hook_file(&path)); assert_eq!( @@ -845,7 +871,47 @@ mod tests { } #[test] - fn heal_is_key_order_insensitive() { + fn heal_preserves_user_hooks_and_keys() { + let extended = r#"{ + "version": 1, + "customTopLevel": { "keep": true }, + "hooks": { + "sessionStart": [ + { "type": "command", "command": "echo hi" } + ], + "PreToolUse": [ + { "type": "command", "command": "rtk hook copilot", "cwd": ".", "timeout": 5 } + ], + "preToolUse": [ + { "type": "command", "bash": "rtk hook copilot", "powershell": "rtk hook copilot", "cwd": ".", "timeoutSec": 5 } + ] + } +} +"#; + let dir = tempfile::TempDir::new().expect("tempdir"); + let path = heal_input(&dir, extended); + + assert!(heal_legacy_hook_file(&path)); + let healed: Value = + serde_json::from_str(&std::fs::read_to_string(&path).expect("read")).expect("json"); + assert!(healed["hooks"].get("preToolUse").is_none()); + assert_eq!( + healed["hooks"]["PreToolUse"][0]["command"], + "rtk hook copilot" + ); + assert_eq!(healed["hooks"]["sessionStart"][0]["command"], "echo hi"); + assert_eq!(healed["customTopLevel"]["keep"], true); + let keys: Vec<&str> = healed["hooks"] + .as_object() + .expect("hooks object") + .keys() + .map(String::as_str) + .collect(); + assert_eq!(keys, ["sessionStart", "PreToolUse"], "key order preserved"); + } + + #[test] + fn heal_matches_legacy_entry_regardless_of_field_order() { let reordered = r#"{ "hooks": { "preToolUse": [ @@ -865,7 +931,7 @@ mod tests { #[test] fn heal_second_run_is_a_noop() { let dir = tempfile::TempDir::new().expect("tempdir"); - let path = heal_input(&dir, COPILOT_LEGACY_HOOK_JSON); + let path = heal_input(&dir, LEGACY_STOCK); assert!(heal_legacy_hook_file(&path)); assert!(!heal_legacy_hook_file(&path)); @@ -876,34 +942,45 @@ mod tests { } #[test] - fn heal_refuses_any_deviation_from_legacy_stock() { - let customized = COPILOT_LEGACY_HOOK_JSON.replace( + fn heal_refuses_non_stock_camelcase_or_missing_pascalcase() { + let customized = LEGACY_STOCK.replace( r#""bash": "rtk hook copilot""#, r#""bash": "my-wrapper.sh""#, ); - let extra_key = - COPILOT_LEGACY_HOOK_JSON.replace(r#""version": 1,"#, r#""version": 1, "extra": true,"#); - let missing_pascal = COPILOT_LEGACY_HOOK_JSON.replace( + let extra_field = LEGACY_STOCK.replace(r#""timeoutSec": 5"#, r#""timeoutSec": 5, "x": 1"#); + let two_entries = LEGACY_STOCK.replace( + r#""preToolUse": [ + {"#, + r#""preToolUse": [ + { "type": "command", "bash": "rtk hook copilot", "powershell": "rtk hook copilot", "cwd": ".", "timeoutSec": 5 }, + {"#, + ); + let missing_pascal = LEGACY_STOCK.replace( r#""PreToolUse": [ - { - "type": "command", - "command": "rtk hook copilot", - "cwd": ".", - "timeout": 5 - } + { "type": "command", "command": "rtk hook copilot", "cwd": ".", "timeout": 5 } ], "#, "", ); + let foreign_pascal = LEGACY_STOCK.replace( + r#""command": "rtk hook copilot""#, + r#""command": "other-tool --hook""#, + ); for content in [ customized.as_str(), - extra_key.as_str(), + extra_field.as_str(), + two_entries.as_str(), missing_pascal.as_str(), + foreign_pascal.as_str(), crate::hooks::init::COPILOT_HOOK_JSON, "{ not json", "{}", "", ] { + let parsed: Result = serde_json::from_str(content); + if content == missing_pascal || content == two_entries { + assert!(parsed.is_ok(), "fixture must stay valid JSON: {content:?}"); + } let dir = tempfile::TempDir::new().expect("tempdir"); let path = heal_input(&dir, content); assert!(!heal_legacy_hook_file(&path), "input: {content:?}"); From f676ad7537f7717f5ff6e0e97a34f7dadd643174 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Wed, 5 Aug 2026 16:24:18 +0200 Subject: [PATCH 18/22] test(hooks): end-to-end matrix for Copilot self-heal safety Real-binary tests: heal correctness, refusal matrix, response integrity, concurrency, unwritable dir. --- tests/copilot_selfheal_test.rs | 445 +++++++++++++++++++++++++++++++++ 1 file changed, 445 insertions(+) create mode 100644 tests/copilot_selfheal_test.rs diff --git a/tests/copilot_selfheal_test.rs b/tests/copilot_selfheal_test.rs new file mode 100644 index 0000000000..f2e519eb5f --- /dev/null +++ b/tests/copilot_selfheal_test.rs @@ -0,0 +1,445 @@ +//! End-to-end tests for the Copilot legacy hook config self-heal. +//! +//! Runs the real `rtk hook copilot` binary against crafted configs in +//! sandboxed HOME/COPILOT_HOME and asserts the hook protocol is never +//! broken: correct responses, exit 0, silent stderr, and configs only +//! ever modified when they contain rtk's own stale camelCase entry. + +use std::io::Write; +use std::path::{Path, PathBuf}; +use std::process::{Command, Stdio}; +use tempfile::TempDir; + +const LEGACY_STOCK: &str = r#"{ + "version": 1, + "hooks": { + "PreToolUse": [ + { "type": "command", "command": "rtk hook copilot", "cwd": ".", "timeout": 5 } + ], + "preToolUse": [ + { "type": "command", "bash": "rtk hook copilot", "powershell": "rtk hook copilot", "cwd": ".", "timeoutSec": 5 } + ] + } +} +"#; + +const CURRENT_STOCK: &str = r#"{ + "version": 1, + "hooks": { + "PreToolUse": [ + { + "type": "command", + "command": "rtk hook copilot", + "cwd": ".", + "timeout": 5 + } + ] + } +} +"#; + +const LEGACY_PAYLOAD: &str = r#"{"toolName":"bash","toolArgs":"{\"command\":\"git status\"}"}"#; +const LEGACY_PAYLOAD_PS: &str = + r#"{"toolName":"powershell","toolArgs":"{\"command\":\"git status\"}"}"#; +const JETBRAINS_PAYLOAD: &str = + r#"{"toolName":"run_in_terminal","toolArgs":"{\"command\":\"git status\"}"}"#; +const PASCAL_PAYLOAD: &str = r#"{"tool_name":"Bash","tool_input":{"command":"git status"}}"#; +const UNKNOWN_TOOL_PAYLOAD: &str = r#"{"tool_name":"Edit","tool_input":{"command":"git status"}}"#; + +struct Sandbox { + _root: TempDir, + home: PathBuf, + copilot_home: PathBuf, + project: PathBuf, +} + +impl Sandbox { + fn new() -> Self { + let root = TempDir::new().expect("tempdir"); + let home = root.path().join("home"); + let copilot_home = root.path().join("copilot-home"); + let project = root.path().join("project"); + std::fs::create_dir_all(&home).expect("mkdir home"); + std::fs::create_dir_all(copilot_home.join("hooks")).expect("mkdir copilot hooks"); + std::fs::create_dir_all(project.join(".github/hooks")).expect("mkdir project hooks"); + Self { + _root: root, + home, + copilot_home, + project, + } + } + + fn project_config(&self) -> PathBuf { + self.project.join(".github/hooks/rtk-rewrite.json") + } + + fn global_config(&self) -> PathBuf { + self.copilot_home.join("hooks/rtk-rewrite.json") + } + + fn write_project(&self, content: &str) { + std::fs::write(self.project_config(), content).expect("write project config"); + } + + fn write_global(&self, content: &str) { + std::fs::write(self.global_config(), content).expect("write global config"); + } + + fn run_hook(&self, payload: &str) -> (String, String, Option) { + let mut child = Command::new(env!("CARGO_BIN_EXE_rtk")) + .args(["hook", "copilot"]) + .current_dir(&self.project) + .env("HOME", &self.home) + .env("COPILOT_HOME", &self.copilot_home) + .env("LC_ALL", "C") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn rtk"); + child + .stdin + .take() + .expect("stdin") + .write_all(payload.as_bytes()) + .expect("write payload"); + let out = child.wait_with_output().expect("wait rtk"); + ( + String::from_utf8_lossy(&out.stdout).into_owned(), + String::from_utf8_lossy(&out.stderr).into_owned(), + out.status.code(), + ) + } +} + +fn read(path: &Path) -> String { + std::fs::read_to_string(path).expect("read config") +} + +fn has_camel(path: &Path) -> bool { + read(path).contains("\"preToolUse\"") +} + +fn assert_hook_ok(payload: &str, stdout: &str, stderr: &str, code: Option) { + assert_eq!(code, Some(0), "hook must exit 0 for payload {payload}"); + assert!( + stderr.is_empty(), + "stderr must stay silent (protocol safety) for payload {payload}, got: {stderr}" + ); + if !stdout.trim().is_empty() { + serde_json::from_str::(stdout.trim()) + .unwrap_or_else(|e| panic!("stdout must be valid JSON for {payload}: {e}\n{stdout}")); + } +} + +// ── Heal correctness ───────────────────────────────────────── + +#[test] +fn legacy_invocation_heals_project_and_global_to_current_stock() { + let sb = Sandbox::new(); + sb.write_project(LEGACY_STOCK); + sb.write_global(LEGACY_STOCK); + + let (stdout, stderr, code) = sb.run_hook(LEGACY_PAYLOAD); + + assert_hook_ok(LEGACY_PAYLOAD, &stdout, &stderr, code); + assert!( + stdout.contains("rtk git status"), + "rewrite must still work during heal: {stdout}" + ); + assert_eq!(read(&sb.project_config()), CURRENT_STOCK); + assert_eq!(read(&sb.global_config()), CURRENT_STOCK); +} + +#[test] +fn powershell_legacy_invocation_also_heals() { + let sb = Sandbox::new(); + sb.write_project(LEGACY_STOCK); + + let (stdout, stderr, code) = sb.run_hook(LEGACY_PAYLOAD_PS); + + assert_hook_ok(LEGACY_PAYLOAD_PS, &stdout, &stderr, code); + assert!(!has_camel(&sb.project_config())); +} + +#[test] +fn heal_preserves_user_hooks_and_key_order() { + let extended = r#"{ + "version": 1, + "customTopLevel": { "keep": true }, + "hooks": { + "sessionStart": [ + { "type": "command", "command": "echo hi" } + ], + "PreToolUse": [ + { "type": "command", "command": "rtk hook copilot", "cwd": ".", "timeout": 5 } + ], + "preToolUse": [ + { "type": "command", "bash": "rtk hook copilot", "powershell": "rtk hook copilot", "cwd": ".", "timeoutSec": 5 } + ] + } +} +"#; + let sb = Sandbox::new(); + sb.write_project(extended); + + let (stdout, stderr, code) = sb.run_hook(LEGACY_PAYLOAD); + + assert_hook_ok(LEGACY_PAYLOAD, &stdout, &stderr, code); + let healed: serde_json::Value = + serde_json::from_str(&read(&sb.project_config())).expect("healed config valid JSON"); + assert!(healed["hooks"].get("preToolUse").is_none()); + assert_eq!(healed["customTopLevel"]["keep"], true); + assert_eq!(healed["hooks"]["sessionStart"][0]["command"], "echo hi"); + assert_eq!( + healed["hooks"]["PreToolUse"][0]["command"], + "rtk hook copilot" + ); + let keys: Vec<&str> = healed["hooks"] + .as_object() + .expect("hooks object") + .keys() + .map(String::as_str) + .collect(); + assert_eq!(keys, ["sessionStart", "PreToolUse"], "key order preserved"); +} + +#[test] +fn heal_is_idempotent_across_invocations() { + let sb = Sandbox::new(); + sb.write_project(LEGACY_STOCK); + sb.write_global(LEGACY_STOCK); + + sb.run_hook(LEGACY_PAYLOAD); + let project_after = read(&sb.project_config()); + let global_after = read(&sb.global_config()); + + for _ in 0..3 { + let (stdout, stderr, code) = sb.run_hook(LEGACY_PAYLOAD); + assert_hook_ok(LEGACY_PAYLOAD, &stdout, &stderr, code); + } + assert_eq!(read(&sb.project_config()), project_after); + assert_eq!(read(&sb.global_config()), global_after); +} + +// ── Response integrity: heal never changes hook behavior ───── + +#[test] +fn response_is_byte_identical_before_and_after_heal() { + let sb = Sandbox::new(); + sb.write_project(LEGACY_STOCK); + + let (before, _, _) = sb.run_hook(LEGACY_PAYLOAD); + assert!(!has_camel(&sb.project_config()), "first run must heal"); + let (after, _, _) = sb.run_hook(LEGACY_PAYLOAD); + + assert_eq!(before, after, "heal must not alter the hook response"); +} + +#[test] +fn pascalcase_invocation_works_and_never_touches_configs() { + let sb = Sandbox::new(); + sb.write_project(LEGACY_STOCK); + sb.write_global(LEGACY_STOCK); + + let (stdout, stderr, code) = sb.run_hook(PASCAL_PAYLOAD); + + assert_hook_ok(PASCAL_PAYLOAD, &stdout, &stderr, code); + assert!( + stdout.contains("rtk git status"), + "PascalCase rewrite must work: {stdout}" + ); + assert_eq!(read(&sb.project_config()), LEGACY_STOCK); + assert_eq!(read(&sb.global_config()), LEGACY_STOCK); +} + +#[test] +fn jetbrains_invocation_works_and_never_touches_configs() { + let sb = Sandbox::new(); + sb.write_project(LEGACY_STOCK); + + let (stdout, stderr, code) = sb.run_hook(JETBRAINS_PAYLOAD); + + assert_hook_ok(JETBRAINS_PAYLOAD, &stdout, &stderr, code); + assert!( + stdout.contains("rtk git status"), + "JetBrains deny-with-suggestion must carry the rewrite: {stdout}" + ); + assert_eq!(read(&sb.project_config()), LEGACY_STOCK); +} + +#[test] +fn non_shell_tool_passes_through_and_never_touches_configs() { + let sb = Sandbox::new(); + sb.write_project(LEGACY_STOCK); + + let (stdout, stderr, code) = sb.run_hook(UNKNOWN_TOOL_PAYLOAD); + + assert_hook_ok(UNKNOWN_TOOL_PAYLOAD, &stdout, &stderr, code); + assert!(stdout.trim().is_empty(), "pass-through must stay silent"); + assert_eq!(read(&sb.project_config()), LEGACY_STOCK); +} + +#[test] +fn garbage_and_empty_stdin_exit_zero_without_touching_configs() { + let sb = Sandbox::new(); + for payload in ["", "not json at all", "{\"toolName\":\"bash\"}"] { + sb.write_project(LEGACY_STOCK); + let (_, _, code) = sb.run_hook(payload); + assert_eq!(code, Some(0), "payload {payload:?} must exit 0"); + assert_eq!( + read(&sb.project_config()), + LEGACY_STOCK, + "payload {payload:?} must not modify configs" + ); + } +} + +// ── Refusal matrix: configs never wrongly modified ─────────── + +#[test] +fn non_stock_configs_are_never_modified_by_legacy_invocations() { + let customized = LEGACY_STOCK.replace( + r#""bash": "rtk hook copilot""#, + r#""bash": "my-wrapper.sh""#, + ); + let extra_field = LEGACY_STOCK.replace(r#""timeoutSec": 5"#, r#""timeoutSec": 5, "x": 1"#); + let two_entries = LEGACY_STOCK.replace( + r#""preToolUse": [ + {"#, + r#""preToolUse": [ + { "type": "command", "bash": "rtk hook copilot", "powershell": "rtk hook copilot", "cwd": ".", "timeoutSec": 5 }, + {"#, + ); + let missing_pascal = LEGACY_STOCK.replace( + r#""PreToolUse": [ + { "type": "command", "command": "rtk hook copilot", "cwd": ".", "timeout": 5 } + ], + "#, + "", + ); + let foreign_pascal = LEGACY_STOCK.replace( + r#""command": "rtk hook copilot""#, + r#""command": "other-tool --hook""#, + ); + for (label, content) in [ + ("customized camelCase", customized.as_str()), + ("extra field in entry", extra_field.as_str()), + ("two camelCase entries", two_entries.as_str()), + ("missing PascalCase", missing_pascal.as_str()), + ("foreign PascalCase", foreign_pascal.as_str()), + ("current stock", CURRENT_STOCK), + ("malformed", "{ not json"), + ("empty object", "{}"), + ("empty file", ""), + ("array root", "[1, 2]"), + ] { + let sb = Sandbox::new(); + sb.write_project(content); + sb.write_global(content); + + let (stdout, stderr, code) = sb.run_hook(LEGACY_PAYLOAD); + + assert_hook_ok(LEGACY_PAYLOAD, &stdout, &stderr, code); + assert!( + stdout.contains("rtk git status"), + "{label}: hook must keep rewriting: {stdout}" + ); + assert_eq!( + read(&sb.project_config()), + content, + "{label}: project modified" + ); + assert_eq!( + read(&sb.global_config()), + content, + "{label}: global modified" + ); + } +} + +#[test] +fn missing_configs_are_never_created() { + let sb = Sandbox::new(); + + let (stdout, stderr, code) = sb.run_hook(LEGACY_PAYLOAD); + + assert_hook_ok(LEGACY_PAYLOAD, &stdout, &stderr, code); + assert!(!sb.project_config().exists()); + assert!(!sb.global_config().exists()); +} + +// ── Robustness ─────────────────────────────────────────────── + +#[test] +fn concurrent_legacy_invocations_leave_valid_healed_configs() { + let sb = Sandbox::new(); + sb.write_project(LEGACY_STOCK); + sb.write_global(LEGACY_STOCK); + + let sb_ref = &sb; + std::thread::scope(|scope| { + let handles: Vec<_> = (0..8) + .map(|_| scope.spawn(move || sb_ref.run_hook(LEGACY_PAYLOAD))) + .collect(); + for handle in handles { + let (stdout, stderr, code) = handle.join().expect("thread"); + assert_hook_ok(LEGACY_PAYLOAD, &stdout, &stderr, code); + } + }); + + assert_eq!(read(&sb.project_config()), CURRENT_STOCK); + assert_eq!(read(&sb.global_config()), CURRENT_STOCK); + let stray: Vec<_> = walk(&sb.project) + .into_iter() + .chain(walk(&sb.copilot_home)) + .filter(|p| p.to_string_lossy().contains(".heal.")) + .collect(); + assert!(stray.is_empty(), "temp files left behind: {stray:?}"); +} + +#[cfg(unix)] +#[test] +fn unwritable_hooks_dir_never_breaks_the_hook() { + use std::os::unix::fs::PermissionsExt; + + let sb = Sandbox::new(); + sb.write_project(LEGACY_STOCK); + let hooks_dir = sb.project.join(".github/hooks"); + let mut perms = std::fs::metadata(&hooks_dir).expect("meta").permissions(); + perms.set_mode(0o555); + std::fs::set_permissions(&hooks_dir, perms.clone()).expect("chmod"); + + let (stdout, stderr, code) = sb.run_hook(LEGACY_PAYLOAD); + + perms.set_mode(0o755); + std::fs::set_permissions(&hooks_dir, perms).expect("chmod back"); + + assert_hook_ok(LEGACY_PAYLOAD, &stdout, &stderr, code); + assert!( + stdout.contains("rtk git status"), + "rewrite must survive write failure: {stdout}" + ); + assert_eq!( + read(&sb.project_config()), + LEGACY_STOCK, + "config must stay intact" + ); +} + +fn walk(dir: &Path) -> Vec { + let mut out = Vec::new(); + let Ok(entries) = std::fs::read_dir(dir) else { + return out; + }; + for entry in entries.flatten() { + let path = entry.path(); + if path.is_dir() { + out.extend(walk(&path)); + } else { + out.push(path); + } + } + out +} From 157b0acc989d8fa703ae0c25cdf9487ebdb66bfa Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Wed, 5 Aug 2026 16:31:51 +0200 Subject: [PATCH 19/22] refacto(copilot): rm migration tests --- src/hooks/hook_cmd.rs | 168 ------------------------------------------ 1 file changed, 168 deletions(-) diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index 0dbe5ca644..4acd3528ff 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -837,174 +837,6 @@ mod tests { crate::discover::registry::rewrite_command(cmd, excluded, &[]) } - // --- Copilot legacy config self-heal --- - - const LEGACY_STOCK: &str = r#"{ - "version": 1, - "hooks": { - "PreToolUse": [ - { "type": "command", "command": "rtk hook copilot", "cwd": ".", "timeout": 5 } - ], - "preToolUse": [ - { "type": "command", "bash": "rtk hook copilot", "powershell": "rtk hook copilot", "cwd": ".", "timeoutSec": 5 } - ] - } -} -"#; - - fn heal_input(dir: &tempfile::TempDir, content: &str) -> std::path::PathBuf { - let path = dir.path().join("rtk-rewrite.json"); - std::fs::write(&path, content).expect("write test config"); - path - } - - #[test] - fn heal_rewrites_legacy_stock_to_current_stock_bytes() { - let dir = tempfile::TempDir::new().expect("tempdir"); - let path = heal_input(&dir, LEGACY_STOCK); - - assert!(heal_legacy_hook_file(&path)); - assert_eq!( - std::fs::read_to_string(&path).expect("read"), - crate::hooks::init::COPILOT_HOOK_JSON - ); - } - - #[test] - fn heal_preserves_user_hooks_and_keys() { - let extended = r#"{ - "version": 1, - "customTopLevel": { "keep": true }, - "hooks": { - "sessionStart": [ - { "type": "command", "command": "echo hi" } - ], - "PreToolUse": [ - { "type": "command", "command": "rtk hook copilot", "cwd": ".", "timeout": 5 } - ], - "preToolUse": [ - { "type": "command", "bash": "rtk hook copilot", "powershell": "rtk hook copilot", "cwd": ".", "timeoutSec": 5 } - ] - } -} -"#; - let dir = tempfile::TempDir::new().expect("tempdir"); - let path = heal_input(&dir, extended); - - assert!(heal_legacy_hook_file(&path)); - let healed: Value = - serde_json::from_str(&std::fs::read_to_string(&path).expect("read")).expect("json"); - assert!(healed["hooks"].get("preToolUse").is_none()); - assert_eq!( - healed["hooks"]["PreToolUse"][0]["command"], - "rtk hook copilot" - ); - assert_eq!(healed["hooks"]["sessionStart"][0]["command"], "echo hi"); - assert_eq!(healed["customTopLevel"]["keep"], true); - let keys: Vec<&str> = healed["hooks"] - .as_object() - .expect("hooks object") - .keys() - .map(String::as_str) - .collect(); - assert_eq!(keys, ["sessionStart", "PreToolUse"], "key order preserved"); - } - - #[test] - fn heal_matches_legacy_entry_regardless_of_field_order() { - let reordered = r#"{ - "hooks": { - "preToolUse": [ - { "timeoutSec": 5, "cwd": ".", "powershell": "rtk hook copilot", "bash": "rtk hook copilot", "type": "command" } - ], - "PreToolUse": [ - { "timeout": 5, "cwd": ".", "command": "rtk hook copilot", "type": "command" } - ] - }, - "version": 1 -}"#; - let dir = tempfile::TempDir::new().expect("tempdir"); - let path = heal_input(&dir, reordered); - assert!(heal_legacy_hook_file(&path)); - } - - #[test] - fn heal_second_run_is_a_noop() { - let dir = tempfile::TempDir::new().expect("tempdir"); - let path = heal_input(&dir, LEGACY_STOCK); - assert!(heal_legacy_hook_file(&path)); - - assert!(!heal_legacy_hook_file(&path)); - assert_eq!( - std::fs::read_to_string(&path).expect("read"), - crate::hooks::init::COPILOT_HOOK_JSON - ); - } - - #[test] - fn heal_refuses_non_stock_camelcase_or_missing_pascalcase() { - let customized = LEGACY_STOCK.replace( - r#""bash": "rtk hook copilot""#, - r#""bash": "my-wrapper.sh""#, - ); - let extra_field = LEGACY_STOCK.replace(r#""timeoutSec": 5"#, r#""timeoutSec": 5, "x": 1"#); - let two_entries = LEGACY_STOCK.replace( - r#""preToolUse": [ - {"#, - r#""preToolUse": [ - { "type": "command", "bash": "rtk hook copilot", "powershell": "rtk hook copilot", "cwd": ".", "timeoutSec": 5 }, - {"#, - ); - let missing_pascal = LEGACY_STOCK.replace( - r#""PreToolUse": [ - { "type": "command", "command": "rtk hook copilot", "cwd": ".", "timeout": 5 } - ], - "#, - "", - ); - let foreign_pascal = LEGACY_STOCK.replace( - r#""command": "rtk hook copilot""#, - r#""command": "other-tool --hook""#, - ); - for content in [ - customized.as_str(), - extra_field.as_str(), - two_entries.as_str(), - missing_pascal.as_str(), - foreign_pascal.as_str(), - crate::hooks::init::COPILOT_HOOK_JSON, - "{ not json", - "{}", - "", - ] { - let parsed: Result = serde_json::from_str(content); - if content == missing_pascal || content == two_entries { - assert!(parsed.is_ok(), "fixture must stay valid JSON: {content:?}"); - } - let dir = tempfile::TempDir::new().expect("tempdir"); - let path = heal_input(&dir, content); - assert!(!heal_legacy_hook_file(&path), "input: {content:?}"); - assert_eq!( - std::fs::read_to_string(&path).expect("read"), - content, - "input: {content:?}" - ); - } - } - - #[test] - fn heal_missing_file_is_a_noop() { - let dir = tempfile::TempDir::new().expect("tempdir"); - let path = dir.path().join("rtk-rewrite.json"); - assert!(!heal_legacy_hook_file(&path)); - assert!(!path.exists()); - assert_eq!( - std::fs::read_dir(dir.path()).expect("readdir").count(), - 0, - "heal must not create files" - ); - } - // --- Copilot format detection --- fn vscode_input(tool: &str, cmd: &str) -> Value { From 99841dc40d2397aa00230979b7f2d6b82ce8639a Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Wed, 5 Aug 2026 17:03:35 +0200 Subject: [PATCH 20/22] chore(hooks): suppress semgrep on temp-file cleanup --- src/hooks/hook_cmd.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index 4acd3528ff..17fbf41b27 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -225,7 +225,8 @@ fn heal_legacy_hook_file(path: &std::path::Path) -> bool { std::fs::write(&tmp, content) .and_then(|()| std::fs::rename(&tmp, path)) .map_err(|_| { - let _ = std::fs::remove_file(&tmp); + // Cleanup of our own temp file after a failed atomic write. + let _ = std::fs::remove_file(&tmp); // nosemgrep: filesystem-deletion }) .is_ok() } From 4ff41bd12b2f3acc9af1cb43dd538cbcd425546d Mon Sep 17 00:00:00 2001 From: Xavier Pestel Date: Wed, 5 Aug 2026 17:10:26 +0200 Subject: [PATCH 21/22] ci(semgrep): suppress filesystem-deletion warnings on Vibe uninstall paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Semgrep security scan on PR #3391 flagged 2 new fs::remove_file calls in uninstall_vibe_at as blocking findings under the filesystem-deletion rule (WARNING severity, but the CI runs semgrep --error which promotes all findings). Both calls are legitimate uninstall behavior: - prompt file removal at src/hooks/init.rs:4697 — removes only ~/.vibe/prompts/rtk.md, which RTK installed itself. - hooks.toml removal at src/hooks/init.rs:4714 — removes the file only when it becomes empty after stripping the RTK entry, so no orphan empty file is left behind. Both suppressions follow the existing repo convention (`// nosemgrep: -- ` on the line above the code), matching precedents in src/discover/lexer.rs and src/core/stream.rs. --- src/hooks/init.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 89024219dc..0e6ff3db84 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -4694,6 +4694,7 @@ fn uninstall_vibe_at(vibe_dir: &Path, ctx: InitContext) -> Result> { prompt_path.display() ); } else { + // nosemgrep: filesystem-deletion -- uninstall path removes only RTK's own prompt file fs::remove_file(&prompt_path) .with_context(|| format!("Failed to remove {}", prompt_path.display()))?; } @@ -4711,6 +4712,7 @@ fn uninstall_vibe_at(vibe_dir: &Path, ctx: InitContext) -> Result> { hooks_path.display() ); } else if new_content.trim().is_empty() { + // nosemgrep: filesystem-deletion -- uninstall removes hooks.toml only when it becomes empty after stripping the RTK entry fs::remove_file(&hooks_path) .with_context(|| format!("Failed to remove {}", hooks_path.display()))?; } else { From 9dae4d703638c253e90c6319c5236ddb56f092c1 Mon Sep 17 00:00:00 2001 From: Xavier Pestel Date: Wed, 5 Aug 2026 17:33:45 +0200 Subject: [PATCH 22/22] refactor(vibe): address second-round nits from @aeppling Two follow-ups to https://github.com/rtk-ai/rtk/pull/3391#pullrequestreview-4865940066: 1. Move the summary-verb mapping onto VibeHookPatchOutcome as summary_verb() -> Option<&'static str>, returning None for Skipped. The call site becomes 'else if let Some(v) = outcome.summary_verb()' which collapses the guard and the match into a single decision point and removes the unreachable!() branch. If a future variant is added, the compiler forces a decision in summary_verb() and the caller handles it naturally through the Option. 2. uninstall_vibe now prints a stderr warning when resolve_vibe_dir() fails instead of silently returning Ok(()). Users asking to uninstall no longer see an empty response when the home dir can't be resolved. uninstall_gemini has the same swallow-and-return-Ok pattern; leaving that untouched here to keep the diff scoped to Vibe, but the same improvement would apply as a follow-up. Both are quality improvements with no behavior change on the happy path. --- src/hooks/init.rs | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 0e6ff3db84..370c957635 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -4501,12 +4501,7 @@ fn run_vibe_mode_at( if dry_run { print_dry_run_footer(); - } else if hook_outcome != VibeHookPatchOutcome::Skipped { - let summary_verb = match hook_outcome { - VibeHookPatchOutcome::Installed => "installed", - VibeHookPatchOutcome::AlreadyPresent => "already present", - VibeHookPatchOutcome::Skipped => unreachable!(), - }; + } else if let Some(summary_verb) = hook_outcome.summary_verb() { println!("\nMistral Vibe CLI hook {summary_verb} (global).\n"); println!(" Hook registry: {}", hooks_path.display()); if !hook_only { @@ -4532,6 +4527,16 @@ enum VibeHookPatchOutcome { Skipped, } +impl VibeHookPatchOutcome { + fn summary_verb(self) -> Option<&'static str> { + match self { + Self::Installed => Some("installed"), + Self::AlreadyPresent => Some("already present"), + Self::Skipped => None, + } + } +} + /// Append the RTK `[[hooks]]` entry to `~/.vibe/hooks.toml` if not already present. /// /// Uses append-based patching (string level) rather than parse-serialize round-trip @@ -4652,7 +4657,10 @@ pub fn uninstall_vibe(ctx: InitContext) -> Result<()> { let InitContext { dry_run, .. } = ctx; let vibe_dir = match resolve_vibe_dir() { Ok(d) => d, - Err(_) => return Ok(()), + Err(e) => { + eprintln!("RTK Vibe uninstall skipped: could not resolve ~/.vibe/ ({e})"); + return Ok(()); + } }; let removed = uninstall_vibe_at(&vibe_dir, ctx)?;