Skip to content

More commit queue fixes #1158

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
panva merged 2 commits into from
Aug 10, 2026
Merged

More commit queue fixes #1158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d141b0a
fix: filter check suites to GitHub Actions
panva Aug 10, 2026
aae09fd
feat: report GraphQL error details
panva Aug 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions components/git/metadata.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <identifier>';
export const describe =
Expand Down Expand Up @@ -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;
}
}
Expand Down
16 changes: 5 additions & 11 deletions lib/pr_checker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
});
Expand Down
9 changes: 5 additions & 4 deletions lib/queries/PR.gql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) {

@aduh95 aduh95 Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should solve the issue of phantom Jenkins jobs, right?

@panva panva Aug 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, but it's possible?. We still process the same suites as before downstream because we used to filter by app.slug amongst the results, now we just filter by the appId behind that slug server side.

nodes {
app {
slug
}
conclusion,
status,
checkRuns(first: 40) {
Expand Down
5 changes: 3 additions & 2 deletions lib/request.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
11 changes: 9 additions & 2 deletions lib/run.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}
Expand All @@ -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();
});
Expand Down
2 changes: 0 additions & 2 deletions test/fixtures/github-ci/both-apis-failure.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
"checkSuites": {
"nodes": [
{
"app": { "slug": "github-actions" },
"status": "COMPLETED",
"conclusion": "FAILURE"
}
Expand All @@ -22,4 +21,3 @@
}
}
]

2 changes: 0 additions & 2 deletions test/fixtures/github-ci/both-apis-success.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
"checkSuites": {
"nodes": [
{
"app": { "slug": "github-actions" },
"status": "COMPLETED",
"conclusion": "SUCCESS"
}
Expand All @@ -22,4 +21,3 @@
}
}
]

1 change: 0 additions & 1 deletion test/fixtures/github-ci/check-suite-cancelled.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"checkSuites": {
"nodes": [
{
"app": { "slug": "github-actions" },
"status": "COMPLETED",
"conclusion": "CANCELLED",
"checkRuns": {
Expand Down
1 change: 0 additions & 1 deletion test/fixtures/github-ci/check-suite-failure.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"checkSuites": {
"nodes": [
{
"app": { "slug": "github-actions" },
"status": "COMPLETED",
"conclusion": "FAILURE",
"checkRuns": {
Expand Down
2 changes: 0 additions & 2 deletions test/fixtures/github-ci/check-suite-pending.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,10 @@
"checkSuites": {
"nodes": [
{
"app": { "slug": "github-actions" },
"status": "IN_PROGRESS"
}
]
}
}
}
]

1 change: 0 additions & 1 deletion test/fixtures/github-ci/check-suite-skipped.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"checkSuites": {
"nodes": [
{
"app": { "slug": "github-actions" },
"status": "COMPLETED",
"conclusion": "SKIPPED"
}
Expand Down
1 change: 0 additions & 1 deletion test/fixtures/github-ci/check-suite-success.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"checkSuites": {
"nodes": [
{
"app": { "slug": "github-actions" },
"status": "COMPLETED",
"conclusion": "SUCCESS"
}
Expand Down
2 changes: 0 additions & 2 deletions test/fixtures/github-ci/status-failure-check-suite-succeed.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
"checkSuites": {
"nodes": [
{
"app": { "slug": "github-actions" },
"status": "COMPLETED",
"conclusion": "SUCCESS"
}
Expand All @@ -22,4 +21,3 @@
}
}
]

2 changes: 0 additions & 2 deletions test/fixtures/github-ci/status-succeed-check-suite-failure.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
"checkSuites": {
"nodes": [
{
"app": { "slug": "github-actions" },
"status": "COMPLETED",
"conclusion": "FAILURE"
}
Expand All @@ -22,4 +21,3 @@
}
}
]

29 changes: 0 additions & 29 deletions test/fixtures/github-ci/success-dependabot-queued.json
View file
Open in desktop

This file was deleted.

5 changes: 4 additions & 1 deletion test/unit/graphql_queries.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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*\{/);
Expand Down
28 changes: 0 additions & 28 deletions test/unit/pr_checker.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -2009,7 +1987,6 @@ describe('PRChecker', () => {
author: { login: 'foo' },
checkSuites: {
nodes: [{
app: { slug: 'github-actions' },
status: 'COMPLETED',
conclusion: 'FAILURE',
checkRuns: {
Expand Down Expand Up @@ -2066,7 +2043,6 @@ describe('PRChecker', () => {
author: { login: 'foo' },
checkSuites: {
nodes: [{
app: { slug: 'github-actions' },
status: 'COMPLETED',
conclusion: 'FAILURE',
checkRuns: {
Expand Down Expand Up @@ -2118,7 +2094,6 @@ describe('PRChecker', () => {
author: { login: 'foo' },
checkSuites: {
nodes: [{
app: { slug: 'github-actions' },
status: 'COMPLETED',
conclusion: 'CANCELLED'
// No checkRuns field
Expand Down Expand Up @@ -2153,7 +2128,6 @@ describe('PRChecker', () => {
author: { login: 'foo' },
checkSuites: {
nodes: [{
app: { slug: 'github-actions' },
status: 'COMPLETED',
conclusion: 'FAILURE',
checkRuns: { nodes: [] }
Expand Down Expand Up @@ -2188,7 +2162,6 @@ describe('PRChecker', () => {
author: { login: 'foo' },
checkSuites: {
nodes: [{
app: { slug: 'github-actions' },
status: 'COMPLETED',
conclusion: 'FAILURE',
checkRuns: {
Expand Down Expand Up @@ -2230,7 +2203,6 @@ describe('PRChecker', () => {
author: { login: 'foo' },
checkSuites: {
nodes: [{
app: { slug: 'github-actions' },
status: 'COMPLETED',
conclusion: 'FAILURE',
checkRuns: {
Expand Down
1 change: 0 additions & 1 deletion test/unit/pr_data.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ const headCommitChecks = {
oid: oddCommits.at(-1).commit.oid,
checkSuites: {
nodes: [{
app: { slug: 'github-actions' },
status: 'COMPLETED',
conclusion: 'SUCCESS',
checkRuns: { nodes: [] }
Expand Down
Loading
Loading