Skip to content

perf(hooks): narrow Bash hook matchers to git commands only#655

Open
carlos-alm wants to merge 3 commits intomainfrom
perf/narrow-bash-hook-matchers
Open

perf(hooks): narrow Bash hook matchers to git commands only#655
carlos-alm wants to merge 3 commits intomainfrom
perf/narrow-bash-hook-matchers

Conversation

@carlos-alm
Copy link
Contributor

Summary

  • Changed Bash hook matchers from "Bash" to "Bash(git )" in both .claude/settings.json and docs/examples/claude-code-hooks/settings.json
  • Git-specific hooks (guard-git, pre-commit, lint-staged, check-readme, guard-pr-body, track-moves, post-git-ops) now only fire on git commands instead of every Bash invocation
  • Reduces token consumption by eliminating unnecessary hook executions on non-git commands (ls, npm test, node, etc.)

Test plan

  • Verify git commands still trigger all hooks (e.g. git commit fires pre-commit, lint-staged, guard-git)
  • Verify non-git Bash commands (ls, npm test) no longer trigger git hooks
  • Verify Edit/Write hooks (update-graph, track-edits) are unaffected
  • Verify Read/Grep hooks (enrich-context) are unaffected

Bash hooks (guard-git, pre-commit, lint-staged, check-readme,
guard-pr-body, track-moves, post-git-ops) now use `Bash(git )`
matcher instead of `Bash`, so they only fire on git commands
rather than every Bash invocation — reduces token consumption.
@claude
Copy link

claude bot commented Mar 27, 2026

Claude finished @carlos-alm's task —— View job


PR Review Complete ⭐⭐⭐⭐☆

Todo List:

  • Read and analyze changed files
  • Understand hook system context from CLAUDE.md
  • Apply 5-phase review framework
  • Provide comprehensive review feedback

Root Cause Analysis

Problem Being Solved: Excessive token consumption due to git-specific hooks firing on every Bash command invocation (including ls, npm test, node, etc.), not just git operations.

Why This Approach: The matcher pattern "Bash(git )" specifically targets commands beginning with "git ", which is the correct and surgical fix. This maintains all existing git hook functionality while eliminating unnecessary executions.

Risk Assessment: Low risk. This is a pure optimization that narrows trigger scope without changing hook behavior or removing any safety checks.


Backlog Compliance

  • Zero-dep: ✓ — No new dependencies, uses existing hook system
  • Foundation-aligned: ✓ — Fully compliant, optimizes local infrastructure without violating any principles
  • Problem-fit: 3 — Useful optimization that reduces token waste when AI agents run non-git commands, but doesn't directly address the core "lost AI" navigation problem
  • Breaking: No — Pure optimization, no behavior change for git commands
  • Tier: 1 (zero-dep ✓ + foundation-aligned ✓)

Technical Analysis

Excellent Changes

  1. Surgical precision: Changes only the matcher patterns, preserving all hook logic
  2. Consistent implementation: Applied to both production (.claude/settings.json) and documentation (docs/examples/) files
  3. Clear intent: Commit message precisely explains the optimization goal

Hook System Understanding

The changes correctly target these git-specific hooks:

  • PreToolUse: guard-git.sh, pre-commit.sh, lint-staged.sh, check-readme.sh, guard-pr-body.sh
  • PostToolUse: track-moves.sh, post-git-ops.sh

While preserving non-git hooks:

  • Read/Grep: enrich-context.sh (unchanged)
  • Edit/Write: update-graph.sh, track-edits.sh (unchanged)

Performance Impact

This optimization will significantly reduce unnecessary hook executions during common development tasks:

  • npm test, npm install, node script.js → No longer trigger git hooks
  • ls, cat, grep → No longer trigger git hooks
  • git commit, git push, git add → Still trigger all appropriate hooks

Minor Observations

  1. Test Coverage: The test plan in the PR description is comprehensive and covers the key scenarios
  2. Documentation Consistency: Both production and example configurations are updated consistently
  3. Backward Compatibility: Existing git workflows remain completely unchanged

Final Recommendation

  • Rating: ⭐⭐⭐⭐☆ (4/5)
  • Action: APPROVE
  • Reasoning: This is a well-executed performance optimization with clear benefits, minimal risk, and proper implementation. The only reason it doesn't get 5 stars is that it's an optimization rather than a feature that significantly advances the core mission.

Recommended merge: This PR delivers immediate token consumption benefits for Claude Code users with zero downside risk.


@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 27, 2026

Greptile Summary

