From 725471f7176f0c1b66a5c41d1532cfd2951a55af Mon Sep 17 00:00:00 2001 From: pelikhan Date: Thu, 15 Oct 2026 21:01:23 -0700 Subject: [PATCH 1/5] fix(health): ignore runs awaiting approval Exclude waiting runs and non-dispatched conclusions from health metrics and revision evidence while preserving pagination to executed runs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/src/content/docs/setup/cli.md | 2 ++ pkg/cli/health_command.go | 1 + pkg/cli/health_metrics.go | 6 ++++ pkg/cli/health_revisions.go | 3 ++ pkg/cli/health_revisions_test.go | 25 +++++++++++++++ pkg/cli/health_sampling.go | 4 +-- pkg/cli/health_sampling_test.go | 51 ++++++++++++++++++++++++++++++ 7 files changed, 90 insertions(+), 2 deletions(-) diff --git a/docs/src/content/docs/setup/cli.md b/docs/src/content/docs/setup/cli.md index 12cb829bae2..debdbe1369a 100644 --- a/docs/src/content/docs/setup/cli.md +++ b/docs/src/content/docs/setup/cli.md @@ -787,6 +787,8 @@ The `--days` flag accepts 7, 30, or 90 (default: 7). Other values produce an err For release-scoped evidence, pass the release's publication timestamp as `--start-date`; this filters by time, not compiler version. Text output and JSON `collection.start_date` / `collection.end_date` show the resolved UTC window. Health still samples current workflow revisions toward five completed first-attempt runs per scope, subject to API limits; it does not enumerate every run or reconstruct historical branch heads. A narrow window can leave insufficient evidence. +Runs awaiting approval (`waiting` status or `action_required` conclusion) and skipped runs are excluded from health metrics and revision evidence. Collection continues past these runs to find executed runs within the requested window and API limits. + Shows success/failure rates, trend indicators (↑ improving, → stable, ↓ degrading), execution duration, token usage, costs, and warnings when success rate drops below threshold. Runs that never dispatched a job are excluded from these metrics: `skipped` runs (the activation condition evaluated to false, for example a comment that does not contain the workflow's command) and `action_required` runs (created by GitHub but held for manual approval, for example comment events authored by a bot actor). Including them would report command workflows as near-0% success even when every dispatched run succeeded. diff --git a/pkg/cli/health_command.go b/pkg/cli/health_command.go index 295ea0e0791..61f46a6751e 100644 --- a/pkg/cli/health_command.go +++ b/pkg/cli/health_command.go @@ -57,6 +57,7 @@ Use --start-date and --end-date to bound run creation times. Accepts UTC dates, RFC3339 timestamps, or relative times such as -24h and -1w. An explicit start replaces --days; otherwise --days counts backward from --end-date (default: now). The report remains a bounded current-revision sample, not an exhaustive history. +Runs awaiting approval (waiting or action_required) and skipped runs are ignored. ` + WorkflowIDExplanation, Example: ` ` + string(constants.CLIExtensionPrefix) + ` health # Current status of all workflows diff --git a/pkg/cli/health_metrics.go b/pkg/cli/health_metrics.go index e69e8cb87c7..4e98a4b5b18 100644 --- a/pkg/cli/health_metrics.go +++ b/pkg/cli/health_metrics.go @@ -2,6 +2,7 @@ package cli import ( "fmt" + "slices" "time" "github.com/github/gh-aw/pkg/console" @@ -78,6 +79,7 @@ func (t TrendDirection) String() string { // CalculateWorkflowHealth calculates health metrics for a workflow from its runs func CalculateWorkflowHealth(workflowName string, runs []WorkflowRun, threshold float64) WorkflowHealth { + runs = slices.DeleteFunc(slices.Clone(runs), isIgnoredHealthRun) healthMetricsLog.Printf("Calculating health for workflow: %s, runs: %d", workflowName, len(runs)) if len(runs) == 0 { @@ -102,6 +104,10 @@ func CalculateWorkflowHealth(workflowName string, runs []WorkflowRun, threshold return health } +func isIgnoredHealthRun(run WorkflowRun) bool { + return run.Status == "waiting" || isNonDispatchedConclusion(run.Conclusion) +} + func accumulateWorkflowHealth(runs []WorkflowRun) WorkflowHealth { health := WorkflowHealth{TotalRuns: len(runs)} var durationStats, tokenStats stats.StatVar diff --git a/pkg/cli/health_revisions.go b/pkg/cli/health_revisions.go index ff96c456ca6..8c1f2c45124 100644 --- a/pkg/cli/health_revisions.go +++ b/pkg/cli/health_revisions.go @@ -53,6 +53,9 @@ func healthRevisionStatuses(runs []WorkflowRun, collection *HealthCollection) [] byScope := make(map[string]int) seen := make(map[int64]struct{}) for _, run := range ordered { + if isIgnoredHealthRun(run) { + continue + } if _, duplicate := seen[run.DatabaseID]; duplicate { continue } diff --git a/pkg/cli/health_revisions_test.go b/pkg/cli/health_revisions_test.go index 4edb3eea23c..5f2c7275f67 100644 --- a/pkg/cli/health_revisions_test.go +++ b/pkg/cli/health_revisions_test.go @@ -51,6 +51,31 @@ func TestHealthRevisionRecoveryAndCurrentClusters(t *testing.T) { assert.Equal(t, health, calculateCurrentWorkflowHealth("Example", runs, config)) } +func TestHealthIgnoresRunsAwaitingApproval(t *testing.T) { + for _, blocked := range []WorkflowRun{ + {Status: "waiting"}, + {Status: "completed", Conclusion: "action_required"}, + {Status: "completed", Conclusion: "skipped"}, + } { + t.Run(blocked.Status+"/"+blocked.Conclusion, func(t *testing.T) { + runs, collection := healthRevisionFixture() + config := HealthConfig{Threshold: 80, collection: collection} + expected := calculateCurrentWorkflowHealth("Example", runs, config) + expectedMetrics := CalculateWorkflowHealth("Example", runs, 80) + run := runs[len(runs)-1] + run.DatabaseID, run.HeadSha = 11, "unexecuted" + run.Status, run.Conclusion = blocked.Status, blocked.Conclusion + run.CreatedAt = run.CreatedAt.Add(time.Hour) + run.Duration, run.TokenUsage = time.Hour, 100000 + run.HeadBranch, run.Event = "unapproved", "pull_request" + runs = append([]WorkflowRun{run}, runs...) + assert.Equal(t, expected, calculateCurrentWorkflowHealth("Example", runs, config)) + assert.Equal(t, expectedMetrics, CalculateWorkflowHealth("Example", runs, 80)) + assert.Equal(t, run, runs[0], "health calculation must not mutate caller data") + }) + } +} + func TestHealthRevisionsDoNotClaimUnsupportedRecovery(t *testing.T) { for _, scenario := range []string{"same-content", "one-success", "retry-only", "unknown-revision", "still-failing", "overlapping-runs", "other-branch", "other-event"} { t.Run(scenario, func(t *testing.T) { diff --git a/pkg/cli/health_sampling.go b/pkg/cli/health_sampling.go index 6e9a550060c..fb2e8daefb7 100644 --- a/pkg/cli/health_sampling.go +++ b/pkg/cli/health_sampling.go @@ -103,10 +103,10 @@ func listHealthWorkflowRuns(opts ListWorkflowRunsOptions, page int, collection * } var runs []WorkflowRun for _, response := range data.Runs { - if isNonDispatchedConclusion(response.Conclusion) { + run := response.workflowRun() + if isIgnoredHealthRun(run) { continue } - run := response.workflowRun() runs = append(runs, run) } return runs, len(data.Runs), nil diff --git a/pkg/cli/health_sampling_test.go b/pkg/cli/health_sampling_test.go index 584ca8b40df..6bd0d31b767 100644 --- a/pkg/cli/health_sampling_test.go +++ b/pkg/cli/health_sampling_test.go @@ -6,6 +6,7 @@ import ( "context" "encoding/json" "errors" + "strconv" "strings" "testing" "time" @@ -36,6 +37,56 @@ func TestHealthSamplingCollectsEvidenceForEachWorkflow(t *testing.T) { assert.False(t, collection.Partial) } +func TestHealthSamplingContinuesPastRunsAwaitingApproval(t *testing.T) { + originalRate, originalGH := fetchRateLimitForHostFunc, healthRunGH + t.Cleanup(func() { fetchRateLimitForHostFunc, healthRunGH = originalRate, originalGH }) + fetchRateLimitForHostFunc = func(context.Context, string) (rateLimitResource, error) { + return rateLimitResource{Limit: 5000, Remaining: 4000}, nil + } + pages := 0 + healthRunGH = func(_ context.Context, _ string, args ...string) ([]byte, error) { + endpoint := args[3] + if strings.Contains(endpoint, "/contents/") { + assert.NotContains(t, endpoint, "unapproved", "ignored runs need no revision lookups") + return []byte(`# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"settings","body_hash":"prompt"}`), nil + } + pages++ + assert.Contains(t, endpoint, "page="+strconv.Itoa(pages)) + var responses []workflowRunAPIResponse + count := 10 + if pages == 2 { + count = 5 + } + for i := range count { + run := workflowRunAPIResponse{ID: int64(pages*10 + i), Name: "Example", + Path: ".github/workflows/example.lock.yml", Status: "completed", + Conclusion: "success", Event: "schedule", HeadBranch: "main", + HeadSHA: strings.Repeat("a", 40), RunAttempt: 1} + if pages == 1 { + run.HeadSHA = "unapproved" + if i%2 == 0 { + run.Status, run.Conclusion = "waiting", "" + } else { + run.Conclusion = "action_required" + } + } + responses = append(responses, run) + } + out, err := json.Marshal(map[string]any{"workflow_runs": responses}) + require.NoError(t, err) + return out, nil + } + collection := &HealthCollection{Host: "github.com", repoOverride: "org/repo"} + runs, err := sampleHealthWorkflow(GitHubWorkflow{ID: 1, Name: "Example", + Path: ".github/workflows/example.lock.yml"}, "2026-10-08", "org/repo", false, collection) + require.NoError(t, err) + assert.Equal(t, 2, pages) + assert.Len(t, runs, 5) + health := calculateCurrentWorkflowHealth("Example", runs, HealthConfig{Threshold: 80, collection: collection}) + assert.Equal(t, "healthy", health.Status) + assert.InDelta(t, 100.0, health.SuccessRate, 1e-9) +} + func TestHealthWorkflowSelectionUsesStablePaths(t *testing.T) { original := healthDiscoverWorkflows t.Cleanup(func() { healthDiscoverWorkflows = original }) From ed857be33dda36557089f791571f6fdadb506781 Mon Sep 17 00:00:00 2001 From: pelikhan Date: Thu, 15 Oct 2026 21:14:12 -0700 Subject: [PATCH 2/5] fix(health): exclude confirmed no-job failures Probe the sampled run attempt through the budgeted Jobs API and omit failures with no jobs from metrics, revisions, and failure clusters. Preserve executed failures, paginate to older evidence, and surface unavailable or malformed job responses. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/src/content/docs/setup/cli.md | 2 +- pkg/cli/health_command.go | 2 +- pkg/cli/health_no_jobs_test.go | 153 +++++++++++++++++++++++++++++ pkg/cli/health_sampling.go | 31 ++++++ pkg/cli/health_sampling_test.go | 4 +- 5 files changed, 189 insertions(+), 3 deletions(-) create mode 100644 pkg/cli/health_no_jobs_test.go diff --git a/docs/src/content/docs/setup/cli.md b/docs/src/content/docs/setup/cli.md index debdbe1369a..260b76e2d90 100644 --- a/docs/src/content/docs/setup/cli.md +++ b/docs/src/content/docs/setup/cli.md @@ -787,7 +787,7 @@ The `--days` flag accepts 7, 30, or 90 (default: 7). Other values produce an err For release-scoped evidence, pass the release's publication timestamp as `--start-date`; this filters by time, not compiler version. Text output and JSON `collection.start_date` / `collection.end_date` show the resolved UTC window. Health still samples current workflow revisions toward five completed first-attempt runs per scope, subject to API limits; it does not enumerate every run or reconstruct historical branch heads. A narrow window can leave insufficient evidence. -Runs awaiting approval (`waiting` status or `action_required` conclusion) and skipped runs are excluded from health metrics and revision evidence. Collection continues past these runs to find executed runs within the requested window and API limits. +Runs awaiting approval (`waiting` status or `action_required` conclusion), skipped runs, and failed runs with no jobs are excluded from health metrics, revision evidence, and failure clusters. Failed runs are checked against the Jobs API for their run attempt, including runs whose approval expired. Collection continues past ignored runs to find executed runs within the requested window and API limits. An unavailable or malformed Jobs API response is reported as an error, not treated as evidence that no jobs ran. Shows success/failure rates, trend indicators (↑ improving, → stable, ↓ degrading), execution duration, token usage, costs, and warnings when success rate drops below threshold. diff --git a/pkg/cli/health_command.go b/pkg/cli/health_command.go index 61f46a6751e..6ef93e6365d 100644 --- a/pkg/cli/health_command.go +++ b/pkg/cli/health_command.go @@ -57,7 +57,7 @@ Use --start-date and --end-date to bound run creation times. Accepts UTC dates, RFC3339 timestamps, or relative times such as -24h and -1w. An explicit start replaces --days; otherwise --days counts backward from --end-date (default: now). The report remains a bounded current-revision sample, not an exhaustive history. -Runs awaiting approval (waiting or action_required) and skipped runs are ignored. +Runs awaiting approval (waiting or action_required), skipped runs, and failed runs with no jobs are ignored. ` + WorkflowIDExplanation, Example: ` ` + string(constants.CLIExtensionPrefix) + ` health # Current status of all workflows diff --git a/pkg/cli/health_no_jobs_test.go b/pkg/cli/health_no_jobs_test.go new file mode 100644 index 00000000000..fead1afdab7 --- /dev/null +++ b/pkg/cli/health_no_jobs_test.go @@ -0,0 +1,153 @@ +//go:build !integration + +package cli + +import ( + "context" + "encoding/json" + "errors" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestHealthSamplingContinuesPastNoJobFailures(t *testing.T) { + for _, conclusion := range []string{"failure", "timed_out", "cancelled", "startup_failure"} { + t.Run(conclusion, func(t *testing.T) { + originalRate, originalGH := fetchRateLimitForHostFunc, healthRunGH + t.Cleanup(func() { fetchRateLimitForHostFunc, healthRunGH = originalRate, originalGH }) + fetchRateLimitForHostFunc = func(context.Context, string) (rateLimitResource, error) { + return rateLimitResource{Limit: 5000, Remaining: 4000}, nil + } + pages, jobChecks := 0, 0 + healthRunGH = func(_ context.Context, _ string, args ...string) ([]byte, error) { + endpoint := args[3] + switch { + case strings.HasSuffix(endpoint, "/jobs?per_page=1"): + jobChecks++ + assert.Contains(t, endpoint, "/attempts/2/jobs", "check the sampled attempt, not the latest one") + return []byte(`{"total_count":0,"jobs":[]}`), nil + case strings.Contains(endpoint, "/contents/"): + assert.NotContains(t, endpoint, "unexecuted") + return []byte(`# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"settings","body_hash":"prompt"}`), nil + default: + pages++ + count := 10 + if pages == 2 { + count = 5 + assert.Contains(t, endpoint, "page=2") + } + var runs []workflowRunAPIResponse + for i := range count { + run := workflowRunAPIResponse{ID: int64(pages*10 + i), Name: "Example", + Path: ".github/workflows/example.lock.yml", Status: "completed", + Conclusion: "success", Event: "schedule", HeadBranch: "main", + HeadSHA: strings.Repeat("a", 40), RunAttempt: 1} + if pages == 1 { + run.Conclusion, run.HeadSHA, run.RunAttempt = conclusion, "unexecuted", 2 + } + runs = append(runs, run) + } + out, err := json.Marshal(map[string]any{"workflow_runs": runs}) + require.NoError(t, err) + return out, nil + } + } + collection := &HealthCollection{Host: "github.com", repoOverride: "org/repo"} + runs, err := sampleHealthWorkflow(GitHubWorkflow{ID: 1, Name: "Example", + Path: ".github/workflows/example.lock.yml"}, "2026-10-08", "org/repo", false, collection) + require.NoError(t, err) + assert.Equal(t, 2, pages) + assert.Equal(t, 10, jobChecks) + assert.Len(t, runs, 5) + config := HealthConfig{Threshold: 80, LogsDir: t.TempDir(), RepoOverride: "org/repo", collection: collection} + health := calculateCurrentWorkflowHealth("Example", runs, config) + assert.Equal(t, "healthy", health.Status) + assert.InDelta(t, 100.0, health.SuccessRate, 1e-9) + clusters, plan := buildHealthFailureClusters(runs, config) + assert.Empty(t, clusters) + assert.Nil(t, plan) + }) + } +} + +func TestHealthRunHasJobsRequiresConfirmedEvidence(t *testing.T) { + for _, tc := range []struct { + name string + response string + apiError error + wantJobs bool + wantErr string + }{ + {name: "no jobs", response: `{"jobs":[]}`}, + {name: "executed failure", response: `{"jobs":[{"id":42}]}`, wantJobs: true}, + {name: "API unavailable", apiError: errors.New("HTTP 502"), wantErr: "failed to check jobs"}, + {name: "invalid JSON", response: `{`, wantErr: "failed to parse jobs"}, + {name: "missing jobs", response: `{}`, wantErr: "missing jobs array"}, + {name: "null jobs", response: `{"jobs":null}`, wantErr: "missing jobs array"}, + {name: "malformed jobs", response: `{"jobs":{}}`, wantErr: "failed to parse jobs"}, + } { + t.Run(tc.name, func(t *testing.T) { + originalRate, originalGH := fetchRateLimitForHostFunc, healthRunGH + t.Cleanup(func() { fetchRateLimitForHostFunc, healthRunGH = originalRate, originalGH }) + fetchRateLimitForHostFunc = func(context.Context, string) (rateLimitResource, error) { + return rateLimitResource{Limit: 5000, Remaining: 4000}, nil + } + healthRunGH = func(_ context.Context, _ string, args ...string) ([]byte, error) { + assert.Contains(t, args, "github.example.com") + assert.Contains(t, args, "repos/org/repo/actions/runs/42/attempts/2/jobs?per_page=1") + return []byte(tc.response), tc.apiError + } + collection := &HealthCollection{Host: "github.example.com", repoOverride: "github.example.com/org/repo"} + hasJobs, err := healthRunHasJobs(WorkflowRun{DatabaseID: 42, Attempt: 2}, collection) + if tc.wantErr != "" { + require.ErrorContains(t, err, tc.wantErr) + } else { + require.NoError(t, err) + assert.Equal(t, tc.wantJobs, hasJobs) + } + assert.Equal(t, 1, collection.APIRequests) + }) + } +} + +func TestHealthNoJobLookupHonorsRequestBudget(t *testing.T) { + collection := &HealthCollection{Host: "github.com", APIRequests: healthMaximumAPIRequests} + _, err := healthRunHasJobs(WorkflowRun{DatabaseID: 42, Attempt: 1}, collection) + require.ErrorIs(t, err, errHealthAPIBudget) + assert.True(t, collection.Partial) + assert.Equal(t, "request_budget", collection.Reason) +} + +func TestHealthOnlyNoJobFailuresRemainInsufficient(t *testing.T) { + originalRate, originalGH := fetchRateLimitForHostFunc, healthRunGH + t.Cleanup(func() { fetchRateLimitForHostFunc, healthRunGH = originalRate, originalGH }) + fetchRateLimitForHostFunc = func(context.Context, string) (rateLimitResource, error) { + return rateLimitResource{Limit: 5000, Remaining: 4000}, nil + } + healthRunGH = func(_ context.Context, _ string, args ...string) ([]byte, error) { + switch { + case strings.Contains(args[3], "/actions/workflows?"): + return []byte(`{"workflows":[{"id":1,"name":"Example","path":".github/workflows/example.lock.yml","state":"active"}]}`), nil + case strings.Contains(args[3], "/actions/workflows/1/runs?"): + return []byte(`{"workflow_runs":[{"id":42,"name":"Example","status":"completed","conclusion":"failure","run_attempt":1}]}`), nil + default: + assert.Contains(t, args[3], "/runs/42/attempts/1/jobs?") + return []byte(`{"jobs":[]}`), nil + } + } + output := captureAuditGroupStdout(t, func() { + require.NoError(t, RunHealth(HealthConfig{Days: 7, Threshold: 80, JSONOutput: true, RepoOverride: "org/repo"})) + }) + var report HealthSummary + require.NoError(t, json.Unmarshal([]byte(output), &report)) + assert.Zero(t, report.HealthyWorkflows) + assert.Zero(t, report.BelowThreshold) + assert.Equal(t, 1, report.InsufficientWorkflows) + require.Len(t, report.Workflows, 1) + assert.Zero(t, report.Workflows[0].TotalRuns) + assert.Equal(t, "no_recent_runs", report.Workflows[0].Status) + assert.Empty(t, report.FailureClusters) +} diff --git a/pkg/cli/health_sampling.go b/pkg/cli/health_sampling.go index fb2e8daefb7..1693a6ef09d 100644 --- a/pkg/cli/health_sampling.go +++ b/pkg/cli/health_sampling.go @@ -107,11 +107,42 @@ func listHealthWorkflowRuns(opts ListWorkflowRunsOptions, page int, collection * if isIgnoredHealthRun(run) { continue } + if isClusterFailureConclusion(run.Conclusion) { + hasJobs, err := healthRunHasJobs(run, collection) + if err != nil { + return nil, 0, err + } + if !hasJobs { + healthLog.Printf("Ignoring failed run %d with no jobs", run.DatabaseID) + continue + } + } runs = append(runs, run) } return runs, len(data.Runs), nil } +func healthRunHasJobs(run WorkflowRun, collection *HealthCollection) (bool, error) { + endpoint := fmt.Sprintf("%s/actions/runs/%d", healthRepositoryEndpoint(collection.repoOverride), run.DatabaseID) + if run.Attempt > 0 { + endpoint += fmt.Sprintf("/attempts/%d", run.Attempt) + } + out, err := healthAPI(collection, path.Join(endpoint, "jobs")+"?per_page=1") + if err != nil { + return false, fmt.Errorf("failed to check jobs for health run %d: %w", run.DatabaseID, err) + } + var data struct { + Jobs []json.RawMessage `json:"jobs"` + } + if err := json.Unmarshal(out, &data); err != nil { + return false, fmt.Errorf("failed to parse jobs for health run %d: %w", run.DatabaseID, err) + } + if data.Jobs == nil { + return false, fmt.Errorf("failed to parse jobs for health run %d: missing jobs array", run.DatabaseID) + } + return len(data.Jobs) > 0, nil +} + func fetchWorkflowRuns(workflowName, startDate, repoOverride string, verbose bool, collections ...*HealthCollection) ([]WorkflowRun, error) { collection := &HealthCollection{Host: normalizedGitHubAPIHost(logsRateLimitHost(repoOverride)), repoOverride: repoOverride} for _, supplied := range collections { diff --git a/pkg/cli/health_sampling_test.go b/pkg/cli/health_sampling_test.go index 6bd0d31b767..a3d2b7c8306 100644 --- a/pkg/cli/health_sampling_test.go +++ b/pkg/cli/health_sampling_test.go @@ -207,6 +207,8 @@ func TestHealthCurrentSnapshotFromGitHubMetadata(t *testing.T) { out, err := json.Marshal(map[string]any{"workflow_runs": responses}) require.NoError(t, err) return out, nil + case strings.HasSuffix(endpoint, "/jobs?per_page=1"): + return []byte(`{"jobs":[{"id":42}]}`), nil case strings.Contains(endpoint, "/contents/"): body := "fixed-prompt" if strings.Contains(endpoint, "ref="+strings.Repeat("b", 40)) { @@ -238,7 +240,7 @@ func TestHealthCurrentSnapshotFromGitHubMetadata(t *testing.T) { } assert.Empty(t, report.FailureClusters) assert.False(t, report.Collection.Partial) - assert.Equal(t, 5, requests) + assert.Equal(t, 10, requests) }) } } From 47c5ff1816ac35c58cfd9597dd4cad5db041641f Mon Sep 17 00:00:00 2001 From: pelikhan Date: Thu, 15 Oct 2026 21:26:09 -0700 Subject: [PATCH 3/5] fix(health): search deeper past ignored runs Continue bounded health sampling beyond the fixed page cap and roll over GitHub's 1,000-result search windows without skipping tied timestamps. Keep explicit time bounds and API budgets, and report incomplete boundary coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/src/content/docs/setup/cli.md | 2 + pkg/cli/health_pagination.go | 18 +++ pkg/cli/health_pagination_test.go | 169 +++++++++++++++++++++++++++++ pkg/cli/health_sampling.go | 20 +++- 4 files changed, 204 insertions(+), 5 deletions(-) create mode 100644 pkg/cli/health_pagination.go create mode 100644 pkg/cli/health_pagination_test.go diff --git a/docs/src/content/docs/setup/cli.md b/docs/src/content/docs/setup/cli.md index 260b76e2d90..6992bd17aef 100644 --- a/docs/src/content/docs/setup/cli.md +++ b/docs/src/content/docs/setup/cli.md @@ -789,6 +789,8 @@ For release-scoped evidence, pass the release's publication timestamp as `--star Runs awaiting approval (`waiting` status or `action_required` conclusion), skipped runs, and failed runs with no jobs are excluded from health metrics, revision evidence, and failure clusters. Failed runs are checked against the Jobs API for their run attempt, including runs whose approval expired. Collection continues past ignored runs to find executed runs within the requested window and API limits. An unavailable or malformed Jobs API response is reported as an error, not treated as evidence that no jobs ran. +Filtered runs do not consume the minimum sample. Health searches backward for five completed first-attempt runs per current-revision scope, stopping at the requested time boundary or API budget rather than a fixed page count. It narrows the search window to continue beyond GitHub's 1,000-run search limit, overlapping boundary timestamps and deduplicating runs. If timestamp ties prevent further progress, the report explicitly marks collection as partial. + Shows success/failure rates, trend indicators (↑ improving, → stable, ↓ degrading), execution duration, token usage, costs, and warnings when success rate drops below threshold. Runs that never dispatched a job are excluded from these metrics: `skipped` runs (the activation condition evaluated to false, for example a comment that does not contain the workflow's command) and `action_required` runs (created by GitHub but held for manual approval, for example comment events authored by a bot actor). Including them would report command workflows as near-0% success even when every dispatched run succeeded. diff --git a/pkg/cli/health_pagination.go b/pkg/cli/health_pagination.go new file mode 100644 index 00000000000..cdf24bc589c --- /dev/null +++ b/pkg/cli/health_pagination.go @@ -0,0 +1,18 @@ +package cli + +import "time" + +const healthMaximumRunSearchResults = 1000 + +func advanceHealthRunWindow(opts *ListWorkflowRunsOptions, oldest time.Time, collection *HealthCollection) bool { + // Overlap the boundary second so that tied timestamps are not skipped. + before := oldest.Add(time.Second).Format(time.RFC3339) + if oldest.IsZero() || before == opts.BeforeDate { + collection.Partial, collection.Reason = true, "pagination_limit" + collection.Warning = "Health could not advance past the GitHub run-search limit without skipping runs at the timestamp boundary" + healthLog.Printf("%s", collection.Warning) + return false + } + opts.BeforeDate = before + return true +} diff --git a/pkg/cli/health_pagination_test.go b/pkg/cli/health_pagination_test.go new file mode 100644 index 00000000000..d4d2d31af49 --- /dev/null +++ b/pkg/cli/health_pagination_test.go @@ -0,0 +1,169 @@ +//go:build !integration + +package cli + +import ( + "context" + "encoding/json" + "net/url" + "strconv" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestHealthSamplingContinuesPastFilteredPageLimit(t *testing.T) { + original := healthListWorkflowRuns + t.Cleanup(func() { healthListWorkflowRuns = original }) + runs, collection := healthRevisionFixture() + calls := 0 + healthListWorkflowRuns = func(opts ListWorkflowRunsOptions, page int, _ *HealthCollection) ([]WorkflowRun, int, error) { + calls++ + assert.Equal(t, calls, page) + assert.Equal(t, "2026-10-08", opts.StartDate) + if page <= MaxIterations { + return nil, opts.Limit, nil + } + return runs[5:], 5, nil + } + sample, err := sampleHealthWorkflow(GitHubWorkflow{ID: 1, Name: "Example", + Path: runs[0].WorkflowPath}, "2026-10-08", "org/repo", false, collection) + require.NoError(t, err) + assert.Equal(t, MaxIterations+1, calls) + assert.Len(t, sample, healthMinimumRuns) + assert.False(t, collection.Partial) + health := calculateCurrentWorkflowHealth("Example", sample, HealthConfig{Threshold: 80, collection: collection}) + assert.Equal(t, "healthy", health.Status) +} + +func TestHealthSamplingRollsSearchWindowWithoutSkippingTimestampTies(t *testing.T) { + originalRate, originalGH := fetchRateLimitForHostFunc, healthRunGH + t.Cleanup(func() { fetchRateLimitForHostFunc, healthRunGH = originalRate, originalGH }) + fetchRateLimitForHostFunc = func(context.Context, string) (rateLimitResource, error) { + return rateLimitResource{Limit: 5000, Remaining: 4000}, nil + } + end := time.Date(2026, 10, 16, 4, 0, 0, 0, time.UTC) + start := end.Add(-24 * time.Hour).Format(time.RFC3339) + boundary := end.Add(-990 * time.Second) + var responses []workflowRunAPIResponse + for i := range healthMaximumRunSearchResults + healthMinimumRuns - 1 { + created := end.Add(-time.Duration(i) * time.Second) + if i >= 990 { + created = boundary + } + run := workflowRunAPIResponse{ID: int64(i + 1), Name: "Example", + Path: ".github/workflows/example.lock.yml", Status: "completed", + Conclusion: "action_required", Event: "schedule", HeadBranch: "main", + HeadSHA: "unapproved", RunAttempt: 1, CreatedAt: created} + if i >= healthMaximumRunSearchResults-1 { + run.Conclusion, run.HeadSHA = "success", strings.Repeat("a", 40) + } + responses = append(responses, run) + } + rolledPages := []int{} + healthRunGH = func(_ context.Context, _ string, args ...string) ([]byte, error) { + endpoint := args[3] + if strings.Contains(endpoint, "/contents/") { + assert.NotContains(t, endpoint, "unapproved") + return []byte(`# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"settings","body_hash":"prompt"}`), nil + } + parsed, err := url.Parse(endpoint) + require.NoError(t, err) + created := strings.Split(parsed.Query().Get("created"), "..") + require.Len(t, created, 2) + assert.Equal(t, start, created[0]) + upper, err := time.Parse(time.RFC3339, created[1]) + require.NoError(t, err) + assert.False(t, upper.After(end), "the requested end bound must not expand") + page, err := strconv.Atoi(parsed.Query().Get("page")) + require.NoError(t, err) + limit, err := strconv.Atoi(parsed.Query().Get("per_page")) + require.NoError(t, err) + if upper.Equal(boundary) { + rolledPages = append(rolledPages, page) + } + var matching []workflowRunAPIResponse + for _, run := range responses { + if !run.CreatedAt.After(upper) { + matching = append(matching, run) + } + } + first := min((page-1)*limit, len(matching), healthMaximumRunSearchResults) + last := min(first+limit, len(matching), healthMaximumRunSearchResults) + out, err := json.Marshal(map[string]any{"workflow_runs": matching[first:last]}) + require.NoError(t, err) + return out, nil + } + collection := &HealthCollection{Host: "github.com", repoOverride: "org/repo", EndDate: end.Format(time.RFC3339)} + runs, err := sampleHealthWorkflow(GitHubWorkflow{ID: 1, Name: "Example", + Path: ".github/workflows/example.lock.yml"}, start, "org/repo", false, collection) + require.NoError(t, err) + assert.Equal(t, []int{1, 2}, rolledPages) + require.Len(t, runs, healthMinimumRuns) + assert.Equal(t, int64(healthMaximumRunSearchResults), runs[0].DatabaseID) + assert.False(t, collection.Partial) + assert.Equal(t, end.Format(time.RFC3339), collection.EndDate) + health := calculateCurrentWorkflowHealth("Example", runs, HealthConfig{Threshold: 80, collection: collection}) + assert.Equal(t, "healthy", health.Status) +} + +func TestHealthUnadvanceableSearchWindowRemainsExplicitlyPartial(t *testing.T) { + original := healthListWorkflowRuns + t.Cleanup(func() { healthListWorkflowRuns = original }) + tests := []struct { + name string + oldest time.Time + pages int + }{ + {"saturated timestamp", time.Date(2026, 10, 15, 1, 0, 0, 0, time.UTC), 200}, + {"unavailable timestamp", time.Time{}, 100}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + calls := 0 + healthListWorkflowRuns = func(opts ListWorkflowRunsOptions, _ int, _ *HealthCollection) ([]WorkflowRun, int, error) { + calls++ + *opts.OldestFetchedCreatedAt = tt.oldest + return nil, opts.Limit, nil + } + collection := &HealthCollection{} + runs, err := sampleHealthWorkflow(GitHubWorkflow{Name: "Example", + Path: ".github/workflows/example.lock.yml"}, "2026-10-08", "org/repo", false, collection) + require.NoError(t, err) + assert.Empty(t, runs) + assert.Equal(t, tt.pages, calls) + assert.True(t, collection.Partial) + assert.Equal(t, "pagination_limit", collection.Reason) + assert.Contains(t, collection.Warning, "timestamp boundary") + }) + } +} + +func TestHealthFilteredPagesStillRespectRequestBudget(t *testing.T) { + originalRate, originalGH := fetchRateLimitForHostFunc, healthRunGH + t.Cleanup(func() { fetchRateLimitForHostFunc, healthRunGH = originalRate, originalGH }) + fetchRateLimitForHostFunc = func(context.Context, string) (rateLimitResource, error) { + return rateLimitResource{Limit: 5000, Remaining: 4000}, nil + } + calls := 0 + healthRunGH = func(_ context.Context, _ string, _ ...string) ([]byte, error) { + calls++ + var runs []workflowRunAPIResponse + for i := range 2 * healthMinimumRuns { + runs = append(runs, workflowRunAPIResponse{ID: int64(calls*10 + i), Conclusion: "action_required"}) + } + out, err := json.Marshal(map[string]any{"workflow_runs": runs}) + require.NoError(t, err) + return out, nil + } + collection := &HealthCollection{Host: "github.com", APIRequests: healthMaximumAPIRequests - 2} + _, err := sampleHealthWorkflow(GitHubWorkflow{ID: 1, Name: "Example", + Path: ".github/workflows/example.lock.yml"}, "2026-10-08", "org/repo", false, collection) + require.ErrorIs(t, err, errHealthAPIBudget) + assert.Equal(t, 2, calls) + assert.True(t, collection.Partial) + assert.Equal(t, "request_budget", collection.Reason) +} diff --git a/pkg/cli/health_sampling.go b/pkg/cli/health_sampling.go index 1693a6ef09d..390dfc73573 100644 --- a/pkg/cli/health_sampling.go +++ b/pkg/cli/health_sampling.go @@ -90,7 +90,7 @@ func listHealthWorkflowRuns(opts ListWorkflowRunsOptions, page int, collection * if opts.WorkflowName != "" { endpoint = path.Join(healthRepositoryEndpoint(opts.RepoOverride), "actions/workflows", url.PathEscape(opts.WorkflowName), "runs") } - endpoint += fmt.Sprintf("?per_page=%d&page=%d&created=%s", opts.Limit, page, url.QueryEscape(buildCreatedFilter(opts.StartDate, opts.EndDate, ""))) + endpoint += fmt.Sprintf("?per_page=%d&page=%d&created=%s", opts.Limit, page, url.QueryEscape(buildCreatedFilter(opts.StartDate, opts.EndDate, opts.BeforeDate))) out, err := healthAPI(collection, endpoint) if err != nil { return nil, 0, err @@ -102,7 +102,11 @@ func listHealthWorkflowRuns(opts ListWorkflowRunsOptions, page int, collection * return nil, 0, fmt.Errorf("failed to parse health runs: %w", err) } var runs []WorkflowRun + var oldest time.Time for _, response := range data.Runs { + if oldest.IsZero() || response.CreatedAt.Before(oldest) { + oldest = response.CreatedAt + } run := response.workflowRun() if isIgnoredHealthRun(run) { continue @@ -119,6 +123,9 @@ func listHealthWorkflowRuns(opts ListWorkflowRunsOptions, page int, collection * } runs = append(runs, run) } + if opts.OldestFetchedCreatedAt != nil { + *opts.OldestFetchedCreatedAt = oldest + } return runs, len(data.Runs), nil } @@ -179,7 +186,7 @@ func sampleHealthWorkflow(target GitHubWorkflow, startDate, repoOverride string, RepoOverride: repoOverride, Verbose: verbose, OldestFetchedCreatedAt: &oldest} var sample []WorkflowRun seen := make(map[int64]struct{}) - for page := 1; page <= MaxIterations; page++ { + for page := 1; ; page++ { batch, total, err := healthListWorkflowRuns(opts, page, collection) if err != nil { return sample, err @@ -213,8 +220,11 @@ func sampleHealthWorkflow(target GitHubWorkflow, startDate, repoOverride string, if total < opts.Limit || healthWorkflowEvidenceEnough(sample, collection) { return sample, nil } - opts.BeforeDate = oldest.Format(time.RFC3339) + if page*opts.Limit >= healthMaximumRunSearchResults { + if !advanceHealthRunWindow(&opts, oldest, collection) { + return sample, nil + } + page = 0 + } } - collection.Partial, collection.Reason = true, "pagination_limit" - return sample, nil } From 6f303c57557858c387f4b53fa6a3e1d0da6282ad Mon Sep 17 00:00:00 2001 From: pelikhan Date: Thu, 15 Oct 2026 22:28:59 -0700 Subject: [PATCH 4/5] test: align health filtered-batch pagination contract Assert numbered pages keep the search window stable until rollover, while preserving coverage that filtered batches do not hide older dispatched runs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/cli/logs_non_dispatched_runs_test.go | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/pkg/cli/logs_non_dispatched_runs_test.go b/pkg/cli/logs_non_dispatched_runs_test.go index 101f1f68b78..2017e8a77da 100644 --- a/pkg/cli/logs_non_dispatched_runs_test.go +++ b/pkg/cli/logs_non_dispatched_runs_test.go @@ -92,16 +92,19 @@ func TestCalculateWorkflowHealthOnDispatchedRuns(t *testing.T) { } // TestFetchWorkflowRunsPaginatesPastFilteredBatches verifies that a batch made -// entirely of non-dispatched runs does not terminate pagination: the cursor is -// advanced from the raw batch so older dispatched runs are still collected. +// entirely of non-dispatched runs does not terminate pagination: numbered pages +// advance within the same search window so older dispatched runs are collected. func TestFetchWorkflowRunsPaginatesPastFilteredBatches(t *testing.T) { original := healthListWorkflowRuns t.Cleanup(func() { healthListWorkflowRuns = original }) - now := time.Now() - var calls []string + now := time.Date(2026, 8, 2, 12, 0, 0, 0, time.UTC) + var calls []int healthListWorkflowRuns = func(opts ListWorkflowRunsOptions, page int, collection *HealthCollection) ([]WorkflowRun, int, error) { - calls = append(calls, opts.BeforeDate) + calls = append(calls, page) + if opts.BeforeDate != "" { + t.Fatalf("BeforeDate = %q, want unchanged search window before the search-result limit", opts.BeforeDate) + } if opts.OldestFetchedCreatedAt == nil { t.Fatal("fetchWorkflowRuns must request the oldest raw run timestamp for pagination") } @@ -126,8 +129,11 @@ func TestFetchWorkflowRunsPaginatesPastFilteredBatches(t *testing.T) { if len(calls) != 2 { t.Fatalf("len(calls) = %d, want 2 batches", len(calls)) } - if calls[1] == "" { - t.Error("second batch must carry a BeforeDate cursor derived from the raw batch") + if calls[0] != 1 || calls[1] != 2 { + t.Errorf("pages = %v, want [1 2]", calls) + } + if runs[0].Conclusion != "success" || !runs[0].CreatedAt.Equal(now.Add(-2*time.Hour)) { + t.Errorf("run = %+v, want the older successful run", runs[0]) } } From e57343550b013b8f1bf7ad43aacb8aaa21f89bfd Mon Sep 17 00:00:00 2001 From: pelikhan Date: Thu, 15 Oct 2026 22:42:01 -0700 Subject: [PATCH 5/5] fix(health): preserve waiting evidence and reuse job probes Confirm waiting attempts have no jobs before excluding them, cache successful job-presence results per run and attempt across overlap, and document bounded evidence sampling in ADR-69225. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ...nore-unexecuted-runs-in-health-sampling.md | 55 +++++++++ docs/src/content/docs/setup/cli.md | 2 +- pkg/cli/health_collection.go | 1 + pkg/cli/health_command.go | 2 +- pkg/cli/health_metrics.go | 2 +- pkg/cli/health_no_jobs_test.go | 107 ++++++++++++++++++ pkg/cli/health_pagination_test.go | 68 +++++++++++ pkg/cli/health_revisions_test.go | 24 +++- pkg/cli/health_sampling.go | 20 +++- pkg/cli/health_sampling_test.go | 8 +- 10 files changed, 280 insertions(+), 9 deletions(-) create mode 100644 docs/adr/69225-ignore-unexecuted-runs-in-health-sampling.md diff --git a/docs/adr/69225-ignore-unexecuted-runs-in-health-sampling.md b/docs/adr/69225-ignore-unexecuted-runs-in-health-sampling.md new file mode 100644 index 00000000000..29fbf14c861 --- /dev/null +++ b/docs/adr/69225-ignore-unexecuted-runs-in-health-sampling.md @@ -0,0 +1,55 @@ +# ADR-69225: Ignore Unexecuted Workflow Runs in Health Sampling + +**Date**: 2026-10-15 +**Status**: Proposed (PR #69225 is not yet accepted) +**Deciders**: Not recorded; pending maintainer review of PR #69225. This proposal does not establish approval. +**Author**: pelikhan (PR author) + +## Context + +`gh aw health` samples recent runs for the current workflow revision within a resolved time window. Runs that never execute can occupy the sample before older executions are reached, and a failed run with no jobs can look like an execution failure. Skipped and approval-gated runs therefore distort execution-health results if they count toward either the sample limit or failure metrics. + +Filtering only the first API page is insufficient: an arbitrarily long sequence of ignored runs can hide eligible executions. GitHub's filtered workflow-run API also limits a search window to 1,000 results. Health must search farther back without crossing explicit date bounds, claiming exhaustive coverage, losing runs at timestamp boundaries, or treating an unavailable jobs response as evidence that no jobs ran. + +## Decision + +The proposal in PR #69225 is to apply one eligibility policy before consuming the per-workflow sample quota and when calculating health metrics: + +- Ignore skipped runs and runs explicitly marked `action_required`. +- Treat `waiting` as ambiguous. Ignore it only after a successful jobs query confirms that the run attempt has no jobs; retain waiting runs with jobs as pending work. +- Ignore failed runs only when a successful jobs query confirms zero jobs. A failed run with jobs remains eligible, even when an early job failed before the agent started. +- Keep ignored runs out of execution-health totals, failure rates, and the requested eligible-run count. Continue applying current-revision selection rather than mixing historical revisions to fill the sample. + +Fetch additional pages until enough eligible runs are collected, the available history within the resolved time window is exhausted, or a documented API/search bound prevents further progress. Keep the resolved start and end bounds fixed for the collection. + +When a search window reaches the 1,000-result limit, continue with an older window that overlaps the oldest returned timestamp. Deduplicate runs across the overlap so a run cannot consume the quota twice. If the boundary timestamp is itself saturated and the search cannot advance without potentially omitting runs, stop and report partial coverage rather than silently skipping that timestamp or looping. + +Cache successful jobs-presence results by run ID and run attempt during collection, so overlapping windows do not repeat successful probes or reuse evidence from a different attempt. Jobs API errors must remain explicit collection errors or partial-coverage diagnostics, not become cached zero-job results. Neither filtering nor sampling authorizes reruns or bypasses approval gates. + +This approach favors execution evidence over interpreting ambiguous status names, while keeping collection bounded and its uncertainty visible. + +## Alternatives Considered + +### Count every returned run + +This preserves the simplest pagination and existing status-based totals, but measures scheduling and approval activity as execution health. Recent skipped or unexecuted runs can displace the executions the command is intended to assess. + +### Filter only the initial page or increase its fixed size + +This reduces API calls but leaves the result dependent on how many ignored runs happen to precede eligible runs. Increasing a fixed fetch limit does not address the 1,000-result search-window ceiling. + +### Ignore every waiting or failed run without querying jobs + +This avoids extra API requests but hides genuine pending executions and real failures. Conversely, querying jobs for every run adds avoidable latency; probe only cases requiring execution evidence and cache successful results. + +### Exhaustively enumerate history or skip overlapping timestamps + +Unbounded enumeration conflicts with the explicit time window and bounded sample contract. Advancing past a saturated timestamp makes progress but can silently omit eligible runs; explicit partial coverage is preferable to an unsupported completeness claim. + +## Consequences + +Eligible execution samples are less sensitive to approval and scheduling noise, and confirmed zero-job failures no longer inflate execution-failure rates. Genuine failures and waiting runs with jobs remain visible. + +Sampling may require more API requests and latency when many runs are ignored. Attempt-scoped caching limits repeated probes, but permission failures and API errors still require visible diagnostics. Timestamp saturation can leave a sample incomplete, and reports must preserve that limitation rather than imply healthy or exhaustive results. + +These changes alter the meaning of health totals without changing workflow execution, approval policy, or the resolved time window. Regression coverage must distinguish ambiguous waiting states, failed runs with and without jobs, probe errors, overlapping windows, rerun attempts, and saturated timestamp boundaries. diff --git a/docs/src/content/docs/setup/cli.md b/docs/src/content/docs/setup/cli.md index 6992bd17aef..c7b4d2e8252 100644 --- a/docs/src/content/docs/setup/cli.md +++ b/docs/src/content/docs/setup/cli.md @@ -787,7 +787,7 @@ The `--days` flag accepts 7, 30, or 90 (default: 7). Other values produce an err For release-scoped evidence, pass the release's publication timestamp as `--start-date`; this filters by time, not compiler version. Text output and JSON `collection.start_date` / `collection.end_date` show the resolved UTC window. Health still samples current workflow revisions toward five completed first-attempt runs per scope, subject to API limits; it does not enumerate every run or reconstruct historical branch heads. A narrow window can leave insufficient evidence. -Runs awaiting approval (`waiting` status or `action_required` conclusion), skipped runs, and failed runs with no jobs are excluded from health metrics, revision evidence, and failure clusters. Failed runs are checked against the Jobs API for their run attempt, including runs whose approval expired. Collection continues past ignored runs to find executed runs within the requested window and API limits. An unavailable or malformed Jobs API response is reported as an error, not treated as evidence that no jobs ran. +Runs with `skipped` or `action_required` conclusions are excluded from health metrics, revision evidence, and failure clusters. Waiting and failed runs are excluded only when the Jobs API confirms their run attempt has no jobs, including runs whose approval expired. Job-backed waiting runs remain pending revision evidence. Successful job-presence lookups are reused per run ID and attempt within the collection, including across overlapping search windows. Collection continues past ignored runs to find executed runs within the requested window and API limits. An unavailable or malformed Jobs API response is reported as an error, not treated as evidence that no jobs ran. Filtered runs do not consume the minimum sample. Health searches backward for five completed first-attempt runs per current-revision scope, stopping at the requested time boundary or API budget rather than a fixed page count. It narrows the search window to continue beyond GitHub's 1,000-run search limit, overlapping boundary timestamps and deduplicating runs. If timestamp ties prevent further progress, the report explicitly marks collection as partial. diff --git a/pkg/cli/health_collection.go b/pkg/cli/health_collection.go index 4659df34cb7..0ff2508bdfb 100644 --- a/pkg/cli/health_collection.go +++ b/pkg/cli/health_collection.go @@ -32,6 +32,7 @@ type HealthCollection struct { repoOverride string stopped bool visited map[string]struct{} + jobPresence map[healthRunAttempt]bool } const healthMinimumRuns = 5 diff --git a/pkg/cli/health_command.go b/pkg/cli/health_command.go index 6ef93e6365d..04d9a1b55c0 100644 --- a/pkg/cli/health_command.go +++ b/pkg/cli/health_command.go @@ -57,7 +57,7 @@ Use --start-date and --end-date to bound run creation times. Accepts UTC dates, RFC3339 timestamps, or relative times such as -24h and -1w. An explicit start replaces --days; otherwise --days counts backward from --end-date (default: now). The report remains a bounded current-revision sample, not an exhaustive history. -Runs awaiting approval (waiting or action_required), skipped runs, and failed runs with no jobs are ignored. +Skipped/action_required runs and confirmed jobless waiting/failed attempts are ignored; job-backed waiting runs remain pending evidence. ` + WorkflowIDExplanation, Example: ` ` + string(constants.CLIExtensionPrefix) + ` health # Current status of all workflows diff --git a/pkg/cli/health_metrics.go b/pkg/cli/health_metrics.go index 4e98a4b5b18..bc750565349 100644 --- a/pkg/cli/health_metrics.go +++ b/pkg/cli/health_metrics.go @@ -105,7 +105,7 @@ func CalculateWorkflowHealth(workflowName string, runs []WorkflowRun, threshold } func isIgnoredHealthRun(run WorkflowRun) bool { - return run.Status == "waiting" || isNonDispatchedConclusion(run.Conclusion) + return isNonDispatchedConclusion(run.Conclusion) } func accumulateWorkflowHealth(runs []WorkflowRun) WorkflowHealth { diff --git a/pkg/cli/health_no_jobs_test.go b/pkg/cli/health_no_jobs_test.go index fead1afdab7..474fa555ced 100644 --- a/pkg/cli/health_no_jobs_test.go +++ b/pkg/cli/health_no_jobs_test.go @@ -109,6 +109,113 @@ func TestHealthRunHasJobsRequiresConfirmedEvidence(t *testing.T) { assert.Equal(t, tc.wantJobs, hasJobs) } assert.Equal(t, 1, collection.APIRequests) + if tc.wantErr != "" { + healthRunGH = func(_ context.Context, _ string, _ ...string) ([]byte, error) { + return []byte(`{"jobs":[{"id":42}]}`), nil + } + hasJobs, err = healthRunHasJobs(WorkflowRun{DatabaseID: 42, Attempt: 2}, collection) + require.NoError(t, err) + assert.True(t, hasJobs, "an unsuccessful lookup must not be cached as no jobs") + assert.Equal(t, 2, collection.APIRequests) + } + }) + } +} + +func TestHealthJobPresenceCacheIsPerAttemptAndCollection(t *testing.T) { + originalRate, originalGH := fetchRateLimitForHostFunc, healthRunGH + t.Cleanup(func() { fetchRateLimitForHostFunc, healthRunGH = originalRate, originalGH }) + fetchRateLimitForHostFunc = func(context.Context, string) (rateLimitResource, error) { + return rateLimitResource{Limit: 5000, Remaining: 4000}, nil + } + calls := make(map[string]int) + healthRunGH = func(_ context.Context, _ string, args ...string) ([]byte, error) { + assert.Contains(t, args, "github.example.com") + endpoint := args[3] + calls[endpoint]++ + if strings.Contains(endpoint, "/attempts/2/") { + return []byte(`{"jobs":[{"id":42}]}`), nil + } + return []byte(`{"jobs":[]}`), nil + } + for range 2 { + collection := &HealthCollection{Host: "github.example.com", repoOverride: "github.example.com/org/repo"} + for _, attempt := range []int{0, 1, 2} { + for range 2 { + hasJobs, err := healthRunHasJobs(WorkflowRun{DatabaseID: 42, Attempt: attempt}, collection) + require.NoError(t, err) + assert.Equal(t, attempt == 2, hasJobs) + } + } + assert.Equal(t, 3, collection.APIRequests, "cache both presence and absence within one collection") + } + assert.Equal(t, map[string]int{ + "repos/org/repo/actions/runs/42/jobs?per_page=1": 2, + "repos/org/repo/actions/runs/42/attempts/1/jobs?per_page=1": 2, + "repos/org/repo/actions/runs/42/attempts/2/jobs?per_page=1": 2, + }, calls, "each new collection must check jobs again") +} + +func TestHealthSamplingWaitingJobsPreservePendingRevision(t *testing.T) { + for _, hasJobs := range []bool{false, true} { + t.Run(map[bool]string{false: "approval-only", true: "job-backed"}[hasJobs], func(t *testing.T) { + originalRate, originalGH := fetchRateLimitForHostFunc, healthRunGH + t.Cleanup(func() { fetchRateLimitForHostFunc, healthRunGH = originalRate, originalGH }) + fetchRateLimitForHostFunc = func(context.Context, string) (rateLimitResource, error) { + return rateLimitResource{Limit: 5000, Remaining: 4000}, nil + } + jobChecks := 0 + healthRunGH = func(_ context.Context, _ string, args ...string) ([]byte, error) { + endpoint := args[3] + switch { + case strings.HasSuffix(endpoint, "/jobs?per_page=1"): + jobChecks++ + assert.Contains(t, endpoint, "/runs/11/attempts/2/jobs?") + if hasJobs { + return []byte(`{"jobs":[{"id":42}]}`), nil + } + return []byte(`{"jobs":[]}`), nil + case strings.Contains(endpoint, "/contents/"): + body := "new-prompt" + if strings.Contains(endpoint, "ref="+strings.Repeat("b", 40)) { + body = "old-prompt" + } + return []byte(`# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"settings","body_hash":"` + body + `"}`), nil + default: + var runs []workflowRunAPIResponse + for i := range 6 { + run := workflowRunAPIResponse{ID: int64(11 - i), Name: "Example", + Path: ".github/workflows/example.lock.yml", Status: "completed", Conclusion: "success", + Event: "schedule", HeadBranch: "main", HeadSHA: strings.Repeat("b", 40), RunAttempt: 1} + if i == 0 { + run.Status, run.Conclusion, run.HeadSHA, run.RunAttempt = "waiting", "", strings.Repeat("a", 40), 2 + } + runs = append(runs, run) + } + out, err := json.Marshal(map[string]any{"workflow_runs": runs}) + require.NoError(t, err) + return out, nil + } + } + collection := &HealthCollection{Host: "github.com", repoOverride: "org/repo"} + runs, err := sampleHealthWorkflow(GitHubWorkflow{ID: 1, Name: "Example", + Path: ".github/workflows/example.lock.yml"}, "2026-10-08", "org/repo", false, collection) + require.NoError(t, err) + assert.Equal(t, 1, jobChecks) + health := calculateCurrentWorkflowHealth("Example", runs, HealthConfig{Threshold: 80, collection: collection}) + require.Len(t, health.Revisions, 1) + if hasJobs { + assert.Len(t, runs, 6) + assert.Equal(t, "insufficient_evidence", health.Status) + assert.Equal(t, 1, health.Revisions[0].Current.Pending) + assert.Equal(t, []int64{11}, health.Revisions[0].Current.RunIDs) + } else { + assert.Len(t, runs, 5) + assert.Equal(t, "no_current_revision_runs", health.Status) + assert.Zero(t, health.Revisions[0].Current.Pending) + assert.Empty(t, health.Revisions[0].Current.RunIDs) + } + assert.False(t, collection.Partial) }) } } diff --git a/pkg/cli/health_pagination_test.go b/pkg/cli/health_pagination_test.go index d4d2d31af49..43a2a2b5503 100644 --- a/pkg/cli/health_pagination_test.go +++ b/pkg/cli/health_pagination_test.go @@ -142,6 +142,74 @@ func TestHealthUnadvanceableSearchWindowRemainsExplicitlyPartial(t *testing.T) { } } +func TestHealthSamplingReusesJobProbesAcrossOverlappingWindows(t *testing.T) { + originalRate, originalGH := fetchRateLimitForHostFunc, healthRunGH + t.Cleanup(func() { fetchRateLimitForHostFunc, healthRunGH = originalRate, originalGH }) + fetchRateLimitForHostFunc = func(context.Context, string) (rateLimitResource, error) { + return rateLimitResource{Limit: 5000, Remaining: 4000}, nil + } + end := time.Date(2026, 10, 16, 4, 0, 0, 0, time.UTC) + boundary := end.Add(-100 * time.Second) + jobChecks := make(map[string]int) + rolledPages := []int{} + healthRunGH = func(_ context.Context, _ string, args ...string) ([]byte, error) { + endpoint := args[3] + switch { + case strings.HasSuffix(endpoint, "/jobs?per_page=1"): + jobChecks[endpoint]++ + assert.Contains(t, endpoint, "/attempts/2/jobs") + if strings.Contains(endpoint, "/runs/1000/") { + return []byte(`{"jobs":[{"id":42}]}`), nil + } + return []byte(`{"jobs":[]}`), nil + case strings.Contains(endpoint, "/contents/"): + return []byte(`# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"settings","body_hash":"prompt"}`), nil + } + parsed, err := url.Parse(endpoint) + require.NoError(t, err) + page, err := strconv.Atoi(parsed.Query().Get("page")) + require.NoError(t, err) + rolled := strings.HasSuffix(parsed.Query().Get("created"), ".."+boundary.Format(time.RFC3339)) + if rolled { + rolledPages = append(rolledPages, page) + } + count := 2 * healthMinimumRuns + if rolled && page == 2 { + count = healthMinimumRuns + } + var responses []workflowRunAPIResponse + for i := range count { + run := workflowRunAPIResponse{ID: int64(page*10 + i), Name: "Example", + Path: ".github/workflows/example.lock.yml", Status: "completed", Conclusion: "action_required", + Event: "schedule", HeadBranch: "main", HeadSHA: strings.Repeat("a", 40), RunAttempt: 1, + CreatedAt: end.Add(-time.Duration(page) * time.Second)} + if page == 100 || rolled { + run.ID, run.Conclusion, run.RunAttempt, run.CreatedAt = int64(1000+i), "failure", 2, boundary + } + if rolled && page == 2 { + run.ID, run.Conclusion, run.RunAttempt = int64(2000+i), "success", 1 + } + responses = append(responses, run) + } + out, err := json.Marshal(map[string]any{"workflow_runs": responses}) + require.NoError(t, err) + return out, nil + } + collection := &HealthCollection{Host: "github.com", repoOverride: "org/repo", EndDate: end.Format(time.RFC3339)} + runs, err := sampleHealthWorkflow(GitHubWorkflow{ID: 1, Name: "Example", + Path: ".github/workflows/example.lock.yml"}, end.Add(-time.Hour).Format(time.RFC3339), "org/repo", false, collection) + require.NoError(t, err) + assert.Equal(t, []int{1, 2}, rolledPages) + require.Len(t, runs, healthMinimumRuns+1) + assert.Equal(t, int64(1000), runs[0].DatabaseID, "retain job-backed failure only once") + require.Len(t, jobChecks, 10) + for endpoint, checks := range jobChecks { + assert.Equal(t, 1, checks, "overlap must reuse both empty and nonempty jobs responses: %s", endpoint) + } + assert.False(t, collection.Partial) + assert.Equal(t, 114, collection.APIRequests, "102 run pages, 10 jobs probes, 2 revision lookups") +} + func TestHealthFilteredPagesStillRespectRequestBudget(t *testing.T) { originalRate, originalGH := fetchRateLimitForHostFunc, healthRunGH t.Cleanup(func() { fetchRateLimitForHostFunc, healthRunGH = originalRate, originalGH }) diff --git a/pkg/cli/health_revisions_test.go b/pkg/cli/health_revisions_test.go index 5f2c7275f67..43c042a1e68 100644 --- a/pkg/cli/health_revisions_test.go +++ b/pkg/cli/health_revisions_test.go @@ -51,9 +51,9 @@ func TestHealthRevisionRecoveryAndCurrentClusters(t *testing.T) { assert.Equal(t, health, calculateCurrentWorkflowHealth("Example", runs, config)) } -func TestHealthIgnoresRunsAwaitingApproval(t *testing.T) { +func TestHealthIgnoresNonDispatchedConclusions(t *testing.T) { for _, blocked := range []WorkflowRun{ - {Status: "waiting"}, + {Status: "waiting", Conclusion: "action_required"}, {Status: "completed", Conclusion: "action_required"}, {Status: "completed", Conclusion: "skipped"}, } { @@ -76,6 +76,26 @@ func TestHealthIgnoresRunsAwaitingApproval(t *testing.T) { } } +func TestHealthRetainsAmbiguousWaitingEvidenceWithoutAPI(t *testing.T) { + runs, collection := healthRevisionFixture() + waiting := runs[len(runs)-1] + waiting.DatabaseID, waiting.HeadSha = 11, "pending" + waiting.Status, waiting.Conclusion = "waiting", "" + waiting.CreatedAt = waiting.CreatedAt.Add(time.Hour) + collection.revisions[healthRevisionLookupKey(waiting)] = &workflow.LockMetadata{ + FrontmatterHash: "settings", BodyHash: "new-prompt", + } + runs = append([]WorkflowRun{waiting}, runs...) + health := calculateCurrentWorkflowHealth("Example", runs, HealthConfig{Threshold: 80, collection: collection}) + assert.Equal(t, "insufficient_evidence", health.Status) + require.Len(t, health.Revisions, 1) + assert.Equal(t, 1, health.Revisions[0].Current.Pending) + assert.Equal(t, []int64{11}, health.Revisions[0].Current.RunIDs) + metrics := CalculateWorkflowHealth("Example", runs, 80) + assert.Equal(t, 11, metrics.TotalRuns, "waiting alone cannot prove a run never executed") + assert.Equal(t, waiting, runs[0], "health calculation must not mutate caller data") +} + func TestHealthRevisionsDoNotClaimUnsupportedRecovery(t *testing.T) { for _, scenario := range []string{"same-content", "one-success", "retry-only", "unknown-revision", "still-failing", "overlapping-runs", "other-branch", "other-event"} { t.Run(scenario, func(t *testing.T) { diff --git a/pkg/cli/health_sampling.go b/pkg/cli/health_sampling.go index 390dfc73573..e97a9ea42c8 100644 --- a/pkg/cli/health_sampling.go +++ b/pkg/cli/health_sampling.go @@ -111,13 +111,13 @@ func listHealthWorkflowRuns(opts ListWorkflowRunsOptions, page int, collection * if isIgnoredHealthRun(run) { continue } - if isClusterFailureConclusion(run.Conclusion) { + if run.Status == "waiting" || isClusterFailureConclusion(run.Conclusion) { hasJobs, err := healthRunHasJobs(run, collection) if err != nil { return nil, 0, err } if !hasJobs { - healthLog.Printf("Ignoring failed run %d with no jobs", run.DatabaseID) + healthLog.Printf("Ignoring run %d with no jobs", run.DatabaseID) continue } } @@ -129,7 +129,16 @@ func listHealthWorkflowRuns(opts ListWorkflowRunsOptions, page int, collection * return runs, len(data.Runs), nil } +type healthRunAttempt struct { + id int64 + attempt int +} + func healthRunHasJobs(run WorkflowRun, collection *HealthCollection) (bool, error) { + key := healthRunAttempt{id: run.DatabaseID, attempt: run.Attempt} + if hasJobs, checked := collection.jobPresence[key]; checked { + return hasJobs, nil + } endpoint := fmt.Sprintf("%s/actions/runs/%d", healthRepositoryEndpoint(collection.repoOverride), run.DatabaseID) if run.Attempt > 0 { endpoint += fmt.Sprintf("/attempts/%d", run.Attempt) @@ -147,7 +156,12 @@ func healthRunHasJobs(run WorkflowRun, collection *HealthCollection) (bool, erro if data.Jobs == nil { return false, fmt.Errorf("failed to parse jobs for health run %d: missing jobs array", run.DatabaseID) } - return len(data.Jobs) > 0, nil + if collection.jobPresence == nil { + collection.jobPresence = make(map[healthRunAttempt]bool) + } + hasJobs := len(data.Jobs) > 0 + collection.jobPresence[key] = hasJobs + return hasJobs, nil } func fetchWorkflowRuns(workflowName, startDate, repoOverride string, verbose bool, collections ...*HealthCollection) ([]WorkflowRun, error) { diff --git a/pkg/cli/health_sampling_test.go b/pkg/cli/health_sampling_test.go index a3d2b7c8306..43dc4526197 100644 --- a/pkg/cli/health_sampling_test.go +++ b/pkg/cli/health_sampling_test.go @@ -43,9 +43,14 @@ func TestHealthSamplingContinuesPastRunsAwaitingApproval(t *testing.T) { fetchRateLimitForHostFunc = func(context.Context, string) (rateLimitResource, error) { return rateLimitResource{Limit: 5000, Remaining: 4000}, nil } - pages := 0 + pages, jobChecks := 0, 0 healthRunGH = func(_ context.Context, _ string, args ...string) ([]byte, error) { endpoint := args[3] + if strings.HasSuffix(endpoint, "/jobs?per_page=1") { + jobChecks++ + assert.Contains(t, endpoint, "/attempts/1/jobs") + return []byte(`{"jobs":[]}`), nil + } if strings.Contains(endpoint, "/contents/") { assert.NotContains(t, endpoint, "unapproved", "ignored runs need no revision lookups") return []byte(`# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"settings","body_hash":"prompt"}`), nil @@ -81,6 +86,7 @@ func TestHealthSamplingContinuesPastRunsAwaitingApproval(t *testing.T) { Path: ".github/workflows/example.lock.yml"}, "2026-10-08", "org/repo", false, collection) require.NoError(t, err) assert.Equal(t, 2, pages) + assert.Equal(t, 5, jobChecks, "only ambiguous waiting runs need jobs lookups") assert.Len(t, runs, 5) health := calculateCurrentWorkflowHealth("Example", runs, HealthConfig{Threshold: 80, collection: collection}) assert.Equal(t, "healthy", health.Status)