Skip to content

feat(update): host AppImages on Azure Blob to fix GitHub rate-limit on rio update - #545

Open
amitsingh21 wants to merge 19 commits into
develfrom
feat/appimage-azure-blob
Open

feat(update): host AppImages on Azure Blob to fix GitHub rate-limit on rio update#545
amitsingh21 wants to merge 19 commits into
develfrom
feat/appimage-azure-blob

Conversation

@amitsingh21

Copy link
Copy Markdown
Contributor

Problem

rio update fails on AppImage installs with Failed to retrieve the download URL for the latest AppImage. Root cause (per Slack thread): the updater calls api.github.com/.../releases/latest, which is unauthenticated and shares the office-IP GitHub rate-limit budget. When exhausted, the asset lookup returns nothing and the download fails. Multiple users hit this.

What changed

Move the AppImage rio update download path off api.github.com onto our own public-read Azure Blob containers, channel-keyed by the running binary's __version__. The pip/PyPI path is untouched — blob storage is only used in the AppImage branch.

Consumer (CLI):

  • New focused module riocli/utils/appimage.py: channel detection from version suffix, manifest/URL builders, channel-aware update check, manifest fetch, and an atomic download→verify→swap (os.replace, sha256 required, request timeouts, no devel downgrade).
  • rio update (riocli/bootstrap.py) now branches: pip → existing PyPI flow (unchanged); AppImage → Azure Blob channel. Development builds (-dev.<branch>) are reported as not auto-updatable.
  • Removed the old GitHub-API-based update_appimage from riocli/utils/__init__.py.

Producer (CI):

  • scripts/build-rio-appimage.sh stamps a channel suffix into __version__ for non-release builds and uploads the AppImage + a latest.json manifest to the matching container via azcopy (SAS masked under set -x).
  • upload-appimage.yml: drops the main trigger, sets CHANNEL (PR → dev, devel push → devel), and comments the blob URL on PRs.
  • release.yml: passes CHANNEL=release. GitHub Release asset + PyPI publish are kept (transition fallback for already-installed binaries).

Channels / containers (account riocliartifacts, OKD4 Prod, anonymous read):

Version pattern Channel Container Retention
X.Y.Z release release permanent
X.Y.Z-devel+<sha> devel devel 30d
X.Y.Z-dev.<branch>+<sha> (not auto-updatable) dev 15d

Override the base URL with RIO_APPIMAGE_BASE_URL for staging. Details in docs/update-channels.md.

Infra (already provisioned)

Storage account riocliartifacts (RG rio-cli, japaneast) with public-read release/devel/dev containers, lifecycle policies, and a write SAS in repo secrets AZURE_STORAGE_ACCOUNT / AZURE_SAS_TOKEN (SAS expires 2027-06-14 — rotate before then).

Test plan

  • 170 unit tests pass (uv run pytest tests/unit/), ruff clean
  • Live end-to-end smoke vs the real devel container: anonymous manifest fetch + sha256 verify + atomic swap into a temp target (cleaned up)
  • This PR's upload-appimage.yml run uploads to the dev container and comments the blob link (validates the producer path live)
  • After merge to devel: confirm a devel-channel AppImage updates from the devel container

🤖 Generated with Claude Code

amitsingh21 and others added 16 commits June 14, 2026 05:42
Move `rio update` AppImage download off the rate-limited api.github.com
path to public-read Azure Blob containers (release/devel/dev), channel
keyed by __version__ suffix. pip/PyPI path untouched.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… downgrade

- Replace TemporaryDirectory/remove/move with mkstemp+os.replace for atomic
  same-filesystem swap with no data-loss window
- Raise ValueError when manifest is missing 'sha256' (no silent skip)
- Add timeout=(10, 300) to binary download and timeout=10 to manifest fetch
- Guard semver.Version.parse in channel_for_version with clear ValueError
- Devel channel: never offer a remote whose base version is older than current
- Add 4 new unit tests (invalid version, missing sha256, devel no-downgrade,
  atomic swap happy path); total 19 tests, all passing

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…solve account TODO

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Install azcopy near the mc download block and export it onto PATH before
cd scripts so it remains reachable for the later upload stage. For
non-release builds, read the current __version__ from riocli/bootstrap.py,
compute a short SHA, and sed-in the channel suffix:
  devel  → X.Y.Z-devel+<sha>
  dev/PR → X.Y.Z-dev.<safe-branch>+<sha>