This PR attempted to reduce token consumption by narrowing Bash hook matchers from "Bash" to "Bash(git )", then correctly reverted that change after review feedback identified the syntax as invalid. The Bash(git ) pattern belongs to the Claude Code permissions system (allowedTools), not the hooks matcher system — the hooks matcher regex is applied to the tool name ("Bash"), not the command string, so the pattern would never match and would silently disable all git-safety hooks.\n\nThe final commit restores both .claude/settings.json and docs/examples/claude-code-hooks/settings.json to plain "Bash" matchers. The PR is a net-zero change from main. All previous review concerns have been fully addressed.

Confidence Score: 5/5

Safe to merge — the revert restores both settings files to their correct base-branch state with no functional delta.

All previously identified P0/P1 issues have been addressed by the final revert commit. The PR is a net-zero change from main, leaving hooks fully operational. No new issues were found.

No files require special attention.

Important Files Changed

Filename Overview
.claude/settings.json Reverted PreToolUse and PostToolUse Bash matchers from invalid Bash(git ) back to plain Bash. Net change from base branch is zero; file is functionally correct.
docs/examples/claude-code-hooks/settings.json Same revert as the primary settings file — PreToolUse and PostToolUse matchers restored to plain Bash. Net change from base branch is zero; example docs are correct.

Reviews (2): Last reviewed commit: "fix(hooks): revert Bash matchers to plai..." | Re-trigger Greptile

"PreToolUse": [
{
"matcher": "Bash",
"matcher": "Bash(git )",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Hook matcher syntax is incorrect for the hooks system

The Claude Code hooks documentation is explicit: for PreToolUse and PostToolUse events, the matcher is a regex string matched against the tool name. The tool name for any Bash invocation is the plain string "Bash".

The regex Bash(git ) — when applied to the tool name "Bash" — will never match, because the pattern requires the string to be "Bashgit " (i.e., "Bash" immediately followed by "git "). With no match, none of these hooks will fire at all, silently disabling guard-git.sh, check-readme.sh, guard-pr-body.sh, pre-commit.sh, and lint-staged.sh for every git operation.

The Bash(command_prefix) parenthesis syntax is specific to Claude Code's permissions system (e.g. allowedTools: ["Bash(git commit:*)"]) and is not documented as a valid hook matcher pattern. The hooks reference examples show only bare tool names like "Bash", "Edit|Write", and "mcp__.*" as valid matchers for tool events.

The hooks already perform their own internal command filtering — guard-git.sh checks for (git|gh)\s+ on line 25, and pre-commit.sh checks for git\s+commit on line 26 — so keeping the matcher as "Bash" achieves correctness while the scripts themselves handle the narrowing.

Suggested change
"matcher": "Bash(git )",
"matcher": "Bash",

The same issue affects the PostToolUse matcher on line 72 of this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — reverted all matchers to plain "Bash". The hook scripts already filter internally for git commands (e.g. guard-git.sh checks for (git|gh)\s+, pre-commit.sh checks for git\s+commit), so narrowing at the matcher level is unnecessary and, as you correctly identified, breaks the hooks entirely since the matcher regex is applied to the tool name, not the command string.

},
{
"matcher": "Bash",
"matcher": "Bash(git )",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Same invalid hook matcher disables PostToolUse hooks

For the same reason as the PreToolUse matcher above, "Bash(git )" will never match the tool name "Bash", so track-moves.sh and post-git-ops.sh would also never fire.

Suggested change
"matcher": "Bash(git )",
"matcher": "Bash",

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — same revert applied to the PostToolUse matcher. Both PreToolUse and PostToolUse matchers in .claude/settings.json are now plain "Bash" again.

},
{
"matcher": "Bash",
"matcher": "Bash(git )",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Same invalid matcher pattern in the example docs

This file has the same "Bash(git )" matcher on lines 15 and 52. Both should revert to "Bash" for the same reasons: the hook matcher regex is applied to the tool name, not the command string, and Bash(git ) does not match "Bash".

Suggested change
"matcher": "Bash(git )",
"matcher": "Bash",

Line 52 in this file has the same issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — same revert applied to the docs example file (docs/examples/claude-code-hooks/settings.json). Both PreToolUse and PostToolUse matchers there are now plain "Bash" as well.

The `Bash(git )` matcher syntax does not work — the matcher regex is
applied to the tool name string ("Bash"), not the command argument.
Using `Bash(git )` silently disables all git safety hooks. The hook
scripts already filter internally for git commands, so narrowing at
the matcher level is unnecessary.
@carlos-alm
Copy link
Contributor Author

@greptileai

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant