fix: windows compatible startup hook #47
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReplaces the Unix-only SessionStart hook with a cross-platform script ( Changes
Sequence DiagramsequenceDiagram
participant Hook as SessionStart Hook
participant OS as Operating System
participant FS as File System
participant JSON as JSON Output
Hook->>OS: Detect environment (Unix or Windows)
alt Unix (bash/sh)
Hook->>FS: Resolve PLUGIN_ROOT from script location
Hook->>FS: Check legacy dir (~/.config/superpowers/skills)
Note right of Hook: Build warning if legacy exists
Hook->>FS: Read skills/using-superpowers/SKILL.md (with fallback)
Hook->>Hook: Escape strings for JSON
else Windows (cmd/PowerShell)
Hook->>FS: Resolve PLUGIN_ROOT via script/env
Hook->>FS: Check legacy dir
Note right of Hook: Build warning if legacy exists
Hook->>FS: Read SKILL.md via PowerShell (with fallback)
Hook->>Hook: Delegate JSON construction to PowerShell
end
Hook->>JSON: Emit hookSpecificOutput { hookEventName: "SessionStart", additionalContext: ... }
JSON-->>Hook: Success (exit 0)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
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 |
|
This change "works", but actual execution on startup is blocked by anthropics/claude-code#9542, anthropics/claude-code#7755 |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a cross-platform startup hook by replacing the Unix-only bash script with a single polyglot script that runs under both cmd.exe and bash, and updates the hook configuration to call it.
- Adds hooks/session-start.cmd as a cmd/bash polyglot script that outputs the same JSON as the previous .sh
- Updates hooks/hooks.json to point the hook to the new .cmd script
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| hooks/session-start.sh | Removed legacy bash-only script; functionality moved into polyglot script |
| hooks/session-start.cmd | New polyglot implementation containing both bash and Windows cmd/PowerShell logic |
| hooks/hooks.json | Hook command updated to invoke the new .cmd polyglot script |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![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 (1)
hooks/session-start.cmd (1)
61-61: Refactor the PowerShell one-liner for maintainability.This 600+ character command is difficult to read, maintain, and debug. Additionally, there's a theoretical risk if
%SCRIPT_DIR%,%PLUGIN_ROOT%, or%warning_message%contain single quotes, which would break the PowerShell string parsing.Consider extracting to a separate PowerShell script file:
Create
hooks/session-start.ps1:param( [string]$PluginRoot, [string]$WarningMessage ) $skillFile = Join-Path $PluginRoot 'skills\using-superpowers\SKILL.md' if (Test-Path $skillFile) { $skillContent = Get-Content -Path $skillFile -Raw -Encoding UTF8 } else { $skillContent = 'Error: using-superpowers skill file not found' } $contextContent = @" <EXTREMELY_IMPORTANT> You have superpowers. **The content below is from skills/using-superpowers/SKILL.md - your introduction to using skills:** $skillContent $WarningMessage </EXTREMELY_IMPORTANT> "@ $output = @{ hookSpecificOutput = @{ hookEventName = 'SessionStart' additionalContext = $contextContent } } $output | ConvertTo-Json -Depth 10 -Compress:$falseThen simplify line 61:
-powershell -NoProfile -ExecutionPolicy Bypass -Command "$scriptDir = '%SCRIPT_DIR%'; $pluginRoot = '%PLUGIN_ROOT%'; $warningMessage = '%warning_message%'; $skillFile = Join-Path $pluginRoot 'skills\using-superpowers\SKILL.md'; if (Test-Path $skillFile) { $skillContent = Get-Content -Path $skillFile -Raw -Encoding UTF8 } else { $skillContent = 'Error: using-superpowers skill file not found' }; $contextContent = '<EXTREMELY_IMPORTANT>\nYou have superpowers.\n\n**The content below is from skills/using-superpowers/SKILL.md - your introduction to using skills:**\n\n' + $skillContent + '\n\n' + $warningMessage + '\n</EXTREMELY_IMPORTANT>'; $output = @{ hookSpecificOutput = @{ hookEventName = 'SessionStart'; additionalContext = $contextContent } }; $output | ConvertTo-Json -Depth 10 -Compress:$false" +powershell -NoProfile -ExecutionPolicy Bypass -File "%SCRIPT_DIR%session-start.ps1" -PluginRoot "%PLUGIN_ROOT%" -WarningMessage "%warning_message%"This eliminates quote injection risks and improves maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
hooks/hooks.json(1 hunks)hooks/session-start.cmd(1 hunks)hooks/session-start.sh(0 hunks)
💤 Files with no reviewable changes (1)
- hooks/session-start.sh
🔇 Additional comments (2)
hooks/session-start.cmd (1)
1-4: Request confirmation of Windows execution testing.The polyglot mechanism is the critical feature enabling Windows support. While the PR objectives confirm macOS testing, explicit Windows testing confirmation is not mentioned, and the author notes that execution is blocked by external issues.
Please confirm this polyglot executes correctly in a real Windows cmd.exe environment, not just in theory.
hooks/hooks.json (1)
9-9: Request manual verification - hook command format cannot be verified from codebase alone.The escaped quotes in the
commandfield create a literal quote character in the parsed string value:"${CLAUDE_PLUGIN_ROOT}/hooks/session-start.cmd"(with quotes as part of the value).Why verification is needed:
Hook executor is external: The hook system is part of Claude's plugin runtime, not in this repository. No code here processes these hooks.
Pattern suggests intentional change: The RELEASE-NOTES (v2.0.1) mention "Fixed session-start hook execution in plugin context" - the escaped quotes appear to be part of that fix.
Polyglot script requires Windows compatibility: The session-start.cmd file is a polyglot script (bash + batch). Windows batch execution typically requires quoted paths to handle variable expansion and spaces correctly.
No comparative patterns: This is the only hook command in the codebase, so no other examples to validate against.
To verify this is correct:
- Test the hook in Claude Code environment to confirm the session-start.cmd executes successfully on Windows
- Check Claude's plugin hook executor documentation (if available) for command format requirements
- Verify no execution failures occur when the hook runs
| fi | ||
|
|
||
| # Read using-superpowers content | ||
| using_superpowers_content=$(cat "${PLUGIN_ROOT}/skills/using-superpowers/SKILL.md" 2>&1 || echo "Error reading using-superpowers skill") |
There was a problem hiding this comment.
Fix error output handling.
The 2>&1 redirect sends error messages into $using_superpowers_content, polluting it with error text like "cat: file: No such file or directory" before the fallback message. This would appear in the JSON output.
-using_superpowers_content=$(cat "${PLUGIN_ROOT}/skills/using-superpowers/SKILL.md" 2>&1 || echo "Error reading using-superpowers skill")
+using_superpowers_content=$(cat "${PLUGIN_ROOT}/skills/using-superpowers/SKILL.md" 2>/dev/null || echo "Error reading using-superpowers skill")📝 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.
| using_superpowers_content=$(cat "${PLUGIN_ROOT}/skills/using-superpowers/SKILL.md" 2>&1 || echo "Error reading using-superpowers skill") | |
| using_superpowers_content=$(cat "${PLUGIN_ROOT}/skills/using-superpowers/SKILL.md" 2>/dev/null || echo "Error reading using-superpowers skill") |
🤖 Prompt for AI Agents
In hooks/session-start.cmd around line 24, the command captures stderr into the
using_superpowers_content variable (via 2>&1), which pollutes the content with
cat error messages; change the logic to avoid redirecting stderr into the
variable by either checking the file exists/readable before reading and falling
back, or redirect only stderr to /dev/null during the cat and capture a shell
conditional result so the fallback string is used on failure; ensure any errors
are logged separately (to stderr or a log) rather than being mixed into
using_superpowers_content.
| using_superpowers_escaped=$(echo "$using_superpowers_content" | sed 's/\\/\\\\/g' | sed 's/"/\\"/g' | awk '{printf "%s\\n", $0}') | ||
| warning_escaped=$(echo "$warning_message" | sed 's/\\/\\\\/g' | sed 's/"/\\"/g' | awk '{printf "%s\\n", $0}') |
There was a problem hiding this comment.
🧩 Analysis chain
JSON escaping is insufficient and risks invalid output.
The manual escaping only handles backslashes and quotes, missing critical JSON special characters:
- Control characters: tabs (
\t), carriage returns (\r), form feeds, etc. - The SKILL.md may contain any of these, especially if it has Windows line endings
This creates a critical cross-platform inconsistency: the Windows path uses ConvertTo-Json (line 61) which properly escapes all special characters, while bash uses incomplete manual escaping.
Consider using jq for robust JSON generation:
-# Escape outputs for JSON
-using_superpowers_escaped=$(echo "$using_superpowers_content" | sed 's/\\/\\\\/g' | sed 's/"/\\"/g' | awk '{printf "%s\\n", $0}')
-warning_escaped=$(echo "$warning_message" | sed 's/\\/\\\\/g' | sed 's/"/\\"/g' | awk '{printf "%s\\n", $0}')
-
-# Output context injection as JSON
-cat <<EOF
-{
- "hookSpecificOutput": {
- "hookEventName": "SessionStart",
- "additionalContext": "<EXTREMELY_IMPORTANT>\nYou have superpowers.\n\n**The content below is from skills/using-superpowers/SKILL.md - your introduction to using skills:**\n\n${using_superpowers_escaped}\n\n${warning_escaped}\n</EXTREMELY_IMPORTANT>"
- }
-}
-EOF
+# Build additionalContext with proper newline handling
+additional_context="<EXTREMELY_IMPORTANT>\nYou have superpowers.\n\n**The content below is from skills/using-superpowers/SKILL.md - your introduction to using skills:**\n\n${using_superpowers_content}\n\n${warning_message}\n</EXTREMELY_IMPORTANT>"
+
+# Output context injection as JSON using jq for proper escaping
+jq -n --arg ctx "$additional_context" '{
+ hookSpecificOutput: {
+ hookEventName: "SessionStart",
+ additionalContext: $ctx
+ }
+}'If jq is not available in the execution environment, verify this and document the risk of invalid JSON.
🏁 Script executed:
#!/bin/bash
# Check if jq is mentioned elsewhere in the codebase or if it's an available dependency
rg -i '\bjq\b' --type sh --type json --type mdLength of output: 0
I need to gather more information about the codebase environment and the actual implementation. Let me search for dependency files and examine the actual file structure.
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for dependency/configuration files that might list jq as a dependency
fd -e 'package.json' -e 'Gemfile' -e 'requirements.txt' -e 'Dockerfile' -e 'docker-compose.yml' -e '.tool-versions' -e 'brew' | head -20
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check the actual hooks/session-start.cmd file to understand context
cat hooks/session-start.cmd 2>/dev/null || echo "File not found at that location"Length of output: 3617
🏁 Script executed:
#!/bin/bash
# Search more broadly for jq in all configuration and script types
rg 'jq' --type sh --type json --type md --type yaml --type toml --type xml -iLength of output: 0
Rewritten review comment:
JSON escaping is insufficient and creates invalid output with multi-line content.
The bash implementation (lines 27-28) only escapes backslashes and quotes, missing critical issues:
- Actual newlines in SKILL.md content will appear as literal newlines in the JSON string value, rendering it invalid
- Control characters (tabs, carriage returns, etc.) are unescaped
- The Windows path uses
ConvertTo-Jsonwhich properly handles all escaping—cross-platform inconsistency
The proposed jq solution cannot be verified as viable: jq is not listed anywhere in the codebase or its dependencies. Before implementing this approach, confirm jq is available in all target execution environments, or use an alternative approach:
Alternative 1: Use printf with proper escaping before JSON:
additional_context=$(printf '%s' "$using_superpowers_content" | sed 's/\\/\\\\/g' | sed 's/"/\\"/g' | sed 's/$/\\n/' | tr -d '\n')Alternative 2: Build the JSON structure using only variables and avoid heredoc substitution entirely.
Verify that the chosen solution produces valid JSON when SKILL.md contains newlines or special characters.
🤖 Prompt for AI Agents
In hooks/session-start.cmd around lines 27-28, the current sed/awk escaping only
handles backslashes and double quotes and fails to escape actual newlines and
control characters, producing invalid JSON for multi-line SKILL.md; replace this
with a robust escaping approach: either (A) use a portable printf-based pipeline
that escapes backslashes, quotes, control characters and converts newlines to \n
before embedding into JSON (as suggested in the review), or (B) avoid heredoc
substitution entirely and construct the JSON payload by assembling
properly-escaped variables; do NOT rely on jq unless you first verify it is
present in all target environments—if you choose jq, add a runtime check and
fallback—and add a unit/manual test that builds the JSON when SKILL.md contains
newlines, tabs, carriage returns, and Windows paths to ensure the output is
valid JSON.
|
Thank you for working on this! I can't take something that breaks non-windows installs. Is there a clean way for us to do this that does the right thing on windows without breaking macos and linux? |
|
@joegoldin Can you test out #38? |
@obra Are you seeing this fail on mac/linux? It is working across all three of my environments, macOS Tahoe, NixOS, and Windows 11 |
joegoldin
force-pushed
the
fix-windows-startup-hook
branch
from
01741d7 to
b0afa2c
Compare
|
If you're worried about not seeing the output anymore, re: anthropics/claude-code#9591 it's now hidden rather than shown to the user after claude-code 2.0.17, and you have to get it to print it's context to see it. Windows -- still not seeing startup hook output additionalContext into the convo, I think due to anthropics/claude-code#9542 and anthropics/claude-code#7755, but running it manually works as expected. Looks like they're addressing it (hopefully) since the bug was assigned to oncall: anthropics/claude-code#9542 (comment) Linux (Codespace): Linux (NixOS on WSL): macOS Tahoe: |
2 participants
Summary
Fixes startup hook to work on Windows by creating a polyglot script that works in both cmd.exe and bash. Previously,
the hook only had a
.shscript which Windows couldn't execute.Figured it was best to keep this contained/minimal since (hopefully) we can throw away this additional prompt soon.
Changes
hooks/session-start.cmd- a polyglot script that executes correctly in both cmd.exe (Windows) and bash(Mac/Linux)
hooks/hooks.jsonto call the polyglot.cmdscriptMotivation and Context
The original hook only had
session-start.sh, which worked on Mac/Linux but failed on Windows. Windows doesn'tnatively execute
.shfiles, and the hook command syntax wasn't compatible with Windows cmd.exe, preventing thesuperpowers introduction from loading for Windows users.
How Has This Been Tested?
.cmdextension which works on both Windows and Unix systems:;comment technique to create platform-specific code paths that cmd.exeand bash interpret differently
Technical Details
The polyglot script uses the
:<<"::CMDLITERAL"heredoc technique to separate bash and cmd.exe code:.shscript):CMDSCRIPTlabel, executes Windows PowerShell for JSON generationThis single script replaces the need for separate platform-specific files and complex shell fallback logic.
Types of changes
Additional context
Before:
After:
Summary by CodeRabbit