refactor: deepen Run status into one module

CONTEXT.md

@@ -61,10 +61,12 @@ The visual health indicator for a Repo or PR. Aggregates across all Workflows us
 | Color | Meaning | GitHub statuses |
 |---|---|---|
 | 🟢 Green | Healthy | `success`, `neutral`, `skipped` |
-| 🔴 Red | Broken or blocked | `failure`, `timed_out`, `action_required` |
-| 🟡 Yellow | In progress | `queued`, `in_progress` |
+| 🔴 Red | Broken or blocked | `failure`, `timed_out`, `action_required`, `startup_failure` |
+| 🟡 Yellow | In progress | `queued`, `in_progress`, `requested`, `waiting`, `pending` |
 | ⚪ Grey | No signal | `cancelled`, no runs yet |
 
+A Run's effective status is its conclusion once it has one, and its status until then. This one table drives the Stoplight, the Run and Job icons, which Runs `f` can re-run (a finished 🔴 Run), and stuck alerts (🔴 is a prolonged failure, 🟡 is prolonged in progress).
+
 _Avoid_: badge, indicator, light
 
 ## Review

internal/aggregator/aggregator.go

@@ -1,6 +1,11 @@
+// Package aggregator is the one place that interprets a Run's GitHub status.
+// Everything that colours, sorts, re-runs or alerts on a Run asks it, rather
+// than reading status strings itself.
 package aggregator
 
-// Stoplight represents the health color for a Repo.
+import githubclient "github.com/ericdahl-dev/git-green/internal/github"
+
+// Stoplight represents the health color for a Repo, PR or Run.
 type Stoplight int
 
 const (
@@ -23,41 +28,42 @@ func (s Stoplight) String() string {
 	}
 }
 
