From 8360d5bf1efbe217ff94caea7e202fb9dbaab125 Mon Sep 17 00:00:00 2001 From: Rix Date: Mon, 28 Sep 2026 10:44:19 -0700 Subject: [PATCH] RFC workflow: shared head commits, labeled-run concurrency, escaped titles (from pcsx-redux/nugget) --- .github/scripts/rfc.cjs | 74 +++++++++++++++++++++++------------- .github/scripts/rfc.test.cjs | 28 ++++++++++++++ .github/workflows/rfc.yml | 4 +- 3 files changed, 79 insertions(+), 27 deletions(-) diff --git a/.github/scripts/rfc.cjs b/.github/scripts/rfc.cjs index 83b6162..82bb374 100644 --- a/.github/scripts/rfc.cjs +++ b/.github/scripts/rfc.cjs @@ -70,48 +70,57 @@ function indexBody(owner, repo, rows) { if (rows.length === 0) return head + 'No open RFCs.\n'; rows.sort((a, b) => a.notBefore - b.notBefore); return head + '| Pull request | Merge not before |\n|---|---|\n' + - rows.map((r) => `| #${r.number} ${r.title.replace(/\|/g, '\\|')} | ${r.notBefore ? stamp(r.notBefore) : 'unknown'} |`).join('\n') + '\n'; + rows.map((r) => `| #${r.number} ${r.title.replace(/\|/g, '\\|').replace(/@/g, '@')} | ${r.notBefore ? stamp(r.notBefore) : 'unknown'} |`).join('\n') + '\n'; } -async function run({ github, context, core, now = Date.now(), stakeholdersText }) { - const { owner, repo } = context.repo; - const target = `https://github.com/${owner}/${repo}/blob/main/RFC.md`; - const prs = await github.paginate(github.rest.pulls.list, { owner, repo, state: 'open', per_page: 100 }); - const rows = []; +// Pull requests can share a head commit, and a commit has one status per +// context, so a failing verdict for any of them wins. +function worst(a, b) { + return !a || (b.state === 'failure' && a.state !== 'failure') ? b : a; +} +async function evaluate(github, owner, repo, prs, now) { + const rows = []; + const bySha = new Map(); for (const pr of prs) { const labels = pr.labels.map((l) => l.name); - const events = labels.includes(LABEL) + const isRfc = labels.includes(LABEL); + const events = isRfc ? await github.paginate(github.rest.issues.listEvents, { owner, repo, issue_number: pr.number, per_page: 100 }) : []; const v = verdict(labels, events, now); - if (labels.includes(LABEL)) rows.push({ number: pr.number, title: pr.title, notBefore: v.notBefore }); + if (isRfc) rows.push({ number: pr.number, title: pr.title, notBefore: v.notBefore }); + bySha.set(pr.head.sha, worst(bySha.get(pr.head.sha), v)); + } + return { rows, bySha }; +} - const { data: current } = await github.rest.repos.listCommitStatusesForRef({ - owner, repo, ref: pr.head.sha, per_page: 100, - }); +async function publishStatuses(github, core, owner, repo, bySha, target) { + for (const [sha, v] of bySha) { + const { data: current } = await github.rest.repos.listCommitStatusesForRef({ owner, repo, ref: sha, per_page: 100 }); const last = current.find((s) => s.context === CONTEXT); if (last && last.state === v.state && last.description === v.description) continue; await github.rest.repos.createCommitStatus({ - owner, repo, sha: pr.head.sha, state: v.state, context: CONTEXT, description: v.description, target_url: target, + owner, repo, sha, state: v.state, context: CONTEXT, description: v.description, target_url: target, }); - core.info(`#${pr.number}: ${v.state}, ${v.description}`); + core.info(`${sha.slice(0, 7)}: ${v.state}, ${v.description}`); } +} - const payload = context.payload; - if (context.eventName === 'pull_request_target' && payload.action === 'labeled' && payload.label.name === LABEL) { - const pr = payload.pull_request; - const files = (await github.paginate(github.rest.pulls.listFiles, { owner, repo, pull_number: pr.number, per_page: 100 })) - .map((f) => f.filename); - const text = stakeholdersText !== undefined ? stakeholdersText : readStakeholders(); - const who = stakeholders(parseStakeholders(text), files, pr.user.login); - const row = rows.find((r) => r.number === pr.number); - const body = `This is now an RFC: it cannot merge before ${row && row.notBefore ? stamp(row.notBefore) : 'the window closes'}, ` + - 'so anyone who depends on what it changes has a week to comment. See [RFC.md](' + target + ').' + - (who.length ? '\n\n' + who.map((h) => '@' + h).join(' ') + ', this touches code you depend on.' : ''); - await github.rest.issues.createComment({ owner, repo, issue_number: pr.number, body }); - } +async function announce(github, owner, repo, pr, rows, target, stakeholdersText) { + const files = (await github.paginate(github.rest.pulls.listFiles, { owner, repo, pull_number: pr.number, per_page: 100 })) + .map((f) => f.filename); + const text = stakeholdersText !== undefined ? stakeholdersText : readStakeholders(); + const who = stakeholders(parseStakeholders(text), files, pr.user.login); + const row = rows.find((r) => r.number === pr.number); + const when = row && row.notBefore ? stamp(row.notBefore) : 'the window closes'; + const body = `This is now an RFC: it cannot merge before ${when}, ` + + 'so anyone who depends on what it changes has a week to comment. See [RFC.md](' + target + ').' + + (who.length ? '\n\n' + who.map((h) => '@' + h).join(' ') + ', this touches code you depend on.' : ''); + await github.rest.issues.createComment({ owner, repo, issue_number: pr.number, body }); +} +async function updateIndex(github, core, owner, repo, rows) { const issues = await github.paginate(github.rest.issues.listForRepo, { owner, repo, state: 'open', per_page: 100 }); const index = issues.find((i) => !i.pull_request && i.title === INDEX_TITLE); const body = indexBody(owner, repo, rows); @@ -122,4 +131,17 @@ async function run({ github, context, core, now = Date.now(), stakeholdersText } } } +async function run({ github, context, core, now = Date.now(), stakeholdersText }) { + const { owner, repo } = context.repo; + const target = `https://github.com/${owner}/${repo}/blob/main/RFC.md`; + const prs = await github.paginate(github.rest.pulls.list, { owner, repo, state: 'open', per_page: 100 }); + const { rows, bySha } = await evaluate(github, owner, repo, prs, now); + await publishStatuses(github, core, owner, repo, bySha, target); + const payload = context.payload; + if (context.eventName === 'pull_request_target' && payload.action === 'labeled' && payload.label.name === LABEL) { + await announce(github, owner, repo, payload.pull_request, rows, target, stakeholdersText); + } + await updateIndex(github, core, owner, repo, rows); +} + module.exports = { run, verdict, labeledAt, parseStakeholders, readStakeholders, stakeholders, indexBody, WINDOW_MS }; diff --git a/.github/scripts/rfc.test.cjs b/.github/scripts/rfc.test.cjs index f3dbc65..9b406af 100644 --- a/.github/scripts/rfc.test.cjs +++ b/.github/scripts/rfc.test.cjs @@ -102,3 +102,31 @@ test('run sets statuses, comments on labeling, rewrites the index', async () => assert.strictEqual(update.issue_number, 47); assert.match(update.body, /\| #37 monitor: protocol v3 \| 2026-09-08 12:00 UTC \|/); }); + +test('index escapes mentions in titles', () => { + const body = rfc.indexBody('o', 'r', [{ number: 1, title: 'ping @someone | x', notBefore: 0 }]); + assert.ok(!body.includes('@someone')); + assert.match(body, /@someone \\\| x/); +}); + +test('a shared head commit gets the failing verdict', async () => { + const statuses = []; + const prs = [ + { number: 1, title: 'a', labels: [{ name: 'rfc' }], head: { sha: 'same' }, user: { login: 'x' } }, + { number: 2, title: 'b', labels: [], head: { sha: 'same' }, user: { login: 'x' } }, + ]; + const github = { + paginate: async (fn, args) => fn(args), + rest: { + pulls: { list: () => prs }, + issues: { listEvents: () => [on(t0)], listForRepo: () => [] }, + repos: { + listCommitStatusesForRef: async () => ({ data: [] }), + createCommitStatus: async (a) => statuses.push([a.sha, a.state]), + }, + }, + }; + const context = { repo: { owner: 'o', repo: 'r' }, eventName: 'schedule', payload: {} }; + await rfc.run({ github, context, core: { info() {}, warning() {} }, now: t0 + DAY }); + assert.deepStrictEqual(statuses, [['same', 'failure']]); +}); diff --git a/.github/workflows/rfc.yml b/.github/workflows/rfc.yml index 6937042..7a7fe13 100644 --- a/.github/workflows/rfc.yml +++ b/.github/workflows/rfc.yml @@ -16,8 +16,10 @@ permissions: pull-requests: write statuses: write +# A labeled run posts the announcement, so it gets its own group: a pending +# run in a shared group is replaced by the next one and would be lost. concurrency: - group: rfc + group: ${{ github.event.action == 'labeled' && format('rfc-labeled-{0}', github.run_id) || 'rfc' }} cancel-in-progress: false jobs: