fix(hooks): add .cmd wrapper for Windows compatibility (#491)#496
fix(hooks): add .cmd wrapper for Windows compatibility (#491)#496dotuananh0712 wants to merge 2 commits intoobra:mainfrom
Conversation
Adds guidance to run git diff commands separately instead of chaining with && to avoid consent prompts. Fixes obra#476
Creates session-start.cmd polyglot wrapper that works on both Windows (CMD.exe) and Unix (bash). Updates hooks.json to reference the .cmd file instead of .sh. Fixes obra#491: SessionStart hook fails on Windows because hooks.json points to .sh file which CMD.exe cannot execute.
📝 WalkthroughWalkthroughChanges introduce a Windows-compatible wrapper script ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
hooks/hooks.json (1)
9-9: Change looks correct — consider adding single quotes for paths with spaces.The
.sh→.cmdrename aligns with the new polyglot wrapper. One minor concern: ifCLAUDE_PLUGIN_ROOTexpands to a path containing spaces (e.g.,C:\Users\Name With Spaces\…), the unquoted variable may break. Single-quoting the expansion protects against word splitting.- "command": "${CLAUDE_PLUGIN_ROOT}/hooks/session-start.cmd", + "command": "'${CLAUDE_PLUGIN_ROOT}/hooks/session-start.cmd'",This is a pre-existing concern (the old
.shpath had the same issue), so it can be addressed separately. Based on learnings: "single quotes around the variable are necessary because Claude Code expands variables before passing the command to the shell."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/hooks.json` at line 9, Update the "command" value for the session-start hook to quote the CLAUDE_PLUGIN_ROOT expansion so paths with spaces are preserved: change the unquoted "${CLAUDE_PLUGIN_ROOT}/hooks/session-start.cmd" in the "command" field to use single quotes around the expansion (i.e., quote the whole path string) to prevent word-splitting when CLAUDE_PLUGIN_ROOT contains spaces.hooks/session-start.cmd (1)
21-21: Drop the-l(login shell) flag — it's unnecessary and may cause side effects.The
-lflag makes bash run as a login shell, sourcing~/.bash_profileand similar. This adds startup overhead and may pull in user-specific environment changes that interfere with the hook script. A plainbashinvocation is sufficient here.-"C:\Program Files\Git\bin\bash.exe" -l "%SCRIPT_DIR%/session-start.sh" +"C:\Program Files\Git\bin\bash.exe" "%SCRIPT_DIR%/session-start.sh"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/session-start.cmd` at line 21, Change the Bash invocation that currently uses the login-shell flag by removing "-l" from the command that runs session-start.sh; i.e., update the invocation of "C:\Program Files\Git\bin\bash.exe" -l "%SCRIPT_DIR%/session-start.sh" so it calls bash without "-l" (leave the path to session-start.sh as-is) to avoid login-shell side effects and extra startup overhead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/session-start.cmd`:
- Line 21: The hardcoded Git Bash path in session-start.cmd ("C:\Program
Files\Git\bin\bash.exe") can fail for portable, 32-bit, or custom
installs—update the script to first try resolving bash from PATH (e.g., using
where/which) and, if not found, fall back to checking common locations like
%ProgramFiles%\Git\bin\bash.exe and %ProgramFiles(x86)%\Git\bin\bash.exe before
erroring; modify the call site in session-start.cmd that invokes bash so it uses
the discovered executable or a clear error message if none found.
- Around line 1-27: The hooks/session-start.cmd file must use CRLF line endings
for Windows CMD; update the repository metadata and the file: add a
.gitattributes entry (e.g., "*.cmd text eol=crlf" or "hooks/session-start.cmd
text eol=crlf") to enforce CRLF for .cmd files, then convert
hooks/session-start.cmd to CRLF line endings (ensure the initial CMDBLOCK batch
section and the rest of the file are saved with CRLF) and commit both the
.gitattributes change and the normalized file so CMD.exe will parse the batch
labels and commands correctly.
---
Nitpick comments:
In `@hooks/hooks.json`:
- Line 9: Update the "command" value for the session-start hook to quote the
CLAUDE_PLUGIN_ROOT expansion so paths with spaces are preserved: change the
unquoted "${CLAUDE_PLUGIN_ROOT}/hooks/session-start.cmd" in the "command" field
to use single quotes around the expansion (i.e., quote the whole path string) to
prevent word-splitting when CLAUDE_PLUGIN_ROOT contains spaces.
In `@hooks/session-start.cmd`:
- Line 21: Change the Bash invocation that currently uses the login-shell flag
by removing "-l" from the command that runs session-start.sh; i.e., update the
invocation of "C:\Program Files\Git\bin\bash.exe" -l
"%SCRIPT_DIR%/session-start.sh" so it calls bash without "-l" (leave the path to
session-start.sh as-is) to avoid login-shell side effects and extra startup
overhead.
obra
added
bug
|
Closing — this is superseded by v4.3.1 (PR #523), which takes a similar approach: restoring the polyglot Thank you for the contribution and for helping diagnose these Windows issues! |
2 participants
Summary
session-start.cmdpolyglot wrapper that works on both Windows (CMD.exe) and Unix (bash)hooks.jsonto reference the.cmdfile instead of.shFixes #491
SessionStart hook fails on Windows because
hooks.jsonpoints to.shfile which CMD.exe cannot execute.Changes
hooks/session-start.cmd- polyglot wrapperhooks/hooks.json- changed command path from.shto.cmdVerification
session-start.cmdruns correctly on Unix (outputs valid JSON)hooks.jsonis valid JSON.cmdwrapper that invokes bash to run the original.shscriptSummary by CodeRabbit
New Features
Documentation