fix: extract inline SessionStart bootstrap to separate file#1035
Conversation
Inline `node -e "..."` in hooks.json contained `!` characters (e.g. `!org.isDirectory()`) that bash history expansion in certain shell environments would misinterpret, producing syntax errors and the "SessionStart:startup hook error" banner in the Claude Code CLI header. Extract the bootstrap logic to `scripts/hooks/session-start-bootstrap.js` so the shell never sees the JS source. Behaviour is identical: the script reads stdin, resolves the ECC plugin root via CLAUDE_PLUGIN_ROOT or a set of well-known fallback paths, then delegates to run-with-flags.js. Update the test that asserted the old inline pattern to verify the new file-based approach instead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe SessionStart hook command is refactored from a complex inline node script in Changes
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
|
Analysis Failed
Troubleshooting
Retry: |
Greptile SummaryThe review has been completed and submitted via the Greptile review tools. Two P2 style findings were identified: (1) a stale test function name in Confidence Score: 5/5Safe to merge — all remaining findings are P2 style/documentation issues with no runtime impact. Logic is identical to the previous inline version, the approach matches every other non-lifecycle hook in hooks.json, and the fix correctly resolves the bash history-expansion root cause. No files require special attention; tests/hooks/hooks.test.js has a stale test name worth updating. Important Files Changed
Reviews (1): Last reviewed commit: "fix: extract inline SessionStart bootstr..." | Re-trigger Greptile |
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="hooks/hooks.json">
<violation number="1" location="hooks/hooks.json:149">
P1: SessionStart now depends on `CLAUDE_PLUGIN_ROOT` being valid just to start the bootstrap script, which removes prior entry-point fallback/passthrough behavior when the env var is missing or wrong.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| { | ||
| "type": "command", | ||
| "command": "node -e \"const fs=require('fs');const path=require('path');const {spawnSync}=require('child_process');const raw=fs.readFileSync(0,'utf8');const rel=path.join('scripts','hooks','run-with-flags.js');const hasRunnerRoot=candidate=>{const value=typeof candidate==='string'?candidate.trim():'';return value.length>0&&fs.existsSync(path.join(path.resolve(value),rel));};const root=(()=>{const envRoot=process.env.CLAUDE_PLUGIN_ROOT||'';if(hasRunnerRoot(envRoot))return path.resolve(envRoot.trim());const home=require('os').homedir();const claudeDir=path.join(home,'.claude');if(hasRunnerRoot(claudeDir))return claudeDir;for(const candidate of [path.join(claudeDir,'plugins','everything-claude-code'),path.join(claudeDir,'plugins','everything-claude-code@everything-claude-code'),path.join(claudeDir,'plugins','marketplace','everything-claude-code')]){if(hasRunnerRoot(candidate))return candidate;}try{const cacheBase=path.join(claudeDir,'plugins','cache','everything-claude-code');for(const org of fs.readdirSync(cacheBase,{withFileTypes:true})){if(!org.isDirectory())continue;for(const version of fs.readdirSync(path.join(cacheBase,org.name),{withFileTypes:true})){if(!version.isDirectory())continue;const candidate=path.join(cacheBase,org.name,version.name);if(hasRunnerRoot(candidate))return candidate;}}}catch{}return claudeDir;})();const script=path.join(root,rel);if(fs.existsSync(script)){const result=spawnSync(process.execPath,[script,'session:start','scripts/hooks/session-start.js','minimal,standard,strict'],{input:raw,encoding:'utf8',env:process.env,cwd:process.cwd(),timeout:30000});const stdout=typeof result.stdout==='string'?result.stdout:'';if(stdout)process.stdout.write(stdout);else process.stdout.write(raw);if(result.stderr)process.stderr.write(result.stderr);if(result.error||result.status===null||result.signal){const reason=result.error?result.error.message:(result.signal?'signal '+result.signal:'missing exit status');process.stderr.write('[SessionStart] ERROR: session-start hook failed: '+reason+String.fromCharCode(10));process.exit(1);}process.exit(Number.isInteger(result.status)?result.status:0);}process.stderr.write('[SessionStart] WARNING: could not resolve ECC plugin root; skipping session-start hook'+String.fromCharCode(10));process.stdout.write(raw);\"" | ||
| "command": "node \"${CLAUDE_PLUGIN_ROOT}/scripts/hooks/session-start-bootstrap.js\"" |
There was a problem hiding this comment.
P1: SessionStart now depends on CLAUDE_PLUGIN_ROOT being valid just to start the bootstrap script, which removes prior entry-point fallback/passthrough behavior when the env var is missing or wrong.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/hooks.json, line 149:
<comment>SessionStart now depends on `CLAUDE_PLUGIN_ROOT` being valid just to start the bootstrap script, which removes prior entry-point fallback/passthrough behavior when the env var is missing or wrong.</comment>
<file context>
@@ -146,7 +146,7 @@
{
"type": "command",
- "command": "node -e \"const fs=require('fs');const path=require('path');const {spawnSync}=require('child_process');const raw=fs.readFileSync(0,'utf8');const rel=path.join('scripts','hooks','run-with-flags.js');const hasRunnerRoot=candidate=>{const value=typeof candidate==='string'?candidate.trim():'';return value.length>0&&fs.existsSync(path.join(path.resolve(value),rel));};const root=(()=>{const envRoot=process.env.CLAUDE_PLUGIN_ROOT||'';if(hasRunnerRoot(envRoot))return path.resolve(envRoot.trim());const home=require('os').homedir();const claudeDir=path.join(home,'.claude');if(hasRunnerRoot(claudeDir))return claudeDir;for(const candidate of [path.join(claudeDir,'plugins','everything-claude-code'),path.join(claudeDir,'plugins','everything-claude-code@everything-claude-code'),path.join(claudeDir,'plugins','marketplace','everything-claude-code')]){if(hasRunnerRoot(candidate))return candidate;}try{const cacheBase=path.join(claudeDir,'plugins','cache','everything-claude-code');for(const org of fs.readdirSync(cacheBase,{withFileTypes:true})){if(!org.isDirectory())continue;for(const version of fs.readdirSync(path.join(cacheBase,org.name),{withFileTypes:true})){if(!version.isDirectory())continue;const candidate=path.join(cacheBase,org.name,version.name);if(hasRunnerRoot(candidate))return candidate;}}}catch{}return claudeDir;})();const script=path.join(root,rel);if(fs.existsSync(script)){const result=spawnSync(process.execPath,[script,'session:start','scripts/hooks/session-start.js','minimal,standard,strict'],{input:raw,encoding:'utf8',env:process.env,cwd:process.cwd(),timeout:30000});const stdout=typeof result.stdout==='string'?result.stdout:'';if(stdout)process.stdout.write(stdout);else process.stdout.write(raw);if(result.stderr)process.stderr.write(result.stderr);if(result.error||result.status===null||result.signal){const reason=result.error?result.error.message:(result.signal?'signal '+result.signal:'missing exit status');process.stderr.write('[SessionStart] ERROR: session-start hook failed: '+reason+String.fromCharCode(10));process.exit(1);}process.exit(Number.isInteger(result.status)?result.status:0);}process.stderr.write('[SessionStart] WARNING: could not resolve ECC plugin root; skipping session-start hook'+String.fromCharCode(10));process.stdout.write(raw);\""
+ "command": "node \"${CLAUDE_PLUGIN_ROOT}/scripts/hooks/session-start-bootstrap.js\""
}
],
</file context>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/hooks/hooks.test.js (1)
1972-1979: Prefer a behavior test here over source-string checks.Lines 1975-1979 only look for broad substrings that also exist in the file header comment, so this can stay green even if the real probe/spawn logic regresses. Spawning
session-start-bootstrap.jsagainst a temp plugin root would cover passthrough, exit-code forwarding, and the missing-CLAUDE_PLUGIN_ROOTpath directly.
🤖 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/hooks.json`:
- Line 149: The session-start hook command relies on CLAUDE_PLUGIN_ROOT being
set and fails before session-start-bootstrap.js can run its fallback logic
(resolvePluginRoot()); change the hook to invoke a small launcher that locates
the script relative to the installed package (or node module resolution) so the
bootstrap can execute even if CLAUDE_PLUGIN_ROOT is unset—specifically, replace
the direct expansion of
${CLAUDE_PLUGIN_ROOT}/scripts/hooks/session-start-bootstrap.js with a call to
node that requires a launcher (or uses require.resolve on
"scripts/hooks/session-start-bootstrap.js") which then calls the bootstrap's
main entry (session-start-bootstrap.js) allowing resolvePluginRoot() to run and
apply its fallback paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b87d4d1c-b249-4f18-901c-cbbee1e7d984
📒 Files selected for processing (3)
hooks/hooks.jsonscripts/hooks/session-start-bootstrap.jstests/hooks/hooks.test.js
| { | ||
| "type": "command", | ||
| "command": "node -e \"const fs=require('fs');const path=require('path');const {spawnSync}=require('child_process');const raw=fs.readFileSync(0,'utf8');const rel=path.join('scripts','hooks','run-with-flags.js');const hasRunnerRoot=candidate=>{const value=typeof candidate==='string'?candidate.trim():'';return value.length>0&&fs.existsSync(path.join(path.resolve(value),rel));};const root=(()=>{const envRoot=process.env.CLAUDE_PLUGIN_ROOT||'';if(hasRunnerRoot(envRoot))return path.resolve(envRoot.trim());const home=require('os').homedir();const claudeDir=path.join(home,'.claude');if(hasRunnerRoot(claudeDir))return claudeDir;for(const candidate of [path.join(claudeDir,'plugins','everything-claude-code'),path.join(claudeDir,'plugins','everything-claude-code@everything-claude-code'),path.join(claudeDir,'plugins','marketplace','everything-claude-code')]){if(hasRunnerRoot(candidate))return candidate;}try{const cacheBase=path.join(claudeDir,'plugins','cache','everything-claude-code');for(const org of fs.readdirSync(cacheBase,{withFileTypes:true})){if(!org.isDirectory())continue;for(const version of fs.readdirSync(path.join(cacheBase,org.name),{withFileTypes:true})){if(!version.isDirectory())continue;const candidate=path.join(cacheBase,org.name,version.name);if(hasRunnerRoot(candidate))return candidate;}}}catch{}return claudeDir;})();const script=path.join(root,rel);if(fs.existsSync(script)){const result=spawnSync(process.execPath,[script,'session:start','scripts/hooks/session-start.js','minimal,standard,strict'],{input:raw,encoding:'utf8',env:process.env,cwd:process.cwd(),timeout:30000});const stdout=typeof result.stdout==='string'?result.stdout:'';if(stdout)process.stdout.write(stdout);else process.stdout.write(raw);if(result.stderr)process.stderr.write(result.stderr);if(result.error||result.status===null||result.signal){const reason=result.error?result.error.message:(result.signal?'signal '+result.signal:'missing exit status');process.stderr.write('[SessionStart] ERROR: session-start hook failed: '+reason+String.fromCharCode(10));process.exit(1);}process.exit(Number.isInteger(result.status)?result.status:0);}process.stderr.write('[SessionStart] WARNING: could not resolve ECC plugin root; skipping session-start hook'+String.fromCharCode(10));process.stdout.write(raw);\"" | ||
| "command": "node \"${CLAUDE_PLUGIN_ROOT}/scripts/hooks/session-start-bootstrap.js\"" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cmd="$(node -e "const hooks=require('./hooks/hooks.json'); process.stdout.write(hooks.hooks.SessionStart[0].hooks[0].command)")"
stdout_file="$(mktemp)"
stderr_file="$(mktemp)"
status=0
printf '{}' | env -u CLAUDE_PLUGIN_ROOT bash -lc "$cmd" >"$stdout_file" 2>"$stderr_file" || status=$?
printf 'exit=%s\n' "$status"
printf 'stdout=%s\n' "$(cat "$stdout_file")"
printf 'stderr:\n'
cat "$stderr_file"Repository: affaan-m/everything-claude-code
Length of output: 935
🏁 Script executed:
# First, let's examine hooks.json around line 149
sed -n '140,160p' hooks/hooks.jsonRepository: affaan-m/everything-claude-code
Length of output: 609
🏁 Script executed:
# Check if session-start-bootstrap.js exists and examine its content
cat scripts/hooks/session-start-bootstrap.jsRepository: affaan-m/everything-claude-code
Length of output: 4955
🏁 Script executed:
# Also check the structure of hooks.json to understand the SessionStart hook
jq '.hooks.SessionStart' hooks/hooks.jsonRepository: affaan-m/everything-claude-code
Length of output: 349
SessionStart hook cannot start without CLAUDE_PLUGIN_ROOT env var.
Line 149's command expands ${CLAUDE_PLUGIN_ROOT} at shell time. When this env var is unset, the command becomes node "/scripts/hooks/session-start-bootstrap.js" (an absolute path to a non-existent file), so Node fails to load the module before session-start-bootstrap.js can execute resolvePluginRoot() and apply its fallback logic (~/.claude, cache directories, etc.). The bootstrap's resilience becomes unreachable. Provide a launcher or path resolution that allows the bootstrap script itself to be found without depending on CLAUDE_PLUGIN_ROOT being present.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/hooks.json` at line 149, The session-start hook command relies on
CLAUDE_PLUGIN_ROOT being set and fails before session-start-bootstrap.js can run
its fallback logic (resolvePluginRoot()); change the hook to invoke a small
launcher that locates the script relative to the installed package (or node
module resolution) so the bootstrap can execute even if CLAUDE_PLUGIN_ROOT is
unset—specifically, replace the direct expansion of
${CLAUDE_PLUGIN_ROOT}/scripts/hooks/session-start-bootstrap.js with a call to
node that requires a launcher (or uses require.resolve on
"scripts/hooks/session-start-bootstrap.js") which then calls the bootstrap's
main entry (session-start-bootstrap.js) allowing resolvePluginRoot() to run and
apply its fallback paths.
…#1035) Inline `node -e "..."` in hooks.json contained `!` characters (e.g. `!org.isDirectory()`) that bash history expansion in certain shell environments would misinterpret, producing syntax errors and the "SessionStart:startup hook error" banner in the Claude Code CLI header. Extract the bootstrap logic to `scripts/hooks/session-start-bootstrap.js` so the shell never sees the JS source. Behaviour is identical: the script reads stdin, resolves the ECC plugin root via CLAUDE_PLUGIN_ROOT or a set of well-known fallback paths, then delegates to run-with-flags.js. Update the test that asserted the old inline pattern to verify the new file-based approach instead. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…#1035) Inline `node -e "..."` in hooks.json contained `!` characters (e.g. `!org.isDirectory()`) that bash history expansion in certain shell environments would misinterpret, producing syntax errors and the "SessionStart:startup hook error" banner in the Claude Code CLI header. Extract the bootstrap logic to `scripts/hooks/session-start-bootstrap.js` so the shell never sees the JS source. Behaviour is identical: the script reads stdin, resolves the ECC plugin root via CLAUDE_PLUGIN_ROOT or a set of well-known fallback paths, then delegates to run-with-flags.js. Update the test that asserted the old inline pattern to verify the new file-based approach instead. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
SessionStart:startup hook errorin the CLI header on startup in certain shell environments.SessionStarthook inhooks/hooks.jsonused a massive inlinenode -e "..."command. The JavaScript source embedded in that string contains!characters (e.g.!org.isDirectory()) which bash history expansion misinterprets, causing a syntax error before Node even runs.scripts/hooks/session-start-bootstrap.jsand update the hook command tonode "${CLAUDE_PLUGIN_ROOT}/scripts/hooks/session-start-bootstrap.js". The shell never sees the JS source, so!is safe. The logic is byte-for-byte identical to what was inline.Changes
hooks/hooks.jsonnode "${CLAUDE_PLUGIN_ROOT}/scripts/hooks/session-start-bootstrap.js"scripts/hooks/session-start-bootstrap.js'use strict', and JSDoctests/hooks/hooks.test.jsSessionStart hook uses safe inline resolvertest to verify the new file-based approach instead of asserting the oldnode -e "prefixBehaviour
Identical to before: reads stdin, resolves plugin root via
CLAUDE_PLUGIN_ROOTor well-known fallback paths (direct~/.claude, named plugin dirs, versioned cache), delegates torun-with-flags.js session:start, and passes stdout/stderr + exit code through. Falls back gracefully (warning + stdin pass-through) if the root cannot be found.Test plan
node --check scripts/hooks/session-start-bootstrap.js— syntax OKnode tests/hooks/hooks.test.js— 218 passed, 0 failednode tests/run-all.js— full suite green🤖 Generated with Claude Code
Summary by cubic
Extracted the
SessionStartbootstrap from an inlinenode -e "..."inhooks.jsonintoscripts/hooks/session-start-bootstrap.jsto prevent shell history expansion (!) errors on startup. This removes the “SessionStart:startup hook error” banner while keeping behavior the same.node "${CLAUDE_PLUGIN_ROOT}/scripts/hooks/session-start-bootstrap.js".CLAUDE_PLUGIN_ROOTand known fallbacks, then runsrun-with-flags.js session:start.Written for commit 27dd4c7. Summary will update on new commits.
Summary by CodeRabbit