From df9f37612f95bb2fbc7e4a610664a08f2b46a477 Mon Sep 17 00:00:00 2001 From: breken Date: Sun, 13 Sep 2026 19:09:11 -0700 Subject: [PATCH] fix(hooks): resolve the output dir from GRAPHIFY_OUT in the sh gates, not a literal graphify-out/ #1423 moved the Python rebuild bodies onto GRAPHIFY_OUT, but the shell that runs BEFORE them still hardcoded graphify-out/. With the documented override (#686, e.g. GRAPHIFY_OUT=custom-out for worktree/shared-output setups): - post-checkout exited at `[ ! -d "graphify-out" ]`, so a branch switch never launched the rebuild - a silent no-op with nothing in the log. - post-commit's "only graph artifacts changed" filter only recognised graphify-out/ paths, so a commit touching only custom-out/graph.json launched a full rebuild - the loop the filter exists to prevent. Both gates now read ${GRAPHIFY_OUT:-graphify-out}, the same source the rebuild body reads. The commit filter is a POSIX case loop instead of grep so the dir name is matched literally (no regex metacharacters); the leading ( on each pattern keeps bash 3.2 (macOS /bin/sh) from mis-parsing the pattern's ) inside the $(...) substitution. Regression: tests/test_hooks.py runs the emitted scripts under sh with the launcher stubbed and asserts whether the hook reaches the launch line. Claude-Session: https://claude.ai/code/session_0134ujLF81GyXsCByibLcYsz (cherry picked from commit 9efd052de4c471ed3bed6650878e5c7cacfe7fbf) --- graphify/hooks.py | 23 ++++++-- tests/test_hooks.py | 127 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+), 4 deletions(-) diff --git a/graphify/hooks.py b/graphify/hooks.py index 3576f81c1..e78e291f6 100644 --- a/graphify/hooks.py +++ b/graphify/hooks.py @@ -415,8 +415,20 @@ if [ -z "$CHANGED" ]; then exit 0 fi -# Skip when only graphify-out/ artifacts changed (avoids rebuild loop when graph outputs are tracked in git) -_NON_GRAPH=$(echo "$CHANGED" | grep -v '^graphify-out/' || true) +# Skip when only output-dir artifacts changed (avoids rebuild loop when graph +# outputs are tracked in git). The dir is whatever GRAPHIFY_OUT names, the same +# source the rebuild body reads (#1423): a literal graphify-out/ here let a +# commit touching only a renamed output dir's graph.json trigger a full rebuild. +_GFY_OUT="${GRAPHIFY_OUT:-graphify-out}" +_GFY_OUT="${_GFY_OUT%/}" +# The leading ( on each pattern is POSIX and keeps bash 3.2 (macOS /bin/sh) +# from mis-parsing the pattern's ) as the end of the $(...) substitution. +_NON_GRAPH=$(printf '%s\n' "$CHANGED" | while IFS= read -r _GFY_F; do + case "$_GFY_F" in + ("$_GFY_OUT"/*) ;; + (*) printf '%s\n' "$_GFY_F" ;; + esac +done) if [ -z "$_NON_GRAPH" ]; then exit 0 fi @@ -468,8 +480,11 @@ fi # branch switch but leaves the tree unchanged ΓÇö nothing to rebuild (#2421). [ "$PREV_HEAD" = "$NEW_HEAD" ] && exit 0 -# Only run if graphify-out/ exists (graph has been built before) -if [ ! -d "graphify-out" ]; then +# Only run if the output dir exists (graph has been built before). Resolve it +# from GRAPHIFY_OUT like the rebuild body does (#1423): a literal graphify-out/ +# here made the branch-switch rebuild a silent no-op for every renamed output dir. +_GFY_OUT="${GRAPHIFY_OUT:-graphify-out}" +if [ ! -d "${_GFY_OUT%/}" ]; then exit 0 fi diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 37a187f5f..bd6d7e521 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -1406,3 +1406,130 @@ def test_hook_probes_snap_confined_uv_tool_dirs(): assert '"$HOME"/snap/*/[0-9]*/.local/share/uv/tools' in _HOOK_SCRIPT assert '"$HOME"/snap/*/current/.local/share/uv/tools' in _HOOK_SCRIPT + + +# ── GRAPHIFY_OUT: the shell gates in front of the rebuild must honour it ───── +# +# #1423 moved the Python rebuild bodies onto GRAPHIFY_OUT, but the sh that runs +# BEFORE them still hardcoded graphify-out/: post-checkout exited unless a +# literal graphify-out/ directory existed, and post-commit's "only graph +# artifacts changed" filter only recognised graphify-out/ paths. With the +# documented override (#686) the branch-switch rebuild therefore never launched +# and a commit touching only the renamed output dir triggered a full rebuild. +# Behaviour of the emitted script under a real sh, per the #2126/#2641 +# convention, with the launcher stubbed so the test observes whether the hook +# REACHES the launch rather than running a rebuild. + +_LAUNCH_LINE = "launching background rebuild" + + +def _stub_python(tmp_path: Path) -> Path: + """A 'python' that passes the find_spec probe and swallows the launcher.""" + py = tmp_path / "stubpy" + py.write_text("#!/bin/sh\nexit 0\n", encoding="utf-8", newline="\n") + py.chmod(0o755) + return py + + +def _emitted_hook_run(repo: Path, script: str, args: list[str], env_extra: dict[str, str]): + stub = _stub_python(repo.parent) + rendered = script.replace("__PINNED_PYTHON__", str(stub)).replace("__VIZ_LIMIT_EXPORT__", "") + hook = repo.parent / "emitted-hook.sh" + hook.write_text(rendered, encoding="utf-8", newline="\n") + env = dict(os.environ) + env["HOME"] = str(repo.parent / "home") + for key in ("GIT_DIR", "GRAPHIFY_OUT", "GRAPHIFY_SKIP_HOOK"): + env.pop(key, None) + env.update(env_extra) + return subprocess.run( + ["sh", str(hook), *args], capture_output=True, text=True, cwd=str(repo), env=env, + ) + + +def _git(repo: Path, *args: str) -> None: + subprocess.run( + ["git", "-c", "user.name=t", "-c", "user.email=t@example.com", + "-c", "core.hooksPath=/dev/null", "-C", str(repo), *args], + check=True, capture_output=True, + ) + + +def _repo_with_graph_only_commit(tmp_path: Path, out_dir: str) -> Path: + """Two commits: a source file, then ONLY /graph.json — the + tracked-output case the post-commit filter exists for.""" + repo = tmp_path / "repo" + repo.mkdir() + _git(repo, "init", "-q") + (repo / "a.py").write_text("x = 1\n", encoding="utf-8") + _git(repo, "add", "a.py") + _git(repo, "commit", "-q", "-m", "src") + (repo / out_dir).mkdir() + (repo / out_dir / "graph.json").write_text("{}", encoding="utf-8") + _git(repo, "add", f"{out_dir}/graph.json") + _git(repo, "commit", "-q", "-m", "graph") + return repo + + +_SH_ONLY = pytest.mark.skipif( + shutil.which("sh") is None or os.name == "nt", reason="sh required to run the emitted hook" +) + + +@_SH_ONLY +def test_checkout_hook_rebuilds_when_graphify_out_is_renamed(tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + _git(repo, "init", "-q") + (repo / "custom-out").mkdir() # the configured output dir; no graphify-out/ + result = _emitted_hook_run(repo, _CHECKOUT_SCRIPT, ["aaa", "bbb", "1"], {"GRAPHIFY_OUT": "custom-out"}) + assert result.returncode == 0, result.stderr + assert _LAUNCH_LINE in result.stdout, ( + "post-checkout exited before the launch: it gates on a literal graphify-out/ " + f"instead of $GRAPHIFY_OUT\nstdout={result.stdout!r}\nstderr={result.stderr!r}" + ) + + +@_SH_ONLY +def test_checkout_hook_still_skips_when_no_graph_was_built(tmp_path): + """Control: with neither the default nor the configured dir present there is + no graph to refresh, so the branch-switch hook stays a no-op.""" + repo = tmp_path / "repo" + repo.mkdir() + _git(repo, "init", "-q") + result = _emitted_hook_run(repo, _CHECKOUT_SCRIPT, ["aaa", "bbb", "1"], {"GRAPHIFY_OUT": "custom-out"}) + assert result.returncode == 0, result.stderr + assert _LAUNCH_LINE not in result.stdout + + +@_SH_ONLY +def test_commit_hook_skips_graph_only_commit_when_graphify_out_is_renamed(tmp_path): + repo = _repo_with_graph_only_commit(tmp_path, "custom-out") + result = _emitted_hook_run(repo, _HOOK_SCRIPT, [], {"GRAPHIFY_OUT": "custom-out"}) + assert result.returncode == 0, result.stderr + assert _LAUNCH_LINE not in result.stdout, ( + "post-commit launched a rebuild for a commit that only touched the configured " + f"output dir\nstdout={result.stdout!r}" + ) + + +@_SH_ONLY +def test_commit_hook_skips_graph_only_commit_under_default_dir(tmp_path): + """Control: the default-name case the filter always handled keeps working.""" + repo = _repo_with_graph_only_commit(tmp_path, "graphify-out") + result = _emitted_hook_run(repo, _HOOK_SCRIPT, [], {}) + assert result.returncode == 0, result.stderr + assert _LAUNCH_LINE not in result.stdout + + +@_SH_ONLY +def test_commit_hook_still_rebuilds_for_a_source_change(tmp_path): + """Control: a commit that touches source alongside the renamed output dir + must still launch — the filter drops output-dir paths, not the rebuild.""" + repo = _repo_with_graph_only_commit(tmp_path, "custom-out") + (repo / "b.py").write_text("y = 2\n", encoding="utf-8") + (repo / "custom-out" / "graph.json").write_text("{\"n\": 1}", encoding="utf-8") + _git(repo, "add", "b.py", "custom-out/graph.json") + _git(repo, "commit", "-q", "-m", "src+graph") + result = _emitted_hook_run(repo, _HOOK_SCRIPT, [], {"GRAPHIFY_OUT": "custom-out"}) + assert result.returncode == 0, result.stderr + assert _LAUNCH_LINE in result.stdout, result.stdout