From 2cd22412100a0917cfebb4b3cc399b9cf9542df3 Mon Sep 17 00:00:00 2001 From: Sil Date: Sat, 17 Oct 2026 15:23:50 +0200 Subject: [PATCH 1/6] Add configurable QA test workflows and remove trial CodeRabbit gate --- .github/workflows/ci-check.yml | 2 +- .github/workflows/ready.yml | 50 +++++++-- .github/workflows/test-live.yml | 58 +++++++++++ .github/workflows/test.yml | 173 ++++++++++++++++++++++++++++++++ README.md | 13 ++- tests/preview.test.mjs | 64 ++++++++++++ tests/ready.test.mjs | 26 +++++ 7 files changed, 371 insertions(+), 15 deletions(-) create mode 100644 .github/workflows/test-live.yml create mode 100644 .github/workflows/test.yml create mode 100644 tests/preview.test.mjs diff --git a/.github/workflows/ci-check.yml b/.github/workflows/ci-check.yml index a428eb6..8a34fc9 100644 --- a/.github/workflows/ci-check.yml +++ b/.github/workflows/ci-check.yml @@ -18,4 +18,4 @@ jobs: uses: ./.github/workflows/check.yml with: install: false - command: node --test tests/ready.test.mjs + command: node --test tests/*.test.mjs diff --git a/.github/workflows/ready.yml b/.github/workflows/ready.yml index 8513e71..c91eddd 100644 --- a/.github/workflows/ready.yml +++ b/.github/workflows/ready.yml @@ -12,7 +12,12 @@ on: check-name: type: string default: Check / Check - + require-coderabbit: + type: boolean + default: true + test-checks: + type: string + default: "[]" merge-app-client-id: type: string default: "" @@ -47,11 +52,13 @@ jobs: repositories: ${{ github.event.repository.name }} # Only the merge uses this token, so existing push-triggered releases still run. # Only GitHub metadata is read. Never check out or execute PR code here. - - name: Require selected review and publish CloudCannon changes + - name: Reconcile publication requirements uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 env: NETLIFY_CHECKS: ${{ inputs.netlify-checks }} CHECK_NAME: ${{ inputs.check-name }} + REQUIRE_CODERABBIT: ${{ inputs.require-coderabbit }} + TEST_CHECKS: ${{ inputs.test-checks }} MERGE_TOKEN: ${{ steps.merge-token.outputs.token }} with: script: | @@ -75,7 +82,26 @@ jobs: const pull = await getPull(); if (pull.state !== 'open' || pull.base.ref !== repository.default_branch) return; const sha = pull.head.sha; - const selected = pull.labels.some(label => label.name === 'review-ready'); + const selected = process.env.REQUIRE_CODERABBIT !== 'false' + && pull.labels.some(label => label.name === 'review-ready'); + const requiredTests = JSON.parse(process.env.TEST_CHECKS || '[]'); + const isSelected = candidate => process.env.REQUIRE_CODERABBIT !== 'false' + && candidate.labels.some(label => label.name === 'review-ready'); + async function testsPassed() { + if (!requiredTests.length) return true; + const checks = await github.paginate(github.rest.checks.listForRef, + {owner, repo, ref: sha, per_page: 100}); + const workflow = context.payload.workflow_run; + const runId = check => Number(check.details_url?.match(/\/actions\/runs\/(\d+)(?:\/|$)/)?.[1]); + return requiredTests.every(name => { + const candidates = checks.filter(check => check.name === name && check.app?.id === 15368); + if (!candidates.length || candidates.some(check => !runId(check))) return false; + const latest = candidates.sort((a, b) => runId(b) - runId(a) || b.id - a.id)[0]; + if (workflow?.name === 'Test' && workflow.head_sha === sha + && Number(workflow.id) >= runId(latest) && workflow.status !== 'completed') return false; + return latest.status === 'completed' && latest.conclusion === 'success'; + }); + } async function hasApproval() { if (!selected) return true; const reviews = await github.paginate(github.rest.pulls.listReviews, {owner, repo, pull_number: number, per_page: 100}); @@ -102,16 +128,17 @@ jobs: throw error; } } - const approved = await hasTrustedNotifier() && await hasApproval(); + const approved = await hasTrustedNotifier() && await hasApproval() && await testsPassed(); const current = await getPull(); if (current.base.ref !== pull.base.ref || current.head.sha !== sha || current.draft !== pull.draft - || current.labels.some(label => label.name === 'review-ready') !== selected) return; - const reviewPassed = !pull.draft && approved && await hasTrustedNotifier() && await hasApproval(); + || isSelected(current) !== selected) return; + const reviewPassed = !pull.draft && approved && await hasTrustedNotifier() && await hasApproval() + && await testsPassed(); await github.rest.repos.createCommitStatus({owner, repo, sha, context: 'Review', state: reviewPassed ? 'success' : 'pending', description: pull.draft ? 'Draft: publication is not authorized.' - : approved ? (selected ? 'CodeRabbit approved this revision.' : 'No CodeRabbit review selected.') - : 'Waiting for CodeRabbit approval of this revision.', + : reviewPassed ? (selected ? 'CodeRabbit approved this revision.' : 'Ready requirements passed for this revision.') + : 'Waiting for required tests or selected review of this revision.', target_url: pull.html_url}); if (!reviewPassed) return; // Agents own code merges so they can follow the production deployment. @@ -142,13 +169,18 @@ jobs: if (!repository.allow_auto_merge) return core.info('Repository auto-merge is not enabled yet.'); const final = await getPull(); if (final.state !== 'open' || final.draft || final.base.ref !== pull.base.ref || final.head.sha !== sha - || final.labels.some(label => label.name === 'review-ready') !== selected) return; + || isSelected(final) !== selected) return; if (!isCloudCannonPublication(final)) return; if (selected && (!await hasTrustedNotifier() || !await hasApproval())) { await github.rest.repos.createCommitStatus({owner, repo, sha, context: 'Review', state: 'pending', description: 'CodeRabbit approval changed; waiting for review.', target_url: pull.html_url}); return; } + if (!await testsPassed()) { + await github.rest.repos.createCommitStatus({owner, repo, sha, context: 'Review', + state: 'pending', description: 'Waiting for required tests of this revision.', target_url: pull.html_url}); + return; + } const method = final.head.ref === 'preview' && final.head.repo?.full_name === final.base.repo?.full_name ? 'merge' : 'squash'; // Unrequired failed jobs can make GitHub report unstable; required rules still enforce this merge. diff --git a/.github/workflows/test-live.yml b/.github/workflows/test-live.yml new file mode 100644 index 0000000..51a6f2e --- /dev/null +++ b/.github/workflows/test-live.yml @@ -0,0 +1,58 @@ +name: Test live + +on: + workflow_call: + inputs: + runner: + type: string + default: ubuntu-latest + base-url: + type: string + required: true + revision: + description: Commit confirmed as published before dispatching this workflow. + type: string + required: true + http: + type: boolean + default: true + browser: + type: boolean + default: true + +permissions: + contents: read + +jobs: + live: + name: Live + runs-on: ${{ inputs.runner }} + timeout-minutes: 15 + env: + pnpm_config_ignore_scripts: "true" + pnpm_config_ignore_pnpmfile: "true" + BASE_URL: ${{ inputs.base-url }} + QA_PROFILE: live + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: ${{ inputs.revision }} + persist-credentials: false + - uses: voidzero-dev/setup-vp@68f39c78aaf39be468cabd2afb09c162574c84af # v1.21.2 + with: + version: "1.1.0" + node-version-file: .node-version + cache: true + run-install: | + args: + - --frozen-lockfile + - --ignore-scripts + - name: Safe live HTTP smoke + if: ${{ inputs.http }} + run: vp run test:http + - name: Install desktop browsers + if: ${{ inputs.browser }} + run: vp exec playwright install --with-deps chromium firefox webkit + - name: Safe live browser smoke + if: ${{ inputs.browser }} + run: vp run test:browser diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..26ffaa9 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,173 @@ +name: Test + +on: + workflow_call: + inputs: + runner: + type: string + default: ubuntu-latest + unit: + type: boolean + default: false + http: + type: boolean + default: false + browser: + type: boolean + default: false + preview-checks: + description: JSON array of Netlify status contexts; the first is BASE_URL. + type: string + default: "[]" + node-version-file: + type: string + default: .node-version + secrets: + testing-secret: + required: false + +permissions: + contents: read + statuses: read + pull-requests: read + +jobs: + unit: + name: Unit + if: ${{ !github.event.pull_request.draft && inputs.unit }} + runs-on: ${{ inputs.runner }} + timeout-minutes: 20 + env: + pnpm_config_ignore_scripts: "true" + pnpm_config_ignore_pnpmfile: "true" + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: voidzero-dev/setup-vp@68f39c78aaf39be468cabd2afb09c162574c84af # v1.21.2 + with: + version: "1.1.0" + node-version-file: ${{ inputs.node-version-file }} + cache: true + run-install: | + args: + - --frozen-lockfile + - --ignore-scripts + - name: Unit and service tests + run: vp run test:unit + + web: + name: Web + if: ${{ !github.event.pull_request.draft && (inputs.http || inputs.browser) }} + runs-on: ${{ inputs.runner }} + timeout-minutes: 30 + # Signed-in tests can share preview accounts across browser targets and PRs. + concurrency: + group: qa-web-${{ github.repository }} + cancel-in-progress: false + env: + pnpm_config_ignore_scripts: "true" + pnpm_config_ignore_pnpmfile: "true" + QA_PROFILE: preview + steps: + - name: Resolve previews for this PR revision + id: preview + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + env: + PREVIEW_CHECKS: ${{ inputs.preview-checks }} + with: + script: | + const {owner, repo} = context.repo; + const pull = context.payload.pull_request; + if (!pull) throw new Error('URL tests require a pull_request event.'); + const names = JSON.parse(process.env.PREVIEW_CHECKS); + if (!Array.isArray(names) || !names.length + || names.some(name => typeof name !== 'string' || !/^netlify\/[a-z0-9-]+\/deploy-preview$/.test(name))) { + throw new Error('Enabled URL tests require preview-checks.'); + } + const sha = pull.head.sha; + const deadline = Date.now() + 12 * 60 * 1000; + while (Date.now() < deadline) { + const {data: current} = await github.rest.pulls.get({owner, repo, pull_number: pull.number}); + if (current.state !== 'open' || current.draft || current.head.sha !== sha) { + throw new Error('PR changed or returned to draft; discard this test run.'); + } + const {data} = await github.rest.repos.getCombinedStatusForRef({owner, repo, ref: sha, per_page: 100}); + const urls = {}; + for (const name of names) { + const status = data.statuses.find(status => status.context === name); + if (['error', 'failure'].includes(status?.state)) { + throw new Error(`${name} failed for ${sha}.`); + } + if (status?.state !== 'success') continue; + const url = new URL(status.target_url); + const site = name.split('/')[1]; + if (url.protocol !== 'https:' || url.username || url.password + || url.hostname !== `deploy-preview-${pull.number}--${site}.netlify.app`) { + throw new Error(`${name} did not return this PR's Netlify preview URL.`); + } + urls[name] = url.origin; + } + if (names.every(name => urls[name])) { + core.setOutput('base-url', urls[names[0]]); + core.setOutput('deploy-urls', JSON.stringify(urls)); + core.setOutput('sha', sha); + return; + } + core.info(`Waiting for matching previews: ${names.filter(name => !urls[name]).join(', ')}`); + await new Promise(resolve => setTimeout(resolve, 15000)); + } + throw new Error('Matching previews did not complete within 12 minutes.'); + - name: Checkout the tested preview revision + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: ${{ steps.preview.outputs.sha }} + persist-credentials: false + - uses: voidzero-dev/setup-vp@68f39c78aaf39be468cabd2afb09c162574c84af # v1.21.2 + with: + version: "1.1.0" + node-version-file: ${{ inputs.node-version-file }} + cache: true + run-install: | + args: + - --frozen-lockfile + - --ignore-scripts + - name: HTTP tests + if: ${{ inputs.http }} + env: + BASE_URL: ${{ steps.preview.outputs.base-url }} + QA_DEPLOY_URLS: ${{ steps.preview.outputs.deploy-urls }} + run: vp run test:http + - name: Install desktop browsers + if: ${{ inputs.browser }} + run: vp exec playwright install --with-deps chromium firefox webkit + - name: Browser tests + if: ${{ inputs.browser }} + env: + BASE_URL: ${{ steps.preview.outputs.base-url }} + QA_DEPLOY_URLS: ${{ steps.preview.outputs.deploy-urls }} + TESTING_SECRET: ${{ secrets.testing-secret }} + run: vp run test:browser + + test: + name: Test + if: ${{ always() && !github.event.pull_request.draft }} + needs: [unit, web] + runs-on: ${{ inputs.runner }} + timeout-minutes: 2 + steps: + - name: Require every enabled category + env: + UNIT_ENABLED: ${{ inputs.unit }} + WEB_ENABLED: ${{ inputs.http || inputs.browser }} + UNIT_RESULT: ${{ needs.unit.result }} + WEB_RESULT: ${{ needs.web.result }} + run: | + if [[ "$UNIT_ENABLED" == true && "$UNIT_RESULT" != success ]]; then + echo 'Enabled unit tests did not succeed.' >&2 + exit 1 + fi + if [[ "$WEB_ENABLED" == true && "$WEB_RESULT" != success ]]; then + echo 'Enabled URL tests did not succeed.' >&2 + exit 1 + fi diff --git a/README.md b/README.md index 188b3b6..4399d3e 100644 --- a/README.md +++ b/README.md @@ -1,15 +1,18 @@ # fulldotdev/.github -Organization-wide GitHub defaults and reusable workflows. Repository callers select their own check command; workflows here are not inherited automatically. +Organization-wide GitHub defaults and reusable workflows. Repositories own their package scripts, assertions and enabled categories. These workflows own execution and time limits; they are not inherited automatically. Callers pin a full commit SHA from `main` after the shared workflow PR is merged. New projects take their callers from [astro-template](https://github.com/fulldotdev/astro-template) or [vite-template](https://github.com/fulldotdev/vite-template). -- **Finalize (Prepare)** leaves a complete draft PR. Previews build; full checks wait for Ready. -- **Dispatch (Release)** validates the affected behavior and marks the PR Ready. After the required checks and previews pass, the agent merges the validated head and follows its production deployment. Choose `review-ready` before marking Ready when CodeRabbit approval is needed. -- **Settle (Close)** follows production verification and preserves the result before closing the task. +- **Develop** stays local while implementing and experimenting. +- **Draft PR** builds and checks artifacts locally, then opens a shared draft and Netlify preview. +- **Make PR ready** runs enabled checks and tests locally, then CI confirms them. Source checks and unit tests run alongside the Netlify build. URL tests use the preview of the tested PR head. +- **Merge PR** merges the validated head and follows the production build and short, safe live checks. -Organization rules require `Check / Check` and `Review`, without requiring synchronization with main. Netlify requirements are managed centrally for the repositories with those sites. Production keeps its scoped check and build. A skipped or outdated CodeRabbit review cannot satisfy a selected review. +The trial callers in `fulldev`, `astro-template` and `vite-template` use `check:source` and the selected categories in `test.yml`. Devices stay manual. They pass `require-coderabbit: false` and require `Test / Test` through the existing `Review` status. Missing, skipped, failed or superseded test results cannot pass that gate. Other callers retain their existing behavior until migrated. + +Organization rules require `Check / Check` and `Review`, without requiring synchronization with main. Netlify requirements are managed centrally for repositories with those sites. Production keeps scoped source, build and artifact checks. Live URL tests run after the current production deployment has been confirmed, rather than inside its build command. Keep repository auto-merge disabled except for CloudCannon publishers. CloudCannon's Publish button keeps automatic publication for same-repository `preview` PRs created by `cloudcannon[bot]`, with all required checks and previews. Code PRs are merged by their agent. diff --git a/tests/preview.test.mjs b/tests/preview.test.mjs new file mode 100644 index 0000000..66c4b7d --- /dev/null +++ b/tests/preview.test.mjs @@ -0,0 +1,64 @@ +import assert from 'node:assert/strict'; +import {readFileSync} from 'node:fs'; +import test from 'node:test'; + +const source = readFileSync(new URL('../.github/workflows/test.yml', import.meta.url), 'utf8') + .split(' script: |\n')[1].split('\n - name: Checkout')[0] + .split('\n').map(line => line.slice(12)).join('\n'); +const execute = new (Object.getPrototypeOf(async function () {}).constructor)( + 'github', 'context', 'core', 'process', 'Date', 'setTimeout', source); + +async function scenario(options = {}) { + const pull = {number: 7, state: 'open', draft: false, head: {sha: 'tested-head'}}; + const names = options.names ?? ['netlify/app/deploy-preview']; + const outputs = {}; + const refs = []; + let clock = 0; + const github = {rest: { + pulls: {get: async () => ({data: {...pull, ...options.current}})}, + repos: {getCombinedStatusForRef: async ({ref}) => { + refs.push(ref); + return {data: {statuses: options.statuses ?? names.map(context => ({context, state: 'success', + target_url: `https://deploy-preview-7--${context.split('/')[1]}.netlify.app`}))}}; + }}, + }}; + await execute(github, {repo: {owner: 'o', repo: 'r'}, payload: {pull_request: pull}}, + {info() {}, setOutput: (name, value) => { outputs[name] = value; }}, + {env: {PREVIEW_CHECKS: JSON.stringify(names)}}, + {now: () => clock}, callback => { clock += 60 * 1000; callback(); }); + return {outputs, refs}; +} + +test('URL tests use previews associated with the exact PR head', async () => { + const {outputs, refs} = await scenario({names: ['netlify/app/deploy-preview', 'netlify/site/deploy-preview']}); + assert.deepEqual(refs, ['tested-head']); + assert.equal(outputs.sha, 'tested-head'); + assert.equal(outputs['base-url'], 'https://deploy-preview-7--app.netlify.app'); + assert.equal(Object.keys(JSON.parse(outputs['deploy-urls'])).length, 2); +}); + +test('failed, absent and pending previews cannot become passing URL tests', async () => { + for (const state of ['failure', 'error']) { + await assert.rejects(scenario({statuses: [{context: 'netlify/app/deploy-preview', state}]}), /failed/); + } + for (const statuses of [[], [{context: 'netlify/app/deploy-preview', state: 'pending'}]]) { + await assert.rejects(scenario({statuses}), /within 12 minutes/); + } +}); + +test('superseded, closed and draft PRs stop before testing', async () => { + for (const current of [{head: {sha: 'new-head'}}, {state: 'closed'}, {draft: true}]) { + await assert.rejects(scenario({current}), /PR changed or returned to draft/); + } +}); + +test('production, another PR and untrusted URLs are rejected', async () => { + for (const target_url of ['https://app.netlify.app', 'https://deploy-preview-8--app.netlify.app', + 'https://deploy-preview-7--another-site.netlify.app', + 'http://deploy-preview-7--app.netlify.app', 'https://deploy-preview-7--app.netlify.app.attacker.test', + 'https://user:password@deploy-preview-7--app.netlify.app']) { + await assert.rejects(scenario({statuses: [{context: 'netlify/app/deploy-preview', state: 'success', target_url}]}), + /did not return this PR/); + } + await assert.rejects(scenario({names: []}), /require preview-checks/); +}); diff --git a/tests/ready.test.mjs b/tests/ready.test.mjs index d698800..9ad739c 100644 --- a/tests/ready.test.mjs +++ b/tests/ready.test.mjs @@ -45,6 +45,7 @@ async function scenario(options = {}) { }; await execute(github, {repo: {owner: 'o', repo: 'r'}, payload: options.event ?? {pull_request: {number: 1}}}, {info: () => {}, error: message => errors.push(message), setFailed: message => errors.push(message)}, {env: {CHECK_NAME: 'Check / Check', NETLIFY_CHECKS: JSON.stringify(options.netlify ?? []), + REQUIRE_CODERABBIT: String(options.requireCoderabbit ?? true), TEST_CHECKS: JSON.stringify(options.testChecks ?? []), GITHUB_API_URL: 'https://api.github.com', MERGE_TOKEN: options.mergeToken}}, options.fetch ?? (() => { throw new Error('Unexpected network request'); })); return {statuses, merges, errors}; @@ -258,3 +259,28 @@ test('ordinary Ready code PRs pass Review without any automatic merge', async () } } }); + +test('trial repositories no longer depend on CodeRabbit selection or quota', async () => { + const result = await scenario({selected: true, requireCoderabbit: false, changedNotifier: true, + pull: {user: {type: 'User', login: 'silveltman'}}}); + assert.equal(result.statuses[0].state, 'success'); + assert.equal(result.merges.length, 0); +}); + +test('required tests gate Review for ordinary PRs without adding an organization rule', async () => { + const options = {requireCoderabbit: false, testChecks: ['Test / Test'], + pull: {user: {type: 'User', login: 'silveltman'}}}; + for (const check of [null, + {id: 1, name: 'Test / Test', app: {id: 15368}, status: 'completed', conclusion: 'skipped'}, + {id: 1, name: 'Test / Test', app: {id: 15368}, status: 'completed', conclusion: 'failure'}, + {id: 1, name: 'Test / Test', app: {id: 15368}, status: 'in_progress', conclusion: null}, + {id: 1, name: 'Test / Test', app: {id: 1}, status: 'completed', conclusion: 'success'}]) { + assert.equal((await scenario({...options, checks: check ? [check] : []})).statuses[0].state, 'pending'); + } + const passed = {id: 2, name: 'Test / Test', app: {id: 15368}, status: 'completed', conclusion: 'success'}; + assert.equal((await scenario({...options, checks: [passed]})).statuses[0].state, 'success'); + assert.equal((await scenario({...options, checks: [passed, {...passed, id: 3, status: 'in_progress', conclusion: null}]})).statuses[0].state, 'pending'); + assert.equal((await scenario({...options, checks: [{...passed, details_url: null}]})).statuses[0].state, 'pending'); + assert.equal((await scenario({...options, checks: [passed], event: {workflow_run: { + name: 'Test', head_sha: 'head', id: 3, status: 'queued'}}})).statuses[0].state, 'pending'); +}); From 4499bc2bf0c7810b26992d5bc4ff46cf987e0d2e Mon Sep 17 00:00:00 2001 From: Sil Date: Sat, 17 Oct 2026 15:27:37 +0200 Subject: [PATCH 2/6] Keep the review gate pending while newer test metadata arrives --- .github/workflows/ready.yml | 4 +++- tests/ready.test.mjs | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ready.yml b/.github/workflows/ready.yml index c91eddd..25c36b3 100644 --- a/.github/workflows/ready.yml +++ b/.github/workflows/ready.yml @@ -98,7 +98,9 @@ jobs: if (!candidates.length || candidates.some(check => !runId(check))) return false; const latest = candidates.sort((a, b) => runId(b) - runId(a) || b.id - a.id)[0]; if (workflow?.name === 'Test' && workflow.head_sha === sha - && Number(workflow.id) >= runId(latest) && workflow.status !== 'completed') return false; + && Number(workflow.id) >= runId(latest) + && (Number(workflow.id) !== runId(latest) || workflow.status !== 'completed' + || workflow.conclusion !== 'success')) return false; return latest.status === 'completed' && latest.conclusion === 'success'; }); } diff --git a/tests/ready.test.mjs b/tests/ready.test.mjs index 9ad739c..d763676 100644 --- a/tests/ready.test.mjs +++ b/tests/ready.test.mjs @@ -283,4 +283,8 @@ test('required tests gate Review for ordinary PRs without adding an organization assert.equal((await scenario({...options, checks: [{...passed, details_url: null}]})).statuses[0].state, 'pending'); assert.equal((await scenario({...options, checks: [passed], event: {workflow_run: { name: 'Test', head_sha: 'head', id: 3, status: 'queued'}}})).statuses[0].state, 'pending'); + assert.equal((await scenario({...options, checks: [passed], event: {workflow_run: { + name: 'Test', head_sha: 'head', id: 3, status: 'completed', conclusion: 'failure'}}})).statuses[0].state, 'pending'); + assert.equal((await scenario({...options, checks: [passed], event: {workflow_run: { + name: 'Test', head_sha: 'head', id: 2, status: 'completed', conclusion: 'failure'}}})).statuses[0].state, 'pending'); }); From 81af327d3382552f32697e3a53f9b3b9916047c1 Mon Sep 17 00:00:00 2001 From: Sil Date: Sat, 17 Oct 2026 15:59:24 +0200 Subject: [PATCH 3/6] Gate Review on the latest workflow attempt and retry delayed job metadata --- .github/workflows/ready.yml | 44 +++++++++++++------- tests/ready.test.mjs | 81 +++++++++++++++++++++++++++++++------ 2 files changed, 98 insertions(+), 27 deletions(-) diff --git a/.github/workflows/ready.yml b/.github/workflows/ready.yml index 25c36b3..fcd4404 100644 --- a/.github/workflows/ready.yml +++ b/.github/workflows/ready.yml @@ -26,6 +26,7 @@ on: required: false permissions: + actions: read contents: write pull-requests: write statuses: write @@ -87,22 +88,37 @@ jobs: const requiredTests = JSON.parse(process.env.TEST_CHECKS || '[]'); const isSelected = candidate => process.env.REQUIRE_CODERABBIT !== 'false' && candidate.labels.some(label => label.name === 'review-ready'); + if (requiredTests.length) await github.rest.repos.createCommitStatus({owner, repo, sha, + context: 'Review', state: 'pending', description: 'Checking the current Test run and attempt.', + target_url: pull.html_url}); async function testsPassed() { if (!requiredTests.length) return true; - const checks = await github.paginate(github.rest.checks.listForRef, - {owner, repo, ref: sha, per_page: 100}); - const workflow = context.payload.workflow_run; - const runId = check => Number(check.details_url?.match(/\/actions\/runs\/(\d+)(?:\/|$)/)?.[1]); - return requiredTests.every(name => { - const candidates = checks.filter(check => check.name === name && check.app?.id === 15368); - if (!candidates.length || candidates.some(check => !runId(check))) return false; - const latest = candidates.sort((a, b) => runId(b) - runId(a) || b.id - a.id)[0]; - if (workflow?.name === 'Test' && workflow.head_sha === sha - && Number(workflow.id) >= runId(latest) - && (Number(workflow.id) !== runId(latest) || workflow.status !== 'completed' - || workflow.conclusion !== 'success')) return false; - return latest.status === 'completed' && latest.conclusion === 'success'; - }); + // Read the current run and attempt on every event. Check runs can + // still show a previous success while the workflow is rerunning. + const event = context.payload.workflow_run; + for (let retry = 0; retry < 4; retry++) { + const runs = await github.paginate(github.rest.actions.listWorkflowRunsForRepo, + {owner, repo, head_sha: sha, event: 'pull_request', per_page: 100}); + const run = runs.filter(run => run.name === 'Test' && run.head_sha === sha + && run.path === '.github/workflows/test.yml') + .sort((a, b) => b.id - a.id)[0]; + const eventIsNewer = event?.name === 'Test' && event.head_sha === sha + && (!run || event.id > run.id || (event.id === run.id + && event.run_attempt > run.run_attempt)); + if (eventIsNewer) { + if (event.status !== 'completed' || event.conclusion !== 'success') return false; + } else if (run) { + if (run.status !== 'completed' || run.conclusion !== 'success') return false; + const jobs = await github.paginate(github.rest.actions.listJobsForWorkflowRunAttempt, + {owner, repo, run_id: run.id, attempt_number: run.run_attempt, per_page: 100}); + if (requiredTests.every(name => jobs.some(job => job.name === name + && job.status === 'completed' && job.conclusion === 'success'))) return true; + } + // Successful completion can precede its jobs' metadata. Retry + // briefly so a final completion event cannot strand the PR. + if (retry < 3) await new Promise(resolve => setTimeout(resolve, 2000)); + } + return false; } async function hasApproval() { if (!selected) return true; diff --git a/tests/ready.test.mjs b/tests/ready.test.mjs index d763676..ae8b854 100644 --- a/tests/ready.test.mjs +++ b/tests/ready.test.mjs @@ -4,7 +4,7 @@ import test from 'node:test'; const source = readFileSync(new URL('../.github/workflows/ready.yml', import.meta.url), 'utf8') .split(' script: |\n')[1].split('\n').map(line => line.slice(12)).join('\n'); -const execute = new (Object.getPrototypeOf(async function () {}).constructor)('github', 'context', 'core', 'process', 'fetch', source); +const execute = new (Object.getPrototypeOf(async function () {}).constructor)('github', 'context', 'core', 'process', 'fetch', 'setTimeout', source); async function scenario(options = {}) { const pull = {number: 1, node_id: 'PR1', state: 'open', draft: false, head: {sha: 'head', ref: 'preview', repo: {full_name: 'o/r'}}, @@ -15,6 +15,14 @@ async function scenario(options = {}) { const statuses = []; const merges = []; let reads = 0; + let runReads = 0; + let jobReads = 0; + const checks = () => (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}`}); + const defaultRuns = () => checks().filter(check => check.name === 'Test / Test' && check.app?.id === 15368) + .map(check => ({id: Number(check.details_url?.match(/\/actions\/runs\/(\d+)/)?.[1]), run_attempt: 1, + name: 'Test', path: '.github/workflows/test.yml', head_sha: 'head', status: check.status, conclusion: check.conclusion})) + .filter(run => run.id); const github = { rest: { pulls: {get: async ({pull_number}) => ({data: ++reads >= 2 && options.changed ? {...pull, head: {sha: 'new'}} : pulls.find(candidate => candidate.number === pull_number)}), @@ -27,10 +35,13 @@ async function scenario(options = {}) { }, createCommitStatus: async status => statuses.push(status)}, checks: {listForRef: 'checks'}, + actions: {listWorkflowRunsForRepo: 'runs', listJobsForWorkflowRunAttempt: 'jobs'}, }, - 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'}]).map(check => - Object.hasOwn(check, 'details_url') ? check : {...check, details_url: `https://github.com/o/r/actions/runs/${check.id}/job/${check.id}`}), + paginate: async (endpoint, parameters) => endpoint === 'reviews' ? (options.reviewRead ? options.reviewRead() : options.reviews ?? []) + : endpoint === 'pulls' ? pulls + : endpoint === 'runs' ? options.runRead ? options.runRead(++runReads) : options.runs ?? defaultRuns() + : endpoint === 'jobs' ? options.jobRead ? options.jobRead(++jobReads, parameters) : options.jobs ?? checks() + : checks(), request: async () => ({data: [{type: 'required_status_checks', parameters: { required_status_checks: (options.required ?? ['Check / Check', 'Review']).map(context => ({context})), }}]}), @@ -47,7 +58,7 @@ async function scenario(options = {}) { {info: () => {}, error: message => errors.push(message), setFailed: message => errors.push(message)}, {env: {CHECK_NAME: 'Check / Check', NETLIFY_CHECKS: JSON.stringify(options.netlify ?? []), REQUIRE_CODERABBIT: String(options.requireCoderabbit ?? true), TEST_CHECKS: JSON.stringify(options.testChecks ?? []), GITHUB_API_URL: 'https://api.github.com', MERGE_TOKEN: options.mergeToken}}, - options.fetch ?? (() => { throw new Error('Unexpected network request'); })); + options.fetch ?? (() => { throw new Error('Unexpected network request'); }), resolve => resolve()); return {statuses, merges, errors}; } @@ -275,16 +286,60 @@ test('required tests gate Review for ordinary PRs without adding an organization {id: 1, name: 'Test / Test', app: {id: 15368}, status: 'completed', conclusion: 'failure'}, {id: 1, name: 'Test / Test', app: {id: 15368}, status: 'in_progress', conclusion: null}, {id: 1, name: 'Test / Test', app: {id: 1}, status: 'completed', conclusion: 'success'}]) { - assert.equal((await scenario({...options, checks: check ? [check] : []})).statuses[0].state, 'pending'); + assert.equal((await scenario({...options, checks: check ? [check] : []})).statuses.at(-1).state, 'pending'); } const passed = {id: 2, name: 'Test / Test', app: {id: 15368}, status: 'completed', conclusion: 'success'}; - assert.equal((await scenario({...options, checks: [passed]})).statuses[0].state, 'success'); - assert.equal((await scenario({...options, checks: [passed, {...passed, id: 3, status: 'in_progress', conclusion: null}]})).statuses[0].state, 'pending'); - assert.equal((await scenario({...options, checks: [{...passed, details_url: null}]})).statuses[0].state, 'pending'); + assert.equal((await scenario({...options, checks: [passed]})).statuses.at(-1).state, 'success'); + assert.equal((await scenario({...options, checks: [passed, {...passed, id: 3, status: 'in_progress', conclusion: null}]})).statuses.at(-1).state, 'pending'); + assert.equal((await scenario({...options, checks: [{...passed, details_url: null}]})).statuses.at(-1).state, 'pending'); assert.equal((await scenario({...options, checks: [passed], event: {workflow_run: { - name: 'Test', head_sha: 'head', id: 3, status: 'queued'}}})).statuses[0].state, 'pending'); + name: 'Test', head_sha: 'head', id: 3, status: 'queued'}}})).statuses.at(-1).state, 'pending'); assert.equal((await scenario({...options, checks: [passed], event: {workflow_run: { - name: 'Test', head_sha: 'head', id: 3, status: 'completed', conclusion: 'failure'}}})).statuses[0].state, 'pending'); - assert.equal((await scenario({...options, checks: [passed], event: {workflow_run: { - name: 'Test', head_sha: 'head', id: 2, status: 'completed', conclusion: 'failure'}}})).statuses[0].state, 'pending'); + name: 'Test', head_sha: 'head', id: 3, status: 'completed', conclusion: 'failure'}}})).statuses.at(-1).state, 'pending'); + assert.equal((await scenario({...options, checks: [passed], runs: [{id: 2, run_attempt: 1, name: 'Test', path: '.github/workflows/test.yml', head_sha: 'head', status: 'completed', conclusion: 'failure'}], event: {workflow_run: { + name: 'Test', head_sha: 'head', id: 2, status: 'completed', conclusion: 'failure'}}})).statuses.at(-1).state, 'pending'); +}); + +const trial = {requireCoderabbit: false, testChecks: ['Test / Test'], pull: {user: {type: 'User', login: 'silveltman'}}}; +const testRun = {id: 100, run_attempt: 1, name: 'Test', path: '.github/workflows/test.yml', head_sha: 'head', status: 'completed', conclusion: 'success'}; +const testJob = {id: 200, name: 'Test / Test', status: 'completed', conclusion: 'success'}; + +test('Check events cannot pass a Test rerun using the preceding successful check', async () => { + const result = await scenario({...trial, checks: [{...testJob, app: {id: 15368}}], + runs: [{...testRun, run_attempt: 2, status: 'in_progress', conclusion: null}], jobs: [testJob], + event: {workflow_run: {name: 'Check', head_sha: 'head', status: 'completed', conclusion: 'success'}}}); + assert.equal(result.statuses.at(-1).state, 'pending'); +}); + +test('the exact latest Test attempt supplies every required successful job', async () => { + for (const jobs of [[], [{...testJob, conclusion: 'skipped'}], [{...testJob, conclusion: 'failure'}]]) { + const result = await scenario({...trial, runs: [{...testRun, run_attempt: 2}], jobRead: (_, parameters) => { + assert.equal(parameters.run_id, 100); + assert.equal(parameters.attempt_number, 2); + return jobs; + }}); + assert.equal(result.statuses.at(-1).state, 'pending'); + } + const result = await scenario({...trial, runs: [{...testRun, run_attempt: 2}], jobs: [testJob]}); + assert.equal(result.statuses.at(-1).state, 'success'); +}); + +test('completion retries metadata lag and passes without requiring another event', async () => { + let jobReads = 0; + const result = await scenario({...trial, + event: {workflow_run: {...testRun, id: 101}}, + runRead: reads => [reads === 1 ? testRun : {...testRun, id: 101}], + jobRead: () => ++jobReads === 1 ? [] : [testJob]}); + assert.equal(result.statuses.at(-1).state, 'success'); + assert.ok(jobReads >= 2); +}); + +test('persistent missing metadata is bounded and API errors revoke old success', async () => { + let reads = 0; + const missing = await scenario({...trial, runRead: () => {reads++; return [];}}); + assert.equal(reads, 4); + assert.equal(missing.statuses.at(-1).state, 'pending'); + const failed = await scenario({...trial, runRead: () => {throw new Error('Metadata unavailable');}}); + assert.equal(failed.statuses.at(-1).state, 'pending'); + assert.match(failed.errors[0], /Metadata unavailable/); }); From 96eeef8d0960e0a565b1ef7b58f0ad1612788fa4 Mon Sep 17 00:00:00 2001 From: Sil Date: Sat, 17 Oct 2026 16:00:35 +0200 Subject: [PATCH 4/6] Document the Ready caller permission for test attempt metadata --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 4399d3e..4ad5f90 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,8 @@ New projects take their callers from [astro-template](https://github.com/fulldot The trial callers in `fulldev`, `astro-template` and `vite-template` use `check:source` and the selected categories in `test.yml`. Devices stay manual. They pass `require-coderabbit: false` and require `Test / Test` through the existing `Review` status. Missing, skipped, failed or superseded test results cannot pass that gate. Other callers retain their existing behavior until migrated. +The Ready caller grants `actions: read` so the gate reads the current Test workflow attempt and its jobs, including reruns. Existing repositories remain pinned to their previous workflow revision until migrated. + Organization rules require `Check / Check` and `Review`, without requiring synchronization with main. Netlify requirements are managed centrally for repositories with those sites. Production keeps scoped source, build and artifact checks. Live URL tests run after the current production deployment has been confirmed, rather than inside its build command. Keep repository auto-merge disabled except for CloudCannon publishers. CloudCannon's Publish button keeps automatic publication for same-repository `preview` PRs created by `cloudcannon[bot]`, with all required checks and previews. Code PRs are merged by their agent. From 8c4b77bde6ba0c180f741561410bd0a773b83687 Mon Sep 17 00:00:00 2001 From: Sil Date: Sat, 17 Oct 2026 16:00:50 +0200 Subject: [PATCH 5/6] Grant the central Ready caller its metadata read permission --- .github/workflows/ci-ready.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci-ready.yml b/.github/workflows/ci-ready.yml index 351cfe6..654c643 100644 --- a/.github/workflows/ci-ready.yml +++ b/.github/workflows/ci-ready.yml @@ -9,6 +9,7 @@ on: types: [completed] permissions: + actions: read contents: write pull-requests: write statuses: write From 6aa20f6a6efe023bf21feac43ddd35eb5fa4e11b Mon Sep 17 00:00:00 2001 From: Sil Date: Sat, 17 Oct 2026 16:10:36 +0200 Subject: [PATCH 6/6] fix: reconcile lagging completion metadata without silent waits --- .github/workflows/ready.yml | 21 +++++++++++++++------ tests/ready.test.mjs | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ready.yml b/.github/workflows/ready.yml index fcd4404..15f1598 100644 --- a/.github/workflows/ready.yml +++ b/.github/workflows/ready.yml @@ -96,6 +96,7 @@ jobs: // Read the current run and attempt on every event. Check runs can // still show a previous success while the workflow is rerunning. const event = context.payload.workflow_run; + let completedRunPending = false; for (let retry = 0; retry < 4; retry++) { const runs = await github.paginate(github.rest.actions.listWorkflowRunsForRepo, {owner, repo, head_sha: sha, event: 'pull_request', per_page: 100}); @@ -105,19 +106,27 @@ jobs: const eventIsNewer = event?.name === 'Test' && event.head_sha === sha && (!run || event.id > run.id || (event.id === run.id && event.run_attempt > run.run_attempt)); + const eventIsCurrent = event?.name === 'Test' && event.head_sha === sha + && run && event.id === run.id && event.run_attempt === run.run_attempt; + const completedEvent = (eventIsNewer || eventIsCurrent) + && event.status === 'completed' && event.conclusion === 'success'; + completedRunPending = completedEvent + || (run?.status === 'completed' && run.conclusion === 'success'); if (eventIsNewer) { - if (event.status !== 'completed' || event.conclusion !== 'success') return false; + if (!completedEvent) return false; } else if (run) { - if (run.status !== 'completed' || run.conclusion !== 'success') return false; - const jobs = await github.paginate(github.rest.actions.listJobsForWorkflowRunAttempt, - {owner, repo, run_id: run.id, attempt_number: run.run_attempt, per_page: 100}); - if (requiredTests.every(name => jobs.some(job => job.name === name - && job.status === 'completed' && job.conclusion === 'success'))) return true; + if (run.status === 'completed' && run.conclusion === 'success') { + const jobs = await github.paginate(github.rest.actions.listJobsForWorkflowRunAttempt, + {owner, repo, run_id: run.id, attempt_number: run.run_attempt, per_page: 100}); + if (requiredTests.every(name => jobs.some(job => job.name === name + && job.status === 'completed' && job.conclusion === 'success'))) return true; + } else if (!completedEvent) return false; } // Successful completion can precede its jobs' metadata. Retry // briefly so a final completion event cannot strand the PR. if (retry < 3) await new Promise(resolve => setTimeout(resolve, 2000)); } + if (completedRunPending) throw new Error('Successful Test metadata is still incomplete. Rerun Ready to reconcile it.'); return false; } async function hasApproval() { diff --git a/tests/ready.test.mjs b/tests/ready.test.mjs index ae8b854..bfb35bb 100644 --- a/tests/ready.test.mjs +++ b/tests/ready.test.mjs @@ -343,3 +343,35 @@ test('persistent missing metadata is bounded and API errors revoke old success', assert.equal(failed.statuses.at(-1).state, 'pending'); assert.match(failed.errors[0], /Metadata unavailable/); }); + +test('completion retries the same run and attempt while its status is stale', async () => { + let runReads = 0; + let jobReads = 0; + const result = await scenario({...trial, event: {workflow_run: testRun}, + runRead: () => [++runReads === 1 ? {...testRun, status: 'in_progress', conclusion: null} : testRun], + jobRead: () => {jobReads++; return [testJob];}}); + assert.equal(result.statuses.at(-1).state, 'success'); + assert.ok(runReads >= 3); + assert.equal(jobReads, 2); + assert.deepEqual(result.errors, []); +}); + +test('a completed older attempt cannot authorize a newer pending attempt', async () => { + const result = await scenario({...trial, event: {workflow_run: testRun}, + runs: [{...testRun, run_attempt: 2, status: 'in_progress', conclusion: null}], jobs: [testJob]}); + assert.equal(result.statuses.at(-1).state, 'pending'); + assert.deepEqual(result.errors, []); +}); + +test('persistent lag after successful completion fails Ready instead of silently waiting', async () => { + for (const options of [ + {runs: [{...testRun, status: 'in_progress', conclusion: null}]}, + {runs: [testRun], jobs: []}, + {runs: []}, + ]) { + const result = await scenario({...trial, ...options, event: {workflow_run: testRun}}); + assert.equal(result.statuses.at(-1).state, 'pending'); + assert.match(result.errors[0], /metadata is still incomplete/); + assert.match(result.errors.at(-1), /could not be reconciled/); + } +});