Don't count cancelled checks as failures in `pr status` by BagToad · Pull Request #12906 · cli/cli

BagToad · 2026-03-11T18:55:03Z

Description

gh pr status and gh pr view count cancelled check runs as failures, even when a newer successful run exists for the same check name. The GitHub web UI shows "All checks passed" and gh pr checks shows the check as passing, but pr status gets it wrong.

This affects any scenario that produces cancelled runs: workflows with concurrency: { cancel-in-progress: true }, manual cancellations, API-triggered cancellations, etc.

Changes Overview

Add a cancelled check status category. Cancelled runs are now excluded from all summary counts (passing/failing/pending) and subtracted from the total, matching the web UI behavior. This fixes both the fast path (aggregated counts by state) and the slow path (individual node iteration).
Move eliminateDuplicates to api.EliminateDuplicateChecks (exported). The function operates entirely on api.CheckContext and was previously buried in pkg/cmd/pr/checks. It is now shared by both pr checks and ChecksStatus() (used by pr status and pr view).
Apply deduplication in the ChecksStatus() slow path, keeping only the most recent run per check name, consistent with what pr checks already does.

Notes to Reviewers

The fixture prStatusChecks.json had __typeName (capital N) instead of __typename. This was harmless before dedup was added but caused nodes to lose their type identity. Fixed alongside adding unique name/context fields so dedup works correctly in tests.
checkRunCount/checkRunCountsByState (the fast path) is available on all currently supported GHES versions (3.16 through 3.20). Removing the slow path entirely could be a follow-up cleanup.

Acceptance Testing Results

Tested against a real PR with cancelled + successful runs for the same check on the same commit:

Scenario	Result
`pr status` shows "Checks passing"	✅
`pr checks` still shows check as passing (regression)	✅
`pr view --json statusCheckRollup` confirms API returns both CANCELLED and SUCCESS	✅

Full acceptance test results

When a GitHub Actions workflow uses concurrency with cancel-in-progress, cancelled runs were counted as failures in `gh pr status` and `gh pr view`, even when a newer run for the same check name succeeded. The GitHub web UI and `gh pr checks` both handle this correctly. Three changes fix this: 1. Add a `cancelled` check status category. Cancelled runs are now excluded from all summary counts (passing/failing/pending) and subtracted from the total, matching the web UI behavior. 2. Move `eliminateDuplicates` from pkg/cmd/pr/checks to `api.EliminateDuplicateChecks` (exported). The function operates entirely on `api.CheckContext` and is now shared by both `pr checks` and `ChecksStatus()` (used by `pr status` and `pr view`). 3. Apply deduplication in the `ChecksStatus()` slow path, keeping only the most recent run per check name — consistent with `pr checks`. Fixes #12895 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This pull request updates PR check status summarization and deduplication by centralizing duplicate-check elimination in the api package and refining how cancelled checks are counted, alongside fixture/test updates to reflect correct GraphQL shapes.

Changes:

Move check deduplication logic into api.EliminateDuplicateChecks and reuse it across PR checks aggregation and PullRequest.ChecksStatus().
Exclude cancelled check runs from check status totals (both in count-by-state and node-based summarization paths).
Update fixtures/tests to use __typename and include required node fields (name/context), plus add new deduplication-focused tests.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
pkg/cmd/pr/status/fixtures/prStatusChecks.json	Fix fixture GraphQL discriminator field to `__typename` and add missing `name`/`context` fields used by dedup/summarization logic.
pkg/cmd/pr/checks/checks_test.go	Update tests to call `api.EliminateDuplicateChecks` after moving the helper out of the command package.
pkg/cmd/pr/checks/aggregate.go	Switch aggregation to use `api.EliminateDuplicateChecks` and remove the local duplicate-elimination implementation.
api/queries_pr.go	Change cancelled check handling in `ChecksStatus()` and introduce exported `EliminateDuplicateChecks` utility.
api/pull_request_test.go	Update expectations for cancelled checks, adjust node helpers to include `name`/`context`, and add new deduplication tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-11T18:59:40Z

api/queries_pr.go