Release builds (CHANNEL=release) are left untouched because bump-version.sh
already set a clean semver before this script runs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
After appimagetool produces the AppImage in scripts/, find the built
rio*.AppImage, then:
  release/devel: compute sha256, write latest.json, azcopy-upload both
                 the AppImage and the manifest to the channel container.
  dev/PR:        azcopy-upload the AppImage only to dev/<branch>/
                 (no manifest — dev builds are not an update channel).

Upload is skipped gracefully when AZURE_STORAGE_ACCOUNT / AZURE_SAS_TOKEN
are unset, so the script still works in local builds without Azure creds.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…icit overwrite)

- Wrap entire Azure upload then-body with set +x/set -x so the SAS token
  never appears in bash -x trace logs (CRITICAL credential leak fix)
- Add guard after APPIMAGE_FILE= find: exits 1 with clear error if no
  rio*.AppImage exists before attempting upload
- Add --overwrite=true to all three azcopy copy calls to make clobber
  behaviour explicit
- Hoist RAW_BRANCH assignment to just after CHANNEL= so it is computed
  once unconditionally; remove duplicate assignments in stamping block
  and upload block
- Update upload-block comment to document dev/<branch>/<file> vs
  <channel>/<file>+latest.json layout

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Drop `main` trigger; keep `devel` push and `pull_request`
- Add CHANNEL expression (dev for PRs, devel for devel-branch pushes)
  plus AZURE_STORAGE_ACCOUNT and AZURE_SAS_TOKEN to the Create AppImage step
- Replace the three obsolete GitHub-Actions-artifact-link comment steps
  with a single "Comment blob link on PR" step pointing at the blob URL
- Keep Upload AppImage artifact step as secondary GitHub fallback

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add CHANNEL=release plus AZURE_STORAGE_ACCOUNT and AZURE_SAS_TOKEN to
the Run semantic-release step env. The existing .releaserc.json
prepareCmd (build-rio-appimage.sh) now receives CHANNEL=release and
will upload the versioned AppImage + latest.json to the release
container in addition to attaching it as a GitHub Release asset.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@amitsingh21
amitsingh21 requested a review from a team as a code owner June 14, 2026 01:35
Copilot AI review requested due to automatic review settings June 14, 2026 01:35
@amitsingh21
amitsingh21 requested a review from a team as a code owner June 14, 2026 01:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 moves the AppImage self-update path for rio update off unauthenticated GitHub API calls (which can hit shared rate limits) and onto public-read Azure Blob Storage, while keeping the pip/PyPI update path unchanged.

Changes:

  • Added a new riocli/utils/appimage.py module to derive update channels from __version__, build manifest/asset URLs, fetch latest.json, and perform an atomic download→verify→swap.
  • Rewired riocli/bootstrap.py:update to route AppImage installs through the new blob/manifest-based updater and removed the legacy GitHub-API-based update_appimage implementation.
  • Updated CI build/release workflows and scripts to stamp channel/version metadata and upload AppImages + latest.json manifests to Azure Blob, plus added unit tests and documentation.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
riocli/utils/appimage.py New AppImage channel + blob-manifest updater implementation.
riocli/bootstrap.py Routes rio update between pip vs AppImage update flows.
riocli/utils/__init__.py Removes old GitHub-based update_appimage function and related imports.
scripts/build-rio-appimage.sh Stamps channel suffix into __version__ for non-release builds; uploads AppImage + manifest via azcopy.
.github/workflows/upload-appimage.yml Uses CHANNEL and Azure secrets; comments blob URL on PRs; removes main trigger.
.github/workflows/release.yml Passes CHANNEL=release + Azure secrets to semantic-release.
tests/unit/utils/test_appimage.py Unit tests for channel parsing, URL building, manifest fetch, and swap logic.
tests/unit/test_update_command.py Unit tests asserting rio update uses the AppImage blob updater and handles non-updatable dev builds.
docs/update-channels.md Documents channels, URLs, env override, and required CI secrets.
docs/superpowers/specs/2026-06-14-rio-appimage-azure-blob-design.md Design spec for blob-hosted AppImage updates.
docs/superpowers/plans/2026-06-14-rio-appimage-azure-blob.md Implementation plan/checklist for the change.

