fix: filter prereleases by semver tag instead of trusting GitHub's pinky promise

Replace the GitHub API prerelease boolean check with semver.Version.Prerelease()
filtering — because the version string is the source of truth, not metadata.

Also:
- Extract highestRelease() to eliminate 3x copy-pasted loops
- Extract formatVersion() for the repeated Sprintf pattern
- Fix pre-existing bug: missing continue after semver parse errors
  caused stale version from prior iteration to be used
- Add table-driven tests for both helpers (there were none before)

Author: johnstcn

scripts/coderversion/main.go

@@ -12,90 +12,57 @@ import (
 func main() {
 	releases := fetchReleases()
 
-	mainlineVer := semver.MustParse("v0.0.0")
-	for _, rel := range releases {
-		if rel == "" {
-			debug("ignoring untagged version %s\n", rel)
-			continue
-		}
-
-		ver, err := semver.NewVersion(rel)
-		if err != nil {
-			debug("skipping invalid version %s\n", rel)
-		}
+	mainlineVer := highestRelease(releases, nil)
+	_, _ = fmt.Fprintf(os.Stdout, "CODER_MAINLINE_VERSION=%q\n", formatVersion(mainlineVer))
 
-		if ver.Compare(mainlineVer) > 0 {
-			mainlineVer = ver
-			continue
-		}
+	stableMinor := mainlineVer.Minor() - 1
+	if stableMinor < 0 {
+		stableMinor = 0
 	}
+	debug("expected stable minor: %d\n", stableMinor)
+	stableVer := highestRelease(releases, &stableMinor)
+	_, _ = fmt.Fprintf(os.Stdout, "CODER_STABLE_VERSION=%q\n", formatVersion(stableVer))
 
-	mainline := fmt.Sprintf("v%d.%d.%d", mainlineVer.Major(), mainlineVer.Minor(), mainlineVer.Patch())
-	_, _ = fmt.Fprintf(os.Stdout, "CODER_MAINLINE_VERSION=%q\n", mainline)
-
-	expectedStableMinor := mainlineVer.Minor() - 1
-	if expectedStableMinor < 0 {
-		expectedStableMinor = 0
+	oldStableMinor := mainlineVer.Minor() - 2
+	if oldStableMinor < 0 {
+		oldStableMinor = 0
 	}
-	debug("expected stable minor: %d\n", expectedStableMinor)
-	stableVer := semver.MustParse("v0.0.0")
+	debug("expected old stable minor: %d\n", oldStableMinor)
+	oldStableVer := highestRelease(releases, &oldStableMinor)
+	_, _ = fmt.Fprintf(os.Stdout, "CODER_OLDSTABLE_VERSION=%q\n", formatVersion(oldStableVer))
+}
+
+// highestRelease returns the highest non-prerelease semver from releases,
+// optionally filtered to a specific minor version.
+func highestRelease(releases []string, filterMinor *int64) *semver.Version {
+	best := semver.MustParse("v0.0.0")
 	for _, rel := range releases {
-		debug("check version %s\n", rel)
 		if rel == "" {
-			debug("ignoring untagged version %s\n", rel)
 			continue
 		}
-
 		ver, err := semver.NewVersion(rel)
 		if err != nil {
 			debug("skipping invalid version %s\n", rel)
-		}
-
-		if ver.Minor() != expectedStableMinor {
-			debug("skipping version %s\n", rel)
-			continue
-		}
-
-		if ver.Compare(stableVer) > 0 {
-			stableVer = ver
 			continue
 		}
-	}
-
-	stable := fmt.Sprintf("v%d.%d.%d", stableVer.Major(), stableVer.Minor(), stableVer.Patch())
-	_, _ = fmt.Fprintf(os.Stdout, "CODER_STABLE_VERSION=%q\n", stable)
-
-	expectedOldStableMinor := mainlineVer.Minor() - 2
-	if expectedOldStableMinor < 0 {
-		expectedOldStableMinor = 0
-	}
-	debug("expected old stable minor: %d\n", expectedStableMinor)
-	oldStableVer := semver.MustParse("v0.0.0")
-	for _, rel := range releases {
-		debug("check version %s\n", rel)
-		if rel == "" {
-			debug("ignoring untagged version %s\n", rel)
+		if ver.Prerelease() != "" {
+			debug("skipping prerelease version %s\n", rel)
 			continue
 		}
-
-		ver, err := semver.NewVersion(rel)
-		if err != nil {
-			debug("skipping invalid version %s\n", rel)
-		}
-
-		if ver.Minor() != expectedOldStableMinor {
-			debug("skipping version %s\n", rel)
+		if filterMinor != nil && ver.Minor() != *filterMinor {
 			continue
 		}
-
-		if ver.Compare(oldStableVer) > 0 {
-			oldStableVer = ver
-			continue
+		if ver.Compare(best) > 0 {
+			best = ver
 		}
 	}
+	return best
+}
 
-	oldStable := fmt.Sprintf("v%d.%d.%d", oldStableVer.Major(), oldStableVer.Minor(), oldStableVer.Patch())
-	_, _ = fmt.Fprintf(os.Stdout, "CODER_OLDSTABLE_VERSION=%q\n", oldStable)
+// formatVersion formats a semver version as "vMAJOR.MINOR.PATCH",
+// stripping any prerelease or build metadata.
+func formatVersion(v *semver.Version) string {
+	return fmt.Sprintf("v%d.%d.%d", v.Major(), v.Minor(), v.Patch())
 }
 
 type release struct {
@@ -123,7 +90,6 @@ func fetchReleases() []string {
 	for _, rel := range releases {
 		if rel.TagName != "" {
 			ss = append(ss, rel.TagName)
-
 		}
 	}
 	return ss

scripts/coderversion/main_test.go

@@ -0,0 +1,109 @@
+package main
+
+import (
+	"testing"
+
+	"github.com/masterminds/semver"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func int64Ptr(v int64) *int64 { return &v }
+
+func TestHighestRelease(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name        string
+		releases    []string
+		filterMinor *int64
+		expected    string
+	}{
+		{
+			name:     "skips RC versions",
+			releases: []string{"v2.32.0-rc.0", "v2.31.2", "v2.31.1"},
+			expected: "v2.31.2",
+		},
+		{
+			name:     "skips all prerelease variants",
+			releases: []string{"v2.32.0-rc.0", "v2.32.0-beta.1", "v2.31.0"},
+			expected: "v2.31.0",
+		},
+		{
+			name:     "returns highest when no prereleases",
+			releases: []string{"v2.31.2", "v2.31.1", "v2.30.0"},
+			expected: "v2.31.2",
+		},
+		{
+			name:        "filters by minor version",
+			releases:    []string{"v2.32.1", "v2.31.3", "v2.31.2"},
+			filterMinor: int64Ptr(31),
+			expected:    "v2.31.3",
+		},
+		{
+			name:        "filters by minor AND skips prerelease",
+			releases:    []string{"v2.31.0-rc.1", "v2.31.2", "v2.30.0"},
+			filterMinor: int64Ptr(31),
+			expected:    "v2.31.2",
+		},
+		{
+			name:     "skips invalid version strings",
+			releases: []string{"v2.31.0", "not-a-version", "v2.30.0"},
+			expected: "v2.31.0",
+		},
+		{
+			name:     "handles empty strings",
+			releases: []string{"", "v2.31.0"},
+			expected: "v2.31.0",
+		},
+		{
+			name:     "all prereleases returns v0.0.0",
+			releases: []string{"v2.32.0-rc.0", "v2.31.0-beta.1"},
+			expected: "v0.0.0",
+		},
+		{
+			name:     "empty input returns v0.0.0",
+			releases: []string{},
+			expected: "v0.0.0",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			got := highestRelease(tt.releases, tt.filterMinor)
+			require.NotNil(t, got)
+			expected := semver.MustParse(tt.expected)
+			assert.Equal(t, 0, got.Compare(expected), "expected %s but got %s", tt.expected, got.Original())
+		})
+	}
+}
+
+func TestFormatVersion(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{
+			name:     "standard version",
+			input:    "v2.31.2",
+			expected: "v2.31.2",
+		},
+		{
+			name:     "zero version",
+			input:    "v0.0.0",
+			expected: "v0.0.0",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			v := semver.MustParse(tt.input)
+			assert.Equal(t, tt.expected, formatVersion(v))
+		})
+	}
+}