-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Fix session-start hook execution in plugin context #9
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
Fix session-start hook execution in plugin context #9
Conversation
… grep results The hook was failing with 'Plugin hook error:' on startup due to two issues: 1. BASH_SOURCE[0] is unbound when the script is executed in certain contexts (like Claude Code plugin execution), causing failure with set -u Fix: Use /bin/bash to fallback to $0 2. When filtering status flags, grep returns exit code 1 when all lines are filtered out, causing script to exit with set -e Fix: Add '|| true' to allow empty result without failing These changes allow the hook to run successfully in all execution contexts.
WalkthroughThe session-start hook script updates how it resolves its directory by adding a fallback when BASH_SOURCE[0] is unset and makes the SKILLS_UPDATED/BEHIND flag cleanup tolerant to no-output cases by appending Changes
Sequence Diagram(s)sequenceDiagram
participant U as Invoker (hook)
participant S as session-start.sh
participant FS as Filesystem
U->>S: Execute hook
rect rgba(200,230,255,0.3)
note over S: Resolve script directory
S->>S: SCRIPT_DIR = dir("${BASH_SOURCE[0]:-$0}")
end
rect rgba(220,255,220,0.3)
note over S,FS: Initialize/clean output flags
S->>FS: Remove SKILLS_UPDATED/BEHIND markers
FS-->>S: May return no results
S->>S: Continue on empty via "|| true"
end
S-->>U: Proceed with remaining startup
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
|
this PR is all ai-generated, btw.... i just asked claude/superpowers to help debug the issue. |
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: 0
🧹 Nitpick comments (2)
hooks/session-start.sh (2)
18-18: Simplify flag stripping; avoid masking real errors.Use a single grep (or sed) to drop both lines without needing
|| true, reducing processes and keeping failures visible.Apply one of the following:
-init_output=$(echo "$init_output" | grep -v "SKILLS_UPDATED=true" | grep -v "SKILLS_BEHIND=true" || true) +init_output=$(echo "$init_output" | grep -Ev 'SKILLS_UPDATED=true|SKILLS_BEHIND=true' || true)Or with sed (no need for
|| true):-init_output=$(echo "$init_output" | grep -v "SKILLS_UPDATED=true" | grep -v "SKILLS_BEHIND=true" || true) +init_output=$(echo "$init_output" | sed -E '/SKILLS_UPDATED=true|SKILLS_BEHIND=true/d')
15-16:skills_updatedappears unused (SC2034).Either use it for messaging or remove to avoid confusion.
Option A — remove it:
-skills_updated=$(echo "$init_output" | grep "SKILLS_UPDATED=true" || echo "") skills_behind=$(echo "$init_output" | grep "SKILLS_BEHIND=true" || echo "")Option B — surface a success message:
skills_behind=$(echo "$init_output" | grep "SKILLS_BEHIND=true" || echo "") # Build status messages that go at the end status_message="" +if [ -n "$skills_updated" ]; then + status_message="\n\n✅ Skills updated from upstream."${status_message} +fi if [ -n "$skills_behind" ]; then status_message="\n\n⚠️ New skills available from upstream. Ask me to use the pulling-updates-from-skills-repository skill." fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
hooks/session-start.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
hooks/session-start.sh
[warning] 15-15: skills_updated appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (1)
hooks/session-start.sh (1)
10-10: Good fix for set -u contexts.
${BASH_SOURCE[0]:-$0}safely handles unbound BASH_SOURCE and keeps path resolution robust when sourced or executed.
|
I'm also hitting the empty grep results issue here when using a fork of the skills directory. As summarized by Claude code: When a user is working on a fork with local commits ahead of the tracking branch, initialize-skills.sh outputs only (This PR uses || true instead of || echo "") |
Problem
The
session-start.shhook fails silently when executed by Claude Code with error "⏺ Plugin hook error:" preventing skills context from loading.Root Cause
BASH_SOURCE[0]is unbound in Claude Code's execution context (fails withset -u)grepresults return exit code 1 (fails withset -e)Solution
${BASH_SOURCE[0]:-$0}to fallback to$0whenBASH_SOURCEunavailable|| trueto handle empty grep results gracefullyTesting
✅ Direct execution produces valid JSON
✅ Claude Code plugin execution now works
✅ No breaking changes to existing functionality
Changes
hooks/session-start.sh: 2 lines changed${BASH_SOURCE[0]:-$0}fallback|| truefor empty grep resultsSummary by CodeRabbit
Bug Fixes
Chores