refactor(git): consolidate hooks into single router script - #8
Conversation
WalkthroughConsolidates multiple git-related PreToolUse hooks into a single router-based Bash hook ( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as "Bash tool"
participant Hooks as "hooks.json matcher"
participant Router as "git-bash-router.sh"
participant Detect as "detect-mainline.sh / env"
participant JQ as "jq decision engine"
participant Plugin as "Plugin/Host (receives hookSpecificOutput)"
User->>Hooks: invoke tool (e.g., git push)
Hooks->>Router: match `Bash` -> execute router
Router->>Detect: get mainline (env or detect-mainline.sh)
Detect-->>Router: mainline branch
Router->>JQ: pass tool_input + mainline as jq vars
JQ-->>Router: hookSpecificOutput (permissionDecision / messages)
Router->>Plugin: emit standardized hookSpecificOutput
Plugin-->>User: enforce/notify based on output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
git/scripts/detect-mainline.sh (1)
26-29:⚠️ Potential issue | 🟡 MinorPotential duplicate entries when caching to env file.
If this script is called multiple times before the cache is read, it will append duplicate
CLAUDE_MAINLINE_BRANCHentries toCLAUDE_ENV_FILE. While the last entry typically wins when sourcing, this could cause file growth over long sessions.Consider checking if the variable is already defined in the file before appending, or using a more robust approach.
🛡️ Proposed fix to prevent duplicates
# Cache for session if env file available if [ -n "$CLAUDE_ENV_FILE" ] && [ -f "$CLAUDE_ENV_FILE" ]; then + # Remove any existing entry before adding new one + grep -v "^export CLAUDE_MAINLINE_BRANCH=" "$CLAUDE_ENV_FILE" > "$CLAUDE_ENV_FILE.tmp" && mv "$CLAUDE_ENV_FILE.tmp" "$CLAUDE_ENV_FILE" echo "export CLAUDE_MAINLINE_BRANCH='$mainline'" >> "$CLAUDE_ENV_FILE" fi
🤖 Fix all issues with AI agents
In `@git/scripts/git-bash-router.sh`:
- Around line 68-74: The current rebase detection uses test($mainline) which
does a substring match and can false-positive (e.g., "git rebase
maintain-branch"); update the condition in the git rebase branch of the router
to use a word-boundary match (or reuse the existing pushing_mainline helper
pattern) so it only matches the exact branch name token for $mainline; locate
the elif branch that inspects $cmd and replace the test($mainline) check with a
regex/word-boundary check consistent with pushing_mainline to avoid matching
branch names that merely contain the mainline string.
Replace 8 separate jq hook entries with a single router script pattern: - ~20x performance improvement (15-20ms vs 400ms per Bash tool use) - Pure jq implementation for cross-platform portability - Simplified hooks.json maintenance - Helper functions for reusable patterns (force flags, dry-run, mainline) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
64812b9 to
7981812
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@git/hooks/README.md`:
- Line 14: Edit the README sentence that reads "Pure jq implementation ensures
consistent behavior across Bash 3.2 (macOS system shell) and Bash 5.3+ (modern
systems)." and remove the redundant word "system" so it reads "Pure jq
implementation ensures consistent behavior across Bash 3.2 (macOS shell) and
Bash 5.3+ (modern systems)." Keep the rest of the line unchanged.
In `@git/scripts/git-bash-router.sh`:
- Around line 68-70: The rebase check's regex currently uses "\s" + $mainline +
"(\s|$)" which fails to match remote-prefixed targets like "origin/main"; update
the test expression in the conditional that uses ($cmd | test(...)) to allow an
optional remote prefix before $mainline, e.g. "\s(?:\S+/)?" + $mainline +
"(\s|$)". Apply the same regex change consistently to the pushing_mainline
helper (the helper referenced earlier) so both checks accept remote-prefixed
branch names.
Summary
Changes
git/scripts/git-bash-router.sh: New router script with jq helper functionsgit/hooks/hooks.json: Reduced from 8 matchers to singleBashmatchergit/CLAUDE.md: Updated architecture documentationgit/hooks/README.md: Added Architecture section, updated Technical Notesgit/scripts/detect-mainline.sh: Simplified shebang and removed upstream checkTesting
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.