fix(hooks): a graphify skip must not terminate the whole git hook (#2986)

graphify/hooks.py

@@ -304,10 +304,20 @@ def _detached_launch(rebuild_body: str) -> str:
 """
 
 
+# Both hook bodies run inside a subshell `( ... )`. The generated block is
+# appended to whatever post-commit / post-checkout already exists, and other
+# tools chain their own logic after it; every skip condition in the block is a
+# bare `exit 0`, which in a flat script ends the WHOLE hook, silently dropping
+# anything after graphify's end marker - on every root commit (HEAD~1 does
+# not exist), every rebase/merge, every linked worktree, every
+# GRAPHIFY_SKIP_HOOK=1 (#2986). Inside the subshell an `exit` ends only
+# graphify's section; the detached rebuild launch is unaffected, and the
+# hook's own exit status stays 0 as before.
 _HOOK_SCRIPT = """\
 # graphify-hook-start
 # Auto-rebuilds the knowledge graph after each commit (code files only, no LLM needed).
 # Installed by: graphify hook install
+(
 
 # Deterministic clustering: networkx louvain iterates string-keyed sets whose
 # order is randomized per-process by PYTHONHASHSEED, so community assignments
@@ -355,14 +365,16 @@ def _detached_launch(rebuild_body: str) -> str:
 mkdir -p "$(dirname "$_GRAPHIFY_LOG")"
 export GRAPHIFY_REBUILD_LOG="$_GRAPHIFY_LOG"
 echo "[graphify hook] launching background rebuild (log: $_GRAPHIFY_LOG)"
-""" + _detached_launch(_REBUILD_BODY_COMMIT) + """# graphify-hook-end
+""" + _detached_launch(_REBUILD_BODY_COMMIT) + """)
+# graphify-hook-end
 """
 
 
 _CHECKOUT_SCRIPT = """\
 # graphify-checkout-hook-start
 # Auto-rebuilds the knowledge graph (code only) when switching branches.
 # Installed by: graphify hook install
+(
 
 # Deterministic clustering: networkx louvain iterates string-keyed sets whose
 # order is randomized per-process by PYTHONHASHSEED, so community assignments
@@ -412,7 +424,8 @@ def _detached_launch(rebuild_body: str) -> str:
 mkdir -p "$(dirname "$_GRAPHIFY_LOG")"
 export GRAPHIFY_REBUILD_LOG="$_GRAPHIFY_LOG"
 echo "[graphify] Branch switched - launching background rebuild (log: $_GRAPHIFY_LOG)"
-""" + _detached_launch(_REBUILD_BODY_CHECKOUT) + """# graphify-checkout-hook-end
+""" + _detached_launch(_REBUILD_BODY_CHECKOUT) + """)
+# graphify-checkout-hook-end
 """
 
 

tests/test_hook_chain_survives_skip.py

@@ -0,0 +1,110 @@
+"""A graphify skip must not end the whole hook (#2986).
+
+The generated block is appended to whatever post-commit / post-checkout a
+repo already has, and other tools chain their logic after it. Every skip
+condition in the block is a bare `exit 0`, which in a flat script ends the
+entire hook process — so anything after graphify's end marker was silently
+dropped on every root commit (HEAD~1 does not exist), every rebase/merge,
+every linked worktree and every GRAPHIFY_SKIP_HOOK=1.
+"""
+from __future__ import annotations
+
+import os
+import shutil
+import subprocess
+from pathlib import Path
+
+import pytest
+
+from graphify.hooks import _CHECKOUT_MARKER_END, _HOOK_MARKER_END, install
+
+pytestmark = pytest.mark.skipif(shutil.which("sh") is None, reason="sh required to run the hook")
+
+
+def _repo(tmp_path: Path) -> Path:
+    repo = tmp_path / "repo"
+    repo.mkdir()
+    subprocess.run(["git", "init", "-q", str(repo)], check=True)
+    return repo
+
+
+def _install_with_trailer(repo: Path, name: str, marker_end: str) -> Path:
+    install(repo)
+    hook = repo / ".git" / "hooks" / name
+    text = hook.read_text(encoding="utf-8")
+    assert marker_end in text
+    hook.write_text(text.rstrip() + "\necho AFTER_GRAPHIFY\n", encoding="utf-8", newline="\n")
+    return hook
+
+
+def _run(hook: Path, repo: Path, *args: str, **env_extra: str) -> subprocess.CompletedProcess:
+    env = dict(os.environ)
+    env.update(env_extra)
+    return subprocess.run(["sh", str(hook), *args], capture_output=True, text=True,
+                          cwd=str(repo), env=env)
+
+
+def test_a_skipped_post_commit_still_runs_what_follows(tmp_path):
+    repo = _repo(tmp_path)
+    hook = _install_with_trailer(repo, "post-commit", _HOOK_MARKER_END)
+    r = _run(hook, repo, GRAPHIFY_SKIP_HOOK="1")
+    assert r.returncode == 0, r.stderr
+    assert "AFTER_GRAPHIFY" in r.stdout
+    assert "launching background rebuild" not in r.stdout  # the skip itself still holds
+
+
+def test_a_rebase_in_progress_still_runs_what_follows(tmp_path):
+    repo = _repo(tmp_path)
+    (repo / ".git" / "rebase-merge").mkdir()
+    hook = _install_with_trailer(repo, "post-commit", _HOOK_MARKER_END)
+    r = _run(hook, repo)
+    assert r.returncode == 0, r.stderr
+    assert "AFTER_GRAPHIFY" in r.stdout
+
+
+def test_an_empty_diff_still_runs_what_follows(tmp_path):
+    """A repo with no commits: `git diff HEAD~1 HEAD` and the fallback both
+    yield nothing, the block exits on the empty CHANGED — the root-commit
+    shape every repository hits once."""
+    repo = _repo(tmp_path)
+    hook = _install_with_trailer(repo, "post-commit", _HOOK_MARKER_END)
+    r = _run(hook, repo)
+    assert r.returncode == 0, r.stderr
+    assert "AFTER_GRAPHIFY" in r.stdout
+
+
+def test_a_non_branch_checkout_still_runs_what_follows(tmp_path):
+    repo = _repo(tmp_path)
+    hook = _install_with_trailer(repo, "post-checkout", _CHECKOUT_MARKER_END)
+    r = _run(hook, repo, "abc", "def", "0")  # a file checkout, not a branch switch
+    assert r.returncode == 0, r.stderr
+    assert "AFTER_GRAPHIFY" in r.stdout
+
+
+def test_a_skipped_checkout_hook_still_runs_what_follows(tmp_path):
+    repo = _repo(tmp_path)
+    hook = _install_with_trailer(repo, "post-checkout", _CHECKOUT_MARKER_END)
+    r = _run(hook, repo, "abc", "def", "1", GRAPHIFY_SKIP_HOOK="1")
+    assert r.returncode == 0, r.stderr
+    assert "AFTER_GRAPHIFY" in r.stdout
+
+
+def test_a_preexisting_hook_before_the_block_still_runs_first(tmp_path):
+    repo = _repo(tmp_path)
+    hook = repo / ".git" / "hooks" / "post-commit"
+    hook.write_text("#!/bin/sh\necho BEFORE_GRAPHIFY\n", encoding="utf-8", newline="\n")
+    install(repo)
+    r = _run(hook, repo, GRAPHIFY_SKIP_HOOK="1")
+    assert r.returncode == 0, r.stderr
+    assert "BEFORE_GRAPHIFY" in r.stdout
+
+
+def test_the_block_is_still_recognised_and_updated_in_place(tmp_path):
+    """Markers stay outside the subshell so reinstall/uninstall/status keep
+    finding the block."""
+    repo = _repo(tmp_path)
+    install(repo)
+    hook = repo / ".git" / "hooks" / "post-commit"
+    first = hook.read_text(encoding="utf-8")
+    assert "already installed" in install(repo)
+    assert hook.read_text(encoding="utf-8") == first

tests/test_hooks.py

@@ -769,7 +769,8 @@ def test_checkout_hook_skips_same_head_noop_at_runtime():
     assert guard in _CHECKOUT_SCRIPT, "guard missing from the checkout script"
     # Real script through the same-head guard, then a sentinel — stops before the
     # graphify-out check / detached launch so nothing is actually rebuilt.
-    prefix = _CHECKOUT_SCRIPT.split(guard)[0] + guard + "\necho RAN\n"
+    # The block runs inside a subshell (#2986); close it after the sentinel.
+    prefix = _CHECKOUT_SCRIPT.split(guard)[0] + guard + "\necho RAN\n)\n"
 
     def run(prev, new, flag):
         # sh -c CMD name arg1 arg2 arg3  ->  $0=name $1=prev $2=new $3=flag