diff --git a/graphify/hooks.py b/graphify/hooks.py index e4289ad78..5e5e971a0 100644 --- a/graphify/hooks.py +++ b/graphify/hooks.py @@ -14,7 +14,7 @@ # __PINNED_PYTHON__ is replaced at install time with the absolute path of the # Python interpreter that ran `graphify hook install`. For uv-tool and pipx # installs the interpreter lives inside an isolated venv, so the launcher on -# PATH is the only entry point — and GUI git clients / CI runners often have a +# PATH is the only entry point -- and GUI git clients / CI runners often have a # minimal PATH that omits ~/.local/bin. Pinning sys.executable at install time # makes the hook work regardless of PATH at git-trigger time. _PYTHON_DETECT = """\ @@ -65,7 +65,7 @@ fi fi if [ -z "$GRAPHIFY_PYTHON" ] && [ -n "$GRAPHIFY_BIN" ]; then - # POSIX launcher: parse the shebang. head -c + tr strip NUL bytes first — + # POSIX launcher: parse the shebang. head -c + tr strip NUL bytes first -- # when the launcher is a Windows binary reached without its .exe suffix, # a raw `head -1` reads binary into the command substitution and the # shell warns about ignored null bytes on every commit. @@ -100,6 +100,22 @@ fi """ +# Skip the rebuild in a linked worktree (git worktree add), shared by both hooks. +# In a linked worktree the canonical graphify-out/ belongs to the primary +# checkout; rebuilding from every worktree is wasteful and, for deploy/CI flows +# that `git clean` a worktree, races the background rebuild against the cleanup +# ("failed to remove graphify-out/: Directory not empty"). A linked worktree has +# git-dir != git-common-dir. Compare ABSOLUTE paths so the exported GIT_DIR env +# var (which git sets for hooks, sometimes absolute) cannot false-positive and +# skip the primary checkout. +_WORKTREE_GUARD = """\ +_GFY_GITDIR=$(git rev-parse --absolute-git-dir 2>/dev/null) +_GFY_COMMONDIR=$(cd "$(git rev-parse --git-common-dir 2>/dev/null)" 2>/dev/null && pwd) +if [ -n "$_GFY_COMMONDIR" ] && [ "$_GFY_GITDIR" != "$_GFY_COMMONDIR" ]; then + exit 0 +fi +""" + # The Python that the rebuild runs, shared by both hooks. Embedded verbatim into # the launcher below and re-executed in the detached child. Must not contain the # double-quote, $, backtick or backslash characters: it is carried inside a @@ -195,7 +211,7 @@ # Cross-platform detached-launch shim (#1161). The hooks used to background the # rebuild with `nohup "$GRAPHIFY_PYTHON" -c "..." &`, but Git for Windows' bundled # MSYS shell ships no nohup (nor setsid), so that line died with -# 'nohup: command not found' and the rebuild silently never ran — git commit/pull +# 'nohup: command not found' and the rebuild silently never ran -- git commit/pull # still returned 0, so the graph just went stale with no signal. graphify already # requires Python, so we let Python do the detaching: a tiny outer process spawns # the real rebuild fully detached and returns immediately, so the hook never @@ -266,6 +282,7 @@ def _detached_launch(rebuild_body: str) -> str: [ -f "$GIT_DIR/MERGE_HEAD" ] && exit 0 [ -f "$GIT_DIR/CHERRY_PICK_HEAD" ] && exit 0 +""" + _WORKTREE_GUARD + """ [ "${GRAPHIFY_SKIP_HOOK:-0}" = "1" ] && exit 0 CHANGED=$(git diff --name-only HEAD~1 HEAD 2>/dev/null || git diff --name-only HEAD 2>/dev/null) @@ -334,7 +351,7 @@ def _detached_launch(rebuild_body: str) -> str: [ -f "$GIT_DIR/MERGE_HEAD" ] && exit 0 [ -f "$GIT_DIR/CHERRY_PICK_HEAD" ] && exit 0 -""" + _PYTHON_DETECT + """ +""" + _WORKTREE_GUARD + _PYTHON_DETECT + """ _GRAPHIFY_LOG="${HOME}/.cache/graphify-rebuild.log" mkdir -p "$(dirname "$_GRAPHIFY_LOG")" export GRAPHIFY_REBUILD_LOG="$_GRAPHIFY_LOG" @@ -406,11 +423,11 @@ def _hooks_dir(root: Path) -> Path: ) # In a linked worktree .git is a file not a directory, so constructing # root/.git/hooks directly fails. Ask git for the real hooks path instead. - # NOTE: do NOT pass --path-format=absolute — added in git 2.31; older git + # NOTE: do NOT pass --path-format=absolute -- added in git 2.31; older git # echoes it back as a literal argument, contaminating stdout and causing a # phantom directory to be created (#907). git -C already returns an # absolute path for worktree/external-gitdir cases, and a path relative to - # for normal repos — anchoring on root covers both. + # for normal repos -- anchoring on root covers both. import subprocess as _sp try: res = _sp.run( @@ -505,7 +522,7 @@ def install(path: Path = Path(".")) -> str: if _re.search(r"[^a-zA-Z0-9/_.@:\\-]", _safe): # Path contains characters outside the allowlist (spaces, quotes, etc.). # Embed an empty string so the pinned probe is skipped and the hook - # falls through to the dynamic detection — safe degradation. + # falls through to the dynamic detection -- safe degradation. _safe = "" pinned = _safe hook = _HOOK_SCRIPT.replace("__PINNED_PYTHON__", pinned) diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 6a37bd932..307d7390b 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -1,5 +1,6 @@ """Tests for hooks.py - git hook install/uninstall.""" import os +import shutil import subprocess from types import SimpleNamespace from pathlib import Path @@ -200,7 +201,7 @@ def test_install_fallback_is_loud_not_silent(tmp_path): def test_hook_check_no_additionalContext(tmp_path): - """graphify hook-check must not emit additionalContext — Codex Desktop rejects it.""" + """graphify hook-check must not emit additionalContext -- Codex Desktop rejects it.""" import sys out = tmp_path / "graphify-out" out.mkdir() @@ -263,6 +264,48 @@ def test_hooks_limit_windows_workers_by_default(name, script): assert 'export GRAPHIFY_MAX_WORKERS="${GRAPHIFY_MAX_WORKERS:-1}"' in script +@pytest.mark.parametrize("name,script", _HOOK_SCRIPTS) +def test_hooks_skip_linked_worktrees(name, script): + """Rebuilding from a linked worktree (git worktree add) is wasteful -- the + canonical graphify-out/ belongs to the primary checkout -- and deploy/CI + flows that `git clean` a worktree race the background rebuild against the + cleanup. Both hooks must short-circuit when git-dir != git-common-dir, using + an ABSOLUTE-path compare so an exported GIT_DIR env var cannot false-positive + and skip the primary checkout.""" + assert "git rev-parse --absolute-git-dir" in script, f"{name} missing worktree guard" + assert "git rev-parse --git-common-dir" in script, f"{name} missing worktree guard" + # Exactly one guard, not accidentally duplicated across shared includes. + assert script.count("_GFY_GITDIR=$(git rev-parse --absolute-git-dir") == 1 + + +@pytest.mark.skipif(shutil.which("git") is None, reason="git not available") +def test_worktree_guard_skips_only_linked_worktree(tmp_path): + """End-to-end: the guard snippet exits 0 (skip) inside a linked worktree but + runs through on the primary checkout, exercised against a real git worktree.""" + from graphify.hooks import _WORKTREE_GUARD + + primary = tmp_path / "primary" + primary.mkdir() + env = {**os.environ, "GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@t", + "GIT_COMMITTER_NAME": "t", "GIT_COMMITTER_EMAIL": "t@t"} + subprocess.run(["git", "init", "-q", str(primary)], check=True) + (primary / "f.txt").write_text("x") + subprocess.run(["git", "-C", str(primary), "add", "."], check=True) + subprocess.run(["git", "-C", str(primary), "commit", "-qm", "init"], check=True, env=env) + linked = tmp_path / "linked" + subprocess.run(["git", "-C", str(primary), "worktree", "add", "-q", str(linked)], + check=True, env=env) + + probe = _WORKTREE_GUARD + "echo RAN\n" + + def _run(cwd): + return subprocess.run(["sh", "-c", probe], cwd=str(cwd), + capture_output=True, text=True) + + assert _run(primary).stdout.strip() == "RAN", "primary checkout must rebuild" + assert _run(linked).stdout.strip() == "", "linked worktree must be skipped" + + def _launcher_payload(script: str) -> str: """Extract the `python -c ""` the hook hands to GRAPHIFY_PYTHON. @@ -392,8 +435,8 @@ def test_default_hooks_dir_unaffected(tmp_path): # ── foreground hook cost: probes must be cheap and quiet ───────────────────── def test_probes_use_find_spec_not_full_import(): - """`python -c "import graphify"` executes the FULL package import — 10s+ on a - cold cache or AV-scanned site-packages — and could run up to four times + """`python -c "import graphify"` executes the FULL package import -- 10s+ on a + cold cache or AV-scanned site-packages -- and could run up to four times synchronously before the detached launch even started, so every commit stalled for tens of seconds. Probes must locate the package with importlib.util.find_spec (no execution); the detached rebuild still reports @@ -418,7 +461,7 @@ def test_shebang_read_is_null_byte_safe(): def test_probe_prefers_sibling_python_exe_on_windows_layouts(): """pip on Windows puts Scripts/graphify(.exe) beside ..\\python.exe (or .\\python.exe in a venv). Resolving that directly beats shebang-parsing a - binary launcher — and works whether or not command -v kept the suffix.""" + binary launcher -- and works whether or not command -v kept the suffix.""" from graphify.hooks import _PYTHON_DETECT assert "/../python.exe" in _PYTHON_DETECT assert "/python.exe" in _PYTHON_DETECT @@ -427,6 +470,6 @@ def test_probe_prefers_sibling_python_exe_on_windows_layouts(): @pytest.mark.parametrize("name,script", _HOOK_SCRIPTS) def test_hooks_reuse_git_dir_from_env(name, script): """git exports GIT_DIR to hooks, so the rev-parse fallback should only run - when the script is invoked by hand — each extra git exec costs 1s+ on + when the script is invoked by hand -- each extra git exec costs 1s+ on AV-scanned Windows machines and lands in the commit's foreground.""" assert "GIT_DIR=${GIT_DIR:-" in script, f"{name} always re-runs git rev-parse"