From f3b37caca60529631f413deed98dfdaa1c140e81 Mon Sep 17 00:00:00 2001 From: Kartik Singhal Date: Fri, 22 May 2026 16:35:29 -0500 Subject: [PATCH 1/9] fix: upgrade exact action tags within current major --- README.md | 7 ++- cmd/actupdate/main.go | 11 +--- cmd/actupdate/main_test.go | 41 +++++++++++++ internal/github/client.go | 103 +++++++++++++++++++++++++++++++++ internal/github/client_test.go | 46 +++++++++++++++ 5 files changed, 197 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 9a9752b..91f110e 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # actupdate `actupdate` updates GitHub Action references in workflow YAML files to the latest -stable major version. +eligible stable version. ## What It Does @@ -76,10 +76,11 @@ Each release asset is named like `actupdate_linux_amd64.tar.gz` and contains the - Only stable semver tags are considered - Pre-release tags such as `-rc`, `-beta`, and `-alpha` are ignored -- Updates only move to the latest stable major +- Updates move to the latest eligible stable version - `--cooldown-days` can exclude newer tags until they have aged past the configured threshold -- Same-major patch or minor bumps are not applied in v1 +- When a newer major exists, moving major tags such as `v6` are preferred over + exact tags such as `v6.2.1` - If any candidate update cannot be verified, the tool prints the failures and does not rewrite any files diff --git a/cmd/actupdate/main.go b/cmd/actupdate/main.go index 2683c0e..cf79ae4 100644 --- a/cmd/actupdate/main.go +++ b/cmd/actupdate/main.go @@ -210,7 +210,6 @@ func useColor(out io.Writer) bool { func buildReport(ctx context.Context, scans []workflows.FileScan, client *gh.Client, cooldown time.Duration) (plan.Report, []workflows.Change, bool, error) { report := plan.Report{} var changes []workflows.Change - repoResults := map[string]repoOutcome{} hadVerificationFailure := false for _, scan := range scans { @@ -254,7 +253,7 @@ func buildReport(ctx context.Context, scans []workflows.FileScan, client *gh.Cli continue } - currentMajor, err := actionspec.ParseMajor(spec.Ref) + currentVersion, err := actionspec.ParseStableVersion(spec.Ref) if err != nil { entry.Status = plan.StatusSkipped entry.Reason = "non-semver ref" @@ -262,12 +261,8 @@ func buildReport(ctx context.Context, scans []workflows.FileScan, client *gh.Cli continue } - outcome, ok := repoResults[spec.Repo] - if !ok { - resolution, resolveErr := client.ResolveLatestMajor(ctx, spec.Repo, currentMajor, cooldown) - outcome = repoOutcome{Resolution: resolution, Err: resolveErr} - repoResults[spec.Repo] = outcome - } + resolution, resolveErr := client.ResolveLatestStable(ctx, spec.Repo, currentVersion, cooldown) + outcome := repoOutcome{Resolution: resolution, Err: resolveErr} if outcome.Err != nil { entry.Status = plan.StatusError diff --git a/cmd/actupdate/main_test.go b/cmd/actupdate/main_test.go index 9ed53cb..c5d515f 100644 --- a/cmd/actupdate/main_test.go +++ b/cmd/actupdate/main_test.go @@ -205,6 +205,47 @@ func TestRunCooldownDaysLeavesTooNewMajorUnchanged(t *testing.T) { } } +func TestRunUpdatesOlderExactTagEvenWhenSameRepoHasNewerRef(t *testing.T) { + repo := t.TempDir() + workflowDir := filepath.Join(repo, ".github", "workflows") + if err := os.MkdirAll(workflowDir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + workflowPath := filepath.Join(workflowDir, "build_wheels.yml") + original := "steps:\n - uses: pypa/cibuildwheel@v3.3\n - uses: pypa/cibuildwheel@v3.0\n" + if err := os.WriteFile(workflowPath, []byte(original), 0o644); err != nil { + t.Fatalf("write workflow: %v", err) + } + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/repos/pypa/cibuildwheel/tags" { + http.NotFound(w, r) + return + } + fmt.Fprint(w, `[{"name":"v3.3"},{"name":"v3.0"},{"name":"v2.9"}]`) + })) + defer server.Close() + + var stdout bytes.Buffer + var stderr bytes.Buffer + exitCode := run([]string{"--repo", repo, "--yes"}, strings.NewReader(""), &stdout, &stderr, server.Client(), server.URL) + if exitCode != exitOK { + t.Fatalf("expected exit 0, got %d stderr=%s", exitCode, stderr.String()) + } + + updated, err := os.ReadFile(workflowPath) + if err != nil { + t.Fatalf("read workflow: %v", err) + } + got := string(updated) + if strings.Count(got, "pypa/cibuildwheel@v3.3") != 2 { + t.Fatalf("expected both refs to end at v3.3, got %q", got) + } + if !strings.Contains(stdout.String(), "pypa/cibuildwheel@v3.0 -> @v3.3") { + t.Fatalf("expected same-major upgrade in output, got %q", stdout.String()) + } +} + func TestRunVerificationFailurePreventsWrites(t *testing.T) { repo := t.TempDir() workflowDir := filepath.Join(repo, ".github", "workflows") diff --git a/internal/github/client.go b/internal/github/client.go index a3df8e4..70e0a2a 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -143,6 +143,99 @@ func (c *Client) ResolveLatestMajor(ctx context.Context, repo string, currentMaj }, nil } +func (c *Client) ResolveLatestStable(ctx context.Context, repo string, current actionspec.StableVersion, cooldown time.Duration) (Resolution, error) { + tags, err := c.stableTags(ctx, repo) + if err != nil { + return Resolution{}, err + } + if len(tags) == 0 { + return Resolution{}, fmt.Errorf("%s: no stable semver tags found", repo) + } + + latestMajor := current.Major + for _, tag := range tags { + if tag.Major > latestMajor { + latestMajor = tag.Major + } + } + + cutoff := time.Time{} + if cooldown > 0 { + cutoff = c.now().Add(-cooldown) + } + + foundBlockedNewerMajor := false + for _, major := range newerMajors(tags, current.Major) { + if moving, ok := findMovingMajor(tags, major); ok { + eligible, err := c.tagEligible(ctx, repo, moving.Original, cutoff) + if err != nil { + return Resolution{}, err + } + if eligible { + return Resolution{ + TargetRef: moving.Original, + HasUpgrade: true, + Reason: "moving major tag", + LatestMajor: major, + }, nil + } + foundBlockedNewerMajor = true + } + + best, ok := findHighestForMajor(tags, major) + if !ok { + return Resolution{}, fmt.Errorf("%s: no stable tag found for major v%d", repo, major) + } + eligible, err := c.tagEligible(ctx, repo, best.Original, cutoff) + if err != nil { + return Resolution{}, err + } + if eligible { + return Resolution{ + TargetRef: best.Original, + HasUpgrade: true, + Reason: "exact tag fallback", + LatestMajor: major, + }, nil + } + foundBlockedNewerMajor = true + } + + bestCurrentMajor, ok := findHighestForMajor(tags, current.Major) + if ok && isSameMajorExactUpgrade(current, bestCurrentMajor) { + eligible, err := c.tagEligible(ctx, repo, bestCurrentMajor.Original, cutoff) + if err != nil { + return Resolution{}, err + } + if eligible { + return Resolution{ + TargetRef: bestCurrentMajor.Original, + HasUpgrade: true, + Reason: "newer stable version in current major", + LatestMajor: latestMajor, + }, nil + } + return Resolution{ + HasUpgrade: false, + LatestMajor: latestMajor, + Reason: "newer stable tags in current major are still within cooldown", + }, nil + } + if foundBlockedNewerMajor { + return Resolution{ + HasUpgrade: false, + LatestMajor: latestMajor, + Reason: "newer major tags are still within cooldown", + }, nil + } + + return Resolution{ + HasUpgrade: false, + LatestMajor: latestMajor, + Reason: "already on latest stable version", + }, nil +} + func (c *Client) stableTags(ctx context.Context, repo string) ([]actionspec.StableVersion, error) { if cached, ok := c.cache[repo]; ok { return cached, nil @@ -391,3 +484,13 @@ func findHighestForMajor(tags []actionspec.StableVersion, major int) (actionspec func isMovingMajor(tag actionspec.StableVersion) bool { return !tag.HasMinor && !tag.HasPatch } + +func isSameMajorExactUpgrade(current, candidate actionspec.StableVersion) bool { + if current.Major != candidate.Major { + return false + } + if isMovingMajor(current) || isMovingMajor(candidate) { + return false + } + return compareVersionDesc(current, candidate) > 0 +} diff --git a/internal/github/client_test.go b/internal/github/client_test.go index 5bc47c9..c030fd5 100644 --- a/internal/github/client_test.go +++ b/internal/github/client_test.go @@ -10,6 +10,8 @@ import ( "strings" "testing" "time" + + "actupdate/internal/actionspec" ) func TestResolveLatestMajorPrefersMovingTag(t *testing.T) { @@ -44,6 +46,41 @@ func TestResolveLatestMajorFallsBackToExactTag(t *testing.T) { } } +func TestResolveLatestStableUpdatesWithinCurrentMajor(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `[{"name":"v3.3"},{"name":"v3.0"},{"name":"v2.9"}]`) + })) + defer server.Close() + + client := NewClient(server.Client(), server.URL, "") + result, err := client.ResolveLatestStable(context.Background(), "pypa/cibuildwheel", mustParseStableVersion(t, "v3.0"), 0) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if !result.HasUpgrade || result.TargetRef != "v3.3" || result.Reason != "newer stable version in current major" { + t.Fatalf("unexpected result: %+v", result) + } +} + +func TestResolveLatestStableLeavesCurrentMajorMovingTagUnchanged(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `[{"name":"v3.3"},{"name":"v3"},{"name":"v2.9"}]`) + })) + defer server.Close() + + client := NewClient(server.Client(), server.URL, "") + result, err := client.ResolveLatestStable(context.Background(), "pypa/cibuildwheel", mustParseStableVersion(t, "v3"), 0) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if result.HasUpgrade { + t.Fatalf("expected no upgrade, got %+v", result) + } + if result.Reason != "already on latest stable version" { + t.Fatalf("unexpected reason: %q", result.Reason) + } +} + func TestResolveLatestMajorIgnoresPrerelease(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { fmt.Fprint(w, `[{"name":"v6.0.0-rc1"},{"name":"v5"},{"name":"v4"}]`) @@ -274,6 +311,15 @@ func TestResolveLatestMajorEscapesTagPathSegments(t *testing.T) { } } +func mustParseStableVersion(t *testing.T, ref string) actionspec.StableVersion { + t.Helper() + version, err := actionspec.ParseStableVersion(ref) + if err != nil { + t.Fatalf("parse stable version %q: %v", ref, err) + } + return version +} + type githubTestData struct { tags []string refs map[string]gitRef From a870bb5212066aeba3d3079f41c345530b160508 Mon Sep 17 00:00:00 2001 From: Kartik Singhal Date: Fri, 22 May 2026 16:47:36 -0500 Subject: [PATCH 2/9] fix: address resolver review edge cases --- internal/github/client.go | 30 ++++++++++++++++----------- internal/github/client_test.go | 38 ++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/internal/github/client.go b/internal/github/client.go index 70e0a2a..60e6c33 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -83,12 +83,7 @@ func (c *Client) ResolveLatestMajor(ctx context.Context, repo string, currentMaj return Resolution{}, fmt.Errorf("%s: no stable semver tags found", repo) } - latestMajor := currentMajor - for _, tag := range tags { - if tag.Major > latestMajor { - latestMajor = tag.Major - } - } + latestMajor := latestPublishedMajor(tags) if latestMajor <= currentMajor { return Resolution{ HasUpgrade: false, @@ -152,12 +147,7 @@ func (c *Client) ResolveLatestStable(ctx context.Context, repo string, current a return Resolution{}, fmt.Errorf("%s: no stable semver tags found", repo) } - latestMajor := current.Major - for _, tag := range tags { - if tag.Major > latestMajor { - latestMajor = tag.Major - } - } + latestMajor := latestPublishedMajor(tags) cutoff := time.Time{} if cooldown > 0 { @@ -454,6 +444,18 @@ func compareVersionDesc(a, b actionspec.StableVersion) int { } return 1 } + if a.HasMinor != b.HasMinor { + if a.HasMinor { + return -1 + } + return 1 + } + if a.HasPatch != b.HasPatch { + if a.HasPatch { + return -1 + } + return 1 + } if strings.HasPrefix(a.Original, "v") && !strings.HasPrefix(b.Original, "v") { return -1 } @@ -494,3 +496,7 @@ func isSameMajorExactUpgrade(current, candidate actionspec.StableVersion) bool { } return compareVersionDesc(current, candidate) > 0 } + +func latestPublishedMajor(tags []actionspec.StableVersion) int { + return tags[0].Major +} diff --git a/internal/github/client_test.go b/internal/github/client_test.go index c030fd5..25a3f94 100644 --- a/internal/github/client_test.go +++ b/internal/github/client_test.go @@ -81,6 +81,44 @@ func TestResolveLatestStableLeavesCurrentMajorMovingTagUnchanged(t *testing.T) { } } +func TestResolveLatestStableReportsPublishedLatestMajor(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `[{"name":"v6.2.1"},{"name":"v6"},{"name":"v5.9"}]`) + })) + defer server.Close() + + client := NewClient(server.Client(), server.URL, "") + result, err := client.ResolveLatestStable(context.Background(), "actions/checkout", mustParseStableVersion(t, "v99.0.0"), 0) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if result.HasUpgrade { + t.Fatalf("expected no upgrade, got %+v", result) + } + if result.LatestMajor != 6 { + t.Fatalf("expected published latest major 6, got %d", result.LatestMajor) + } +} + +func TestResolveLatestStableTreatsEquivalentExactTagsAsUnchanged(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `[{"name":"v3.0"},{"name":"v3.0.0"},{"name":"v2.9"}]`) + })) + defer server.Close() + + client := NewClient(server.Client(), server.URL, "") + result, err := client.ResolveLatestStable(context.Background(), "pypa/cibuildwheel", mustParseStableVersion(t, "v3.0.0"), 0) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if result.HasUpgrade { + t.Fatalf("expected no upgrade, got %+v", result) + } + if result.TargetRef != "" { + t.Fatalf("expected no target ref, got %q", result.TargetRef) + } +} + func TestResolveLatestMajorIgnoresPrerelease(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { fmt.Fprint(w, `[{"name":"v6.0.0-rc1"},{"name":"v5"},{"name":"v4"}]`) From 21459ef01351f81bff5bdf47867207148c731c6f Mon Sep 17 00:00:00 2001 From: Kartik Singhal Date: Fri, 22 May 2026 17:04:00 -0500 Subject: [PATCH 3/9] fix: use exact tags for cooldown fallback --- internal/github/client.go | 13 +++++++++++-- internal/github/client_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/internal/github/client.go b/internal/github/client.go index 60e6c33..3f16628 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -113,7 +113,7 @@ func (c *Client) ResolveLatestMajor(ctx context.Context, repo string, currentMaj } } - best, ok := findHighestForMajor(tags, major) + best, ok := findHighestExactForMajor(tags, major) if !ok { return Resolution{}, fmt.Errorf("%s: no stable tag found for major v%d", repo, major) } @@ -172,7 +172,7 @@ func (c *Client) ResolveLatestStable(ctx context.Context, repo string, current a foundBlockedNewerMajor = true } - best, ok := findHighestForMajor(tags, major) + best, ok := findHighestExactForMajor(tags, major) if !ok { return Resolution{}, fmt.Errorf("%s: no stable tag found for major v%d", repo, major) } @@ -483,6 +483,15 @@ func findHighestForMajor(tags []actionspec.StableVersion, major int) (actionspec return actionspec.StableVersion{}, false } +func findHighestExactForMajor(tags []actionspec.StableVersion, major int) (actionspec.StableVersion, bool) { + for _, tag := range tags { + if tag.Major == major && !isMovingMajor(tag) { + return tag, true + } + } + return actionspec.StableVersion{}, false +} + func isMovingMajor(tag actionspec.StableVersion) bool { return !tag.HasMinor && !tag.HasPatch } diff --git a/internal/github/client_test.go b/internal/github/client_test.go index 25a3f94..43beecb 100644 --- a/internal/github/client_test.go +++ b/internal/github/client_test.go @@ -201,6 +201,33 @@ func TestResolveLatestMajorWithCooldownFallsBackToOlderExactTag(t *testing.T) { } } +func TestResolveLatestMajorWithCooldownFallsBackToExactTagWhenMovingTagBlocked(t *testing.T) { + now := time.Date(2026, 5, 13, 12, 0, 0, 0, time.UTC) + server := newGitHubTestServer(t, githubTestData{ + tags: []string{"v6", "v6.0.0", "v4"}, + refs: map[string]gitRef{ + "v6": {Type: "tag", SHA: "tag-v6"}, + "v6.0.0": {Type: "tag", SHA: "tag-v600"}, + }, + tagObjects: map[string]string{ + "tag-v6": now.Add(-2 * 24 * time.Hour).Format(time.RFC3339), + "tag-v600": now.Add(-9 * 24 * time.Hour).Format(time.RFC3339), + }, + }) + defer server.Close() + + client := NewClient(server.Client(), server.URL, "") + client.now = func() time.Time { return now } + + result, err := client.ResolveLatestMajor(context.Background(), "actions/checkout", 4, 7*24*time.Hour) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if !result.HasUpgrade || result.TargetRef != "v6.0.0" || result.Reason != "exact tag fallback" { + t.Fatalf("unexpected result: %+v", result) + } +} + func TestResolveLatestMajorWithCooldownSkipsTooNewMajor(t *testing.T) { now := time.Date(2026, 5, 13, 12, 0, 0, 0, time.UTC) server := newGitHubTestServer(t, githubTestData{ From 1115de78508c1960b7a5c86263f9ff53dfbe1755 Mon Sep 17 00:00:00 2001 From: Kartik Singhal Date: Fri, 22 May 2026 18:29:27 -0500 Subject: [PATCH 4/9] refactor: model action version resolution policy --- internal/github/client.go | 336 ++++++++++++++++++++------------- internal/github/client_test.go | 252 ++++++++++++++++++++++++- 2 files changed, 450 insertions(+), 138 deletions(-) diff --git a/internal/github/client.go b/internal/github/client.go index 3f16628..46358a1 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -31,6 +31,16 @@ type Resolution struct { LatestMajor int } +type majorCandidates struct { + Major int + Moving actionspec.StableVersion + HasMoving bool + MovingEligible bool + Exact actionspec.StableVersion + HasExact bool + ExactEligible bool +} + type tagResponse struct { Name string `json:"name"` } @@ -83,59 +93,15 @@ func (c *Client) ResolveLatestMajor(ctx context.Context, repo string, currentMaj return Resolution{}, fmt.Errorf("%s: no stable semver tags found", repo) } - latestMajor := latestPublishedMajor(tags) - if latestMajor <= currentMajor { - return Resolution{ - HasUpgrade: false, - LatestMajor: latestMajor, - Reason: "already on latest stable major", - }, nil - } - cutoff := time.Time{} if cooldown > 0 { cutoff = c.now().Add(-cooldown) } - - for _, major := range newerMajors(tags, currentMajor) { - if moving, ok := findMovingMajor(tags, major); ok { - eligible, err := c.tagEligible(ctx, repo, moving.Original, cutoff) - if err != nil { - return Resolution{}, err - } - if eligible { - return Resolution{ - TargetRef: moving.Original, - HasUpgrade: true, - Reason: "moving major tag", - LatestMajor: major, - }, nil - } - } - - best, ok := findHighestExactForMajor(tags, major) - if !ok { - return Resolution{}, fmt.Errorf("%s: no stable tag found for major v%d", repo, major) - } - eligible, err := c.tagEligible(ctx, repo, best.Original, cutoff) - if err != nil { - return Resolution{}, err - } - if eligible { - return Resolution{ - TargetRef: best.Original, - HasUpgrade: true, - Reason: "exact tag fallback", - LatestMajor: major, - }, nil - } + candidates := collectMajorCandidates(tags) + if err := c.populateEligibilityForNewerMajors(ctx, repo, candidates, currentMajor, cutoff); err != nil { + return Resolution{}, err } - - return Resolution{ - HasUpgrade: false, - LatestMajor: latestMajor, - Reason: "newer major tags are still within cooldown", - }, nil + return resolveLatestMajorPolicy(currentMajor, candidates), nil } func (c *Client) ResolveLatestStable(ctx context.Context, repo string, current actionspec.StableVersion, cooldown time.Duration) (Resolution, error) { @@ -147,83 +113,18 @@ func (c *Client) ResolveLatestStable(ctx context.Context, repo string, current a return Resolution{}, fmt.Errorf("%s: no stable semver tags found", repo) } - latestMajor := latestPublishedMajor(tags) - cutoff := time.Time{} if cooldown > 0 { cutoff = c.now().Add(-cooldown) } - - foundBlockedNewerMajor := false - for _, major := range newerMajors(tags, current.Major) { - if moving, ok := findMovingMajor(tags, major); ok { - eligible, err := c.tagEligible(ctx, repo, moving.Original, cutoff) - if err != nil { - return Resolution{}, err - } - if eligible { - return Resolution{ - TargetRef: moving.Original, - HasUpgrade: true, - Reason: "moving major tag", - LatestMajor: major, - }, nil - } - foundBlockedNewerMajor = true - } - - best, ok := findHighestExactForMajor(tags, major) - if !ok { - return Resolution{}, fmt.Errorf("%s: no stable tag found for major v%d", repo, major) - } - eligible, err := c.tagEligible(ctx, repo, best.Original, cutoff) - if err != nil { - return Resolution{}, err - } - if eligible { - return Resolution{ - TargetRef: best.Original, - HasUpgrade: true, - Reason: "exact tag fallback", - LatestMajor: major, - }, nil - } - foundBlockedNewerMajor = true - } - - bestCurrentMajor, ok := findHighestForMajor(tags, current.Major) - if ok && isSameMajorExactUpgrade(current, bestCurrentMajor) { - eligible, err := c.tagEligible(ctx, repo, bestCurrentMajor.Original, cutoff) - if err != nil { - return Resolution{}, err - } - if eligible { - return Resolution{ - TargetRef: bestCurrentMajor.Original, - HasUpgrade: true, - Reason: "newer stable version in current major", - LatestMajor: latestMajor, - }, nil - } - return Resolution{ - HasUpgrade: false, - LatestMajor: latestMajor, - Reason: "newer stable tags in current major are still within cooldown", - }, nil + candidates := collectMajorCandidates(tags) + if err := c.populateEligibilityForNewerMajors(ctx, repo, candidates, current.Major, cutoff); err != nil { + return Resolution{}, err } - if foundBlockedNewerMajor { - return Resolution{ - HasUpgrade: false, - LatestMajor: latestMajor, - Reason: "newer major tags are still within cooldown", - }, nil + if err := c.populateEligibilityForCurrentMajorExact(ctx, repo, candidates, current.Major, cutoff); err != nil { + return Resolution{}, err } - - return Resolution{ - HasUpgrade: false, - LatestMajor: latestMajor, - Reason: "already on latest stable version", - }, nil + return resolveLatestStablePolicy(current, candidates), nil } func (c *Client) stableTags(ctx context.Context, repo string) ([]actionspec.StableVersion, error) { @@ -428,6 +329,155 @@ func parseGitHubTime(repo, value, field string) (time.Time, error) { return parsed, nil } +func (c *Client) populateEligibilityForNewerMajors(ctx context.Context, repo string, candidates []majorCandidates, currentMajor int, cutoff time.Time) error { + for i := range candidates { + if candidates[i].Major <= currentMajor { + continue + } + if candidates[i].HasMoving { + eligible, err := c.tagEligible(ctx, repo, candidates[i].Moving.Original, cutoff) + if err != nil { + return err + } + candidates[i].MovingEligible = eligible + } + if candidates[i].HasExact { + eligible, err := c.tagEligible(ctx, repo, candidates[i].Exact.Original, cutoff) + if err != nil { + return err + } + candidates[i].ExactEligible = eligible + } + } + return nil +} + +func (c *Client) populateEligibilityForCurrentMajorExact(ctx context.Context, repo string, candidates []majorCandidates, currentMajor int, cutoff time.Time) error { + for i := range candidates { + if candidates[i].Major != currentMajor || !candidates[i].HasExact { + continue + } + eligible, err := c.tagEligible(ctx, repo, candidates[i].Exact.Original, cutoff) + if err != nil { + return err + } + candidates[i].ExactEligible = eligible + return nil + } + return nil +} + +func resolveLatestMajorPolicy(currentMajor int, candidates []majorCandidates) Resolution { + latestMajor := latestPublishedMajorFromCandidates(candidates) + if latestMajor <= currentMajor { + return Resolution{ + HasUpgrade: false, + LatestMajor: latestMajor, + Reason: "already on latest stable major", + } + } + + for _, candidate := range candidates { + if candidate.Major <= currentMajor { + continue + } + if candidate.HasMoving { + if candidate.MovingEligible { + return Resolution{ + TargetRef: candidate.Moving.Original, + HasUpgrade: true, + Reason: "moving major tag", + LatestMajor: candidate.Major, + } + } + } + if candidate.HasExact { + if candidate.ExactEligible { + return Resolution{ + TargetRef: candidate.Exact.Original, + HasUpgrade: true, + Reason: "exact tag fallback", + LatestMajor: candidate.Major, + } + } + } + } + + return Resolution{ + HasUpgrade: false, + LatestMajor: latestMajor, + Reason: "newer major tags are still within cooldown", + } +} + +func resolveLatestStablePolicy(current actionspec.StableVersion, candidates []majorCandidates) Resolution { + latestMajor := latestPublishedMajorFromCandidates(candidates) + foundBlockedNewerMajor := false + var currentMajor majorCandidates + hasCurrentMajor := false + + for _, candidate := range candidates { + if candidate.Major == current.Major { + currentMajor = candidate + hasCurrentMajor = true + } + if candidate.Major <= current.Major { + continue + } + if candidate.HasMoving { + if candidate.MovingEligible { + return Resolution{ + TargetRef: candidate.Moving.Original, + HasUpgrade: true, + Reason: "moving major tag", + LatestMajor: candidate.Major, + } + } + foundBlockedNewerMajor = true + } + if candidate.HasExact { + if candidate.ExactEligible { + return Resolution{ + TargetRef: candidate.Exact.Original, + HasUpgrade: true, + Reason: "exact tag fallback", + LatestMajor: candidate.Major, + } + } + foundBlockedNewerMajor = true + } + } + + if hasCurrentMajor && currentMajor.HasExact && isSameMajorExactUpgrade(current, currentMajor.Exact) { + if currentMajor.ExactEligible { + return Resolution{ + TargetRef: currentMajor.Exact.Original, + HasUpgrade: true, + Reason: "newer stable version in current major", + LatestMajor: latestMajor, + } + } + return Resolution{ + HasUpgrade: false, + LatestMajor: latestMajor, + Reason: "newer stable tags in current major are still within cooldown", + } + } + if foundBlockedNewerMajor { + return Resolution{ + HasUpgrade: false, + LatestMajor: latestMajor, + Reason: "newer major tags are still within cooldown", + } + } + + return Resolution{ + HasUpgrade: false, + LatestMajor: latestMajor, + Reason: "already on latest stable version", + } +} + func compareVersionDesc(a, b actionspec.StableVersion) int { if a.Major != b.Major { return b.Major - a.Major @@ -438,12 +488,6 @@ func compareVersionDesc(a, b actionspec.StableVersion) int { if a.Patch != b.Patch { return b.Patch - a.Patch } - if isMovingMajor(a) != isMovingMajor(b) { - if isMovingMajor(a) { - return -1 - } - return 1 - } if a.HasMinor != b.HasMinor { if a.HasMinor { return -1 @@ -483,15 +527,6 @@ func findHighestForMajor(tags []actionspec.StableVersion, major int) (actionspec return actionspec.StableVersion{}, false } -func findHighestExactForMajor(tags []actionspec.StableVersion, major int) (actionspec.StableVersion, bool) { - for _, tag := range tags { - if tag.Major == major && !isMovingMajor(tag) { - return tag, true - } - } - return actionspec.StableVersion{}, false -} - func isMovingMajor(tag actionspec.StableVersion) bool { return !tag.HasMinor && !tag.HasPatch } @@ -506,6 +541,35 @@ func isSameMajorExactUpgrade(current, candidate actionspec.StableVersion) bool { return compareVersionDesc(current, candidate) > 0 } -func latestPublishedMajor(tags []actionspec.StableVersion) int { - return tags[0].Major +func collectMajorCandidates(tags []actionspec.StableVersion) []majorCandidates { + byMajor := make([]majorCandidates, 0) + indexByMajor := map[int]int{} + for _, tag := range tags { + index, ok := indexByMajor[tag.Major] + if !ok { + index = len(byMajor) + indexByMajor[tag.Major] = index + byMajor = append(byMajor, majorCandidates{Major: tag.Major}) + } + candidate := &byMajor[index] + if isMovingMajor(tag) { + if !candidate.HasMoving { + candidate.Moving = tag + candidate.HasMoving = true + } + continue + } + if !candidate.HasExact { + candidate.Exact = tag + candidate.HasExact = true + } + } + return byMajor +} + +func latestPublishedMajorFromCandidates(candidates []majorCandidates) int { + if len(candidates) == 0 { + return 0 + } + return candidates[0].Major } diff --git a/internal/github/client_test.go b/internal/github/client_test.go index 43beecb..2063733 100644 --- a/internal/github/client_test.go +++ b/internal/github/client_test.go @@ -228,6 +228,90 @@ func TestResolveLatestMajorWithCooldownFallsBackToExactTagWhenMovingTagBlocked(t } } +func TestResolveLatestMajorWithCooldownSkipsMovingOnlyBlockedMajor(t *testing.T) { + now := time.Date(2026, 5, 13, 12, 0, 0, 0, time.UTC) + server := newGitHubTestServer(t, githubTestData{ + tags: []string{"v7", "v6.2.1", "v4"}, + refs: map[string]gitRef{ + "v7": {Type: "tag", SHA: "tag-v7"}, + "v6.2.1": {Type: "tag", SHA: "tag-v621"}, + }, + tagObjects: map[string]string{ + "tag-v7": now.Add(-2 * 24 * time.Hour).Format(time.RFC3339), + "tag-v621": now.Add(-9 * 24 * time.Hour).Format(time.RFC3339), + }, + }) + defer server.Close() + + client := NewClient(server.Client(), server.URL, "") + client.now = func() time.Time { return now } + + result, err := client.ResolveLatestMajor(context.Background(), "actions/checkout", 4, 7*24*time.Hour) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if !result.HasUpgrade || result.TargetRef != "v6.2.1" || result.Reason != "exact tag fallback" { + t.Fatalf("unexpected result: %+v", result) + } +} + +func TestResolveLatestMajorWithCooldownMovingOnlyBlockedMajorReturnsCooldown(t *testing.T) { + now := time.Date(2026, 5, 13, 12, 0, 0, 0, time.UTC) + server := newGitHubTestServer(t, githubTestData{ + tags: []string{"v6", "v4"}, + refs: map[string]gitRef{ + "v6": {Type: "tag", SHA: "tag-v6"}, + }, + tagObjects: map[string]string{ + "tag-v6": now.Add(-2 * 24 * time.Hour).Format(time.RFC3339), + }, + }) + defer server.Close() + + client := NewClient(server.Client(), server.URL, "") + client.now = func() time.Time { return now } + + result, err := client.ResolveLatestMajor(context.Background(), "actions/checkout", 4, 7*24*time.Hour) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if result.HasUpgrade { + t.Fatalf("expected no upgrade, got %+v", result) + } + if result.Reason != "newer major tags are still within cooldown" { + t.Fatalf("unexpected reason: %q", result.Reason) + } +} + +func TestResolvePoliciesMatchReferenceModel(t *testing.T) { + for currentMajor := 1; currentMajor <= 3; currentMajor++ { + for currentKind := 0; currentKind < 2; currentKind++ { + for currentState := 0; currentState <= 8; currentState++ { + for newerState := 0; newerState <= 8; newerState++ { + for newestState := 0; newestState <= 8; newestState++ { + candidates := makePolicyCandidates(currentMajor, currentState, newerState, newestState) + current := makeCurrentVersion(currentMajor, currentKind) + + gotMajor := resolveLatestMajorPolicy(currentMajor, candidates) + wantMajor := resolveLatestMajorPolicyModel(currentMajor, candidates) + if gotMajor != wantMajor { + t.Fatalf("major policy mismatch currentMajor=%d currentState=%d newerState=%d newestState=%d got=%+v want=%+v", + currentMajor, currentState, newerState, newestState, gotMajor, wantMajor) + } + + gotStable := resolveLatestStablePolicy(current, candidates) + wantStable := resolveLatestStablePolicyModel(current, candidates) + if gotStable != wantStable { + t.Fatalf("stable policy mismatch current=%+v currentState=%d newerState=%d newestState=%d got=%+v want=%+v", + current, currentState, newerState, newestState, gotStable, wantStable) + } + } + } + } + } + } +} + func TestResolveLatestMajorWithCooldownSkipsTooNewMajor(t *testing.T) { now := time.Date(2026, 5, 13, 12, 0, 0, 0, time.UTC) server := newGitHubTestServer(t, githubTestData{ @@ -377,14 +461,178 @@ func TestResolveLatestMajorEscapesTagPathSegments(t *testing.T) { } func mustParseStableVersion(t *testing.T, ref string) actionspec.StableVersion { - t.Helper() + if t != nil { + t.Helper() + } version, err := actionspec.ParseStableVersion(ref) if err != nil { - t.Fatalf("parse stable version %q: %v", ref, err) + if t != nil { + t.Fatalf("parse stable version %q: %v", ref, err) + } + panic(fmt.Sprintf("parse stable version %q: %v", ref, err)) } return version } +func makePolicyCandidates(currentMajor, currentState, newerState, newestState int) []majorCandidates { + states := []struct { + major int + state int + }{ + {currentMajor + 2, newestState}, + {currentMajor + 1, newerState}, + {currentMajor, currentState}, + } + candidates := make([]majorCandidates, 0, len(states)) + for _, item := range states { + candidate, ok := candidateFromState(item.major, item.state) + if ok { + candidates = append(candidates, candidate) + } + } + return candidates +} + +func candidateFromState(major, state int) (majorCandidates, bool) { + candidate := majorCandidates{Major: major} + switch state { + case 0: + return majorCandidates{}, false + case 1: + candidate.Moving = mustParseStableVersion(nil, fmt.Sprintf("v%d", major)) + candidate.HasMoving = true + candidate.MovingEligible = true + case 2: + candidate.Moving = mustParseStableVersion(nil, fmt.Sprintf("v%d", major)) + candidate.HasMoving = true + case 3: + candidate.Exact = mustParseStableVersion(nil, fmt.Sprintf("v%d.2.0", major)) + candidate.HasExact = true + candidate.ExactEligible = true + case 4: + candidate.Exact = mustParseStableVersion(nil, fmt.Sprintf("v%d.2.0", major)) + candidate.HasExact = true + case 5: + candidate.Moving = mustParseStableVersion(nil, fmt.Sprintf("v%d", major)) + candidate.HasMoving = true + candidate.MovingEligible = true + candidate.Exact = mustParseStableVersion(nil, fmt.Sprintf("v%d.2.0", major)) + candidate.HasExact = true + candidate.ExactEligible = true + case 6: + candidate.Moving = mustParseStableVersion(nil, fmt.Sprintf("v%d", major)) + candidate.HasMoving = true + candidate.Exact = mustParseStableVersion(nil, fmt.Sprintf("v%d.2.0", major)) + candidate.HasExact = true + candidate.ExactEligible = true + case 7: + candidate.Moving = mustParseStableVersion(nil, fmt.Sprintf("v%d", major)) + candidate.HasMoving = true + candidate.Exact = mustParseStableVersion(nil, fmt.Sprintf("v%d.2.0", major)) + candidate.HasExact = true + case 8: + candidate.Moving = mustParseStableVersion(nil, fmt.Sprintf("v%d", major)) + candidate.HasMoving = true + candidate.MovingEligible = true + candidate.Exact = mustParseStableVersion(nil, fmt.Sprintf("v%d.2.0", major)) + candidate.HasExact = true + default: + panic(fmt.Sprintf("unknown state %d", state)) + } + return candidate, true +} + +func makeCurrentVersion(currentMajor, currentKind int) actionspec.StableVersion { + if currentKind == 0 { + return mustParseStableVersion(nil, fmt.Sprintf("v%d", currentMajor)) + } + return mustParseStableVersion(nil, fmt.Sprintf("v%d.1.0", currentMajor)) +} + +func resolveLatestMajorPolicyModel(currentMajor int, candidates []majorCandidates) Resolution { + latestMajor := latestPublishedMajorFromCandidates(candidates) + if latestMajor <= currentMajor { + return Resolution{LatestMajor: latestMajor, Reason: "already on latest stable major"} + } + blocked := false + for _, candidate := range candidates { + if candidate.Major <= currentMajor { + continue + } + if candidate.HasMoving { + if candidate.MovingEligible { + return Resolution{TargetRef: candidate.Moving.Original, HasUpgrade: true, Reason: "moving major tag", LatestMajor: candidate.Major} + } + blocked = true + } + if candidate.HasExact { + if candidate.ExactEligible { + return Resolution{TargetRef: candidate.Exact.Original, HasUpgrade: true, Reason: "exact tag fallback", LatestMajor: candidate.Major} + } + blocked = true + } + } + reason := "already on latest stable major" + if latestMajor > currentMajor || blocked { + reason = "newer major tags are still within cooldown" + } + return Resolution{LatestMajor: latestMajor, Reason: reason} +} + +func resolveLatestStablePolicyModel(current actionspec.StableVersion, candidates []majorCandidates) Resolution { + latestMajor := latestPublishedMajorFromCandidates(candidates) + blockedNewer := false + currentExact, hasCurrentExact, currentExactEligible := actionspec.StableVersion{}, false, false + for _, candidate := range candidates { + if candidate.Major == current.Major && candidate.HasExact { + currentExact = candidate.Exact + hasCurrentExact = true + currentExactEligible = candidate.ExactEligible + } + if candidate.Major <= current.Major { + continue + } + if candidate.HasMoving { + if candidate.MovingEligible { + return Resolution{TargetRef: candidate.Moving.Original, HasUpgrade: true, Reason: "moving major tag", LatestMajor: candidate.Major} + } + blockedNewer = true + } + if candidate.HasExact { + if candidate.ExactEligible { + return Resolution{TargetRef: candidate.Exact.Original, HasUpgrade: true, Reason: "exact tag fallback", LatestMajor: candidate.Major} + } + blockedNewer = true + } + } + if hasCurrentExact && isSameMajorExactUpgradeModel(current, currentExact) { + if currentExactEligible { + return Resolution{TargetRef: currentExact.Original, HasUpgrade: true, Reason: "newer stable version in current major", LatestMajor: latestMajor} + } + return Resolution{LatestMajor: latestMajor, Reason: "newer stable tags in current major are still within cooldown"} + } + if blockedNewer { + return Resolution{LatestMajor: latestMajor, Reason: "newer major tags are still within cooldown"} + } + return Resolution{LatestMajor: latestMajor, Reason: "already on latest stable version"} +} + +func isSameMajorExactUpgradeModel(current, candidate actionspec.StableVersion) bool { + if current.Major != candidate.Major { + return false + } + if isMovingMajor(current) || isMovingMajor(candidate) { + return false + } + if current.Minor != candidate.Minor { + return current.Minor < candidate.Minor + } + if current.Patch != candidate.Patch { + return current.Patch < candidate.Patch + } + return false +} + type githubTestData struct { tags []string refs map[string]gitRef From 60d700e5371cf4af50b829d154a8fd82ded350e6 Mon Sep 17 00:00:00 2001 From: Kartik Singhal Date: Fri, 22 May 2026 18:34:48 -0500 Subject: [PATCH 5/9] test: add property checks for resolver policy --- internal/github/client.go | 8 +-- internal/github/client_test.go | 124 +++++++++++++++++++++++++++++++-- 2 files changed, 124 insertions(+), 8 deletions(-) diff --git a/internal/github/client.go b/internal/github/client.go index 46358a1..dd542df 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -387,7 +387,7 @@ func resolveLatestMajorPolicy(currentMajor int, candidates []majorCandidates) Re TargetRef: candidate.Moving.Original, HasUpgrade: true, Reason: "moving major tag", - LatestMajor: candidate.Major, + LatestMajor: latestMajor, } } } @@ -397,7 +397,7 @@ func resolveLatestMajorPolicy(currentMajor int, candidates []majorCandidates) Re TargetRef: candidate.Exact.Original, HasUpgrade: true, Reason: "exact tag fallback", - LatestMajor: candidate.Major, + LatestMajor: latestMajor, } } } @@ -430,7 +430,7 @@ func resolveLatestStablePolicy(current actionspec.StableVersion, candidates []ma TargetRef: candidate.Moving.Original, HasUpgrade: true, Reason: "moving major tag", - LatestMajor: candidate.Major, + LatestMajor: latestMajor, } } foundBlockedNewerMajor = true @@ -441,7 +441,7 @@ func resolveLatestStablePolicy(current actionspec.StableVersion, candidates []ma TargetRef: candidate.Exact.Original, HasUpgrade: true, Reason: "exact tag fallback", - LatestMajor: candidate.Major, + LatestMajor: latestMajor, } } foundBlockedNewerMajor = true diff --git a/internal/github/client_test.go b/internal/github/client_test.go index 2063733..e79c176 100644 --- a/internal/github/client_test.go +++ b/internal/github/client_test.go @@ -4,11 +4,14 @@ import ( "context" "encoding/json" "fmt" + "math/rand" "net/http" "net/http/httptest" "net/url" + "reflect" "strings" "testing" + "testing/quick" "time" "actupdate/internal/actionspec" @@ -312,6 +315,49 @@ func TestResolvePoliciesMatchReferenceModel(t *testing.T) { } } +func TestResolveLatestMajorPolicyQuick(t *testing.T) { + config := &quick.Config{MaxCount: 1000} + property := func(s policyScenario) bool { + candidates := s.candidates() + got := resolveLatestMajorPolicy(int(s.CurrentMajor), candidates) + want := resolveLatestMajorPolicyModel(int(s.CurrentMajor), candidates) + if got != want { + t.Logf("major policy mismatch scenario=%+v got=%+v want=%+v", s, got, want) + return false + } + if err := assertResolutionInvariants(got, candidates, s.currentVersion()); err != nil { + t.Logf("major invariant failed scenario=%+v resolution=%+v err=%v", s, got, err) + return false + } + return true + } + if err := quick.Check(property, config); err != nil { + t.Fatal(err) + } +} + +func TestResolveLatestStablePolicyQuick(t *testing.T) { + config := &quick.Config{MaxCount: 1000} + property := func(s policyScenario) bool { + candidates := s.candidates() + current := s.currentVersion() + got := resolveLatestStablePolicy(current, candidates) + want := resolveLatestStablePolicyModel(current, candidates) + if got != want { + t.Logf("stable policy mismatch scenario=%+v got=%+v want=%+v", s, got, want) + return false + } + if err := assertResolutionInvariants(got, candidates, current); err != nil { + t.Logf("stable invariant failed scenario=%+v resolution=%+v err=%v", s, got, err) + return false + } + return true + } + if err := quick.Check(property, config); err != nil { + t.Fatal(err) + } +} + func TestResolveLatestMajorWithCooldownSkipsTooNewMajor(t *testing.T) { now := time.Date(2026, 5, 13, 12, 0, 0, 0, time.UTC) server := newGitHubTestServer(t, githubTestData{ @@ -561,13 +607,13 @@ func resolveLatestMajorPolicyModel(currentMajor int, candidates []majorCandidate } if candidate.HasMoving { if candidate.MovingEligible { - return Resolution{TargetRef: candidate.Moving.Original, HasUpgrade: true, Reason: "moving major tag", LatestMajor: candidate.Major} + return Resolution{TargetRef: candidate.Moving.Original, HasUpgrade: true, Reason: "moving major tag", LatestMajor: latestMajor} } blocked = true } if candidate.HasExact { if candidate.ExactEligible { - return Resolution{TargetRef: candidate.Exact.Original, HasUpgrade: true, Reason: "exact tag fallback", LatestMajor: candidate.Major} + return Resolution{TargetRef: candidate.Exact.Original, HasUpgrade: true, Reason: "exact tag fallback", LatestMajor: latestMajor} } blocked = true } @@ -594,13 +640,13 @@ func resolveLatestStablePolicyModel(current actionspec.StableVersion, candidates } if candidate.HasMoving { if candidate.MovingEligible { - return Resolution{TargetRef: candidate.Moving.Original, HasUpgrade: true, Reason: "moving major tag", LatestMajor: candidate.Major} + return Resolution{TargetRef: candidate.Moving.Original, HasUpgrade: true, Reason: "moving major tag", LatestMajor: latestMajor} } blockedNewer = true } if candidate.HasExact { if candidate.ExactEligible { - return Resolution{TargetRef: candidate.Exact.Original, HasUpgrade: true, Reason: "exact tag fallback", LatestMajor: candidate.Major} + return Resolution{TargetRef: candidate.Exact.Original, HasUpgrade: true, Reason: "exact tag fallback", LatestMajor: latestMajor} } blockedNewer = true } @@ -633,6 +679,46 @@ func isSameMajorExactUpgradeModel(current, candidate actionspec.StableVersion) b return false } +func assertResolutionInvariants(resolution Resolution, candidates []majorCandidates, current actionspec.StableVersion) error { + latestMajor := latestPublishedMajorFromCandidates(candidates) + if resolution.LatestMajor != latestMajor { + return fmt.Errorf("latest major mismatch: got %d want %d", resolution.LatestMajor, latestMajor) + } + if resolution.HasUpgrade != (resolution.TargetRef != "") { + return fmt.Errorf("HasUpgrade/TargetRef mismatch: %+v", resolution) + } + if !resolution.HasUpgrade { + return nil + } + + target, err := actionspec.ParseStableVersion(resolution.TargetRef) + if err != nil { + return fmt.Errorf("target ref is not stable semver: %w", err) + } + if !candidateContainsEligibleRef(candidates, resolution.TargetRef) { + return fmt.Errorf("target ref %q is not an eligible published candidate", resolution.TargetRef) + } + if target.Major < current.Major { + return fmt.Errorf("resolver downgraded major from %d to %d", current.Major, target.Major) + } + if target.Major == current.Major && !isMovingMajor(current) && !isMovingMajor(target) && compareVersionDesc(current, target) <= 0 { + return fmt.Errorf("same-major exact upgrade is not newer: current=%s target=%s", current.Original, target.Original) + } + return nil +} + +func candidateContainsEligibleRef(candidates []majorCandidates, ref string) bool { + for _, candidate := range candidates { + if candidate.HasMoving && candidate.MovingEligible && candidate.Moving.Original == ref { + return true + } + if candidate.HasExact && candidate.ExactEligible && candidate.Exact.Original == ref { + return true + } + } + return false +} + type githubTestData struct { tags []string refs map[string]gitRef @@ -642,6 +728,36 @@ type githubTestData struct { escapedHits map[string]int } +type policyScenario struct { + CurrentMajor uint8 + CurrentKind bool + CurrentState uint8 + NewerState uint8 + NewestState uint8 +} + +func (policyScenario) Generate(r *rand.Rand, _ int) reflect.Value { + scenario := policyScenario{ + CurrentMajor: uint8(r.Intn(4) + 1), + CurrentKind: r.Intn(2) == 1, + CurrentState: uint8(r.Intn(9)), + NewerState: uint8(r.Intn(9)), + NewestState: uint8(r.Intn(9)), + } + return reflect.ValueOf(scenario) +} + +func (s policyScenario) candidates() []majorCandidates { + return makePolicyCandidates(int(s.CurrentMajor), int(s.CurrentState), int(s.NewerState), int(s.NewestState)) +} + +func (s policyScenario) currentVersion() actionspec.StableVersion { + if s.CurrentKind { + return mustParseStableVersion(nil, fmt.Sprintf("v%d.1.0", s.CurrentMajor)) + } + return mustParseStableVersion(nil, fmt.Sprintf("v%d", s.CurrentMajor)) +} + type gitRef struct { Type string SHA string From 884d18d1b31ff5e0a995020355839555e5d3d60c Mon Sep 17 00:00:00 2001 From: Kartik Singhal Date: Fri, 22 May 2026 18:46:47 -0500 Subject: [PATCH 6/9] fix: preserve moving minor action tags --- internal/github/client.go | 154 ++++++++++++++++++++++++++------- internal/github/client_test.go | 103 ++++++++++++++++------ 2 files changed, 202 insertions(+), 55 deletions(-) diff --git a/internal/github/client.go b/internal/github/client.go index dd542df..bb35e3a 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -32,13 +32,16 @@ type Resolution struct { } type majorCandidates struct { - Major int - Moving actionspec.StableVersion - HasMoving bool - MovingEligible bool - Exact actionspec.StableVersion - HasExact bool - ExactEligible bool + Major int + MovingMajor actionspec.StableVersion + HasMovingMajor bool + MovingMajorEligible bool + MovingMinor actionspec.StableVersion + HasMovingMinor bool + MovingMinorEligible bool + Exact actionspec.StableVersion + HasExact bool + ExactEligible bool } type tagResponse struct { @@ -121,7 +124,7 @@ func (c *Client) ResolveLatestStable(ctx context.Context, repo string, current a if err := c.populateEligibilityForNewerMajors(ctx, repo, candidates, current.Major, cutoff); err != nil { return Resolution{}, err } - if err := c.populateEligibilityForCurrentMajorExact(ctx, repo, candidates, current.Major, cutoff); err != nil { + if err := c.populateEligibilityForCurrentMajorUpgrades(ctx, repo, candidates, current, cutoff); err != nil { return Resolution{}, err } return resolveLatestStablePolicy(current, candidates), nil @@ -334,12 +337,19 @@ func (c *Client) populateEligibilityForNewerMajors(ctx context.Context, repo str if candidates[i].Major <= currentMajor { continue } - if candidates[i].HasMoving { - eligible, err := c.tagEligible(ctx, repo, candidates[i].Moving.Original, cutoff) + if candidates[i].HasMovingMajor { + eligible, err := c.tagEligible(ctx, repo, candidates[i].MovingMajor.Original, cutoff) if err != nil { return err } - candidates[i].MovingEligible = eligible + candidates[i].MovingMajorEligible = eligible + } + if candidates[i].HasMovingMinor { + eligible, err := c.tagEligible(ctx, repo, candidates[i].MovingMinor.Original, cutoff) + if err != nil { + return err + } + candidates[i].MovingMinorEligible = eligible } if candidates[i].HasExact { eligible, err := c.tagEligible(ctx, repo, candidates[i].Exact.Original, cutoff) @@ -352,16 +362,32 @@ func (c *Client) populateEligibilityForNewerMajors(ctx context.Context, repo str return nil } -func (c *Client) populateEligibilityForCurrentMajorExact(ctx context.Context, repo string, candidates []majorCandidates, currentMajor int, cutoff time.Time) error { +func (c *Client) populateEligibilityForCurrentMajorUpgrades(ctx context.Context, repo string, candidates []majorCandidates, current actionspec.StableVersion, cutoff time.Time) error { for i := range candidates { - if candidates[i].Major != currentMajor || !candidates[i].HasExact { + if candidates[i].Major != current.Major { continue } - eligible, err := c.tagEligible(ctx, repo, candidates[i].Exact.Original, cutoff) - if err != nil { - return err + if isMajorMovingRef(current) && candidates[i].HasMovingMajor && isSameMajorMovingUpgrade(current, candidates[i].MovingMajor) { + eligible, err := c.tagEligible(ctx, repo, candidates[i].MovingMajor.Original, cutoff) + if err != nil { + return err + } + candidates[i].MovingMajorEligible = eligible + } + if isMinorMovingRef(current) && candidates[i].HasMovingMinor && isSameMajorMovingUpgrade(current, candidates[i].MovingMinor) { + eligible, err := c.tagEligible(ctx, repo, candidates[i].MovingMinor.Original, cutoff) + if err != nil { + return err + } + candidates[i].MovingMinorEligible = eligible + } + if candidates[i].HasExact && isSameMajorExactUpgrade(current, candidates[i].Exact) { + eligible, err := c.tagEligible(ctx, repo, candidates[i].Exact.Original, cutoff) + if err != nil { + return err + } + candidates[i].ExactEligible = eligible } - candidates[i].ExactEligible = eligible return nil } return nil @@ -381,10 +407,10 @@ func resolveLatestMajorPolicy(currentMajor int, candidates []majorCandidates) Re if candidate.Major <= currentMajor { continue } - if candidate.HasMoving { - if candidate.MovingEligible { + if movingRef, eligible, ok := preferredMovingUpgrade(candidate); ok { + if eligible { return Resolution{ - TargetRef: candidate.Moving.Original, + TargetRef: movingRef.Original, HasUpgrade: true, Reason: "moving major tag", LatestMajor: latestMajor, @@ -424,10 +450,10 @@ func resolveLatestStablePolicy(current actionspec.StableVersion, candidates []ma if candidate.Major <= current.Major { continue } - if candidate.HasMoving { - if candidate.MovingEligible { + if movingRef, eligible, ok := preferredMovingUpgrade(candidate); ok { + if eligible { return Resolution{ - TargetRef: candidate.Moving.Original, + TargetRef: movingRef.Original, HasUpgrade: true, Reason: "moving major tag", LatestMajor: latestMajor, @@ -448,6 +474,23 @@ func resolveLatestStablePolicy(current actionspec.StableVersion, candidates []ma } } + if hasCurrentMajor { + if movingTarget, eligible, ok := samePrecisionMovingCandidate(current, currentMajor); ok && isSameMajorMovingUpgrade(current, movingTarget) { + if eligible { + return Resolution{ + TargetRef: movingTarget.Original, + HasUpgrade: true, + Reason: "newer moving version in current major", + LatestMajor: latestMajor, + } + } + return Resolution{ + HasUpgrade: false, + LatestMajor: latestMajor, + Reason: "newer moving tags in current major are still within cooldown", + } + } + } if hasCurrentMajor && currentMajor.HasExact && isSameMajorExactUpgrade(current, currentMajor.Exact) { if currentMajor.ExactEligible { return Resolution{ @@ -511,7 +554,7 @@ func compareVersionDesc(a, b actionspec.StableVersion) int { func findMovingMajor(tags []actionspec.StableVersion, major int) (actionspec.StableVersion, bool) { for _, tag := range tags { - if tag.Major == major && isMovingMajor(tag) { + if tag.Major == major && isMajorMovingRef(tag) { return tag, true } } @@ -527,15 +570,33 @@ func findHighestForMajor(tags []actionspec.StableVersion, major int) (actionspec return actionspec.StableVersion{}, false } -func isMovingMajor(tag actionspec.StableVersion) bool { +func isMovingRef(tag actionspec.StableVersion) bool { + return !tag.HasPatch +} + +func isMajorMovingRef(tag actionspec.StableVersion) bool { return !tag.HasMinor && !tag.HasPatch } +func isMinorMovingRef(tag actionspec.StableVersion) bool { + return tag.HasMinor && !tag.HasPatch +} + func isSameMajorExactUpgrade(current, candidate actionspec.StableVersion) bool { if current.Major != candidate.Major { return false } - if isMovingMajor(current) || isMovingMajor(candidate) { + if isMovingRef(current) || isMovingRef(candidate) { + return false + } + return compareVersionDesc(current, candidate) > 0 +} + +func isSameMajorMovingUpgrade(current, candidate actionspec.StableVersion) bool { + if current.Major != candidate.Major { + return false + } + if !isMovingRef(current) || !isMovingRef(candidate) { return false } return compareVersionDesc(current, candidate) > 0 @@ -552,10 +613,17 @@ func collectMajorCandidates(tags []actionspec.StableVersion) []majorCandidates { byMajor = append(byMajor, majorCandidates{Major: tag.Major}) } candidate := &byMajor[index] - if isMovingMajor(tag) { - if !candidate.HasMoving { - candidate.Moving = tag - candidate.HasMoving = true + if isMajorMovingRef(tag) { + if !candidate.HasMovingMajor { + candidate.MovingMajor = tag + candidate.HasMovingMajor = true + } + continue + } + if isMinorMovingRef(tag) { + if !candidate.HasMovingMinor { + candidate.MovingMinor = tag + candidate.HasMovingMinor = true } continue } @@ -567,6 +635,32 @@ func collectMajorCandidates(tags []actionspec.StableVersion) []majorCandidates { return byMajor } +func preferredMovingUpgrade(candidate majorCandidates) (actionspec.StableVersion, bool, bool) { + if candidate.HasMovingMajor { + return candidate.MovingMajor, candidate.MovingMajorEligible, true + } + if candidate.HasMovingMinor { + return candidate.MovingMinor, candidate.MovingMinorEligible, true + } + return actionspec.StableVersion{}, false, false +} + +func samePrecisionMovingCandidate(current actionspec.StableVersion, candidate majorCandidates) (actionspec.StableVersion, bool, bool) { + if isMajorMovingRef(current) { + if candidate.HasMovingMajor { + return candidate.MovingMajor, candidate.MovingMajorEligible, true + } + return actionspec.StableVersion{}, false, false + } + if isMinorMovingRef(current) { + if candidate.HasMovingMinor { + return candidate.MovingMinor, candidate.MovingMinorEligible, true + } + return actionspec.StableVersion{}, false, false + } + return actionspec.StableVersion{}, false, false +} + func latestPublishedMajorFromCandidates(candidates []majorCandidates) int { if len(candidates) == 0 { return 0 diff --git a/internal/github/client_test.go b/internal/github/client_test.go index e79c176..52839b2 100644 --- a/internal/github/client_test.go +++ b/internal/github/client_test.go @@ -60,7 +60,7 @@ func TestResolveLatestStableUpdatesWithinCurrentMajor(t *testing.T) { if err != nil { t.Fatalf("resolve: %v", err) } - if !result.HasUpgrade || result.TargetRef != "v3.3" || result.Reason != "newer stable version in current major" { + if !result.HasUpgrade || result.TargetRef != "v3.3" || result.Reason != "newer moving version in current major" { t.Fatalf("unexpected result: %+v", result) } } @@ -84,6 +84,25 @@ func TestResolveLatestStableLeavesCurrentMajorMovingTagUnchanged(t *testing.T) { } } +func TestResolveLatestStableLeavesCurrentMinorMovingTagUnchanged(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `[{"name":"v3.4.1"},{"name":"v3.4"},{"name":"v3.3.9"}]`) + })) + defer server.Close() + + client := NewClient(server.Client(), server.URL, "") + result, err := client.ResolveLatestStable(context.Background(), "pypa/cibuildwheel", mustParseStableVersion(t, "v3.4"), 0) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if result.HasUpgrade { + t.Fatalf("expected no upgrade, got %+v", result) + } + if result.Reason != "already on latest stable version" { + t.Fatalf("unexpected reason: %q", result.Reason) + } +} + func TestResolveLatestStableReportsPublishedLatestMajor(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { fmt.Fprint(w, `[{"name":"v6.2.1"},{"name":"v6"},{"name":"v5.9"}]`) @@ -545,12 +564,12 @@ func candidateFromState(major, state int) (majorCandidates, bool) { case 0: return majorCandidates{}, false case 1: - candidate.Moving = mustParseStableVersion(nil, fmt.Sprintf("v%d", major)) - candidate.HasMoving = true - candidate.MovingEligible = true + candidate.MovingMajor = mustParseStableVersion(nil, fmt.Sprintf("v%d", major)) + candidate.HasMovingMajor = true + candidate.MovingMajorEligible = true case 2: - candidate.Moving = mustParseStableVersion(nil, fmt.Sprintf("v%d", major)) - candidate.HasMoving = true + candidate.MovingMajor = mustParseStableVersion(nil, fmt.Sprintf("v%d", major)) + candidate.HasMovingMajor = true case 3: candidate.Exact = mustParseStableVersion(nil, fmt.Sprintf("v%d.2.0", major)) candidate.HasExact = true @@ -559,27 +578,27 @@ func candidateFromState(major, state int) (majorCandidates, bool) { candidate.Exact = mustParseStableVersion(nil, fmt.Sprintf("v%d.2.0", major)) candidate.HasExact = true case 5: - candidate.Moving = mustParseStableVersion(nil, fmt.Sprintf("v%d", major)) - candidate.HasMoving = true - candidate.MovingEligible = true + candidate.MovingMajor = mustParseStableVersion(nil, fmt.Sprintf("v%d", major)) + candidate.HasMovingMajor = true + candidate.MovingMajorEligible = true candidate.Exact = mustParseStableVersion(nil, fmt.Sprintf("v%d.2.0", major)) candidate.HasExact = true candidate.ExactEligible = true case 6: - candidate.Moving = mustParseStableVersion(nil, fmt.Sprintf("v%d", major)) - candidate.HasMoving = true + candidate.MovingMajor = mustParseStableVersion(nil, fmt.Sprintf("v%d", major)) + candidate.HasMovingMajor = true candidate.Exact = mustParseStableVersion(nil, fmt.Sprintf("v%d.2.0", major)) candidate.HasExact = true candidate.ExactEligible = true case 7: - candidate.Moving = mustParseStableVersion(nil, fmt.Sprintf("v%d", major)) - candidate.HasMoving = true + candidate.MovingMajor = mustParseStableVersion(nil, fmt.Sprintf("v%d", major)) + candidate.HasMovingMajor = true candidate.Exact = mustParseStableVersion(nil, fmt.Sprintf("v%d.2.0", major)) candidate.HasExact = true case 8: - candidate.Moving = mustParseStableVersion(nil, fmt.Sprintf("v%d", major)) - candidate.HasMoving = true - candidate.MovingEligible = true + candidate.MovingMajor = mustParseStableVersion(nil, fmt.Sprintf("v%d", major)) + candidate.HasMovingMajor = true + candidate.MovingMajorEligible = true candidate.Exact = mustParseStableVersion(nil, fmt.Sprintf("v%d.2.0", major)) candidate.HasExact = true default: @@ -605,9 +624,9 @@ func resolveLatestMajorPolicyModel(currentMajor int, candidates []majorCandidate if candidate.Major <= currentMajor { continue } - if candidate.HasMoving { - if candidate.MovingEligible { - return Resolution{TargetRef: candidate.Moving.Original, HasUpgrade: true, Reason: "moving major tag", LatestMajor: latestMajor} + if movingRef, eligible, ok := preferredMovingUpgrade(candidate); ok { + if eligible { + return Resolution{TargetRef: movingRef.Original, HasUpgrade: true, Reason: "moving major tag", LatestMajor: latestMajor} } blocked = true } @@ -638,9 +657,9 @@ func resolveLatestStablePolicyModel(current actionspec.StableVersion, candidates if candidate.Major <= current.Major { continue } - if candidate.HasMoving { - if candidate.MovingEligible { - return Resolution{TargetRef: candidate.Moving.Original, HasUpgrade: true, Reason: "moving major tag", LatestMajor: latestMajor} + if movingRef, eligible, ok := preferredMovingUpgrade(candidate); ok { + if eligible { + return Resolution{TargetRef: movingRef.Original, HasUpgrade: true, Reason: "moving major tag", LatestMajor: latestMajor} } blockedNewer = true } @@ -651,6 +670,12 @@ func resolveLatestStablePolicyModel(current actionspec.StableVersion, candidates blockedNewer = true } } + if movingTarget, eligible, ok := samePrecisionMovingCandidate(current, currentMajorCandidateForModel(current.Major, candidates)); ok && isSameMajorMovingUpgradeModel(current, movingTarget) { + if eligible { + return Resolution{TargetRef: movingTarget.Original, HasUpgrade: true, Reason: "newer moving version in current major", LatestMajor: latestMajor} + } + return Resolution{LatestMajor: latestMajor, Reason: "newer moving tags in current major are still within cooldown"} + } if hasCurrentExact && isSameMajorExactUpgradeModel(current, currentExact) { if currentExactEligible { return Resolution{TargetRef: currentExact.Original, HasUpgrade: true, Reason: "newer stable version in current major", LatestMajor: latestMajor} @@ -667,7 +692,7 @@ func isSameMajorExactUpgradeModel(current, candidate actionspec.StableVersion) b if current.Major != candidate.Major { return false } - if isMovingMajor(current) || isMovingMajor(candidate) { + if isMovingRef(current) || isMovingRef(candidate) { return false } if current.Minor != candidate.Minor { @@ -679,6 +704,28 @@ func isSameMajorExactUpgradeModel(current, candidate actionspec.StableVersion) b return false } +func isSameMajorMovingUpgradeModel(current, candidate actionspec.StableVersion) bool { + if current.Major != candidate.Major { + return false + } + if !isMovingRef(current) || !isMovingRef(candidate) { + return false + } + if current.Minor != candidate.Minor { + return current.Minor < candidate.Minor + } + return false +} + +func currentMajorCandidateForModel(major int, candidates []majorCandidates) majorCandidates { + for _, candidate := range candidates { + if candidate.Major == major { + return candidate + } + } + return majorCandidates{Major: major} +} + func assertResolutionInvariants(resolution Resolution, candidates []majorCandidates, current actionspec.StableVersion) error { latestMajor := latestPublishedMajorFromCandidates(candidates) if resolution.LatestMajor != latestMajor { @@ -701,7 +748,10 @@ func assertResolutionInvariants(resolution Resolution, candidates []majorCandida if target.Major < current.Major { return fmt.Errorf("resolver downgraded major from %d to %d", current.Major, target.Major) } - if target.Major == current.Major && !isMovingMajor(current) && !isMovingMajor(target) && compareVersionDesc(current, target) <= 0 { + if target.Major == current.Major && isMovingRef(current) && isMovingRef(target) && compareVersionDesc(current, target) <= 0 { + return fmt.Errorf("same-major moving upgrade is not newer: current=%s target=%s", current.Original, target.Original) + } + if target.Major == current.Major && !isMovingRef(current) && !isMovingRef(target) && compareVersionDesc(current, target) <= 0 { return fmt.Errorf("same-major exact upgrade is not newer: current=%s target=%s", current.Original, target.Original) } return nil @@ -709,7 +759,10 @@ func assertResolutionInvariants(resolution Resolution, candidates []majorCandida func candidateContainsEligibleRef(candidates []majorCandidates, ref string) bool { for _, candidate := range candidates { - if candidate.HasMoving && candidate.MovingEligible && candidate.Moving.Original == ref { + if candidate.HasMovingMajor && candidate.MovingMajorEligible && candidate.MovingMajor.Original == ref { + return true + } + if candidate.HasMovingMinor && candidate.MovingMinorEligible && candidate.MovingMinor.Original == ref { return true } if candidate.HasExact && candidate.ExactEligible && candidate.Exact.Original == ref { From ff980b755d84bb3b5bdd647cf606f56167c3eee8 Mon Sep 17 00:00:00 2001 From: Kartik Singhal Date: Fri, 22 May 2026 18:52:10 -0500 Subject: [PATCH 7/9] fix: ignore semver representation-only upgrades --- internal/github/client.go | 24 +++++++++++++++++++++++- internal/github/client_test.go | 29 +++++++++++++++++++++-------- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/internal/github/client.go b/internal/github/client.go index bb35e3a..419111e 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -589,7 +589,7 @@ func isSameMajorExactUpgrade(current, candidate actionspec.StableVersion) bool { if isMovingRef(current) || isMovingRef(candidate) { return false } - return compareVersionDesc(current, candidate) > 0 + return compareNumericVersion(current, candidate) < 0 } func isSameMajorMovingUpgrade(current, candidate actionspec.StableVersion) bool { @@ -602,6 +602,28 @@ func isSameMajorMovingUpgrade(current, candidate actionspec.StableVersion) bool return compareVersionDesc(current, candidate) > 0 } +func compareNumericVersion(a, b actionspec.StableVersion) int { + if a.Major != b.Major { + if a.Major < b.Major { + return -1 + } + return 1 + } + if a.Minor != b.Minor { + if a.Minor < b.Minor { + return -1 + } + return 1 + } + if a.Patch != b.Patch { + if a.Patch < b.Patch { + return -1 + } + return 1 + } + return 0 +} + func collectMajorCandidates(tags []actionspec.StableVersion) []majorCandidates { byMajor := make([]majorCandidates, 0) indexByMajor := map[int]int{} diff --git a/internal/github/client_test.go b/internal/github/client_test.go index 52839b2..4a70db4 100644 --- a/internal/github/client_test.go +++ b/internal/github/client_test.go @@ -141,6 +141,25 @@ func TestResolveLatestStableTreatsEquivalentExactTagsAsUnchanged(t *testing.T) { } } +func TestResolveLatestStableTreatsEquivalentExactTagsAsUnchangedForShorterCurrentRef(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `[{"name":"v3.0.0"},{"name":"v3.0"},{"name":"v2.9"}]`) + })) + defer server.Close() + + client := NewClient(server.Client(), server.URL, "") + result, err := client.ResolveLatestStable(context.Background(), "pypa/cibuildwheel", mustParseStableVersion(t, "v3.0"), 0) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if result.HasUpgrade { + t.Fatalf("expected no upgrade, got %+v", result) + } + if result.TargetRef != "" { + t.Fatalf("expected no target ref, got %q", result.TargetRef) + } +} + func TestResolveLatestMajorIgnoresPrerelease(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { fmt.Fprint(w, `[{"name":"v6.0.0-rc1"},{"name":"v5"},{"name":"v4"}]`) @@ -695,13 +714,7 @@ func isSameMajorExactUpgradeModel(current, candidate actionspec.StableVersion) b if isMovingRef(current) || isMovingRef(candidate) { return false } - if current.Minor != candidate.Minor { - return current.Minor < candidate.Minor - } - if current.Patch != candidate.Patch { - return current.Patch < candidate.Patch - } - return false + return compareNumericVersion(current, candidate) < 0 } func isSameMajorMovingUpgradeModel(current, candidate actionspec.StableVersion) bool { @@ -751,7 +764,7 @@ func assertResolutionInvariants(resolution Resolution, candidates []majorCandida if target.Major == current.Major && isMovingRef(current) && isMovingRef(target) && compareVersionDesc(current, target) <= 0 { return fmt.Errorf("same-major moving upgrade is not newer: current=%s target=%s", current.Original, target.Original) } - if target.Major == current.Major && !isMovingRef(current) && !isMovingRef(target) && compareVersionDesc(current, target) <= 0 { + if target.Major == current.Major && !isMovingRef(current) && !isMovingRef(target) && compareNumericVersion(current, target) >= 0 { return fmt.Errorf("same-major exact upgrade is not newer: current=%s target=%s", current.Original, target.Original) } return nil From 15b0862c4151c3a66e31945ab0c7824c096c83ac Mon Sep 17 00:00:00 2001 From: Kartik Singhal Date: Fri, 22 May 2026 18:55:02 -0500 Subject: [PATCH 8/9] fix: tighten resolver review follow-ups --- internal/github/client.go | 30 +++++++++++++++++++++--------- internal/github/client_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/internal/github/client.go b/internal/github/client.go index 419111e..30df83a 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -101,7 +101,7 @@ func (c *Client) ResolveLatestMajor(ctx context.Context, repo string, currentMaj cutoff = c.now().Add(-cooldown) } candidates := collectMajorCandidates(tags) - if err := c.populateEligibilityForNewerMajors(ctx, repo, candidates, currentMajor, cutoff); err != nil { + if _, err := c.populateEligibilityForNewerMajors(ctx, repo, candidates, currentMajor, cutoff); err != nil { return Resolution{}, err } return resolveLatestMajorPolicy(currentMajor, candidates), nil @@ -121,11 +121,14 @@ func (c *Client) ResolveLatestStable(ctx context.Context, repo string, current a cutoff = c.now().Add(-cooldown) } candidates := collectMajorCandidates(tags) - if err := c.populateEligibilityForNewerMajors(ctx, repo, candidates, current.Major, cutoff); err != nil { + foundNewerEligible, err := c.populateEligibilityForNewerMajors(ctx, repo, candidates, current.Major, cutoff) + if err != nil { return Resolution{}, err } - if err := c.populateEligibilityForCurrentMajorUpgrades(ctx, repo, candidates, current, cutoff); err != nil { - return Resolution{}, err + if !foundNewerEligible { + if err := c.populateEligibilityForCurrentMajorUpgrades(ctx, repo, candidates, current, cutoff); err != nil { + return Resolution{}, err + } } return resolveLatestStablePolicy(current, candidates), nil } @@ -332,7 +335,7 @@ func parseGitHubTime(repo, value, field string) (time.Time, error) { return parsed, nil } -func (c *Client) populateEligibilityForNewerMajors(ctx context.Context, repo string, candidates []majorCandidates, currentMajor int, cutoff time.Time) error { +func (c *Client) populateEligibilityForNewerMajors(ctx context.Context, repo string, candidates []majorCandidates, currentMajor int, cutoff time.Time) (bool, error) { for i := range candidates { if candidates[i].Major <= currentMajor { continue @@ -340,26 +343,35 @@ func (c *Client) populateEligibilityForNewerMajors(ctx context.Context, repo str if candidates[i].HasMovingMajor { eligible, err := c.tagEligible(ctx, repo, candidates[i].MovingMajor.Original, cutoff) if err != nil { - return err + return false, err } candidates[i].MovingMajorEligible = eligible + if eligible { + return true, nil + } } if candidates[i].HasMovingMinor { eligible, err := c.tagEligible(ctx, repo, candidates[i].MovingMinor.Original, cutoff) if err != nil { - return err + return false, err } candidates[i].MovingMinorEligible = eligible + if eligible && !candidates[i].HasMovingMajor { + return true, nil + } } if candidates[i].HasExact { eligible, err := c.tagEligible(ctx, repo, candidates[i].Exact.Original, cutoff) if err != nil { - return err + return false, err } candidates[i].ExactEligible = eligible + if eligible { + return true, nil + } } } - return nil + return false, nil } func (c *Client) populateEligibilityForCurrentMajorUpgrades(ctx context.Context, repo string, candidates []majorCandidates, current actionspec.StableVersion, cutoff time.Time) error { diff --git a/internal/github/client_test.go b/internal/github/client_test.go index 4a70db4..499f6e5 100644 --- a/internal/github/client_test.go +++ b/internal/github/client_test.go @@ -512,6 +512,39 @@ func TestResolveLatestMajorCachesTagTimestamps(t *testing.T) { } } +func TestResolveLatestStableCooldownStopsAfterFirstEligibleNewerMajor(t *testing.T) { + now := time.Date(2026, 5, 13, 12, 0, 0, 0, time.UTC) + hits := map[string]int{} + server := newGitHubTestServer(t, githubTestData{ + tags: []string{"v7", "v7.1.0", "v6", "v6.2.1", "v4"}, + refs: map[string]gitRef{ + "v7": {Type: "tag", SHA: "tag-v7"}, + }, + tagObjects: map[string]string{ + "tag-v7": now.Add(-10 * 24 * time.Hour).Format(time.RFC3339), + }, + hits: hits, + }) + defer server.Close() + + client := NewClient(server.Client(), server.URL, "") + client.now = func() time.Time { return now } + + result, err := client.ResolveLatestStable(context.Background(), "actions/checkout", mustParseStableVersion(t, "v4"), 7*24*time.Hour) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if !result.HasUpgrade || result.TargetRef != "v7" || result.Reason != "moving major tag" { + t.Fatalf("unexpected result: %+v", result) + } + if hits["/repos/actions/checkout/git/ref/tags/v6"] != 0 { + t.Fatalf("expected no lower-major moving lookup, got %d", hits["/repos/actions/checkout/git/ref/tags/v6"]) + } + if hits["/repos/actions/checkout/git/ref/tags/v6.2.1"] != 0 { + t.Fatalf("expected no lower-major exact lookup, got %d", hits["/repos/actions/checkout/git/ref/tags/v6.2.1"]) + } +} + func TestResolveLatestMajorEscapesTagPathSegments(t *testing.T) { now := time.Date(2026, 5, 13, 12, 0, 0, 0, time.UTC) hits := map[string]int{} From 1ff3f89acd36a016b9eeb07a630870829aec06d2 Mon Sep 17 00:00:00 2001 From: Kartik Singhal Date: Fri, 22 May 2026 19:14:17 -0500 Subject: [PATCH 9/9] fix: label moving minor upgrades accurately --- internal/github/client.go | 11 +++++++++-- internal/github/client_test.go | 36 ++++++++++++++++++++++++++++++++-- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/internal/github/client.go b/internal/github/client.go index 30df83a..5fd38a8 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -424,7 +424,7 @@ func resolveLatestMajorPolicy(currentMajor int, candidates []majorCandidates) Re return Resolution{ TargetRef: movingRef.Original, HasUpgrade: true, - Reason: "moving major tag", + Reason: movingUpgradeReason(movingRef), LatestMajor: latestMajor, } } @@ -467,7 +467,7 @@ func resolveLatestStablePolicy(current actionspec.StableVersion, candidates []ma return Resolution{ TargetRef: movingRef.Original, HasUpgrade: true, - Reason: "moving major tag", + Reason: movingUpgradeReason(movingRef), LatestMajor: latestMajor, } } @@ -679,6 +679,13 @@ func preferredMovingUpgrade(candidate majorCandidates) (actionspec.StableVersion return actionspec.StableVersion{}, false, false } +func movingUpgradeReason(tag actionspec.StableVersion) string { + if isMajorMovingRef(tag) { + return "moving major tag" + } + return "moving minor tag" +} + func samePrecisionMovingCandidate(current actionspec.StableVersion, candidate majorCandidates) (actionspec.StableVersion, bool, bool) { if isMajorMovingRef(current) { if candidate.HasMovingMajor { diff --git a/internal/github/client_test.go b/internal/github/client_test.go index 499f6e5..f7c5d10 100644 --- a/internal/github/client_test.go +++ b/internal/github/client_test.go @@ -49,6 +49,22 @@ func TestResolveLatestMajorFallsBackToExactTag(t *testing.T) { } } +func TestResolveLatestMajorLabelsMovingMinorUpgrade(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `[{"name":"v6.2"},{"name":"v6.2.1"},{"name":"v5.9.0"}]`) + })) + defer server.Close() + + client := NewClient(server.Client(), server.URL, "") + result, err := client.ResolveLatestMajor(context.Background(), "actions/checkout", 4, 0) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if !result.HasUpgrade || result.TargetRef != "v6.2" || result.Reason != "moving minor tag" { + t.Fatalf("unexpected result: %+v", result) + } +} + func TestResolveLatestStableUpdatesWithinCurrentMajor(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { fmt.Fprint(w, `[{"name":"v3.3"},{"name":"v3.0"},{"name":"v2.9"}]`) @@ -84,6 +100,22 @@ func TestResolveLatestStableLeavesCurrentMajorMovingTagUnchanged(t *testing.T) { } } +func TestResolveLatestStableLabelsNewerMajorMovingMinorUpgrade(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `[{"name":"v6.2"},{"name":"v6.2.1"},{"name":"v5.1.0"}]`) + })) + defer server.Close() + + client := NewClient(server.Client(), server.URL, "") + result, err := client.ResolveLatestStable(context.Background(), "pypa/cibuildwheel", mustParseStableVersion(t, "v5.1.0"), 0) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if !result.HasUpgrade || result.TargetRef != "v6.2" || result.Reason != "moving minor tag" { + t.Fatalf("unexpected result: %+v", result) + } +} + func TestResolveLatestStableLeavesCurrentMinorMovingTagUnchanged(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { fmt.Fprint(w, `[{"name":"v3.4.1"},{"name":"v3.4"},{"name":"v3.3.9"}]`) @@ -678,7 +710,7 @@ func resolveLatestMajorPolicyModel(currentMajor int, candidates []majorCandidate } if movingRef, eligible, ok := preferredMovingUpgrade(candidate); ok { if eligible { - return Resolution{TargetRef: movingRef.Original, HasUpgrade: true, Reason: "moving major tag", LatestMajor: latestMajor} + return Resolution{TargetRef: movingRef.Original, HasUpgrade: true, Reason: movingUpgradeReason(movingRef), LatestMajor: latestMajor} } blocked = true } @@ -711,7 +743,7 @@ func resolveLatestStablePolicyModel(current actionspec.StableVersion, candidates } if movingRef, eligible, ok := preferredMovingUpgrade(candidate); ok { if eligible { - return Resolution{TargetRef: movingRef.Original, HasUpgrade: true, Reason: "moving major tag", LatestMajor: latestMajor} + return Resolution{TargetRef: movingRef.Original, HasUpgrade: true, Reason: movingUpgradeReason(movingRef), LatestMajor: latestMajor} } blockedNewer = true }