diff --git a/docs/adr/47730-cooldown-fallback-to-older-release.md b/docs/adr/47730-cooldown-fallback-to-older-release.md new file mode 100644 index 00000000000..c1b03f3a280 --- /dev/null +++ b/docs/adr/47730-cooldown-fallback-to-older-release.md @@ -0,0 +1,71 @@ +# ADR-47730: Cooldown Fallback to Older Release + +**Date**: 2026-07-24 +**Status**: Draft +**Deciders**: Unknown + +--- + +### Context + +The `gh aw update` command applies a cooldown policy to avoid pinning actions and +workflow sources to releases that are too recent (e.g. within 7 days of publication), +providing a safety buffer against newly-discovered bugs. Before this change, when the +newest available release was still within the cooldown window the update was silently +skipped entirely, leaving users on the current (potentially outdated) version even when +an earlier—but still upgrading—release had already cooled down. This affected both the +`updateActions` path (lock-file pins) and the `updateActionRefsInContentWithDeps` path +(inline workflow refs), as well as the `resolveLatestReleaseWithDeps` helper used for +workflow sources. + +### Decision + +We will change the cooldown handling in all three update paths so that, when the newest +upgrade candidate is still in cooldown, the system fetches the full release list, sorts +upgrade candidates newest-first, and walks them until it finds the first release that has +passed the cooldown period. That cooled-down release is used instead of skipping the +update. If every candidate is still hot, the behaviour degrades gracefully to the previous +no-op (return `currentRef` / skip). We also guard `ActionCache.Set` against empty SHAs to +prevent an unresolvable pin from being written to `actions-lock.json`. + +### Alternatives Considered + +#### Alternative 1: Keep the original skip-on-cooldown behaviour + +The prior approach—skip the entire update when the newest release is in cooldown—was +simple: one cooldown check per repo, no additional API calls, predictable behaviour. It +was not chosen because it left users on older versions when a perfectly valid, cooled-down +intermediate release existed, defeating the point of running the update command. + +#### Alternative 2: Disable or make cooldown opt-in + +Removing the cooldown constraint entirely would eliminate the fallback complexity and +guarantee every update reaches the absolute latest release. This was not chosen because +the cooldown policy exists specifically to avoid pinning to too-fresh releases that may +carry undiscovered bugs. Removing it would undermine that safety goal. + +#### Alternative 3: Surface a warning and require manual override + +Instead of automatically selecting an older release, the tool could report that the latest +is in cooldown and require the user to pass `--ignore-cooldown` or similar to proceed. +This was not chosen because it places operational burden on the user for a decision (use +the latest cooled-down release) that is both safe and predictable. + +### Consequences + +#### Positive +- Users automatically receive the latest upgrade that has passed the cooldown period rather than being silently held back. +- `checkCoolDown` is now an injectable dependency on both `actionUpdateDeps` and `workflowUpdateDeps`, enabling deterministic unit testing of cooldown scenarios without time manipulation. +- The `ActionCache.Set` empty-SHA guard prevents `actions-lock.json` from being polluted with unresolvable pin entries. + +#### Negative +- Each cooldown fallback triggers an additional GitHub Releases API call (`/repos/{owner}/{repo}/releases`) to enumerate candidates; this increases latency and API usage for repos whose latest release is in cooldown. +- The release-selection logic is more complex (collect, sort descending, iterate with per-candidate cooldown checks) compared to the previous single-check approach, increasing maintenance surface. + +#### Neutral +- The fallback is fail-open: if the releases API call fails or returns no suitable candidates, the code falls back to the original skip behaviour, so existing semantics are preserved on error. +- All three update paths (`updateActions`, `updateActionRefsInContentWithDeps`, `resolveLatestReleaseWithDeps`) now share the same logical pattern, improving consistency at the cost of the change touching multiple callsites. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* diff --git a/pkg/cli/release_candidates.go b/pkg/cli/release_candidates.go new file mode 100644 index 00000000000..35564f6e42f --- /dev/null +++ b/pkg/cli/release_candidates.go @@ -0,0 +1,58 @@ +package cli + +import ( + "slices" + + "github.com/github/gh-aw/pkg/semverutil" +) + +type releaseCandidate struct { + tag string + version *semverutil.SemanticVersion +} + +// sortedCompatibleReleaseCandidates filters tags to stable semantic versions, +// applies the major-version compatibility rule, and returns candidates sorted +// newest-first. +func sortedCompatibleReleaseCandidates(releases []string, currentVer *semverutil.SemanticVersion, allowMajor bool) []releaseCandidate { + var compatibleReleases []releaseCandidate + for _, release := range releases { + releaseVer := parseVersion(release) + if releaseVer == nil || releaseVer.Pre != "" { + continue + } + if !allowMajor && currentVer != nil && releaseVer.Major != currentVer.Major { + continue + } + compatibleReleases = append(compatibleReleases, releaseCandidate{tag: release, version: releaseVer}) + } + + slices.SortFunc(compatibleReleases, func(a, b releaseCandidate) int { + switch { + case a.version.IsNewer(b.version): + return -1 + case b.version.IsNewer(a.version): + return 1 + default: + return 0 + } + }) + + return compatibleReleases +} + +// newerReleaseCandidates keeps only candidates newer than currentVer. When +// currentVer is nil, all candidates are returned unchanged. +func newerReleaseCandidates(candidates []releaseCandidate, currentVer *semverutil.SemanticVersion) []releaseCandidate { + if currentVer == nil { + return candidates + } + + var newer []releaseCandidate + for _, c := range candidates { + if c.version.IsNewer(currentVer) { + newer = append(newer, c) + } + } + return newer +} diff --git a/pkg/cli/update_actions.go b/pkg/cli/update_actions.go index ba50cedfe99..830658be1b0 100644 --- a/pkg/cli/update_actions.go +++ b/pkg/cli/update_actions.go @@ -41,14 +41,16 @@ type actionUpdateDeps struct { getLatestReleaseViaGit func(ctx context.Context, repo, currentVersion string, allowMajor, verbose bool) (string, string, error) runGHReleasesAPI func(ctx context.Context, baseRepo string) ([]byte, error) getActionSHAForTag func(ctx context.Context, repo, tag string) (string, error) + checkCoolDown func(ctx context.Context, repo, tag string, coolDown time.Duration) coolDownCheckResult } func defaultActionUpdateDeps() actionUpdateDeps { return actionUpdateDeps{ getLatestRelease: getLatestActionRelease, getLatestReleaseViaGit: getLatestActionReleaseViaGit, + checkCoolDown: checkReleaseCoolDown, runGHReleasesAPI: func(ctx context.Context, baseRepo string) ([]byte, error) { - return workflow.RunGHCombinedContext(ctx, "Fetching releases...", "api", fmt.Sprintf("/repos/%s/releases", baseRepo), "--jq", ".[].tag_name") + return workflow.RunGHCombinedContext(ctx, "Fetching releases...", "api", "--paginate", fmt.Sprintf("/repos/%s/releases", baseRepo), "--jq", ".[].tag_name") }, getActionSHAForTag: getActionSHAForTag, } @@ -190,18 +192,39 @@ func updateActions(ctx context.Context, deps actionUpdateDeps, allowMajor, verbo coolDownResult = checkReleaseCoolDownWithDate(entry.Repo, latestVersion, cachedDate, coolDown) } else { // Fetch from API and cache the date for future runs. - coolDownResult = checkReleaseCoolDown(ctx, entry.Repo, latestVersion, coolDown) + coolDownResult = deps.checkCoolDown(ctx, entry.Repo, latestVersion, coolDown) if !coolDownResult.PublishedAt.IsZero() { actionCache.SetReleasedAt(entry.Repo, latestVersion, coolDownResult.PublishedAt) } } if coolDownResult.InCoolDown { cooldownLog.Printf("Action %s: %s", entry.Repo, coolDownResult.Message) - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Skipping update for %s: %s", entry.Repo, coolDownResult.Message))) - skippedActions = append(skippedActions, entry.Repo) - continue + + // Try to find an older release that has passed the cooldown period. + olderVersion, olderSHA, findErr := findCooledDownActionVersion(ctx, deps, entry.Repo, entry.Version, effectiveAllowMajor, verbose, coolDown, latestVersion) + if findErr != nil || olderVersion == "" || olderSHA == "" { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Skipping release candidate %s@%s: %s", entry.Repo, latestVersion, coolDownResult.Message))) + skippedActions = append(skippedActions, entry.Repo) + continue + } + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Falling back to %s for %s (latest release candidate is still in cooldown)", olderVersion, entry.Repo))) + } + // Use the older, cooled-down release instead. + latestVersion = olderVersion + latestSHA = olderSHA } } + if latestSHA == "" { + skipErr := "could not resolve SHA for " + latestVersion + updateLog.Printf("Skipping update for %s: %s", entry.Repo, skipErr) + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Skipping %s: %s", entry.Repo, skipErr))) + failedActions = append(failedActions, actionUpdateFailure{ + name: entry.Repo, + err: skipErr, + }) + continue + } // Update the entry using ActionCache.Set which: // - Preserves cached inputs/descriptions when the SHA is unchanged (moving tag) @@ -218,13 +241,23 @@ func updateActions(ctx context.Context, deps actionUpdateDeps, allowMajor, verbo updateLog.Printf("Updating %s from %s (%s) to %s (%s)", entry.Repo, entry.Version, oldSHAStr, latestVersion, newSHAStr) fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Updated %s from %s to %s", entry.Repo, entry.Version, latestVersion))) + // Set the new entry first; ActionCache.Set handles inputs/description preservation. + // If the write is rejected (e.g. empty SHA), keep the old entry untouched. + if !actionCache.Set(entry.Repo, latestVersion, latestSHA) { + skipErr := fmt.Sprintf("failed to write action cache entry for %s@%s (resolved SHA may be empty)", entry.Repo, latestVersion) + updateLog.Printf("Skipping update for %s: %s", entry.Repo, skipErr) + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Skipping %s: %s", entry.Repo, skipErr))) + failedActions = append(failedActions, actionUpdateFailure{ + name: entry.Repo, + err: skipErr, + }) + continue + } // Remove the old key when the version changes, using the original map key from // the snapshot to handle any key/version mismatches in the stored cache file. if latestVersion != entry.Version { actionCache.DeleteByKey(s.key) } - // Set the new entry; ActionCache.Set handles inputs/description preservation. - actionCache.Set(entry.Repo, latestVersion, latestSHA) updatedActions = append(updatedActions, entry.Repo) } @@ -537,6 +570,63 @@ func getLatestActionReleaseViaGit(ctx context.Context, repo, currentVersion stri return latestCompatible, sha, nil } +// findCooledDownActionVersion searches for the newest release that is strictly +// newer than currentVersion but has passed the cooldown period. It is used as +// a fallback when the highest candidate is still in cooldown: rather than +// skipping the update entirely, we walk down the release list toward older +// (but still upgrading) versions until one has cooled down. +// +// Returns ("", "", nil) when no suitable release is found (fail-open). +func findCooledDownActionVersion( + ctx context.Context, + deps actionUpdateDeps, + repo, currentVersion string, + allowMajor, verbose bool, + coolDown time.Duration, + skipTag string, +) (string, string, error) { + baseRepo := gitutil.ExtractBaseRepo(repo) + + output, err := deps.runGHReleasesAPI(ctx, baseRepo) + if err != nil { + updateLog.Printf("findCooledDownActionVersion: failed to fetch releases for %s: %v", repo, err) + return "", "", nil // fail-open + } + + releases := strings.Split(strings.TrimSpace(string(output)), "\n") + + currentVer := parseVersion(currentVersion) + + compatibleReleases := sortedCompatibleReleaseCandidates(releases, currentVer, allowMajor) + candidates := newerReleaseCandidates(compatibleReleases, currentVer) + + for _, c := range candidates { + if skipTag != "" && c.tag == skipTag { + continue + } + result := deps.checkCoolDown(ctx, repo, c.tag, coolDown) + if result.InCoolDown { + cooldownLog.Printf("Action fallback %s@%s: %s", repo, c.tag, result.Message) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Skipping release candidate %s@%s: %s", repo, c.tag, result.Message))) + } + continue + } + sha, err := deps.getActionSHAForTag(ctx, baseRepo, c.tag) + if err != nil { + updateLog.Printf("findCooledDownActionVersion: failed to get SHA for %s@%s: %v", repo, c.tag, err) + continue // try next candidate + } + if sha == "" { + updateLog.Printf("findCooledDownActionVersion: empty SHA returned for %s@%s; skipping", repo, c.tag) + continue // skip; never store an entry without a SHA + } + return c.tag, sha, nil + } + + return "", "", nil +} + // getActionSHAForTag gets the commit SHA for a given tag in an action repository. // For annotated tags (and chained tag objects), it iteratively peels until it // reaches the underlying non-tag object SHA, matching what tools like Renovate expect. @@ -894,20 +984,35 @@ func updateActionRefsInContentWithDeps(ctx context.Context, deps actionUpdateDep } } - // Apply cooldown: if the repo is not exempt and the release is too recent, skip. + // Apply cooldown: if the repo is not exempt and the release is too recent, try + // progressively older releases (still newer than current) until finding one that + // has passed the cooldown period. if !isExemptFromCoolDown(repo) { coolDownKey := repo + "@" + latestVersion coolDownResult, coolDownCached := coolDownCache[coolDownKey] if !coolDownCached { - coolDownResult = checkReleaseCoolDown(ctx, repo, latestVersion, coolDown) + coolDownResult = deps.checkCoolDown(ctx, repo, latestVersion, coolDown) coolDownCache[coolDownKey] = coolDownResult } if coolDownResult.InCoolDown { cooldownLog.Printf("Action ref %s in workflow: %s", repo, coolDownResult.Message) + + // Try to find an older release that has passed the cooldown period. + olderVersion, olderSHA, findErr := findCooledDownActionVersion(ctx, deps, repo, currentVersion, effectiveAllowMajor, verbose, coolDown, latestVersion) + if findErr != nil || olderVersion == "" || olderSHA == "" { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Skipping release candidate %s@%s: %s", repo, latestVersion, coolDownResult.Message))) + } + continue + } if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Skipping update for %s: %s", repo, coolDownResult.Message))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Falling back to %s for %s (latest release candidate is still in cooldown)", olderVersion, repo))) } - continue + // Use the older, cooled-down release and update the per-invocation cache. + result = latestReleaseResult{version: olderVersion, sha: olderSHA} + cache[cacheKey] = result + latestVersion = olderVersion + latestSHA = olderSHA } } diff --git a/pkg/cli/update_actions_test.go b/pkg/cli/update_actions_test.go index 1fabe96b763..6a3a38229f2 100644 --- a/pkg/cli/update_actions_test.go +++ b/pkg/cli/update_actions_test.go @@ -894,3 +894,285 @@ func savedEntryKeys(cache *workflow.ActionCache) []string { } return keys } + +// TestFindCooledDownActionVersion_SelectsOlderCooledRelease verifies that when the +// newest release is in cooldown, findCooledDownActionVersion falls back to the next +// older release that has passed the cooldown period. +func TestFindCooledDownActionVersion_SelectsOlderCooledRelease(t *testing.T) { + deps := newTestActionUpdateDeps() + + // v1.321.0 is the newest (in cooldown); v1.320.0 has cooled down. + deps.runGHReleasesAPI = func(_ context.Context, _ string) ([]byte, error) { + return []byte("v1.321.0\nv1.320.0\nv1.300.0"), nil + } + deps.checkCoolDown = func(_ context.Context, repo, tag string, cd time.Duration) coolDownCheckResult { + switch tag { + case "v1.321.0": + return checkReleaseCoolDownWithDate(repo, tag, time.Now().Add(-1*24*time.Hour), cd) + default: + return coolDownCheckResult{} + } + } + const wantSHA = "cooled1234567890123456789012345678901234" + deps.getActionSHAForTag = func(_ context.Context, _, tag string) (string, error) { + if tag == "v1.320.0" { + return wantSHA, nil + } + return "", errors.New("unexpected tag: " + tag) + } + + version, sha, err := findCooledDownActionVersion(context.Background(), deps, "ruby/setup-ruby", "v1.300.0", true, false, 7*24*time.Hour, "") + if err != nil { + t.Fatalf("findCooledDownActionVersion() error = %v", err) + } + if version != "v1.320.0" { + t.Errorf("version = %q, want %q", version, "v1.320.0") + } + if sha != wantSHA { + t.Errorf("sha = %q, want %q", sha, wantSHA) + } +} + +// TestFindCooledDownActionVersion_AllInCooldownReturnsEmpty verifies that +// findCooledDownActionVersion returns empty strings when all candidate releases +// are still within the cooldown window. +func TestFindCooledDownActionVersion_AllInCooldownReturnsEmpty(t *testing.T) { + deps := newTestActionUpdateDeps() + deps.runGHReleasesAPI = func(_ context.Context, _ string) ([]byte, error) { + return []byte("v1.321.0\nv1.320.0"), nil + } + // All releases too new. + deps.checkCoolDown = func(_ context.Context, repo, tag string, cd time.Duration) coolDownCheckResult { + return checkReleaseCoolDownWithDate(repo, tag, time.Now().Add(-1*24*time.Hour), cd) + } + deps.getActionSHAForTag = func(_ context.Context, _, _ string) (string, error) { + return "somesha1234567890123456789012345678901234", nil + } + + version, sha, err := findCooledDownActionVersion(context.Background(), deps, "ruby/setup-ruby", "v1.319.0", true, false, 7*24*time.Hour, "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if version != "" || sha != "" { + t.Errorf("expected empty result when all releases in cooldown, got version=%q sha=%q", version, sha) + } +} + +// TestFindCooledDownActionVersion_EmptySHASkipped verifies that a release whose SHA +// resolves to an empty string is silently skipped (never returned as a valid candidate). +func TestFindCooledDownActionVersion_EmptySHASkipped(t *testing.T) { + deps := newTestActionUpdateDeps() + deps.runGHReleasesAPI = func(_ context.Context, _ string) ([]byte, error) { + return []byte("v1.321.0\nv1.320.0"), nil + } + deps.checkCoolDown = func(_ context.Context, _ string, _ string, _ time.Duration) coolDownCheckResult { + return coolDownCheckResult{} // not in cooldown + } + // SHA API returns empty for all tags. + deps.getActionSHAForTag = func(_ context.Context, _, _ string) (string, error) { + return "", nil // empty SHA + } + + version, sha, err := findCooledDownActionVersion(context.Background(), deps, "docker/login-action", "v1.319.0", true, false, 7*24*time.Hour, "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if version != "" || sha != "" { + t.Errorf("expected empty result when SHA is empty, got version=%q sha=%q", version, sha) + } +} + +// TestUpdateActions_CooldownFallbackToOlderRelease verifies that updateActions +// falls back to an older cooled-down release instead of skipping the update +// entirely when the newest candidate is still in cooldown. +func TestUpdateActions_CooldownFallbackToOlderRelease(t *testing.T) { + deps := newTestActionUpdateDeps() + + // Latest version for ruby/setup-ruby is v1.321.0 (in cooldown). + deps.getLatestRelease = func(_ context.Context, repo, _ string, _, _ bool) (string, string, error) { + if repo == "ruby/setup-ruby" { + return "v1.321.0", "sha321_12345678901234567890123456789012", nil + } + return "v1.0.0", "default_1234567890123456789012345678901234", nil + } + + // v1.321.0 is in cooldown; v1.320.0 has cooled down. + deps.runGHReleasesAPI = func(_ context.Context, _ string) ([]byte, error) { + return []byte("v1.321.0\nv1.320.0\nv1.300.0"), nil + } + deps.checkCoolDown = func(_ context.Context, repo, tag string, cd time.Duration) coolDownCheckResult { + switch tag { + case "v1.321.0": + return checkReleaseCoolDownWithDate(repo, tag, time.Now().Add(-1*24*time.Hour), cd) + default: + return coolDownCheckResult{} + } + } + const fallbackSHA = "sha320_12345678901234567890123456789012" + deps.getActionSHAForTag = func(_ context.Context, _, tag string) (string, error) { + if tag == "v1.320.0" { + return fallbackSHA, nil + } + return "sha321_12345678901234567890123456789012", nil + } + + tmpDir := testutil.TempDir(t, "test-*") + cache := workflow.NewActionCache(tmpDir) + cache.Set("ruby/setup-ruby", "v1.300.0", "sha300_12345678901234567890123456789012") + if err := cache.Save(); err != nil { + t.Fatalf("failed to save initial cache: %v", err) + } + + wd, err := os.Getwd() + if err != nil { + t.Fatalf("failed to get working directory: %v", err) + } + t.Cleanup(func() { _ = os.Chdir(wd) }) + if err := os.Chdir(tmpDir); err != nil { + t.Fatalf("failed to chdir: %v", err) + } + + if err := updateActions(context.Background(), deps, true, false, false, 7*24*time.Hour); err != nil { + t.Fatalf("updateActions() error = %v", err) + } + + saved := workflow.NewActionCache(tmpDir) + if err := saved.Load(); err != nil { + t.Fatalf("failed to reload cache: %v", err) + } + + // Should be updated to v1.320.0 (cooled-down fallback), not v1.321.0. + entry, ok := saved.Entries["ruby/setup-ruby@v1.320.0"] + if !ok { + t.Errorf("expected ruby/setup-ruby@v1.320.0 in cache after cooldown fallback; got entries: %v", savedEntryKeys(saved)) + } else if entry.SHA != fallbackSHA { + t.Errorf("SHA = %q, want %q", entry.SHA, fallbackSHA) + } +} + +// TestUpdateActionRefsInContent_CooldownFallback verifies that +// updateActionRefsInContentWithDeps falls back to an older cooled-down release +// when the newest candidate is still within the cooldown window. +func TestUpdateActionRefsInContent_CooldownFallback(t *testing.T) { + deps := newTestActionUpdateDeps() + + // Latest version is v1.321.0 (in cooldown); fallback is v1.320.0. + deps.getLatestRelease = func(_ context.Context, repo, _ string, _, _ bool) (string, string, error) { + if repo == "ruby/setup-ruby" { + return "v1.321.0", "sha321_12345678901234567890123456789012", nil + } + return "v1.0.0", "default_1234567890123456789012345678901234", nil + } + deps.runGHReleasesAPI = func(_ context.Context, _ string) ([]byte, error) { + return []byte("v1.321.0\nv1.320.0\nv1.300.0"), nil + } + deps.checkCoolDown = func(_ context.Context, repo, tag string, cd time.Duration) coolDownCheckResult { + switch tag { + case "v1.321.0": + return checkReleaseCoolDownWithDate(repo, tag, time.Now().Add(-1*24*time.Hour), cd) + default: + return coolDownCheckResult{} + } + } + const fallbackSHA = "sha320_12345678901234567890123456789012" + deps.getActionSHAForTag = func(_ context.Context, _, tag string) (string, error) { + if tag == "v1.320.0" { + return fallbackSHA, nil + } + return "sha321_12345678901234567890123456789012", nil + } + + input := "steps:\n - uses: ruby/setup-ruby@v1.300.0\n" + + changed, got, err := updateActionRefsInContentWithDeps( + context.Background(), deps, input, + make(map[string]latestReleaseResult), + make(map[string]coolDownCheckResult), + true, false, 7*24*time.Hour, + ) + if err != nil { + t.Fatalf("updateActionRefsInContentWithDeps() error = %v", err) + } + if !changed { + t.Fatal("expected content to be updated, but changed = false") + } + if want := "steps:\n - uses: ruby/setup-ruby@v1.320.0\n"; got != want { + t.Errorf("got:\n%s\nwant:\n%s", got, want) + } +} + +func TestFindCooledDownActionVersion_SHAErrorFallsToNext(t *testing.T) { + deps := newTestActionUpdateDeps() + deps.runGHReleasesAPI = func(_ context.Context, _ string) ([]byte, error) { + return []byte("v1.322.0\nv1.321.0\nv1.320.0"), nil + } + deps.checkCoolDown = func(_ context.Context, _, _ string, _ time.Duration) coolDownCheckResult { + return coolDownCheckResult{} // all candidates cooled down + } + deps.getActionSHAForTag = func(_ context.Context, _, tag string) (string, error) { + if tag == "v1.322.0" { + return "", errors.New("not found") + } + if tag == "v1.321.0" { + return "sha321_12345678901234567890123456789012", nil + } + return "", errors.New("unexpected tag: " + tag) + } + + version, sha, err := findCooledDownActionVersion(context.Background(), deps, "ruby/setup-ruby", "v1.320.0", true, false, 7*24*time.Hour, "") + if err != nil { + t.Fatalf("findCooledDownActionVersion() error = %v", err) + } + if version != "v1.321.0" { + t.Errorf("version = %q, want %q", version, "v1.321.0") + } + if sha != "sha321_12345678901234567890123456789012" { + t.Errorf("sha = %q, want %q", sha, "sha321_12345678901234567890123456789012") + } +} + +func TestUpdateActions_EmptyResolvedSHARetainsExistingEntry(t *testing.T) { + deps := newTestActionUpdateDeps() + deps.getLatestRelease = func(_ context.Context, repo, currentVersion string, _, _ bool) (string, string, error) { + if repo == "ruby/setup-ruby" { + return "v1.321.0", "", nil + } + return currentVersion, "", nil + } + + tmpDir := testutil.TempDir(t, "test-*") + cache := workflow.NewActionCache(tmpDir) + cache.Set("ruby/setup-ruby", "v1.300.0", "sha300_12345678901234567890123456789012") + if err := cache.Save(); err != nil { + t.Fatalf("failed to save initial cache: %v", err) + } + + wd, err := os.Getwd() + if err != nil { + t.Fatalf("failed to get working directory: %v", err) + } + t.Cleanup(func() { _ = os.Chdir(wd) }) + if err := os.Chdir(tmpDir); err != nil { + t.Fatalf("failed to chdir: %v", err) + } + + if err := updateActions(context.Background(), deps, true, false, false, 0); err != nil { + t.Fatalf("updateActions() error = %v", err) + } + + saved := workflow.NewActionCache(tmpDir) + if err := saved.Load(); err != nil { + t.Fatalf("failed to reload cache: %v", err) + } + + existing, ok := saved.Entries["ruby/setup-ruby@v1.300.0"] + if !ok { + t.Fatalf("expected existing entry to be retained when resolved SHA is empty; got entries: %v", savedEntryKeys(saved)) + } + if existing.SHA != "sha300_12345678901234567890123456789012" { + t.Fatalf("expected retained entry SHA to stay unchanged, got %q", existing.SHA) + } + if _, ok := saved.Entries["ruby/setup-ruby@v1.321.0"]; ok { + t.Fatalf("did not expect new empty-SHA version entry to be created; got entries: %v", savedEntryKeys(saved)) + } +} diff --git a/pkg/cli/update_workflows.go b/pkg/cli/update_workflows.go index d1a2abc2bdb..e8fca780aff 100644 --- a/pkg/cli/update_workflows.go +++ b/pkg/cli/update_workflows.go @@ -386,6 +386,35 @@ func fetchPublicGitHubAPI(ctx context.Context, endpoint string) ([]byte, error) return body, nil } +func fetchPublicReleaseTagsPaginated(ctx context.Context, repo string) ([]string, error) { + var tags []string + for page := 1; ; page++ { + endpoint := fmt.Sprintf("/repos/%s/releases?per_page=100&page=%d", repo, page) + body, err := fetchPublicGitHubAPI(ctx, endpoint) + if err != nil { + return nil, err + } + var releases []struct { + TagName string `json:"tag_name"` + } + if err := json.Unmarshal(body, &releases); err != nil { + return nil, fmt.Errorf("failed to parse releases response: %w", err) + } + if len(releases) == 0 { + break + } + for _, r := range releases { + if r.TagName != "" { + tags = append(tags, r.TagName) + } + } + if len(releases) < 100 { + break + } + } + return tags, nil +} + // getRepoDefaultBranch fetches the default branch name for a repository. func getRepoDefaultBranch(ctx context.Context, repo string) (string, error) { output, err := workflow.RunGHContext(ctx, "Fetching repo info...", "api", "/repos/"+repo, "--jq", ".default_branch") @@ -456,31 +485,21 @@ func getLatestBranchCommitSHA(ctx context.Context, repo, branch string) (string, type workflowUpdateDeps struct { runReleasesAPI func(ctx context.Context, repo string) ([]byte, error) + checkCoolDown func(ctx context.Context, repo, tag string, coolDown time.Duration) coolDownCheckResult } func defaultWorkflowUpdateDeps() workflowUpdateDeps { return workflowUpdateDeps{ + checkCoolDown: checkReleaseCoolDown, runReleasesAPI: func(ctx context.Context, repo string) ([]byte, error) { endpoint := fmt.Sprintf("/repos/%s/releases", repo) - output, err := workflow.RunGHContext(ctx, "Fetching releases...", "api", endpoint, "--jq", ".[].tag_name") + output, err := workflow.RunGHContext(ctx, "Fetching releases...", "api", "--paginate", endpoint, "--jq", ".[].tag_name") if err != nil && gitutil.IsAuthError(err.Error()) { updateLog.Printf("GitHub API auth failed for releases of %s, retrying without token", repo) - body, fallbackErr := fetchPublicGitHubAPI(ctx, endpoint) + tags, fallbackErr := fetchPublicReleaseTagsPaginated(ctx, repo) if fallbackErr != nil { return nil, fmt.Errorf("failed (with token: %w; without token: %w)", err, fallbackErr) } - var releases []struct { - TagName string `json:"tag_name"` - } - if fallbackErr = json.Unmarshal(body, &releases); fallbackErr != nil { - return nil, fmt.Errorf("failed to parse releases response: %w", fallbackErr) - } - var tags []string - for _, r := range releases { - if r.TagName != "" { - tags = append(tags, r.TagName) - } - } return []byte(strings.Join(tags, "\n")), nil } return output, err @@ -541,50 +560,57 @@ func resolveLatestReleaseWithDeps(ctx context.Context, deps workflowUpdateDeps, return latestStable, nil } - // Find the latest compatible non-prerelease release. + // Find all compatible non-prerelease releases. // 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 + compatibleReleases := sortedCompatibleReleaseCandidates(releases, currentVer, allowMajor) - for _, release := range releases { - releaseVer := parseVersion(release) - if releaseVer == nil || releaseVer.Pre != "" { - continue - } + if len(compatibleReleases) == 0 { + return "", errors.New("no compatible release found") + } - // Check if compatible based on major version - if !allowMajor && releaseVer.Major != currentVer.Major { - continue - } + // Collect upgrade candidates: releases strictly newer than current. + upgradeCandidates := newerReleaseCandidates(compatibleReleases, currentVer) - // Check if this is newer than what we have - if latestCompatibleVersion == nil || releaseVer.IsNewer(latestCompatibleVersion) { - latestCompatible = release - latestCompatibleVersion = releaseVer - } + // No upgrade available – already at the latest compatible release. + if len(upgradeCandidates) == 0 { + return currentRef, nil } - if latestCompatible == "" { - return "", errors.New("no compatible release found") + // If the repo is exempt from cooldown, return the best (newest) upgrade candidate. + if isExemptFromCoolDown(repo) { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Found newer release: "+upgradeCandidates[0].tag)) + } + return upgradeCandidates[0].tag, nil } - // Apply cooldown: if the latest release is newer than the current and the repo is not - // exempt from cooldown, check whether the release is recent enough to be held back. - if latestCompatible != currentRef && !isExemptFromCoolDown(repo) { - if result := checkReleaseCoolDown(ctx, repo, latestCompatible, coolDown); result.InCoolDown { - cooldownLog.Printf("Workflow source %s: %s", repo, result.Message) - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Skipping update for %s: %s", repo, result.Message))) - // Return the current ref — no update until cooldown expires. - return currentRef, nil + // Iterate from newest to oldest upgrade candidate, stopping at the first release + // that has passed the cooldown period. This lets users receive the latest cooled + // release rather than being blocked on a single too-recent version. + loggedCandidateSkip := false + cooldownSkippedCount := 0 + for _, c := range upgradeCandidates { + result := deps.checkCoolDown(ctx, repo, c.tag, coolDown) + if !result.InCoolDown { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Found newer release: "+c.tag)) + } + return c.tag, nil + } + cooldownSkippedCount++ + cooldownLog.Printf("Workflow source %s: %s", repo, result.Message) + if !loggedCandidateSkip { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Skipping release candidate %s@%s: %s", repo, c.tag, result.Message))) + loggedCandidateSkip = true } } - - if verbose && latestCompatible != currentRef { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Found newer release: "+latestCompatible)) + if cooldownSkippedCount > 1 { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Evaluated %d release candidates for %s; all are still in cooldown", cooldownSkippedCount, repo))) } - return latestCompatible, nil + // All upgrade candidates are still within the cooldown window. + return currentRef, nil } // updateWorkflow updates a single workflow from its source diff --git a/pkg/cli/update_workflows_test.go b/pkg/cli/update_workflows_test.go index 5e53b41eb58..583da159f0e 100644 --- a/pkg/cli/update_workflows_test.go +++ b/pkg/cli/update_workflows_test.go @@ -5,6 +5,7 @@ package cli import ( "context" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -90,3 +91,49 @@ func TestResolveLatestRelease_MixedPrereleaseAndStable(t *testing.T) { 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") } + +// TestResolveLatestRelease_CooldownFallback verifies that when the newest upgrade +// candidate is within the cooldown window the resolver falls back to the next +// older release that has already passed the cooldown period. +func TestResolveLatestRelease_CooldownFallback(t *testing.T) { + t.Parallel() + + deps := mockWorkflowUpdateDeps(func(_ context.Context, _ string) ([]byte, error) { + // v1.3.0 is the newest; v1.2.0 is one step older and has cooled down. + return []byte("v1.3.0\nv1.2.0\nv1.0.0"), nil + }) + deps.checkCoolDown = func(_ context.Context, repo, tag string, cd time.Duration) coolDownCheckResult { + switch tag { + case "v1.3.0": + // Too new: published 2 days ago. + return checkReleaseCoolDownWithDate(repo, tag, time.Now().Add(-2*24*time.Hour), cd) + case "v1.2.0": + // Cooled down: published 10 days ago. + return checkReleaseCoolDownWithDate(repo, tag, time.Now().Add(-10*24*time.Hour), cd) + default: + return coolDownCheckResult{} + } + } + + result, err := resolveLatestReleaseWithDeps(context.Background(), deps, "owner/repo", "v1.0.0", false, false, 7*24*time.Hour) + require.NoError(t, err) + assert.Equal(t, "v1.2.0", result, "should fall back to v1.2.0 when v1.3.0 is in cooldown") +} + +// TestResolveLatestRelease_CooldownAllInWindow returns currentRef when every +// upgrade candidate is still within the cooldown window. +func TestResolveLatestRelease_CooldownAllInWindow(t *testing.T) { + t.Parallel() + + deps := mockWorkflowUpdateDeps(func(_ context.Context, _ string) ([]byte, error) { + return []byte("v1.2.0\nv1.1.0\nv1.0.0"), nil + }) + // All upgrade candidates are too new. + deps.checkCoolDown = func(_ context.Context, repo, tag string, cd time.Duration) coolDownCheckResult { + return checkReleaseCoolDownWithDate(repo, tag, time.Now().Add(-1*24*time.Hour), cd) + } + + result, err := resolveLatestReleaseWithDeps(context.Background(), deps, "owner/repo", "v1.0.0", false, false, 7*24*time.Hour) + require.NoError(t, err) + assert.Equal(t, "v1.0.0", result, "should return currentRef when all upgrade candidates are in cooldown") +} diff --git a/pkg/workflow/action_cache.go b/pkg/workflow/action_cache.go index 56aa0264ee5..5ad8371408e 100644 --- a/pkg/workflow/action_cache.go +++ b/pkg/workflow/action_cache.go @@ -429,7 +429,11 @@ func (c *ActionCache) FindAnyEntryForRepo(repo string) (string, ActionCacheEntry // Set stores a new cache entry, preserving any already-cached inputs when the SHA // is unchanged. If the SHA changes (e.g. a moving tag points to a new commit), // cached inputs are cleared to stay consistent with the newly-pinned commit. -func (c *ActionCache) Set(repo, version, sha string) { +func (c *ActionCache) Set(repo, version, sha string) bool { + if sha == "" { + actionCacheLog.Printf("refusing to store action pin entry with empty SHA for %s@%s; entry skipped", repo, version) + return false + } key := formatActionCacheKey(repo, version) // Check if there are existing entries with the same repo+SHA but different version @@ -473,6 +477,7 @@ func (c *ActionCache) Set(repo, version, sha string) { ActionDescription: description, } c.dirty = true // Mark cache as modified + return true } // GetInputs retrieves the cached action inputs for the given repo and version. diff --git a/pkg/workflow/action_cache_test.go b/pkg/workflow/action_cache_test.go index 87e7863a369..6fb12638780 100644 --- a/pkg/workflow/action_cache_test.go +++ b/pkg/workflow/action_cache_test.go @@ -1045,3 +1045,35 @@ func TestPruneOrphanedEntries_PreservesCompilerGenerated(t *testing.T) { t.Error("Expected runtime-managed ruby/setup-ruby@v1 to be preserved") } } + +// TestActionCache_Set_EmptySHARejected verifies that calling Set with an empty SHA +// is a no-op: the entry must not be stored in the cache, preventing an invalid +// (SHA-less) pin from being persisted to actions-lock.json. +func TestActionCache_Set_EmptySHARejected(t *testing.T) { + cache := NewActionCache(t.TempDir()) + + // Calling Set with an empty SHA must not create any entry. + if ok := cache.Set("docker/login-action", "v3.1.0", ""); ok { + t.Error("Set with empty SHA must report failure") + } + + if _, exists := cache.Entries["docker/login-action@v3.1.0"]; exists { + t.Error("Set with empty SHA must not create a cache entry") + } + if len(cache.Entries) != 0 { + t.Errorf("Expected 0 entries after Set with empty SHA, got %d", len(cache.Entries)) + } + + // A subsequent Set with a real SHA must succeed normally. + const sha = "abc1234567890123456789012345678901234567" + if ok := cache.Set("docker/login-action", "v3.1.0", sha); !ok { + t.Fatal("Set with valid SHA must report success") + } + entry, exists := cache.Entries["docker/login-action@v3.1.0"] + if !exists { + t.Fatal("Set with valid SHA did not create a cache entry") + } + if entry.SHA != sha { + t.Errorf("entry SHA = %q, want %q", entry.SHA, sha) + } +}