fix(hooks): strip shebang argument so pipx launchers resolve (#2629)

[ousamabenyounes]

Summary

Addresses the argument-bearing launcher case in #2629. pipx launchers such as #!/absolute/venv/bin/python -E now resolve to their interpreter instead of treating the argument as part of its path.

The hook, generated POSIX skills, subcommand guard, and Aider/Devin source fragments remove only the leading shebang marker, handle /usr/bin/env [-S] /absolute/python [args], strip interpreter arguments, validate the path and verify the interpreter. Relative interpreter paths and PATH-controlled env names are rejected before execution. The hook accepts Windows drive/UNC spellings only in an MSYS, MinGW or Cygwin shell; those spellings are relative filenames on POSIX hosts. Regenerated skill outputs match their source fragments.

Merged current v8 while preserving the PR regressions and upstream hook coverage. The #2629 changelog entry is under Unreleased.

Scope: the later #!/bin/sh trampoline variant and whitespace-containing interpreter paths remain outside this change.

Parser boundary: additional env options and assignments fall through to the existing interpreter probes. Native PowerShell probes discover tool interpreters directly and do not parse launcher shebangs.

Test verification (RED → GREEN)

On unmodified current v8, with only the original regression tests copied in:

FAILED test_hook_probe_resolves_pipx_shebang_with_argument
FAILED test_hook_probe_resolves_env_split_shebang_with_argument
2 failed

The relative-path reviewer counterexample fails on the previous PR implementation in all five affected probe sources, for both direct and env-S launchers:

10 failed

The retired relative-path drift forms also reproduce 3 failing checks before their removal.

Six Windows-shaped relative-path cases also fail on POSIX before platform qualification, then pass with the fix.

Final focused hook and generator suite:

273 passed

Full validation: ./run-ci.sh final passed its blocking upstream-baseline comparison on all four CI Python versions.

Python	Upstream passes	PR passes	Unchanged failures	Skipped
3.10	8391	8425	2	13
3.12	8390	8424	2	14
3.13	8390	8424	2	14
3.14	8390	8424	2	14

The two local environment failures are unchanged: test_atomic_replace_temp_name_is_bounded_and_does_not_embed_long_filename (long allocation path) and test_unix_socket_is_rejected (AF_UNIX path too long). Every added test passes on every matrix version.

All five skillgen validators, Ruff, CLI/install smoke and 20 Pi guard tests pass. Pyright (637 diagnostics), Bandit (12 findings), and pip-audit (44 advisories) match upstream exactly. Supplemental unused-import checking matches the existing single dataclasses.field finding.

Changed executable coverage: Python 4/4 statement anchors; Bash xtrace 29/29 source-command lines across the hook and authoritative fragments, including both Devin probes. The hook’s Windows rooted/relative path grammar checks pass. Native Windows execution was not part of the Linux CI matrix.

[graphify-labs]

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

[graphify-labs]

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).

[ousamabenyounes]

Updated this PR at ff8aa7a668435948a380802871fa239eceee8d60.

What changed since the previous version:

• Rebased onto v8.
• Extended the pipx shebang-argument fix to generated skill probes plus the Aider/Devin monolith sources.
• Fixed the Devin subcommand interpreter guard so it strips shebang args and verifies the stripped candidate before writing graphify-out/.graphify_python.
• Regenerated skill artifacts and moved the changelog entry to Unreleased.

Validation:

• RED on origin/v8: copied-in tests fail for the hook probe, generated skill probe, and interpreter guard cases.
• Extra RED: removing the Devin guard hunk makes test_monolith_sources_strip_shebang_argument fail.
• Focused GREEN: 6 targeted tests pass.
• Final local ./run-ci.sh: same four local baseline failures as clean origin/v8, with 5110 passes vs 5106 baseline.
• GitHub checks on the pushed SHA are green: skillgen-check, security-scan, test (3.10), test (3.12).

[graphify-labs]

Graphify reviewed this change.

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

---

Graphify review — findings

