Skip to content

fix(hooks): route local-preflight through uv run, not bare python3 - #1486

Merged
tvna merged 1 commit into
mainfrom
claude/pr-1485-prep-8mbkcd
Aug 30, 2026
Merged

fix(hooks): route local-preflight through uv run, not bare python3#1486
tvna merged 1 commit into
mainfrom
claude/pr-1485-prep-8mbkcd

Conversation

@tvna

@tvna tvna commented Aug 29, 2026

Copy link
Copy Markdown
Owner

Summary

The local-preflight pre-push hook invoked its runner with a bare system python3, but the runner imports _gitapex_schema_validation.py, which needs jsonschema -- a real, non-stdlib dependency. On a system whose bare python3 lacks jsonschema, the runner crashed on import before any of the 42 wired gates ran individually. This routes the hook's entry, and every doc/docstring example that shows how to run it standalone, through uv run --frozen python3 instead.

Facts

  • .pre-commit-config.yaml's local-preflight hook (language: system) invoked python3 .github/scripts/gitapex_gate_local_preflight.py directly.
  • That runner imports _gitapex_schema_validation.py (line 160), which does import jsonschema at module top level (line 54 of that file). jsonschema is a project dependency (pyproject.toml: jsonschema>=4.23), installed in the uv-managed .venv, not necessarily on the bare system interpreter.
  • Reproduced directly in this session: the bare system python3 here (3.11.15) lacks jsonschema and crashes the runner's own entry with ModuleNotFoundError: No module named 'jsonschema' (exit 0 from the shell wrapper in this environment's own outer script, but the Python process itself would exit non-zero on python3 <file>.py directly -- confirmed both ways during this session).
  • After the fix, running the actual pre-push hook end to end via prek run local-preflight --hook-stage pre-push reports all 42 wired gates individually (local preflight: all 42 wired gate(s) passed.), including the two gates with no CI backstop at all (behind-base, real-checkout-git-write).
  • CONTRIBUTING.md line 157 (pre-fix) carried the identical stale claim as the runner's own docstring ("The runner itself needs no dependencies, but all 42 wired gates run through uv"), and line 146 showed the same bare python3 .../gitapex_gate_local_preflight.py standalone-run example. Both updated alongside the docstring fix.
  • gitapex_gate_stdlib_only_claim_drift.py (this repo's own stale-claim CI gate) only fires on a diff that adds a new top-level third-party import to a .github/scripts/*.py/evals/scripts/*.py file; the jsonschema import here is pre-existing, so this defect predates that gate. Ran it against this PR's actual diff: OK: no stale stdlib-only/bare-python3 claims found for files gaining a third-party import.
  • No individual gate's local_invocation field changed (uv run --frozen python3 .github/scripts/gitapex_scan_ssot_schema.py .gitapex/ssot.json reports No ssot.json drift found.).

Assumptions

Risk / blast radius

Low. Scope is limited to how the pre-push hook and two docs invoke an existing script; no gate logic, registry entry, or local_invocation changed. Worst case if wrong: the pre-push hook fails to resolve uv, which is already a documented prerequisite for this repository and produces a loud FAIL ... failed to run rather than a silent gap.

Rollback

Revert this commit; .pre-commit-config.yaml's entry and the two doc/docstring examples return to invoking the runner with a bare python3 (restoring the pre-fix defect on any system whose bare python3 lacks jsonschema).

Verification

Acceptance Criteria Map (from issue #1485, re-verified against the current repository state -- see the issue's own re-verification comment):

Criterion Interpretation Planned ops Proof method Residual risk Result
The local-preflight hook starts and runs its wired gates even on a system whose python3 lacks jsonschema Route the hook's own entry through uv run instead of a bare system python3 Changed .pre-commit-config.yaml's local-preflight hook entry to uv run --frozen python3 .github/scripts/gitapex_gate_local_preflight.py With jsonschema absent from the system python3, ran the hook via prek run local-preflight --hook-stage pre-push Low Pass -- all 42 wired gates ran and reported individually
The runner's own module docstring accurately describes its real dependency shape Update the "the runner itself needs no dependencies" claim and the bare-python3 standalone-run examples Edited gitapex_gate_local_preflight.py's module docstring (3 locations: intro line, "Every wired gate runs through uv" paragraph, "Run standalone" line) Grepped the file for "needs no dependencies" / bare python3 .../gitapex_gate_local_preflight.py -- none remain Low Pass
None of the 40+ individually wired gates' own local_invocation changes Scope discipline No registry entry touched uv run --frozen python3 .github/scripts/gitapex_scan_ssot_schema.py .gitapex/ssot.json Low Pass -- No ssot.json drift found.
Whether CONTRIBUTING.md's own prose needs a matching update Resolved during re-verification: yes -- line 157 carried the identical stale claim, line 146 the identical bare-invocation example Edited both lines in CONTRIBUTING.md Grepped the file for the same stale phrases -- none remain Low Pass

Commands run and their output:

  • uv run --frozen python3 -m pytest tests/test_gitapex_gate_local_preflight.py -q -> 111 passed
  • uv run --frozen ruff format --check .github/scripts/gitapex_gate_local_preflight.py -> 1 file already formatted
  • uv run --frozen python3 .github/scripts/gitapex_run_precommit_mypy.py -> mypy: all groups clean.
  • uv run --frozen python3 .github/scripts/gitapex_scan_ssot_schema.py .gitapex/ssot.json -> No ssot.json drift found.
  • git diff -U0 -- '.github/scripts/*.py' 'evals/scripts/*.py' | uv run --frozen python3 .github/scripts/gitapex_gate_stdlib_only_claim_drift.py -> OK: no stale stdlib-only/bare-python3 claims found for files gaining a third-party import.
  • uv run --frozen prek run local-preflight --hook-stage pre-push -> local preflight: all 42 wired gate(s) passed. (this exact hook is what git push on this branch ran end to end)

Checklist

  • Tests pass locally
  • Docs updated if behavior changed (CONTRIBUTING.md)
  • Issue number cited in every commit
  • If this PR adds/modifies a skills/*/SKILL.md, a docs/superpowers/specs/*.md design doc, a security-relevant skill, or a deterministic checker script (skills/*/scripts/*.py, evals/scripts/*.py, .github/scripts/*.py), a ## Skill audit evidence section discloses the required verdicts/waivers (see .github/scripts/gitapex_gate_skill_audit_disclosure.py) -- see ## Skill audit evidence below
  • If this PR adds a new Kept-edit-log entry to any evals/*/split.md, that entry discloses a Transfer check line -- N/A, no such entry added
  • If this PR adds or increases a skills/*/SKILL.md's Stop-boundary bullets or named dispatch branches, evals/<skill>/tasks/*.yaml gained at least as many new fixtures -- N/A, no skill changed

Skill audit evidence

This PR modifies .github/scripts/gitapex_gate_local_preflight.py, a deterministic checker/gate script, so it is in scope for gitapex_gate_skill_audit_disclosure.py's three process-disclosure checks:

  • checker-script-adversarial-review: NOT-RUN -- the change is module-docstring prose only (3 locations: the intro line, the "Every wired gate runs through uv" paragraph, and the "Run standalone" example). No adversarial review round was run because no detection/execution logic (load_local_checks, run_check, run_checks, format_report, main) changed.
  • deterministic-gate-quality: WAIVED: this gate's own detection/execution logic is byte-for-byte unchanged (only module-docstring prose changed here, plus the pre-commit hook's own entry invocation shape in .pre-commit-config.yaml, from bare python3 to uv run --frozen python3), so re-reading it against skills/evaluating-deterministic-gate-quality/references/dimensions.md (dimension 15's fail-closed-on-malformed-input default included) would grade code this PR did not touch.
  • defeat-test-disclosure: NOT-RUN -- no detection logic changed for a defeat test to meaningfully target; the existing tests/test_gitapex_gate_local_preflight.py suite (111 tests) still passes unchanged.

Independent review verdict

  • Verdict: CLEAN
  • Verified commit: 7002c89

Outer layer (GitHub-native reviewer): did not produce a review; disclosed, not equated with a pass. Anthropic's "Claude Code Review" GitHub App is not confirmed installed on this repository, so request_copilot_review was called for copilot-pull-request-reviewer[bot] instead. It returned no output, and pull_request_read still reports zero reviews and zero review threads on this PR. Copilot's review is in any case Comment-only, with no pass/fail signal of its own -- materially weaker than the App's severity summary, not equivalent to it. Two third-party statuses on this PR's head commit are not this layer and are recorded only for completeness: Devin Review posted "Full review skipped: trial expired and no credits remaining"; CodeRabbit posted "Review skipped: manual review required for this OSS repository". Neither reviewed the diff.

Inner layer (reviewing-an-artifact, low effort): ran to completion; 0 confirmed findings, 1 disclosed non-blocking convention note. Step 0 found no specialist deferral (the changed gate script's own detection/execution logic is unchanged -- see Skill audit evidence above -- so evaluating-deterministic-gate-quality does not own this diff; no workflow file, no SKILL.md, no instruction channel, no secrets-scan request), no causal-diagnosis redirect (the defect was already reproduced and fixed before this review began, not merely reported), and the target in scope (three ordinary text/config files). Step 1 could not confidently classify the change as purely "safe" (it changes a pre-push hook's real invocation shape, not only prose), so it was treated as dangerous and the Step 2 fan-out ran.

Isolation disclosure, per that skill's own Stop boundary: the five persona passes DID run in fresh, isolated dispatches (separate subagents with no memory of this session's own authoring of the diff), each given the diff plus read access to the repository, with PR-description/commit-message narrative withheld per the redaction rule.

Per-axis results:

  • Correctness: 0 findings. Independently reran uv run prek run local-preflight --hook-stage pre-push --all-files; confirmed the new entry: argv splits and executes correctly, --frozen resolves cleanly against the committed uv.lock, and no stale "no dependencies"/bare-python3 claim remains anywhere in the two touched docs.
  • Blast-radius: 0 findings. Confirmed no test asserts the hook's entry string verbatim (only a substring/other-field check), no workflow or flake.nix separately invokes this script, uv is already a universal CI prerequisite (38 other workflows use astral-sh/setup-uv), and the "42 wired" prose counts still match the live registry.
  • Reuse-and-simplification: 0 findings above nitpick level. Noted the same root-cause explanation is now restated near-verbatim in three files, but that triplication is this file's own pre-existing style, not something this diff introduced; the chosen uv run --frozen prefix (matching sibling hooks) was confirmed to be the more reuse-respecting choice over an alternative language: python hook shape (which would introduce a second, pip-managed version source for jsonschema alongside uv.lock/flake.nix).
  • Security: 0 findings. entry: is parsed as a literal argv, not shell-interpreted, and carries no interpolation; --frozen pins against the existing, unchanged uv.lock (the safer, not weaker, reading of that flag) and was already used by every sibling hook in this same file; the commit touches only the three named files with no secret-shaped content.
  • Convention: 1 non-blocking note, independently verified. This PR's own commit trailer reads Fixes #1485, where CONTRIBUTING.md's "Issue citation convention" section names Closes #N as the convention for a fully-resolving PR. Confirmed both the commit trailer's literal text (git log) and the convention's literal wording (CONTRIBUTING.md lines 187-192). Fixes is still a real GitHub closing keyword (issue fix(hooks): local-preflight pre-push hook crashes when system python3 lacks jsonschema #1485's closed_by_pull_requests already lists this PR), so this is a documented-style deviation, not a functional defect. Not fixed here: correcting it would require amending an already-pushed, already-reviewed commit and force-pushing -- a disproportionate, hard-to-reverse action for a wording-only gap with no functional effect. Disclosed rather than silently dropped.

Audit trail: the commit-trailer wording note above (convention axis) is the only candidate raised across all five axes; it was not treated as a fix-worthy defect at the confidence-times-severity judgment (real deviation, but functionally inert and not worth a history rewrite), not rejected at the FABRICATED or verification stage.

Zero unconfirmed-concern findings: no security-tier signal is present anywhere in the target (no credential handling, no injection sink, no auth path, no network call, no subprocess argument built from untrusted input), and low effort produces no other speculative class.

Merge gate: independent review

This PR is also subject to the independent-review-pending required
status check (see .github/workflows/independent-review-pending.yml /
.github/scripts/gitapex_gate_independent_review_pending.py). It stays
pending/failing until a ## Independent review verdict section naming
this PR's current head commit is recorded in this body --
drafting-a-pr-to-merge's own Step 8 records it once its independent
review completes. There is nothing for you to do here now: do not
pre-fill this section yourself, and do not remove this note.

Related Issue

Closes #1485

The pre-push local-preflight hook invoked the runner with a bare system
python3, but the runner imports _gitapex_schema_validation.py, which
needs jsonschema -- a real, non-stdlib dependency. On any system whose
bare python3 lacks jsonschema, the runner crashed on import before any
of the 42 wired gates ran individually, silently losing the two gates
with no CI backstop (behind-base, real-checkout-git-write).

Route the hook entry, and the docs/docstring examples showing how to
run it standalone, through uv run --frozen python3 instead, matching
every individually wired gate own invocation shape already carries. No
individual gate local_invocation field changes.

Fixes #1485

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0193FSVTMtDafkWgjY3bet9k
@tvna
tvna deployed to ruleset-verify August 29, 2026 23:57 — with GitHub Actions Active
@coderabbitai

coderabbitai Bot commented Aug 29, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 21e86977-ea22-4b75-aa65-93e4ef2fddfd


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Aug 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.59%. Comparing base (c967270) to head (7002c89).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1486   +/-   ##
=======================================
  Coverage   99.59%   99.59%           
=======================================
  Files         133      133           
  Lines       23507    23507           
  Branches     2840     2840           
=======================================
  Hits        23411    23411           
  Misses         96       96           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tvna
tvna marked this pull request as draft August 30, 2026 00:18
@tvna
tvna marked this pull request as ready for review August 30, 2026 01:56
@tvna
tvna merged commit ff4f669 into main Aug 30, 2026
39 of 44 checks passed
@tvna
tvna deleted the claude/pr-1485-prep-8mbkcd branch August 30, 2026 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(hooks): local-preflight pre-push hook crashes when system python3 lacks jsonschema

2 participants