audit-merges: a watermark, and a read-only patrol that runs it

.github/scripts/audit-merges.mjs

@@ -1,12 +1,12 @@
-// Audit that every merge on main went through the merge queue (#689).
+// Audit that every merge on main went through the merge queue (#689, #697).
 //
-//   GH_REPO=owner/repo GH_TOKEN=... node audit-merges.mjs [--limit 50] [--repo owner/repo]
+//   GH_REPO=owner/repo GH_TOKEN=... node audit-merges.mjs [--limit 50] [--since <sha|ISO date> | --baseline] [--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.
+// The merge gate judges a queue entry, and only the queue should turn that entry into a commit on
+// `main`. That second half is still a convention, and it has been broken: `#650` (`6438cb2b`) and
+// `#680` (`6b9820bb`) are commits that a pull request names as its merge commit, while the
+// `Merge gate` run of the queue entry it built failed. A real queue merge — `#672` (`93e52347`),
+// `#674` (`9393d558`) — leaves a **successful** `Merge gate` run whose `head_sha` is that commit.
 //
 // 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:
@@ -21,6 +21,18 @@
 // 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.
 //
+// **The watermark (#697).** This history contains bypasses from before `merge-gate` became a
+// required check, so a check that reports all of them is red on its first run and stays red until
+// they slide out of the window — a check nobody reads. `--since <sha|ISO date>` moves the start of
+// the walk: a sha is resolved to its committer date, the commits list is fetched from that date, and
+// a finding at or before the watermark prints as `earlier bypass: …` / `earlier direct commit: …`
+// and does not affect the exit code. Only a finding newer than the watermark fails the run, so red
+// means "new". With no `--since` everything is a finding, which is what a hand run wants.
+//
+// `--baseline` is the first run of a patrol that has no previous run: it walks the window unfiltered
+// and marks every finding `earlier`, so the history it finds becomes the watermark instead of a red
+// first run. `--since` and `--baseline` are exclusive — one starts at a point, the other sets one.
+//
 // 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'
@@ -59,29 +71,56 @@ export function pullOf(sha, pulls) {
 }
 
 // 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.
+// interface: the patrol workflow 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})` }
+  return { sha, ok: false, pr: pull.number, state, line: `bypassed queue: #${pull.number} ${short(sha)} (merge_group: ${state})` }
+}
+
+// The committer date of a commit in the shape `GET /commits` returns it, in milliseconds, or null
+// when the object carries no readable date (a test may pass a bare `{ sha }`).
+export const commitMs = (commit) => {
+  const raw = commit?.commit?.committer?.date ?? commit?.commit?.author?.date
+  if (raw === undefined || raw === null) return null
+  const ms = Date.parse(raw)
+  return Number.isNaN(ms) ? null : ms
 }
 
+// A finding at or before the watermark. It stays in the report — the history is the record — but it
+// never fails the run, so `exit 1` always means "something merged after the watermark bypassed".
+const earlierLine = (finding) =>
+  finding.pr
+    ? `earlier bypass: #${finding.pr} ${short(finding.sha)} (merge_group: ${finding.state})`
+    : `earlier direct commit: ${finding.sha}`
+
 // 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 }) {
+// commit; keeping them injected is what lets a test drive the whole audit from fixtures. `since` is
+// an ISO timestamp (or milliseconds): findings at or before it are marked `earlier` and never fail.
+export function audit({ commits, pullsFor, runsFor, since, baseline = false }) {
+  const sinceMs =
+    since === undefined || since === null || since === '' ? null : typeof since === 'number' ? since : Date.parse(since)
+  if (sinceMs !== null && Number.isNaN(sinceMs)) throw new Error(`--since is not an ISO date: ${since}`)
   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)
+    if (finding.ok) continue
+    const ms = typeof commit === 'string' ? null : commitMs(commit)
+    if (baseline || (sinceMs !== null && ms !== null && ms <= sinceMs)) {
+      finding.earlier = true
+      finding.line = earlierLine(finding)
+    }
+    findings.push(finding)
   }
   return findings
 }
 
-export const exitCodeFor = (findings) => (findings.length ? 1 : 0)
+// Only a finding newer than the watermark fails: an all-earlier report is the baseline, not a defect.
+export const exitCodeFor = (findings) => ((findings ?? []).some((f) => !f.earlier) ? 1 : 0)
 
 const realGh = (...args) => execFileSync('gh', args, { encoding: 'utf8', maxBuffer: 256 << 20 })
 
