Add retries when setting maintainer-approval success check

.github/workflows/maintainer-approval.js

@@ -430,6 +430,57 @@ async function upsertComment(github, owner, repo, prNumber, newBody) {
   });
 }
 
+// --- Approval check ---
+
+const CHECK_VERIFY_ATTEMPTS = 3;
+
+/**
+ * Create the maintainer-approval success check and confirm it took effect.
+ *
+ * checks.create can return 2xx yet leave the required "maintainer-approval"
+ * status stuck as "Expected" (yellow dot): the write is lost to GitHub
+ * eventual consistency while the workflow run itself still goes green. Because
+ * the pending path deliberately posts no check at all, nothing re-establishes
+ * it until an unrelated event happens to re-run this workflow, so the merge
+ * queue is silently blocked even though a maintainer approved (see PR #5868).
+ * Re-read the check from the head SHA after each create and retry until it is
+ * visible; fail the run if it never lands so the breakage surfaces loudly
+ * instead of blocking the merge with a green run.
+ */
+async function createApprovalCheck(github, checkParams, summary, core) {
+  const { owner, repo, head_sha: headSha, name } = checkParams;
+  const delayMs = Number(process.env.MAINTAINER_APPROVAL_VERIFY_DELAY_MS ?? 2000);
+
+  for (let attempt = 1; attempt <= CHECK_VERIFY_ATTEMPTS; attempt++) {
+    await github.rest.checks.create({
+      ...checkParams,
+      status: "completed",
+      conclusion: "success",
+      output: { title: name, summary },
+    });
+
+    const { data } = await github.rest.checks.listForRef({
+      owner, repo, ref: headSha, check_name: name,
+    });
+    if (data.check_runs.some(r => r.conclusion === "success")) {
+      return;
+    }
+
+    core.warning(
+      `${name} not visible on ${headSha} after create ` +
+      `(attempt ${attempt}/${CHECK_VERIFY_ATTEMPTS})`
+    );
+    if (attempt < CHECK_VERIFY_ATTEMPTS) {
+      await new Promise(resolve => setTimeout(resolve, delayMs));
+    }
+  }
+
+  core.setFailed(
+    `${name} could not be set on ${headSha} after ${CHECK_VERIFY_ATTEMPTS} ` +
+    `attempts; re-run this workflow to unblock the merge`
+  );
+}
+
 // --- Main ---
 
 module.exports = async ({ github, context, core }) => {
@@ -475,12 +526,7 @@ module.exports = async ({ github, context, core }) => {
   if (maintainerApproval) {
     const approver = maintainerApproval.user.login;
     core.info(`Maintainer approval from @${approver}`);
-    await github.rest.checks.create({
-      ...checkParams,
-      status: "completed",
-      conclusion: "success",
-      output: { title: STATUS_CONTEXT, summary: `Approved by @${approver}` },
-    });
+    await createApprovalCheck(github, checkParams, `Approved by @${approver}`, core);
     await deleteMarkerComments(github, owner, repo, prNumber);
     return;
   }
@@ -493,12 +539,7 @@ module.exports = async ({ github, context, core }) => {
     );
     if (hasAnyApproval) {
       core.info(`Maintainer-authored PR approved by a reviewer.`);
-      await github.rest.checks.create({
-        ...checkParams,
-        status: "completed",
-        conclusion: "success",
-        output: { title: STATUS_CONTEXT, summary: "Approved (maintainer-authored PR)" },
-      });
+      await createApprovalCheck(github, checkParams, "Approved (maintainer-authored PR)", core);
       await deleteMarkerComments(github, owner, repo, prNumber);
       return;
     }
@@ -533,12 +574,7 @@ module.exports = async ({ github, context, core }) => {
   // the GitHub UI, which blocks the merge until approval is granted.
   if (result.allCovered && approverLogins.length > 0) {
     core.info("All ownership groups have per-path approval.");
-    await github.rest.checks.create({
-      ...checkParams,
-      status: "completed",
-      conclusion: "success",
-      output: { title: STATUS_CONTEXT, summary: "All ownership groups approved" },
-    });
+    await createApprovalCheck(github, checkParams, "All ownership groups approved", core);
     await deleteMarkerComments(github, owner, repo, prNumber);
     return;
   }

.github/workflows/maintainer-approval.test.js

@@ -60,12 +60,16 @@ function makeCore() {
  * @param {Array} opts.files - PR files to return (objects with .filename)
  * @param {Object} opts.teamMembers - { teamSlug: [logins] }
  * @param {Array} opts.existingComments - Existing PR comments to return
+ * @param {number} opts.flakyCreates - Number of leading checks.create calls that
+ *   return 2xx but do not persist (not visible to the follow-up listForRef read),
+ *   simulating GitHub eventual-consistency flakes.
  */
-function makeGithub({ reviews = [], files = [], teamMembers = {}, existingComments = [] } = {}) {
+function makeGithub({ reviews = [], files = [], teamMembers = {}, existingComments = [], flakyCreates = 0 } = {}) {
   const listReviews = Symbol("listReviews");
   const listFiles = Symbol("listFiles");
   const listComments = Symbol("listComments");
   const checkRuns = [];
+  const persistedCheckRuns = [];
   const createdComments = [];
   const updatedComments = [];
   const deletedCommentIds = [];
@@ -85,7 +89,17 @@ function makeGithub({ reviews = [], files = [], teamMembers = {}, existingCommen
       checks: {
         create: async (params) => {
           checkRuns.push(params);
+          if (checkRuns.length > flakyCreates) {
+            persistedCheckRuns.push(params);
+          }
         },
+        listForRef: async ({ ref, check_name }) => ({
+          data: {
+            check_runs: persistedCheckRuns.filter(
+              (r) => r.head_sha === ref && r.name === check_name
+            ),
+          },
+        }),
       },
       issues: {
         listComments,
@@ -111,6 +125,7 @@ function makeGithub({ reviews = [], files = [], teamMembers = {}, existingCommen
       },
     },
     _checkRuns: checkRuns,
+    _persistedCheckRuns: persistedCheckRuns,
     _comments: createdComments,
     _updatedComments: updatedComments,
     _deletedCommentIds: deletedCommentIds,
@@ -128,6 +143,8 @@ describe("maintainer-approval", () => {
     originalWorkspace = process.env.GITHUB_WORKSPACE;
     tmpDir = makeTmpOwners(OWNERS_CONTENT, OWNERTEAMS_CONTENT);
     process.env.GITHUB_WORKSPACE = tmpDir;
+    // Skip the retry backoff so flake tests do not sleep.
+    process.env.MAINTAINER_APPROVAL_VERIFY_DELAY_MS = "0";
   });
 
   after(() => {
@@ -136,6 +153,7 @@ describe("maintainer-approval", () => {
     } else {
       delete process.env.GITHUB_WORKSPACE;
     }
+    delete process.env.MAINTAINER_APPROVAL_VERIFY_DELAY_MS;
     fs.rmSync(tmpDir, { recursive: true });
   });
 
@@ -558,4 +576,47 @@ describe("maintainer-approval", () => {
     assert.ok(body.includes("4 files changed"), "should show count for bundle group");
     assert.ok(!body.includes("`bundle/a.go`"), "should not list individual bundle files");
   });
+
+  // --- Check persistence (PR #5868 flake) ---
+
+  it("retries checks.create when the first write does not persist", async () => {
+    const github = makeGithub({
+      reviews: [
+        { state: "APPROVED", user: { login: "maintainer1" } },
+      ],
+      files: [{ filename: "cmd/pipelines/foo.go" }],
+      flakyCreates: 1,
+    });
+    const core = makeCore();
+    const context = makeContext();
+
+    await runModule({ github, context, core });
+
+    // First create was lost; second landed. No failure recorded.
+    assert.equal(github._checkRuns.length, 2);
+    assert.equal(github._persistedCheckRuns.length, 1);
+    assert.equal(github._persistedCheckRuns[0].conclusion, "success");
+    assert.equal(core._log.failed.length, 0);
+    assert.ok(core._log.warning.length >= 1);
+  });
+
+  it("fails the run when the check never persists", async () => {
+    const github = makeGithub({
+      reviews: [
+        { state: "APPROVED", user: { login: "maintainer1" } },
+      ],
+      files: [{ filename: "cmd/pipelines/foo.go" }],
+      flakyCreates: Infinity,
+    });
+    const core = makeCore();
+    const context = makeContext();
+
+    await runModule({ github, context, core });
+
+    // Exhausted all attempts without the required check becoming visible.
+    assert.equal(github._checkRuns.length, 3);
+    assert.equal(github._persistedCheckRuns.length, 0);
+    assert.equal(core._log.failed.length, 1);
+    assert.ok(core._log.failed[0].includes("could not be set"));
+  });
 });