Skip to content

upgrade: package rework, --pick-version picker, install.sh gate#262

Merged
l-hellmann merged 13 commits into
mainfrom
refactor/upgrade-package
May 26, 2026
Merged

upgrade: package rework, --pick-version picker, install.sh gate#262
l-hellmann merged 13 commits into
mainfrom
refactor/upgrade-package

Conversation

@l-hellmann
Copy link
Copy Markdown
Collaborator

What Type of Change is this?

  • New Feature
  • Fix
  • Improvement
  • Release
  • Other

Description (required)

Larger rework of the upgrade flow ahead of the next minor release (v1.1.0), the first one that ships `checksums.txt` and is therefore reachable by `zcli upgrade`. Older releases need install.sh; the command and `--check` flow now surface that.

Major user-facing changes:

  • Fix `zcli upgrade --check` false-positive on local builds past the latest tag. `git describe` stamps `vX.Y.Z-N-gHASH`, which semver parses as a pre-release ranking below `vX.Y.Z`. The Makefile rewrites the suffix to `+N.gHASH` (build metadata, ignored by `semver.Compare`), and the command's "needs upgrade" check switched from string equality to a semver-aware `Plan.NeedsUpgrade`.
  • Refuse pre-v1.1.0 targets with an install.sh hint. `Plan.RequireSelfUpgradable()` checks the chosen tag and tells the user to run install.sh for older releases, both in the Apply path (error) and in `--check --version` (warning + new exit code 3). The hint contains the curl command pre-filled with the picked tag.
  • `--check --version` prints Current / Target / Latest. Latest is read from the on-disk cache (background `RefreshCacheIfStale` keeps it warm); previously the Latest line was actually the user-pinned target, mislabeled.
  • `--no-cache` flag. Bypasses the disk version cache and resolves "latest" from the API directly; impacts both the Plan resolution and the Latest line in `--check --version`.
  • `--download-timeout` flag. Go duration string, 0 disables the timeout. Threaded through `Upgrader.WithDownloadTimeout`.
  • `--pick-version` flag (+ `--include-pre-release`). Opens a TUI selector listing every GitHub release. Pre-v1.1.0 rows are selectable and show "requires install.sh"; picking one prints the install.sh hint as a warning and exits cleanly without attempting Apply. Page size capped at 15 rows (new `selector.WithMaxRowsPerPage`).
  • Verify `--version` tag exists. A HEAD on the platform binary URL up front turns typos like `--version v1.2.3.4` into "release X does not exist" instead of a confusing checksums-fetch error mid-Apply.
  • Stamp install channel for `make install`. Defaults to `manual`; override with `make install CHANNEL=brew` for package-tap clones.

Refactor under the hood:

  • Renamed `src/version` → `src/upgrade` with symbol cleanup (`UpgradePlan`/`UpgradeOptions`/`Upgrade` → `Plan`/`Options`/`Apply`). `Plan` is a value type with unexported fields and `Current()`/`Target()` accessors.
  • Introduced `upgrade.Upgrader` that bundles per-binary state and dependencies (current version, channel, release URL template, list URL, apply func, download timeout). Eliminates every test-only `SetXForTest` helper; same-package tests construct bare `Upgrader{}` literals, cross-package tests inject through new env vars (`ZEROPS_CLI_VERSION`, `ZEROPS_RELEASES_URL`, `ZEROPS_RELEASES_LIST_URL`) that mirror the existing `ZEROPS_VERSION_API_URL` pattern.
  • Ported the `src/upgrade` test files to testify (`assert`/`require`) for consistency with the rest of the codebase.

l-hellmann added 13 commits May 26, 2026 13:50
…dd --download-timeout

Rename src/version -> src/upgrade. Symbols renamed for clarity now that
the package name no longer doubles as a prefix: UpgradePlan -> Plan,
UpgradeOptions -> Options, Upgrade -> Apply. Plan is a value type with
unexported fields and Current()/Target() accessors.

Introduce Upgrader bundling per-binary state and dependencies the
upgrade flow needs (current version, channel, release-asset URL
template, apply func, download timeout). NewUpgrader builds the
production instance; tests construct their own with custom fields so
no test-only SetXForTest helpers or mutable package globals are
needed. Methods that depend on those fields (PlanUpgrade, Apply,
Detect, RequireSelfUpdatable, PrintVersionCheck, MismatchWarning)
moved onto Upgrader.

