Repository navigation
fix(aqua): preserve locked HTTP override metadata - #11499
Conversation
📝 WalkthroughWalkthroughAqua installation now fetches the registry package directly, derives install-time version candidates through a dedicated helper, and applies options with ChangesAqua install resolution
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
Greptile SummaryPreserves prefix-scoped Aqua metadata for locked HTTP artifacts while retaining exact-tag selection for GitHub release URLs.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (2): Last reviewed commit: "test(aqua): cover locked HTTP install op..." | Re-trigger Greptile |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/backend/aqua.rs (1)
3690-3714: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the install wiring, not only candidate computation.
This test exercises
install_package_version_candidatesandAquaPackage::with_versiondirectly, but notpackage_with_options_for_pkgfrominstall_version_. Add a focused boundary or locked-HTTP integration test asserting the resolvedversion_prefixandformat.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/aqua.rs` around lines 3690 - 3714, The existing test only verifies candidate generation and AquaPackage construction; add coverage through the install wiring entry point package_with_options_for_pkg in install_version_. Create a focused boundary or locked-HTTP integration test using the inherited prefix and format override, then assert that the resolved package options preserve version_prefix and format.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/backend/aqua.rs`:
- Around line 3690-3714: The existing test only verifies candidate generation
and AquaPackage construction; add coverage through the install wiring entry
point package_with_options_for_pkg in install_version_. Create a focused
boundary or locked-HTTP integration test using the inherited prefix and format
override, then assert that the resolved package options preserve version_prefix
and format.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: e3295d91-ce74-4f1c-b9a0-2ff99ad7231e
📒 Files selected for processing (1)
src/backend/aqua.rs
Instruction counts
No instruction-count regression above 1%. Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run. Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.
|
* Update to latest mise and backend plugins - Bump `min_version` to `2026.8.8`. - Unpin `mise-action` from `2026.7.16`, the aqua regression that forced the pin (jdx/mise#11499) is fixed upstream. - Switch the `[plugins]` prefix from `vfox:` to `vfox-backend:`, which is what current mise requires for backend plugins. * Pin mise-action to the newest mise version Keeps CI on an explicit version instead of floating on latest, matching `min_version` in `mise.toml`. * Bump
…4218) `mise lock codex --platform windows-x64` can now lock Codex 0.154.0 when the release-tag listing does not contain that pinned version. Previously, override selection received only `0.154.0` and `v0.154.0`, so the aqua registry's `rust-` prefix rejected every override. The empty asset name became `.exe`, and the Windows platform entry was skipped. It now resolves `codex-package-x86_64-pc-windows-msvc.tar.gz` and records its checksum. Reuse the prefix-aware candidate selection already used during installation before applying lock-time version overrides. Exact release tags remain authoritative, and overrides still use the requested target platform. When tag lookup misses and multiple prefix families match, warn and skip that platform entry instead of choosing a release by registry order. Disjoint version constraints that select one prefix remain supported. This extends the installation metadata fix in #11499 to lock generation. Validation: all 122 aqua backend unit tests and three focused lockfile end-to-end tests passed. The regression covers both the Codex-shaped Windows package and ambiguous prefixes with reachable archives; the ambiguity case fails against the previous PR binary. The real Codex 0.154.0 Windows lock reproduction succeeds with the expected archive and checksum. Strict workspace Clippy (`cargo clippy --workspace --all-features --all-targets -- -D warnings`) passed. Scoped hk checks passed; the full `mise run lint-fix` task could not finish because the local environment lacks `npm:markdownlint-cli@0.49.1`. *AI-assisted — Tool: Codex; model: openai/unavailable; version: unavailable.* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Aqua package resolution now applies version-specific overrides consistently for pinned releases and packages served from HTTP registries. * Lockfiles correctly record platform-specific archive URLs when registry versions use a prefix, including when a release tag is available. * Resolution reports an ambiguity error when multiple matching overrides point to different version prefixes, rather than selecting one unpredictably. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Root cause
Locked HTTP URLs such as Cabal's
downloads.haskell.orgartifacts do not contain a GitHub release tag, and mise skips tag discovery when a lockfile URL already exists. After prefix-aware Aqua override matching was introduced, install resolution still supplied only the bare version and itsvvariant. Cabal'scabal-install-voverride therefore did not match, itsformat: tar.xzmetadata was lost, and the archive was copied as the executable.Fixes the regression reported in #11494.
Impact
Locked Aqua HTTP installs retain prefix-scoped format and file metadata, so archives are extracted correctly. Packages with an exact release tag continue to use only that tag, preserving disambiguation between multiple prefix families.
Validation
cargo test backend::aqua::tests::test_package_version_candidates -- --nocapturecargo test backend::aqua::lock_candidate_tests:: -- --nocapturemise run lint-fixAI-assisted — Tool: Codex; model: unavailable/unavailable; version: unavailable.
Note
Medium Risk
Changes install-time Aqua package/override resolution for locked installs; wrong candidate logic could mis-apply formats or overrides, but scope is narrow and covered by new regression tests.
Overview
Fixes Aqua installs where a lockfile URL exists but carries no GitHub release tag (e.g. Cabal on
downloads.haskell.org). Install resolution no longer passes only the bare version and optionalvvariant into override matching; it usesinstall_package_version_candidates, which delegates topackage_version_candidatesso registryversion_prefixvalues (likecabal-install-v) still match and inherit metadata such asformat: tar.xz.When the lock URL does encode a GitHub release tag, resolution still uses only that tag, avoiding ambiguity between prefix families. The redundant async
package_with_optionshelper is removed in favor of loading the package once and callingpackage_with_options_for_pkgdirectly. A Cabal-style HTTP regression test asserts prefix and format are applied.Reviewed by Cursor Bugbot for commit 4708fe7. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit