Audit that every merge on main went through the merge queue

.github/scripts/audit-merges.mjs

@@ -0,0 +1,125 @@
+// Audit that every merge on main went through the merge queue (#689).
+//
+//   GH_REPO=owner/repo GH_TOKEN=... node audit-merges.mjs [--limit 50] [--repo owner/repo]
+//
+// The merge gate judges a queue entry, and only the queue can turn that entry into a commit on
+// `main`. That second half was a convention, and it has been broken: `#650` (`6438cb2b`) and `#680`
+// (`6b9820bb`) are build commits whose `Merge gate` run failed, and `main` was nevertheless pushed
+// to them (both carry `merged_by: Birfy`). A real queue merge — `#672` (`93e52347`), `#674`
+// (`9393d558`) — leaves a **successful** `Merge gate` run whose `head_sha` is the commit that landed.
+//
+// So the difference is machine-checkable. This script walks the last `--limit` commits on `main`,
+// asks which pull request each came from, and asks for the `merge_group` runs on that commit:
+//   - a commit with a successful `Merge gate` run on it went through the queue: no output;
+//   - a commit whose only `Merge gate` run on it failed (or which has none) prints
+//     `bypassed queue: #<pr> <commit> (merge_group: failure|absent)` and the run exits 1;
+//   - a commit that came from no pull request prints `direct commit: <commit>` and also exits 1:
+//     a direct push to `main` did not go through the queue either. It is a separate defect, so it
+//     gets a separate line rather than being counted as a bypass.
+//
+// The gate run is identified by its workflow, not by "some `merge_group` run succeeded": one queue
+// entry runs several workflows (`Test`, `Merge gate`, …), and on `#680` the `Test` run of the very
+// entry the gate refused is green. Matching a green `Test` run would call that bypass a queue merge.
+//
+// Read-only: it reads commits, pull requests and workflow runs, and writes nothing — no comment, no
+// label, no merge, no ref push. Token scope: `contents:read`, `pulls:read`, `actions:read`.
+import { execFileSync } from 'node:child_process'
+import { fileURLToPath } from 'node:url'
+
+const SHORT = 12
+// The gate workflow that a queue entry has to pass. `path` is what the runs API carries for the
+// workflow; `name` is the fallback for fixtures and for API versions that omit it.
+export const GATE_PATH = '.github/workflows/merge-gate.yml'
+export const GATE_NAME = 'Merge gate'
+
+export const short = (sha) => String(sha ?? '').slice(0, SHORT)
+export const isGateRun = (run) => run?.path === GATE_PATH || run?.name === GATE_NAME
+
+// The successful gate run for this exact commit, or null. `head_sha` is the entry's built commit —
+// the commit the queue would fast-forward `main` to — so an entry built on top of an older `main`
+// says nothing about this commit.
+export function queueRun(sha, runs) {
+  const target = String(sha ?? '').toLowerCase()
+  return (runs ?? []).find((run) => String(run?.head_sha ?? '').toLowerCase() === target && isGateRun(run) && run?.conclusion === 'success') ?? null
+}
+
+// Whether the gate ran on this commit at all: it separates "the gate said no" from "nobody built a
+// queue entry this commit could have come from".
+export const gateRan = (sha, runs) => {
+  const target = String(sha ?? '').toLowerCase()
+  return (runs ?? []).some((run) => String(run?.head_sha ?? '').toLowerCase() === target && isGateRun(run))
+}
+
+// The pull request a commit came from. A merge commit names its own pull request; a squash merge
+// carries one too. Prefer the pull request whose `merge_commit_sha` is this commit, because a
+// cherry-pick can list more than one.
+export function pullOf(sha, pulls) {
+  const list = (pulls ?? []).filter((p) => p?.number)
+  return list.find((p) => String(p.merge_commit_sha ?? '').toLowerCase() === String(sha).toLowerCase()) ?? list[0] ?? null
+}
+
+// One finding for one commit, or null when the commit went through the queue. The line is the whole
+// interface: the Maintainer patrol and a hand run both read it, so its shape is fixed.
+export function auditCommit({ sha, pulls = [], runs = [] }) {
+  const pull = pullOf(sha, pulls)
+  if (!pull) return { sha, ok: false, line: `direct commit: ${sha}` }
+  if (queueRun(sha, runs)) return { sha, ok: true, pr: pull.number }
+  const state = gateRan(sha, runs) ? 'failure' : 'absent'
+  return { sha, ok: false, pr: pull.number, line: `bypassed queue: #${pull.number} ${short(sha)} (merge_group: ${state})` }
+}
+
+// The walk, over data already fetched. `pullsFor(sha)` and `runsFor(sha)` are the two calls per
+// commit; keeping them injected is what lets a test drive the whole audit from fixtures.
+export function audit({ commits, pullsFor, runsFor }) {
+  const findings = []
+  for (const commit of commits) {
+    const sha = String(typeof commit === 'string' ? commit : commit?.sha ?? '')
+    if (!sha) continue
+    const finding = auditCommit({ sha, pulls: pullsFor(sha) ?? [], runs: runsFor(sha) ?? [] })
+    if (!finding.ok) findings.push(finding)
+  }
+  return findings
+}
+
+export const exitCodeFor = (findings) => (findings.length ? 1 : 0)
+
+const realGh = (...args) => execFileSync('gh', args, { encoding: 'utf8', maxBuffer: 256 << 20 })
+
+function arg(name) {
+  const i = process.argv.indexOf(name)
+  return i > 0 ? process.argv[i + 1] : undefined
+}
+
+// The entry point a workflow or a hand run calls. `api`, `env` and `out` are injectable so a test
+// drives the whole walk from fixtures without the network.
+export function main({ api, gh = realGh, env = process.env, repo: repoOption, limit: limitOption } = {}) {
+  const fetchJson = api ?? ((path) => JSON.parse(gh('api', path)))
+  const repo = repoOption || env.GH_REPO || arg('--repo')
+  if (!repo) throw new Error('set GH_REPO or pass --repo owner/name')
+  const limit = Number(limitOption ?? arg('--limit') ?? env.LIMIT ?? 50)
+  if (!Number.isInteger(limit) || limit < 1) throw new Error(`--limit is not a positive integer: ${limit}`)
+
+  const base = `repos/${repo}`
+  const commits = fetchJson(`${base}/commits?sha=main&per_page=${limit}`)
+  if (!Array.isArray(commits)) throw new Error('the commits API did not return a list')
+  const findings = audit({
+    commits,
+    pullsFor: (sha) => fetchJson(`${base}/commits/${sha}/pulls?per_page=100`) ?? [],
+    // The runs endpoint answers `{ total_count, workflow_runs }`; only the runs matter here.
+    runsFor: (sha) => fetchJson(`${base}/actions/runs?event=merge_group&head_sha=${sha}&per_page=100`)?.workflow_runs ?? [],
+  })
+  for (const finding of findings) console.log(finding.line)
+  const bypassed = findings.filter((f) => f.pr).length
+  console.error(`audit-merges: ${commits.length} commits on main, ${bypassed} bypassed the queue, ${findings.length - bypassed} direct`)
+  return { findings, bypassed, direct: findings.length - bypassed, commits: commits.length, ok: findings.length === 0 }
+}
+
+if (process.argv[1] === fileURLToPath(import.meta.url)) {
+  try {
+    const result = main()
+    process.exit(exitCodeFor(result.findings))
+  } catch (e) {
+    console.error(`audit-merges: ${e.message}`)
+    process.exit(1)
+  }
+}