For cross-package integration tests, NewUpgrader honors a new
VersionEnvVar (ZEROPS_CLI_VERSION) before falling back to the
ldflag-stamped value, mirroring how VersionApiUrlEnvVar overrides
the API URL.

Fix `zcli upgrade --check` reporting "behind" on local builds past
the latest tag. git-describe stamps `vX.Y.Z-N-gHASH`, which semver
parses as a pre-release ranking below vX.Y.Z. The Makefile now
rewrites the suffix to `+N.gHASH` (build metadata, ignored by
semver.Compare), and the upgrade command's "needs upgrade?" check
switched from string equality to a semver-aware Plan.NeedsUpgrade
method.

Stamp the install channel for `make install` (defaults to "manual";
override with `make install CHANNEL=brew`), matching what
.goreleaser.yaml sets for each artifact.

Add --download-timeout flag to `zcli upgrade` (Go duration string,
0 disables the timeout, default 2m), threaded through via
Upgrader.WithDownloadTimeout.
Releases before v1.1.0 didn't ship checksums.txt, so Apply would 404
mid-way at the checksums fetch with a confusing error. Add
Plan.RequireSelfUpgradable as the upfront gate (mirroring
Upgrader.RequireSelfUpdatable) that explains the issue and tells the
user to use install.sh to pin an older release:

  curl -fsSL https://raw.githubusercontent.com/zeropsio/zcli/main/install.sh | sh -s -- vX.Y.Z

The command checks this after RequireSelfUpdatable and before the
prompt/Apply path. --check is left alone (it never touches the
checksums endpoint and its existing semantics are sufficient).
Run Plan.RequireSelfUpgradable inside --check so scripts can tell
"target requires install.sh" apart from "actual upgrade available".
Stdout still prints the current/latest pair; the install.sh hint
goes to stderr. New exit code 3 for this case (0 up to date, 1
behind, 2 lookup error, 3 target unreachable).

Updated TestUpgradeCheckAheadOfTagViaBuildMetadata to use v1.1.0 as
the released tag (was v1.0.67, which now also trips the
reachability gate and would conflate the two concerns).
PlanUpgrade now HEADs the platform binary URL when the user pinned a
target (--version vX.Y.Z), so typos like `--version v1.2.3.4` or
`--version vfoo` fail fast with "release vXXX does not exist" instead
of mid-Apply with a confusing checksums-fetch error.

In --check mode the error returns exit 2 (existing "lookup error"
code) so the same exit-code contract continues to mean "we couldn't
determine the answer".

Verification is gated on u.releasesURL != "" so same-package tests
that construct bare Upgrader literals (TestPlanUpgradeAlwaysSucceeds)
keep working without standing up a release-asset stub server. The
production NewUpgrader always sets releasesURL, so production always
verifies.

Add ReleasesURLEnvVar (ZEROPS_RELEASES_URL) so the cmd integration
tests can point the release endpoint at httptest, mirroring how
VersionApiUrlEnvVar overrides the API URL. The fixture wires it
automatically; tests opt in to specific tags via stubReleaseTag.
Unregistered tags fall through to the default 404, which is exactly
what "release does not exist" lookups expect.
…rning

Route the install.sh hint through cmdData.UxBlocks.PrintWarningText so
it picks up the same "! WARN" styling other warnings use (e.g. the
background MismatchWarning) instead of a bare stderr printf. Exit
code 3 contract is unchanged.
When --version pins a target, the existing output (Current/Latest)
was misleading because Latest was actually the user's target. Split
into three lines now:

  Current: v1.1.0
  Target:  v1.2.3
  Latest:  v9.9.9   (from disk cache; omitted if cache empty)

Without --version the output is unchanged (Current/Latest, where
Latest comes from the API as before).

Latest is read via the new Upgrader.CachedLatest() which never makes
a network call - the background RefreshCacheIfStale on every command
invocation keeps it warm. Added stubLatestCache fixture helper that
writes the cache JSON layout so cmd integration tests don't depend on
the upgrade package's unexported cacheEntry/apiResponse types.
Bypasses the on-disk version cache when resolving "latest". When set:

* PlanUpgrade fetches directly from the version API instead of going
  through fetch() (which checks the disk cache first). The result is
  written back to the cache so a follow-up call without --no-cache
  sees the same value.
