From 14b8ed209c3eb2f39b3fd81c4dff00c308441a86 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 1 May 2026 06:17:53 +0000 Subject: [PATCH 1/4] feat: add --cool-down flag to update command with 7d default - Add new --cool-down flag (default 7d) to gh aw update command - Create update_cooldown.go with cooldown logic: - parseCoolDownFlag: parse "7d", "168h", "0" formats - isExemptFromCoolDown: exempt actions/* and github/* repos - checkReleaseCoolDown: check if release is within cooldown window - getReleasePublishedAtFn: injectable for tests - Thread coolDown parameter through UpdateWorkflows, UpdateActions, UpdateActionsInWorkflowFiles, updateActionRefsInContent, resolveLatestRelease, resolveLatestRef, resolveRedirectedUpdateLocation - Apply cooldown check after finding a newer release (fail-open if API unavailable) - Update upgrade_command.go to pass 0 cooldown (no cooldown for upgrade) - Add comprehensive tests in update_cooldown_test.go - Update all existing tests to pass 0 cooldown parameter Agent-Logs-Url: https://github.com/github/gh-aw/sessions/259f1ed8-f33f-44f5-99d1-0def0d5d86eb Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/README.md | 2 +- pkg/cli/update_actions.go | 42 +++++++- pkg/cli/update_actions_test.go | 16 +-- pkg/cli/update_command.go | 24 +++-- pkg/cli/update_command_test.go | 12 +-- pkg/cli/update_cooldown.go | 122 +++++++++++++++++++++ pkg/cli/update_cooldown_test.go | 175 +++++++++++++++++++++++++++++++ pkg/cli/update_redirects.go | 5 +- pkg/cli/update_redirects_test.go | 10 +- pkg/cli/update_workflows.go | 28 +++-- pkg/cli/update_workflows_test.go | 10 +- pkg/cli/upgrade_command.go | 2 +- 12 files changed, 403 insertions(+), 45 deletions(-) create mode 100644 pkg/cli/update_cooldown.go create mode 100644 pkg/cli/update_cooldown_test.go diff --git a/pkg/cli/README.md b/pkg/cli/README.md index b0d97c052a2..7fb6956e1fe 100644 --- a/pkg/cli/README.md +++ b/pkg/cli/README.md @@ -132,7 +132,7 @@ All diagnostic output MUST go to `stderr` using `console` formatting helpers. St | `AddMCPTool` | `func(string, string, ...) error` | Adds an MCP server to a workflow file | | `InspectWorkflowMCP` | `func(string, ...) error` | Inspects MCP server configurations | | `ListWorkflowMCP` | `func(string, bool) error` | Lists MCP server info for a workflow | -| `UpdateActions` | `func(bool, bool, bool) error` | Bulk-updates GitHub Action versions in workflows | +| `UpdateActions` | `func(bool, bool, bool, time.Duration) error` | Bulk-updates GitHub Action versions in workflows | | `ActionsBuildCommand` | `func() error` | Builds all custom actions in `actions/` | | `ActionsValidateCommand` | `func() error` | Validates all `action.yml` files under `actions/` | | `ActionsCleanCommand` | `func() error` | Removes generated action build artifacts | diff --git a/pkg/cli/update_actions.go b/pkg/cli/update_actions.go index c29dbd36e75..9a170348fa2 100644 --- a/pkg/cli/update_actions.go +++ b/pkg/cli/update_actions.go @@ -10,6 +10,7 @@ import ( "regexp" "sort" "strings" + "time" "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/gitutil" @@ -36,10 +37,13 @@ func isGhAwNativeAction(repo string) bool { // By default all actions are updated to the latest major version; pass disableReleaseBump=true // to revert to the old behaviour where only core (actions/*) actions bypass the --major flag. // +// coolDown specifies the minimum age a release must have before it is applied. Repos under the +// "actions/" and "github/" namespaces are always exempt from the cooldown. +// // The ActionCache helpers from pkg/workflow are used so that cached inputs and descriptions // for safe-outputs.actions entries are preserved when their SHA is unchanged, and cleared // when the SHA changes (prompting a re-fetch on the next compile). -func UpdateActions(ctx context.Context, allowMajor, verbose, disableReleaseBump bool) error { +func UpdateActions(ctx context.Context, allowMajor, verbose, disableReleaseBump bool, coolDown time.Duration) error { updateLog.Print("Starting action updates") if verbose { @@ -137,6 +141,16 @@ func UpdateActions(ctx context.Context, allowMajor, verbose, disableReleaseBump continue } + // Apply cooldown: if the repo is not exempt and the release is too recent, skip. + if !isExemptFromCoolDown(entry.Repo) { + if result := checkReleaseCoolDown(ctx, entry.Repo, latestVersion, coolDown); result.InCoolDown { + cooldownLog.Printf("Action %s: %s", entry.Repo, result.Message) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Skipping update for %s: %s", entry.Repo, result.Message))) + skippedActions = append(skippedActions, entry.Repo) + continue + } + } + // Update the entry using ActionCache.Set which: // - Preserves cached inputs/descriptions when the SHA is unchanged (moving tag) // - Clears cached inputs/descriptions when the SHA changes, prompting a re-fetch @@ -540,7 +554,7 @@ type latestReleaseResult struct { // major version. Updated files are recompiled. By default all actions are updated to // the latest major version; pass disableReleaseBump=true to only update core // (actions/*) references. -func UpdateActionsInWorkflowFiles(ctx context.Context, workflowsDir, engineOverride string, verbose, disableReleaseBump bool, noCompile bool) error { +func UpdateActionsInWorkflowFiles(ctx context.Context, workflowsDir, engineOverride string, verbose, disableReleaseBump bool, noCompile bool, coolDown time.Duration) error { if workflowsDir == "" { workflowsDir = getWorkflowsDir() } @@ -549,6 +563,8 @@ func UpdateActionsInWorkflowFiles(ctx context.Context, workflowsDir, engineOverr // Per-invocation cache: key = "repo@currentVersion", avoids repeated API calls cache := make(map[string]latestReleaseResult) + // Per-invocation cooldown cache: key = "repo@tag", avoids redundant date API calls + coolDownCache := make(map[string]coolDownCheckResult) var updatedFiles []string @@ -571,7 +587,7 @@ func UpdateActionsInWorkflowFiles(ctx context.Context, workflowsDir, engineOverr return nil } - updated, newContent, err := updateActionRefsInContent(ctx, string(content), cache, !disableReleaseBump, verbose) + updated, newContent, err := updateActionRefsInContent(ctx, string(content), cache, coolDownCache, !disableReleaseBump, verbose, coolDown) if err != nil { if verbose { fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to update action refs in %s: %v", path, err))) @@ -614,10 +630,11 @@ func UpdateActionsInWorkflowFiles(ctx context.Context, workflowsDir, engineOverr // updateActionRefsInContent replaces outdated "uses: org/repo@version" references // in content with the latest major version and SHA. Returns (changed, newContent, error). // cache is keyed by "repo@currentVersion" and avoids redundant API calls across lines/files. +// coolDownCache is keyed by "repo@tag" and avoids redundant cooldown date API calls. // When allowMajor is true (the default), all matched actions are updated to the latest // major version. When allowMajor is false (--disable-release-bump), non-core (non // actions/*) action refs are skipped; core actions are always updated. -func updateActionRefsInContent(ctx context.Context, content string, cache map[string]latestReleaseResult, allowMajor, verbose bool) (bool, string, error) { +func updateActionRefsInContent(ctx context.Context, content string, cache map[string]latestReleaseResult, coolDownCache map[string]coolDownCheckResult, allowMajor, verbose bool, coolDown time.Duration) (bool, string, error) { changed := false lines := strings.Split(content, "\n") @@ -689,6 +706,23 @@ func updateActionRefsInContent(ctx context.Context, content string, cache map[st } } + // Apply cooldown: if the repo is not exempt and the release is too recent, skip. + if !isExemptFromCoolDown(repo) { + coolDownKey := repo + "@" + latestVersion + coolDownResult, coolDownCached := coolDownCache[coolDownKey] + if !coolDownCached { + coolDownResult = checkReleaseCoolDown(ctx, repo, latestVersion, coolDown) + coolDownCache[coolDownKey] = coolDownResult + } + if coolDownResult.InCoolDown { + cooldownLog.Printf("Action ref %s in workflow: %s", repo, coolDownResult.Message) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Skipping update for %s: %s", repo, coolDownResult.Message))) + } + continue + } + } + // Build the new uses line var newRef string if isSHA { diff --git a/pkg/cli/update_actions_test.go b/pkg/cli/update_actions_test.go index da6785ef3f1..bdf887bd30e 100644 --- a/pkg/cli/update_actions_test.go +++ b/pkg/cli/update_actions_test.go @@ -146,7 +146,7 @@ func TestUpdateActions_SafeOutputsInputsPreserved(t *testing.T) { t.Fatalf("failed to chdir: %v", err) } - if err := UpdateActions(context.Background(), false, false, false); err != nil { + if err := UpdateActions(context.Background(), false, false, false, 0); err != nil { t.Fatalf("UpdateActions() error = %v", err) } @@ -353,7 +353,7 @@ func TestUpdateActionRefsInContent_NonCoreActionsUnchanged(t *testing.T) { - run: echo hello` cache := make(map[string]latestReleaseResult) - changed, newContent, err := updateActionRefsInContent(context.Background(), input, cache, false, false) + changed, newContent, err := updateActionRefsInContent(context.Background(), input, cache, make(map[string]coolDownCheckResult), false, false, 0) if err != nil { t.Fatalf("updateActionRefsInContent() error = %v", err) } @@ -372,7 +372,7 @@ steps: - run: echo world` cache := make(map[string]latestReleaseResult) - changed, _, err := updateActionRefsInContent(context.Background(), input, cache, true, false) + changed, _, err := updateActionRefsInContent(context.Background(), input, cache, make(map[string]coolDownCheckResult), true, false, 0) if err != nil { t.Fatalf("updateActionRefsInContent() error = %v", err) } @@ -408,7 +408,7 @@ func TestUpdateActionRefsInContent_VersionTagReplacement(t *testing.T) { - run: echo hello` cache := make(map[string]latestReleaseResult) - changed, got, err := updateActionRefsInContent(context.Background(), input, cache, true, false) + changed, got, err := updateActionRefsInContent(context.Background(), input, cache, make(map[string]coolDownCheckResult), true, false, 0) if err != nil { t.Fatalf("updateActionRefsInContent() error = %v", err) } @@ -435,7 +435,7 @@ func TestUpdateActionRefsInContent_SHAPinnedReplacement(t *testing.T) { want := " uses: actions/checkout@" + newSHA + " # v6.0.2" cache := make(map[string]latestReleaseResult) - changed, got, err := updateActionRefsInContent(context.Background(), input, cache, true, false) + changed, got, err := updateActionRefsInContent(context.Background(), input, cache, make(map[string]coolDownCheckResult), true, false, 0) if err != nil { t.Fatalf("updateActionRefsInContent() error = %v", err) } @@ -464,7 +464,7 @@ func TestUpdateActionRefsInContent_CacheReusedAcrossLines(t *testing.T) { - uses: actions/github-script@v7` cache := make(map[string]latestReleaseResult) - changed, _, err := updateActionRefsInContent(context.Background(), input, cache, true, false) + changed, _, err := updateActionRefsInContent(context.Background(), input, cache, make(map[string]coolDownCheckResult), true, false, 0) if err != nil { t.Fatalf("updateActionRefsInContent() error = %v", err) } @@ -502,7 +502,7 @@ func TestUpdateActionRefsInContent_AllOrgsUpdatedWhenAllowMajor(t *testing.T) { - uses: github/codeql-action@v4` cache := make(map[string]latestReleaseResult) - changed, got, err := updateActionRefsInContent(context.Background(), input, cache, true, false) + changed, got, err := updateActionRefsInContent(context.Background(), input, cache, make(map[string]coolDownCheckResult), true, false, 0) if err != nil { t.Fatalf("updateActionRefsInContent() error = %v", err) } @@ -665,7 +665,7 @@ func TestUpdateActions_GhAwNativeActionCappedAtCLIVersion(t *testing.T) { t.Fatalf("failed to chdir: %v", err) } - if err := UpdateActions(context.Background(), false, false, false); err != nil { + if err := UpdateActions(context.Background(), false, false, false, 0); err != nil { t.Fatalf("UpdateActions() error = %v", err) } diff --git a/pkg/cli/update_command.go b/pkg/cli/update_command.go index cb305d3da02..5f9c4960b67 100644 --- a/pkg/cli/update_command.go +++ b/pkg/cli/update_command.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "time" "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/constants" @@ -48,7 +49,9 @@ Examples: ` + string(constants.CLIExtensionPrefix) + ` update --no-compile # Update without regenerating lock files ` + string(constants.CLIExtensionPrefix) + ` update --no-redirect # Refuse workflows that use redirect frontmatter ` + string(constants.CLIExtensionPrefix) + ` update --dir custom/workflows # Update workflows in custom directory - ` + string(constants.CLIExtensionPrefix) + ` update --create-pull-request # Update and open a pull request`, + ` + string(constants.CLIExtensionPrefix) + ` update --create-pull-request # Update and open a pull request + ` + string(constants.CLIExtensionPrefix) + ` update --cool-down 0 # Disable cooldown and apply all pending releases immediately + ` + string(constants.CLIExtensionPrefix) + ` update --cool-down 3d # Apply a custom 3-day cooldown period`, RunE: func(cmd *cobra.Command, args []string) error { majorFlag, _ := cmd.Flags().GetBool("major") forceFlag, _ := cmd.Flags().GetBool("force") @@ -64,18 +67,24 @@ Examples: createPRFlag, _ := cmd.Flags().GetBool("create-pull-request") prFlagAlias, _ := cmd.Flags().GetBool("pr") createPR := createPRFlag || prFlagAlias + coolDownStr, _ := cmd.Flags().GetString("cool-down") if err := validateEngine(engineOverride); err != nil { return err } + coolDown, err := parseCoolDownFlag(coolDownStr) + if err != nil { + return fmt.Errorf("invalid --cool-down value: %w", err) + } + if createPR { if err := PreflightCheckForCreatePR(verbose); err != nil { return err } } - if err := RunUpdateWorkflows(cmd.Context(), args, majorFlag, forceFlag, verbose, engineOverride, workflowDir, noStopAfter, stopAfter, noMergeFlag, disableReleaseBump, noCompile, noRedirect); err != nil { + if err := RunUpdateWorkflows(cmd.Context(), args, majorFlag, forceFlag, verbose, engineOverride, workflowDir, noStopAfter, stopAfter, noMergeFlag, disableReleaseBump, noCompile, noRedirect, coolDown); err != nil { return err } @@ -101,6 +110,7 @@ Examples: cmd.Flags().Bool("no-redirect", false, "Refuse updates when redirect frontmatter is present") cmd.Flags().Bool("create-pull-request", false, "Create a pull request with the update changes") cmd.Flags().Bool("pr", false, "Alias for --create-pull-request") + cmd.Flags().String("cool-down", "7d", "Cooldown period before applying a new release (e.g. 7d, 24h, 0 to disable). Does not apply to actions/* or github/* repositories") _ = cmd.Flags().MarkHidden("pr") // Hide the short alias from help output // Register completions for update command @@ -113,12 +123,12 @@ Examples: // RunUpdateWorkflows updates workflows from their source repositories. // Each workflow is compiled immediately after update. -func RunUpdateWorkflows(ctx context.Context, workflowNames []string, allowMajor, force, verbose bool, engineOverride string, workflowsDir string, noStopAfter bool, stopAfter string, noMerge bool, disableReleaseBump bool, noCompile bool, noRedirect bool) error { - updateLog.Printf("Starting update process: workflows=%v, allowMajor=%v, force=%v, noMerge=%v, disableReleaseBump=%v, noCompile=%v, noRedirect=%v", workflowNames, allowMajor, force, noMerge, disableReleaseBump, noCompile, noRedirect) +func RunUpdateWorkflows(ctx context.Context, workflowNames []string, allowMajor, force, verbose bool, engineOverride string, workflowsDir string, noStopAfter bool, stopAfter string, noMerge bool, disableReleaseBump bool, noCompile bool, noRedirect bool, coolDown time.Duration) error { + updateLog.Printf("Starting update process: workflows=%v, allowMajor=%v, force=%v, noMerge=%v, disableReleaseBump=%v, noCompile=%v, noRedirect=%v, coolDown=%v", workflowNames, allowMajor, force, noMerge, disableReleaseBump, noCompile, noRedirect, coolDown) var firstErr error - if err := UpdateWorkflows(ctx, workflowNames, allowMajor, force, verbose, engineOverride, workflowsDir, noStopAfter, stopAfter, noMerge, noCompile, noRedirect); err != nil { + if err := UpdateWorkflows(ctx, workflowNames, allowMajor, force, verbose, engineOverride, workflowsDir, noStopAfter, stopAfter, noMerge, noCompile, noRedirect, coolDown); err != nil { firstErr = fmt.Errorf("workflow update failed: %w", err) } @@ -126,7 +136,7 @@ func RunUpdateWorkflows(ctx context.Context, workflowNames []string, allowMajor, // By default all actions are updated to the latest major version. // Pass --disable-release-bump to revert to only forcing updates for core (actions/*) actions. updateLog.Printf("Updating GitHub Actions versions in actions-lock.json: allowMajor=%v, disableReleaseBump=%v", allowMajor, disableReleaseBump) - if err := UpdateActions(ctx, allowMajor, verbose, disableReleaseBump); err != nil { + if err := UpdateActions(ctx, allowMajor, verbose, disableReleaseBump, coolDown); err != nil { // Non-fatal: warn but don't fail the update fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Warning: Failed to update actions-lock.json: %v", err))) } @@ -141,7 +151,7 @@ func RunUpdateWorkflows(ctx context.Context, workflowNames []string, allowMajor, // Update action references in user-provided steps within workflow .md files. // By default all org/repo@version references are updated to the latest major version. updateLog.Print("Updating action references in workflow .md files") - if err := UpdateActionsInWorkflowFiles(ctx, workflowsDir, engineOverride, verbose, disableReleaseBump, noCompile); err != nil { + if err := UpdateActionsInWorkflowFiles(ctx, workflowsDir, engineOverride, verbose, disableReleaseBump, noCompile, coolDown); err != nil { // Non-fatal: warn but don't fail the update fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Warning: Failed to update action references in workflow files: %v", err))) } diff --git a/pkg/cli/update_command_test.go b/pkg/cli/update_command_test.go index 4aac6b3143c..c5256f272c9 100644 --- a/pkg/cli/update_command_test.go +++ b/pkg/cli/update_command_test.go @@ -813,7 +813,7 @@ func TestUpdateActions_NoFile(t *testing.T) { os.Chdir(tmpDir) // Should not error when file doesn't exist - err := UpdateActions(context.Background(), false, false, false) + err := UpdateActions(context.Background(), false, false, false, 0) if err != nil { t.Errorf("Expected no error when actions-lock.json doesn't exist, got: %v", err) } @@ -844,7 +844,7 @@ func TestUpdateActions_EmptyFile(t *testing.T) { os.Chdir(tmpDir) // Should not error with empty file - err := UpdateActions(context.Background(), false, false, false) + err := UpdateActions(context.Background(), false, false, false, 0) if err != nil { t.Errorf("Expected no error with empty actions-lock.json, got: %v", err) } @@ -873,7 +873,7 @@ func TestUpdateActions_InvalidJSON(t *testing.T) { os.Chdir(tmpDir) // Should error with invalid JSON - err := UpdateActions(context.Background(), false, false, false) + err := UpdateActions(context.Background(), false, false, false, 0) if err == nil { t.Error("Expected error with invalid JSON, got nil") } @@ -894,7 +894,7 @@ func TestResolveLatestRef_CommitSHA(t *testing.T) { // in authenticated environments it will succeed. Either outcome is // acceptable — the key invariant is that the SHA is correctly // identified (tested above) and the function does not panic. - _, _ = resolveLatestRef(context.Background(), "test/repo", sha, false, false) + _, _ = resolveLatestRef(context.Background(), "test/repo", sha, false, false, 0) } // TestResolveLatestRef_NotCommitSHA tests that non-SHA refs are handled appropriately @@ -980,7 +980,7 @@ func TestRunUpdateWorkflows_NoSourceWorkflows(t *testing.T) { os.Chdir(tmpDir) // Running update with no source workflows should succeed with an info message, not an error - err := RunUpdateWorkflows(context.Background(), nil, false, false, false, "", "", false, "", false, false, false, false) + err := RunUpdateWorkflows(context.Background(), nil, false, false, false, "", "", false, "", false, false, false, false, 0) assert.NoError(t, err, "Should not error when no workflows with source field exist") } @@ -996,7 +996,7 @@ func TestRunUpdateWorkflows_SpecificWorkflowNotFound(t *testing.T) { os.Chdir(tmpDir) // Running update with a specific name that doesn't exist should fail - err := RunUpdateWorkflows(context.Background(), []string{"nonexistent"}, false, false, false, "", "", false, "", false, false, false, false) + err := RunUpdateWorkflows(context.Background(), []string{"nonexistent"}, false, false, false, "", "", false, "", false, false, false, false, 0) require.Error(t, err, "Should error when specified workflow not found") assert.Contains(t, err.Error(), "no workflows found matching the specified names") } diff --git a/pkg/cli/update_cooldown.go b/pkg/cli/update_cooldown.go new file mode 100644 index 00000000000..20cd1e39924 --- /dev/null +++ b/pkg/cli/update_cooldown.go @@ -0,0 +1,122 @@ +package cli + +import ( + "context" + "fmt" + "strconv" + "strings" + "time" + + "github.com/cli/go-gh/v2/pkg/api" + "github.com/github/gh-aw/pkg/gitutil" + "github.com/github/gh-aw/pkg/logger" +) + +var cooldownLog = logger.New("cli:update_cooldown") + +// parseCoolDownFlag parses a cooldown duration string. +// Accepts day-suffix notation ("7d") or Go duration format ("168h", "0"). +// Returns 0 for "0", "0d", or "0h" (cooldown disabled). +func parseCoolDownFlag(s string) (time.Duration, error) { + if daysStr, ok := strings.CutSuffix(s, "d"); ok { + days, err := strconv.Atoi(daysStr) + if err != nil || days < 0 { + return 0, fmt.Errorf("invalid cooldown value %q: expected a non-negative number of days (e.g. 7d)", s) + } + return time.Duration(days) * 24 * time.Hour, nil + } + d, err := time.ParseDuration(s) + if err != nil { + return 0, fmt.Errorf("invalid cooldown value %q: %w", s, err) + } + if d < 0 { + return 0, fmt.Errorf("invalid cooldown value %q: duration must be non-negative", s) + } + return d, nil +} + +// isExemptFromCoolDown returns true for repositories that bypass the cooldown period. +// Repositories under the "actions/" and "github/" namespaces are always updated immediately. +func isExemptFromCoolDown(repo string) bool { + base := gitutil.ExtractBaseRepo(repo) + return strings.HasPrefix(base, "actions/") || strings.HasPrefix(base, "github/") +} + +// githubReleaseInfo holds the publication date from a GitHub release API response. +type githubReleaseInfo struct { + PublishedAt time.Time `json:"published_at"` +} + +// getReleasePublishedAtFn fetches the published_at timestamp for a release tag. +// It is a package-level variable so that tests can replace it without network calls. +var getReleasePublishedAtFn = func(ctx context.Context, repo, tag string) (time.Time, error) { + return getReleasePublishedAt(ctx, repo, tag) +} + +func getReleasePublishedAt(ctx context.Context, repo, tag string) (time.Time, error) { + client, err := api.NewRESTClient(api.ClientOptions{}) + if err != nil { + return time.Time{}, fmt.Errorf("failed to create GitHub client: %w", err) + } + var release githubReleaseInfo + if err := client.Get(fmt.Sprintf("repos/%s/releases/tags/%s", repo, tag), &release); err != nil { + return time.Time{}, fmt.Errorf("failed to fetch release info for %s@%s: %w", repo, tag, err) + } + return release.PublishedAt, nil +} + +// coolDownCheckResult holds the result of a single cooldown check. +type coolDownCheckResult struct { + // InCoolDown is true when the release is too recent to apply. + InCoolDown bool + // Message is a human-readable explanation when InCoolDown is true. + Message string +} + +// checkReleaseCoolDown returns a result indicating whether a release tag is +// within the cooldown window and should be skipped. +// +// The function is fail-open: if the publication date cannot be fetched (e.g. +// due to network issues), the update is allowed and InCoolDown is false. +func checkReleaseCoolDown(ctx context.Context, repo, tag string, coolDown time.Duration) coolDownCheckResult { + if coolDown <= 0 { + return coolDownCheckResult{} + } + + base := gitutil.ExtractBaseRepo(repo) + publishedAt, err := getReleasePublishedAtFn(ctx, base, tag) + if err != nil { + cooldownLog.Printf("Failed to get published date for %s@%s (allowing update): %v", repo, tag, err) + return coolDownCheckResult{} + } + + age := time.Since(publishedAt) + if age >= coolDown { + return coolDownCheckResult{} + } + + remaining := coolDown - age + msg := fmt.Sprintf("%s@%s was published %s ago and needs to cool down (%s remaining out of %s cooldown period)", + repo, tag, formatCoolDownDuration(age), formatCoolDownDuration(remaining), formatCoolDownDuration(coolDown)) + return coolDownCheckResult{InCoolDown: true, Message: msg} +} + +// formatCoolDownDuration formats a duration for display in cooldown messages. +// Uses "Xd Yh" format for multi-day durations, "Xh" for sub-day durations. +func formatCoolDownDuration(d time.Duration) string { + if d < 0 { + d = 0 + } + days := int(d.Hours()) / 24 + hours := int(d.Hours()) % 24 + switch { + case days > 0 && hours > 0: + return fmt.Sprintf("%dd%dh", days, hours) + case days > 0: + return fmt.Sprintf("%dd", days) + case int(d.Hours()) > 0: + return fmt.Sprintf("%dh", int(d.Hours())) + default: + return "< 1h" + } +} diff --git a/pkg/cli/update_cooldown_test.go b/pkg/cli/update_cooldown_test.go new file mode 100644 index 00000000000..5a1ace0ff81 --- /dev/null +++ b/pkg/cli/update_cooldown_test.go @@ -0,0 +1,175 @@ +//go:build !integration + +package cli + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseCoolDownFlag(t *testing.T) { + tests := []struct { + name string + input string + want time.Duration + wantErr bool + }{ + { + name: "default 7d", + input: "7d", + want: 7 * 24 * time.Hour, + }, + { + name: "0d disables cooldown", + input: "0d", + want: 0, + }, + { + name: "0 disables cooldown", + input: "0", + want: 0, + }, + { + name: "hours format", + input: "168h", + want: 168 * time.Hour, + }, + { + name: "single day", + input: "1d", + want: 24 * time.Hour, + }, + { + name: "negative days", + input: "-1d", + wantErr: true, + }, + { + name: "negative duration", + input: "-1h", + wantErr: true, + }, + { + name: "invalid string", + input: "foobar", + wantErr: true, + }, + { + name: "invalid days value", + input: "xd", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseCoolDownFlag(tt.input) + if tt.wantErr { + assert.Error(t, err, "parseCoolDownFlag(%q) should return error", tt.input) + return + } + require.NoError(t, err, "parseCoolDownFlag(%q) unexpected error", tt.input) + assert.Equal(t, tt.want, got, "parseCoolDownFlag(%q) duration mismatch", tt.input) + }) + } +} + +func TestIsExemptFromCoolDown(t *testing.T) { + tests := []struct { + repo string + want bool + }{ + {"actions/checkout", true}, + {"actions/setup-node", true}, + {"actions/cache/restore", true}, + {"github/codeql-action", true}, + {"github/gh-aw", true}, + {"github/codeql-action/upload-sarif", true}, + {"owner/repo", false}, + {"myorg/my-action", false}, + {"notactions/checkout", false}, + {"notgithub/repo", false}, + } + + for _, tt := range tests { + t.Run(tt.repo, func(t *testing.T) { + got := isExemptFromCoolDown(tt.repo) + assert.Equal(t, tt.want, got, "isExemptFromCoolDown(%q) result mismatch", tt.repo) + }) + } +} + +func TestCheckReleaseCoolDown_Disabled(t *testing.T) { + result := checkReleaseCoolDown(context.Background(), "owner/repo", "v1.2.0", 0) + assert.False(t, result.InCoolDown, "cooldown should be disabled when duration is 0") +} + +func TestCheckReleaseCoolDown_OldRelease(t *testing.T) { + orig := getReleasePublishedAtFn + defer func() { getReleasePublishedAtFn = orig }() + + // Simulate a release published 10 days ago (older than 7-day cooldown) + getReleasePublishedAtFn = func(_ context.Context, _, _ string) (time.Time, error) { + return time.Now().Add(-10 * 24 * time.Hour), nil + } + + result := checkReleaseCoolDown(context.Background(), "owner/repo", "v1.2.0", 7*24*time.Hour) + assert.False(t, result.InCoolDown, "release older than cooldown period should not be blocked") +} + +func TestCheckReleaseCoolDown_RecentRelease(t *testing.T) { + orig := getReleasePublishedAtFn + defer func() { getReleasePublishedAtFn = orig }() + + // Simulate a release published 2 days ago (within 7-day cooldown) + getReleasePublishedAtFn = func(_ context.Context, _, _ string) (time.Time, error) { + return time.Now().Add(-2 * 24 * time.Hour), nil + } + + result := checkReleaseCoolDown(context.Background(), "owner/repo", "v1.2.0", 7*24*time.Hour) + assert.True(t, result.InCoolDown, "release within cooldown period should be blocked") + assert.Contains(t, result.Message, "v1.2.0", "message should mention the release tag") + assert.Contains(t, result.Message, "cool down", "message should mention cooldown") +} + +func TestCheckReleaseCoolDown_FetchError(t *testing.T) { + orig := getReleasePublishedAtFn + defer func() { getReleasePublishedAtFn = orig }() + + // Simulate API error + getReleasePublishedAtFn = func(_ context.Context, _, _ string) (time.Time, error) { + return time.Time{}, errors.New("network error") + } + + // Fail-open: when date can't be fetched, allow update + result := checkReleaseCoolDown(context.Background(), "owner/repo", "v1.2.0", 7*24*time.Hour) + assert.False(t, result.InCoolDown, "should allow update when published date cannot be fetched") +} + +func TestFormatCoolDownDuration(t *testing.T) { + tests := []struct { + duration time.Duration + want string + }{ + {7 * 24 * time.Hour, "7d"}, + {7*24*time.Hour + 12*time.Hour, "7d12h"}, + {24 * time.Hour, "1d"}, + {48 * time.Hour, "2d"}, + {12 * time.Hour, "12h"}, + {1 * time.Hour, "1h"}, + {30 * time.Minute, "< 1h"}, + {0, "< 1h"}, + } + + for _, tt := range tests { + t.Run(tt.want, func(t *testing.T) { + got := formatCoolDownDuration(tt.duration) + assert.Equal(t, tt.want, got, "formatCoolDownDuration(%v) mismatch", tt.duration) + }) + } +} diff --git a/pkg/cli/update_redirects.go b/pkg/cli/update_redirects.go index b68212f3b89..9afb45c8314 100644 --- a/pkg/cli/update_redirects.go +++ b/pkg/cli/update_redirects.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "strings" + "time" "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/logger" @@ -28,7 +29,7 @@ type resolvedUpdateLocation struct { redirectHistory []string } -func resolveRedirectedUpdateLocation(ctx context.Context, workflowName string, initialSource *SourceSpec, allowMajor, verbose bool, noRedirect bool) (*resolvedUpdateLocation, error) { +func resolveRedirectedUpdateLocation(ctx context.Context, workflowName string, initialSource *SourceSpec, allowMajor, verbose bool, noRedirect bool, coolDown time.Duration) (*resolvedUpdateLocation, error) { updateRedirectsLog.Printf("Resolving update location: workflow=%s, source=%s/%s@%s", workflowName, initialSource.Repo, initialSource.Path, initialSource.Ref) current := &SourceSpec{ Repo: initialSource.Repo, @@ -51,7 +52,7 @@ func resolveRedirectedUpdateLocation(ctx context.Context, workflowName string, i } visited[locationKey] = struct{}{} - latestRef, err := resolveLatestRefFn(ctx, current.Repo, currentRef, allowMajor, verbose) + latestRef, err := resolveLatestRefFn(ctx, current.Repo, currentRef, allowMajor, verbose, coolDown) if err != nil { return nil, fmt.Errorf("failed to resolve latest ref for %s: %w", sourceSpecWithRef(current, currentRef), err) } diff --git a/pkg/cli/update_redirects_test.go b/pkg/cli/update_redirects_test.go index 74ee04c283c..13bffacb462 100644 --- a/pkg/cli/update_redirects_test.go +++ b/pkg/cli/update_redirects_test.go @@ -6,6 +6,7 @@ import ( "context" "fmt" "testing" + "time" "github.com/github/gh-aw/pkg/testutil" "github.com/stretchr/testify/assert" @@ -39,7 +40,7 @@ func TestResolveRedirectedUpdateLocation(t *testing.T) { }) t.Run("follows redirect chain", func(t *testing.T) { - resolveLatestRefFn = func(_ context.Context, _ string, currentRef string, _ bool, _ bool) (string, error) { + resolveLatestRefFn = func(_ context.Context, _ string, currentRef string, _ bool, _ bool, _ time.Duration) (string, error) { return currentRef, nil } downloadWorkflowContentFn = func(_ context.Context, repo, path, ref string, _ bool) ([]byte, error) { @@ -64,6 +65,7 @@ func TestResolveRedirectedUpdateLocation(t *testing.T) { false, false, false, + 0, ) require.NoError(t, err, "redirect chain should resolve") }) @@ -75,7 +77,7 @@ func TestResolveRedirectedUpdateLocation(t *testing.T) { }) t.Run("detects redirect loops", func(t *testing.T) { - resolveLatestRefFn = func(_ context.Context, _ string, currentRef string, _ bool, _ bool) (string, error) { + resolveLatestRefFn = func(_ context.Context, _ string, currentRef string, _ bool, _ bool, _ time.Duration) (string, error) { return currentRef, nil } downloadWorkflowContentFn = func(_ context.Context, repo, path, ref string, _ bool) ([]byte, error) { @@ -97,13 +99,14 @@ func TestResolveRedirectedUpdateLocation(t *testing.T) { false, false, false, + 0, ) require.Error(t, err, "redirect loop should return an error") assert.Contains(t, err.Error(), "redirect loop detected", "error should explain redirect loop") }) t.Run("refuses redirect when no-redirect is enabled", func(t *testing.T) { - resolveLatestRefFn = func(_ context.Context, _ string, currentRef string, _ bool, _ bool) (string, error) { + resolveLatestRefFn = func(_ context.Context, _ string, currentRef string, _ bool, _ bool, _ time.Duration) (string, error) { return currentRef, nil } downloadWorkflowContentFn = func(_ context.Context, repo, path, ref string, _ bool) ([]byte, error) { @@ -123,6 +126,7 @@ func TestResolveRedirectedUpdateLocation(t *testing.T) { false, false, true, + 0, ) require.Error(t, err, "redirect should be refused with --no-redirect") assert.Contains(t, err.Error(), "redirect is disabled by --no-redirect", "error should explain redirect refusal") diff --git a/pkg/cli/update_workflows.go b/pkg/cli/update_workflows.go index 60ef2d9d571..94d3cb25ce1 100644 --- a/pkg/cli/update_workflows.go +++ b/pkg/cli/update_workflows.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "strings" + "time" "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/gitutil" @@ -17,8 +18,8 @@ import ( ) // UpdateWorkflows updates workflows from their source repositories -func UpdateWorkflows(ctx context.Context, workflowNames []string, allowMajor, force, verbose bool, engineOverride string, workflowsDir string, noStopAfter bool, stopAfter string, noMerge bool, noCompile bool, noRedirect bool) error { - updateLog.Printf("Scanning for workflows with source field: dir=%s, filter=%v, noMerge=%v, noCompile=%v, noRedirect=%v", workflowsDir, workflowNames, noMerge, noCompile, noRedirect) +func UpdateWorkflows(ctx context.Context, workflowNames []string, allowMajor, force, verbose bool, engineOverride string, workflowsDir string, noStopAfter bool, stopAfter string, noMerge bool, noCompile bool, noRedirect bool, coolDown time.Duration) error { + updateLog.Printf("Scanning for workflows with source field: dir=%s, filter=%v, noMerge=%v, noCompile=%v, noRedirect=%v, coolDown=%v", workflowsDir, workflowNames, noMerge, noCompile, noRedirect, coolDown) // Use provided workflows directory or default if workflowsDir == "" { @@ -50,7 +51,7 @@ func UpdateWorkflows(ctx context.Context, workflowNames []string, allowMajor, fo // Update each workflow for _, wf := range workflows { updateLog.Printf("Updating workflow: %s (source: %s)", wf.Name, wf.SourceSpec) - if err := updateWorkflow(ctx, wf, allowMajor, force, verbose, engineOverride, noStopAfter, stopAfter, noMerge, noCompile, noRedirect); err != nil { + if err := updateWorkflow(ctx, wf, allowMajor, force, verbose, engineOverride, noStopAfter, stopAfter, noMerge, noCompile, noRedirect, coolDown); err != nil { updateLog.Printf("Failed to update workflow %s: %v", wf.Name, err) failedUpdates = append(failedUpdates, updateFailure{ Name: wf.Name, @@ -177,7 +178,7 @@ func findWorkflowsWithSource(workflowsDir string, filterNames []string, verbose } // resolveLatestRef resolves the latest ref for a workflow source -func resolveLatestRef(ctx context.Context, repo, currentRef string, allowMajor, verbose bool) (string, error) { +func resolveLatestRef(ctx context.Context, repo, currentRef string, allowMajor, verbose bool, coolDown time.Duration) (string, error) { updateLog.Printf("Resolving latest ref: repo=%s, currentRef=%s, allowMajor=%v", repo, currentRef, allowMajor) if verbose { @@ -187,7 +188,7 @@ func resolveLatestRef(ctx context.Context, repo, currentRef string, allowMajor, // Check if current ref is a tag (looks like a semantic version) if isSemanticVersionTag(currentRef) { updateLog.Print("Current ref is semantic version tag, resolving latest release") - return resolveLatestRelease(ctx, repo, currentRef, allowMajor, verbose) + return resolveLatestRelease(ctx, repo, currentRef, allowMajor, verbose, coolDown) } // Check if current ref is a commit SHA (40-character hex string) @@ -284,7 +285,7 @@ var runWorkflowReleasesAPIFn = func(ctx context.Context, repo string) ([]byte, e } // resolveLatestRelease resolves the latest compatible release for a workflow source -func resolveLatestRelease(ctx context.Context, repo, currentRef string, allowMajor, verbose bool) (string, error) { +func resolveLatestRelease(ctx context.Context, repo, currentRef string, allowMajor, verbose bool, coolDown time.Duration) (string, error) { updateLog.Printf("Resolving latest release for repo %s (current: %s, allowMajor=%v)", repo, currentRef, allowMajor) if verbose { @@ -360,6 +361,17 @@ func resolveLatestRelease(ctx context.Context, repo, currentRef string, allowMaj return "", errors.New("no compatible release found") } + // 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 + } + } + if verbose && latestCompatible != currentRef { fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Found newer release: "+latestCompatible)) } @@ -368,7 +380,7 @@ func resolveLatestRelease(ctx context.Context, repo, currentRef string, allowMaj } // updateWorkflow updates a single workflow from its source -func updateWorkflow(ctx context.Context, wf *workflowWithSource, allowMajor, force, verbose bool, engineOverride string, noStopAfter bool, stopAfter string, noMerge bool, noCompile bool, noRedirect bool) error { +func updateWorkflow(ctx context.Context, wf *workflowWithSource, allowMajor, force, verbose bool, engineOverride string, noStopAfter bool, stopAfter string, noMerge bool, noCompile bool, noRedirect bool, coolDown time.Duration) error { updateLog.Printf("Updating workflow: name=%s, source=%s, force=%v, noMerge=%v", wf.Name, wf.SourceSpec, force, noMerge) if verbose { @@ -384,7 +396,7 @@ func updateWorkflow(ctx context.Context, wf *workflowWithSource, allowMajor, for return fmt.Errorf("failed to parse source spec: %w", err) } - resolvedLocation, err := resolveRedirectedUpdateLocation(ctx, wf.Name, initialSourceSpec, allowMajor, verbose, noRedirect) + resolvedLocation, err := resolveRedirectedUpdateLocation(ctx, wf.Name, initialSourceSpec, allowMajor, verbose, noRedirect, coolDown) if err != nil { return err } diff --git a/pkg/cli/update_workflows_test.go b/pkg/cli/update_workflows_test.go index d8a72057437..5c6a47e4f2f 100644 --- a/pkg/cli/update_workflows_test.go +++ b/pkg/cli/update_workflows_test.go @@ -27,7 +27,7 @@ func TestResolveLatestRelease_PrereleaseTagsSkipped(t *testing.T) { return []byte("v1.1.0-beta.1\nv1.0.0"), nil }) - result, err := resolveLatestRelease(context.Background(), "owner/repo", "v1.0.0", true, false) + result, err := resolveLatestRelease(context.Background(), "owner/repo", "v1.0.0", true, false, 0) require.NoError(t, err, "should not error when stable release exists") assert.Equal(t, "v1.0.0", result, "should select latest stable release, not prerelease") } @@ -42,7 +42,7 @@ func TestResolveLatestRelease_PrereleaseSkippedWhenCurrentVersionInvalid(t *test return []byte("v2.0.0-rc.1\nv1.3.0\nv1.5.0"), nil }) - result, err := resolveLatestRelease(context.Background(), "owner/repo", "not-a-version", true, false) + result, err := resolveLatestRelease(context.Background(), "owner/repo", "not-a-version", true, false, 0) require.NoError(t, err, "should not error when stable release exists") assert.Equal(t, "v1.5.0", result, "should skip prerelease and return highest stable release by semver") } @@ -54,7 +54,7 @@ func TestResolveLatestRelease_ErrorWhenOnlyPrereleasesExist(t *testing.T) { return []byte("v2.0.0-beta.1\nv1.0.0-rc.1"), nil }) - _, err := resolveLatestRelease(context.Background(), "owner/repo", "v1.0.0", true, false) + _, err := resolveLatestRelease(context.Background(), "owner/repo", "v1.0.0", true, false, 0) assert.Error(t, err, "should error when no stable releases exist") } @@ -65,7 +65,7 @@ func TestResolveLatestRelease_StableReleaseSelected(t *testing.T) { return []byte("v1.2.0\nv1.1.0\nv1.0.0"), nil }) - result, err := resolveLatestRelease(context.Background(), "owner/repo", "v1.0.0", false, false) + result, err := resolveLatestRelease(context.Background(), "owner/repo", "v1.0.0", false, false, 0) require.NoError(t, err, "should not error when stable releases exist") assert.Equal(t, "v1.2.0", result, "should select highest compatible stable release") } @@ -78,7 +78,7 @@ func TestResolveLatestRelease_MixedPrereleaseAndStable(t *testing.T) { }) // Without allowMajor, should stay on v1.x and skip prereleases. - result, err := resolveLatestRelease(context.Background(), "owner/repo", "v1.1.0", false, false) + result, err := resolveLatestRelease(context.Background(), "owner/repo", "v1.1.0", false, false, 0) 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") } diff --git a/pkg/cli/upgrade_command.go b/pkg/cli/upgrade_command.go index 643f3f6cb31..30a5a7f70ec 100644 --- a/pkg/cli/upgrade_command.go +++ b/pkg/cli/upgrade_command.go @@ -221,7 +221,7 @@ func runUpgradeCommand(ctx context.Context, verbose bool, workflowDir string, no fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Updating GitHub Actions versions...")) upgradeLog.Print("Updating GitHub Actions versions") - if err := UpdateActions(ctx, false, verbose, false); err != nil { + if err := UpdateActions(ctx, false, verbose, false, 0); err != nil { upgradeLog.Printf("Failed to update actions: %v", err) // Don't fail the upgrade if action updates fail - this is non-critical fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Warning: Failed to update actions: %v", err))) From 8162599bc71f6fa9dc43d9338d3ef3581c1f2f6e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 1 May 2026 06:19:15 +0000 Subject: [PATCH 2/4] refactor: extract cooldown message formatting to variables for readability Agent-Logs-Url: https://github.com/github/gh-aw/sessions/259f1ed8-f33f-44f5-99d1-0def0d5d86eb Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/update_cooldown.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/cli/update_cooldown.go b/pkg/cli/update_cooldown.go index 20cd1e39924..8cfaf739dfd 100644 --- a/pkg/cli/update_cooldown.go +++ b/pkg/cli/update_cooldown.go @@ -96,8 +96,11 @@ func checkReleaseCoolDown(ctx context.Context, repo, tag string, coolDown time.D } remaining := coolDown - age + ageStr := formatCoolDownDuration(age) + remainingStr := formatCoolDownDuration(remaining) + periodStr := formatCoolDownDuration(coolDown) msg := fmt.Sprintf("%s@%s was published %s ago and needs to cool down (%s remaining out of %s cooldown period)", - repo, tag, formatCoolDownDuration(age), formatCoolDownDuration(remaining), formatCoolDownDuration(coolDown)) + repo, tag, ageStr, remainingStr, periodStr) return coolDownCheckResult{InCoolDown: true, Message: msg} } From f6f1f166771ca6ad9f9c05ad4cc90f2866b3edbb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 1 May 2026 09:03:10 +0000 Subject: [PATCH 3/4] feat: store release date in actions-lock.json for cooldown caching - Add ReleasedAt *time.Time field to ActionCacheEntry (released_at JSON tag) - Add GetReleasedAt / SetReleasedAt methods to ActionCache - Preserve ReleasedAt across Set calls when SHA is unchanged - Clear ReleasedAt when SHA changes (stale date) - UpdateActions now uses cached release date (avoids extra API call on re-runs) and stores the fetched date after the first cooldown check - Add checkReleaseCoolDownWithDate helper for date-only checks - checkReleaseCoolDown now returns PublishedAt in result for caller caching - Add tests for ReleasedAt round-trip, SHA-change clearing, and new helpers Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3a9ffd04-058c-411a-8134-6993565f6214 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/update_actions.go | 17 +++++-- pkg/cli/update_cooldown.go | 39 +++++++++++----- pkg/cli/update_cooldown_test.go | 34 ++++++++++++++ pkg/workflow/action_cache.go | 33 ++++++++++++++ pkg/workflow/action_cache_test.go | 75 +++++++++++++++++++++++++++++++ 5 files changed, 183 insertions(+), 15 deletions(-) diff --git a/pkg/cli/update_actions.go b/pkg/cli/update_actions.go index 9a170348fa2..8ab112a2235 100644 --- a/pkg/cli/update_actions.go +++ b/pkg/cli/update_actions.go @@ -143,9 +143,20 @@ func UpdateActions(ctx context.Context, allowMajor, verbose, disableReleaseBump // Apply cooldown: if the repo is not exempt and the release is too recent, skip. if !isExemptFromCoolDown(entry.Repo) { - if result := checkReleaseCoolDown(ctx, entry.Repo, latestVersion, coolDown); result.InCoolDown { - cooldownLog.Printf("Action %s: %s", entry.Repo, result.Message) - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Skipping update for %s: %s", entry.Repo, result.Message))) + var coolDownResult coolDownCheckResult + if cachedDate, ok := actionCache.GetReleasedAt(entry.Repo, latestVersion); ok { + // Use cached release date to avoid an extra API call. + 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) + 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 } diff --git a/pkg/cli/update_cooldown.go b/pkg/cli/update_cooldown.go index 8cfaf739dfd..02aa01d33bd 100644 --- a/pkg/cli/update_cooldown.go +++ b/pkg/cli/update_cooldown.go @@ -71,10 +71,34 @@ type coolDownCheckResult struct { InCoolDown bool // Message is a human-readable explanation when InCoolDown is true. Message string + // PublishedAt is the fetched release publication date. It is populated when + // checkReleaseCoolDown makes an API call so callers can cache the date. + PublishedAt time.Time +} + +// checkReleaseCoolDownWithDate checks cooldown using an already-known publication date. +// This variant skips the API call and is used when the date is already cached locally. +func checkReleaseCoolDownWithDate(repo, tag string, publishedAt time.Time, coolDown time.Duration) coolDownCheckResult { + if coolDown <= 0 { + return coolDownCheckResult{} + } + age := time.Since(publishedAt) + if age >= coolDown { + return coolDownCheckResult{} + } + remaining := coolDown - age + ageStr := formatCoolDownDuration(age) + remainingStr := formatCoolDownDuration(remaining) + periodStr := formatCoolDownDuration(coolDown) + msg := fmt.Sprintf("%s@%s was published %s ago and needs to cool down (%s remaining out of %s cooldown period)", + repo, tag, ageStr, remainingStr, periodStr) + return coolDownCheckResult{InCoolDown: true, Message: msg} } // checkReleaseCoolDown returns a result indicating whether a release tag is // within the cooldown window and should be skipped. +// The result always includes the fetched PublishedAt date (when available) so that +// callers can cache the date for future runs. // // The function is fail-open: if the publication date cannot be fetched (e.g. // due to network issues), the update is allowed and InCoolDown is false. @@ -90,18 +114,9 @@ func checkReleaseCoolDown(ctx context.Context, repo, tag string, coolDown time.D return coolDownCheckResult{} } - age := time.Since(publishedAt) - if age >= coolDown { - return coolDownCheckResult{} - } - - remaining := coolDown - age - ageStr := formatCoolDownDuration(age) - remainingStr := formatCoolDownDuration(remaining) - periodStr := formatCoolDownDuration(coolDown) - msg := fmt.Sprintf("%s@%s was published %s ago and needs to cool down (%s remaining out of %s cooldown period)", - repo, tag, ageStr, remainingStr, periodStr) - return coolDownCheckResult{InCoolDown: true, Message: msg} + r := checkReleaseCoolDownWithDate(repo, tag, publishedAt, coolDown) + r.PublishedAt = publishedAt + return r } // formatCoolDownDuration formats a duration for display in cooldown messages. diff --git a/pkg/cli/update_cooldown_test.go b/pkg/cli/update_cooldown_test.go index 5a1ace0ff81..e3549d750ba 100644 --- a/pkg/cli/update_cooldown_test.go +++ b/pkg/cli/update_cooldown_test.go @@ -135,6 +135,40 @@ func TestCheckReleaseCoolDown_RecentRelease(t *testing.T) { assert.True(t, result.InCoolDown, "release within cooldown period should be blocked") assert.Contains(t, result.Message, "v1.2.0", "message should mention the release tag") assert.Contains(t, result.Message, "cool down", "message should mention cooldown") + assert.False(t, result.PublishedAt.IsZero(), "PublishedAt should be populated for caching") +} + +func TestCheckReleaseCoolDown_OldReleaseReturnsPublishedAt(t *testing.T) { + orig := getReleasePublishedAtFn + defer func() { getReleasePublishedAtFn = orig }() + + published := time.Now().Add(-10 * 24 * time.Hour) + getReleasePublishedAtFn = func(_ context.Context, _, _ string) (time.Time, error) { + return published, nil + } + + result := checkReleaseCoolDown(context.Background(), "owner/repo", "v1.2.0", 7*24*time.Hour) + assert.False(t, result.InCoolDown, "old release should not be blocked") + assert.True(t, result.PublishedAt.Equal(published), "PublishedAt should be populated even when not in cooldown") +} + +func TestCheckReleaseCoolDownWithDate_InCoolDown(t *testing.T) { + publishedAt := time.Now().Add(-2 * 24 * time.Hour) + result := checkReleaseCoolDownWithDate("owner/repo", "v1.2.0", publishedAt, 7*24*time.Hour) + assert.True(t, result.InCoolDown, "release 2d old should be in cooldown with 7d window") + assert.Contains(t, result.Message, "v1.2.0", "message should mention the tag") +} + +func TestCheckReleaseCoolDownWithDate_NotInCoolDown(t *testing.T) { + publishedAt := time.Now().Add(-10 * 24 * time.Hour) + result := checkReleaseCoolDownWithDate("owner/repo", "v1.2.0", publishedAt, 7*24*time.Hour) + assert.False(t, result.InCoolDown, "release 10d old should not be in cooldown with 7d window") +} + +func TestCheckReleaseCoolDownWithDate_ZeroDuration(t *testing.T) { + publishedAt := time.Now() + result := checkReleaseCoolDownWithDate("owner/repo", "v1.2.0", publishedAt, 0) + assert.False(t, result.InCoolDown, "zero cooldown should always allow update") } func TestCheckReleaseCoolDown_FetchError(t *testing.T) { diff --git a/pkg/workflow/action_cache.go b/pkg/workflow/action_cache.go index 63ed05b9939..1f922c12f26 100644 --- a/pkg/workflow/action_cache.go +++ b/pkg/workflow/action_cache.go @@ -7,6 +7,7 @@ import ( "path/filepath" "sort" "strings" + "time" "github.com/github/gh-aw/pkg/logger" ) @@ -23,6 +24,7 @@ type ActionCacheEntry struct { Repo string `json:"repo"` Version string `json:"version"` SHA string `json:"sha"` + ReleasedAt *time.Time `json:"released_at,omitempty"` // publication date of this release, used for cooldown checks Inputs map[string]*ActionYAMLInput `json:"inputs,omitempty"` // cached inputs from action.yml ActionDescription string `json:"action_description,omitempty"` // cached description from action.yml } @@ -356,9 +358,11 @@ func (c *ActionCache) Set(repo, version, sha string) { existing := c.Entries[key] var inputs map[string]*ActionYAMLInput var description string + var releasedAt *time.Time if existing.SHA == sha { inputs = existing.Inputs description = existing.ActionDescription + releasedAt = existing.ReleasedAt } else if existing.SHA != "" { // Log when an existing entry's SHA is being changed (covers both the case // where cached inputs exist and where they don't, for consistent observability). @@ -368,6 +372,7 @@ func (c *ActionCache) Set(repo, version, sha string) { Repo: repo, Version: version, SHA: sha, + ReleasedAt: releasedAt, Inputs: inputs, ActionDescription: description, } @@ -444,6 +449,34 @@ func (c *ActionCache) SetActionDescription(repo, version, description string) { actionCacheLog.Printf("Cached description for key=%s", key) } +// GetReleasedAt retrieves the cached release date for the given repo and version. +// Returns the time and true if a release date is cached, otherwise zero time and false. +func (c *ActionCache) GetReleasedAt(repo, version string) (time.Time, bool) { + key := formatActionCacheKey(repo, version) + entry, exists := c.Entries[key] + if !exists || entry.ReleasedAt == nil { + return time.Time{}, false + } + return *entry.ReleasedAt, true +} + +// SetReleasedAt stores the release publication date for the given repo and version. +// If no cache entry exists for the key, a new entry is created. +func (c *ActionCache) SetReleasedAt(repo, version string, t time.Time) { + key := formatActionCacheKey(repo, version) + entry, exists := c.Entries[key] + if !exists { + entry = ActionCacheEntry{ + Repo: repo, + Version: version, + } + } + entry.ReleasedAt = &t + c.Entries[key] = entry + c.dirty = true + actionCacheLog.Printf("Cached release date for key=%s: %s", key, t.Format(time.RFC3339)) +} + // GetCachePath returns the path to the cache file func (c *ActionCache) GetCachePath() string { return c.path diff --git a/pkg/workflow/action_cache_test.go b/pkg/workflow/action_cache_test.go index be5f69a2016..9014f2aa58f 100644 --- a/pkg/workflow/action_cache_test.go +++ b/pkg/workflow/action_cache_test.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "testing" + "time" "github.com/github/gh-aw/pkg/testutil" ) @@ -806,3 +807,77 @@ func TestPruneStaleGHAWEntriesNoOp(t *testing.T) { t.Errorf("Expected 2 entries (no pruning for dirty release build), got %d", len(cache.Entries)) } } + +// TestActionCacheReleasedAt verifies that GetReleasedAt / SetReleasedAt +// round-trip correctly and are preserved across a Save/Load cycle. +func TestActionCacheReleasedAt(t *testing.T) { + tmpDir := testutil.TempDir(t, "test-*") + cache := NewActionCache(tmpDir) + cache.Set("owner/action", "v1", "sha1234567890123456789012345678901234567890") + + // No date cached yet + _, ok := cache.GetReleasedAt("owner/action", "v1") + if ok { + t.Error("Expected no release date before SetReleasedAt") + } + + // Store a date + ts := time.Date(2024, 3, 15, 12, 0, 0, 0, time.UTC) + cache.SetReleasedAt("owner/action", "v1", ts) + + got, ok := cache.GetReleasedAt("owner/action", "v1") + if !ok { + t.Fatal("Expected release date after SetReleasedAt") + } + if !got.Equal(ts) { + t.Errorf("GetReleasedAt() = %v, want %v", got, ts) + } + + // Persist to disk and reload + if err := cache.Save(); err != nil { + t.Fatalf("Failed to save: %v", err) + } + cache2 := NewActionCache(tmpDir) + if err := cache2.Load(); err != nil { + t.Fatalf("Failed to load: %v", err) + } + got2, ok2 := cache2.GetReleasedAt("owner/action", "v1") + if !ok2 { + t.Fatal("Expected release date to survive save/load") + } + if !got2.Equal(ts) { + t.Errorf("After reload: GetReleasedAt() = %v, want %v", got2, ts) + } +} + +// TestActionCacheReleasedAtPreservedOnSHAMatch verifies that ReleasedAt is +// preserved by Set when the SHA does not change. +func TestActionCacheReleasedAtPreservedOnSHAMatch(t *testing.T) { + cache := NewActionCache(t.TempDir()) + ts := time.Date(2024, 5, 1, 0, 0, 0, 0, time.UTC) + cache.Set("owner/action", "v2", "samesha1234567890123456789012345678901234") + cache.SetReleasedAt("owner/action", "v2", ts) + + // Set again with the same SHA — ReleasedAt must be preserved. + cache.Set("owner/action", "v2", "samesha1234567890123456789012345678901234") + got, ok := cache.GetReleasedAt("owner/action", "v2") + if !ok || !got.Equal(ts) { + t.Errorf("ReleasedAt should be preserved when SHA unchanged; got %v, ok=%v", got, ok) + } +} + +// TestActionCacheReleasedAtClearedOnSHAChange verifies that ReleasedAt is +// cleared by Set when the SHA changes (stale date no longer valid). +func TestActionCacheReleasedAtClearedOnSHAChange(t *testing.T) { + cache := NewActionCache(t.TempDir()) + ts := time.Date(2024, 5, 1, 0, 0, 0, 0, time.UTC) + cache.Set("owner/action", "v2", "oldsha1234567890123456789012345678901234a") + cache.SetReleasedAt("owner/action", "v2", ts) + + // Set with a different SHA — ReleasedAt must be cleared. + cache.Set("owner/action", "v2", "newsha1234567890123456789012345678901234b") + _, ok := cache.GetReleasedAt("owner/action", "v2") + if ok { + t.Error("ReleasedAt should be cleared when SHA changes") + } +} From 1f7f45e6a359e83a8732358b9274115bdc9587b8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 1 May 2026 09:33:04 +0000 Subject: [PATCH 4/4] fix: URL-escape release tag in REST path; clamp negative age to 0 - Use url.PathEscape(tag) when building repos/{repo}/releases/tags/{tag} to safely handle tags containing '/', spaces, or other special chars - Replace manual if/clamp with max(age, 0) in checkReleaseCoolDownWithDate to guard against future-dated published_at (clock skew / API skew) - Add TestCheckReleaseCoolDownWithDate_FutureTimestamp test Agent-Logs-Url: https://github.com/github/gh-aw/sessions/14860b0f-e157-48f8-b0ba-db670494813c Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/update_cooldown.go | 7 +++++-- pkg/cli/update_cooldown_test.go | 8 ++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/pkg/cli/update_cooldown.go b/pkg/cli/update_cooldown.go index 02aa01d33bd..f38dd56ab2b 100644 --- a/pkg/cli/update_cooldown.go +++ b/pkg/cli/update_cooldown.go @@ -3,6 +3,7 @@ package cli import ( "context" "fmt" + "net/url" "strconv" "strings" "time" @@ -59,7 +60,7 @@ func getReleasePublishedAt(ctx context.Context, repo, tag string) (time.Time, er return time.Time{}, fmt.Errorf("failed to create GitHub client: %w", err) } var release githubReleaseInfo - if err := client.Get(fmt.Sprintf("repos/%s/releases/tags/%s", repo, tag), &release); err != nil { + if err := client.Get(fmt.Sprintf("repos/%s/releases/tags/%s", repo, url.PathEscape(tag)), &release); err != nil { return time.Time{}, fmt.Errorf("failed to fetch release info for %s@%s: %w", repo, tag, err) } return release.PublishedAt, nil @@ -78,11 +79,13 @@ type coolDownCheckResult struct { // checkReleaseCoolDownWithDate checks cooldown using an already-known publication date. // This variant skips the API call and is used when the date is already cached locally. +// A negative age (clock skew / future timestamp) is clamped to 0 so such releases +// are treated as just-published rather than mistakenly allowed past the cooldown. func checkReleaseCoolDownWithDate(repo, tag string, publishedAt time.Time, coolDown time.Duration) coolDownCheckResult { if coolDown <= 0 { return coolDownCheckResult{} } - age := time.Since(publishedAt) + age := max(time.Since(publishedAt), 0) // clamp to 0 for future timestamps (clock skew) if age >= coolDown { return coolDownCheckResult{} } diff --git a/pkg/cli/update_cooldown_test.go b/pkg/cli/update_cooldown_test.go index e3549d750ba..a6962b9126a 100644 --- a/pkg/cli/update_cooldown_test.go +++ b/pkg/cli/update_cooldown_test.go @@ -171,6 +171,14 @@ func TestCheckReleaseCoolDownWithDate_ZeroDuration(t *testing.T) { assert.False(t, result.InCoolDown, "zero cooldown should always allow update") } +func TestCheckReleaseCoolDownWithDate_FutureTimestamp(t *testing.T) { + // Simulate a future published_at (clock skew / API returning future time). + // The release should be treated as just-published and kept in cooldown. + publishedAt := time.Now().Add(1 * time.Hour) + result := checkReleaseCoolDownWithDate("owner/repo", "v1.2.0", publishedAt, 7*24*time.Hour) + assert.True(t, result.InCoolDown, "future timestamp should be treated as just-published and kept in cooldown") +} + func TestCheckReleaseCoolDown_FetchError(t *testing.T) { orig := getReleasePublishedAtFn defer func() { getReleasePublishedAtFn = orig }()