Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ Hotfix on top of 2.2.3 for two bugs surfaced by a full first-time-user smoke tes
### Changed
- Community detection is now bounded — large repos complete in reasonable time instead of hanging indefinitely.

### Fixed
- **`install_hooks` now merges instead of overwriting** (PR #203, fixes #114): `install_hooks()` previously used `dict.update()` which clobbered any user-defined hooks in `.claude/settings.json`. Now merges new entries into existing hook arrays, preserving user hooks. Creates a backup (`settings.json.bak`) before modification.

## [2.2.2] - 2026-04-08

### Added
Expand Down
26 changes: 23 additions & 3 deletions code_review_graph/skills.py
Original file line number Diff line number Diff line change
Expand Up @@ -484,8 +484,9 @@ def install_git_hook(repo_root: Path) -> Path | None:
def install_hooks(repo_root: Path) -> None:
"""Write hooks config to .claude/settings.json.

Merges with existing settings if present, preserving non-hook
configuration.
Merges new hook entries into existing settings, preserving both
non-hook configuration and user-defined hooks. A backup of the
original file is created before any modifications.

Args:
repo_root: Repository root directory.
Expand All @@ -498,11 +499,30 @@ def install_hooks(repo_root: Path) -> None:
if settings_path.exists():
try:
existing = json.loads(settings_path.read_text(encoding="utf-8"))
backup_path = settings_dir / "settings.json.bak"
shutil.copy2(settings_path, backup_path)
logger.info("Backed up existing settings to %s", backup_path)
except (json.JSONDecodeError, OSError) as exc:
logger.warning("Could not read existing %s: %s", settings_path, exc)

hooks_config = generate_hooks_config()
existing.update(hooks_config)
existing_hooks = existing.get("hooks", {})
if not isinstance(existing_hooks, dict):
logger.warning("Existing hooks config is not a dict; replacing with defaults")
existing_hooks = {}

merged_hooks = dict(existing_hooks)
for hook_name, hook_entries in hooks_config.get("hooks", {}).items():
if isinstance(merged_hooks.get(hook_name), list):
merged_list = list(merged_hooks[hook_name])
for entry in hook_entries:
if entry not in merged_list:
merged_list.append(entry)
merged_hooks[hook_name] = merged_list
else:
merged_hooks[hook_name] = hook_entries

existing["hooks"] = merged_hooks

settings_path.write_text(json.dumps(existing, indent=2) + "\n", encoding="utf-8")
logger.info("Wrote hooks config: %s", settings_path)
Expand Down
27 changes: 27 additions & 0 deletions tests/test_skills.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,33 @@ def test_hook_entries_use_nested_hooks_array(self):
assert "hooks" in entry, f"{hook_type} entry missing 'hooks' array"
assert "command" not in entry, f"{hook_type} has bare 'command' outside hooks[]"

def test_entries_use_claude_code_hook_schema(self):
"""Regression guard for the Claude Code hook schema.

Claude Code rejects entries that put ``command`` directly on the
event entry. Each entry must wrap its command(s) in a
``hooks: [{"type": "command", "command": ..., "timeout": ...}]``
array — missing that wrapper causes the entire settings.json to
fail to parse ("Expected array, but received undefined").
"""
config = generate_hooks_config()
for event_name, entries in config["hooks"].items():
for entry in entries:
assert "command" not in entry, (
f"{event_name} entry has a flat `command` field; "
"it must be wrapped in an inner `hooks` array"
)
assert "hooks" in entry, (
f"{event_name} entry is missing the inner `hooks` array"
)
assert isinstance(entry["hooks"], list)
for hook in entry["hooks"]:
assert hook.get("type") == "command", (
f"{event_name} inner hook missing type=\"command\""
)
assert "command" in hook
assert "timeout" in hook


class TestInstallGitHook:
def _make_git_repo(self, tmp_path: Path) -> Path:
Expand Down