* In --check --version output, the Latest line is read fresh from
  the API too via the new Upgrader.LatestTag(ctx, noCache) helper.

Has no effect on Apply itself (binary download is never cached) or
on the passive background MismatchWarning (which by design only ever
reads the cache).
Added five why-comments where the reasoning behind a hunk isn't
recoverable from the code alone:

  * --download-timeout's StringFlag-then-parse pattern (cmdBuilder
    has no DurationFlag).
  * --check's bespoke error-to-exit-code conversion (scripting
    interface vs the styled error printer).
  * Why the Latest line is looked up separately under --version.
  * Channel gate ordering vs the target gate.
  * The both-conditions guard on the "already on" early return.

No code changes.
Adds an interactive selector listing every release from GitHub (or
the configured override). Pre-v1.1.0 entries are shown with a
"requires install.sh" status and marked disabled so the user can see
the tag name without picking something `zcli upgrade` can't fulfil.

Implementation pieces:

* upgrade.Release + Upgrader.AvailableReleases - hits the releases
  list endpoint and filters out drafts and non-semver tags.
  SelfUpgradable is precomputed against firstSelfUpgradableTag so
  the cmd doesn't recompute per row.
* defaultReleasesListURL points at api.github.com/.../releases,
  overridable via ReleasesListURLEnvVar (mirrors the existing
  VersionApiUrlEnvVar / ReleasesURLEnvVar pattern).
* --pick-version is mutually exclusive with --version; setting both
  fails up front before any network call.
* pickReleaseInteractive (cmd) renders the table-backed selector
  using the same helpers PrintOrgSelector etc. use.

Tests cover the JSON parsing (draft/non-semver filtering,
SelfUpgradable computation) and the flag-conflict path. The TUI
itself only runs in a real terminal, so it's not exercised by the
integration suite.
…-version

* AvailableReleases gained an includePrerelease bool. By default it
  drops anything GitHub flagged as a prerelease or whose tag carries a
  semver prerelease component (e.g. v1.5.0-rc.1); --include-pre-release
  keeps them in the list. Release also exposes a Prerelease bool so
  the picker can label them.
* Picker status column distinguishes stable / pre-release / requires
  install.sh. Pre-release rows are still selectable; only pre-v1.1.0
  rows stay disabled.
* selector.WithMaxRowsPerPage caps the page size; the upgrade picker
  passes 15 so long release histories paginate predictably instead of
  filling the terminal.

Expanded TestAvailableReleases to cover both prerelease sources
(semver suffix + GitHub flag) under both modes.
Pre-v1.1.0 rows in the --pick-version picker are no longer disabled.
When the user picks one, the install.sh fallback is printed as a
uxBlock warning and the command exits cleanly - no upgrade attempt,
no error, just an actionable hint with the tag they wanted in the
copy-pasteable curl command.

The hint string was the message Plan.RequireSelfUpgradable already
emits. Extracted it into upgrade.InstallScriptHint(tag) so both the
automatic gate (returning an error) and the interactive pick (rendered
as a warning) speak with one voice; if the wording or URL ever
changes, only one place needs to follow.

pickReleaseInteractive's return type changed from string to
upgrade.Release so the caller can branch on SelfUpgradable without
re-deriving it from the tag.
The cmd integration suite already uses testify; the upgrade package's
four test files were the last holdouts using raw t.Errorf/t.Fatalf.
Convert them to require/assert for consistency with the rest of the
codebase.

assert.Equal-on-struct-slices also collapses a few hand-rolled
loops (TestAvailableReleases) into single assertions, which is a net
-100 lines across the four files.

No behavior changes; the same scenarios are covered with the same
assertions, just expressed through testify.
Three lint findings from the previous commit:

* gofmt - over-aligned trailing comments in
  TestUpgradeCheckNoCacheBypassesStaleCache.
* testifylint require-error - error assertions inside the channel
  loops in TestRequireSelfUpdatable and TestPlanUpgradeAlwaysSucceeds.
  Swap to require and wrap each iteration in t.Run so one bad
  channel doesn't suppress the others.
@l-hellmann l-hellmann self-assigned this May 26, 2026
@l-hellmann l-hellmann requested a review from tikinang May 26, 2026 13:38
@l-hellmann l-hellmann merged commit 6007908 into main May 26, 2026
10 checks passed
@l-hellmann l-hellmann deleted the refactor/upgrade-package branch May 26, 2026 15:18
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.

2 participants