From 7bd31b5bcd5f34298e99db1a26df1d54a4652d10 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 13:58:30 -0400 Subject: [PATCH 1/7] perf(pr-merge): list only new artifacts on top of a trusted carried listing The pr-verify verdict reader paged every repository artifact on each cold read (69 pages on 2026-10-27) on the job's GITHUB_TOKEN. It now builds on the newest whole listing begun within three hours: its own, the run's saved one, or the pr-merge-verify-listing artifact the last scheduled or hand-started pr-merge run on main uploaded (listing-carry.mjs, found as the head-ref map is). It reads pages newest id first down past that listing's highest id until an artifact created two hours before it was read, drops entries the pages no longer hold or that expired by the first page's Date, and keeps the result only when it holds exactly the first page's live total_count. Any other count, a missing, older or malformed base, or a failed read lists every page as before. Co-Authored-By: Claude Opus 5.5 --- ...erf-verify-incremental-listing-5b1e7c2a.md | 7 + ...erf-verify-incremental-listing-5b1e7c2a.md | 7 + .../scripts/pr-merge/clear-escalations.mjs | 1 + .github/scripts/pr-merge/listing-carry.mjs | 87 +++++ .../scripts/pr-merge/listing-carry.test.mjs | 98 ++++++ .github/scripts/pr-merge/merge-holds.mjs | 1 + .github/scripts/pr-merge/workflow.test.mjs | 4 +- .github/workflows/pr-merge.yml | 48 +++ .github/workflows/pr-require-checks.yml | 18 +- .github/workflows/pr-review-hosted.yml | 1 + .../workflows/pr-review-security-hosted.yml | 1 + docs/conventions/pr-pipeline/README.md | 9 + .../skills/babysit-prs/scripts/babysit_gh.py | 17 + .../babysit-prs/scripts/babysit_risk.py | 1 + .../scripts/babysit_verify_evidence.py | 216 ++++++++++-- .../scripts/tests/test_babysit_risk.py | 1 + .../tests/test_babysit_verify_evidence.py | 325 ++++++++++++++++++ .../scripts/tests/test_merge_holds_sync.py | 2 +- scripts/ci-lanes-pr-hold.test.sh | 13 +- 19 files changed, 826 insertions(+), 31 deletions(-) create mode 100644 .changes/pr-pipeline/perf-verify-incremental-listing-5b1e7c2a.md create mode 100644 .changes/source-control/perf-verify-incremental-listing-5b1e7c2a.md create mode 100755 .github/scripts/pr-merge/listing-carry.mjs create mode 100644 .github/scripts/pr-merge/listing-carry.test.mjs diff --git a/.changes/pr-pipeline/perf-verify-incremental-listing-5b1e7c2a.md b/.changes/pr-pipeline/perf-verify-incremental-listing-5b1e7c2a.md new file mode 100644 index 0000000000..1890153395 --- /dev/null +++ b/.changes/pr-pipeline/perf-verify-incremental-listing-5b1e7c2a.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Changed + +- pr-merge's merge job uploads its last saved artifact listing as `pr-merge-verify-listing` (scheduled and hand-started runs, one day), and the merge job, enable-auto-merge and ci-status's merge-queue reader fetch it from the newest such run on main (`.github/scripts/pr-merge/listing-carry.mjs`), so the verdict reader lists only the artifacts uploaded since instead of every page, one GITHUB_TOKEN request each (69 pages on 2026-10-27). diff --git a/.changes/source-control/perf-verify-incremental-listing-5b1e7c2a.md b/.changes/source-control/perf-verify-incremental-listing-5b1e7c2a.md new file mode 100644 index 0000000000..37f10c4d75 --- /dev/null +++ b/.changes/source-control/perf-verify-incremental-listing-5b1e7c2a.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Changed + +- `babysit_verify_evidence.py` lists only the newest artifact pages on top of a whole listing begun within three hours: its own, the run's saved one, or the one a trusted pr-merge run carried (`BABYSIT_VERIFY_LISTING_CARRY`). It reads pages newest id first down past that listing's highest id until an artifact created two hours before that listing was read, drops entries expired by the first page's `Date` or missing from the pages it read, and keeps the result only when it holds exactly the first page's live `total_count`. Any other count, a missing, older or malformed base, or a failed read lists every page as before. On 2026-10-27 this read 2 pages in 1.3 s where the full listing read 69 pages in about 51 s, with the same artifacts. diff --git a/.github/scripts/pr-merge/clear-escalations.mjs b/.github/scripts/pr-merge/clear-escalations.mjs index 9e342c25cf..c42f00f914 100755 --- a/.github/scripts/pr-merge/clear-escalations.mjs +++ b/.github/scripts/pr-merge/clear-escalations.mjs @@ -86,6 +86,7 @@ // GITHUB_API_URL // GITHUB_RUN_NUMBER where the sweep starts (rotated) // BABYSIT_VERIFY_GH_TOKEN, BABYSIT_VERIFY_LISTING, BABYSIT_VERIFY_HEAD_REFS, +// BABYSIT_VERIFY_LISTING_CARRY, // BABYSIT_RUN_CACHE the gate reader's Actions token and the run's shared // reads, as enable-auto-merge passes them import process from "node:process"; diff --git a/.github/scripts/pr-merge/listing-carry.mjs b/.github/scripts/pr-merge/listing-carry.mjs new file mode 100755 index 0000000000..224c321459 --- /dev/null +++ b/.github/scripts/pr-merge/listing-carry.mjs @@ -0,0 +1,87 @@ +#!/usr/bin/env node +// listing-carry.mjs: hands the verdict reader the repository artifact listing +// the last run saved (babysit_verify_evidence.py BABYSIT_VERIFY_LISTING_CARRY), +// so it lists only the artifacts uploaded since (`Reader._incremental`) +// instead of every page. +// +// The file is the newest scheduled or hand-started run of main's pr-merge.yml +// (rotation.mjs `newestTrustedRun`), this run aside, holding the +// pr-merge-verify-listing artifact, and only when its head commit is on main +// (head-ref-map.mjs `onMain`), as the head-ref map and the carried run cache +// are read. Only a run's own steps can upload its artifacts, so the file was +// written by main's pr-merge.yml; it never goes through actions/cache, which +// any job in main's cache scope can write. Any file already at +// BABYSIT_VERIFY_LISTING_CARRY is removed first, so one left there is never +// read. The reader checks the file itself, and keeps a listing built on it +// only when it holds exactly the live artifact count GitHub reports; +// otherwise it lists every page. +// +// Any failure writes nothing and warns: the reader then lists every page. +// +// Environment: +// ACTIONS_TOKEN the job's GITHUB_TOKEN (actions: read) +// GITHUB_REPOSITORY owner/repo +// GITHUB_RUN_ID this run, which has no listing yet +// RUNNER_TEMP where the artifact is downloaded +// PR_MERGE_READS_PATH the job's shared GET answers +// (rotation.mjs sharedReads), if set +// BABYSIT_VERIFY_LISTING_CARRY the file to write +// GITHUB_API_URL +import { renameSync, rmSync, writeFileSync } from "node:fs"; +import process from "node:process"; +import { pathToFileURL } from "node:url"; + +import { createGitHub } from "../../actions/check-trusted-trigger/check-trusted-trigger.mjs"; +import { onMain } from "./head-ref-map.mjs"; +import { ghDownload, newestTrustedRun, sharedReads } from "./rotation.mjs"; + +export const LISTING_ARTIFACT = "pr-merge-verify-listing"; +export const LISTING_FILE = "verify-listing.json"; +const REPO = /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/; + +const escapeCommand = (text) => String(text).replaceAll(/[\r\n%]/g, " "); +const isObject = (value) => value !== null && typeof value === "object" && !Array.isArray(value); + +// The newest trusted run's saved listing and its run id, or undefined when no +// run has one; an error for a file of another shape. +export async function readListing({ repo, runId, actions: client, download, shared }) { + const actions = sharedReads(client, shared); + const accept = (run) => onMain({ repo, actions, sha: run.head_sha }); + const id = await newestTrustedRun({ repo, runId, actions, name: LISTING_ARTIFACT, accept }); + if (id === undefined) { + return undefined; + } + const text = download(id); + const data = JSON.parse(text); + if (!isObject(data) || typeof data.fetchedAt !== "string" || !Array.isArray(data.artifacts)) { + throw new Error(`the artifact listing of run ${id} has an unexpected shape`); + } + return { run: id, text, count: data.artifacts.length }; +} + +export async function main({ env = process.env, actions, download } = {}) { + const repo = env.GITHUB_REPOSITORY ?? ""; + const path = env.BABYSIT_VERIFY_LISTING_CARRY; + try { + if (!path) throw new Error("BABYSIT_VERIFY_LISTING_CARRY is not set"); + rmSync(path, { force: true }); + if (!REPO.test(repo)) throw new Error("GITHUB_REPOSITORY is not owner/repo"); + actions ??= createGitHub({ token: env.ACTIONS_TOKEN, apiUrl: env.GITHUB_API_URL || undefined }); + download ??= ghDownload({ repo, token: env.ACTIONS_TOKEN, dir: env.RUNNER_TEMP, name: LISTING_ARTIFACT, file: LISTING_FILE }); + const found = await readListing({ repo, runId: String(env.GITHUB_RUN_ID), actions, download, shared: env.PR_MERGE_READS_PATH }); + if (found === undefined) { + console.log("no earlier artifact listing; the verdict reader lists every page"); + return 0; + } + writeFileSync(`${path}.tmp`, found.text); + renameSync(`${path}.tmp`, path); + console.log(`artifact listing of run ${found.run}: ${found.count} artifacts`); + } catch (error) { + console.log(`::warning title=artifact listing::${escapeCommand(error?.message ?? error)}; the verdict reader lists every page`); + } + return 0; +} + +if (import.meta.url === pathToFileURL(process.argv[1] ?? "").href) { + process.exitCode = await main(); +} diff --git a/.github/scripts/pr-merge/listing-carry.test.mjs b/.github/scripts/pr-merge/listing-carry.test.mjs new file mode 100644 index 0000000000..e665566380 --- /dev/null +++ b/.github/scripts/pr-merge/listing-carry.test.mjs @@ -0,0 +1,98 @@ +// Expected values come from the incremental listing design: the carried +// listing is read only from the newest scheduled or hand-started pr-merge.yml +// run on main other than this one (found as head-ref-map.mjs finds its map) +// whose head commit is on main, under the name pr-merge-verify-listing, never +// through actions/cache; a file already at the path is never read; any +// failure writes nothing and warns, so the reader lists every page. +import assert from "node:assert/strict"; +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { after, test } from "node:test"; + +import { forgetMainTip } from "./head-ref-map.mjs"; +import { LISTING_ARTIFACT, main } from "./listing-carry.mjs"; +import { mainCall } from "./on-main.fixture.mjs"; + +const REPO = "melodic-software/claude-code-plugins"; +const dir = mkdtempSync(join(tmpdir(), "listing-carry-")); +after(() => rmSync(dir, { recursive: true, force: true })); +const listingOf = (artifacts, extra = {}) => JSON.stringify({ fetchedAt: "2026-10-27T15:00:00+00:00", artifacts, ...extra }); +const sha = (id) => String(id).padStart(40, "a"); +const wf = (id, event, branch = "main") => ({ id, event, head_branch: branch, path: ".github/workflows/pr-merge.yml", head_sha: sha(id) }); + +let n = 0; +async function readRun({ pages = [[]], files = {}, offMain = [], fail, before } = {}) { + forgetMainTip(); + n += 1; + const path = join(dir, `carry-${n}.json`); + if (before !== undefined) writeFileSync(path, before); + const downloads = []; + const actions = async (method, url) => { + if (fail) throw fail; + const page = /\/actions\/workflows\/pr-merge\.yml\/runs\?branch=main&.*&page=(\d+)$/.exec(url)?.[1]; + if (page) return { workflow_runs: pages[Number(page) - 1] ?? [] }; + const onMain = mainCall(url, { offMain }); + if (onMain !== undefined) return onMain; + const id = /\/actions\/runs\/(\d+)\/artifacts\?per_page=100$/.exec(url)?.[1]; + assert.ok(id, `unexpected call ${url}`); + const artifacts = id in files ? [{ name: LISTING_ARTIFACT, expired: false }] : [{ name: "pr-merge-head-refs", expired: false }]; + return { total_count: artifacts.length, artifacts }; + }; + const download = (id) => { + downloads.push(id); + return files[id]; + }; + const lines = []; + const log = console.log; + console.log = (line) => lines.push(String(line)); + try { + assert.equal(await main({ env: { GITHUB_REPOSITORY: REPO, GITHUB_RUN_ID: "999", BABYSIT_VERIFY_LISTING_CARRY: path }, actions, download }), 0); + } finally { + console.log = log; + } + return { written: existsSync(path) ? readFileSync(path, "utf8") : undefined, downloads, lines }; +} + +test("the newest trusted run on main's listing is written as it was saved", async () => { + const kept = listingOf([{ id: 1 }, { id: 2 }]); + const result = await readRun({ + pages: [[wf(999, "schedule"), wf(9, "pull_request_target"), wf(8, "schedule"), wf(7, "workflow_dispatch"), wf(6, "schedule")]], + files: { 9: listingOf([{ id: 66 }]), 8: listingOf([]), 7: kept, 6: listingOf([]) }, + offMain: [sha(8)], + }); + // This run is skipped, a pull_request_target run is never asked, and run 8 + // is not on main. + assert.deepEqual(result.downloads, [7]); + assert.equal(result.written, kept); + assert.ok(result.lines.includes("artifact listing of run 7: 2 artifacts"), result.lines.join("\n")); +}); + +test("an untrusted run's listing alone writes nothing, so the reader lists every page", async () => { + const result = await readRun({ + pages: [[wf(9, "pull_request_target"), wf(8, "push"), wf(7, "schedule", "feature")]], + files: { 9: listingOf([]), 8: listingOf([]), 7: listingOf([]) }, + }); + assert.deepEqual(result.downloads, []); + assert.equal(result.written, undefined); +}); + +test("no earlier listing writes nothing, and removes a file left at the path", async () => { + const result = await readRun({ pages: [[wf(5, "schedule")]], before: listingOf([{ id: 1 }]) }); + assert.equal(result.written, undefined); + assert.deepEqual(result.lines, ["no earlier artifact listing; the verdict reader lists every page"]); +}); + +test("an unreadable or malformed listing writes nothing and warns", async () => { + for (const options of [ + { fail: new Error("HTTP 502") }, + { pages: [[wf(1, "schedule")]], files: { 1: "not json" } }, + { pages: [[wf(1, "schedule")]], files: { 1: JSON.stringify([]) } }, + { pages: [[wf(1, "schedule")]], files: { 1: listingOf({}) } }, + { pages: [[wf(1, "schedule")]], files: { 1: listingOf([], { fetchedAt: 1 }) } }, + ]) { + const result = await readRun({ ...options, before: listingOf([{ id: 1 }]) }); + assert.equal(result.written, undefined); + assert.ok(result.lines.some((line) => line.startsWith("::warning title=artifact listing::")), result.lines.join("\n")); + } +}); diff --git a/.github/scripts/pr-merge/merge-holds.mjs b/.github/scripts/pr-merge/merge-holds.mjs index 16a1d73231..4d4b895b6f 100755 --- a/.github/scripts/pr-merge/merge-holds.mjs +++ b/.github/scripts/pr-merge/merge-holds.mjs @@ -272,6 +272,7 @@ export const HOLD_PATH_CLASSES = Object.freeze([ [".github/scripts/pr-merge/who.mjs", "gate"], [".github/scripts/pr-merge/post-unarmed-status.mjs", "gate"], [".github/scripts/pr-merge/verify-answers.mjs", "gate"], + [".github/scripts/pr-merge/listing-carry.mjs", "gate"], ["plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py", "gate"], ["plugins/source-control/skills/babysit-prs/scripts/babysit_check_inputs.py", "enforcement"], ["plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py", "enforcement"], diff --git a/.github/scripts/pr-merge/workflow.test.mjs b/.github/scripts/pr-merge/workflow.test.mjs index 1ce3588696..e0d76992c2 100644 --- a/.github/scripts/pr-merge/workflow.test.mjs +++ b/.github/scripts/pr-merge/workflow.test.mjs @@ -333,8 +333,8 @@ test("enable-auto-merge skips its work when a waiting run's enable-auto-merge jo // answers in that job's RUNNER_TEMP; the readers' tests count what it saves. test("the merge and enable-auto-merge jobs' kept-file readers share their reads", () => { for (const [id, scripts] of [ - ["merge", ["rotation.mjs", "head-ref-map.mjs read", "lane-records.mjs read"]], - ["enable-auto-merge", ["head-ref-map.mjs read", "lane-records.mjs read", "verify-answers.mjs"]], + ["merge", ["rotation.mjs", "head-ref-map.mjs read", "lane-records.mjs read", "listing-carry.mjs"]], + ["enable-auto-merge", ["head-ref-map.mjs read", "lane-records.mjs read", "verify-answers.mjs", "listing-carry.mjs"]], ]) { for (const script of scripts) { const step = new RegExp(`\\n {10}PR_MERGE_READS_PATH: (.*)\\n {8}run: node \\.base/\\.github/scripts/pr-merge/${script.replace(".", "\\.")}\\n`).exec(job(id)); diff --git a/.github/workflows/pr-merge.yml b/.github/workflows/pr-merge.yml index 6b7c137e22..10d8d64c4d 100644 --- a/.github/workflows/pr-merge.yml +++ b/.github/workflows/pr-merge.yml @@ -840,6 +840,23 @@ jobs: CARRY_TO: ${{ runner.temp }}/babysit-run-cache.json run: node .base/.github/scripts/pr-merge/carry-run-cache.mjs + # The artifact listing the last run saved (the pr-merge-verify-listing + # artifact, found the way the head-ref map is): the verdict reader lists + # only the pages uploaded since and keeps the result only when it holds + # GitHub's live artifact count, else lists every page. Any error leaves + # no file, and the reader lists every page. Rules: + # .github/scripts/pr-merge/listing-carry.mjs and + # babysit_verify_evidence.py `Reader._incremental`. + - name: Read the artifact listing the last run kept + if: steps.candidates.outcome == 'success' + continue-on-error: true + timeout-minutes: 2 + env: + ACTIONS_TOKEN: ${{ github.token }} + BABYSIT_VERIFY_LISTING_CARRY: ${{ runner.temp }}/verify-listing-carry.json + PR_MERGE_READS_PATH: ${{ runner.temp }}/pr-merge-reads.json + run: node .base/.github/scripts/pr-merge/listing-carry.mjs + # The lane dispatch and re-run records earlier post-merge checks proved # (the pr-merge-lane-records artifact, found the way the cursor is): # each check below, the gate's included, answers those with no read and @@ -1014,6 +1031,7 @@ jobs: API_BUDGET_OVERRIDE_TOKEN: github-token BABYSIT_VERIFY_GH_TOKEN: ${{ github.token }} BABYSIT_VERIFY_LISTING: ${{ runner.temp }}/verify-listing.json + BABYSIT_VERIFY_LISTING_CARRY: ${{ runner.temp }}/verify-listing-carry.json REPO: ${{ github.repository }} RUNG_CAP: ${{ vars.CLAUDE_LANES_MERGE_RUNG_CAP }} MAX_MINUTES: ${{ steps.mode.outputs.skip-max-minutes }} @@ -1075,6 +1093,8 @@ jobs: # candidate the saved listing would allow lists again before it is # allowed. BABYSIT_VERIFY_LISTING: ${{ runner.temp }}/verify-listing.json + # The listing the last run kept, which a listing builds on. + BABYSIT_VERIFY_LISTING_CARRY: ${{ runner.temp }}/verify-listing-carry.json # The head-ref map read above, if any. BABYSIT_VERIFY_HEAD_REFS: ${{ runner.temp }}/head-refs-in.json # The lane records read and added to above, for a gate that checks @@ -1451,6 +1471,7 @@ jobs: GH_TOKEN: ${{ steps.lane-token.outputs.token }} BABYSIT_VERIFY_GH_TOKEN: ${{ github.token }} BABYSIT_VERIFY_LISTING: ${{ runner.temp }}/verify-listing.json + BABYSIT_VERIFY_LISTING_CARRY: ${{ runner.temp }}/verify-listing-carry.json BABYSIT_VERIFY_HEAD_REFS: ${{ runner.temp }}/head-refs-in.json BABYSIT_RUN_CACHE: ${{ runner.temp }}/babysit-run-cache.json API_BUDGET_TOKEN: lanes-app @@ -1459,6 +1480,23 @@ jobs: LANE_PRS: ${{ steps.mode.outputs.lane-prs }} run: node .base/.github/scripts/pr-merge/clear-escalations.mjs + # The next run's base for its first listing: the run's last saved + # listing, written only by a listing that read whole (every page, or + # the pages since a base, holding GitHub's live count). Only a + # scheduled or hand-started run's file is read back, so only one + # uploads it. Rules: .github/scripts/pr-merge/listing-carry.mjs. + - name: Keep the artifact listing for the next run + if: >- + always() && steps.gate.outcome != 'skipped' + && (github.event_name == 'schedule' || github.event_name == 'workflow_dispatch') + continue-on-error: true + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: + name: pr-merge-verify-listing + path: ${{ runner.temp }}/verify-listing.json + retention-days: 1 + if-no-files-found: ignore + - name: Log the API budget at job end id: budget-end if: always() && steps.lane-token.outputs.token != '' @@ -1669,6 +1707,15 @@ jobs: PR_MERGE_READS_PATH: ${{ runner.temp }}/pr-merge-reads.json run: node .base/.github/scripts/pr-merge/head-ref-map.mjs read + - name: Read the artifact listing the last gate run kept + if: steps.lane-token.outputs.token != '' && (github.event_name == 'schedule' || github.event_name == 'workflow_dispatch') + continue-on-error: true + env: + ACTIONS_TOKEN: ${{ github.token }} + BABYSIT_VERIFY_LISTING_CARRY: ${{ runner.temp }}/verify-listing-carry.json + PR_MERGE_READS_PATH: ${{ runner.temp }}/pr-merge-reads.json + run: node .base/.github/scripts/pr-merge/listing-carry.mjs + - name: Read the verdict reads the last run kept if: steps.lane-token.outputs.token != '' && (github.event_name == 'schedule' || github.event_name == 'workflow_dispatch') continue-on-error: true @@ -1753,6 +1800,7 @@ jobs: EVENT_NAME: ${{ github.event_name }} BABYSIT_VERIFY_GH_TOKEN: ${{ github.token }} BABYSIT_VERIFY_LISTING: ${{ runner.temp }}/verify-listing.json + BABYSIT_VERIFY_LISTING_CARRY: ${{ runner.temp }}/verify-listing-carry.json BABYSIT_VERIFY_HEAD_REFS: ${{ runner.temp }}/head-refs-in.json BABYSIT_RUN_CACHE: ${{ runner.temp }}/babysit-run-cache.json # The verifier answers read above, and the file this run keeps. diff --git a/.github/workflows/pr-require-checks.yml b/.github/workflows/pr-require-checks.yml index 1224401ff4..9eff7edd88 100644 --- a/.github/workflows/pr-require-checks.yml +++ b/.github/workflows/pr-require-checks.yml @@ -3851,7 +3851,8 @@ jobs: # image's default. The same checkout of the default branch holds the # merge queue's verdict reader as main has it: babysit_verify_evidence.py # with its sibling modules and its config parser, and the pr-merge - # scripts that start its run cache and read its head-ref map, with every + # scripts that start its run cache and read its head-ref map and + # carried artifact listing, with every # module they import (scripts/ci-lanes-pr-hold.test.sh checks the list). # No pull request code is checked out and no credential is kept. - name: Check out main's Node pin and verdict reader @@ -3868,6 +3869,7 @@ jobs: /plugins/source-control/lib/yaml-subset.awk /.github/scripts/pr-merge/head-ref-map.mjs /.github/scripts/pr-merge/carry-run-cache.mjs + /.github/scripts/pr-merge/listing-carry.mjs /.github/scripts/pr-merge/rotation.mjs /.github/actions/check-trusted-trigger/check-trusted-trigger.mjs /.github/actions/check-kill-switch/check-kill-switch.mjs @@ -4431,7 +4433,7 @@ jobs: } if [[ $AUTHOR_ID =~ ^[0-9]+$ && " $exempt_author_ids " == *" $AUTHOR_ID "* ]]; then # The trust-gate paths, as bash patterns (`*` crosses `/`). - hold_patterns=('.github/standards/trusted-actors/*' '.github/actions/check-trusted-trigger/*' '.github/actions/resolve-config/*' '.github/actions/request-lane-token/*' '.github/actions/check-kill-switch/*' '.github/actions/log-api-budget/rate-guard.mjs' '.github/actions/log-api-budget/rate-limits.json' '.github/scripts/pr-merge/merge-holds.mjs' '.github/scripts/pr-merge/enable-auto-merge.mjs' '.github/scripts/pr-merge/superseded.mjs' '.github/scripts/pr-merge/clear-escalations.mjs' '.github/scripts/pr-merge/carry-run-cache.mjs' '.github/scripts/pr-merge/gate-check.mjs' '.github/scripts/pr-merge/head-ref-map.mjs' '.github/scripts/pr-merge/lane-records.mjs' '.github/scripts/pr-merge/rate-stop.bash' '.github/scripts/pr-merge/refire-refused.mjs' '.github/scripts/pr-merge/rotation.mjs' '.github/scripts/pr-merge/sweep-summary.mjs' '.github/scripts/pr-merge/who.mjs' '.github/scripts/pr-merge/post-unarmed-status.mjs' '.github/scripts/pr-merge/verify-answers.mjs' '.github/scripts/pr-unblock/address-feedback.mjs' '.github/scripts/pr-unblock/gate-source.mjs' '.github/scripts/pr-unblock/thread-scope.mjs' '.github/scripts/pr-unblock/retry-fetch.mjs' '.github/scripts/work-resolve-issue/report-outcome.mjs' '.github/scripts/work-resolve-issue/limits.mjs' '.github/scripts/work-resolve-issue/retry-fetch.mjs' '.github/workflows/pr-require-checks.yml' '.github/workflows/pr-merge.yml' 'plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_check_inputs.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_classify.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_dependabot.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_feedback.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_gh.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_repo_config.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_review_trigger.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_run_cache.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_state.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_util.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_whole_review.py' 'docs/conventions/pr-pipeline.yaml' 'CODEOWNERS' '*/CODEOWNERS' '.gitmodules' '*/.gitmodules') + hold_patterns=('.github/standards/trusted-actors/*' '.github/actions/check-trusted-trigger/*' '.github/actions/resolve-config/*' '.github/actions/request-lane-token/*' '.github/actions/check-kill-switch/*' '.github/actions/log-api-budget/rate-guard.mjs' '.github/actions/log-api-budget/rate-limits.json' '.github/scripts/pr-merge/merge-holds.mjs' '.github/scripts/pr-merge/enable-auto-merge.mjs' '.github/scripts/pr-merge/superseded.mjs' '.github/scripts/pr-merge/clear-escalations.mjs' '.github/scripts/pr-merge/carry-run-cache.mjs' '.github/scripts/pr-merge/gate-check.mjs' '.github/scripts/pr-merge/head-ref-map.mjs' '.github/scripts/pr-merge/lane-records.mjs' '.github/scripts/pr-merge/rate-stop.bash' '.github/scripts/pr-merge/refire-refused.mjs' '.github/scripts/pr-merge/rotation.mjs' '.github/scripts/pr-merge/sweep-summary.mjs' '.github/scripts/pr-merge/who.mjs' '.github/scripts/pr-merge/post-unarmed-status.mjs' '.github/scripts/pr-merge/verify-answers.mjs' '.github/scripts/pr-merge/listing-carry.mjs' '.github/scripts/pr-unblock/address-feedback.mjs' '.github/scripts/pr-unblock/gate-source.mjs' '.github/scripts/pr-unblock/thread-scope.mjs' '.github/scripts/pr-unblock/retry-fetch.mjs' '.github/scripts/work-resolve-issue/report-outcome.mjs' '.github/scripts/work-resolve-issue/limits.mjs' '.github/scripts/work-resolve-issue/retry-fetch.mjs' '.github/workflows/pr-require-checks.yml' '.github/workflows/pr-merge.yml' 'plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_check_inputs.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_classify.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_dependabot.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_feedback.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_gh.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_repo_config.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_review_trigger.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_run_cache.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_state.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_util.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_whole_review.py' 'docs/conventions/pr-pipeline.yaml' 'CODEOWNERS' '*/CODEOWNERS' '.gitmodules' '*/.gitmodules') if [[ ! $PR_NUMBER =~ ^[1-9][0-9]*$ ]]; then echo "::error::the pull request number '$PR_NUMBER' is not a number." exit 1 @@ -4833,9 +4835,10 @@ jobs: # babysit_verify_evidence.py `evidence --base-sha ` from # the .base checkout of main for the pull request at its head, the # reader enable-auto-merge armed it on, on a listing of its own (every - # page, saved to BABYSIT_VERIFY_LISTING) with pr-merge's head-ref map and - # carried run cache, so a FAIL uploaded after enable-auto-merge's one - # listing of its run still holds. Its report goes to merge-holds.mjs + # page, or the pages since pr-merge's last saved listing when they hold + # GitHub's live artifact count, saved to BABYSIT_VERIFY_LISTING) with + # pr-merge's head-ref map and carried run cache, so a FAIL uploaded + # after enable-auto-merge's one listing of its run still holds. Its report goes to merge-holds.mjs # evidenceHold, which holds on every refusal but the three a pass decides # (those stay with verdictHold and linkHold): every FAIL class, # verify:unreadable and verify:too-old. The same approval of the head @@ -4869,6 +4872,7 @@ jobs: PR_MERGE_SCRIPTS: .base/.github/scripts/pr-merge ACTIONS_TOKEN: ${{ github.token }} BABYSIT_VERIFY_LISTING: ${{ runner.temp }}/verify-listing.json + BABYSIT_VERIFY_LISTING_CARRY: ${{ runner.temp }}/verify-listing-carry.json BABYSIT_VERIFY_HEAD_REFS: ${{ runner.temp }}/head-refs-in.json HEAD_REFS_PATH: ${{ runner.temp }}/head-refs-in.json PR_MERGE_READS_PATH: ${{ runner.temp }}/pr-merge-reads.json @@ -5013,6 +5017,10 @@ jobs: # pr-merge's head-ref map: a read that fails leaves none, and the # reader downloads each untied FAIL instead. node "$PR_MERGE_SCRIPTS/head-ref-map.mjs" read || echo "::notice::no head-ref map from pr-merge's last run; the verdict reader reads each untied FAIL." + # pr-merge's last saved artifact listing: the reader lists only the + # pages since it and keeps that listing only when it holds GitHub's + # live artifact count, else lists every page, as with no file. + node "$PR_MERGE_SCRIPTS/listing-carry.mjs" || echo "::notice::no artifact listing from pr-merge's last run; the verdict reader lists every page." # A cold reader runs only with the headroom for it. With no carried # run cache, or one whose carried sections (carry-run-cache.mjs # carry) are all empty, the reader reads every FAIL: about 860 diff --git a/.github/workflows/pr-review-hosted.yml b/.github/workflows/pr-review-hosted.yml index 8674a1c918..50b7598c37 100644 --- a/.github/workflows/pr-review-hosted.yml +++ b/.github/workflows/pr-review-hosted.yml @@ -222,6 +222,7 @@ jobs: .github/scripts/pr-merge/who.mjs .github/scripts/pr-merge/post-unarmed-status.mjs .github/scripts/pr-merge/verify-answers.mjs + .github/scripts/pr-merge/listing-carry.mjs .github/scripts/pr-unblock/address-feedback.mjs .github/scripts/pr-unblock/gate-source.mjs .github/scripts/pr-unblock/thread-scope.mjs diff --git a/.github/workflows/pr-review-security-hosted.yml b/.github/workflows/pr-review-security-hosted.yml index 47fb86da95..38f046adc3 100644 --- a/.github/workflows/pr-review-security-hosted.yml +++ b/.github/workflows/pr-review-security-hosted.yml @@ -222,6 +222,7 @@ jobs: .github/scripts/pr-merge/who.mjs .github/scripts/pr-merge/post-unarmed-status.mjs .github/scripts/pr-merge/verify-answers.mjs + .github/scripts/pr-merge/listing-carry.mjs .github/scripts/pr-unblock/address-feedback.mjs .github/scripts/pr-unblock/gate-source.mjs .github/scripts/pr-unblock/thread-scope.mjs diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index b409e0b706..8a6bbd87d3 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -1856,6 +1856,15 @@ network error is read again, three reads in all with backoff, all inside the one read would get; any other failure, a timeout, a 429 or a rate-limit 403 included, is not. A failed listing is never saved, and is kept for that candidate's gate run: the pull request and every queued entry refuse `verify:unreadable` without the listing being paged again, so a listing outage holds the whole queue rather than merging blind. +A listing read from the API builds on the newest whole listing begun within three hours (the +reader's own, the run's saved one, or the `pr-merge-verify-listing` artifact the last scheduled or +hand-started run of main's pr-merge.yml uploaded, which `.github/scripts/pr-merge/listing-carry.mjs` +fetches as the head-ref map is fetched): it reads pages newest id first down past that listing's +highest id until an artifact created two hours before that listing was read, keeps that listing's +entries below the lowest id those pages hold, less those expired by the first page's `Date`, and is +kept only when it holds exactly that page's `total_count`. Any other count, a missing, older or +malformed base, or any failed read lists every page, so a carried listing that leaves out a FAIL +lists every page too (the rule and its basis: `babysit_verify_evidence.py`, "Carried reads"). The lane token holds no Actions permission, so pr-merge.yml grants its job `actions: read` and passes the job's `GITHUB_TOKEN` as `BABYSIT_VERIFY_GH_TOKEN`; the reader uses it as `GH_TOKEN` for the artifact listing, the run records, the download of a verdict.json (by the listed artifact's id, one request where `gh run diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_gh.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_gh.py index 0abede5193..b7a97dcc30 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_gh.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_gh.py @@ -1013,6 +1013,23 @@ def gh_capture( ) +def gh_api_with_headers( + args: list[str], + *, + timeout_seconds: float | None = None, + env_overrides: Mapping[str, str] | None = None, +) -> tuple[subprocess.CompletedProcess[str], dict[str, str]]: + """One `gh api` request, as `gh_capture` sends it, and its response + headers by lowercase name (none for a call `_includes_headers` sends + without -i).""" + return _send( + args, + timeout_seconds=timeout_seconds, + env_overrides=env_overrides, + binary=False, + ) + + def gh_json(args: list[str], env_overrides: Mapping[str, str] | None = None) -> Any: text = _run_gh( args, timeout_seconds=None, check=True, env_overrides=env_overrides diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py index 985ef2e3b2..9038e4956a 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py @@ -232,6 +232,7 @@ class RiskResult: (".github/scripts/pr-merge/who.mjs", "gate"), (".github/scripts/pr-merge/post-unarmed-status.mjs", "gate"), (".github/scripts/pr-merge/verify-answers.mjs", "gate"), + (".github/scripts/pr-merge/listing-carry.mjs", "gate"), ("plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py", "gate"), ("plugins/source-control/skills/babysit-prs/scripts/babysit_check_inputs.py", "enforcement"), ("plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py", "enforcement"), diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py index 5d14441583..646c326998 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py @@ -86,8 +86,31 @@ comments for an address-feedback round, whose push gives a head the verifier judges again. Any read that fails refuses (`verify:unreadable`). The repository's artifacts are listed once per -`Reader`, every page, and filtered on the name here: the API's name filter +`Reader`, every artifact, and filtered on the name here: the API's name filter matches whole names only, and a FAIL counts under any number and head. + +Carried reads. What an earlier run read is used only where it counts a FAIL +(a carried record counts the FAIL when it shows origin; any other answer from +it could turn the FAIL away, so it is read again), with one exception: the +artifact listing. A listing begun at most `LISTING_CARRY_MAX_AGE` ago (this +Reader's own, the run's saved one, or the one a scheduled or hand-started run +of main's pr-merge.yml saved and `LISTING_CARRY_ENV` names, which +.github/scripts/pr-merge/listing-carry.mjs fetches only from such a run whose +head is on main) is a base: `Reader._incremental` reads only the newest pages, +newest id first, down past the base's highest id until an artifact created +`LISTING_OVERLAP` before the base was read, and keeps the base's entries below +the lowest id those pages hold, less those expired or past `expires_at` by the +first page's `Date`. Such a listing could hide a FAIL by leaving it out, so it +is kept only when it holds exactly the first page's `total_count`, GitHub's +live count of the repository's artifacts. A probe on 2026-10-27 saw that count +fall by exactly the five artifacts whose `expires_at` had passed, and an +incremental listing match a full one to the artifact; an artifact that expires +during the read can only make the counts differ. Any other count, a base that +is missing, older or of another shape, or any failed read lists every page as +before. A carry forged to drop a FAIL must therefore also hold a fabricated +entry in its place to match the live count, and forging it already needs code +running in a scheduled or hand-started run of main's pr-merge.yml, whose merge +job holds the lanes App token. pr-merge.yml builds one `Reader` per candidate gate run and names a file in `LISTING_ENV`: a successful listing is saved there, and later candidates of the run read it instead of paging again. No judgment uses a listing, saved @@ -136,6 +159,7 @@ from collections.abc import Callable from dataclasses import asdict, dataclass, field, replace from datetime import UTC, datetime, timedelta +from email.utils import parsedate_to_datetime from pathlib import Path from typing import Any @@ -146,6 +170,7 @@ from babysit_gh import ( fetch_paginated_api, flatten_paginated_items, + gh_api_with_headers, gh_http_status, pull_request_commits_once, ) @@ -155,6 +180,7 @@ TRUSTED_ACTORS_PATH = ".github/standards/trusted-actors/trusted-actors.json" TOKEN_ENV = "BABYSIT_VERIFY_GH_TOKEN" LISTING_ENV = "BABYSIT_VERIFY_LISTING" +LISTING_CARRY_ENV = "BABYSIT_VERIFY_LISTING_CARRY" HEAD_REFS_ENV = "BABYSIT_VERIFY_HEAD_REFS" # The head-ref map's version and its keys' shape, an artifact id # (head-ref-map.mjs writes both). @@ -224,6 +250,22 @@ def verdict_name(name: str) -> re.Match[str] | None: # The oldest listing a judgment uses, saved or in memory (see the bound), # unless `evidence --shared-listing` lifts it (`Reader.listing_max_age`). LISTING_MAX_AGE = run_cache.MAX_AGE +# The incremental listing (`Reader._incremental`). The listing comes newest id +# first, but a lower id can be created after a higher one: across 6,835 +# artifacts listed on 2026-10-27 (created 2026-10-16 to 10-27) the largest lead +# of a lower id's created_at over a higher id's was 58m49s, and each day's +# largest was 34 to 59 minutes. Paging stops past the base's highest id only +# at an artifact created `LISTING_OVERLAP` before the base was read, twice that +# lead (judgment on that data). +LISTING_OVERLAP = timedelta(hours=2) +# The oldest base an incremental listing starts from. Scheduled pr-merge runs +# come four times an hour (pr-merge.yml `cron`), and rotation.mjs asks at most +# 10 trusted runs (`MAX_RUNS`), about 2.5 hours of them, for the carried +# listing. At the most uploads seen in an hour on 2026-10-27 (255), a base this +# old and the overlap page at most about 1,300 artifacts, 13 pages, against 69 +# for the whole listing that day (judgment on that data). +LISTING_CARRY_MAX_AGE = timedelta(hours=3) +LISTING_PAGE_SIZE = 100 # How gh reports a request that never got an HTTP response: its own # "error connecting to " for a failed lookup, else Go's net/http error. NETWORK_ERRORS = ( @@ -469,8 +511,9 @@ def task_lines(item: dict[str, Any]) -> list[str]: RECORD_SECTION = "fail-head-ref" # What earlier pr-merge runs read, copied into this run's cache by # .github/scripts/pr-merge/carry-run-cache.mjs from the last trusted runs' -# pr-merge-run-cache and pr-merge-verify-run-cache artifacts. Each is still used only where it counts a FAIL; -# any other answer is read from GitHub. +# pr-merge-run-cache and pr-merge-verify-run-cache artifacts. Each is still +# used only where it counts a FAIL; any other answer is read from GitHub (the +# module docstring's "Carried reads", which names the listing's exception). CARRIED_RUN_SECTION = "carried-run-record" CARRIED_RECORD_SECTION = "carried-fail-head-ref" CARRIED_ON_BRANCH = "carried-on-branch" @@ -680,6 +723,19 @@ def _is_artifact_id(value: Any) -> bool: return isinstance(value, int) and not isinstance(value, bool) +def _dated_entry(item: Any) -> bool: + """A listed artifact `Reader._incremental` can place: a positive id, a + boolean `expired`, and a `created_at` and `expires_at` with a zone.""" + return ( + is_json_object(item) + and _is_artifact_id(item.get("id")) + and item["id"] > 0 + and isinstance(item.get("expired"), bool) + and _when(item.get("created_at")) is not None + and _when(item.get("expires_at")) is not None + ) + + def load_head_refs(path: str | None, repo: str) -> dict[int, str]: """The head-ref map at `path` for `repo`, by artifact id. A missing or unreadable file, another version or repository, or any entry of another @@ -725,6 +781,11 @@ class Reader: listing_path: str | None = field( default_factory=lambda: os.environ.get(LISTING_ENV) or None ) + # The listing a trusted pr-merge run saved (`LISTING_CARRY_ENV`), which + # listing-carry.mjs fetched, as a base for `_incremental`. + carry_path: str | None = field( + default_factory=lambda: os.environ.get(LISTING_CARRY_ENV) or None + ) # The head-ref map (`HEAD_REFS_ENV`), when pr-merge.yml names one. head_refs_path: str | None = field( default_factory=lambda: os.environ.get(HEAD_REFS_ENV) or None @@ -742,6 +803,11 @@ class Reader: _fetched_at: datetime | None = None # when `_artifacts` began to be read _saved: bool = False # `_artifacts` came from `listing_path` _own: bool = False # `refresh()` asked for a listing of this Reader's own + # The last listing this Reader held and when it began to be read, kept + # past `refresh()` as a base for `_incremental`. + _base: tuple[datetime, list[dict[str, Any]]] | None = None + # How the last listing was read from the API: "full" or "incremental". + listing_mode: str | None = None _listing_error: str | None = None _runs: dict[int, str] = field(default_factory=dict) _started: dict[int, Any] = field(default_factory=dict) @@ -792,20 +858,30 @@ def _current_listing(self) -> list[dict[str, Any]]: self._fetched_at, self.listing_max_age ): return self._artifacts - saved = None if self._own else self._load_saved() + saved = ( + None + if self._own + else self._load_saved(self.listing_path, self.listing_max_age) + ) if saved is not None: self._fetched_at, self._artifacts = saved + self._base = saved self._saved = True return self._artifacts fetched_at = _wall_clock() - try: - listing = flatten_paginated_items( - self._read_listing(), "repository artifacts", items_key="artifacts" - ) - except (_Unreadable, RuntimeError) as exc: - self._listing_error = str(exc) - raise _Unreadable(self._listing_error) from exc + listing = self._incremental() + self.listing_mode = "incremental" + if listing is None: + self.listing_mode = "full" + try: + listing = flatten_paginated_items( + self._read_listing(), "repository artifacts", items_key="artifacts" + ) + except (_Unreadable, RuntimeError) as exc: + self._listing_error = str(exc) + raise _Unreadable(self._listing_error) from exc self._artifacts, self._fetched_at = listing, fetched_at + self._base = (fetched_at, listing) self._saved = self._own = False self._save(listing, fetched_at) return listing @@ -830,28 +906,125 @@ def refresh(self) -> None: if self._saved: self._artifacts, self._saved, self._own = None, False, True - def _load_saved(self) -> tuple[datetime, list[dict[str, Any]]] | None: - """The run's saved listing and when it began to be read. A missing or - malformed file, or one with no `fetchedAt` within `listing_max_age`, - is none, so the listing is read from the API instead.""" - if not self.listing_path: + def _load_saved( + self, path: str | None, max_age: timedelta | None + ) -> tuple[datetime, list[dict[str, Any]]] | None: + """The listing saved at `path` and when it began to be read. A + missing or malformed file, or one with no `fetchedAt` within + `max_age`, is none, so the listing is read from the API instead.""" + if not path: return None try: - saved = json_object( - json.loads(Path(self.listing_path).read_text(encoding="utf-8")) - ) + saved = json_object(json.loads(Path(path).read_text(encoding="utf-8"))) fetched_at = datetime.fromisoformat(saved["fetchedAt"]) except (OSError, ValueError, KeyError, TypeError): return None listing = saved.get("artifacts") if ( - not _recent(fetched_at, self.listing_max_age) + not _recent(fetched_at, max_age) or not is_json_array(listing) or not all(map(is_json_object, listing)) ): return None return fetched_at, listing + def _incremental_base(self) -> tuple[datetime, list[dict[str, Any]]] | None: + """The newest whole listing begun at most `LISTING_CARRY_MAX_AGE` ago + whose every entry `_dated_entry` takes: this Reader's own, the run's + saved one, or the one a trusted run carried (`carry_path`).""" + bases = [ + base + for base in ( + self._base, + self._load_saved(self.listing_path, LISTING_CARRY_MAX_AGE), + self._load_saved(self.carry_path, LISTING_CARRY_MAX_AGE), + ) + if base is not None + and _recent(base[0], LISTING_CARRY_MAX_AGE) + and all(_dated_entry(a) for a in base[1]) + ] + return max(bases, key=lambda base: base[0], default=None) + + def _incremental(self) -> list[dict[str, Any]] | None: + """Every repository artifact, as the base (`_incremental_base`) and + the newest pages read now show them, or None for a full listing. + + The listing comes newest id first. Pages are read from the first + until one holds an artifact at or below the base's highest id that + was created `LISTING_OVERLAP` or more before the base was read: every + lower id was created before the base was read, so the base holds it. + Those pages decide every id down to the lowest they hold, an id the + base holds there and they do not (deleted, or an overwrite's old + copy) included; below it the base's entries stand, less each one + expired or past its `expires_at` by the first page's `Date`. The + result must hold exactly the first page's `total_count` artifacts, + GitHub's live count. Any other count, no base, an entry or page of + another shape, a missing `Date`, or any failed read gives None.""" + base = self._incremental_base() + if base is None: + return None + base_at, carried = base + high = max((a["id"] for a in carried), default=0) + floor = base_at - LISTING_OVERLAP + deadline = _listing_clock() + LISTING_TIMEOUT_SECONDS + fresh: dict[int, dict[str, Any]] = {} + total: Any = None + server_now: datetime | None = None + page = 1 + while True: + left = deadline - _listing_clock() + if left <= 0: + return None + try: + proc, headers = gh_api_with_headers( + [ + "api", + f"repos/{self.repo}/actions/artifacts" + f"?per_page={LISTING_PAGE_SIZE}&page={page}", + ], + timeout_seconds=left, + env_overrides=self._env(), + ) + body = json_object(json.loads(proc.stdout)) + except (RuntimeError, ValueError, OSError): + return None + items = body.get("artifacts") + if ( + proc.returncode != 0 + or not is_json_array(items) + or not all(_dated_entry(a) for a in items) + ): + return None + if page == 1: + total = body.get("total_count") + try: + server_now = parsedate_to_datetime(headers.get("date", "")) + except (TypeError, ValueError): + return None + if server_now.tzinfo is None or not _is_artifact_id(total): + return None + for item in items: + fresh.setdefault(item["id"], item) + reached = any( + a["id"] <= high and _when(a["created_at"]) < floor for a in items + ) + if reached or len(items) < LISTING_PAGE_SIZE: + break + page += 1 + if server_now is None: + return None + # A short page ends the listing: no id is left below it. + lowest = min(fresh) if reached else 0 + kept = [ + a + for a in carried + if a["id"] < lowest + and a["expired"] is False + and _when(a["expires_at"]) > server_now + ] + listing = sorted([*fresh.values(), *kept], key=lambda a: -a["id"]) + return listing if len(listing) == total else None + def _save(self, listing: list[dict[str, Any]], fetched_at: datetime) -> None: """Best effort: a failed write leaves the next candidate to list.""" if not self.listing_path: @@ -956,7 +1129,8 @@ def _run_state(self, run_id: int, *, shared: bool = False) -> str: if shared and not cached: # A carried record counts the FAIL when it shows origin; any # other answer from it could turn the FAIL away, so the run is - # read instead. + # read instead ("Carried reads" in the module docstring, which + # also states the listing's exception and its guard). carried = run_cache.get(CARRIED_RUN_SECTION, key, None) if is_json_object(carried): try: diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk.py index b08074a324..917a0981a1 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk.py @@ -303,6 +303,7 @@ def test_floor_paths_are_high_whatever_the_switch(self): (".github/scripts/pr-merge/who.mjs", "gate"), (".github/scripts/pr-merge/post-unarmed-status.mjs", "gate"), (".github/scripts/pr-merge/verify-answers.mjs", "gate"), + (".github/scripts/pr-merge/listing-carry.mjs", "gate"), ("plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py", "gate"), ("plugins/source-control/skills/babysit-prs/scripts/babysit_check_inputs.py", "enforcement"), ("plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py", "enforcement"), diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py index 477dcd6c1e..2306d6f06c 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py @@ -2695,6 +2695,10 @@ def test_a_slim_record_of_another_repository_is_not_origin(self) -> None: # Every test runs without the backoff sleep; `judge` reads the waits off it. _no_sleep = mock.patch.object(ve, "_listing_sleep") +# No test reads an incremental listing's page unless it patches this again. +_no_pages = mock.patch.object( + ve, "gh_api_with_headers", side_effect=AssertionError("an unexpected page read") +) # The tracker's human-gated label read an autonomous gate makes: stubbed so @@ -2704,11 +2708,13 @@ def test_a_slim_record_of_another_repository_is_not_origin(self) -> None: def setUpModule() -> None: _no_sleep.start() + _no_pages.start() _role_label.start() def tearDownModule() -> None: _role_label.stop() + _no_pages.stop() _no_sleep.stop() @@ -2889,6 +2895,325 @@ def send(args, *, timeout_seconds, env_overrides, binary): self.assertEqual(len(sent), self.PAGES) +class PagedGitHub(FakeGitHub): + """Also answers the incremental listing's single pages + (`?per_page=&page=

