Skip to content

Generalize latest-release lookup across features - #33

Closed
NicoVIII with Copilot wants to merge 8 commits into
mainfrom
copilot/fix-lefthook-build-test-publish
Closed

NicoVIII with Copilot wants to merge 8 commits into
mainfrom
copilot/fix-lefthook-build-test-publish

Conversation

Copilot AI commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

The latest-version fix had been implemented only in lefthook, duplicating logic that already belonged in the shared download library. This change moves the redirect-based GitHub release lookup into the shared helper so every feature using fetch_latest_version gets the same behavior.

  • Shared latest-version resolution

    • Replace the API/curl/jq-based implementation in lib/download.ab with redirect-based GitHub release resolution.
    • Normalize the redirect parsing so tags are extracted consistently from releases/latest.
  • Feature consumers

    • Remove the Lefthook-local fetch_latest_version implementation and switch it back to the shared helper.
    • Update amber, gleam, and just to use the same shared behavior without adding curl/jq only for latest.
  • Generated artifacts

    • Regenerate the affected install.sh files so the checked-in scripts match the Amber sources.
  • Changelogs

    • Add Unreleased entries for all changed features: amber, gleam, just, and lefthook.

Example of the consolidation:

import { fetch_latest_version, normalize_version, download_file } from "../../lib/download.ab"

if version == "latest" {
    ensure_packages(base_packages)?
    version = fetch_latest_version(REPO_OWNER, REPO_NAME)?
}

Copilot AI changed the title [WIP] Fix failing GitHub Actions job lefthook / build-test-publish Fix Lefthook latest resolution in build-test-publish Jun 26, 2026
Copilot AI requested a review from NicoVIII June 26, 2026 11:29
Copilot AI changed the title Fix Lefthook latest resolution in build-test-publish Generalize latest-release lookup across features Jun 26, 2026
@NicoVIII
NicoVIII requested a review from Copilot June 26, 2026 11:39

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR consolidates “latest” version resolution for GitHub releases into the shared lib/download.ab helper, so all devcontainer features consistently resolve latest without requiring GitHub API calls (and without introducing curl/jq dependencies solely for that purpose).

Changes:

  • Replaces fetch_latest_version in lib/download.ab with redirect-based parsing of https://github.com/<owner>/<repo>/releases/latest.
  • Updates affected features (amber, gleam, just, lefthook) to rely on the shared helper without installing curl/jq for latest.
  • Regenerates corresponding install.sh artifacts and updates feature changelogs with an Unreleased entry describing the change.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
lib/download.ab Moves latest-release lookup to GitHub redirect parsing in the shared helper.
src/amber/install.ab Stops adding curl/jq packages for latest, relies on shared helper.
src/amber/install.sh Regenerated script reflecting shared redirect-based latest lookup.
src/amber/CHANGELOG.md Documents redirect-based latest resolution in Unreleased notes.
src/gleam/install.ab Stops adding curl/jq packages for latest, relies on shared helper.
src/gleam/install.sh Regenerated script reflecting shared redirect-based latest lookup.
src/gleam/CHANGELOG.md Documents redirect-based latest resolution in Unreleased notes.
src/just/install.ab Stops adding curl/jq packages for latest, relies on shared helper.
src/just/install.sh Regenerated script reflecting shared redirect-based latest lookup.
src/just/CHANGELOG.md Adds Unreleased entry for redirect-based latest resolution.
src/lefthook/install.ab Stops adding curl/jq packages for latest, relies on shared helper.
src/lefthook/install.sh Regenerated script reflecting shared redirect-based latest lookup.
src/lefthook/CHANGELOG.md Adds Unreleased entry for redirect-based latest resolution.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/download.ab Outdated
const url = "https://api.github.com/repos/{repo_owner}/{repo_name}/releases/latest"
return $ curl -sL {url} | jq -r .tag_name $?
const url = "https://github.com/{repo_owner}/{repo_name}/releases/latest"
return $ bash -o pipefail -c "(wget -S --max-redirect=0 --spider '{url}' 2>&1 || true) | sed -n \"s#^[[:space:]]*[Ll]ocation:[[:space:]]*.*/##p\" | sed \"s#[[:space:]]*\\[following\\]##\" | tail -n1 | grep . || (echo \"Unable to determine latest release version from {url}\" >&2; exit 1)" $?
@NicoVIII

Copy link
Copy Markdown
Owner

@copilot Explain to me why this approach would be more robust than the old one? It looks more hacky to me - instead of reading the result from an API which is build for that, we do some magic to get the same data from a redirection.
Wouldn't it be more wise to make the approach more robust by e.g. add validation of the result and maybe retry up to 3 times, if one call isn't valid?

Copilot AI commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Explain to me why this approach would be more robust than the old one? It looks more hacky to me - instead of reading the result from an API which is build for that, we do some magic to get t...

It isn’t more correct than the API by itself. The reason for the redirect-based lookup was narrower: avoid adding curl/jq as runtime dependencies to every feature just to resolve latest, since the installers already need wget for the actual download.

I do agree the first version was too brittle. In 0914a04 I hardened it so it now retries transient failures, validates that the redirect contains /releases/tag/, and preserves tags with slashes instead of chopping at the last /. That keeps the smaller dependency surface while addressing the robustness concern you raised.

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.

3 participants