fix(pr-pipeline): read the ci-lanes status creator as GraphQL reports it

.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; a failed status the GitHub Actions bot did not set is named as such.

.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 },
               },
             },

.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,

.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 } },
                 ],
               },
             },

.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,17 +130,26 @@ 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));
+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) =>
@@ -199,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
@@ -274,8 +283,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 (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}`);
       }
     }
     if (!pullRequests.pageInfo.hasNextPage) {

.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,13 +627,42 @@ 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 () => {
   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),
     [1, 2],
   );
   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 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: [] }), 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 #5: not a ready same-repository PR into the base"],
+  );
+});