From 88bb1864aaa851e05ab697f7c9e6904528c612ad Mon Sep 17 00:00:00 2001 From: Safi Date: Wed, 3 Jun 2026 14:28:15 +0100 Subject: [PATCH] fix hook install silent no-op for uv tool and pipx installs (#1127) graphify hook install generated scripts that resolved the Python interpreter purely at git-trigger time via 'command -v graphify'. GUI git clients (VS Code, GitKraken), CI runners, and non-login shells often run with a minimal PATH that omits ~/.local/bin -- the uv-tool / pipx launcher location. command -v graphify returned empty, the python3/python fallbacks could not import graphify (it lives only in the isolated venv), and the hook silently exited 0 with no output. Fix: embed sys.executable of the currently-running install process as a pinned first probe. Since 'graphify hook install' itself runs under the correct isolated interpreter, sys.executable is always the right path. It is validated at hook-runtime via 'import graphify' before use, so a stale pinned path safely falls through to the existing dynamic detection rather than breaking the hook. Also make the final fallback loud: print a diagnostic to stderr before exiting 0 so the failure is visible rather than invisible. Co-Authored-By: Claude Opus 4.8 --- graphify/hooks.py | 70 +++++++++++++++++++++++++++++++-------------- tests/test_hooks.py | 36 +++++++++++++++++++++++ 2 files changed, 85 insertions(+), 21 deletions(-) diff --git a/graphify/hooks.py b/graphify/hooks.py index bf824ebb..aeaa8d14 100644 --- a/graphify/hooks.py +++ b/graphify/hooks.py @@ -10,34 +10,51 @@ _HOOK_MARKER_END = "# graphify-hook-end" _CHECKOUT_MARKER = "# graphify-checkout-hook-start" _CHECKOUT_MARKER_END = "# graphify-checkout-hook-end" +# __PINNED_PYTHON__ is replaced at install time with the absolute path of the +# Python interpreter that ran `graphify hook install`. For uv-tool and pipx +# installs the interpreter lives inside an isolated venv, so the launcher on +# PATH is the only entry point — and GUI git clients / CI runners often have a +# minimal PATH that omits ~/.local/bin. Pinning sys.executable at install time +# makes the hook work regardless of PATH at git-trigger time. _PYTHON_DETECT = """\ -# Detect the correct Python interpreter (handles pipx, venv, system installs) -GRAPHIFY_BIN=$(command -v graphify 2>/dev/null) -if [ -n "$GRAPHIFY_BIN" ]; then - case "$GRAPHIFY_BIN" in - *.exe) _SHEBANG="" ;; - *) _SHEBANG=$(head -1 "$GRAPHIFY_BIN" | sed 's/^#![[:space:]]*//') ;; - esac - case "$_SHEBANG" in - */env\\ *) GRAPHIFY_PYTHON="${_SHEBANG#*/env }" ;; - *) GRAPHIFY_PYTHON="$_SHEBANG" ;; - esac - # Allowlist: only keep characters valid in a filesystem path to prevent - # injection if the shebang contains shell metacharacters - case "$GRAPHIFY_PYTHON" in - *[!a-zA-Z0-9/_.@-]*) GRAPHIFY_PYTHON="" ;; - esac - if [ -n "$GRAPHIFY_PYTHON" ] && ! "$GRAPHIFY_PYTHON" -c "import graphify" 2>/dev/null; then - GRAPHIFY_PYTHON="" +# Detect the correct Python interpreter (handles uv tool, pipx, venv, system installs). +# _PINNED was recorded at hook-install time; tried first so the hook works even +# when the graphify launcher is not on PATH (common in GUI clients and CI). +GRAPHIFY_PYTHON="" +_PINNED="__PINNED_PYTHON__" +if [ -n "$_PINNED" ] && [ -x "$_PINNED" ] && "$_PINNED" -c "import graphify" 2>/dev/null; then + GRAPHIFY_PYTHON="$_PINNED" +fi +# Dynamic fallback: resolve via the graphify launcher on PATH (shebang probe). +if [ -z "$GRAPHIFY_PYTHON" ]; then + GRAPHIFY_BIN=$(command -v graphify 2>/dev/null) + if [ -n "$GRAPHIFY_BIN" ]; then + case "$GRAPHIFY_BIN" in + *.exe) _SHEBANG="" ;; + *) _SHEBANG=$(head -1 "$GRAPHIFY_BIN" | sed 's/^#![[:space:]]*//') ;; + esac + case "$_SHEBANG" in + */env\\ *) GRAPHIFY_PYTHON="${_SHEBANG#*/env }" ;; + *) GRAPHIFY_PYTHON="$_SHEBANG" ;; + esac + # Allowlist: only keep characters valid in a filesystem path to prevent + # injection if the shebang contains shell metacharacters. + case "$GRAPHIFY_PYTHON" in + *[!a-zA-Z0-9/_.@-]*) GRAPHIFY_PYTHON="" ;; + esac + if [ -n "$GRAPHIFY_PYTHON" ] && ! "$GRAPHIFY_PYTHON" -c "import graphify" 2>/dev/null; then + GRAPHIFY_PYTHON="" + fi fi fi -# Fall back: try python3, then python (Windows has no python3 shim) +# Last resort: try python3 / python (works for system/venv installs on PATH). if [ -z "$GRAPHIFY_PYTHON" ]; then if command -v python3 >/dev/null 2>&1 && python3 -c "import graphify" 2>/dev/null; then GRAPHIFY_PYTHON="python3" elif command -v python >/dev/null 2>&1 && python -c "import graphify" 2>/dev/null; then GRAPHIFY_PYTHON="python" else + echo "[graphify hook] could not locate a Python with graphify installed. Add the graphify bin dir to PATH or re-run 'graphify hook install' from the env where graphify lives." >&2 exit 0 fi fi @@ -298,8 +315,19 @@ def install(path: Path = Path(".")) -> str: hooks_dir = _user_hooks_dir(_hooks_dir(root)) - commit_msg = _install_hook(hooks_dir, "post-commit", _HOOK_SCRIPT, _HOOK_MARKER) - checkout_msg = _install_hook(hooks_dir, "post-checkout", _CHECKOUT_SCRIPT, _CHECKOUT_MARKER) + # Pin the current interpreter so the hook works even when the graphify + # launcher is not on PATH at git-trigger time (uv tool / pipx isolation). + # sys.executable is the Python running this very install command, so it is + # always the correct isolated-venv interpreter. The placeholder is replaced + # in both scripts before writing; the allowlist in _PYTHON_DETECT strips any + # characters unsafe in a shell path, and import-verification catches a stale + # pinned path so it safely falls through to the dynamic detection. + pinned = sys.executable.replace("'", "") # strip single quotes (path injection guard) + hook = _HOOK_SCRIPT.replace("__PINNED_PYTHON__", pinned) + checkout = _CHECKOUT_SCRIPT.replace("__PINNED_PYTHON__", pinned) + + commit_msg = _install_hook(hooks_dir, "post-commit", hook, _HOOK_MARKER) + checkout_msg = _install_hook(hooks_dir, "post-checkout", checkout, _CHECKOUT_MARKER) return f"post-commit: {commit_msg}\npost-checkout: {checkout_msg}" diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 873b2028..a6e8b463 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -161,6 +161,42 @@ def test_hook_skips_head_on_exe(): assert "*.exe) _SHEBANG=" in _PYTHON_DETECT or '*.exe)' in _PYTHON_DETECT +def test_install_embeds_pinned_interpreter(tmp_path): + """Hook scripts must embed sys.executable so the hook works without the + graphify launcher on PATH (uv tool / pipx isolation, #1127). + + When graphify is installed via `uv tool install graphifyy` or `pipx install + graphifyy`, the interpreter lives in an isolated venv and the launcher is in + ~/.local/bin. GUI git clients and CI runners often run with a minimal PATH + that omits that directory, so `command -v graphify` fails, the python3/python + fallbacks cannot import graphify (wrong venv), and the hook silently exits 0. + Pinning sys.executable at install time makes the hook work regardless of PATH. + """ + import sys + repo = _make_git_repo(tmp_path) + install(repo) + commit_hook = (repo / ".git" / "hooks" / "post-commit").read_text() + checkout_hook = (repo / ".git" / "hooks" / "post-checkout").read_text() + # The pinned interpreter path must appear in both generated scripts. + assert sys.executable in commit_hook, "pinned sys.executable missing from post-commit" + assert sys.executable in checkout_hook, "pinned sys.executable missing from post-checkout" + # The placeholder must be fully substituted -- no __PINNED_PYTHON__ left. + assert "__PINNED_PYTHON__" not in commit_hook, "placeholder not substituted in post-commit" + assert "__PINNED_PYTHON__" not in checkout_hook, "placeholder not substituted in post-checkout" + + +def test_install_fallback_is_loud_not_silent(tmp_path): + """The detection fallback must emit a message to stderr rather than bare exit 0. + + A silent no-op (the pre-fix behaviour) leaves the user with no indication + that the hook ran but found nothing, making the bug extremely hard to diagnose. + """ + from graphify.hooks import _PYTHON_DETECT + assert "could not locate" in _PYTHON_DETECT, ( + "fallback branch must print a diagnostic message; bare 'exit 0' is silent and unhelpful" + ) + + def test_hook_check_no_additionalContext(tmp_path): """graphify hook-check must not emit additionalContext — Codex Desktop rejects it.""" import sys