diff --git a/pkg/cli/update_check.go b/pkg/cli/update_check.go index 4870dd573c2..aad411f0ec7 100644 --- a/pkg/cli/update_check.go +++ b/pkg/cli/update_check.go @@ -25,9 +25,10 @@ const ( // Release represents a GitHub release type Release struct { - TagName string `json:"tag_name"` - Name string `json:"name"` - HTMLURL string `json:"html_url"` + TagName string `json:"tag_name"` + Name string `json:"name"` + HTMLURL string `json:"html_url"` + Prerelease bool `json:"prerelease"` } // shouldCheckForUpdate determines if we should check for updates based on: @@ -222,7 +223,14 @@ func getLatestRelease() (string, error) { return "", fmt.Errorf("failed to query latest release: %w", err) } - updateCheckLog.Printf("Latest release: %s", release.TagName) + updateCheckLog.Printf("Latest release: %s (prerelease: %v)", release.TagName, release.Prerelease) + + // /releases/latest already excludes prereleases per the GitHub API contract, + // but guard defensively in case the response ever changes. + if release.Prerelease { + return "", nil + } + return release.TagName, nil } diff --git a/pkg/cli/update_workflows.go b/pkg/cli/update_workflows.go index a8b8643a2ce..01769177c17 100644 --- a/pkg/cli/update_workflows.go +++ b/pkg/cli/update_workflows.go @@ -276,6 +276,13 @@ func getLatestBranchCommitSHA(repo, branch string) (string, error) { return sha, nil } +// runWorkflowReleasesAPIFn calls the GitHub Releases API for the given repository and +// returns the newline-delimited tag names. It is a package-level variable so that +// tests can replace it without spawning real gh CLI processes. +var runWorkflowReleasesAPIFn = func(repo string) ([]byte, error) { + return workflow.RunGH("Fetching releases...", "api", fmt.Sprintf("/repos/%s/releases", repo), "--jq", ".[].tag_name") +} + // resolveLatestRelease resolves the latest compatible release for a workflow source func resolveLatestRelease(repo, currentRef string, allowMajor, verbose bool) (string, error) { updateLog.Printf("Resolving latest release for repo %s (current: %s, allowMajor=%v)", repo, currentRef, allowMajor) @@ -285,7 +292,7 @@ func resolveLatestRelease(repo, currentRef string, allowMajor, verbose bool) (st } // Get all releases using gh CLI - output, err := workflow.RunGH("Fetching releases...", "api", fmt.Sprintf("/repos/%s/releases", repo), "--jq", ".[].tag_name") + output, err := runWorkflowReleasesAPIFn(repo) if err != nil { return "", fmt.Errorf("failed to fetch releases: %w", err) } @@ -298,21 +305,42 @@ func resolveLatestRelease(repo, currentRef string, allowMajor, verbose bool) (st // Parse current version currentVer := parseVersion(currentRef) if currentVer == nil { - // If current version is not a valid semantic version, just return the latest release - latestRelease := releases[0] + // If current version is not a valid semantic version, select the latest stable release + // by semantic version so we are not sensitive to the ordering of the API response. + var latestStable string + var latestStableVersion *semverutil.SemanticVersion + + for _, release := range releases { + releaseVer := parseVersion(release) + if releaseVer == nil || releaseVer.Pre != "" { + continue + } + if latestStableVersion == nil || releaseVer.IsNewer(latestStableVersion) { + latestStable = release + latestStableVersion = releaseVer + } + } + + if latestStable == "" { + return "", fmt.Errorf("no stable releases found for %s", repo) + } + if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Current version is not valid, using latest release: "+latestRelease)) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Current version is not valid, using latest stable release: "+latestStable)) } - return latestRelease, nil + + return latestStable, nil } - // Find the latest compatible release + // Find the latest compatible non-prerelease release. + // Per semver rules, v1.1.0-beta.1 > v1.0.0, so without this filter a prerelease + // of a higher base version could be incorrectly selected as the upgrade target. var latestCompatible string var latestCompatibleVersion *semverutil.SemanticVersion for _, release := range releases { releaseVer := parseVersion(release) - if releaseVer == nil { + if releaseVer == nil || releaseVer.Pre != "" { continue } diff --git a/pkg/cli/update_workflows_test.go b/pkg/cli/update_workflows_test.go new file mode 100644 index 00000000000..b29433d090b --- /dev/null +++ b/pkg/cli/update_workflows_test.go @@ -0,0 +1,83 @@ +//go:build !integration + +package cli + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// mockWorkflowReleasesAPI stubs runWorkflowReleasesAPIFn for the duration of a test. +func mockWorkflowReleasesAPI(t *testing.T, mockFn func(string) ([]byte, error)) { + t.Helper() + orig := runWorkflowReleasesAPIFn + t.Cleanup(func() { runWorkflowReleasesAPIFn = orig }) + runWorkflowReleasesAPIFn = mockFn +} + +// TestResolveLatestRelease_PrereleaseTagsSkipped verifies that prerelease tags are +// not selected as the upgrade target even when they have a higher base version than +// the latest stable release. Per semver rules, v1.1.0-beta.1 > v1.0.0, so without +// explicit filtering a prerelease could be picked incorrectly. +func TestResolveLatestRelease_PrereleaseTagsSkipped(t *testing.T) { + mockWorkflowReleasesAPI(t, func(_ string) ([]byte, error) { + return []byte("v1.1.0-beta.1\nv1.0.0"), nil + }) + + result, err := resolveLatestRelease("owner/repo", "v1.0.0", true, false) + require.NoError(t, err, "should not error when stable release exists") + assert.Equal(t, "v1.0.0", result, "should select latest stable release, not prerelease") +} + +// TestResolveLatestRelease_PrereleaseSkippedWhenCurrentVersionInvalid verifies that when +// the current version is not a valid semantic version, the highest stable release by +// semver is returned rather than the first item in the list (which could be a prerelease +// or an older release listed first by the API). +func TestResolveLatestRelease_PrereleaseSkippedWhenCurrentVersionInvalid(t *testing.T) { + mockWorkflowReleasesAPI(t, func(_ string) ([]byte, error) { + // Prerelease appears first, and older stable release appears before newer one. + return []byte("v2.0.0-rc.1\nv1.3.0\nv1.5.0"), nil + }) + + result, err := resolveLatestRelease("owner/repo", "not-a-version", true, false) + require.NoError(t, err, "should not error when stable release exists") + assert.Equal(t, "v1.5.0", result, "should skip prerelease and return highest stable release by semver") +} + +// TestResolveLatestRelease_ErrorWhenOnlyPrereleasesExist verifies that an error is +// returned when the releases list contains only prerelease versions. +func TestResolveLatestRelease_ErrorWhenOnlyPrereleasesExist(t *testing.T) { + mockWorkflowReleasesAPI(t, func(_ string) ([]byte, error) { + return []byte("v2.0.0-beta.1\nv1.0.0-rc.1"), nil + }) + + _, err := resolveLatestRelease("owner/repo", "v1.0.0", true, false) + assert.Error(t, err, "should error when no stable releases exist") +} + +// TestResolveLatestRelease_StableReleaseSelected verifies that stable releases are +// correctly selected when there are no prereleases. +func TestResolveLatestRelease_StableReleaseSelected(t *testing.T) { + mockWorkflowReleasesAPI(t, func(_ string) ([]byte, error) { + return []byte("v1.2.0\nv1.1.0\nv1.0.0"), nil + }) + + result, err := resolveLatestRelease("owner/repo", "v1.0.0", false, false) + require.NoError(t, err, "should not error when stable releases exist") + assert.Equal(t, "v1.2.0", result, "should select highest compatible stable release") +} + +// TestResolveLatestRelease_MixedPrereleaseAndStable verifies correct selection when +// releases include both prerelease and stable versions across major versions. +func TestResolveLatestRelease_MixedPrereleaseAndStable(t *testing.T) { + mockWorkflowReleasesAPI(t, func(_ string) ([]byte, error) { + return []byte("v2.0.0-alpha.1\nv1.3.0\nv1.2.0-rc.1\nv1.1.0"), nil + }) + + // Without allowMajor, should stay on v1.x and skip prereleases. + result, err := resolveLatestRelease("owner/repo", "v1.1.0", false, false) + require.NoError(t, err, "should not error when stable v1.x releases exist") + assert.Equal(t, "v1.3.0", result, "should select latest stable v1.x release, skipping prereleases") +}