Fixes the git post-commit hook and skill interpreter probes to strip a shebang argument (#!/.../python -E from pipx) before treating it as a path, so the valid interpreter no longer trips the character allowlist and silently falls back to a python3 without graphify. The in-repo .graphify_python resolution now also verifies the chosen interpreter can import graphify and drops to python3 only when it can't, applied consistently across graphify/hooks.py and every generated skill file.

Worth a look

• env -S shebang can be accepted as interpreter path — graphify/hooks.py:88 · Escalate · medium
  • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
• env -S shebangs are parsed as the env option instead of the interpreter — graphify/hooks.py:90 · Escalate · medium
  • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
• tr -d '#!' strips all '#' and '!' chars from shebang, corrupting env-based paths — graphify/skill-copilot.md:78 · Escalate · medium
  • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
• Valid env -S shebangs are parsed as the interpreter name '-S' — graphify/skill-copilot.md:79 · Escalate · medium
  • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
• Graphify python guard verifies PATH-controlled interpreter — tools/skillgen/fragments/shell/interpreter-guard-posix.md:12 · 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 — 1580 functions depend on the 1352 functions this change touches.

Health — this change adds coupling hotspots:

• new: dispatch_command() — 2 callers, 122 callees
• new: install() — 33 callers, 7 callees
• new: render() — 13 callers, 5 callees
• new: dispatch_install_cli() — 2 callers, 31 callees
• new: status() — 8 callers, 6 callees
• new: audit_coverage() — 8 callers, 6 callees
• new: uninstall() — 9 callers, 5 callees
• new: main() — 3 callers, 11 callees
• …and 4 more — each is listed as a finding

Verification — 1580 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: 1424 function(s) in the blast radius were not formally verified this run

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

[ousamabenyounes]

Updated this PR at d3b92f9d146b411f75b358d4ed0827accb5f15d8.

What changed since the previous version:

• Rebased onto current v8.
• Addressed the Graphify advisory items for /usr/bin/env -S parsing, destructive tr -d '#!' shebang stripping, and PATH-controlled env-derived interpreter names.
• Kept Windows-style shebang paths accepted while rejecting bare env command names before verification.
• Regenerated skill artifacts from tools/skillgen and tightened the monolith drift guard.

Validation:

• RED: new env-S, shebang-corruption, and PATH-controlled-interpreter tests fail before the fix.
• Focused GREEN: 17 passed.
• python3 -m pytest reviewer repro subset: 3 passed.
• tools.skillgen --check, tools.skillgen --monolith-roundtrip, ruff, git diff --check: pass.
• Coverage: framework diff gate pass; AST-aware Python diff coverage 5/5 changed executable statements covered.
• Final local ./run-ci.sh: same four local baseline failures as clean origin/v8, with 5310 passes vs 5295 baseline on both Python 3.10 and 3.12.
• GitHub Actions on the pushed SHA are green: skillgen-check, security-scan, test (3.10), test (3.12).

[graphify-labs]

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

Fixes the git post-commit hook and every skill interpreter probe to derive the Python path from a graphify shebang correctly: they strip a shebang argument (so pipx's #!/.../python -E resolves instead of tripping the path allowlist), parse /usr/bin/env -S ... launchers alongside plain /usr/bin/env, and reject bare PATH-controlled env names like python so an unverified interpreter can't be trusted. When the shebang yields no usable path, the skills fall back to python3 and verify import graphify before caching it in .graphify_python, ending the "could not locate a Python with graphify installed" message printed after every commit (#2629).

No blocking issues surfaced. 15 lower-confidence candidates did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 1674 functions depend on the 1503 functions this change touches.

Health — this change adds coupling hotspots:

• new: dispatch_command() — 2 callers, 123 callees
• new: install() — 33 callers, 7 callees
• new: render() — 13 callers, 5 callees
• new: dispatch_install_cli() — 2 callers, 31 callees
• new: status() — 8 callers, 6 callees
• new: audit_coverage() — 8 callers, 6 callees
• new: uninstall() — 9 callers, 5 callees
• new: main() — 3 callers, 11 callees
• …and 4 more — each is listed as a finding

Verification — 1674 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: 1510 function(s) in the blast radius were not formally verified this run

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

[ousamabenyounes]

Rebased onto the current v8 head to clear the merge conflict (the PR had gone CONFLICTING after v8 advanced to the 0.9.56 (unreleased) cycle). Pushed as 09edf3d.

The only conflict was in CHANGELOG.md: v8 had replaced the old ## Unreleased section with ## 0.9.56 (unreleased). Resolved by moving the #2629 entry into that section (keeping the hardened wording from the last commit) and leaving every 0.9.56 entry untouched — the diff vs v8 is now exactly one added changelog line. The three fix commits (hook probe, skill probes, env-shebang hardening) are unchanged.

Validation on the rebased tree:

• RED (new tests on pristine v8 base): 11 of the 15 added tests fail — the positive pipx/env -S resolution paths plus the skillgen guard _is_shebang_argument_fix_line, which does not exist on base.
• GREEN (focused): tests/test_hooks.py + tests/test_skillgen.py → 182 passed.
• skillgen --check / --audit-coverage / --schema-singleton / --monolith-roundtrip / --always-on-roundtrip all OK; ruff clean; git diff --check clean.
• Full suite (local run-ci.sh, Python 3.12, --all-extras): baseline v8 = 1 failed, 5474 passed, 14 skipped; rebased = 1 failed, 5489 passed, 14 skipped (+15 new tests). The single failure is the pre-existing environment-only test_extract.py::test_collect_files_skips_hidden (the checkout carries hidden .venv/.hypothesis dirs), identical on both trees.
• Remote CI on 09edf3d: skillgen-check, test (3.10), test (3.12), security-scan all green.

[graphify-labs]

Graphify reviewed this change.

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

---

Graphify review — findings

Fixes graphify's Python-interpreter discovery in the post-commit hook and every generated skill probe (#2629): shebang parsing now strips a trailing interpreter argument (so pipx's #!/.../python -E resolves instead of tripping the path allowlist), handles /usr/bin/env -S ... launchers, and rejects bare PATH-controlled env names like python before verification. Skill probes additionally import-check the resolved interpreter and fall back to python3 only when it lacks graphify, so a valid pipx interpreter no longer silently degrades to a graphify-less python3 that prints "could not locate a Python with graphify installed" after each commit. Regenerates the expected skill fixtures and adds shebang-parsing tests to match.

Worth a look

• env -S parser treats env options as the interpreter — graphify/hooks.py:84 · Escalate · medium
  • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
• env-name shebang like #!/usr/bin/env python is dropped, losing valid interpreter — graphify/skill-kilo.md:84 · Escalate · medium
  • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
• PowerShell/core generator not updated to match shebang parsing change — tools/skillgen/gen.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

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 1690 functions depend on the 1511 functions this change touches.

Health — this change adds coupling hotspots:

• new: install() — 37 callers, 7 callees
• new: dispatch_command() — 2 callers, 124 callees
• new: render() — 13 callers, 5 callees
• new: dispatch_install_cli() — 2 callers, 31 callees
• new: status() — 8 callers, 6 callees
• new: audit_coverage() — 8 callers, 6 callees
• new: uninstall() — 9 callers, 5 callees
• new: main() — 3 callers, 11 callees
• …and 4 more — each is listed as a finding

Verification — 1690 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: 1526 function(s) in the blast radius were not formally verified this run

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

[graphify-labs]

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

Fixes interpreter detection in the git post-commit hook and every skill's Python-probe so a pipx launcher (#!/.../python -E) resolves instead of silently degrading to a python3 without graphify. The probes now parse /usr/bin/env -S ... launchers, strip any trailing shebang argument before treating the shebang as a path, reject PATH-controlled bare env names (e.g. python) so only real interpreter paths are trusted, and verify the fallback candidate actually imports graphify before caching it in .graphify_python. Regenerated skill fixtures and expanded hook/skillgen tests cover the shebang-argument, launcher-quoting, and per-platform fallback paths.

Worth a look

• Relative shebang interpreter paths are trusted and executed — graphify/hooks.py:94 · Escalate · medium
  • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
• env -S with multiple flags loses interpreter path after first-word strip — graphify/hooks.py:83 · 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 — 1741 functions depend on the 1552 functions this change touches.

Health — this change adds coupling hotspots:

• new: main() — 98 callers, 3 callees
• new: install() — 37 callers, 7 callees
• new: dispatch_command() — 2 callers, 125 callees
• new: render() — 13 callers, 5 callees
• new: dispatch_install_cli() — 2 callers, 31 callees
• new: status() — 8 callers, 6 callees
• new: audit_coverage() — 8 callers, 6 callees
• new: uninstall() — 9 callers, 5 callees
• …and 5 more — each is listed as a finding

Verification — 1741 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: 1567 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

300 of 300 test file(s) selected (100%) via static blast radius.

Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.

• tests/test_affected_cli.py — impact, full-run-safety
• tests/test_affected_member_seed.py — full-run-safety
• tests/test_agents_platform.py — impact, full-run-safety
• tests/test_analyze.py — full-run-safety
• tests/test_anthropic_custom_endpoint.py — full-run-safety
• tests/test_antigravity_install.py — full-run-safety
• tests/test_apm_fallback_version.py — full-run-safety
• tests/test_architecture_doc.py — full-run-safety
• tests/test_astro_extraction.py — full-run-safety
• tests/test_astro_import_ids.py — full-run-safety
• tests/test_atomic_canvas_export.py — full-run-safety
• tests/test_atomic_version_stamp.py — full-run-safety
• tests/test_atomic_writes.py — full-run-safety
• tests/test_backend_env_isolation.py — full-run-safety
• tests/test_backend_extras.py — full-run-safety
• tests/test_benchmark.py — full-run-safety
• tests/test_benchmark_raw_graph.py — full-run-safety
• tests/test_build.py — full-run-safety
• tests/test_build_merge_dedup_scope.py — full-run-safety
• tests/test_build_merge_hyperedges_and_prune.py — full-run-safety
• tests/test_build_merge_shrink_guard.py — full-run-safety
• tests/test_builtin_global_type_refs.py — full-run-safety
• tests/test_cache.py — full-run-safety
• tests/test_callflow_html.py — full-run-safety
• tests/test_cargo_introspect.py — full-run-safety
• tests/test_cargo_missing_manifest.py — full-run-safety
• tests/test_carried_hyperedge_remap.py — full-run-safety
• tests/test_case_sensitive_resolution.py — full-run-safety
• tests/test_charmap_encoding.py — full-run-safety
• tests/test_chunking.py — full-run-safety
• tests/test_cjs_module_extension.py — full-run-safety
• tests/test_claude_cli_backend.py — full-run-safety
• tests/test_claude_md.py — full-run-safety
• tests/test_cli_broken_pipe.py — full-run-safety
• tests/test_cli_export.py — full-run-safety
• tests/test_cli_help.py — full-run-safety
• tests/test_cluster.py — full-run-safety
• tests/test_cobol_extractor.py — full-run-safety
• tests/test_codebuddy.py — impact, full-run-safety
• tests/test_community_hub_labels.py — full-run-safety
• tests/test_community_labels_skill.py — full-run-safety
• tests/test_confidence.py — full-run-safety
• tests/test_corrupt_graph_json.py — full-run-safety
• tests/test_cpp_nested_and_cli.py — full-run-safety
• tests/test_cpp_objc_cross_file_calls.py — full-run-safety
• tests/test_cpp_preprocess.py — full-run-safety
• tests/test_cross_extension_reexport_self_cycle.py — full-run-safety
• tests/test_cross_language_call_resolution.py — full-run-safety
• tests/test_cross_repo_external_call_guards.py — full-run-safety
• tests/test_cross_repo_member_calls.py — full-run-safety
• … and 250 more

non-code file(s) changed (CHANGELOG.md, graphify/skill-agents.md, graphify/skill-aider.md, graphify/skill-amp.md, graphify/skill-claw.md …) → running the full suite for safety (a code graph can't see config/fixture/data deps)

changed code file(s) with no mapped test (CHANGELOG.md, graphify/skill-agents.md, graphify/skill-aider.md, graphify/skill-amp.md, graphify/skill-claw.md …) — a coverage gap or a missing link — running the full suite rather than only the selected tests

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.

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

[ousamabenyounes]

Updated at fecc718cfa6180f824ee83c840e82c942a54176d. Merged current v8, preserved both sets of hook tests, and addressed relative shebang execution, including Windows-shaped names on POSIX hosts, across the hook and skill probes. Extra env options/assignments now have an explicit documented fallback boundary; ordinary env [-S] /absolute/python [args] remains supported. Regenerated all skill artifacts. The later trampoline/space-path variant remains separate.

Validation: original upstream repro RED, 10 relative-path cases RED → GREEN, 273 focused tests pass; all four local Python matrix runs match baseline with 34 added passes and the same two environment failures. Generator checks, Ruff, 20 Pi guard tests and 100% changed-code coverage pass; static/security findings match baseline. Fresh Claude expert review passed and GitHub Actions passed on this SHA; Graphify formal verification returned neutral (no functions verified).