fix(update): make "update available" nag actually print#188
Merged
AlephNotation merged 2 commits intomainfrom May 6, 2026
Merged
fix(update): make "update available" nag actually print#188AlephNotation merged 2 commits intomainfrom
AlephNotation merged 2 commits intomainfrom
Conversation
The startup update check was racy and broken in practice: 1. `UpdateCheckTime()` ran *before* the HTTP fetch, immediately pushing NextCheck out by the full check interval (1h). 2. The fetch ran in a detached `go func()` with no synchronization. 3. Short commands like `vers ps` exit in ~50ms — well before the goroutine's HTTP call to api.github.com completes — so the banner never printed and the goroutine got killed mid-flight. 4. On the next invocation, ShouldCheckForUpdate() returned false (we bumped NextCheck on the previous run), so no check happened either. Net effect: the nag was effectively never shown. Users (e.g. Jordan) who built from source or grabbed an older bundled binary had no way to discover that newer releases existed without manually running `vers upgrade`. This change introduces `update.MaybeNotifyUpdate` which: - Caches the latest known release tag in `~/.vers/config.json` (`update_check.latest_version`). If a previously-cached version is newer than the running build, the banner prints synchronously on every invocation with zero network I/O. - Performs a single, synchronous, bounded (800ms) HTTP refresh once per check interval — no goroutine, no race. - Only advances NextCheck after a *successful* fetch. On failure, uses a short 5-minute backoff so a flaky network at the moment of first launch doesn't suppress the nag for an hour. - Skips dev/dirty builds via a centralized `IsDevVersion` helper. - Does proper semver-aware comparison so e.g. v0.9.0 < v0.10.0 (the previous string-equality check would have flagged any difference, including downgrades). The banner now writes to stderr (was stdout) so it doesn't contaminate piped output of commands like `vers ps | grep ...`. Fully covered by unit tests in internal/update/update_test.go: fast-path-without-network, network refresh, failure backoff, dev-version skip, no-banner-when-on-latest, and a banner format regression test.
…hatch CI integration test `TestRepoLifecycle` failed on PR #188: expected 'v1' in quiet tag list, got: 💡 vers update available: v0.0.0-20260506213344-eb1e5d25fa7c -> v0.10.0 (run 'vers upgrade') Two issues: 1. The integration test runner builds the CLI via `go build` against the PR merge commit, producing a Go module pseudo-version like `v0.0.0-<timestamp>-<commit>`. The previous `IsDevVersion` only matched `dev` / `dev-*` / `*-dirty`, so the pseudo-version sailed through, hit the live GitHub API, and printed the banner. 2. `testutil.RunVers` uses `cmd.CombinedOutput()`, so even though the banner correctly writes to stderr, it still ended up in the buffer that tests parse. Tests doing `strings.TrimSpace(out) != "v1"` are particularly fragile. Fixes: - IsDevVersion now also matches `v0.0.0-...` pseudo-versions. This protects `go install`, `go run`, and any module-graph build against an untagged commit — none of which represent a published release the user can "upgrade" to. - New `VERS_NO_UPDATE_CHECK` env var (also accepts `NO_UPDATE_NOTIFIER` for cross-tool consistency with npm) silences the check entirely before any other logic runs. Truthy parsing follows convention: unset/empty/0/false/no/off mean "don't suppress", anything else suppresses. - testutil.RunVers and RunVersInDir now set `VERS_NO_UPDATE_CHECK=1` unconditionally so future integration tests are immune to this class of contamination even if a real release ships during a CI run. New tests cover pseudo-version detection, both env-var names, and falsy-value rejection.
AlephNotation
added a commit
that referenced
this pull request
May 7, 2026
Adopt the lowercase-prefix convention used by serious dev tools (gcc, rustc, clippy, go) for status messages, and drop decorative emoji that were inherited from earlier prototypes. Rationale: - Emoji and unusual glyphs in CLI output break programmatic consumption (grep / awk / jq / log shippers must handle multi-byte noise), break alignment in fixed-width displays (most emoji are East-Asian-Wide and occupy 2 cells inconsistently across terminals), and turn into '?' on non-UTF-8 transports (serial consoles, some log re-encoders, older Windows terminals). - The recent CI failure on #188 (`TestRepoLifecycle` couldn't find 'v1' because the update banner was in the buffer) is the natural consequence of putting decorative output through stdout. - None of the tools we want vers-cli to feel adjacent to (git, go, cargo, kubectl, terraform, docker, aws, gcloud, ripgrep) decorate their output with emoji. The closest, `gh`, uses only the Unicode check/cross glyphs and TTY-gates them. Replacements applied: ✓ <message> -> <message> (drop; success implied) ✗ <message> -> error: <message> (lowercase compiler-style prefix)⚠️ <message> -> warning: <message> 💡 <message> -> note: <message> 📧 Verification email -> Verification email (drop; text already explanatory) Println("✓") -> Println("ok") Println("✗") -> Println("failed") ├─ / └─ -> |- / \\- (ASCII tree in commit lineage) → -> -> (ASCII arrow everywhere) Also lowercased the first character following the new lowercase prefixes (`error: failed to ...` rather than `error: Failed to ...`) to match clippy/rustc/gcc convention exactly. Output changes are user-facing but minor; they make grepping CLI output more reliable and the project look less prototype-y. No behavior changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Jordan flagged in Slack that he had no idea a newer
vershad shipped — the "update available" banner never printed for him. Investigation showed the startup check has been silently broken since it landed.The bug
PersistentPreRunEincmd/root.godid this:UpdateCheckTime()immediately pushesNextCheckan hour into the future.vers psexit in ~50ms — well before the HTTP call toapi.github.comfinishes — so the banner never prints and the goroutine is killed mid-flight.ShouldCheckForUpdate()returns false (we just bumped NextCheck), so no check happens either.Net effect: the nag was effectively never shown. Reproduced locally with
v0.9.0against the livev0.10.0release — banner never appeared.What this PR does
Replaces the racy goroutine with
update.MaybeNotifyUpdate, which:~/.vers/config.json(update_check.latest_version). If the cached tag is newer than the running build, the banner prints synchronously with zero network I/O on every invocation. This is the path Jordan would hit on his secondversrun.api.github.comfrom any reasonable network.NextCheckon a successful fetch. On failure (timeout, 5xx), uses a short 5-minute backoff so a flaky network at the moment of first launch doesn't suppress the nag for an hour.IsDevVersionhelper (dev,unknown,dev-<sha>,*-dirty, empty).v0.9.0 < v0.10.0. The previous string-equality check would also flag downgrades, which is wrong.vers ps | grep ...isn't contaminated.Smoke test
Built
v0.9.0locally, pointed it at a fresh$HOME, ran twice against real GitHub:Tests
New file
internal/update/update_test.gocovers:TestIsNewerSemver— semver comparison edge cases (0.9 < 0.10, missing components, prereleases, lexical fallback).TestIsDevVersion— dev / dirty / unknown / clean.TestMaybeNotifyUpdate_PrintsFromCacheWithoutNetwork— fast path: pre-seeds cache, asserts banner prints and zero HTTP calls (httptest server fails the test if hit).TestMaybeNotifyUpdate_RefreshesWhenStale— slow path: staleNextCheck, fake server returnsv0.10.0, asserts exactly 1 HTTP call, banner printed, cache +NextCheckupdated.TestMaybeNotifyUpdate_NetworkFailureDoesNotSuppressForFullInterval— server returns 500, assertsNextCheckis the 5-min backoff, not 1h.TestMaybeNotifyUpdate_DevVersionSkipsEverything— dev build → no network, no print.TestMaybeNotifyUpdate_NoBannerWhenAlreadyOnLatest— same version → no banner.TestPrintUpdateBannerFormat— banner wording regression test (downstream onboarding docs reference the "vers upgrade" phrase).Full unit test suite (
make test-unitminus the unrelated tunnel integration test) passes.Out of scope / follow-ups
file vers-linux-amd64on the latest GitHub release — already statically linked. His dynamic copy must come from somewhere other than the Releases page (likely an older bundle inside an agent-services image). Worth pinging him forwhich -a versonce he installs viainstall.sh.vers upgradeitself is unchanged. Once this PR lands, users on any release ≥ this one will start seeing the nag and discoververs upgradeorganically.