fix(hooks): route local-preflight through uv run, not bare python3 - #1486
Merged
Conversation
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
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
tvna
marked this pull request as draft
August 30, 2026 00:18
tvna
marked this pull request as ready for review
August 30, 2026 01:56
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
local-preflightpre-push hook invoked its runner with a bare systempython3, but the runner imports_gitapex_schema_validation.py, which needsjsonschema-- a real, non-stdlib dependency. On a system whose barepython3lacksjsonschema, 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, throughuv run --frozen python3instead.Facts
.pre-commit-config.yaml'slocal-preflighthook (language: system) invokedpython3 .github/scripts/gitapex_gate_local_preflight.pydirectly._gitapex_schema_validation.py(line 160), which doesimport jsonschemaat module top level (line 54 of that file).jsonschemais a project dependency (pyproject.toml:jsonschema>=4.23), installed in theuv-managed.venv, not necessarily on the bare system interpreter.python3here (3.11.15) lacksjsonschemaand crashes the runner's own entry withModuleNotFoundError: 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 onpython3 <file>.pydirectly -- confirmed both ways during this session).prek run local-preflight --hook-stage pre-pushreports 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.mdline 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 throughuv"), and line 146 showed the same barepython3 .../gitapex_gate_local_preflight.pystandalone-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/*.pyfile; thejsonschemaimport 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.local_invocationfield changed (uv run --frozen python3 .github/scripts/gitapex_scan_ssot_schema.py .gitapex/ssot.jsonreportsNo ssot.json drift found.).Assumptions
local_invocationchanged (issue fix(hooks): local-preflight pre-push hook crashes when system python3 lacks jsonschema #1485's own Constraints/Non-goals section states this explicitly, and Facts confirm it via the schema-drift scan).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_invocationchanged. Worst case if wrong: the pre-push hook fails to resolveuv, which is already a documented prerequisite for this repository and produces a loudFAIL ... failed to runrather than a silent gap.Rollback
Revert this commit;
.pre-commit-config.yaml'sentryand the two doc/docstring examples return to invoking the runner with a barepython3(restoring the pre-fix defect on any system whose barepython3lacksjsonschema).Verification
Acceptance Criteria Map (from issue #1485, re-verified against the current repository state -- see the issue's own re-verification comment):
local-preflighthook starts and runs its wired gates even on a system whosepython3lacksjsonschemauv runinstead of a bare systempython3.pre-commit-config.yaml'slocal-preflighthookentrytouv run --frozen python3 .github/scripts/gitapex_gate_local_preflight.pyjsonschemaabsent from the systempython3, ran the hook viaprek run local-preflight --hook-stage pre-pushpython3standalone-run examplesgitapex_gate_local_preflight.py's module docstring (3 locations: intro line, "Every wired gate runs through uv" paragraph, "Run standalone" line)python3 .../gitapex_gate_local_preflight.py-- none remainlocal_invocationchangesuv run --frozen python3 .github/scripts/gitapex_scan_ssot_schema.py .gitapex/ssot.jsonNo ssot.json drift found.CONTRIBUTING.md's own prose needs a matching updateCONTRIBUTING.mdCommands run and their output:
uv run --frozen python3 -m pytest tests/test_gitapex_gate_local_preflight.py -q->111 passeduv run --frozen ruff format --check .github/scripts/gitapex_gate_local_preflight.py->1 file already formatteduv 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 whatgit pushon this branch ran end to end)Checklist
CONTRIBUTING.md)skills/*/SKILL.md, adocs/superpowers/specs/*.mddesign doc, a security-relevant skill, or a deterministic checker script (skills/*/scripts/*.py,evals/scripts/*.py,.github/scripts/*.py), a## Skill audit evidencesection discloses the required verdicts/waivers (see.github/scripts/gitapex_gate_skill_audit_disclosure.py) -- see## Skill audit evidencebelowevals/*/split.md, that entry discloses a Transfer check line -- N/A, no such entry addedskills/*/SKILL.md's Stop-boundary bullets or named dispatch branches,evals/<skill>/tasks/*.yamlgained at least as many new fixtures -- N/A, no skill changedSkill audit evidence
This PR modifies
.github/scripts/gitapex_gate_local_preflight.py, a deterministic checker/gate script, so it is in scope forgitapex_gate_skill_audit_disclosure.py's three process-disclosure checks:load_local_checks,run_check,run_checks,format_report,main) changed.entryinvocation shape in.pre-commit-config.yaml, from barepython3touv run --frozen python3), so re-reading it againstskills/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.tests/test_gitapex_gate_local_preflight.pysuite (111 tests) still passes unchanged.Independent review verdict
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_reviewwas called forcopilot-pull-request-reviewer[bot]instead. It returned no output, andpull_request_readstill 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 Reviewposted "Full review skipped: trial expired and no credits remaining";CodeRabbitposted "Review skipped: manual review required for this OSS repository". Neither reviewed the diff.Inner layer (
reviewing-an-artifact,loweffort): 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 -- soevaluating-deterministic-gate-qualitydoes 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:
uv run prek run local-preflight --hook-stage pre-push --all-files; confirmed the newentry:argv splits and executes correctly,--frozenresolves cleanly against the committeduv.lock, and no stale "no dependencies"/bare-python3claim remains anywhere in the two touched docs.entrystring verbatim (only a substring/other-field check), no workflow orflake.nixseparately invokes this script,uvis already a universal CI prerequisite (38 other workflows useastral-sh/setup-uv), and the "42 wired" prose counts still match the live registry.uv run --frozenprefix (matching sibling hooks) was confirmed to be the more reuse-respecting choice over an alternativelanguage: pythonhook shape (which would introduce a second, pip-managed version source forjsonschemaalongsideuv.lock/flake.nix).entry:is parsed as a literal argv, not shell-interpreted, and carries no interpolation;--frozenpins against the existing, unchangeduv.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.Fixes #1485, whereCONTRIBUTING.md's "Issue citation convention" section namesCloses #Nas the convention for a fully-resolving PR. Confirmed both the commit trailer's literal text (git log) and the convention's literal wording (CONTRIBUTING.mdlines 187-192).Fixesis still a real GitHub closing keyword (issue fix(hooks): local-preflight pre-push hook crashes when system python3 lacks jsonschema #1485'sclosed_by_pull_requestsalready 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-concernfindings: 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), andloweffort produces no other speculative class.Merge gate: independent review
This PR is also subject to the
independent-review-pendingrequiredstatus check (see
.github/workflows/independent-review-pending.yml/.github/scripts/gitapex_gate_independent_review_pending.py). It stayspending/failing until a
## Independent review verdictsection namingthis PR's current head commit is recorded in this body --
drafting-a-pr-to-merge's own Step 8 records it once its independentreview 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