From 9d6f230797382133c43f0eec0c2e7414211ae5d5 Mon Sep 17 00:00:00 2001 From: Julian Blacher Date: Sat, 24 Oct 2026 15:40:20 -0300 Subject: [PATCH 1/5] feat: add `prw checks` to list every check of a PR `prw checks ` shows each check run and commit status of the PR's head commit with its result, how long it took and a link: failures first, then cancelled and running ones, then passed, neutral and skipped. Superseded runs of the same check are left out, as they don't count towards the result. The header has the short SHA, overall state, counts per bucket and merge state. `--json` prints the same as an object with a `checks` array. Exit codes match `prw wait` (0 passed, 1 failed, 3 error, 4 no CI), plus 8 while checks are still running, like `gh pr checks`. Verified live on devblac/docker-lint#5, zephyrproject-rtos/zephyr#105183 (35 checks; the cancelled run superseded by a newer one is hidden) and devblac/prw#13. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + README.md | 19 +++ TASKS.md | 4 +- cmd/prw/checks.go | 294 ++++++++++++++++++++++++++++++++++++++ cmd/prw/checks_test.go | 163 +++++++++++++++++++++ internal/github/client.go | 11 +- 6 files changed, 487 insertions(+), 5 deletions(-) create mode 100644 cmd/prw/checks.go create mode 100644 cmd/prw/checks_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ebb476..96b8283 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `prw run` now notifies once (`merged` or `closed`) and automatically stops watching a PR once it is merged or closed, instead of polling it forever. This notification always fires, regardless of `--on`/`notification_filter`. ### Added +- `prw checks `: every check run and commit status of a PR's head commit with its result, duration and link, failures first; `--json` for scripts. Superseded runs are left out. Exits `0` passed, `1` failed, `3` error, `4` no CI, `8` still running. - Review and merge events: `prw run` notifies when a PR is approved, gets changes requested, runs into merge conflicts, or becomes ready to merge after branch protection was holding it back. Shown in console, native, Slack/Discord and generic webhooks (`type` `pr_review_change` / `pr_merge_state_change`), filtered by `--on` (`fail`: changes requested and conflicts; `success`: approved and ready). `prw list` gains `REVIEW` and `MERGE` columns (`review` / `merge` in `--json` and MCP `list_watched`); `prw wait --json` and the MCP tools report `merge_state`. Disable with `prw config set pr_events false`. - Auto-watch: `prw watch --mine` follows all your open PRs and `prw watch --review-requested` the PRs where your review is requested, across every repository. Matches are added immediately; `prw run` refreshes the searches every 5 minutes. Review-request PRs are dropped when the request goes away; PRs watched by hand are never touched by it. `prw unwatch --mine` / `--review-requested` turns it off and removes the PRs it added. `prw list` gains an `ADDED BY` column (and `source` in `--json`); `prw config show` shows `auto_watch`. - `prw mcp`: a Model Context Protocol server over stdio for AI coding agents, with tools `get_ci_status`, `wait_for_ci` (progress notifications, cancellation, fail-fast), `list_watched`, `watch_pr` and `unwatch_pr`. Implemented without new dependencies (the official Go SDK needs Go 1.25). diff --git a/README.md b/README.md index 76f651b..767b2c2 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,7 @@ The tool is deliberately minimal. It does one thing well: watch PRs and tell you ## Features +- **`prw checks`**: every check of a PR with its result, duration and link - **`prw wait`**: block until a PR's CI finishes and exit 0/1/2 — for scripts, pipelines and AI coding agents - **MCP server** (`prw mcp`): lets Claude Code, Claude Desktop, Cursor and other agents check and wait for PR CI as tools - **Auto-watch**: `prw watch --mine` / `--review-requested` follow your PRs and review requests across all repositories, no URLs needed @@ -266,6 +267,24 @@ With `--json`, stdout is a single object: **PRs without CI.** Some PRs never get a single check — the repository has no CI, or none runs for the PR's base branch (for example a stacked PR whose base isn't `main`). Right after a push, though, every PR briefly has zero checks while CI registers them. So prw only concludes `no_ci` once the PR has had no checks or statuses at all **and** hasn't been updated for 3 minutes; until then it reports `pending`. `no_ci` exits with `4`, not `0`, so `prw wait && deploy` doesn't deploy a PR that no CI looked at. `prw run` shows `no_ci` in `prw list` but doesn't send a notification for it, and notifies normally if CI shows up later (e.g. after the PR is retargeted). +## Seeing every check: `prw checks` + +`prw checks` lists each check of a PR's head commit (GitHub Actions jobs, other check runs and legacy commit statuses) with its result, how long it took and a link to it — failures first, then running ones. The PR does not need to be on the watch list. + +``` +$ prw checks https://github.com/owner/repo/pull/123 +owner/repo#123 — Add feature +abc1234 · failure · 1 fail, 1 pending, 3 pass · merge: blocked + +❌ fail Test (ubuntu-latest) 4m02s https://github.com/owner/repo/actions/runs/…/job/… +⏳ pending E2E 1m10s… https://github.com/owner/repo/actions/runs/…/job/… +✅ pass Build 45s https://github.com/owner/repo/actions/runs/…/job/… +✅ pass Lint 21s https://github.com/owner/repo/actions/runs/…/job/… +✅ pass ci/jenkins https://jenkins.example.com/job/… +``` + +Runs superseded by a newer run of the same check (e.g. cancelled by `concurrency: cancel-in-progress`) are left out, as they don't count. `--json` prints the same as an object with a `checks` array (`name`, `bucket`, `state`, `kind`, `url`, `started_at`, `completed_at`). Exit codes match `prw wait` — `0` passed, `1` failed, `3` error, `4` no CI — plus `8` while checks are still running. + ## Using prw from AI agents: `prw mcp` `prw mcp` runs a [Model Context Protocol](https://modelcontextprotocol.io) server over stdio, so coding agents can check a PR's CI and wait for it after pushing — and read which checks failed — without screen-scraping `gh` output. diff --git a/TASKS.md b/TASKS.md index 2880380..e8673ed 100644 --- a/TASKS.md +++ b/TASKS.md @@ -92,7 +92,9 @@ Legend — Impact: 1–5 · Effort: S (≤½ day), M (1–2 days), L (3+ days) - ✅ #18 `no_ci` state — done (0 checks + PR not updated for 3 min → `no_ci`; `prw wait` exits 4; no notification; verified live: 11 real PRs went from `pending` to `no_ci`, `prw wait` on one returns in 2s instead of the 30-min timeout) - ✅ Codex review feedback on #6, #7, #13–#17 — done in `fix/codex-review`: superseded check runs (verified live on zephyrproject-rtos/zephyr#105183: was `failure`, now `success`), head re-check in `prw wait`, context propagation into every API request, closed review-request PRs keep their notification, no `no_ci` flapping, ETag cache eviction, GovSlack. - ✅ #11 Review & mergeability events — done (approved / changes_requested / conflict / ready; verified live: devblac/prw#12 `ready`, cli/cli#14727 `blocked`, a simulated blocked → ready notified once and ready → blocked stayed silent) -- Next: #12 `prw checks ` +- ✅ Codex follow-ups on #18/#19 — failed head re-check no longer accepted as "unchanged"; review-request PRs only dropped when proven withdrawn (open or 404), never on a cancelled or failed lookup; all review pages read. +- ✅ #12 `prw checks ` — done (buckets fail/cancel/pending/pass/neutral/skipping, durations, links, `--json`, exit 8 while running; verified live on devblac/docker-lint#5, zephyrproject-rtos/zephyr#105183 — 35 checks, superseded run hidden — and devblac/prw#13) +- Next: #15 failed job log excerpt ## Planned diff --git a/cmd/prw/checks.go b/cmd/prw/checks.go new file mode 100644 index 0000000..e452ea6 --- /dev/null +++ b/cmd/prw/checks.go @@ -0,0 +1,294 @@ +package main + +import ( + "context" + "encoding/json" + "fmt" + "io" + "os" + "sort" + "strings" + "text/tabwriter" + "time" + + "github.com/spf13/cobra" + + "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/github" +) + +// exitChecksPending is `prw checks`' exit code while CI is still running; the other +// codes are the ones of `prw wait`. 8 is what `gh pr checks` uses for pending. +const exitChecksPending = 8 + +// Buckets of individual checks, in display order. +const ( + checkFail = "fail" + checkCancel = "cancel" + checkPending = "pending" + checkPass = "pass" + checkNeutral = "neutral" + checkSkipping = "skipping" +) + +var checksJSON bool + +func init() { + rootCmd.AddCommand(checksCmd) + checksCmd.Flags().BoolVar(&checksJSON, "json", false, "print the checks as JSON") +} + +var checksCmd = &cobra.Command{ + Use: "checks ", + Short: "Show every check of a PR with its result, duration and link", + Long: `Show the individual checks (GitHub Actions jobs and other check runs, plus legacy +commit statuses) of a pull request's head commit: failures first, then running +ones, then the rest. The PR does not need to be on the watch list. + +Exit codes: + 0 all checks passed + 1 a check failed + 3 error (invalid URL, API error) + 4 the PR has no CI + 8 checks are still running`, + Example: ` prw checks https://github.com/owner/repo/pull/123 + prw checks https://github.com/owner/repo/pull/123 --json`, + SilenceUsage: true, + Args: func(cmd *cobra.Command, args []string) error { + if len(args) != 1 { + return waitErrorf(exitWaitError, "expected exactly one PR URL, got %d argument(s)", len(args)) + } + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { + owner, repo, number, err := github.ParsePRURL(args[0]) + if err != nil { + return waitErrorf(exitWaitError, "invalid PR URL: %v", err) + } + cfg, err := config.Load() + if err != nil { + return waitErrorf(exitWaitError, "failed to load config: %v", err) + } + token := cfg.GetToken() + if token == "" { + return &exitError{code: exitWaitError, err: errMissingToken} + } + + report, err := fetchChecks(context.Background(), newGitHubClient(token), owner, repo, number, time.Now()) + if err != nil { + return &exitError{code: exitWaitError, err: err} + } + if checksJSON { + enc := json.NewEncoder(os.Stdout) + enc.SetIndent("", " ") + if err := enc.Encode(report); err != nil { + return &exitError{code: exitWaitError, err: err} + } + } else { + printChecks(os.Stdout, report, time.Now()) + } + return report.exitErr() + }, +} + +// checkEntry is one check run or commit status; it is also the --json output. +type checkEntry struct { + Name string `json:"name"` + Bucket string `json:"bucket"` // fail, cancel, pending, pass, neutral, skipping + State string `json:"state"` // GitHub's conclusion, or status while not completed + Kind string `json:"kind"` // check_run or status + URL string `json:"url,omitempty"` // job / details page + StartedAt *time.Time `json:"started_at,omitempty"` // check runs only + CompletedAt *time.Time `json:"completed_at,omitempty"` +} + +// checksReport is the output of `prw checks`. +type checksReport struct { + Owner string `json:"owner"` + Repo string `json:"repo"` + Number int `json:"number"` + Title string `json:"title,omitempty"` + URL string `json:"url"` + SHA string `json:"sha"` + State string `json:"state"` // overall, as in `prw wait` + MergeState string `json:"merge_state,omitempty"` + Checks []checkEntry `json:"checks"` +} + +// fetchChecks gets the head commit's check runs and statuses. Superseded runs of the +// same check are left out, as they don't count towards the result. +func fetchChecks(ctx context.Context, client ciClient, owner, repo string, number int, now time.Time) (*checksReport, error) { + pr, err := client.GetPullRequest(ctx, owner, repo, number) + if err != nil { + return nil, fmt.Errorf("failed to fetch PR: %w", err) + } + sha := pr.Head.SHA + status, err := client.GetCombinedStatus(ctx, owner, repo, sha) + if err != nil { + return nil, fmt.Errorf("failed to fetch commit status: %w", err) + } + runs, err := client.GetCheckRuns(ctx, owner, repo, sha) + if err != nil { + return nil, fmt.Errorf("failed to fetch check runs: %w", err) + } + + ci := github.SummarizeCI(status, runs) + github.ApplyNoCI(ci, pr.UpdatedAt, now) + report := &checksReport{ + Owner: owner, Repo: repo, Number: number, Title: pr.Title, SHA: sha, + URL: github.FormatPRURL(owner, repo, number), State: ci.State, MergeState: github.MergeStatus(pr), + Checks: []checkEntry{}, + } + if pr.IsClosed() { + report.State = "closed" + if pr.Merged { + report.State = "merged" + } + } + + for _, run := range github.LatestCheckRuns(runs) { + entry := checkEntry{Name: run.Name, Kind: "check_run", URL: run.HTMLURL, State: strings.ToLower(run.Conclusion)} + if !strings.EqualFold(run.Status, "completed") { + entry.State = strings.ToLower(run.Status) + } + entry.Bucket = checkRunBucket(run) + if !run.StartedAt.IsZero() { + t := run.StartedAt + entry.StartedAt = &t + } + if !run.CompletedAt.IsZero() { + t := run.CompletedAt + entry.CompletedAt = &t + } + report.Checks = append(report.Checks, entry) + } + for _, s := range status.Statuses { + state := github.NormalizeState(s.State) + bucket := checkPending + switch state { + case "success": + bucket = checkPass + case "failure", "error": + bucket = checkFail + } + report.Checks = append(report.Checks, checkEntry{Name: s.Context, Kind: "status", URL: s.TargetURL, State: state, Bucket: bucket}) + } + + order := map[string]int{checkFail: 0, checkCancel: 1, checkPending: 2, checkPass: 3, checkNeutral: 4, checkSkipping: 5} + sort.SliceStable(report.Checks, func(i, j int) bool { + a, b := report.Checks[i], report.Checks[j] + if order[a.Bucket] != order[b.Bucket] { + return order[a.Bucket] < order[b.Bucket] + } + return strings.ToLower(a.Name) < strings.ToLower(b.Name) + }) + return report, nil +} + +// checkRunBucket classifies a check run like SummarizeCI does: everything that isn't a +// pass, neutral or skip is a failure (cancelled ones are shown apart). +func checkRunBucket(run github.CheckRun) string { + if !strings.EqualFold(run.Status, "completed") { + return checkPending + } + switch strings.ToLower(run.Conclusion) { + case "success": + return checkPass + case "neutral": + return checkNeutral + case "skipped": + return checkSkipping + case "cancelled": + return checkCancel + default: // failure, timed_out, action_required, startup_failure, stale + return checkFail + } +} + +func (r *checksReport) exitErr() error { + pr := fmt.Sprintf("%s/%s#%d", r.Owner, r.Repo, r.Number) + switch r.State { + case "success", "merged", "closed": + return nil + case "failure", "error": + return waitErrorf(exitWaitFailed, "CI of %s finished with %s", pr, r.State) + case github.StateNoCI: + return waitErrorf(exitWaitNoCI, "%s has no CI", pr) + default: + return &exitError{code: exitChecksPending, err: fmt.Errorf("CI of %s is still running", pr)} + } +} + +var bucketIcon = map[string]string{ + checkFail: "❌", checkCancel: "🚫", checkPending: "⏳", checkPass: "✅", checkNeutral: "⚪", checkSkipping: "⏭", +} + +func printChecks(w io.Writer, r *checksReport, now time.Time) { + fmt.Fprintf(w, "%s/%s#%d", r.Owner, r.Repo, r.Number) + if r.Title != "" { + fmt.Fprintf(w, " — %s", r.Title) + } + fmt.Fprintln(w) + + counts := map[string]int{} + for _, c := range r.Checks { + counts[c.Bucket]++ + } + var parts []string + for _, b := range []string{checkFail, checkCancel, checkPending, checkPass, checkNeutral, checkSkipping} { + if counts[b] > 0 { + parts = append(parts, fmt.Sprintf("%d %s", counts[b], b)) + } + } + summary := fmt.Sprintf("%s · %s", shortSHA(r.SHA), r.State) + if len(parts) > 0 { + summary += " · " + strings.Join(parts, ", ") + } + if r.MergeState != "" { + summary += " · merge: " + r.MergeState + } + fmt.Fprintln(w, summary) + + if len(r.Checks) == 0 { + fmt.Fprintln(w, "\nNo checks or statuses reported for this commit.") + return + } + fmt.Fprintln(w) + tw := tabwriter.NewWriter(w, 0, 0, 2, ' ', 0) + for _, c := range r.Checks { + fmt.Fprintf(tw, "%s %s\t%s\t%s\t%s\n", bucketIcon[c.Bucket], c.Bucket, c.Name, checkDuration(c, now), c.URL) + } + tw.Flush() +} + +// checkDuration is how long a check took, or has been running so far ("3m10s…"). +func checkDuration(c checkEntry, now time.Time) string { + if c.StartedAt == nil { + return "" + } + if c.CompletedAt == nil { + return formatCheckDuration(now.Sub(*c.StartedAt)) + "…" + } + return formatCheckDuration(c.CompletedAt.Sub(*c.StartedAt)) +} + +func formatCheckDuration(d time.Duration) string { + if d < 0 { + d = 0 + } + d = d.Round(time.Second) + if d < time.Minute { + return fmt.Sprintf("%ds", int(d.Seconds())) + } + if d < time.Hour { + return fmt.Sprintf("%dm%02ds", int(d.Minutes()), int(d.Seconds())%60) + } + return fmt.Sprintf("%dh%02dm", int(d.Hours()), int(d.Minutes())%60) +} + +func shortSHA(sha string) string { + if len(sha) > 7 { + return sha[:7] + } + return sha +} diff --git a/cmd/prw/checks_test.go b/cmd/prw/checks_test.go new file mode 100644 index 0000000..3f1c094 --- /dev/null +++ b/cmd/prw/checks_test.go @@ -0,0 +1,163 @@ +package main + +import ( + "bytes" + "context" + "fmt" + "net/http" + "strings" + "testing" + "time" + + "github.com/devblac/prw/internal/github" +) + +var checksNow = time.Date(2026, 10, 24, 12, 0, 0, 0, time.UTC) + +func timedRun(id int64, name, status, conclusion string, started, completed time.Duration) github.CheckRun { + r := github.CheckRun{ID: id, Name: name, Status: status, Conclusion: conclusion, HTMLURL: "https://ci/" + name} + r.App.ID = 15368 + if started != 0 { + r.StartedAt = checksNow.Add(-started) + } + if completed != 0 { + r.CompletedAt = checksNow.Add(-completed) + } + return r +} + +func checksFixture() *scriptedClient { + pr := openPR("abc1234def") + pr.MergeableState = "blocked" + return &scriptedClient{steps: []pollStep{{ + pr: pr, + status: &github.CombinedStatus{State: "failure", TotalCount: 1, Statuses: []github.CommitStatus{ + {Context: "ci/jenkins", State: "error", TargetURL: "https://jenkins/1"}, + }}, + runs: []github.CheckRun{ + timedRun(1, "Build", "completed", "success", 5*time.Minute, 4*time.Minute), + timedRun(2, "Test", "completed", "failure", 5*time.Minute, time.Minute), + timedRun(3, "Lint", "in_progress", "", 70*time.Second, 0), + timedRun(4, "Deploy", "completed", "skipped", 0, 0), + timedRun(5, "E2E", "completed", "cancelled", 3*time.Minute, 2*time.Minute), + timedRun(6, "Docs", "completed", "failure", 9*time.Minute, 8*time.Minute), // superseded by 7 + timedRun(7, "Docs", "completed", "success", 2*time.Minute, time.Minute), + }, + }}} +} + +func TestFetchChecks_BucketsAndOrder(t *testing.T) { + report, err := fetchChecks(context.Background(), checksFixture(), "o", "r", 1, checksNow) + if err != nil { + t.Fatal(err) + } + var got []string + for _, c := range report.Checks { + got = append(got, c.Bucket+":"+c.Name) + } + want := "fail:ci/jenkins fail:Test cancel:E2E pending:Lint pass:Build pass:Docs skipping:Deploy" + if strings.Join(got, " ") != want { + t.Errorf("checks = %s\nwant %s", strings.Join(got, " "), want) + } + if report.State != "failure" || report.MergeState != github.MergeBlocked || report.SHA != "abc1234def" { + t.Errorf("report = %+v", report) + } + if code := exitCode(report.exitErr()); code != exitWaitFailed { + t.Errorf("exit code = %d, want 1", code) + } +} + +func TestPrintChecks(t *testing.T) { + report, err := fetchChecks(context.Background(), checksFixture(), "o", "r", 1, checksNow) + if err != nil { + t.Fatal(err) + } + var buf bytes.Buffer + printChecks(&buf, report, checksNow) + out := buf.String() + for _, want := range []string{ + "o/r#1 — Add feature", + "abc1234 · failure · 2 fail, 1 cancel, 1 pending, 2 pass, 1 skipping · merge: blocked", + "❌ fail", "https://jenkins/1", + "Test", "4m00s", // 5m ago → 1m ago + "Lint", "1m10s…", // still running + } { + if !strings.Contains(out, want) { + t.Errorf("output lacks %q:\n%s", want, out) + } + } + if strings.Count(out, "https://ci/Docs") != 1 { + t.Errorf("superseded run of Docs must not be listed:\n%s", out) + } +} + +func TestChecksExitCodes(t *testing.T) { + for state, want := range map[string]int{ + "success": 0, "merged": 0, "failure": exitWaitFailed, "error": exitWaitFailed, + github.StateNoCI: exitWaitNoCI, "pending": exitChecksPending, + } { + r := &checksReport{Owner: "o", Repo: "r", Number: 1, State: state} + if got := exitCode(r.exitErr()); got != want { + t.Errorf("%s: exit %d, want %d", state, got, want) + } + } +} + +func TestFormatCheckDuration(t *testing.T) { + for d, want := range map[time.Duration]string{ + 0: "0s", 42 * time.Second: "42s", 61 * time.Second: "1m01s", 3*time.Hour + 5*time.Minute: "3h05m", -time.Second: "0s", + } { + if got := formatCheckDuration(d); got != want { + t.Errorf("formatCheckDuration(%s) = %q, want %q", d, got, want) + } + } +} + +func TestChecksCmd_PendingExitCode(t *testing.T) { + setupWaitCommand(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.HasSuffix(r.URL.Path, "/pulls/1"): + fmt.Fprint(w, `{"number":1,"title":"WIP","state":"open","mergeable_state":"unstable","head":{"sha":"abc1234"}}`) + case strings.HasSuffix(r.URL.Path, "/status"): + fmt.Fprint(w, `{"state":"pending","total_count":0,"statuses":[]}`) + default: + fmt.Fprint(w, `{"total_count":1,"check_runs":[{"id":1,"name":"Test","status":"in_progress","started_at":"2026-10-24T11:59:00Z"}]}`) + } + }) + checksJSON = false + + out, err := captureStdout(func() error { + return checksCmd.RunE(checksCmd, []string{"https://github.com/owner/repo/pull/1"}) + }) + if code := exitCode(err); code != exitChecksPending { + t.Errorf("exit code = %d (err=%v), want 8", code, err) + } + if !strings.Contains(out, "⏳ pending Test") || !strings.Contains(out, "merge: unstable") { + t.Errorf("stdout:\n%s", out) + } +} + +func TestChecksCmd_NoChecks(t *testing.T) { + setupWaitCommand(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.HasSuffix(r.URL.Path, "/pulls/1"): + fmt.Fprint(w, `{"number":1,"state":"open","updated_at":"2026-01-01T00:00:00Z","head":{"sha":"abc1234"}}`) + case strings.HasSuffix(r.URL.Path, "/status"): + fmt.Fprint(w, `{"state":"pending","total_count":0,"statuses":[]}`) + default: + fmt.Fprint(w, `{"total_count":0,"check_runs":[]}`) + } + }) + checksJSON = true + t.Cleanup(func() { checksJSON = false }) + + out, err := captureStdout(func() error { + return checksCmd.RunE(checksCmd, []string{"https://github.com/owner/repo/pull/1"}) + }) + if code := exitCode(err); code != exitWaitNoCI { + t.Errorf("exit code = %d (err=%v), want 4", code, err) + } + if !strings.Contains(out, `"state": "no_ci"`) || !strings.Contains(out, `"checks": []`) { + t.Errorf("stdout:\n%s", out) + } +} diff --git a/internal/github/client.go b/internal/github/client.go index ddbf810..87ba773 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -169,7 +169,10 @@ type CheckRun struct { Status string `json:"status"` // queued, in_progress, completed, ... Conclusion string `json:"conclusion"` // success, failure, neutral, cancelled, ... HTMLURL string `json:"html_url"` - App struct { + // StartedAt and CompletedAt are zero while queued / running. + StartedAt time.Time `json:"started_at"` + CompletedAt time.Time `json:"completed_at"` + App struct { ID int64 `json:"id"` } `json:"app"` } @@ -386,7 +389,7 @@ func SummarizeCI(status *CombinedStatus, runs []CheckRun) *CIStatus { } } - for _, run := range latestCheckRuns(runs) { + for _, run := range LatestCheckRuns(runs) { total++ if NormalizeState(run.Status) != "completed" { hasPending = true @@ -416,12 +419,12 @@ func SummarizeCI(status *CombinedStatus, runs []CheckRun) *CIStatus { return &CIStatus{State: state, FailedChecks: failed, Total: total, Pending: pending} } -// latestCheckRuns drops check runs superseded by a newer run of the same check (same +// LatestCheckRuns drops check runs superseded by a newer run of the same check (same // app and name). The API returns one run per check suite, so a workflow that ran more // than once for the same commit (e.g. an older run cancelled by `concurrency: // cancel-in-progress`) shows up twice; like GitHub's merge box and required checks, // only the newest one counts. Run IDs grow with creation time. -func latestCheckRuns(runs []CheckRun) []CheckRun { +func LatestCheckRuns(runs []CheckRun) []CheckRun { type checkKey struct { app int64 name string From 49854218e4a773dbbb10cd3b3b98f0a9659db72c Mon Sep 17 00:00:00 2001 From: Julian Blacher Date: Sat, 24 Oct 2026 15:57:35 -0300 Subject: [PATCH 2/5] feat: show why CI failed: failed job log excerpts (`--logs`, MCP `get_failed_logs`) For each failed GitHub Actions job, prw now downloads the job log and extracts the output of the step that failed: its last lines up to the first `##[error]`, starting after the step's "Run ..." header, with timestamps, ANSI colors and runner markup (groups, composite action boundaries, debug) stripped, and errors/warnings rendered as "Error:" / "Warning:". That's what you'd scroll to in the browser. - `prw checks --logs` and `prw wait --logs` (`--log-lines`, default 40) print the excerpts after the result; `logs` / `failed_logs` in `--json`. - MCP tool `get_failed_logs` returns them to agents (text rendered for reading, data in structuredContent); `get_ci_status` and `wait_for_ci` point to it on failure. - Bounded: only the tail (4 MiB) of a log is kept, at most 5 logs per call, superseded and cancelled runs are skipped. Logs aren't cached. - Checks from other CI systems and failed commit statuses are listed with their link, since the API has no log for them; expired logs (404/410) and a missing "Actions: read" permission get a clear message. - The log download redirects to blob storage; Go drops the Authorization header on that cross-host redirect (tested). Parser tested on a real staticcheck failure log (devblac/go-semver-audit). Verified live on cloudposse/atmos#3476 (action download failure), #3422 (git fetch failure, matrix job), #2199 (failed commit status) and over MCP stdio. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + README.md | 19 +- TASKS.md | 3 +- cmd/prw/checks.go | 26 +- cmd/prw/logs.go | 74 +++++ cmd/prw/logs_test.go | 193 ++++++++++++++ cmd/prw/mcp.go | 83 +++++- cmd/prw/mcp_test.go | 2 +- cmd/prw/wait.go | 46 +++- cmd/prw/wait_test.go | 9 + internal/github/logs.go | 252 ++++++++++++++++++ internal/github/logs_test.go | 223 ++++++++++++++++ .../github/testdata/staticcheck_failure.log | 58 ++++ 13 files changed, 965 insertions(+), 24 deletions(-) create mode 100644 cmd/prw/logs.go create mode 100644 cmd/prw/logs_test.go create mode 100644 internal/github/logs.go create mode 100644 internal/github/logs_test.go create mode 100644 internal/github/testdata/staticcheck_failure.log diff --git a/CHANGELOG.md b/CHANGELOG.md index 96b8283..d6030e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `prw run` now notifies once (`merged` or `closed`) and automatically stops watching a PR once it is merged or closed, instead of polling it forever. This notification always fires, regardless of `--on`/`notification_filter`. ### Added +- Failed job logs: `prw checks --logs` and `prw wait --logs` print, for each failed GitHub Actions job, the output of the step that failed (its last `--log-lines` lines up to the error, timestamps/colors/runner markup stripped); `failed_logs` / `logs` in `--json`. New MCP tool `get_failed_logs` returns the same to agents, and `get_ci_status` / `wait_for_ci` point to it on failure. Checks from other CI systems are listed with their link, as they have no log in the API. - `prw checks `: every check run and commit status of a PR's head commit with its result, duration and link, failures first; `--json` for scripts. Superseded runs are left out. Exits `0` passed, `1` failed, `3` error, `4` no CI, `8` still running. - Review and merge events: `prw run` notifies when a PR is approved, gets changes requested, runs into merge conflicts, or becomes ready to merge after branch protection was holding it back. Shown in console, native, Slack/Discord and generic webhooks (`type` `pr_review_change` / `pr_merge_state_change`), filtered by `--on` (`fail`: changes requested and conflicts; `success`: approved and ready). `prw list` gains `REVIEW` and `MERGE` columns (`review` / `merge` in `--json` and MCP `list_watched`); `prw wait --json` and the MCP tools report `merge_state`. Disable with `prw config set pr_events false`. - Auto-watch: `prw watch --mine` follows all your open PRs and `prw watch --review-requested` the PRs where your review is requested, across every repository. Matches are added immediately; `prw run` refreshes the searches every 5 minutes. Review-request PRs are dropped when the request goes away; PRs watched by hand are never touched by it. `prw unwatch --mine` / `--review-requested` turns it off and removes the PRs it added. `prw list` gains an `ADDED BY` column (and `source` in `--json`); `prw config show` shows `auto_watch`. diff --git a/README.md b/README.md index 767b2c2..42c0c17 100644 --- a/README.md +++ b/README.md @@ -243,7 +243,7 @@ prw wait "$PR_URL" --timeout 15m --json | `4` | The PR has **no CI**: no checks or statuses were reported within 3 minutes of its last update (see below) | | `130` | Interrupted (Ctrl+C) | -It fails fast — as soon as one check fails it exits `1` with the failed checks and their links, without waiting for the rest. If someone pushes to the PR meanwhile, it follows the new head commit. Transient errors (network, 5xx, rate limits) are retried up to 3 times in a row. Polls every 15s by default (`--interval`). +It fails fast — as soon as one check fails it exits `1` with the failed checks and their links, without waiting for the rest. If someone pushes to the PR meanwhile, it follows the new head commit. Transient errors (network, 5xx, rate limits) are retried up to 3 times in a row. Polls every 15s by default (`--interval`). Add `--logs` to also get the output of the failed step of each failed job (see below; `failed_logs` in `--json`). With `--json`, stdout is a single object: @@ -285,6 +285,22 @@ abc1234 · failure · 1 fail, 1 pending, 3 pass · merge: blocked Runs superseded by a newer run of the same check (e.g. cancelled by `concurrency: cancel-in-progress`) are left out, as they don't count. `--json` prints the same as an object with a `checks` array (`name`, `bucket`, `state`, `kind`, `url`, `started_at`, `completed_at`). Exit codes match `prw wait` — `0` passed, `1` failed, `3` error, `4` no CI — plus `8` while checks are still running. +### Why did it fail? `--logs` + +`prw checks --logs` (and `prw wait --logs`) also print, for each failed GitHub Actions job, the output of the step that failed: its last lines up to the error, with timestamps, colors and runner markup stripped — what you'd scroll to in the browser. + +``` +$ prw checks https://github.com/owner/repo/pull/123 --logs +… +── Lint · step: staticcheck ./... ── + -: internal error in importing "cmp" (cannot decode "cmp", export data version 5 is greater than maximum supported version 4) + -: internal error in importing "math/bits" (…) + Error: Process completed with exit code 1. + Full log: https://github.com/owner/repo/actions/runs/…/job/… +``` + +`--log-lines` sets how many lines (default 40). At most 5 job logs are downloaded per command. Checks from other CI systems (and legacy commit statuses) have no log in the API, so you get their link instead. Logs expire after GitHub's retention period (90 days by default). Fine-grained tokens need the **Actions: read** permission. + ## Using prw from AI agents: `prw mcp` `prw mcp` runs a [Model Context Protocol](https://modelcontextprotocol.io) server over stdio, so coding agents can check a PR's CI and wait for it after pushing — and read which checks failed — without screen-scraping `gh` output. @@ -310,6 +326,7 @@ Tools: | Tool | What it does | |------|--------------| | `get_ci_status` | Current state of a PR (`success`, `failure`, `error`, `pending`, `merged`, `closed`, `no_ci`), check counts, failed checks with links. Returns immediately. | +| `get_failed_logs` | For each failed GitHub Actions job, the output of the step that failed (last `max_lines` lines up to the error, default 40). What an agent needs to fix the failure without a browser. | | `wait_for_ci` | Blocks until CI finishes (`timeout_seconds` default 600, max 3600). Fails fast on the first failed check, follows new pushes, sends progress notifications. On timeout it returns `pending` with `timed_out: true` so the agent can call it again. | | `list_watched` | PRs on the local watch list with their last known state. | | `watch_pr` / `unwatch_pr` | Add or remove a PR from the watch list (a running `prw run` picks it up). | diff --git a/TASKS.md b/TASKS.md index e8673ed..a7bd82b 100644 --- a/TASKS.md +++ b/TASKS.md @@ -94,7 +94,8 @@ Legend — Impact: 1–5 · Effort: S (≤½ day), M (1–2 days), L (3+ days) - ✅ #11 Review & mergeability events — done (approved / changes_requested / conflict / ready; verified live: devblac/prw#12 `ready`, cli/cli#14727 `blocked`, a simulated blocked → ready notified once and ready → blocked stayed silent) - ✅ Codex follow-ups on #18/#19 — failed head re-check no longer accepted as "unchanged"; review-request PRs only dropped when proven withdrawn (open or 404), never on a cancelled or failed lookup; all review pages read. - ✅ #12 `prw checks ` — done (buckets fail/cancel/pending/pass/neutral/skipping, durations, links, `--json`, exit 8 while running; verified live on devblac/docker-lint#5, zephyrproject-rtos/zephyr#105183 — 35 checks, superseded run hidden — and devblac/prw#13) -- Next: #15 failed job log excerpt +- ✅ #15 Failed job log excerpt — done (`--logs` on `checks`/`wait`, MCP `get_failed_logs`; parser tested on a real staticcheck failure log; verified live on cloudposse/atmos#3476, #3422 and #2199 — a commit status — and over MCP stdio) +- Next: #16 flakiness detection + status history ## Planned diff --git a/cmd/prw/checks.go b/cmd/prw/checks.go index e452ea6..8118b6f 100644 --- a/cmd/prw/checks.go +++ b/cmd/prw/checks.go @@ -31,11 +31,17 @@ const ( checkSkipping = "skipping" ) -var checksJSON bool +var ( + checksJSON bool + checksLogs bool + checksLogLines int +) func init() { rootCmd.AddCommand(checksCmd) checksCmd.Flags().BoolVar(&checksJSON, "json", false, "print the checks as JSON") + checksCmd.Flags().BoolVar(&checksLogs, "logs", false, "show the end of the log of each failed GitHub Actions job") + checksCmd.Flags().IntVar(&checksLogLines, "log-lines", github.DefaultLogLines, "lines of each failed job's log to show with --logs") } var checksCmd = &cobra.Command{ @@ -45,6 +51,10 @@ var checksCmd = &cobra.Command{ commit statuses) of a pull request's head commit: failures first, then running ones, then the rest. The PR does not need to be on the watch list. +With --logs, it also prints the output of the step that failed in each failed +GitHub Actions job (the last --log-lines lines up to the error), so you can see +why without opening the browser. + Exit codes: 0 all checks passed 1 a check failed @@ -52,6 +62,7 @@ Exit codes: 4 the PR has no CI 8 checks are still running`, Example: ` prw checks https://github.com/owner/repo/pull/123 + prw checks https://github.com/owner/repo/pull/123 --logs prw checks https://github.com/owner/repo/pull/123 --json`, SilenceUsage: true, Args: func(cmd *cobra.Command, args []string) error { @@ -74,10 +85,18 @@ Exit codes: return &exitError{code: exitWaitError, err: errMissingToken} } - report, err := fetchChecks(context.Background(), newGitHubClient(token), owner, repo, number, time.Now()) + ctx := context.Background() + client := newGitHubClient(token) + report, err := fetchChecks(ctx, client, owner, repo, number, time.Now()) if err != nil { return &exitError{code: exitWaitError, err: err} } + if checksLogs { + report.Logs, err = fetchFailedLogs(ctx, client, owner, repo, report.SHA, checksLogLines) + if err != nil { + fmt.Fprintf(os.Stderr, "Warning: failed to fetch logs: %v\n", err) + } + } if checksJSON { enc := json.NewEncoder(os.Stdout) enc.SetIndent("", " ") @@ -86,6 +105,7 @@ Exit codes: } } else { printChecks(os.Stdout, report, time.Now()) + printFailedLogs(os.Stdout, report.Logs) } return report.exitErr() }, @@ -113,6 +133,8 @@ type checksReport struct { State string `json:"state"` // overall, as in `prw wait` MergeState string `json:"merge_state,omitempty"` Checks []checkEntry `json:"checks"` + // Logs has an excerpt for each failed job, with --logs. + Logs []github.FailedJobLog `json:"logs,omitempty"` } // fetchChecks gets the head commit's check runs and statuses. Superseded runs of the diff --git a/cmd/prw/logs.go b/cmd/prw/logs.go new file mode 100644 index 0000000..43c4fe8 --- /dev/null +++ b/cmd/prw/logs.go @@ -0,0 +1,74 @@ +package main + +import ( + "context" + "fmt" + "io" + "strings" + + "github.com/devblac/prw/internal/github" +) + +// maxMCPLogLines caps the log lines per failed job returned to agents. +const maxMCPLogLines = 200 + +// fetchFailedLogs gets log excerpts of the failed GitHub Actions jobs of a commit. +// Failed commit statuses (external CI) are listed too, with their link, since they +// have no log in the API. +func fetchFailedLogs(ctx context.Context, client ciClient, owner, repo, sha string, maxLines int) ([]github.FailedJobLog, error) { + fetcher, ok := client.(github.LogFetcher) + if !ok { + return nil, fmt.Errorf("this client can't download job logs") + } + runs, err := client.GetCheckRuns(ctx, owner, repo, sha) + if err != nil { + return nil, fmt.Errorf("failed to fetch check runs: %w", err) + } + logs, err := github.FailedJobLogs(ctx, fetcher, owner, repo, runs, maxLines) + if err != nil { + return nil, err + } + status, err := client.GetCombinedStatus(ctx, owner, repo, sha) + if err != nil { + return nil, fmt.Errorf("failed to fetch commit status: %w", err) + } + for _, s := range status.Statuses { + if state := github.NormalizeState(s.State); state == "failure" || state == "error" { + logs = append(logs, github.FailedJobLog{Name: s.Context, URL: s.TargetURL, + Error: "commit status from another CI system; its log isn't available through the API"}) + } + } + return logs, nil +} + +// printFailedLogs prints each failed job's excerpt below a header naming the job and +// its failed step. +func printFailedLogs(w io.Writer, logs []github.FailedJobLog) { + for _, l := range logs { + header := "── " + l.Name + if l.Excerpt != nil && l.Excerpt.Step != "" { + header += " · step: " + l.Excerpt.Step + } + fmt.Fprintf(w, "\n%s ──\n", header) + if l.Excerpt == nil { + fmt.Fprintf(w, " (no log: %s)\n", l.Error) + } else { + if l.Excerpt.Truncated { + fmt.Fprintln(w, " …") + } + for _, line := range l.Excerpt.Lines { + fmt.Fprintf(w, " %s\n", line) + } + } + if l.URL != "" { + fmt.Fprintf(w, " Full log: %s\n", l.URL) + } + } +} + +// failedLogsText renders logs for an agent: plain text, one block per failed job. +func failedLogsText(logs []github.FailedJobLog) string { + var b strings.Builder + printFailedLogs(&b, logs) + return strings.TrimLeft(b.String(), "\n") +} diff --git a/cmd/prw/logs_test.go b/cmd/prw/logs_test.go new file mode 100644 index 0000000..7a3a998 --- /dev/null +++ b/cmd/prw/logs_test.go @@ -0,0 +1,193 @@ +package main + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "net/http" + "strings" + "testing" + + "github.com/devblac/prw/internal/github" +) + +func actionsJob(id int64, name, conclusion string) github.CheckRun { + r := run(name, "completed", conclusion) + r.ID = id + r.HTMLURL = fmt.Sprintf("https://github.com/o/r/actions/runs/1/job/%d", id) + return r +} + +const goTestFailureLog = "2026-10-24T10:00:00.0000000Z ##[group]Run go test ./...\n" + + "2026-10-24T10:00:00.0000001Z go test ./...\n" + + "2026-10-24T10:00:00.0000002Z ##[endgroup]\n" + + "2026-10-24T10:00:05.0000000Z --- FAIL: TestParse (0.00s)\n" + + "2026-10-24T10:00:05.0000001Z parse_test.go:12: got 1, want 2\n" + + "2026-10-24T10:00:05.0000002Z FAIL\n" + + "2026-10-24T10:00:05.0000003Z ##[error]Process completed with exit code 1.\n" + + "2026-10-24T10:00:06.0000000Z Post job cleanup.\n" + +func failingClient() *scriptedClient { + return &scriptedClient{ + steps: []pollStep{{ + pr: openPR("abc1234def"), + runs: []github.CheckRun{actionsJob(7, "Test", "failure"), actionsJob(8, "Build", "success")}, + }}, + logs: map[int64]string{7: goTestFailureLog}, + } +} + +func TestPrintFailedLogs(t *testing.T) { + c := failingClient() + c.GetPullRequest(context.Background(), "o", "r", 1) // select the step + logs, err := fetchFailedLogs(context.Background(), c, "o", "r", "abc1234def", 40) + if err != nil { + t.Fatal(err) + } + var buf bytes.Buffer + printFailedLogs(&buf, logs) + want := "\n── Test · step: go test ./... ──\n" + + " --- FAIL: TestParse (0.00s)\n" + + " parse_test.go:12: got 1, want 2\n" + + " FAIL\n" + + " Error: Process completed with exit code 1.\n" + + " Full log: https://github.com/o/r/actions/runs/1/job/7\n" + if buf.String() != want { + t.Errorf("got:\n%s\nwant:\n%s", buf.String(), want) + } +} + +func TestMCP_GetFailedLogs(t *testing.T) { + h := startMCP(t, staticClient(failingClient())) + h.initialize() + + result, text := toolResultOf(t, h.call(2, "tools/call", map[string]interface{}{ + "name": "get_failed_logs", "arguments": map[string]interface{}{"pr_url": "https://github.com/owner/repo/pull/1", "max_lines": 2}, + })) + if result["isError"] == true { + t.Fatalf("unexpected tool error: %s", text) + } + if !strings.HasPrefix(text, "1 failed check(s) on owner/repo#1 (abc1234):") || + !strings.Contains(text, "── Test · step: go test ./... ──\n …\n FAIL\n Error: Process completed") { + t.Errorf("text = %q", text) + } + if strings.Contains(text, `"failed_logs"`) { + t.Error("the text should show the excerpts, not repeat them as JSON") + } + logs := result["structuredContent"].(map[string]interface{})["failed_logs"].([]interface{}) + excerpt := logs[0].(map[string]interface{})["excerpt"].(map[string]interface{}) + if len(excerpt["lines"].([]interface{})) != 2 || excerpt["truncated"] != true { + t.Errorf("excerpt = %v, want max_lines honored", excerpt) + } +} + +func TestMCP_GetFailedLogs_NothingFailed(t *testing.T) { + h := startMCP(t, staticClient(successClient())) + h.initialize() + + result, text := toolResultOf(t, h.call(2, "tools/call", map[string]interface{}{ + "name": "get_failed_logs", "arguments": map[string]string{"pr_url": "https://github.com/owner/repo/pull/1"}, + })) + if result["isError"] == true || !strings.HasPrefix(text, "No failed checks on the head commit of owner/repo#1") { + t.Errorf("text = %q", text) + } + if logs := result["structuredContent"].(map[string]interface{})["failed_logs"].([]interface{}); len(logs) != 0 { + t.Errorf("failed_logs = %v", logs) + } +} + +// logsHandler serves a PR whose "Test" job failed, and that job's log. +func logsHandler(w http.ResponseWriter, r *http.Request) { + switch { + case strings.HasSuffix(r.URL.Path, "/pulls/1"): + fmt.Fprint(w, `{"number":1,"title":"Fix parser","state":"open","head":{"sha":"abc1234"}}`) + case strings.HasSuffix(r.URL.Path, "/status"): + fmt.Fprint(w, `{"state":"pending","total_count":0,"statuses":[]}`) + case strings.HasSuffix(r.URL.Path, "/actions/jobs/7/logs"): + fmt.Fprint(w, goTestFailureLog) + default: + fmt.Fprint(w, `{"total_count":1,"check_runs":[{"id":7,"name":"Test","status":"completed","conclusion":"failure",`+ + `"html_url":"https://github.com/owner/repo/actions/runs/1/job/7"}]}`) + } +} + +func TestWaitCmd_LogsOnFailure(t *testing.T) { + setupWaitCommand(t, logsHandler) + waitLogs, waitLogLines, waitJSON = true, 40, false + t.Cleanup(func() { waitLogs = false }) + + out, err := captureStdout(func() error { + return waitCmd.RunE(waitCmd, []string{"https://github.com/owner/repo/pull/1"}) + }) + if code := exitCode(err); code != exitWaitFailed { + t.Errorf("exit code = %d (err=%v)", code, err) + } + if !strings.Contains(out, "── Test · step: go test ./... ──") || !strings.Contains(out, "parse_test.go:12: got 1, want 2") { + t.Errorf("stdout:\n%s", out) + } +} + +func TestWaitCmd_LogsInJSON(t *testing.T) { + setupWaitCommand(t, logsHandler) + waitLogs, waitLogLines, waitJSON = true, 40, true + t.Cleanup(func() { waitLogs, waitJSON = false, false }) + + out, _ := captureStdout(func() error { + return waitCmd.RunE(waitCmd, []string{"https://github.com/owner/repo/pull/1"}) + }) + var result waitResult + if err := json.Unmarshal([]byte(out), &result); err != nil { + t.Fatalf("%v:\n%s", err, out) + } + if len(result.FailedLogs) != 1 || result.FailedLogs[0].Excerpt == nil || result.FailedLogs[0].Excerpt.Step != "go test ./..." { + t.Errorf("failed_logs = %+v", result.FailedLogs) + } +} + +func TestChecksCmd_Logs(t *testing.T) { + setupWaitCommand(t, logsHandler) + checksLogs, checksLogLines, checksJSON = true, 40, false + t.Cleanup(func() { checksLogs = false }) + + out, _ := captureStdout(func() error { + return checksCmd.RunE(checksCmd, []string{"https://github.com/owner/repo/pull/1"}) + }) + if !strings.Contains(out, "❌ fail Test") || !strings.Contains(out, " --- FAIL: TestParse (0.00s)") { + t.Errorf("stdout:\n%s", out) + } +} + +func TestWaitCmd_NoLogsWithoutTheFlag(t *testing.T) { + fetched := false + setupWaitCommand(t, func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "/logs") { + fetched = true + } + logsHandler(w, r) + }) + waitLogs, waitJSON = false, false + + captureStdout(func() error { return waitCmd.RunE(waitCmd, []string{"https://github.com/owner/repo/pull/1"}) }) + if fetched { + t.Error("logs must only be downloaded with --logs") + } +} + +func TestFetchFailedLogs_ListsFailedCommitStatuses(t *testing.T) { + c := &scriptedClient{steps: []pollStep{{ + pr: openPR("sha"), + status: &github.CombinedStatus{State: "failure", TotalCount: 2, Statuses: []github.CommitStatus{ + {Context: "Release milestone", State: "failure"}, + {Context: "ci/jenkins", State: "success", TargetURL: "https://jenkins/1"}, + }}, + }}} + c.GetPullRequest(context.Background(), "o", "r", 1) + logs, err := fetchFailedLogs(context.Background(), c, "o", "r", "sha", 40) + if err != nil { + t.Fatal(err) + } + if len(logs) != 1 || logs[0].Name != "Release milestone" || !strings.Contains(logs[0].Error, "another CI system") { + t.Errorf("logs = %+v", logs) + } +} diff --git a/cmd/prw/mcp.go b/cmd/prw/mcp.go index c126960..d111626 100644 --- a/cmd/prw/mcp.go +++ b/cmd/prw/mcp.go @@ -79,6 +79,7 @@ Authentication is the same as for every other command (gh auth login, GITHUB_TOK // mcpClient is what the MCP tools need from GitHub. type mcpClient interface { ciClient + github.LogFetcher } func defaultMCPClient() (mcpClient, error) { @@ -428,7 +429,7 @@ var mcpTools = []map[string]interface{}{ { "name": "get_ci_status", "title": "Get PR CI status", - "description": "Get the current CI status of a GitHub pull request: overall state (success, failure, error, pending, merged, closed, or no_ci when the PR has no checks at all), check counts, failed checks with links, and merge_state (ready, conflict, blocked, behind, unstable, draft) telling whether the PR can be merged. Aggregates GitHub Actions check runs and legacy commit statuses. Returns immediately.", + "description": "Get the current CI status of a GitHub pull request: overall state (success, failure, error, pending, merged, closed, or no_ci when the PR has no checks at all), check counts, failed checks with links, and merge_state (ready, conflict, blocked, behind, unstable, draft) telling whether the PR can be merged. Aggregates GitHub Actions check runs and legacy commit statuses. Returns immediately. On failure, call get_failed_logs to see why.", "inputSchema": map[string]interface{}{ "type": "object", "properties": map[string]interface{}{"pr_url": prURLSchema}, @@ -440,7 +441,7 @@ var mcpTools = []map[string]interface{}{ { "name": "wait_for_ci", "title": "Wait for PR CI", - "description": "Block until a GitHub pull request's CI finishes, then return the result. Fails fast: returns as soon as any check fails, with the failed checks and links. Follows new pushes to the PR. If the timeout is reached first, returns state 'pending' with timed_out=true (call again to keep waiting). A merge while waiting returns state 'merged'; a PR with no checks or statuses at all returns 'no_ci' instead of waiting until the timeout.", + "description": "Block until a GitHub pull request's CI finishes, then return the result. Fails fast: returns as soon as any check fails, with the failed checks and links. Follows new pushes to the PR. If the timeout is reached first, returns state 'pending' with timed_out=true (call again to keep waiting). A merge while waiting returns state 'merged'; a PR with no checks or statuses at all returns 'no_ci' instead of waiting until the timeout. On failure, call get_failed_logs to see why.", "inputSchema": map[string]interface{}{ "type": "object", "properties": map[string]interface{}{ @@ -462,6 +463,26 @@ var mcpTools = []map[string]interface{}{ }, "annotations": map[string]interface{}{"readOnlyHint": true, "openWorldHint": true}, }, + { + "name": "get_failed_logs", + "title": "Get failed CI job logs", + "description": "For each failed GitHub Actions job on a pull request's head commit, return the output of the step that failed: its last lines up to the error, with timestamps and colors stripped. Use it after get_ci_status or wait_for_ci report a failure, to see why without opening the browser. Checks from other CI systems have no log here; their links are returned instead. At most 5 job logs are downloaded per call.", + "inputSchema": map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{ + "pr_url": prURLSchema, + "max_lines": map[string]interface{}{ + "type": "integer", + "description": "Lines of each failed step's output to return (default 40, max 200).", + "minimum": 1, + "maximum": maxMCPLogLines, + }, + }, + "required": []string{"pr_url"}, + "additionalProperties": false, + }, + "annotations": map[string]interface{}{"readOnlyHint": true, "openWorldHint": true}, + }, { "name": "list_watched", "title": "List watched PRs", @@ -504,11 +525,12 @@ var mcpToolHandlers map[string]mcpToolHandler func init() { // Assigned in init to avoid an initialization cycle (handlers reference mcpServer). mcpToolHandlers = map[string]mcpToolHandler{ - "get_ci_status": toolGetCIStatus, - "wait_for_ci": toolWaitForCI, - "list_watched": toolListWatched, - "watch_pr": toolWatchPR, - "unwatch_pr": toolUnwatchPR, + "get_ci_status": toolGetCIStatus, + "wait_for_ci": toolWaitForCI, + "get_failed_logs": toolGetFailedLogs, + "list_watched": toolListWatched, + "watch_pr": toolWatchPR, + "unwatch_pr": toolUnwatchPR, } } @@ -619,6 +641,53 @@ func toolWaitForCI(ctx context.Context, s *mcpServer, args json.RawMessage, prog return toolOK(ciSummary(result), result) } +func toolGetFailedLogs(ctx context.Context, s *mcpServer, args json.RawMessage, _ io.Writer) mcpToolResult { + owner, repo, number, err := parsePRArg(args) + if err != nil { + return toolError("%v", err) + } + var opts struct { + MaxLines int `json:"max_lines"` + } + if err := json.Unmarshal(args, &opts); err != nil { + return toolError("invalid arguments: %v", err) + } + maxLines := opts.MaxLines + if maxLines <= 0 { + maxLines = github.DefaultLogLines + } + if maxLines > maxMCPLogLines { + maxLines = maxMCPLogLines + } + + client, err := s.githubClient() + if err != nil { + return toolError("%v", err) + } + pr, err := client.GetPullRequest(ctx, owner, repo, number) + if err != nil { + return toolError("failed to fetch %s/%s#%d: %v", owner, repo, number, err) + } + logs, err := fetchFailedLogs(ctx, client, owner, repo, pr.Head.SHA, maxLines) + if err != nil { + return toolError("failed to get logs of %s/%s#%d: %v", owner, repo, number, err) + } + if logs == nil { + logs = []github.FailedJobLog{} + } + data := map[string]interface{}{"sha": pr.Head.SHA, "failed_logs": logs} + ref := fmt.Sprintf("%s/%s#%d (%s)", owner, repo, number, shortSHA(pr.Head.SHA)) + if len(logs) == 0 { + return toolOK("No failed checks on the head commit of "+ref+".", data) + } + // The excerpts are the point of this tool, so the text has them rendered for + // reading rather than repeated as JSON (they're in structuredContent too). + return mcpToolResult{ + Content: []mcpContent{{Type: "text", Text: fmt.Sprintf("%d failed check(s) on %s:\n\n%s", len(logs), ref, failedLogsText(logs))}}, + StructuredContent: data, + } +} + type watchedPROutput struct { URL string `json:"url"` Owner string `json:"owner"` diff --git a/cmd/prw/mcp_test.go b/cmd/prw/mcp_test.go index fffea80..45c49f5 100644 --- a/cmd/prw/mcp_test.go +++ b/cmd/prw/mcp_test.go @@ -215,7 +215,7 @@ func TestMCP_ToolsList(t *testing.T) { t.Errorf("%s is listed but has no handler", name) } } - for _, want := range []string{"get_ci_status", "wait_for_ci", "list_watched", "watch_pr", "unwatch_pr"} { + for _, want := range []string{"get_ci_status", "wait_for_ci", "get_failed_logs", "list_watched", "watch_pr", "unwatch_pr"} { if !names[want] { t.Errorf("tool %s missing from tools/list", want) } diff --git a/cmd/prw/wait.go b/cmd/prw/wait.go index 134e4fd..f090916 100644 --- a/cmd/prw/wait.go +++ b/cmd/prw/wait.go @@ -36,6 +36,8 @@ var ( waitTimeout time.Duration waitInterval time.Duration waitJSON bool + waitLogs bool + waitLogLines int ) // exitError makes main exit with a specific code. @@ -66,6 +68,8 @@ func init() { waitCmd.Flags().DurationVar(&waitTimeout, "timeout", 30*time.Minute, "give up after this long (0 = wait forever)") waitCmd.Flags().DurationVar(&waitInterval, "interval", 15*time.Second, "how often to poll GitHub") waitCmd.Flags().BoolVar(&waitJSON, "json", false, "print the result as JSON on stdout") + waitCmd.Flags().BoolVar(&waitLogs, "logs", false, "on failure, also print the end of the log of each failed GitHub Actions job") + waitCmd.Flags().IntVar(&waitLogLines, "log-lines", github.DefaultLogLines, "lines of each failed job's log to show with --logs") } var waitCmd = &cobra.Command{ @@ -87,9 +91,12 @@ Exit codes: last update (e.g. the repository has no CI, or none runs for its base branch) 130 interrupted (Ctrl+C) -Progress goes to stderr; the result goes to stdout (as JSON with --json).`, +Progress goes to stderr; the result goes to stdout (as JSON with --json). With +--logs, a failure also prints the output of the step that failed in each failed +GitHub Actions job (failed_logs in --json).`, Example: ` prw wait https://github.com/owner/repo/pull/123 prw wait https://github.com/owner/repo/pull/123 --timeout 10m --json + prw wait https://github.com/owner/repo/pull/123 --logs prw wait "$PR_URL" && echo "green, deploying"`, SilenceUsage: true, Args: func(cmd *cobra.Command, args []string) error { @@ -136,29 +143,44 @@ Progress goes to stderr; the result goes to stdout (as JSON with --json).`, } }() - result, err := waitForCI(ctx, newGitHubClient(token), owner, repo, number, waitInterval, os.Stderr) + client := newGitHubClient(token) + result, err := waitForCI(ctx, client, owner, repo, number, waitInterval, os.Stderr) if err != nil { return &exitError{code: exitWaitError, err: err} } + if waitLogs && (result.State == "failure" || result.State == "error") { + // A fresh context: the wait's may be about to time out. + logCtx, cancelLogs := context.WithTimeout(context.Background(), time.Minute) + result.FailedLogs, err = fetchFailedLogs(logCtx, client, owner, repo, result.SHA, waitLogLines) + cancelLogs() + if err != nil { + fmt.Fprintf(os.Stderr, "Warning: failed to fetch logs: %v\n", err) + } + } if err := printWaitResult(os.Stdout, result, waitJSON); err != nil { return &exitError{code: exitWaitError, err: err} } + if !waitJSON { + printFailedLogs(os.Stdout, result.FailedLogs) + } return result.exitErr() }, } // waitResult is the outcome of waitForCI; it is also the --json output. type waitResult struct { - Owner string `json:"owner"` - Repo string `json:"repo"` - Number int `json:"number"` - Title string `json:"title,omitempty"` - URL string `json:"url"` - SHA string `json:"sha,omitempty"` - State string `json:"state"` // success, failure, error, pending, merged, closed - FailedChecks []github.FailedCheck `json:"failed_checks,omitempty"` - TotalChecks int `json:"total_checks"` - PendingChecks int `json:"pending_checks"` + Owner string `json:"owner"` + Repo string `json:"repo"` + Number int `json:"number"` + Title string `json:"title,omitempty"` + URL string `json:"url"` + SHA string `json:"sha,omitempty"` + State string `json:"state"` // success, failure, error, pending, merged, closed + FailedChecks []github.FailedCheck `json:"failed_checks,omitempty"` + // FailedLogs has the log excerpt of each failed GitHub Actions job (--logs). + FailedLogs []github.FailedJobLog `json:"failed_logs,omitempty"` + TotalChecks int `json:"total_checks"` + PendingChecks int `json:"pending_checks"` // MergeState is GitHub's mergeability (ready, conflict, blocked, behind, unstable, // draft); empty while GitHub is still computing it. MergeState string `json:"merge_state,omitempty"` diff --git a/cmd/prw/wait_test.go b/cmd/prw/wait_test.go index c3da461..2c684c0 100644 --- a/cmd/prw/wait_test.go +++ b/cmd/prw/wait_test.go @@ -32,6 +32,14 @@ type pollStep struct { revalidate func() error } +// GetJobLog returns the log set for the job in logs, or a "not available" error. +func (c *scriptedClient) GetJobLog(ctx context.Context, owner, repo string, jobID int64) (string, error) { + if log, ok := c.logs[jobID]; ok { + return log, nil + } + return "", fmt.Errorf("log not available") +} + // scriptedClient returns steps[i] on the i-th poll (repeating the last one). A PR // lookup right after a poll that found a final CI result is the head re-check, not a // new poll. @@ -41,6 +49,7 @@ type scriptedClient struct { revalidations int current pollStep afterCI bool + logs map[int64]string // job logs by check run id } func (c *scriptedClient) GetPullRequest(ctx context.Context, owner, repo string, number int) (*github.PullRequest, error) { diff --git a/internal/github/logs.go b/internal/github/logs.go new file mode 100644 index 0000000..af88887 --- /dev/null +++ b/internal/github/logs.go @@ -0,0 +1,252 @@ +package github + +import ( + "context" + "errors" + "fmt" + "io" + "net/http" + "regexp" + "sort" + "strings" +) + +// maxLogBytes is how much of the end of a job log is kept. Logs can be many MB; the +// failure is near the end (only post-job cleanup follows it). +const maxLogBytes = 4 << 20 + +// DefaultLogLines is how many lines of a failed step an excerpt has by default. +const DefaultLogLines = 40 + +var ( + // actionsJobURL matches the html_url of a GitHub Actions job's check run; its + // check run id is the job id. + actionsJobURL = regexp.MustCompile(`/actions/runs/\d+/job/\d+`) + logTimestamp = regexp.MustCompile(`^\d{4}-\d\d-\d\dT\d\d:\d\d:\d\d(\.\d+)?Z ?`) + ansiEscape = regexp.MustCompile(`\x1b\[[0-9;?]*[A-Za-z]`) + // workflowMarker matches runner markup such as ##[group], ##[endgroup], + // ##[start-action display=...] or ##[debug]. + workflowMarker = regexp.MustCompile(`^##\[[a-z-]+[\] ]`) +) + +// IsActionsJob reports whether a check run is a GitHub Actions job, whose log can be +// downloaded. Other apps' check runs (and commit statuses) have no log in the API. +func IsActionsJob(run CheckRun) bool { + return actionsJobURL.MatchString(run.HTMLURL) +} + +// GetJobLog downloads the log of a GitHub Actions job, keeping at most the last +// maxLogBytes. GitHub redirects to a short-lived storage URL; Go's client doesn't +// forward the Authorization header to it. Logs aren't cached (they're large and +// only fetched for failed jobs, which don't change). +func (c *Client) GetJobLog(ctx context.Context, owner, repo string, jobID int64) (string, error) { + path := fmt.Sprintf("/repos/%s/%s/actions/jobs/%d/logs", owner, repo, jobID) + req, err := http.NewRequestWithContext(ctx, "GET", c.BaseURL+path, nil) + if err != nil { + return "", err + } + if c.Token != "" { + req.Header.Set("Authorization", "Bearer "+c.Token) + } + req.Header.Set("Accept", "application/vnd.github.v3+json") + + resp, err := c.HTTPClient.Do(req) + if err != nil { + return "", fmt.Errorf("request failed: %w", err) + } + defer resp.Body.Close() + c.recordRateLimit(resp.Header) + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(io.LimitReader(resp.Body, 4<<10)) + apiErr := newAPIError(resp, body) + switch { + case apiErr.StatusCode == http.StatusForbidden && !apiErr.RateLimited: + return "", fmt.Errorf("%w (fine-grained tokens need the \"Actions: read\" permission)", apiErr) + case apiErr.StatusCode == http.StatusNotFound || apiErr.StatusCode == http.StatusGone: + return "", fmt.Errorf("log not available: it expired or the job hasn't finished (HTTP %d)", apiErr.StatusCode) + } + return "", apiErr + } + return readTail(resp.Body, maxLogBytes) +} + +// readTail reads r to the end and returns at most its last limit bytes. +func readTail(r io.Reader, limit int) (string, error) { + buf := make([]byte, 0, 64<<10) + chunk := make([]byte, 64<<10) + cut := false + for { + n, err := r.Read(chunk) + buf = append(buf, chunk[:n]...) + if len(buf) > 2*limit { + buf = append(buf[:0], buf[len(buf)-limit:]...) + cut = true + } + if errors.Is(err, io.EOF) { + break + } + if err != nil { + return "", fmt.Errorf("failed to read log: %w", err) + } + } + if len(buf) > limit { + buf = buf[len(buf)-limit:] + cut = true + } + if cut { + if i := strings.IndexByte(string(buf), '\n'); i >= 0 { + buf = buf[i+1:] // drop the partial first line + } + } + return string(buf), nil +} + +// LogExcerpt is the interesting part of a failed job's log. +type LogExcerpt struct { + // Step is the command or action of the failed step, e.g. "go test ./..." + // (from its "Run ..." header); empty if it couldn't be found. + Step string `json:"step,omitempty"` + // Lines are the last lines the step printed, up to and including the error. + Lines []string `json:"lines"` + // Truncated is set when the step printed more than the lines kept. + Truncated bool `json:"truncated,omitempty"` +} + +// ExtractFailure finds the first error in a GitHub Actions job log and returns the +// output of that step leading up to it (at most maxLines lines): what a developer +// scrolls to in the web UI. Timestamps, colors and workflow commands are stripped. +// Without an error marker it returns the end of the log. +func ExtractFailure(log string, maxLines int) LogExcerpt { + if maxLines <= 0 { + maxLines = DefaultLogLines + } + raw := strings.Split(strings.ReplaceAll(log, "\r\n", "\n"), "\n") + lines := make([]string, len(raw)) + for i, l := range raw { + lines[i] = ansiEscape.ReplaceAllString(logTimestamp.ReplaceAllString(l, ""), "") + } + + errAt := -1 + for i, l := range lines { + if strings.HasPrefix(l, "##[error]") { + errAt = i + break + } + } + if errAt < 0 { + end := len(lines) + for end > 0 && strings.TrimSpace(lines[end-1]) == "" { + end-- + } + start := end - maxLines + if start < 0 { + start = 0 + } + return LogExcerpt{Lines: cleanLogLines(lines[start:end]), Truncated: start > 0} + } + + // The step's output starts after its "##[group]Run " header block. + stepStart, step := 0, "" + for i := errAt - 1; i >= 0; i-- { + if strings.HasPrefix(lines[i], "##[group]Run ") { + step = strings.TrimPrefix(lines[i], "##[group]Run ") + stepStart = i + 1 + for j := i + 1; j < errAt; j++ { + if strings.HasPrefix(lines[j], "##[endgroup]") { + stepStart = j + 1 + break + } + } + break + } + } + end := errAt + 1 + for end < len(lines) && strings.HasPrefix(lines[end], "##[error]") { + end++ // keep consecutive errors together + } + start := stepStart + if end-start > maxLines { + start = end - maxLines + } + return LogExcerpt{Step: step, Lines: cleanLogLines(lines[start:end]), Truncated: start > stepStart} +} + +// cleanLogLines turns error/warning/notice workflow commands into readable text and +// drops the other "##[...]" markers (groups, composite action boundaries, debug). +func cleanLogLines(lines []string) []string { + out := make([]string, 0, len(lines)) + for _, l := range lines { + switch { + case strings.HasPrefix(l, "##[error]"): + l = "Error: " + strings.TrimPrefix(l, "##[error]") + case strings.HasPrefix(l, "##[warning]"): + l = "Warning: " + strings.TrimPrefix(l, "##[warning]") + case strings.HasPrefix(l, "##[notice]"): + l = "Notice: " + strings.TrimPrefix(l, "##[notice]") + case workflowMarker.MatchString(l): + continue + } + out = append(out, l) + } + return out +} + +// LogFetcher downloads GitHub Actions job logs. +type LogFetcher interface { + GetJobLog(ctx context.Context, owner, repo string, jobID int64) (string, error) +} + +// FailedJobLog is the log excerpt of one failed check, or why there is none. +type FailedJobLog struct { + Name string `json:"name"` + URL string `json:"url,omitempty"` + Excerpt *LogExcerpt `json:"excerpt,omitempty"` + Error string `json:"error,omitempty"` +} + +// maxLogFetches bounds how many job logs one call downloads, e.g. for a matrix +// where every job failed the same way. +const maxLogFetches = 5 + +// FailedJobLogs returns log excerpts for the failed (not cancelled) runs among runs, +// newest run per check only. Runs that aren't GitHub Actions jobs, and failures past +// the first maxLogFetches, get an Error explaining why there's no excerpt. If ctx is +// cancelled it returns ctx.Err(). +func FailedJobLogs(ctx context.Context, c LogFetcher, owner, repo string, runs []CheckRun, maxLines int) ([]FailedJobLog, error) { + var out []FailedJobLog + fetched := 0 + latest := LatestCheckRuns(runs) + sorted := make([]CheckRun, len(latest)) + copy(sorted, latest) + sort.SliceStable(sorted, func(i, j int) bool { return strings.ToLower(sorted[i].Name) < strings.ToLower(sorted[j].Name) }) + for _, run := range sorted { + if !strings.EqualFold(run.Status, "completed") { + continue + } + switch strings.ToLower(run.Conclusion) { + case "success", "neutral", "skipped", "cancelled": + continue + } + entry := FailedJobLog{Name: run.Name, URL: run.HTMLURL} + switch { + case !IsActionsJob(run): + entry.Error = "not a GitHub Actions job; its log isn't available through the API" + case fetched >= maxLogFetches: + entry.Error = fmt.Sprintf("skipped: only the first %d failed jobs' logs are fetched", maxLogFetches) + default: + fetched++ + log, err := c.GetJobLog(ctx, owner, repo, run.ID) + if ctx.Err() != nil { + return nil, ctx.Err() + } + if err != nil { + entry.Error = err.Error() + } else { + excerpt := ExtractFailure(log, maxLines) + entry.Excerpt = &excerpt + } + } + out = append(out, entry) + } + return out, nil +} diff --git a/internal/github/logs_test.go b/internal/github/logs_test.go new file mode 100644 index 0000000..cb0fb9d --- /dev/null +++ b/internal/github/logs_test.go @@ -0,0 +1,223 @@ +package github + +import ( + "context" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" +) + +func TestExtractFailure_RealStaticcheckLog(t *testing.T) { + // End of a real job log (devblac/go-semver-audit, Lint job): timestamps, ANSI + // colors, step groups, the failing step's output, and post-job cleanup after it. + data, err := os.ReadFile("testdata/staticcheck_failure.log") + if err != nil { + t.Fatal(err) + } + ex := ExtractFailure(string(data), 40) + + if ex.Step != "staticcheck ./..." { + t.Errorf("step = %q", ex.Step) + } + if len(ex.Lines) != 6 || ex.Truncated { + t.Fatalf("got %d lines (truncated=%v), want the 5 output lines + the error:\n%s", len(ex.Lines), ex.Truncated, strings.Join(ex.Lines, "\n")) + } + if !strings.HasPrefix(ex.Lines[0], `-: internal error in importing "cmp"`) { + t.Errorf("first line = %q", ex.Lines[0]) + } + if ex.Lines[5] != "Error: Process completed with exit code 1." { + t.Errorf("last line = %q", ex.Lines[5]) + } + for _, l := range ex.Lines { + if strings.Contains(l, "\x1b") || strings.HasPrefix(l, "2026-") || strings.Contains(l, "Post job cleanup") { + t.Errorf("line not cleaned up or outside the step: %q", l) + } + } +} + +func TestExtractFailure_KeepsTheEndOfLongOutput(t *testing.T) { + var b strings.Builder + b.WriteString("##[group]Run go test ./...\ngo test ./...\n##[endgroup]\n") + for i := 1; i <= 100; i++ { + fmt.Fprintf(&b, "line %d\n", i) + } + b.WriteString("##[error]Process completed with exit code 1.\n") + + ex := ExtractFailure(b.String(), 10) + if len(ex.Lines) != 10 || !ex.Truncated || ex.Lines[0] != "line 92" || ex.Step != "go test ./..." { + t.Errorf("excerpt = %+v", ex) + } +} + +func TestExtractFailure_WithoutErrorMarker(t *testing.T) { + ex := ExtractFailure("a\nb\nc\n\n", 2) + if strings.Join(ex.Lines, ",") != "b,c" || !ex.Truncated { + t.Errorf("excerpt = %+v, want the last 2 non-empty lines", ex) + } +} + +func TestExtractFailure_DropsRunnerMarkup(t *testing.T) { + // From a composite action failing in cloudposse/atmos. + log := "##[group]Run ./.github/actions/setup\n##[endgroup]\n" + + "##[start-action display=Download artifact;id=__self_2.__self]\n" + + "##[debug]Evaluating condition\n" + + "Getting action download info\n" + + "##[warning]Back off 27s before retry.\n" + + "##[error]Resource temporarily unavailable\n" + + "##[end-action id=__self_2.__self;outcome=failure]\n" + ex := ExtractFailure(log, 40) + want := "Getting action download info|Warning: Back off 27s before retry.|Error: Resource temporarily unavailable" + if strings.Join(ex.Lines, "|") != want || ex.Step != "./.github/actions/setup" { + t.Errorf("excerpt = %+v", ex) + } +} + +func TestExtractFailure_ConsecutiveErrorsStayTogether(t *testing.T) { + ex := ExtractFailure("##[group]Run make\nmake\n##[endgroup]\nboom\n##[error]first\n##[error]second\nPost job cleanup.\n", 40) + if strings.Join(ex.Lines, "|") != "boom|Error: first|Error: second" { + t.Errorf("lines = %q", ex.Lines) + } +} + +func TestReadTail(t *testing.T) { + data := strings.Repeat("x", 100) + "\nkeep 1\nkeep 2\n" + got, err := readTail(strings.NewReader(data), 20) + if err != nil { + t.Fatal(err) + } + if got != "keep 1\nkeep 2\n" { + t.Errorf("tail = %q, want the last whole lines within 20 bytes", got) + } +} + +func TestIsActionsJob(t *testing.T) { + if !IsActionsJob(CheckRun{HTMLURL: "https://github.com/o/r/actions/runs/1/job/2"}) { + t.Error("an Actions job URL must be recognized") + } + if IsActionsJob(CheckRun{HTMLURL: "https://circleci.com/gh/o/r/1"}) { + t.Error("an external check must not be") + } +} + +func TestGetJobLog_FollowsRedirectWithoutLeakingToken(t *testing.T) { + storage := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") != "" { + t.Error("the token must not be sent to the log storage host") + } + fmt.Fprint(w, "##[group]Run go test\n##[endgroup]\nFAIL\n##[error]exit 1\n") + })) + defer storage.Close() + api := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/repos/o/r/actions/jobs/42/logs" || r.Header.Get("Authorization") != "Bearer secret" { + t.Errorf("unexpected request %s (auth %q)", r.URL.Path, r.Header.Get("Authorization")) + } + // A different host, like GitHub's blob storage. + http.Redirect(w, r, strings.Replace(storage.URL, "127.0.0.1", "localhost", 1)+"/log.txt", http.StatusFound) + })) + defer api.Close() + c := NewClient("secret") + c.BaseURL = api.URL + + log, err := c.GetJobLog(context.Background(), "o", "r", 42) + if err != nil { + t.Fatal(err) + } + if ex := ExtractFailure(log, 40); strings.Join(ex.Lines, "|") != "FAIL|Error: exit 1" { + t.Errorf("excerpt = %+v", ex) + } +} + +func TestGetJobLog_ExplainsMissingPermission(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + fmt.Fprint(w, `{"message":"Resource not accessible by personal access token"}`) + })) + defer srv.Close() + c := NewClient("t") + c.BaseURL = srv.URL + if _, err := c.GetJobLog(context.Background(), "o", "r", 1); err == nil || !strings.Contains(err.Error(), "Actions: read") { + t.Errorf("err = %v", err) + } +} + +type fakeLogs struct { + logs map[int64]string + fetched []int64 +} + +func (f *fakeLogs) GetJobLog(ctx context.Context, owner, repo string, jobID int64) (string, error) { + f.fetched = append(f.fetched, jobID) + if log, ok := f.logs[jobID]; ok { + return log, nil + } + return "", errors.New("log not available") +} + +func TestFailedJobLogs(t *testing.T) { + job := func(id int64, name, conclusion string) CheckRun { + r := actionsRun(id, name, "completed", conclusion) + r.HTMLURL = fmt.Sprintf("https://github.com/o/r/actions/runs/1/job/%d", id) + return r + } + external := CheckRun{ID: 9, Name: "circleci", Status: "completed", Conclusion: "failure", HTMLURL: "https://circleci.com/x"} + runs := []CheckRun{ + job(1, "Test", "failure"), + job(2, "Build", "success"), + job(3, "E2E", "cancelled"), + job(4, "Lint", "timed_out"), + job(5, "Docs", "failure"), job(6, "Docs", "success"), // superseded failure + external, + } + f := &fakeLogs{logs: map[int64]string{1: "boom\n##[error]exit 1\n"}} + + got, err := FailedJobLogs(context.Background(), f, "o", "r", runs, 40) + if err != nil { + t.Fatal(err) + } + var names []string + for _, l := range got { + names = append(names, l.Name) + } + if strings.Join(names, ",") != "circleci,Lint,Test" { // sorted by name, like `prw checks` + t.Fatalf("failed jobs = %v", names) + } + if got[2].Excerpt == nil || got[2].Excerpt.Lines[0] != "boom" { + t.Errorf("Test excerpt = %+v", got[2]) + } + if got[1].Error != "log not available" || got[0].Excerpt != nil || !strings.Contains(got[0].Error, "not a GitHub Actions job") { + t.Errorf("errors: %+v / %+v", got[1], got[0]) + } + if fmt.Sprint(f.fetched) != "[4 1]" { + t.Errorf("fetched logs of %v, want only the failed Actions jobs", f.fetched) + } +} + +func TestFailedJobLogs_BoundsDownloads(t *testing.T) { + var runs []CheckRun + for i := int64(1); i <= 8; i++ { + r := actionsRun(i, fmt.Sprintf("Test %d", i), "completed", "failure") + r.HTMLURL = fmt.Sprintf("https://github.com/o/r/actions/runs/1/job/%d", i) + runs = append(runs, r) + } + f := &fakeLogs{} + got, _ := FailedJobLogs(context.Background(), f, "o", "r", runs, 40) + if len(got) != 8 || len(f.fetched) != maxLogFetches || !strings.Contains(got[7].Error, "skipped") { + t.Errorf("got %d entries, fetched %d logs", len(got), len(f.fetched)) + } +} + +func TestReadTail_LongStream(t *testing.T) { + // Larger than the internal buffer, so it is compacted while reading. + data := strings.Repeat("noise line\n", 50000) + "last line\n" + got, err := readTail(strings.NewReader(data), 1000) + if err != nil { + t.Fatal(err) + } + if len(got) > 1000 || !strings.HasPrefix(got, "noise line\n") || !strings.HasSuffix(got, "last line\n") { + t.Errorf("tail has %d bytes, starts %q", len(got), got[:20]) + } +} diff --git a/internal/github/testdata/staticcheck_failure.log b/internal/github/testdata/staticcheck_failure.log new file mode 100644 index 0000000..d83d886 --- /dev/null +++ b/internal/github/testdata/staticcheck_failure.log @@ -0,0 +1,58 @@ +2026-10-18T05:33:46.5363687Z GOVCS='' +2026-10-18T05:33:46.5363893Z GOVERSION='go1.27.2' +2026-10-18T05:33:46.5364297Z GOWORK='' +2026-10-18T05:33:46.5364526Z PKG_CONFIG='pkg-config' +2026-10-18T05:33:46.5364684Z +2026-10-18T05:33:46.5365101Z ##[endgroup] +2026-10-18T05:33:46.5612780Z ##[group]Run go vet ./... +2026-10-18T05:33:46.5613392Z go vet ./... +2026-10-18T05:33:46.5811043Z shell: /usr/bin/bash -e {0} +2026-10-18T05:33:46.5811392Z ##[endgroup] +2026-10-18T05:33:46.6036162Z go: downloading golang.org/x/tools v0.52.0 +2026-10-18T05:33:46.6074832Z go: downloading golang.org/x/mod v0.42.0 +2026-10-18T05:33:46.9296217Z go: downloading golang.org/x/sync v0.24.0 +2026-10-18T05:33:58.0757945Z ##[group]Run if [ -n "$(gofmt -s -l .)" ]; then +2026-10-18T05:33:58.0758407Z if [ -n "$(gofmt -s -l .)" ]; then +2026-10-18T05:33:58.0758826Z  echo "Go code is not formatted:" +2026-10-18T05:33:58.0759197Z  gofmt -s -d . +2026-10-18T05:33:58.0759509Z  exit 1 +2026-10-18T05:33:58.0759794Z fi +2026-10-18T05:33:58.0803634Z shell: /usr/bin/bash -e {0} +2026-10-18T05:33:58.0804252Z ##[endgroup] +2026-10-18T05:33:58.3356443Z ##[group]Run go install honnef.co/go/tools/cmd/staticcheck@v0.8.1 +2026-10-18T05:33:58.3356976Z go install honnef.co/go/tools/cmd/staticcheck@v0.8.1 +2026-10-18T05:33:58.3398833Z shell: /usr/bin/bash -e {0} +2026-10-18T05:33:58.3399217Z ##[endgroup] +2026-10-18T05:33:58.4207160Z go: downloading honnef.co/go/tools v0.8.1 +2026-10-18T05:33:58.8575104Z go: downloading golang.org/x/tools v0.44.1-0.20260420230617-19499e7caabc +2026-10-18T05:33:59.1468384Z go: downloading golang.org/x/exp/typeparams v0.0.0-20231108232855-2478ac86f678 +2026-10-18T05:33:59.1489150Z go: downloading github.com/BurntSushi/toml v1.4.1-0.20240526193622-a339e1f7089c +2026-10-18T05:33:59.1612522Z go: downloading golang.org/x/sync v0.20.0 +2026-10-18T05:33:59.1817542Z go: downloading golang.org/x/mod v0.35.0 +2026-10-18T05:33:59.3596895Z go: downloading golang.org/x/exp v0.0.0-20231110203233-9a3e6036ecaa +2026-10-18T05:34:11.9521709Z ##[group]Run staticcheck ./... +2026-10-18T05:34:11.9522017Z staticcheck ./... +2026-10-18T05:34:11.9563879Z shell: /usr/bin/bash -e {0} +2026-10-18T05:34:11.9564321Z ##[endgroup] +2026-10-18T05:34:12.6788196Z -: internal error in importing "cmp" (cannot decode "cmp", export data version 5 is greater than maximum supported version 4); please report an issue (compile) +2026-10-18T05:34:12.6789635Z -: internal error in importing "internal/byteorder" (cannot decode "internal/byteorder", export data version 5 is greater than maximum supported version 4); please report an issue (compile) +2026-10-18T05:34:12.6791322Z -: internal error in importing "internal/cpu" (cannot decode "internal/cpu", export data version 5 is greater than maximum supported version 4); please report an issue (compile) +2026-10-18T05:34:12.6792450Z -: internal error in importing "internal/goarch" (cannot decode "internal/goarch", export data version 5 is greater than maximum supported version 4); please report an issue (compile) +2026-10-18T05:34:12.6793771Z -: internal error in importing "math/bits" (cannot decode "math/bits", export data version 5 is greater than maximum supported version 4); please report an issue (compile) +2026-10-18T05:34:12.6818000Z ##[error]Process completed with exit code 1. +2026-10-18T05:34:12.6958570Z Post job cleanup. +2026-10-18T05:34:12.7759987Z [command]/usr/bin/git version +2026-10-18T05:34:12.7803060Z git version 2.55.0 +2026-10-18T05:34:12.7843356Z Temporarily overriding HOME='/home/runner/work/_temp/69829b6b-508a-4c41-939c-64a926e51c35' before making global git config changes +2026-10-18T05:34:12.7844589Z Adding repository directory to the temporary git global config as a safe directory +2026-10-18T05:34:12.7850078Z [command]/usr/bin/git config --global --add safe.directory /home/runner/work/go-semver-audit/go-semver-audit +2026-10-18T05:34:12.7887749Z [command]/usr/bin/git config --local --name-only --get-regexp core\.sshCommand +2026-10-18T05:34:12.7918151Z [command]/usr/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :" +2026-10-18T05:34:12.8120895Z [command]/usr/bin/git config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader +2026-10-18T05:34:12.8145657Z http.https://github.com/.extraheader +2026-10-18T05:34:12.8156871Z [command]/usr/bin/git config --local --unset-all http.https://github.com/.extraheader +2026-10-18T05:34:12.8190765Z [command]/usr/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'http\.https\:\/\/github\.com\/\.extraheader' && git config --local --unset-all 'http.https://github.com/.extraheader' || :" +2026-10-18T05:34:12.8413564Z [command]/usr/bin/git config --local --name-only --get-regexp ^includeIf\.gitdir: +2026-10-18T05:34:12.8451965Z [command]/usr/bin/git submodule foreach --recursive git config --local --show-origin --name-only --get-regexp remote.origin.url +2026-10-18T05:34:12.8821636Z Cleaning up orphan processes +2026-10-18T05:34:12.9054557Z ##[warning]Node.js 20 is deprecated. The following actions target Node.js 20 but are being forced to run on Node.js 24: actions/checkout@v4, actions/setup-go@v5. For more information see: https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/ From 92eac0c78eb8e1ace3b4c7f29b91a6079de195aa Mon Sep 17 00:00:00 2001 From: Julian Blacher Date: Sat, 24 Oct 2026 15:58:35 -0300 Subject: [PATCH 3/5] fix: `prw checks` exits 3 for a PR closed without merging It exited 0, so `prw checks "$PR" && ...` went ahead on an abandoned PR, and the exit codes didn't match `prw wait` as documented. Addresses Codex review on #20. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 767b2c2..211264e 100644 --- a/README.md +++ b/README.md @@ -283,7 +283,7 @@ abc1234 · failure · 1 fail, 1 pending, 3 pass · merge: blocked ✅ pass ci/jenkins https://jenkins.example.com/job/… ``` -Runs superseded by a newer run of the same check (e.g. cancelled by `concurrency: cancel-in-progress`) are left out, as they don't count. `--json` prints the same as an object with a `checks` array (`name`, `bucket`, `state`, `kind`, `url`, `started_at`, `completed_at`). Exit codes match `prw wait` — `0` passed, `1` failed, `3` error, `4` no CI — plus `8` while checks are still running. +Runs superseded by a newer run of the same check (e.g. cancelled by `concurrency: cancel-in-progress`) are left out, as they don't count. `--json` prints the same as an object with a `checks` array (`name`, `bucket`, `state`, `kind`, `url`, `started_at`, `completed_at`). Exit codes match `prw wait` — `0` passed (or merged), `1` failed, `3` error or closed without merging, `4` no CI — plus `8` while checks are still running. ## Using prw from AI agents: `prw mcp` From 71fd25dfd6b7518a689f5e0c92962252ee9ed3fa Mon Sep 17 00:00:00 2001 From: Julian Blacher Date: Sat, 24 Oct 2026 15:59:15 -0300 Subject: [PATCH 4/5] fix: actually return exit 3 from `prw checks` for closed PRs The previous commit only updated the README; this is the code change: a PR closed without merging now exits 3 (like `prw wait`) instead of 0. Addresses Codex review on #20. Co-Authored-By: Claude Opus 5.5 --- cmd/prw/checks.go | 7 +++++-- cmd/prw/checks_test.go | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/cmd/prw/checks.go b/cmd/prw/checks.go index e452ea6..8266324 100644 --- a/cmd/prw/checks.go +++ b/cmd/prw/checks.go @@ -48,7 +48,7 @@ ones, then the rest. The PR does not need to be on the watch list. Exit codes: 0 all checks passed 1 a check failed - 3 error (invalid URL, API error) + 3 error (invalid URL, API error, PR closed without merging) 4 the PR has no CI 8 checks are still running`, Example: ` prw checks https://github.com/owner/repo/pull/123 @@ -208,8 +208,11 @@ func checkRunBucket(run github.CheckRun) string { func (r *checksReport) exitErr() error { pr := fmt.Sprintf("%s/%s#%d", r.Owner, r.Repo, r.Number) switch r.State { - case "success", "merged", "closed": + case "success", "merged": return nil + case "closed": + // Like `prw wait`: an abandoned PR must not pass `prw checks "$PR" && ...`. + return waitErrorf(exitWaitError, "%s was closed without merging", pr) case "failure", "error": return waitErrorf(exitWaitFailed, "CI of %s finished with %s", pr, r.State) case github.StateNoCI: diff --git a/cmd/prw/checks_test.go b/cmd/prw/checks_test.go index 3f1c094..4d343b3 100644 --- a/cmd/prw/checks_test.go +++ b/cmd/prw/checks_test.go @@ -93,7 +93,7 @@ func TestPrintChecks(t *testing.T) { func TestChecksExitCodes(t *testing.T) { for state, want := range map[string]int{ - "success": 0, "merged": 0, "failure": exitWaitFailed, "error": exitWaitFailed, + "success": 0, "merged": 0, "closed": exitWaitError, "failure": exitWaitFailed, "error": exitWaitFailed, github.StateNoCI: exitWaitNoCI, "pending": exitChecksPending, } { r := &checksReport{Owner: "o", Repo: "r", Number: 1, State: state} From c15292f2f992886c38f7169b33283cc48e55b226 Mon Sep 17 00:00:00 2001 From: Julian Blacher Date: Sat, 24 Oct 2026 21:02:21 -0300 Subject: [PATCH 5/5] feat: find flaky checks (`prw flaky`, MCP `get_flaky_checks`) A job that failed and then passed when its workflow run was re-run is flaky: same commit, same code, different result. `prw flaky ` scans the repository's last --runs workflow runs (default 300, max 1000), fetches the jobs of every attempt of the re-run ones, and reports each job that failed (failure / timed_out) in one attempt and succeeded in a later one, grouped by workflow and job: how many runs it flaked in, when last, and the latest failed attempt's link. Cancelled jobs (fail-fast victims) don't count; jobs that kept failing aren't flaky. `--json` for scripts. The MCP tool `get_flaky_checks` gives agents the same, so they can tell a known-flaky failure from one their change caused. GitHub takes seconds per runs/jobs request on large repositories, so pages and attempts are fetched in parallel (4 at a time) and runs are listed without their pull_requests payload: grafana/grafana went from 24s to 9s. Verified live: grafana/grafana (4 flaky jobs in 300 runs), hashicorp/terraform ("Unit Tests"), devblac/prw (none) and over MCP stdio. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + README.md | 20 ++- TASKS.md | 3 +- cmd/prw/flaky.go | 123 ++++++++++++++ cmd/prw/flaky_test.go | 126 +++++++++++++++ cmd/prw/mcp.go | 76 ++++++++- cmd/prw/mcp_test.go | 2 +- cmd/prw/wait_test.go | 10 ++ internal/github/flaky.go | 295 ++++++++++++++++++++++++++++++++++ internal/github/flaky_test.go | 158 ++++++++++++++++++ 10 files changed, 805 insertions(+), 9 deletions(-) create mode 100644 cmd/prw/flaky.go create mode 100644 cmd/prw/flaky_test.go create mode 100644 internal/github/flaky.go create mode 100644 internal/github/flaky_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index d6030e3..2842d97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `prw run` now notifies once (`merged` or `closed`) and automatically stops watching a PR once it is merged or closed, instead of polling it forever. This notification always fires, regardless of `--on`/`notification_filter`. ### Added +- `prw flaky `: finds flaky GitHub Actions jobs — jobs that failed and then passed when their workflow run was re-run on the same commit — in the repository's last `--runs` workflow runs (default 300, max 1000), grouped by workflow and job with counts, last occurrence and a link to the latest failed attempt; `--json`. Cancelled jobs (fail-fast victims) don't count. New MCP tool `get_flaky_checks`. - Failed job logs: `prw checks --logs` and `prw wait --logs` print, for each failed GitHub Actions job, the output of the step that failed (its last `--log-lines` lines up to the error, timestamps/colors/runner markup stripped); `failed_logs` / `logs` in `--json`. New MCP tool `get_failed_logs` returns the same to agents, and `get_ci_status` / `wait_for_ci` point to it on failure. Checks from other CI systems are listed with their link, as they have no log in the API. - `prw checks `: every check run and commit status of a PR's head commit with its result, duration and link, failures first; `--json` for scripts. Superseded runs are left out. Exits `0` passed, `1` failed, `3` error, `4` no CI, `8` still running. - Review and merge events: `prw run` notifies when a PR is approved, gets changes requested, runs into merge conflicts, or becomes ready to merge after branch protection was holding it back. Shown in console, native, Slack/Discord and generic webhooks (`type` `pr_review_change` / `pr_merge_state_change`), filtered by `--on` (`fail`: changes requested and conflicts; `success`: approved and ready). `prw list` gains `REVIEW` and `MERGE` columns (`review` / `merge` in `--json` and MCP `list_watched`); `prw wait --json` and the MCP tools report `merge_state`. Disable with `prw config set pr_events false`. diff --git a/README.md b/README.md index 7d6f139..86eb893 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,8 @@ The tool is deliberately minimal. It does one thing well: watch PRs and tell you ## Features -- **`prw checks`**: every check of a PR with its result, duration and link +- **`prw checks`**: every check of a PR with its result, duration and link — and with `--logs`, the output of the step that failed +- **`prw flaky`**: find a repository's flaky jobs (failed, then passed on re-run of the same commit) - **`prw wait`**: block until a PR's CI finishes and exit 0/1/2 — for scripts, pipelines and AI coding agents - **MCP server** (`prw mcp`): lets Claude Code, Claude Desktop, Cursor and other agents check and wait for PR CI as tools - **Auto-watch**: `prw watch --mine` / `--review-requested` follow your PRs and review requests across all repositories, no URLs needed @@ -301,6 +302,22 @@ $ prw checks https://github.com/owner/repo/pull/123 --logs `--log-lines` sets how many lines (default 40). At most 5 job logs are downloaded per command. Checks from other CI systems (and legacy commit statuses) have no log in the API, so you get their link instead. Logs expire after GitHub's retention period (90 days by default). Fine-grained tokens need the **Actions: read** permission. +## Finding flaky checks: `prw flaky` + +A job that fails and then passes when its workflow run is re-run — same commit, same code — is flaky. `prw flaky` scans a repository's recent GitHub Actions runs (300 by default, `--runs` up to 1000) for exactly that: + +``` +$ prw flaky grafana/grafana +grafana/grafana: scanned 300 workflow run(s) since 2026-10-24, 3 of them re-run. + +FLAKED WORKFLOW JOB LAST SEEN LATEST FAILED ATTEMPT +1x Backend Unit Tests Grafana Enterprise (3/8) 2026-10-24 15:32 https://github.com/grafana/grafana/actions/runs/…/job/… +1x Integration Tests Sqlite Enterprise (2/16) 2026-10-24 15:25 https://github.com/grafana/grafana/actions/runs/…/job/… +1x End-to-end tests Playwright E2E tests (5/8) 2026-10-24 07:55 https://github.com/grafana/grafana/actions/runs/…/job/… +``` + +It takes a repository (`owner/repo`) or any GitHub URL in it, such as a PR's. Only re-run runs can tell flakiness apart from real failures, and jobs cancelled because another job failed first don't count. `--json` for scripts. It costs a few requests per 100 runs plus a couple per re-run run; on large repositories GitHub takes a few seconds per request, so prw fetches in parallel. + ## Using prw from AI agents: `prw mcp` `prw mcp` runs a [Model Context Protocol](https://modelcontextprotocol.io) server over stdio, so coding agents can check a PR's CI and wait for it after pushing — and read which checks failed — without screen-scraping `gh` output. @@ -327,6 +344,7 @@ Tools: |------|--------------| | `get_ci_status` | Current state of a PR (`success`, `failure`, `error`, `pending`, `merged`, `closed`, `no_ci`), check counts, failed checks with links. Returns immediately. | | `get_failed_logs` | For each failed GitHub Actions job, the output of the step that failed (last `max_lines` lines up to the error, default 40). What an agent needs to fix the failure without a browser. | +| `get_flaky_checks` | A repository's jobs that failed and then passed on re-run of the same commit, with counts and links — so an agent can tell a known-flaky failure from one its change caused. | | `wait_for_ci` | Blocks until CI finishes (`timeout_seconds` default 600, max 3600). Fails fast on the first failed check, follows new pushes, sends progress notifications. On timeout it returns `pending` with `timed_out: true` so the agent can call it again. | | `list_watched` | PRs on the local watch list with their last known state. | | `watch_pr` / `unwatch_pr` | Add or remove a PR from the watch list (a running `prw run` picks it up). | diff --git a/TASKS.md b/TASKS.md index a7bd82b..10db5e1 100644 --- a/TASKS.md +++ b/TASKS.md @@ -95,7 +95,8 @@ Legend — Impact: 1–5 · Effort: S (≤½ day), M (1–2 days), L (3+ days) - ✅ Codex follow-ups on #18/#19 — failed head re-check no longer accepted as "unchanged"; review-request PRs only dropped when proven withdrawn (open or 404), never on a cancelled or failed lookup; all review pages read. - ✅ #12 `prw checks ` — done (buckets fail/cancel/pending/pass/neutral/skipping, durations, links, `--json`, exit 8 while running; verified live on devblac/docker-lint#5, zephyrproject-rtos/zephyr#105183 — 35 checks, superseded run hidden — and devblac/prw#13) - ✅ #15 Failed job log excerpt — done (`--logs` on `checks`/`wait`, MCP `get_failed_logs`; parser tested on a real staticcheck failure log; verified live on cloudposse/atmos#3476, #3422 and #2199 — a commit status — and over MCP stdio) -- Next: #16 flakiness detection + status history +- ✅ #16 Flakiness detection — done as `prw flaky` + MCP `get_flaky_checks` (failed-then-passed across attempts of the same run; verified live: grafana/grafana 4 flaky jobs in 300 runs, hashicorp/terraform "Unit Tests"; parallel fetching cut grafana from 24s to 9s). The "status history" half was dropped: low value for its cost. +- Next: #17 packaging (Homebrew/Scoop) + keychain token storage, #8 `gh` extension publishing — both need owner decisions ## Planned diff --git a/cmd/prw/flaky.go b/cmd/prw/flaky.go new file mode 100644 index 0000000..213084c --- /dev/null +++ b/cmd/prw/flaky.go @@ -0,0 +1,123 @@ +package main + +import ( + "context" + "encoding/json" + "fmt" + "io" + "os" + "regexp" + "text/tabwriter" + + "github.com/spf13/cobra" + + "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/github" +) + +var ( + flakyRuns int + flakyJSON bool +) + +func init() { + rootCmd.AddCommand(flakyCmd) + flakyCmd.Flags().IntVar(&flakyRuns, "runs", github.DefaultFlakyRuns, fmt.Sprintf("how many recent workflow runs to scan (max %d)", github.MaxFlakyRuns)) + flakyCmd.Flags().BoolVar(&flakyJSON, "json", false, "print the result as JSON") +} + +var flakyCmd = &cobra.Command{ + Use: "flaky ", + Short: "Find flaky checks: jobs that failed and then passed on re-run", + Long: `Scan a repository's recent GitHub Actions workflow runs for flaky jobs: jobs that +failed and then passed when the run was re-run, on the same commit. The code +didn't change between the attempts, so the failure wasn't caused by it. + +Only re-run workflow runs can show this. Jobs cancelled because another job failed +first are not counted. Results are grouped by workflow and job, most frequent first, +with a link to the latest failed attempt.`, + Example: ` prw flaky owner/repo + prw flaky https://github.com/owner/repo/pull/123 --runs 1000 + prw flaky owner/repo --json`, + SilenceUsage: true, + Args: func(cmd *cobra.Command, args []string) error { + if len(args) != 1 { + return waitErrorf(exitWaitError, "expected a repository (owner/repo) or a PR URL, got %d argument(s)", len(args)) + } + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { + owner, repo, err := parseRepoArg(args[0]) + if err != nil { + return waitErrorf(exitWaitError, "%v", err) + } + cfg, err := config.Load() + if err != nil { + return waitErrorf(exitWaitError, "failed to load config: %v", err) + } + token := cfg.GetToken() + if token == "" { + return &exitError{code: exitWaitError, err: errMissingToken} + } + + fmt.Fprintf(os.Stderr, "Scanning the last %d workflow runs of %s/%s...\n", flakyRuns, owner, repo) + report, err := github.FindFlaky(context.Background(), newGitHubClient(token), owner, repo, flakyRuns) + if err != nil { + return &exitError{code: exitWaitError, err: err} + } + for _, w := range report.Warnings { + fmt.Fprintf(os.Stderr, "Warning: %s\n", w) + } + if flakyJSON { + enc := json.NewEncoder(os.Stdout) + enc.SetIndent("", " ") + if err := enc.Encode(report); err != nil { + return &exitError{code: exitWaitError, err: err} + } + return nil + } + printFlaky(os.Stdout, report) + return nil + }, +} + +var ( + repoSlug = regexp.MustCompile(`^([A-Za-z0-9_.-]+)/([A-Za-z0-9_.-]+)$`) + repoURL = regexp.MustCompile(`^(?:https?://)?github\.com/([^/]+)/([^/#?]+)`) +) + +// parseRepoArg accepts "owner/repo", a repository URL or a PR URL. +func parseRepoArg(arg string) (owner, repo string, err error) { + if m := repoSlug.FindStringSubmatch(arg); m != nil { + return m[1], m[2], nil + } + if m := repoURL.FindStringSubmatch(arg); m != nil { + return m[1], m[2], nil + } + return "", "", fmt.Errorf("invalid repository %q: use owner/repo or a GitHub URL", arg) +} + +func printFlaky(w io.Writer, r *github.FlakyReport) { + scope := fmt.Sprintf("%d workflow run(s)", r.Scanned) + if !r.Since.IsZero() { + scope += " since " + r.Since.Local().Format("2006-01-02") + } + fmt.Fprintf(w, "%s/%s: scanned %s, %d of them re-run.\n", r.Owner, r.Repo, scope, r.Reruns) + + switch { + case r.Reruns == 0: + fmt.Fprintln(w, "\nNo run was re-run, so there's nothing to compare: flakiness shows up when a failed\njob passes after re-running it on the same commit. Try a larger --runs.") + return + case len(r.Flaky) == 0: + fmt.Fprintln(w, "\nNo flaky checks: no job failed and then passed in a re-run.") + return + } + + fmt.Fprintln(w) + tw := tabwriter.NewWriter(w, 0, 0, 2, ' ', 0) + fmt.Fprintln(tw, "FLAKED\tWORKFLOW\tJOB\tLAST SEEN\tLATEST FAILED ATTEMPT") + for _, f := range r.Flaky { + fmt.Fprintf(tw, "%dx\t%s\t%s\t%s\t%s\n", f.Count, f.Workflow, f.Job, f.LastSeen.Local().Format("2006-01-02 15:04"), f.FailedJobURL) + } + tw.Flush() +} diff --git a/cmd/prw/flaky_test.go b/cmd/prw/flaky_test.go new file mode 100644 index 0000000..491a42d --- /dev/null +++ b/cmd/prw/flaky_test.go @@ -0,0 +1,126 @@ +package main + +import ( + "bytes" + "fmt" + "net/http" + "strings" + "testing" + "time" + + "github.com/devblac/prw/internal/github" +) + +func TestParseRepoArg(t *testing.T) { + for arg, want := range map[string]string{ + "owner/repo": "owner/repo", + "devblac/prw.go": "devblac/prw.go", + "https://github.com/owner/repo": "owner/repo", + "https://github.com/owner/repo/pull/123": "owner/repo", + "github.com/owner/repo/actions/runs/1": "owner/repo", + "https://github.com/owner/repo#readme": "owner/repo", + } { + owner, repo, err := parseRepoArg(arg) + if err != nil || owner+"/"+repo != want { + t.Errorf("parseRepoArg(%q) = %s/%s, %v; want %s", arg, owner, repo, err, want) + } + } + for _, bad := range []string{"", "owner", "https://gitlab.com/o/r", "a/b/c"} { + if _, _, err := parseRepoArg(bad); err == nil { + t.Errorf("parseRepoArg(%q) should fail", bad) + } + } +} + +func flakyReport(flaky ...github.FlakyCheck) *github.FlakyReport { + return &github.FlakyReport{Owner: "o", Repo: "r", Scanned: 300, Reruns: 4, Since: time.Date(2026, 10, 20, 0, 0, 0, 0, time.UTC), Flaky: flaky} +} + +func TestPrintFlaky(t *testing.T) { + var buf bytes.Buffer + printFlaky(&buf, flakyReport(github.FlakyCheck{Workflow: "CI", Job: "Unit Tests", Count: 3, + LastSeen: time.Date(2026, 10, 24, 10, 0, 0, 0, time.UTC), FailedJobURL: "https://github.com/o/r/actions/runs/1/job/2"})) + out := buf.String() + for _, want := range []string{"o/r: scanned 300 workflow run(s) since 2026-10-", "4 of them re-run", "FLAKED", "3x", "Unit Tests", "https://github.com/o/r/actions/runs/1/job/2"} { + if !strings.Contains(out, want) { + t.Errorf("output lacks %q:\n%s", want, out) + } + } +} + +func TestPrintFlaky_NothingToCompare(t *testing.T) { + var buf bytes.Buffer + r := flakyReport() + r.Reruns = 0 + printFlaky(&buf, r) + if !strings.Contains(buf.String(), "No run was re-run") { + t.Errorf("output:\n%s", buf.String()) + } + buf.Reset() + printFlaky(&buf, flakyReport()) + if !strings.Contains(buf.String(), "No flaky checks") { + t.Errorf("output:\n%s", buf.String()) + } +} + +func TestFlakyCmd(t *testing.T) { + setupWaitCommand(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.HasSuffix(r.URL.Path, "/actions/runs"): + if r.URL.Query().Get("page") != "1" { + fmt.Fprint(w, `{"workflow_runs":[]}`) + return + } + fmt.Fprint(w, `{"workflow_runs":[{"id":9,"name":"CI","run_attempt":2,"created_at":"2026-10-24T10:00:00Z","html_url":"https://github.com/owner/repo/actions/runs/9"}]}`) + case strings.HasSuffix(r.URL.Path, "/runs/9/attempts/1/jobs"): + fmt.Fprint(w, `{"jobs":[{"name":"Test","conclusion":"failure","html_url":"https://github.com/owner/repo/actions/runs/9/job/1","completed_at":"2026-10-24T10:05:00Z"}]}`) + case strings.HasSuffix(r.URL.Path, "/runs/9/attempts/2/jobs"): + fmt.Fprint(w, `{"jobs":[{"name":"Test","conclusion":"success"}]}`) + default: + http.NotFound(w, r) + } + }) + flakyRuns, flakyJSON = 300, false + + out, err := captureStdout(func() error { + return flakyCmd.RunE(flakyCmd, []string{"https://github.com/owner/repo/pull/5"}) + }) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out, "1x") || !strings.Contains(out, "Test") || !strings.Contains(out, "runs/9/job/1") { + t.Errorf("stdout:\n%s", out) + } +} + +func TestMCP_GetFlakyChecks(t *testing.T) { + c := successClient() + c.workflowRuns = []github.WorkflowRun{{ID: 9, Name: "CI", RunAttempt: 2}} + c.attemptJobs = map[string][]github.Job{ + "9/1": {{Name: "Test", Conclusion: "failure", HTMLURL: "https://github.com/o/r/actions/runs/9/job/1"}}, + "9/2": {{Name: "Test", Conclusion: "success"}}, + } + h := startMCP(t, staticClient(c)) + h.initialize() + + result, text := toolResultOf(t, h.call(2, "tools/call", map[string]interface{}{ + "name": "get_flaky_checks", "arguments": map[string]interface{}{"repo": "owner/repo"}, + })) + if result["isError"] == true { + t.Fatalf("tool error: %s", text) + } + if !strings.HasPrefix(text, "owner/repo: 1 recent workflow run(s) scanned, 1 re-run. Flaky: CI / Test (1x).") { + t.Errorf("text = %q", strings.SplitN(text, "\n", 2)[0]) + } + flaky := result["structuredContent"].(map[string]interface{})["flaky"].([]interface{}) + if len(flaky) != 1 || flaky[0].(map[string]interface{})["failed_job_url"] != "https://github.com/o/r/actions/runs/9/job/1" { + t.Errorf("flaky = %v", flaky) + } + + result, text = toolResultOf(t, h.call(3, "tools/call", map[string]interface{}{ + "name": "get_flaky_checks", "arguments": map[string]interface{}{"repo": "not a repo"}, + })) + if result["isError"] != true { + t.Errorf("an invalid repo must be a tool error: %s", text) + } +} diff --git a/cmd/prw/mcp.go b/cmd/prw/mcp.go index d111626..a88173a 100644 --- a/cmd/prw/mcp.go +++ b/cmd/prw/mcp.go @@ -80,6 +80,7 @@ Authentication is the same as for every other command (gh auth login, GITHUB_TOK type mcpClient interface { ciClient github.LogFetcher + github.FlakyFetcher } func defaultMCPClient() (mcpClient, error) { @@ -483,6 +484,29 @@ var mcpTools = []map[string]interface{}{ }, "annotations": map[string]interface{}{"readOnlyHint": true, "openWorldHint": true}, }, + { + "name": "get_flaky_checks", + "title": "Find flaky checks", + "description": "Find a repository's flaky GitHub Actions jobs: jobs that failed and then passed when the workflow run was re-run on the same commit, in its recent runs. Use it when a check fails to see whether that job is known to fail intermittently (a re-run may fix it) rather than because of the change. Returns workflow, job, how many re-runs it flaked in, when last, and the latest failed attempt's link. Scanning takes a few seconds on large repositories.", + "inputSchema": map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{ + "repo": map[string]interface{}{ + "type": "string", + "description": "The repository as owner/repo, or any GitHub URL in it (a PR URL works).", + }, + "runs": map[string]interface{}{ + "type": "integer", + "description": "How many recent workflow runs to scan (default 300, max 1000).", + "minimum": 1, + "maximum": github.MaxFlakyRuns, + }, + }, + "required": []string{"repo"}, + "additionalProperties": false, + }, + "annotations": map[string]interface{}{"readOnlyHint": true, "openWorldHint": true}, + }, { "name": "list_watched", "title": "List watched PRs", @@ -525,12 +549,13 @@ var mcpToolHandlers map[string]mcpToolHandler func init() { // Assigned in init to avoid an initialization cycle (handlers reference mcpServer). mcpToolHandlers = map[string]mcpToolHandler{ - "get_ci_status": toolGetCIStatus, - "wait_for_ci": toolWaitForCI, - "get_failed_logs": toolGetFailedLogs, - "list_watched": toolListWatched, - "watch_pr": toolWatchPR, - "unwatch_pr": toolUnwatchPR, + "get_ci_status": toolGetCIStatus, + "wait_for_ci": toolWaitForCI, + "get_failed_logs": toolGetFailedLogs, + "get_flaky_checks": toolGetFlakyChecks, + "list_watched": toolListWatched, + "watch_pr": toolWatchPR, + "unwatch_pr": toolUnwatchPR, } } @@ -688,6 +713,45 @@ func toolGetFailedLogs(ctx context.Context, s *mcpServer, args json.RawMessage, } } +func toolGetFlakyChecks(ctx context.Context, s *mcpServer, args json.RawMessage, _ io.Writer) mcpToolResult { + var opts struct { + Repo string `json:"repo"` + Runs int `json:"runs"` + } + if err := json.Unmarshal(args, &opts); err != nil { + return toolError("invalid arguments: %v", err) + } + owner, repo, err := parseRepoArg(opts.Repo) + if err != nil { + return toolError("%v", err) + } + client, err := s.githubClient() + if err != nil { + return toolError("%v", err) + } + report, err := github.FindFlaky(ctx, client, owner, repo, opts.Runs) + if err != nil { + return toolError("failed to scan %s/%s: %v", owner, repo, err) + } + return toolOK(flakySummary(report), report) +} + +// flakySummary is the one-line answer for an agent. +func flakySummary(r *github.FlakyReport) string { + scope := fmt.Sprintf("%s/%s: %d recent workflow run(s) scanned, %d re-run", r.Owner, r.Repo, r.Scanned, r.Reruns) + switch { + case r.Reruns == 0: + return scope + "; none was re-run, so flakiness can't be told apart from real failures." + case len(r.Flaky) == 0: + return scope + "; no job failed and then passed on re-run." + } + names := make([]string, 0, len(r.Flaky)) + for _, f := range r.Flaky { + names = append(names, fmt.Sprintf("%s / %s (%dx)", f.Workflow, f.Job, f.Count)) + } + return scope + ". Flaky: " + strings.Join(names, ", ") + "." +} + type watchedPROutput struct { URL string `json:"url"` Owner string `json:"owner"` diff --git a/cmd/prw/mcp_test.go b/cmd/prw/mcp_test.go index 45c49f5..35062cc 100644 --- a/cmd/prw/mcp_test.go +++ b/cmd/prw/mcp_test.go @@ -215,7 +215,7 @@ func TestMCP_ToolsList(t *testing.T) { t.Errorf("%s is listed but has no handler", name) } } - for _, want := range []string{"get_ci_status", "wait_for_ci", "get_failed_logs", "list_watched", "watch_pr", "unwatch_pr"} { + for _, want := range []string{"get_ci_status", "wait_for_ci", "get_failed_logs", "get_flaky_checks", "list_watched", "watch_pr", "unwatch_pr"} { if !names[want] { t.Errorf("tool %s missing from tools/list", want) } diff --git a/cmd/prw/wait_test.go b/cmd/prw/wait_test.go index 2c684c0..ba46fc3 100644 --- a/cmd/prw/wait_test.go +++ b/cmd/prw/wait_test.go @@ -50,6 +50,16 @@ type scriptedClient struct { current pollStep afterCI bool logs map[int64]string // job logs by check run id + workflowRuns []github.WorkflowRun + attemptJobs map[string][]github.Job // "runID/attempt" +} + +func (c *scriptedClient) ListWorkflowRuns(ctx context.Context, owner, repo string, max int) ([]github.WorkflowRun, error) { + return c.workflowRuns, nil +} + +func (c *scriptedClient) GetAttemptJobs(ctx context.Context, owner, repo string, runID int64, attempt int) ([]github.Job, error) { + return c.attemptJobs[fmt.Sprintf("%d/%d", runID, attempt)], nil } func (c *scriptedClient) GetPullRequest(ctx context.Context, owner, repo string, number int) (*github.PullRequest, error) { diff --git a/internal/github/flaky.go b/internal/github/flaky.go new file mode 100644 index 0000000..e5a393f --- /dev/null +++ b/internal/github/flaky.go @@ -0,0 +1,295 @@ +package github + +import ( + "context" + "fmt" + "sort" + "strings" + "sync" + "time" +) + +const ( + workflowRunsPerPage = 100 + // MaxFlakyRuns bounds how many recent workflow runs FindFlaky scans. + MaxFlakyRuns = 1000 + // DefaultFlakyRuns is how many recent workflow runs are scanned by default. + DefaultFlakyRuns = 300 + attemptJobsPerPage = 100 + maxAttemptJobPages = 5 +) + +// WorkflowRun is one run of a GitHub Actions workflow. +type WorkflowRun struct { + ID int64 `json:"id"` + Name string `json:"name"` // the workflow's name + HeadSHA string `json:"head_sha"` + RunAttempt int `json:"run_attempt"` + Status string `json:"status"` + Conclusion string `json:"conclusion"` + HTMLURL string `json:"html_url"` + CreatedAt time.Time `json:"created_at"` +} + +// Job is one job of one attempt of a workflow run. +type Job struct { + ID int64 `json:"id"` + Name string `json:"name"` + Status string `json:"status"` + Conclusion string `json:"conclusion"` + HTMLURL string `json:"html_url"` + CompletedAt time.Time `json:"completed_at"` +} + +// flakyConcurrency is how many requests FindFlaky has in flight at once. On large +// repositories GitHub takes seconds per runs/jobs request. +const flakyConcurrency = 4 + +// ListWorkflowRuns returns up to max of the repository's most recent workflow runs, +// newest first. Pages are fetched concurrently. +func (c *Client) ListWorkflowRuns(ctx context.Context, owner, repo string, max int) ([]WorkflowRun, error) { + pages := (max + workflowRunsPerPage - 1) / workflowRunsPerPage + results := make([][]WorkflowRun, pages) + err := forEachConcurrently(ctx, pages, func(i int) error { + path := fmt.Sprintf("/repos/%s/%s/actions/runs?per_page=%d&page=%d&exclude_pull_requests=true", + owner, repo, workflowRunsPerPage, i+1) + var resp struct { + WorkflowRuns []WorkflowRun `json:"workflow_runs"` + } + if err := c.getJSON(ctx, path, &resp); err != nil { + return err + } + results[i] = resp.WorkflowRuns + return nil + }) + if err != nil { + return nil, err + } + var runs []WorkflowRun + for _, page := range results { + runs = append(runs, page...) + if len(page) < workflowRunsPerPage { + break // the last page; later ones are empty + } + } + if len(runs) > max { + runs = runs[:max] + } + return runs, nil +} + +// forEachConcurrently calls fn(0..n-1) with at most flakyConcurrency in flight and +// returns the first error (by index). +func forEachConcurrently(ctx context.Context, n int, fn func(i int) error) error { + errs := make([]error, n) + sem := make(chan struct{}, flakyConcurrency) + var wg sync.WaitGroup + for i := 0; i < n; i++ { + select { + case sem <- struct{}{}: + case <-ctx.Done(): + wg.Wait() + return ctx.Err() + } + wg.Add(1) + go func(i int) { + defer wg.Done() + defer func() { <-sem }() + errs[i] = fn(i) + }(i) + } + wg.Wait() + for _, err := range errs { + if err != nil { + return err + } + } + return nil +} + +// GetAttemptJobs returns the jobs of one attempt of a workflow run. +func (c *Client) GetAttemptJobs(ctx context.Context, owner, repo string, runID int64, attempt int) ([]Job, error) { + var jobs []Job + for page := 1; page <= maxAttemptJobPages; page++ { + path := fmt.Sprintf("/repos/%s/%s/actions/runs/%d/attempts/%d/jobs?per_page=%d&page=%d", + owner, repo, runID, attempt, attemptJobsPerPage, page) + var resp struct { + Jobs []Job `json:"jobs"` + } + if err := c.getJSON(ctx, path, &resp); err != nil { + return nil, err + } + jobs = append(jobs, resp.Jobs...) + if len(resp.Jobs) < attemptJobsPerPage { + break + } + } + return jobs, nil +} + +// FlakyFetcher is what FindFlaky needs from the GitHub client. +type FlakyFetcher interface { + ListWorkflowRuns(ctx context.Context, owner, repo string, max int) ([]WorkflowRun, error) + GetAttemptJobs(ctx context.Context, owner, repo string, runID int64, attempt int) ([]Job, error) +} + +// FlakyCheck is a job that failed and then passed when re-run on the same commit. +type FlakyCheck struct { + Workflow string `json:"workflow"` + Job string `json:"job"` + // Count is how many re-run workflow runs it flaked in. + Count int `json:"count"` + LastSeen time.Time `json:"last_seen"` + // FailedJobURL is the most recent failed attempt, to look at its log. + FailedJobURL string `json:"failed_job_url"` +} + +// FlakyReport is the result of FindFlaky. +type FlakyReport struct { + Owner string `json:"owner"` + Repo string `json:"repo"` + // Scanned is how many workflow runs were looked at; Since is the oldest one. + Scanned int `json:"scanned_runs"` + Since time.Time `json:"since,omitempty"` + // Reruns is how many of them were re-run (only those can show flakiness). + Reruns int `json:"rerun_runs"` + Flaky []FlakyCheck `json:"flaky"` + // Warnings are runs whose jobs couldn't be fetched. + Warnings []string `json:"warnings,omitempty"` +} + +// FindFlaky scans the repository's last maxRuns workflow runs for jobs that failed +// and then passed in a later attempt of the same run: the same commit and workflow, +// so the failure wasn't caused by the code. Jobs cancelled because another job +// failed first are not counted. Results are grouped by workflow and job name, most +// frequent first. +func FindFlaky(ctx context.Context, c FlakyFetcher, owner, repo string, maxRuns int) (*FlakyReport, error) { + if maxRuns <= 0 { + maxRuns = DefaultFlakyRuns + } + if maxRuns > MaxFlakyRuns { + maxRuns = MaxFlakyRuns + } + runs, err := c.ListWorkflowRuns(ctx, owner, repo, maxRuns) + if err != nil { + return nil, fmt.Errorf("failed to list workflow runs: %w", err) + } + report := &FlakyReport{Owner: owner, Repo: repo, Scanned: len(runs), Flaky: []FlakyCheck{}} + if len(runs) > 0 { + report.Since = runs[len(runs)-1].CreatedAt + } + + // Fetch the jobs of every attempt of every re-run run, concurrently. + type fetch struct { + run int // index into reruns + attempt int + } + var reruns []WorkflowRun + var fetches []fetch + for _, run := range runs { + if run.RunAttempt < 2 { + continue + } + for a := 1; a <= run.RunAttempt; a++ { + fetches = append(fetches, fetch{len(reruns), a}) + } + reruns = append(reruns, run) + } + report.Reruns = len(reruns) + jobs := make([][][]Job, len(reruns)) // run → attempt → jobs + failed := make([]error, len(reruns)) + for i, run := range reruns { + jobs[i] = make([][]Job, run.RunAttempt) + } + var mu sync.Mutex + _ = forEachConcurrently(ctx, len(fetches), func(i int) error { + f := fetches[i] + got, err := c.GetAttemptJobs(ctx, owner, repo, reruns[f.run].ID, f.attempt) + mu.Lock() + defer mu.Unlock() + if err != nil && failed[f.run] == nil { + failed[f.run] = fmt.Errorf("attempt %d: %w", f.attempt, err) + } + jobs[f.run][f.attempt-1] = got + return nil // a failed run is reported as a warning, not an error + }) + if err := ctx.Err(); err != nil { + return nil, err + } + + type key struct{ workflow, job string } + found := map[key]*FlakyCheck{} + for i, run := range reruns { + if failed[i] != nil { + report.Warnings = append(report.Warnings, fmt.Sprintf("run %s %v", run.HTMLURL, failed[i])) + continue + } + for _, f := range flakyJobs(jobs[i]) { + k := key{run.Name, f.Name} + entry := found[k] + if entry == nil { + entry = &FlakyCheck{Workflow: run.Name, Job: f.Name} + found[k] = entry + } + entry.Count++ + seen := f.CompletedAt + if seen.IsZero() { + seen = run.CreatedAt + } + if entry.FailedJobURL == "" || seen.After(entry.LastSeen) { + entry.LastSeen, entry.FailedJobURL = seen, f.HTMLURL + } + } + } + + for _, f := range found { + report.Flaky = append(report.Flaky, *f) + } + sort.Slice(report.Flaky, func(i, j int) bool { + a, b := report.Flaky[i], report.Flaky[j] + if a.Count != b.Count { + return a.Count > b.Count + } + return a.LastSeen.After(b.LastSeen) + }) + return report, nil +} + +// flakyJobs returns, for one workflow run, the failed jobs (the latest failed +// attempt of each) whose name passed in a later attempt. +func flakyJobs(attempts [][]Job) []Job { + var flaky []Job + seen := map[string]bool{} + for a := len(attempts) - 2; a >= 0; a-- { // newest failure first + for _, job := range attempts[a] { + if seen[job.Name] || !jobFailed(job) { + continue + } + for _, later := range attempts[a+1:] { + if passed(later, job.Name) { + flaky = append(flaky, job) + seen[job.Name] = true + break + } + } + } + } + return flaky +} + +func jobFailed(j Job) bool { + switch strings.ToLower(j.Conclusion) { + case "failure", "timed_out": + return true + } + return false +} + +func passed(jobs []Job, name string) bool { + for _, j := range jobs { + if j.Name == name && strings.EqualFold(j.Conclusion, "success") { + return true + } + } + return false +} diff --git a/internal/github/flaky_test.go b/internal/github/flaky_test.go new file mode 100644 index 0000000..d0b35d0 --- /dev/null +++ b/internal/github/flaky_test.go @@ -0,0 +1,158 @@ +package github + +import ( + "context" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" +) + +type fakeFlaky struct { + runs []WorkflowRun + jobs map[string][]Job // "runID/attempt" + errs map[string]error + max int +} + +func (f *fakeFlaky) ListWorkflowRuns(ctx context.Context, owner, repo string, max int) ([]WorkflowRun, error) { + f.max = max + return f.runs, nil +} + +func (f *fakeFlaky) GetAttemptJobs(ctx context.Context, owner, repo string, runID int64, attempt int) ([]Job, error) { + k := fmt.Sprintf("%d/%d", runID, attempt) + if err := f.errs[k]; err != nil { + return nil, err + } + return f.jobs[k], nil +} + +var flakyNow = time.Date(2026, 10, 24, 12, 0, 0, 0, time.UTC) + +func wfRun(id int64, workflow string, attempts int, ago time.Duration) WorkflowRun { + return WorkflowRun{ID: id, Name: workflow, RunAttempt: attempts, CreatedAt: flakyNow.Add(-ago), + HTMLURL: fmt.Sprintf("https://github.com/o/r/actions/runs/%d", id)} +} + +func job(name, conclusion string, ago time.Duration) Job { + return Job{Name: name, Conclusion: conclusion, Status: "completed", CompletedAt: flakyNow.Add(-ago), + HTMLURL: "https://github.com/o/r/actions/runs/x/job/" + strings.ReplaceAll(name, " ", "_")} +} + +func TestFindFlaky(t *testing.T) { + f := &fakeFlaky{ + runs: []WorkflowRun{ + wfRun(1, "CI", 2, time.Hour), // Unit Tests failed, passed on re-run → flaky + wfRun(2, "CI", 1, 2*time.Hour), // single attempt: can't tell + wfRun(3, "CI", 3, 3*time.Hour), // Unit Tests flaked again (attempts 1 and 2) + wfRun(4, "Deploy", 2, 4*time.Hour), // Release failed twice: a real failure + }, + jobs: map[string][]Job{ + "1/1": {job("Unit Tests", "failure", 70*time.Minute), job("E2E", "cancelled", 70*time.Minute), job("Lint", "success", 70*time.Minute)}, + "1/2": {job("Unit Tests", "success", 60*time.Minute), job("E2E", "success", 60*time.Minute), job("Lint", "success", 70*time.Minute)}, + "3/1": {job("Unit Tests", "failure", 200*time.Minute)}, + "3/2": {job("Unit Tests", "timed_out", 190*time.Minute)}, + "3/3": {job("Unit Tests", "success", 180*time.Minute)}, + "4/1": {job("Release", "failure", 230*time.Minute)}, + "4/2": {job("Release", "failure", 220*time.Minute)}, + }, + } + + report, err := FindFlaky(context.Background(), f, "o", "r", 0) + if err != nil { + t.Fatal(err) + } + if f.max != DefaultFlakyRuns || report.Scanned != 4 || report.Reruns != 3 || !report.Since.Equal(flakyNow.Add(-4*time.Hour)) { + t.Errorf("report = %+v (max %d)", report, f.max) + } + if len(report.Flaky) != 1 { + t.Fatalf("flaky = %+v, want only Unit Tests (E2E was cancelled, Release never passed)", report.Flaky) + } + got := report.Flaky[0] + if got.Workflow != "CI" || got.Job != "Unit Tests" || got.Count != 2 || !got.LastSeen.Equal(flakyNow.Add(-70*time.Minute)) { + t.Errorf("flaky check = %+v, want CI / Unit Tests counted once per run, last seen in run 1", got) + } +} + +func TestFindFlaky_OrdersByCountThenRecency(t *testing.T) { + f := &fakeFlaky{ + runs: []WorkflowRun{wfRun(1, "CI", 2, time.Hour), wfRun(2, "CI", 2, 2*time.Hour), wfRun(3, "CI", 2, 3*time.Hour)}, + jobs: map[string][]Job{ + "1/1": {job("B", "failure", time.Hour)}, "1/2": {job("B", "success", time.Hour)}, + "2/1": {job("A", "failure", 2*time.Hour)}, "2/2": {job("A", "success", 2*time.Hour)}, + "3/1": {job("A", "failure", 3*time.Hour), job("C", "failure", 3*time.Hour)}, + "3/2": {job("A", "success", 3*time.Hour), job("C", "success", 3*time.Hour)}, + }, + } + report, _ := FindFlaky(context.Background(), f, "o", "r", 100) + var order []string + for _, c := range report.Flaky { + order = append(order, fmt.Sprintf("%s:%d", c.Job, c.Count)) + } + if strings.Join(order, " ") != "A:2 B:1 C:1" { + t.Errorf("order = %v", order) + } +} + +func TestFindFlaky_SkipsRunsWhoseJobsFail(t *testing.T) { + f := &fakeFlaky{ + runs: []WorkflowRun{wfRun(1, "CI", 2, time.Hour)}, + errs: map[string]error{"1/1": errors.New("boom")}, + } + report, err := FindFlaky(context.Background(), f, "o", "r", 100) + if err != nil { + t.Fatal(err) + } + if len(report.Flaky) != 0 || len(report.Warnings) != 1 || !strings.Contains(report.Warnings[0], "boom") { + t.Errorf("report = %+v", report) + } +} + +func TestFindFlaky_CapsRuns(t *testing.T) { + f := &fakeFlaky{} + FindFlaky(context.Background(), f, "o", "r", 5000) + if f.max != MaxFlakyRuns { + t.Errorf("max = %d, want %d", f.max, MaxFlakyRuns) + } +} + +func TestListWorkflowRuns_PagesInOrderAndStopsAtTheLastPage(t *testing.T) { + var requests int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt32(&requests, 1) + if r.URL.Query().Get("exclude_pull_requests") != "true" { + t.Error("runs must be listed without the pull_requests payload") + } + var page int + fmt.Sscan(r.URL.Query().Get("page"), &page) + count := map[int]int{1: 100, 2: 100, 3: 30}[page] // pages 4+ are empty + parts := make([]string, count) + for i := range parts { + parts[i] = fmt.Sprintf(`{"id":%d,"run_attempt":1}`, (page-1)*100+i+1) + } + fmt.Fprintf(w, `{"workflow_runs":[%s]}`, strings.Join(parts, ",")) + })) + defer srv.Close() + c := NewClient("t") + c.BaseURL = srv.URL + + runs, err := c.ListWorkflowRuns(context.Background(), "o", "r", 500) + if err != nil { + t.Fatal(err) + } + if len(runs) != 230 || runs[0].ID != 1 || runs[100].ID != 101 || runs[229].ID != 230 { + t.Errorf("got %d runs (first %d), want 230 in page order", len(runs), runs[0].ID) + } + if n := atomic.LoadInt32(&requests); n != 5 { + t.Errorf("%d requests, want 5 pages for max 500", n) + } + + if runs, _ := c.ListWorkflowRuns(context.Background(), "o", "r", 150); len(runs) != 150 { + t.Errorf("max 150: got %d runs", len(runs)) + } +}