fix(hooks): strip shebang argument so pipx launchers resolve (#2629) - #2687
fix(hooks): strip shebang argument so pipx launchers resolve (#2629)#2687ousamabenyounes wants to merge 1 commit into
Conversation
![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.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR fixes shebang parsing in the git post-commit hook and the interpreter-detection probes across the various agent skill files (graphify/hooks.py, skill-*.md, and their generated tools/skillgen expected fixtures). It adds a step that strips a trailing interpreter argument (e.g., the -E in pipx's #!/.../python -E) so the leading path word is kept before the character allowlist check. The change spans a CHANGELOG entry and the corresponding hook test file (tests/test_hooks.py), touching many parallel skill-template copies that share the same snippet.
No blocking issues surfaced. 3 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1133 functions depend on the 987 functions this change touches.
Health — grade B; 7 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):
dispatch_command()— 2 callers, 117 callees (high)install()— 25 callers, 6 callees (high)dispatch_install_cli()— 2 callers, 31 callees (high)uninstall()— 9 callers, 5 callees (high)status()— 6 callers, 5 callees (medium)uninstall_all()— 2 callers, 13 callees (high)test_poisoned_manifest_is_healed()— 0 callers, 6 callees (medium)
Verification — 1133 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: 994 function(s) in the blast radius were not formally verified this run
…y-Labs#2629) pipx writes the graphify launcher shebang with an argument (`#!/.../python -E`). The interpreter probes used the whole shebang as a path; the space tripped the character allowlist, the valid interpreter was discarded, and the probe silently fell back to a `python3` without graphify — printing "could not locate a Python with graphify installed" after every commit and leaving the graph stale. Strip any interpreter argument (and resolve `/usr/bin/env python`) before using the shebang as a path, in the post-commit hook probe and both skill interpreter probes (regenerated via skillgen).
43eff52 to
cd98290
Compare
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR fixes handling of Python interpreter shebangs in the git post-commit hook (graphify/hooks.py) and across the generated per-agent skill files. The change strips any trailing interpreter argument (e.g. pipx's #!/.../python -E) from the parsed shebang before it's checked against the path allowlist, so the valid interpreter path is retained rather than falling back to python3. The same logic is applied consistently across all the skill-*.md files, the skillgen tool fragments/expected outputs, and a CHANGELOG entry (#2629) is added. Tests in test_hooks.py are also touched, presumably to cover the new shebang-argument stripping behavior. Reviewers should check that the shell parameter-expansion edits are duplicated identically across every skill file and that the generated/expected skillgen fixtures stay in sync with the source templates.
No blocking issues surfaced. 4 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1166 functions depend on the 1017 functions this change touches.
Health — this change adds coupling hotspots:
- new:
dispatch_command()— 2 callers, 117 callees - new:
install()— 33 callers, 7 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 — 1166 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: 1024 function(s) in the blast radius were not formally verified this run
· 7 more finding(s) on lines outside this diff (see the check run).
1 participant
Summary
Fixes #2629.
pipx installs write the
graphifylauncher with a shebang that carries anargument, e.g.
#!/Users/me/.local/pipx/venvs/graphifyy/bin/python -E. Everyinterpreter probe read that shebang and used the whole string as a path. The
trailing
-Eputs a space in the string, which trips the character allowlist,so the valid pipx interpreter is discarded and the probe silently falls back to
a bare
python3that has no graphify installed. On the reporter's machine thepost-commit hook therefore printed
after every commit and the graph silently stopped updating.
Fix
Strip any interpreter argument (and resolve
/usr/bin/env python) before usingthe shebang as a path, at all three POSIX probe sites:
graphify/hooks.py— the generated post-commit / post-checkout hook probe.tools/skillgen/fragments/shell/posix.md— the skill's step-1 interpreter probe.tools/skillgen/fragments/shell/interpreter-guard-posix.md— the subcommandguard that pins
graphify-out/.graphify_python.The
graphify/skill*.mdandtools/skillgen/expected/*changes are theregenerated skill artifacts (
python -m tools.skillgen); the two sourcefragments are the only hand edits there. The
aider/devinmonoliths inlinethe same probe but are frozen by
--monolith-roundtrip, so they are out ofscope for this change.
Reproduces with
A launcher whose shebang carries an argument resolves to a bare
python3instead of the real interpreter:
TDD verification (RED -> GREEN)
New tests in
tests/test_hooks.py:test_hook_probe_resolves_pipx_shebang_with_argumentexecutes the real_PYTHON_DETECTblock against a pipx-style launcher, andtest_generated_skill_probes_strip_shebang_argumentguards both rendered probesites.
RED (production fix reverted, tests kept):
GREEN (with fix):
Full suite:
uv run --frozen pytest tests/ -q— 4330 passed, 3 skipped. The 3failing
tests/test_ollama.py::test_detect_backend_*are pre-existing on aclean
v8checkout (backend-detection tests sensitive to a locally-runningOllama) and are unrelated to this change. skillgen validators (
--check,--audit-coverage,--schema-singleton,--monolith-roundtrip,--always-on-roundtrip) all pass.