From 46a249f8067cd1989c959fcb1eee2d7c932129c5 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sat, 24 Oct 2026 08:32:24 -0400 Subject: [PATCH 1/3] fix(pr-pipeline): read the ci-lanes status creator as GraphQL reports it fix-ci dispatch compared the ci-lanes status creator's login to github-actions[bot], the REST spelling. The GraphQL rollup reports the login as github-actions, so no head ever counted as failed and nothing was dispatched. Trust the creator by type and id instead (Bot 41898282), still refusing any other actor or a missing creator, and log why a head whose lanes failed is skipped so such a skip is visible. Test fixtures now use the creator shape read live from a pull request's ci-lanes status. Co-Authored-By: Claude Opus 5.5 --- .../fix-fix-ci-creator-login-7d2f4b91.md | 8 ++++ .../actions/resolve-config/callers.test.mjs | 6 +-- .../resolve-config/dispatch-limits.test.mjs | 2 +- .github/scripts/pr-unblock/dispatch.test.mjs | 2 +- .../scripts/pr-unblock/fix-ci-dispatch.mjs | 31 +++++++++---- .../pr-unblock/fix-ci-dispatch.test.mjs | 44 ++++++++++++++++--- 6 files changed, 74 insertions(+), 19 deletions(-) create mode 100644 .changes/pr-pipeline/fix-fix-ci-creator-login-7d2f4b91.md diff --git a/.changes/pr-pipeline/fix-fix-ci-creator-login-7d2f4b91.md b/.changes/pr-pipeline/fix-fix-ci-creator-login-7d2f4b91.md new file mode 100644 index 0000000000..6804976f03 --- /dev/null +++ b/.changes/pr-pipeline/fix-fix-ci-creator-login-7d2f4b91.md @@ -0,0 +1,8 @@ +--- +bump: patch +--- + +### Fixed + +- `fix-ci` dispatch trusts a `ci-lanes` commit status by its creator's type and id (the GitHub Actions bot, `Bot` 41898282) instead of the login `github-actions[bot]`, which GraphQL never returns (it reports `github-actions`); no pull request was dispatched while the login check stood. A status set by any other actor, or with no creator, is still ignored. +- The dispatcher logs why it skips a listed pull request whose `ci-lanes` status failed or errored, so a skip of that kind is no longer silent. diff --git a/.github/actions/resolve-config/callers.test.mjs b/.github/actions/resolve-config/callers.test.mjs index 28072912a7..1a4c821d37 100644 --- a/.github/actions/resolve-config/callers.test.mjs +++ b/.github/actions/resolve-config/callers.test.mjs @@ -595,7 +595,7 @@ test("pr-unblock.yml: fix-ci is reported neutral when fix-ci-context skips, and }, ] .map((run) => ({ __typename: "CheckRun", checkSuite: { app: { databaseId: 15368 } }, ...run })) - .concat({ __typename: "StatusContext", context: "ci-lanes", state: "FAILURE", creator: { login: "github-actions[bot]" } }), + .concat({ __typename: "StatusContext", context: "ci-lanes", state: "FAILURE", creator: { __typename: "Bot", login: "github-actions", databaseId: 41898282 } }), pageInfo: { hasNextPage: false }, }, }, @@ -3674,7 +3674,7 @@ test("report job: fix-ci-dispatch reads the neutral not-assessed post as the hea contexts: { nodes: [{ name: "ci-status", status: "COMPLETED", conclusion: "FAILURE" }, ...runs] .map((run) => ({ __typename: "CheckRun", checkSuite: { app: { databaseId: 15368 } }, ...run })) - .concat({ __typename: "StatusContext", context: "ci-lanes", state: "FAILURE", creator: { login: "github-actions[bot]" } }), + .concat({ __typename: "StatusContext", context: "ci-lanes", state: "FAILURE", creator: { __typename: "Bot", login: "github-actions", databaseId: 41898282 } }), pageInfo: { hasNextPage: false }, }, }, @@ -5038,7 +5038,7 @@ test("pr-unblock.yml: a fix-ci round with no posted check posts fix-ci failure o })), ] .map((run) => ({ __typename: "CheckRun", checkSuite: { app: { databaseId: 15368 } }, ...run })) - .concat({ __typename: "StatusContext", context: "ci-lanes", state: "FAILURE", creator: { login: "github-actions[bot]" } }), + .concat({ __typename: "StatusContext", context: "ci-lanes", state: "FAILURE", creator: { __typename: "Bot", login: "github-actions", databaseId: 41898282 } }), pageInfo: { hasNextPage: false }, }, }, diff --git a/.github/actions/resolve-config/dispatch-limits.test.mjs b/.github/actions/resolve-config/dispatch-limits.test.mjs index 1272f64d8d..2309ef1390 100644 --- a/.github/actions/resolve-config/dispatch-limits.test.mjs +++ b/.github/actions/resolve-config/dispatch-limits.test.mjs @@ -47,7 +47,7 @@ const LANES_APP = 337595936; const ciFixPull = (number) => { const head = String(number).padStart(40, "c"); const ciStatus = { __typename: "CheckRun", name: "ci-status", status: "COMPLETED", conclusion: "FAILURE", checkSuite: { app: { databaseId: 15368 } } }; - const ciLanes = { __typename: "StatusContext", context: "ci-lanes", state: "FAILURE", creator: { login: "github-actions[bot]" } }; + const ciLanes = { __typename: "StatusContext", context: "ci-lanes", state: "FAILURE", creator: { __typename: "Bot", login: "github-actions", databaseId: 41898282 } }; return { number, isDraft: false, diff --git a/.github/scripts/pr-unblock/dispatch.test.mjs b/.github/scripts/pr-unblock/dispatch.test.mjs index f76be39571..a7819e2fb6 100644 --- a/.github/scripts/pr-unblock/dispatch.test.mjs +++ b/.github/scripts/pr-unblock/dispatch.test.mjs @@ -725,7 +725,7 @@ const ciFixPull = (number) => { contexts: { nodes: [ { __typename: "CheckRun", name: "ci-status", status: "COMPLETED", conclusion: "FAILURE", checkSuite: { app: { databaseId: 15368 } } }, - { __typename: "StatusContext", context: "ci-lanes", state: "FAILURE", creator: { login: "github-actions[bot]" } }, + { __typename: "StatusContext", context: "ci-lanes", state: "FAILURE", creator: { __typename: "Bot", login: "github-actions", databaseId: 41898282 } }, ], }, }, diff --git a/.github/scripts/pr-unblock/fix-ci-dispatch.mjs b/.github/scripts/pr-unblock/fix-ci-dispatch.mjs index 05f2457a75..aaacc85326 100644 --- a/.github/scripts/pr-unblock/fix-ci-dispatch.mjs +++ b/.github/scripts/pr-unblock/fix-ci-dispatch.mjs @@ -92,7 +92,7 @@ export const LIST_QUERY = `query($owner: String!, $name: String!, $base: String! nodes { __typename ... on CheckRun { name status conclusion title completedAt checkSuite { app { databaseId } } } - ... on StatusContext { context state creator { login } } + ... on StatusContext { context state creator { __typename ... on Bot { databaseId } } } } pageInfo { hasNextPage } } @@ -130,14 +130,22 @@ function ownerIdle(node, idleHours, now) { return commit?.oid === node.headRefOid && Date.parse(node.updatedAt) <= since && Date.parse(commit.committedDate) <= since; } -// The head's `ci-lanes` commit status when github-actions[bot] wrote it, as -// ci-status's aggregate step does: the lanes' verdict alone. ci-status itself -// also fails on a hold label, a pending or missing review and the PR -// contract, none of which a CI fix mends. -function lanesState(node) { +// The GitHub Actions bot's user id. GraphQL reports its login as +// `github-actions`, REST as `github-actions[bot]`; the id is the same in both. +const ACTIONS_BOT_ID = 41898282; + +// The head's `ci-lanes` commit status, whoever set it. +function lanesStatus(node) { const contexts = node.commits?.nodes?.[0]?.commit?.statusCheckRollup?.contexts?.nodes ?? []; - const status = contexts.find((context) => context?.__typename === "StatusContext" && context.context === "ci-lanes"); - return status?.creator?.login === "github-actions[bot]" ? status.state : null; + return contexts.find((context) => context?.__typename === "StatusContext" && context.context === "ci-lanes"); +} + +// The ci-lanes state when the GitHub Actions bot set it, else null: the +// lanes' verdict alone. ci-status itself also fails on a hold label, a +// pending or missing review and the PR contract, none of which a CI fix mends. +function lanesState(node) { + const status = lanesStatus(node); + return status?.creator?.__typename === "Bot" && status.creator.databaseId === ACTIONS_BOT_ID ? status.state : null; } const lanesFailed = (node) => ["FAILURE", "ERROR"].includes(lanesState(node)); @@ -274,8 +282,13 @@ export async function dispatchCiFixes({ log(`fix-ci listing page ${page}: rateLimit.cost=${rateLimit?.cost} remaining=${rateLimit?.remaining}`); baseAt ??= Date.parse(baseCommittedAt ?? ""); for (const node of pullRequests.nodes) { - if (skipReason(node, { base, appSenderId, actionsAppId, checkName, holdLabels, gateLabel, restarts, maxRunsPerHead, baseAt, adoption, now }) === "") { + const reason = skipReason(node, { base, appSenderId, actionsAppId, checkName, holdLabels, gateLabel, restarts, maxRunsPerHead, baseAt, adoption, now }); + if (reason === "") { candidates.push(node); + } else if (["FAILURE", "ERROR"].includes(lanesStatus(node)?.state)) { + // A head whose lanes failed, whoever set the status, is the one a + // silent skip would hide. + log(`skip fix-ci #${node.number}: ${reason}`); } } if (!pullRequests.pageInfo.hasNextPage) { diff --git a/.github/scripts/pr-unblock/fix-ci-dispatch.test.mjs b/.github/scripts/pr-unblock/fix-ci-dispatch.test.mjs index 9d8506b25a..ec33990c34 100644 --- a/.github/scripts/pr-unblock/fix-ci-dispatch.test.mjs +++ b/.github/scripts/pr-unblock/fix-ci-dispatch.test.mjs @@ -37,8 +37,11 @@ const check = (name, conclusion, extra = {}) => ({ ...extra, }); -// The head's ci-lanes commit status, as ci-status's aggregate step writes it. -const lanes = (state, creator = "github-actions[bot]") => ({ __typename: "StatusContext", context: "ci-lanes", state, creator: { login: creator } }); +// The head's ci-lanes commit status as the GraphQL rollup returns it: the +// creator recorded live from a pull request's ci-lanes status, which GraphQL +// reports as login `github-actions`, not REST's `github-actions[bot]`. +const ACTIONS_BOT = Object.freeze({ __typename: "Bot", login: "github-actions", databaseId: 41898282 }); +const lanes = (state, creator = ACTIONS_BOT) => ({ __typename: "StatusContext", context: "ci-lanes", state, creator }); // A lanes App PR whose head's lanes failed and has no fix-ci run yet. function pr(number, { checks = [lanes("FAILURE")], ...overrides } = {}) { @@ -499,7 +502,7 @@ test("the adapter sends one query naming the PR, its last commit's check rollup assert.equal(requests[0].url, "https://api.example/graphql"); assert.equal(requests[0].body.query, LIST_QUERY); assert.deepEqual(requests[0].body.variables, { owner: "octo", name: "repo", base: "main", after: null }); - for (const field of ["rateLimit", "cost", "statusCheckRollup", "commits(last: 1)", "databaseId", "... on User { databaseId }", "updatedAt", "committedDate", "body", "... on StatusContext { context state creator { login } }"]) { + for (const field of ["rateLimit", "cost", "statusCheckRollup", "commits(last: 1)", "databaseId", "... on User { databaseId }", "updatedAt", "committedDate", "body", "... on StatusContext { context state creator { __typename ... on Bot { databaseId } } }"]) { assert.ok(LIST_QUERY.includes(field), field); } }); @@ -624,9 +627,11 @@ test("a red ci-status over green or absent ci-lanes summons no fix-ci", async () assert.ok(!lines.some((line) => line.includes("fix-ci #")), lines.join("\n")); }); -test("a ci-lanes failure or error from github-actions[bot] is dispatched, whatever ci-status says", async () => { +test("a ci-lanes failure or error from the GitHub Actions bot is dispatched, whatever ci-status says", async () => { + // The REST spelling the old check compared against never matches GraphQL's login. + assert.notEqual(lanes("FAILURE").creator.login, "github-actions[bot]"); const { api, result } = await run({ - pages: [[pr(1, { checks: [check("ci-status", "SUCCESS"), lanes("FAILURE")] }), pr(2, { checks: [lanes("ERROR")] }), pr(3, { checks: [lanes("FAILURE", "octocat")] })]], + pages: [[pr(1, { checks: [check("ci-status", "SUCCESS"), lanes("FAILURE")] }), pr(2, { checks: [lanes("ERROR")] })]], }, { maxDispatch: 5 }); assert.deepEqual( result.dispatched.map((d) => d.number), @@ -634,3 +639,32 @@ test("a ci-lanes failure or error from github-actions[bot] is dispatched, whatev ); assert.deepEqual(api.calls.paths, [REQUIRED, REQUIRED]); }); + +test("a ci-lanes failure set by any other actor, or with no creator, is not trusted, and its skip is logged", async () => { + const untrusted = [ + { __typename: "User", login: "octocat", databaseId: 583231 }, + { __typename: "User", login: "github-actions", databaseId: 41898282 }, + { __typename: "Bot", login: "github-actions", databaseId: 1 }, + { __typename: "Bot", login: "github-actions[bot]" }, + null, + ]; + const { api, lines, result } = await run({ + pages: [[...untrusted.map((creator, index) => pr(index + 1, { checks: [lanes("FAILURE", creator)] })), pr(9, { checks: [{ ...lanes("FAILURE"), creator: undefined }] })]], + }, { maxDispatch: 9 }); + assert.deepEqual(result.dispatched, []); + assert.deepEqual(api.calls.paths, []); + for (const number of [1, 2, 3, 4, 5, 9]) { + assert.ok(lines.includes(`skip fix-ci #${number}: ci-lanes has not failed`), lines.join("\n")); + } +}); + +test("a listed head with failed lanes that is skipped logs why; a passing or pending one stays quiet", async () => { + const { lines, result } = await run({ + pages: [[pr(1, { isDraft: true }), pr(2, { checks: [lanes("SUCCESS")] }), pr(3, { checks: [lanes("PENDING")] }), pr(4, { checks: [] })]], + }); + assert.deepEqual(result.dispatched, []); + assert.deepEqual( + lines.filter((line) => line.includes("fix-ci #")), + ["skip fix-ci #1: not a ready same-repository PR into the base"], + ); +}); From d5d1ffe79908e10527cea7e0efbe08d38fbfe1c4 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sat, 24 Oct 2026 08:34:30 -0400 Subject: [PATCH 2/3] test(pr-pipeline): drop a fixture-only login assertion Co-Authored-By: Claude Opus 5.5 --- .github/scripts/pr-unblock/fix-ci-dispatch.test.mjs | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/scripts/pr-unblock/fix-ci-dispatch.test.mjs b/.github/scripts/pr-unblock/fix-ci-dispatch.test.mjs index ec33990c34..acbba23eae 100644 --- a/.github/scripts/pr-unblock/fix-ci-dispatch.test.mjs +++ b/.github/scripts/pr-unblock/fix-ci-dispatch.test.mjs @@ -628,8 +628,6 @@ test("a red ci-status over green or absent ci-lanes summons no fix-ci", async () }); test("a ci-lanes failure or error from the GitHub Actions bot is dispatched, whatever ci-status says", async () => { - // The REST spelling the old check compared against never matches GraphQL's login. - assert.notEqual(lanes("FAILURE").creator.login, "github-actions[bot]"); const { api, result } = await run({ pages: [[pr(1, { checks: [check("ci-status", "SUCCESS"), lanes("FAILURE")] }), pr(2, { checks: [lanes("ERROR")] })]], }, { maxDispatch: 5 }); From 564d0970a8560d728a8736674718b374d762dab8 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sat, 24 Oct 2026 08:40:28 -0400 Subject: [PATCH 3/3] fix(pr-pipeline): name an untrusted ci-lanes creator in the skip reason A failed ci-lanes status the GitHub Actions bot did not set was logged as "ci-lanes has not failed", the misdiagnosis this branch fixes. Give it its own reason, share one failed-state predicate between dispatch and the skip log, and cover the ERROR state in the log test. Co-Authored-By: Claude Opus 5.5 --- .changes/pr-pipeline/fix-fix-ci-creator-login-7d2f4b91.md | 2 +- .github/scripts/pr-unblock/fix-ci-dispatch.mjs | 7 ++++--- .github/scripts/pr-unblock/fix-ci-dispatch.test.mjs | 6 +++--- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/.changes/pr-pipeline/fix-fix-ci-creator-login-7d2f4b91.md b/.changes/pr-pipeline/fix-fix-ci-creator-login-7d2f4b91.md index 6804976f03..9422f6e235 100644 --- a/.changes/pr-pipeline/fix-fix-ci-creator-login-7d2f4b91.md +++ b/.changes/pr-pipeline/fix-fix-ci-creator-login-7d2f4b91.md @@ -5,4 +5,4 @@ bump: patch ### Fixed - `fix-ci` dispatch trusts a `ci-lanes` commit status by its creator's type and id (the GitHub Actions bot, `Bot` 41898282) instead of the login `github-actions[bot]`, which GraphQL never returns (it reports `github-actions`); no pull request was dispatched while the login check stood. A status set by any other actor, or with no creator, is still ignored. -- The dispatcher logs why it skips a listed pull request whose `ci-lanes` status failed or errored, so a skip of that kind is no longer silent. +- The dispatcher logs why it skips a listed pull request whose `ci-lanes` status failed or errored, so a skip of that kind is no longer silent; a failed status the GitHub Actions bot did not set is named as such. diff --git a/.github/scripts/pr-unblock/fix-ci-dispatch.mjs b/.github/scripts/pr-unblock/fix-ci-dispatch.mjs index aaacc85326..bacd32c601 100644 --- a/.github/scripts/pr-unblock/fix-ci-dispatch.mjs +++ b/.github/scripts/pr-unblock/fix-ci-dispatch.mjs @@ -148,7 +148,8 @@ function lanesState(node) { return status?.creator?.__typename === "Bot" && status.creator.databaseId === ACTIONS_BOT_ID ? status.state : null; } -const lanesFailed = (node) => ["FAILURE", "ERROR"].includes(lanesState(node)); +const failedState = (state) => ["FAILURE", "ERROR"].includes(state); +const lanesFailed = (node) => failedState(lanesState(node)); // The lanes passed and another check failed: an advisory lane's, if any. const advisoryFailed = (node, checks, checkName) => @@ -207,7 +208,7 @@ export function skipReason( } const advisory = advisoryFailed(node, checks, checkName); if (!lanesFailed(node) && !advisory) { - return "ci-lanes has not failed"; + return failedState(lanesStatus(node)?.state) ? "ci-lanes failed but the GitHub Actions bot did not set it" : "ci-lanes has not failed"; } // node.requiredRun, the head's tested pr-require-checks run (pr-test-windows // when only an advisory check failed) and its latest jobs, is attached by @@ -285,7 +286,7 @@ export async function dispatchCiFixes({ const reason = skipReason(node, { base, appSenderId, actionsAppId, checkName, holdLabels, gateLabel, restarts, maxRunsPerHead, baseAt, adoption, now }); if (reason === "") { candidates.push(node); - } else if (["FAILURE", "ERROR"].includes(lanesStatus(node)?.state)) { + } else if (failedState(lanesStatus(node)?.state)) { // A head whose lanes failed, whoever set the status, is the one a // silent skip would hide. log(`skip fix-ci #${node.number}: ${reason}`); diff --git a/.github/scripts/pr-unblock/fix-ci-dispatch.test.mjs b/.github/scripts/pr-unblock/fix-ci-dispatch.test.mjs index acbba23eae..9a3ee7064b 100644 --- a/.github/scripts/pr-unblock/fix-ci-dispatch.test.mjs +++ b/.github/scripts/pr-unblock/fix-ci-dispatch.test.mjs @@ -652,17 +652,17 @@ test("a ci-lanes failure set by any other actor, or with no creator, is not trus assert.deepEqual(result.dispatched, []); assert.deepEqual(api.calls.paths, []); for (const number of [1, 2, 3, 4, 5, 9]) { - assert.ok(lines.includes(`skip fix-ci #${number}: ci-lanes has not failed`), lines.join("\n")); + assert.ok(lines.includes(`skip fix-ci #${number}: ci-lanes failed but the GitHub Actions bot did not set it`), lines.join("\n")); } }); test("a listed head with failed lanes that is skipped logs why; a passing or pending one stays quiet", async () => { const { lines, result } = await run({ - pages: [[pr(1, { isDraft: true }), pr(2, { checks: [lanes("SUCCESS")] }), pr(3, { checks: [lanes("PENDING")] }), pr(4, { checks: [] })]], + pages: [[pr(1, { isDraft: true }), pr(2, { checks: [lanes("SUCCESS")] }), pr(3, { checks: [lanes("PENDING")] }), pr(4, { checks: [] }), pr(5, { isDraft: true, checks: [lanes("ERROR")] })]], }); assert.deepEqual(result.dispatched, []); assert.deepEqual( lines.filter((line) => line.includes("fix-ci #")), - ["skip fix-ci #1: not a ready same-repository PR into the base"], + ["skip fix-ci #1: not a ready same-repository PR into the base", "skip fix-ci #5: not a ready same-repository PR into the base"], ); });