From fc6fe5e5df759f02a1aa1644a2786e10731e0037 Mon Sep 17 00:00:00 2001 From: Juan Garibay Date: Thu, 17 Sep 2026 15:06:49 -0400 Subject: [PATCH] fix(hooks): pre-push skipped every Python project that uses a virtualenv The Python block gates on `command -v pytest`, so it only runs when pytest is on PATH. Installing a project's tools into a virtualenv is the norm rather than the exception, so in practice the hook printed [ECC pre-push] Python project detected but pytest is not installed. Skipping. while standing in a directory with `.venv/bin/pytest` in it, and pushed. The failure mode is worse than not having the hook. A skip line reads like a pass: the push succeeds, the output looks healthy, and nothing indicates the gate declined to gate. A repository can sit behind it for months believing its tests run on every push. Found on a project with 893 tests, none of which the hook had ever executed. `resolve_pytest` now looks, in order, at `ECC_PYTEST_CMD`, `$VIRTUAL_ENV`, `.venv`, `venv`, `env`, `uv run` when a `uv.lock` is present, `poetry run` when a `poetry.lock` is, and finally PATH. Each candidate is confirmed by importing pytest rather than by the path existing, so a half-built venv falls through to the next one instead of failing the push. Two deliberate choices: The log line names the command it resolved -- `Running: .venv/bin/python -m pytest -q` -- so which interpreter ran is visible in the push output rather than inferred. When nothing resolves, the message says where it looked and names `ECC_PYTEST_CMD`, instead of asserting pytest is not installed when it may well be. `uv run` passes `--no-sync` so the hook cannot mutate the developer's environment on its way to running the tests. Behaviour change worth flagging for the release note: on any Python project with a working virtualenv this hook now actually runs the suite, and will block a push whose tests fail. That is the intent, but it is new behaviour for every such repository, and `ECC_SKIP_PREPUSH=1` remains the escape. Verified on two real repositories: a uv/venv Python project (resolves `.venv/bin/python -m pytest`, 893 tests, exits 0; exits 1 when the suite fails) and a Node project (unchanged, still runs lint/typecheck/test/build). --- scripts/codex-git-hooks/pre-push | 53 +++++++++++++++++++++++++++++--- 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/scripts/codex-git-hooks/pre-push b/scripts/codex-git-hooks/pre-push index 2ee23c7f4..472c2f194 100755 --- a/scripts/codex-git-hooks/pre-push +++ b/scripts/codex-git-hooks/pre-push @@ -117,16 +117,61 @@ if [[ -f "go.mod" ]] && command -v go >/dev/null 2>&1; then go test ./... || fail "go test failed" fi -if [[ -f "pyproject.toml" || -f "requirements.txt" ]]; then +# Resolve how this project runs pytest. +# +# Looking only for `pytest` on PATH meant the hook skipped every project that keeps +# its tools in a virtualenv -- which is most of them -- and reported "pytest is not +# installed" while sitting next to a .venv with pytest in it. A gate that silently +# declines to gate is worse than no gate, because the skip line reads like a pass. +# +# Echoes the command it will run, so the reason for a skip is always visible. +resolve_pytest() { + if [[ -n "${ECC_PYTEST_CMD:-}" ]]; then + echo "$ECC_PYTEST_CMD" + return 0 + fi + local venv + for venv in "${VIRTUAL_ENV:-}" .venv venv env; do + if [[ -n "$venv" && -x "$venv/bin/python" ]]; then + if "$venv/bin/python" -c "import pytest" >/dev/null 2>&1; then + echo "$venv/bin/python -m pytest" + return 0 + fi + fi + done + if [[ -f "uv.lock" ]] && command -v uv >/dev/null 2>&1; then + if uv run --no-sync python -c "import pytest" >/dev/null 2>&1; then + echo "uv run --no-sync pytest" + return 0 + fi + fi + if [[ -f "poetry.lock" ]] && command -v poetry >/dev/null 2>&1; then + if poetry run python -c "import pytest" >/dev/null 2>&1; then + echo "poetry run pytest" + return 0 + fi + fi if command -v pytest >/dev/null 2>&1; then + echo "pytest" + return 0 + fi + return 1 +} + +if [[ -f "pyproject.toml" || -f "requirements.txt" ]]; then + if pytest_cmd="$(resolve_pytest)"; then ran_any_check=1 - log "Python project detected. Running: pytest -q" - pytest -q || fail "pytest failed" + log "Python project detected. Running: $pytest_cmd -q" + # Unquoted on purpose: the resolver returns a command with arguments. + # shellcheck disable=SC2086 + $pytest_cmd -q || fail "pytest failed" else - log "Python project detected but pytest is not installed. Skipping." + log "Python project detected but no pytest found (checked \$VIRTUAL_ENV, .venv," + log " venv, env, uv, poetry, PATH). Set ECC_PYTEST_CMD to point at it." fi fi + if [[ "$ran_any_check" -eq 0 ]]; then log "No supported checks found in this repository. Skipping." else