From df075b9ecf86a35a040e97ed5d215488dacfba68 Mon Sep 17 00:00:00 2001 From: grok-reviewer-andresmgsl Date: Wed, 29 Jul 2026 14:42:59 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20review=20round=20=E2=80=94=20pin-wins=20?= =?UTF-8?q?candidates,=20no=20BOX=5FRAW=5FKIND=20guess?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address codex REQUEST_CHANGES + claude's live Forgejo measurements: - ref_candidate_urls is host-only: same refs/tags then refs/heads on every forge (Forgejo serves both paths; bare /archive/ no longer special). - bootstrap box raw fetch tries /raw/tag/ then /raw/branch/ and never guesses kind from spelling; download is separate from execute. - BOX_HOST defaults through RIG_HOST; comments narrow the zero-GitHub claim (box@0.9.0 installer still hardcodes GitHub for its archive). - SC2016 suppressions on the new grep assertions; tests updated. --- changelog.d/111.md | 2 +- commands/bootstrap.sh | 62 ++++++++++++++++++++++++++++--------------- install.sh | 26 +++++++----------- test/cli.sh | 23 ++++++++++------ test/release.sh | 35 +++++++++++++----------- 5 files changed, 87 insertions(+), 61 deletions(-) diff --git a/changelog.d/111.md b/changelog.d/111.md index ffd0cb2..edbb7b6 100644 --- a/changelog.d/111.md +++ b/changelog.d/111.md @@ -1,3 +1,3 @@ ### Changed -- `install.sh` and bootstrap's box fetch take `RIG_HOST` / `BOX_HOST` so a Forgejo origin can serve the install channel (#111) +- `install.sh` and bootstrap's box fetch take `RIG_HOST` / `BOX_HOST` so a Forgejo origin can serve the install channel (#111). Archive candidates are host-only (`refs/tags` then `refs/heads` on every forge); box's raw fetch tries `/raw/tag/` then `/raw/branch/` and never guesses kind from spelling. `BOX_HOST` defaults through `RIG_HOST`. diff --git a/commands/bootstrap.sh b/commands/bootstrap.sh index ae5f934..b20617d 100755 --- a/commands/bootstrap.sh +++ b/commands/bootstrap.sh @@ -723,26 +723,33 @@ if [ "$HOST" = "yes" ]; then BOX_RELEASE=0.9.0 BOX_REPO="${BOX_REPO:-heavy-duty/box}" BOX_REF="${BOX_REF:-$BOX_RELEASE}" - # BOX_HOST: which forge serves box (#111). Parallel to RIG_HOST / - # RIG_TEMPLATES_HOST — default GitHub keeps every existing bootstrap - # byte-unchanged; set BOX_HOST=https://forgejo.heavyduty.builders when - # box is installed from this instance. Raw-file grammar differs: + # BOX_HOST: which forge serves box's *installer script* (#111). Parallel to + # RIG_HOST / RIG_TEMPLATES_HOST. Defaults to RIG_HOST when set, else GitHub, + # so a Forgejo-sourced rig stays Forgejo-native for this fetch without a + # second knob — override with BOX_HOST when the two must diverge. + # Raw-file grammar: # GitHub raw.githubusercontent.com///install.sh # Forgejo //raw/{tag|branch}//install.sh - # Version-shaped refs (BOX_RELEASE pins) use raw/tag/; anything else - # (BOX_REF=main) uses raw/branch/. - BOX_HOST="${BOX_HOST:-https://github.com}" + # Forgejo's bare /raw// is branch-first (opposite of /archive/), + # so we never guess kind from spelling: try /raw/tag/ then /raw/branch/ + # and let the fetch decide (same pin-wins rule as ref_candidate_urls). + # SCOPE: this only moves the script fetch. box@0.9.0's installer still + # hardcodes GitHub for its own archive — zero-GitHub bootstrap needs a + # BOX_HOST knob in heavy-duty/box (tracked separately). + BOX_HOST="${BOX_HOST:-${RIG_HOST:-https://github.com}}" BOX_HOST="${BOX_HOST%/}" - case "$BOX_HOST" in - https://github.com|http://github.com|*//github.com) - BOX_INSTALL_URL="https://raw.githubusercontent.com/${BOX_REPO}/${BOX_REF}/install.sh" ;; - *) - case "$BOX_REF" in - [0-9]*|v[0-9]*) BOX_RAW_KIND=tag ;; - *) BOX_RAW_KIND=branch ;; - esac - BOX_INSTALL_URL="${BOX_HOST}/${BOX_REPO}/raw/${BOX_RAW_KIND}/${BOX_REF}/install.sh" ;; - esac + box_install_urls() { + case "$BOX_HOST" in + https://github.com|http://github.com|*//github.com) + printf 'https://raw.githubusercontent.com/%s/%s/install.sh\n' "$BOX_REPO" "$BOX_REF" ;; + *) + printf '%s/%s/raw/tag/%s/install.sh\n' "$BOX_HOST" "$BOX_REPO" "$BOX_REF" + printf '%s/%s/raw/branch/%s/install.sh\n' "$BOX_HOST" "$BOX_REPO" "$BOX_REF" ;; + esac + } + # Prefer the first candidate for the operator-facing manual path; the + # install loop below walks the full list when the network is live. + BOX_INSTALL_URL="$(box_install_urls | head -n1)" BOX_MANUAL="curl -fsSL ${BOX_INSTALL_URL} | BOX_YES=1 BOX_REF=${BOX_REF} bash" if [ "${RIG_SKIP_BOX_INSTALL:-}" = "1" ]; then log "RIG_SKIP_BOX_INSTALL=1 — skipping box install; to prepare Incus by hand later: ${BOX_MANUAL}" @@ -754,10 +761,22 @@ if [ "$HOST" = "yes" ]; then # builds the Incus stack rather than only dropping the CLI on PATH. Running as # root, box installs globally (/opt/box + /usr/local/bin). No-op if box is # already installed, so re-running bootstrap converges instead of reinstalling. - # A curl failure (no network) fails the pipe under pipefail and lands in the - # else — a warning, never an abort: box is the host extra, the OS+tailnet core - # is already done. - if curl -fsSL "$BOX_INSTALL_URL" | BOX_YES=1 BOX_REF="$BOX_REF" bash; then + # Download and execute are separate so a 404 on /raw/tag/ can fall through + # to /raw/branch/ without running a half-fetched body, and so an installer + # that runs and fails is NOT retried against the next candidate. + # A curl failure (no network) lands in the else — a warning, never an + # abort: box is the host extra, the OS+tailnet core is already done. + BOX_SCRIPT="$(mktemp)" + BOX_GOT="" + while IFS= read -r _box_url; do + if curl -fsSL "$_box_url" -o "$BOX_SCRIPT"; then + BOX_GOT="$_box_url" + BOX_INSTALL_URL="$_box_url" + BOX_MANUAL="curl -fsSL ${BOX_INSTALL_URL} | BOX_YES=1 BOX_REF=${BOX_REF} bash" + break + fi + done < <(box_install_urls) + if [ -n "$BOX_GOT" ] && BOX_YES=1 BOX_REF="$BOX_REF" bash "$BOX_SCRIPT"; then # Don't trust the exit code — prove the effective state (issue #12). An # installer can exit 0 having done less than it claims: box's setup-host # is written for a sudo-capable user, and one of its paths exits 0 after @@ -785,6 +804,7 @@ if [ "$HOST" = "yes" ]; then else warn "box install did not complete (no network, or box's installer failed); bootstrap's core work is done. Finish the host by hand: ${BOX_MANUAL}" fi + rm -f "$BOX_SCRIPT" fi fi diff --git a/install.sh b/install.sh index c64eb5c..aa480d2 100644 --- a/install.sh +++ b/install.sh @@ -125,22 +125,16 @@ resolve_latest_tag() { } # ref_candidate_urls — the download candidates for an -# explicit RIG_REF, in order. Host comes from RIG_HOST; the two forges are -# not URL-compatible (#111 / #109): -# GitHub refs/tags first so a tag always outranks a same-named branch -# (the pin must win), then refs/heads for RIG_REF=main. -# Forgejo one form: /archive/.tar.gz resolves tags, branches and -# SHAs alike (same grammar as templates_archive_urls). +# explicit RIG_REF, in order. Host comes from RIG_HOST. Both GitHub and +# Forgejo (measured 2026-07-29 on forgejo.heavyduty.builders 8.0.3) serve +# the same two paths and the same disambiguation: refs/tags first so a pin +# always outranks a same-named branch, then refs/heads for RIG_REF=main. +# Host is the only forge-specific input — no second grammar (#111). ref_candidate_urls() { local host="${RIG_HOST:-https://github.com}" host="${host%/}" - case "$host" in - https://github.com|http://github.com|*//github.com) - printf '%s/%s/archive/refs/tags/%s.tar.gz\n' "$host" "$1" "$2" - printf '%s/%s/archive/refs/heads/%s.tar.gz\n' "$host" "$1" "$2" ;; - *) - printf '%s/%s/archive/%s.tar.gz\n' "$host" "$1" "$2" ;; - esac + printf '%s/%s/archive/refs/tags/%s.tar.gz\n' "$host" "$1" "$2" + printf '%s/%s/archive/refs/heads/%s.tar.gz\n' "$host" "$1" "$2" } # install_script_url — the curl|bash entrypoint URL for this REPO on RIG_HOST. @@ -267,8 +261,8 @@ else die "set RIG_REF: e.g. curl -fsSL $(install_script_url) | RIG_REF=main bash" fi log "latest release: $REF" - # Same candidate grammar as an explicit pin: on GitHub the tag form wins - # first; on Forgejo the single bare archive URL is the whole list (#111). + # Same candidate grammar as an explicit pin: tags first so the pin wins, + # then heads — identical on every forge RIG_HOST names (#111). mapfile -t urls < <(ref_candidate_urls "$REPO" "$REF") else mapfile -t urls < <(ref_candidate_urls "$REPO" "$REF") @@ -285,7 +279,7 @@ else fi done [ -n "$got" ] \ - || die "failed to download $REPO@$REF — no candidate URL worked (host ${RIG_HOST:-https://github.com}; tried tags then heads on GitHub, or the single archive URL on other forges)" + || die "failed to download $REPO@$REF — no candidate URL worked (host ${RIG_HOST:-https://github.com}; tried refs/tags then refs/heads)" log "extracting archive" tar -xzf "$TMPDIR/rig.tar.gz" -C "$TMPDIR" \ diff --git a/test/cli.sh b/test/cli.sh index a01506a..4812aac 100644 --- a/test/cli.sh +++ b/test/cli.sh @@ -249,15 +249,22 @@ check "bootstrap: manual box install carries the pinned ref" 0 "" \ grep -qF 'BOX_YES=1 BOX_REF=${BOX_REF} bash' "$ROOT/commands/bootstrap.sh" check "bootstrap: box repository remains pinnable" 0 "" \ grep -qF 'BOX_REPO:-heavy-duty/box' "$ROOT/commands/bootstrap.sh" -# BOX_HOST selects the forge that serves box's installer (#111). Default -# GitHub keeps the raw.githubusercontent.com grammar; a non-GitHub host -# uses Forgejo's /raw/{tag|branch}// form. -check "bootstrap: BOX_HOST defaults to GitHub" 0 "" \ - grep -qF 'BOX_HOST="${BOX_HOST:-https://github.com}"' "$ROOT/commands/bootstrap.sh" +# BOX_HOST selects the forge that serves box's installer script (#111). +# Defaults through RIG_HOST so a Forgejo-sourced rig stays Forgejo-native +# for this fetch; a non-GitHub host tries /raw/tag/ then /raw/branch/ +# (never guesses kind from spelling). +# shellcheck disable=SC2016 +check "bootstrap: BOX_HOST defaults through RIG_HOST then GitHub" 0 "" \ + grep -qF 'BOX_HOST="${BOX_HOST:-${RIG_HOST:-https://github.com}}"' "$ROOT/commands/bootstrap.sh" +# shellcheck disable=SC2016 check "bootstrap: GitHub box install uses raw.githubusercontent.com" 0 "" \ - grep -qF 'raw.githubusercontent.com/${BOX_REPO}/${BOX_REF}/install.sh' "$ROOT/commands/bootstrap.sh" -check "bootstrap: non-GitHub box install uses /raw/{tag|branch}/" 0 "" \ - grep -qF '/raw/${BOX_RAW_KIND}/${BOX_REF}/install.sh' "$ROOT/commands/bootstrap.sh" + grep -qF 'raw.githubusercontent.com/%s/%s/install.sh' "$ROOT/commands/bootstrap.sh" +# shellcheck disable=SC2016 +check "bootstrap: non-GitHub box install tries /raw/tag/ first" 0 "" \ + grep -qF 'raw/tag/%s/install.sh' "$ROOT/commands/bootstrap.sh" +# shellcheck disable=SC2016 +check "bootstrap: non-GitHub box install falls back to /raw/branch/" 0 "" \ + grep -qF 'raw/branch/%s/install.sh' "$ROOT/commands/bootstrap.sh" # Opt-out for rehearsals / offline / hand-managed hosts. check "bootstrap: box install honors RIG_SKIP_BOX_INSTALL opt-out" 0 "" \ grep -q "RIG_SKIP_BOX_INSTALL" "$ROOT/commands/bootstrap.sh" diff --git a/test/release.sh b/test/release.sh index 3608c5b..8b140b0 100644 --- a/test/release.sh +++ b/test/release.sh @@ -104,16 +104,20 @@ check "candidates: refs/tags first — the pin outranks a same-named branch" 0 \ "https://github.com/acme/widgets/archive/refs/tags/1.2.3.tar.gz" rcu_line 1 check "candidates: refs/heads is the fallback" 0 \ "https://github.com/acme/widgets/archive/refs/heads/1.2.3.tar.gz" rcu_line 2 -# RIG_HOST selects the forge grammar (#111): non-GitHub hosts emit one bare -# /archive/.tar.gz form (Forgejo), never the refs/{tags,heads}/ pair. -check "candidates: Forgejo host emits the bare archive URL" 0 \ - "https://forgejo.example/acme/widgets/archive/1.2.3.tar.gz" \ +# RIG_HOST is host-only (#111): Forgejo serves the same refs/{tags,heads}/ +# pair (measured 2026-07-29). Host substituted; grammar unchanged. +check "candidates: Forgejo host uses the same refs/tags form" 0 \ + "https://forgejo.example/acme/widgets/archive/refs/tags/1.2.3.tar.gz" \ rcu_line 1 RIG_HOST=https://forgejo.example -check "candidates: Forgejo host emits exactly one candidate" 0 "1" \ +check "candidates: Forgejo host keeps refs/heads as fallback" 0 \ + "https://forgejo.example/acme/widgets/archive/refs/heads/1.2.3.tar.gz" \ + rcu_line 2 RIG_HOST=https://forgejo.example +# shellcheck disable=SC2016 +check "candidates: Forgejo host emits exactly two candidates" 0 "2" \ env RIG_HOST=https://forgejo.example bash -c 'set -euo pipefail . "$1"; ref_candidate_urls acme/widgets 1.2.3 | grep -c .' _ "$RL" check "candidates: trailing slash on RIG_HOST is stripped" 0 \ - "https://forgejo.example/acme/widgets/archive/1.2.3.tar.gz" \ + "https://forgejo.example/acme/widgets/archive/refs/tags/1.2.3.tar.gz" \ rcu_line 1 RIG_HOST=https://forgejo.example/ # install_script_url — the curl|bash hint must match the forge (#111). @@ -206,23 +210,24 @@ H5="$WORK/h5"; B5="$WORK/b5" check "channel: a ref that is neither tag nor branch dies naming the tries" \ 1 "no candidate URL worked" rinst "$H5" "$B5" RIG_REF=no-such-ref -# Channel 4 — RIG_HOST=Forgejo: one archive URL, same redirect grammar for -# latest (#111). The stub only succeeds when the bare /archive/ form is -# requested — a regression that still emitted refs/tags would fail here. +# Channel 4 — RIG_HOST=Forgejo: same refs/tags→refs/heads candidate order and +# the same /releases/latest redirect grammar (#111). The stub succeeds only +# when the refs/tags form is requested — a regression that still emitted the +# bare /archive/ form would fail here. H9="$WORK/h9"; B9="$WORK/b9"; LOG9="$WORK/log9" -check "channel forgejo latest: resolves and installs via bare archive URL" 0 "done" \ +check "channel forgejo latest: resolves and installs via refs/tags archive URL" 0 "done" \ rinst "$H9" "$B9" RIG_HOST=https://forgejo.example \ CURL_STUB_REDIRECT=https://forgejo.example/heavy-duty/rig/releases/tag/7.7.7-relflow \ - CURL_STUB_OK='/archive/7.7.7-relflow.tar.gz' CURL_STUB_LOG="$LOG9" -check "channel forgejo latest: download URL is the bare archive form" 0 \ - "https://forgejo.example/heavy-duty/rig/archive/7.7.7-relflow.tar.gz" \ + CURL_STUB_OK='/archive/refs/tags/7.7.7-relflow.tar.gz' CURL_STUB_LOG="$LOG9" +check "channel forgejo latest: download URL is the refs/tags form" 0 \ + "https://forgejo.example/heavy-duty/rig/archive/refs/tags/7.7.7-relflow.tar.gz" \ cat "$LOG9" check "channel forgejo latest: the tree landed" 0 "" \ test -x "$H9/versions/7.7.7-relflow/bin/rig" H10="$WORK/h10"; B10="$WORK/b10" -check "channel forgejo pinned: RIG_REF uses the bare archive URL" 0 "done" \ +check "channel forgejo pinned: RIG_REF=main falls through to refs/heads" 0 "done" \ rinst "$H10" "$B10" RIG_HOST=https://forgejo.example RIG_REF=main \ - CURL_STUB_OK='/archive/main.tar.gz' + CURL_STUB_OK='/archive/refs/heads/main.tar.gz' check "channel forgejo pinned: the tree landed" 0 "" \ test -x "$H10/versions/7.7.7-relflow/bin/rig" # Refusal hint on a non-GitHub host must not send the operator to