Conversation
…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.
…ok config 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.
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 <dir> && 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).
…ot 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.
…owershell 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.
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
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.
- 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.
…on-decision fix(hooks): stop Copilot from silently deciding permission on unconfigured commands
feat(rewrite): rewrite multiline blocks
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.
pszymkowiak
approved these changes
Aug 4, 2026
pszymkowiak
left a comment
pszymkowiak
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed the two constituent changes (#3212 already reviewed/approved separately; #3319 read independently despite prior approvals).
For #3319, went beyond a code read — built develop HEAD (3044911) in an isolated worktree and verified:
- fmt/clippy/
cargo test --all: 2562+ tests, 0 failed - Manually exercised the multiline-rewrite path via
rtk hook check:- independent lines → each rewritten independently
&&-continuation → joined and rewritten as one unit- multi-line quoted commit message → newline inside quotes preserved, not split
- shell function definition → correctly classified unsafe, full passthrough
- blank line inside a continuation → collapses correctly
- plain single-line commands (git status, cargo test) → unaffected, confirming no regression on existing behavior
All CI green (including full cross-platform pre-release builds). Bundle diff matches its description exactly, nothing unreviewed slipped in. Approving.
…docs 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<String> 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.
Every other agent with a dedicated hook implementation carries a hooks/<agent>/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').
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.
Legacy camelCase invocation rewrites the old stock config to the single-schema form, so upgrades need no manual rtk init --copilot.
Match the exact stock camelCase entry instead of the whole file, so extended configs (extra hooks/keys) heal too; anything non-stock stays untouched.
Real-binary tests: heal correctness, refusal matrix, response integrity, concurrency, unwritable dir.
…paths 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: <rule-id> -- <justification>` on the line above the code), matching precedents in src/discover/lexer.rs and src/core/stream.rs.
Two follow-ups to #3391 (review): 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.
feat(hooks): transparent pre_tool rewrite for Mistral Vibe CLI (closes #800)
aeppling
approved these changes
Aug 6, 2026
Contributor
|
needs : #3449 |
fix(hooks): copilot self heal dual hooks (drop camelCase entry)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Feats
Fix
Other