From cf756f4033e577b755b2d636f7be93ea217351a1 Mon Sep 17 00:00:00 2001
From: Nguyen Truong Luu <lngyeen@openwt.com>
Date: Fri, 10 Apr 2026 15:29:51 +0700
Subject: [PATCH] fix: valid Claude Code hook schema + merge existing hooks on
 install

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 #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 #145)

Closes #97, #114, #172.
---
 CHANGELOG.md                |  3 +++
 code_review_graph/skills.py | 26 +++++++++++++++++++++++---
 tests/test_skills.py        | 27 +++++++++++++++++++++++++++
 3 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 55a97e70..0f5a70fe 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
diff --git a/code_review_graph/skills.py b/code_review_graph/skills.py
index fe3abab6..514b5d38 100644
--- a/code_review_graph/skills.py
+++ b/code_review_graph/skills.py
@@ -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.
@@ -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)
diff --git a/tests/test_skills.py b/tests/test_skills.py
index 15c5eeb8..903002f7 100644
--- a/tests/test_skills.py
+++ b/tests/test_skills.py
@@ -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: