Conversation
affe055 to
1fdcdbe
Compare
1fdcdbe to
8d5a86f
Compare
26a39ae to
5b0a53c
Compare
ab390f5 to
7795f34
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b0a53c887
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
7795f34 to
39461a6
Compare
aspect-launcher manual test resultsTested on: macOS (aarch64-apple-darwin), Setupexport ASPECT_CLI_DOWNLOADER_CACHE=/tmp/aspect-test-cache
export ASPECT_DEBUG=1All tests run from a directory with no Test 1 — Unpinned first run: queries releases API, downloads stable releaserm -rf /tmp/aspect-test-cache
mkdir /tmp/axl-test && cd /tmp/axl-test
ASPECT_DEBUG=1 ASPECT_CLI_DOWNLOADER_CACHE=/tmp/aspect-test-cache \
cargo run --manifest-path ... -p aspect-launcher -- versionExpected: queries API, skips prereleases, downloads latest stable release. Result: ✅ Resolved Test 2 — Unpinned warm cache: no API callSame command, second run. Expected: fresh hint + cached binary → no network call. Result: ✅ No Test 3 — Stale hint: re-queries API, reuses cached binarytouch -t "$(date -v-25H +%Y%m%d%H%M)" /tmp/aspect-test-cache/launcher/latest/*Expected: hint older than 24h → re-queries API, finds same version, hits binary cache (no re-download). Result: ✅ Stale hint triggered API re-query; binary was already cached so no download. Test 4 — API failure with stale hint: falls back to cached binarytouch -t "$(date -v-25H +%Y%m%d%H%M)" /tmp/aspect-test-cache/launcher/latest/*
GITHUB_TOKEN=invalid ...Expected: stale hint + API 401 → fall back to cached binary, reset hint expiry. Result: ✅ Launched successfully from cached binary. API error body surfaced in debug output. Test 5 — Pinned version: direct download, no API callmkdir -p /tmp/axl-test-pinned/.aspect
printf 'version("2026.15.2")\n' > /tmp/axl-test-pinned/.aspect/version.axl
touch /tmp/axl-test-pinned/MODULE.bazel
cd /tmp/axl-test-pinnedExpected: pinned tag computed directly, no API call, downloads Result: ✅ Test 5 (re-run) — Pinned version: cache hitSame command, second run. Expected: same pinned tag → cache hit, no download, no API call. Result: ✅ Note: the pinned and unpinned runs share the same cached binary when they resolve |
a98b1ce to
fdbb3df
Compare
fdbb3df to
1148d4d
Compare
Currently the release fetcher fails if it finds a release without any assets. This is possible is you happen to check the latest release mid-release when it doesn't have any assets yet. This improves the code so that it falls back to the latest release that has an asset to download when searching.
Before
The GitHub source handler had a single code path for both pinned and unpinned versions:
v{version}— for unpinned,versionwas always the launcher's own compiled-in versionGET /repos/{org}/{repo}/releases/tags/{tag}assetsarrayapi.github.com/.../assets/{id})Problems:
Releasestruct required theassetsfield, but GitHub sometimes omits it →missing field 'assets'deserialization errorversion_pinnedboolean was threaded throughToolSpec/AspectCliConfigbut both paths used the same download logicAfter
The GitHub source handler is now split into two clean steps — resolve, then download:
Step 1 — Resolve the tag:
version = Some("2026.11.6")fromversion.axl): compute the tag directly (e.g.v2026.11.6). No API call.version = None— noversion.axl, orversion()with no positional arg): queryGET /repos/{org}/{repo}/releases?per_page=10, scan the most recent releases, and pick the first one whoseassetscontain the matching artifact.Step 2 — Download using the resolved tag:
Both paths now have a concrete tag and use the same direct download URL:
https://github.com/{org}/{repo}/releases/download/{resolved_tag}/{artifact}What this fixes:
assetsnow has#[serde(default)], so releases with missing or empty assets are skipped instead of causing a deserialization errorversion_pinnedboolean is removed;version: Option<String>onToolSpecnaturally encodes the distinction (Some= pinned,None= resolve from API)