.github/scripts/audit-merges.test.mjs

@@ -0,0 +1,130 @@
+// audit-merges: a merge counts as a queue merge only when the gate itself passed on the commit that
+// landed (#689). The fixtures carry one commit/PR/run set per shape; the walk is driven from them.
+import { test } from 'node:test'
+import assert from 'node:assert/strict'
+import { readFileSync } from 'node:fs'
+import { audit, auditCommit, exitCodeFor, main, queueRun } from './audit-merges.mjs'
+
+const fixture = (name) => JSON.parse(readFileSync(new URL(`./fixtures/audit-merges/${name}.json`, import.meta.url), 'utf8'))
+const one = (name) => {
+  const f = fixture(name)
+  return auditCommit({ sha: f.sha, pulls: f.pulls, runs: f.runs })
+}
+
+test('a green Merge gate run on the landed commit is a queue merge, and prints nothing', () => {
+  const finding = one('queued')
+  assert.equal(finding.ok, true)
+  assert.equal(finding.pr, 674)
+  assert.equal(exitCodeFor([]), 0)
+})
+
+// The whole check exists because of this shape: #680 landed as 6b9820bb while the gate refused that
+// entry, and the entry's `Test` run was still green. Reading "some merge_group run succeeded" would
+// call the bypass a queue merge, which is why the run is matched by workflow.
+test('a failed Merge gate run on the landed commit is a bypass, even when Test of the same entry is green', () => {
+  const f = fixture('bypassed')
+  assert.equal(queueRun(f.sha, f.runs), null, 'the green Test run of the same entry is not the gate passing')
+  const finding = auditCommit({ sha: f.sha, pulls: f.pulls, runs: f.runs })
+  assert.equal(finding.ok, false)
+  assert.equal(finding.line, `bypassed queue: #680 ${f.sha.slice(0, 12)} (merge_group: failure)`)
+  assert.equal(exitCodeFor([finding]), 1)
+})
+
+test('a merge the gate never judged says absent, not failure', () => {
+  const finding = one('absent')
+  assert.equal(finding.line, `bypassed queue: #622 ${fixture('absent').sha.slice(0, 12)} (merge_group: absent)`)
+})
+
+test('a push that came from no pull request is its own defect, not a bypass', () => {
+  const f = fixture('direct')
+  const finding = auditCommit({ sha: f.sha, pulls: f.pulls, runs: f.runs })
+  assert.equal(finding.line, `direct commit: ${f.sha}`)
+  assert.doesNotMatch(finding.line, /bypassed queue/)
+})
+
+test('a green gate run on a different commit says nothing about this one', () => {
+  const f = fixture('queued')
+  const other = { ...f.runs[0], head_sha: 'ab'.repeat(20) }
+  const finding = auditCommit({ sha: f.sha, pulls: f.pulls, runs: [other] })
+  assert.equal(finding.line, `bypassed queue: #674 ${f.sha.slice(0, 12)} (merge_group: absent)`)
+})
+
+test('a gate run that did not conclude success (cancelled) is a failure', () => {
+  const f = fixture('queued')
+  const cancelled = { ...f.runs[0], conclusion: 'cancelled' }
+  const finding = auditCommit({ sha: f.sha, pulls: f.pulls, runs: [cancelled] })
+  assert.equal(finding.line, `bypassed queue: #674 ${f.sha.slice(0, 12)} (merge_group: failure)`)
+})
+
+test('the walk prints one line per merge that did not go through the queue, in commit order', () => {
+  const queued = fixture('queued')
+  const bypassed = fixture('bypassed')
+  const direct = fixture('direct')
+  const bySha = new Map([queued, bypassed, direct].map((f) => [f.sha, f]))
+  const paths = []
+  const api = (path) => {
+    paths.push(path)
+    if (path === 'repos/o/r/commits?sha=main&per_page=50') return [queued, bypassed, direct].map((f) => ({ sha: f.sha }))
+    const pulls = /^repos\/o\/r\/commits\/([0-9a-f]+)\/pulls\?per_page=100$/.exec(path)
+    if (pulls) return bySha.get(pulls[1]).pulls
+    const runs = /^repos\/o\/r\/actions\/runs\?event=merge_group&head_sha=([0-9a-f]+)&per_page=100$/.exec(path)
+    if (runs) return { total_count: bySha.get(runs[1]).runs.length, workflow_runs: bySha.get(runs[1]).runs }
+    throw new Error(`unexpected path ${path}`)
+  }
+  const lines = []
+  const errors = []
+  const [log, error] = [console.log, console.error]
+  console.log = (line) => lines.push(line)
+  console.error = (line) => errors.push(line)
+  let result
+  try {
+    result = main({ api, repo: 'o/r' })
+  } finally {
+    console.log = log
+    console.error = error
+  }
+  assert.deepEqual(lines, [
+    `bypassed queue: #680 ${bypassed.sha.slice(0, 12)} (merge_group: failure)`,
+    `direct commit: ${direct.sha}`,
+  ])
+  assert.equal(result.bypassed, 1)
+  assert.equal(result.direct, 1)
+  assert.equal(exitCodeFor(result.findings), 1)
+  assert.match(errors.join('\n'), /3 commits on main, 1 bypassed the queue, 1 direct/)
+  assert.ok(paths.some((p) => p.includes(`head_sha=${queued.sha}`)), 'the queued commit is audited too, it just finds nothing')
+})
+
+test('the walk is clean when every merge went through the queue', () => {
+  const queued = fixture('queued')
+  const api = (path) => {
+    if (path === 'repos/o/r/commits?sha=main&per_page=50') return [{ sha: queued.sha }]
+    if (path === `repos/o/r/commits/${queued.sha}/pulls?per_page=100`) return queued.pulls
+    if (path === `repos/o/r/actions/runs?event=merge_group&head_sha=${queued.sha}&per_page=100`) return { workflow_runs: queued.runs }
+    throw new Error(`unexpected path ${path}`)
+  }
+  const logs = []
+  const [log, error] = [console.log, console.error]
+  console.log = (line) => logs.push(line)
+  console.error = () => {}
+  let result
+  try {
+    result = main({ api, repo: 'o/r' })
+  } finally {
+    console.log = log
+    console.error = error
+  }
+  assert.deepEqual(logs, [])
+  assert.deepEqual(result.findings, [])
+  assert.equal(exitCodeFor(result.findings), 0)
+})
+
+test('audit() keeps commit order and skips a commit with no sha', () => {
+  const queued = fixture('queued')
+  const bypassed = fixture('bypassed')
+  const findings = audit({
+    commits: [{ sha: bypassed.sha }, { sha: queued.sha }, {}],
+    pullsFor: (sha) => (sha === queued.sha ? queued.pulls : bypassed.pulls),
+    runsFor: (sha) => (sha === queued.sha ? queued.runs : bypassed.runs),
+  })
+  assert.deepEqual(findings.map((f) => f.pr), [680])
+})

