-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add Windows support for plugin hooks #134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Create polyglot session-start.cmd that works in both CMD and bash - Update hooks.json to use the .cmd polyglot launcher - Replace sed/awk with pure bash for JSON escaping (Windows compatibility) The polyglot script uses a heredoc trick: - CMD sees the @echo off block and runs bash.exe with cygpath conversion - Bash sees a heredoc and skips to the Unix section 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Add docs/windows/polyglot-hooks.md explaining the cross-platform technique - Add reusable run-hook.cmd wrapper for parameterized hook execution - Document how the polyglot works in CMD vs bash - Include troubleshooting section and related GitHub issues 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Run ./test-polyglot.sh from repo root to verify: - Required files exist with execute permissions - Simple wrapper (session-start.cmd) produces valid JSON - Parameterized wrapper (run-hook.cmd) works - Heredoc correctly skips CMD block on Unix 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Remove session-start.cmd in favor of run-hook.cmd - Update hooks.json to use: run-hook.cmd session-start.sh - Simplify test script to only test run-hook.cmd This makes it easy to add more hooks - just create the .sh file and add a line to hooks.json pointing to run-hook.cmd with the script name. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Pass path directly to bash instead of using cygpath in a subshell. The complex quoting was causing issues on Windows. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Testing complete - polyglot hooks work on Windows and macOS. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
WalkthroughAdds cross-platform polyglot hooks for Claude Code with a new wrapper script enabling execution across Windows and Unix environments. Introduces documentation describing the pattern, updates hook configuration to use the wrapper, and refactors JSON escaping logic from sed/awk to pure bash. Changes
Sequence DiagramsequenceDiagram
autonumber
participant IDE as IDE/Claude Code
participant Wrapper as run-hook.cmd
participant Detector as Platform Check
participant Git as Git Bash
participant Script as session-start.sh
IDE->>Wrapper: Execute run-hook.cmd<br/>(session-start.sh arg)
alt Windows Platform
Wrapper->>Detector: Detect OS
Detector-->>Wrapper: Windows detected
Wrapper->>Git: Invoke via Git Bash<br/>bash.exe session-start.sh
Git->>Script: Execute script
Script-->>Git: Generate JSON output<br/>(pure-bash escaping)
Git-->>Wrapper: Return output
else Unix-like Platform
Wrapper->>Detector: Detect OS
Detector-->>Wrapper: Unix detected
Wrapper->>Script: Invoke directly<br/>./session-start.sh
Script-->>Wrapper: Generate JSON output<br/>(pure-bash escaping)
end
Wrapper-->>IDE: Return hook result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
hooks/session-start.sh (1)
20-40: Pure-bash JSON escaping is sound for expected inputs; consider stricter control-char coverage only if neededThe
escape_for_jsonhelper correctly handles the important cases for this hook (backslashes, quotes, and the common control characters\n,\r,\t), and the move away fromsed/awkis a good fit for Git Bash / Windows compatibility. The usage forusing_superpowers_escapedandwarning_escapedis also consistent.If you ever expect less common control characters (any byte
< 0x20beyond newline, carriage return, and tab), you might extend thecaseto map those to\u00XXescapes for fully strict JSON compliance, but for markdown‑style content this is probably unnecessary.docs/windows/polyglot-hooks.md (1)
136-185: Clarify examplerun-hook.cmdvs. the actual implementation to avoid confusionThe “Reusable Wrapper Pattern” section shows a
run-hook.cmdexample that usescygpathand abash -l -c "cd ... && ./SCRIPT_NAME"pattern, while the repository’shooks/run-hook.cmdcurrently uses a simplerbash -l "%~dp0%~1"approach.Both patterns are valid, but it may help readers if the doc explicitly labels this as an illustrative alternative, or if you update the snippet to mirror the actual
hooks/run-hook.cmdimplementation so users don’t wonder why the checked-in script looks different.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/windows/polyglot-hooks.md(1 hunks)hooks/hooks.json(1 hunks)hooks/run-hook.cmd(1 hunks)hooks/session-start.sh(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/windows/polyglot-hooks.md
54-54: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (1)
hooks/hooks.json (1)
5-10: SessionStart hook command wiring looks correctThe quoted wrapper path plus
session-start.shargument is appropriate for both Unix and Windows and matches the newrun-hook.cmdcontract. No issues from my side here.
| ``` | ||
| hooks/ | ||
| ├── hooks.json # Points to the .cmd wrapper | ||
| ├── session-start.cmd # Polyglot wrapper (cross-platform entry point) | ||
| └── session-start.sh # Actual hook logic (bash script) | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a language to the file-structure code fence to satisfy markdownlint
The file-structure block is the only fenced block without a language, which triggers MD040. You can fix this by tagging it, e.g.:
-```
+```text
hooks/
├── hooks.json # Points to the .cmd wrapper
├── session-start.cmd # Polyglot wrapper (cross-platform entry point)
└── session-start.sh # Actual hook logic (bash script)
This keeps formatting the same while satisfying markdownlint-cli2.
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
54-54: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In docs/windows/polyglot-hooks.md around lines 54 to 59, the fenced
file-structure block lacks a language tag which triggers markdownlint rule
MD040; update the opening fence to include a language (for example change ``` to
resolved, leaving the contents and closing fence unchanged.
| : << 'CMDBLOCK' | ||
| @echo off | ||
| REM Polyglot wrapper: runs .sh scripts cross-platform | ||
| REM Usage: run-hook.cmd <script-name> [args...] | ||
| REM The script should be in the same directory as this wrapper | ||
|
|
||
| "C:\Program Files\Git\bin\bash.exe" -l "%~dp0%~1" | ||
| exit /b | ||
| CMDBLOCK | ||
|
|
||
| # Unix shell runs from here | ||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd)" | ||
| SCRIPT_NAME="$1" | ||
| shift | ||
| "${SCRIPT_DIR}/${SCRIPT_NAME}" "$@" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
find . -name "run-hook.cmd" -o -name "run-hook.sh" | head -20Repository: obra/superpowers
Length of output: 80
🏁 Script executed:
cat -n ./hooks/run-hook.cmdRepository: obra/superpowers
Length of output: 574
🏁 Script executed:
rg "run-hook" --type sh --type cmd --type json --type yaml -A 2 -B 2Repository: obra/superpowers
Length of output: 87
🏁 Script executed:
rg "run-hook" -A 2 -B 2Repository: obra/superpowers
Length of output: 1500
Windows side drops additional args; align with documented run-hook.cmd <script-name> [args...] contract
The Unix section forwards extra args (shift then "$@"), but the CMD section only passes %~dp0%~1 into Bash. Any arguments after the script name are silently discarded on Windows, despite the documented [args...] usage.
To keep behavior symmetric and future‑proof, update the CMD block to forward all arguments:
: << 'CMDBLOCK'
@echo off
REM Polyglot wrapper: runs .sh scripts cross-platform
REM Usage: run-hook.cmd <script-name> [args...]
REM The script should be in the same directory as this wrapper
-
-"C:\Program Files\Git\bin\bash.exe" -l "%~dp0%~1"
-exit /b
+if "%~1"=="" (
+ echo run-hook.cmd: missing script name 1>&2
+ exit /b 1
+)
+set "SCRIPT_DIR=%~dp0"
+set "SCRIPT_NAME=%~1"
+shift
+"C:\Program Files\Git\bin\bash.exe" -l "%SCRIPT_DIR%%SCRIPT_NAME%" %*
+exit /b
CMDBLOCKThis preserves the polyglot structure, provides clearer error messaging when the script name is missing, and forwards all additional args on Windows to match the Unix behavior.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| : << 'CMDBLOCK' | |
| @echo off | |
| REM Polyglot wrapper: runs .sh scripts cross-platform | |
| REM Usage: run-hook.cmd <script-name> [args...] | |
| REM The script should be in the same directory as this wrapper | |
| "C:\Program Files\Git\bin\bash.exe" -l "%~dp0%~1" | |
| exit /b | |
| CMDBLOCK | |
| # Unix shell runs from here | |
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd)" | |
| SCRIPT_NAME="$1" | |
| shift | |
| "${SCRIPT_DIR}/${SCRIPT_NAME}" "$@" | |
| : << 'CMDBLOCK' | |
| @echo off | |
| REM Polyglot wrapper: runs .sh scripts cross-platform | |
| REM Usage: run-hook.cmd <script-name> [args...] | |
| REM The script should be in the same directory as this wrapper | |
| if "%~1"=="" ( | |
| echo run-hook.cmd: missing script name 1>&2 | |
| exit /b 1 | |
| ) | |
| set "SCRIPT_DIR=%~dp0" | |
| set "SCRIPT_NAME=%~1" | |
| shift | |
| "C:\Program Files\Git\bin\bash.exe" -l "%SCRIPT_DIR%%SCRIPT_NAME%" %* | |
| exit /b | |
| CMDBLOCK | |
| # Unix shell runs from here | |
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd)" | |
| SCRIPT_NAME="$1" | |
| shift | |
| "${SCRIPT_DIR}/${SCRIPT_NAME}" "$@" |
🤖 Prompt for AI Agents
In hooks/run-hook.cmd lines 1-15, the Windows CMD stub only passes the script
name to bash and drops any additional args; update it to (1) validate the script
name is present and print a short usage/error if missing, (2) capture the full
path to the script into a variable (e.g. set "SCRIPT=%~dp0%~1"), (3) shift the
cmd args so %* then contains only the script arguments, and (4) invoke bash with
the script path and the remaining args (e.g. "C:\Program Files\Git\bin\bash.exe"
-l "%SCRIPT%" %*), then exit /b with the bash exit code.
- Add validation for missing script name - Forward up to 8 additional arguments to bash on Windows - Fixes CodeRabbit review feedback from #134
* feat: Add Windows support for session-start hook - Create polyglot session-start.cmd that works in both CMD and bash - Update hooks.json to use the .cmd polyglot launcher - Replace sed/awk with pure bash for JSON escaping (Windows compatibility) The polyglot script uses a heredoc trick: - CMD sees the @echo off block and runs bash.exe with cygpath conversion - Bash sees a heredoc and skips to the Unix section 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: Add execute permission to session-start.cmd for Unix 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * docs: Add comprehensive polyglot hooks documentation - Add docs/windows/polyglot-hooks.md explaining the cross-platform technique - Add reusable run-hook.cmd wrapper for parameterized hook execution - Document how the polyglot works in CMD vs bash - Include troubleshooting section and related GitHub issues 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * test: Add polyglot hook test script for macOS/Linux Run ./test-polyglot.sh from repo root to verify: - Required files exist with execute permissions - Simple wrapper (session-start.cmd) produces valid JSON - Parameterized wrapper (run-hook.cmd) works - Heredoc correctly skips CMD block on Unix 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: Use direct pipe to jq in test to avoid variable escaping issues 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * refactor: Use single reusable run-hook.cmd for all hooks - Remove session-start.cmd in favor of run-hook.cmd - Update hooks.json to use: run-hook.cmd session-start.sh - Simplify test script to only test run-hook.cmd This makes it easy to add more hooks - just create the .sh file and add a line to hooks.json pointing to run-hook.cmd with the script name. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: Simplify run-hook.cmd CMD block Pass path directly to bash instead of using cygpath in a subshell. The complex quoting was causing issues on Windows. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * chore: Remove test-polyglot.sh Testing complete - polyglot hooks work on Windows and macOS. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> --------- Co-authored-by: Claude <[email protected]>
- Add validation for missing script name - Forward up to 8 additional arguments to bash on Windows - Fixes CodeRabbit review feedback from obra#134
Summary
run-hook.cmd) that works on Windows CMD and Unix bashsession-start.shfor Windows compatibilitydocs/windows/polyglot-hooks.mdHow it works
The polyglot uses a heredoc trick:
:as a label, ignores the heredoc syntax, runs the Windows block, thenexit /b:as a no-op, consumes the Windows block as a heredoc, then runs the Unix blockRequirements
Test plan
Related issues
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.