@@ -90,28 +129,55 @@ function arg(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 } = {}) {
+// `--since` takes either a sha (resolved to that commit's committer date) or an ISO date. Resolving
+// it here keeps the walk one request with `since=`, and lets the patrol pass the `head_sha` of its
+// own previous successful run without a file having to be committed.
+export function resolveSince(fetchJson, base, raw) {
+  if (raw === undefined || raw === null || raw === '') return null
+  const value = String(raw).trim()
+  if (/^[0-9a-f]{4,40}$/i.test(value)) {
+    const commit = fetchJson(`${base}/commits/${value}`)
+    const iso = commit?.commit?.committer?.date ?? commit?.commit?.author?.date
+    if (!iso) throw new Error(`--since sha ${value} has no committer date`)
+    return new Date(iso).toISOString()
+  }
+  const ms = Date.parse(value)
+  if (Number.isNaN(ms)) throw new Error(`--since is not a sha or an ISO date: ${value}`)
+  return new Date(ms).toISOString()
+}
+
+// The entry point a workflow or a hand run calls. `api`, `gh`, `env` and the explicit options 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, since: sinceOption, baseline: baselineOption } = {}) {
   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}`)
+  const baseline = baselineOption ?? (arg('--baseline') !== undefined || env.BASELINE === '1')
+  if (baseline && (sinceOption ?? arg('--since') ?? env.SINCE)) throw new Error('--baseline and --since are exclusive')
+  const since = baseline ? null : resolveSince(fetchJson, base, sinceOption ?? arg('--since') ?? env.SINCE)
+  const commits = fetchJson(`${base}/commits?sha=main&per_page=${limit}${since ? `&since=${encodeURIComponent(since)}` : ''}`)
   if (!Array.isArray(commits)) throw new Error('the commits API did not return a list')
   const findings = audit({
     commits,
+    since,
+    baseline: baseline === true,
     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 }
+  const fresh = (f) => !f.earlier
+  const bypassed = findings.filter((f) => f.pr && fresh(f)).length
+  const direct = findings.filter((f) => !f.pr && fresh(f)).length
+  const earlier = findings.filter((f) => f.earlier).length
+  console.error(
+    `audit-merges: ${commits.length} commits on main, ${bypassed} bypassed the queue, ${direct} direct, ${earlier} earlier than the watermark${baseline ? ' (baseline)' : since ? ` (${since})` : ''}`,
+  )
+  return { findings, bypassed, direct, earlier, since, baseline: baseline === true, commits: commits.length, ok: exitCodeFor(findings) === 0 }
 }
 
 if (process.argv[1] === fileURLToPath(import.meta.url)) {

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

@@ -128,3 +128,142 @@ test('audit() keeps commit order and skips a commit with no sha', () => {
   })
   assert.deepEqual(findings.map((f) => f.pr), [680])
 })
+
+// #697: the watermark. This history holds bypasses from before `merge-gate` became a required check,
+// so an unfiltered walk is red forever and a check that is always red is a check nobody reads.
+const stamp = (fixture, date) => ({ sha: fixture.sha, commit: { committer: { date } } })
+
+// A minimal API for the watermark cases: every commit carries its date, the list path may or may
+// not carry `since=`, and one commit can be asked for by sha to resolve a watermark.
+const watermarkApi = (entries) => {
+  const bySha = new Map(entries.map((entry) => [entry.fixture.sha, entry]))
+  const paths = []
+  const api = (path) => {
+    paths.push(path)
+    if (/^repos\/o\/r\/commits\?sha=main&per_page=50(&since=[^&]+)?$/.test(path)) return entries.map((entry) => stamp(entry.fixture, entry.date))
+    const commit = /^repos\/o\/r\/commits\/([0-9a-f]+)$/.exec(path)
+    if (commit) return { commit: { committer: { date: bySha.get(commit[1])?.date } } }
+    const pulls = /^repos\/o\/r\/commits\/([0-9a-f]+)\/pulls\?per_page=100$/.exec(path)
+    if (pulls) return bySha.get(pulls[1]).fixture.pulls
+    const runs = /^repos\/o\/r\/actions\/runs\?event=merge_group&head_sha=([0-9a-f]+)&per_page=100$/.exec(path)
+    if (runs) return { workflow_runs: bySha.get(runs[1]).fixture.runs }
+    throw new Error(`unexpected path ${path}`)
+  }
+  return { api, paths }
+}
+
+const runMain = (args) => {
+  const logs = []
+  const errors = []
+  const [log, error] = [console.log, console.error]
+  console.log = (line) => logs.push(line)
+  console.error = (line) => errors.push(line)
+  try {
+    return { result: main(args), logs, errors }
+  } finally {
+    console.log = log
+    console.error = error
+  }
+}
+
+test('a finding at or before the watermark is printed as earlier and leaves the exit code at 0', () => {
+  const bypassed = fixture('bypassed')
+  const direct = fixture('direct')
+  const since = '2026-10-21T12:00:00Z'
+  const { api } = watermarkApi([
+    { fixture: bypassed, date: '2026-10-21T00:00:00Z' },
+    { fixture: direct, date: since }, // the boundary itself counts as earlier
+  ])
+  const { result, logs, errors } = runMain({ api, repo: 'o/r', since })
+  assert.deepEqual(logs, [
+    `earlier bypass: #680 ${bypassed.sha.slice(0, 12)} (merge_group: failure)`,
+    `earlier direct commit: ${direct.sha}`,
+  ])
+  assert.equal(exitCodeFor(result.findings), 0)
+  assert.equal(result.earlier, 2)
+  assert.equal(result.bypassed, 0)
+  assert.equal(result.direct, 0)
+  assert.match(errors.join('\n'), /2 commits on main, 0 bypassed the queue, 0 direct, 2 earlier than the watermark/)
+})
+
+test('only a finding newer than the watermark fails, and it alone counts as bypassed', () => {
+  const bypassed = fixture('bypassed')
+  const absent = fixture('absent')
+  const since = '2026-10-21T12:00:00Z'
+  const { api } = watermarkApi([
+    { fixture: bypassed, date: '2026-10-21T00:00:00Z' },
+    { fixture: absent, date: '2026-10-22T06:00:00Z' },
+  ])
+  const { result, logs } = runMain({ api, repo: 'o/r', since })
+  assert.deepEqual(logs, [
+    `earlier bypass: #680 ${bypassed.sha.slice(0, 12)} (merge_group: failure)`,
+    `bypassed queue: #622 ${absent.sha.slice(0, 12)} (merge_group: absent)`,
+  ])
+  assert.equal(exitCodeFor(result.findings), 1)
+  assert.equal(result.bypassed, 1)
+  assert.equal(result.earlier, 1)
+})
+
+test('a sha watermark is resolved to its committer date and the walk starts there', () => {
+  const queued = fixture('queued')
+  const bypassed = fixture('bypassed')
+  const date = '2026-10-21T12:00:00Z'
+  const { api, paths } = watermarkApi([
+    { fixture: queued, date },
+    { fixture: bypassed, date: '2026-10-22T06:00:00Z' },
+  ])
+  const { result } = runMain({ api, repo: 'o/r', since: queued.sha })
+  assert.equal(result.since, '2026-10-21T12:00:00.000Z')
+  assert.ok(paths.includes(`repos/o/r/commits/${queued.sha}`), 'the sha is resolved through the commits API')
+  assert.ok(
+    paths.includes('repos/o/r/commits?sha=main&per_page=50&since=2026-10-21T12%3A00%3A00.000Z'),
+    'the walk asks for commits from the watermark on',
+  )
+})
+
+test('an unreadable watermark is an error, not a silent unfiltered walk', () => {
+  const queued = fixture('queued')
+  const { api } = watermarkApi([{ fixture: queued, date: '2026-10-21T12:00:00Z' }])
+  assert.throws(() => main({ api, repo: 'o/r', since: 'not-a-date' }), /--since is not a sha or an ISO date/)
+  assert.throws(() => audit({ commits: [], since: 'nope' }), /--since is not an ISO date/)
+})
+
+test('the patrol workflow is read-only and takes its watermark from its own previous run', () => {
+  const text = readFileSync(new URL('../workflows/audit-merges.yml', import.meta.url), 'utf8')
+  const permissions = text.slice(text.indexOf('\npermissions:'), text.indexOf('\nconcurrency:'))
+  assert.match(permissions, /^ {2}contents: read$/m)
+  assert.match(permissions, /^ {2}actions: read$/m)
+  assert.match(permissions, /^ {2}pull-requests: read$/m)
+  assert.doesNotMatch(permissions, /write/, 'the patrol must never be able to write')
+  assert.match(text, /^ {2}schedule:$/m, 'the patrol runs on a schedule')
+  assert.match(text, /^ {2}workflow_dispatch:$/m, 'and by hand')
+  assert.match(text, /status=success&per_page=1/)
+  assert.match(text, /workflow_runs\[0\]\.head_sha/)
+  assert.match(text, /--since "\$\{\{ steps\.watermark\.outputs\.since \}\}"/)
+  assert.match(text, /--baseline/, "a first run baselines instead of failing on the history")
+  assert.doesNotMatch(text, /pull_request_target|gh pr merge|git push|contents: write/)
+})
+
+test('a baseline walk reports the whole window as earlier and exits 0', () => {
+  const bypassed = fixture('bypassed')
+  const absent = fixture('absent')
+  const { api, paths } = watermarkApi([
+    { fixture: bypassed, date: '2026-10-21T00:00:00Z' },
+    { fixture: absent, date: '2026-10-22T06:00:00Z' },
+  ])
+  const { result, logs, errors } = runMain({ api, repo: 'o/r', baseline: true })
+  assert.deepEqual(logs, [
+    `earlier bypass: #680 ${bypassed.sha.slice(0, 12)} (merge_group: failure)`,
+    `earlier bypass: #622 ${absent.sha.slice(0, 12)} (merge_group: absent)`,
+  ])
+  assert.equal(exitCodeFor(result.findings), 0)
+  assert.equal(result.earlier, 2)
+  assert.ok(!paths.some((p) => p.includes('since=')), 'a baseline walk does not ask for commits from a date')
+  assert.match(errors.join('\n'), /2 earlier than the watermark \(baseline\)/)
+})
+
+test('--baseline and --since together is an error, not one silently winning', () => {
+  const queued = fixture('queued')
+  const { api } = watermarkApi([{ fixture: queued, date: '2026-10-21T12:00:00Z' }])
+  assert.throws(() => main({ api, repo: 'o/r', baseline: true, since: '2026-10-21T12:00:00Z' }), /--baseline and --since are exclusive/)
+})

