From 41303bb28d5fb88417ed0ab4b4aa1844999d7902 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Thu, 27 Aug 2026 23:34:39 +0200 Subject: [PATCH] fix: ignore fully skipped startup failures --- lib/pr_checker.js | 79 ++++++++++++++++------------- lib/queries/PR.gql | 1 + test/unit/graphql_queries.test.js | 1 + test/unit/pr_checker.test.js | 82 +++++++++++++++++++++++++++++++ 4 files changed, 129 insertions(+), 34 deletions(-) diff --git a/lib/pr_checker.js b/lib/pr_checker.js index 5e41eb8b..b241a6cb 100644 --- a/lib/pr_checker.js +++ b/lib/pr_checker.js @@ -467,43 +467,54 @@ export default class PRChecker { } if (!GITHUB_SUCCESS_CONCLUSIONS.includes(conclusion)) { - hasFailures = true; + const runs = checkRuns?.nodes ?? []; + const allRunsSkipped = runs.length > 0 && + runs.length === checkRuns.totalCount && + runs.every((checkRun) => checkRun.status === 'COMPLETED' && + checkRun.conclusion === 'SKIPPED'); + + // GitHub can report a STARTUP_FAILURE for a workflow even though all + // of its conditional jobs were skipped. There is no failed check in + // that case, so the suite should not make the commit unlandable. + if (conclusion === 'STARTUP_FAILURE' && allRunsSkipped) { + continue; + } - // If we have detailed checkRuns, show specific failing jobs - if (checkRuns && checkRuns.nodes && checkRuns.nodes.length > 0) { - for (const checkRun of checkRuns.nodes) { - if (checkRun.status === 'COMPLETED' && - !GITHUB_SUCCESS_CONCLUSIONS.includes(checkRun.conclusion)) { - if (checkRun.conclusion === 'CANCELLED') { - cancelledJobs.push({ - name: checkRun.name, - conclusion: checkRun.conclusion, - url: checkRun.detailsUrl - }); - } else { - failedJobs.push({ - name: checkRun.name, - conclusion: checkRun.conclusion, - url: checkRun.detailsUrl - }); - } + hasFailures = true; + let reportedFailure = false; + + // If we have detailed checkRuns, show specific failing jobs. + for (const checkRun of runs) { + if (checkRun.status === 'COMPLETED' && + !GITHUB_SUCCESS_CONCLUSIONS.includes(checkRun.conclusion)) { + reportedFailure = true; + if (checkRun.conclusion === 'CANCELLED') { + cancelledJobs.push({ + name: checkRun.name, + conclusion: checkRun.conclusion, + url: checkRun.detailsUrl + }); + } else { + failedJobs.push({ + name: checkRun.name, + conclusion: checkRun.conclusion, + url: checkRun.detailsUrl + }); } } - } else { - // Fallback to check suite level information if no checkRuns - if (conclusion === 'CANCELLED') { - cancelledJobs.push({ - name: GITHUB_ACTIONS_APP, - conclusion, - url: null - }); - } else { - failedJobs.push({ - name: GITHUB_ACTIONS_APP, - conclusion, - url: null - }); - } + } + + // Fall back to the suite when its failed conclusion is not reflected + // by an individual check run. + if (!reportedFailure) { + const failures = conclusion === 'CANCELLED' + ? cancelledJobs + : failedJobs; + failures.push({ + name: GITHUB_ACTIONS_APP, + conclusion, + url: null + }); } } } diff --git a/lib/queries/PR.gql b/lib/queries/PR.gql index 10400af4..1aa7bdb2 100644 --- a/lib/queries/PR.gql +++ b/lib/queries/PR.gql @@ -38,6 +38,7 @@ query PR($prid: Int!, $owner: String!, $repo: String!) { conclusion, status, checkRuns(first: 40) { + totalCount nodes { name status diff --git a/test/unit/graphql_queries.test.js b/test/unit/graphql_queries.test.js index 39a31bcc..867f29d1 100644 --- a/test/unit/graphql_queries.test.js +++ b/test/unit/graphql_queries.test.js @@ -20,6 +20,7 @@ describe('GraphQL queries', () => { headCommitQuery, /checkSuites\(first: 100, filterBy: \{ appId: 15368 \}\)/); assert.match(headCommitQuery, /checkRuns\(first: 40\)/); + assert.match(headCommitQuery, /checkRuns\(first: 40\) \{\s+totalCount/); assert.match(headCommitQuery, /status \{\s+state\s+\}/); assert.doesNotMatch(headCommitQuery, /\bapp\s*\{/); assert.doesNotMatch(commitsQuery, /checkSuites/); diff --git a/test/unit/pr_checker.test.js b/test/unit/pr_checker.test.js index a64c4855..7ae9d5be 100644 --- a/test/unit/pr_checker.test.js +++ b/test/unit/pr_checker.test.js @@ -1860,6 +1860,88 @@ describe('PRChecker', () => { cli.assertCalledWith(expectedLogs); }); + it('should ignore startup failures with only skipped check runs', + async() => { + const cli = new TestCLI(); + + const expectedLogs = { + ok: [ + ['Last GitHub CI successful'] + ] + }; + + const commits = [{ + commit: { + checkSuites: { + nodes: [{ + status: 'COMPLETED', + conclusion: 'STARTUP_FAILURE', + checkRuns: { + totalCount: 2, + nodes: [ + { + name: 'stale-comment', + status: 'COMPLETED', + conclusion: 'SKIPPED' + }, + { + name: 'notable-change', + status: 'COMPLETED', + conclusion: 'SKIPPED' + } + ] + } + }] + } + } + }]; + const data = Object.assign({}, baseData, { commits }); + + const checker = new PRChecker(cli, data, {}, testArgv); + + const status = await checker.checkCI(); + assert(status); + cli.assertCalledWith(expectedLogs); + }); + + it('should report a suite failure when check runs are incomplete', + async() => { + const cli = new TestCLI(); + + const expectedLogs = { + error: [ + ['1 GitHub CI job(s) failed:'], + [' - github-actions: STARTUP_FAILURE'] + ] + }; + + const commits = [{ + commit: { + checkSuites: { + nodes: [{ + status: 'COMPLETED', + conclusion: 'STARTUP_FAILURE', + checkRuns: { + totalCount: 2, + nodes: [{ + name: 'stale-comment', + status: 'COMPLETED', + conclusion: 'SKIPPED' + }] + } + }] + } + } + }]; + const data = Object.assign({}, baseData, { commits }); + + const checker = new PRChecker(cli, data, {}, testArgv); + + const status = await checker.checkCI(); + assert(!status); + cli.assertCalledWith(expectedLogs); + }); + it('should succeed if commit status succeeded', async() => { const cli = new TestCLI();