fix(install): resilient latest-version fetch (both installers)#946
fix(install): resilient latest-version fetch (both installers)#946mdesmet wants to merge 1 commit into
Conversation
Reported on #930: a transient 504 from api.github.com/.../releases/latest (or the 60/hr/IP unauthenticated rate limit) aborted the whole install with "Failed to fetch version information" — even though the download itself uses releases/latest/download/<file>, which GitHub resolves server-side with no API call. The API response only feeds the version-string display and the already-installed short-circuit. Both installers now, in the latest path: - retry the API call up to 3x with linear backoff (bash uses curl --fail so a 504 retries instead of parsing an error body); - on continued failure, print a muted notice and proceed to install latest anyway (version string shown as "latest"); - only short-circuit as "already installed" on a real version match — never treat empty==empty (unresolved version + unreadable binary) as installed. Pinned-version installs (-Version / --version) are unchanged: a genuine 404 still hard-fails. Tests: version-fetch-resilience.test.ts pins the retry + graceful-degrade behavior in both installers. bash -n clean; install.ps1 parses clean and the Pester suite (6/6) still passes on PowerShell 7.6.2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
❌ Tests — Failures DetectedTypeScript — 15 failure(s)
Next StepPlease address the failing cases above and re-run verification. cc @mdesmet |
dev-punia-altimate
left a comment
There was a problem hiding this comment.
🤖 Code Review — OpenCodeReview (Gemini) — 1 finding(s)
- 1 anchored to a line (posted inline when the comment stream is on)
- 0 without a line anchor
All findings (full text)
1. install.ps1 (L130)
[🟠 MEDIUM] By changing the API failure behavior from exit 1 to a fallback mechanism, $specificVersion can now safely be an empty string "".
However, this introduces a subtle bug further down in the script when it checks if the app is already installed:
try { $installedVersion = (& $probe --version 2>$null | Select-Object -First 1).ToString().Trim() } catch {}
if ($installedVersion -eq $specificVersion) {
Write-Muted "Version $specificVersion already installed"
exit 0
}If the GitHub API fails ($specificVersion = "") AND the local executable is missing or corrupted such that the --version command fails ($installedVersion defaults to ""), the condition "" -eq "" will evaluate to $true. The installer will output Version already installed and incorrectly short-circuit with exit 0 instead of downloading and replacing the broken installation.
To avoid this, you can explicitly reset $specificVersion = $null here when the API fails, which ensures the downstream string-comparison evaluates to $false.
Suggested change:
Write-Muted "Could not resolve the latest version from GitHub (API unavailable) — installing the latest release anyway."
$specificVersion = $null
| if ([string]::IsNullOrWhiteSpace($specificVersion)) { | ||
| Write-Err "Failed to fetch version information" | ||
| exit 1 | ||
| Write-Muted "Could not resolve the latest version from GitHub (API unavailable) — installing the latest release anyway." |
There was a problem hiding this comment.
[🟠 MEDIUM] By changing the API failure behavior from exit 1 to a fallback mechanism, $specificVersion can now safely be an empty string "".
However, this introduces a subtle bug further down in the script when it checks if the app is already installed:
try { $installedVersion = (& $probe --version 2>$null | Select-Object -First 1).ToString().Trim() } catch {}
if ($installedVersion -eq $specificVersion) {
Write-Muted "Version $specificVersion already installed"
exit 0
}If the GitHub API fails ($specificVersion = "") AND the local executable is missing or corrupted such that the --version command fails ($installedVersion defaults to ""), the condition "" -eq "" will evaluate to $true. The installer will output Version already installed and incorrectly short-circuit with exit 0 instead of downloading and replacing the broken installation.
To avoid this, you can explicitly reset $specificVersion = $null here when the API fails, which ensures the downstream string-comparison evaluates to $false.
Suggested change:
| Write-Muted "Could not resolve the latest version from GitHub (API unavailable) — installing the latest release anyway." | |
| Write-Muted "Could not resolve the latest version from GitHub (API unavailable) — installing the latest release anyway." | |
| $specificVersion = $null |
🤖 Code Review — OpenCodeReview (Gemini) — No Issues FoundNo supported files changed. |
sahrizvi
left a comment
There was a problem hiding this comment.
Three findings inline below — one Critical (the retry loop is currently a no-op under set -euo pipefail), and two High (install.ps1 short-circuit on empty==empty, and no HTTP timeout on either retry). Repro details and one-line suggested fixes inline.
| # without the version string. | ||
| specific_version="" | ||
| for attempt in 1 2 3; do | ||
| specific_version=$(curl -fsSL https://api.github.com/repos/AltimateAI/altimate-code/releases/latest 2>/dev/null | sed -n 's/.*"tag_name": *"v\([^"]*\)".*/\1/p') |
There was a problem hiding this comment.
One subtle interaction to flag here — under the script's set -euo pipefail (line 2), the retry loop may not iterate the way it reads.
When curl --fail is paired with pipefail + set -e, a non-zero curl exit (e.g. a real 504 or rate-limit 403) propagates through the pipeline → the command substitution → the assignment, and set -e exits the script before [ -n "$specific_version" ] && break is evaluated. The loop iterates at most once and the muted "installing latest anyway" notice doesn't get a chance to print.
I ran a quick repro against a local 504-serving stub using these exact lines and the script aborted at attempt 1 with curl's exit 22, never reaching the degrade path.
A one-line fix that lets the inner pipeline failure be absorbed cleanly:
| specific_version=$(curl -fsSL https://api.github.com/repos/AltimateAI/altimate-code/releases/latest 2>/dev/null | sed -n 's/.*"tag_name": *"v\([^"]*\)".*/\1/p') | |
| specific_version=$(curl -fsSL https://api.github.com/repos/AltimateAI/altimate-code/releases/latest 2>/dev/null | sed -n 's/.*"tag_name": *"v\([^"]*\)".*/\1/p' || true) |
With || true appended, all three attempts iterate and the degrade banner prints on persistent failure.
One related note for a possible follow-up: packages/opencode/test/install/version-fetch-resilience.test.ts substring-matches the script source, so expect(BASH).toContain("for attempt in 1 2 3") passes regardless of whether the loop body actually runs. A small behavioural test that runs install against a stubbed 504 server and asserts the degrade banner appears would catch this kind of regression in CI.
| if ([string]::IsNullOrWhiteSpace($specificVersion)) { | ||
| Write-Err "Failed to fetch version information" | ||
| exit 1 | ||
| Write-Muted "Could not resolve the latest version from GitHub (API unavailable) — installing the latest release anyway." |
There was a problem hiding this comment.
A subtle interaction worth guarding against on the PS side: if this branch fires (API blip → $specificVersion = "") and the installed binary's --version probe also fails (corrupt/missing binary → catch absorbs → $installedVersion = ""), the downstream check at install.ps1:161 lands on "" -eq "" and the installer exits 0 with "Version already installed" instead of reinstalling.
For reference, the equivalent bash check at install:270 has the matching [ -n "$specific_version" ] guard — this is just to bring the PS side in line with it. (The Gemini reviewer flagged a similar concern on 2026-06-15 with a producer-side patch; same idea here:)
| Write-Muted "Could not resolve the latest version from GitHub (API unavailable) — installing the latest release anyway." | |
| Write-Muted "Could not resolve the latest version from GitHub (API unavailable) — installing the latest release anyway." | |
| $specificVersion = $null |
$null -eq "" evaluates $false in PowerShell, so the comparison at line 161 falls through cleanly. The "Installing $App version: ..." banner at line 186 still renders "latest" because if ($specificVersion) { ... } treats both "" and $null as falsy.
| $specificVersion = "" | ||
| for ($attempt = 1; $attempt -le 3; $attempt++) { | ||
| try { | ||
| $rel = Invoke-RestMethod -Uri "https://api.github.com/repos/AltimateAI/altimate-code/releases/latest" -Headers @{ "User-Agent" = "altimate-install" } |
There was a problem hiding this comment.
Worth considering — neither retry sets an explicit HTTP timeout.
Invoke-RestMethod defaults to 100s on PS 5.1 and effectively unbounded on PS 7+, so a stuck/dead-air socket can hold each attempt for a long time. Across three back-to-back attempts that can add up to multi-minute apparent freezes during what's normally a quick irm | iex.
A short timeout here keeps the worst case bounded:
| $rel = Invoke-RestMethod -Uri "https://api.github.com/repos/AltimateAI/altimate-code/releases/latest" -Headers @{ "User-Agent" = "altimate-install" } | |
| $rel = Invoke-RestMethod -Uri "https://api.github.com/repos/AltimateAI/altimate-code/releases/latest" -Headers @{ "User-Agent" = "altimate-install" } -TimeoutSec 10 |
Same gap on the bash side at install:216 if you want to keep parity — curl -fsSL has a 60s connect-timeout default and no transfer cap, so --max-time 10 would be the matching knob there.
What
Follow-up to #930 (reported by @ralphstodomingo): a transient 504 from
api.github.com/.../releases/latest— or the 60/hr/IP unauthenticated rate limit — aborted the whole install withFailed to fetch version information, even though the download usesreleases/latest/download/<file>(GitHub resolves "latest" server-side, no API call needed). The API response only feeds the version-string display and the already-installed short-circuit.This issue exists in both installers (
installbash:206-213 andinstall.ps1), so both are fixed here for parity.Stacked on
feat/windows-powershell-installer(sibling to #942) becauseinstall.ps1isn't onmainyet.Changes (latest path, both installers)
releases/latestAPI call up to 3× with linear backoff (bash usescurl --failso a 504 retries instead of parsing an error body).latest).-Version/--versionunchanged: a genuine 404 still hard-fails.Note (from the same review comment): the
exit 0-closes-an-interactive-window quirk underirm | iexis PowerShell-only and only affects pasting directly into an interactive shell (not the documentedpowershell -c "…"form);curl | bashexits in a subshell. Left as-is — low severity, no parity gap to fix.Verification
bash -n installclean;install.ps1parses clean and the Pester suite (6/6) still passes on PowerShell 7.6.2.version-fetch-resilience.test.tspins retry + graceful-degrade in both installers (57 pass across the install/branding set).🤖 Generated with Claude Code
Summary by cubic
Make the "latest" install path resilient in both
installandinstall.ps1so transient GitHub API errors or rate limits no longer abort the install. We retry the version lookup and, if it still fails, proceed to install the latest release and show "latest" in the banner.releases/latestAPI up to 3 times with linear backoff (bash usescurl -fsSL; PowerShell has a retry loop).latest.--version/-Versioninstalls unchanged; a real 404 still fails.packages/opencode/test/install/version-fetch-resilience.test.tsto pin retry and graceful-degrade behavior.Written for commit be6c0c0. Summary will update on new commits.