From e3fe412e895752fa1b92ebaa701cd9073582fc52 Mon Sep 17 00:00:00 2001 From: Sil Veltman Date: Wed, 14 Oct 2026 20:47:42 +0200 Subject: [PATCH 1/2] Order Ready checks by workflow event before delayed job creation --- .github/workflows/ready.yml | 11 +++++++++-- tests/ready.test.mjs | 26 ++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ready.yml b/.github/workflows/ready.yml index 089a399..2e78fa5 100644 --- a/.github/workflows/ready.yml +++ b/.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,14 @@ 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]; + .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]; 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}', diff --git a/tests/ready.test.mjs b/tests/ready.test.mjs index e392d69..dc86a11 100644 --- a/tests/ready.test.mjs +++ b/tests/ready.test.mjs @@ -194,3 +194,29 @@ 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); + } +}); From 5e7c5ac14ad83af77865d05b1ffff4fd39e8d991 Mon Sep 17 00:00:00 2001 From: Sil Veltman Date: Wed, 14 Oct 2026 20:51:30 +0200 Subject: [PATCH 2/2] Require comparable workflow metadata before automatic merge --- .github/workflows/ready.yml | 9 ++++----- tests/ready.test.mjs | 16 +++++++++++++++- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ready.yml b/.github/workflows/ready.yml index 2e78fa5..8577b5b 100644 --- a/.github/workflows/ready.yml +++ b/.github/workflows/ready.yml @@ -114,11 +114,10 @@ jobs: // 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) => { - const runId = check => Number(check.details_url?.match(/\/actions\/runs\/(\d+)(?:\/|$)/)?.[1] ?? check.id); - return runId(b) - runId(a) || 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}', diff --git a/tests/ready.test.mjs b/tests/ready.test.mjs index dc86a11..8a56cba 100644 --- a/tests/ready.test.mjs +++ b/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})), }}]}), @@ -220,3 +221,16 @@ test('newer Ready workflow and latest attempt supersede old successful jobs', as 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); +});