Comment thread riocli/bootstrap.py Outdated
Comment thread riocli/utils/appimage.py Outdated
Comment thread riocli/utils/appimage.py Outdated
Comment thread scripts/build-rio-appimage.sh
Comment thread .github/workflows/upload-appimage.yml
Comment thread riocli/utils/appimage.py Outdated
Comment thread riocli/utils/appimage.py Outdated
Comment thread riocli/bootstrap.py Outdated
… accepts it

The semver-style channel suffix (X.Y.Z-devel+sha / X.Y.Z-dev.branch+sha)
is not a valid PEP 440 version, so uv build failed to build the wheel
bundled into the AppImage for devel/dev channels. Move the channel marker
into the PEP 440 local-version segment (X.Y.Z+devel.sha / X.Y.Z+dev.branch.sha)
— semver parses it as build metadata — and detect the channel from .build
instead of .prerelease.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jun 14, 2026

Copy link
Copy Markdown

🤖 PR AppImage: rio-ae5cd0b-x86_64.AppImage

amitsingh21 and others added 2 commits June 14, 2026 07:26
Copilot review caught a feature-breaking bug: the custom AppRun execs the
bundled interpreter (.../python3.13), so sys.executable contains 'python'
and is_pip_installation() returned True inside the AppImage — routing
rio update down the pip path and never reaching the blob updater. Detect
the AppImage via the APPIMAGE env var (set by the AppImage runtime to the
.AppImage path) and replace that file as the target, not sys.executable.

Also:
- remove the GitHub-hosted upload-artifact step (Azure blob is the source
  of truth; eliminates the confusing github.com download)