`) from `artifacts`, newest id first, each with + `total_count` (`live_total`, by default the artifacts' count, GitHub's + live count) and the response's `Date` header (`DATE`).""" + + def __init__(self, *args: Any, **kwargs: Any): + super().__init__(*args, **kwargs) + self.live_total: int | None = None + self.page_fail: str | None = None + + def __call__( + self, args: list[str], **kwargs: Any + ) -> subprocess.CompletedProcess[str]: + prefix = f"repos/{REPO}/actions/artifacts?per_page=" + path = args[1] if len(args) > 1 else "" + if path.startswith(prefix) and "&page=" in path: + self.calls.append(args) + if self.page_fail: + return subprocess.CompletedProcess(args, 1, "", self.page_fail) + size, page = (int(v) for v in path[len(prefix) :].split("&page=")) + ordered = sorted(self.artifacts, key=lambda a: -a["id"]) + body = { + "total_count": len(ordered) + if self.live_total is None + else self.live_total, + "artifacts": ordered[(page - 1) * size : page * size], + } + return subprocess.CompletedProcess(args, 0, json.dumps(body), "") + return super().__call__(args, **kwargs) + + def pages(self) -> int: + return sum("&page=" in " ".join(c) for c in self.calls) + + def full_listings(self) -> int: + return sum( + c[:2] == ["api", f"repos/{REPO}/actions/artifacts?per_page=100"] + for c in self.calls + ) + + +# The runner's clock while these tests list, and GitHub's `Date` a few seconds +# later. +LISTED_AT = datetime(2026, 10, 16, 12, 0, tzinfo=UTC) +DATE = "Fri, 16 Oct 2026 12:00:05 GMT" + + +def stamp(when: datetime) -> str: + return when.strftime("%Y-%m-%dT%H:%M:%SZ") + + +def dated( + name: str, + run_id: int, + artifact_id: int, + created: datetime, + *, + expires: datetime | None = None, + expired: bool = False, +) -> dict[str, Any]: + """A listed artifact as GitHub gives it: an id, created_at and expires_at.""" + return artifact(name, run_id, expired=expired) | { + "id": artifact_id, + "created_at": stamp(created), + "expires_at": stamp(expires or created + timedelta(days=90)), + } + + +class TheListingBuildsOnACarriedOne(unittest.TestCase): + """A listing a trusted pr-merge run saved (`BABYSIT_VERIFY_LISTING_CARRY`) + is a base: the reader pages only down past it, keeps the result only when + it holds the first page's live `total_count`, and lists every page on any + doubt.""" + + def setUp(self) -> None: + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + self.carry = pathlib.Path(tmp.name, "verify-listing-carry.json") + self.saved = pathlib.Path(tmp.name, "verify-listing.json") + for patcher in ( + mock.patch.dict( + "os.environ", + { + "BABYSIT_VERIFY_LISTING_CARRY": str(self.carry), + "BABYSIT_VERIFY_LISTING": str(self.saved), + }, + ), + mock.patch.object(ve, "_wall_clock", side_effect=lambda: LISTED_AT), + mock.patch.object(ve, "LISTING_PAGE_SIZE", 2), + ): + patcher.start() + self.addCleanup(patcher.stop) + # The base: a pass at the head (run 1) and three older artifacts, + # read 10 minutes ago. + self.base_at = LISTED_AT - timedelta(minutes=10) + old = self.base_at - timedelta(hours=3) + self.base = [ + dated(named(HEAD, "pass"), 1, 1000, self.base_at - timedelta(minutes=5)), + dated("pr-merge-gate-cursor", 3, 900, old), + dated("pr-merge-gate-cursor", 3, 800, old), + dated("pr-merge-gate-cursor", 3, 700, old), + ] + + def write_carry(self, artifacts: list[Any], at: datetime | None = None) -> None: + self.carry.write_text( + json.dumps( + {"fetchedAt": (at or self.base_at).isoformat(), "artifacts": artifacts} + ) + ) + + def fake( + self, *extra: dict[str, Any], base: list[Any] | None = None + ) -> PagedGitHub: + """GitHub now: the base's artifacts (or `base`) and `extra`.""" + artifacts = [*(self.base if base is None else base), *extra] + fake = PagedGitHub( + artifacts, {1: run(1), 2: run(2)}, {named(HEAD, "pass"): record()} + ) + return fake + + def judge(self, fake: PagedGitHub) -> tuple[ve.Reader, ve.Evidence]: + def page(args: list[str], **kwargs: Any) -> Any: + return fake(args, **kwargs), {"date": DATE} + + with ( + mock.patch.object(postmerge, "gh_capture", side_effect=fake), + mock.patch.object(ve, "gh_api_with_headers", side_effect=page), + ): + reader = ve.Reader(REPO, "main", now=NOW) + return reader, reader.evidence(PR, HEAD, pull()) + + def test_a_fail_uploaded_since_is_read_from_the_first_page_alone(self) -> None: + self.write_carry(self.base) + fake = self.fake(dated(named(HEAD, "fail"), 2, 1100, LISTED_AT)) + reader, evidence = self.judge(fake) + self.assertEqual(evidence.refusal, "verify:fail") + self.assertEqual(reader.listing_mode, "incremental") + # Page 1 holds 1100 and 1000 (created within the overlap), page 2 + # holds 900, created before it: two pages, no full listing. + self.assertEqual((fake.pages(), fake.full_listings()), (2, 0)) + saved = json.loads(self.saved.read_text()) + self.assertEqual(saved["fetchedAt"], LISTED_AT.isoformat()) + self.assertEqual( + [a["id"] for a in saved["artifacts"]], [1100, 1000, 900, 800, 700] + ) + + def test_the_overlap_reads_a_lower_id_created_after_the_base(self) -> None: + # 950 is below the base's highest id (1000) but was created after the + # base was read, so the base lacks it. The first page (1000, 990) + # already reaches the highest id; paging goes on to 900, created + # before the overlap, so the second page brings 950. + base = [ + self.base[0], + dated("pr-merge-gate-cursor", 3, 990, self.base_at - timedelta(minutes=30)), + *self.base[1:], + ] + self.write_carry(base) + fake = self.fake(dated(named(HEAD, "fail"), 2, 950, LISTED_AT), base=base) + reader, evidence = self.judge(fake) + self.assertEqual(evidence.refusal, "verify:fail") + self.assertEqual(reader.listing_mode, "incremental") + self.assertEqual((fake.pages(), fake.full_listings()), (2, 0)) + + def test_an_expired_carried_entry_is_dropped_by_githubs_date(self) -> None: + # GitHub no longer lists 800 (its expiry passed by the page's Date, + # 12:00:05, though not by the runner's clock, 12:00:00) nor 700 + # (marked expired); its live count leaves both out. + lapsed = dated( + "pr-merge-gate-cursor", + 3, + 800, + self.base_at, + expires=LISTED_AT + timedelta(seconds=2), + ) + gone = dated("pr-merge-gate-cursor", 3, 700, self.base_at, expired=True) + self.write_carry([*self.base[:2], lapsed, gone]) + fake = self.fake(base=self.base[:2]) + reader, evidence = self.judge(fake) + self.assertIsNone(evidence.refusal) + self.assertEqual(reader.listing_mode, "incremental") + self.assertEqual(fake.full_listings(), 0) + ids = [a["id"] for a in json.loads(self.saved.read_text())["artifacts"]] + self.assertEqual(ids, [1000, 900]) + + def test_a_carried_entry_the_pages_no_longer_hold_is_dropped(self) -> None: + # An overwrite deletes 1000's artifact and uploads 1050 in its place. + self.write_carry(self.base) + fake = self.fake( + dated(named(HEAD, "pass"), 1, 1050, LISTED_AT), base=self.base[1:] + ) + reader, evidence = self.judge(fake) + self.assertIsNone(evidence.refusal) + self.assertEqual(reader.listing_mode, "incremental") + self.assertEqual(fake.full_listings(), 0) + ids = [a["id"] for a in json.loads(self.saved.read_text())["artifacts"]] + self.assertEqual(ids, [1050, 900, 800, 700]) + + def test_a_count_other_than_githubs_lists_every_page(self) -> None: + # A forged carry drops an old FAIL (600): the live count holds it, so + # the merged listing is one short and every page is read. + fail = dated(named(EARLIER, "fail"), 2, 600, self.base_at - timedelta(days=2)) + self.write_carry(self.base) + fake = self.fake(fail) + reader, evidence = self.judge(fake) + self.assertEqual(evidence.refusal, "verify:earlier-fail") + self.assertEqual(reader.listing_mode, "full") + self.assertEqual((fake.pages(), fake.full_listings()), (1, 1)) + + def test_a_live_count_off_by_any_amount_lists_every_page(self) -> None: + for delta in (-1, 1): + with self.subTest(delta): + self.write_carry(self.base) + self.saved.unlink(missing_ok=True) + fake = self.fake() + fake.live_total = len(self.base) + delta + reader, evidence = self.judge(fake) + self.assertIsNone(evidence.refusal) + self.assertEqual(reader.listing_mode, "full") + self.assertEqual((fake.pages(), fake.full_listings()), (1, 1)) + + def test_a_carry_past_its_age_bound_lists_every_page(self) -> None: + # The bound is three hours (LISTING_CARRY_MAX_AGE's basis). + for age, mode in ( + (timedelta(hours=3), "incremental"), + (timedelta(hours=3, seconds=1), "full"), + ): + with self.subTest(age): + self.write_carry(self.base, LISTED_AT - age) + self.saved.unlink(missing_ok=True) + fake = self.fake() + reader, _ = self.judge(fake) + self.assertEqual(reader.listing_mode, mode) + self.assertEqual(fake.full_listings(), int(mode == "full")) + + def test_a_carry_that_does_not_parse_lists_every_page(self) -> None: + undated = dict(self.base[1]) + del undated["created_at"] + for text in ( + "not json", + json.dumps({"artifacts": self.base}), + json.dumps({"fetchedAt": "2026-10-16T11:50:00", "artifacts": self.base}), + json.dumps({"fetchedAt": self.base_at.isoformat(), "artifacts": {}}), + json.dumps({"fetchedAt": self.base_at.isoformat(), "artifacts": [undated]}), + json.dumps( + { + "fetchedAt": self.base_at.isoformat(), + "artifacts": [self.base[0] | {"id": True}], + } + ), + ): + with self.subTest(text[:60]): + self.carry.write_text(text) + self.saved.unlink(missing_ok=True) + fake = self.fake() + reader, _ = self.judge(fake) + self.assertEqual(reader.listing_mode, "full") + self.assertEqual((fake.pages(), fake.full_listings()), (0, 1)) + + def test_no_carry_lists_every_page(self) -> None: + fake = self.fake() + reader, _ = self.judge(fake) + self.assertEqual(reader.listing_mode, "full") + self.assertEqual((fake.pages(), fake.full_listings()), (0, 1)) + + def test_a_failed_page_read_lists_every_page(self) -> None: + for error in ("gh: Bad Gateway (HTTP 502)", "gh: Not Found (HTTP 404)"): + with self.subTest(error): + self.write_carry(self.base) + self.saved.unlink(missing_ok=True) + fake = self.fake(dated(named(HEAD, "fail"), 2, 1100, LISTED_AT)) + fake.page_fail = error + reader, evidence = self.judge(fake) + self.assertEqual(evidence.refusal, "verify:fail") + self.assertEqual(reader.listing_mode, "full") + self.assertEqual((fake.pages(), fake.full_listings()), (1, 1)) + + def test_a_page_with_no_date_or_an_odd_entry_lists_every_page(self) -> None: + self.write_carry(self.base) + fake = self.fake() + + def undated(args: list[str], **kwargs: Any) -> Any: + return fake(args, **kwargs), {} + + with ( + mock.patch.object(postmerge, "gh_capture", side_effect=fake), + mock.patch.object(ve, "gh_api_with_headers", side_effect=undated), + ): + reader = ve.Reader(REPO, "main", now=NOW) + reader.evidence(PR, HEAD, pull()) + self.assertEqual(reader.listing_mode, "full") + self.assertEqual((fake.pages(), fake.full_listings()), (1, 1)) + self.saved.unlink(missing_ok=True) + odd = self.fake(dated("x", 3, 1200, LISTED_AT) | {"expires_at": "soon"}) + reader, _ = self.judge(odd) + self.assertEqual(reader.listing_mode, "full") + self.assertEqual((odd.pages(), odd.full_listings()), (1, 1)) + + def test_a_refresh_builds_on_the_listing_it_drops(self) -> None: + # The gate's re-list before an allow (babysit_merge.py `refresh`) + # pages only past the run's own listing. + self.write_carry(self.base) + fake = self.fake() + page = lambda args, **kwargs: (fake(args, **kwargs), {"date": DATE}) # noqa: E731 + with ( + mock.patch.object(postmerge, "gh_capture", side_effect=fake), + mock.patch.object(ve, "gh_api_with_headers", side_effect=page), + ): + reader = ve.Reader(REPO, "main", now=NOW) + self.assertIsNone(reader.evidence(PR, HEAD, pull()).refusal) + reader = ve.Reader(REPO, "main", now=NOW) + self.assertIsNone(reader.evidence(PR, HEAD, pull()).refusal) + self.assertTrue(reader.saved) + fake.artifacts.append(dated(named(HEAD, "fail"), 2, 1100, LISTED_AT)) + reader.refresh() + self.assertEqual(reader.evidence(PR, HEAD, pull()).refusal, "verify:fail") + self.assertEqual(reader.listing_mode, "incremental") + self.assertEqual(fake.full_listings(), 0) + + # --- The --authority pr-merge path ------------------------------------------------ diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py index 127cdc8f71..0b7356c553 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py @@ -226,7 +226,7 @@ def test_the_floor_is_the_same_on_both_sides(self) -> None: self.assertEqual(answer["classes"], [list(row) for row in risk.FLOOR_CLASSES]) self.assertEqual(answer["floor"], list(risk.HIGH_FLOOR)) self.assertEqual(answer["review"], list(risk.REVIEW_FLOOR)) - self.assertEqual(len(risk.FLOOR_CLASSES), 51) + self.assertEqual(len(risk.FLOOR_CLASSES), 52) self.assertEqual(len(risk.REVIEW_FLOOR), 21) self.assertEqual( {kind for _, kind in risk.FLOOR_CLASSES}, {"trust", "gate", "enforcement"} diff --git a/scripts/ci-lanes-pr-hold.test.sh b/scripts/ci-lanes-pr-hold.test.sh index e9e5ec3bd4..2e8a45b180 100755 --- a/scripts/ci-lanes-pr-hold.test.sh +++ b/scripts/ci-lanes-pr-hold.test.sh @@ -226,6 +226,13 @@ import process from "node:process"; appendFileSync(process.env.STUB_LOG, `head-ref-map ${process.argv.slice(2).join(" ")}\n`); process.exitCode = process.env.STUB_MAP_FAILS ? 1 : 0; EOF +# The listing carry stub logs `listing-carry`; it always exits 0, as the real +# one does after a warning. +cat >"$TMP_ROOT/pr-merge/listing-carry.mjs" <<'EOF' +import { appendFileSync } from "node:fs"; +import process from "node:process"; +appendFileSync(process.env.STUB_LOG, "listing-carry\n"); +EOF SHA=0123456789abcdef0123456789abcdef01234567 BASE=89abcdef0123456789abcdef0123456789abcdef @@ -322,6 +329,7 @@ for line in 'VERIFY_EVIDENCE_SCRIPT: .base/plugins/source-control/skills/babysit 'PR_MERGE_SCRIPTS: .base/.github/scripts/pr-merge' \ 'ACTIONS_TOKEN: ${{ github.token }}' \ 'BABYSIT_VERIFY_LISTING: ${{ runner.temp }}/verify-listing.json' \ + 'BABYSIT_VERIFY_LISTING_CARRY: ${{ runner.temp }}/verify-listing-carry.json' \ 'BABYSIT_VERIFY_HEAD_REFS: ${{ runner.temp }}/head-refs-in.json' \ 'HEAD_REFS_PATH: ${{ runner.temp }}/head-refs-in.json' \ 'BABYSIT_RUN_CACHE: ${{ runner.temp }}/babysit-run-cache.json' \ @@ -344,8 +352,9 @@ else fi sparse="$(awk '/sparse-checkout: \|/ { on = 1; next } on && /^ \// { print substr($0, 13); next } on { exit }' <<<"$base_step")" want_sparse=(/.node-version '/plugins/source-control/skills/babysit-prs/scripts/*.py' /plugins/source-control/lib/parse-concern-value.sh /plugins/source-control/lib/yaml-subset.awk) -# Every module head-ref-map.mjs and carry-run-cache.mjs import, transitively. -queue=(.github/scripts/pr-merge/head-ref-map.mjs .github/scripts/pr-merge/carry-run-cache.mjs) +# Every module head-ref-map.mjs, carry-run-cache.mjs and listing-carry.mjs +# import, transitively. +queue=(.github/scripts/pr-merge/head-ref-map.mjs .github/scripts/pr-merge/carry-run-cache.mjs .github/scripts/pr-merge/listing-carry.mjs) while ((${#queue[@]})); do file=${queue[0]} queue=("${queue[@]:1}") From 8b6655d2d4a068b673dbb21a47db68a2866eccae Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 13:58:31 -0400 Subject: [PATCH 2/7] docs(source-control): state the listing probe results as measured Co-Authored-By: Claude Opus 5.5 --- .../perf-verify-incremental-listing-5b1e7c2a.md | 2 +- .../scripts/babysit_verify_evidence.py | 15 +++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/.changes/source-control/perf-verify-incremental-listing-5b1e7c2a.md b/.changes/source-control/perf-verify-incremental-listing-5b1e7c2a.md index 37f10c4d75..7fc8dd9e49 100644 --- a/.changes/source-control/perf-verify-incremental-listing-5b1e7c2a.md +++ b/.changes/source-control/perf-verify-incremental-listing-5b1e7c2a.md @@ -4,4 +4,4 @@ bump: patch ### Changed -- `babysit_verify_evidence.py` lists only the newest artifact pages on top of a whole listing begun within three hours: its own, the run's saved one, or the one a trusted pr-merge run carried (`BABYSIT_VERIFY_LISTING_CARRY`). It reads pages newest id first down past that listing's highest id until an artifact created two hours before that listing was read, drops entries expired by the first page's `Date` or missing from the pages it read, and keeps the result only when it holds exactly the first page's live `total_count`. Any other count, a missing, older or malformed base, or a failed read lists every page as before. On 2026-10-27 this read 2 pages in 1.3 s where the full listing read 69 pages in about 51 s, with the same artifacts. +- `babysit_verify_evidence.py` lists only the newest artifact pages on top of a whole listing begun within three hours: its own, the run's saved one, or the one a trusted pr-merge run carried (`BABYSIT_VERIFY_LISTING_CARRY`). It reads pages newest id first down past that listing's highest id until an artifact created two hours before that listing was read, drops entries expired by the first page's `Date` or missing from the pages it read, and keeps the result only when it holds exactly the first page's live `total_count`. Any other count, a missing, older or malformed base, or a failed read lists every page as before. On 2026-10-27 this read 2 pages in 1.3 to 1.6 s where the full listing read 69 pages in 50 to 58 s, and held every artifact the full listing held plus those the full listing skipped at its page boundaries. diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py index 646c326998..7d38da158d 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py @@ -102,12 +102,15 @@ the lowest id those pages hold, less those expired or past `expires_at` by the first page's `Date`. Such a listing could hide a FAIL by leaving it out, so it is kept only when it holds exactly the first page's `total_count`, GitHub's -live count of the repository's artifacts. A probe on 2026-10-27 saw that count -fall by exactly the five artifacts whose `expires_at` had passed, and an -incremental listing match a full one to the artifact; an artifact that expires -during the read can only make the counts differ. Any other count, a base that -is missing, older or of another shape, or any failed read lists every page as -before. A carry forged to drop a FAIL must therefore also hold a fabricated +live count of the repository's artifacts. Across 17 first-page reads on +2026-10-27 that count moved by exactly the uploads since the last read less +the artifacts whose `expires_at` had passed by the response's `Date`. Three +incremental listings that day each held exactly that count and every artifact +a full listing taken just after held, plus 0, 1 and 3 that the full listing +skipped at its page boundaries though they still existed. An artifact that +expires during the read can only make the counts differ. Any other count, a +base that is missing, older or of another shape, or any failed read lists +every page as before. A carry forged to drop a FAIL must therefore also hold a fabricated entry in its place to match the live count, and forging it already needs code running in a scheduled or hand-started run of main's pr-merge.yml, whose merge job holds the lanes App token. From fb1ca60e7757701f56dcc528defb0d3a49ec93c8 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 13:58:33 -0400 Subject: [PATCH 3/7] fix(source-control): never judge on a whole listing short of the live count A whole artifact listing can skip the artifact at a page boundary when an earlier one expires while it pages; on 2026-10-27 two of three whole listings skipped live artifacts, FAIL verdicts among them. Each whole listing is now deduplicated by id, added to the ones read before it and kept only when a top-up from the first page shows it holds the live total_count. After three whole listings that never hold it the pull request refuses verify:unreadable. Closes #10486 Co-Authored-By: Claude Opus 5.5 --- ...erf-verify-incremental-listing-5b1e7c2a.md | 4 + docs/conventions/pr-pipeline/README.md | 4 + .../scripts/babysit_verify_evidence.py | 89 ++++- .../tests/test_babysit_verify_evidence.py | 309 ++++++++++++------ 4 files changed, 293 insertions(+), 113 deletions(-) diff --git a/.changes/source-control/perf-verify-incremental-listing-5b1e7c2a.md b/.changes/source-control/perf-verify-incremental-listing-5b1e7c2a.md index 7fc8dd9e49..0ec9bc7b6a 100644 --- a/.changes/source-control/perf-verify-incremental-listing-5b1e7c2a.md +++ b/.changes/source-control/perf-verify-incremental-listing-5b1e7c2a.md @@ -5,3 +5,7 @@ bump: patch ### Changed - `babysit_verify_evidence.py` lists only the newest artifact pages on top of a whole listing begun within three hours: its own, the run's saved one, or the one a trusted pr-merge run carried (`BABYSIT_VERIFY_LISTING_CARRY`). It reads pages newest id first down past that listing's highest id until an artifact created two hours before that listing was read, drops entries expired by the first page's `Date` or missing from the pages it read, and keeps the result only when it holds exactly the first page's live `total_count`. Any other count, a missing, older or malformed base, or a failed read lists every page as before. On 2026-10-27 this read 2 pages in 1.3 to 1.6 s where the full listing read 69 pages in 50 to 58 s, and held every artifact the full listing held plus those the full listing skipped at its page boundaries. + +### Fixed + +- `babysit_verify_evidence.py` no longer judges on a whole artifact listing that skipped an artifact at a page boundary. A whole listing can skip one when an earlier artifact expires while it pages: on 2026-10-27 two of three whole listings skipped live artifacts, FAIL verdicts among them, so a pull request could be allowed past a FAIL. Each whole listing is now deduplicated by id, added to the ones read before it, and kept only when a top-up from the first page shows it holds GitHub's live `total_count`; after three whole listings that never hold it the pull request refuses `verify:unreadable`. diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index c340b08860..83c31e65ad 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -1879,6 +1879,10 @@ entries below the lowest id those pages hold, less those expired by the first pa kept only when it holds exactly that page's `total_count`. Any other count, a missing, older or malformed base, or any failed read lists every page, so a carried listing that leaves out a FAIL lists every page too (the rule and its basis: `babysit_verify_evidence.py`, "Carried reads"). +A whole listing is held to the same count: deduplicated by id, added to the whole listings read +before it and checked by a top-up from the first page, since one can skip an artifact at a page +boundary while an earlier one expires (#10486); after three whole listings that never hold the +live count the candidate refuses `verify:unreadable`. The lane token holds no Actions permission, so pr-merge.yml grants its job `actions: read` and passes the job's `GITHUB_TOKEN` as `BABYSIT_VERIFY_GH_TOKEN`; the reader uses it as `GH_TOKEN` for the artifact listing, the run records, the download of a verdict.json (by the listed artifact's id, one request where `gh run diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py index 7d38da158d..e1e22d91a5 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py @@ -87,7 +87,16 @@ judges again. Any read that fails refuses (`verify:unreadable`). The repository's artifacts are listed once per `Reader`, every artifact, and filtered on the name here: the API's name filter -matches whole names only, and a FAIL counts under any number and head. +matches whole names only, and a FAIL counts under any number and head. No +judgment uses a listing whose artifact ids, each counted once, are not +exactly GitHub's live `total_count` (`Reader._incremental`'s check). A whole +listing can skip the artifact at a page boundary when an earlier one expires +while it pages, and return one twice when an upload moves it down a page +(#10486). So a whole listing is deduplicated by id, added to the whole +listings read before it, and checked by a top-up read from the first page +(`Reader._complete_listing`); after `LISTING_READS` whole listings that never +hold the live count it refuses `verify:unreadable`. The refresh before an +allow reads the same way. Carried reads. What an earlier run read is used only where it counts a FAIL (a carried record counts the FAIL when it shows origin; any other answer from @@ -110,8 +119,8 @@ skipped at its page boundaries though they still existed. An artifact that expires during the read can only make the counts differ. Any other count, a base that is missing, older or of another shape, or any failed read lists -every page as before. A carry forged to drop a FAIL must therefore also hold a fabricated -entry in its place to match the live count, and forging it already needs code +every page, checked the same way. A carry forged to drop a FAIL must therefore +also hold a fabricated entry in its place to match the live count, and forging it already needs code running in a scheduled or hand-started run of main's pr-merge.yml, whose merge job holds the lanes App token. pr-merge.yml builds one `Reader` per candidate gate run and names a file in @@ -269,6 +278,13 @@ def verdict_name(name: str) -> re.Match[str] | None: # for the whole listing that day (judgment on that data). LISTING_CARRY_MAX_AGE = timedelta(hours=3) LISTING_PAGE_SIZE = 100 +# The whole listings `Reader._complete_listing` reads before it refuses. Of +# three whole listings read on 2026-10-27, two skipped live artifacts at page +# boundaries (1 and 3, a FAIL in each), and each skipped artifact was in every +# other whole listing: a second read restores a skip, a third is margin +# (judgment on that data). The reads and their top-ups share one +# `LISTING_TIMEOUT_SECONDS`, as one read did: what does not fit refuses. +LISTING_READS = 3 # How gh reports a request that never got an HTTP response: its own # "error connecting to " for a failed lookup, else Go's net/http error. NETWORK_ERRORS = ( @@ -812,6 +828,8 @@ class Reader: # How the last listing was read from the API: "full" or "incremental". listing_mode: str | None = None _listing_error: str | None = None + # When the whole listings of one `_complete_listing` must end. + _read_deadline: float | None = None _runs: dict[int, str] = field(default_factory=dict) _started: dict[int, Any] = field(default_factory=dict) _created: dict[int, Any] = field(default_factory=dict) @@ -877,9 +895,7 @@ def _current_listing(self) -> list[dict[str, Any]]: if listing is None: self.listing_mode = "full" try: - listing = flatten_paginated_items( - self._read_listing(), "repository artifacts", items_key="artifacts" - ) + listing = self._complete_listing() except (_Unreadable, RuntimeError) as exc: self._listing_error = str(exc) raise _Unreadable(self._listing_error) from exc @@ -948,7 +964,40 @@ def _incremental_base(self) -> tuple[datetime, list[dict[str, Any]]] | None: ] return max(bases, key=lambda base: base[0], default=None) - def _incremental(self) -> list[dict[str, Any]] | None: + def _complete_listing(self) -> list[dict[str, Any]]: + """Every repository artifact from whole listings: each read is added + to the ones before it by id, and the union is kept only when the + top-up `_incremental` reads on it holds GitHub's live `total_count`. + A whole listing can skip an artifact at a page boundary when an + earlier one expires while it pages, and the next read returns it. + Raises `_Unreadable` when `LISTING_READS` reads never hold the live + count, so no judgment allows on a listing that may lack a FAIL.""" + union: dict[int, dict[str, Any]] = {} + deadline: float | None = None + for _ in range(LISTING_READS): + read_at = _wall_clock() + payload = self._read_listing(deadline) + deadline = self._read_deadline + for item in flatten_paginated_items( + payload, "repository artifacts", items_key="artifacts" + ): + if is_json_object(item) and _is_artifact_id(item.get("id")): + union[item["id"]] = item + else: + raise _Unreadable("repository artifacts: an entry with no id") + listing = self._incremental((read_at, list(union.values())), deadline) + if listing is not None: + return listing + raise _Unreadable( + f"repository artifacts: {LISTING_READS} whole listings never held " + "GitHub's live artifact count" + ) + + def _incremental( + self, + base: tuple[datetime, list[dict[str, Any]]] | None = None, + deadline: float | None = None, + ) -> list[dict[str, Any]] | None: """Every repository artifact, as the base (`_incremental_base`) and the newest pages read now show them, or None for a full listing. @@ -962,14 +1011,19 @@ def _incremental(self) -> list[dict[str, Any]] | None: expired or past its `expires_at` by the first page's `Date`. The result must hold exactly the first page's `total_count` artifacts, GitHub's live count. Any other count, no base, an entry or page of - another shape, a missing `Date`, or any failed read gives None.""" - base = self._incremental_base() + another shape, a missing `Date`, or any failed read gives None. + `base` replaces `_incremental_base` and `deadline` its own + `LISTING_TIMEOUT_SECONDS` (`_complete_listing`'s top-up).""" + if base is not None and not all(_dated_entry(a) for a in base[1]): + return None + base = base or self._incremental_base() if base is None: return None base_at, carried = base high = max((a["id"] for a in carried), default=0) floor = base_at - LISTING_OVERLAP - deadline = _listing_clock() + LISTING_TIMEOUT_SECONDS + if deadline is None: + deadline = _listing_clock() + LISTING_TIMEOUT_SECONDS fresh: dict[int, dict[str, Any]] = {} total: Any = None server_now: datetime | None = None @@ -1039,19 +1093,26 @@ def _save(self, listing: list[dict[str, Any]], fetched_at: datetime) -> None: tmp.write_text(json.dumps(payload), encoding="utf-8") os.replace(tmp, path) - def _read_listing(self) -> Any: + def _read_listing(self, deadline: float | None = None) -> Any: """The listing payload, read again after a 5xx or a fast network error while time is left: every page, read and wait fits in `LISTING_TIMEOUT_SECONDS`, so a retry adds no time to the run's worst - case.""" + case. A `deadline` set by an earlier read is kept; this read's is + saved as `_read_deadline`.""" args = [ "api", f"repos/{self.repo}/actions/artifacts?per_page=100", "--paginate", "--slurp", ] - deadline = _listing_clock() + LISTING_TIMEOUT_SECONDS - timeout = LISTING_TIMEOUT_SECONDS + if deadline is None: + deadline = _listing_clock() + LISTING_TIMEOUT_SECONDS + timeout = LISTING_TIMEOUT_SECONDS + else: + timeout = deadline - _listing_clock() + if timeout <= 0: + raise _Unreadable("repository artifacts: no time left to list") + self._read_deadline = deadline for wait in LISTING_WAITS: try: return self._actions(args, "repository artifacts", timeout) diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py index 2306d6f06c..16c8c306bb 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py @@ -111,6 +111,8 @@ def artifact(name: str, run_id: int, *, expired: bool = False) -> dict[str, Any] "id": run_id * 10, "name": name, "expired": expired, + "created_at": "2026-10-01T00:00:00Z", + "expires_at": "2027-01-01T00:00:00Z", "workflow_run": { "id": run_id, "head_branch": "main", @@ -246,6 +248,31 @@ def __init__( # Each issue's REST timeline events, and each login's permission. self.timeline: dict[int, list[dict[str, Any]]] = {} self.permissions: dict[str, str] = {} + # The single pages the reader checks a listing against: the live + # `total_count` they report (None: the artifacts' count), and a gh + # error for each. + self.live_total: int | None = None + self.page_fail: str | None = None + # Ids each whole listing leaves out in turn, as one that pages while + # an earlier artifact expires skips the one at a page boundary. + self.full_skips: list[set[int]] = [] + + def listed(self) -> list[dict[str, Any]]: + """The artifacts a whole listing returns now, newest id first as + GitHub orders them.""" + skip = self.full_skips.pop(0) if self.full_skips else set() + ordered = sorted(self.artifacts, key=lambda a: -a.get("id", 0)) + return [a for a in ordered if a.get("id") not in skip] + + def pages(self) -> int: + """Single-page reads (the incremental listing and its top-ups).""" + return sum("&page=" in " ".join(c) for c in self.calls) + + def full_listings(self) -> int: + return sum( + c[:2] == ["api", f"repos/{REPO}/actions/artifacts?per_page=100"] + for c in self.calls + ) def __call__( self, args: list[str], **kwargs: Any @@ -380,10 +407,23 @@ def rest( args, 1, b"", "gh: Not Found (HTTP 404)" ) return subprocess.CompletedProcess(args, 0, zipped(self.records[name]), "") + prefix = f"repos/{REPO}/actions/artifacts?per_page=" + if path.startswith(prefix) and "&page=" in path: + if self.page_fail: + return subprocess.CompletedProcess(args, 1, "", self.page_fail) + size, page = (int(v) for v in path[len(prefix) :].split("&page=")) + ordered = sorted(self.artifacts, key=lambda a: -a.get("id", 0)) + return ok( + { + "total_count": len({a.get("id") for a in self.artifacts}) + if self.live_total is None + else self.live_total, + "artifacts": ordered[(page - 1) * size : page * size], + } + ) if path == f"repos/{REPO}/actions/artifacts?per_page=100": - pages = [ - self.artifacts[i : i + 2] for i in range(0, len(self.artifacts), 2) - ] + listed = self.listed() + pages = [listed[i : i + 2] for i in range(0, len(listed), 2)] return ok( [{"total_count": len(self.artifacts), "artifacts": p} for p in pages] or [{"total_count": 0, "artifacts": []}] @@ -440,7 +480,12 @@ def passing(*extra: dict[str, Any], runs: dict[int, Any] | None = None) -> FakeG def read(fake: FakeGitHub, pr: dict[str, Any] | None = None) -> ve.Evidence: - with mock.patch.object(postmerge, "gh_capture", side_effect=fake): + # The single-page patch too, for modules that import this helper without + # this module's setUpModule. + with ( + mock.patch.object(postmerge, "gh_capture", side_effect=fake), + mock.patch.object(ve, "gh_api_with_headers", side_effect=_page_with_date), + ): return ve.Reader(REPO, "main", now=NOW).evidence(PR, HEAD, pr or pull()) @@ -2207,7 +2252,9 @@ def test_a_failed_listing_refuses_every_entry_and_is_not_read_per_entry( def listings(fake: FakeGitHub) -> int: - return sum(any("actions/artifacts?" in a for a in c) for c in fake.calls) + """Listings read from the API: each one, whole or built on a base, + reads the first single page once when it succeeds.""" + return sum(any(a.endswith("&page=1") for a in c) for c in fake.calls) class TheListingIsReadOncePerRun(unittest.TestCase): @@ -2534,7 +2581,8 @@ def test_a_pass_run_is_compared_and_not_counted_off_the_branch(self) -> None: # the carried yes, and the PASS must not reuse it. other = named(UNRELATED, "fail", 99) fake = FakeGitHub( - [artifact(other, 1), artifact(named(HEAD, "pass"), 1)], + # Two artifacts of one run, so two ids. + [artifact(other, 1), artifact(named(HEAD, "pass"), 1) | {"id": 11}], {1: run(1)}, {named(HEAD, "pass"): record(), other: record(99, UNRELATED, head_ref="o")}, ) @@ -2695,10 +2743,16 @@ def test_a_slim_record_of_another_repository_is_not_origin(self) -> None: # Every test runs without the backoff sleep; `judge` reads the waits off it. _no_sleep = mock.patch.object(ve, "_listing_sleep") -# No test reads an incremental listing's page unless it patches this again. -_no_pages = mock.patch.object( - ve, "gh_api_with_headers", side_effect=AssertionError("an unexpected page read") -) + + +# Single pages go to whatever fake `postmerge.gh_capture` is patched to, with +# GitHub's `Date` header (`DATE`); a test that needs other headers patches +# this again. +def _page_with_date(args: list[str], **kwargs: Any) -> Any: + return postmerge.gh_capture(args, **kwargs), {"date": DATE} + + +_no_pages = mock.patch.object(ve, "gh_api_with_headers", side_effect=_page_with_date) # The tracker's human-gated label read an autonomous gate makes: stubbed so @@ -2742,7 +2796,7 @@ def __init__(self, errors: list[str | BaseException]): def __call__( self, args: list[str], **kwargs: Any ) -> subprocess.CompletedProcess[str]: - if any("actions/artifacts" in a for a in args): + if "--paginate" in args and any("actions/artifacts" in a for a in args): self.timeouts.append(kwargs.get("timeout_seconds")) if self.errors: self.calls.append(args) @@ -2753,7 +2807,11 @@ def __call__( return super().__call__(args, **kwargs) def listings(self) -> int: - return sum(any("actions/artifacts?" in a for a in c) for c in self.calls) + """Whole listings tried.""" + return sum( + "--paginate" in c and any("actions/artifacts?" in a for a in c) + for c in self.calls + ) class ATransientListingFailureIsReadAgain(unittest.TestCase): @@ -2895,47 +2953,6 @@ def send(args, *, timeout_seconds, env_overrides, binary): self.assertEqual(len(sent), self.PAGES) -class PagedGitHub(FakeGitHub): - """Also answers the incremental listing's single pages - (`?per_page=&page=

`) from `artifacts`, newest id first, each with - `total_count` (`live_total`, by default the artifacts' count, GitHub's - live count) and the response's `Date` header (`DATE`).""" - - def __init__(self, *args: Any, **kwargs: Any): - super().__init__(*args, **kwargs) - self.live_total: int | None = None - self.page_fail: str | None = None - - def __call__( - self, args: list[str], **kwargs: Any - ) -> subprocess.CompletedProcess[str]: - prefix = f"repos/{REPO}/actions/artifacts?per_page=" - path = args[1] if len(args) > 1 else "" - if path.startswith(prefix) and "&page=" in path: - self.calls.append(args) - if self.page_fail: - return subprocess.CompletedProcess(args, 1, "", self.page_fail) - size, page = (int(v) for v in path[len(prefix) :].split("&page=")) - ordered = sorted(self.artifacts, key=lambda a: -a["id"]) - body = { - "total_count": len(ordered) - if self.live_total is None - else self.live_total, - "artifacts": ordered[(page - 1) * size : page * size], - } - return subprocess.CompletedProcess(args, 0, json.dumps(body), "") - return super().__call__(args, **kwargs) - - def pages(self) -> int: - return sum("&page=" in " ".join(c) for c in self.calls) - - def full_listings(self) -> int: - return sum( - c[:2] == ["api", f"repos/{REPO}/actions/artifacts?per_page=100"] - for c in self.calls - ) - - # The runner's clock while these tests list, and GitHub's `Date` a few seconds # later. LISTED_AT = datetime(2026, 10, 16, 12, 0, tzinfo=UTC) @@ -3005,17 +3022,15 @@ def write_carry(self, artifacts: list[Any], at: datetime | None = None) -> None: ) ) - def fake( - self, *extra: dict[str, Any], base: list[Any] | None = None - ) -> PagedGitHub: + def fake(self, *extra: dict[str, Any], base: list[Any] | None = None) -> FakeGitHub: """GitHub now: the base's artifacts (or `base`) and `extra`.""" artifacts = [*(self.base if base is None else base), *extra] - fake = PagedGitHub( + fake = FakeGitHub( artifacts, {1: run(1), 2: run(2)}, {named(HEAD, "pass"): record()} ) return fake - def judge(self, fake: PagedGitHub) -> tuple[ve.Reader, ve.Evidence]: + def judge(self, fake: FakeGitHub) -> tuple[ve.Reader, ve.Evidence]: def page(args: list[str], **kwargs: Any) -> Any: return fake(args, **kwargs), {"date": DATE} @@ -3101,9 +3116,10 @@ def test_a_count_other_than_githubs_lists_every_page(self) -> None: reader, evidence = self.judge(fake) self.assertEqual(evidence.refusal, "verify:earlier-fail") self.assertEqual(reader.listing_mode, "full") - self.assertEqual((fake.pages(), fake.full_listings()), (1, 1)) + # The incremental first page, then the whole listing's own check. + self.assertEqual((fake.pages(), fake.full_listings()), (2, 1)) - def test_a_live_count_off_by_any_amount_lists_every_page(self) -> None: + def test_a_live_count_no_listing_can_hold_refuses(self) -> None: for delta in (-1, 1): with self.subTest(delta): self.write_carry(self.base) @@ -3111,9 +3127,11 @@ def test_a_live_count_off_by_any_amount_lists_every_page(self) -> None: fake = self.fake() fake.live_total = len(self.base) + delta reader, evidence = self.judge(fake) - self.assertIsNone(evidence.refusal) + self.assertEqual(evidence.refusal, "verify:unreadable") + self.assertIn("live artifact count", evidence.detail or "") self.assertEqual(reader.listing_mode, "full") - self.assertEqual((fake.pages(), fake.full_listings()), (1, 1)) + self.assertEqual(fake.full_listings(), ve.LISTING_READS) + self.assertFalse(self.saved.exists()) def test_a_carry_past_its_age_bound_lists_every_page(self) -> None: # The bound is three hours (LISTING_CARRY_MAX_AGE's basis). @@ -3151,13 +3169,14 @@ def test_a_carry_that_does_not_parse_lists_every_page(self) -> None: fake = self.fake() reader, _ = self.judge(fake) self.assertEqual(reader.listing_mode, "full") - self.assertEqual((fake.pages(), fake.full_listings()), (0, 1)) + # No incremental page: the whole listing and its own check. + self.assertEqual((fake.pages(), fake.full_listings()), (1, 1)) def test_no_carry_lists_every_page(self) -> None: fake = self.fake() reader, _ = self.judge(fake) self.assertEqual(reader.listing_mode, "full") - self.assertEqual((fake.pages(), fake.full_listings()), (0, 1)) + self.assertEqual((fake.pages(), fake.full_listings()), (1, 1)) def test_a_failed_page_read_lists_every_page(self) -> None: for error in ("gh: Bad Gateway (HTTP 502)", "gh: Not Found (HTTP 404)"): @@ -3167,9 +3186,11 @@ def test_a_failed_page_read_lists_every_page(self) -> None: fake = self.fake(dated(named(HEAD, "fail"), 2, 1100, LISTED_AT)) fake.page_fail = error reader, evidence = self.judge(fake) - self.assertEqual(evidence.refusal, "verify:fail") self.assertEqual(reader.listing_mode, "full") - self.assertEqual((fake.pages(), fake.full_listings()), (1, 1)) + # The whole listing is read, but no page can check it: it + # refuses rather than judge on it. + self.assertEqual(evidence.refusal, "verify:unreadable") + self.assertEqual(fake.full_listings(), ve.LISTING_READS) def test_a_page_with_no_date_or_an_odd_entry_lists_every_page(self) -> None: self.write_carry(self.base) @@ -3183,14 +3204,15 @@ def undated(args: list[str], **kwargs: Any) -> Any: mock.patch.object(ve, "gh_api_with_headers", side_effect=undated), ): reader = ve.Reader(REPO, "main", now=NOW) - reader.evidence(PR, HEAD, pull()) + evidence = reader.evidence(PR, HEAD, pull()) self.assertEqual(reader.listing_mode, "full") - self.assertEqual((fake.pages(), fake.full_listings()), (1, 1)) + self.assertEqual(evidence.refusal, "verify:unreadable") + self.assertEqual(fake.full_listings(), ve.LISTING_READS) self.saved.unlink(missing_ok=True) odd = self.fake(dated("x", 3, 1200, LISTED_AT) | {"expires_at": "soon"}) - reader, _ = self.judge(odd) + reader, evidence = self.judge(odd) self.assertEqual(reader.listing_mode, "full") - self.assertEqual((odd.pages(), odd.full_listings()), (1, 1)) + self.assertEqual(evidence.refusal, "verify:unreadable") def test_a_refresh_builds_on_the_listing_it_drops(self) -> None: # The gate's re-list before an allow (babysit_merge.py `refresh`) @@ -3214,6 +3236,95 @@ def test_a_refresh_builds_on_the_listing_it_drops(self) -> None: self.assertEqual(fake.full_listings(), 0) +class AWholeListingMustHoldTheLiveCount(unittest.TestCase): + """A whole listing skips the artifact at a page boundary when an earlier + one expires while it pages (#10486: on 2026-10-27 two of three whole + listings skipped live artifacts, FAILs among them). It is kept only when, + deduplicated by id and checked by a top-up from the first page, it holds + GitHub's live `total_count`; the next read restores a skip, and after + `LISTING_READS` reads that never hold it the gate refuses.""" + + def setUp(self) -> None: + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + self.saved = pathlib.Path(tmp.name, "verify-listing.json") + for patcher in ( + mock.patch.dict("os.environ", {"BABYSIT_VERIFY_LISTING": str(self.saved)}), + # One artifact a page, so the top-up's first page holds only the + # newest id and the FAIL sits below it. + mock.patch.object(ve, "LISTING_PAGE_SIZE", 1), + ): + patcher.start() + self.addCleanup(patcher.stop) + + @staticmethod + def fake() -> FakeGitHub: + """A pass at the head (run 5, id 50), and an earlier head's FAIL of + this pull request (run 2, id 20) below it.""" + return FakeGitHub( + [artifact(named(HEAD, "pass"), 5), artifact(named(EARLIER, "fail"), 2)], + {5: run(5), 2: run(2)}, + {named(HEAD, "pass"): record()}, + ) + + def test_a_fail_the_whole_listing_skips_is_restored_by_the_next_read(self) -> None: + fake = self.fake() + fake.full_skips = [{20}] + evidence = read(fake) + # On the first listing alone the pass would allow. + self.assertEqual(evidence.refusal, "verify:earlier-fail") + self.assertEqual(listings(fake), 2) + self.assertEqual( + sorted(a["id"] for a in json.loads(self.saved.read_text())["artifacts"]), + [20, 50], + ) + + def test_a_listing_that_never_holds_the_live_count_refuses(self) -> None: + fake = self.fake() + fake.full_skips = [{20}] * ve.LISTING_READS + evidence = read(fake) + self.assertEqual(evidence.refusal, "verify:unreadable") + self.assertIn("live artifact count", evidence.detail or "") + self.assertEqual(fake.full_listings(), ve.LISTING_READS) + self.assertFalse(self.saved.exists()) + + def test_a_duplicate_at_a_page_boundary_is_counted_once(self) -> None: + # An upload while it pages moves an artifact down a page, so the + # whole listing returns it twice. + fake = self.fake() + listed = fake.listed + + def doubled() -> list[dict[str, Any]]: + items = listed() + return [*items[:1], *items] + + fake.listed = doubled # type: ignore[method-assign] + evidence = read(fake) + self.assertEqual(evidence.refusal, "verify:earlier-fail") + self.assertEqual(fake.full_listings(), 1) + ids = [a["id"] for a in json.loads(self.saved.read_text())["artifacts"]] + self.assertEqual(sorted(ids), [20, 50]) + + def test_the_refresh_before_an_allow_holds_the_live_count_too(self) -> None: + # babysit_merge.py calls `refresh()` before an allow: with no listing + # to build on, its whole listing is checked the same way. + fake = self.fake() + del fake.artifacts[1] + with mock.patch.object(postmerge, "gh_capture", side_effect=fake): + reader = ve.Reader(REPO, "main", now=NOW) + self.assertIsNone(reader.evidence(PR, HEAD, pull()).refusal) + self.saved.unlink() + reader._base = None + fake.artifacts.append(artifact(named(EARLIER, "fail"), 2)) + fake.runs[2] = run(2) + fake.full_skips = [{20}] * ve.LISTING_READS + reader._artifacts, reader._saved = None, True + reader.refresh() + self.assertEqual( + reader.evidence(PR, HEAD, pull()).refusal, "verify:unreadable" + ) + + # --- The --authority pr-merge path ------------------------------------------------ @@ -3982,37 +4093,35 @@ def heads(tie_created: str = OLDER_START, ids: bool = True) -> FakeGitHub: return fake def test_a_later_pass_of_another_change_clears_both_fails(self) -> None: - for ids, requests in ((True, 3), (False, 6)): - with self.subTest(ids=ids): - fake = self.heads(ids=ids) - evidence = read(fake) - self.assertIsNone(evidence.refusal) - self.assertEqual( - evidence.report()["earlierFails"], sorted([EARLIER, TIE]) - ) - # The pass and the two untied FAILs' records. - self.assertEqual(download_requests(fake), requests) + fake = self.heads() + evidence = read(fake) + self.assertIsNone(evidence.refusal) + self.assertEqual(evidence.report()["earlierFails"], sorted([EARLIER, TIE])) + # The pass and the two untied FAILs' records. + self.assertEqual(download_requests(fake), 3) def test_a_tied_fail_after_the_pass_holds(self) -> None: - for ids, requests in ((True, 2), (False, 4)): - with self.subTest(ids=ids): - fake = self.heads("2026-10-15T00:00:01Z", ids=ids) - self.assertEqual(read(fake).refusal, "verify:earlier-fail") - # Only the untied FAILs' records: the pass decides nothing. - self.assertEqual(download_requests(fake), requests) + fake = self.heads("2026-10-15T00:00:01Z") + self.assertEqual(read(fake).refusal, "verify:earlier-fail") + # Only the untied FAILs' records: the pass decides nothing. + self.assertEqual(download_requests(fake), 2) def test_a_third_failed_head_goes_to_a_human(self) -> None: - for ids in (True, False): - with self.subTest(ids=ids): - fake = self.heads(ids=ids) - item = artifact(named(IN_PR, "fail", 96), 5) - if not ids: - del item["id"] - fake.artifacts.append(item) - fake.runs[5] = run(5) - evidence = read(fake) - self.assertEqual(evidence.refusal, "verify:repeated-fail") - self.assertTrue(evidence.needs_human) + fake = self.heads() + fake.artifacts.append(artifact(named(IN_PR, "fail", 96), 5)) + fake.runs[5] = run(5) + evidence = read(fake) + self.assertEqual(evidence.refusal, "verify:repeated-fail") + self.assertTrue(evidence.needs_human) + + def test_a_listing_with_an_artifact_of_no_id_refuses(self) -> None: + # An entry with no id cannot be counted against the live total, so + # the listing is never whole and no pass allows on it. + fake = self.heads(ids=False) + evidence = read(fake) + self.assertEqual(evidence.refusal, "verify:unreadable") + self.assertIn("no id", evidence.detail or "") + self.assertEqual(download_requests(fake), 0) def test_an_unreadable_untied_record_refuses(self) -> None: for ids in (True, False): @@ -4227,14 +4336,16 @@ def test_a_later_candidate_refuses_without_downloading_again(self) -> None: self.assertIsNone(read(fake).refusal) self.assertEqual(OneRunSharesItsFailRunReads.downloads(fake, 3), 2) - def test_an_artifact_without_an_id_is_read_again(self) -> None: + def test_an_artifact_without_an_id_refuses_every_read(self) -> None: + # A listing entry with no id cannot be counted against the live + # total, so the listing never reads whole, record or not. fake = self.foreign() del fake.artifacts[1]["id"] self.assertEqual(read(fake).refusal, "verify:unreadable") fake.records[named(UNRELATED, "fail", 98)] = record( 98, UNRELATED, verdict="fail", head_ref="feat/other" ) - self.assertIsNone(read(fake).refusal) + self.assertEqual(read(fake).refusal, "verify:unreadable") # --- The head-ref digest in a verdict's name ------------------------------------ From 4e8695c887b3ba05c8287cb1dacdf8195c5d26fd Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 14:41:19 -0400 Subject: [PATCH 4/7] test(pr-merge): stub the reader's listing at its whole-listing seam The listing-count test stubbed _read_listing, so the new completeness top-up read a real first page. It now stubs _complete_listing, with no base to build on. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/pr-merge/enable-auto-merge.test.mjs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/scripts/pr-merge/enable-auto-merge.test.mjs b/.github/scripts/pr-merge/enable-auto-merge.test.mjs index b92f7b9393..dd0a1934a7 100644 --- a/.github/scripts/pr-merge/enable-auto-merge.test.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.test.mjs @@ -1842,8 +1842,9 @@ with mock.patch.object(policy, "gh_capture", gh), mock.patch.object(postmerge, " }); // The reader's own listing rules (babysit_verify_evidence.py main and -// Reader._current_listing), its judgment stubbed to a pass that reads the -// listing: every candidate is allowed, so a run that lists again before an +// Reader._current_listing), each listing from the API stubbed to one call +// (`_complete_listing`, with no base to build on) and its judgment to a pass +// that reads the listing: every candidate is allowed, so a run that lists again before an // allow on the saved listing would page once per candidate. test("a reader run pages the artifact listing once for all its candidates", async () => { const dir = mkdtempSync(join(tmpdir(), "enable-auto-merge-listing-")); @@ -1870,7 +1871,7 @@ def gh(args, **_): def listing(self): with open(os.environ["LISTINGS"], "a") as log: log.write("listing\\n") - return [{"total_count": 0, "artifacts": []}] + return [] def passes(self, number, head, pr=None): self._listing() @@ -1878,7 +1879,7 @@ def passes(self, number, head, pr=None): argv = sys.argv[1:] pr = lambda repo, number: {"number": number, "head": {"sha": argv[-1]}} -with mock.patch.object(policy, "gh_capture", gh), mock.patch.object(postmerge, "gh_capture", gh), mock.patch.object(ve.Reader, "_read_listing", listing), mock.patch.object(ve.Reader, "_evidence", passes), mock.patch.object(policy, "pull_request", pr), mock.patch.object(ve, "lane_risk_refusal", lambda *args, **kwargs: None), mock.patch.object(policy, "read_config_at", lambda repo, sha: policy.PipelineConfig(sha, "C2", "pr-merge", "agent[bot]", None, (), "c2-mechanical")), mock.patch.object(policy, "with_default_branch", lambda repo, base, *rest: base), mock.patch.object(babysit_merge, "read_merge_queue", lambda repo, number: {"readError": None, "failedEjections": 0}): +with mock.patch.object(policy, "gh_capture", gh), mock.patch.object(postmerge, "gh_capture", gh), mock.patch.object(ve.Reader, "_incremental", lambda self, *args: None), mock.patch.object(ve.Reader, "_complete_listing", listing), mock.patch.object(ve.Reader, "_evidence", passes), mock.patch.object(policy, "pull_request", pr), mock.patch.object(ve, "lane_risk_refusal", lambda *args, **kwargs: None), mock.patch.object(policy, "read_config_at", lambda repo, sha: policy.PipelineConfig(sha, "C2", "pr-merge", "agent[bot]", None, (), "c2-mechanical")), mock.patch.object(policy, "with_default_branch", lambda repo, base, *rest: base), mock.patch.object(babysit_merge, "read_merge_queue", lambda repo, number: {"readError": None, "failedEjections": 0}): sys.exit(ve.main(argv)) `, ); From b08200f7584c86b46fa258b4a6b19dcae4b821c5 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 15:34:38 -0400 Subject: [PATCH 5/7] fix(source-control): size the cold-read headroom to the listing's caps The 900-request headroom per queue entry came from one 69-page whole listing. A cold reader now reads up to LISTING_READS (3) whole listings, each up to 1 + len(LISTING_WAITS) (3) attempts, with a top-up after each, after an incremental try on a carried listing. Its worst case is 793 per-FAIL reads + 13 + 3 x (3 x 77 + 7) = 1,520 requests, so ci-status reserves 1,520 per entry, and a test pins that figure to the caps. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/pr-require-checks.yml | 40 ++++++++++++++----- docs/conventions/pr-pipeline/README.md | 12 ++++-- .../tests/test_babysit_verify_evidence.py | 22 ++++++++++ scripts/ci-lanes-pr-hold.test.sh | 16 ++++---- 4 files changed, 67 insertions(+), 23 deletions(-) diff --git a/.github/workflows/pr-require-checks.yml b/.github/workflows/pr-require-checks.yml index 9cb50a2346..90a5e4b93a 100644 --- a/.github/workflows/pr-require-checks.yml +++ b/.github/workflows/pr-require-checks.yml @@ -3235,8 +3235,9 @@ jobs: # 15 s of retry sleeps for each of the hold-lift step's two reads, the # review-lane step's six and the lanes-hold step's eight (in the merge # queue the verdict reader's reads more, about 73 with pr-merge's carried - # reads and about 860 without them, measured 2026-10-25; its wall time is - # unmeasured, and a cold one can reach the cap, which fails red), about four + # reads and about 860 without them, measured 2026-10-25, up to 1,520 under + # the listing's caps; its wall time is unmeasured, and a cold one can reach + # the cap, which fails red), about four # minutes, plus the cost-hold step's reads: one for most pull requests, # which stop at the label events; for a lane-labeled one whose body cites a # decision, about eight (nine on a Refs issue), and two more per review @@ -3244,7 +3245,9 @@ jobs: # (seconds; its retries replace the contents reads' 15 s each). # 20: a cold verdict reader in the merge queue takes about 6 to 11 # minutes (0.4 to 0.7 s a call, as pr-merge's reader logs, for about 860 - # calls), on top of the job's other reads. + # calls), and at most about 15 under the listing's caps (793 per-FAIL + # reads at 0.7 s, plus the whole listings' shared 300 s + # LISTING_TIMEOUT_SECONDS), on top of the job's other reads. timeout-minutes: 20 env: # gh_read's wait budget: a rate-limit wait must end by @@ -4780,7 +4783,9 @@ jobs: # so the lanes-hold step's reader reads only what landed since: about 73 # requests for a lanes App pull request where a cold reader makes about # 860 (one run read and compare per FAIL and a download per untied - # FAIL), measured 2026-10-25 on 6,620 artifacts with 294 FAILs. They + # FAIL), measured 2026-10-25 on 6,620 artifacts with 294 FAILs, and at + # most 1,520 under the listing's caps (the cold-read headroom check + # below states the derivation). They # come from that run's pr-merge-run-cache artifact, found as pr-merge.yml # finds it (carry-run-cache.mjs `read`): only that run's steps could # upload it. This job holds write scopes and is the required check, so it @@ -5056,10 +5061,23 @@ jobs: node "$PR_MERGE_SCRIPTS/listing-carry.mjs" || echo "::notice::no artifact listing from pr-merge's last run; the verdict reader lists every page." # A cold reader runs only with the headroom for it. With no carried # run cache, or one whose carried sections (carry-run-cache.mjs - # carry) are all empty, the reader reads every FAIL: about 860 - # GITHUB_TOKEN requests, measured 2026-10-25, so a cold read costs - # 900. A missing cache is shared, so every entry the queue builds at - # once goes cold together: the cost counts max_entries_to_build of + # carry) are all empty, the reader reads every FAIL: 860 + # GITHUB_TOKEN requests measured 2026-10-25, 67 of them the one + # whole listing of 6,620 artifacts, so 793 per-FAIL reads. Its + # listing is capped by babysit_verify_evidence.py: an incremental + # try on a carried listing (at most 13 pages, LISTING_CARRY_MAX_AGE), + # then LISTING_READS (3) whole listings, each up to + # 1 + len(LISTING_WAITS) (3) attempts that page from the first, + # and a top-up after each (at most 7 pages: LISTING_OVERLAP's 2 h + # at the 255 uploads an hour seen 2026-10-27, plus one). At 77 + # pages a whole listing (76 at the 7,522-artifact peak of + # 2026-10-27, plus one) a cold read costs at most + # 793 + 13 + 3 x (3 x 77 + 7) = 1,520; test_babysit_verify_evidence + # pins this figure to those caps. It covers this path only: an + # entry with a carried run cache but no carried listing pays the + # listing's part (up to 727) unguarded. A missing cache is shared, + # so every entry the queue builds at once goes cold together: the + # cost counts max_entries_to_build of # the base branch's merge_queue rule (one read of # repos//rules/branches/, the largest any ruleset sets). # It runs when the core pool's remaining count, read once from @@ -5088,12 +5106,12 @@ jobs: if ! remaining=$(jq -er '.resources.core.remaining | numbers | floor' <<<"$rate" 2>/dev/null) || ! limit=$(jq -er '.resources.core.limit | numbers | floor' <<<"$rate" 2>/dev/null) || ! need=$(awk -v floor="$API_BUDGET_READ_FLOOR" -v cap="$API_BUDGET_LIMIT_CAP" -v limit="$limit" -v entries="$entries" \ - 'BEGIN { if (floor !~ /^[0-9.]+$/ || cap !~ /^[0-9]+$/) exit 1; if (limit < cap) cap = limit; printf "%d\n", 900 * entries + int(floor * cap + 0.999999) }'); then - echo "::error::$cold, and GITHUB_TOKEN's remaining requests could not be read, so a cold read (about 860 requests) is not started. Re-queue it." + 'BEGIN { if (floor !~ /^[0-9.]+$/ || cap !~ /^[0-9]+$/) exit 1; if (limit < cap) cap = limit; printf "%d\n", 1520 * entries + int(floor * cap + 0.999999) }'); then + echo "::error::$cold, and GITHUB_TOKEN's remaining requests could not be read, so a cold read (up to 1,520 requests) is not started. Re-queue it." exit 1 fi if [ "$remaining" -lt "$need" ]; then - echo "::error::$cold, and GITHUB_TOKEN has $remaining requests left, under the $need a cold read needs (900 for each of the $entries entries the queue builds, plus the read floor). Re-queue it after the reset or after pr-merge's next scheduled run uploads its cache." + echo "::error::$cold, and GITHUB_TOKEN has $remaining requests left, under the $need a cold read needs (1,520 for each of the $entries entries the queue builds, plus the read floor). Re-queue it after the reset or after pr-merge's next scheduled run uploads its cache." exit 1 fi echo "::notice::no carried run cache from pr-merge's last run; GITHUB_TOKEN has $remaining requests left (cold reads for $entries queue entries need $need), so the verdict reader reads every FAIL." diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index 83c31e65ad..4cc4470fcf 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -1397,12 +1397,16 @@ leaves it reading more. With no carried run cache, or one whose carried sections (the artifact is kept for one day, so pr-merge with no successful scheduled or hand-started run that long leaves none), the step reads the base branch's `merge_queue` rule and GitHub's `/rate_limit`, which costs no request. A missing cache is shared, so every entry the queue builds -at once goes cold together: the cold reader starts only when the token's core pool has at least 900 -requests (one cold read's cost) times `max_entries_to_build`, plus the read floor (`limits.api` -`read-floor` of the planned limit, inlined in the job env), left: 4,600 with the queue building 5. A rule or rate limit that does not read, a stop in the +at once goes cold together: the cold reader starts only when the token's core pool has at least +1,520 requests (one cold read's worst case under the listing's caps: 793 per-FAIL reads, an +incremental try of at most 13 pages, and 3 whole listings of up to 3 attempts of 77 pages, each +with a top-up of at most 7) times `max_entries_to_build`, plus the read floor (`limits.api` +`read-floor` of the planned limit, inlined in the job env), left: 7,700 with the queue building 5. +An entry with a carried run cache but no carried listing pays the listing's part (up to 727) +without this check. A rule or rate limit that does not read, a stop in the job's stop file, or less headroom than that holds the pull request, so the queue ejects it and the ejection counts toward `merge.max-queue-ejections`. `ci-status` allows 20 minutes for a cold read, -about 6 to 11. It reads with `ci-status`'s token, which holds no `issues` +about 6 to 11, at most about 15. It reads with `ci-status`'s token, which holds no `issues` scope: an issue read that does not answer refuses `verify:unreadable`, which holds. The trust hold has three checks. At PR open, report-outcome writes a `Hold:` line when the issue diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py index 16c8c306bb..768f4f9544 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py @@ -28,6 +28,7 @@ import json import os import pathlib +import re import subprocess import sys import tempfile @@ -3325,6 +3326,27 @@ def test_the_refresh_before_an_allow_holds_the_live_count_too(self) -> None: ) +class TheColdReadHeadroomCoversTheListingCaps(unittest.TestCase): + """ci-status holds a cold reader unless GITHUB_TOKEN's pool has one cold + read's worst case per queue entry (pr-require-checks.yml). That figure + must cover the reader's listing caps, so raising `LISTING_READS` or + `LISTING_WAITS` without it fails here. The other terms are the figures + the workflow comment derives: 793 per-FAIL reads (860 measured + 2026-10-25 less its 67-page listing), an incremental try of at most 13 + pages, 77 pages a whole listing and a top-up of at most 7.""" + + def test_the_workflow_constant_is_the_capped_worst_case(self) -> None: + workflow = ( + pathlib.Path(__file__).resolve().parents[6] + / ".github/workflows/pr-require-checks.yml" + ).read_text(encoding="utf-8") + found = re.findall(r'printf "%d\\n", (\d+) \* entries', workflow) + self.assertEqual(len(found), 1, found) + attempts = 1 + len(ve.LISTING_WAITS) + worst = 793 + 13 + ve.LISTING_READS * (attempts * 77 + 7) + self.assertEqual(int(found[0]), worst) + + # --- The --authority pr-merge path ------------------------------------------------ diff --git a/scripts/ci-lanes-pr-hold.test.sh b/scripts/ci-lanes-pr-hold.test.sh index 2e8a45b180..4ce0421d8e 100755 --- a/scripts/ci-lanes-pr-hold.test.sh +++ b/scripts/ci-lanes-pr-hold.test.sh @@ -685,21 +685,21 @@ STUB_NO_LISTING=1 STUB_EVIDENCE="$(jq -nc --arg head "$SHA" '{refusal: null, hea # A cold reader (no carried run cache) runs only when GITHUB_TOKEN's core # pool has the cold read's cost for every entry the queue builds at once # (the merge_queue rule's max_entries_to_build, STUB_RULES, 1 by default) -# plus the read floor left: 900 x 1 + 0.1 x min(limit, 1000) = 1000 here. +# plus the read floor left: 1520 x 1 + 0.1 x min(limit, 1000) = 1620 here. rate() { jq -nc --argjson remaining "$1" '{resources: {core: {limit: 5000, remaining: $remaining, reset: 1793030000}}}'; } -STUB_COLD=1 STUB_RATE="$(rate 1000)" expect "in the merge queue, a cold reader with the headroom for it reads" 0 3 "#7: no pr-verify FAIL hold: the reader allows this head." +STUB_COLD=1 STUB_RATE="$(rate 1620)" expect "in the merge queue, a cold reader with the headroom for it reads" 0 3 "#7: no pr-verify FAIL hold: the reader allows this head." if [[ "$(readers)" -eq 1 ]]; then ok "a cold reader with headroom is started"; else fail "a cold reader with headroom ran $(readers) times"; fi -STUB_COLD=1 STUB_RATE="$(rate 999)" expect "in the merge queue, a cold reader without the headroom holds without reading" 1 3 "::error::#7 is held: no carried run cache from pr-merge's last run, and GITHUB_TOKEN has 999 requests left, under the 1000 a cold read needs" +STUB_COLD=1 STUB_RATE="$(rate 1619)" expect "in the merge queue, a cold reader without the headroom holds without reading" 1 3 "::error::#7 is held: no carried run cache from pr-merge's last run, and GITHUB_TOKEN has 1619 requests left, under the 1620 a cold read needs" if [[ "$(readers)" -eq 0 ]]; then ok "a cold reader without headroom is never started"; else fail "a cold reader without headroom ran: '$(grep '^reader ' "$TMP_ROOT/log")'"; fi STUB_COLD=1 STUB_RATE='' expect "in the merge queue, a cold reader whose rate limit does not read holds" 1 3 "::error::#7 is held: no carried run cache from pr-merge's last run, and GITHUB_TOKEN's remaining requests could not be read" if [[ "$(readers)" -eq 0 ]]; then ok "a cold reader with an unread rate limit is never started"; else fail "a cold reader with an unread rate limit ran"; fi STUB_COLD=1 STUB_RATE='{"resources":{"core":{"limit":5000}}}' expect "in the merge queue, a rate limit with no remaining count holds a cold reader" 1 3 "GITHUB_TOKEN's remaining requests could not be read" # Every entry the queue builds at once goes cold together on a missing cache, -# so the need scales with max_entries_to_build: 900 x 5 + 100 = 4600. -STUB_COLD=1 STUB_RULES=5 STUB_RATE="$(rate 4600)" expect "in the merge queue, a cold reader reads with headroom for every entry the queue builds" 0 3 "#7: no pr-verify FAIL hold: the reader allows this head." -STUB_COLD=1 STUB_RULES=5 STUB_RATE="$(rate 4599)" expect "in the merge queue, a cold reader holds under the headroom of every entry the queue builds" 1 3 "GITHUB_TOKEN has 4599 requests left, under the 4600 a cold read needs" -STUB_COLD=1 STUB_RULES=2 STUB_RATE="$(rate 1899)" expect "in the merge queue, the cold headroom scales with max_entries_to_build" 1 3 "under the 1900 a cold read needs" -STUB_COLD=1 STUB_RULES='' STUB_RATE="$(rate 4600)" expect "in the merge queue, a merge queue rule that does not read holds a cold reader" 1 3 "the merge queue's max_entries_to_build could not be read" +# so the need scales with max_entries_to_build: 1520 x 5 + 100 = 7700. +STUB_COLD=1 STUB_RULES=5 STUB_RATE="$(rate 7700)" expect "in the merge queue, a cold reader reads with headroom for every entry the queue builds" 0 3 "#7: no pr-verify FAIL hold: the reader allows this head." +STUB_COLD=1 STUB_RULES=5 STUB_RATE="$(rate 7699)" expect "in the merge queue, a cold reader holds under the headroom of every entry the queue builds" 1 3 "GITHUB_TOKEN has 7699 requests left, under the 7700 a cold read needs" +STUB_COLD=1 STUB_RULES=2 STUB_RATE="$(rate 3139)" expect "in the merge queue, the cold headroom scales with max_entries_to_build" 1 3 "under the 3140 a cold read needs" +STUB_COLD=1 STUB_RULES='' STUB_RATE="$(rate 7700)" expect "in the merge queue, a merge queue rule that does not read holds a cold reader" 1 3 "the merge queue's max_entries_to_build could not be read" if [[ "$(readers)" -eq 0 ]]; then ok "a cold reader with an unread queue rule is never started"; else fail "a cold reader with an unread queue rule ran"; fi STUB_CARRIED='{"carried-run-record":{},"carried-fail-head-ref":{},"carried-on-branch":{}}' expect "in the merge queue, a carried cache with every section empty is cold" 1 3 "::error::#7 is held: no carried run cache from pr-merge's last run, and GITHUB_TOKEN's remaining requests could not be read" STUB_CARRIED='not json' expect "in the merge queue, a carried cache that does not parse is cold" 1 3 "GITHUB_TOKEN's remaining requests could not be read" From d9d29e9a249ee4f16cf76ba4225642c7f1ad9bd9 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 17:10:32 -0400 Subject: [PATCH 6/7] test(pr-merge): count both floor classes added on each side of the main merge --- .../skills/babysit-prs/scripts/tests/test_merge_holds_sync.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py index 8d293558e1..0128b75761 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py @@ -227,7 +227,7 @@ def test_the_floor_is_the_same_on_both_sides(self) -> None: self.assertEqual(answer["classes"], [list(row) for row in risk.FLOOR_CLASSES]) self.assertEqual(answer["floor"], list(risk.HIGH_FLOOR)) self.assertEqual(answer["review"], list(risk.REVIEW_FLOOR)) - self.assertEqual(len(risk.FLOOR_CLASSES), 52) + self.assertEqual(len(risk.FLOOR_CLASSES), 53) self.assertEqual(len(risk.REVIEW_FLOOR), 21) self.assertEqual( {kind for _, kind in risk.FLOOR_CLASSES}, {"trust", "gate", "enforcement"} From 37d3d321bf3477a2a31eccaf33b34ee36c0dfe98 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 17:39:07 -0400 Subject: [PATCH 7/7] fix(source-control): keep an artifact with null dates in the listing GitHub's artifact schema marks created_at and expires_at nullable, but the reader refused any listing holding such an entry, so one undated artifact made every whole listing's top-up fail and every candidate refuse verify:unreadable. An entry may now carry null dates: it is never pruned by date, and paging stops only on a dated entry, so the result is still checked against the live total_count. A missing date key or a malformed date still lists every page. Co-Authored-By: Claude Opus 5.5 --- .../scripts/babysit_verify_evidence.py | 36 ++++++++++------ .../tests/test_babysit_verify_evidence.py | 42 +++++++++++++++++++ 2 files changed, 65 insertions(+), 13 deletions(-) diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py index 4b326f21e6..3312885e43 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py @@ -732,16 +732,20 @@ def _is_artifact_id(value: Any) -> bool: return isinstance(value, int) and not isinstance(value, bool) -def _dated_entry(item: Any) -> bool: +def _listed_entry(item: Any) -> bool: """A listed artifact `Reader._incremental` can place: a positive id, a - boolean `expired`, and a `created_at` and `expires_at` with a zone.""" + boolean `expired`, and a `created_at` and `expires_at` each present and + null (GitHub's artifact schema marks both nullable) or a time with a + zone.""" return ( is_json_object(item) and _is_artifact_id(item.get("id")) and item["id"] > 0 and isinstance(item.get("expired"), bool) - and _when(item.get("created_at")) is not None - and _when(item.get("expires_at")) is not None + and all( + key in item and (item[key] is None or _when(item[key]) is not None) + for key in ("created_at", "expires_at") + ) ) @@ -939,7 +943,7 @@ def _load_saved( def _incremental_base(self) -> tuple[datetime, list[dict[str, Any]]] | None: """The newest whole listing begun at most `LISTING_CARRY_MAX_AGE` ago - whose every entry `_dated_entry` takes: this Reader's own, the run's + whose every entry `_listed_entry` takes: this Reader's own, the run's saved one, or the one a trusted run carried (`carry_path`).""" bases = [ base @@ -950,7 +954,7 @@ def _incremental_base(self) -> tuple[datetime, list[dict[str, Any]]] | None: ) if base is not None and _recent(base[0], LISTING_CARRY_MAX_AGE) - and all(_dated_entry(a) for a in base[1]) + and all(_listed_entry(a) for a in base[1]) ] return max(bases, key=lambda base: base[0], default=None) @@ -993,18 +997,20 @@ def _incremental( The listing comes newest id first. Pages are read from the first until one holds an artifact at or below the base's highest id that - was created `LISTING_OVERLAP` or more before the base was read: every - lower id was created before the base was read, so the base holds it. + was created `LISTING_OVERLAP` or more before the base was read (an + undated one never stops it): every lower id was created before the + base was read, so the base holds it. Those pages decide every id down to the lowest they hold, an id the base holds there and they do not (deleted, or an overwrite's old copy) included; below it the base's entries stand, less each one - expired or past its `expires_at` by the first page's `Date`. The + expired or past its `expires_at` by the first page's `Date` (one with + no `expires_at` stands unless expired). The result must hold exactly the first page's `total_count` artifacts, GitHub's live count. Any other count, no base, an entry or page of another shape, a missing `Date`, or any failed read gives None. `base` replaces `_incremental_base` and `deadline` its own `LISTING_TIMEOUT_SECONDS` (`_complete_listing`'s top-up).""" - if base is not None and not all(_dated_entry(a) for a in base[1]): + if base is not None and not all(_listed_entry(a) for a in base[1]): return None base = base or self._incremental_base() if base is None: @@ -1039,7 +1045,7 @@ def _incremental( if ( proc.returncode != 0 or not is_json_array(items) - or not all(_dated_entry(a) for a in items) + or not all(_listed_entry(a) for a in items) ): return None if page == 1: @@ -1052,8 +1058,12 @@ def _incremental( return None for item in items: fresh.setdefault(item["id"], item) + # Only a dated entry can show its id was created before the base. reached = any( - a["id"] <= high and _when(a["created_at"]) < floor for a in items + a["id"] <= high + and a["created_at"] is not None + and _when(a["created_at"]) < floor + for a in items ) if reached or len(items) < LISTING_PAGE_SIZE: break @@ -1067,7 +1077,7 @@ def _incremental( for a in carried if a["id"] < lowest and a["expired"] is False - and _when(a["expires_at"]) > server_now + and (a["expires_at"] is None or _when(a["expires_at"]) > server_now) ] listing = sorted([*fresh.values(), *kept], key=lambda a: -a["id"]) return listing if len(listing) == total else None diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py index 06d77d0f55..3e0a9754d8 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py @@ -3130,6 +3130,48 @@ def undated(args: list[str], **kwargs: Any) -> Any: self.assertEqual(reader.listing_mode, "full") self.assertEqual(evidence.refusal, "verify:unreadable") + @staticmethod + def undated(name: str, run_id: int, artifact_id: int) -> dict[str, Any]: + """A listed artifact whose dates GitHub gives as null (its artifact + schema marks `created_at` and `expires_at` nullable).""" + return artifact(name, run_id) | { + "id": artifact_id, + "created_at": None, + "expires_at": None, + } + + def test_a_whole_listing_with_an_undated_artifact_lists_fully(self) -> None: + # No carry: the whole listing's top-up stops on its dated entries, and + # the undated FAIL is kept, so the listing holds the live count. + fake = self.fake(self.undated(named(HEAD, "fail"), 2, 850)) + reader, evidence = self.judge(fake) + self.assertEqual(evidence.refusal, "verify:fail") + self.assertEqual(reader.listing_mode, "full") + self.assertEqual(fake.full_listings(), 1) + ids = [a["id"] for a in json.loads(self.saved.read_text())["artifacts"]] + self.assertEqual(ids, [1000, 900, 850, 800, 700]) + + def test_an_undated_carried_entry_is_kept_below_the_new_pages(self) -> None: + # The carry holds an undated FAIL below the pages read now: no date + # prunes it, and the incremental listing still holds the live count. + fail = self.undated(named(HEAD, "fail"), 2, 850) + self.write_carry([*self.base[:2], fail, *self.base[2:]]) + fake = self.fake(fail) + reader, evidence = self.judge(fake) + self.assertEqual(evidence.refusal, "verify:fail") + self.assertEqual(reader.listing_mode, "incremental") + self.assertEqual(fake.full_listings(), 0) + + def test_an_undated_upload_since_is_read_and_paging_stops_on_dated_ones( + self, + ) -> None: + self.write_carry(self.base) + fake = self.fake(self.undated(named(HEAD, "fail"), 2, 1100)) + reader, evidence = self.judge(fake) + self.assertEqual(evidence.refusal, "verify:fail") + self.assertEqual(reader.listing_mode, "incremental") + self.assertEqual((fake.pages(), fake.full_listings()), (2, 0)) + def test_a_refresh_builds_on_the_listing_it_drops(self) -> None: # The gate's re-list before an allow (babysit_merge.py `refresh`) # pages only past the run's own listing.