diff --git a/code_review_graph/skills.py b/code_review_graph/skills.py index 2ecc6d3..b94078a 100644 --- a/code_review_graph/skills.py +++ b/code_review_graph/skills.py @@ -382,11 +382,30 @@ def install_hooks(repo_root: Path) -> None: if settings_path.exists(): try: existing = json.loads(settings_path.read_text()) + 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") logger.info("Wrote hooks config: %s", settings_path) diff --git a/tests/test_skills.py b/tests/test_skills.py index bad8b8b..d8d4e8e 100644 --- a/tests/test_skills.py +++ b/tests/test_skills.py @@ -130,10 +130,24 @@ def test_merges_with_existing(self, tmp_path): data = json.loads((settings_dir / "settings.json").read_text()) assert data["customSetting"] is True + assert "OtherHook" in data["hooks"] assert "PostToolUse" in data["hooks"] assert "SessionStart" in data["hooks"] assert "PreCommit" in data["hooks"] + def test_creates_settings_backup(self, tmp_path): + settings_dir = tmp_path / ".claude" + settings_dir.mkdir(parents=True) + existing = {"hooks": {"OtherHook": []}} + (settings_dir / "settings.json").write_text(json.dumps(existing)) + + install_hooks(tmp_path) + + backup_path = settings_dir / "settings.json.bak" + assert backup_path.exists() + backup = json.loads(backup_path.read_text()) + assert backup == existing + def test_creates_claude_directory(self, tmp_path): install_hooks(tmp_path) assert (tmp_path / ".claude").is_dir()