Skip to content

Commit c2d80a0

Browse files
committed
Refreshing PR does not refresh rerun status checks
Fixes #8449
1 parent 820db25 commit c2d80a0

File tree

2 files changed

+121
-44
lines changed

2 files changed

+121
-44
lines changed

src/github/githubRepository.ts

Lines changed: 118 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import {
4949
MergeMethod,
5050
PullRequest,
5151
PullRequestChecks,
52+
PullRequestCheckStatus,
5253
PullRequestReviewRequirement,
5354
RepoAccessAndMergeMethods,
5455
User,
@@ -1688,51 +1689,56 @@ export class GitHubRepository extends Disposable {
16881689
// We always fetch the status checks for only the last commit, so there should only be one node present
16891690
const statusCheckRollup = result.data.repository.pullRequest.commits.nodes[0].commit.statusCheckRollup;
16901691

1691-
const checks: PullRequestChecks = !statusCheckRollup
1692-
? {
1692+
let checks: PullRequestChecks;
1693+
if (!statusCheckRollup) {
1694+
checks = {
16931695
state: CheckState.Success,
16941696
statuses: []
1695-
}
1696-
: {
1697-
state: this.mapStateAsCheckState(statusCheckRollup.state),
1698-
statuses: statusCheckRollup.contexts.nodes.map(context => {
1699-
if (isCheckRun(context)) {
1700-
return {
1701-
id: context.id,
1702-
url: context.checkSuite?.app?.url,
1703-
avatarUrl:
1704-
context.checkSuite?.app?.logoUrl &&
1705-
getAvatarWithEnterpriseFallback(
1706-
context.checkSuite.app.logoUrl,
1707-
undefined,
1708-
this.remote.isEnterprise,
1709-
),
1710-
state: this.mapStateAsCheckState(context.conclusion),
1711-
description: context.title,
1712-
context: context.name,
1713-
workflowName: context.checkSuite?.workflowRun?.workflow.name,
1714-
event: context.checkSuite?.workflowRun?.event,
1715-
targetUrl: context.detailsUrl,
1716-
isRequired: context.isRequired,
1717-
};
1718-
} else {
1719-
return {
1720-
id: context.id,
1721-
url: context.targetUrl ?? undefined,
1722-
avatarUrl: context.avatarUrl
1723-
? getAvatarWithEnterpriseFallback(context.avatarUrl, undefined, this.remote.isEnterprise)
1724-
: undefined,
1725-
state: this.mapStateAsCheckState(context.state),
1726-
description: context.description,
1727-
context: context.context,
1728-
workflowName: undefined,
1729-
event: undefined,
1730-
targetUrl: context.targetUrl,
1731-
isRequired: context.isRequired,
1732-
};
1733-
}
1734-
}),
17351697
};
1698+
} else {
1699+
const dedupedStatuses = this.deduplicateStatusChecks(statusCheckRollup.contexts.nodes.map(context => {
1700+
if (isCheckRun(context)) {
1701+
return {
1702+
id: context.id,
1703+
url: context.checkSuite?.app?.url,
1704+
avatarUrl:
1705+
context.checkSuite?.app?.logoUrl &&
1706+
getAvatarWithEnterpriseFallback(
1707+
context.checkSuite.app.logoUrl,
1708+
undefined,
1709+
this.remote.isEnterprise,
1710+
),
1711+
state: this.mapStateAsCheckState(context.conclusion),
1712+
description: context.title,
1713+
context: context.name,
1714+
workflowName: context.checkSuite?.workflowRun?.workflow.name,
1715+
event: context.checkSuite?.workflowRun?.event,
1716+
targetUrl: context.detailsUrl,
1717+
isRequired: context.isRequired,
1718+
};
1719+
} else {
1720+
return {
1721+
id: context.id,
1722+
url: context.targetUrl ?? undefined,
1723+
avatarUrl: context.avatarUrl
1724+
? getAvatarWithEnterpriseFallback(context.avatarUrl, undefined, this.remote.isEnterprise)
1725+
: undefined,
1726+
state: this.mapStateAsCheckState(context.state),
1727+
description: context.description,
1728+
context: context.context,
1729+
workflowName: undefined,
1730+
event: undefined,
1731+
targetUrl: context.targetUrl,
1732+
isRequired: context.isRequired,
1733+
};
1734+
}
1735+
}));
1736+
1737+
checks = {
1738+
state: this.computeOverallCheckState(dedupedStatuses),
1739+
statuses: dedupedStatuses
1740+
};
1741+
}
17361742

17371743
let reviewRequirement: PullRequestReviewRequirement | null = null;
17381744
const rule = result.data.repository.pullRequest.baseRef?.refUpdateRule;
@@ -1807,4 +1813,74 @@ export class GitHubRepository extends Disposable {
18071813

18081814
return CheckState.Unknown;
18091815
}
1816+
1817+
/**
1818+
* Deduplicate status checks by context (check name).
1819+
* When a check is re-run on the same commit (e.g., when a PR is closed and reopened),
1820+
* GitHub's API returns all check run instances. This method keeps only one entry per
1821+
* check context, preferring pending/running checks over completed ones, and the most
1822+
* recent completed check when all are completed.
1823+
*/
1824+
private deduplicateStatusChecks(statuses: PullRequestCheckStatus[]): PullRequestCheckStatus[] {
1825+
const statusByContext = new Map<string, PullRequestCheckStatus>();
1826+
1827+
for (const status of statuses) {
1828+
const existing = statusByContext.get(status.context);
1829+
if (!existing) {
1830+
statusByContext.set(status.context, status);
1831+
continue;
1832+
}
1833+
1834+
// Prefer pending/unknown checks over completed ones (they represent the latest run)
1835+
const existingIsPending = existing.state === CheckState.Pending || existing.state === CheckState.Unknown;
1836+
const currentIsPending = status.state === CheckState.Pending || status.state === CheckState.Unknown;
1837+
1838+
if (currentIsPending && !existingIsPending) {
1839+
// Current is pending, existing is completed - prefer current
1840+
statusByContext.set(status.context, status);
1841+
} else if (!currentIsPending && existingIsPending) {
1842+
// Current is completed, existing is pending - keep existing
1843+
continue;
1844+
} else {
1845+
// Both are same type (both pending or both completed)
1846+
// Prefer the one with a higher ID (more recent), as GitHub IDs are monotonically increasing
1847+
if (status.id > existing.id) {
1848+
statusByContext.set(status.context, status);
1849+
}
1850+
}
1851+
}
1852+
1853+
return Array.from(statusByContext.values());
1854+
}
1855+
1856+
/**
1857+
* Compute the overall check state from individual status checks.
1858+
* - If any check has failed, the overall state is failure
1859+
* - If any check is pending/unknown (and none have failed), the overall state is pending
1860+
* - If all checks are successful or neutral/skipped, the overall state is success
1861+
*/
1862+
private computeOverallCheckState(statuses: PullRequestCheckStatus[]): CheckState {
1863+
if (statuses.length === 0) {
1864+
return CheckState.Success;
1865+
}
1866+
1867+
let hasFailure = false;
1868+
let hasPending = false;
1869+
1870+
for (const status of statuses) {
1871+
if (status.state === CheckState.Failure) {
1872+
hasFailure = true;
1873+
} else if (status.state === CheckState.Pending || status.state === CheckState.Unknown) {
1874+
hasPending = true;
1875+
}
1876+
}
1877+
1878+
if (hasFailure) {
1879+
return CheckState.Failure;
1880+
}
1881+
if (hasPending) {
1882+
return CheckState.Pending;
1883+
}
1884+
return CheckState.Success;
1885+
}
18101886
}

webviews/components/merge.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ const StatusChecks = ({ pr }: { pr: PullRequest }) => {
6666
toggleDetails();
6767
}
6868
}
69-
}, status?.statuses);
69+
}, [status?.statuses]);
7070

7171
return state === GithubItemStateEnum.Open && status?.statuses.length ? (
7272
<>
@@ -589,7 +589,8 @@ const CHECK_STATE_ORDER: Record<CheckState, number> = {
589589

590590
const StatusCheckDetails = ({ statuses }: { statuses: PullRequestCheckStatus[] }) => {
591591
// Sort statuses to group by state: failure first, then pending, neutral, and success
592-
const sortedStatuses = statuses.sort((a, b) => {
592+
// Use slice() to avoid mutating the original array
593+
const sortedStatuses = statuses.slice().sort((a, b) => {
593594
return CHECK_STATE_ORDER[a.state] - CHECK_STATE_ORDER[b.state];
594595
});
595596

0 commit comments

Comments
 (0)