Keep delayed Draft jobs from blocking Ready checks

.github/workflows/ready.yml

@@ -79,7 +79,10 @@ jobs:
                 const decisions = reviews.filter(review => review.user?.type === 'Bot'
                   && review.user.login === 'coderabbitai[bot]'
                   && ['APPROVED', 'CHANGES_REQUESTED', 'DISMISSED'].includes(review.state));
-                const latest = decisions.sort((a, b) => b.id - a.id)[0];
+                const latest = decisions.sort((a, b) => {
+                  const runId = check => Number(check.details_url?.match(/\/actions\/runs\/(\d+)(?:\/|$)/)?.[1] ?? check.id);
+                  return runId(b) - runId(a) || b.id - a.id;
+                })[0];
                 return latest?.state === 'APPROVED' && latest.commit_id === sha;
               }
               async function hasTrustedNotifier() {
@@ -108,10 +111,13 @@ jobs:
                   : 'Waiting for CodeRabbit approval of this revision.',
                 target_url: pull.html_url});
               if (!reviewPassed) return;
+              // Workflow events can create jobs out of order. Compare the originating run first.
               // A skipped draft Check must never authorize automatic publication.
               const checks = await github.paginate(github.rest.checks.listForRef, {owner, repo, ref: sha, per_page: 100});
-              const check = checks.filter(run => run.name === process.env.CHECK_NAME && run.app?.id === 15368)
-                .sort((a, b) => b.id - a.id)[0];
+              const candidates = checks.filter(run => run.name === process.env.CHECK_NAME && run.app?.id === 15368);
+              const runId = check => Number(check.details_url?.match(/\/actions\/runs\/(\d+)(?:\/|$)/)?.[1]);
+              if (candidates.some(run => !runId(run))) return core.info('Waiting for comparable Ready Check metadata.');
+              const check = candidates.sort((a, b) => runId(b) - runId(a) || b.id - a.id)[0];
               if (check?.conclusion !== 'success') return core.info('Waiting for a successful Ready Check.');
               // Rollout must install central requirements before enabling auto-merge.
               const {data: rules} = await github.request('GET /repos/{owner}/{repo}/rules/branches/{branch}',

tests/ready.test.mjs

@@ -29,7 +29,8 @@ async function scenario(options = {}) {
       checks: {listForRef: 'checks'},
     },
     paginate: async endpoint => endpoint === 'reviews' ? (options.reviewRead ? options.reviewRead() : options.reviews ?? [])
-      : endpoint === 'pulls' ? pulls : (options.checks ?? [{id: 1, name: 'Check / Check', app: {id: 15368}, conclusion: 'success'}]),
+      : endpoint === 'pulls' ? pulls : (options.checks ?? [{id: 1, name: 'Check / Check', app: {id: 15368}, conclusion: 'success'}]).map(check =>
+        Object.hasOwn(check, 'details_url') ? check : {...check, details_url: `https://github.com/o/r/actions/runs/${check.id}/job/${check.id}`}),
     request: async () => ({data: [{type: 'required_status_checks', parameters: {
       required_status_checks: (options.required ?? ['Check / Check', 'Review']).map(context => ({context})),
     }}]}),
@@ -194,3 +195,42 @@ test('release app rejection is reported without treating the PR as merged', asyn
   assert.equal(result.merges.length, 0);
   assert.match(result.errors[0], /Merge request failed \(409\)/);
 });
+
+
+test('delayed draft job does not supersede the newer Ready workflow', async () => {
+  const checks = [
+    {id: 116021792205, name: 'Check / Check', app: {id: 15368}, conclusion: 'skipped',
+      details_url: 'https://github.com/o/r/actions/runs/38613265941/job/116021792205'},
+    {id: 116021772853, name: 'Check / Check', app: {id: 15368}, conclusion: 'success',
+      details_url: 'https://github.com/o/r/actions/runs/38613266353/job/116021772853'},
+  ];
+  assert.equal((await scenario({checks})).merges.length, 1);
+  for (const conclusion of [null, 'failure', 'skipped']) {
+    checks[1].conclusion = conclusion;
+    assert.equal((await scenario({checks})).merges.length, 0);
+  }
+});
+test('newer Ready workflow and latest attempt supersede old successful jobs', async () => {
+  for (const newerRun of [100, 101]) {
+    const checks = [
+      {id: 200, name: 'Check / Check', app: {id: 15368}, conclusion: 'success',
+        details_url: 'https://github.com/o/r/actions/runs/100/job/200'},
+      {id: newerRun === 100 ? 201 : 199, name: 'Check / Check', app: {id: 15368}, conclusion: 'failure',
+        details_url: `https://github.com/o/r/actions/runs/${newerRun}/job/new`},
+    ];
+    assert.equal((await scenario({checks})).merges.length, 0);
+  }
+});
+
+test('mixed or absent workflow metadata cannot authorize an old successful result', async () => {
+  for (const conclusion of ['failure', null, 'skipped', 'success']) {
+    const checks = [
+      {id: 116021792205, name: 'Check / Check', app: {id: 15368}, conclusion: 'success', details_url: null},
+      {id: 116021772853, name: 'Check / Check', app: {id: 15368}, conclusion,
+        details_url: 'https://github.com/o/r/actions/runs/38613266353/job/116021772853'},
+    ];
+    assert.equal((await scenario({checks})).merges.length, 0);
+  }
+  assert.equal((await scenario({checks: [{id: 1, name: 'Check / Check', app: {id: 15368},
+    conclusion: 'success', details_url: 'invalid'}]})).merges.length, 0);
+});