fix(skills): remove all existing code-review-graph hook entries on reinstall (#558)

code_review_graph/flows.py

@@ -10,8 +10,10 @@
 
 import json
 import logging
+import os
 import re
 from collections import deque
+from pathlib import Path
 from typing import Optional
 
 from .constants import SECURITY_KEYWORDS as _SECURITY_KEYWORDS
@@ -478,19 +480,35 @@ def incremental_trace_flows(
 
     # Graph identity uses POSIX separators (#774); bridge native-separator
     # caller paths before matching against stored file_path values.
-    changed_files = [normalize_file_path(p) for p in changed_files]
+    # Bridge relative vs absolute path representation mismatches (#569).
+    expanded_set: set[str] = set()
+    for p in changed_files:
+        norm_p = normalize_file_path(p)
+        expanded_set.add(norm_p)
+        try:
+            abs_p = normalize_file_path(os.path.abspath(p))
+            expanded_set.add(abs_p)
+        except Exception:
+            pass
+        try:
+            res_p = normalize_file_path(Path(p).resolve())
+            expanded_set.add(res_p)
+        except Exception:
+            pass
+
+    query_changed_files = list(expanded_set)
+    changed_file_set = expanded_set
     conn = store._conn
-    changed_file_set = set(changed_files)
 
     # ------------------------------------------------------------------
     # 1. Find affected flow IDs
     # ------------------------------------------------------------------
-    placeholders = ",".join("?" * len(changed_files))
+    placeholders = ",".join("?" * len(query_changed_files))
     affected_rows = conn.execute(
         f"SELECT DISTINCT fm.flow_id FROM flow_memberships fm "  # nosec B608
         f"JOIN nodes n ON n.id = fm.node_id "
         f"WHERE n.file_path IN ({placeholders})",
-        changed_files,
+        query_changed_files,
     ).fetchall()
     affected_ids = [r[0] for r in affected_rows]
 
@@ -530,10 +548,26 @@ def incremental_trace_flows(
     # ------------------------------------------------------------------
     # 4. Re-detect entry points and filter to relevant ones
     # ------------------------------------------------------------------
+    def _matches_changed_files(ep_path: str) -> bool:
+        norm_ep = normalize_file_path(ep_path)
+        if norm_ep in changed_file_set:
+            return True
+        try:
+            if normalize_file_path(os.path.abspath(ep_path)) in changed_file_set:
+                return True
+        except Exception:
+            pass
+        try:
+            if normalize_file_path(Path(ep_path).resolve()) in changed_file_set:
+                return True
+        except Exception:
+            pass
+        return False
+
     entry_points = detect_entry_points(store)
     relevant_eps = [
         ep for ep in entry_points
-        if ep.file_path in changed_file_set or ep.id in entry_point_ids
+        if _matches_changed_files(ep.file_path) or ep.id in entry_point_ids
     ]
 
     # ------------------------------------------------------------------

code_review_graph/skills.py

@@ -1202,6 +1202,65 @@ def install_git_hook(repo_root: Path) -> Path | None:
     return hook_path
 
 
+# Command shapes that code-review-graph itself installs. Matching these
+# instead of the bare "code-review-graph" substring avoids purging a
+# user-defined hook that merely mentions the tool (e.g. an echo/comment or a
+# custom wrapper) while still catching every generated and legacy CRG command,
+# including stale absolute-path forms like "/opt/venv/bin/code-review-graph
+# update --repo /old/path".
+_CRG_COMMAND_MARKERS = (
+    "code-review-graph update",
+    "code-review-graph status",
+    "code-review-graph detect-changes",
+)
+
+
+def _is_crg_command(command: Any) -> bool:
+    """Return True if ``command`` is a code-review-graph-installed command."""
+    if not isinstance(command, str):
+        return False
+    return any(marker in command for marker in _CRG_COMMAND_MARKERS)
+
+
+def _strip_owned_hooks(entry: Any) -> Any | None:
+    """Remove code-review-graph-owned commands from a single hook entry.
+
+    Returns the entry (a copy when modified) with only CRG-owned inner hooks
+    removed, so sibling user commands in a mixed entry survive. Returns
+    ``None`` when nothing user-owned remains and the whole entry should be
+    dropped.
+    """
+    if not isinstance(entry, dict):
+        # Preserve anything we do not recognise as a CRG entry.
+        return entry
+
+    inner_hooks = entry.get("hooks")
+    if isinstance(inner_hooks, list):
+        kept = [
+            inner
+            for inner in inner_hooks
+            if not (isinstance(inner, dict) and _is_crg_command(inner.get("command")))
+        ]
+        if not kept:
+            # Every inner hook was CRG-owned. Keep the entry only if it also
+            # carries a non-CRG top-level command; otherwise drop it entirely.
+            if entry.get("command") is not None and not _is_crg_command(entry.get("command")):
+                new_entry = dict(entry)
+                new_entry["hooks"] = kept
+                return new_entry
+            return None
+        if len(kept) != len(inner_hooks):
+            new_entry = dict(entry)
+            new_entry["hooks"] = kept
+            return new_entry
+        return entry
+
+    # Flat entry with no nested hooks array.
+    if _is_crg_command(entry.get("command")):
+        return None
+    return entry
+
+
 def _merge_hooks_into_settings(
     settings_dir: Path,
     hooks_config: dict[str, Any],
@@ -1226,9 +1285,31 @@ def _merge_hooks_into_settings(
         existing_hooks = {}
 
     merged_hooks = dict(existing_hooks)
+
+    # Purge code-review-graph-owned hooks from every existing event, not only
+    # the events we are about to re-emit. This clears accumulated duplicates,
+    # stale absolute-path entries, and orphaned hooks parked under event names
+    # the generator no longer emits (e.g. the legacy "PreCommit"). Sibling
+    # user commands in a mixed entry are preserved (#558).
+    for hook_name in list(merged_hooks.keys()):
+        entries = merged_hooks[hook_name]
+        if not isinstance(entries, list):
+            continue
+        cleaned = [
+            stripped for e in entries if (stripped := _strip_owned_hooks(e)) is not None
+        ]
+        if not cleaned and entries:
+            # Every entry was CRG-owned — drop the now-orphaned event. An
+            # already-empty user list is left untouched.
+            del merged_hooks[hook_name]
+        else:
+            merged_hooks[hook_name] = cleaned
+
+    # Append the freshly generated hooks, avoiding exact duplicates.
     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])
+        existing_list = merged_hooks.get(hook_name)
+        if isinstance(existing_list, list):
+            merged_list = list(existing_list)
             for entry in hook_entries:
                 if entry not in merged_list:
                     merged_list.append(entry)

tests/test_flows.py

@@ -591,6 +591,21 @@ def test_incremental_trace_flows_new_entry_point(self):
         new_flows = [f for f in after if f["name"] == "new_entry"]
         assert len(new_flows) == 1
 
+    def test_incremental_trace_flows_relative_vs_absolute_path_mismatch(self):
+        """Regression test for #569: relative vs absolute path mismatch in entry points."""
+        abs_b_path = str(Path("b.py").resolve())
+        self._add_func("abs_entry", path=abs_b_path)
+        self._add_func("abs_callee", path=abs_b_path)
+        self._add_call(f"{abs_b_path}::abs_entry", f"{abs_b_path}::abs_callee", abs_b_path)
+
+        # Pass relative path "b.py" to incremental_trace_flows while node file_path is absolute.
+        count = incremental_trace_flows(self.store, ["b.py"])
+        assert count >= 1
+
+        after = get_flows(self.store)
+        abs_flows = [f for f in after if f["name"] == "abs_entry"]
+        assert len(abs_flows) == 1
+
     def test_incremental_trace_flows_no_affected_flows(self):
         """When changed files have no existing flows, only new entry points are checked."""
         self._add_func("handler", path="routes.py")