.github/scripts/fixtures/audit-merges/absent.json

@@ -0,0 +1,8 @@
+{
+  "note": "A shape, not a merge in this repository's history: a merge commit whose only merge_group runs belong to other workflows, so the gate never judged it.",
+  "sha": "1f4024be0c1d2e3f4a5b6c7d8e9f0a1b2c3d4e5f",
+  "pulls": [{ "number": 622, "merge_commit_sha": "1f4024be0c1d2e3f4a5b6c7d8e9f0a1b2c3d4e5f" }],
+  "runs": [
+    { "id": 39700000001, "name": "Test", "path": ".github/workflows/test.yml", "event": "merge_group", "head_sha": "1f4024be0c1d2e3f4a5b6c7d8e9f0a1b2c3d4e5f", "conclusion": "success" }
+  ]
+}

.github/scripts/fixtures/audit-merges/bypassed.json

@@ -0,0 +1,9 @@
+{
+  "note": "The bypass: #680 landed as 6b9820bb, its Merge gate run on that commit failed (39730969259) and the Test run of the same entry is green (39730969253) — a green Test run must not be read as the gate passing.",
+  "sha": "6b9820bbd75304a6eb41e90e045fc58596dd1a41",
+  "pulls": [{ "number": 680, "merge_commit_sha": "6b9820bbd75304a6eb41e90e045fc58596dd1a41" }],
+  "runs": [
+    { "id": 39730969259, "name": "Merge gate", "path": ".github/workflows/merge-gate.yml", "event": "merge_group", "head_sha": "6b9820bbd75304a6eb41e90e045fc58596dd1a41", "conclusion": "failure" },
+    { "id": 39730969253, "name": "Test", "path": ".github/workflows/test.yml", "event": "merge_group", "head_sha": "6b9820bbd75304a6eb41e90e045fc58596dd1a41", "conclusion": "success" }
+  ]
+}

