Skip to content

perf(aqua): skip public probes for private releases - #10914

Merged
jdx merged 2 commits into
jdx:mainfrom
risu729:agent/aqua-private-release-routing
Jul 12, 2026
Merged

jdx merged 2 commits into
jdx:mainfrom
risu729:agent/aqua-private-release-routing

Conversation

@risu729

@risu729 risu729 commented Jul 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • preserve aqua's private package field in YAML and compiled registry data
  • use the authenticated GitHub API asset URL immediately for declared-private release assets
  • retain browser-URL probing and API fallback when private is absent or false
  • bump the compiled aqua registry cache version from v5 to v6

Why

Aqua registries can mark GitHub-hosted packages as private. Mise previously discarded that field, so private release downloads always made a browser-URL probe that is known to fail before selecting the authenticated API asset endpoint.

This keeps the fallback behavior for registries with missing or incorrect metadata while avoiding the unnecessary request for correctly declared private packages. The same selection applies to auxiliary release assets such as checksums, signatures, and provenance.

Validation

  • cargo fmt --all -- --check
  • cargo test -p aqua-registry private
  • cargo test -p aqua-registry test_rkyv_package_roundtrip_preserves_var_default
  • cargo test test_private_github_release_uses_api_url_without_probe
  • cargo test test_github_release_without_api_url_uses_browser_url

The full cargo test -p aqua-registry run passed 107 tests and hit one pre-existing platform-dependent failure in test_override_envs_all_matches_any_platform (.exe expectation). The repository-wide mise run format task could not complete because ajv is unavailable on PATH.

Summary by CodeRabbit

  • New Features
    • Registry packages can now be marked as private (with a default of public when omitted).
    • Private GitHub-hosted releases use direct authenticated API download URLs for package artifacts and related verification assets.
  • Bug Fixes
    • Improved GitHub release asset URL resolution for private packages, avoiding reachability probing when an API URL is available.
    • Preserved package privacy settings during serialization/deserialization and version overrides.
    • Updated compiled registry cache version to refresh stored cache data.

@coderabbitai

coderabbitai Bot commented Jul 10, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

AquaPackage now stores private-package metadata through YAML and rkyv serialization. GitHub release downloads use API URLs directly for private packages across installation, SLSA checks, and auxiliary asset retrieval, while public-package behavior remains reachability-based.

Changes

Aqua private package downloads

Layer / File(s) Summary
Registry privacy contract and cache version
crates/aqua-registry/src/types.rs, crates/aqua-registry/src/codec.rs, crates/aqua-registry/src/cache.rs
AquaPackage.private defaults to false, is preserved in YAML and rkyv serialization, and increments the compiled registry cache version.
Private-aware GitHub URL selection
src/backend/aqua.rs
A centralized helper selects API URLs for private packages, probes public package URLs when applicable, and falls back to browser URLs without API URLs.
Aqua release download integrations
src/backend/aqua.rs
Version installation, SLSA provenance checks, and auxiliary GitHub release assets use the centralized selection helper.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant AquaPackage
  participant AquaBackend
  participant GitHubRelease
  AquaPackage->>AquaBackend: provide private metadata and release URLs
  AquaBackend->>GitHubRelease: select API URL directly for private packages
  GitHubRelease-->>AquaBackend: download artifact or provenance asset
Loading

Possibly related PRs

  • jdx/mise#10622: Also changes GitHub release asset URL resolution in src/backend/aqua.rs.
  • jdx/mise#10167: Also bumps the compiled registry cache version.
  • jdx/mise#10473: Also bumps the compiled registry cache version to invalidate existing caches.

Poem

A bunny found a private release,
And hopped past probes with ease.
The API path was bright and clear,
For every asset far and near.
YAML and caches kept it right—
Then downloads bounded out of sight!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: private Aqua releases skip browser/probe URL selection in favor of the API URL.

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.

@greptile-apps

greptile-apps Bot commented Jul 10, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a private field to AquaPackage so that GitHub-hosted packages marked private in the aqua registry skip the browser-URL reachability probe and use the authenticated API asset URL directly. The cache version is bumped to v6 to invalidate existing compiled registry data that lacks the new field.

  • types.rs: adds private: bool to AquaPackage with YAML deserialization and a false default, plus three new tests.
  • aqua.rs: extracts select_github_release_download_url and applies it uniformly to main asset, SLSA provenance, and auxiliary-asset downloads, short-circuiting to the API URL when private is true.
  • cache.rs / codec.rs: version bump and rkyv round-trip test extended to cover the new field.

Confidence Score: 4/5

Safe to merge for the common case; one edge case in apply_override means packages declaring private: true only inside a version_overrides block will silently lose that flag and still probe the browser URL.

The main optimization path and all three call sites in aqua.rs are correct. The gap is in apply_override in types.rs: no_asset (an identically-typed bool) is explicitly propagated but private is not. A registry package that sets private: true exclusively inside a version_overrides block would have the flag dropped after override application, causing the unnecessary browser-URL probe to fire anyway for those versions.

crates/aqua-registry/src/types.rs — the apply_override function needs a if avo.private { orig.private = true; } guard, mirroring the existing no_asset handling at line 1087.

Important Files Changed

Filename Overview
crates/aqua-registry/src/types.rs Adds private: bool field to AquaPackage with correct YAML deserialization and default, but apply_override does not propagate private from a version override to the base package — meaning packages that declare private: true only inside a version_overrides block will silently lose that flag
crates/aqua-registry/src/cache.rs Bumps compiled registry cache version from v5 to v6 to invalidate existing caches after the private field was added to the serialized schema — correct and necessary
crates/aqua-registry/src/codec.rs Extends the rkyv round-trip test to set and verify private: true, confirming the field survives serialization/deserialization
src/backend/aqua.rs Extracts a select_github_release_download_url helper that skips the browser-URL probe when pkg.private is true and an API URL is available; applied consistently to main asset, SLSA provenance, and auxiliary asset download sites

Comments Outside Diff (1)

  1. crates/aqua-registry/src/types.rs, line 1087-1089 (link)

    P1 private not propagated in apply_override

    The no_asset bool is propagated at line 1087 with the pattern if avo.no_asset { orig.no_asset = true; }, but the identically-typed private field added by this PR is absent from apply_override. If an aqua registry entry declares private: true inside a version_overrides block (rather than at the top level), apply_override is called with that override as avo — and because avo.private is true but there is no corresponding if avo.private { orig.private = true; } guard, the value is silently dropped. The browser-URL probe will still fire for those versions, defeating the optimization.

    The existing test test_registry_package_private_survives_version_override only covers the case where private: true is on the top-level package (so orig.private starts as true and is never overwritten), not the inverse case where only the override carries private: true.

Reviews (2): Last reviewed commit: "test(aqua): cover private override inher..." | Re-trigger Greptile

@risu729

risu729 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the Greptile concern with a regression test confirming that top-level private: true survives version override application.

I intentionally did not merge private from the selected version_overrides entry: upstream aqua’s VersionOverride type and generated registry schema do not define a private property. The supported behavior is for package-level privacy to remain inherited while version-specific fields are applied, which the new test now covers.

This comment was generated by an AI coding assistant.

@risu729
risu729 marked this pull request as ready for review July 10, 2026 12:29
@jdx
jdx merged commit ce7c76f into jdx:main Jul 12, 2026
33 checks passed
@risu729
risu729 deleted the agent/aqua-private-release-routing branch July 12, 2026 03:39
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.

2 participants