From 8fdc05e6d88e7e8dd6d6a55603440c4973b9591a Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:34:15 -0400 Subject: [PATCH 01/12] ci: fleet review lanes show up as commit statuses on the PR head --- .github/workflows/fleet-status.yml | 79 ++++++++++ scripts/fleet-status.mjs | 240 ++++++++++++++++++++++++++++ scripts/fleet-status.test.mjs | 244 +++++++++++++++++++++++++++++ 3 files changed, 563 insertions(+) create mode 100644 .github/workflows/fleet-status.yml create mode 100644 scripts/fleet-status.mjs create mode 100644 scripts/fleet-status.test.mjs diff --git a/.github/workflows/fleet-status.yml b/.github/workflows/fleet-status.yml new file mode 100644 index 0000000..54559e0 --- /dev/null +++ b/.github/workflows/fleet-status.yml @@ -0,0 +1,79 @@ +# Posts fleet review-lane commit statuses for pull requests. +# Reads labels, comments, reviews and checks; writes statuses. Runs the default branch's script. +# self-test runs the script's tests on the PR's own code, read-only. +name: Fleet status + +on: + pull_request: + types: [opened, synchronize, reopened, ready_for_review, labeled, unlabeled] + pull_request_review: + types: [submitted, dismissed] + issue_comment: + types: [created, edited] + workflow_run: + workflows: [CI, CodeQL] + types: [completed] + +permissions: {} + +concurrency: + group: fleet-status-${{ github.event.pull_request.number || github.event.issue.number || github.event.workflow_run.pull_requests[0].number }} + cancel-in-progress: true + +jobs: + status: + # Fork PRs are not reviewed by the fleet, and their events carry a read-only token anyway. + # issue_comment fires for issues too; only PR comments matter. The script re-checks both. + if: >- + (github.event_name == 'issue_comment' && github.event.issue.pull_request != null) || + (github.event_name == 'workflow_run' && github.event.workflow_run.event == 'pull_request' && + github.event.workflow_run.head_repository.full_name == github.repository && + github.event.workflow_run.pull_requests[0] != null) || + (github.event_name != 'issue_comment' && github.event_name != 'workflow_run' && + github.event.pull_request.head.repo.full_name == github.repository) + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + pull-requests: read + issues: read + checks: read + statuses: write + steps: + - uses: askalf/checkout-with-retry@115a6407547e9711edbc2e915838d495cad9583f # v1.1.0 + with: + ref: ${{ github.event.repository.default_branch }} + sparse-checkout: scripts/fleet-status.mjs + sparse-checkout-cone-mode: false + persist-credentials: false + + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: 22 + + - name: Post the lane statuses + if: hashFiles('scripts/fleet-status.mjs') != '' + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + PR: ${{ github.event.pull_request.number || github.event.issue.number || github.event.workflow_run.pull_requests[0].number }} + TARGET_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: node scripts/fleet-status.mjs + + self-test: + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + steps: + - uses: askalf/checkout-with-retry@115a6407547e9711edbc2e915838d495cad9583f # v1.1.0 + with: + persist-credentials: false + + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: 22 + + - name: Test the lane rules + run: node scripts/fleet-status.test.mjs diff --git a/scripts/fleet-status.mjs b/scripts/fleet-status.mjs new file mode 100644 index 0000000..61ea229 --- /dev/null +++ b/scripts/fleet-status.mjs @@ -0,0 +1,240 @@ +#!/usr/bin/env node +// Computes fleet review-lane commit statuses from pull-request metadata: one status per lane on +// the PR head, pending while the lane waits, success once it has spoken at the head, failure when +// it said no. Reads labels, comments and reviews; writes statuses. +// +// The rules match the fleet dispatcher's: +// - A code PR (anything beyond docs, assets and .github config, from a person, on a non-bot +// branch) is verified first. Where the base branch requires status checks, it is verified +// when every required check has passed at the head. Elsewhere it needs the `verified` label +// AND a "## Verification at " comment by askalf naming the live head. +// - Redline's verdict counts only at the head. On code, its deterministic low-risk approval is +// not a verdict. +// - On code, the Second Read gates too: the newest of its reviews at the head that carries a +// `SECOND READ: READY` or `SECOND READ: NOT READY - ` line is its verdict. +// +// CLI (the workflow's only step): +// GITHUB_TOKEN=... REPO=owner/name PR= node scripts/fleet-status.mjs [--dry-run] + +import { pathToFileURL } from 'node:url'; + +export const REDLINE_LOGIN = 'sprayberry-redline'; +export const SECOND_READ_LOGIN = 'sprayberry-secondread'; +export const VERIFIER_LOGIN = 'askalf'; +export const DETERMINISTIC_APPROVAL_MARKER = '**Deterministic approval'; +export const CONTEXTS = { verify: 'fleet/verify', review: 'fleet/review', secondRead: 'fleet/second-read' }; + +const BOT_BRANCH = /^(bot\/|release\/|release-v?[0-9]|chore\/release-v?[0-9]|dependabot\/|receipts-)/; +const SCRIPT_EXT = /\.(js|mjs|cjs|ts|mts|cts|py|sh|bash|go|rb|ps1)$/i; + +/** A changed path that is not docs, an asset, .github config or .gitattributes. */ +export function isCodePath(path) { + if (/\.(md|svg|png|jpe?g|webp|gif)$/i.test(path)) return false; + if (/^docs\/.*\.txt$/i.test(path)) return false; + if (path === '.gitattributes') return false; + if (path.startsWith('.github/') && !/^\.github\/(actions|scripts)\//.test(path) && !SCRIPT_EXT.test(path)) return false; + return true; +} + +/** Dependabot, or a bot-shaped branch opened by askalf or github-actions: verification-exempt. */ +export function isBotPr(author, headRef) { + if (/^(app\/)?dependabot(\[bot\])?$/i.test(author ?? '')) return true; + return /^(askalf|(app\/)?github-actions(\[bot\])?)$/i.test(author ?? '') && BOT_BRANCH.test(headRef ?? ''); +} + +export function needsVerify(facts) { + if (isBotPr(facts.author, facts.headRef)) return false; + return facts.files.length >= 100 || facts.files.some(isCodePath); +} + +/** The label AND the verifier's latest "## Verification at " comment naming this head. */ +export function verifiedAtHead(facts) { + if (!facts.labels.includes('verified') || !facts.head) return false; + let at = null; + for (const c of facts.comments) { + if (c.login !== VERIFIER_LOGIN) continue; + const m = /^## Verification at ([0-9a-f]{7,40})/.exec(c.body ?? ''); + if (m) at = m[1]; + } + return at !== null && facts.head.startsWith(at); +} + +const CHECK_WAITING = /^(PENDING|EXPECTED|QUEUED|IN_PROGRESS|WAITING|REQUESTED)$/; +const CHECK_PASSED = /^(SUCCESS|NEUTRAL|SKIPPED)$/; + +/** + * The head's required checks: 'none' (the branch requires none), 'pending' (one has not reported + * or is still running), 'failed', or 'passed'. `checks` is in the order GitHub reported them; the + * last result per name counts. + * @param {string[]} required + * @param {Array<{name:string, state:string}>} checks + */ +export function requiredCiState(required, checks) { + if (!required.length) return 'none'; + const last = new Map(); + for (const c of checks) if (c.name) last.set(c.name, String(c.state ?? '').toUpperCase()); + let pending = false; + for (const r of required) { + const s = last.get(r) ?? ''; + if (s === '' || CHECK_WAITING.test(s)) pending = true; + else if (!CHECK_PASSED.test(s)) return 'failed'; + } + return pending ? 'pending' : 'passed'; +} + +/** Redline's latest verdict review, or null. On code, its deterministic approval does not count. */ +export function redlineVerdict(facts, code) { + let v = null; + for (const r of facts.reviews) { + if (r.login !== REDLINE_LOGIN) continue; + if (r.state !== 'APPROVED' && r.state !== 'CHANGES_REQUESTED') continue; + if (code && (r.body ?? '').startsWith(DETERMINISTIC_APPROVAL_MARKER)) continue; + v = r; + } + return v; +} + +/** The Second Read's verdict at this head: { state: READY | NOT READY | none, reason }. */ +export function secondReadAtHead(facts) { + let out = { state: 'none', reason: '' }; + for (const r of facts.reviews) { + if (r.login !== SECOND_READ_LOGIN || r.commitId !== facts.head) continue; + let last = null; + for (const m of (r.body ?? '').matchAll(/^SECOND READ: (READY[ \t\r]*$|NOT READY\b.*)$/gm)) last = m[1]; + if (last === null) continue; + out = last.startsWith('NOT READY') + ? { state: 'NOT READY', reason: last.replace(/^NOT READY\W*/, '').trim() } + : { state: 'READY', reason: '' }; + } + return out; +} + +const short = (sha) => (sha ?? '').slice(0, 7); +const fit = (s) => (s.length <= 140 ? s : `${s.slice(0, 137)}...`); + +/** + * The three statuses for a PR, from what GitHub says about it. + * @param {{head:string, headRef:string, author:string, files:string[], labels:string[], + * reviews:Array<{login:string,state:string,commitId:string,body:string}>, + * comments:Array<{login:string,body:string}>, requiredCi?:'none'|'pending'|'failed'|'passed'}} facts + * @returns {Array<{context:string, state:'pending'|'success'|'failure', description:string}>} + */ +export function laneStatuses(facts) { + const h = short(facts.head); + const code = needsVerify(facts); + const ci = facts.requiredCi ?? 'none'; + const verified = code && (ci === 'passed' || (ci === 'none' && verifiedAtHead(facts))); + const out = []; + + out.push(!code + ? { context: CONTEXTS.verify, state: 'success', description: 'Not required: docs, assets, .github config or a bot branch' } + : verified + ? { context: CONTEXTS.verify, state: 'success', description: ci === 'passed' ? `Required CI passed at ${h}` : `Verified at ${h}` } + : ci === 'failed' + ? { context: CONTEXTS.verify, state: 'failure', description: `A required check failed at ${h}` } + : ci === 'pending' + ? { context: CONTEXTS.verify, state: 'pending', description: `Waiting on required CI at ${h}` } + : { context: CONTEXTS.verify, state: 'pending', description: `Waiting on the Breaker to verify ${h}` }); + + const gated = code && !verified; + const rv = redlineVerdict(facts, code); + if (gated) { + out.push({ context: CONTEXTS.review, state: 'pending', description: `Redline reads ${h} once it is verified` }); + } else if (rv && rv.commitId === facts.head) { + out.push(rv.state === 'APPROVED' + ? { context: CONTEXTS.review, state: 'success', description: `Redline approved ${h}` } + : { context: CONTEXTS.review, state: 'failure', description: `Redline requested changes at ${h}` }); + } else { + const was = rv ? ` (its last verdict was on ${short(rv.commitId)})` : ''; + out.push({ context: CONTEXTS.review, state: 'pending', description: `Waiting on Redline at ${h}${was}` }); + } + + if (!code) { + out.push({ context: CONTEXTS.secondRead, state: 'success', description: 'Not gating: one non-gating opinion on this PR' }); + } else if (gated) { + out.push({ context: CONTEXTS.secondRead, state: 'pending', description: `The Second Read reads ${h} once it is verified` }); + } else { + const sr = secondReadAtHead(facts); + out.push(sr.state === 'READY' + ? { context: CONTEXTS.secondRead, state: 'success', description: `READY at ${h}` } + : sr.state === 'NOT READY' + ? { context: CONTEXTS.secondRead, state: 'failure', description: `NOT READY at ${h}${sr.reason ? `: ${sr.reason}` : ''}` } + : { context: CONTEXTS.secondRead, state: 'pending', description: `Waiting on the Second Read at ${h}` }); + } + + return out.map((s) => ({ ...s, description: fit(s.description) })); +} + +// CLI +async function gh(path, token, init = {}) { + const res = await fetch(`https://api.github.com${path}`, { + ...init, + headers: { authorization: `Bearer ${token}`, accept: 'application/vnd.github+json', 'x-github-api-version': '2022-11-28', ...(init.headers ?? {}) }, + }); + if (!res.ok) throw new Error(`${init.method ?? 'GET'} ${path}: HTTP ${res.status} ${await res.text()}`); + return res; +} + +async function ghAll(path, token) { + const out = []; + for (let page = 1; ; page++) { + const rows = await (await gh(`${path}${path.includes('?') ? '&' : '?'}per_page=100&page=${page}`, token)).json(); + out.push(...rows); + if (rows.length < 100) return out; + } +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + const { GITHUB_TOKEN: token, REPO: repo, PR: pr, TARGET_URL: targetUrl } = process.env; + const dryRun = process.argv.includes('--dry-run'); + if (!token || !repo || !/^\d+$/.test(pr ?? '')) { + console.error('usage: GITHUB_TOKEN=... REPO=owner/name PR= node scripts/fleet-status.mjs [--dry-run]'); + process.exit(2); + } + const p = await (await gh(`/repos/${repo}/pulls/${pr}`, token)).json(); + if (p.state !== 'open') { console.log(`#${pr} is ${p.state}; nothing to report`); process.exit(0); } + if (p.head?.repo?.full_name !== repo) { console.log(`#${pr} is a fork PR; the fleet does not review it`); process.exit(0); } + const [files, reviews, comments] = await Promise.all([ + ghAll(`/repos/${repo}/pulls/${pr}/files`, token), + ghAll(`/repos/${repo}/pulls/${pr}/reviews`, token), + ghAll(`/repos/${repo}/issues/${pr}/comments`, token), + ]); + // Unreadable rules count as none (the label-and-comment rule applies); unreadable checks as + // pending. Neither can turn fleet/verify green. + let required = []; + try { + const rules = await (await gh(`/repos/${repo}/rules/branches/${encodeURIComponent(p.base.ref)}?per_page=100`, token)).json(); + required = rules.filter((r) => r.type === 'required_status_checks') + .flatMap((r) => (r.parameters?.required_status_checks ?? []).map((c) => c.context)); + } catch { required = []; } + let requiredCi = 'none'; + if (required.length) { + try { + const statuses = (await ghAll(`/repos/${repo}/commits/${p.head.sha}/statuses`, token)).reverse() + .map((s) => ({ name: s.context, state: s.state })); + const runs = (await (await gh(`/repos/${repo}/commits/${p.head.sha}/check-runs?per_page=100`, token)).json()).check_runs ?? []; + const checks = runs.sort((a, b) => a.id - b.id) + .map((c) => ({ name: c.name, state: c.status === 'completed' ? (c.conclusion ?? '') : c.status })); + requiredCi = requiredCiState(required, [...statuses, ...checks]); + } catch { requiredCi = 'pending'; } + } + const facts = { + head: p.head.sha, + headRef: p.head.ref, + author: p.user?.login ?? '', + files: files.map((f) => f.filename), + labels: (p.labels ?? []).map((l) => l.name), + reviews: reviews.map((r) => ({ login: r.user?.login ?? '', state: r.state, commitId: r.commit_id ?? '', body: r.body ?? '' })), + comments: comments.map((c) => ({ login: c.user?.login ?? '', body: c.body ?? '' })), + requiredCi, + }; + for (const s of laneStatuses(facts)) { + console.log(`${s.context.padEnd(18)} ${s.state.padEnd(8)} ${s.description}`); + if (dryRun) continue; + await gh(`/repos/${repo}/statuses/${facts.head}`, token, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ ...s, target_url: targetUrl || p.html_url }), + }); + } +} diff --git a/scripts/fleet-status.test.mjs b/scripts/fleet-status.test.mjs new file mode 100644 index 0000000..fa89550 --- /dev/null +++ b/scripts/fleet-status.test.mjs @@ -0,0 +1,244 @@ +// Unit tests for scripts/fleet-status.mjs. Run: node scripts/fleet-status.test.mjs + +import { + laneStatuses, + isCodePath, + isBotPr, + verifiedAtHead, + secondReadAtHead, + CONTEXTS, + requiredCiState, + REDLINE_LOGIN, + SECOND_READ_LOGIN, + VERIFIER_LOGIN, +} from './fleet-status.mjs'; + +let pass = 0; +let fail = 0; +function check(name, cond) { + if (cond) { console.log(` ok ${name}`); pass++; } + else { console.log(` FAIL ${name}`); fail++; } +} + +const HEAD = '47536435fb5c9540d8cb36fd26d81e101955b364'; +const OLD = '34b7875f46525f7899a4e6601fbca4be75443903'; +const base = (over = {}) => ({ + head: HEAD, headRef: 'feat/opus-alias-5-5', author: 'askalf', + files: ['src/proxy.ts', 'test/opus-alias-fallback.mjs', 'CHANGELOG.md'], + labels: [], reviews: [], comments: [], ...over, +}); +const verification = (sha, login = VERIFIER_LOGIN) => ({ login, body: `## Verification at ${sha}\n\nbody` }); +const review = (login, state, commitId, body = '') => ({ login, state, commitId, body }); +const by = (rows) => Object.fromEntries(rows.map((s) => [s.context, s])); + +console.log('\n isCodePath / isBotPr'); +check('src is code', isCodePath('src/proxy.ts')); +check('markdown and images are not', !isCodePath('README.md') && !isCodePath('docs/art/x.PNG') && !isCodePath('docs/notes.txt')); +check('.github config is not', !isCodePath('.github/workflows/ci.yml') && !isCodePath('.github/labeler.yml')); +check('.github scripts are', isCodePath('.github/scripts/a.sh') && isCodePath('.github/workflows/helper.mjs')); +check('.gitattributes is not', !isCodePath('.gitattributes')); +check('askalf on bot/ is a bot PR', isBotPr('askalf', 'bot/cc-drift-v2.1.281')); +check('askalf on a feature branch is not', !isBotPr('askalf', 'feat/opus-alias-5-5')); +check('a person on bot/ is not', !isBotPr('someone', 'bot/cc-drift-v2.1.281')); +check('dependabot always is', isBotPr('dependabot[bot]', 'dependabot/npm/x')); + +console.log('\n verifiedAtHead'); +check('label + comment at head', verifiedAtHead(base({ labels: ['verified'], comments: [verification(HEAD)] }))); +check('a 7-char prefix counts', verifiedAtHead(base({ labels: ['verified'], comments: [verification(HEAD.slice(0, 7))] }))); +check('the label alone does not', !verifiedAtHead(base({ labels: ['verified'] }))); +check('a comment at an older head does not', !verifiedAtHead(base({ labels: ['verified'], comments: [verification(OLD)] }))); +check('the latest comment wins', !verifiedAtHead(base({ labels: ['verified'], comments: [verification(HEAD), verification(OLD)] }))); +check('another login does not count', !verifiedAtHead(base({ labels: ['verified'], comments: [verification(HEAD, 'someone')] }))); +check('findings heading does not count', !verifiedAtHead(base({ labels: ['verified'], comments: [{ login: VERIFIER_LOGIN, body: `## Verification findings at ${HEAD}` }] }))); + +console.log('\n code PR, not verified: everything waits on the Breaker'); +{ + const s = by(laneStatuses(base({ reviews: [review(REDLINE_LOGIN, 'APPROVED', HEAD)] }))); + check('verify pending', s[CONTEXTS.verify].state === 'pending' && s[CONTEXTS.verify].description.includes('4753643')); + check('review pending even with an approval at head', s[CONTEXTS.review].state === 'pending'); + check('second read pending', s[CONTEXTS.secondRead].state === 'pending'); +} + +console.log('\n the dario#1403 morning: verdicts on an older head'); +{ + const s = by(laneStatuses(base({ + labels: ['verified'], comments: [verification(HEAD)], + reviews: [ + review(REDLINE_LOGIN, 'CHANGES_REQUESTED', OLD), + review(SECOND_READ_LOGIN, 'COMMENTED', OLD, 'text\nSECOND READ: NOT READY \u2014 stale stack'), + ], + }))); + check('verify green', s[CONTEXTS.verify].state === 'success'); + check('an old CHANGES_REQUESTED is not a red at this head', s[CONTEXTS.review].state === 'pending' && s[CONTEXTS.review].description.includes('34b7875')); + check('an old NOT READY is not a red at this head', s[CONTEXTS.secondRead].state === 'pending'); +} + +console.log('\n verdicts at the head'); +{ + const s = by(laneStatuses(base({ + labels: ['verified'], comments: [verification(HEAD)], + reviews: [ + review(REDLINE_LOGIN, 'CHANGES_REQUESTED', OLD), + review(REDLINE_LOGIN, 'APPROVED', HEAD), + review(SECOND_READ_LOGIN, 'COMMENTED', HEAD, 'body\n\nSECOND READ: READY\n'), + ], + }))); + check('Redline approved', s[CONTEXTS.review].state === 'success'); + check('Second Read READY', s[CONTEXTS.secondRead].state === 'success'); +} +{ + const s = by(laneStatuses(base({ + labels: ['verified'], comments: [verification(HEAD)], + reviews: [ + review(REDLINE_LOGIN, 'CHANGES_REQUESTED', HEAD), + review(SECOND_READ_LOGIN, 'COMMENTED', HEAD, 'SECOND READ: NOT READY \u2014 commit subject has an em dash'), + ], + }))); + check('Redline changes requested is red', s[CONTEXTS.review].state === 'failure'); + check('NOT READY is red with its reason', s[CONTEXTS.secondRead].state === 'failure' && s[CONTEXTS.secondRead].description.endsWith('commit subject has an em dash')); +} +check('a Second Read without a verdict line is not READY', + secondReadAtHead(base({ reviews: [review(SECOND_READ_LOGIN, 'COMMENTED', HEAD, 'no verdict here')] })).state === 'none'); +check('READY followed by text is not READY', + secondReadAtHead(base({ reviews: [review(SECOND_READ_LOGIN, 'COMMENTED', HEAD, 'SECOND READ: READY, mostly')] })).state === 'none'); +check('the last verdict line in a body wins', + secondReadAtHead(base({ reviews: [review(SECOND_READ_LOGIN, 'COMMENTED', HEAD, 'SECOND READ: READY\nSECOND READ: NOT READY - x')] })).state === 'NOT READY'); + +console.log('\n deterministic approvals'); +{ + const det = review(REDLINE_LOGIN, 'APPROVED', HEAD, '**Deterministic approval** low-risk'); + const code = by(laneStatuses(base({ labels: ['verified'], comments: [verification(HEAD)], reviews: [det] }))); + check('on code it is not Redline\'s verdict', code[CONTEXTS.review].state === 'pending'); + const docs = by(laneStatuses(base({ files: ['README.md'], reviews: [det] }))); + check('on docs it is', docs[CONTEXTS.review].state === 'success'); +} + +console.log('\n exempt PRs'); +{ + const docs = by(laneStatuses(base({ files: ['README.md', 'docs/routing.md'] }))); + check('docs: verify not required', docs[CONTEXTS.verify].state === 'success' && docs[CONTEXTS.verify].description.startsWith('Not required')); + check('docs: second read not gating', docs[CONTEXTS.secondRead].state === 'success'); + check('docs: review still waits on Redline', docs[CONTEXTS.review].state === 'pending'); + const bot = by(laneStatuses(base({ headRef: 'bot/cc-drift-v2.1.281', reviews: [review(REDLINE_LOGIN, 'APPROVED', HEAD)] }))); + check('bot branch: verify not required, approval counts', bot[CONTEXTS.verify].state === 'success' && bot[CONTEXTS.review].state === 'success'); + const many = Array.from({ length: 100 }, (_, i) => `docs/p${i}.md`); + check('100 files is code whatever they are', by(laneStatuses(base({ files: many })))[CONTEXTS.verify].state === 'pending'); +} + +console.log('\n descriptions'); +{ + const long = 'x'.repeat(300); + const s = by(laneStatuses(base({ + labels: ['verified'], comments: [verification(HEAD)], + reviews: [review(SECOND_READ_LOGIN, 'COMMENTED', HEAD, `SECOND READ: NOT READY - ${long}`)], + }))); + check('capped at 140 characters', laneStatuses(base()).every((r) => r.description.length <= 140) && s[CONTEXTS.secondRead].description.length === 140); +} + +console.log('\n verifiedAtHead: the label and the comment are each required'); +check('a comment at head without the label does not count', !verifiedAtHead(base({ comments: [verification(HEAD)] }))); +check('an older comment followed by one at head counts', verifiedAtHead(base({ labels: ['verified'], comments: [verification(OLD), verification(HEAD)] }))); +check('a blocked heading does not count', !verifiedAtHead(base({ labels: ['verified'], comments: [{ login: VERIFIER_LOGIN, body: `## Verification blocked at ${HEAD}` }] }))); + +console.log('\n Redline reviews that are not verdicts, and verdicts in order'); +{ + const s = by(laneStatuses(base({ + labels: ['verified'], comments: [verification(HEAD)], + reviews: [review(REDLINE_LOGIN, 'APPROVED', OLD), review(REDLINE_LOGIN, 'DISMISSED', HEAD), review(REDLINE_LOGIN, 'COMMENTED', HEAD, 'notes')], + }))); + check('a dismissal and a comment at head leave the older approval as the last verdict', s[CONTEXTS.review].state === 'pending' && s[CONTEXTS.review].description.includes('34b7875')); +} +{ + const s = by(laneStatuses(base({ + labels: ['verified'], comments: [verification(HEAD)], + reviews: [review(REDLINE_LOGIN, 'APPROVED', HEAD), review(REDLINE_LOGIN, 'CHANGES_REQUESTED', HEAD)], + }))); + check('changes requested after an approval at the same head is red', s[CONTEXTS.review].state === 'failure'); +} +{ + const s = by(laneStatuses(base({ reviews: [review(REDLINE_LOGIN, 'CHANGES_REQUESTED', HEAD)] }))); + check('changes requested at an unverified head still waits on the Breaker', s[CONTEXTS.verify].state === 'pending' && s[CONTEXTS.review].state === 'pending' && s[CONTEXTS.secondRead].state === 'pending'); +} + +console.log('\n the Second Read, review by review'); +check('a Redline review carrying the line is not the Second Read', + secondReadAtHead(base({ reviews: [review(REDLINE_LOGIN, 'COMMENTED', HEAD, 'SECOND READ: READY')] })).state === 'none'); +check('the latest review at head wins', + secondReadAtHead(base({ reviews: [review(SECOND_READ_LOGIN, 'COMMENTED', HEAD, 'SECOND READ: NOT READY - x'), review(SECOND_READ_LOGIN, 'COMMENTED', HEAD, 'SECOND READ: READY')] })).state === 'READY'); +check('a later review without the line keeps the verdict', + secondReadAtHead(base({ reviews: [review(SECOND_READ_LOGIN, 'COMMENTED', HEAD, 'SECOND READ: NOT READY - x'), review(SECOND_READ_LOGIN, 'COMMENTED', HEAD, 'follow-up')] })).state === 'NOT READY'); +check('a verdict at an older head plus a lineless review at this head is none', + secondReadAtHead(base({ reviews: [review(SECOND_READ_LOGIN, 'COMMENTED', OLD, 'SECOND READ: READY'), review(SECOND_READ_LOGIN, 'COMMENTED', HEAD, 'read again')] })).state === 'none'); +{ + const s = by(laneStatuses(base({ + labels: ['verified'], comments: [verification(HEAD)], + reviews: [review(SECOND_READ_LOGIN, 'COMMENTED', HEAD, 'SECOND READ: NOT READY')], + }))); + check('NOT READY with no reason is red without a trailing colon', s[CONTEXTS.secondRead].state === 'failure' && s[CONTEXTS.secondRead].description === 'NOT READY at 4753643'); +} + +console.log('\n what counts as code'); +check('.github actions are', isCodePath('.github/actions/retry/action.yml')); +check('a .txt outside docs is', isCodePath('notes.txt') && isCodePath('src/fixtures/a.txt')); +check('a .github script keeps its case', isCodePath('.github/workflows/helper.MJS')); + +console.log('\n bot PRs'); +check('github-actions on bot/ is', isBotPr('github-actions[bot]', 'bot/x') && isBotPr('app/github-actions', 'bot/x')); +check('askalf on a release, receipts or dependabot branch is', + isBotPr('askalf', 'release-v6.12.0') && isBotPr('askalf', 'release/6.12') && isBotPr('askalf', 'chore/release-v6.12.0') && isBotPr('askalf', 'receipts-2026-09-24') && isBotPr('askalf', 'dependabot/npm/x')); +{ + const docs = (n) => Array.from({ length: n }, (_, i) => `docs/p${i}.md`); + check('a bot PR with 100 files is not code', by(laneStatuses(base({ headRef: 'bot/cc-drift-v2.1.281', files: docs(100) })))[CONTEXTS.verify].state === 'success'); + check('99 docs files are not code', by(laneStatuses(base({ files: docs(99) })))[CONTEXTS.verify].state === 'success'); +} + +console.log('\n docs PRs still show Redline\'s verdict'); +{ + const s = by(laneStatuses(base({ files: ['README.md'], reviews: [review(REDLINE_LOGIN, 'CHANGES_REQUESTED', HEAD)] }))); + check('changes requested on docs is red', s[CONTEXTS.review].state === 'failure' && s[CONTEXTS.secondRead].state === 'success'); + const old = by(laneStatuses(base({ files: ['README.md'], reviews: [review(REDLINE_LOGIN, 'CHANGES_REQUESTED', OLD)] }))); + check('changes requested on an older docs head is pending and says where', old[CONTEXTS.review].state === 'pending' && old[CONTEXTS.review].description.endsWith('(its last verdict was on 34b7875)')); +} + +console.log('\n the 140-character edge'); +{ + const at = (reason) => by(laneStatuses(base({ + labels: ['verified'], comments: [verification(HEAD)], + reviews: [review(SECOND_READ_LOGIN, 'COMMENTED', HEAD, `SECOND READ: NOT READY - ${reason}`)], + })))[CONTEXTS.secondRead].description; + check('exactly 140 characters is kept whole', at('y'.repeat(118)) === `NOT READY at 4753643: ${'y'.repeat(118)}`); + check('141 characters is cut to 137 and an ellipsis', at('y'.repeat(119)) === `NOT READY at 4753643: ${'y'.repeat(115)}...`); +} + +console.log('\n required CI is the verification where the base branch requires checks'); +{ + const REQ = ['test', 'build (22)', 'live-test']; + const ok = (name) => ({ name, state: 'success' }); + check('no required checks -> none', requiredCiState([], [ok('test')]) === 'none'); + check('all required passed -> passed', requiredCiState(REQ, REQ.map(ok)) === 'passed'); + check('a required check not reported yet -> pending', requiredCiState(REQ, [ok('test'), ok('build (22)')]) === 'pending'); + check('a required check still running -> pending', requiredCiState(REQ, [ok('test'), ok('build (22)'), { name: 'live-test', state: 'in_progress' }]) === 'pending'); + check('a required check failed -> failed, even with another pending', + requiredCiState(REQ, [{ name: 'test', state: 'failure' }, ok('build (22)')]) === 'failed'); + check('the last result per check counts (a rerun that passed)', + requiredCiState(['test'], [{ name: 'test', state: 'failure' }, ok('test')]) === 'passed'); + check('skipped and neutral pass', requiredCiState(['a', 'b'], [{ name: 'a', state: 'skipped' }, { name: 'b', state: 'neutral' }]) === 'passed'); + check('checks nobody requires do not hold it', requiredCiState(['test'], [ok('test'), { name: 'fleet/verify', state: 'pending' }]) === 'passed'); + + const lanes = (over) => by(laneStatuses(base(over))); + const passed = lanes({ requiredCi: 'passed' }); + check('CI passed: fleet/verify green with no label or comment', passed[CONTEXTS.verify].state === 'success' + && passed[CONTEXTS.verify].description === 'Required CI passed at 4753643'); + check('CI passed: Redline and the Second Read are waited on, not held for a Breaker', + passed[CONTEXTS.review].description === 'Waiting on Redline at 4753643' + && passed[CONTEXTS.secondRead].description === 'Waiting on the Second Read at 4753643'); + check('CI pending: fleet/verify waits on CI, not the Breaker', lanes({ requiredCi: 'pending' })[CONTEXTS.verify].description === 'Waiting on required CI at 4753643'); + check('CI failed: fleet/verify red', lanes({ requiredCi: 'failed' })[CONTEXTS.verify].state === 'failure'); + check('CI pending: an old Breaker label and comment do not count', + lanes({ requiredCi: 'pending', labels: ['verified'], comments: [verification(HEAD)] })[CONTEXTS.verify].state === 'pending'); + check('no required checks: the label and comment still verify', + lanes({ requiredCi: 'none', labels: ['verified'], comments: [verification(HEAD)] })[CONTEXTS.verify].description === 'Verified at 4753643'); +} + +console.log(`\n ${pass} pass, ${fail} fail`); +if (fail > 0) process.exit(1); From 743e0f7f2a5c31a1db2df03836a5bf338a14aa91 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:36:47 -0400 Subject: [PATCH 02/12] ci(fleet-status): a manual run posts the lanes on every open PR --- .github/workflows/fleet-status.yml | 41 ++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/.github/workflows/fleet-status.yml b/.github/workflows/fleet-status.yml index 54559e0..8dcd6be 100644 --- a/.github/workflows/fleet-status.yml +++ b/.github/workflows/fleet-status.yml @@ -1,9 +1,11 @@ # Posts fleet review-lane commit statuses for pull requests. # Reads labels, comments, reviews and checks; writes statuses. Runs the default branch's script. # self-test runs the script's tests on the PR's own code, read-only. +# backfill (manual) posts them on every open PR, e.g. once they become required. name: Fleet status on: + workflow_dispatch: pull_request: types: [opened, synchronize, reopened, ready_for_review, labeled, unlabeled] pull_request_review: @@ -77,3 +79,42 @@ jobs: - name: Test the lane rules run: node scripts/fleet-status.test.mjs + + backfill: + # Manual: post the lane statuses on every open same-repo PR. Run it right after + # the fleet/* contexts become required checks, so PRs opened before that report. + if: github.event_name == 'workflow_dispatch' + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: read + pull-requests: read + issues: read + checks: read + statuses: write + steps: + - uses: askalf/checkout-with-retry@115a6407547e9711edbc2e915838d495cad9583f # v1.1.0 + with: + ref: ${{ github.event.repository.default_branch }} + sparse-checkout: scripts/fleet-status.mjs + sparse-checkout-cone-mode: false + persist-credentials: false + + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: 22 + + - name: Post the lane statuses on every open PR + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + TARGET_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + set -euo pipefail + prs="$(gh pr list --repo "$REPO" --state open --limit 100 --json number,isCrossRepository \ + --jq '.[] | select(.isCrossRepository | not) | .number')" + for pr in $prs; do + echo "== #$pr" + PR="$pr" node scripts/fleet-status.mjs + done From 93f585051e17d1ec7a2a3f2512da1fb01c6cec6e Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:58:51 -0400 Subject: [PATCH 03/12] ci(fleet-status): recompute on deleted comments and edited reviews; pin the bot rule A deleted `## Verification at` comment or an edited review's SECOND READ line changes the lanes, but neither event re-ran the workflow, so a green status could outlive what it stood for. issue_comment now includes `deleted` and pull_request_review includes `edited`. isBotPr is unchanged on purpose: it is the dispatcher's rule. review-dispatch.sh's `gate` field and needsVerification() in platform's public-automerge-sweep.ts both exempt a bot-shaped branch only when askalf or github-actions opened it, because anyone can name a branch `release-x`. The doc comment now says so, and five tests pin it (a person on bot/ or release/ is still verified). --- .github/workflows/fleet-status.yml | 4 ++-- scripts/fleet-status.mjs | 7 ++++++- scripts/fleet-status.test.mjs | 9 +++++++++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/.github/workflows/fleet-status.yml b/.github/workflows/fleet-status.yml index 8dcd6be..60f9cc8 100644 --- a/.github/workflows/fleet-status.yml +++ b/.github/workflows/fleet-status.yml @@ -9,9 +9,9 @@ on: pull_request: types: [opened, synchronize, reopened, ready_for_review, labeled, unlabeled] pull_request_review: - types: [submitted, dismissed] + types: [submitted, edited, dismissed] issue_comment: - types: [created, edited] + types: [created, edited, deleted] workflow_run: workflows: [CI, CodeQL] types: [completed] diff --git a/scripts/fleet-status.mjs b/scripts/fleet-status.mjs index 61ea229..dd03d05 100644 --- a/scripts/fleet-status.mjs +++ b/scripts/fleet-status.mjs @@ -36,7 +36,12 @@ export function isCodePath(path) { return true; } -/** Dependabot, or a bot-shaped branch opened by askalf or github-actions: verification-exempt. */ +/** + * Dependabot, or a bot-shaped branch opened by askalf or github-actions: verification-exempt. + * The branch name alone is not enough, because anyone can name a branch `release-x`. This is + * the dispatcher's rule: review-dispatch.sh's `gate` field and needsVerification() in + * public-automerge-sweep.ts both require one of our identities AND a bot-shaped branch. + */ export function isBotPr(author, headRef) { if (/^(app\/)?dependabot(\[bot\])?$/i.test(author ?? '')) return true; return /^(askalf|(app\/)?github-actions(\[bot\])?)$/i.test(author ?? '') && BOT_BRANCH.test(headRef ?? ''); diff --git a/scripts/fleet-status.test.mjs b/scripts/fleet-status.test.mjs index fa89550..7489cdb 100644 --- a/scripts/fleet-status.test.mjs +++ b/scripts/fleet-status.test.mjs @@ -240,5 +240,14 @@ console.log('\n required CI is the verification where the base branch requires lanes({ requiredCi: 'none', labels: ['verified'], comments: [verification(HEAD)] })[CONTEXTS.verify].description === 'Verified at 4753643'); } +{ + // The dispatcher exempts a bot-shaped branch only when one of our identities opened it. + check('a person on a bot-shaped branch is not a bot PR', !isBotPr('contributor', 'bot/maintenance')); + check('a person on release/1.2 is not a bot PR', !isBotPr('someone', 'release/1.2')); + check('askalf on bot/drift is a bot PR', isBotPr('askalf', 'bot/drift')); + check('github-actions on receipts-2026 is a bot PR', isBotPr('github-actions[bot]', 'receipts-2026')); + check('dependabot on any branch is a bot PR', isBotPr('dependabot[bot]', 'feature/x')); +} + console.log(`\n ${pass} pass, ${fail} fail`); if (fail > 0) process.exit(1); From 6e771d80ab8f2e2e8b336fd18cf1a8f01d6fef89 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Thu, 24 Sep 2026 23:04:13 -0400 Subject: [PATCH 04/12] ci(fleet-status): no cancelled runs on the PR; a newer run always wins The concurrency group cancelled an in-flight run whenever a review, comment or CI completion landed close behind another event. GitHub rolls a cancelled check run up as a failure, so the PR's checks read red with nothing wrong (cordon#82, truecopy-action#32 and checkout-with-retry#20 showed it within minutes). The group goes. Ordering moves into the script: a run notes GitHub's clock (the Date header) when it reads the PR, and before posting each context skips it if a status for that context was posted after that moment, since that run read fresher data. postedSince() is pure and has five tests (83/83). --- .github/workflows/fleet-status.yml | 6 ++---- scripts/fleet-status.mjs | 17 ++++++++++++++++- scripts/fleet-status.test.mjs | 12 ++++++++++++ 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/.github/workflows/fleet-status.yml b/.github/workflows/fleet-status.yml index 60f9cc8..6703d73 100644 --- a/.github/workflows/fleet-status.yml +++ b/.github/workflows/fleet-status.yml @@ -2,6 +2,8 @@ # Reads labels, comments, reviews and checks; writes statuses. Runs the default branch's script. # self-test runs the script's tests on the PR's own code, read-only. # backfill (manual) posts them on every open PR, e.g. once they become required. +# No concurrency group: a cancelled run rolls up as a failed check on the PR. The script +# orders runs itself, so an older run never overwrites a newer one's statuses. name: Fleet status on: @@ -18,10 +20,6 @@ on: permissions: {} -concurrency: - group: fleet-status-${{ github.event.pull_request.number || github.event.issue.number || github.event.workflow_run.pull_requests[0].number }} - cancel-in-progress: true - jobs: status: # Fork PRs are not reviewed by the fleet, and their events carry a read-only token anyway. diff --git a/scripts/fleet-status.mjs b/scripts/fleet-status.mjs index dd03d05..ed0ae51 100644 --- a/scripts/fleet-status.mjs +++ b/scripts/fleet-status.mjs @@ -114,6 +114,16 @@ export function secondReadAtHead(facts) { return out; } +/** + * True when a status for `context` was posted after `readAtMs` (GitHub's clock when this run + * read the PR): another run read fresher data and posted it, so this run must not overwrite it. + * @param {Array<{context:string, created_at:string}>} statuses + */ +export function postedSince(statuses, context, readAtMs) { + if (!Number.isFinite(readAtMs)) return false; + return statuses.some((s) => s.context === context && Date.parse(s.created_at) > readAtMs); +} + const short = (sha) => (sha ?? '').slice(0, 7); const fit = (s) => (s.length <= 140 ? s : `${s.slice(0, 137)}...`); @@ -196,7 +206,10 @@ if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) console.error('usage: GITHUB_TOKEN=... REPO=owner/name PR= node scripts/fleet-status.mjs [--dry-run]'); process.exit(2); } - const p = await (await gh(`/repos/${repo}/pulls/${pr}`, token)).json(); + const pres = await gh(`/repos/${repo}/pulls/${pr}`, token); + // GitHub's clock at the read, the same clock that stamps statuses (see postedSince). + const readAt = Date.parse(pres.headers.get('date') ?? ''); + const p = await pres.json(); if (p.state !== 'open') { console.log(`#${pr} is ${p.state}; nothing to report`); process.exit(0); } if (p.head?.repo?.full_name !== repo) { console.log(`#${pr} is a fork PR; the fleet does not review it`); process.exit(0); } const [files, reviews, comments] = await Promise.all([ @@ -233,9 +246,11 @@ if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) comments: comments.map((c) => ({ login: c.user?.login ?? '', body: c.body ?? '' })), requiredCi, }; + const posted = dryRun ? [] : await ghAll(`/repos/${repo}/commits/${facts.head}/statuses`, token); for (const s of laneStatuses(facts)) { console.log(`${s.context.padEnd(18)} ${s.state.padEnd(8)} ${s.description}`); if (dryRun) continue; + if (postedSince(posted, s.context, readAt)) { console.log(' (a newer run already posted this; skipped)'); continue; } await gh(`/repos/${repo}/statuses/${facts.head}`, token, { method: 'POST', headers: { 'content-type': 'application/json' }, diff --git a/scripts/fleet-status.test.mjs b/scripts/fleet-status.test.mjs index 7489cdb..ca96d88 100644 --- a/scripts/fleet-status.test.mjs +++ b/scripts/fleet-status.test.mjs @@ -4,6 +4,7 @@ import { laneStatuses, isCodePath, isBotPr, + postedSince, verifiedAtHead, secondReadAtHead, CONTEXTS, @@ -249,5 +250,16 @@ console.log('\n required CI is the verification where the base branch requires check('dependabot on any branch is a bot PR', isBotPr('dependabot[bot]', 'feature/x')); } +{ + // A newer run's status wins; an older run skips a context posted after its read. + const readAt = Date.parse('2026-09-25T03:00:10Z'); + const st = (context, at) => ({ context, created_at: at }); + check('posted after our read: skip', postedSince([st('fleet/review', '2026-09-25T03:00:11Z')], 'fleet/review', readAt)); + check('posted before our read: overwrite', !postedSince([st('fleet/review', '2026-09-25T03:00:09Z')], 'fleet/review', readAt)); + check('posted in the same second: overwrite', !postedSince([st('fleet/review', '2026-09-25T03:00:10Z')], 'fleet/review', readAt)); + check('another context does not count', !postedSince([st('fleet/verify', '2026-09-25T03:00:30Z')], 'fleet/review', readAt)); + check('no server date: never skip', !postedSince([st('fleet/review', '2026-09-25T03:00:30Z')], 'fleet/review', NaN)); +} + console.log(`\n ${pass} pass, ${fail} fail`); if (fail > 0) process.exit(1); From 0e1fca7b28921d5e53adeb06730d24e2a6b13cb8 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Thu, 24 Sep 2026 23:09:05 -0400 Subject: [PATCH 05/12] ci(fleet-status): the lanes this script posts are never CI The Second Read on amnesia#83 and redstamp#162: once fleet/verify, fleet/review and fleet/second-read are required checks (the step this PR plans next), the branch rules list them, and requiredCiState counted them as CI the head waits on. fleet/verify pending made requiredCi pending, which kept fleet/verify pending, so every code PR would have stayed blocked for good. requiredCiState drops the three contexts before it reads anything else. Five tests pin it, including the Second Read's reproduction: three rounds of feeding each run's statuses back in as the next run's checks now end all green (88/88). --- scripts/fleet-status.mjs | 5 ++++- scripts/fleet-status.test.mjs | 26 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/scripts/fleet-status.mjs b/scripts/fleet-status.mjs index ed0ae51..965ebb2 100644 --- a/scripts/fleet-status.mjs +++ b/scripts/fleet-status.mjs @@ -23,6 +23,7 @@ export const SECOND_READ_LOGIN = 'sprayberry-secondread'; export const VERIFIER_LOGIN = 'askalf'; export const DETERMINISTIC_APPROVAL_MARKER = '**Deterministic approval'; export const CONTEXTS = { verify: 'fleet/verify', review: 'fleet/review', secondRead: 'fleet/second-read' }; +const OWN_CONTEXTS = new Set(Object.values(CONTEXTS)); const BOT_BRANCH = /^(bot\/|release\/|release-v?[0-9]|chore\/release-v?[0-9]|dependabot\/|receipts-)/; const SCRIPT_EXT = /\.(js|mjs|cjs|ts|mts|cts|py|sh|bash|go|rb|ps1)$/i; @@ -70,11 +71,13 @@ const CHECK_PASSED = /^(SUCCESS|NEUTRAL|SKIPPED)$/; /** * The head's required checks: 'none' (the branch requires none), 'pending' (one has not reported * or is still running), 'failed', or 'passed'. `checks` is in the order GitHub reported them; the - * last result per name counts. + * last result per name counts. The lanes this script posts (CONTEXTS) are never CI: once they are + * required checks themselves, counting them would leave fleet/verify waiting on itself forever. * @param {string[]} required * @param {Array<{name:string, state:string}>} checks */ export function requiredCiState(required, checks) { + required = required.filter((r) => !OWN_CONTEXTS.has(r)); if (!required.length) return 'none'; const last = new Map(); for (const c of checks) if (c.name) last.set(c.name, String(c.state ?? '').toUpperCase()); diff --git a/scripts/fleet-status.test.mjs b/scripts/fleet-status.test.mjs index ca96d88..bb4c7f0 100644 --- a/scripts/fleet-status.test.mjs +++ b/scripts/fleet-status.test.mjs @@ -261,5 +261,31 @@ console.log('\n required CI is the verification where the base branch requires check('no server date: never skip', !postedSince([st('fleet/review', '2026-09-25T03:00:30Z')], 'fleet/review', NaN)); } +{ + // The fleet/* lanes as required checks (the step after rollout) must not hold themselves. + const ci = ['test', 'analyze']; + const own = [CONTEXTS.verify, CONTEXTS.review, CONTEXTS.secondRead]; + const green = ci.map((name) => ({ name, state: 'SUCCESS' })); + check('own lanes required, CI green: passed', requiredCiState([...ci, ...own], green) === 'passed'); + check('own lanes required and pending, CI green: still passed', + requiredCiState([...ci, ...own], [...green, ...own.map((name) => ({ name, state: 'PENDING' }))]) === 'passed'); + check('only own lanes required: none (the Breaker rule applies)', requiredCiState(own, []) === 'none'); + check('own lanes required, a real check running: pending', + requiredCiState([...ci, ...own], [{ name: 'test', state: 'SUCCESS' }, { name: 'analyze', state: 'IN_PROGRESS' }]) === 'pending'); + // Feed each run's statuses back in as the next run's checks, three rounds, as the Second Read did. + let posted = []; + let states = []; + for (let round = 0; round < 3; round++) { + const requiredCi = requiredCiState([...ci, ...own], [...green, ...posted]); + const out = laneStatuses(base({ requiredCi, reviews: [ + review(REDLINE_LOGIN, 'APPROVED', HEAD), + review(SECOND_READ_LOGIN, 'COMMENTED', HEAD, 'SECOND READ: READY'), + ] })); + posted = out.map((x) => ({ name: x.context, state: x.state.toUpperCase() })); + states = out.map((x) => x.state); + } + check('own lanes required, three rounds: all three green', states.join() === 'success,success,success'); +} + console.log(`\n ${pass} pass, ${fail} fail`); if (fail > 0) process.exit(1); From 0f39ffc13b57ddc23edb06d11d25b89adba19abb Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Thu, 24 Sep 2026 23:15:22 -0400 Subject: [PATCH 06/12] ci(fleet-status): post then verify; dismissed reads do not count; >100 files Redline on browser-bridge#114, truecopy#212 and plumbline#52: - A dismissed Second Read review is not a verdict. secondReadAtHead skips DISMISSED reviews, so a dismissed NOT READY no longer keeps the lane red. - Ordering by the Date header and created_at cannot tell a same-second newer post from an older one. postedSince is gone. Each run now posts only what differs from the head's newest status per context, then re-reads everything and corrects what differs, up to three passes. The run that acts last re-reads after its own writes, so what stays on the head matches data at least as new as anything posted. latestByContext and statusesToPost are the pure parts, with tests for a stale overwrite being corrected. - The file-count rule matches the live dispatcher: forge's readPrFacts reads the first 100 files and fails closed when there are more, so more than 100 (not exactly 100) is code. A large docs-only PR still verifies once its required CI passes. 94/94. --- scripts/fleet-status.mjs | 142 ++++++++++++++++++++-------------- scripts/fleet-status.test.mjs | 58 ++++++++++---- 2 files changed, 128 insertions(+), 72 deletions(-) diff --git a/scripts/fleet-status.mjs b/scripts/fleet-status.mjs index 965ebb2..8b338ee 100644 --- a/scripts/fleet-status.mjs +++ b/scripts/fleet-status.mjs @@ -50,7 +50,9 @@ export function isBotPr(author, headRef) { export function needsVerify(facts) { if (isBotPr(facts.author, facts.headRef)) return false; - return facts.files.length >= 100 || facts.files.some(isCodePath); + // More than 100 files: the dispatcher reads the first 100 and fails closed on the rest + // (readPrFacts in platform's review-events.ts), so the lanes do the same. + return facts.files.length > 100 || facts.files.some(isCodePath); } /** The label AND the verifier's latest "## Verification at " comment naming this head. */ @@ -106,7 +108,8 @@ export function redlineVerdict(facts, code) { export function secondReadAtHead(facts) { let out = { state: 'none', reason: '' }; for (const r of facts.reviews) { - if (r.login !== SECOND_READ_LOGIN || r.commitId !== facts.head) continue; + // A dismissed review no longer stands, whatever its body says. + if (r.login !== SECOND_READ_LOGIN || r.commitId !== facts.head || r.state === 'DISMISSED') continue; let last = null; for (const m of (r.body ?? '').matchAll(/^SECOND READ: (READY[ \t\r]*$|NOT READY\b.*)$/gm)) last = m[1]; if (last === null) continue; @@ -117,14 +120,21 @@ export function secondReadAtHead(facts) { return out; } -/** - * True when a status for `context` was posted after `readAtMs` (GitHub's clock when this run - * read the PR): another run read fresher data and posted it, so this run must not overwrite it. - * @param {Array<{context:string, created_at:string}>} statuses - */ -export function postedSince(statuses, context, readAtMs) { - if (!Number.isFinite(readAtMs)) return false; - return statuses.some((s) => s.context === context && Date.parse(s.created_at) > readAtMs); +/** The newest status per context. GitHub lists a commit's statuses newest first. */ +export function latestByContext(statuses) { + const out = new Map(); + for (const s of statuses) { + if (!out.has(s.context)) out.set(s.context, { state: s.state, description: s.description ?? '' }); + } + return out; +} + +/** The statuses in `want` that the head does not already show exactly (state and description). */ +export function statusesToPost(want, have) { + return want.filter((s) => { + const h = have.get(s.context); + return !h || h.state !== s.state || h.description !== s.description; + }); } const short = (sha) => (sha ?? '').slice(0, 7); @@ -209,55 +219,71 @@ if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) console.error('usage: GITHUB_TOKEN=... REPO=owner/name PR= node scripts/fleet-status.mjs [--dry-run]'); process.exit(2); } - const pres = await gh(`/repos/${repo}/pulls/${pr}`, token); - // GitHub's clock at the read, the same clock that stamps statuses (see postedSince). - const readAt = Date.parse(pres.headers.get('date') ?? ''); - const p = await pres.json(); - if (p.state !== 'open') { console.log(`#${pr} is ${p.state}; nothing to report`); process.exit(0); } - if (p.head?.repo?.full_name !== repo) { console.log(`#${pr} is a fork PR; the fleet does not review it`); process.exit(0); } - const [files, reviews, comments] = await Promise.all([ - ghAll(`/repos/${repo}/pulls/${pr}/files`, token), - ghAll(`/repos/${repo}/pulls/${pr}/reviews`, token), - ghAll(`/repos/${repo}/issues/${pr}/comments`, token), - ]); - // Unreadable rules count as none (the label-and-comment rule applies); unreadable checks as - // pending. Neither can turn fleet/verify green. - let required = []; - try { - const rules = await (await gh(`/repos/${repo}/rules/branches/${encodeURIComponent(p.base.ref)}?per_page=100`, token)).json(); - required = rules.filter((r) => r.type === 'required_status_checks') - .flatMap((r) => (r.parameters?.required_status_checks ?? []).map((c) => c.context)); - } catch { required = []; } - let requiredCi = 'none'; - if (required.length) { + + /** Everything the lanes depend on, read fresh. Null when the PR is closed or from a fork. */ + async function readFacts() { + const p = await (await gh(`/repos/${repo}/pulls/${pr}`, token)).json(); + if (p.state !== 'open') { console.log(`#${pr} is ${p.state}; nothing to report`); return null; } + if (p.head?.repo?.full_name !== repo) { console.log(`#${pr} is a fork PR; the fleet does not review it`); return null; } + const [files, reviews, comments] = await Promise.all([ + ghAll(`/repos/${repo}/pulls/${pr}/files`, token), + ghAll(`/repos/${repo}/pulls/${pr}/reviews`, token), + ghAll(`/repos/${repo}/issues/${pr}/comments`, token), + ]); + // Unreadable rules count as none (the label-and-comment rule applies); unreadable checks as + // pending. Neither can turn fleet/verify green. + let required = []; try { - const statuses = (await ghAll(`/repos/${repo}/commits/${p.head.sha}/statuses`, token)).reverse() - .map((s) => ({ name: s.context, state: s.state })); - const runs = (await (await gh(`/repos/${repo}/commits/${p.head.sha}/check-runs?per_page=100`, token)).json()).check_runs ?? []; - const checks = runs.sort((a, b) => a.id - b.id) - .map((c) => ({ name: c.name, state: c.status === 'completed' ? (c.conclusion ?? '') : c.status })); - requiredCi = requiredCiState(required, [...statuses, ...checks]); - } catch { requiredCi = 'pending'; } + const rules = await (await gh(`/repos/${repo}/rules/branches/${encodeURIComponent(p.base.ref)}?per_page=100`, token)).json(); + required = rules.filter((r) => r.type === 'required_status_checks') + .flatMap((r) => (r.parameters?.required_status_checks ?? []).map((c) => c.context)); + } catch { required = []; } + let requiredCi = 'none'; + if (required.length) { + try { + const statuses = (await ghAll(`/repos/${repo}/commits/${p.head.sha}/statuses`, token)).reverse() + .map((s) => ({ name: s.context, state: s.state })); + const runs = (await (await gh(`/repos/${repo}/commits/${p.head.sha}/check-runs?per_page=100`, token)).json()).check_runs ?? []; + const checks = runs.sort((a, b) => a.id - b.id) + .map((c) => ({ name: c.name, state: c.status === 'completed' ? (c.conclusion ?? '') : c.status })); + requiredCi = requiredCiState(required, [...statuses, ...checks]); + } catch { requiredCi = 'pending'; } + } + return { + url: p.html_url, + facts: { + head: p.head.sha, + headRef: p.head.ref, + author: p.user?.login ?? '', + files: files.map((f) => f.filename), + labels: (p.labels ?? []).map((l) => l.name), + reviews: reviews.map((r) => ({ login: r.user?.login ?? '', state: r.state, commitId: r.commit_id ?? '', body: r.body ?? '' })), + comments: comments.map((c) => ({ login: c.user?.login ?? '', body: c.body ?? '' })), + requiredCi, + }, + }; } - const facts = { - head: p.head.sha, - headRef: p.head.ref, - author: p.user?.login ?? '', - files: files.map((f) => f.filename), - labels: (p.labels ?? []).map((l) => l.name), - reviews: reviews.map((r) => ({ login: r.user?.login ?? '', state: r.state, commitId: r.commit_id ?? '', body: r.body ?? '' })), - comments: comments.map((c) => ({ login: c.user?.login ?? '', body: c.body ?? '' })), - requiredCi, - }; - const posted = dryRun ? [] : await ghAll(`/repos/${repo}/commits/${facts.head}/statuses`, token); - for (const s of laneStatuses(facts)) { - console.log(`${s.context.padEnd(18)} ${s.state.padEnd(8)} ${s.description}`); - if (dryRun) continue; - if (postedSince(posted, s.context, readAt)) { console.log(' (a newer run already posted this; skipped)'); continue; } - await gh(`/repos/${repo}/statuses/${facts.head}`, token, { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ ...s, target_url: targetUrl || p.html_url }), - }); + + // Post, then read everything again and correct what differs. Another run can read older data + // and post after this one; the run that acts last re-reads after its own writes, so what stays + // on the head matches data at least as new as anything posted. Three passes bound a busy PR; + // the next event covers anything after that. + for (let pass = 1; pass <= 3; pass++) { + const read = await readFacts(); + if (!read) break; + const want = laneStatuses(read.facts); + if (pass === 1) for (const s of want) console.log(`${s.context.padEnd(18)} ${s.state.padEnd(8)} ${s.description}`); + if (dryRun) break; + const have = latestByContext(await ghAll(`/repos/${repo}/commits/${read.facts.head}/statuses`, token)); + const todo = statusesToPost(want, have); + if (!todo.length) break; + if (pass > 1) console.log(`pass ${pass}: correcting ${todo.map((s) => s.context).join(', ')}`); + for (const s of todo) { + await gh(`/repos/${repo}/statuses/${read.facts.head}`, token, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ ...s, target_url: targetUrl || read.url }), + }); + } } } diff --git a/scripts/fleet-status.test.mjs b/scripts/fleet-status.test.mjs index bb4c7f0..3a87552 100644 --- a/scripts/fleet-status.test.mjs +++ b/scripts/fleet-status.test.mjs @@ -4,7 +4,8 @@ import { laneStatuses, isCodePath, isBotPr, - postedSince, + latestByContext, + statusesToPost, verifiedAtHead, secondReadAtHead, CONTEXTS, @@ -122,8 +123,12 @@ console.log('\n exempt PRs'); check('docs: review still waits on Redline', docs[CONTEXTS.review].state === 'pending'); const bot = by(laneStatuses(base({ headRef: 'bot/cc-drift-v2.1.281', reviews: [review(REDLINE_LOGIN, 'APPROVED', HEAD)] }))); check('bot branch: verify not required, approval counts', bot[CONTEXTS.verify].state === 'success' && bot[CONTEXTS.review].state === 'success'); - const many = Array.from({ length: 100 }, (_, i) => `docs/p${i}.md`); - check('100 files is code whatever they are', by(laneStatuses(base({ files: many })))[CONTEXTS.verify].state === 'pending'); + const many = Array.from({ length: 101 }, (_, i) => `docs/p${i}.md`); + check('more than 100 files is code whatever they are', by(laneStatuses(base({ files: many })))[CONTEXTS.verify].state === 'pending'); + const hundred = Array.from({ length: 100 }, (_, i) => `docs/p${i}.md`); + check('exactly 100 docs files is not code', by(laneStatuses(base({ files: hundred })))[CONTEXTS.verify].state === 'success'); + check('more than 100 docs files still verifies once required CI passes', + by(laneStatuses(base({ files: many, requiredCi: 'passed' })))[CONTEXTS.verify].state === 'success'); } console.log('\n descriptions'); @@ -250,17 +255,6 @@ console.log('\n required CI is the verification where the base branch requires check('dependabot on any branch is a bot PR', isBotPr('dependabot[bot]', 'feature/x')); } -{ - // A newer run's status wins; an older run skips a context posted after its read. - const readAt = Date.parse('2026-09-25T03:00:10Z'); - const st = (context, at) => ({ context, created_at: at }); - check('posted after our read: skip', postedSince([st('fleet/review', '2026-09-25T03:00:11Z')], 'fleet/review', readAt)); - check('posted before our read: overwrite', !postedSince([st('fleet/review', '2026-09-25T03:00:09Z')], 'fleet/review', readAt)); - check('posted in the same second: overwrite', !postedSince([st('fleet/review', '2026-09-25T03:00:10Z')], 'fleet/review', readAt)); - check('another context does not count', !postedSince([st('fleet/verify', '2026-09-25T03:00:30Z')], 'fleet/review', readAt)); - check('no server date: never skip', !postedSince([st('fleet/review', '2026-09-25T03:00:30Z')], 'fleet/review', NaN)); -} - { // The fleet/* lanes as required checks (the step after rollout) must not hold themselves. const ci = ['test', 'analyze']; @@ -287,5 +281,41 @@ console.log('\n required CI is the verification where the base branch requires check('own lanes required, three rounds: all three green', states.join() === 'success,success,success'); } +{ + // Post-then-verify ordering: what differs from the head's newest status per context is posted. + const have = latestByContext([ + { context: 'fleet/review', state: 'success', description: 'Redline approved abc1234' }, + { context: 'fleet/review', state: 'pending', description: 'older' }, + { context: 'fleet/verify', state: 'success', description: 'Required CI passed at abc1234' }, + ]); + check('latestByContext keeps the newest per context', have.get('fleet/review').state === 'success' && have.size === 2); + const want = [ + { context: 'fleet/verify', state: 'success', description: 'Required CI passed at abc1234' }, + { context: 'fleet/review', state: 'pending', description: 'Waiting on Redline at abc1234' }, + { context: 'fleet/second-read', state: 'pending', description: 'Waiting on the Second Read at abc1234' }, + ]; + const todo = statusesToPost(want, have).map((s) => s.context); + check('an identical status is not posted again', !todo.includes('fleet/verify')); + check('a differing state is posted', todo.includes('fleet/review')); + check('a missing context is posted', todo.includes('fleet/second-read')); + check('same state, new description is posted', + statusesToPost([{ context: 'fleet/verify', state: 'success', description: 'Verified at abc1234' }], have).length === 1); + // A stale run posted after a fresh one: the fresh run's verify pass sees the difference and corrects it. + const stale = latestByContext([{ context: 'fleet/review', state: 'pending', description: 'Waiting on Redline at abc1234' }]); + const fresh = [{ context: 'fleet/review', state: 'success', description: 'Redline approved abc1234' }]; + check('a stale overwrite is corrected on the next pass', statusesToPost(fresh, stale).length === 1); + check('and then left alone', statusesToPost(fresh, latestByContext(fresh)).length === 0); +} + +{ + // A dismissed Second Read review is not a verdict. + const verified = { requiredCi: 'passed' }; + const dismissed = review(SECOND_READ_LOGIN, 'DISMISSED', HEAD, 'SECOND READ: NOT READY - old finding'); + check('dismissed NOT READY at head: waiting, not red', + by(laneStatuses(base({ ...verified, reviews: [dismissed] })))[CONTEXTS.secondRead].state === 'pending'); + check('a later READY still counts after a dismissed NOT READY', + by(laneStatuses(base({ ...verified, reviews: [dismissed, review(SECOND_READ_LOGIN, 'COMMENTED', HEAD, 'SECOND READ: READY')] })))[CONTEXTS.secondRead].state === 'success'); +} + console.log(`\n ${pass} pass, ${fail} fail`); if (fail > 0) process.exit(1); From adb8b0b9488cdd6dfc164586d2159c4fcad69ae0 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Thu, 24 Sep 2026 23:18:43 -0400 Subject: [PATCH 07/12] test(fleet-status): name cases by behavior, not by their history Three test lines described where a case came from instead of what it checks: a section title naming an old PR, and two comments referring to a review and to the planned rollout step. They now describe the behavior only. The workflow comment on ordering matches the post-then-verify loop. No logic change (94/94). --- .github/workflows/fleet-status.yml | 2 +- scripts/fleet-status.test.mjs | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/workflows/fleet-status.yml b/.github/workflows/fleet-status.yml index 6703d73..0a17d32 100644 --- a/.github/workflows/fleet-status.yml +++ b/.github/workflows/fleet-status.yml @@ -3,7 +3,7 @@ # self-test runs the script's tests on the PR's own code, read-only. # backfill (manual) posts them on every open PR, e.g. once they become required. # No concurrency group: a cancelled run rolls up as a failed check on the PR. The script -# orders runs itself, so an older run never overwrites a newer one's statuses. +# re-reads after posting and corrects what differs, so the last run to act leaves current statuses. name: Fleet status on: diff --git a/scripts/fleet-status.test.mjs b/scripts/fleet-status.test.mjs index 3a87552..cd1c10b 100644 --- a/scripts/fleet-status.test.mjs +++ b/scripts/fleet-status.test.mjs @@ -61,7 +61,7 @@ console.log('\n code PR, not verified: everything waits on the Breaker'); check('second read pending', s[CONTEXTS.secondRead].state === 'pending'); } -console.log('\n the dario#1403 morning: verdicts on an older head'); +console.log('\n verdicts on an older head'); { const s = by(laneStatuses(base({ labels: ['verified'], comments: [verification(HEAD)], @@ -256,7 +256,7 @@ console.log('\n required CI is the verification where the base branch requires } { - // The fleet/* lanes as required checks (the step after rollout) must not hold themselves. + // With the fleet/* lanes listed as required checks, they must not wait on themselves. const ci = ['test', 'analyze']; const own = [CONTEXTS.verify, CONTEXTS.review, CONTEXTS.secondRead]; const green = ci.map((name) => ({ name, state: 'SUCCESS' })); @@ -266,7 +266,6 @@ console.log('\n required CI is the verification where the base branch requires check('only own lanes required: none (the Breaker rule applies)', requiredCiState(own, []) === 'none'); check('own lanes required, a real check running: pending', requiredCiState([...ci, ...own], [{ name: 'test', state: 'SUCCESS' }, { name: 'analyze', state: 'IN_PROGRESS' }]) === 'pending'); - // Feed each run's statuses back in as the next run's checks, three rounds, as the Second Read did. let posted = []; let states = []; for (let round = 0; round < 3; round++) { From 7e6473ca83f44bd3555f5b9cd096e8a28bb78107 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Thu, 24 Sep 2026 23:57:34 -0400 Subject: [PATCH 08/12] ci(fleet-status): ASCII fixtures, separator-only reason strip, fail-closed rules - The two NOT READY test fixtures use an ASCII hyphen instead of a dash escape. - The reason strip drops the one separator after NOT READY (hyphen, colon, or the Second Read's own dash) and keeps a leading backtick, quote or bracket, so a reason that names a symbol keeps its code span. Four tests. - Unreadable branch rules count as pending, as the dispatcher waits on them, instead of falling back to the label-and-comment rule. - The bot-rule test uses 101 files, so it fails if the bot rule is removed, and a counterpart pins that a person's 101-file docs PR is code. 99/99. --- scripts/fleet-status.mjs | 15 ++++++++------- scripts/fleet-status.test.mjs | 19 +++++++++++++++---- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/scripts/fleet-status.mjs b/scripts/fleet-status.mjs index 8b338ee..aab0a0e 100644 --- a/scripts/fleet-status.mjs +++ b/scripts/fleet-status.mjs @@ -114,7 +114,8 @@ export function secondReadAtHead(facts) { for (const m of (r.body ?? '').matchAll(/^SECOND READ: (READY[ \t\r]*$|NOT READY\b.*)$/gm)) last = m[1]; if (last === null) continue; out = last.startsWith('NOT READY') - ? { state: 'NOT READY', reason: last.replace(/^NOT READY\W*/, '').trim() } + // Drop the one separator after NOT READY; keep a leading backtick, quote or bracket. + ? { state: 'NOT READY', reason: last.replace(/^NOT READY\s*(?:[^\w\s`'"([{]\s*)?/, '').trim() } : { state: 'READY', reason: '' }; } return out; @@ -230,16 +231,16 @@ if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) ghAll(`/repos/${repo}/pulls/${pr}/reviews`, token), ghAll(`/repos/${repo}/issues/${pr}/comments`, token), ]); - // Unreadable rules count as none (the label-and-comment rule applies); unreadable checks as - // pending. Neither can turn fleet/verify green. - let required = []; + // Unreadable rules or checks count as pending, as the dispatcher waits on them; neither can + // turn fleet/verify green. + let required = null; try { const rules = await (await gh(`/repos/${repo}/rules/branches/${encodeURIComponent(p.base.ref)}?per_page=100`, token)).json(); required = rules.filter((r) => r.type === 'required_status_checks') .flatMap((r) => (r.parameters?.required_status_checks ?? []).map((c) => c.context)); - } catch { required = []; } - let requiredCi = 'none'; - if (required.length) { + } catch { required = null; } + let requiredCi = required === null ? 'pending' : 'none'; + if (required?.length) { try { const statuses = (await ghAll(`/repos/${repo}/commits/${p.head.sha}/statuses`, token)).reverse() .map((s) => ({ name: s.context, state: s.state })); diff --git a/scripts/fleet-status.test.mjs b/scripts/fleet-status.test.mjs index cd1c10b..d776f96 100644 --- a/scripts/fleet-status.test.mjs +++ b/scripts/fleet-status.test.mjs @@ -67,7 +67,7 @@ console.log('\n verdicts on an older head'); labels: ['verified'], comments: [verification(HEAD)], reviews: [ review(REDLINE_LOGIN, 'CHANGES_REQUESTED', OLD), - review(SECOND_READ_LOGIN, 'COMMENTED', OLD, 'text\nSECOND READ: NOT READY \u2014 stale stack'), + review(SECOND_READ_LOGIN, 'COMMENTED', OLD, 'text\nSECOND READ: NOT READY - stale stack'), ], }))); check('verify green', s[CONTEXTS.verify].state === 'success'); @@ -93,11 +93,11 @@ console.log('\n verdicts at the head'); labels: ['verified'], comments: [verification(HEAD)], reviews: [ review(REDLINE_LOGIN, 'CHANGES_REQUESTED', HEAD), - review(SECOND_READ_LOGIN, 'COMMENTED', HEAD, 'SECOND READ: NOT READY \u2014 commit subject has an em dash'), + review(SECOND_READ_LOGIN, 'COMMENTED', HEAD, 'SECOND READ: NOT READY - commit subject is too long'), ], }))); check('Redline changes requested is red', s[CONTEXTS.review].state === 'failure'); - check('NOT READY is red with its reason', s[CONTEXTS.secondRead].state === 'failure' && s[CONTEXTS.secondRead].description.endsWith('commit subject has an em dash')); + check('NOT READY is red with its reason', s[CONTEXTS.secondRead].state === 'failure' && s[CONTEXTS.secondRead].description.endsWith('commit subject is too long')); } check('a Second Read without a verdict line is not READY', secondReadAtHead(base({ reviews: [review(SECOND_READ_LOGIN, 'COMMENTED', HEAD, 'no verdict here')] })).state === 'none'); @@ -194,7 +194,9 @@ check('askalf on a release, receipts or dependabot branch is', isBotPr('askalf', 'release-v6.12.0') && isBotPr('askalf', 'release/6.12') && isBotPr('askalf', 'chore/release-v6.12.0') && isBotPr('askalf', 'receipts-2026-09-24') && isBotPr('askalf', 'dependabot/npm/x')); { const docs = (n) => Array.from({ length: n }, (_, i) => `docs/p${i}.md`); - check('a bot PR with 100 files is not code', by(laneStatuses(base({ headRef: 'bot/cc-drift-v2.1.281', files: docs(100) })))[CONTEXTS.verify].state === 'success'); + // The bot rule is read before the file count: a bot PR over 100 files is still exempt, a person's is code. + check('a bot PR with more than 100 files is still not code', by(laneStatuses(base({ headRef: 'bot/cc-drift-v2.1.281', files: docs(101) })))[CONTEXTS.verify].state === 'success'); + check('a person with more than 100 docs files is code', by(laneStatuses(base({ files: docs(101) })))[CONTEXTS.verify].state === 'pending'); check('99 docs files are not code', by(laneStatuses(base({ files: docs(99) })))[CONTEXTS.verify].state === 'success'); } @@ -316,5 +318,14 @@ console.log('\n required CI is the verification where the base branch requires by(laneStatuses(base({ ...verified, reviews: [dismissed, review(SECOND_READ_LOGIN, 'COMMENTED', HEAD, 'SECOND READ: READY')] })))[CONTEXTS.secondRead].state === 'success'); } +{ + // The reason keeps its first character; only the separator after NOT READY goes. + const reasonOf = (body) => secondReadAtHead(base({ reviews: [review(SECOND_READ_LOGIN, 'COMMENTED', HEAD, body)] })).reason; + check('a reason that starts with a code span keeps it', reasonOf('SECOND READ: NOT READY - `x` is null') === '`x` is null'); + check('a hyphen separator is dropped', reasonOf('SECOND READ: NOT READY - stale stack') === 'stale stack'); + check('a colon separator is dropped', reasonOf('SECOND READ: NOT READY: (a) and (b)') === '(a) and (b)'); + check('no separator: the reason is kept whole', reasonOf('SECOND READ: NOT READY [scope] missing') === '[scope] missing'); +} + console.log(`\n ${pass} pass, ${fail} fail`); if (fail > 0) process.exit(1); From 4726036b85c59d9efb9cdc6e1d33ca757b9e7663 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Fri, 25 Sep 2026 00:00:18 -0400 Subject: [PATCH 09/12] ci(fleet-status): refresh after every pull-request workflow, and test that it does The workflow_run list named only the workflows behind today's required checks, so a required check added later from another workflow would leave fleet/verify pending until the next PR event. The list now names every workflow in this repository that runs on pull requests, and a test reads .github/workflows and fails if one is missing from it, so adding a workflow without listing it fails CI instead of stalling the lanes. --- .github/workflows/fleet-status.yml | 2 +- scripts/fleet-status.test.mjs | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/.github/workflows/fleet-status.yml b/.github/workflows/fleet-status.yml index 0a17d32..4fe11a6 100644 --- a/.github/workflows/fleet-status.yml +++ b/.github/workflows/fleet-status.yml @@ -15,7 +15,7 @@ on: issue_comment: types: [created, edited, deleted] workflow_run: - workflows: [CI, CodeQL] + workflows: [CI, CodeQL, Image, labels, 'PR triage'] types: [completed] permissions: {} diff --git a/scripts/fleet-status.test.mjs b/scripts/fleet-status.test.mjs index d776f96..1347ea4 100644 --- a/scripts/fleet-status.test.mjs +++ b/scripts/fleet-status.test.mjs @@ -14,6 +14,9 @@ import { SECOND_READ_LOGIN, VERIFIER_LOGIN, } from './fleet-status.mjs'; +import { readdirSync, readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; let pass = 0; let fail = 0; @@ -327,5 +330,30 @@ console.log('\n required CI is the verification where the base branch requires check('no separator: the reason is kept whole', reasonOf('SECOND READ: NOT READY [scope] missing') === '[scope] missing'); } +{ + // Every workflow that runs on pull requests is in fleet-status.yml's workflow_run list, so a + // required check it produces refreshes the lanes when it finishes. + const dir = join(fileURLToPath(new URL('..', import.meta.url)), '.github', 'workflows'); + const own = readFileSync(join(dir, 'fleet-status.yml'), 'utf8'); + const listed = (/^ workflow_run:\s*\n\s+workflows:\s*\[([^\]]*)\]/m.exec(own)?.[1] ?? '') + .split(',').map((w) => w.trim().replace(/^['"]|['"]$/g, '')).filter(Boolean); + const onBlock = (y) => { + const m = /^on:(.*)$/m.exec(y); + if (!m) return ''; + const lines = [m[1]]; + for (const l of y.slice(m.index + m[0].length).split('\n').slice(1)) { + if (/^[^\s#]/.test(l)) break; + lines.push(l); + } + return lines.join('\n').replace(/#.*$/gm, ''); + }; + for (const f of readdirSync(dir).filter((x) => /\.ya?ml$/.test(x) && x !== 'fleet-status.yml')) { + const y = readFileSync(join(dir, f), 'utf8'); + if (!/\bpull_request(_target)?\b/.test(onBlock(y))) continue; + const name = (/^name:\s*(.+)$/m.exec(y)?.[1] ?? f).trim().replace(/^['"]|['"]$/g, ''); + check(`workflow_run lists "${name}" (${f} runs on pull requests)`, listed.includes(name)); + } +} + console.log(`\n ${pass} pass, ${fail} fail`); if (fail > 0) process.exit(1); From bc8270029cf0a26222e907111be8d70540771edc Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Fri, 25 Sep 2026 00:09:38 -0400 Subject: [PATCH 10/12] ci(fleet-status): list pull_request workflows only; pull_request_target cannot move a required check A pull_request_target workflow runs against the base branch's latest commit, so its checks land on that commit and never on the PR head, and the status job drops its workflow_run events anyway. Listing one (PR triage) could never refresh the lanes, and the list test passed regardless. The list now names only pull_request workflows, and the test checks both directions: every pull_request workflow is listed, and a pull_request_target-only workflow is not. --- .github/workflows/fleet-status.yml | 2 +- scripts/fleet-status.test.mjs | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/.github/workflows/fleet-status.yml b/.github/workflows/fleet-status.yml index 4fe11a6..347102d 100644 --- a/.github/workflows/fleet-status.yml +++ b/.github/workflows/fleet-status.yml @@ -15,7 +15,7 @@ on: issue_comment: types: [created, edited, deleted] workflow_run: - workflows: [CI, CodeQL, Image, labels, 'PR triage'] + workflows: [CI, CodeQL, Image, labels] types: [completed] permissions: {} diff --git a/scripts/fleet-status.test.mjs b/scripts/fleet-status.test.mjs index 1347ea4..ea58258 100644 --- a/scripts/fleet-status.test.mjs +++ b/scripts/fleet-status.test.mjs @@ -331,8 +331,10 @@ console.log('\n required CI is the verification where the base branch requires } { - // Every workflow that runs on pull requests is in fleet-status.yml's workflow_run list, so a - // required check it produces refreshes the lanes when it finishes. + // fleet-status.yml's workflow_run list names every workflow that runs on pull_request, so a + // required check it produces refreshes the lanes when it finishes. A workflow that runs only on + // pull_request_target is left out: it runs against the base branch's commit, so its checks + // never land on the PR head, and the status job drops its workflow_run events. const dir = join(fileURLToPath(new URL('..', import.meta.url)), '.github', 'workflows'); const own = readFileSync(join(dir, 'fleet-status.yml'), 'utf8'); const listed = (/^ workflow_run:\s*\n\s+workflows:\s*\[([^\]]*)\]/m.exec(own)?.[1] ?? '') @@ -349,9 +351,13 @@ console.log('\n required CI is the verification where the base branch requires }; for (const f of readdirSync(dir).filter((x) => /\.ya?ml$/.test(x) && x !== 'fleet-status.yml')) { const y = readFileSync(join(dir, f), 'utf8'); - if (!/\bpull_request(_target)?\b/.test(onBlock(y))) continue; + const on = onBlock(y); const name = (/^name:\s*(.+)$/m.exec(y)?.[1] ?? f).trim().replace(/^['"]|['"]$/g, ''); - check(`workflow_run lists "${name}" (${f} runs on pull requests)`, listed.includes(name)); + if (/\bpull_request\b(?!_target)/.test(on)) { + check(`workflow_run lists "${name}" (${f} runs on pull_request)`, listed.includes(name)); + } else if (/\bpull_request_target\b/.test(on)) { + check(`workflow_run leaves out "${name}" (${f} runs only on pull_request_target)`, !listed.includes(name)); + } } } From e5c423da0f769fa731454e4ddc96845bab5cc7ff Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Fri, 25 Sep 2026 00:25:43 -0400 Subject: [PATCH 11/12] ci(fleet-status): backfill pages every open PR; pin the Second Read's own separator - backfill read one page of 100 open PRs, so a larger repository would leave some PRs without lanes. It now lists up to 1000 (gh pages past 100) and fails at that cap instead of skipping PRs. Two tests pin the limit and the guard. - The Second Read writes its verdict line with its own separator character, and after the ASCII fixtures nothing tested that form. The tests now build every separator form at run time with String.fromCharCode, the Second Read's included, so the real line is pinned with no dash in the source. --- .github/workflows/fleet-status.yml | 9 +++++++-- scripts/fleet-status.test.mjs | 21 +++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/.github/workflows/fleet-status.yml b/.github/workflows/fleet-status.yml index 347102d..a377119 100644 --- a/.github/workflows/fleet-status.yml +++ b/.github/workflows/fleet-status.yml @@ -110,8 +110,13 @@ jobs: TARGET_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} run: | set -euo pipefail - prs="$(gh pr list --repo "$REPO" --state open --limit 100 --json number,isCrossRepository \ - --jq '.[] | select(.isCrossRepository | not) | .number')" + # gh pages past 100 on its own; at the cap, fail rather than skip PRs silently. + all="$(gh pr list --repo "$REPO" --state open --limit 1000 --json number,isCrossRepository)" + if [ "$(printf '%s' "$all" | jq 'length')" -ge 1000 ]; then + echo "::error::1000 or more open PRs; raise the backfill limit" + exit 1 + fi + prs="$(printf '%s' "$all" | jq -r '.[] | select(.isCrossRepository | not) | .number')" for pr in $prs; do echo "== #$pr" PR="$pr" node scripts/fleet-status.mjs diff --git a/scripts/fleet-status.test.mjs b/scripts/fleet-status.test.mjs index ea58258..1358984 100644 --- a/scripts/fleet-status.test.mjs +++ b/scripts/fleet-status.test.mjs @@ -361,5 +361,26 @@ console.log('\n required CI is the verification where the base branch requires } } +{ + // The Second Read's own separator (U+2014) is built at run time, as are the other forms. + const reasonOf = (body) => secondReadAtHead(base({ reviews: [review(SECOND_READ_LOGIN, 'COMMENTED', HEAD, body)] })).reason; + for (const code of [0x2d, 0x3a, 0x2013, 0x2014]) { + const hex = code.toString(16).toUpperCase().padStart(4, '0'); + check(`separator U+${hex} is dropped`, reasonOf(`SECOND READ: NOT READY ${String.fromCharCode(code)} stale stack`) === 'stale stack'); + } + const line = `SECOND READ: NOT READY ${String.fromCharCode(0x2014)} \`x\` is null`; + const s = by(laneStatuses(base({ requiredCi: 'passed', reviews: [review(SECOND_READ_LOGIN, 'COMMENTED', HEAD, line)] }))); + check('the Second Read verdict line is red with its reason', s[CONTEXTS.secondRead].state === 'failure' + && s[CONTEXTS.secondRead].description.endsWith('`x` is null')); +} + +{ + // backfill pages past one page of open PRs and stops loudly at its cap. + const wf = readFileSync(join(fileURLToPath(new URL('..', import.meta.url)), '.github', 'workflows', 'fleet-status.yml'), 'utf8'); + const limit = Number(/gh pr list --repo "\$REPO" --state open --limit (\d+)/.exec(wf)?.[1] ?? 0); + check('backfill reads more than one page of open PRs', limit > 100); + check('backfill fails at its cap instead of skipping PRs', new RegExp(`-ge ${limit}\\b`).test(wf) && /::error::/.test(wf)); +} + console.log(`\n ${pass} pass, ${fail} fail`); if (fail > 0) process.exit(1); From 6ff4cf15d615c792ba81b91f1454e52f1c431dd7 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Fri, 25 Sep 2026 00:33:39 -0400 Subject: [PATCH 12/12] ci(fleet-status): read check runs across every page The check-runs list was read once with per_page=100, so on a head with more than 100 runs a required check on a later page never counted, and fleet/verify could wait on it indefinitely. Paging is now one pure helper, collectPages(), used by the list endpoints and by check runs alike. Four tests: 101 rows over two pages, a full last page, a short first page, and a required check found on page 2. --- scripts/fleet-status.mjs | 25 +++++++++++++++++-------- scripts/fleet-status.test.mjs | 11 +++++++++++ 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/scripts/fleet-status.mjs b/scripts/fleet-status.mjs index aab0a0e..bc82e67 100644 --- a/scripts/fleet-status.mjs +++ b/scripts/fleet-status.mjs @@ -138,6 +138,19 @@ export function statusesToPost(want, have) { }); } +/** + * Every row across pages. `page(n)` returns page n's rows; a page shorter than `size` is the last. + * @param {(n: number) => Promise} page + */ +export async function collectPages(page, size = 100) { + const out = []; + for (let n = 1; ; n++) { + const rows = await page(n); + out.push(...rows); + if (rows.length < size) return out; + } +} + const short = (sha) => (sha ?? '').slice(0, 7); const fit = (s) => (s.length <= 140 ? s : `${s.slice(0, 137)}...`); @@ -204,13 +217,8 @@ async function gh(path, token, init = {}) { return res; } -async function ghAll(path, token) { - const out = []; - for (let page = 1; ; page++) { - const rows = await (await gh(`${path}${path.includes('?') ? '&' : '?'}per_page=100&page=${page}`, token)).json(); - out.push(...rows); - if (rows.length < 100) return out; - } +function ghAll(path, token) { + return collectPages(async (n) => (await gh(`${path}${path.includes('?') ? '&' : '?'}per_page=100&page=${n}`, token)).json()); } if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { @@ -244,7 +252,8 @@ if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) try { const statuses = (await ghAll(`/repos/${repo}/commits/${p.head.sha}/statuses`, token)).reverse() .map((s) => ({ name: s.context, state: s.state })); - const runs = (await (await gh(`/repos/${repo}/commits/${p.head.sha}/check-runs?per_page=100`, token)).json()).check_runs ?? []; + const runs = await collectPages(async (n) => + (await (await gh(`/repos/${repo}/commits/${p.head.sha}/check-runs?per_page=100&page=${n}`, token)).json()).check_runs ?? []); const checks = runs.sort((a, b) => a.id - b.id) .map((c) => ({ name: c.name, state: c.status === 'completed' ? (c.conclusion ?? '') : c.status })); requiredCi = requiredCiState(required, [...statuses, ...checks]); diff --git a/scripts/fleet-status.test.mjs b/scripts/fleet-status.test.mjs index 1358984..d7734af 100644 --- a/scripts/fleet-status.test.mjs +++ b/scripts/fleet-status.test.mjs @@ -13,6 +13,7 @@ import { REDLINE_LOGIN, SECOND_READ_LOGIN, VERIFIER_LOGIN, + collectPages, } from './fleet-status.mjs'; import { readdirSync, readFileSync } from 'node:fs'; import { join } from 'node:path'; @@ -382,5 +383,15 @@ console.log('\n required CI is the verification where the base branch requires check('backfill fails at its cap instead of skipping PRs', new RegExp(`-ge ${limit}\\b`).test(wf) && /::error::/.test(wf)); } +{ + // Paging reads to the first short page, so a required check on page 2 still counts. + const pager = (sizes) => async (n) => Array.from({ length: sizes[n - 1] ?? 0 }, (_, i) => ({ name: `check-${n}-${i}`, state: 'SUCCESS' })); + const two = await collectPages(pager([100, 1])); + check('101 rows over two pages are all read', two.length === 101); + check('a full last page reads one more, empty, page', (await collectPages(pager([100, 100]))).length === 200); + check('a short first page is the only page', (await collectPages(pager([5]))).length === 5); + check('a required check on page 2 counts', requiredCiState(['check-2-0'], two) === 'passed'); +} + console.log(`\n ${pass} pass, ${fail} fail`); if (fail > 0) process.exit(1);