From 17abd55552ac1d6231da968b382ffd8aae9f33d3 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Tue, 24 Mar 2026 18:29:13 -0400 Subject: [PATCH] Fix update detection for pseudo-version builds The CLI treated any different GitHub release tag as an available update, even when the current build was actually newer than the latest tagged release. This caused outputs like: - current: 0.4.1-0.20260313174735-243815fa23b2 - update available: 0.4.0 Use semantic version comparison for doctor, upgrade, and background update notices so updates are only shown when the latest release is actually newer than the current build. Tests: - go test ./internal/commands/... --- go.mod | 1 + go.sum | 2 ++ internal/commands/doctor.go | 4 +-- internal/commands/doctor_test.go | 15 +++++++++++ internal/commands/update_notice.go | 2 +- internal/commands/update_notice_test.go | 14 ++++++++++ internal/commands/upgrade.go | 2 +- internal/commands/upgrade_test.go | 16 +++++++++++ internal/commands/version_compare.go | 33 +++++++++++++++++++++++ internal/commands/version_compare_test.go | 27 +++++++++++++++++++ 10 files changed, 112 insertions(+), 4 deletions(-) create mode 100644 internal/commands/version_compare.go create mode 100644 internal/commands/version_compare_test.go diff --git a/go.mod b/go.mod index 631803072..ace5a0ed3 100644 --- a/go.mod +++ b/go.mod @@ -20,6 +20,7 @@ require ( github.com/spf13/pflag v1.0.10 github.com/stretchr/testify v1.11.1 github.com/zalando/go-keyring v0.2.7 + golang.org/x/mod v0.34.0 golang.org/x/sys v0.42.0 golang.org/x/text v0.35.0 gopkg.in/yaml.v3 v3.0.1 diff --git a/go.sum b/go.sum index ade8f85e6..31a8e4688 100644 --- a/go.sum +++ b/go.sum @@ -163,6 +163,8 @@ github.com/zalando/go-keyring v0.2.7/go.mod h1:tsMo+VpRq5NGyKfxoBVjCuMrG47yj8cma go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI= golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo= +golang.org/x/mod v0.34.0 h1:xIHgNUUnW6sYkcM5Jleh05DvLOtwc6RitGHbDk4akRI= +golang.org/x/mod v0.34.0/go.mod h1:ykgH52iCZe79kzLLMhyCUzhMci+nQj+0XkbXpNYtVjY= golang.org/x/net v0.38.0 h1:vRMAPTMaeGqVhG5QyLJHqNDwecKTomGeqbnfZyKlBI8= golang.org/x/net v0.38.0/go.mod h1:ivrbrMbzFq5J41QOQh0siUuly180yBYtLp+CKbEaFx8= golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= diff --git a/internal/commands/doctor.go b/internal/commands/doctor.go index 293a79eeb..d6db6df0c 100644 --- a/internal/commands/doctor.go +++ b/internal/commands/doctor.go @@ -252,8 +252,8 @@ func checkVersion(verbose bool) Check { check.Message = v // Try to check for latest version (non-blocking, best effort) - latest, err := fetchLatestVersion() - if err == nil && latest != "" && latest != v { + latest, err := versionChecker() + if err == nil && isUpdateAvailable(v, latest) { check.Status = "warn" check.Message = fmt.Sprintf("%s (update available: %s)", v, latest) check.Hint = "Run: basecamp upgrade" diff --git a/internal/commands/doctor_test.go b/internal/commands/doctor_test.go index dd92f7367..55cf290e9 100644 --- a/internal/commands/doctor_test.go +++ b/internal/commands/doctor_test.go @@ -116,6 +116,21 @@ func TestCheckVersion(t *testing.T) { assert.Contains(t, checkVerbose.Message, "commit") } +func TestCheckVersionSuppressesOlderLatestRelease(t *testing.T) { + origVersion := version.Version + version.Version = "0.4.1-0.20260313174735-243815fa23b2" + defer func() { version.Version = origVersion }() + + origChecker := versionChecker + versionChecker = func() (string, error) { return "0.4.0", nil } + defer func() { versionChecker = origChecker }() + + check := checkVersion(false) + assert.Equal(t, "pass", check.Status) + assert.Equal(t, "0.4.1-0.20260313174735-243815fa23b2", check.Message) + assert.Empty(t, check.Hint) +} + func TestCheckSDKProvenance(t *testing.T) { // Non-verbose: shows version string (works for both pseudo-versions and semver) check := checkSDKProvenance(false) diff --git a/internal/commands/update_notice.go b/internal/commands/update_notice.go index faf49c5e0..71557bfc6 100644 --- a/internal/commands/update_notice.go +++ b/internal/commands/update_notice.go @@ -85,7 +85,7 @@ func (uc *UpdateCheck) Notice() string { return "" } - if uc.latest == "" || uc.latest == version.Version { + if !isUpdateAvailable(version.Version, uc.latest) { return "" } diff --git a/internal/commands/update_notice_test.go b/internal/commands/update_notice_test.go index b54217450..b3f3a0055 100644 --- a/internal/commands/update_notice_test.go +++ b/internal/commands/update_notice_test.go @@ -185,6 +185,20 @@ func TestNotice_NonBlocking(t *testing.T) { assert.Equal(t, "", uc.Notice()) } +func TestNotice_SuppressesOlderLatestVersion(t *testing.T) { + origVersion := version.Version + version.Version = "0.4.1-0.20260313174735-243815fa23b2" + defer func() { version.Version = origVersion }() + + uc := &UpdateCheck{ + latest: "0.4.0", + done: make(chan struct{}), + } + close(uc.done) + + assert.Equal(t, "", uc.Notice()) +} + func TestUpdateCacheRoundTrip(t *testing.T) { configDir := t.TempDir() t.Setenv("XDG_CONFIG_HOME", configDir) diff --git a/internal/commands/upgrade.go b/internal/commands/upgrade.go index efe0a83fb..860150329 100644 --- a/internal/commands/upgrade.go +++ b/internal/commands/upgrade.go @@ -55,7 +55,7 @@ func runUpgrade(cmd *cobra.Command, args []string) error { return fmt.Errorf("could not check for updates: %w", err) } - if latest == current { + if !isUpdateAvailable(current, latest) { fmt.Fprintln(w, "already up to date") return app.OK( map[string]string{"status": "up_to_date", "version": current}, diff --git a/internal/commands/upgrade_test.go b/internal/commands/upgrade_test.go index 884226fc9..388bd2a35 100644 --- a/internal/commands/upgrade_test.go +++ b/internal/commands/upgrade_test.go @@ -86,6 +86,22 @@ func TestUpgradeAvailable(t *testing.T) { assert.Contains(t, appBuf.String(), "releases/tag/v1.3.0") } +func TestUpgradeSuppressesOlderLatestRelease(t *testing.T) { + app, appBuf := setupPeopleTestApp(t) + + orig := version.Version + version.Version = "0.4.1-0.20260313174735-243815fa23b2" + t.Cleanup(func() { version.Version = orig }) + + stubUpgradeCheckers(t, "0.4.0", false) + + cmdOut, err := executeUpgradeCommand(t, app) + require.NoError(t, err) + assert.Contains(t, cmdOut, "already up to date") + assert.Contains(t, appBuf.String(), "up_to_date") + assert.NotContains(t, cmdOut, "update available") +} + // TestUpgradeOutputGoesToWriter verifies output uses cmd.OutOrStdout(), not os.Stdout. func TestUpgradeOutputGoesToWriter(t *testing.T) { app, _ := setupPeopleTestApp(t) diff --git a/internal/commands/version_compare.go b/internal/commands/version_compare.go new file mode 100644 index 000000000..73d57d1b2 --- /dev/null +++ b/internal/commands/version_compare.go @@ -0,0 +1,33 @@ +package commands + +import ( + "strings" + + "golang.org/x/mod/semver" +) + +// isUpdateAvailable reports whether latest is newer than current. +// +// Versions are compared as semantic versions with an optional leading "v". +// This correctly handles prerelease/pseudo versions such as +// "0.4.1-0.20260313174735-243815fa23b2", which should compare newer than +// "0.4.0". +func isUpdateAvailable(current, latest string) bool { + current = normalizeSemver(current) + latest = normalizeSemver(latest) + if !semver.IsValid(current) || !semver.IsValid(latest) { + return latest != "" && latest != current + } + return semver.Compare(latest, current) > 0 +} + +func normalizeSemver(v string) string { + v = strings.TrimSpace(v) + if v == "" { + return "" + } + if !strings.HasPrefix(v, "v") { + v = "v" + v + } + return v +} diff --git a/internal/commands/version_compare_test.go b/internal/commands/version_compare_test.go new file mode 100644 index 000000000..0208b8b8b --- /dev/null +++ b/internal/commands/version_compare_test.go @@ -0,0 +1,27 @@ +package commands + +import "testing" + +func TestIsUpdateAvailable(t *testing.T) { + tests := []struct { + name string + current string + latest string + want bool + }{ + {name: "newer stable release", current: "1.0.0", latest: "1.1.0", want: true}, + {name: "same version", current: "1.0.0", latest: "1.0.0", want: false}, + {name: "older latest suppressed", current: "1.1.0", latest: "1.0.0", want: false}, + {name: "pseudo current newer than older release", current: "0.4.1-0.20260313174735-243815fa23b2", latest: "0.4.0", want: false}, + {name: "release newer than pseudo prerelease of same base", current: "0.4.1-0.20260313174735-243815fa23b2", latest: "0.4.1", want: true}, + {name: "invalid fallback treats different as update", current: "custom-build", latest: "1.0.0", want: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isUpdateAvailable(tt.current, tt.latest); got != tt.want { + t.Fatalf("isUpdateAvailable(%q, %q) = %v, want %v", tt.current, tt.latest, got, tt.want) + } + }) + } +}