Distinguish cancelled PR checks from failures (#37789)

Show cancelled PR checks as Cancelled with separate counts on desktop and mobile, preserving genuine failures, pending checks, existing action permissions and older-client fallback. Carry complete provider summaries through caches and presentation; preserve Azure source completeness, folder attention priority and muted cancellation details.

Tracked in #15847 and STA-5102.
This commit is contained in:
Brennan Benson
2026-11-03 18:23:26 -08:00
committed by GitHub
parent 63df66d9b6
commit a9676ea6eb
139 changed files with 23705 additions and 20739 deletions
@@ -53,6 +53,87 @@ describe('getPRForBranch', () => {
resetPRForBranchMocks(clientMocks)
})
it('publishes complete cancellation after REST discovery is replaced by exact hydration', async () => {
getOwnerRepoMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' })
ghExecFileAsyncMock
.mockResolvedValueOnce({
stdout: JSON.stringify([
{
number: 7,
title: 'Cancelled run',
state: 'open',
html_url: 'https://github.com/acme/widgets/pull/7',
head: { ref: 'feature/test', sha: 'head-oid' },
base: { ref: 'main' }
}
])
})
.mockResolvedValueOnce({
stdout: JSON.stringify({
number: 7,
title: 'Cancelled run',
state: 'OPEN',
url: 'https://github.com/acme/widgets/pull/7',
headRefOid: 'head-oid',
statusCheckRollup: [{ status: 'COMPLETED', conclusion: 'CANCELLED' }]
})
})
const pr = await getPRForBranch('/repo-root', 'feature/test')
expect(ghExecFileAsyncMock.mock.calls.slice(0, 2).map(([args]) => args.slice(0, 2))).toEqual([
['api', 'repos/acme/widgets/pulls?head=acme%3Afeature%2Ftest&state=all&per_page=10'],
['pr', 'view']
])
expect(pr).toMatchObject({
checksStatus: 'failure',
headSha: 'head-oid',
checksSummary: {
state: 'failure',
total: 1,
passed: 0,
failed: 1,
cancelled: 1,
pending: 0,
neutral: 0
}
})
})
it('keeps branch-list fallback coarse when exact hydration cannot read a failure beyond page 100', async () => {
getOwnerRepoMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' })
resolvePRRepositoryCandidatesMock.mockResolvedValue({
candidates: [{ owner: 'acme', repo: 'widgets' }],
headRepo: null
})
const completeChecks = Array.from({ length: 101 }, (_, index) => ({
status: 'COMPLETED',
conclusion: index === 100 ? 'FAILURE' : 'CANCELLED'
}))
ghExecFileAsyncMock.mockImplementation(async (args: string[]) => {
if (args[0] === 'pr' && args[1] === 'list') {
return {
stdout: JSON.stringify([
{
number: 7,
title: 'Partial check page',
state: 'OPEN',
url: 'https://github.com/acme/widgets/pull/7',
headRefOid: 'head-oid',
statusCheckRollup: completeChecks.slice(0, 100)
}
])
}
}
throw new Error('network unavailable')
})
const pr = await getPRForBranch('/repo-root', 'feature/test')
expect(ghExecFileAsyncMock.mock.calls.slice(0, 2).map(([args]) => args.slice(0, 2))).toEqual([
['pr', 'list'],
['pr', 'view']
])
expect(pr).toMatchObject({ number: 7, checksStatus: 'failure' })
expect(pr?.checksSummary).toBeUndefined()
})
it.each([
{ restEnabled: false, graphqlEnabled: true },
{ restEnabled: true, graphqlEnabled: false }
@@ -232,6 +313,7 @@ describe('getPRForBranch', () => {
expect(pr?.mergeable).toBe('MERGEABLE')
expect(pr?.prRepo).toEqual({ owner: 'acme', repo: 'widgets' })
expect(pr?.headRepo).toEqual({ owner: 'acme', repo: 'widgets' })
expect(pr?.checksSummary).toBeUndefined()
})
it('resolves fork PRs from the upstream PR repo with the origin head owner', async () => {
@@ -97,8 +97,24 @@ describe('work item checksSummary', () => {
total: 3,
passed: 2,
failed: 0,
cancelled: 0,
pending: 0,
neutral: 1
})
})
it.each([
[[{ state: 'SUCCESS' }, { state: 'EXPECTED' }], 'pending', 0],
[[{ state: 'SUCCESS' }, { status: 'COMPLETED', conclusion: 'CANCELLED' }], 'failure', 1]
] as const)('preserves the raw task rollup contract for %j', async (rollup, state, cancelled) => {
mockPullRequestDetail({
number: 43,
title: 'Waiting checks',
state: 'OPEN',
url: 'https://github.com/acme/widgets/pull/43',
statusCheckRollup: rollup
})
const item = await getWorkItem('/repo-root', 43, 'pr', null, {}, 'origin')
expect(item?.checksSummary).toMatchObject({ state, total: 2, passed: 1, cancelled })
})
})
@@ -70,7 +70,9 @@ export async function getFallbackPRListForBranch(
{ ...ghOptions, ...githubHostExecOptions(prRepo) }
)
const list = JSON.parse(stdout) as PullRequestLookupData[]
return preferOpenPullRequest(list) ?? null
const pr = preferOpenPullRequest(list)
// Branch lists paginate PRs, not their nested check contexts.
return pr ? { ...pr, checksUnavailable: true } : null
}
export async function hydrateBranchLookupWithExactPR(
@@ -186,8 +188,12 @@ export async function lookupPRByBranchName(args: {
['pr', 'view', args.branchName, '--json', PR_LOOKUP_JSON_FIELDS],
args.ghOptions
)
const parsedData: PullRequestLookupData = JSON.parse(stdout)
const lookup = {
data: normalizePullRequestLookupData(JSON.parse(stdout) as PullRequestLookupData),
data: normalizePullRequestLookupData({
...parsedData,
checksUnavailable: false
}),
dataRepo: null
}
return !args.policy || (await args.policy.accepts(lookup.data, null))
@@ -97,6 +97,7 @@ export async function getPRByNumber(
{
...knownPullRequestData,
...exactData,
checksUnavailable: false,
...(knownPullRequestData?.stack ? { stack: knownPullRequestData.stack } : {})
},
ghOptions,
@@ -169,8 +170,12 @@ export async function lookupPRByNumber(args: {
['pr', 'view', String(args.number), '--json', PR_LOOKUP_JSON_FIELDS],
args.ghOptions
)
const parsedData: PullRequestLookupData = JSON.parse(stdout)
const lookup = {
data: normalizePullRequestLookupData(JSON.parse(stdout) as PullRequestLookupData),
data: normalizePullRequestLookupData({
...parsedData,
checksUnavailable: false
}),
dataRepo: null
}
return !args.policy || (await args.policy.accepts(lookup.data, null))
@@ -1,10 +1,11 @@
import { summarizePRCheckRollup } from '../../../../shared/pr-check-status'
import type { PRRefreshOutcome } from '../../../../shared/github/pull-request-refresh-types'
import type {
PRConflictSummary,
PRMergeableState,
GitHubPRStack
} from '../../../../shared/github/pull-request-types'
import { deriveCheckStatus, mapPRState } from '../../mappers'
import { mapPRState } from '../../mappers'
import type { OwnerRepo } from '../../gh-utils'
import type { PullRequestLookupData } from './pull-request-lookup-data'
@@ -30,6 +31,7 @@ export function assemblePRRefreshFoundOutcome(args: {
headDivergedFromMergedPRAtOid,
conflictSummary
} = args
const checksSummary = summarizePRCheckRollup(data.statusCheckRollup)
return {
kind: 'found',
fetchedAt: Date.now(),
@@ -38,7 +40,8 @@ export function assemblePRRefreshFoundOutcome(args: {
title: data.title,
state: mapPRState(data.state, data.isDraft),
url: data.url,
checksStatus: deriveCheckStatus(data.statusCheckRollup),
checksStatus: checksSummary.state === 'none' ? 'neutral' : checksSummary.state,
...(data.checksUnavailable === false ? { checksSummary } : {}),
updatedAt: data.updatedAt,
mergeable,
...(data.reviewDecision !== undefined ? { reviewDecision: data.reviewDecision } : {}),
@@ -24,6 +24,7 @@ export type PullRequestLookupData = {
state: string
url: string
statusCheckRollup: unknown[]
checksUnavailable?: boolean
updatedAt: string
isDraft?: boolean
mergeable: string
@@ -131,6 +132,7 @@ export function mapRestPullRequest(pr: RestPullRequest): PullRequestLookupData {
state: pr.merged_at ? 'MERGED' : pr.state,
url: pr.html_url ?? pr.url ?? '',
statusCheckRollup: [],
checksUnavailable: true,
updatedAt: pr.updated_at ?? '',
isDraft: pr.draft,
mergeable: mapRestPRMergeable(pr),
@@ -249,6 +249,7 @@ export async function readPullRequestSidebarQuery(
baseRefOid: pr.baseRefOid ?? undefined,
headRefOid: pr.headRefOid ?? undefined,
statusCheckRollup,
checksUnavailable: false,
stackMetadataChecked,
stack:
stackMetadataChecked && validStack