feat(hook): native Claude Code hook for Windows (no bash/jq required)#954
feat(hook): native Claude Code hook for Windows (no bash/jq required)#954Warrio111 wants to merge 4 commits intortk-ai:developfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a Windows-native Claude Code hook entrypoint so rtk init -g can install a hook without relying on bash/jq, enabling automatic command rewriting in Claude Code on Windows.
Changes:
- Add
rtk hook claudesubcommand routing (native hook processor). - Update Windows
rtk init -gdefault path to install a native hook command into~/.claude/settings.json. - Extend hook detection/removal logic to recognize
"hook claude"entries.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
src/main.rs |
Adds hook claude subcommand to the CLI router and documents intended settings.json usage. |
src/hooks/init.rs |
Implements Windows-native hook install flow (native_hook_command, patch_settings_native) and recognizes/removes hook claude entries. |
src/hooks/hook_cmd.rs |
Adds run_claude() wrapper that reuses existing Copilot/Claude-compatible hook JSON handling. |
| println!("\n MANUAL STEP: Add this to ~/.claude/settings.json:"); | ||
| println!(" {{"); | ||
| println!(" \"hooks\": {{ \"PreToolUse\": [{{"); | ||
| println!(" \"matcher\": \"Bash\","); | ||
| println!(" \"hooks\": [{{ \"type\": \"command\","); | ||
| println!(" \"command\": \"{}\"", hook_command); | ||
| println!(" }}]"); |
There was a problem hiding this comment.
In PatchMode::Skip, the manual settings.json snippet prints the hook_command inside JSON quotes, but hook_command itself contains embedded double-quotes ("") so the resulting example line is invalid JSON (unescaped quotes). Consider JSON-escaping the command when printing (e.g., serialize hook_command as a JSON string) or avoid embedding quotes in the displayed snippet.
| let claude_dir = resolve_claude_dir()?; | ||
| let rtk_md_path = claude_dir.join("RTK.md"); | ||
| let claude_md_path = claude_dir.join("CLAUDE.md"); | ||
|
|
||
| // 1. Write RTK.md | ||
| write_if_changed(&rtk_md_path, RTK_SLIM, "RTK.md", verbose)?; |
There was a problem hiding this comment.
Windows run_default_mode writes ~/.claude/RTK.md and ~/.claude/CLAUDE.md via write_if_changed/patch_claude_md, but unlike the unix path it never creates the ~/.claude directory first. If the user runs rtk init -g before Claude has created the folder, these writes will fail. Consider fs::create_dir_all(&claude_dir) right after resolve_claude_dir() (and/or ensure parents exist before each write).
| println!("\nRTK hook installed (Windows native).\n"); | ||
| println!(" Hook: {} hook claude", hook_command); | ||
| println!(" RTK.md: {} (10 lines)", rtk_md_path.display()); | ||
| println!(" CLAUDE.md: @RTK.md reference added"); | ||
|
|
||
| if migrated { | ||
| println!("\n [ok] Migrated: removed RTK block from CLAUDE.md, replaced with @RTK.md"); | ||
| } | ||
|
|
||
| match patch_result { | ||
| PatchResult::Patched => {} | ||
| PatchResult::AlreadyPresent => { | ||
| println!("\n settings.json: hook already present"); | ||
| println!(" Restart Claude Code. Test with: git status"); | ||
| } | ||
| PatchResult::Declined | PatchResult::Skipped => {} | ||
| } | ||
|
|
There was a problem hiding this comment.
This success banner is printed unconditionally, even when patch_settings_native returns Declined/Skipped (user opted out or manual mode). That can mislead users into thinking the hook is installed when settings.json wasn’t patched. Consider tailoring the message based on PatchResult (e.g., only say “installed” on Patched/AlreadyPresent, and otherwise say “manual step required”).
| println!("\nRTK hook installed (Windows native).\n"); | |
| println!(" Hook: {} hook claude", hook_command); | |
| println!(" RTK.md: {} (10 lines)", rtk_md_path.display()); | |
| println!(" CLAUDE.md: @RTK.md reference added"); | |
| if migrated { | |
| println!("\n [ok] Migrated: removed RTK block from CLAUDE.md, replaced with @RTK.md"); | |
| } | |
| match patch_result { | |
| PatchResult::Patched => {} | |
| PatchResult::AlreadyPresent => { | |
| println!("\n settings.json: hook already present"); | |
| println!(" Restart Claude Code. Test with: git status"); | |
| } | |
| PatchResult::Declined | PatchResult::Skipped => {} | |
| } | |
| match patch_result { | |
| PatchResult::Patched => { | |
| println!("\nRTK hook installed (Windows native).\n"); | |
| println!(" Hook: {} hook claude", hook_command); | |
| println!(" RTK.md: {} (10 lines)", rtk_md_path.display()); | |
| println!(" CLAUDE.md: @RTK.md reference added"); | |
| println!(" settings.json: hook added"); | |
| println!(" Restart Claude Code. Test with: git status"); | |
| } | |
| PatchResult::AlreadyPresent => { | |
| println!("\nRTK hook installed (Windows native).\n"); | |
| println!(" Hook: {} hook claude", hook_command); | |
| println!(" RTK.md: {} (10 lines)", rtk_md_path.display()); | |
| println!(" CLAUDE.md: @RTK.md reference added"); | |
| println!(" settings.json: hook already present"); | |
| println!(" Restart Claude Code. Test with: git status"); | |
| } | |
| PatchResult::Declined | PatchResult::Skipped => { | |
| println!("\nRTK hook not installed into settings.json (user opted out / manual mode).\n"); | |
| println!(" Hook command (for manual setup): {} hook claude", hook_command); | |
| println!(" RTK.md: {} (10 lines)", rtk_md_path.display()); | |
| println!(" CLAUDE.md: @RTK.md reference added"); | |
| println!(" Next step: update settings.json manually to call the hook command above."); | |
| } | |
| } | |
| if migrated { | |
| println!("\n [ok] Migrated: removed RTK block from CLAUDE.md, replaced with @RTK.md"); | |
| } |
| .any(|cmd| { | ||
| // Exact match OR both contain rtk-rewrite.sh | ||
| // Exact match OR both contain same hook fingerprint | ||
| cmd == hook_command | ||
| || (cmd.contains("rtk-rewrite.sh") && hook_command.contains("rtk-rewrite.sh")) | ||
| || (cmd.contains("hook claude") && hook_command.contains("hook claude")) | ||
| }) |
There was a problem hiding this comment.
hook_already_present now treats any command containing hook claude as an installed RTK hook, but the existing unit tests only cover the rtk-rewrite.sh fingerprint. Please add a test case for a native Windows command (e.g., "C:\\...\\rtk.exe" hook claude) to ensure idempotency works for the new hook path.
| if let Some(hooks_array) = entry.get("hooks").and_then(|h| h.as_array()) { | ||
| for hook in hooks_array { | ||
| if let Some(command) = hook.get("command").and_then(|c| c.as_str()) { | ||
| if command.contains("rtk-rewrite.sh") { | ||
| if command.contains("rtk-rewrite.sh") || command.contains("hook claude") { | ||
| return false; // Remove this entry |
There was a problem hiding this comment.
remove_hook_from_json now removes entries whose command contains hook claude, but the existing remove_hook_from_json tests only cover removal of rtk-rewrite.sh. Please add a test that verifies native hook commands are removed on uninstall without affecting other hooks.
Warrio111
force-pushed
the
feat/windows-native-hook-rewrite
branch
from
b6abc40 to
e831a63
Compare
|
|
Warrio111
force-pushed
the
feat/windows-native-hook-rewrite
branch
from
e831a63 to
02d9683
Compare
|
@copilot apply changes based on the comments in this thread |
|
Hello, i have a few things to solve on others PR then i can get to you for this review You can run all test on your machine and repush once solved because you got failing a check |
Warrio111
force-pushed
the
feat/windows-native-hook-rewrite
branch
from
e96e4a3 to
f631af6
Compare
|
Sorry @aeppling I didn't saw the failure last day. |
|
Hi, I went through the code carefully and the installation-side fixes look solid the JSON escaping, 🔴 The hook silently ignores deny and ask rulesThis is the main one. The existing bash hook doesn't do the rewrite logic itself — it delegates to
The fix would be giving 🟡 A few smaller things worth noting
Copilot CLI format can leak through Silent double-install when migrating from bash hook 🟡 Test coverage gapsThe idempotency and removal tests that were added cover the happy path well. But the functional
Per the contributing guide: "Every change must include tests." A test for the deny rule case Overall the direction is right and the install-side is clean. The permission bypass is the blocker |
|
@marcoalc-uco Thanks for the thorough analysis — all points addressed in the latest push. 🔴 Permission bypass — fixed
🟡 Copilot CLI format leak — fixed
🟡 Double-install on bash → native migration — fixed
🟡 Tests added
|
|
@Warrio111 Thanks for the fast turnaround — went through the new commit and everything checks out. Permission bypass : Copilot CLI leak : the explicit Double-install : the new Tests : 9 new tests, all passing (1357 total). The coexistence cases On the LGTM from my side. 👍 |
1llum1n4t1s
left a comment
1llum1n4t1s
left a comment
There was a problem hiding this comment.
Bug: hook_check.rs doesn't detect the native hook — "No hook installed" warning on every invocation
hook_installed_path() (src/hooks/hook_check.rs, L93-104) determines hook status solely by checking whether ~/.claude/hooks/rtk-rewrite.sh exists. However, the native hook introduced in this PR writes directly to settings.json ("rtk.exe" hook claude) without deploying a bash script, so status() always returns HookStatus::Missing.
Reproduction (Windows 11 Pro, rtk 0.34.1 built from this branch, Rust 1.94.1)
$ rtk init -g # installs native hook into settings.json ✅
$ rtk git status # hook works correctly, command is rewritten ✅
[rtk] /!\ No hook installed — run `rtk init -g` for automatic token savings # ← spurious warning ❌
Root cause
hook_check.rs checks only for the bash script file:
fn hook_installed_path() -> Option<PathBuf> {
// ...
let path = home.join(CLAUDE_DIR).join(HOOKS_SUBDIR).join(REWRITE_HOOK_FILE); // "rtk-rewrite.sh"
if path.exists() { Some(path) } else { None }
}Meanwhile, init.rs L864 (hook_already_present()) already detects both variants correctly:
.any(|cmd| cmd.contains(REWRITE_HOOK_FILE) || cmd.contains("hook claude"))This inconsistency means init knows the native hook is installed, but hook_check does not.
Suggested fix
Add a settings.json check to status() in hook_check.rs — if PreToolUse hooks contain "hook claude", return HookStatus::Ok. Reusing the same predicate as hook_already_present() keeps the detection logic consistent.
// Pseudocode for the additional path in status():
// 1. ~/.claude/hooks/rtk-rewrite.sh exists → existing version check
// 2. settings.json PreToolUse contains "hook claude" → HookStatus::Ok
// 3. Neither → HookStatus::Missing
|
@1llum1n4t1s Good catch — fixed in the latest commit. Root cause: Fix: Added Three unit tests added for the new helper (native hook detected, bash-only returns false, missing file returns false). The existing |
…red) Adds `rtk hook claude` subcommand that reads Claude Code PreToolUse JSON from stdin and rewrites commands natively — no bash or jq dependency. On Windows, `rtk init -g` now installs `rtk hook claude` directly in settings.json instead of printing a warning and falling back to --claude-md. Changes: - src/hooks/hook_cmd.rs: add run_claude() (wraps run_copilot(), same format) - src/main.rs: add HookCommands::Claude routing to run_claude() - src/hooks/init.rs: - native_hook_command(): resolves rtk binary path - patch_settings_native(): installs command string in settings.json - fs::create_dir_all before writing RTK.md/CLAUDE.md on Windows - JSON-escape hook_command in manual snippet to avoid invalid JSON - Conditional banner based on PatchResult (not unconditional) - hook_already_present/remove_hook_from_json recognize 'hook claude' - Tests: hook_already_present and remove_hook_from_json for native hook
…install
- run_claude() now has its own implementation instead of delegating to
run_copilot(). It only handles Claude Code's snake_case format, avoiding
accidental routing of Copilot CLI camelCase payloads through the wrong
response format.
- Permission rules are now checked before rewriting (mirrors rewrite_cmd.rs):
* Deny verdict → silent passthrough, Claude Code enforces natively
* Ask verdict → emit updatedInput but omit permissionDecision so
Claude Code prompts the user
* Allow/Default → emit updatedInput with permissionDecision: "allow"
- hook_already_present() now treats any RTK hook (bash or native) as
installed, preventing double-installation when a shared settings.json
already contains rtk-rewrite.sh and the user runs rtk init -g on Windows.
- Added tests: deny/ask/allow/default verdicts, format validation,
unsupported command passthrough, and bash+native coexistence detection.
…arning rtk init -g on Windows installs hook claude directly in settings.json without writing rtk-rewrite.sh, so status() returned HookStatus::Missing and printed "No hook installed" on every command invocation. Add native_hook_in_settings() which reads ~/.claude/settings.json and checks for a PreToolUse hook containing "hook claude". status() consults this before the file-existence check, returning Ok immediately when the native hook is active. Updated test_status_returns_valid_variant to recognise the native hook as a valid installed state; added three unit tests for the new helper.
Warrio111
force-pushed
the
feat/windows-native-hook-rewrite
branch
from
4d4feff to
46f8f5f
Compare
…pdate docs
- Replace contains("hook claude") with ends_with("hook claude") in
hook_check.rs, init.rs (hook_already_present, remove_hook_from_json)
and init.rs (run_uninstall_mode) — anchored suffix match avoids
theoretical false positives from unrelated commands that happen to
contain the substring.
- Add test_claude_excluded_command_returns_none: verifies that commands
in the exclude list produce no hook output via build_claude_response().
- Fix misleading comment: "Explicit allow" → "Allow (explicit) or Default
(no rules configured)" to accurately describe both handled verdicts.
- Update src/hooks/README.md: document Windows native hook installation
mode in the Installation Modes table, add Windows native hook section
with example settings.json snippet, and update per-tool support table
to distinguish Unix bash hook from Windows native hook.
|
@aeppling Do you see improvements that i have to implement? |
|
@pszymkowiak It looks good to you? |
|
Looking for it |
|
Superseded by #956 |
Problem
On Windows, `rtk init -g` prints a warning and silently falls back to `--claude-md` mode because the Claude Code hook was implemented as a bash script (`rtk-rewrite.sh`) requiring `bash` + `jq` — neither available natively on Windows.
```
[warn] Hook-based mode requires Unix (macOS/Linux).
Windows: use --claude-md mode for full injection.
Falling back to --claude-md mode.
```
This means Windows users get no automatic command rewriting in Claude Code sessions.
Solution
Add `rtk hook claude` — a native Rust subcommand that reads Claude Code's `PreToolUse` JSON from stdin, checks permission rules, and rewrites commands with zero external dependencies.
On Windows, `rtk init -g` now installs `"<rtk.exe>" hook claude` directly in `settings.json` instead of falling back.
Changes
Permission protocol
`run_claude()` mirrors the exit-code contract of `rewrite_cmd.rs`:
Before / After
Before (Windows):
```
$ rtk init -g
[warn] Hook-based mode requires Unix (macOS/Linux).
Falling back to --claude-md mode.
```
After (Windows):
```
$ rtk init -g
RTK hook installed (Windows native).
Hook: "C:\Users...\rtk.exe" hook claude
RTK.md: C:\Users....claude\RTK.md (10 lines)
CLAUDE.md: @RTK.md reference added
settings.json: hook added
```
Relation to PR #150
PR #150 proposes a similar fix with a full standalone reimplementation (~1126 lines). This PR reuses existing infrastructure (`permissions.rs`, `registry`, `init.rs`) and adds ~350 lines of changes with no new dependencies.
Test plan