- guard the PR comment step when the Azure secret is absent (fork PRs)
- mkstemp inside try + bare raise in download_and_replace
- remove the now-dead is_pip_installation helper

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- bootstrap: wrap channel_for_version in try/except (graceful CLI error on
  an invalid version instead of an unhandled ValueError) [#1]
- appimage.fetch_manifest: validate JSON is an object with version/file/
  sha256; raise a clear error instead of a downstream KeyError [#2]
- appimage.download_and_replace: stream the download with incremental
  sha256 (bounded memory for tens-of-MB AppImages) and clean up the temp
  file on ANY failure, not just OSError [#3]
- build script + workflow: sanitize the dev branch consistently (SAFE_BRANCH)
  for the blob path and the PR-comment URL so they always match [#4]
- appimage.download_and_replace: default target to APPIMAGE env when present,
  falling back to sys.executable [#7]

(#5 fork-PR comment guard, #6 mkstemp-in-try/bare-raise, #8 APPIMAGE
dispatch were fixed in 022b9d2.)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@ankitrgadiya ankitrgadiya left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Simplification-focused review — one confirmed correctness regression (misleading error message on network failure during AppImage download) plus several simplification opportunities: drop the AI-agent planning/spec docs (62% of this PR's added lines, already stale vs. the shipped code), fix the build script now mutating the tracked bootstrap.py on local runs, de-duplicate the branch-sanitization logic across two files, and trim repeated boilerplate in bootstrap.py's update(). Full context in the PR conversation summary.

Comment thread riocli/bootstrap.py
appimage_path = os.environ.get("APPIMAGE")

if not appimage_path:
available, latest = check_for_updates(__version__)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

todo: The channel_for_version function expects different prefixes for devel builds and production builds. But the __version__ variable is not updated to match that expectation.

Comment thread riocli/utils/appimage.py
raise ValueError("Checksum mismatch for the downloaded AppImage")
os.chmod(tmp_path, 0o755)
os.replace(tmp_path, target)
except OSError:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This except OSError: also catches network failures from the requests.get(...) call above, not just the os.chmod/os.replace swap. requests.exceptions.RequestException (and thus ConnectionError, Timeout, HTTPError from raise_for_status()) subclasses OSError in this repo's requests version — confirmed:

>>> issubclass(requests.exceptions.ConnectionError, OSError)
True

So a flaky connection or a 403/404 from blob storage during the download will print "Please consider running as a root user." — a misleading message unrelated to the actual failure. This is a regression vs. the old update_appimage, which scoped that root-user warning to only the file-swap step (its own requests.get had a separate, generic except Exception). Consider splitting the download and the swap into separate try blocks, or excluding requests.RequestException from the OSError branch.

# CHANNEL is set by the calling workflow (release|devel|dev). Release
# builds are versioned upstream by bump-version.sh and are left as-is.
# cwd = repo root here, so riocli/bootstrap.py has no leading ../
CHANNEL="${CHANNEL:-dev}"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This script can no longer be run locally without side effects. CHANNEL defaults to dev when unset (the case for any ad-hoc local run), so this branch always executes and sed -i on line 48 rewrites the tracked riocli/bootstrap.py in place, leaving an uncommitted diff a developer has to notice and discard. The azcopy install a few lines up (17-22) is also unconditional, adding a network dependency to what used to be a side-effect-free build script.

Notably, the companion planning doc's own "dry run" step manually backs up and restores bootstrap.py around this exact logic (cp riocli/bootstrap.py /tmp/bootstrap.bak ...), which suggests the mutation was recognized as unsafe during development — that safety net just didn't make it into the actual script.

Suggest pulling the CI-only concerns (version stamping, azcopy install, blob upload block at the bottom) out of this script — either into a separate script invoked as a follow-up CI step, or directly as run: steps in upload-appimage.yml/release.yml — so build-rio-appimage.sh stays a pure "build an AppImage from the current tree" script that's safe to run by hand.

azcopy copy "${APPIMAGE_FILE}" "${DEST}" --overwrite=true
else
SHA256=$(sha256sum "${APPIMAGE_FILE}" | cut -d' ' -f1)
VERSION=$(grep -m1 '^__version__' ../riocli/bootstrap.py | sed -E 's/.*"([^"]+)".*/\1/')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This reassigns VERSION, which was already exported above (line 103/105) for appimagetool's file-naming convention, to a different value here (the stamped __version__ for the manifest). Same name, two unrelated meanings in the same script — worth a distinct name like MANIFEST_VERSION to avoid confusing a future edit.

fi
# Same sanitization as build-rio-appimage.sh so the link matches the
# actual dev/<safe-branch>/ upload path.
BRANCH=$(echo "${{ github.head_ref }}" | tr -c '0-9A-Za-z' '-' | sed -E 's/-+/-/g; s/^-|-$//g')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This branch-sanitization one-liner (tr -c '0-9A-Za-z' '-' | sed -E 's/-+/-/g; s/^-|-$//g') is duplicated verbatim in scripts/build-rio-appimage.sh (the SAFE_BRANCH computation). If one is edited without the other, this PR-comment link silently stops matching the real upload path. Consider computing the branch slug once (e.g. exposed as a step/job output from the build step) instead of keeping two copies in sync by comment convention.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This file (plus the companion spec doc) is ~994 of the ~1613 added lines in this PR (62%) and is the first thing ever added under docs/superpowers/ in this repo's history. It reads as an AI-agent working/planning artifact (checkbox task lists, "For agentic workers" instructions) rather than product documentation, and it's already stale relative to the shipped code: it describes a semver prerelease channel scheme (10.6.0-devel+abc1234), while the actual appimage.py and docs/update-channels.md ended up using a PEP 440 local-version scheme (10.6.0+devel.abc1234) — a deliberate late change (uv build requires PEP 440-valid versions) that never made it back into this doc. Suggest dropping this and the spec doc from the PR — docs/update-channels.md already captures the accurate, durable reference documentation.

Comment thread riocli/bootstrap.py
if not appimage_path:
available, latest = check_for_updates(__version__)
if not available:
click.secho("🎉 You are using the latest version", fg=Colors.GREEN)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The "already latest" message, the confirm prompt, and the try: ... except Exception as e: click.secho(f"{Symbols.ERROR} Failed to update: {e}", ...); raise SystemExit(1) from e pattern each repeat 2-3 times across the pip and AppImage branches of this function (lines 138/177, 142/182, 146/173/187). A couple of small helpers would trim this by ~15 lines without hurting readability.

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