chore(claude): add granular hook timeouts per event type - #1213
Conversation
- notify.sh (osascript): 3s — local notification, very fast - pushover.sh (curl w/ --max-time 5): 6s — just above curl ceiling - git-ai checkpoint: 15s — git network operation - rtk-rewrite.sh / security.sh: 5s — blocking pre-tool scripts - UserPromptSubmit jq: 3s — pure local I/O - WorktreeCreate/Remove: 30s — git worktree operations Entire-Checkpoint: fa19a44077e5
|
Warning Rate limit exceeded
⌛ 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: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the timeout configurations for various internal hooks within the Claude system, aiming to optimize performance and prevent indefinite waits. It introduces granular timeouts for several existing commands and adds timeouts to previously unbounded operations, ensuring more predictable execution and responsiveness. Additionally, a new Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
Mesa DescriptionTL;DRAdded granular, per-event type hook timeouts within Claude's configuration to prevent slow operations from blocking the system, particularly for notifications and various Git operations. What changed?
Description generated by Mesa. Update settings |
There was a problem hiding this comment.
Code Review
This pull request refactors the hook timeouts to be more granular and appropriate for each event type, which is a great improvement for performance and reliability. I've reviewed the changes and they look good, with adjusted timeouts for notifications, git operations, and other scripts. I found one issue: a duplicate hook configuration for Bash tools was added in PreToolUse, which could cause issues. I've left a comment with a suggestion to remove the redundant block. Other than that, the changes are solid.
| }, | ||
| { | ||
| "matcher": "Bash", | ||
| "hooks": [ | ||
| { | ||
| "type": "command", | ||
| "command": "dcg" | ||
| } | ||
| ] | ||
| } |
There was a problem hiding this comment.
This adds a duplicate hook configuration for matcher: "Bash" within the PreToolUse event. An existing configuration for this matcher is already defined on lines 125-158, which includes an asynchronous, safer call to dcg. This new block introduces a synchronous, blocking call to dcg without a timeout, which is redundant and could lead to performance issues or failures if dcg is not present or hangs. This block should be removed to avoid duplication and potential problems.
}There was a problem hiding this comment.
2 issues found across 1 file
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="config/claude/settings.json">
<violation number="1" location="config/claude/settings.json:169">
P2: This adds a second `PreToolUse` entry with `matcher: "Bash"`, duplicating the existing Bash matcher block (which already includes an async `dcg` hook among others). Additionally, this synchronous `dcg` command has no `timeout` set, which contradicts the purpose of this PR to add granular timeouts to all hooks. If `dcg` hangs, it will block tool execution indefinitely.</violation>
<violation number="2" location="config/claude/settings.json:173">
P1: The new Bash hook runs `dcg` unconditionally and without an explicit timeout. This can fail on machines where `dcg` is not installed and weakens the timeout bounding introduced in this change.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Pull request overview
Adjusts Claude Code hook configuration to use per-event timeout ceilings, aiming to bound hook latency (notably Stop hooks) while allowing slower git-related operations more time.
Changes:
- Reduce notification hook timeouts (
notify.sh→ 3s,pushover.sh→ 6s) across multiple events. - Increase
git-ai checkpointhook timeout to 15s for tool-write/edit events. - Add/adjust explicit timeouts for
rtk-rewrite.sh,UserPromptSubmithistory append, and worktree create/remove hooks.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| }, | ||
| { | ||
| "matcher": "Bash", | ||
| "hooks": [ | ||
| { | ||
| "type": "command", | ||
| "command": "dcg" | ||
| } | ||
| ] |
| { | ||
| "matcher": "Bash", | ||
| "hooks": [ | ||
| { | ||
| "type": "command", | ||
| "command": "dcg" | ||
| } | ||
| ] |
Entire-Checkpoint: 194eb96a8bbc
Summary
notify.sh(all events):3s— osascript local notificationpushover.sh(all events):6s— curl has--max-time 5, so 6s is the ceilinggit-ai checkpoint:15s— git network op (was 10s)rtk-rewrite.sh:5s— blocking pre-tool script, now has explicit timeoutUserPromptSubmitjq:3s— pure local I/O (was 5s)WorktreeCreate/Remove:30s— git worktree ops (was no timeout)Follows the curl timeout fix in pushover.sh from #1212 to fully bound Stop hook latency to ≤6s.
🤖 Generated with Claude Code
Summary by cubic
Add per-event hook timeouts in
config/claude/settings.jsonto cap notification latency and prevent slow hooks from blocking. Sets practical ceilings for git/network tasks and fast local scripts.New Features
notify.sh3s;pushover.sh6s (Stop hook ≤6s).git-ai checkpoint15s;WorktreeCreate/Remove30s.rtk-rewrite.shandsecurity.sh5s.UserPromptSubmitjq 3s.dcghook (and async probe) with 5s.Bug Fixes
dcghook entry.Written for commit 1996613. Summary will update on new commits.