Skip to content

▚▚ fix(hooks): resolve the output dir from GRAPHIFY_OUT in the sh gates, not a literal graphify-out/ - #3546

Closed
breken-ai wants to merge 1 commit into
Graphify-Labs:v8from
breken-ai:fix/hooks-honor-graphify-out
Closed

breken-ai wants to merge 1 commit into
Graphify-Labs:v8from
breken-ai:fix/hooks-honor-graphify-out

Conversation

@breken-ai

Copy link
Copy Markdown

With GRAPHIFY_OUT set to anything but the default (the worktree / shared-output setup from #686), graphify hook install produces a post-checkout hook that never rebuilds: switching branches prints nothing and writes nothing to ~/.cache/graphify-rebuild.log. The post-commit hook has the mirror problem — a commit that only touches custom-out/graph.json (tracked outputs) launches a full rebuild, which is exactly the loop the "only graph artifacts changed" guard was added to prevent.

Root cause

#1423 moved the Python rebuild bodies onto os.environ.get('GRAPHIFY_OUT', 'graphify-out'), but the shell that runs before them still hardcodes the directory name:

  • _CHECKOUT_SCRIPT: if [ ! -d "graphify-out" ]; then exit 0; fi — with a renamed output dir this is always true, so the hook exits before _PYTHON_DETECT and the launcher ever run.
  • _HOOK_SCRIPT: _NON_GRAPH=$(echo "$CHANGED" | grep -v '^graphify-out/' || true) — paths under the configured dir are not filtered, so a graph-only commit looks like a source change.

The existing test only asserts that the Python body mentions GRAPHIFY_OUT, so the sh gates in front of it were never pinned.

Fix

  • graphify/hooks.py (_CHECKOUT_SCRIPT): gate on ${GRAPHIFY_OUT:-graphify-out} (trailing slash stripped), the same source the rebuild body reads.
  • graphify/hooks.py (_HOOK_SCRIPT): build _NON_GRAPH with a POSIX case loop keyed on "$_GFY_OUT"/* instead of grep -v, so the dir name is matched literally rather than as a regex. Each pattern carries the optional leading ( because bash 3.2 (macOS /bin/sh) otherwise mis-parses the pattern's ) as the end of the $(...) substitution — that was caught by the test run, not guessed.

Tests

tests/test_hooks.py — five tests that run the emitted hook scripts under a real sh in a scratch git repo, with the pinned python replaced by a stub that passes the find_spec probe and swallows the launcher, so the tests observe whether the hook reaches the launch line rather than running a rebuild:

  • test_checkout_hook_rebuilds_when_graphify_out_is_renamed — fails on HEAD (no launch line).
  • test_commit_hook_skips_graph_only_commit_when_graphify_out_is_renamed — fails on HEAD (launch line present).
  • Controls that pass before and after: checkout still no-ops when neither dir exists; the default graphify-out/ graph-only commit is still skipped; a commit with a source file alongside the renamed dir still launches.

Reverting only hooks.py makes the first two fail again.

Not changed

  • _PYTHON_DETECT's second probe still reads graphify-out/.graphify_python literally. It is one of five fallbacks for locating an interpreter (the pinned python is tried first), and no writer of that file lives in graphify/, so it is left alone rather than widened without a witness.
  • An absolute GRAPHIFY_OUT that points inside the repo (/repo/custom-out) still is not relativised for the commit filter; git diff --name-only emits repo-relative paths, so that case behaves as it did before this change (a rebuild is launched). Relative names and the default are the documented forms and are what this fixes.
  • fix(hooks): resolve merge-driver output path from .graphifyrc, skip gitignored graph.json #3207 (persisting GRAPHIFY_OUT to .graphifyrc so it survives a fresh shell) is complementary: once the value is available in the hook's environment, these gates now honour it.

▚▚ Shipped by breken — self-healing software. This one's on us. breken.ai

… not a literal graphify-out/

Graphify-Labs#1423 moved the Python rebuild bodies onto GRAPHIFY_OUT, but the shell that
runs BEFORE them still hardcoded graphify-out/. With the documented override
(Graphify-Labs#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

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.


Graphify review — findings

Makes the shell gates in front of the rebuild honour GRAPHIFY_OUT instead of a hardcoded graphify-out/: post-commit's "only graph artifacts changed" filter now strips paths under the configured output dir (via a POSIX case loop that survives bash 3.2's $(...) parsing), and post-checkout's existence check resolves the same dir before deciding whether a graph was ever built. Without this, a renamed output dir made the branch-switch rebuild a silent no-op and let a graph-only commit trigger a full rebuild. Adds sh-level tests that run the emitted hooks with the launcher stubbed to assert the rename and default-name paths both launch (or skip) correctly.

Worth a look

  • Post-commit graph-only filter misses ./-prefixed GRAPHIFY_OUTgraphify/hooks.py:424 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
  • Post-commit GRAPHIFY_OUT filter does not normalize equivalent relative pathsgraphify/hooks.py:429 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 416 functions depend on the 227 functions this change touches.

Health — this change adds coupling hotspots:

  • new: install() — 37 callers, 7 callees
  • new: dispatch_command() — 2 callers, 124 callees
  • new: dispatch_install_cli() — 2 callers, 31 callees
  • new: status() — 8 callers, 6 callees
  • new: uninstall() — 9 callers, 5 callees
  • new: uninstall_all() — 2 callers, 13 callees
  • new: test_poisoned_manifest_is_healed() — 0 callers, 6 callees

Verification — 416 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 242 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

21 of 275 test file(s) selected (8%) via static blast radius.

  • tests/test_affected_cli.py — impact
  • tests/test_agents_platform.py — impact
  • tests/test_codebuddy.py — impact
  • tests/test_devin.py — impact
  • tests/test_explain_cli.py — impact
  • tests/test_extract_cli.py — impact
  • tests/test_global_add_tag_inference.py — impact
  • tests/test_god_nodes_cli.py — impact
  • tests/test_hollow_chunks_arm_shrink_guard.py — impact
  • tests/test_hook_chain_survives_skip.py — impact
  • tests/test_hooks.py — impact, changed-test
  • tests/test_incomplete_build_guard.py — impact
  • tests/test_install.py — impact
  • tests/test_install_references.py — impact
  • tests/test_merge_chunks_validation.py — impact
  • tests/test_multigraph_diagnostics.py — impact
  • tests/test_no_dedup_flag.py — impact
  • tests/test_path_cli.py — impact
  • tests/test_query_cli.py — impact
  • tests/test_query_induced_edges.py — impact
  • tests/test_unverified_semantic_shrink.py — impact

Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.

· 7 more finding(s) on lines outside this diff (see the check run).

@safishamsi

Copy link
Copy Markdown
Collaborator

Shipped in v0.9.62 (now on PyPI: uv tool install graphifyy==0.9.62). Your commits were cherry-picked with authorship preserved, so this shows up under your GitHub contributions. Thanks @breken-ai!

@safishamsi safishamsi closed this Sep 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants