Repository navigation
fix: pin managed uv to reviewed 0.12.24 across install and self-update - #206
Merged
Merged
Conversation
Plan: ares-cicd-plan-20261026 Task 1.1 (OD-1), A-01/X-01. One pin (.github/ uv-version) governs every uv acquisition and update point: - scripts/install.sh install_uv(): versioned installer URL; managed uv outside [0.9.28, 0.13.0) is replaced with the pin instead of reused - install.sh (root): versioned installer URL - hermes_cli/managed_uv.py: uv self update targets the pin (was untargeted, drifting to unreviewed uv 0.13); out-of-range managed uv replaced with pin before any locked sync; versioned URLs for posix/windows installers - scripts/install_uv_policy.py: UV_MIN_VERSION/UV_MAX_EXCLUSIVE exposed as the machine-readable reviewed range - new .github/actions/setup-uv-pinned composite action (default = pin) Tests: 12 new (consistency incl. cross-surface pin agreement + live URL check, installer behavior with fake curl/uv under bash, self-update argv contract); legacy self-update contract updated to the pinned-target form. The 2 test_install_uv_policy network probes fail identically on pristine main (pre-existing, environment-dependent) and are out of scope here. RED was verified: all 12 failed against unpatched source before GREEN.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
૮ >ﻌ< ა ci reviewran on 3d20f21 — Merge branch 'main' into fix/uv-pin-reviewed-20261026
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b63328217
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Owner
Author
|
Review summary (Ares CI/CD plan execution, Task 1.1): Diff surface: 3 modified install surfaces ( |
RecursiveIntell
added
the
ci-reviewed
Independent verifier findings on PR #206, all addressed: - install.ps1 astral + GitHub-release URLs pinned (Windows leg was still unversioned); setup-hermes.sh and nix module and README/FAQ one-liners pinned - consistency test is now a REPO-WIDE git-tracked scan (no fixed file list); vendored upstream reference docs excluded - URL_RX fixed (was over-escaped); versioned-URL match groups asserted - fake curl is URL-sensitive: an unversioned/wrong URL fails the bash test - failed or ineffective out-of-range reinstall now FAILS CLOSED: update returns None (callers refuse the sync) instead of returning the stale binary; two new tests cover both failure modes - network URL probe skips (not fails) on offline hosts - ps1 contract tests updated to the pinned-URL shapes
Round-2 verifier HIGH: update_cmd.py discarded update_managed_uv() return and called ensure_uv() anyway, so an unrepairable out-of-range uv still reached the dependency sync (ensure_uv resolves by existence, not range). Both call sites now refuse the sync (exit 1) when update_managed_uv() returns None while a managed binary exists; source-contract test locks both guards in place.
Round-3 verifier HIGHs: (1) the no-new-commits retry path in update_cmd.py
called update_managed_uv() and discarded None; (2) ensure_uv() resolved by
existence alone, so any caller reaching a sync via ensure_uv() without
update_managed_uv() was unguarded.
- ensure_uv()/_ensure_uv_path now applies the reviewed-range admission
itself: out-of-range or unreadable managed uv is replaced with the pin
before use; failure returns falsy (None / _UvResult("")), which all
callers already treat as "no usable uv"
- the third call site guards the None return like the other two
- source-contract test now requires the guard at all 3 call sites;
2 new ensure_uv admission tests
Round-4 verifier findings: (HIGH) tools/lazy_deps.py resolved the managed uv by existence and ran uv pip install on it — bypassing the admission rule on a sibling path; (medium) _ensure_uv_for_termux same; (medium) the inspect.getsource contract test violated the no-source-reading-tests rule. New managed_uv.resolve_uv_admitted(): pure, side-effect-free lookup that returns the managed path only when in range, falls back to PATH only when the managed binary is ABSENT (never when present-but-unadmitted). lazy_deps and the termux helper use it. The source-contract test is replaced by behavioral tests (admission semantics + lazy_deps routes the pip tier when the managed uv is out of range). 61 tests pass; ruff + footguns clean.
Round-4 public HIGH: _ensure_uv_path returned a fresh install on existence alone — an installer delivering a wrong/unreadable version reached syncs. The fresh path now validates the installed version against the reviewed range and fails closed (falsy) otherwise; behavioral test added. Legacy fake fixtures updated from uv 0.1.2 to an in-range 0.12.10 so the admission check admits them (the tests assert repair/observer behavior, not the pin).
Round-5 verifier HIGH: _ensure_uv_for_termux returned an unchecked PATH uv when the managed uv was present-but-unadmitted, contradicting the admission contract. Every candidate in the Termux helper now passes the reviewed-range check before reaching the sync call sites.
Round-6 verifier HIGH: with the managed uv absent, resolve_uv_admitted returned shutil.which(uv) unchecked, so an out-of-range PATH uv reached the Termux helper and lazy_deps. The PATH fallback now passes the same reviewed-range check; the gap-encoding test is replaced by a behavioral one (out-of-range PATH uv suppressed, in-range returned).
Round-7 verifier findings (5): browser_use_cli fell through to an unchecked PATH uv; install.ps1 adopted managed/PATH uvs on existence; setup-hermes.sh reused PATH/cargo uv unchecked; _early_recovery _find_uv_binary fed the recovery install with the first existing candidate; install_psutil_android and google-workspace setup ran which(uv) unchecked. All now require a version inside [0.9.28, 0.13.0) before the binary may run pip/sync/tool-install; out-of-range or unreadable binaries are refused (browser-use errors with repair instructions; recovery skips to the next candidate; scripts fall back to stdlib pip).
Round-9 findings: the nix module ran uv python install/venv --seed with an existence-only check (HIGH); lazy_deps fell back to a bare which(uv) if the admitted lookup raised. The nix gate version-checks before running; lazy_deps routes lookup failures to the pip tier. Body updated (nix added to the surface list; Dockerfile 0.11.6 image noted as in-range but not the pin and outside the consistency test; explicit user overrides qualified).
My round-7 admission probe in install_psutil_android.py used text=True without encoding, tripping the windows-footguns gate and its full-repo scan test. Patched; full repo scan clean.
Round-11 verifier reproduced: the regex patch doubled the encoding arg in install_psutil_android.py, breaking compile and the update/Android path. Deduplicated; footguns scan clean; pin-scope tests pass.
- lazy_deps console-window test: stub resolve_uv_admitted; one capturing fake_run answers --version probes and the install call (fixes monkeypatch self-clobber where the probe lambda replaced the capturer) - cmd_update Termux bootstrap + browser_use fixtures: version-aware fakes reporting uv 0.12.24 (in range) - local pre-existing test_cmd_update failures verified unchanged on the committed head (13 there, 12 with this diff; hosted CI green)
resolve_uv_admitted(fallback=True) is the one sanctioned bare which(uv) PATH rung (version-gated before use); the lazy_deps/browser_use sites it replaced drop their now-orphaned allowlist entries per the test's own drop-on-fix protocol. Locally: tests/test_managed_runtime_resolution.py 7 passed.
The uv tier now emits [uv, --version] before the install call (Task 1.1 admission contract); the mocked-ladder tests answer the probe in-range and assert probe-then-install.
Five fixtures drive tools.lazy_deps' real admission path (no resolve_uv_admitted stub): out-of-range and unreadable managed uv are probed but never used; PATH uv out-of-range likewise; absent uv means no probe at all; in-range uv (control) is probed then used. Closes the round-13 public finding that migrated fixtures only pinned the admitted path.
- tests/test_uv_pin_consistency.py rewritten without source-regex checks (banned by root AGENTS.md): runtime admission assertions through the real resolver, installer-shell command capture via recording shim, composite-action YAML semantics, live URL resolution with redirect follow, admission argv contract. 7/7 pass. - setup-uv-pinned action: prepend $HOME/.local/bin to the step's PATH and register it unconditionally on GITHUB_PATH (codex P2: conditional GITHUB_PATH only affects later steps; a pre-existing uv would win)
This was referenced Oct 28, 2026
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.
Problem
scripts/install.sh, rootinstall.sh,scripts/install.ps1(astral and GitHub-releases fallback),setup-hermes.sh,nix/nixosModules.nix, README/FAQ one-liners.update_managed_uv()ran untargeteduv self update→ the managed uv drifted to unreviewed releases on the next update, re-breaking installs.Fix
One reviewed pin — uv 0.12.24 — declared in
.github/uv-versionand embedded in every acquisition surface above plushermes_cli/managed_uv.pyand the newsetup-uv-pinnedcomposite action:update_managed_uv()runs targeteduv self update 0.12.24(never drifts).[0.9.28, 0.13.0)is reinstalled at the pin before any dependency sync. If the reinstall fails — or the reinstalled binary still reports out-of-range — the updater fails closed (returns no usable uv; callers refuse uv use or take their documented fallback, e.g. lazy dependency installs deliberately fall back to pip) rather than proceeding on a stale binary.scripts/install_uv_policy.pyexposes the reviewed range as constants.Enforcement is repo-wide, not a fixed file list:
tests/test_uv_pin_consistency.pyscans every git-tracked file for uv installer URLs (astral.sh and the GitHub-releases mirror) and requires the pin; cross-surface pin agreement and the policy-range containment are consistency-tested. The bash behavior test uses a URL-sensitive fake curl — an unversioned or wrong-version URL fails the test. (Excluded from the scan: vendored third-party reference docs, e.g. unsloth/axolotl upstream manuals.)Tests
69 tests in pin scope pass on Python 3.14 (
uv run --python 3.14 --with pytest,pyyaml python -m pytest tests/test_uv_pin_consistency.py tests/test_install_uv_pin.py tests/hermes_cli/test_managed_uv_pin.py tests/hermes_cli/test_managed_uv.py tests/test_install_ps1_uv_install_fallback.py tests/test_install_ps1_uv_powershell_host.py -q); 2 pre-existing network-dependent failures intests/test_install_uv_policy.pyfail identically on pristinemainand are out of scope. RED was verified before GREEN on the new tests. The URL-resolves probe skips (not fails) on hosts without network access.Notes & limits
0.9.28literal until next edit; the Ares-owned-workflow consistency check covers onlyares-*.yml..github/uv-versionis the declared single source; today it is checked against embedded pins by tests (no runtime file loading).Rollback
Revert this PR. Out-of-range installs are repaired on their next successful install or update.