From d141b0a7799f58369c15b5d4d62649cf6a3e28e8 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Mon, 10 Aug 2026 16:01:20 +0200 Subject: [PATCH 1/2] fix: filter check suites to GitHub Actions Filter check suites by the GitHub Actions App ID rather than requesting App metadata for every suite, which GITHUB_TOKEN cannot access for third-party Apps. Assisted-by: codex:gpt-5.6-sol Signed-off-by: Filip Skokan --- lib/pr_checker.js | 16 ++++------ lib/queries/PR.gql | 9 +++--- .../fixtures/github-ci/both-apis-failure.json | 2 -- .../fixtures/github-ci/both-apis-success.json | 2 -- .../github-ci/check-suite-cancelled.json | 1 - .../github-ci/check-suite-failure.json | 1 - .../github-ci/check-suite-pending.json | 2 -- .../github-ci/check-suite-skipped.json | 1 - .../github-ci/check-suite-success.json | 1 - .../status-failure-check-suite-succeed.json | 2 -- .../status-succeed-check-suite-failure.json | 2 -- .../github-ci/success-dependabot-queued.json | 29 ------------------- test/unit/graphql_queries.test.js | 5 +++- test/unit/pr_checker.test.js | 28 ------------------ test/unit/pr_data.test.js | 1 - 15 files changed, 14 insertions(+), 88 deletions(-) delete mode 100644 test/fixtures/github-ci/success-dependabot-queued.json diff --git a/lib/pr_checker.js b/lib/pr_checker.js index c4058f12..5e41eb8b 100644 --- a/lib/pr_checker.js +++ b/lib/pr_checker.js @@ -24,6 +24,7 @@ const WAIT_TIME_MULTI_APPROVAL = 24 * 2; const WAIT_TIME_SINGLE_APPROVAL = 24 * 7; const GITHUB_SUCCESS_CONCLUSIONS = ['SUCCESS', 'NEUTRAL', 'SKIPPED']; +const GITHUB_ACTIONS_APP = 'github-actions'; const FAST_TRACK_RE = /^Fast-track has been requested by @(.+?)\. Please 👍 to approve\.$/; const FAST_TRACK_MIN_APPROVALS = 2; @@ -459,16 +460,9 @@ export default class PRChecker { const pendingJobs = []; // GitHub new Check API - for (const { status, conclusion, app, checkRuns } of checkSuites.nodes) { - if (app.slug !== 'github-actions') { - // Ignore all non-github check suites, such as Dependabot and Codecov. - // They are expected to show up on PRs whose head branch is not on a - // fork and never complete. - continue; - } - + for (const { status, conclusion, checkRuns } of checkSuites.nodes) { if (status !== 'COMPLETED') { - pendingJobs.push({ app: app.slug, status, conclusion }); + pendingJobs.push({ status, conclusion }); continue; } @@ -499,13 +493,13 @@ export default class PRChecker { // Fallback to check suite level information if no checkRuns if (conclusion === 'CANCELLED') { cancelledJobs.push({ - name: app.slug, + name: GITHUB_ACTIONS_APP, conclusion, url: null }); } else { failedJobs.push({ - name: app.slug, + name: GITHUB_ACTIONS_APP, conclusion, url: null }); diff --git a/lib/queries/PR.gql b/lib/queries/PR.gql index 824e5c5a..10400af4 100644 --- a/lib/queries/PR.gql +++ b/lib/queries/PR.gql @@ -29,11 +29,12 @@ query PR($prid: Int!, $owner: String!, $repo: String!) { nodes { commit { oid - checkSuites(first: 100) { + # 15368 is the GitHub Actions App ID on GitHub.com. Filtering avoids + # querying App metadata that GITHUB_TOKEN cannot read for suites + # created by third-party Apps such as Jenkins and Codecov. + # https://api.github.com/apps/github-actions + checkSuites(first: 100, filterBy: { appId: 15368 }) { nodes { - app { - slug - } conclusion, status, checkRuns(first: 40) { diff --git a/test/fixtures/github-ci/both-apis-failure.json b/test/fixtures/github-ci/both-apis-failure.json index aa552cce..391c8c4f 100644 --- a/test/fixtures/github-ci/both-apis-failure.json +++ b/test/fixtures/github-ci/both-apis-failure.json @@ -13,7 +13,6 @@ "checkSuites": { "nodes": [ { - "app": { "slug": "github-actions" }, "status": "COMPLETED", "conclusion": "FAILURE" } @@ -22,4 +21,3 @@ } } ] - diff --git a/test/fixtures/github-ci/both-apis-success.json b/test/fixtures/github-ci/both-apis-success.json index c58e7abc..10eb9b49 100644 --- a/test/fixtures/github-ci/both-apis-success.json +++ b/test/fixtures/github-ci/both-apis-success.json @@ -13,7 +13,6 @@ "checkSuites": { "nodes": [ { - "app": { "slug": "github-actions" }, "status": "COMPLETED", "conclusion": "SUCCESS" } @@ -22,4 +21,3 @@ } } ] - diff --git a/test/fixtures/github-ci/check-suite-cancelled.json b/test/fixtures/github-ci/check-suite-cancelled.json index b42d16d6..41f1c809 100644 --- a/test/fixtures/github-ci/check-suite-cancelled.json +++ b/test/fixtures/github-ci/check-suite-cancelled.json @@ -10,7 +10,6 @@ "checkSuites": { "nodes": [ { - "app": { "slug": "github-actions" }, "status": "COMPLETED", "conclusion": "CANCELLED", "checkRuns": { diff --git a/test/fixtures/github-ci/check-suite-failure.json b/test/fixtures/github-ci/check-suite-failure.json index d07172f0..f2bec367 100644 --- a/test/fixtures/github-ci/check-suite-failure.json +++ b/test/fixtures/github-ci/check-suite-failure.json @@ -10,7 +10,6 @@ "checkSuites": { "nodes": [ { - "app": { "slug": "github-actions" }, "status": "COMPLETED", "conclusion": "FAILURE", "checkRuns": { diff --git a/test/fixtures/github-ci/check-suite-pending.json b/test/fixtures/github-ci/check-suite-pending.json index c09d11bf..afa6e2ec 100644 --- a/test/fixtures/github-ci/check-suite-pending.json +++ b/test/fixtures/github-ci/check-suite-pending.json @@ -10,7 +10,6 @@ "checkSuites": { "nodes": [ { - "app": { "slug": "github-actions" }, "status": "IN_PROGRESS" } ] @@ -18,4 +17,3 @@ } } ] - diff --git a/test/fixtures/github-ci/check-suite-skipped.json b/test/fixtures/github-ci/check-suite-skipped.json index 6a2cb755..42012526 100644 --- a/test/fixtures/github-ci/check-suite-skipped.json +++ b/test/fixtures/github-ci/check-suite-skipped.json @@ -10,7 +10,6 @@ "checkSuites": { "nodes": [ { - "app": { "slug": "github-actions" }, "status": "COMPLETED", "conclusion": "SKIPPED" } diff --git a/test/fixtures/github-ci/check-suite-success.json b/test/fixtures/github-ci/check-suite-success.json index 1dc5f395..ed7f3d6b 100644 --- a/test/fixtures/github-ci/check-suite-success.json +++ b/test/fixtures/github-ci/check-suite-success.json @@ -10,7 +10,6 @@ "checkSuites": { "nodes": [ { - "app": { "slug": "github-actions" }, "status": "COMPLETED", "conclusion": "SUCCESS" } diff --git a/test/fixtures/github-ci/status-failure-check-suite-succeed.json b/test/fixtures/github-ci/status-failure-check-suite-succeed.json index 28faa58c..13bb9ab5 100644 --- a/test/fixtures/github-ci/status-failure-check-suite-succeed.json +++ b/test/fixtures/github-ci/status-failure-check-suite-succeed.json @@ -13,7 +13,6 @@ "checkSuites": { "nodes": [ { - "app": { "slug": "github-actions" }, "status": "COMPLETED", "conclusion": "SUCCESS" } @@ -22,4 +21,3 @@ } } ] - diff --git a/test/fixtures/github-ci/status-succeed-check-suite-failure.json b/test/fixtures/github-ci/status-succeed-check-suite-failure.json index 6a336cd8..5c4801c7 100644 --- a/test/fixtures/github-ci/status-succeed-check-suite-failure.json +++ b/test/fixtures/github-ci/status-succeed-check-suite-failure.json @@ -13,7 +13,6 @@ "checkSuites": { "nodes": [ { - "app": { "slug": "github-actions" }, "status": "COMPLETED", "conclusion": "FAILURE" } @@ -22,4 +21,3 @@ } } ] - diff --git a/test/fixtures/github-ci/success-dependabot-queued.json b/test/fixtures/github-ci/success-dependabot-queued.json deleted file mode 100644 index 557452f5..00000000 --- a/test/fixtures/github-ci/success-dependabot-queued.json +++ /dev/null @@ -1,29 +0,0 @@ -[ - { - "commit": { - "committedDate": "2017-10-26T12:10:20Z", - "oid": "9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d", - "messageHeadline": "doc: add api description README", - "author": { - "login": "foo" - }, - "checkSuites": { - "nodes": [ - { - "app": { - "slug": "dependabot" - }, - "status": "QUEUED", - "conclusion": null - }, - { - "app": { "slug": "github-actions" }, - "status": "COMPLETED", - "conclusion": "SUCCESS" - } - ] - } - } - } -] - diff --git a/test/unit/graphql_queries.test.js b/test/unit/graphql_queries.test.js index 9b8d750e..39a31bcc 100644 --- a/test/unit/graphql_queries.test.js +++ b/test/unit/graphql_queries.test.js @@ -16,9 +16,12 @@ describe('GraphQL queries', () => { assert.notStrictEqual(headCommitStart, -1); assert.notStrictEqual(headCommitEnd, -1); - assert.match(headCommitQuery, /checkSuites\(first: 100\)/); + assert.match( + headCommitQuery, + /checkSuites\(first: 100, filterBy: \{ appId: 15368 \}\)/); assert.match(headCommitQuery, /checkRuns\(first: 40\)/); assert.match(headCommitQuery, /status \{\s+state\s+\}/); + assert.doesNotMatch(headCommitQuery, /\bapp\s*\{/); assert.doesNotMatch(commitsQuery, /checkSuites/); assert.doesNotMatch(commitsQuery, /checkRuns/); assert.doesNotMatch(commitsQuery, /\sstatus\s*\{/); diff --git a/test/unit/pr_checker.test.js b/test/unit/pr_checker.test.js index 109fc2cf..a64c4855 100644 --- a/test/unit/pr_checker.test.js +++ b/test/unit/pr_checker.test.js @@ -1745,28 +1745,6 @@ describe('PRChecker', () => { cli.assertCalledWith(expectedLogs); }); - it( - 'should succeed if status succeeded with queued Dependabot check', - async() => { - const cli = new TestCLI(); - - const expectedLogs = { - ok: [ - ['Last GitHub CI successful'] - ] - }; - - const commits = githubCI['success-dependabot-queued']; - 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 error if Check suite failed', async() => { const cli = new TestCLI(); @@ -2009,7 +1987,6 @@ describe('PRChecker', () => { author: { login: 'foo' }, checkSuites: { nodes: [{ - app: { slug: 'github-actions' }, status: 'COMPLETED', conclusion: 'FAILURE', checkRuns: { @@ -2066,7 +2043,6 @@ describe('PRChecker', () => { author: { login: 'foo' }, checkSuites: { nodes: [{ - app: { slug: 'github-actions' }, status: 'COMPLETED', conclusion: 'FAILURE', checkRuns: { @@ -2118,7 +2094,6 @@ describe('PRChecker', () => { author: { login: 'foo' }, checkSuites: { nodes: [{ - app: { slug: 'github-actions' }, status: 'COMPLETED', conclusion: 'CANCELLED' // No checkRuns field @@ -2153,7 +2128,6 @@ describe('PRChecker', () => { author: { login: 'foo' }, checkSuites: { nodes: [{ - app: { slug: 'github-actions' }, status: 'COMPLETED', conclusion: 'FAILURE', checkRuns: { nodes: [] } @@ -2188,7 +2162,6 @@ describe('PRChecker', () => { author: { login: 'foo' }, checkSuites: { nodes: [{ - app: { slug: 'github-actions' }, status: 'COMPLETED', conclusion: 'FAILURE', checkRuns: { @@ -2230,7 +2203,6 @@ describe('PRChecker', () => { author: { login: 'foo' }, checkSuites: { nodes: [{ - app: { slug: 'github-actions' }, status: 'COMPLETED', conclusion: 'FAILURE', checkRuns: { diff --git a/test/unit/pr_data.test.js b/test/unit/pr_data.test.js index 7123845a..5fa1b528 100644 --- a/test/unit/pr_data.test.js +++ b/test/unit/pr_data.test.js @@ -27,7 +27,6 @@ const headCommitChecks = { oid: oddCommits.at(-1).commit.oid, checkSuites: { nodes: [{ - app: { slug: 'github-actions' }, status: 'COMPLETED', conclusion: 'SUCCESS', checkRuns: { nodes: [] } From aae09fd82091b7cddd6b7a3b31fc2548eb1fce95 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Mon, 10 Aug 2026 16:01:58 +0200 Subject: [PATCH 2/2] feat: report GraphQL error details Preserve all GraphQL errors and render their structured data as JSON so paths, locations, extensions, and additional errors remain visible in command logs. Assisted-by: codex:gpt-5.6-sol Signed-off-by: Filip Skokan --- components/git/metadata.js | 4 +-- lib/request.js | 5 +-- lib/run.js | 11 +++++-- test/unit/request.test.js | 65 ++++++++++++++++++++++++++++++++++++++ test/unit/run.test.js | 38 ++++++++++++++++++++++ 5 files changed, 117 insertions(+), 6 deletions(-) create mode 100644 test/unit/request.test.js create mode 100644 test/unit/run.test.js diff --git a/components/git/metadata.js b/components/git/metadata.js index 7f974b53..c1812440 100644 --- a/components/git/metadata.js +++ b/components/git/metadata.js @@ -2,7 +2,7 @@ import { parsePRFromURL } from '../../lib/links.js'; import { getMetadata } from '../metadata.js'; import CLI from '../../lib/cli.js'; import { getMergedConfig } from '../../lib/config.js'; -import { runPromise, IGNORE } from '../../lib/run.js'; +import { runPromise, reportError, IGNORE } from '../../lib/run.js'; export const command = 'metadata '; export const describe = @@ -67,7 +67,7 @@ export async function writeMetadataJsonResult(metadataPromise) { await writeStdout(`${JSON.stringify(json, null, 2)}\n`); process.exitCode = json.exitCode; } catch (error) { - console.error(error); + reportError(error); process.exitCode = 1; } } diff --git a/lib/request.js b/lib/request.js index 4e2cf5dc..f7396690 100644 --- a/lib/request.js +++ b/lib/request.js @@ -335,11 +335,12 @@ export default class Request { }; const result = await this.json(url, options); - if (result.errors) { + if (result.errors?.length > 0) { const { type, message } = result.errors[0]; const err = new Error(`[${type}] GraphQL request Error: ${message}`); err.data = { - variables + variables, + errors: result.errors }; throw err; } diff --git a/lib/run.js b/lib/run.js index c109b2fe..12462c91 100644 --- a/lib/run.js +++ b/lib/run.js @@ -8,6 +8,13 @@ import { export const IGNORE = '__ignore__'; +export function reportError(error, write = console.error) { + write(error?.stack ?? error); + if (error?.data !== undefined) { + write(JSON.stringify(error.data, null, 2)); + } +} + function runAsyncBase(cmd, args, { ignoreFailure = true, spawnArgs, @@ -72,7 +79,7 @@ export function forceRunAsync(cmd, args, options) { return runAsyncBase(cmd, args, options).catch((error) => { if (error.message !== IGNORE) { if (!error.messageOnly) { - console.error(error); + reportError(error); } throw error; } @@ -82,7 +89,7 @@ export function forceRunAsync(cmd, args, options) { export function runPromise(promise) { return promise.catch((error) => { if (error.message !== IGNORE) { - console.error(error); + reportError(error); } exit(); }); diff --git a/test/unit/request.test.js b/test/unit/request.test.js new file mode 100644 index 00000000..349bb61c --- /dev/null +++ b/test/unit/request.test.js @@ -0,0 +1,65 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert'; + +import Request from '../../lib/request.js'; + +function createRequest(response) { + const request = Object.create(Request.prototype); + request.credentials = { github: 'credentials' }; + request.proxyAgent = undefined; + request.json = async() => response; + return request; +} + +describe('Request', () => { + describe('query', () => { + it('preserves detailed GraphQL errors', async() => { + const variables = { owner: 'nodejs', repo: 'node', prid: 65130 }; + const errors = [ + { + type: 'FORBIDDEN', + path: [ + 'repository', 'pullRequest', 'commits', 'nodes', 0, 'commit', + 'checkSuites', 'edges', 4, 'node', 'app' + ], + extensions: { saml_failure: false }, + locations: [{ line: 26, column: 7 }], + message: 'Resource not accessible by integration' + }, + { + type: 'FORBIDDEN', + path: ['repository', 'pullRequest', 'files'], + locations: [{ line: 42, column: 5 }], + message: 'A second error' + } + ]; + const request = createRequest({ errors }); + + await assert.rejects( + request.query('query PR { pullRequest { id } }', variables), + (error) => { + assert.strictEqual( + error.message, + '[FORBIDDEN] GraphQL request Error: ' + + 'Resource not accessible by integration'); + assert.deepStrictEqual(error.data, { variables, errors }); + return true; + }); + }); + + it('preserves top-level GraphQL API errors', async() => { + const variables = { owner: 'nodejs', repo: 'node', prid: 65130 }; + const request = createRequest({ message: 'Bad credentials' }); + + await assert.rejects( + request.query('query PR { pullRequest { id } }', variables), + (error) => { + assert.strictEqual( + error.message, + 'GraphQL request Error: Bad credentials'); + assert.deepStrictEqual(error.data, { variables }); + return true; + }); + }); + }); +}); diff --git a/test/unit/run.test.js b/test/unit/run.test.js new file mode 100644 index 00000000..f3a19702 --- /dev/null +++ b/test/unit/run.test.js @@ -0,0 +1,38 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert'; + +import { reportError } from '../../lib/run.js'; + +describe('reportError', () => { + it('renders structured error data as formatted JSON', () => { + const error = new Error( + '[FORBIDDEN] GraphQL request Error: Resource not accessible'); + error.data = { + variables: { owner: 'nodejs', repo: 'node', prid: 65130 }, + errors: [{ + type: 'FORBIDDEN', + path: ['repository', 'pullRequest', 'checkSuites', 0, 'app'], + locations: [{ line: 26, column: 7 }], + extensions: { saml_failure: false }, + message: 'Resource not accessible' + }] + }; + const output = []; + + reportError(error, (value) => output.push(value)); + + assert.deepStrictEqual(output, [ + error.stack, + JSON.stringify(error.data, null, 2) + ]); + }); + + it('renders errors without structured data once', () => { + const error = new Error('boom'); + const output = []; + + reportError(error, (value) => output.push(value)); + + assert.deepStrictEqual(output, [error.stack]); + }); +});