From 469e3007a9e775ce0e0b1f28a704bc738ac3ccd3 Mon Sep 17 00:00:00 2001 From: Sil Veltman Date: Fri, 30 Oct 2026 08:22:08 +0100 Subject: [PATCH 1/2] ci: write Review only when it changes Every Ready event reconciles every open PR, and each run wrote a pending Review before its checks and a success after them. A long-lived ready PR reached GitHub's limit of 1000 statuses per commit and context, after which every Ready run in the repository failed on that PR. A failed read still revokes a previous success. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ready.yml | 22 +++++++++++++++------- tests/ready.test.mjs | 8 ++++++++ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ready.yml b/.github/workflows/ready.yml index cef50a5..ef9be29 100644 --- a/.github/workflows/ready.yml +++ b/.github/workflows/ready.yml @@ -100,7 +100,6 @@ jobs: await setReview('pending', 'Draft: publication is not authorized.'); return; } - if (requiredTests.length) await setReview('pending', 'Checking the current Test run and attempt.'); async function testsPassed() { if (!requiredTests.length) return true; // Read the current run and attempt on every event. Check runs can @@ -146,12 +145,21 @@ jobs: const {data} = await github.rest.repos.getCombinedStatusForRef({owner, repo, ref: sha, per_page: 100}); return names.every(name => data.statuses.find(status => status.context === name)?.state === 'success'); } - const approved = await testsPassed(); - const previews = await previewsPassed(); - const current = await getPull(); - if (current.state !== 'open' || current.base.ref !== pull.base.ref - || current.head.sha !== sha || current.draft !== pull.draft) return; - const reviewPassed = !pull.draft && approved && previews && await testsPassed(); + // Every event reconciles every open PR, and GitHub refuses more than 1000 + // statuses per commit and context. Write Review only when it changes. + let previews, reviewPassed; + try { + const approved = await testsPassed(); + previews = await previewsPassed(); + const current = await getPull(); + if (current.state !== 'open' || current.base.ref !== pull.base.ref + || current.head.sha !== sha || current.draft !== pull.draft) return; + reviewPassed = !pull.draft && approved && previews && await testsPassed(); + } catch (error) { + // A failed read must not leave a previous success in force. + await setReview('pending', 'Unable to verify the Ready requirements of this revision.'); + throw error; + } await setReview(reviewPassed ? 'success' : 'pending', reviewPassed ? 'Ready requirements passed for this revision.' : previews ? 'Waiting for required tests of this revision.' diff --git a/tests/ready.test.mjs b/tests/ready.test.mjs index 40b3767..b318f2a 100644 --- a/tests/ready.test.mjs +++ b/tests/ready.test.mjs @@ -311,6 +311,14 @@ test('the exact latest Test attempt supplies every required successful job', asy assert.equal(result.statuses.at(-1).state, 'success'); }); +test('an unchanged successful Review is not written again on later events', async () => { + // GitHub refuses more than 1000 statuses per commit and context. + const result = await scenario({...trial, runs: [testRun], jobs: [testJob], + review: {state: 'success', description: 'Ready requirements passed for this revision.'}}); + assert.deepEqual(result.statuses, []); + assert.deepEqual(result.errors, []); +}); + test('completion retries metadata lag and passes without requiring another event', async () => { let jobReads = 0; const result = await scenario({...trial, From 07a50ed5454e9961074f35158df64e1718e03558 Mon Sep 17 00:00:00 2001 From: Sil Veltman Date: Fri, 30 Oct 2026 15:42:36 +0100 Subject: [PATCH 2/2] ci: block an old success only for the PR's own events An event about this PR, such as a Test rerun of its head, still writes pending before checking. A PR that changes while it is checked loses an old success. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ready.yml | 15 ++++++++++++--- tests/ready.test.mjs | 20 +++++++++++++++----- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ready.yml b/.github/workflows/ready.yml index ef9be29..2ead6b8 100644 --- a/.github/workflows/ready.yml +++ b/.github/workflows/ready.yml @@ -100,6 +100,13 @@ jobs: await setReview('pending', 'Draft: publication is not authorized.'); return; } + // Every event reconciles every open PR, and GitHub refuses more than 1000 + // statuses per commit and context. Block an old success before checking + // only when the event concerns this PR, such as a Test rerun of its head. + const {payload} = context; + const ownEvent = payload.pull_request?.number === number + || payload.workflow_run?.head_sha === sha || payload.sha === sha; + if (requiredTests.length && ownEvent) await setReview('pending', 'Checking the current Test run and attempt.'); async function testsPassed() { if (!requiredTests.length) return true; // Read the current run and attempt on every event. Check runs can @@ -145,15 +152,17 @@ jobs: const {data} = await github.rest.repos.getCombinedStatusForRef({owner, repo, ref: sha, per_page: 100}); return names.every(name => data.statuses.find(status => status.context === name)?.state === 'success'); } - // Every event reconciles every open PR, and GitHub refuses more than 1000 - // statuses per commit and context. Write Review only when it changes. let previews, reviewPassed; try { const approved = await testsPassed(); previews = await previewsPassed(); const current = await getPull(); if (current.state !== 'open' || current.base.ref !== pull.base.ref - || current.head.sha !== sha || current.draft !== pull.draft) return; + || current.head.sha !== sha || current.draft !== pull.draft) { + // The change's own event reconciles it; an old success must not stay meanwhile. + if (review?.state === 'success') await setReview('pending', 'The PR changed while it was checked.'); + return; + } reviewPassed = !pull.draft && approved && previews && await testsPassed(); } catch (error) { // A failed read must not leave a previous success in force. diff --git a/tests/ready.test.mjs b/tests/ready.test.mjs index b318f2a..bbd0446 100644 --- a/tests/ready.test.mjs +++ b/tests/ready.test.mjs @@ -311,12 +311,22 @@ test('the exact latest Test attempt supplies every required successful job', asy assert.equal(result.statuses.at(-1).state, 'success'); }); -test('an unchanged successful Review is not written again on later events', async () => { +test('other PRs\' events do not rewrite an unchanged successful Review', async () => { // GitHub refuses more than 1000 statuses per commit and context. - const result = await scenario({...trial, runs: [testRun], jobs: [testJob], - review: {state: 'success', description: 'Ready requirements passed for this revision.'}}); - assert.deepEqual(result.statuses, []); - assert.deepEqual(result.errors, []); + const passed = {...trial, runs: [testRun], jobs: [testJob], + review: {state: 'success', description: 'Ready requirements passed for this revision.'}}; + for (const event of [{pull_request: {number: 2}}, {workflow_run: {name: 'Check', head_sha: 'other'}}, {sha: 'other'}]) { + const result = await scenario({...passed, event}); + assert.deepEqual(result.statuses, []); + assert.deepEqual(result.errors, []); + } + // The PR's own events still block the old success before checking. + const own = await scenario(passed); + assert.deepEqual(own.statuses.map(status => status.state), ['pending', 'success']); +}); +test('a PR changing while it is checked loses its old success', async () => { + const result = await scenario({changed: true, review: {state: 'success'}, event: {pull_request: {number: 2}}}); + assert.deepEqual(result.statuses.map(status => status.state), ['pending']); }); test('completion retries metadata lag and passes without requiring another event', async () => {