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

[Blazehue]

I'll update #558 for the same:

Summary

• Updated _merge_hooks_into_settings in code_review_graph/skills.py to filter out all existing code-review-graph hook entries before appending the new dynamic hook config. This purges stale absolute-path entries and accumulated duplicate hooks on re-installation.
• Added test_install_hooks_replaces_old_and_duplicate_hooks in tests/test_skills.py seeding an old-style absolute-path hook and duplicate entries, asserting that custom user hooks are preserved while all old CRG entries are replaced by a single dynamic git rev-parse hook.

Verification

• Tested with pytest tests/test_skills.py (192 passed).

[tirth8205]

Thanks. Two parts.

The flows change does not fix #569 in the scenario the issue describes. os.path.abspath and Path.resolve at flows.py:489-497 resolve against the process cwd, not the repo root, and incremental_trace_flows never receives the root (tools/build.py:164 has it and does not pass it). Run the MCP server or daemon with cwd outside the repo and the new entry point is still missed. Your test only passes because pytest runs from the repo root. #673 threads repo_root through properly, so I will take that for flows. Please drop the flows.py and test_flows.py hunks.

The hooks dedup is worth keeping, with two changes. Remove all existing code-review-graph entries instead of only the first (skills.py:1041-1046 breaks after one match), otherwise users who already accumulated duplicates keep the stale ones. And the regression test passes on main unchanged, since identical reinstalls were already deduped. Seed an old-style absolute-path hook and assert it is replaced.

#576 is already fixed on main and closed, so drop it from the description.

[tirth8205]

Changes required: the hook migration deletes unrelated user commands within a mixed entry. In .claude/settings.json, place custom-audit --important and code-review-graph update --repo /old/path as sibling commands in one PostToolUse entry, then call code_review_graph.skills.install_hooks(repo_root, platform='claude'); this PR removes the whole entry, including custom-audit. Remove only demonstrably owned inner hooks, and resolve flow paths against the repository root rather than the process working directory.

[tirth8205]

This fails the checks once merged into staging.

pytest (py3.13): "3225 passed, 9 skipped, 2 xpassed, 1 warning in 176.81s". ruff: "E501 Line too long (104 > 100) --> code_review_graph/skills.py:1263:101 ... Found 1 error." (the PR's own new comment line; origin/staging skills.py passes ruff clean, so this is the PR's fault). mypy: "Success: no issues found in 72 source files".

To fix:

• Fix ruff E501 at code_review_graph/skills.py:1263 (comment line is 104 chars).
• Test that a user-defined hook whose command contains the substring 'code-review-graph' (but is not a CRG hook) is preserved, or narrow _is_crg_hook to match the actual generated command shapes (e.g. 'code-review-graph update' / 'code-review-graph status').
• Test for stale CRG hooks under event names the generator no longer emits (legacy 'PreCommit'), which [Bug]: claude-code hooks in settings.json shall not contain absolute paths to the repo #558 asks to be replaced but which this PR leaves in place.
• Test seeding a stale SessionStart CRG entry and asserting it is replaced.

Merge origin/staging into your branch first to reproduce it.

PRs now target staging, not main. Yours was retargeted already, so nothing to do there.

[Blazehue]

Thanks for the detailed checklist — all four items are addressed in 9b7e32c.

1. ruff E501 (skills.py) fixed. The over-length merge comment is gone; ruff check code_review_graph/ is clean.

2. _is_crg_hook too broad narrowed. Detection now keys off the command shapes the generator actually installs (code-review-graph update / status / detect-changes) via a new _is_crg_command() helper, instead of the bare code-review-graph substring. Added a test seeding a user hook whose command only mentions code-review-graph (an echo) and asserting it survives reinstall.

3. Stale legacy PreCommit entries — the merge now sweeps every existing hook event, not just the ones being re-emitted, so orphaned CRG entries under events the generator no longer emits are removed. Added a test seeding a stale PreCommit CRG entry and asserting it's purged.

4. Stale SessionStart entry added a test seeding a stale absolute-path SessionStart CRG entry and asserting it's replaced by the current dynamic (git rev-parse --show-toplevel) hook.

I also carried over your earlier mixed-entry concern: _strip_owned_hooks() removes only the CRG-owned inner hooks from an entry, so a sibling user command (e.g. custom-audit --important) in the same PostToolUse entry is preserved rather than the whole entry being dropped. Already-empty user event lists are left untouched. There's a dedicated test for that case too.

Merged origin/staging locally to reproduce as you suggested. Verified against staging:

ruff check code_review_graph/ — clean
mypy code_review_graph/ — no issues in 72 source files
pytest tests/ — 3232 passed, 9 skipped, 2 xpassed

[tirth8205]

This no longer merges into staging. The one conflict is code_review_graph/skills.py, and the branch is 228 commits behind.

git fetch origin
git merge origin/staging
# resolve, then
uv run pytest tests/ -q
uv run ruff check code_review_graph/
uv run mypy code_review_graph/ --ignore-missing-imports --no-strict-optional
git push

I have not reviewed the change itself yet. That comes once it merges and the checks run against the merged state, since staging has moved a long way and the result is what matters.