Probe for the Interpreter in the Order the Spec Declares, and Enforce Its Floor - #1167
Conversation
… Its Floor Both hooks said the interpreter is chosen "by running the probes spec/host-tools.json declares, in its order", then probed python3 before py -3 while the spec declares py -3 first. The spec's order is deliberate and carries its reason: a bare python3 is reached through PATH, so on Windows an activated virtual environment or an interpreter from MSYS2, Cygwin or Scoop answers ahead of the managed one and would be graded in its place, while the py launcher reaches a registered interpreter whatever is active and exists on Windows alone. The hooks now probe in that order and say why, and the pre-commit comment loses the claim that Windows registers no python3, which is no longer true. The pre-push hook also enforces the 3.13 floor the spec declares rather than accepting any interpreter whose --version runs. The engines import datetime.UTC at module level, so an older interpreter fails at import and exits 1, which reads as a gate refusal rather than as the gate never running, the one distinction the refusal table exists to keep. An unparseable version refuses for the same reason. The comparison is sort -V rather than a string test, checked across the cases that separate the two: 3.13.5, 3.13.0, 3.13, 3.14.1 and 4.0 pass, 3.12.9, 3.9.6 and 3.2 refuse, and an empty or unrecognized line refuses.
PR Summary by QodoAlign Python Probes and Enforce the Pre-Push Version Floor
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 Changes recommended
The new version-floor comparison uses sort -V, which is not portable on macOS/BSD and can cause incorrect refusals.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Aligns the Husky hooks’ Python interpreter probing with spec/host-tools.json (try py -3 before python3) and adds an explicit Python version-floor refusal in pre-push so an interpreter that is too old is reported as “gate did not run” rather than failing inside the engines.
Changes:
- Reordered interpreter probing in
.husky/pre-pushand.husky/pre-committo match the spec-declared probe order. - Added a Python version-floor check to
.husky/pre-pushwith clearer refusal messaging when the gate cannot run.
File summaries
| File | Description |
|---|---|
| .husky/pre-push | Probe py -3 before python3 and enforce the Python version floor before running review engines. |
| .husky/pre-commit | Probe py -3 before python3 and update comments/messages to match the spec rationale. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The floor check compared version strings through sort -V piped into head, which carried two defects the reviewers caught in my own new code. sort -V is a GNU spelling. This repository uses it in host-setup/linux scripts alone, and macOS is a first-class platform here, so a hook is the wrong place to depend on it. A failed pipeline would also have yielded no matching floor, which takes the below-floor branch and refuses a push on a host that satisfies the floor. Piping sort into head -n 1 under set -Eeuo pipefail is the early-reader pattern the shell rules name. Measured here: harmless at two lines, and exit 141 once the producer cannot finish writing before the reader leaves. Both go away by asking the interpreter about itself, which needs no text comparison and no pipeline. Verified across the version_info values that separate the two arms: 3.13, 3.13.0, 3.13.5, 3.14.1 and 4.0.0 pass, and 3.12.9, 3.9.6, 3.2.0 and 2.7.18 refuse while still reporting the version they found. The probe comments come down to the one line each that carries a reason the code does not.
There was a problem hiding this comment.
🟡 Changes recommended
The pre-push hook hardcodes the Python floor instead of reading it from spec/host-tools.json, risking spec drift and reintroducing the same class of mismatch this PR is fixing.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
The floor was a literal in the hook, which is the shape that produced the defect this branch exists to fix: the probe order lived in two places, the spec moved, and the hook did not. A floor written twice drifts the same way. The hook reads spec/host-tools.json for the python3 minimum with the interpreter it just selected. A spec it cannot read refuses the push and says so, rather than falling through to an unchecked interpreter, since a floor that cannot be read is the gate not running rather than a version verdict.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe pre-commit and pre-push hooks now prefer ChangesPython hook validation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The hooks now follow the declared interpreter probe order and enforce the minimum supported Python version; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Title checkExplanation The title 'Probe for the Interpreter in the Order the Spec Declares, and Enforce Its Floor' directly reflects the main changes in the pull request. The changeset updates both Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.) ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🟢 Approval recommended
The hook changes match the spec’s declared probe order and correctly enforce the spec floor using a portable version check without introducing new failure modes in the edited regions.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
Unblocks the
develop -> mainpromotion #1163, whose last two open threads are against.husky/pre-pushfrom #1166.What was wrong
Both hooks said the interpreter is chosen "by running the probes
spec/host-tools.jsondeclares, in its order", then probedpython3beforepy -3. The spec declares the opposite:#1166 changed that order deliberately and wrote the reason into the spec's own
why: a barepython3is reached through PATH, so on Windows an activated virtual environment or an interpreter from MSYS2, Cygwin or Scoop answers ahead of the managed one and would be graded in its place, while thepylauncher reaches a registered interpreter whatever is active and exists on Windows alone. It changed the spec and left both hooks behind, and the hooks' own comments then asserted an order they did not follow.Two reviewers found this independently on #1163, one of them twice.
The pre-commit comment also still said native Windows "registers
pyand notpython3", which #1166 made untrue:host-setup/windows/install-tools.ps1now supplies a realpython3there.Separately, neither hook enforced the
3.13floor the same spec entry declares. The engines importdatetime.UTCat module level, so an older interpreter fails at import and exits 1, which reads as a gate refusal rather than as the gate never running. That is the one distinctionlocal-strict-review's refusal table exists to keep, and it was silently collapsed.What changed
Both hooks probe in the spec's order and state why. The pre-push hook reads the version out of the probe it already ran and refuses below the floor, naming the interpreter and the version it found, and refuses an unparseable version for the same reason.
Verification
The comparison is
sort -Vrather than a string test, since3.9.6sorts above3.13as a string. Checked across the cases that separate the two:Python 3.13.5,3.13.0,3.13,3.14.1,4.0Python 3.12.9,3.9.6,3.2bash: py: command not foundThe push that opened this pull request ran through the modified hook itself, on a host where
py -3is absent, so the fallback topython3and the floor check are exercised live.shellcheckandshfmtpass on both files, as doprose_lint,spec/validate.py,host_gate.pyand the full test suite.Note for #1161
The probe order here follows
spec/host-tools.jsonbecause that is the declared ground truth and the hooks were the stragglers. If #1161 landspython3everywhere on Windows and decides the order should flip, flipping the spec and these six lines together is the whole change. The floor check is independent of that question.Summary by CodeRabbit
py -3launcher withpython3as a fallback.