From ab05699c5d7fcb008d0377016f4e771d6bdc730a Mon Sep 17 00:00:00 2001 From: Don Syme Date: Wed, 15 Apr 2026 15:03:04 +1000 Subject: [PATCH 1/7] fix: propagate context through all CLI subprocess calls for Ctrl-C cancellation Thread context.Context through the full call chain in pkg/cli/ so that Ctrl-C (via signal.NotifyContext in main.go) correctly terminates all background gh CLI subprocesses and git operations. Previously, many functions called workflow.RunGH / RunGHCombined (no context) or exec.Command (no context), so Ctrl-C had no effect on in-flight network operations like artifact downloads, log fetches, and workflow file downloads. Changes: - pkg/workflow/github_cli.go: add RunGHCombinedContext - pkg/cli/download_workflow.go: add ctx to downloadWorkflowContent, downloadWorkflowContentViaGit, downloadWorkflowContentViaGitClone; all exec.Command -> exec.CommandContext - pkg/cli/update_workflows.go: pass ctx to all downloadWorkflowContent calls - pkg/cli/update_actions.go: exec.Command(git ls-remote) -> exec.CommandContext - pkg/cli/audit.go: add ctx to fetchWorkflowRunMetadata, resolveWorkflowDisplayName, renderAuditReport - pkg/cli/audit_comparison.go: add ctx to findPreviousSuccessfulWorkflowRuns, buildAuditComparisonForRun - pkg/cli/audit_diff.go: add ctx to loadRunSummaryForDiff - pkg/cli/audit_diff_command.go: pass ctx to loadRunSummaryForDiff calls - pkg/cli/logs_download.go: add ctx to downloadWorkflowRunLogs, listRunArtifactNames, downloadArtifactsByName, retryCriticalArtifacts, downloadRunArtifacts; use ExecGHContext / RunGHContext - pkg/cli/logs_orchestrator.go: pass ctx to downloadRunArtifacts in pool - pkg/cli/trial_runner.go: add ctx to getCurrentGitHubUsername - All affected test files updated accordingly --- pkg/cli/audit.go | 22 ++++++------- pkg/cli/audit_comparison.go | 11 ++++--- pkg/cli/audit_diff.go | 5 +-- pkg/cli/audit_diff_command.go | 4 +-- pkg/cli/audit_test.go | 5 +-- pkg/cli/download_workflow.go | 33 ++++++++++--------- pkg/cli/logs_download.go | 33 ++++++++++--------- pkg/cli/logs_download_test.go | 2 +- pkg/cli/logs_orchestrator.go | 2 +- pkg/cli/trial_runner.go | 6 ++-- pkg/cli/update_actions.go | 56 ++++++++++++++++++++------------ pkg/cli/update_actions_test.go | 43 ++++++++++++------------ pkg/cli/update_command.go | 6 ++-- pkg/cli/update_command_test.go | 10 +++--- pkg/cli/update_workflows.go | 42 ++++++++++++------------ pkg/cli/update_workflows_test.go | 23 ++++++------- pkg/cli/upgrade_command.go | 2 +- pkg/workflow/github_cli.go | 11 +++++++ 18 files changed, 174 insertions(+), 142 deletions(-) diff --git a/pkg/cli/audit.go b/pkg/cli/audit.go index 8e685b31b55..5f50a4a87f2 100644 --- a/pkg/cli/audit.go +++ b/pkg/cli/audit.go @@ -221,14 +221,14 @@ func AuditWorkflowRun(ctx context.Context, runID int64, owner, repo, hostname st // file reads (created items, aw_info, etc.) resolve correctly even if the run // directory has been moved or copied since the summary was first written. processedRun.Run.LogsPath = runOutputDir - return renderAuditReport(processedRun, summary.Metrics, summary.MCPToolUsage, runOutputDir, owner, repo, hostname, verbose, parse, jsonOutput) + return renderAuditReport(ctx, processedRun, summary.Metrics, summary.MCPToolUsage, runOutputDir, owner, repo, hostname, verbose, parse, jsonOutput) } // Check if we have locally cached artifacts first hasLocalCache := fileutil.DirExists(runOutputDir) && !fileutil.IsDirEmpty(runOutputDir) // Try to get run metadata from GitHub API - run, metadataErr := fetchWorkflowRunMetadata(runID, owner, repo, hostname, verbose) + run, metadataErr := fetchWorkflowRunMetadata(ctx, runID, owner, repo, hostname, verbose) var useLocalCache bool if metadataErr != nil { @@ -259,7 +259,7 @@ func AuditWorkflowRun(ctx context.Context, runID int64, owner, repo, hostname st // Download artifacts for the run auditLog.Printf("Downloading artifacts for run %d", runID) - err := downloadRunArtifacts(runID, runOutputDir, verbose, owner, repo, hostname, artifactFilter) + err := downloadRunArtifacts(ctx, runID, runOutputDir, verbose, owner, repo, hostname, artifactFilter) if err != nil { // Gracefully handle cases where the run legitimately has no artifacts if errors.Is(err, ErrNoArtifacts) { @@ -483,20 +483,20 @@ func AuditWorkflowRun(ctx context.Context, runID int64, owner, repo, hostname st } } - return renderAuditReport(processedRun, metrics, mcpToolUsage, runOutputDir, owner, repo, hostname, verbose, parse, jsonOutput) + return renderAuditReport(ctx, processedRun, metrics, mcpToolUsage, runOutputDir, owner, repo, hostname, verbose, parse, jsonOutput) } // renderAuditReport builds and renders the audit report from a fully-populated processedRun. // It is called both when serving from a cached run summary and after a fresh processing pass, // ensuring that the two paths produce identical output. -func renderAuditReport(processedRun ProcessedRun, metrics LogMetrics, mcpToolUsage *MCPToolUsageData, runOutputDir string, owner, repo, hostname string, verbose bool, parse bool, jsonOutput bool) error { +func renderAuditReport(ctx context.Context, processedRun ProcessedRun, metrics LogMetrics, mcpToolUsage *MCPToolUsageData, runOutputDir string, owner, repo, hostname string, verbose bool, parse bool, jsonOutput bool) error { runID := processedRun.Run.DatabaseID currentCreatedItems := extractCreatedItemsFromManifest(runOutputDir) processedRun.Run.SafeItemsCount = len(currentCreatedItems) currentSnapshot := buildAuditComparisonSnapshot(processedRun, currentCreatedItems) - comparison := buildAuditComparisonForRun(processedRun, currentSnapshot, runOutputDir, owner, repo, hostname, verbose) + comparison := buildAuditComparisonForRun(ctx, processedRun, currentSnapshot, runOutputDir, owner, repo, hostname, verbose) // Build structured audit data auditData := buildAuditData(processedRun, metrics, mcpToolUsage) @@ -752,7 +752,7 @@ func findFirstFailingStep(jobLog string) (int, string) { } // fetchWorkflowRunMetadata fetches metadata for a single workflow run -func fetchWorkflowRunMetadata(runID int64, owner, repo, hostname string, verbose bool) (WorkflowRun, error) { +func fetchWorkflowRunMetadata(ctx context.Context, runID int64, owner, repo, hostname string, verbose bool) (WorkflowRun, error) { // Build the API endpoint var endpoint string if owner != "" && repo != "" { @@ -780,7 +780,7 @@ func fetchWorkflowRunMetadata(runID int64, owner, repo, hostname string, verbose fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Executing: gh "+strings.Join(args, " "))) } - output, err := workflow.RunGHCombined("Fetching run metadata...", args...) + output, err := workflow.RunGHCombinedContext(ctx, "Fetching run metadata...", args...) if err != nil { if verbose { fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(string(output))) @@ -807,7 +807,7 @@ func fetchWorkflowRunMetadata(runID int64, owner, repo, hostname string, verbose // that were cancelled or failed before any jobs started), resolve the actual workflow // display name so that audit output is consistent with 'gh aw logs'. if strings.HasPrefix(run.WorkflowName, ".github/") { - if displayName := resolveWorkflowDisplayName(run.WorkflowPath, owner, repo, hostname); displayName != "" { + if displayName := resolveWorkflowDisplayName(ctx, run.WorkflowPath, owner, repo, hostname); displayName != "" { auditLog.Printf("Resolved workflow display name: %q -> %q", run.WorkflowName, displayName) run.WorkflowName = displayName } @@ -821,7 +821,7 @@ func fetchWorkflowRunMetadata(runID int64, owner, repo, hostname string, verbose // relative to the git repository root so that it works from any working directory inside // the repo); if that fails it falls back to a GitHub API call. An empty string is // returned on any error so that callers can gracefully keep the original value. -func resolveWorkflowDisplayName(workflowPath, owner, repo, hostname string) string { +func resolveWorkflowDisplayName(ctx context.Context, workflowPath, owner, repo, hostname string) string { // Try local file first. workflowPath is a repo-relative path like // ".github/workflows/foo.lock.yml", so we resolve it against the git root to // produce a correct absolute path regardless of the current working directory. @@ -849,7 +849,7 @@ func resolveWorkflowDisplayName(workflowPath, owner, repo, hostname string) stri } args = append(args, endpoint, "--jq", ".name") - out, err := workflow.RunGHCombined("Fetching workflow name...", args...) + out, err := workflow.RunGHCombinedContext(ctx, "Fetching workflow name...", args...) if err != nil { auditLog.Printf("Failed to fetch workflow display name for %q: %v", workflowPath, err) return "" diff --git a/pkg/cli/audit_comparison.go b/pkg/cli/audit_comparison.go index e7223d8c30a..24a9f70abd3 100644 --- a/pkg/cli/audit_comparison.go +++ b/pkg/cli/audit_comparison.go @@ -1,6 +1,7 @@ package cli import ( + "context" "encoding/json" "fmt" "net/url" @@ -457,7 +458,7 @@ func collectMCPFailureServers(failures []MCPFailureReport) []string { return servers } -func findPreviousSuccessfulWorkflowRuns(current WorkflowRun, owner, repo, hostname string, verbose bool) ([]WorkflowRun, error) { +func findPreviousSuccessfulWorkflowRuns(ctx context.Context, current WorkflowRun, owner, repo, hostname string, verbose bool) ([]WorkflowRun, error) { _ = verbose workflowID := filepath.Base(current.WorkflowPath) if workflowID == "." || workflowID == "" { @@ -497,7 +498,7 @@ func findPreviousSuccessfulWorkflowRuns(current WorkflowRun, owner, repo, hostna for index := range runs { if strings.HasPrefix(runs[index].WorkflowName, ".github/") { - if displayName := resolveWorkflowDisplayName(runs[index].WorkflowPath, owner, repo, hostname); displayName != "" { + if displayName := resolveWorkflowDisplayName(ctx, runs[index].WorkflowPath, owner, repo, hostname); displayName != "" { runs[index].WorkflowName = displayName } } @@ -506,8 +507,8 @@ func findPreviousSuccessfulWorkflowRuns(current WorkflowRun, owner, repo, hostna return runs, nil } -func buildAuditComparisonForRun(currentRun ProcessedRun, currentSnapshot auditComparisonSnapshot, outputDir string, owner, repo, hostname string, verbose bool) *AuditComparisonData { - baselineRuns, err := findPreviousSuccessfulWorkflowRuns(currentRun.Run, owner, repo, hostname, verbose) +func buildAuditComparisonForRun(ctx context.Context, currentRun ProcessedRun, currentSnapshot auditComparisonSnapshot, outputDir string, owner, repo, hostname string, verbose bool) *AuditComparisonData { + baselineRuns, err := findPreviousSuccessfulWorkflowRuns(ctx, currentRun.Run, owner, repo, hostname, verbose) if err != nil { auditLog.Printf("Skipping audit comparison: failed to find baseline: %v", err) return &AuditComparisonData{BaselineFound: false} @@ -520,7 +521,7 @@ func buildAuditComparisonForRun(currentRun ProcessedRun, currentSnapshot auditCo for _, baselineRun := range baselineRuns { baselineOutputDir := filepath.Join(outputDir, fmt.Sprintf("baseline-%d", baselineRun.DatabaseID)) if _, err := os.Stat(baselineOutputDir); err != nil { - if downloadErr := downloadRunArtifacts(baselineRun.DatabaseID, baselineOutputDir, verbose, owner, repo, hostname, nil); downloadErr != nil { + if downloadErr := downloadRunArtifacts(ctx, baselineRun.DatabaseID, baselineOutputDir, verbose, owner, repo, hostname, nil); downloadErr != nil { auditLog.Printf("Skipping candidate baseline for run %d: failed to download baseline artifacts: %v", baselineRun.DatabaseID, downloadErr) continue } diff --git a/pkg/cli/audit_diff.go b/pkg/cli/audit_diff.go index 3cbbb095666..57b739f317b 100644 --- a/pkg/cli/audit_diff.go +++ b/pkg/cli/audit_diff.go @@ -1,6 +1,7 @@ package cli import ( + "context" "errors" "fmt" "math" @@ -681,7 +682,7 @@ func formatCountChange(count1, count2 int) string { // metrics); otherwise it downloads artifacts and analyzes firewall logs, returning a partial // summary with only FirewallAnalysis populated. // artifactFilter restricts which artifacts are downloaded; nil means download all. -func loadRunSummaryForDiff(runID int64, outputDir string, owner, repo, hostname string, verbose bool, artifactFilter []string) (*RunSummary, error) { +func loadRunSummaryForDiff(ctx context.Context, runID int64, outputDir string, owner, repo, hostname string, verbose bool, artifactFilter []string) (*RunSummary, error) { runOutputDir := filepath.Join(outputDir, fmt.Sprintf("run-%d", runID)) if absDir, err := filepath.Abs(runOutputDir); err == nil { runOutputDir = absDir @@ -694,7 +695,7 @@ func loadRunSummaryForDiff(runID int64, outputDir string, owner, repo, hostname } // Download artifacts if needed - if err := downloadRunArtifacts(runID, runOutputDir, verbose, owner, repo, hostname, artifactFilter); err != nil { + if err := downloadRunArtifacts(ctx, runID, runOutputDir, verbose, owner, repo, hostname, artifactFilter); err != nil { if !errors.Is(err, ErrNoArtifacts) { return nil, fmt.Errorf("failed to download artifacts for run %d: %w", runID, err) } diff --git a/pkg/cli/audit_diff_command.go b/pkg/cli/audit_diff_command.go index 5de3c3e3d41..afa39f88b8e 100644 --- a/pkg/cli/audit_diff_command.go +++ b/pkg/cli/audit_diff_command.go @@ -136,7 +136,7 @@ func RunAuditDiff(ctx context.Context, baseRunID int64, compareRunIDs []int64, o // Load base run summary once (shared across all comparisons) fmt.Fprintln(os.Stderr, console.FormatProgressMessage(fmt.Sprintf("Loading data for base run %d...", baseRunID))) - baseSummary, err := loadRunSummaryForDiff(baseRunID, outputDir, owner, repo, hostname, verbose, artifactFilter) + baseSummary, err := loadRunSummaryForDiff(ctx, baseRunID, outputDir, owner, repo, hostname, verbose, artifactFilter) if err != nil { return fmt.Errorf("failed to load data for base run %d: %w", baseRunID, err) } @@ -153,7 +153,7 @@ func RunAuditDiff(ctx context.Context, baseRunID int64, compareRunIDs []int64, o } fmt.Fprintln(os.Stderr, console.FormatProgressMessage(fmt.Sprintf("Loading data for run %d...", compareRunID))) - compareSummary, err := loadRunSummaryForDiff(compareRunID, outputDir, owner, repo, hostname, verbose, artifactFilter) + compareSummary, err := loadRunSummaryForDiff(ctx, compareRunID, outputDir, owner, repo, hostname, verbose, artifactFilter) if err != nil { return fmt.Errorf("failed to load data for run %d: %w", compareRunID, err) } diff --git a/pkg/cli/audit_test.go b/pkg/cli/audit_test.go index dab9aaed9d5..31cb5f3bc37 100644 --- a/pkg/cli/audit_test.go +++ b/pkg/cli/audit_test.go @@ -3,6 +3,7 @@ package cli import ( + "context" "encoding/json" "errors" "fmt" @@ -423,7 +424,7 @@ func TestAuditCachingBehavior(t *testing.T) { // Verify that downloadRunArtifacts skips download when valid summary exists // This is tested by checking that the function returns without error // and doesn't attempt to call `gh run download` - err := downloadRunArtifacts(run.DatabaseID, runOutputDir, false, "", "", "", nil) + err := downloadRunArtifacts(context.Background(), run.DatabaseID, runOutputDir, false, "", "", "", nil) if err != nil { t.Errorf("downloadRunArtifacts should skip download when valid summary exists, but got error: %v", err) } @@ -579,7 +580,7 @@ func TestRenderAuditReportUsesProvidedMetrics(t *testing.T) { // renderAuditReport should complete without error even without GitHub API access. // No GitHub calls are made because WorkflowPath is empty, causing findPreviousSuccessfulWorkflowRuns // to return early with an error before any network requests are issued. - err := renderAuditReport(processedRun, metrics, nil, runOutputDir, "", "", "", false, false, false) + err := renderAuditReport(context.Background(), processedRun, metrics, nil, runOutputDir, "", "", "", false, false, false) if err != nil { t.Errorf("renderAuditReport returned unexpected error: %v", err) } diff --git a/pkg/cli/download_workflow.go b/pkg/cli/download_workflow.go index cb1708b6ea0..529642c743d 100644 --- a/pkg/cli/download_workflow.go +++ b/pkg/cli/download_workflow.go @@ -1,6 +1,7 @@ package cli import ( + "context" "encoding/base64" "fmt" "os" @@ -18,7 +19,7 @@ import ( var downloadLog = logger.New("cli:download_workflow") // downloadWorkflowContentViaGit downloads a workflow file using git archive -func downloadWorkflowContentViaGit(repo, path, ref string, verbose bool) ([]byte, error) { +func downloadWorkflowContentViaGit(ctx context.Context, repo, path, ref string, verbose bool) ([]byte, error) { if verbose { fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Fetching %s/%s@%s via git", repo, path, ref))) } @@ -31,12 +32,12 @@ func downloadWorkflowContentViaGit(repo, path, ref string, verbose bool) ([]byte // git archive command: git archive --remote= // #nosec G204 -- repoURL, ref, and path are from workflow import configuration authored by the - // developer; exec.Command with separate args (not shell execution) prevents shell injection. - cmd := exec.Command("git", "archive", "--remote="+repoURL, ref, path) + // developer; exec.CommandContext with separate args (not shell execution) prevents shell injection. + cmd := exec.CommandContext(ctx, "git", "archive", "--remote="+repoURL, ref, path) archiveOutput, err := cmd.Output() if err != nil { // If git archive fails, try with git clone + read file as a fallback - return downloadWorkflowContentViaGitClone(repo, path, ref, verbose) + return downloadWorkflowContentViaGitClone(ctx, repo, path, ref, verbose) } // Extract the file from the tar archive using Go's archive/tar (cross-platform) @@ -53,7 +54,7 @@ func downloadWorkflowContentViaGit(repo, path, ref string, verbose bool) ([]byte } // downloadWorkflowContentViaGitClone downloads a workflow file by shallow cloning with sparse checkout -func downloadWorkflowContentViaGitClone(repo, path, ref string, verbose bool) ([]byte, error) { +func downloadWorkflowContentViaGitClone(ctx context.Context, repo, path, ref string, verbose bool) ([]byte, error) { if verbose { fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Fetching %s/%s@%s via git clone", repo, path, ref))) } @@ -71,19 +72,19 @@ func downloadWorkflowContentViaGitClone(repo, path, ref string, verbose bool) ([ repoURL := fmt.Sprintf("%s/%s.git", githubHost, repo) // Initialize git repository - initCmd := exec.Command("git", "-C", tmpDir, "init") + initCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "init") if output, err := initCmd.CombinedOutput(); err != nil { return nil, fmt.Errorf("failed to initialize git repository: %w\nOutput: %s", err, string(output)) } // Add remote - remoteCmd := exec.Command("git", "-C", tmpDir, "remote", "add", "origin", repoURL) + remoteCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "remote", "add", "origin", repoURL) if output, err := remoteCmd.CombinedOutput(); err != nil { return nil, fmt.Errorf("failed to add remote: %w\nOutput: %s", err, string(output)) } // Enable sparse-checkout - sparseCmd := exec.Command("git", "-C", tmpDir, "config", "core.sparseCheckout", "true") + sparseCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "config", "core.sparseCheckout", "true") if output, err := sparseCmd.CombinedOutput(); err != nil { return nil, fmt.Errorf("failed to enable sparse checkout: %w\nOutput: %s", err, string(output)) } @@ -109,29 +110,29 @@ func downloadWorkflowContentViaGitClone(repo, path, ref string, verbose bool) ([ // Note: sparse-checkout with SHA refs may not reduce bandwidth as much as with branch refs, // because the server needs to send enough history to reach the specific commit. // However, it still limits the working directory to only the requested file. - fetchCmd := exec.Command("git", "-C", tmpDir, "fetch", "--depth", "1", "origin", ref) + fetchCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "fetch", "--depth", "1", "origin", ref) if _, err := fetchCmd.CombinedOutput(); err != nil { // If fetching specific SHA fails, try fetching all branches with depth 1 - fetchCmd = exec.Command("git", "-C", tmpDir, "fetch", "--depth", "1", "origin") + fetchCmd = exec.CommandContext(ctx, "git", "-C", tmpDir, "fetch", "--depth", "1", "origin") if output, err := fetchCmd.CombinedOutput(); err != nil { return nil, fmt.Errorf("failed to fetch repository: %w\nOutput: %s", err, string(output)) } } // Checkout the specific commit - checkoutCmd := exec.Command("git", "-C", tmpDir, "checkout", ref) + checkoutCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "checkout", ref) if output, err := checkoutCmd.CombinedOutput(); err != nil { return nil, fmt.Errorf("failed to checkout commit %s: %w\nOutput: %s", ref, err, string(output)) } } else { // For branch/tag refs, fetch the specific ref - fetchCmd := exec.Command("git", "-C", tmpDir, "fetch", "--depth", "1", "origin", ref) + fetchCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "fetch", "--depth", "1", "origin", ref) if output, err := fetchCmd.CombinedOutput(); err != nil { return nil, fmt.Errorf("failed to fetch ref %s: %w\nOutput: %s", ref, err, string(output)) } // Checkout FETCH_HEAD - checkoutCmd := exec.Command("git", "-C", tmpDir, "checkout", "FETCH_HEAD") + checkoutCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "checkout", "FETCH_HEAD") if output, err := checkoutCmd.CombinedOutput(); err != nil { return nil, fmt.Errorf("failed to checkout FETCH_HEAD: %w\nOutput: %s", err, string(output)) } @@ -155,21 +156,21 @@ func downloadWorkflowContentViaGitClone(repo, path, ref string, verbose bool) ([ } // downloadWorkflowContent downloads the content of a workflow file from GitHub -func downloadWorkflowContent(repo, path, ref string, verbose bool) ([]byte, error) { +func downloadWorkflowContent(ctx context.Context, repo, path, ref string, verbose bool) ([]byte, error) { downloadLog.Printf("Downloading workflow content: %s/%s@%s", repo, path, ref) if verbose { fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Fetching %s/%s@%s", repo, path, ref))) } // Use gh CLI to download the file - output, err := workflow.RunGHCombined("Downloading workflow...", "api", fmt.Sprintf("/repos/%s/contents/%s?ref=%s", repo, path, ref), "--jq", ".content") + output, err := workflow.RunGHCombinedContext(ctx, "Downloading workflow...", "api", fmt.Sprintf("/repos/%s/contents/%s?ref=%s", repo, path, ref), "--jq", ".content") if err != nil { // Check if this is an authentication error outputStr := string(output) if gitutil.IsAuthError(outputStr) || gitutil.IsAuthError(err.Error()) { downloadLog.Printf("GitHub API authentication failed, attempting git fallback for %s/%s@%s", repo, path, ref) // Try fallback using git commands - content, gitErr := downloadWorkflowContentViaGit(repo, path, ref, verbose) + content, gitErr := downloadWorkflowContentViaGit(ctx, repo, path, ref, verbose) if gitErr != nil { return nil, fmt.Errorf("failed to fetch file content via GitHub API and git: API error: %w, Git error: %w", err, gitErr) } diff --git a/pkg/cli/logs_download.go b/pkg/cli/logs_download.go index 2a03a4c0b93..1a23f4f5e2d 100644 --- a/pkg/cli/logs_download.go +++ b/pkg/cli/logs_download.go @@ -12,6 +12,7 @@ package cli import ( "archive/zip" + "context" "errors" "fmt" "io" @@ -278,7 +279,7 @@ func flattenAgentOutputsArtifact(outputDir string, verbose bool) error { } // downloadWorkflowRunLogs downloads and unzips workflow run logs using GitHub API -func downloadWorkflowRunLogs(runID int64, outputDir string, verbose bool, owner, repo, hostname string) error { +func downloadWorkflowRunLogs(ctx context.Context, runID int64, outputDir string, verbose bool, owner, repo, hostname string) error { logsDownloadLog.Printf("Downloading workflow run logs: run_id=%d, output_dir=%s, owner=%s, repo=%s", runID, outputDir, owner, repo) // Create a temporary file for the zip download @@ -303,7 +304,7 @@ func downloadWorkflowRunLogs(runID int64, outputDir string, verbose bool, owner, args = append(args, "--hostname", hostname) } - output, err := workflow.RunGH("Downloading workflow logs...", args...) + output, err := workflow.RunGHContext(ctx, "Downloading workflow logs...", args...) if err != nil { // Check for authentication errors if strings.Contains(err.Error(), "exit status 4") { @@ -487,7 +488,7 @@ func isDockerBuildArtifact(name string) bool { // listRunArtifactNames returns the names of all artifacts for the given workflow run // by querying the GitHub Actions API. Returns an error if the API call fails. -func listRunArtifactNames(runID int64, owner, repo, hostname string, verbose bool) ([]string, error) { +func listRunArtifactNames(ctx context.Context, runID int64, owner, repo, hostname string, verbose bool) ([]string, error) { var endpoint string if owner != "" && repo != "" { endpoint = fmt.Sprintf("repos/%s/%s/actions/runs/%d/artifacts", owner, repo, runID) @@ -505,7 +506,7 @@ func listRunArtifactNames(runID int64, owner, repo, hostname string, verbose boo fmt.Fprintln(os.Stderr, console.FormatVerboseMessage("Listing artifacts: gh "+strings.Join(args, " "))) } - cmd := workflow.ExecGH(args...) + cmd := workflow.ExecGHContext(ctx, args...) output, err := cmd.Output() if err != nil { return nil, fmt.Errorf("failed to list artifacts for run %d: %w", runID, err) @@ -524,7 +525,7 @@ func listRunArtifactNames(runID int64, owner, repo, hostname string, verbose boo // downloadArtifactsByName downloads a list of artifacts individually by name. // This is used when some artifacts (e.g. .dockerbuild) need to be skipped and // only a subset of the run's artifacts should be downloaded. -func downloadArtifactsByName(runID int64, outputDir string, names []string, verbose bool, owner, repo, hostname string) error { +func downloadArtifactsByName(ctx context.Context, runID int64, outputDir string, names []string, verbose bool, owner, repo, hostname string) error { var repoFlag string if owner != "" && repo != "" { if hostname != "" && hostname != "github.com" { @@ -545,7 +546,7 @@ func downloadArtifactsByName(runID int64, outputDir string, names []string, verb fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Downloading artifact: "+name)) } - cmd := workflow.ExecGH(args...) + cmd := workflow.ExecGHContext(ctx, args...) cmdOutput, cmdErr := cmd.CombinedOutput() if cmdErr != nil { logsDownloadLog.Printf("Failed to download artifact %q: %v (%s)", name, cmdErr, string(cmdOutput)) @@ -570,7 +571,7 @@ var criticalArtifactNames = []string{"activation", "agent"} // was only partially successful. gh run download aborts on the first non-zip artifact, // which may prevent valid artifacts from being downloaded. // artifactFilter limits which critical artifacts are retried; nil means retry all. -func retryCriticalArtifacts(runID int64, outputDir string, verbose bool, owner, repo, hostname string, artifactFilter []string) { +func retryCriticalArtifacts(ctx context.Context, runID int64, outputDir string, verbose bool, owner, repo, hostname string, artifactFilter []string) { // Build the repo flag once for reuse across retries var repoFlag string if owner != "" && repo != "" { @@ -603,7 +604,7 @@ func retryCriticalArtifacts(runID int64, outputDir string, verbose bool, owner, fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Retrying download for missing artifact: "+name)) } - retryCmd := workflow.ExecGH(retryArgs...) + retryCmd := workflow.ExecGHContext(ctx, retryArgs...) retryOutput, retryErr := retryCmd.CombinedOutput() if retryErr != nil { logsDownloadLog.Printf("Failed to download artifact %q individually: %v (%s)", name, retryErr, string(retryOutput)) @@ -621,7 +622,7 @@ func retryCriticalArtifacts(runID int64, outputDir string, verbose bool, owner, // downloadRunArtifacts downloads artifacts for a specific workflow run. // artifactFilter is a list of artifact base names to download; nil means download all. -func downloadRunArtifacts(runID int64, outputDir string, verbose bool, owner, repo, hostname string, artifactFilter []string) error { +func downloadRunArtifacts(ctx context.Context, runID int64, outputDir string, verbose bool, owner, repo, hostname string, artifactFilter []string) error { logsDownloadLog.Printf("Downloading run artifacts: run_id=%d, output_dir=%s, owner=%s, repo=%s, artifactFilter=%v", runID, outputDir, owner, repo, artifactFilter) // Check if artifacts already exist on disk (since they're immutable) @@ -677,7 +678,7 @@ func downloadRunArtifacts(runID int64, outputDir string, verbose bool, owner, re // Proactively list artifacts to detect .dockerbuild files that gh run download cannot // extract (they are not zip archives). When found, skip them and download the // remaining artifacts individually so the bulk download never encounters them. - artifactNames, listErr := listRunArtifactNames(runID, owner, repo, hostname, verbose) + artifactNames, listErr := listRunArtifactNames(ctx, runID, owner, repo, hostname, verbose) var dockerBuildArtifacts, downloadableNames []string if listErr == nil { for _, name := range artifactNames { @@ -712,7 +713,7 @@ func downloadRunArtifacts(runID int64, outputDir string, verbose bool, owner, re if len(downloadableNames) == 0 { // Nothing to download (all artifacts are either .dockerbuild or excluded by filter). // Attempt workflow run logs for diagnostics before returning. - if logErr := downloadWorkflowRunLogs(runID, outputDir, verbose, owner, repo, hostname); logErr != nil { + if logErr := downloadWorkflowRunLogs(ctx, runID, outputDir, verbose, owner, repo, hostname); logErr != nil { if verbose { fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to download workflow run logs: %v", logErr))) } @@ -724,7 +725,7 @@ func downloadRunArtifacts(runID int64, outputDir string, verbose bool, owner, re } return ErrNoArtifacts } - if err := downloadArtifactsByName(runID, outputDir, downloadableNames, verbose, owner, repo, hostname); err != nil { + if err := downloadArtifactsByName(ctx, runID, outputDir, downloadableNames, verbose, owner, repo, hostname); err != nil { return err } if fileutil.IsDirEmpty(outputDir) { @@ -747,7 +748,7 @@ func downloadRunArtifacts(runID int64, outputDir string, verbose bool, owner, re fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Executing: gh "+strings.Join(ghArgs, " "))) } - cmd := workflow.ExecGH(ghArgs...) + cmd := workflow.ExecGHContext(ctx, ghArgs...) output, err := cmd.CombinedOutput() // skippedNonZipArtifacts is set when gh run download fails due to non-zip artifacts @@ -770,7 +771,7 @@ func downloadRunArtifacts(runID int64, outputDir string, verbose bool, owner, re } // Even with no artifacts, attempt to download workflow run logs so that // pre-agent step failures (e.g., activation job errors) can be diagnosed. - if logErr := downloadWorkflowRunLogs(runID, outputDir, verbose, owner, repo, hostname); logErr != nil { + if logErr := downloadWorkflowRunLogs(ctx, runID, outputDir, verbose, owner, repo, hostname); logErr != nil { if verbose { fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to download workflow run logs: %v", logErr))) } @@ -807,7 +808,7 @@ func downloadRunArtifacts(runID int64, outputDir string, verbose bool, owner, re // before downloading all valid artifacts. Retry individually for critical artifacts // that are missing, so flattening and audit analysis can proceed. if skippedNonZipArtifacts { - retryCriticalArtifacts(runID, outputDir, verbose, owner, repo, hostname, artifactFilter) + retryCriticalArtifacts(ctx, runID, outputDir, verbose, owner, repo, hostname, artifactFilter) } if skippedNonZipArtifacts && fileutil.IsDirEmpty(outputDir) { @@ -844,7 +845,7 @@ func downloadRunArtifacts(runID int64, outputDir string, verbose bool, owner, re } // Download and unzip workflow run logs - if err := downloadWorkflowRunLogs(runID, outputDir, verbose, owner, repo, hostname); err != nil { + if err := downloadWorkflowRunLogs(ctx, runID, outputDir, verbose, owner, repo, hostname); err != nil { // Log the error but don't fail the entire download process // Logs may not be available for all runs (e.g., expired or deleted) if verbose { diff --git a/pkg/cli/logs_download_test.go b/pkg/cli/logs_download_test.go index ce62b3d92c4..3ef7a04693c 100644 --- a/pkg/cli/logs_download_test.go +++ b/pkg/cli/logs_download_test.go @@ -258,7 +258,7 @@ func TestRetryCriticalArtifactsSkipsExisting(t *testing.T) { } // Should complete without panic or error — all dirs exist so no gh CLI calls are made - retryCriticalArtifacts(12345, tmpDir, false, "owner", "repo", "", nil) + retryCriticalArtifacts(context.Background(), 12345, tmpDir, false, "owner", "repo", "", nil) // Verify the directories are still intact for _, name := range criticalArtifactNames { diff --git a/pkg/cli/logs_orchestrator.go b/pkg/cli/logs_orchestrator.go index 8d643e6f617..029af5d72e3 100644 --- a/pkg/cli/logs_orchestrator.go +++ b/pkg/cli/logs_orchestrator.go @@ -715,7 +715,7 @@ func downloadRunArtifactsConcurrent(ctx context.Context, runs []WorkflowRun, out } // No cached summary or version mismatch - download and process - err := downloadRunArtifacts(run.DatabaseID, runOutputDir, verbose, dlOwner, dlRepo, "", artifactFilter) + err := downloadRunArtifacts(ctx, run.DatabaseID, runOutputDir, verbose, dlOwner, dlRepo, "", artifactFilter) result := DownloadResult{ Run: run, diff --git a/pkg/cli/trial_runner.go b/pkg/cli/trial_runner.go index bde973dbfc8..6931a8f6326 100644 --- a/pkg/cli/trial_runner.go +++ b/pkg/cli/trial_runner.go @@ -108,7 +108,7 @@ func RunWorkflowTrials(ctx context.Context, workflowSpecs []string, opts TrialOp trialLog.Printf("Using specified host repository: %s", hostRepoSlug) } else { // Use default trial repo with current username - username, err := getCurrentGitHubUsername() + username, err := getCurrentGitHubUsername(ctx) if err != nil { return fmt.Errorf("failed to get GitHub username for default trial repo: %w", err) } @@ -246,8 +246,8 @@ func RunWorkflowTrials(ctx context.Context, workflowSpecs []string, opts TrialOp } // getCurrentGitHubUsername gets the current GitHub username from gh CLI -func getCurrentGitHubUsername() (string, error) { - output, err := workflow.RunGH("Fetching GitHub username...", "api", "user", "--jq", ".login") +func getCurrentGitHubUsername(ctx context.Context) (string, error) { + output, err := workflow.RunGHContext(ctx, "Fetching GitHub username...", "api", "user", "--jq", ".login") if err != nil { return "", fmt.Errorf("failed to get GitHub username: %w", err) } diff --git a/pkg/cli/update_actions.go b/pkg/cli/update_actions.go index 80ac06eb553..017ffd2495a 100644 --- a/pkg/cli/update_actions.go +++ b/pkg/cli/update_actions.go @@ -1,6 +1,7 @@ package cli import ( + "context" "errors" "fmt" "os" @@ -30,7 +31,7 @@ func isCoreAction(repo string) bool { // 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(allowMajor, verbose, disableReleaseBump bool) error { +func UpdateActions(ctx context.Context, allowMajor, verbose, disableReleaseBump bool) error { updateLog.Print("Starting action updates") if verbose { @@ -70,6 +71,9 @@ func UpdateActions(allowMajor, verbose, disableReleaseBump bool) error { } for _, s := range snapshot { + if ctx.Err() != nil { + return ctx.Err() + } entry := s.entry updateLog.Printf("Checking action: %s@%s", entry.Repo, entry.Version) @@ -78,7 +82,7 @@ func UpdateActions(allowMajor, verbose, disableReleaseBump bool) error { effectiveAllowMajor := !disableReleaseBump || allowMajor || isCoreAction(entry.Repo) // Check for latest release using the injectable function (also used by updateActionRefsInContent) - latestVersion, latestSHA, err := getLatestActionReleaseFn(entry.Repo, entry.Version, effectiveAllowMajor, verbose) + latestVersion, latestSHA, err := getLatestActionReleaseFn(ctx, entry.Repo, entry.Version, effectiveAllowMajor, verbose) if err != nil { if verbose { fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to check %s: %v", entry.Repo, err))) @@ -162,7 +166,7 @@ func UpdateActions(allowMajor, verbose, disableReleaseBump bool) error { // getLatestActionRelease gets the latest release for an action repository // It respects semantic versioning and the allowMajor flag -func getLatestActionRelease(repo, currentVersion string, allowMajor, verbose bool) (string, string, error) { +func getLatestActionRelease(ctx context.Context, repo, currentVersion string, allowMajor, verbose bool) (string, string, error) { updateLog.Printf("Getting latest release for %s@%s (allowMajor=%v)", repo, currentVersion, allowMajor) // Extract base repository (e.g., "actions/cache/restore" -> "actions/cache") @@ -170,14 +174,14 @@ func getLatestActionRelease(repo, currentVersion string, allowMajor, verbose boo updateLog.Printf("Using base repository: %s for action: %s", baseRepo, repo) // Use gh CLI to get releases - output, err := runGHReleasesAPIFn(baseRepo) + output, err := runGHReleasesAPIFn(ctx, baseRepo) if err != nil { // Check if this is an authentication error outputStr := string(output) if gitutil.IsAuthError(outputStr) || gitutil.IsAuthError(err.Error()) { updateLog.Printf("GitHub API authentication failed, attempting git ls-remote fallback for %s", repo) // Try fallback using git ls-remote - latestRelease, latestSHA, gitErr := getLatestActionReleaseViaGit(repo, currentVersion, allowMajor, verbose) + latestRelease, latestSHA, gitErr := getLatestActionReleaseViaGit(ctx, repo, currentVersion, allowMajor, verbose) if gitErr != nil { return "", "", fmt.Errorf("failed to fetch releases via GitHub API and git: API error: %w, Git Error: %w", err, gitErr) } @@ -199,7 +203,7 @@ func getLatestActionRelease(repo, currentVersion string, allowMajor, verbose boo if verbose { fmt.Fprintln(os.Stderr, console.FormatInfoMessage(baseRepo+": no GitHub Releases found, falling back to tag scanning (safe to ignore)")) } - latestRelease, latestSHA, gitErr := getLatestActionReleaseViaGitFn(repo, currentVersion, allowMajor, verbose) + latestRelease, latestSHA, gitErr := getLatestActionReleaseViaGitFn(ctx, repo, currentVersion, allowMajor, verbose) if gitErr != nil { return "", "", fmt.Errorf("no releases or tags found for %s: %w", baseRepo, gitErr) } @@ -239,7 +243,7 @@ func getLatestActionRelease(repo, currentVersion string, allowMajor, verbose boo // If current version is not valid, return the highest semver release if currentVer == nil { latestRelease := validReleases[0].tag - sha, err := getActionSHAForTagFn(baseRepo, latestRelease) + sha, err := getActionSHAForTagFn(ctx, baseRepo, latestRelease) if err != nil { return "", "", fmt.Errorf("failed to get SHA for %s: %w", latestRelease, err) } @@ -278,7 +282,7 @@ func getLatestActionRelease(repo, currentVersion string, allowMajor, verbose boo } // Get the SHA for the latest compatible release - sha, err := getActionSHAForTagFn(baseRepo, latestCompatible) + sha, err := getActionSHAForTagFn(ctx, baseRepo, latestCompatible) if err != nil { return "", "", fmt.Errorf("failed to get SHA for %s: %w", latestCompatible, err) } @@ -287,7 +291,7 @@ func getLatestActionRelease(repo, currentVersion string, allowMajor, verbose boo } // getLatestActionReleaseViaGit gets the latest release using git ls-remote (fallback) -func getLatestActionReleaseViaGit(repo, currentVersion string, allowMajor, verbose bool) (string, string, error) { +func getLatestActionReleaseViaGit(ctx context.Context, repo, currentVersion string, allowMajor, verbose bool) (string, string, error) { if verbose { fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Fetching latest release for %s via git ls-remote (current: %s, allow major: %v)", repo, currentVersion, allowMajor))) } @@ -300,7 +304,8 @@ func getLatestActionReleaseViaGit(repo, currentVersion string, allowMajor, verbo repoURL := fmt.Sprintf("%s/%s.git", githubHost, baseRepo) // List all tags - cmd := exec.Command("git", "ls-remote", "--tags", repoURL) + // #nosec G204 -- repoURL is constructed from workflow configuration authored by the developer + cmd := exec.CommandContext(ctx, "git", "ls-remote", "--tags", repoURL) output, err := cmd.Output() if err != nil { return "", "", fmt.Errorf("failed to fetch releases via git ls-remote: %w", err) @@ -412,11 +417,11 @@ func getLatestActionReleaseViaGit(repo, currentVersion string, allowMajor, verbo } // getActionSHAForTag gets the commit SHA for a given tag in an action repository -func getActionSHAForTag(repo, tag string) (string, error) { +func getActionSHAForTag(ctx context.Context, repo, tag string) (string, error) { updateLog.Printf("Getting SHA for %s@%s", repo, tag) // Use gh CLI to get the git ref for the tag - output, err := workflow.RunGH("Fetching tag info...", "api", fmt.Sprintf("/repos/%s/git/ref/tags/%s", repo, tag), "--jq", ".object.sha") + output, err := workflow.RunGHContext(ctx, "Fetching tag info...", "api", fmt.Sprintf("/repos/%s/git/ref/tags/%s", repo, tag), "--jq", ".object.sha") if err != nil { return "", fmt.Errorf("failed to resolve tag: %w", err) } @@ -445,21 +450,27 @@ var actionRefPattern = regexp.MustCompile(`(uses:\s+)([a-zA-Z0-9][a-zA-Z0-9_-]*/ // getLatestActionReleaseFn is the function used to fetch the latest release for an action. // It is used by both UpdateActions and updateActionRefsInContent and can be replaced in // tests to avoid network calls. -var getLatestActionReleaseFn = getLatestActionRelease +var getLatestActionReleaseFn = func(ctx context.Context, repo, currentVersion string, allowMajor, verbose bool) (string, string, error) { + return getLatestActionRelease(ctx, repo, currentVersion, allowMajor, verbose) +} // getLatestActionReleaseViaGitFn is the function used to fetch the latest release via git // ls-remote as a fallback. It can be replaced in tests to avoid network calls. -var getLatestActionReleaseViaGitFn = getLatestActionReleaseViaGit +var getLatestActionReleaseViaGitFn = func(ctx context.Context, repo, currentVersion string, allowMajor, verbose bool) (string, string, error) { + return getLatestActionReleaseViaGit(ctx, repo, currentVersion, allowMajor, verbose) +} // runGHReleasesAPIFn calls the GitHub Releases API for the given base repository and // returns the raw output. It can be replaced in tests to avoid network calls. -var runGHReleasesAPIFn = func(baseRepo string) ([]byte, error) { - return workflow.RunGHCombined("Fetching releases...", "api", fmt.Sprintf("/repos/%s/releases", baseRepo), "--jq", ".[].tag_name") +var runGHReleasesAPIFn = func(ctx context.Context, baseRepo string) ([]byte, error) { + return workflow.RunGHCombinedContext(ctx, "Fetching releases...", "api", fmt.Sprintf("/repos/%s/releases", baseRepo), "--jq", ".[].tag_name") } // getActionSHAForTagFn resolves the commit SHA for a given tag. It can be replaced in // tests to avoid network calls. -var getActionSHAForTagFn = getActionSHAForTag +var getActionSHAForTagFn = func(ctx context.Context, repo, tag string) (string, error) { + return getActionSHAForTag(ctx, repo, tag) +} // latestReleaseResult caches a resolved version/SHA pair. type latestReleaseResult struct { @@ -472,7 +483,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(workflowsDir, engineOverride string, verbose, disableReleaseBump bool, noCompile bool) error { +func UpdateActionsInWorkflowFiles(ctx context.Context, workflowsDir, engineOverride string, verbose, disableReleaseBump bool, noCompile bool) error { if workflowsDir == "" { workflowsDir = getWorkflowsDir() } @@ -488,6 +499,9 @@ func UpdateActionsInWorkflowFiles(workflowsDir, engineOverride string, verbose, if walkErr != nil { return walkErr } + if ctx.Err() != nil { + return ctx.Err() + } if d.IsDir() || !strings.HasSuffix(d.Name(), ".md") { return nil } @@ -500,7 +514,7 @@ func UpdateActionsInWorkflowFiles(workflowsDir, engineOverride string, verbose, return nil } - updated, newContent, err := updateActionRefsInContent(string(content), cache, !disableReleaseBump, verbose) + updated, newContent, err := updateActionRefsInContent(ctx, string(content), cache, !disableReleaseBump, verbose) if err != nil { if verbose { fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to update action refs in %s: %v", path, err))) @@ -546,7 +560,7 @@ func UpdateActionsInWorkflowFiles(workflowsDir, engineOverride string, verbose, // 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(content string, cache map[string]latestReleaseResult, allowMajor, verbose bool) (bool, string, error) { +func updateActionRefsInContent(ctx context.Context, content string, cache map[string]latestReleaseResult, allowMajor, verbose bool) (bool, string, error) { changed := false lines := strings.Split(content, "\n") @@ -597,7 +611,7 @@ func updateActionRefsInContent(content string, cache map[string]latestReleaseRes cacheKey := repo + "|" + currentVersion result, cached := cache[cacheKey] if !cached { - latestVersion, latestSHA, err := getLatestActionReleaseFn(repo, currentVersion, effectiveAllowMajor, verbose) + latestVersion, latestSHA, err := getLatestActionReleaseFn(ctx, repo, currentVersion, effectiveAllowMajor, verbose) if err != nil { updateLog.Printf("Failed to get latest release for %s: %v", repo, err) continue diff --git a/pkg/cli/update_actions_test.go b/pkg/cli/update_actions_test.go index 94d7baa96e6..a2254d40603 100644 --- a/pkg/cli/update_actions_test.go +++ b/pkg/cli/update_actions_test.go @@ -3,6 +3,7 @@ package cli import ( + "context" "errors" "os" "testing" @@ -108,7 +109,7 @@ func TestUpdateActions_SafeOutputsInputsPreserved(t *testing.T) { // actions/checkout gets a bump; owner/my-safe-action is already at latest. orig := getLatestActionReleaseFn defer func() { getLatestActionReleaseFn = orig }() - getLatestActionReleaseFn = func(repo, currentVersion string, allowMajor, verbose bool) (string, string, error) { + getLatestActionReleaseFn = func(_ context.Context, repo, currentVersion string, allowMajor, verbose bool) (string, string, error) { switch repo { case "actions/checkout": return "v5", "newcheckoutsha1234567890123456789012345", nil @@ -145,7 +146,7 @@ func TestUpdateActions_SafeOutputsInputsPreserved(t *testing.T) { t.Fatalf("failed to chdir: %v", err) } - if err := UpdateActions(false, false, false); err != nil { + if err := UpdateActions(context.Background(), false, false, false); err != nil { t.Fatalf("UpdateActions() error = %v", err) } @@ -352,7 +353,7 @@ func TestUpdateActionRefsInContent_NonCoreActionsUnchanged(t *testing.T) { - run: echo hello` cache := make(map[string]latestReleaseResult) - changed, newContent, err := updateActionRefsInContent(input, cache, false, false) + changed, newContent, err := updateActionRefsInContent(context.Background(), input, cache, false, false) if err != nil { t.Fatalf("updateActionRefsInContent() error = %v", err) } @@ -371,7 +372,7 @@ steps: - run: echo world` cache := make(map[string]latestReleaseResult) - changed, _, err := updateActionRefsInContent(input, cache, true, false) + changed, _, err := updateActionRefsInContent(context.Background(), input, cache, true, false) if err != nil { t.Fatalf("updateActionRefsInContent() error = %v", err) } @@ -385,7 +386,7 @@ func TestUpdateActionRefsInContent_VersionTagReplacement(t *testing.T) { orig := getLatestActionReleaseFn defer func() { getLatestActionReleaseFn = orig }() - getLatestActionReleaseFn = func(repo, currentVersion string, allowMajor, verbose bool) (string, string, error) { + getLatestActionReleaseFn = func(_ context.Context, repo, currentVersion string, allowMajor, verbose bool) (string, string, error) { switch repo { case "actions/checkout": return "v6", "de0fac2e4500dabe0009e67214ff5f5447ce83dd", nil @@ -407,7 +408,7 @@ func TestUpdateActionRefsInContent_VersionTagReplacement(t *testing.T) { - run: echo hello` cache := make(map[string]latestReleaseResult) - changed, got, err := updateActionRefsInContent(input, cache, true, false) + changed, got, err := updateActionRefsInContent(context.Background(), input, cache, true, false) if err != nil { t.Fatalf("updateActionRefsInContent() error = %v", err) } @@ -425,7 +426,7 @@ func TestUpdateActionRefsInContent_SHAPinnedReplacement(t *testing.T) { defer func() { getLatestActionReleaseFn = orig }() newSHA := "de0fac2e4500dabe0009e67214ff5f5447ce83dd" - getLatestActionReleaseFn = func(repo, currentVersion string, allowMajor, verbose bool) (string, string, error) { + getLatestActionReleaseFn = func(_ context.Context, repo, currentVersion string, allowMajor, verbose bool) (string, string, error) { return "v6.0.2", newSHA, nil } @@ -434,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(input, cache, true, false) + changed, got, err := updateActionRefsInContent(context.Background(), input, cache, true, false) if err != nil { t.Fatalf("updateActionRefsInContent() error = %v", err) } @@ -452,7 +453,7 @@ func TestUpdateActionRefsInContent_CacheReusedAcrossLines(t *testing.T) { defer func() { getLatestActionReleaseFn = orig }() callCount := 0 - getLatestActionReleaseFn = func(repo, currentVersion string, allowMajor, verbose bool) (string, string, error) { + getLatestActionReleaseFn = func(_ context.Context, repo, currentVersion string, allowMajor, verbose bool) (string, string, error) { callCount++ return "v8", "ed597411d8f9245be5a6f5b7f5d52e63b7e62e96", nil } @@ -463,7 +464,7 @@ func TestUpdateActionRefsInContent_CacheReusedAcrossLines(t *testing.T) { - uses: actions/github-script@v7` cache := make(map[string]latestReleaseResult) - changed, _, err := updateActionRefsInContent(input, cache, true, false) + changed, _, err := updateActionRefsInContent(context.Background(), input, cache, true, false) if err != nil { t.Fatalf("updateActionRefsInContent() error = %v", err) } @@ -481,7 +482,7 @@ func TestUpdateActionRefsInContent_AllOrgsUpdatedWhenAllowMajor(t *testing.T) { orig := getLatestActionReleaseFn defer func() { getLatestActionReleaseFn = orig }() - getLatestActionReleaseFn = func(repo, currentVersion string, allowMajor, verbose bool) (string, string, error) { + getLatestActionReleaseFn = func(_ context.Context, repo, currentVersion string, allowMajor, verbose bool) (string, string, error) { switch repo { case "docker/login-action": return "v4", "newsha11234567890123456789012345678901234", nil @@ -501,7 +502,7 @@ func TestUpdateActionRefsInContent_AllOrgsUpdatedWhenAllowMajor(t *testing.T) { - uses: github/codeql-action@v4` cache := make(map[string]latestReleaseResult) - changed, got, err := updateActionRefsInContent(input, cache, true, false) + changed, got, err := updateActionRefsInContent(context.Background(), input, cache, true, false) if err != nil { t.Fatalf("updateActionRefsInContent() error = %v", err) } @@ -525,17 +526,17 @@ func TestGetLatestActionRelease_FallsBackToGitWhenNoReleases(t *testing.T) { }() // Simulate the GitHub Releases API returning an empty list (no releases published). - runGHReleasesAPIFn = func(baseRepo string) ([]byte, error) { + runGHReleasesAPIFn = func(_ context.Context, baseRepo string) ([]byte, error) { return []byte(""), nil } gitFnCalled := false - getLatestActionReleaseViaGitFn = func(repo, currentVersion string, allowMajor, verbose bool) (string, string, error) { + getLatestActionReleaseViaGitFn = func(_ context.Context, repo, currentVersion string, allowMajor, verbose bool) (string, string, error) { gitFnCalled = true return "v1.2.3", "abc1234567890123456789012345678901234567", nil } - version, sha, err := getLatestActionRelease("github/gh-aw-actions/setup", "v1", false, false) + version, sha, err := getLatestActionRelease(context.Background(), "github/gh-aw-actions/setup", "v1", false, false) if err != nil { t.Fatalf("expected no error when git fallback succeeds, got: %v", err) } @@ -562,16 +563,16 @@ func TestGetLatestActionRelease_FallbackReturnsErrorWhenBothFail(t *testing.T) { }() // Simulate the GitHub Releases API returning an empty list. - runGHReleasesAPIFn = func(baseRepo string) ([]byte, error) { + runGHReleasesAPIFn = func(_ context.Context, baseRepo string) ([]byte, error) { return []byte(""), nil } // Simulate the git fallback also finding nothing. - getLatestActionReleaseViaGitFn = func(repo, currentVersion string, allowMajor, verbose bool) (string, string, error) { + getLatestActionReleaseViaGitFn = func(_ context.Context, repo, currentVersion string, allowMajor, verbose bool) (string, string, error) { return "", "", errors.New("no releases found") } - _, _, err := getLatestActionRelease("github/gh-aw-actions/setup", "v1", false, false) + _, _, err := getLatestActionRelease(context.Background(), "github/gh-aw-actions/setup", "v1", false, false) if err == nil { t.Fatal("expected error when both releases API and git fallback fail, got nil") } @@ -590,15 +591,15 @@ func TestGetLatestActionRelease_PrereleaseTagsSkipped(t *testing.T) { }() // Return a stable release alongside a higher-versioned prerelease. - runGHReleasesAPIFn = func(baseRepo string) ([]byte, error) { + runGHReleasesAPIFn = func(_ context.Context, baseRepo string) ([]byte, error) { return []byte("v1.0.0\nv1.1.0-beta.1"), nil } - getActionSHAForTagFn = func(repo, tag string) (string, error) { + getActionSHAForTagFn = func(_ context.Context, repo, tag string) (string, error) { return "stablesha1234567890123456789012345678901", nil } - version, _, err := getLatestActionRelease("actions/checkout", "v1.0.0", true, false) + version, _, err := getLatestActionRelease(context.Background(), "actions/checkout", "v1.0.0", true, false) if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/pkg/cli/update_command.go b/pkg/cli/update_command.go index 2858b32ea44..02487e4649c 100644 --- a/pkg/cli/update_command.go +++ b/pkg/cli/update_command.go @@ -115,7 +115,7 @@ func RunUpdateWorkflows(ctx context.Context, workflowNames []string, allowMajor, var firstErr error - if err := UpdateWorkflows(workflowNames, allowMajor, force, verbose, engineOverride, workflowsDir, noStopAfter, stopAfter, noMerge, noCompile); err != nil { + if err := UpdateWorkflows(ctx, workflowNames, allowMajor, force, verbose, engineOverride, workflowsDir, noStopAfter, stopAfter, noMerge, noCompile); err != nil { firstErr = fmt.Errorf("workflow update failed: %w", err) } @@ -123,7 +123,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(allowMajor, verbose, disableReleaseBump); err != nil { + if err := UpdateActions(ctx, allowMajor, verbose, disableReleaseBump); 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))) } @@ -138,7 +138,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(workflowsDir, engineOverride, verbose, disableReleaseBump, noCompile); err != nil { + if err := UpdateActionsInWorkflowFiles(ctx, workflowsDir, engineOverride, verbose, disableReleaseBump, noCompile); 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 c4ff4d6644d..07113e4b167 100644 --- a/pkg/cli/update_command_test.go +++ b/pkg/cli/update_command_test.go @@ -785,7 +785,7 @@ func TestGetActionSHAForTag(t *testing.T) { } // Test with a known action and version - sha, err := getActionSHAForTag("actions/checkout", "v4") + sha, err := getActionSHAForTag(context.Background(), "actions/checkout", "v4") if err != nil { t.Skipf("Could not resolve action SHA (network or auth issue): %v", err) } @@ -813,7 +813,7 @@ func TestUpdateActions_NoFile(t *testing.T) { os.Chdir(tmpDir) // Should not error when file doesn't exist - err := UpdateActions(false, false, false) + err := UpdateActions(context.Background(), false, false, false) 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(false, false, false) + err := UpdateActions(context.Background(), false, false, false) 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(false, false, false) + err := UpdateActions(context.Background(), false, false, false) 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("test/repo", sha, false, false) + _, _ = resolveLatestRef(context.Background(), "test/repo", sha, false, false) } // TestResolveLatestRef_NotCommitSHA tests that non-SHA refs are handled appropriately diff --git a/pkg/cli/update_workflows.go b/pkg/cli/update_workflows.go index 15d0156ab32..dc997ae8bc4 100644 --- a/pkg/cli/update_workflows.go +++ b/pkg/cli/update_workflows.go @@ -17,7 +17,7 @@ import ( ) // UpdateWorkflows updates workflows from their source repositories -func UpdateWorkflows(workflowNames []string, allowMajor, force, verbose bool, engineOverride string, workflowsDir string, noStopAfter bool, stopAfter string, noMerge bool, noCompile bool) error { +func UpdateWorkflows(ctx context.Context, workflowNames []string, allowMajor, force, verbose bool, engineOverride string, workflowsDir string, noStopAfter bool, stopAfter string, noMerge bool, noCompile bool) error { updateLog.Printf("Scanning for workflows with source field: dir=%s, filter=%v, noMerge=%v, noCompile=%v", workflowsDir, workflowNames, noMerge, noCompile) // Use provided workflows directory or default @@ -50,7 +50,7 @@ func UpdateWorkflows(workflowNames []string, allowMajor, force, verbose bool, en // Update each workflow for _, wf := range workflows { updateLog.Printf("Updating workflow: %s (source: %s)", wf.Name, wf.SourceSpec) - if err := updateWorkflow(wf, allowMajor, force, verbose, engineOverride, noStopAfter, stopAfter, noMerge, noCompile); err != nil { + if err := updateWorkflow(ctx, wf, allowMajor, force, verbose, engineOverride, noStopAfter, stopAfter, noMerge, noCompile); err != nil { updateLog.Printf("Failed to update workflow %s: %v", wf.Name, err) failedUpdates = append(failedUpdates, updateFailure{ Name: wf.Name, @@ -177,7 +177,7 @@ func findWorkflowsWithSource(workflowsDir string, filterNames []string, verbose } // resolveLatestRef resolves the latest ref for a workflow source -func resolveLatestRef(repo, currentRef string, allowMajor, verbose bool) (string, error) { +func resolveLatestRef(ctx context.Context, repo, currentRef string, allowMajor, verbose bool) (string, error) { updateLog.Printf("Resolving latest ref: repo=%s, currentRef=%s, allowMajor=%v", repo, currentRef, allowMajor) if verbose { @@ -187,7 +187,7 @@ func resolveLatestRef(repo, currentRef string, allowMajor, verbose bool) (string // 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(repo, currentRef, allowMajor, verbose) + return resolveLatestRelease(ctx, repo, currentRef, allowMajor, verbose) } // Check if current ref is a commit SHA (40-character hex string) @@ -195,7 +195,7 @@ func resolveLatestRef(repo, currentRef string, allowMajor, verbose bool) (string updateLog.Printf("Current ref is a commit SHA: %s, fetching latest from default branch", currentRef) // The source field only contains a pinned SHA with no branch information. // Fetch the latest commit from the default branch to check for updates. - return resolveLatestCommitFromDefaultBranch(repo, currentRef, verbose) + return resolveLatestCommitFromDefaultBranch(ctx, repo, currentRef, verbose) } // Otherwise, treat as branch and get latest commit @@ -204,7 +204,7 @@ func resolveLatestRef(repo, currentRef string, allowMajor, verbose bool) (string } // Get the latest commit SHA for the branch - latestSHA, err := getLatestBranchCommitSHA(repo, currentRef) + latestSHA, err := getLatestBranchCommitSHA(ctx, repo, currentRef) if err != nil { return "", fmt.Errorf("failed to get latest commit for branch %s: %w", currentRef, err) } @@ -221,9 +221,9 @@ func resolveLatestRef(repo, currentRef string, allowMajor, verbose bool) (string // the default branch of a repo. This is used when the source field is pinned // to a commit SHA with no branch information — in that case we can only // logically track the default branch. -func resolveLatestCommitFromDefaultBranch(repo, currentSHA string, verbose bool) (string, error) { +func resolveLatestCommitFromDefaultBranch(ctx context.Context, repo, currentSHA string, verbose bool) (string, error) { // Get the default branch name - defaultBranch, err := getRepoDefaultBranch(context.Background(), repo) + defaultBranch, err := getRepoDefaultBranch(ctx, repo) if err != nil { return "", fmt.Errorf("failed to get default branch for %s: %w", repo, err) } @@ -235,7 +235,7 @@ func resolveLatestCommitFromDefaultBranch(repo, currentSHA string, verbose bool) fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Source has no branch ref, tracking default branch %q", defaultBranch))) // Get the latest commit SHA from the default branch - latestSHA, err := getLatestBranchCommitSHA(repo, defaultBranch) + latestSHA, err := getLatestBranchCommitSHA(ctx, repo, defaultBranch) if err != nil { return "", fmt.Errorf("failed to get latest commit for default branch %s: %w", defaultBranch, err) } @@ -261,9 +261,9 @@ func getRepoDefaultBranch(ctx context.Context, repo string) (string, error) { } // getLatestBranchCommitSHA fetches the latest commit SHA for a given branch. -func getLatestBranchCommitSHA(repo, branch string) (string, error) { +func getLatestBranchCommitSHA(ctx context.Context, repo, branch string) (string, error) { // URL-encode the branch name since it may contain slashes (e.g. "feature/foo") - output, err := workflow.RunGH("Fetching branch info...", "api", fmt.Sprintf("/repos/%s/branches/%s", repo, url.PathEscape(branch)), "--jq", ".commit.sha") + output, err := workflow.RunGHContext(ctx, "Fetching branch info...", "api", fmt.Sprintf("/repos/%s/branches/%s", repo, url.PathEscape(branch)), "--jq", ".commit.sha") if err != nil { return "", err } @@ -279,12 +279,12 @@ func getLatestBranchCommitSHA(repo, branch string) (string, error) { // runWorkflowReleasesAPIFn calls the GitHub Releases API for the given repository and // returns the newline-delimited tag names. It is a package-level variable so that // tests can replace it without spawning real gh CLI processes. -var runWorkflowReleasesAPIFn = func(repo string) ([]byte, error) { - return workflow.RunGH("Fetching releases...", "api", fmt.Sprintf("/repos/%s/releases", repo), "--jq", ".[].tag_name") +var runWorkflowReleasesAPIFn = func(ctx context.Context, repo string) ([]byte, error) { + return workflow.RunGHContext(ctx, "Fetching releases...", "api", fmt.Sprintf("/repos/%s/releases", repo), "--jq", ".[].tag_name") } // resolveLatestRelease resolves the latest compatible release for a workflow source -func resolveLatestRelease(repo, currentRef string, allowMajor, verbose bool) (string, error) { +func resolveLatestRelease(ctx context.Context, repo, currentRef string, allowMajor, verbose bool) (string, error) { updateLog.Printf("Resolving latest release for repo %s (current: %s, allowMajor=%v)", repo, currentRef, allowMajor) if verbose { @@ -292,7 +292,7 @@ func resolveLatestRelease(repo, currentRef string, allowMajor, verbose bool) (st } // Get all releases using gh CLI - output, err := runWorkflowReleasesAPIFn(repo) + output, err := runWorkflowReleasesAPIFn(ctx, repo) if err != nil { return "", fmt.Errorf("failed to fetch releases: %w", err) } @@ -368,7 +368,7 @@ func resolveLatestRelease(repo, currentRef string, allowMajor, verbose bool) (st } // updateWorkflow updates a single workflow from its source -func updateWorkflow(wf *workflowWithSource, allowMajor, force, verbose bool, engineOverride string, noStopAfter bool, stopAfter string, noMerge bool, noCompile bool) error { +func updateWorkflow(ctx context.Context, wf *workflowWithSource, allowMajor, force, verbose bool, engineOverride string, noStopAfter bool, stopAfter string, noMerge bool, noCompile bool) error { updateLog.Printf("Updating workflow: name=%s, source=%s, force=%v, noMerge=%v", wf.Name, wf.SourceSpec, force, noMerge) if verbose { @@ -391,7 +391,7 @@ func updateWorkflow(wf *workflowWithSource, allowMajor, force, verbose bool, eng } // Resolve latest ref - latestRef, err := resolveLatestRef(sourceSpec.Repo, currentRef, allowMajor, verbose) + latestRef, err := resolveLatestRef(ctx, sourceSpec.Repo, currentRef, allowMajor, verbose) if err != nil { return fmt.Errorf("failed to resolve latest ref: %w", err) } @@ -414,7 +414,7 @@ func updateWorkflow(wf *workflowWithSource, allowMajor, force, verbose bool, eng updateLog.Printf("Workflow already at latest ref: %s, checking for local modifications", currentRef) // Download the source content to check if local file has been modified - sourceContent, err := downloadWorkflowContent(sourceSpec.Repo, sourceSpec.Path, currentRef, verbose) + sourceContent, err := downloadWorkflowContent(ctx, sourceSpec.Repo, sourceSpec.Path, currentRef, verbose) if err != nil { // If we can't download for comparison, just show the up-to-date message if verbose { @@ -448,7 +448,7 @@ func updateWorkflow(wf *workflowWithSource, allowMajor, force, verbose bool, eng fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Downloading latest version from %s/%s@%s", sourceSpec.Repo, sourceSpec.Path, latestRef))) } - newContent, err := downloadWorkflowContent(sourceSpec.Repo, sourceSpec.Path, latestRef, verbose) + newContent, err := downloadWorkflowContent(ctx, sourceSpec.Repo, sourceSpec.Path, latestRef, verbose) if err != nil { return fmt.Errorf("failed to download workflow: %w", err) } @@ -461,7 +461,7 @@ func updateWorkflow(wf *workflowWithSource, allowMajor, force, verbose bool, eng // When merge mode is on, detect local modifications to confirm we // actually need to merge (if no local mods, override is fine either way). if merge { - baseContent, dlErr := downloadWorkflowContent(sourceSpec.Repo, sourceSpec.Path, currentRef, verbose) + baseContent, dlErr := downloadWorkflowContent(ctx, sourceSpec.Repo, sourceSpec.Path, currentRef, verbose) if dlErr == nil { localContent, readErr := os.ReadFile(wf.Path) if readErr == nil && hasLocalModifications(string(baseContent), string(localContent), wf.SourceSpec, filepath.Dir(wf.Path), verbose) { @@ -489,7 +489,7 @@ func updateWorkflow(wf *workflowWithSource, allowMajor, force, verbose bool, eng fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Downloading base version from %s/%s@%s", sourceSpec.Repo, sourceSpec.Path, currentRef))) } - baseContent, err := downloadWorkflowContent(sourceSpec.Repo, sourceSpec.Path, currentRef, verbose) + baseContent, err := downloadWorkflowContent(ctx, sourceSpec.Repo, sourceSpec.Path, currentRef, verbose) if err != nil { return fmt.Errorf("failed to download base workflow: %w", err) } diff --git a/pkg/cli/update_workflows_test.go b/pkg/cli/update_workflows_test.go index b29433d090b..d8a72057437 100644 --- a/pkg/cli/update_workflows_test.go +++ b/pkg/cli/update_workflows_test.go @@ -3,6 +3,7 @@ package cli import ( + "context" "testing" "github.com/stretchr/testify/assert" @@ -10,7 +11,7 @@ import ( ) // mockWorkflowReleasesAPI stubs runWorkflowReleasesAPIFn for the duration of a test. -func mockWorkflowReleasesAPI(t *testing.T, mockFn func(string) ([]byte, error)) { +func mockWorkflowReleasesAPI(t *testing.T, mockFn func(context.Context, string) ([]byte, error)) { t.Helper() orig := runWorkflowReleasesAPIFn t.Cleanup(func() { runWorkflowReleasesAPIFn = orig }) @@ -22,11 +23,11 @@ func mockWorkflowReleasesAPI(t *testing.T, mockFn func(string) ([]byte, error)) // the latest stable release. Per semver rules, v1.1.0-beta.1 > v1.0.0, so without // explicit filtering a prerelease could be picked incorrectly. func TestResolveLatestRelease_PrereleaseTagsSkipped(t *testing.T) { - mockWorkflowReleasesAPI(t, func(_ string) ([]byte, error) { + mockWorkflowReleasesAPI(t, func(_ context.Context, _ string) ([]byte, error) { return []byte("v1.1.0-beta.1\nv1.0.0"), nil }) - result, err := resolveLatestRelease("owner/repo", "v1.0.0", true, false) + result, err := resolveLatestRelease(context.Background(), "owner/repo", "v1.0.0", true, false) 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") } @@ -36,12 +37,12 @@ func TestResolveLatestRelease_PrereleaseTagsSkipped(t *testing.T) { // semver is returned rather than the first item in the list (which could be a prerelease // or an older release listed first by the API). func TestResolveLatestRelease_PrereleaseSkippedWhenCurrentVersionInvalid(t *testing.T) { - mockWorkflowReleasesAPI(t, func(_ string) ([]byte, error) { + mockWorkflowReleasesAPI(t, func(_ context.Context, _ string) ([]byte, error) { // Prerelease appears first, and older stable release appears before newer one. return []byte("v2.0.0-rc.1\nv1.3.0\nv1.5.0"), nil }) - result, err := resolveLatestRelease("owner/repo", "not-a-version", true, false) + result, err := resolveLatestRelease(context.Background(), "owner/repo", "not-a-version", true, false) 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") } @@ -49,22 +50,22 @@ func TestResolveLatestRelease_PrereleaseSkippedWhenCurrentVersionInvalid(t *test // TestResolveLatestRelease_ErrorWhenOnlyPrereleasesExist verifies that an error is // returned when the releases list contains only prerelease versions. func TestResolveLatestRelease_ErrorWhenOnlyPrereleasesExist(t *testing.T) { - mockWorkflowReleasesAPI(t, func(_ string) ([]byte, error) { + mockWorkflowReleasesAPI(t, func(_ context.Context, _ string) ([]byte, error) { return []byte("v2.0.0-beta.1\nv1.0.0-rc.1"), nil }) - _, err := resolveLatestRelease("owner/repo", "v1.0.0", true, false) + _, err := resolveLatestRelease(context.Background(), "owner/repo", "v1.0.0", true, false) assert.Error(t, err, "should error when no stable releases exist") } // TestResolveLatestRelease_StableReleaseSelected verifies that stable releases are // correctly selected when there are no prereleases. func TestResolveLatestRelease_StableReleaseSelected(t *testing.T) { - mockWorkflowReleasesAPI(t, func(_ string) ([]byte, error) { + mockWorkflowReleasesAPI(t, func(_ context.Context, _ string) ([]byte, error) { return []byte("v1.2.0\nv1.1.0\nv1.0.0"), nil }) - result, err := resolveLatestRelease("owner/repo", "v1.0.0", false, false) + result, err := resolveLatestRelease(context.Background(), "owner/repo", "v1.0.0", false, false) require.NoError(t, err, "should not error when stable releases exist") assert.Equal(t, "v1.2.0", result, "should select highest compatible stable release") } @@ -72,12 +73,12 @@ func TestResolveLatestRelease_StableReleaseSelected(t *testing.T) { // TestResolveLatestRelease_MixedPrereleaseAndStable verifies correct selection when // releases include both prerelease and stable versions across major versions. func TestResolveLatestRelease_MixedPrereleaseAndStable(t *testing.T) { - mockWorkflowReleasesAPI(t, func(_ string) ([]byte, error) { + mockWorkflowReleasesAPI(t, func(_ context.Context, _ string) ([]byte, error) { return []byte("v2.0.0-alpha.1\nv1.3.0\nv1.2.0-rc.1\nv1.1.0"), nil }) // Without allowMajor, should stay on v1.x and skip prereleases. - result, err := resolveLatestRelease("owner/repo", "v1.1.0", false, false) + result, err := resolveLatestRelease(context.Background(), "owner/repo", "v1.1.0", false, false) 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 19ca74221c4..a55b4d0708f 100644 --- a/pkg/cli/upgrade_command.go +++ b/pkg/cli/upgrade_command.go @@ -214,7 +214,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(false, verbose, false); err != nil { + if err := UpdateActions(ctx, false, verbose, false); 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))) diff --git a/pkg/workflow/github_cli.go b/pkg/workflow/github_cli.go index 88c629ba91d..787d33abfca 100644 --- a/pkg/workflow/github_cli.go +++ b/pkg/workflow/github_cli.go @@ -185,6 +185,17 @@ func RunGHCombined(spinnerMessage string, args ...string) ([]byte, error) { return runGHWithSpinner(spinnerMessage, true, args...) } +// RunGHCombinedContext executes a gh CLI command with context support (for cancellation/timeout), +// a spinner, and returns combined stdout+stderr output. The spinner is shown in interactive +// terminals to provide feedback during network operations. +// +// Usage: +// +// output, err := RunGHCombinedContext(ctx, "Fetching releases...", "api", "/repos/owner/repo/releases") +func RunGHCombinedContext(ctx context.Context, spinnerMessage string, args ...string) ([]byte, error) { + return runGHWithSpinnerContext(ctx, spinnerMessage, true, args...) +} + // RunGHWithHost executes a gh CLI command with a spinner, targeting a specific GitHub host. // For non-github.com hosts (GHES, Proxima/data residency), the GH_HOST environment variable // is set on the command. This is necessary because most gh subcommands (repo, pr, run, etc.) From cf243fced901c97faf260cfc82e3e590f2b2c8d0 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Wed, 15 Apr 2026 15:08:22 +1000 Subject: [PATCH 2/7] fix: update_integration_test.go missing ctx for getLatestBranchCommitSHA --- pkg/cli/update_integration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cli/update_integration_test.go b/pkg/cli/update_integration_test.go index eb5f83173bc..de67c2df240 100644 --- a/pkg/cli/update_integration_test.go +++ b/pkg/cli/update_integration_test.go @@ -369,7 +369,7 @@ func TestGetRepoDefaultBranch_Integration(t *testing.T) { func TestGetLatestBranchCommitSHA_Integration(t *testing.T) { skipWithoutGitHubAuth(t) - sha, err := getLatestBranchCommitSHA("actions/checkout", "main") + sha, err := getLatestBranchCommitSHA(context.Background(), "actions/checkout", "main") require.NoError(t, err, "Should fetch latest commit SHA for actions/checkout main branch") assert.True(t, IsCommitSHA(sha), "Result should be a 40-char commit SHA, got: %s", sha) } From d3c4e8f0ba96f87a4527b8f043e208828150fd35 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Wed, 15 Apr 2026 15:52:35 +1000 Subject: [PATCH 3/7] Update pkg/cli/update_actions.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- pkg/cli/update_actions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cli/update_actions.go b/pkg/cli/update_actions.go index 017ffd2495a..dc76621352e 100644 --- a/pkg/cli/update_actions.go +++ b/pkg/cli/update_actions.go @@ -181,7 +181,7 @@ func getLatestActionRelease(ctx context.Context, repo, currentVersion string, al if gitutil.IsAuthError(outputStr) || gitutil.IsAuthError(err.Error()) { updateLog.Printf("GitHub API authentication failed, attempting git ls-remote fallback for %s", repo) // Try fallback using git ls-remote - latestRelease, latestSHA, gitErr := getLatestActionReleaseViaGit(ctx, repo, currentVersion, allowMajor, verbose) + latestRelease, latestSHA, gitErr := getLatestActionReleaseViaGitFn(ctx, repo, currentVersion, allowMajor, verbose) if gitErr != nil { return "", "", fmt.Errorf("failed to fetch releases via GitHub API and git: API error: %w, Git Error: %w", err, gitErr) } From 3a51ede14a1e066e7b3a1e663ac7145f0f77238f Mon Sep 17 00:00:00 2001 From: Don Syme Date: Wed, 15 Apr 2026 15:54:42 +1000 Subject: [PATCH 4/7] update --- pkg/cli/audit_comparison.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cli/audit_comparison.go b/pkg/cli/audit_comparison.go index 24a9f70abd3..66895da81d5 100644 --- a/pkg/cli/audit_comparison.go +++ b/pkg/cli/audit_comparison.go @@ -480,7 +480,7 @@ func findPreviousSuccessfulWorkflowRuns(ctx context.Context, current WorkflowRun args = append(args, "--hostname", hostname) } args = append(args, endpoint, "--jq", jq) - +Context(ctx, output, err := workflow.RunGHCombined("Fetching previous successful workflow run...", args...) if err != nil { return nil, fmt.Errorf("failed to fetch previous successful workflow run: %w", err) From c5ab3fe2dcc8de8f1187e827c8db08602c837b47 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Wed, 15 Apr 2026 15:55:06 +1000 Subject: [PATCH 5/7] fix: use RunGHCombinedContext in findPreviousSuccessfulWorkflowRuns --- pkg/cli/audit_comparison.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cli/audit_comparison.go b/pkg/cli/audit_comparison.go index 66895da81d5..8fe7fcdf197 100644 --- a/pkg/cli/audit_comparison.go +++ b/pkg/cli/audit_comparison.go @@ -480,8 +480,8 @@ func findPreviousSuccessfulWorkflowRuns(ctx context.Context, current WorkflowRun args = append(args, "--hostname", hostname) } args = append(args, endpoint, "--jq", jq) -Context(ctx, - output, err := workflow.RunGHCombined("Fetching previous successful workflow run...", args...) + + output, err := workflow.RunGHCombinedContext(ctx, "Fetching previous successful workflow run...", args...) if err != nil { return nil, fmt.Errorf("failed to fetch previous successful workflow run: %w", err) } From 46257c61328b089f68c307a070779956917feb54 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Wed, 15 Apr 2026 15:56:16 +1000 Subject: [PATCH 6/7] fix: update_integration_test.go missing ctx for resolveLatestRef --- pkg/cli/update_integration_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/cli/update_integration_test.go b/pkg/cli/update_integration_test.go index de67c2df240..ac02cce85c4 100644 --- a/pkg/cli/update_integration_test.go +++ b/pkg/cli/update_integration_test.go @@ -102,7 +102,7 @@ func TestResolveLatestRef_TagIntegration(t *testing.T) { // Use a well-known public repo with releases // actions/checkout has many tagged releases (v3.x, v4.x, etc.) - latestRef, err := resolveLatestRef("actions/checkout", "v4.0.0", false, true) + latestRef, err := resolveLatestRef(context.Background(), "actions/checkout", "v4.0.0", false, true) require.NoError(t, err, "Should resolve latest release for actions/checkout v4.x") // The resolved ref should be a newer v4.x tag @@ -115,7 +115,7 @@ func TestResolveLatestRef_TagMajorUpdateIntegration(t *testing.T) { skipWithoutGitHubAuth(t) // With allowMajor=true, it should resolve to the latest release across all major versions - latestRef, err := resolveLatestRef("actions/checkout", "v3.0.0", true, true) + latestRef, err := resolveLatestRef(context.Background(), "actions/checkout", "v3.0.0", true, true) require.NoError(t, err, "Should resolve latest release with major updates allowed") assert.True(t, isSemanticVersionTag(latestRef), "Resolved ref should be a semantic version tag, got: %s", latestRef) @@ -128,7 +128,7 @@ func TestResolveLatestRef_BranchIntegration(t *testing.T) { skipWithoutGitHubAuth(t) // Use a well-known branch on a public repo - latestRef, err := resolveLatestRef("actions/checkout", "main", false, true) + latestRef, err := resolveLatestRef(context.Background(), "actions/checkout", "main", false, true) require.NoError(t, err, "Should resolve latest commit for branch 'main'") // The result should be a 40-char commit SHA From 3e16324efbe5ade87a017a90fae5b89f82aae6bf Mon Sep 17 00:00:00 2001 From: Don Syme Date: Wed, 15 Apr 2026 15:57:57 +1000 Subject: [PATCH 7/7] fix: update_integration_test.go missing ctx for resolveLatestRef (SHA case) --- pkg/cli/update_integration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cli/update_integration_test.go b/pkg/cli/update_integration_test.go index ac02cce85c4..5fc693daad9 100644 --- a/pkg/cli/update_integration_test.go +++ b/pkg/cli/update_integration_test.go @@ -144,7 +144,7 @@ func TestResolveLatestRef_CommitSHAIntegration(t *testing.T) { // This is an older commit — the resolution should return the latest commit on the default branch oldSHA := "f43a0e5ff2bd294095638e18286ca9a3d1956744" // Known old commit - latestRef, err := resolveLatestRef("actions/checkout", oldSHA, false, true) + latestRef, err := resolveLatestRef(context.Background(), "actions/checkout", oldSHA, false, true) require.NoError(t, err, "Should resolve latest commit from default branch") // The result should be a 40-char commit SHA (the latest on main)