Cache GitHub Actions job metadata in logs output (#59039)

Author: Copilot

pkg/cli/audit_analysis_fanout.go

@@ -20,6 +20,13 @@ func collectAuditAnalysisResults(ctx context.Context, run WorkflowRun, runOutput
 	if includeFirewallAnalyses {
 		launchFirewallAuditAnalyses(g, gctx, &results, runOutputDir, verbose)
 	}
+	if err := g.Wait(); err != nil {
+		return results, err
+	}
+	if ctx.Err() != nil {
+		return results, ctx.Err()
+	}
+	g, gctx = errgroup.WithContext(ctx)
 	launchSupplementalAuditAnalyses(g, gctx, &results, runOutputDir, verbose)
 	if err := g.Wait(); err != nil {
 		return results, err
@@ -44,7 +51,7 @@ func launchCoreAuditAnalyses(g *errgroup.Group, gctx context.Context, results *a
 	expName, expVariant, _ := firstExperimentAssignment(extractExperimentData(runOutputDir))
 
 	launchMetricsAnalysis(g, gctx, results, runOutputDir, verbose, run.WorkflowPath)
-	launchJobDetailsAnalysis(g, gctx, results, run.DatabaseID, verbose)
+	launchJobDetailsAnalysis(g, gctx, results, run.DatabaseID, runOutputDir, verbose)
 	runAuditAnalysis(g, gctx, verbose, "extractMissingToolsFromRun", "Failed to extract missing tools", func(v []MissingToolReport) {
 		results.missingTools = v
 	}, func() ([]MissingToolReport, error) {
@@ -100,12 +107,12 @@ func launchMetricsAnalysis(g *errgroup.Group, gctx context.Context, results *aud
 }
 
 // launchJobDetailsAnalysis exclusively writes results.jobDetails and results.failedJobCount.
-func launchJobDetailsAnalysis(g *errgroup.Group, gctx context.Context, results *auditAnalysisResults, runID int64, verbose bool) {
+func launchJobDetailsAnalysis(g *errgroup.Group, gctx context.Context, results *auditAnalysisResults, runID int64, runOutputDir string, verbose bool) {
 	g.Go(func() error {
 		if err := gctx.Err(); err != nil {
 			return err
 		}
-		jobDetails, failedJobCount, err := fetchJobDetailsWithCounts(gctx, runID, verbose)
+		jobDetails, failedJobCount, err := fetchJobDetailsWithCounts(gctx, runID, runOutputDir, verbose)
 		if err != nil {
 			if gctx.Err() != nil {
 				return gctx.Err()

pkg/cli/logs_download_artifacts.go

@@ -9,6 +9,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"path"
 	"path/filepath"
 	"strconv"
 	"strings"
@@ -27,9 +28,9 @@ func buildRepoFlag(owner, repo, hostname string) string {
 		return ""
 	}
 	if hostname != "" && hostname != "github.com" {
-		return hostname + "/" + owner + "/" + repo
+		return path.Join(hostname, owner, repo)
 	}
-	return owner + "/" + repo
+	return path.Join(owner, repo)
 }
 
 // listArtifacts creates a list of all artifact files in the output directory
@@ -41,8 +42,8 @@ func listArtifacts(outputDir string) ([]string, error) {
 			return err
 		}
 
-		// Skip directories and the summary file itself
-		if info.IsDir() || filepath.Base(path) == runSummaryFileName {
+		// Skip directories and synthesized cache/summary files
+		if info.IsDir() || filepath.Base(path) == runSummaryFileName || filepath.Base(path) == jobsAPIResponseFileName {
 			return nil
 		}
 
@@ -214,54 +215,58 @@ func retryCriticalArtifacts(ctx context.Context, opts downloadArtifactsOptions)
 			logsDownloadLog.Printf("Critical artifact %q already present, skipping retry", name)
 			continue
 		}
+		retryCriticalArtifact(ctx, opts, repoFlag, name, artifactDir)
+	}
+}
 
-		// Stage next to the output directory so promotion can use an atomic same-filesystem rename.
-		stagingDir, err := os.MkdirTemp(filepath.Dir(opts.outputDir), "."+filepath.Base(opts.outputDir)+"-"+name+"-")
-		if err != nil {
-			logsDownloadLog.Printf("Failed to create staging directory for critical artifact %q: %v", name, err)
-			continue
-		}
+func retryCriticalArtifact(ctx context.Context, opts downloadArtifactsOptions, repoFlag, name, artifactDir string) {
+	// Stage next to the output directory so promotion can use an atomic same-filesystem rename.
+	stagingDir, err := os.MkdirTemp(filepath.Dir(opts.outputDir), "."+filepath.Base(opts.outputDir)+"-"+name+"-")
+	if err != nil {
+		logsDownloadLog.Printf("Failed to create staging directory for critical artifact %q: %v", name, err)
+		return
+	}
 
-		retryArgs := []string{"run", "download", strconv.FormatInt(opts.runID, 10), "--name", name, "--dir", stagingDir}
-		if repoFlag != "" {
-			retryArgs = append(retryArgs, "-R", repoFlag)
-		}
+	retryArgs := []string{"run", "download", strconv.FormatInt(opts.runID, 10), "--name", name, "--dir", stagingDir}
+	if repoFlag != "" {
+		retryArgs = append(retryArgs, "-R", repoFlag)
+	}
+
+	logsDownloadLog.Printf("Retrying individual download for artifact %q: gh %s", name, strings.Join(retryArgs, " "))
+	if opts.verbose {
+		fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Retrying download for missing artifact: "+name))
+	}
 
-		logsDownloadLog.Printf("Retrying individual download for artifact %q: gh %s", name, strings.Join(retryArgs, " "))
+	retryCmd := workflow.ExecGHContext(ctx, retryArgs...)
+	retryOutput, retryErr := retryCmd.CombinedOutput()
+	if retryErr != nil {
+		_ = os.RemoveAll(stagingDir)
+		logsDownloadLog.Printf("Failed to download artifact %q individually: %v (%s)", name, retryErr, string(retryOutput))
 		if opts.verbose {
-			fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Retrying download for missing artifact: "+name))
+			fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Could not download artifact %q: %v", name, retryErr)))
 		}
+		return
+	}
 
-		retryCmd := workflow.ExecGHContext(ctx, retryArgs...)
-		retryOutput, retryErr := retryCmd.CombinedOutput()
-		if retryErr != nil {
-			_ = os.RemoveAll(stagingDir)
-			logsDownloadLog.Printf("Failed to download artifact %q individually: %v (%s)", name, retryErr, string(retryOutput))
-			if opts.verbose {
-				fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Could not download artifact %q: %v", name, retryErr)))
-			}
-		} else {
-			logsDownloadLog.Printf("Successfully downloaded artifact %q individually", name)
-			if err := os.RemoveAll(artifactDir); err != nil {
-				_ = os.RemoveAll(stagingDir)
-				logsDownloadLog.Printf("Failed to remove existing critical artifact directory %q: %v", artifactDir, err)
-				continue
-			}
-			if err := os.Rename(stagingDir, artifactDir); err != nil {
-				_ = os.RemoveAll(stagingDir)
-				logsDownloadLog.Printf("Failed to promote critical artifact %q from staging: %v", name, err)
-				continue
-			}
-			// Marker write failures are non-fatal in the retry path: retryCriticalArtifacts
-			// is a best-effort recovery after a partial bulk download, so a missing marker
-			// only causes a redundant re-download on the next run (not data loss).
-			if err := markArtifactDownloaded(opts.outputDir, name); err != nil {
-				logsDownloadLog.Printf("Failed to mark artifact %q as downloaded: %v", name, err)
-			}
-			if opts.verbose {
-				fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Downloaded missing artifact: "+name))
-			}
-		}
+	logsDownloadLog.Printf("Successfully downloaded artifact %q individually", name)
+	if err := os.RemoveAll(artifactDir); err != nil {
+		_ = os.RemoveAll(stagingDir)
+		logsDownloadLog.Printf("Failed to remove existing critical artifact directory %q: %v", artifactDir, err)
+		return
+	}
+	if err := os.Rename(stagingDir, artifactDir); err != nil {
+		_ = os.RemoveAll(stagingDir)
+		logsDownloadLog.Printf("Failed to promote critical artifact %q from staging: %v", name, err)
+		return
+	}
+	// Marker write failures are non-fatal in the retry path: retryCriticalArtifacts
+	// is a best-effort recovery after a partial bulk download, so a missing marker
+	// only causes a redundant re-download on the next run (not data loss).
+	if err := markArtifactDownloaded(opts.outputDir, name); err != nil {
+		logsDownloadLog.Printf("Failed to mark artifact %q as downloaded: %v", name, err)
+	}
+	if opts.verbose {
+		fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Downloaded missing artifact: "+name))
 	}
 }
 

pkg/cli/logs_github_api.go

@@ -16,12 +16,14 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"slices"
 	"strconv"
 	"strings"
 	"time"
 
 	"github.com/github/gh-aw/pkg/console"
+	"github.com/github/gh-aw/pkg/constants"
 	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/workflow"
 )
@@ -76,63 +78,121 @@ func buildCreatedFilter(startDate, endDate, beforeDate string) string {
 // call and returns the full detail slice together with the count of failed jobs.
 // It is the single source of truth for the jobs endpoint; fetchJobDetails and
 // fetchJobStatuses are thin wrappers that each return only the value they need.
-func fetchJobDetailsWithCounts(ctx context.Context, runID int64, verbose bool) ([]JobInfoWithDuration, int, error) {
+func fetchJobDetailsWithCounts(ctx context.Context, runID int64, outputDir string, verbose bool) ([]JobInfoWithDuration, int, error) {
 	logsGitHubAPILog.Printf("Fetching job details: runID=%d", runID)
 	if verbose {
 		fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Fetching job details for run %d", runID)))
 	}
 
 	output, err := workflow.RunGHCombinedContext(ctx, "Fetching job details...", "api",
-		fmt.Sprintf("repos/{owner}/{repo}/actions/runs/%d/jobs", runID),
-		"--jq", ".jobs[] | {name: .name, status: .status, conclusion: (.conclusion // \"\"), started_at: .started_at, completed_at: .completed_at, steps: ((.steps // []) | map({name: .name, status: .status, conclusion: (.conclusion // \"\")}))}")
+		fmt.Sprintf("repos/{owner}/{repo}/actions/runs/%d/jobs?per_page=100", runID),
+		"--paginate", "--slurp")
 	if err != nil {
 		if verbose {
 			fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Failed to fetch job details for run %d: %v", runID, err)))
 		}
 		return nil, 0, err
 	}
 
-	var jobs []JobInfoWithDuration
-	failedJobs := 0
-	lines := strings.SplitSeq(strings.TrimSpace(string(output)), "\n")
-	for line := range lines {
-		if strings.TrimSpace(line) == "" {
-			continue
-		}
+	var responses []struct {
+		Jobs []json.RawMessage `json:"jobs"`
+	}
+	if err := json.Unmarshal(output, &responses); err != nil {
+		return nil, 0, fmt.Errorf("failed to parse jobs API response: %w", err)
+	}
 
-		var job JobInfo
-		if err := json.Unmarshal([]byte(line), &job); err != nil {
-			if verbose {
-				fmt.Fprintln(os.Stderr, console.FormatVerboseMessage("Failed to parse job info: "+line))
+	jobs := []JobInfoWithDuration{}
+	failedJobs := 0
+	for _, response := range responses {
+		for _, rawJob := range response.Jobs {
+			var job JobInfo
+			if err := json.Unmarshal(rawJob, &job); err != nil {
+				logsGitHubAPILog.Printf("Skipping malformed job in run %d: %v", runID, err)
+				continue
+			}
+			jobWithDuration := JobInfoWithDuration{JobInfo: job}
+			if !job.StartedAt.IsZero() && !job.CompletedAt.IsZero() {
+				jobWithDuration.Duration = job.CompletedAt.Sub(job.StartedAt)
+			}
+			jobs = append(jobs, jobWithDuration)
+
+			if isFailureConclusion(job.Conclusion) {
+				failedJobs++
+				logsGitHubAPILog.Printf("Found failed job: name=%s, conclusion=%s", job.Name, job.Conclusion)
+				if verbose {
+					fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Found failed job '%s' with conclusion '%s'", job.Name, job.Conclusion)))
+				}
 			}
-			continue
-		}
-
-		jobWithDuration := JobInfoWithDuration{JobInfo: job}
-		if !job.StartedAt.IsZero() && !job.CompletedAt.IsZero() {
-			jobWithDuration.Duration = job.CompletedAt.Sub(job.StartedAt)
 		}
-		jobs = append(jobs, jobWithDuration)
+	}
 
-		if isFailureConclusion(job.Conclusion) {
-			failedJobs++
-			logsGitHubAPILog.Printf("Found failed job: name=%s, conclusion=%s", job.Name, job.Conclusion)
-			if verbose {
-				fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Found failed job '%s' with conclusion '%s'", job.Name, job.Conclusion)))
-			}
+	if outputDir != "" {
+		responsePath := filepath.Join(outputDir, jobsAPIResponseFileName)
+		if err := writeSensitiveFile(responsePath, output); err != nil {
+			return jobs, failedJobs, &jobDetailsCacheError{err: fmt.Errorf("failed to cache jobs API response: %w", err)}
 		}
+		logsGitHubAPILog.Printf("Cached jobs API response: path=%s", responsePath)
 	}
 
 	logsGitHubAPILog.Printf("Job fetch complete: total=%d failed=%d", len(jobs), failedJobs)
 	return jobs, failedJobs, nil
 }
 
+type jobDetailsCacheError struct {
+	err error
+}
+
+func (e *jobDetailsCacheError) Error() string {
+	return e.err.Error()
+}
+
+func (e *jobDetailsCacheError) Unwrap() error {
+	return e.err
+}
+
+func writeSensitiveFile(path string, data []byte) (err error) {
+	file, err := os.CreateTemp(filepath.Dir(path), "."+filepath.Base(path)+".tmp-*")
+	if err != nil {
+		return err
+	}
+	tempPath := file.Name()
+	closed := false
+	defer func() {
+		if !closed {
+			if closeErr := file.Close(); err == nil {
+				err = closeErr
+			}
+		}
+		if removeErr := os.Remove(tempPath); err == nil && removeErr != nil && !errors.Is(removeErr, os.ErrNotExist) {
+			err = removeErr
+		}
+	}()
+	if err := file.Chmod(constants.FilePermSensitive); err != nil {
+		return err
+	}
+	if _, err := file.Write(data); err != nil {
+		return err
+	}
+	if err := file.Close(); err != nil {
+		return err
+	}
+	closed = true
+	if err := os.Rename(tempPath, path); err != nil {
+		return err
+	}
+	return nil
+}
+
 // fetchJobDetails gets detailed job information including durations for a workflow run.
 // Errors from the underlying API call are suppressed so that callers can continue
 // processing even when job data is unavailable (e.g. missing permissions).
-func fetchJobDetails(ctx context.Context, runID int64, verbose bool) ([]JobInfoWithDuration, error) {
-	jobs, _, err := fetchJobDetailsWithCounts(ctx, runID, verbose)
+func fetchJobDetails(ctx context.Context, runID int64, outputDir string, verbose bool) ([]JobInfoWithDuration, error) {
+	jobs, _, err := fetchJobDetailsWithCounts(ctx, runID, outputDir, verbose)
 	if err != nil {
+		var cacheErr *jobDetailsCacheError
+		if errors.As(err, &cacheErr) {
+			return jobs, err
+		}
 		// Don't fail the entire operation if we can't get job info
 		return nil, nil
 	}
@@ -143,7 +203,7 @@ func fetchJobDetails(ctx context.Context, runID int64, verbose bool) ([]JobInfoW
 // Errors from the underlying API call are suppressed so that callers can continue
 // processing even when job data is unavailable (e.g. missing permissions).
 func fetchJobStatuses(ctx context.Context, runID int64, verbose bool) (int, error) {
-	_, failedJobs, err := fetchJobDetailsWithCounts(ctx, runID, verbose)
+	_, failedJobs, err := fetchJobDetailsWithCounts(ctx, runID, "", verbose)
 	if err != nil {
 		// Don't fail the entire operation if we can't get job info
 		return 0, nil
@@ -189,7 +249,7 @@ type ListWorkflowRunsOptions struct {
 // not the total number of matching runs the user wants to find.
 //
 // The processedCount and targetCount parameters are used to display progress in the spinner message.
-func listWorkflowRunsWithPagination(opts ListWorkflowRunsOptions) ([]WorkflowRun, int, error) {
+func listWorkflowRunsWithPagination(opts ListWorkflowRunsOptions) ([]WorkflowRun, int, error) { //nolint:largefunc // Existing run listing keeps pagination, error classification, and filtering together.
 	logsGitHubAPILog.Printf("Listing workflow runs: workflow=%s, limit=%d, startDate=%s, endDate=%s, ref=%s", opts.WorkflowName, opts.Limit, opts.StartDate, opts.EndDate, opts.Ref)
 	args := []string{"run", "list", "--json", "databaseId,number,url,status,conclusion,workflowName,createdAt,startedAt,updatedAt,event,headBranch,headSha,displayTitle"}