fix(claude-plugin): replace agent-type stop hook with command-type block decision#213
Conversation
…k decision The agent-type stop hook spawned a sub-agent without conversation context, so the learn skill had nothing to analyze. Switch to a command-type hook that returns a block decision, keeping execution in the parent agent which has the full conversation history.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 32 minutes and 20 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplaces the Stop hook's agent-based execution with a command that runs a new Python (and companion shell) stop script which reads JSON stdin, checks Changes
Sequence Diagram(s)sequenceDiagram
participant Hook as Stop Hook (hooks.json)
participant Runner as Command Runner
participant Script as on_stop.py / on_stop.sh
participant Skill as evolve-lite:learn
Hook->>Runner: invoke configured command (stdin JSON)
Runner->>Script: execute script with stdin
Script->>Script: parse stdin JSON, check stop_hook_active
alt stop_hook_active == true
Script-->>Hook: exit silently (no output)
else stop_hook_active != true
Script-->>Hook: output JSON decision {action: "block", suppressOutput: true, systemMessage: "run /evolve-lite:learn", ...}
Hook->>Skill: trigger /evolve-lite:learn per decision
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
vinodmut
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
platform-integrations/claude/plugins/evolve-lite/hooks/hooks.json (1)
30-31: Quote the expanded plugin-root script path.If
CLAUDE_PLUGIN_ROOTcontains whitespace, Line 31 splits the path beforebashreceives it and the Stop hook will not run. Claude Code’s security guidance also recommends quoting shell variables: Claude Code hooks reference.🛠️ Proposed fix
{ "type": "command", - "command": "bash ${CLAUDE_PLUGIN_ROOT}/skills/learn/scripts/on_stop.sh" + "command": "bash \"${CLAUDE_PLUGIN_ROOT}/skills/learn/scripts/on_stop.sh\"" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platform-integrations/claude/plugins/evolve-lite/hooks/hooks.json` around lines 30 - 31, The Stop hook command string uses an unquoted environment variable expansion ("bash ${CLAUDE_PLUGIN_ROOT}/skills/learn/scripts/on_stop.sh") which breaks if CLAUDE_PLUGIN_ROOT contains whitespace; update the "command" value for this hook to quote the expanded path so bash receives the full path (e.g., use bash with the path wrapped in double quotes), preserving the same script path and keeping the "command" JSON key and its value intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@platform-integrations/claude/plugins/evolve-lite/skills/learn/scripts/on_stop.sh`:
- Around line 10-15: The stop hook script on_stop.sh is emitting an invalid
field (statusMessage) in the hook output JSON; remove statusMessage from the
JSON returned by on_stop.sh so it only includes the allowed fields (decision,
reason, and optional continue) and keep suppressOutput only if supported, and
then add the spinner text as the hook configuration in hooks.json (attach the
statusMessage or equivalent display text to the stop hook entry in hooks.json).
Update on_stop.sh to output only decision and reason and move the
spinner/display text into hooks.json's stop hook configuration using the
statusMessage key there.
---
Nitpick comments:
In `@platform-integrations/claude/plugins/evolve-lite/hooks/hooks.json`:
- Around line 30-31: The Stop hook command string uses an unquoted environment
variable expansion ("bash
${CLAUDE_PLUGIN_ROOT}/skills/learn/scripts/on_stop.sh") which breaks if
CLAUDE_PLUGIN_ROOT contains whitespace; update the "command" value for this hook
to quote the expanded path so bash receives the full path (e.g., use bash with
the path wrapped in double quotes), preserving the same script path and keeping
the "command" JSON key and its value intact.
🪄 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: 02dfae30-b45a-4967-a218-26c0a61901e5
📒 Files selected for processing (2)
platform-integrations/claude/plugins/evolve-lite/hooks/hooks.jsonplatform-integrations/claude/plugins/evolve-lite/skills/learn/scripts/on_stop.sh
…decision Addresses CodeRabbit review finding: statusMessage is not a valid hook response field; replaced with systemMessage.
The test_command_hook_scripts_exist test only accepted .py scripts, causing it to fail for the new on_stop.sh bash hook.
Code Review
|
visahak
left a comment
visahak
left a comment
There was a problem hiding this comment.
added comments in the issue
… key Addresses review feedback from visahak: - Replace on_stop.sh (requires jq) with on_stop.py using stdlib json module, eliminating the unstated external dependency on jq - Fix incorrect key "systemMessage" → "statusMessage" in hook JSON output
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@platform-integrations/claude/plugins/evolve-lite/skills/learn/scripts/on_stop.py`:
- Around line 17-22: The JSON printed in on_stop.py uses the wrong output field:
replace the "statusMessage" key with "systemMessage" in the dict printed by the
print(json.dumps(...)) call so the message is shown to the user per the Claude
Code hooks contract; update the dict in the on_stop.py print block (the produced
JSON with "decision": "block", "reason": ..., "suppressOutput": True) to include
"systemMessage": "Running the evolve-lite learn skill..." instead of
"statusMessage".
🪄 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: 27e4e14c-ddd7-4945-8d46-66a7602c0e41
📒 Files selected for processing (3)
platform-integrations/claude/plugins/evolve-lite/hooks/hooks.jsonplatform-integrations/claude/plugins/evolve-lite/skills/learn/scripts/on_stop.pytests/platform_integrations/test_plugin_structure.py
🚧 Files skipped from review as they are similar to previous changes (1)
- platform-integrations/claude/plugins/evolve-lite/hooks/hooks.json
The hook output contract uses systemMessage (displayed to user), not statusMessage (which is a hook config field in hooks.json). Addresses CodeRabbit review finding: Change statusMessage to systemMessage
Good catch on the jq dependency — rewrote On the key name though:
The Fixed in @visahak could you check if the latest commits address your review? |
2 participants
Summary
on_stop.shscript with recursion guard and block decision JSONCloses #212
Test plan
stop_hook_activeguard prevents recursion on block-decision continuationsuppressOutputandstatusMessageproduce clean UXSummary by CodeRabbit
Improvements
Tests