fix(install): emit a bare hook command for the default install too (#3129) - #3268
fix(install): emit a bare hook command for the default install too (#3129)#3268yotamleo wants to merge 1 commit into
Conversation
…raphify-Labs#3129) v0.9.52 (Graphify-Labs#3149) fixed --project installs so the committed hook config carries a bare `graphify` command instead of a machine-absolute path. The plain (non---project) install was left as a follow-up, on the theory that it targets a user-profile location nobody else maintains. That theory doesn't hold for the three hook writers themselves: _install_codex_hook, _claude_pretooluse_hooks (claude + codebuddy) and _gemini_hook all write into project_dir, which defaults to the cwd, whether or not --project was passed. There is no separate user-profile destination for these specific files today, so they are always a commit candidate, and the installing machine's absolute path is wrong in every other clone regardless of the flag. All three (plus codebuddy, which has no --project variant at all) now always resolve the bare command; the `project` parameter threaded through them by Graphify-Labs#3149 is dropped since it no longer affects the outcome. `_resolve_graphify_exe`'s absolute-path branch is untouched and still correct for a genuine, non-shared destination -- if Graphify-Labs#2164 ever gives the plain install one, that is the point to wire it back in. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
Formal verification. 2 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_claude\_pretooluse\_hooks changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_claude\_pretooluse\_hooks behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"strict":"True"\}, the old code produced \[\{'matcher': 'Bash\|Grep', 'hooks': \[\{'type': 'command', 'command': '/opt/venv/bin/graphify hook\-guard search'\}\]\}, \{'matcher': 'Read\|Glob', 'hooks': \[\{'type': 'command', 'command': '/opt/… but the new code produces \[\{'matcher': 'Bash\|Grep', 'hooks': \[\{'type': 'command', 'command': 'graphify hook\-guard search'\}\]\}, \{'matcher': 'Read\|Glob', 'hooks': \[\{'type': 'command', 'command': 'graphify hook\-guar…. Paste that input straight into a regression test.
Behavior changes: \_gemini\_hook changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_gemini\_hook behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This is a sound refutation from differential testing (both versions run on many generated inputs): a concrete input on which the two versions provably differ.
Graphify review — findings
Makes agent hook commands always emit the bare graphify invocation instead of a machine-absolute exe path, since .claude/.codebuddy/.gemini/.codex settings files are always written into the working directory and thus always commit candidates — an absolute path resolved on the installing machine would break in every other clone (#3129). Drops the now-dead project parameter threaded through _install_claude_hook, _claude_pretooluse_hooks, _gemini_hook, _install_gemini_hook, _install_codex_hook, _agents_install, and claude_install, hardcoding _resolve_graphify_exe(project=True) at each call site. Adds tests asserting hook commands stay portable, including CodeBuddy which has no project variant at all.
Worth a look
- resolve_graphify_exe now returns bare name for non-project installs, breaking Scripts/-not-on-PATH hooks —
graphify/install.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Removed public claude_install project keyword —
graphify/install.py:1751· 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 — 503 functions depend on the 286 functions this change touches.
Health — this change adds coupling hotspots:
- new:
dispatch_command()— 2 callers, 123 callees - new:
codebuddy_install()— 21 callers, 5 callees - new:
claude_install()— 19 callers, 4 callees - new:
gemini_install()— 10 callers, 7 callees - new:
claude_uninstall()— 17 callers, 4 callees - new:
_project_uninstall()— 5 callers, 13 callees - new:
dispatch_install_cli()— 2 callers, 31 callees - new:
codebuddy_uninstall()— 14 callers, 4 callees - …and 13 more — each is listed as a finding
Verification — 503 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: 431 function(s) in the blast radius were not formally verified this run
Formal verification
Behavior changes: \_claude\_pretooluse\_hooks changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_claude\_pretooluse\_hooks behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"strict":"True"\}, the old code produced \[\{'matcher': 'Bash\|Grep', 'hooks': \[\{'type': 'command', 'command': '/opt/venv/bin/graphify hook\-guard search'\}\]\}, \{'matcher': 'Read\|Glob', 'hooks': \[\{'type': 'command', 'command': '/opt/… but the new code produces \[\{'matcher': 'Bash\|Grep', 'hooks': \[\{'type': 'command', 'command': 'graphify hook\-guard search'\}\]\}, \{'matcher': 'Read\|Glob', 'hooks': \[\{'type': 'command', 'command': 'graphify hook\-guar…. Paste that input straight into a regression test.
Behavior changes: \_gemini\_hook changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_gemini\_hook behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This is a sound refutation from differential testing (both versions run on many generated inputs): a concrete input on which the two versions provably differ.
Could not verify: Could not verify \_agents\_install.
The verifier did not have enough to check \_agents\_install, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly TypeError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify claude\_install.
The verifier did not have enough to check claude\_install, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 58 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly TypeError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify gemini\_install.
The verifier did not have enough to check gemini\_install, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `project_dir` is annotated `Path | None` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_install\_claude\_hook.
The verifier did not have enough to check \_install\_claude\_hook, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `project_dir` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_install\_codex\_hook.
The verifier did not have enough to check \_install\_codex\_hook, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `project_dir` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_install\_gemini\_hook.
The verifier did not have enough to check \_install\_gemini\_hook, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `project_dir` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_project\_install.
The verifier did not have enough to check \_project\_install, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `project_dir` is annotated `Path | None` — outside the synthesizable primitive/collection set
· 5 grounded finding(s) anchored inline below; 16 more finding(s) on lines outside this diff (see the check run).
| shutil.copy2(settings_path, backup) | ||
| settings_path.write_text(output, encoding="utf-8") | ||
| def _install_gemini_hook(project_dir: Path, project: bool = False) -> None: | ||
| def _install_gemini_hook(project_dir: Path) -> None: |
There was a problem hiding this comment.
_install_gemini_hook()
high coupling complexity (Ca·Ce = 12).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| break | ||
| return (found or "graphify").replace("\\", "/") | ||
| def _install_codex_hook(project_dir: Path, project: bool = False) -> None: | ||
| def _install_codex_hook(project_dir: Path) -> None: |
There was a problem hiding this comment.
_install_codex_hook()
high coupling complexity (Ca·Ce = 20).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| hooks_path.write_text(json.dumps(existing, indent=2), encoding="utf-8") | ||
| print(f" .codex/hooks.json -> PreToolUse hook removed") | ||
| def _agents_install(project_dir: Path, platform: str, project: bool = False) -> None: | ||
| def _agents_install(project_dir: Path, platform: str) -> None: |
There was a problem hiding this comment.
_agents_install()
fans out to 6 callees (efferent coupling); 6 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| removed = _kilo_uninstall_global() | ||
| print("; ".join(removed) if removed else "nothing to remove") | ||
| def claude_install(project_dir: Path | None = None, strict: bool = False, project: bool = False) -> None: | ||
| def claude_install(project_dir: Path | None = None, strict: bool = False) -> None: |
There was a problem hiding this comment.
claude_install()
19 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| print("Strict mode: the first raw file read per session is blocked until") | ||
| print("one `graphify query` runs (toggle with GRAPHIFY_HOOK_STRICT=0).") | ||
| def _install_claude_hook(project_dir: Path, strict: bool = False, project: bool = False) -> None: | ||
| def _install_claude_hook(project_dir: Path, strict: bool = False) -> None: |
There was a problem hiding this comment.
_install_claude_hook()
high coupling complexity (Ca·Ce = 20).
Grounded coupling-delta finding (deterministic), not an LLM guess.
1 participant
Summary
v0.9.52 (#3149, thanks @davidbhoward for the original analysis) fixed
graphify install --project: a committed, shared install now emits a bare,run-time-resolved
graphifycommand instead of pinning an absoluteinterpreter path. That PR was deliberately scoped to
--projectonly, andsaid so explicitly:
This is the deferred half, offered as the patch mentioned on the issue thread
on 2026-09-01. It only extends #3149's own fix to the case its own scope
section named as open.
The remaining defect
_install_codex_hook,_claude_pretooluse_hooks(claude + codebuddy) and_gemini_hookall write intoproject_dir, which defaults to the currentdirectory -- whether or not
--projectwas passed. There is no separateuser-profile destination for these specific hook files today; only the flag
that picked which exe path got resolved into them differed. So a plain
graphify codex install(no flag) still wrote a machine-absolute path(e.g.
C:/Users/<user>/.local/bin/graphify.EXE hook-check) into.codex/hooks.json, and if that gets committed -- which is exactly what theissue's own reproduction does -- it is wrong on every other machine and in CI.
Checked whether this was codex-specific, per the discussion on the issue: it
is not.
claude_install,gemini_installandcodebuddy_installall resolvetheir hook destination the same way (
project_dir or Path("."), nodistinction by scope), so all three shared the identical defect --
_claude_pretooluse_hooksis used by both claude and codebuddy, and codebuddyhas no
--projectvariant at all, meaning every codebuddy install hit this.The fix
Small and destination-keyed, not flag-keyed: since these particular hook files
are always written into the current directory regardless of
--projecttoday, the exe they embed is now always the bare
graphifycommand,independent of that flag. The
projectparameter #3149 threaded through thesethree functions is dropped since it no longer changes the outcome for them;
_resolve_graphify_exeitself, and its absolute-path branch, are untouched.Why this doesn't preempt #2164
#2164 is about where the non-project install writes (cwd vs. a real
user-profile directory). This PR doesn't touch that -- it only changes what
gets embedded in the file at whatever destination is used today, which is
unconditionally the project directory. If #2164 is ever fixed so the
non-project install gets a genuine, non-shared destination,
_resolve_graphify_exe'sabsolute-path branch (left exactly as-is here) is what that destination should
resolve to -- that's the point to reintroduce a project/non-project
distinction on these three call sites, not before. This PR should need no
rework when that lands; it just stops being the only caller of that branch's
bare-command side.
The cost this pays, stated plainly
There is a real trade-off here and it deserves naming rather than burying,
because
_resolve_graphify_exe's own docstring makes the case against thischange:
That is true, and this PR gives it up for these three hook files. On a machine
where
graphifyis not on PATH, a bare command fails where the absolute pathwould have worked.
I think #3149 already answered this, and the answer now covers more ground than
it did when written:
The last sentence is the hinge. That argument was scoped to "the committed
case" because the non-project install was assumed to have a different,
uncommitted destination. For these three hook files it does not — every
install writes them into the working directory — so every case is the
committed case, and #3149's own reasoning extends to all of them.
If you would rather keep the absolute path for the PATH-fragile environments
and accept the churn, the alternative is (2) from the issue thread: stop
writing these files into a tracked location for non-project installs. That is
the larger change and it overlaps #2164, which is presumably why you deferred
it — I have not attempted it here.
One consequence worth flagging for review
After this change all three call sites pass
_resolve_graphify_exe(project=True),so its absolute-path branch has no live caller. I deliberately did not
delete it: #2164 is exactly the change that should revive it, and removing it
now would mean re-deriving the Windows/PATH reasoning later from the git log.
But it does mean the parameter now reads oddly at the call sites — a
non-project install asking for
project=True. If you take this, a rename(
bare=True, or an explicit_bare_graphify_command()helper) would say whatis actually meant; I left the signature alone to keep the diff reviewable and
because naming is yours to pick.
Scope
Matches #3149's own boundary: the trailing-newline and
customized-hook-clobber issues it flagged as deliberately out of scope are
still untouched here.
This is an offer, not a demand
Per the maintainer's own framing on the issue (partial fix landed, remainder
"left for a maintainer call"), this follows the "(1)" option floated on the
thread -- happy to close this in favor of a different design, or adjust scope,
if a different approach is preferred.
Refs #3129.
Verification
Re-verified the defect at current
v8HEAD (33362d9, "chore: bump to0.9.53") before writing any code:
graphify codex install(no--project)still writes
_resolve_graphify_exe(project=False)'s absolute result into.codex/hooks.jsonin the cwd;claude_install/gemini_install/codebuddy_installall showed the same
project_dir or Path(".")pattern.Red first. Added/renamed tests in
tests/test_install.py(
test_default_install_hook_command_is_also_portable, parametrized overclaude/codex/gemini, plus
test_codebuddy_hook_command_is_portable), andconfirmed they fail against unmodified
v8:All four pass after the fix.
Existing #3149 test conflict, resolved explicitly, not silently.
test_user_profile_install_still_resolves_absolute_pathpinned the premisethat the non-project install has a separate, non-shared destination. That
premise doesn't hold for these three writers (see above), so the fix
necessarily changes what that test asserts. Rather than delete or quietly
tweak it, it's replaced by
test_default_install_hook_command_is_also_portablewith a docstring stating exactly why the old test's premise no longer holds
and cross-referencing this PR, plus a module comment explaining the
supersession for anyone reading the test file later.
Scoped run:
The 3 failures (
test_codex_skill_uses_graphify_with_existing_graph,test_hermes_skill_destination_posix_uses_home,test_skill_roundtrip_at_real_destination[user-hermes]) are pre-existing andunrelated (Windows
LOCALAPPDATA/hermes path handling) -- confirmed present,identical, on a clean
v8checkout before this change (170 passed, 3 failed;the one extra passing test on the fixed branch is the added
test_codebuddy_hook_command_is_portable).Full suite, both branches, failure sets diffed:
Identical 30-test failure set on both (symlinks/fifo/unix-socket, hermes/LOCALAPPDATA,
ollama retry-cap, terraform parser, ambient-environment tests unrelated to
install.py) -- zero introduced, zero fixed. Windows-only run; the Ubuntu CI
matrix is exact parity per the README.
Lint / CI gates:
(
install.pyis not a skillgen-generated file; ran the gate anyway since it'sa blocking CI job.)