Refactor git plugin with dynamic context and improved hooks - #4
Conversation
Major improvements to the git plugin: **Dynamic Context Injection:** - Added dynamic context sections to commit, branch, and pr skills - Skills now pre-fetch git state using \!`command` syntax - Reduces redundant git command execution during skill invocation **New detect-conventions Skill:** - Haiku-based subagent for fast conventional commits detection - Analyzes config files, commit history, and CLAUDE.md - Replaces inline detection logic in commit and branch skills **Shared Mainline Detection Script:** - Single source of truth: git/scripts/detect-mainline.sh - Symlinked into each skill's scripts directory - Priority: cached → upstream → origin → local fallback **Refactored Hooks:** - Replaced prompt-based hooks with jq-based command hooks - Faster execution and more reliable parsing - Added comprehensive hooks/README.md with test plan - 9 hooks: force push protection, skill suggestions, safety warnings **Skill Improvements:** - Quick Reference sections moved to top with navigation links - Pre-flight checks before convention detection in all skills - Integrated error handling into workflow steps - Safety sections moved before Core Workflow **Cleanup:** - Deleted git/reference/ directory (replaced by scripts and inline docs) - Removed broken symlinks to deleted reference files - All reference material now in skills or shared scripts Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Simplify markdownlint configuration by removing reference symlink ignores. Update CLAUDE.md to include explicit config path for markdownlint command. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Documents architectural decisions and patterns from the refactoring: - Dynamic context injection benefits and patterns - Skill structure standardization (Quick Reference at top) - Convention detection with haiku subagent - Mainline detection with symlinked scripts - Command-based hooks architecture and categories - Workflow patterns (pre-flight checks first, integrated error handling) - File organization decisions (deleted reference/, inline docs) - Development learnings (performance improvements, best practices) - Testing and maintenance procedures Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
WalkthroughThis PR restructures the Git plugin from prompt-based to command-based hooks, consolidates documentation, and rewrites skill workflows. Changes include hook system refactoring in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 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
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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
git/skills/commit/SKILL.md (1)
237-243: Missingdetect-conventionsin the integration list.The workflow invokes
detect-conventionsskill in step 2 (line 71), but this isn't listed under "This skill invokes". This inconsistency could confuse users about skill dependencies.Suggested fix
## Integration with Other Skills **This skill invokes:** - **`/git:branch`** - When user tries to commit on mainline +- **`/git:detect-conventions`** - To determine commit message style **This skill is invoked by:** - **`/git:pr`** - When creating PR with uncommitted changes
🤖 Fix all issues with AI agents
In `@git/hooks/hooks.json`:
- Around line 5-12: The force-push detection regex inside the jq test in the
"command" value (the expression that sets $is_force currently using
test("\\s--force|\\s-[a-zA-Z]*f")) is too narrow and misses variants like
--force-with-lease and --force-if-includes; update that test to match all force
variants (e.g., include --force(-with-lease|-if-includes)? or equivalent
word-boundary patterns while keeping the short-flag match \\s-[a-zA-Z]*f) so
$is_force becomes true for --force, --force-with-lease, --force-if-includes and
-f; modify the regex only in the jq test expression inside the command string
referenced in the hooks JSON.
🧹 Nitpick comments (5)
git/scripts/detect-mainline.sh (1)
34-37: Guard env-file caching to avoid errors/duplication.
Consider checking writability and skipping duplicate exports to prevent noisy stderr and growth over repeated runs.♻️ Proposed refinement
-if [ -n "$CLAUDE_ENV_FILE" ] && [ -f "$CLAUDE_ENV_FILE" ]; then - echo "export CLAUDE_MAINLINE_BRANCH='$mainline'" >> "$CLAUDE_ENV_FILE" -fi +if [ -n "$CLAUDE_ENV_FILE" ] && [ -f "$CLAUDE_ENV_FILE" ] && [ -w "$CLAUDE_ENV_FILE" ]; then + if ! grep -q "^export CLAUDE_MAINLINE_BRANCH=" "$CLAUDE_ENV_FILE"; then + echo "export CLAUDE_MAINLINE_BRANCH='$mainline'" >> "$CLAUDE_ENV_FILE" + fi +figit/CLAUDE.md (1)
5-7: Consider removing duplicated content.Lines 5-7 (Key decisions) duplicate the content in lines 11-15 (Architecture > Dynamic Context Injection). Consider keeping just the Architecture section for a single source of truth.
♻️ Suggested fix
Development learnings and architectural decisions for the git plugin. -Key decisions: -- Use [dynamic context injection](https://code.claude.com/docs/en/skills#inject-dynamic-context) to pre-fetch information where possible. -- Place dynamic context section after Quick Reference, before Core Workflow. - ## Architecturegit/skills/pr/SKILL.md (1)
22-28: Consider caching the mainline value in the dynamic context command.Line 28 executes
detect-mainline.shin a subshell, but line 25 already captures the mainline branch. If the dynamic context system supports variable references, you could avoid the redundant script execution.If variable references aren't supported and the script has caching (as mentioned in PR objectives), this is fine as-is.
git/skills/commit/SKILL.md (1)
69-71: Consider clarifying skill invocation syntax.The instruction "Invoke the
detect-conventionsskill" is clear but doesn't specify whether to use the Skill tool or another mechanism. For consistency with line 58 which says "Invoke/git:branchskill using the Skill tool", consider adding similar specificity here.Suggested clarification
### 2. Detect Commit Style -Invoke the `detect-conventions` skill to determine commit style (conventional commits or standard). +Invoke the `/git:detect-conventions` skill using the Skill tool to determine commit style (conventional commits or standard).git/hooks/hooks.json (1)
53-60: Potential false positive in mainline detection regex.The check
test($mainline)performs substring matching, which could trigger on branch names containing the mainline name. For example, if mainline ismain, it would matchgit rebase maintain-featureordomain-branch.Consider using word boundaries for more precise matching:
Suggested fix with word boundaries
- "command": "mainline=$(\"${CLAUDE_PLUGIN_ROOT}/scripts/detect-mainline.sh\"); cat | jq -r --arg mainline \"$mainline\" 'if .tool_input.command | test($mainline) then {\"hookSpecificOutput\":{\"hookEventName\":\"PreToolUse\",\"additionalContext\":\"WARNING: Rebasing onto mainline. Ensure this is intentional.\"}} else null end'", + "command": "mainline=$(\"${CLAUDE_PLUGIN_ROOT}/scripts/detect-mainline.sh\"); jq -r --arg mainline \"$mainline\" 'if .tool_input.command | test(\"(^|\\\\s)\" + $mainline + \"(\\\\s|$)\") then {\"hookSpecificOutput\":{\"hookEventName\":\"PreToolUse\",\"additionalContext\":\"WARNING: Rebasing onto mainline. Ensure this is intentional.\"}} else null end'",Note: Also removed the unnecessary
cat |sincejqreads from stdin by default.
1 participant
Summary
Major refactoring of the git plugin to improve performance, maintainability, and reliability:
Changes
Dynamic Context Injection
!command`` syntaxNew detect-conventions Skill
Shared Mainline Detection
git/scripts/detect-mainline.shas single source of truthHook Refactoring
hooks/README.mdwith test planSkill Improvements
Cleanup
git/reference/directory (4 markdown files)Testing
npx markdownlint-cli2 --config ${CLAUDE_PROJECT_DIR}/.markdownlint-cli2.jsonc "**/*.md"Documentation
git/CLAUDE.mddocumenting all architectural decisionsgit/hooks/README.mdwith hook descriptions and test plan🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Version Updates
✏️ Tip: You can customize this high-level summary in your review settings.