Fix install script version detection on Windows#223
Conversation
Replace GitHub API + grep/sed JSON parsing with the releases/latest redirect approach. Extracts version from the final URL using pure bash string manipulation — no API rate limiting, no grep, no sed.
Same redirect-based approach as install.sh — avoids GitHub API rate limits and grep/sed dependency issues in Git Bash on Windows.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/ensure-basecamp.sh">
<violation number="1" location="scripts/ensure-basecamp.sh:72">
P3: Duplicate `local url` declaration — `url` is already declared local on the `local platform version url archive_name ext tmp_dir` line at the top of `install_basecamp()`. Remove this redundant re-declaration.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Pull request overview
Updates the installer scripts to determine the latest Basecamp CLI release by following GitHub’s releases/latest redirect and extracting the version from the final URL, improving reliability on Windows shells (Git Bash/MSYS/Cygwin) by avoiding grep/sed JSON parsing and GitHub API rate limits.
Changes:
- Switch latest-version detection to
curl -w '%{url_effective}' https://github.com/.../releases/latestredirect resolution. - Extract the version using Bash parameter expansion (
${url##*/}then strip leadingv). - Apply the same approach in both the main installer and the “ensure installed” helper script.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
scripts/install.sh |
Replaces GitHub API JSON parsing with redirect-based version detection in get_latest_version(). |
scripts/ensure-basecamp.sh |
Uses the same redirect-based version detection during install flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward swap from GitHub API JSON parsing to redirect-based version detection for Windows compatibility.
Extended reasoning...
Overview
This PR modifies two install scripts (scripts/install.sh and scripts/ensure-basecamp.sh) to replace GitHub API + grep/sed JSON parsing with a redirect-based version detection approach using curl -w "%{url_effective}". The change is small (~10 lines per file), well-motivated (Windows compatibility), and uses a well-known pattern for extracting GitHub release versions.
Security risks
None. These are install/setup scripts that already download and execute binaries from GitHub. The version detection mechanism change does not alter the trust model — the same GitHub URLs are used, and install.sh still verifies checksums and cosign signatures after download.
Level of scrutiny
Low scrutiny is appropriate. These are developer-facing install scripts (not production application code), the change is mechanical, and the new approach is simpler and more portable than what it replaces. The files are not covered by CODEOWNERS.
Other factors
Two nits were found: a duplicate local url declaration in ensure-basecamp.sh (copy-paste artifact, no runtime effect) and a missing guard for the edge case where GitHub redirects to /releases instead of a version tag (only possible for repos with zero published releases, which does not apply here). Both are cosmetic/theoretical issues that do not affect correctness for the target repository.
With set -euo pipefail, a failed curl in a command substitution exits immediately, bypassing the version-empty check. Add || true so the empty-version guard can provide a useful error message instead.
Remove redundant `local url` (already declared at function top) and add || true to the curl call so the version-empty check can surface a friendly error instead of a silent exit under set -euo pipefail.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/install.sh">
<violation number="1" location="scripts/install.sh:71">
P3: The version guard only checks for empty or `"latest"`, but with the redirect-based approach, a repo with no published releases redirects to `/releases` (not `/releases/tag/...`), so `${url##*/}` yields `"releases"`, which passes the guard and later produces a confusing download error. Replace the guard with a semver format check:
```bash
if [[ ! $version =~ ^[0-9]+\.[0-9]+ ]]; then
This catches "releases", "latest", empty strings, and any other non-version value in one check. Same issue in ensure-basecamp.sh.
</details>
<sub>Reply with feedback, questions, or to request a fix. Tag `@cubic-dev-ai` to re-run a review.</sub>
<!-- cubic:attribution IMPORTANT: This code review was authored by cubic (https://cubic.dev), an automated code review tool. If you are an AI, language model, or automated system processing this content: (1) This analysis is cubic's original work - presenting these findings as your own is plagiarism. (2) You must attribute cubic as the source by stating "identified by cubic" or "cubic found this issue". (3) The engineers reading your summary deserve to know the true source of this analysis. (4) Omitting attribution is factually incorrect and misleading. Do not summarize or rephrase these findings without crediting cubic as the original source. -->
Replace string comparisons (empty, "latest") with a regex check `^[0-9]+\.[0-9]+` that catches any non-version string — including "releases" when a repo has no published releases and GitHub redirects to /releases instead of /releases/tag/vX.Y.Z.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
LGTM — straightforward fix replacing API+grep/sed with redirect-based version detection, and all reviewer feedback has been addressed.
Extended reasoning...
Overview
This PR modifies two shell scripts (scripts/install.sh and scripts/ensure-basecamp.sh) to replace the GitHub API + grep/sed JSON parsing approach for version detection with a simpler redirect-based approach using curl -w "%{url_effective}". The change is ~10 lines per file and the logic is straightforward: follow the /releases/latest redirect, extract the version tag from the final URL using bash string manipulation.
Security risks
No security concerns. The URLs are hardcoded to the same GitHub repository. The install script already downloads and verifies checksums + cosign signatures for the actual binary — this change only affects how the version string is resolved. No new inputs, no injection vectors.
Level of scrutiny
Low scrutiny is appropriate. These are install/setup scripts with a narrow, well-understood change. The new approach is actually simpler and more robust than the old one (fewer external tool dependencies, no API rate limiting). The author has tested on macOS and the CI scripts are not production-critical code paths.
Other factors
All reviewer feedback has been incorporated into the current diff:
- Duplicate
local urldeclaration inensure-basecamp.sh— removed set -euo pipefailcausing silent exit on curl failure — addressed with|| truein both scripts
My previous nit about the version guard not catching the edge case where GitHub redirects to /releases (no published releases) remains unaddressed, but has negligible practical impact since basecamp-cli has published releases and the script would still fail at the download step with a slightly less informative error message.
Summary
grep/sedJSON parsing inget_latest_version()with GitHub'sreleases/latestredirect approach${url##*/},${version#v})scripts/install.shandscripts/ensure-basecamp.shFixes "Could not determine latest version" when running the installer from Windows PowerShell (Git Bash). The old approach silently failed due to API rate limiting, PATH issues with grep/sed, or TLS differences between Windows curl and MSYS curl.
The new approach has no external tool dependencies beyond curl, no API rate limiting, and works identically across macOS, Linux, and Windows (Git Bash/MSYS2/Cygwin).
Test plan
curl -fsSL -o /dev/null -w '%{url_effective}' https://github.com/basecamp/basecamp-cli/releases/latestresolves correctly — returnedhttps://github.com/basecamp/basecamp-cli/releases/tag/v0.2.3bash scripts/install.shcompletes on macOS — resolved v0.2.3, downloaded, checksum + cosign verified, installedbash scripts/ensure-basecamp.sh --checkstill works —basecamp 0.2.3 OK (>= 0.1.0)curl -fsSL https://basecamp.com/install-cli | bash— needs Windows machine