.github/scripts/fixtures/audit-merges/direct.json

@@ -0,0 +1,8 @@
+{
+  "note": "A push to main that came from no pull request: reported as its own defect, not as a queue bypass.",
+  "sha": "0d1e2f3a4b5c6d7e8f9012345678abcdef012345",
+  "pulls": [],
+  "runs": [
+    { "id": 39700000002, "name": "Test", "path": ".github/workflows/test.yml", "event": "push", "head_sha": "0d1e2f3a4b5c6d7e8f9012345678abcdef012345", "conclusion": "success" }
+  ]
+}

.github/scripts/fixtures/audit-merges/queued.json

@@ -0,0 +1,9 @@
+{
+  "note": "A real queue merge: #674 landed as 9393d558 and its Merge gate run on that commit is green (run 39722858008).",
+  "sha": "9393d5586c2a06a806c04fb99cc09c4d71ebf161",
+  "pulls": [{ "number": 674, "merge_commit_sha": "9393d5586c2a06a806c04fb99cc09c4d71ebf161" }],
+  "runs": [
+    { "id": 39722858008, "name": "Merge gate", "path": ".github/workflows/merge-gate.yml", "event": "merge_group", "head_sha": "9393d5586c2a06a806c04fb99cc09c4d71ebf161", "conclusion": "success" },
+    { "id": 39722858269, "name": "Test", "path": ".github/workflows/test.yml", "event": "merge_group", "head_sha": "9393d5586c2a06a806c04fb99cc09c4d71ebf161", "conclusion": "success" }
+  ]
+}