Skip to content

Cache GitHub Actions job metadata in logs output #59039

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 4 commits into from
Sep 6, 2026
Merged

Cache GitHub Actions job metadata in logs output #59039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8e7ba1c
Cache GitHub jobs API responses
Copilot Sep 6, 2026
f2c1e1a
Secure cached jobs API data
Copilot Sep 6, 2026
2fa1775
Harden jobs cache persistence
Copilot Sep 6, 2026
96d1cd6
Refine jobs cache error handling
Copilot Sep 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions pkg/cli/audit_analysis_fanout.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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()
Expand Down
97 changes: 51 additions & 46 deletions pkg/cli/logs_download_artifacts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"context"
"fmt"
"os"
"path"
"path/filepath"
"strconv"
"strings"
Expand All @@ -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
Expand All @@ -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
}

Expand Down Expand Up @@ -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))
}
}

Expand Down
122 changes: 91 additions & 31 deletions pkg/cli/logs_github_api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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 {

@github-actions github-actions Bot Sep 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/diagnosing-bugs] Resilience regression: parsing now aborts entirely on any malformed byte in the whole paginated response, whereas the old per-line loop skipped just the bad job and kept the rest.

💡 Details

The previous implementation iterated line-by-line and logged+skipped any job that failed to unmarshal, so a single corrupted/partial page didn't discard every other job. Now json.Unmarshal(output, &responses) is all-or-nothing — one bad page (e.g. truncated network response, unexpected field type) zeroes out job info for the entire run, even though most pages were fine. Since this directly affects the failed-job count and audit summaries, consider decoding per-page (or per-job) with a fallback that logs and continues, matching the old resilience behavior. Add a test with one malformed page among several valid ones to lock in the desired behavior.

@copilot please address this.

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
}
Expand All @@ -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
Expand Down Expand Up @@ -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"}

Expand Down
Loading