-// RunStatus mirrors the relevant GitHub API status/conclusion values.
-type RunStatus string
-
-const (
-	StatusSuccess        RunStatus = "success"
-	StatusNeutral        RunStatus = "neutral"
-	StatusSkipped        RunStatus = "skipped"
-	StatusFailure        RunStatus = "failure"
-	StatusTimedOut       RunStatus = "timed_out"
-	StatusActionRequired RunStatus = "action_required"
-	StatusQueued         RunStatus = "queued"
-	StatusInProgress     RunStatus = "in_progress"
-	StatusCancelled      RunStatus = "cancelled"
-)
-
-func statusToStoplight(s RunStatus) Stoplight {
+// ActiveFirst ranks a Stoplight for Active-first sorting: in progress, then
+// failing, then passing, then no signal. Lower sorts first.
+func (s Stoplight) ActiveFirst() int {
 	switch s {
-	case StatusSuccess, StatusNeutral, StatusSkipped:
+	case StoplightYellow:
+		return 0
+	case StoplightRed:
+		return 1
+	case StoplightGreen:
+		return 2
+	default:
+		return 3
+	}
+}
+
+// Of maps one effective GitHub status (a Run or Job's conclusion, or its
+// status while it has none) to a Stoplight.
+func Of(status string) Stoplight {
+	switch status {
+	case "success", "neutral", "skipped":
 		return StoplightGreen
-	case StatusFailure, StatusTimedOut, StatusActionRequired:
+	case "failure", "timed_out", "action_required", "startup_failure":
 		return StoplightRed
-	case StatusQueued, StatusInProgress:
+	case "queued", "in_progress", "requested", "waiting", "pending":
 		return StoplightYellow
 	default:
 		return StoplightGrey
 	}
 }
 
-// Aggregate returns the worst-case Stoplight across all provided run statuses.
-// Red > Yellow > Green > Grey.
-func Aggregate(statuses []RunStatus) Stoplight {
+// Runs returns the worst-case Stoplight across runs: Red > Yellow > Green >
+// Grey. No runs is Grey.
+func Runs(runs []githubclient.WorkflowRun) Stoplight {
 	result := StoplightGrey
-	for _, s := range statuses {
-		light := statusToStoplight(s)
-		if light > result {
+	for _, r := range runs {
+		if light := Of(r.Effective()); light > result {
 			result = light
 		}
 	}

internal/aggregator/aggregator_test.go

@@ -1,67 +1,77 @@
 package aggregator
 
-import "testing"
+import (
+	"sort"
+	"testing"
 
-func TestStatusMappings(t *testing.T) {
-	cases := []struct {
-		status   RunStatus
-		expected Stoplight
-	}{
-		{StatusSuccess, StoplightGreen},
-		{StatusNeutral, StoplightGreen},
-		{StatusSkipped, StoplightGreen},
-		{StatusFailure, StoplightRed},
-		{StatusTimedOut, StoplightRed},
-		{StatusActionRequired, StoplightRed},
-		{StatusQueued, StoplightYellow},
-		{StatusInProgress, StoplightYellow},
-		{StatusCancelled, StoplightGrey},
+	githubclient "github.com/ericdahl-dev/git-green/internal/github"
+)
+
+func TestOfMapsEveryStatus(t *testing.T) {
+	cases := map[string]Stoplight{
+		"success":         StoplightGreen,
+		"neutral":         StoplightGreen,
+		"skipped":         StoplightGreen,
+		"failure":         StoplightRed,
+		"timed_out":       StoplightRed,
+		"action_required": StoplightRed,
+		"startup_failure": StoplightRed,
+		"queued":          StoplightYellow,
+		"in_progress":     StoplightYellow,
+		"requested":       StoplightYellow,
+		"waiting":         StoplightYellow,
+		"pending":         StoplightYellow,
+		"cancelled":       StoplightGrey,
+		"stale":           StoplightGrey,
+		"":                StoplightGrey,
 	}
-	for _, tc := range cases {
-		got := Aggregate([]RunStatus{tc.status})
-		if got != tc.expected {
-			t.Errorf("status %q: expected %v, got %v", tc.status, tc.expected, got)
+	for status, want := range cases {
+		if got := Of(status); got != want {
+			t.Errorf("Of(%q) = %v, want %v", status, got, want)
 		}
 	}
 }
 
-func TestEmptyReturnsGrey(t *testing.T) {
-	if got := Aggregate(nil); got != StoplightGrey {
-		t.Errorf("expected grey for empty, got %v", got)
-	}
+func run(status, conclusion string) githubclient.WorkflowRun {
+	return githubclient.WorkflowRun{Status: status, Conclusion: conclusion}
 }
 
-func TestAllCancelledReturnsGrey(t *testing.T) {
-	got := Aggregate([]RunStatus{StatusCancelled, StatusCancelled})
-	if got != StoplightGrey {
-		t.Errorf("expected grey, got %v", got)
+func TestRunsIsWorstCase(t *testing.T) {
+	cases := []struct {
+		name string
+		runs []githubclient.WorkflowRun
+		want Stoplight
+	}{
+		{"no runs", nil, StoplightGrey},
+		{"all cancelled", []githubclient.WorkflowRun{run("completed", "cancelled"), run("completed", "cancelled")}, StoplightGrey},
+		{"green and running", []githubclient.WorkflowRun{run("completed", "success"), run("in_progress", "")}, StoplightYellow},
+		{"red beats everything", []githubclient.WorkflowRun{run("completed", "success"), run("in_progress", ""), run("completed", "failure")}, StoplightRed},
+		{"red beats queued", []githubclient.WorkflowRun{run("queued", ""), run("completed", "timed_out")}, StoplightRed},
 	}
-}
-
-func TestMixedGreenYellowReturnsYellow(t *testing.T) {
-	got := Aggregate([]RunStatus{StatusSuccess, StatusInProgress})
-	if got != StoplightYellow {
-		t.Errorf("expected yellow, got %v", got)
+	for _, tc := range cases {
+		if got := Runs(tc.runs); got != tc.want {
+			t.Errorf("%s: got %v, want %v", tc.name, got, tc.want)
+		}
 	}
 }
 
-func TestAnyRedOverridesAll(t *testing.T) {
-	got := Aggregate([]RunStatus{StatusSuccess, StatusInProgress, StatusFailure})
-	if got != StoplightRed {
-		t.Errorf("expected red, got %v", got)
+// A finished run reports its conclusion; one still going only has a status.
+func TestRunsReadsConclusionBeforeStatus(t *testing.T) {
+	if got := Runs([]githubclient.WorkflowRun{run("completed", "failure")}); got != StoplightRed {
+		t.Errorf("completed/failure: got %v, want red", got)
 	}
-}
-
-func TestRedBeatsYellow(t *testing.T) {
-	got := Aggregate([]RunStatus{StatusQueued, StatusTimedOut})
-	if got != StoplightRed {
-		t.Errorf("expected red, got %v", got)
+	if got := Runs([]githubclient.WorkflowRun{run("in_progress", "")}); got != StoplightYellow {
+		t.Errorf("in_progress: got %v, want yellow", got)
 	}
 }
 
-func TestGreenOnlyReturnsGreen(t *testing.T) {
-	got := Aggregate([]RunStatus{StatusSuccess, StatusNeutral, StatusSkipped})
-	if got != StoplightGreen {
-		t.Errorf("expected green, got %v", got)
+func TestActiveFirstOrder(t *testing.T) {
+	lights := []Stoplight{StoplightGrey, StoplightGreen, StoplightRed, StoplightYellow}
+	sort.Slice(lights, func(a, b int) bool { return lights[a].ActiveFirst() < lights[b].ActiveFirst() })
+	want := []Stoplight{StoplightYellow, StoplightRed, StoplightGreen, StoplightGrey}
+	for i := range want {
+		if lights[i] != want[i] {
+			t.Fatalf("got %v, want %v", lights, want)
+		}
 	}
 }

internal/github/client.go

@@ -21,6 +21,14 @@ type Job struct {
 	Conclusion string
 }
 
+// Effective is the job's conclusion once it has one, and its status until then.
+func (j Job) Effective() string {
+	if j.Conclusion != "" {
+		return j.Conclusion
+	}
+	return j.Status
+}
+
 // WorkflowRun holds the latest run for a single workflow.
 type WorkflowRun struct {
 	WorkflowName string
@@ -31,6 +39,14 @@ type WorkflowRun struct {
 	Jobs         []Job
 }
 
+// Effective is the run's conclusion once it has one, and its status until then.
+func (r WorkflowRun) Effective() string {
+	if r.Conclusion != "" {
+		return r.Conclusion
+	}
+	return r.Status
+}
+
 // PR represents an open pull request.
 type PR struct {
 	Number    int

internal/poller/poller.go

@@ -428,33 +428,14 @@ func (p *Poller) fetchRepo(ctx context.Context, repo config.Repo, prev state.Rep
 	runs := data.BranchRuns
 	prRuns := data.PRRuns
 
-	// Aggregate stoplight from default-branch runs.
-	statuses := make([]aggregator.RunStatus, 0, len(runs))
-	for _, r := range runs {
-		s := r.Conclusion
-		if s == "" {
-			s = r.Status
-		}
-		statuses = append(statuses, aggregator.RunStatus(s))
-	}
-
 	// Build PRStates.
 	prStates := make([]state.PRState, 0, len(prRuns))
 	for _, pr := range prRuns {
-		prStatuses := make([]aggregator.RunStatus, 0, len(pr.Runs))
-		for _, r := range pr.Runs {
-			s := r.Conclusion
-			if s == "" {
-				s = r.Status
-			}
-			prStatuses = append(prStatuses, aggregator.RunStatus(s))
-		}
-
 		prStates = append(prStates, state.PRState{
 			Number:    pr.PR.Number,
 			Title:     pr.PR.Title,
 			HTMLURL:   pr.PR.HTMLURL,
-			Stoplight: aggregator.Aggregate(prStatuses),
+			Stoplight: aggregator.Runs(pr.Runs),
 			Runs:      pr.Runs,
 			Mergeable: pr.PR.Mergeable,
 			Stack:     pr.PR.Stack,
@@ -466,7 +447,7 @@ func (p *Poller) fetchRepo(ctx context.Context, repo config.Repo, prev state.Rep
 		Owner:     repo.Owner,
 		Name:      repo.Name,
 		Branch:    data.ResolvedBranch,
-		Stoplight: aggregator.Aggregate(statuses),
+		Stoplight: aggregator.Runs(runs),
 		Runs:      runs,
 		PRs:       prStates,
 		StaleAt:   nil,
@@ -587,27 +568,25 @@ func (p *Poller) evaluateStuck(repos []state.RepoState) []webhooks.Event {
 
 // branchStuckReason returns whether the branch is stuck and why.
 func (p *Poller) branchStuckReason(rs *state.RepoState) (bool, string) {
-	for _, run := range rs.Runs {
-		if run.Conclusion == "failure" || run.Conclusion == "timed_out" {
-			return true, "prolonged_failure"
-		}
-		if run.Status == "in_progress" {
-			return true, "prolonged_in_progress"
-		}
-	}
-	return false, ""
+	return runsStuckReason(rs.Runs)
 }
 
 // prStuckReason returns whether the PR is stuck and why.
 func (p *Poller) prStuckReason(pr *state.PRState) (bool, string) {
 	if pr.Mergeable == "dirty" || pr.Mergeable == "conflicting" {
 		return true, "conflict"
 	}
-	for _, run := range pr.Runs {
-		if run.Conclusion == "failure" || run.Conclusion == "timed_out" {
+	return runsStuckReason(pr.Runs)
+}
+
+// runsStuckReason reports the first Run that is failing or still going, which
+// is what "stuck" means once it has lasted past the threshold.
+func runsStuckReason(runs []githubclient.WorkflowRun) (bool, string) {
+	for _, run := range runs {
+		switch aggregator.Of(run.Effective()) {
+		case aggregator.StoplightRed:
 			return true, "prolonged_failure"
-		}
-		if run.Status == "in_progress" {
+		case aggregator.StoplightYellow:
 			return true, "prolonged_in_progress"
 		}
 	}

internal/poller/poller_test.go

@@ -593,3 +593,25 @@ func TestRateLimitBacksOffUntilReset(t *testing.T) {
 		t.Error("did not resume polling after the reset time")
 	}
 }
+
+// Stuck follows the Stoplight: every red conclusion is a failure and every
+// yellow status is still going, not just failure/timed_out and in_progress.
+func TestRunsStuckReasonFollowsStoplight(t *testing.T) {
+	cases := []struct {
+		run    githubclient.WorkflowRun
+		stuck  bool
+		reason string
+	}{
+		{githubclient.WorkflowRun{Status: "completed", Conclusion: "action_required"}, true, "prolonged_failure"},
+		{githubclient.WorkflowRun{Status: "completed", Conclusion: "startup_failure"}, true, "prolonged_failure"},
+		{githubclient.WorkflowRun{Status: "queued"}, true, "prolonged_in_progress"},
+		{githubclient.WorkflowRun{Status: "completed", Conclusion: "success"}, false, ""},
+		{githubclient.WorkflowRun{Status: "completed", Conclusion: "cancelled"}, false, ""},
+	}
+	for _, tc := range cases {
+		stuck, reason := runsStuckReason([]githubclient.WorkflowRun{tc.run})
+		if stuck != tc.stuck || reason != tc.reason {
+			t.Errorf("%s/%s: got (%v, %q), want (%v, %q)", tc.run.Status, tc.run.Conclusion, stuck, reason, tc.stuck, tc.reason)
+		}
+	}
+}