fix(hooks): fix Windows hooks for Claude Code 2.1.x#398
fix(hooks): fix Windows hooks for Claude Code 2.1.x#398
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughReplaced the SessionStart hook invocation with a Windows CMD wrapper ( Changes
Sequence Diagram(s)sequenceDiagram
participant CMD as run-hook.cmd (Windows CMD)
participant Shell as CLAUDE_BASH_EXE / Git Bash
participant Script as hooks/<script>.sh
CMD->>CMD: parse SCRIPT_NAME and shift args (exclude MSYS2 conversion)
CMD->>Shell: convert paths (cygpath) and call "SCRIPT_DIR/SCRIPT_NAME.sh" with args
Shell->>Script: execute "/path/to/SCRIPT_NAME.sh" (bash -l -c)
Script-->>Shell: exit status / output
Shell-->>CMD: propagate exit status / output
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hooks/run-hook.cmd (1)
4-24:⚠️ Potential issue | 🟡 MinorMisleading deprecation comments contradict actual usage.
The header states the wrapper is "no longer used" and that "hooks.json now calls session-start.sh directly" (line 21), but
hooks.jsonactually invokesrun-hook.cmd session-start. This will confuse future maintainers.Consider updating the comments to accurately describe the current role: the wrapper is still used, but the
.shsuffix is now appended internally to avoid Claude Code's auto-detection.
8635647 to
2cc7d89
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@hooks/run-hook.cmd`:
- Around line 39-43: Add a guard that checks whether SCRIPT_NAME (the first
positional argument) is empty before attempting to execute
"${SCRIPT_DIR}/${SCRIPT_NAME}.sh": if SCRIPT_NAME is unset or empty, print a
clear error message and exit with non-zero status (mirror the existing
Windows/cmd branch behavior) so the script does not try to run
"${SCRIPT_DIR}/.sh"; update the logic around SCRIPT_NAME and SCRIPT_DIR to
perform this check and early exit.
- Around line 33-34: The current invocation of bash forwards only %~2–%9
unquoted which truncates args, breaks spaces and enables injection; change the
logic around the "%~1" script dispatch to SHIFT once and invoke bash with the
full remaining argv forwarded safely using -- %* (so the script name comes from
%~1 and the rest become "$@" inside bash), ensure arguments are properly quoted
when passed to bash, set MSYS2_ARG_CONV_EXCL='*' in the environment before
calling bash to avoid MSYS path conversion, and make the bash path configurable
(e.g., support %CLAUDE_BASH_EXE% or resolve bash from PATH) instead of
hardcoding "C:\Program Files\Git\bin\bash.exe".
2cc7d89 to
f5803d7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@hooks/run-hook.cmd`:
- Around line 33-43: The batch wrapper leaves the original first argument
present in %* which ends up as $1 inside the invoked bash command, so update the
bash invocation lines (the -c argument used in the CLAUDE_BASH_EXE branch and
the default Git Bash branch) to run a shell-side shift before sourcing the hook;
specifically, modify the command passed to bash in the calls that reference
"%CLAUDE_PLUGIN_ROOT%"/hooks/%SCRIPT_NAME%.sh so the inline -c script begins
with shift; and then executes the hook with "$@" so the hook's $1 is the first
original user arg rather than the script name.
Claude Code 2.1.x auto-detects .sh files in hook commands and prepends "bash " on Windows. This breaks the polyglot wrapper because: "run-hook.cmd" session-start.sh → bash "run-hook.cmd" session-start.sh → bash cannot execute .cmd files Solution: - Remove .sh suffix from hook command arguments - Auto-append .sh suffix inside run-hook.cmd This avoids Claude Code's .sh detection while maintaining cross-platform compatibility.
f5803d7 to
2e6b44e
Compare
|
Thank you for the thorough work on this! We ended up going a different direction — removing the polyglot wrapper and calling The core path mangling issue (CC strips backslashes from |
Problem
Claude Code 2.1.x changed the Windows execution model for hooks. It now auto-detects
.shfiles in hook commands and prependsbashon Windows.However, there are two issues:
Issue 1: Polyglot wrapper broken by auto-detection
When the command contains
.shsuffix:Issue 2:
CLAUDE_PLUGIN_ROOTnot normalized on WindowsClaude Code does not normalize
${CLAUDE_PLUGIN_ROOT}to Unix-style paths on Windows. The variable expands to a Windows path with backslashes:This mixed path format (Windows backslashes + Unix forward slashes) causes bash to fail:
So even calling
.shdirectly doesn't work on Windows.Solution
Use the polyglot wrapper
run-hook.cmdwhich:cygpath -uto convert Windows paths to Unix format.shsuffix to avoid Claude Code's auto-detection.shsuffix internallyChanges
hooks/hooks.json: Userun-hook.cmdwrapper with script name without.shsuffixhooks/run-hook.cmd: Update comments to reflect current usage; auto-append.shsuffixTesting
Tested on Windows 11 with Claude Code 2.1.25.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.