.github/workflows/audit-merges.yml

@@ -0,0 +1,63 @@
+# The read-only patrol for every merge on main (#689, #697). `merge-gate` refuses a queue entry it
+# does not approve, and the merge queue is the only route into `main` — this job checks the second
+# half: `audit-merges.mjs` walks the recent commits and reports the ones that landed without a
+# successful `Merge gate` run on the commit that landed.
+#
+# Red means "new". History holds bypasses from before `merge-gate` became a required check, so the
+# walk starts at the `head_sha` of this workflow's own previous successful run: everything at or
+# before it prints as `earlier bypass` and never fails. A first run has no previous run, so it
+# baselines on the current time — the history at that moment is the baseline, and the first green is
+# the watermark. Nothing is committed for this; the state is the run history.
+#
+# Read-only by construction: `contents: read`, `actions: read`, `pull-requests: read`, and the script
+# only reads the API. It never comments, labels, enqueues or merges.
+
+name: Audit merges
+
+on:
+  schedule:
+    - cron: "41 1 * * *" # daily 09:41 Asia/Shanghai
+  workflow_dispatch:
+
+permissions:
+  contents: read
+  actions: read
+  pull-requests: read
+
+concurrency:
+  group: audit-merges
+  cancel-in-progress: false
+
+jobs:
+  audit:
+    name: audit-merges
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+    env:
+      GH_REPO: ${{ github.repository }}
+      GH_TOKEN: ${{ github.token }}
+    steps:
+      - name: Check out main
+        uses: actions/checkout@v7
+        with:
+          ref: main
+          fetch-depth: 1
+          persist-credentials: false
+
+      - name: Watermark: the head of the previous successful run
+        id: watermark
+        run: |
+          previous="$(gh api "repos/$GITHUB_REPOSITORY/actions/workflows/audit-merges.yml/runs?status=success&per_page=1" --jq '.workflow_runs[0].head_sha // empty')"
+          if [ -z "$previous" ]; then
+            echo "no previous successful run: baselining the history this run finds"
+          else
+            echo "since=$previous" >> "$GITHUB_OUTPUT"
+          fi
+
+      - name: Audit merges against the watermark
+        run: |
+          if [ -n "${{ steps.watermark.outputs.since }}" ]; then
+            node .github/scripts/audit-merges.mjs --limit 50 --since "${{ steps.watermark.outputs.since }}"
+          else
+            node .github/scripts/audit-merges.mjs --limit 50 --baseline
+          fi