Skip to content

feat: implement update checker logic and tests - #1232

Draft
Anne (Ant1gua) wants to merge 6 commits into
nextfrom
feat/issue-1091-tell-new-version-available
Draft

feat: implement update checker logic and tests#1232
Anne (Ant1gua) wants to merge 6 commits into
nextfrom
feat/issue-1091-tell-new-version-available

Conversation

@Ant1gua

@Ant1gua Anne (Ant1gua) commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

What changed?

  • Added package internal/update.go that holds logic to check if an CLI update is available.
  • Updated cmd/root.go to run the update check in a background goroutine and print an update notification if an update is available.

Implementation heavily inspired by the github cli repository

Why?

Users have no way of knowing whether their installed version is current or out of date.

Implementation Details

Flow [update available and requirements met]:

  1. starts update check in background go-routine when command is executed
  2. verifies first that requirements for update check are met. *
  3. fetches release info from our GitHub pages (see ci: publish version.json on GitHub Pages and rebuild on release #1248)
  4. saves/loads release info in/from cache
  5. compares fetched version against build version
  6. determines installation context
  7. renders notification with release info and fitting update instructions

(*) Requirements for running an update check:

  • cached release info is older (time of fetching) than 24h (-> defined in updateCheckInterval, so can be adjusted)
  • user has not disabled the upate notifications via CLI flag or environment variable.
  • not running as dev build or in CI environment / GitHub Action

Other noteworthy details:

  • notification is written to stderr meaning it is written after the command finishes
  • User can discable check through CLI flag or envariable
  • Homebrew users are not notified immediately after a release, as it can take up to a day for the new version to become available through Homebrew
  • the notification will only be shown every 24 hours (updateCheckInterval) to reduce noise, because timestamp of release fetching == last time the notification was shown (in most cases)

How was it tested?

Unit tests in internal/update/update_test.go cover:

  • Version comparisons across multiple scenarios, including newer, identical, older, pre-release, and source-build versions.
  • Cache read/write round trips.
  • Skipping network requests while the cache is still fresh.
  • ShouldCheckForUpdate behavior for all suppression conditions, including CI, development builds, and user opt-out.
  • Installation-context detection for Homebrew and APT.

Feedback wanted

✨ Would especially appreciate feedback on the following things:

  • Notifications are based on when the release information was last fetched. Because the cache is refreshed only once every 24 hours, users may not be notified until up to 24 hours after a new release. Is this delay acceptable?
  • is the current installation-context detection reliable enough?
  • is the update-check timeout too short? Depending on the network
    connection I sometimes receive a context canceled error caused by the timeout.
  • the notification is not displayed when command execution returns early with exit code 1. Is that acceptable or should update notifications also be shown after failed commands?
  • Does the Shopware CLI even run in CI or as part of a GitHub Action in scenarios?
  • How can I reliably detect a manually downloaded binary?
  • If version.json becomes the source of truth for the latest CLI release, should this be documented to ensure future release-process changes continue to update it?

Related issue or description

#1091

@Ant1gua Anne (Ant1gua) self-assigned this Jul 22, 2026
Comment thread internal/update/update.go Outdated
@Ant1gua
Anne (Ant1gua) force-pushed the feat/issue-1091-tell-new-version-available branch from 37681cc to a492b5e Compare July 29, 2026 09:24
@Ant1gua
Anne (Ant1gua) force-pushed the feat/issue-1091-tell-new-version-available branch from 5f754fe to e930b6a Compare July 30, 2026 12:26
@Ant1gua
Anne (Ant1gua) force-pushed the feat/issue-1091-tell-new-version-available branch 3 times, most recently from a34ffff to 85ea73b Compare July 31, 2026 10:31
@Ant1gua
Anne (Ant1gua) requested a review from Copilot August 3, 2026 05:40

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 introduces an update-checking subsystem for shopware-cli, including a cached version manifest fetch, user-managed notification preferences, and a CLI notification shown after command execution.

Changes:

  • Added internal/update package to fetch/cache latest release info and decide whether update checks should run.
  • Added update-notifications command to enable/disable/show update notification preference.
  • Wired an asynchronous update check into cmd/root.go to print a styled update notice to stderr when an update is available.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File Description
internal/update/update.go Implements update checking, caching, CI/dev suppression, install-context detection, and notification rendering.
internal/update/update_test.go Adds unit tests for version comparison, caching, gating logic, and install-context detection.
cmd/update_notifications.go Adds update-notifications {status,enable,disable} for managing persisted opt-out.
cmd/root.go Runs update check in a background goroutine and prints update notification at end of run.

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

Comment thread internal/update/update.go
Comment on lines +332 to +335
brewPrefixBytes, err := exec.CommandContext(context.Background(), brewExe, "--prefix").Output()
if err != nil {
return false
}
Comment thread internal/update/update.go
Comment on lines +79 to +83
// Fetch latest release info.
latestReleaseInfo, err := fetchLatestReleaseInfo(ctx, client)
if latestReleaseInfo == nil || err != nil {
return nil, err
}
Comment thread internal/update/update.go
Comment on lines +74 to +76
if !lastCheck.IsZero() && time.Since(lastCheck) < updateCheckInterval {
return nil, ErrNoUpdateAvailable
}
Comment thread internal/update/update.go
Comment on lines +136 to +148
if version == "dev" {
return false
}

if IsCI() {
return false
}

if IsGitHubActions() {
return false
}

return true
Comment thread cmd/root.go
Comment on lines 106 to 109
if err != nil {
logging.FromContext(ctx).Errorln(err)
return 1
}
Comment thread cmd/update_notifications.go Outdated
@Ant1gua
Anne (Ant1gua) force-pushed the feat/issue-1091-tell-new-version-available branch 2 times, most recently from b265fae to 7a085dd Compare August 3, 2026 08:38
@Ant1gua
Anne (Ant1gua) force-pushed the feat/issue-1091-tell-new-version-available branch from 7a085dd to 7744dc2 Compare August 3, 2026 09:11
@Ant1gua
Anne (Ant1gua) requested a review from Copilot August 3, 2026 09:13

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (6)

internal/update/update.go:67

  • When a cache entry is still fresh (< updateCheckInterval), CheckForUpdate returns ErrNoUpdateAvailable without comparing the cached version to the installed version. This means users won’t get an update hint on subsequent runs within 24h even if the cached version is newer, and it also prevents using the cache to avoid network calls while still reporting available updates.
		if !lastCheck.IsZero() && time.Since(lastCheck) < updateCheckInterval {
			return nil, ErrNoUpdateAvailable
		}

internal/update/update.go:284

  • IsUnderHomebrew runs brew --prefix with context.Background(), so a slow/hung brew invocation can delay or hang CLI shutdown when rendering the update hint. Use a bounded timeout for this probe.
	brewPrefixBytes, err := exec.CommandContext(context.Background(), brewExe, "--prefix").Output()
	if err != nil {
		return false
	}

internal/update/update_test.go:341

  • This test enforces that update hints are suppressed for the full updateCheckInterval (second call returns ErrNoUpdateAvailable). That behavior contradicts the goal of showing an update notification when an update is available; caching should primarily avoid the network call, not suppress returning cached update info.
	second, err := CheckForUpdate(t.Context(), "v0.1.0", client)
	require.ErrorIs(t, err, ErrNoUpdateAvailable)
	assert.Nil(t, second)

internal/update/update.go:123

  • ShouldCheckForUpdate treats "-n" as a no-update flag, but "-n" is already used for "--no-interaction" in cmd/root.go. This unintentionally disables update hints whenever users run with --no-interaction/-n.
		for _, arg := range args {
			if arg == "--no-update-hint" || arg == "-n" {
				return false
			}
		}

internal/update/update_test.go:129

  • This test currently expects ErrNoUpdateAvailable when the cache is still fresh, even though the cached version is newer than the installed version. If CheckForUpdate is meant to use the cache to avoid network calls while still reporting available updates, the cached release should be returned here with requestCount staying at 0.

This issue also appears on line 338 of the same file.

	rel, checkErr := CheckForUpdate(t.Context(), "v1.0.0", client)
	require.ErrorIs(t, checkErr, ErrNoUpdateAvailable)
	assert.Nil(t, rel)
	assert.Equal(t, 0, requestCount)

cmd/root.go:122

  • run() blocks on <-updateChan after command execution. For fast commands, this can add up to ~300ms to every invocation (or longer if the goroutine stalls), which defeats the intent of doing the update check in the background.
	// Wait for the update check to finish and print a message to stderr if an update is available
	newRelease := <-updateChan
	if newRelease != nil {
		binaryPath, err := os.Executable()
		if err != nil {

@Ant1gua
Anne (Ant1gua) force-pushed the feat/issue-1091-tell-new-version-available branch from 7744dc2 to b7c1776 Compare August 3, 2026 17:27
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.

CLI tells user when their installed CLI version is current or not

4 participants