Skip to content

fix: emit valid Claude Code hook schema in settings.json#180

Closed
pashak1207 wants to merge 1 commit intotirth8205:mainfrom
pashak1207:fix/claude-code-hook-schema
Closed

fix: emit valid Claude Code hook schema in settings.json#180
pashak1207 wants to merge 1 commit intotirth8205:mainfrom
pashak1207:fix/claude-code-hook-schema

Conversation

@pashak1207
Copy link
Copy Markdown

Claude Code rejects the file generated by code-review-graph install with "Expected array, but received undefined" because each entry under PostToolUse/SessionStart needs an inner hooks: [{type, command}] array, not a flat command: field. On top of that, the generator emits a PreCommit block — which isn't a Claude Code event at all.

Consequence: the entire settings.json is discarded, so neither the PostToolUse auto-update hook nor the SessionStart status hook ever fires.

This rewrites generate_hooks_config() to produce the correct schema, drops the bogus PreCommit block, and narrows the PostToolUse matcher from Edit|Write|Bash to Edit|Write. Bash calls in Claude Code don't modify source files directly, so running an incremental graph update after every shell command was just noise.

Tests in TestGenerateHooksConfig were updated to assert the new schema, plus a new regression test (test_entries_use_claude_code_hook_schema) guards against the flat-command shape recurring.

Fixes #97, #163, #172.

Claude Code rejects the file generated by `code-review-graph install`
with "Expected array, but received undefined" because each entry under
PostToolUse/SessionStart needs an inner `hooks: [{type, command}]`
array, not a flat `command:` field. On top of that, the generator
emits a `PreCommit` block — which isn't a Claude Code event at all.

Consequence: the entire settings.json is discarded, so neither the
PostToolUse auto-update hook nor the SessionStart status hook ever
fires.

This rewrites generate_hooks_config() to produce the correct schema,
drops the bogus PreCommit block, and narrows the PostToolUse matcher
from `Edit|Write|Bash` to `Edit|Write`. Bash calls in Claude Code
don't modify source files directly, so running an incremental graph
update after every shell command was just noise.

Tests in TestGenerateHooksConfig were updated to assert the new
schema, plus a new regression test (test_entries_use_claude_code_hook_schema)
guards against the flat-command shape recurring.

Fixes tirth8205#97, tirth8205#163, tirth8205#172.
@ZzAve
Copy link
Copy Markdown

ZzAve commented Apr 9, 2026

FWIW: I applied these fixed to my local setup and can confirm code-review-graph to be working with claude.

lngyeen added a commit to lngyeen/code-review-graph that referenced this pull request Apr 10, 2026
Two fixes for Claude Code hooks integration:

1. Hook schema: use proper Claude Code format with nested hooks array
   and remove unsupported PreCommit event. Narrow PostToolUse matcher
   from Edit|Write|Bash to Edit|Write since Bash commands do not
   directly modify source files. (cherry-picked from PR tirth8205#180)

2. Merge logic: install_hooks now merges new entries into existing
   hooks instead of overwriting them. Creates a backup of
   settings.json before modification. (based on PR tirth8205#145)

Closes tirth8205#97, tirth8205#114, tirth8205#172.
lngyeen added a commit to lngyeen/code-review-graph that referenced this pull request Apr 10, 2026
Two fixes for Claude Code hooks integration:

1. Hook schema: use proper Claude Code format with nested hooks array
   and remove unsupported PreCommit event. Narrow PostToolUse matcher
   from Edit|Write|Bash to Edit|Write since Bash commands do not
   directly modify source files. (cherry-picked from PR tirth8205#180)

2. Merge logic: install_hooks now merges new entries into existing
   hooks instead of overwriting them. Creates a backup of
   settings.json before modification. (based on PR tirth8205#145)

Closes tirth8205#97, tirth8205#114, tirth8205#172.
@tirth8205
Copy link
Copy Markdown
Owner

Closing in favor of #208 which was just merged and provides a more complete fix.

Your PR correctly fixes the two critical bugs: nested hooks array wrapper and removal of the invalid PreCommit event. The narrowing of the PostToolUse matcher from Edit|Write|Bash to Edit|Write is a reasonable call too.

However, there is one remaining issue in this PR: the timeouts (5000, 3000) are still in milliseconds. According to the Claude Code docs, timeout values are in seconds. Leaving them at 5000 would mean an 83-minute timeout on the PostToolUse hook. PR #208 correctly converts these to 30s and 10s.

PR #208 also adds a new install_git_hook() function that restores the pre-commit intent (which was lost when removing the invalid PreCommit hook event) by writing a proper .git/hooks/pre-commit script instead — with tests for create, append, idempotent, and no-.git-dir cases.

Thank you for the contribution — identifying the timeout unit bug was a key insight.

@tirth8205 tirth8205 closed this Apr 11, 2026
lngyeen added a commit to lngyeen/code-review-graph that referenced this pull request Apr 13, 2026
Two fixes for Claude Code hooks integration:

1. Hook schema: use proper Claude Code format with nested hooks array
   and remove unsupported PreCommit event. Narrow PostToolUse matcher
   from Edit|Write|Bash to Edit|Write since Bash commands do not
   directly modify source files. (cherry-picked from PR tirth8205#180)

2. Merge logic: install_hooks now merges new entries into existing
   hooks instead of overwriting them. Creates a backup of
   settings.json before modification. (based on PR tirth8205#145)

Closes tirth8205#97, tirth8205#114, tirth8205#172.
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.

Plugin hooks fail to load - Hooks use a matcher + hooks array.

3 participants