-
Notifications
You must be signed in to change notification settings - Fork 473
feat: fall back to older cooled-down release instead of skipping update #47730
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
2982dcb
1cb029a
00cb7e3
71000af
cfd776e
22bd799
f649ce5
eac8516
6d06210
ee0097f
369e968
b16d1e4
4644052
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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.* |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/codebase-design] Consider extracting a shared @copilot please address this. |
||
| // (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) | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/tdd] 💡 Suggested testfunc TestFindCooledDownActionVersion_SHAErrorFallsToNext(t *testing.T) {
deps := newTestActionUpdateDeps()
deps.runGHReleasesAPI = func(_ context.Context, _ string) ([]byte, error) {
return []byte("v1.322.0
v1.321.0
v1.320.0"), nil
}
deps.checkCoolDown = func(_ context.Context, _, tag string, cd time.Duration) coolDownCheckResult {
return coolDownCheckResult{} // all 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...", nil
}
return "", errors.New("unexpected")
}
version, _, err := findCooledDownActionVersion(context.Background(), deps, "repo/action", "v1.320.0", true, false, 7*24*time.Hour)
require.NoError(t, err)
assert.Equal(t, "v1.321.0", version)
}@copilot please address this. |
||
| 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) | ||
|
Comment on lines
+603
to
+607
|
||
| 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 | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[/diagnosing-bugs] The
"Skipping update for %s"message is printed to stderr unconditionally before attempting the fallback — so users see "Skipping update for ruby/setup-ruby" in the output even when the update actually proceeds to an older fallback version.💡 Suggested fix
Move the stderr user-facing message inside the
if findErr != nil || olderVersion == "" ...branch, and add an informational message on the fallback path:@copilot please address this.