+func EliminateDuplicateChecks(checkContexts []CheckContext) []CheckContext {
+	sort.Slice(checkContexts, func(i, j int) bool { return checkContexts[i].StartedAt.After(checkContexts[j].StartedAt) })


EliminateDuplicateChecks sorts/deduplicates using StartedAt for all nodes, but StatusContext objects from the GraphQL query only include createdAt (see api/query_builder.go StatusContext selection) and do not populate startedAt. This means duplicate StatusContext entries can be deduplicated in an arbitrary order, potentially keeping an older failure instead of a newer success. Consider sorting by StartedAt for CheckRun nodes and by CreatedAt for StatusContext nodes (or by max(StartedAt, CreatedAt)) before deduping.

Suggested change

func EliminateDuplicateChecks(checkContexts []CheckContext) []CheckContext {

sort.Slice(checkContexts, func(i, j int) bool { return checkContexts[i].StartedAt.After(checkContexts[j].StartedAt) })

func checkContextTime(ctx CheckContext) time.Time {

if !ctx.StartedAt.IsZero() {

return ctx.StartedAt

}

return ctx.CreatedAt

}

func EliminateDuplicateChecks(checkContexts []CheckContext) []CheckContext {

sort.Slice(checkContexts, func(i, j int) bool {

return checkContextTime(checkContexts[i]).After(checkContextTime(checkContexts[j]))

})

Copilot · 2026-03-11T18:59:41Z

api/pull_request_test.go

 		{
-			name:                 "COMPLETED / CANCELLED is treated as Failing",
+			name:                 "COMPLETED / CANCELLED is excluded from counts",
 			payload:              singleCompletedCheckRunWithConclusion("CANCELLED"),
-			expectedChecksStatus: PullRequestChecksStatus{Failing: 1, Total: 1},
+			expectedChecksStatus: PullRequestChecksStatus{},
 		},
 		{
-			name:                 "COMPLETED / CANCELLED is treated as Failing",
+			name:                 "COMPLETED / CANCELLED is excluded from counts (duplicate)",
 			payload:              singleCompletedCheckRunWithConclusion("CANCELLED"),
-			expectedChecksStatus: PullRequestChecksStatus{Failing: 1, Total: 1},
+			expectedChecksStatus: PullRequestChecksStatus{},
 		},


This table-driven test includes two identical cases for CANCELLED (same payload and same expected status). The duplicate doesn’t add coverage and makes the test suite noisier; it should either be removed or changed to cover a distinct scenario.

See below for a potential fix:

Copilot · 2026-03-11T18:59:41Z

api/pull_request_test.go

+					TypeName:  "StatusContext",
+					Context:   "ci/test",
+					State:     "SUCCESS",
+					StartedAt: time.Date(2022, 2, 2, 2, 2, 2, 2, time.UTC),
+				},
+			},
+		},
+		{
+			name: "unique checks are preserved",
+			checkContexts: []CheckContext{
+				{
+					TypeName:   "CheckRun",
+					Name:       "build",
+					Status:     "COMPLETED",


The StatusContext deduplication test data uses StartedAt timestamps, but StatusContext nodes from the GraphQL query populate CreatedAt (not StartedAt). This test can pass even if StatusContext deduplication is wrong in real responses; it should set CreatedAt and assert the function uses it when StartedAt is zero.

Copilot · 2026-03-11T18:59:42Z

pkg/cmd/pr/checks/checks_test.go

-			got := eliminateDuplicates(tt.checkContexts)
+			got := api.EliminateDuplicateChecks(tt.checkContexts)
 			if !reflect.DeepEqual(tt.want, got) {
 				t.Errorf("got eliminateDuplicates %+v, want %+v\n", got, tt.want)


The failure message still refers to the old function name "eliminateDuplicates" even though the test now calls api.EliminateDuplicateChecks. Updating the error text will make failures easier to interpret.

Suggested change

t.Errorf("got eliminateDuplicates %+v, want %+v\n", got, tt.want)

t.Errorf("got EliminateDuplicateChecks %+v, want %+v\n", got, tt.want)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

williammartin

Beyond copilot's comments which I will let you address, this looks good to me.

Copilot AI review requested due to automatic review settings March 11, 2026 18:55

BagToad requested a review from a team as a code owner March 11, 2026 18:55

BagToad requested a review from williammartin March 11, 2026 18:55

Copilot started reviewing on behalf of BagToad March 11, 2026 18:55 View session

fix: remove unused test helpers and fix gofmt

f7ff8f2

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI reviewed Mar 11, 2026

View reviewed changes

BagToad mentioned this pull request Mar 11, 2026

Remove ChecksStatus slow path (dead code on all supported GHES) #12909

Open

test: use require.Equal instead of reflect.DeepEqual

db1dc97

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

williammartin approved these changes Mar 25, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't count cancelled checks as failures in `pr status`#12906

Don't count cancelled checks as failures in `pr status`#12906
BagToad wants to merge 3 commits intotrunkfrom
fix-pr-status-cancelled

BagToad commented Mar 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

williammartin left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		func EliminateDuplicateChecks(checkContexts []CheckContext) []CheckContext {
		sort.Slice(checkContexts, func(i, j int) bool { return checkContexts[i].StartedAt.After(checkContexts[j].StartedAt) })

-func EliminateDuplicateChecks(checkContexts []CheckContext) []CheckContext {
-	sort.Slice(checkContexts, func(i, j int) bool { return checkContexts[i].StartedAt.After(checkContexts[j].StartedAt) })
+func checkContextTime(ctx CheckContext) time.Time {
+	if !ctx.StartedAt.IsZero() {
+		return ctx.StartedAt
+	}
+	return ctx.CreatedAt
+}
+func EliminateDuplicateChecks(checkContexts []CheckContext) []CheckContext {
+	sort.Slice(checkContexts, func(i, j int) bool {
+		return checkContextTime(checkContexts[i]).After(checkContextTime(checkContexts[j]))
+	})

	t.Errorf("got eliminateDuplicates %+v, want %+v\n", got, tt.want)
	t.Errorf("got EliminateDuplicateChecks %+v, want %+v\n", got, tt.want)

Conversation

BagToad commented Mar 11, 2026

Description

Changes Overview

Notes to Reviewers

Acceptance Testing Results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

williammartin left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants