From e6193ab8166c8b6582de2e2fddfd1e6d4b2608bf Mon Sep 17 00:00:00 2001 From: dherreraj Date: Thu, 22 Oct 2026 19:38:39 -0500 Subject: [PATCH] fix(hooks): run the rebuild from the checkout's top level (#3258) The rebuild bodies read graphify-out/ and its .graphify_root relative to the cwd, but git does not always start hooks at the top level. With GIT_DIR exported and no GIT_WORK_TREE (a nested git call from another hook or tool), git takes the caller's cwd as the work tree and `git rev-parse --show-toplevel` echoes that subdirectory. A stray graphify-out/ left there by an earlier run then roots the rebuild at the subdirectory; for .claude/worktrees/ that walks every linked worktree. The #3265 guard rejects a `..` marker, but its fallback is the same cwd. Add a shared _TOPLEVEL_ANCHOR snippet right after _WORKTREE_GUARD in all four hooks. In the standard layout it cds to the parent of the absolute common .git dir; with GIT_WORK_TREE or a separate git dir it keeps git's own --show-toplevel. GIT_DIR is rewritten to its absolute form so later git calls still resolve. The block already runs in a subshell, so the cd does not leak into chained hooks. --- graphify/hooks.py | 31 ++++++++++++++++++--- tests/test_hooks.py | 68 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 4 deletions(-) diff --git a/graphify/hooks.py b/graphify/hooks.py index 88ed7650b..4071a5927 100644 --- a/graphify/hooks.py +++ b/graphify/hooks.py @@ -413,6 +413,29 @@ fi """ +# Run the rest of each hook from the checkout's top level, shared by all hooks +# and placed right after _WORKTREE_GUARD (it reuses _GFY_GITDIR/_GFY_COMMONDIR). +# The rebuild reads graphify-out/ and its .graphify_root relative to the cwd, +# but git does not always start hooks at the top: with GIT_DIR exported and no +# GIT_WORK_TREE (a nested git call from another hook or tool), git takes the +# caller's cwd as the work tree, and `--show-toplevel` echoes that same +# subdirectory. A stray graphify-out/ left there by an earlier run then roots the +# rebuild at the subdirectory - for .claude/worktrees/ that is every linked +# worktree (#3258). In the standard layout the work tree is the parent of the +# common .git dir; GIT_WORK_TREE and --separate-git-dir layouts keep git's own +# answer. GIT_DIR is rewritten to its absolute form (not newly exported) so the +# git calls below still resolve after the cd. +_TOPLEVEL_ANCHOR = """\ +if [ -z "${GIT_WORK_TREE:-}" ] && [ "${_GFY_COMMONDIR##*/}" = ".git" ]; then + _GFY_TOP=${_GFY_COMMONDIR%/*} +else + _GFY_TOP=$(git rev-parse --show-toplevel 2>/dev/null) +fi +[ -n "$_GFY_TOP" ] && cd "$_GFY_TOP" || exit 0 +[ -n "$_GFY_GITDIR" ] && GIT_DIR=$_GFY_GITDIR +""" + + # 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 @@ -449,7 +472,7 @@ GIT_DIR=${GIT_DIR:-$(git rev-parse --git-dir 2>/dev/null)} [ -f "$GIT_DIR/MERGE_HEAD" ] && exit 0 [ "${GRAPHIFY_SKIP_HOOK:-0}" = "1" ] && exit 0 -""" + _WORKTREE_GUARD + """ +""" + _WORKTREE_GUARD + _TOPLEVEL_ANCHOR + """ # CHERRY_PICK_HEAD can't be the skip signal: git leaves it on disk while post-commit # runs, so every pick was silenced (#4490, same trap as MERGE_HEAD in #4463). For a # range pick or revert, skip every step but the last and rebuild once from @@ -579,7 +602,7 @@ GIT_DIR=${GIT_DIR:-$(git rev-parse --git-dir 2>/dev/null)} # suppressed commit-triggered rebuilds but not branch-switch ones (#1809). [ "${GRAPHIFY_SKIP_HOOK:-0}" = "1" ] && exit 0 -""" + _WORKTREE_GUARD + _PYTHON_DETECT + """ +""" + _WORKTREE_GUARD + _TOPLEVEL_ANCHOR + _PYTHON_DETECT + """ _GRAPHIFY_LOG="${HOME}/.cache/graphify-rebuild.log" mkdir -p "$(dirname "$_GRAPHIFY_LOG")" export GRAPHIFY_REBUILD_LOG="$_GRAPHIFY_LOG" @@ -632,7 +655,7 @@ GIT_DIR=${GIT_DIR:-$(git rev-parse --git-dir 2>/dev/null)} # Honor the same opt out the commit and checkout hooks already respect. [ "${GRAPHIFY_SKIP_HOOK:-0}" = "1" ] && exit 0 -""" + _WORKTREE_GUARD + _PYTHON_DETECT + """ +""" + _WORKTREE_GUARD + _TOPLEVEL_ANCHOR + _PYTHON_DETECT + """ _GRAPHIFY_LOG="${HOME}/.cache/graphify-rebuild.log" mkdir -p "$(dirname "$_GRAPHIFY_LOG")" export GRAPHIFY_REBUILD_LOG="$_GRAPHIFY_LOG" @@ -684,7 +707,7 @@ fi # Honor the same opt out the commit and checkout hooks already respect. [ "${GRAPHIFY_SKIP_HOOK:-0}" = "1" ] && exit 0 -""" + _WORKTREE_GUARD + _PYTHON_DETECT + """ +""" + _WORKTREE_GUARD + _TOPLEVEL_ANCHOR + _PYTHON_DETECT + """ _GRAPHIFY_LOG="${HOME}/.cache/graphify-rebuild.log" mkdir -p "$(dirname "$_GRAPHIFY_LOG")" export GRAPHIFY_REBUILD_LOG="$_GRAPHIFY_LOG" diff --git a/tests/test_hooks.py b/tests/test_hooks.py index db95089de..a986deedf 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -1213,6 +1213,74 @@ def test_worktree_guard_runs_on_primary_skips_linked(tmp_path): assert "RAN" not in r_linked.stdout, "guard failed to skip the linked worktree" +@pytest.mark.parametrize("name,script", _HOOK_SCRIPTS) +def test_hooks_anchor_to_the_top_level_after_the_worktree_guard(name, script): + """Every hook cds to the checkout's top level before anything reads + graphify-out/ or launches the rebuild (#3258), and does so after the + worktree guard, whose _GFY_GITDIR/_GFY_COMMONDIR it reuses.""" + from graphify.hooks import _TOPLEVEL_ANCHOR, _WORKTREE_GUARD + assert script.count(_TOPLEVEL_ANCHOR) == 1, f"{name} anchor not present exactly once" + assert script.index(_WORKTREE_GUARD) < script.index(_TOPLEVEL_ANCHOR) + # The cheap "is there a graph here?" early exits may stay ahead of it: at + # worst they skip a rebuild. Choosing the interpreter and launching the + # rebuild (which reads .graphify_root) must not. + code = "\n".join(line for line in script.splitlines() if not line.lstrip().startswith("#")) + anchor_at = code.index(_TOPLEVEL_ANCHOR.splitlines()[0]) + for later in ("GRAPHIFY_PYTHON", "_rebuild_code"): + assert anchor_at < code.index(later), f"{name} uses {later} before anchoring" + + +def test_toplevel_anchor_runs_from_the_top_when_git_starts_the_hook_in_a_subdir(tmp_path): + """End-to-end against a real repo (#3258): with GIT_DIR exported and no + GIT_WORK_TREE, git runs hooks in the caller's cwd and `--show-toplevel` + echoes that subdirectory. The anchored snippet must still run at the + repository top, keep later git calls working, and leave a linked worktree + to the guard.""" + if shutil.which("git") is None: # pragma: no cover + pytest.skip("git not available") + primary = tmp_path / "primary" + primary.mkdir() + + def _git(*args, cwd): + subprocess.run(["git", *args], cwd=cwd, check=True, + capture_output=True, text=True) + + _git("init", "-q", ".", cwd=primary) + _git("config", "user.email", "t@t.co", cwd=primary) + _git("config", "user.name", "t", cwd=primary) + (primary / "a.txt").write_text("x") + _git("add", "-A", cwd=primary) + _git("commit", "-qm", "init", cwd=primary) + sub = primary / ".claude" / "worktrees" + sub.mkdir(parents=True) + linked = sub / "wt1" + _git("worktree", "add", "-q", str(linked), "-b", "feature", cwd=primary) + + from graphify.hooks import _TOPLEVEL_ANCHOR, _WORKTREE_GUARD + snippet = (_WORKTREE_GUARD + _TOPLEVEL_ANCHOR + + 'git rev-parse --verify -q HEAD >/dev/null && echo GIT_OK\n' + + ': > anchored.txt\n') + env = {k: v for k, v in os.environ.items() if not k.startswith("GIT_")} + + cases = [ + ("top", primary, {}), + ("subdir", sub, {}), + ("subdir+GIT_DIR", sub, {"GIT_DIR": "../../.git"}), + ] + for label, cwd, extra in cases: + r = subprocess.run(["sh", "-c", snippet], cwd=cwd, env={**env, **extra}, + capture_output=True, text=True) + assert (primary / "anchored.txt").exists(), f"{label}: did not run at the top ({r.stderr})" + assert not (sub / "anchored.txt").exists(), f"{label}: ran in the subdirectory" + assert "GIT_OK" in r.stdout, f"{label}: git broke after the cd ({r.stderr})" + (primary / "anchored.txt").unlink() + + r = subprocess.run(["sh", "-c", snippet], cwd=linked, env=env, + capture_output=True, text=True) + assert not (linked / "anchored.txt").exists() and not (primary / "anchored.txt").exists(), \ + "the linked worktree must still be skipped by the guard" + + # ── #1907: duplicate keys in .git/config must not trigger spurious warnings ── def _append_duplicate_config_entries(repo: Path) -> None: