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) + } + }) + } +}