From 1dd3e16d8da28c862645203b6faab15c51a869ef Mon Sep 17 00:00:00 2001 From: Avocado Date: Mon, 14 Sep 2026 16:10:31 +0900 Subject: [PATCH] fix: ignore check suites superseded by a later run A commit keeps every check suite ever attached to it, so a branch that is force-pushed away from a SHA and back again ends up with one suite per run. checkGitHubCI() treated every non-successful suite as a current failure, so a cancelled run that a later run had already replaced kept the commit queue from accepting the pull request. Select only the latest run of each workflow before inspecting suites. Runs triggered by different events do not supersede one another, so the event is part of the key. Suites without run metadata cannot be compared and are kept as they were. Fixes: https://github.com/nodejs/node-core-utils/issues/1151 Signed-off-by: Avocado --- lib/pr_checker.js | 30 ++++++++- lib/queries/PR.gql | 7 ++ test/unit/pr_checker.test.js | 127 +++++++++++++++++++++++++++++++++++ 3 files changed, 163 insertions(+), 1 deletion(-) diff --git a/lib/pr_checker.js b/lib/pr_checker.js index 5e41eb8b..b45c8e76 100644 --- a/lib/pr_checker.js +++ b/lib/pr_checker.js @@ -26,6 +26,33 @@ const WAIT_TIME_SINGLE_APPROVAL = 24 * 7; const GITHUB_SUCCESS_CONCLUSIONS = ['SUCCESS', 'NEUTRAL', 'SKIPPED']; const GITHUB_ACTIONS_APP = 'github-actions'; +// A commit keeps every check suite ever attached to it, so a branch that is +// force-pushed away from a SHA and back again accumulates one suite per run. +// Runs of the same workflow supersede one another, but runs triggered by +// different events do not, so both make up the key. +function selectLatestCheckSuites(nodes) { + const latest = new Map(); + const unkeyed = []; + + for (const suite of nodes) { + const { workflowRun } = suite; + if (!workflowRun) { + // A suite without run metadata cannot be compared against another, so + // it is kept as is rather than dropped. + unkeyed.push(suite); + continue; + } + + const key = `${workflowRun.workflow.id}:${workflowRun.event}`; + const current = latest.get(key); + if (!current || workflowRun.runNumber > current.workflowRun.runNumber) { + latest.set(key, suite); + } + } + + return [...unkeyed, ...latest.values()]; +} + const FAST_TRACK_RE = /^Fast-track has been requested by @(.+?)\. Please 👍 to approve\.$/; const FAST_TRACK_MIN_APPROVALS = 2; const GIT_CONFIG_GUIDE_URL = 'https://github.com/nodejs/node/blob/99b1ada/doc/guides/contributing/pull-requests.md#step-1-fork'; @@ -460,7 +487,8 @@ export default class PRChecker { const pendingJobs = []; // GitHub new Check API - for (const { status, conclusion, checkRuns } of checkSuites.nodes) { + for (const { status, conclusion, checkRuns } of + selectLatestCheckSuites(checkSuites.nodes)) { if (status !== 'COMPLETED') { pendingJobs.push({ status, conclusion }); continue; diff --git a/lib/queries/PR.gql b/lib/queries/PR.gql index 10400af4..4c2631a8 100644 --- a/lib/queries/PR.gql +++ b/lib/queries/PR.gql @@ -37,6 +37,13 @@ query PR($prid: Int!, $owner: String!, $repo: String!) { nodes { conclusion, status, + workflowRun { + event + runNumber + workflow { + id + } + } checkRuns(first: 40) { nodes { name diff --git a/test/unit/pr_checker.test.js b/test/unit/pr_checker.test.js index a64c4855..0183f458 100644 --- a/test/unit/pr_checker.test.js +++ b/test/unit/pr_checker.test.js @@ -2117,6 +2117,133 @@ describe('PRChecker', () => { cli.assertCalledWith(expectedLogs); }); + it('should ignore a check suite superseded by a later run', async() => { + const cli = new TestCLI(); + + const supersededSuite = [{ + commit: { + committedDate: '2017-10-26T12:10:20Z', + oid: '9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d', + messageHeadline: 'doc: add api description README', + author: { login: 'foo' }, + checkSuites: { + nodes: [ + { + status: 'COMPLETED', + conclusion: 'CANCELLED', + workflowRun: { + event: 'pull_request', + runNumber: 1, + workflow: { id: 'workflow-1' } + }, + checkRuns: { + nodes: [{ + name: 'test-linux', + status: 'COMPLETED', + conclusion: 'CANCELLED', + detailsUrl: 'https://github.com/nodejs/node/runs/1' + }] + } + }, + { + status: 'COMPLETED', + conclusion: 'SUCCESS', + workflowRun: { + event: 'pull_request', + runNumber: 2, + workflow: { id: 'workflow-1' } + }, + checkRuns: { + nodes: [{ + name: 'test-linux', + status: 'COMPLETED', + conclusion: 'SUCCESS', + detailsUrl: 'https://github.com/nodejs/node/runs/2' + }] + } + } + ] + } + } + }]; + + const expectedLogs = { + ok: [['Last GitHub CI successful']] + }; + + const data = Object.assign({}, baseData, { commits: supersededSuite }); + const checker = new PRChecker(cli, data, {}, testArgv); + + const status = await checker.checkCI(); + assert(status); + cli.assertCalledWith(expectedLogs); + }); + + it('should not let a run supersede one from a different event', async() => { + const cli = new TestCLI(); + + const differentEvents = [{ + commit: { + committedDate: '2017-10-26T12:10:20Z', + oid: '9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d', + messageHeadline: 'doc: add api description README', + author: { login: 'foo' }, + checkSuites: { + nodes: [ + { + status: 'COMPLETED', + conclusion: 'FAILURE', + workflowRun: { + event: 'push', + runNumber: 1, + workflow: { id: 'workflow-1' } + }, + checkRuns: { + nodes: [{ + name: 'test-linux', + status: 'COMPLETED', + conclusion: 'FAILURE', + detailsUrl: 'https://github.com/nodejs/node/runs/1' + }] + } + }, + { + status: 'COMPLETED', + conclusion: 'SUCCESS', + workflowRun: { + event: 'pull_request', + runNumber: 2, + workflow: { id: 'workflow-1' } + }, + checkRuns: { + nodes: [{ + name: 'test-linux', + status: 'COMPLETED', + conclusion: 'SUCCESS', + detailsUrl: 'https://github.com/nodejs/node/runs/2' + }] + } + } + ] + } + } + }]; + + const expectedLogs = { + error: [ + ['1 GitHub CI job(s) failed:'], + [' - test-linux: FAILURE (https://github.com/nodejs/node/runs/1)'] + ] + }; + + const data = Object.assign({}, baseData, { commits: differentEvents }); + const checker = new PRChecker(cli, data, {}, testArgv); + + const status = await checker.checkCI(); + assert(!status); + cli.assertCalledWith(expectedLogs); + }); + it('should handle empty checkRuns array', async() => { const cli = new TestCLI();