From e6e88f6dbdcc8ccf39e9030cc1d7dece52e37e9b Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 2 Nov 2026 00:25:57 -0500 Subject: [PATCH 1/8] perf(source-control): upload a FAIL index before each pr-verify FAIL Each FAIL now uploads one small artifact per tie the merge gate counts it by: pr-verify-index-pr-, -sha-, -ref- and -issue- (write-verdict.mjs failIndexNames, the upload-fail-index action). The verdict is scanned and uploads only when it is no FAIL or every index uploaded, so a FAIL never exists without its index. The API's name filter returns every artifact of an exact name across runs, which lets the reader find a FAIL without the whole listing. Co-Authored-By: Claude Opus 5.5 --- .github/actions/upload-fail-index/action.yml | 103 ++++++++++++++++++ .github/artifact-retention.json | 11 ++ .github/scripts/pr-verify/workflow.test.mjs | 19 +++- .github/scripts/pr-verify/write-verdict.mjs | 33 +++++- .../scripts/pr-verify/write-verdict.test.mjs | 36 +++++- .github/workflows/pr-verify.yml | 56 ++++++++-- 6 files changed, 246 insertions(+), 12 deletions(-) create mode 100644 .github/actions/upload-fail-index/action.yml diff --git a/.github/actions/upload-fail-index/action.yml b/.github/actions/upload-fail-index/action.yml new file mode 100644 index 00000000000..2431c6afbe7 --- /dev/null +++ b/.github/actions/upload-fail-index/action.yml @@ -0,0 +1,103 @@ +name: Upload a FAIL's index artifacts +description: > + Upload one small artifact per name in `names` (write-verdict.mjs + failIndexNames: the pull request, the head, the head branch's digest and each + issue the FAIL records), each holding `path`, so the pr-merge lane's reader + finds every FAIL by exact-name lookups instead of the repository's whole + artifact listing (babysit_verify_evidence.py `Reader._targeted`). The FAIL's + upload must require this step's success: a FAIL with no index is one that + reader cannot see, and a failed index upload leaves no verdict at all. More + names than slots fail the step. + +inputs: + names: + description: The index artifact names, one per line (at most 13). + required: true + path: + description: The file each index artifact holds. + required: true + retention-days: + description: The FAIL's retention, which each index keeps. + required: true + +runs: + using: composite + steps: + - name: Split the names + id: slots + shell: bash + env: + NAMES: ${{ inputs.names }} + RETENTION: ${{ inputs.retention-days }} + run: | + case "$RETENTION" in + [1-9] | [1-8][0-9] | 90) echo "retention-days=$RETENTION" >>"$GITHUB_OUTPUT" ;; + *) + echo "::error::retention-days is not 1 to 90" + exit 1 + ;; + esac + mapfile -t names < <(printf '%s\n' "$NAMES" | sed '/^$/d') + if [ "${#names[@]}" -lt 3 ] || [ "${#names[@]}" -gt 13 ]; then + echo "::error::${#names[@]} FAIL index names, not 3 to 13" + exit 1 + fi + for i in "${!names[@]}"; do + case "${names[$i]}" in + pr-verify-index-[a-z]*-[0-9a-f]*) ;; + *) + echo "::error::a FAIL index name no reader takes" + exit 1 + ;; + esac + echo "n$((i + 1))=${names[$i]}" >>"$GITHUB_OUTPUT" + done + - name: Upload index 1 + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: { name: "${{ steps.slots.outputs.n1 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + - name: Upload index 2 + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: { name: "${{ steps.slots.outputs.n2 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + - name: Upload index 3 + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: { name: "${{ steps.slots.outputs.n3 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + - name: Upload index 4 + if: ${{ steps.slots.outputs.n4 != '' }} + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: { name: "${{ steps.slots.outputs.n4 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + - name: Upload index 5 + if: ${{ steps.slots.outputs.n5 != '' }} + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: { name: "${{ steps.slots.outputs.n5 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + - name: Upload index 6 + if: ${{ steps.slots.outputs.n6 != '' }} + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: { name: "${{ steps.slots.outputs.n6 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + - name: Upload index 7 + if: ${{ steps.slots.outputs.n7 != '' }} + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: { name: "${{ steps.slots.outputs.n7 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + - name: Upload index 8 + if: ${{ steps.slots.outputs.n8 != '' }} + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: { name: "${{ steps.slots.outputs.n8 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + - name: Upload index 9 + if: ${{ steps.slots.outputs.n9 != '' }} + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: { name: "${{ steps.slots.outputs.n9 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + - name: Upload index 10 + if: ${{ steps.slots.outputs.n10 != '' }} + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: { name: "${{ steps.slots.outputs.n10 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + - name: Upload index 11 + if: ${{ steps.slots.outputs.n11 != '' }} + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: { name: "${{ steps.slots.outputs.n11 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + - name: Upload index 12 + if: ${{ steps.slots.outputs.n12 != '' }} + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: { name: "${{ steps.slots.outputs.n12 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + - name: Upload index 13 + if: ${{ steps.slots.outputs.n13 != '' }} + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: { name: "${{ steps.slots.outputs.n13 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } diff --git a/.github/artifact-retention.json b/.github/artifact-retention.json index b196bc2ccbc..08dff92e6a5 100644 --- a/.github/artifact-retention.json +++ b/.github/artifact-retention.json @@ -1,6 +1,17 @@ { "$schema": "./artifact-retention.schema.json", "families": { + "pr-verify-index-*": { + "retention-days": "max", + "readers": [ + "plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py Reader._targeted finds every FAIL by these exact names instead of the whole artifact listing" + ], + "max-reader": "plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py", + "reason": "Each index artifact is how the gate finds its FAIL, so it keeps the FAIL's own retention (write-verdict.mjs retentionDays, max for a FAIL): an index that expired first would hide a FAIL that still counts. It holds only the FAIL's artifact name, built from numbers and a SHA, and no model text, so no leak scan runs over it.", + "scan": "none", + "computed-by": ".github/scripts/pr-verify/write-verdict.mjs", + "uploads": [".github/actions/upload-fail-index/action.yml#runs"] + }, "pr-verify-*": { "retention-days": "max", "readers": [ diff --git a/.github/scripts/pr-verify/workflow.test.mjs b/.github/scripts/pr-verify/workflow.test.mjs index a7aa867c9ab..62e325ecc2b 100644 --- a/.github/scripts/pr-verify/workflow.test.mjs +++ b/.github/scripts/pr-verify/workflow.test.mjs @@ -125,13 +125,28 @@ test("the upload is each job's only artifact step and follows the written verdic assert.ok(index("Judge the pull request", job) >= 0 && index("Write the verdict", job) > index("Judge the pull request", job), job); // The written verdict is scanned just before it uploads, and uploads only on the scan's clean output. assert.ok(index("Scan the upload for credentials", job) > index("Write the verdict", job), job); + // A FAIL's index uploads before the scan, and the verdict is scanned and + // uploads only when it is no FAIL or every index uploaded + // (write-verdict.mjs failIndexNames). + assert.equal(index("Upload the FAIL index", job), index("Write the verdict", job) + 1, job); + assert.equal(index("Scan the upload for credentials", job), index("Upload the FAIL index", job) + 1, job); assert.equal(index("Upload the verdict", job), index("Scan the upload for credentials", job) + 1, job); + const indexed = "steps\\.verdict\\.outcome == 'success' && \\(steps\\.verdict\\.outputs\\.verdict != 'fail' \\|\\| steps\\.fail-index\\.outcome == 'success'\\)"; const scan = step("Scan the upload for credentials", job); - assert.match(scan, /^ {8}id: scan-upload\n {8}if: >-\n {10}\$\{\{ !cancelled\(\) && steps\.verdict\.outcome == 'success' \}\}$/m, job); + assert.match(scan, new RegExp(`^ {8}id: scan-upload\\n {8}if: >-\\n {10}\\$\\{\\{ !cancelled\\(\\) && ${indexed} \\}\\}$`, "m"), job); assert.match(scan, /^ {10}path: \$\{\{ runner\.temp \}\}\/pr-verify-verdict\/verdict\.json$/m, job); + const failIndex = step("Upload the FAIL index", job); + assert.match( + failIndex, + /^ {8}id: fail-index\n {8}if: >-\n {10}\$\{\{ !cancelled\(\) && steps\.verdict\.outcome == 'success' && steps\.verdict\.outputs\.verdict == 'fail' \}\}$/m, + job, + ); + assert.match(failIndex, /^ {8}uses: \.\/\.base\/\.github\/actions\/upload-fail-index /m, job); + assert.equal(input(failIndex, "names"), "${{ steps.verdict.outputs.index-names }}", job); + assert.equal(input(failIndex, "retention-days"), "${{ steps.verdict.outputs.retention-days }}", job); assert.match( uploads[0].block, - /^ {8}if: >-\n {10}\(!cancelled\(\) && steps\.verdict\.outcome == 'success'\) && steps\.scan-upload\.outputs\.clean == 'true'$/m, + new RegExp(`^ {8}if: >-\\n {10}\\(!cancelled\\(\\) && ${indexed}\\) && steps\\.scan-upload\\.outputs\\.clean == 'true'$`, "m"), job, ); // The verdict step names the retention (write-verdict.mjs retentionDays). diff --git a/.github/scripts/pr-verify/write-verdict.mjs b/.github/scripts/pr-verify/write-verdict.mjs index bf98b3e0480..cd84d4b9b2d 100644 --- a/.github/scripts/pr-verify/write-verdict.mjs +++ b/.github/scripts/pr-verify/write-verdict.mjs @@ -505,6 +505,28 @@ export function artifactName(verdict) { return name; } +// The exact names under which a FAIL's index artifacts upload, one per tie the +// merge gate's reader looks up (babysit_verify_evidence.INDEX_PREFIX and +// `Reader._targeted`): the pull request, the head, the head branch's digest, +// then each issue the FAIL records. The API's name filter matches whole names +// across runs, so each lookup returns every FAIL so tied without the whole +// listing. None starts like a verdict name, which the reader would hold. +export const INDEX_PREFIX = "pr-verify-index-"; +export function failIndexNames(verdict) { + const issues = [...new Set((verdict.linked_issues ?? []).map((issue) => issue.number))] + .filter((n) => Number.isSafeInteger(n) && n > 0) + .sort((a, b) => a - b); + if (issues.length > NAMED_ISSUES_MAX) { + throw new Error(`the FAIL records ${issues.length} issues, more than its index carries`); + } + return [ + `${INDEX_PREFIX}pr-${verdict.pr}`, + `${INDEX_PREFIX}sha-${verdict.head_sha}`, + `${INDEX_PREFIX}ref-${headRefDigest(verdict.head_ref)}`, + ...issues.map((n) => `${INDEX_PREFIX}issue-${n}`), + ]; +} + // How many days the verdict's artifact lasts. A PASS or PASS-NOTES judged // after no body edit or issue connect or disconnect since the head commit // (resolve-pr.mjs relinkedSince, RELINK_SINCE_HEAD `false`) is read only at @@ -802,7 +824,16 @@ export function main(env = process.env) { mkdirSync(env.VERDICT_DIR, { recursive: true }); writeFileSync(path.join(env.VERDICT_DIR, "verdict.json"), `${JSON.stringify(verdict, null, 2)}\n`); const name = artifactName(verdict); - appendFileSync(env.GITHUB_OUTPUT, `name=${name}\nverdict=${verdict.verdict}\nretention-days=${retentionDays(verdict.verdict, env.RELINK_SINCE_HEAD !== "false")}\n`); + // A FAIL's index (failIndexNames): each artifact holds the FAIL's name. + const index = verdict.verdict === "fail" ? failIndexNames(verdict) : []; + if (index.length > 0) { + writeFileSync(path.join(env.VERDICT_DIR, "fail-index.txt"), `${name}\n`); + } + appendFileSync( + env.GITHUB_OUTPUT, + `name=${name}\nverdict=${verdict.verdict}\nretention-days=${retentionDays(verdict.verdict, env.RELINK_SINCE_HEAD !== "false")}\n` + + `index-names< console.log(`::notice title=pr-verify chunk::pr=${verdict.pr} chunk=${k}/${count} verdict=${one?.verdict ?? "none"}`)); console.log(`write-verdict: ${name}`); } diff --git a/.github/scripts/pr-verify/write-verdict.test.mjs b/.github/scripts/pr-verify/write-verdict.test.mjs index fadb91af2a3..fb0bf33f2af 100644 --- a/.github/scripts/pr-verify/write-verdict.test.mjs +++ b/.github/scripts/pr-verify/write-verdict.test.mjs @@ -23,6 +23,7 @@ import { buildVerdict, checkChunk, checkKeyLeg, + failIndexNames, HEAD_REF_DIGEST_HEX, headRefDigest, localLoop, @@ -57,6 +58,25 @@ const ok = JSON.stringify({ verdict: "pass", summary: "s", notes: [] }); const NO_APP = { capture: "no-app", reason: "no app check ran", capture_count: 0, features: [], local_loop: null }; const BLOCKED = { capture: "blocked", reason: "the capture summary is unreadable or outside its schema", capture_count: 0, features: [], local_loop: null }; +test("a FAIL's index names each tie the merge gate looks up, none like a verdict name", () => { + const fail = (linkedIssues) => + buildVerdict({ ...ids, linkedIssues, output: JSON.stringify({ verdict: "fail", summary: "s", notes: [] }) }); + const digest = headRefDigest("feat/x"); + assert.deepEqual(failIndexNames(fail([])), [ + "pr-verify-index-pr-41", + `pr-verify-index-sha-${HEAD}`, + `pr-verify-index-ref-${digest}`, + ]); + const issues = [9, 7, 9].map((number) => ({ ...ISSUE, number })); + assert.deepEqual(failIndexNames(fail(issues)).slice(3), ["pr-verify-index-issue-7", "pr-verify-index-issue-9"]); + for (const name of failIndexNames(fail(issues))) { + // babysit_verify_evidence.FAIL_PREFIX_RE holds anything that starts like a FAIL. + assert.doesNotMatch(name, /^pr-verify-[0-9]+-[0-9a-f]{40}-fail/); + } + const eleven = Array.from({ length: 11 }, (_, i) => ({ ...ISSUE, number: i + 1 })); + assert.throws(() => failIndexNames({ pr: 41, head_sha: HEAD, head_ref: "feat/x", linked_issues: eleven }), /more than its index carries/); +}); + test("each verdict names its artifact with the PR, the head and the verdict", () => { for (const verdict of ["pass", "pass-notes", "fail"]) { const built = buildVerdict({ ...ids, output: JSON.stringify({ verdict, summary: "s", notes: ["n"] }) }); @@ -498,7 +518,13 @@ test("main writes the verdict file and the artifact name, and nothing on bad out assert.equal(written.summary, "wrong"); assert.deepEqual(written.linked_issues, [ISSUE]); assert.deepEqual(written.evidence, NO_APP); - assert.equal(readFileSync(env.GITHUB_OUTPUT, "utf8"), `name=pr-verify-41-${HEAD}-fail-i7\nverdict=fail\nretention-days=90\n`); + assert.equal( + readFileSync(env.GITHUB_OUTPUT, "utf8"), + `name=pr-verify-41-${HEAD}-fail-i7\nverdict=fail\nretention-days=90\n` + + `index-names<--, a # FAIL's followed by -i (write-verdict.mjs artifactName); its readers also -# accept the - before that (write-verdict.mjs headRefDigest). +# accept the - before that (write-verdict.mjs headRefDigest). A +# FAIL first uploads its index (upload-fail-index, write-verdict.mjs +# failIndexNames), which the gate reads by exact name instead of the whole +# artifact listing, and uploads only once every index did. # Only steps of a run of this file on main can upload that run's artifacts, so # the pr-merge lane counts a verdict only through its run record # (plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py). @@ -1865,17 +1868,30 @@ jobs: VERDICT_DIR: ${{ runner.temp }}/pr-verify-verdict run: node .base/.github/scripts/pr-verify/write-verdict.mjs + # A FAIL uploads its index first (write-verdict.mjs failIndexNames), and + # the FAIL only after every index uploaded: the merge gate finds a FAIL + # by its index alone, so a failed index upload leaves no verdict. + - name: Upload the FAIL index + id: fail-index + if: >- + ${{ !cancelled() && steps.verdict.outcome == 'success' && steps.verdict.outputs.verdict == 'fail' }} + uses: ./.base/.github/actions/upload-fail-index # zizmor: ignore[self-repository] + with: + names: ${{ steps.verdict.outputs.index-names }} + path: ${{ runner.temp }}/pr-verify-verdict/fail-index.txt + retention-days: ${{ steps.verdict.outputs.retention-days }} + - name: Scan the upload for credentials id: scan-upload if: >- - ${{ !cancelled() && steps.verdict.outcome == 'success' }} + ${{ !cancelled() && steps.verdict.outcome == 'success' && (steps.verdict.outputs.verdict != 'fail' || steps.fail-index.outcome == 'success') }} uses: ./.base/.github/actions/scan-upload # zizmor: ignore[self-repository] with: path: ${{ runner.temp }}/pr-verify-verdict/verdict.json - name: Upload the verdict if: >- - (!cancelled() && steps.verdict.outcome == 'success') && steps.scan-upload.outputs.clean == 'true' + (!cancelled() && steps.verdict.outcome == 'success' && (steps.verdict.outputs.verdict != 'fail' || steps.fail-index.outcome == 'success')) && steps.scan-upload.outputs.clean == 'true' uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 with: name: ${{ steps.verdict.outputs.name }} @@ -3671,17 +3687,30 @@ jobs: VERDICT_DIR: ${{ runner.temp }}/pr-verify-verdict run: node .base/.github/scripts/pr-verify/write-verdict.mjs + # A FAIL uploads its index first (write-verdict.mjs failIndexNames), and + # the FAIL only after every index uploaded: the merge gate finds a FAIL + # by its index alone, so a failed index upload leaves no verdict. + - name: Upload the FAIL index + id: fail-index + if: >- + ${{ !cancelled() && steps.verdict.outcome == 'success' && steps.verdict.outputs.verdict == 'fail' }} + uses: ./.base/.github/actions/upload-fail-index # zizmor: ignore[self-repository] + with: + names: ${{ steps.verdict.outputs.index-names }} + path: ${{ runner.temp }}/pr-verify-verdict/fail-index.txt + retention-days: ${{ steps.verdict.outputs.retention-days }} + - name: Scan the upload for credentials id: scan-upload if: >- - ${{ !cancelled() && steps.verdict.outcome == 'success' }} + ${{ !cancelled() && steps.verdict.outcome == 'success' && (steps.verdict.outputs.verdict != 'fail' || steps.fail-index.outcome == 'success') }} uses: ./.base/.github/actions/scan-upload # zizmor: ignore[self-repository] with: path: ${{ runner.temp }}/pr-verify-verdict/verdict.json - name: Upload the verdict if: >- - (!cancelled() && steps.verdict.outcome == 'success') && steps.scan-upload.outputs.clean == 'true' + (!cancelled() && steps.verdict.outcome == 'success' && (steps.verdict.outputs.verdict != 'fail' || steps.fail-index.outcome == 'success')) && steps.scan-upload.outputs.clean == 'true' uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 with: name: ${{ steps.verdict.outputs.name }} @@ -5211,17 +5240,30 @@ jobs: VERDICT_DIR: ${{ runner.temp }}/pr-verify-verdict run: node .base/.github/scripts/pr-verify/write-verdict.mjs + # A FAIL uploads its index first (write-verdict.mjs failIndexNames), and + # the FAIL only after every index uploaded: the merge gate finds a FAIL + # by its index alone, so a failed index upload leaves no verdict. + - name: Upload the FAIL index + id: fail-index + if: >- + ${{ !cancelled() && steps.verdict.outcome == 'success' && steps.verdict.outputs.verdict == 'fail' }} + uses: ./.base/.github/actions/upload-fail-index # zizmor: ignore[self-repository] + with: + names: ${{ steps.verdict.outputs.index-names }} + path: ${{ runner.temp }}/pr-verify-verdict/fail-index.txt + retention-days: ${{ steps.verdict.outputs.retention-days }} + - name: Scan the upload for credentials id: scan-upload if: >- - ${{ !cancelled() && steps.verdict.outcome == 'success' }} + ${{ !cancelled() && steps.verdict.outcome == 'success' && (steps.verdict.outputs.verdict != 'fail' || steps.fail-index.outcome == 'success') }} uses: ./.base/.github/actions/scan-upload # zizmor: ignore[self-repository] with: path: ${{ runner.temp }}/pr-verify-verdict/verdict.json - name: Upload the verdict if: >- - (!cancelled() && steps.verdict.outcome == 'success') && steps.scan-upload.outputs.clean == 'true' + (!cancelled() && steps.verdict.outcome == 'success' && (steps.verdict.outputs.verdict != 'fail' || steps.fail-index.outcome == 'success')) && steps.scan-upload.outputs.clean == 'true' uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 with: name: ${{ steps.verdict.outputs.name }} From aab5d415b5ce97680cd2ee1605fa7a9ad763a07e Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 2 Nov 2026 00:26:09 -0500 Subject: [PATCH 2/8] perf(source-control): judge verdicts by exact-name index reads Once verify-legacy-fails.json (every FAIL from before the index) sits beside the reader, a judgment reads no artifact listing. It reads the index names of the pull request, its closers, its head and commits, its head branch and its live issues, each named run's artifacts, and the pass names at the head and at each FAILed head. Each read must hold its own total_count; any failed read refuses. The answer key, the skip filter's key (Reader.own_verdicts) and fail_findings read the same way. With no legacy file, or past INDEX_NAMES_CAP names, the whole listing is read as before. legacy-fails builds the file from a listing that holds the live count, after every verifier run from before the cutover has completed. Co-Authored-By: Claude Opus 5.5 --- docs/conventions/pr-pipeline/README.md | 7 + .../babysit-prs/scripts/babysit_merge_skip.py | 16 +- .../scripts/babysit_verify_evidence.py | 466 +++++++++++++++++- .../tests/test_babysit_verify_index.py | 385 +++++++++++++++ 4 files changed, 860 insertions(+), 14 deletions(-) create mode 100644 plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index 13d1a685567..c2d68779e07 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -2661,6 +2661,13 @@ 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. +Once the legacy FAILs file (`verify-legacy-fails.json` beside the reader) is committed, the gate +reads no listing at all: each FAIL's run uploads index artifacts first (`pr-verify-index-pr-`, +`-sha-`, `-ref-`, `-issue-`; `.github/actions/upload-fail-index`), the FAIL only +after every index did, and a judgment reads those exact names for the pull request's number, its +head and commits, its head branch, its live issues and their other closing pull requests, then each +run they name and the pass names at each FAILed head; the file holds every FAIL from before the +index. The module docstring's "Index mode" states the rule. Until then the listing below applies. 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` diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_skip.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_skip.py index cabd818ad0f..0e720bb514f 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_skip.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_skip.py @@ -471,12 +471,15 @@ def keys_for( rung_cap: str, listing: Callable[[], list[Any]], graphql: Callable[[list[str]], Any] | None = None, + own_verdicts: Callable[[int, str], list[Any]] | None = None, ) -> tuple[dict[int, dict[str, Any]], int]: """Each candidate's key, head and whether it may be skipped at all, and - the GraphQL cost spent. Raises `RuntimeError` when the clone or the - artifact listing cannot be read, so nothing is skipped.""" + the GraphQL cost spent. With `own_verdicts` (the reader's index mode, + `Reader.own_verdicts`) each candidate's verdicts are read by exact name + instead of from the whole listing. Raises `RuntimeError` when the clone + or the artifact reads fail, so nothing is skipped.""" main = main_inputs(base) - artifacts = listing() + artifacts = listing() if own_verdicts is None else [] numbers = [c["number"] for c in candidates if isinstance(c.get("number"), int)] states, cost = read_states(repo, numbers, graphql) keys: dict[int, dict[str, Any]] = {} @@ -486,7 +489,11 @@ def keys_for( state = pr_state(node) if node is not None else None if state is None or state["head"] != head: continue - found = verdicts(artifacts, number, head) + found = verdicts( + artifacts if own_verdicts is None else own_verdicts(number, head), + number, + head, + ) keys[number] = { "key": state_key(candidate, state, found, main, rung_cap), "head": head, @@ -547,6 +554,7 @@ def run_filter(args: argparse.Namespace) -> int: base=args.base, rung_cap=args.rung_cap, listing=reader.listing, + own_verdicts=reader.own_verdicts if reader.index_mode() else None, ) except RuntimeError as exc: print(f"::warning::no candidate is skipped: {exc}") 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 a860432e159..7eae12da6cf 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 @@ -140,6 +140,24 @@ job whose pages over-count refuses without another, whatever that listing's outcome. +Index mode. Before a FAIL uploads, its run uploads one index artifact per +tie, `INDEX_PREFIX` then `pr-`, `sha-`, `ref-` and +`issue-` for each issue the FAIL records (write-verdict.mjs +`failIndexNames`), and the FAIL uploads only if every index did. When the +legacy FAILs file (`LEGACY_FAILS_FILE`, every FAIL uploaded before the index, +built by `legacy-fails` from a listing that holds the live count) loads, a +judgment reads no listing (`Reader._targeted`): the name filter returns every +artifact of an exact name across runs, so it reads the index names of this +pull request's number and each other pull request that closes one of its +live issues, of its head and each of its commits, of its head branch, and of +each live issue, then each run those name (one read each), then the passes +at the head and at each FAILed head by their exact names. Each read must +hold its own `total_count`, which one name's few artifacts do; every rule +above then judges what those reads and the legacy file hold, as it judges a +listing. A run holding an index and no FAIL that has not completed refuses; +one that has completed failed its FAIL upload. A failed read refuses. With +no legacy file the whole listing below is read, as before. + 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 @@ -286,6 +304,21 @@ # `1`: no wait between incremental reads (`no_incremental_waits`). NO_INCREMENTAL_WAITS_ENV = "BABYSIT_VERIFY_NO_INCREMENTAL_WAITS" HEAD_REFS_ENV = "BABYSIT_VERIFY_HEAD_REFS" +# A FAIL's index artifacts (write-verdict.mjs failIndexNames), one per tie: +# `pr-`, `sha-`, `ref-`, `issue-`. +INDEX_PREFIX = "pr-verify-index-" +# The FAILs uploaded before the index (`Reader._legacy`), committed beside +# this module once built (`legacy-fails`); its presence turns the index on. +LEGACY_FAILS_FILE = "verify-legacy-fails.json" +LEGACY_FAILS_ENV = "BABYSIT_VERIFY_LEGACY_FAILS" +LEGACY_FAILS_VERSION = 1 +# The verifier runs one judgment's index reads may lead to, more refusing, +# and the names past which it reads the whole listing instead: each costs +# a read, and a whole listing was about 135 pages on 2026-11-02. +INDEX_RUNS_CAP = 120 +INDEX_NAMES_CAP = 100 +# One line per judgment on stderr: the exact-name reads it made. +INDEX_READS_PREFIX = "babysit-index-reads" # A file holding the pull request's REST record as the caller read it # (`merge_policy.cached_pull_request`). PULL_ENV = "BABYSIT_VERIFY_PULL" @@ -1339,6 +1372,28 @@ class Reader: _linked: dict[int, dict[str, Any]] = field(default_factory=dict) # The issue number `link_refusal` took for each pull request. linked_issue: dict[int, Any] = field(default_factory=dict) + # The legacy FAILs file (`_legacy`): when it loads, judgments read the + # FAIL index by exact name instead of the whole listing. `LEGACY_FAILS_ENV` + # names another file, or none when empty. + legacy_path: str | None = field( + default_factory=lambda: os.environ.get( + LEGACY_FAILS_ENV, str(Path(__file__).with_name(LEGACY_FAILS_FILE)) + ) + or None + ) + _legacy_entries: list[dict[str, Any]] | None = None + _legacy_loaded: bool = False + # Each legacy FAIL's recorded head branch, by artifact id. + _legacy_refs: dict[int, str] = field(default_factory=dict) + # The live issues `_targeted` read, taken by the next `_elsewhere`. + _live_issues: dict[int, dict[Any, Any]] = field(default_factory=dict) + # A targeted listing read for the answer key, taken by the next judgment + # of the same pull request and head (`_verdict_artifacts`). + _handoff: dict[tuple[int, str], list[dict[str, Any]]] = field( + default_factory=dict + ) + # The exact-name and run-artifact reads this Reader made. + index_reads: int = 0 def _listing(self) -> list[dict[str, Any]]: """`_current_listing`, noting when it began to be read @@ -1428,7 +1483,9 @@ def saved(self) -> bool: def refresh(self) -> None: """Drop a saved listing: the next judgment pages the API itself (and - saves that listing for the rest of the run).""" + saves that listing for the rest of the run). In index mode every + judgment reads the index afresh; this drops a kept one too.""" + self._handoff.clear() if self._saved: self._artifacts, self._saved, self._own = None, False, True @@ -2260,6 +2317,8 @@ def _head_ref(self, run_id: int, name: str, artifact_id: Any, head_ref: str) -> dated = run_cache.get_dated(RECORD_SECTION, key, None) if shared else None if dated is not None and is_json_object(dated[0]): return dated[0].get("head_ref") + if shared and artifact_id in self._legacy_refs: + return self._legacy_refs[artifact_id] if shared: if self._mapped is None: self._mapped = load_head_refs(self.head_refs_path, self.repo) @@ -2619,6 +2678,267 @@ def _closing_issues(self, number: int) -> dict[Any, Any]: ) return live + def index_mode(self) -> bool: + """Whether judgments read the FAIL index (`_targeted`): only once the + legacy FAILs file loads, which holds every FAIL uploaded before the + index existed.""" + return self._legacy() is not None + + def _legacy(self) -> list[dict[str, Any]] | None: + """The legacy FAILs file's entries, once loaded: listing-shaped + artifacts, each recording its run's verified origin and its verdict's + head branch. A missing, malformed or other repository's file is none, + so judgments use the whole listing as before. The file is committed + code (`legacy-fails` builds it from a listing holding GitHub's live + count), so its origins and head branches are taken as read.""" + if self._legacy_loaded: + return self._legacy_entries + self._legacy_loaded = True + if not self.legacy_path: + return None + try: + saved = json_object( + json.loads(Path(self.legacy_path).read_text(encoding="utf-8")) + ) + except (OSError, ValueError): + return None + entries = saved.get("artifacts") + if ( + saved.get("version") != LEGACY_FAILS_VERSION + or saved.get("repo") != self.repo + or not is_json_array(entries) + ): + return None + for entry in entries: + run_id = json_object(json_object(entry).get("workflow_run")).get("id") + if ( + not _listed_entry(entry) + or entry.get("origin") is not True + or not isinstance(entry.get("name"), str) + or not _is_id(run_id, run_id) + or not (entry.get("head_ref") is None or isinstance(entry["head_ref"], str)) + ): + return None + for entry in entries: + run_id = entry["workflow_run"]["id"] + self._runs.setdefault(run_id, ORIGIN) + self._shared_runs.add(run_id) + if isinstance(entry.get("head_ref"), str): + self._legacy_refs[entry["id"]] = entry["head_ref"] + self._legacy_entries = entries + return entries + + def _named(self, name: str, deadline: float) -> list[dict[str, Any]]: + """Every unexpired artifact named exactly `name`, under any run: the + API's name filter matches whole names. The pages must hold, each id + once, the `total_count` every page states; anything else, read twice, + raises `_Unreadable`.""" + for _ in range(2): + found: dict[int, dict[str, Any]] = {} + totals: set[Any] = set() + page = 1 + while True: + read = self._api_object( + f"repos/{self.repo}/actions/artifacts" + f"?name={urllib.parse.quote(name)}&per_page={LISTING_PAGE_SIZE}&page={page}", + deadline, + ) + self.index_reads += 1 + if not isinstance(read, tuple): + raise _Unreadable(f"the artifacts named {name} could not be read") + items, count = read[0].get("artifacts"), read[0].get("total_count") + if ( + not is_json_array(items) + or type(count) is not int + or not all(map(_listed_entry, items)) + or any(a.get("name") != name for a in items) + ): + raise _Unreadable(f"the artifacts named {name} are malformed") + totals.add(count) + found.update((a["id"], a) for a in items) + if len(items) < LISTING_PAGE_SIZE or page * LISTING_PAGE_SIZE >= count: + break + if page >= LISTING_TOP_UP_PAGES: + raise _Unreadable(f"the artifacts named {name} run past {page} pages") + page += 1 + if totals == {len(found)}: + return [a for a in found.values() if a["expired"] is False] + raise _Unreadable(f"the artifacts named {name} do not hold their count") + + def _run_artifacts(self, run_id: int, deadline: float) -> list[dict[str, Any]]: + """The unexpired artifacts of a run an index artifact names, in one + read that must hold its `total_count`. A run that holds an index but + no FAIL-shaped name may still be uploading its FAIL: while it has not + completed, `_Unreadable`; once it has, its FAIL upload failed and + there is none.""" + read = self._api_object( + f"repos/{self.repo}/actions/runs/{run_id}/artifacts?per_page={LISTING_PAGE_SIZE}", + deadline, + ) + self.index_reads += 1 + items = read[0].get("artifacts") if isinstance(read, tuple) else None + if ( + not is_json_array(items) + or read[0].get("total_count") != len(items) + or not all(map(_listed_entry, items)) + or any(json_object(a.get("workflow_run")).get("id") != run_id for a in items) + ): + raise _Unreadable(f"the artifacts of run {run_id} could not be read") + items = [a for a in items if a["expired"] is False] + if not any(FAIL_PREFIX_RE.match(str(a.get("name") or "")) for a in items): + record = json_object( + self._actions( + ["api", f"repos/{self.repo}/actions/runs/{run_id}"], f"run {run_id}" + ) + ) + self.index_reads += 1 + if record.get("status") != "completed": + raise _Unreadable(f"run {run_id} holds a FAIL index and is still running") + return items + + def _index_hits(self, names: Iterable[str], deadline: float) -> list[dict[str, Any]]: + """The artifacts, but the index ones, of every run that uploaded an + artifact named one of `names`. An index from any run only points: each + FAIL it leads to is judged by its own run, as in the listing.""" + runs: set[int] = set() + for name in dict.fromkeys(names): + for item in self._named(name, deadline): + run_id = json_object(item.get("workflow_run")).get("id") + if _is_id(run_id, run_id): + runs.add(run_id) + if len(runs) > INDEX_RUNS_CAP: + raise _Unreadable(f"the FAIL index names {len(runs)} runs, over {INDEX_RUNS_CAP}") + return [ + a + for run_id in sorted(runs) + for a in self._run_artifacts(run_id, deadline) + if not str(a.get("name") or "").startswith(INDEX_PREFIX) + ] + + def _passes_named( + self, number: int, shas: Iterable[str], deadline: float + ) -> list[dict[str, Any]]: + """This pull request's passes at each of `shas`, by the exact names + write-verdict.mjs `artifactName` writes for a pass (no digest yet; a + pass this misses leaves a refusal, never an allow).""" + return [ + a + for sha in sorted(set(shas)) + for verdict in PASSES + for a in self._named(f"pr-verify-{number}-{sha}-{verdict}", deadline) + ] + + def _targeted( + self, number: int, head: str, head_ref: str, commits: set[str] + ) -> list[dict[str, Any]]: + """Every verdict artifact a judgment of this pull request at `head` + can count, read by exact name instead of the whole listing: the + legacy FAILs (`_legacy`) and, through the FAIL index, each FAIL tied + by this pull request's number, by its head or one of its commits, by + its head branch, by one of its live issues (`_closing_issues`), or by + the number of another pull request that closes one, with the passes + at the head and at each FAILed head of this pull request. Past + `INDEX_NAMES_CAP` names it is the whole listing. Any read that fails + raises `_Unreadable`.""" + deadline = _listing_clock() + listing_cap()[0] + reads = self.index_reads + live = self._closing_issues(number) + self._live_issues[number] = live + closers = {p for issue in live.values() for p in self._closers(issue)} + names = [f"{INDEX_PREFIX}pr-{n}" for n in sorted({number, *closers})] + names += [f"{INDEX_PREFIX}sha-{sha}" for sha in sorted(commits | {head})] + if head_ref: + names.append(f"{INDEX_PREFIX}ref-{head_ref_digest(head_ref)}") + names += [f"{INDEX_PREFIX}issue-{n}" for n in sorted(live) if type(n) is int] + if len(names) > INDEX_NAMES_CAP: + # More reads than one whole listing costs: read that instead. + return self._listing() + now = self.now or _wall_clock() + found = { + a["id"]: a + for a in (self._legacy() or []) + if a["expires_at"] is None or _when(a["expires_at"]) > now + } + found.update((a["id"], a) for a in self._index_hits(names, deadline)) + failed = { + match[2] + for a in found.values() + if (match := verdict_name(str(a.get("name") or ""))) is not None + and int(match[1]) == number + and match[3] == "fail" + } + found.update( + (a["id"], a) for a in self._passes_named(number, failed | {head}, deadline) + ) + print( + f"{INDEX_READS_PREFIX} " + + json.dumps( + {"pr": number, "names": len(names), "reads": self.index_reads - reads} + ), + file=sys.stderr, + ) + return list(found.values()) + + def targeted(self, number: int, head: str) -> list[dict[str, Any]]: + """`_targeted` for the answer key, kept for this pull request's next + judgment at `head`. Raises `RuntimeError` when it cannot be read.""" + try: + pr = merge_policy.pull_request(self.repo, number) + head_ref = str(json_object(json_object(pr).get("head")).get("ref") or "") + listing = self._targeted( + number, head, head_ref, self._commits(number, head) | {head} + ) + except (_Unreadable, merge_policy.PolicyReadError, postmerge.PostMergeReadError) as exc: + raise RuntimeError(str(exc)) from exc + self._handoff[(number, head)] = listing + return listing + + def own_verdicts(self, number: int, head: str) -> list[dict[str, Any]]: + """The verdict artifacts naming this pull request or `head`, through + the FAIL index and the legacy FAILs, with the passes at the head and + at each FAILed head: babysit_merge_skip.py's key. Raises + `RuntimeError` when a read fails.""" + deadline = _listing_clock() + listing_cap()[0] + now = self.now or _wall_clock() + try: + found = { + a["id"]: a + for a in (self._legacy() or []) + if a["expires_at"] is None or _when(a["expires_at"]) > now + } + found.update( + (a["id"], a) + for a in self._index_hits( + [f"{INDEX_PREFIX}pr-{number}", f"{INDEX_PREFIX}sha-{head}"], deadline + ) + ) + failed = { + match[2] + for a in found.values() + if (match := verdict_name(str(a.get("name") or ""))) is not None + and int(match[1]) == number + and match[3] == "fail" + } + found.update( + (a["id"], a) + for a in self._passes_named(number, failed | {head}, deadline) + ) + except _Unreadable as exc: + raise RuntimeError(str(exc)) from exc + return list(found.values()) + + def _verdict_artifacts( + self, number: int, head: str, head_ref: str, commits: set[str] + ) -> list[dict[str, Any]]: + """What a judgment iterates: the whole listing, or in index mode the + targeted one (the answer key's, when it read one for this head).""" + if not self.index_mode(): + return self._listing() + handed = self._handoff.pop((number, head), None) + if handed is not None: + return handed + return self._targeted(number, head, head_ref, commits) + def fail_findings(self, number: int) -> list[dict[str, Any]]: """What each counted FAIL of the pull request the last judgment of it counted wrote: head, run, and the verdict file's summary and notes @@ -2656,9 +2976,20 @@ def _own_fails(self, number: int) -> dict[str, list[tuple[int, str, Any]]]: """The pull request's own FAIL artifacts in the listing, by head, whose runs are not of another origin and did not judge a run that started on the draft (`_draft_judged`); one whose run cannot be read - is kept, so `fail_findings` names it unreadable.""" + is kept, so `fail_findings` names it unreadable. In index mode the + legacy FAILs and the `pr-` index stand in for the listing.""" own: dict[str, list[tuple[int, str, Any]]] = {} - for item in map(json_object, self._listing()): + artifacts = ( + [ + *(self._legacy() or []), + *self._index_hits( + [f"{INDEX_PREFIX}pr-{number}"], _listing_clock() + listing_cap()[0] + ), + ] + if self.index_mode() + else self._listing() + ) + for item in map(json_object, artifacts): match = verdict_name(str(item.get("name") or "")) run_id = json_object(item.get("workflow_run")).get("id") if ( @@ -2873,7 +3204,9 @@ def _evidence(self, number: int, head: str, pr: dict[str, Any] | None) -> Eviden # request and head, for the cap's fresh-attempt floor (`_elsewhere`). fail_times: dict[tuple[int, str], list[Any]] = {} self._fail_runs = {} - for item in map(json_object, self._listing()): + for item in map( + json_object, self._verdict_artifacts(number, head, head_ref, commits) + ): name = str(item.get("name") or "") match = verdict_name(name) if item.get("expired") is not False: @@ -3085,8 +3418,11 @@ def _elsewhere( one of its FAILs could count, a failed read raising `_Unreadable`. A FAIL of a counted one whose run cannot be read (`unread_prs`, by upload time) raises `_Unreadable` (`SIBLING_UNREADABLE`) unless it - predates the record on every issue that pull request is for.""" - live = self._closing_issues(number) + predates the record on every issue that pull request is for. In + index mode the issues are those `_targeted` read its index by.""" + live = self._live_issues.pop(number, None) + if live is None: + live = self._closing_issues(number) closers = {n: self._closers(issue) for n, issue in live.items()} floors = {n: self._attempt_floor(issue) for n, issue in live.items()} @@ -3873,10 +4209,11 @@ def _decides(report: dict[str, Any]) -> tuple[Any, ...]: def _code_digest() -> str: - """The reader's code: every module beside this one. A deploy that - changes any of them invalidates every kept answer.""" + """The reader's code: every module beside this one, and the legacy FAILs + file. A deploy that changes any of them invalidates every kept answer.""" digest = hashlib.sha256() - for path in sorted(Path(__file__).resolve().parent.glob("*.py")): + here = Path(__file__).resolve().parent + for path in sorted([*here.glob("*.py"), *here.glob(LEGACY_FAILS_FILE)]): digest.update(path.name.encode("utf-8") + b"\0" + path.read_bytes() + b"\0") return digest.hexdigest() @@ -4046,7 +4383,12 @@ def _answer_key( config = merge_policy.with_default_branch(args.repo, base) parts["config"] = asdict(config) bot = (config.agent_identity or "").removesuffix("[bot]") or None - listing = reader.listing() + # In index mode only the verdict artifacts this judgment can count. + listing = ( + reader.targeted(args.number, args.head) + if reader.index_mode() + else reader.listing() + ) # `_task_list` reads the rules at the default branch, whatever the base. default = merge_policy._default_branch(args.repo, shared=True) except (merge_policy.PolicyReadError, RuntimeError) as exc: @@ -4932,6 +5274,103 @@ def _capped_close_main(args: argparse.Namespace, now: datetime | None) -> int: return 0 +RUNNING_STATUSES = ("queued", "in_progress", "waiting", "requested", "pending") + + +def build_legacy_fails(reader: Reader, cutover: datetime, now: datetime) -> dict[str, Any]: + """The legacy FAILs file (`Reader._legacy`): every unexpired FAIL-shaped + artifact whose run uploaded no FAIL index, from one listing that holds + GitHub's live count (`Reader.listing`), each with its run's origin + verified and its verdict's head branch. The cutover is when the index + reached main; a verifier run created before it may still upload a FAIL + with none, so one still running refuses. A run of another origin is left + out, as every judgment ignores it; one that cannot be read, or a head + branch that cannot be read, refuses (`_Unreadable`), so the file never + holds less than a judgment would count.""" + if cutover > now: + raise _Unreadable("the cutover is in the future") + for status in RUNNING_STATUSES: + page = json_object( + reader._actions( + [ + "api", + f"repos/{reader.repo}/actions/workflows/{Path(WORKFLOW_PATH).name}" + f"/runs?status={status}&per_page={LISTING_PAGE_SIZE}", + ], + f"{status} verifier runs", + ) + ) + runs = page.get("workflow_runs") + if not is_json_array(runs) or page.get("total_count") != len(runs): + raise _Unreadable(f"the {status} verifier runs could not be read") + for run in map(json_object, runs): + created = _when(run.get("created_at")) + if created is None or created < cutover: + raise _Unreadable(f"verifier run {run.get('id')} from before the cutover is {status}") + try: + listing = reader.listing() + except RuntimeError as exc: + raise _Unreadable(str(exc)) from exc + indexed = { + json_object(a.get("workflow_run")).get("id") + for a in map(json_object, listing) + if str(a.get("name") or "").startswith(INDEX_PREFIX) + } + entries = [] + for item in map(json_object, listing): + name = str(item.get("name") or "") + run_id = json_object(item.get("workflow_run")).get("id") + if ( + not FAIL_PREFIX_RE.match(name) + or item.get("expired") is not False + or run_id in indexed + ): + continue + if not _is_id(run_id, run_id): + continue # No run record to authenticate it: every judgment ignores it. + state = reader._run_state(run_id, shared=True) + if state == FOREIGN: + continue + if state == UNREAD: + raise _Unreadable(f"the run of {name} cannot be read") + entries.append( + { + "id": item["id"], + "name": name, + "created_at": item.get("created_at"), + "expires_at": item.get("expires_at"), + "expired": False, + "workflow_run": {"id": run_id}, + "origin": True, + "head_ref": reader._head_ref(run_id, name, item["id"], ""), + } + ) + return { + "version": LEGACY_FAILS_VERSION, + "repo": reader.repo, + "cutover": cutover.isoformat(), + "listedAt": reader._fetched_at.isoformat() if reader._fetched_at else None, + "artifacts": sorted(entries, key=lambda e: e["id"]), + } + + +def _legacy_fails_main(args: argparse.Namespace, now: datetime | None) -> int: + """`legacy-fails`: write the legacy FAILs file to `--out`, or exit 1.""" + reader = Reader(args.repo, args.branch, now=now, legacy_path=None) + cutover = _when(args.cutover) + if cutover is None: + print("legacy-fails: --cutover must be a time with a zone", file=sys.stderr) + return 2 + try: + built = build_legacy_fails(reader, cutover, now or _wall_clock()) + except _Unreadable as exc: + print(f"legacy-fails: {exc}", file=sys.stderr) + return 1 + Path(args.out).write_text(json.dumps(built, indent=1) + "\n", encoding="utf-8") + print(f"legacy-fails: {len(built['artifacts'])} FAILs", file=sys.stderr) + return 0 + + def main(argv: list[str] | None = None, *, now: datetime | None = None) -> int: """`evidence `: one pull request's evidence at `head` as one JSON line (`Evidence.report`), as pr-merge's gate judges it. @@ -5032,7 +5471,14 @@ def main(argv: list[str] | None = None, *, now: datetime | None = None) -> int: settle.add_argument("--postmerge-result", required=True) settle.add_argument("--code-sha", required=True) settle.add_argument("--idle-tick", action="store_true") + legacy = sub.add_parser("legacy-fails", allow_abbrev=False) + legacy.add_argument("--repo", required=True) + legacy.add_argument("--branch", default="main") + legacy.add_argument("--cutover", required=True) + legacy.add_argument("--out", required=True) args = parser.parse_args(argv) + if args.command == "legacy-fails": + return _legacy_fails_main(args, now) if args.command == "capped-close": if not ( merge_policy.SHA_RE.fullmatch(args.head) diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py new file mode 100644 index 00000000000..bd6f2f12337 --- /dev/null +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py @@ -0,0 +1,385 @@ +# test-scope: plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py +"""The verdict reader's index mode (`babysit_verify_evidence.Reader._targeted`). + +Expected values come from the module docstring's FAIL rules and +write-verdict.mjs `failIndexNames`: a FAIL uploads one index artifact per tie +(`pr-verify-index-pr-`, `-sha-`, `-ref-`, `-issue-`) +before the FAIL itself, the API's name filter returns every artifact of that +exact name across runs, and the legacy FAILs file holds every FAIL uploaded +before the index. A judgment then reads those names, never the whole listing, +and any read that fails or does not hold its count refuses. +""" + +from __future__ import annotations + +import json +import pathlib +import re +import sys +import tempfile +import unittest +from datetime import UTC, datetime +from typing import Any +from unittest import mock + +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent)) +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent.parent)) + +import babysit_postmerge as postmerge +import babysit_verify_evidence as ve +import test_babysit_verify_evidence as base +from test_babysit_verify_evidence import ( + EARLIER, + HEAD, + NOW, + PR, + REPO, + UNRELATED, + artifact, + named, + pull, + record, + run, +) + +DIGEST = ve.head_ref_digest("feat/x") + + +def index(tie: str, run_id: int, n: int) -> dict[str, Any]: + """One index artifact of run `run_id`, its id unlike any verdict's.""" + item = artifact(f"{ve.INDEX_PREFIX}{tie}", run_id) + item["id"] = run_id * 10 + n + return item + + +def fail(number: int, sha: str, run_id: int, suffix: str = "") -> list[dict[str, Any]]: + """A FAIL of pull request `number` at `sha` and the index its run + uploads first (write-verdict.mjs failIndexNames).""" + issues = ( + [f"issue-{i}" for i in suffix.removeprefix("-i").split(".")] if suffix else [] + ) + ties = [ + f"pr-{number}", + f"sha-{sha}", + f"ref-{DIGEST if number == PR else 'f' * 16}", + *issues, + ] + return [ + artifact(named(sha, "fail", number) + suffix, run_id), + *(index(tie, run_id, n) for n, tie in enumerate(ties, 1)), + ] + + +class IndexGitHub(base.FakeGitHub): + """The fake GitHub, answering exact-name reads page by page and each + run's artifacts, and counting both.""" + + def __init__(self, *args: Any, **kwargs: Any): + super().__init__(*args, **kwargs) + self.name_total: dict[str, int] = {} + self.still_running: set[int] = set() + # The verifier runs not completed, as the workflow's run list gives them. + self.running: list[dict[str, Any]] = [] + + def rest(self, args, ok, missing, kwargs): # type: ignore[no-untyped-def] + path = next(a for a in args if a.startswith("repos/")) + if not any(k in path for k in self.fail): + by_name = re.fullmatch( + rf"repos/{REPO}/actions/artifacts\?name=([^&]+)&per_page=100&page=([0-9]+)", + path, + ) + if by_name: + held = [a for a in self.artifacts if a.get("name") == by_name[1]] + page = int(by_name[2]) + return ok( + { + "total_count": self.name_total.get(by_name[1], len(held)), + "artifacts": held[(page - 1) * 100 : page * 100], + } + ) + of_run = re.fullmatch( + rf"repos/{REPO}/actions/runs/([0-9]+)/artifacts\?per_page=100", path + ) + if of_run: + held = [ + a + for a in self.artifacts + if a["workflow_run"]["id"] == int(of_run[1]) + ] + return ok({"total_count": len(held), "artifacts": held}) + running = re.fullmatch( + rf"repos/{REPO}/actions/workflows/pr-verify\.yml/runs\?status=([a-z_]+)&per_page=100", + path, + ) + if running: + listed = [r for r in self.running if r["status"] == running[1]] + return ok({"total_count": len(listed), "workflow_runs": listed}) + ran = re.fullmatch(rf"repos/{REPO}/actions/runs/([0-9]+)", path) + if ran and int(ran[1]) in self.still_running: + return ok(run(int(ran[1]), status="in_progress", conclusion=None)) + return super().rest(args, ok, missing, kwargs) + + def whole(self) -> int: + """Reads of the whole listing or its count, which index mode never makes.""" + return ( + self.full_listings() + + self.probes() + + sum("actions/artifacts?per_page=" in " ".join(c) for c in self.calls) + ) + + +class IndexMode(unittest.TestCase): + def setUp(self) -> None: + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + self.legacy_file = pathlib.Path(self.tmp.name, ve.LEGACY_FAILS_FILE) + self.legacy([]) + + def legacy(self, entries: list[dict[str, Any]], **overrides: Any) -> None: + self.legacy_file.write_text( + json.dumps( + { + "version": ve.LEGACY_FAILS_VERSION, + "repo": REPO, + "cutover": "2026-10-15T00:00:00+00:00", + "artifacts": entries, + **overrides, + } + ) + ) + + def reader(self, **kwargs: Any) -> ve.Reader: + return ve.Reader( + REPO, "main", now=NOW, legacy_path=str(self.legacy_file), **kwargs + ) + + def judge(self, fake: IndexGitHub, **kwargs: Any) -> ve.Evidence: + with ( + mock.patch.object(postmerge, "gh_capture", side_effect=fake), + mock.patch.object(ve.merge_policy, "gh_capture", side_effect=fake), + ): + return self.reader(**kwargs).evidence(PR, HEAD, pull()) + + def passing( + self, *extra: dict[str, Any], runs: dict[int, Any] | None = None + ) -> IndexGitHub: + return IndexGitHub( + [artifact(named(HEAD, "pass"), 1), *extra], + {1: run(1), **(runs or {})}, + {named(HEAD, "pass"): record()}, + ) + + def test_a_pass_allows_without_the_whole_listing(self) -> None: + fake = self.passing() + self.assertTrue(self.reader().index_mode()) + self.assertIsNone(self.judge(fake).refusal) + self.assertEqual(fake.whole(), 0) + + def test_no_legacy_file_keeps_the_whole_listing(self) -> None: + for text in ( + None, + "{not json", + json.dumps({"version": 1, "repo": "other/repo", "artifacts": []}), + ): + if text is None: + self.legacy_file.unlink() + else: + self.legacy_file.write_text(text) + self.assertFalse(self.reader().index_mode(), text) + self.assertFalse(ve.Reader(REPO, "main", legacy_path=None).index_mode()) + + def test_a_fail_at_the_head_under_another_number_refuses(self) -> None: + # Found by the head's sha index alone: PR 90 is tied to nothing else. + fake = self.passing(*fail(90, HEAD, 3), runs={3: run(3)}) + self.assertEqual(self.judge(fake).refusal, ve.FAIL) + self.assertEqual(fake.whole(), 0) + + def test_a_fail_at_an_earlier_head_refuses(self) -> None: + fake = self.passing( + *fail(PR, EARLIER, 3), runs={3: run(3, created_at="2026-10-16T00:00:00Z")} + ) + self.assertEqual(self.judge(fake).refusal, ve.EARLIER_FAIL) + + def test_a_fail_tied_by_head_branch_alone_refuses(self) -> None: + # Another number and head; its run uploaded this branch's ref index, + # and its verdict records the branch. + items = [ + artifact(named(UNRELATED, "fail", 90), 3), + index(f"ref-{DIGEST}", 3, 1), + ] + fake = self.passing(*items, runs={3: run(3, created_at="2026-10-16T00:00:00Z")}) + fake.records[named(UNRELATED, "fail", 90)] = record( + 90, UNRELATED, verdict="fail" + ) + self.assertEqual(self.judge(fake).refusal, ve.EARLIER_FAIL) + + def test_a_fail_naming_a_live_issue_counts_toward_the_cap(self) -> None: + # PR 60 closes nothing; its FAIL names issue 7, which PR 41 closes. + own = fail(PR, HEAD, 3) + other = fail(60, UNRELATED, 4, "-i7") + fake = self.passing(*own, *other, runs={3: run(3), 4: run(4)}) + fake.pulls[60] = { + "state": "open", + "user": {"login": "maintainer", "type": "User"}, + } + self.assertEqual(self.judge(fake, max_fails=2).refusal, ve.REPEATED_FAIL) + + def test_a_legacy_fail_counts_on_its_recorded_origin(self) -> None: + old = artifact(named(EARLIER, "fail"), 5) + self.legacy( + [old | {"workflow_run": {"id": 5}, "origin": True, "head_ref": "feat/x"}] + ) + # Its origin is the file's, read with no call. + idle = IndexGitHub([], {}) + with mock.patch.object(postmerge, "gh_capture", side_effect=idle): + reader = self.reader() + self.assertTrue(reader.index_mode()) + self.assertEqual(reader._run_state(5, shared=True), ve.ORIGIN) + self.assertEqual(idle.calls, []) + # Its creation time is still read fresh, against the pass's. + fake = self.passing(runs={5: run(5, created_at="2026-10-16T00:00:00Z")}) + self.assertEqual(self.judge(fake).refusal, ve.EARLIER_FAIL) + + def test_an_expired_legacy_fail_no_longer_counts(self) -> None: + old = artifact(named(EARLIER, "fail"), 5) | { + "expires_at": "2026-10-15T00:00:00Z" + } + self.legacy( + [old | {"workflow_run": {"id": 5}, "origin": True, "head_ref": "feat/x"}] + ) + self.assertIsNone(self.judge(self.passing()).refusal) + + def test_an_index_whose_run_still_uploads_refuses(self) -> None: + fake = self.passing(index(f"pr-{PR}", 3, 1), runs={3: run(3)}) + fake.still_running.add(3) + self.assertEqual(self.judge(fake).refusal, ve.UNREADABLE) + # Once that run completes with no FAIL, its upload failed: none counts. + fake.still_running.clear() + self.assertIsNone(self.judge(fake).refusal) + + def test_a_name_read_that_fails_or_misses_its_count_refuses(self) -> None: + fake = self.passing() + fake.fail.add(f"name={ve.INDEX_PREFIX}sha-{HEAD}") + self.assertEqual(self.judge(fake).refusal, ve.UNREADABLE) + fake = self.passing() + fake.name_total[f"{ve.INDEX_PREFIX}pr-{PR}"] = 1 + self.assertEqual(self.judge(fake).refusal, ve.UNREADABLE) + + def test_more_names_than_a_listing_costs_read_the_listing(self) -> None: + fake = self.passing( + *fail(PR, EARLIER, 3), runs={3: run(3, created_at="2026-10-16T00:00:00Z")} + ) + fake.commits = [f"{n:040x}" for n in range(1, ve.INDEX_NAMES_CAP + 1)] + [HEAD] + self.assertEqual(self.judge(fake).refusal, ve.EARLIER_FAIL) + self.assertGreater(fake.whole(), 0) + + def test_too_many_index_runs_refuse(self) -> None: + many = [index(f"pr-{PR}", 100 + n, 1) for n in range(ve.INDEX_RUNS_CAP + 1)] + self.assertEqual(self.judge(self.passing(*many)).refusal, ve.UNREADABLE) + + def test_own_verdicts_reads_the_pull_request_and_head_names(self) -> None: + fake = self.passing( + *fail(PR, EARLIER, 3), *fail(90, UNRELATED, 4), runs={3: run(3), 4: run(4)} + ) + with ( + mock.patch.object(postmerge, "gh_capture", side_effect=fake), + mock.patch.object(ve.merge_policy, "gh_capture", side_effect=fake), + ): + names = {a["name"] for a in self.reader().own_verdicts(PR, HEAD)} + self.assertEqual(names, {named(HEAD, "pass"), named(EARLIER, "fail")}) + self.assertEqual(fake.whole(), 0) + + +class LegacyFailsBuild(unittest.TestCase): + CUTOVER = datetime(2026, 10, 15, tzinfo=UTC) + + def build(self, fake: IndexGitHub) -> dict[str, Any]: + with ( + mock.patch.object(postmerge, "gh_capture", side_effect=fake), + mock.patch.object(ve.merge_policy, "gh_capture", side_effect=fake), + ): + reader = ve.Reader( + REPO, + "main", + now=NOW, + legacy_path=None, + listing_path=None, + carry_path=None, + ) + return ve.build_legacy_fails(reader, self.CUTOVER, NOW) + + def fake(self) -> IndexGitHub: + fake = IndexGitHub( + [ + artifact(named(EARLIER, "fail"), 5), # before the index + *fail(PR, UNRELATED, 3), # indexed: found by its names + artifact(named(HEAD, "fail", 90), 6), # another workflow's + artifact(named(HEAD, "pass"), 1), + ], + { + 1: run(1), + 3: run(3), + 5: run(5), + 6: run(6, path=".github/workflows/other.yml"), + }, + {named(EARLIER, "fail"): record(sha=EARLIER, verdict="fail")}, + ) + return fake + + def test_it_holds_each_unindexed_fail_of_verifier_origin_with_its_branch( + self, + ) -> None: + built = self.build(self.fake()) + self.assertEqual( + [ + (a["name"], a["workflow_run"], a["origin"], a["head_ref"]) + for a in built["artifacts"] + ], + [(named(EARLIER, "fail"), {"id": 5}, True, "feat/x")], + ) + self.assertEqual(built["repo"], REPO) + # What it writes, the reader takes. + with tempfile.TemporaryDirectory() as tmp: + path = pathlib.Path(tmp, "legacy.json") + path.write_text(json.dumps(built)) + self.assertTrue(ve.Reader(REPO, "main", legacy_path=str(path)).index_mode()) + + def test_a_verifier_run_from_before_the_cutover_still_running_refuses(self) -> None: + fake = self.fake() + fake.running = [ + run( + 9, + status="in_progress", + conclusion=None, + created_at="2026-10-14T23:00:00Z", + ) + ] + with self.assertRaisesRegex(ve._Unreadable, "before the cutover"): + self.build(fake) + fake.running = [ + run( + 9, + status="in_progress", + conclusion=None, + created_at="2026-10-15T01:00:00Z", + ) + ] + self.build(fake) + + def test_an_unreadable_branch_refuses(self) -> None: + fake = self.fake() + fake.records.clear() + with self.assertRaises(ve._Unreadable): + self.build(fake) + + +def setUpModule() -> None: + base.setUpModule() + + +def tearDownModule() -> None: + base.tearDownModule() + + +if __name__ == "__main__": + unittest.main() From fae2a7428bbaec33045d27882bceb762c039ef65 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 2 Nov 2026 00:35:43 -0500 Subject: [PATCH 3/8] fix(source-control): write no unindexed FAIL and reach the legacy file A re-run of a pr-verify run from before the index runs its old workflow, which has no index step, on main's write-verdict.mjs. Only a job that sets FAIL_INDEX may now write a FAIL, so no FAIL appears without its index once this lands. The index carries the issues the FAIL's name carries, so a FAIL whose issues fit no name still uploads under its plain name, as before. ci-status's sparse checkout now takes verify-legacy-fails.json beside the reader, and the babysit test package pins no legacy file, so the suites keep faking the whole listing once the file is committed. The answer key's index branch gets a test. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/pr-verify/workflow.test.mjs | 2 ++ .github/scripts/pr-verify/write-verdict.mjs | 21 +++++++++-------- .../scripts/pr-verify/write-verdict.test.mjs | 10 +++++++- .github/workflows/pr-require-checks.yml | 1 + .github/workflows/pr-verify.yml | 6 +++++ .../scripts/babysit_verify_evidence.py | 4 +++- .../babysit-prs/scripts/tests/__init__.py | 6 +++++ .../tests/test_babysit_verify_answers.py | 23 +++++++++++++++++++ scripts/ci-lanes-pr-hold.test.sh | 2 +- 9 files changed, 62 insertions(+), 13 deletions(-) diff --git a/.github/scripts/pr-verify/workflow.test.mjs b/.github/scripts/pr-verify/workflow.test.mjs index 62e325ecc2b..e14133bc564 100644 --- a/.github/scripts/pr-verify/workflow.test.mjs +++ b/.github/scripts/pr-verify/workflow.test.mjs @@ -153,6 +153,8 @@ test("the upload is each job's only artifact step and follows the written verdic assert.equal(input(uploads[0].block, "retention-days"), "${{ steps.verdict.outputs.retention-days }}", job); // A pass's retention follows this job's resolver (resolve-pr.mjs relinkedSince). assert.match(step("Write the verdict", job), /^ {10}RELINK_SINCE_HEAD: \$\{\{ steps\.pr\.outputs\.relink-since-head \}\}$/m, job); + // Only a job with the index step may write a FAIL (write-verdict.mjs FAIL_INDEX). + assert.match(step("Write the verdict", job), /^ {10}FAIL_INDEX: "1"$/m, job); // A verdict needs a judge step that succeeded, which run-claude-cli never // reports with a lane-class, so a credit stop writes none. assert.match(step("Write the verdict", job), /^ {8}if: \$\{\{ !cancelled\(\) && \(/m, job); diff --git a/.github/scripts/pr-verify/write-verdict.mjs b/.github/scripts/pr-verify/write-verdict.mjs index cd84d4b9b2d..621a5c1e0c2 100644 --- a/.github/scripts/pr-verify/write-verdict.mjs +++ b/.github/scripts/pr-verify/write-verdict.mjs @@ -508,17 +508,13 @@ export function artifactName(verdict) { // The exact names under which a FAIL's index artifacts upload, one per tie the // merge gate's reader looks up (babysit_verify_evidence.INDEX_PREFIX and // `Reader._targeted`): the pull request, the head, the head branch's digest, -// then each issue the FAIL records. The API's name filter matches whole names -// across runs, so each lookup returns every FAIL so tied without the whole -// listing. None starts like a verdict name, which the reader would hold. +// then each issue the FAIL's name carries (`artifactName`), as the reader ties +// a FAIL only to those. The API's name filter matches whole names across runs, +// so each lookup returns every FAIL so tied without the whole listing. None +// starts like a verdict name, which the reader would hold. export const INDEX_PREFIX = "pr-verify-index-"; export function failIndexNames(verdict) { - const issues = [...new Set((verdict.linked_issues ?? []).map((issue) => issue.number))] - .filter((n) => Number.isSafeInteger(n) && n > 0) - .sort((a, b) => a - b); - if (issues.length > NAMED_ISSUES_MAX) { - throw new Error(`the FAIL records ${issues.length} issues, more than its index carries`); - } + const issues = NAME.exec(artifactName(verdict))?.[4]?.split(".") ?? []; return [ `${INDEX_PREFIX}pr-${verdict.pr}`, `${INDEX_PREFIX}sha-${verdict.head_sha}`, @@ -824,7 +820,12 @@ export function main(env = process.env) { mkdirSync(env.VERDICT_DIR, { recursive: true }); writeFileSync(path.join(env.VERDICT_DIR, "verdict.json"), `${JSON.stringify(verdict, null, 2)}\n`); const name = artifactName(verdict); - // A FAIL's index (failIndexNames): each artifact holds the FAIL's name. + // A FAIL's index (failIndexNames): each artifact holds the FAIL's name. A + // job without FAIL_INDEX (a re-run of a pr-verify.yml from before the + // index, on this script from main) has no index step, so it writes no FAIL. + if (verdict.verdict === "fail" && env.FAIL_INDEX !== "1") { + throw new Error("a FAIL needs its index step (FAIL_INDEX), which this job lacks"); + } const index = verdict.verdict === "fail" ? failIndexNames(verdict) : []; if (index.length > 0) { writeFileSync(path.join(env.VERDICT_DIR, "fail-index.txt"), `${name}\n`); diff --git a/.github/scripts/pr-verify/write-verdict.test.mjs b/.github/scripts/pr-verify/write-verdict.test.mjs index fb0bf33f2af..77900883fe7 100644 --- a/.github/scripts/pr-verify/write-verdict.test.mjs +++ b/.github/scripts/pr-verify/write-verdict.test.mjs @@ -73,8 +73,9 @@ test("a FAIL's index names each tie the merge gate looks up, none like a verdict // babysit_verify_evidence.FAIL_PREFIX_RE holds anything that starts like a FAIL. assert.doesNotMatch(name, /^pr-verify-[0-9]+-[0-9a-f]{40}-fail/); } + // Issues no name can carry: the plain name, whose index ties none of them. const eleven = Array.from({ length: 11 }, (_, i) => ({ ...ISSUE, number: i + 1 })); - assert.throws(() => failIndexNames({ pr: 41, head_sha: HEAD, head_ref: "feat/x", linked_issues: eleven }), /more than its index carries/); + assert.deepEqual(failIndexNames(fail(eleven)), failIndexNames(fail([]))); }); test("each verdict names its artifact with the PR, the head and the verdict", () => { @@ -509,6 +510,7 @@ test("main writes the verdict file and the artifact name, and nothing on bad out HEAD_REF: "feat/x", SPEC_FILE: spec, GITHUB_RUN_ID: "9001", + FAIL_INDEX: "1", VERDICT_DIR: path.join(dir, "verdict"), GITHUB_OUTPUT: path.join(dir, "out"), }; @@ -525,6 +527,10 @@ test("main writes the verdict file and the artifact name, and nothing on bad out ); // Each index artifact holds the FAIL's name. assert.equal(readFileSync(path.join(env.VERDICT_DIR, "fail-index.txt"), "utf8"), `pr-verify-41-${HEAD}-fail-i7\n`); + // A job with no index step (a re-run of a pr-verify.yml from before it) writes no FAIL. + const unindexed = { ...env, FAIL_INDEX: undefined, VERDICT_DIR: path.join(dir, "unindexed"), GITHUB_OUTPUT: path.join(dir, "out-unindexed") }; + assert.throws(() => main({ ...unindexed, EXECUTION_FILES: execution("fail-unindexed", { verdict: "fail", summary: "wrong", notes: [] }) }), /needs its index step/); + assert.equal(existsSync(unindexed.GITHUB_OUTPUT), false); // The capture summary and the trusted PR text: a missing summary file is // no-app, an unparsable one blocked, and neither fails the verdict. @@ -679,6 +685,7 @@ test("main writes one verdict from every chunk, and none when a chunk leaves non HEAD_REF: "feat/x", SPEC_FILE: spec, GITHUB_RUN_ID: "9001", + FAIL_INDEX: "1", CHUNKS: "2", DIFF_DIR: diff, }; @@ -932,6 +939,7 @@ test("a head of twelve chunks gets a verdict from every chunk, and none when one HEAD_REF: "feat/x", SPEC_FILE: spec, GITHUB_RUN_ID: "9001", + FAIL_INDEX: "1", CHUNKS: String(COUNT), DIFF_DIR: diff, VERDICT_DIR: path.join(dir, `verdict-${label}`), diff --git a/.github/workflows/pr-require-checks.yml b/.github/workflows/pr-require-checks.yml index 5219ad1b599..b95ccbcaea1 100644 --- a/.github/workflows/pr-require-checks.yml +++ b/.github/workflows/pr-require-checks.yml @@ -4943,6 +4943,7 @@ jobs: /.node-version /.github/actions/setup-pinned-node/ /plugins/source-control/skills/babysit-prs/scripts/*.py + /plugins/source-control/skills/babysit-prs/scripts/verify-legacy-fails.json /plugins/source-control/lib/parse-concern-value.sh /plugins/source-control/lib/yaml-subset.awk /.github/scripts/pr-merge/head-ref-map.mjs diff --git a/.github/workflows/pr-verify.yml b/.github/workflows/pr-verify.yml index d9969e455d9..fb687bf0dfb 100644 --- a/.github/workflows/pr-verify.yml +++ b/.github/workflows/pr-verify.yml @@ -1866,6 +1866,8 @@ jobs: SPEC_FILE: ${{ github.workspace }}/pr-verify-data/spec.json SPEC_TEXT_FILE: ${{ github.workspace }}/pr-verify-data/spec-text.json VERDICT_DIR: ${{ runner.temp }}/pr-verify-verdict + # This job uploads a FAIL's index (write-verdict.mjs FAIL_INDEX). + FAIL_INDEX: "1" run: node .base/.github/scripts/pr-verify/write-verdict.mjs # A FAIL uploads its index first (write-verdict.mjs failIndexNames), and @@ -3685,6 +3687,8 @@ jobs: SPEC_FILE: ${{ github.workspace }}/pr-verify-data/spec.json SPEC_TEXT_FILE: ${{ github.workspace }}/pr-verify-data/spec-text.json VERDICT_DIR: ${{ runner.temp }}/pr-verify-verdict + # This job uploads a FAIL's index (write-verdict.mjs FAIL_INDEX). + FAIL_INDEX: "1" run: node .base/.github/scripts/pr-verify/write-verdict.mjs # A FAIL uploads its index first (write-verdict.mjs failIndexNames), and @@ -5238,6 +5242,8 @@ jobs: SPEC_FILE: ${{ github.workspace }}/pr-verify-data/spec.json SPEC_TEXT_FILE: ${{ github.workspace }}/pr-verify-data/spec-text.json VERDICT_DIR: ${{ runner.temp }}/pr-verify-verdict + # This job uploads a FAIL's index (write-verdict.mjs FAIL_INDEX). + FAIL_INDEX: "1" run: node .base/.github/scripts/pr-verify/write-verdict.mjs # A FAIL uploads its index first (write-verdict.mjs failIndexNames), and 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 7eae12da6cf..0fabc8fadc0 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 @@ -143,7 +143,9 @@ Index mode. Before a FAIL uploads, its run uploads one index artifact per tie, `INDEX_PREFIX` then `pr-`, `sha-`, `ref-` and `issue-` for each issue the FAIL records (write-verdict.mjs -`failIndexNames`), and the FAIL uploads only if every index did. When the +`failIndexNames`), and the FAIL uploads only if every index did; a job with +no index step (`FAIL_INDEX` unset: a re-run of an older pr-verify.yml) writes +no FAIL at all. When the legacy FAILs file (`LEGACY_FAILS_FILE`, every FAIL uploaded before the index, built by `legacy-fails` from a listing that holds the live count) loads, a judgment reads no listing (`Reader._targeted`): the name filter returns every diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/__init__.py b/plugins/source-control/skills/babysit-prs/scripts/tests/__init__.py index e69de29bb2d..c7214cc085f 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/__init__.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/__init__.py @@ -0,0 +1,6 @@ +import os + +# The verdict reader's index mode turns on when its legacy FAILs file sits +# beside it (babysit_verify_evidence.LEGACY_FAILS_FILE); these suites fake the +# whole listing, so they read none unless a test names one. +os.environ.setdefault("BABYSIT_VERIFY_LEGACY_FAILS", "") diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_answers.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_answers.py index 7f186b94385..f351f544d20 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_answers.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_answers.py @@ -259,6 +259,29 @@ def key(world: World, **overrides: Any) -> str: class KeyTest(unittest.TestCase): """Each input the read takes moves the key; the lane's own writes do not.""" + def test_in_index_mode_the_key_reads_the_targeted_artifacts_not_the_listing( + self, + ) -> None: + def indexed(world: World) -> tuple[str, mock.MagicMock]: + with ( + mock.patch.object(ve.Reader, "index_mode", return_value=True), + mock.patch.object( + ve.Reader, "targeted", return_value=world.listing + ) as targeted, + mock.patch.object( + ve.Reader, "listing", side_effect=AssertionError("listed") + ), + ): + return key(world), targeted + + world = World() + found, targeted = indexed(world) + targeted.assert_called_once_with(PR, HEAD) + # The same artifacts give the key the listing gives. + self.assertEqual(found, key(World())) + world.listing.append(artifact(f"pr-verify-9-{HEAD}-fail", 5)) + self.assertNotEqual(indexed(world)[0], found) + def moved(self, change: Callable[[World], None], **overrides: Any) -> bool: before = World() after = World() diff --git a/scripts/ci-lanes-pr-hold.test.sh b/scripts/ci-lanes-pr-hold.test.sh index f4c429d5dcc..345265d6366 100755 --- a/scripts/ci-lanes-pr-hold.test.sh +++ b/scripts/ci-lanes-pr-hold.test.sh @@ -463,7 +463,7 @@ else fail "no default-branch checkout named 'Check out main's Node pin and verdict reader' without kept credentials" 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) +want_sparse=(/.node-version '/plugins/source-control/skills/babysit-prs/scripts/*.py' /plugins/source-control/skills/babysit-prs/scripts/verify-legacy-fails.json /plugins/source-control/lib/parse-concern-value.sh /plugins/source-control/lib/yaml-subset.awk) # 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) From 2b9d334ceac3bf7da9acd5da4cc9a45a5b698b27 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 2 Nov 2026 00:51:54 -0500 Subject: [PATCH 4/8] chore(source-control): add changelog fragments for the FAIL index Co-Authored-By: Claude Opus 5.5 --- .../pr-pipeline/perf-verdict-reader-name-index-8d2f4b16.md | 7 +++++++ .../perf-verdict-reader-name-index-5c1e7a93.md | 7 +++++++ .../babysit-prs/scripts/tests/test_babysit_verify_index.py | 3 ++- 3 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 .changes/pr-pipeline/perf-verdict-reader-name-index-8d2f4b16.md create mode 100644 .changes/source-control/perf-verdict-reader-name-index-5c1e7a93.md diff --git a/.changes/pr-pipeline/perf-verdict-reader-name-index-8d2f4b16.md b/.changes/pr-pipeline/perf-verdict-reader-name-index-8d2f4b16.md new file mode 100644 index 00000000000..9f9d5232796 --- /dev/null +++ b/.changes/pr-pipeline/perf-verdict-reader-name-index-8d2f4b16.md @@ -0,0 +1,7 @@ +--- +bump: minor +--- + +### Changed + +- A pr-verify FAIL first uploads one index artifact per tie the merge gate counts (`.github/actions/upload-fail-index`), and uploads only once every index did; a job without the index step writes no FAIL. The gate reads those exact names instead of the whole artifact listing once the legacy FAILs file is committed. diff --git a/.changes/source-control/perf-verdict-reader-name-index-5c1e7a93.md b/.changes/source-control/perf-verdict-reader-name-index-5c1e7a93.md new file mode 100644 index 00000000000..4ade71c2b7f --- /dev/null +++ b/.changes/source-control/perf-verdict-reader-name-index-5c1e7a93.md @@ -0,0 +1,7 @@ +--- +bump: minor +--- + +### Added + +- The pr-merge verdict reader has an index mode: once `verify-legacy-fails.json` sits beside it, a judgment finds every FAIL by exact-name reads of the FAIL index (`pr-verify-index-pr-`, `-sha-`, `-ref-`, `-issue-`) and the runs they name, instead of paging the whole artifact listing. Any read that fails or misses its own count refuses. `babysit_verify_evidence.py legacy-fails` builds that file from a listing that holds GitHub's live count. diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py index bd6f2f12337..fe6cbe4f943 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py @@ -214,7 +214,8 @@ def test_a_fail_tied_by_head_branch_alone_refuses(self) -> None: self.assertEqual(self.judge(fake).refusal, ve.EARLIER_FAIL) def test_a_fail_naming_a_live_issue_counts_toward_the_cap(self) -> None: - # PR 60 closes nothing; its FAIL names issue 7, which PR 41 closes. + # Pull request 60 closes nothing; its FAIL's name carries the issue + # the judged pull request closes. own = fail(PR, HEAD, 3) other = fail(60, UNRELATED, 4, "-i7") fake = self.passing(*own, *other, runs={3: run(3), 4: run(4)}) From ea4f4f058dd87e3c031ba05ab7d7c06a4cb2a14a Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 2 Nov 2026 01:07:24 -0500 Subject: [PATCH 5/8] feat(source-control): build the legacy FAILs file from verifier runs legacy-fails no longer depends on an artifact listing settling. It lists every pr-verify.yml run in one-day created windows (split at the 1,000-result cap), requiring each window's listed ids to equal its total_count, then reads each run's artifacts in one page that must hold its own count. A run not completed or any read that fails or miscounts refuses with nothing written. Every call is paced (0.5 a second by default), capped at 1,800 per rate-limit reset window, and stops on a rate-limit answer, an open breaker file or core remaining under 2,000, with a per-window call line on stderr. A stop exits 3 and resumes from the checkpoint file. The listing build stays only as --cross-check-listing. Co-Authored-By: Claude Opus 5.5 --- ...perf-verdict-reader-name-index-5c1e7a93.md | 2 +- .../scripts/babysit_verify_evidence.py | 397 +++++++++++++++++- .../tests/test_babysit_verify_index.py | 234 ++++++++++- 3 files changed, 617 insertions(+), 16 deletions(-) diff --git a/.changes/source-control/perf-verdict-reader-name-index-5c1e7a93.md b/.changes/source-control/perf-verdict-reader-name-index-5c1e7a93.md index 4ade71c2b7f..0281a9cbec8 100644 --- a/.changes/source-control/perf-verdict-reader-name-index-5c1e7a93.md +++ b/.changes/source-control/perf-verdict-reader-name-index-5c1e7a93.md @@ -4,4 +4,4 @@ bump: minor ### Added -- The pr-merge verdict reader has an index mode: once `verify-legacy-fails.json` sits beside it, a judgment finds every FAIL by exact-name reads of the FAIL index (`pr-verify-index-pr-`, `-sha-`, `-ref-`, `-issue-`) and the runs they name, instead of paging the whole artifact listing. Any read that fails or misses its own count refuses. `babysit_verify_evidence.py legacy-fails` builds that file from a listing that holds GitHub's live count. +- The pr-merge verdict reader has an index mode: once `verify-legacy-fails.json` sits beside it, a judgment finds every FAIL by exact-name reads of the FAIL index (`pr-verify-index-pr-`, `-sha-`, `-ref-`, `-issue-`) and the runs they name, instead of paging the whole artifact listing. Any read that fails or misses its own count refuses. `babysit_verify_evidence.py legacy-fails` builds that file from every pr-verify run in one-day `created` windows and each run's artifacts, each read holding its own `total_count`, paced, capped per rate-limit window and resumable from a checkpoint. 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 0fabc8fadc0..df5c4a15041 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 @@ -147,7 +147,7 @@ no index step (`FAIL_INDEX` unset: a re-run of an older pr-verify.yml) writes no FAIL at all. When the legacy FAILs file (`LEGACY_FAILS_FILE`, every FAIL uploaded before the index, -built by `legacy-fails` from a listing that holds the live count) loads, a +built by `legacy-fails` from the verifier's runs and their artifacts) loads, a judgment reads no listing (`Reader._targeted`): the name filter returns every artifact of an exact name across runs, so it reads the index names of this pull request's number and each other pull request that closes one of its @@ -274,7 +274,7 @@ import time import urllib.parse import zipfile -from collections.abc import Callable, Iterable +from collections.abc import Callable, Iterable, Iterator from dataclasses import asdict, dataclass, field, replace from datetime import UTC, datetime, timedelta from email.utils import parsedate_to_datetime @@ -2691,8 +2691,8 @@ def _legacy(self) -> list[dict[str, Any]] | None: artifacts, each recording its run's verified origin and its verdict's head branch. A missing, malformed or other repository's file is none, so judgments use the whole listing as before. The file is committed - code (`legacy-fails` builds it from a listing holding GitHub's live - count), so its origins and head branches are taken as read.""" + code (`legacy-fails` builds it from every verifier run and its + artifacts), so its origins and head branches are taken as read.""" if self._legacy_loaded: return self._legacy_entries self._legacy_loaded = True @@ -5279,9 +5279,11 @@ def _capped_close_main(args: argparse.Namespace, now: datetime | None) -> int: RUNNING_STATUSES = ("queued", "in_progress", "waiting", "requested", "pending") -def build_legacy_fails(reader: Reader, cutover: datetime, now: datetime) -> dict[str, Any]: - """The legacy FAILs file (`Reader._legacy`): every unexpired FAIL-shaped - artifact whose run uploaded no FAIL index, from one listing that holds +def build_legacy_fails_from_listing( + reader: Reader, cutover: datetime, now: datetime +) -> dict[str, Any]: + """`legacy-fails --cross-check-listing`, only a cross-check: every FAIL-shaped + unexpired artifact whose run uploaded no FAIL index, from one listing that holds GitHub's live count (`Reader.listing`), each with its run's origin verified and its verdict's head branch. The cutover is when the index reached main; a verifier run created before it may still upload a FAIL @@ -5356,23 +5358,381 @@ def build_legacy_fails(reader: Reader, cutover: datetime, now: datetime) -> dict } +LEGACY_CHECKPOINT_VERSION = 1 +# GitHub returns at most 1,000 results for a filtered run search, so a window +# whose count reaches it is split. +RUN_SEARCH_RESULTS_CAP = 1000 +RUN_FIELDS_KEPT = ( + "id", + "status", + "path", + "event", + "head_branch", + "head_sha", + "created_at", +) +RATE_LIMIT_SIGNS = ("rate limit", "HTTP 429", "HTTP 403", "retry-after", "Retry-After") + + +class _BuildStopped(Exception): + """The per-run build stopped for a limit; its checkpoint resumes it.""" + + +@dataclass +class _Pacer: + """Every GitHub call the per-run build makes, paced and counted: at most + `per_second` calls a second and `window_cap` calls a rate-limit window + (the core `reset` the `rate_limit` read gives), stopping on the first + rate-limit or secondary-limit answer. A new window starts only after one + `rate_limit` read that shows at least `min_remaining` and no + `breaker_file`.""" + + real: Callable[..., Any] + per_second: float + window_cap: int + min_remaining: int + breaker_file: str | None + calls: int = 0 + window_calls: int = 0 + window_reset: int | None = None + limited: str | None = None + # Why the build stopped, kept as well as raised: a read that wraps any + # error (`Reader._record`) would otherwise turn a stop into a refusal. + stopped: str | None = None + _last: float | None = None + + def _stop(self, why: str) -> _BuildStopped: + self.stopped = why + return _BuildStopped(why) + + def _pace(self) -> None: + if self._last is not None and self.per_second > 0: + wait = self._last + 1.0 / self.per_second - time.monotonic() + if wait > 0: + _listing_sleep(wait) + self._last = time.monotonic() + + def _raw(self, args: list[str], **kwargs: Any) -> Any: + self._pace() + self.calls += 1 + self.window_calls += 1 + proc = self.real(args, **kwargs) + detail = proc.stderr if isinstance(proc.stderr, str) else "" + if proc.returncode != 0 and any(s in detail for s in RATE_LIMIT_SIGNS): + self.limited = detail.strip()[:300] or "a rate-limit answer" + return proc + + def open_window(self) -> None: + """Read the rate limit and start a window, or raise `_BuildStopped`.""" + if self.breaker_file and Path(self.breaker_file).exists(): + raise self._stop(f"the breaker is on ({self.breaker_file})") + proc = self._raw(["api", "rate_limit"]) + if self.limited: + raise self._stop(f"rate limited: {self.limited}") + try: + core = json_object(json_object(json.loads(proc.stdout)).get("resources")).get("core") + remaining, reset = json_object(core)["remaining"], json_object(core)["reset"] + except (ValueError, KeyError, TypeError) as exc: + raise self._stop(f"the rate limit could not be read: {exc}") from exc + if type(remaining) is not int or type(reset) is not int: + raise self._stop("the rate limit could not be read") + if remaining < self.min_remaining: + raise self._stop(f"core remaining {remaining} is under {self.min_remaining}") + if self.window_reset is not None: + self.report() + self.window_reset, self.window_calls = reset, 1 + + def report(self) -> None: + reset = ( + datetime.fromtimestamp(self.window_reset, UTC).isoformat() + if self.window_reset is not None + else None + ) + print( + f"legacy-fails: window reset={reset} calls={self.window_calls} total={self.calls}", + file=sys.stderr, + ) + + def __call__(self, args: list[str], **kwargs: Any) -> Any: + if self.limited: + raise self._stop(f"rate limited: {self.limited}") + if self.window_reset is None or _wall_clock().timestamp() >= self.window_reset: + self.open_window() + if self.window_calls >= self.window_cap: + raise self._stop( + f"{self.window_calls} calls in the window that resets at " + f"{datetime.fromtimestamp(self.window_reset or 0, UTC).isoformat()}, the cap" + ) + proc = self._raw(args, **kwargs) + if self.limited: + raise self._stop(f"rate limited: {self.limited}") + return proc + + +def _windows(since: datetime, until: datetime) -> list[tuple[datetime, datetime]]: + """Whole UTC days from `since` up to `until`, the last cut at `until`.""" + out, start = [], since + while start < until: + end = min(start + timedelta(days=1), until) + out.append((start, end)) + start = end + return out + + +def _stamp(at: datetime) -> str: + return at.astimezone(UTC).strftime("%Y-%m-%dT%H:%M:%SZ") + + +@dataclass +class _PerRunBuild: + """`build_legacy_fails`, resumable from a checkpoint file.""" + + reader: Reader + since: datetime + cutover: datetime + checkpoint_path: Path + state: dict[str, Any] = field(default_factory=dict) + + def load(self) -> None: + fresh = { + "version": LEGACY_CHECKPOINT_VERSION, + "repo": self.reader.repo, + "since": self.since.isoformat(), + "cutover": self.cutover.isoformat(), + "windows": {}, + "runs": {}, + } + try: + saved = json_object(json.loads(self.checkpoint_path.read_text(encoding="utf-8"))) + except FileNotFoundError: + self.state = fresh + return + except (OSError, ValueError) as exc: + raise _Unreadable(f"the checkpoint cannot be read: {exc}") from exc + if any(saved.get(k) != fresh[k] for k in ("version", "repo", "since", "cutover")): + raise _Unreadable("the checkpoint is for another repository, range or cutover") + self.state = saved + + def save(self) -> None: + tmp = self.checkpoint_path.with_name(self.checkpoint_path.name + ".tmp") + tmp.write_text(json.dumps(self.state), encoding="utf-8") + os.replace(tmp, self.checkpoint_path) + + def _window(self, start: datetime, end: datetime) -> list[dict[str, Any]]: + """Every verifier run created in [start, end): each page holds the + same `total_count`, and the ids, each once, equal it.""" + created = f"{_stamp(start)}..{_stamp(end - timedelta(seconds=1))}" + found: dict[int, dict[str, Any]] = {} + totals: set[Any] = set() + page = 1 + while True: + body = json_object( + self.reader._actions( + [ + "api", + f"repos/{self.reader.repo}/actions/workflows/{Path(WORKFLOW_PATH).name}" + f"/runs?created={created}&per_page={LISTING_PAGE_SIZE}&page={page}", + ], + f"verifier runs created {created}", + ) + ) + runs, count = body.get("workflow_runs"), body.get("total_count") + if not is_json_array(runs) or type(count) is not int: + raise _Unreadable(f"the verifier runs created {created} are malformed") + if count >= RUN_SEARCH_RESULTS_CAP: + if end - start <= timedelta(minutes=1): + raise _Unreadable(f"{count} verifier runs created {created}") + middle = start + (end - start) / 2 + return self._window(start, middle) + self._window(middle, end) + totals.add(count) + for run in map(json_object, runs): + if not _is_id(run.get("id"), run.get("id")): + raise _Unreadable(f"a verifier run created {created} has no id") + found[run["id"]] = { + **{k: run.get(k) for k in RUN_FIELDS_KEPT}, + "repository": { + "full_name": json_object(run.get("repository")).get("full_name") + }, + "head_repository": { + "full_name": json_object(run.get("head_repository")).get("full_name") + }, + } + if len(runs) < LISTING_PAGE_SIZE or page * LISTING_PAGE_SIZE >= count: + break + page += 1 + if totals != {len(found)}: + raise _Unreadable( + f"the verifier runs created {created} list {len(found)} of {sorted(totals)}" + ) + return list(found.values()) + + def _run(self, record: dict[str, Any]) -> list[dict[str, Any]]: + """The FAILs of one completed verifier run, from its artifacts read in + one page that must hold its `total_count`: none for a run that + uploaded a FAIL index or is of another origin.""" + run_id = record["id"] + if record.get("status") != "completed": + raise _Unreadable(f"verifier run {run_id} is {record.get('status')}") + body = json_object( + self.reader._actions( + [ + "api", + f"repos/{self.reader.repo}/actions/runs/{run_id}/artifacts" + f"?per_page={LISTING_PAGE_SIZE}", + ], + f"artifacts of run {run_id}", + ) + ) + items, count = body.get("artifacts"), body.get("total_count") + if ( + not is_json_array(items) + or type(count) is not int + or count != len({json_object(a).get("id") for a in items}) + or count != len(items) + or count >= LISTING_PAGE_SIZE + or not all(map(_listed_entry, items)) + or any(json_object(a.get("workflow_run")).get("id") != run_id for a in items) + ): + raise _Unreadable(f"the artifacts of run {run_id} do not hold their count") + names = [str(a.get("name") or "") for a in items] + fails = [ + a + for a, name in zip(items, names, strict=True) + if FAIL_PREFIX_RE.match(name) and a["expired"] is False + ] + if not fails or any(n.startswith(INDEX_PREFIX) for n in names): + return [] + try: + origin = self.reader._origin(record, run_id, shared=False) + except _Unreadable as exc: + raise _Unreadable(f"the origin of run {run_id}: {exc}") from exc + if not origin: + return [] + return [ + { + "id": a["id"], + "name": a["name"], + "created_at": a.get("created_at"), + "expires_at": a.get("expires_at"), + "expired": False, + "workflow_run": {"id": run_id}, + "origin": True, + "head_ref": self.reader._head_ref(run_id, a["name"], a["id"], ""), + } + for a in fails + ] + + def build(self) -> dict[str, Any]: + self.load() + windows, runs = self.state["windows"], self.state["runs"] + for start, end in _windows(self.since, self.cutover): + key = _stamp(start) + if key not in windows: + windows[key] = self._window(start, end) + self.save() + for record in windows[key]: + if str(record["id"]) not in runs: + runs[str(record["id"])] = self._run(record) + self.save() + entries = [e for listed in runs.values() for e in listed] + return { + "version": LEGACY_FAILS_VERSION, + "repo": self.reader.repo, + "cutover": self.cutover.isoformat(), + "since": self.since.isoformat(), + "runs": len(runs), + "artifacts": sorted(entries, key=lambda e: e["id"]), + } + + +def build_legacy_fails( + reader: Reader, + since: datetime, + cutover: datetime, + now: datetime, + checkpoint: Path, +) -> dict[str, Any]: + """The legacy FAILs file (`Reader._legacy`), built from runs, never from + the artifact listing: every pr-verify.yml run created from `since` to the + cutover, listed in one-day `created` windows (split while a window's count + reaches GitHub's 1,000-result cap) whose listed ids must equal each + window's `total_count`, which holds for a past window since runs are not + deleted inside retention; then each run's artifacts, one page that must + hold its own `total_count`. It keeps every unexpired FAIL-shaped artifact + of a run of verifier origin that uploaded no FAIL index, with its + verdict's head branch. A run not completed, a read that fails or does + not hold its count, an origin or head branch that cannot be read raise + `_Unreadable`; every finished window and run is kept in `checkpoint`, so + a stopped build resumes where it stopped.""" + if not since < cutover <= now: + raise _Unreadable("the range must run from --since to a cutover in the past") + return _PerRunBuild(reader, since, cutover, checkpoint).build() + + def _legacy_fails_main(args: argparse.Namespace, now: datetime | None) -> int: - """`legacy-fails`: write the legacy FAILs file to `--out`, or exit 1.""" + """`legacy-fails`: write the legacy FAILs file to `--out`. Exit 0 written, + 1 refused (nothing written), 2 usage, 3 stopped on a limit (resume from + the checkpoint).""" reader = Reader(args.repo, args.branch, now=now, legacy_path=None) - cutover = _when(args.cutover) - if cutover is None: - print("legacy-fails: --cutover must be a time with a zone", file=sys.stderr) + since, cutover = _when(args.since), _when(args.cutover) + if since is None or cutover is None: + print("legacy-fails: --since and --cutover must be times with a zone", file=sys.stderr) return 2 + pacer = _Pacer( + postmerge.gh_capture, + args.calls_per_second, + args.window_cap, + args.min_remaining, + args.breaker_file, + ) try: - built = build_legacy_fails(reader, cutover, now or _wall_clock()) + with _paced_gh(pacer): + pacer.open_window() + built = build_legacy_fails( + reader, since, cutover, now or _wall_clock(), Path(args.checkpoint) + ) + if args.cross_check_listing: + listed = build_legacy_fails_from_listing(reader, cutover, now or _wall_clock()) + missing = {a["id"] for a in listed["artifacts"]} - { + a["id"] for a in built["artifacts"] + } + if missing: + raise _Unreadable(f"the listing holds FAILs the runs lack: {sorted(missing)}") + except _BuildStopped as exc: + pacer.report() + print(f"legacy-fails: stopped, resume from {args.checkpoint}: {exc}", file=sys.stderr) + return 3 except _Unreadable as exc: - print(f"legacy-fails: {exc}", file=sys.stderr) + pacer.report() + if pacer.stopped: + print( + f"legacy-fails: stopped, resume from {args.checkpoint}: {pacer.stopped}", + file=sys.stderr, + ) + return 3 + print(f"legacy-fails: refused, nothing written: {exc}", file=sys.stderr) return 1 + pacer.report() Path(args.out).write_text(json.dumps(built, indent=1) + "\n", encoding="utf-8") - print(f"legacy-fails: {len(built['artifacts'])} FAILs", file=sys.stderr) + print( + f"legacy-fails: {len(built['artifacts'])} FAILs from {built['runs']} runs, " + f"{pacer.calls} calls", + file=sys.stderr, + ) return 0 +@contextlib.contextmanager +def _paced_gh(pacer: _Pacer) -> Iterator[None]: + """Route the reader's gh calls (`postmerge.gh_capture`) through `pacer`.""" + real = postmerge.gh_capture + postmerge.gh_capture = pacer + try: + yield + finally: + postmerge.gh_capture = real + + def main(argv: list[str] | None = None, *, now: datetime | None = None) -> int: """`evidence `: one pull request's evidence at `head` as one JSON line (`Evidence.report`), as pr-merge's gate judges it. @@ -5476,8 +5836,17 @@ def main(argv: list[str] | None = None, *, now: datetime | None = None) -> int: legacy = sub.add_parser("legacy-fails", allow_abbrev=False) legacy.add_argument("--repo", required=True) legacy.add_argument("--branch", default="main") + legacy.add_argument("--since", required=True) legacy.add_argument("--cutover", required=True) legacy.add_argument("--out", required=True) + legacy.add_argument("--checkpoint", required=True) + # The owner token's 5,000 an hour is shared by every session: about one + # call each 2 s, at most 1,800 a reset window, none under 2,000 left. + legacy.add_argument("--calls-per-second", type=float, default=0.5) + legacy.add_argument("--window-cap", type=int, default=1800) + legacy.add_argument("--min-remaining", type=int, default=2000) + legacy.add_argument("--breaker-file", default=None) + legacy.add_argument("--cross-check-listing", action="store_true") args = parser.parse_args(argv) if args.command == "legacy-fails": return _legacy_fails_main(args, now) diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py index fe6cbe4f943..66f3446f4a0 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py @@ -12,9 +12,12 @@ from __future__ import annotations +import contextlib +import io import json import pathlib import re +import subprocess import sys import tempfile import unittest @@ -307,7 +310,7 @@ def build(self, fake: IndexGitHub) -> dict[str, Any]: listing_path=None, carry_path=None, ) - return ve.build_legacy_fails(reader, self.CUTOVER, NOW) + return ve.build_legacy_fails_from_listing(reader, self.CUTOVER, NOW) def fake(self) -> IndexGitHub: fake = IndexGitHub( @@ -374,6 +377,235 @@ def test_an_unreadable_branch_refuses(self) -> None: self.build(fake) +class RunsGitHub: + """GitHub as the per-run build reads it: the verifier's runs by + `created` window, each run's artifacts, the rate limit, a compare of a + run head against main, and a verdict archive by artifact id.""" + + def __init__(self) -> None: + self.runs = [ + run(5, created_at="2026-10-14T03:00:00Z"), # FAIL before the index + run(3, created_at="2026-10-15T01:00:00Z"), # FAIL with its index + run(1, created_at="2026-10-15T02:00:00Z"), # a pass + run( + 6, created_at="2026-10-15T03:00:00Z", path=".github/workflows/other.yml" + ), + ] + self.artifacts = { + 5: [artifact(named(EARLIER, "fail"), 5)], + 3: fail(PR, UNRELATED, 3), + 1: [artifact(named(HEAD, "pass"), 1)], + 6: [artifact(named(HEAD, "fail", 90), 6)], + } + self.records = {named(EARLIER, "fail"): record(sha=EARLIER, verdict="fail")} + self.window_total: int | None = None + self.run_total: dict[int, int] = {} + self.remaining = 4000 + self.limit_from: int | None = None # the call that answers rate limited + self.calls: list[str] = [] + + def __call__(self, args: list[str], **kwargs: Any) -> subprocess.CompletedProcess: + path = next(a for a in args if a.startswith("repos/") or a == "rate_limit") + self.calls.append(path) + + def ok(payload: Any) -> subprocess.CompletedProcess: + return subprocess.CompletedProcess(args, 0, json.dumps(payload), "") + + if self.limit_from is not None and len(self.calls) >= self.limit_from: + return subprocess.CompletedProcess( + args, 1, "", "gh: API rate limit exceeded (HTTP 403)" + ) + if path == "rate_limit": + reset = 4_000_000_000 + return ok( + {"resources": {"core": {"remaining": self.remaining, "reset": reset}}} + ) + window = re.fullmatch( + rf"repos/{REPO}/actions/workflows/pr-verify\.yml/runs\?created=(\S+)\.\.(\S+)" + r"&per_page=100&page=1", + path, + ) + if window: + listed = [r for r in self.runs if window[1] <= r["created_at"] <= window[2]] + total = len(listed) if self.window_total is None else self.window_total + return ok({"total_count": total, "workflow_runs": listed}) + of_run = re.fullmatch( + rf"repos/{REPO}/actions/runs/([0-9]+)/artifacts\?per_page=100", path + ) + if of_run: + held = self.artifacts.get(int(of_run[1]), []) + total = self.run_total.get(int(of_run[1]), len(held)) + return ok({"total_count": total, "artifacts": held}) + if path.startswith(f"repos/{REPO}/compare/"): + return ok({"status": "ahead"}) + zips = re.fullmatch(rf"repos/{REPO}/actions/artifacts/([0-9]+)/zip", path) + if zips: + name = next( + a["name"] + for held in self.artifacts.values() + for a in held + if a["id"] == int(zips[1]) + ) + return subprocess.CompletedProcess( + args, 0, base.zipped(self.records[name]), "" + ) + raise AssertionError(f"unexpected gh call: {args}") + + +class PerRunLegacyFails(unittest.TestCase): + def setUp(self) -> None: + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + self.out = pathlib.Path(self.tmp.name, "legacy.json") + self.checkpoint = pathlib.Path(self.tmp.name, "checkpoint.json") + + def run_build(self, fake: RunsGitHub, *extra: str) -> tuple[int, str]: + stderr = io.StringIO() + with ( + mock.patch.object(postmerge, "gh_capture", side_effect=fake), + contextlib.redirect_stderr(stderr), + ): + code = ve.main( + [ + "legacy-fails", + "--repo", + REPO, + "--since", + "2026-10-14T00:00:00Z", + "--cutover", + "2026-10-16T00:00:00Z", + "--out", + str(self.out), + "--checkpoint", + str(self.checkpoint), + "--calls-per-second", + "0", + *extra, + ], + now=NOW, + ) + return code, stderr.getvalue() + + def test_it_holds_each_unindexed_fail_of_a_verifier_run(self) -> None: + fake = RunsGitHub() + code, err = self.run_build(fake) + self.assertEqual(code, 0, err) + built = json.loads(self.out.read_text()) + self.assertEqual( + [(a["name"], a["head_ref"], a["origin"]) for a in built["artifacts"]], + [(named(EARLIER, "fail"), "feat/x", True)], + ) + self.assertEqual(built["runs"], 4) + # One rate-limit read, two day windows, four runs, one compare, one download. + self.assertEqual(len(fake.calls), 9) + self.assertIn("calls=9 total=9", err) + self.assertTrue(ve.Reader(REPO, "main", legacy_path=str(self.out)).index_mode()) + + def test_a_window_that_misses_its_count_refuses_and_writes_nothing(self) -> None: + fake = RunsGitHub() + fake.window_total = 9 + code, err = self.run_build(fake) + self.assertEqual(code, 1, err) + self.assertIn("list 1 of [9]", err) + self.assertFalse(self.out.exists()) + + def test_a_run_whose_artifacts_miss_their_count_refuses(self) -> None: + fake = RunsGitHub() + fake.run_total[5] = 2 + code, err = self.run_build(fake) + self.assertEqual(code, 1, err) + self.assertIn("artifacts of run 5 do not hold their count", err) + self.assertFalse(self.out.exists()) + + def test_a_run_not_completed_refuses(self) -> None: + fake = RunsGitHub() + fake.runs[2] = run(1, created_at="2026-10-15T02:00:00Z", status="in_progress") + code, err = self.run_build(fake) + self.assertEqual(code, 1, err) + self.assertIn("verifier run 1 is in_progress", err) + + def test_a_rate_limit_stops_and_the_checkpoint_resumes(self) -> None: + fake = RunsGitHub() + # Calls: rate limit, window 1, run 5, its compare and download, then + # window 2, which answers rate limited. + fake.limit_from = 6 + code, err = self.run_build(fake) + self.assertEqual(code, 3, err) + self.assertIn("stopped, resume from", err) + self.assertFalse(self.out.exists()) + saved = json.loads(self.checkpoint.read_text()) + self.assertEqual(len(saved["windows"]), 1) + self.assertEqual(sorted(saved["runs"]), ["5"]) + # Resumed: nothing read before is read again. + resumed = RunsGitHub() + code, err = self.run_build(resumed) + self.assertEqual(code, 0, err) + self.assertNotIn( + f"repos/{REPO}/actions/runs/5/artifacts?per_page=100", resumed.calls + ) + self.assertEqual(len(json.loads(self.out.read_text())["artifacts"]), 1) + + def test_a_rate_limit_inside_a_verdict_download_still_stops(self) -> None: + # The download's reader wraps every error; the stop is kept anyway. + fake = RunsGitHub() + fake.limit_from = 5 + code, err = self.run_build(fake) + self.assertEqual(code, 3, err) + self.assertEqual(json.loads(self.checkpoint.read_text())["runs"], {}) + + def test_calls_are_paced(self) -> None: + with mock.patch.object(ve, "_listing_sleep") as slept: + stderr = io.StringIO() + with ( + mock.patch.object(postmerge, "gh_capture", side_effect=RunsGitHub()), + contextlib.redirect_stderr(stderr), + ): + ve.main( + [ + "legacy-fails", + "--repo", + REPO, + "--since", + "2026-10-14T00:00:00Z", + "--cutover", + "2026-10-16T00:00:00Z", + "--out", + str(self.out), + "--checkpoint", + str(self.checkpoint), + ], + now=NOW, + ) + # The default 0.5 calls a second: about 2 s before each call after the first. + self.assertEqual(slept.call_count, 8) + self.assertTrue(all(1.9 < c.args[0] <= 2.0 for c in slept.call_args_list)) + + def test_the_window_cap_stops(self) -> None: + code, err = self.run_build(RunsGitHub(), "--window-cap", "4") + self.assertEqual(code, 3, err) + self.assertIn("4 calls in the window", err) + + def test_the_breaker_or_low_remaining_stops_before_any_read(self) -> None: + breaker = pathlib.Path(self.tmp.name, "breaker.txt") + breaker.write_text("tripped") + fake = RunsGitHub() + code, err = self.run_build(fake, "--breaker-file", str(breaker)) + self.assertEqual((code, fake.calls), (3, []), err) + fake = RunsGitHub() + fake.remaining = 1999 + code, err = self.run_build(fake) + self.assertEqual((code, fake.calls), (3, ["rate_limit"]), err) + self.assertIn("core remaining 1999 is under 2000", err) + + def test_a_checkpoint_for_another_range_refuses(self) -> None: + self.checkpoint.write_text( + json.dumps({"version": 1, "repo": REPO, "since": "x"}) + ) + code, err = self.run_build(RunsGitHub()) + self.assertEqual(code, 1, err) + self.assertIn("another repository, range or cutover", err) + + def setUpModule() -> None: base.setUpModule() From 8342d7565f6c176a904afb9f3210c0d1f8a92bf9 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 2 Nov 2026 01:34:36 -0500 Subject: [PATCH 6/8] fix(source-control): build legacy FAILs from runs and the listing's union A FAIL can come from a run created before --since: a re-run keeps its created_at, and a three-job chain can upload hours after it. The build now also reads one whole artifact listing through the pacer, reads each run a listed FAIL names outside every window with the same exact per-run artifact read, and refuses, naming the runs, when a listed FAIL is still missing. The cutover comes from --merge-sha, the commit on main whose pr-verify.yml sets FAIL_INDEX, plus a one-hour margin; an earlier --cutover refuses. The last window now reads the cutover's own second. Also: index uploads overwrite a same-named artifact, so a re-run attempt does not fail on its earlier attempt's index; _own_fails drops expired legacy entries; a permission 403 refuses instead of stopping as a rate limit; the merge-skip docstring names index mode. Co-Authored-By: Claude Opus 5.5 --- ...perf-verdict-reader-name-index-5c1e7a93.md | 3 +- .github/actions/upload-fail-index/action.yml | 26 +- .../babysit-prs/scripts/babysit_merge_skip.py | 3 +- .../scripts/babysit_verify_evidence.py | 342 ++++++++++-------- .../tests/test_babysit_verify_index.py | 234 ++++++------ 5 files changed, 326 insertions(+), 282 deletions(-) diff --git a/.changes/source-control/perf-verdict-reader-name-index-5c1e7a93.md b/.changes/source-control/perf-verdict-reader-name-index-5c1e7a93.md index 0281a9cbec8..9118a5e8f18 100644 --- a/.changes/source-control/perf-verdict-reader-name-index-5c1e7a93.md +++ b/.changes/source-control/perf-verdict-reader-name-index-5c1e7a93.md @@ -4,4 +4,5 @@ bump: minor ### Added -- The pr-merge verdict reader has an index mode: once `verify-legacy-fails.json` sits beside it, a judgment finds every FAIL by exact-name reads of the FAIL index (`pr-verify-index-pr-`, `-sha-`, `-ref-`, `-issue-`) and the runs they name, instead of paging the whole artifact listing. Any read that fails or misses its own count refuses. `babysit_verify_evidence.py legacy-fails` builds that file from every pr-verify run in one-day `created` windows and each run's artifacts, each read holding its own `total_count`, paced, capped per rate-limit window and resumable from a checkpoint. +- The pr-merge verdict reader has an index mode: once `verify-legacy-fails.json` sits beside it, a judgment finds every FAIL by exact-name reads of the FAIL index (`pr-verify-index-pr-`, `-sha-`, `-ref-`, `-issue-`) and the runs they name, instead of paging the whole artifact listing. Any read that fails or misses its own count refuses. +- `babysit_verify_evidence.py legacy-fails --merge-sha ` builds that file. It reads every pr-verify run in one-day `created` windows up to the cutover the merge commit sets, and each run's artifacts, each read holding its own `total_count`. It then adds every run a whole artifact listing names, and refuses when a listed FAIL is still missing. Its calls are paced, capped per rate-limit window and resumable from a checkpoint. diff --git a/.github/actions/upload-fail-index/action.yml b/.github/actions/upload-fail-index/action.yml index 2431c6afbe7..41544a4607e 100644 --- a/.github/actions/upload-fail-index/action.yml +++ b/.github/actions/upload-fail-index/action.yml @@ -54,50 +54,50 @@ runs: done - name: Upload index 1 uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n1 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + with: { name: "${{ steps.slots.outputs.n1 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } - name: Upload index 2 uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n2 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + with: { name: "${{ steps.slots.outputs.n2 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } - name: Upload index 3 uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n3 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + with: { name: "${{ steps.slots.outputs.n3 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } - name: Upload index 4 if: ${{ steps.slots.outputs.n4 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n4 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + with: { name: "${{ steps.slots.outputs.n4 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } - name: Upload index 5 if: ${{ steps.slots.outputs.n5 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n5 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + with: { name: "${{ steps.slots.outputs.n5 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } - name: Upload index 6 if: ${{ steps.slots.outputs.n6 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n6 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + with: { name: "${{ steps.slots.outputs.n6 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } - name: Upload index 7 if: ${{ steps.slots.outputs.n7 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n7 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + with: { name: "${{ steps.slots.outputs.n7 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } - name: Upload index 8 if: ${{ steps.slots.outputs.n8 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n8 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + with: { name: "${{ steps.slots.outputs.n8 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } - name: Upload index 9 if: ${{ steps.slots.outputs.n9 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n9 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + with: { name: "${{ steps.slots.outputs.n9 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } - name: Upload index 10 if: ${{ steps.slots.outputs.n10 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n10 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + with: { name: "${{ steps.slots.outputs.n10 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } - name: Upload index 11 if: ${{ steps.slots.outputs.n11 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n11 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + with: { name: "${{ steps.slots.outputs.n11 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } - name: Upload index 12 if: ${{ steps.slots.outputs.n12 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n12 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + with: { name: "${{ steps.slots.outputs.n12 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } - name: Upload index 13 if: ${{ steps.slots.outputs.n13 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n13 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } + with: { name: "${{ steps.slots.outputs.n13 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_skip.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_skip.py index 0e720bb514f..19146dc8a8e 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_skip.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_skip.py @@ -22,7 +22,8 @@ - the ids of the unexpired `pr-verify---` artifacts naming this pull request or its head, from the repository listing the verdict reader saves for the gate's first candidate (`Reader.listing`), so the read - is not added; + is not added, or, in the reader's index mode, from that candidate's exact + name reads (`Reader.own_verdicts`); - the default branch's blob or tree ids of the gate's config, trusted list, tracker config, the pr-merge workflow, the config resolver that picks the gate's script and flags, and the gate's scripts, read from the `.base` 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 df5c4a15041..f15d1253258 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 @@ -263,6 +263,7 @@ from __future__ import annotations import argparse +import base64 import contextlib import hashlib import io @@ -2730,6 +2731,15 @@ def _legacy(self) -> list[dict[str, Any]] | None: self._legacy_entries = entries return entries + def _live_legacy(self) -> list[dict[str, Any]]: + """The legacy FAILs whose `expires_at` has not passed.""" + now = self.now or _wall_clock() + return [ + a + for a in self._legacy() or [] + if a["expires_at"] is None or _when(a["expires_at"]) > now + ] + def _named(self, name: str, deadline: float) -> list[dict[str, Any]]: """Every unexpired artifact named exactly `name`, under any run: the API's name filter matches whole names. The pages must hold, each id @@ -2855,12 +2865,7 @@ def _targeted( if len(names) > INDEX_NAMES_CAP: # More reads than one whole listing costs: read that instead. return self._listing() - now = self.now or _wall_clock() - found = { - a["id"]: a - for a in (self._legacy() or []) - if a["expires_at"] is None or _when(a["expires_at"]) > now - } + found = {a["id"]: a for a in self._live_legacy()} found.update((a["id"], a) for a in self._index_hits(names, deadline)) failed = { match[2] @@ -2901,13 +2906,8 @@ def own_verdicts(self, number: int, head: str) -> list[dict[str, Any]]: at each FAILed head: babysit_merge_skip.py's key. Raises `RuntimeError` when a read fails.""" deadline = _listing_clock() + listing_cap()[0] - now = self.now or _wall_clock() try: - found = { - a["id"]: a - for a in (self._legacy() or []) - if a["expires_at"] is None or _when(a["expires_at"]) > now - } + found = {a["id"]: a for a in self._live_legacy()} found.update( (a["id"], a) for a in self._index_hits( @@ -2983,7 +2983,7 @@ def _own_fails(self, number: int) -> dict[str, list[tuple[int, str, Any]]]: own: dict[str, list[tuple[int, str, Any]]] = {} artifacts = ( [ - *(self._legacy() or []), + *self._live_legacy(), *self._index_hits( [f"{INDEX_PREFIX}pr-{number}"], _listing_clock() + listing_cap()[0] ), @@ -5276,89 +5276,12 @@ def _capped_close_main(args: argparse.Namespace, now: datetime | None) -> int: return 0 -RUNNING_STATUSES = ("queued", "in_progress", "waiting", "requested", "pending") - - -def build_legacy_fails_from_listing( - reader: Reader, cutover: datetime, now: datetime -) -> dict[str, Any]: - """`legacy-fails --cross-check-listing`, only a cross-check: every FAIL-shaped - unexpired artifact whose run uploaded no FAIL index, from one listing that holds - GitHub's live count (`Reader.listing`), each with its run's origin - verified and its verdict's head branch. The cutover is when the index - reached main; a verifier run created before it may still upload a FAIL - with none, so one still running refuses. A run of another origin is left - out, as every judgment ignores it; one that cannot be read, or a head - branch that cannot be read, refuses (`_Unreadable`), so the file never - holds less than a judgment would count.""" - if cutover > now: - raise _Unreadable("the cutover is in the future") - for status in RUNNING_STATUSES: - page = json_object( - reader._actions( - [ - "api", - f"repos/{reader.repo}/actions/workflows/{Path(WORKFLOW_PATH).name}" - f"/runs?status={status}&per_page={LISTING_PAGE_SIZE}", - ], - f"{status} verifier runs", - ) - ) - runs = page.get("workflow_runs") - if not is_json_array(runs) or page.get("total_count") != len(runs): - raise _Unreadable(f"the {status} verifier runs could not be read") - for run in map(json_object, runs): - created = _when(run.get("created_at")) - if created is None or created < cutover: - raise _Unreadable(f"verifier run {run.get('id')} from before the cutover is {status}") - try: - listing = reader.listing() - except RuntimeError as exc: - raise _Unreadable(str(exc)) from exc - indexed = { - json_object(a.get("workflow_run")).get("id") - for a in map(json_object, listing) - if str(a.get("name") or "").startswith(INDEX_PREFIX) - } - entries = [] - for item in map(json_object, listing): - name = str(item.get("name") or "") - run_id = json_object(item.get("workflow_run")).get("id") - if ( - not FAIL_PREFIX_RE.match(name) - or item.get("expired") is not False - or run_id in indexed - ): - continue - if not _is_id(run_id, run_id): - continue # No run record to authenticate it: every judgment ignores it. - state = reader._run_state(run_id, shared=True) - if state == FOREIGN: - continue - if state == UNREAD: - raise _Unreadable(f"the run of {name} cannot be read") - entries.append( - { - "id": item["id"], - "name": name, - "created_at": item.get("created_at"), - "expires_at": item.get("expires_at"), - "expired": False, - "workflow_run": {"id": run_id}, - "origin": True, - "head_ref": reader._head_ref(run_id, name, item["id"], ""), - } - ) - return { - "version": LEGACY_FAILS_VERSION, - "repo": reader.repo, - "cutover": cutover.isoformat(), - "listedAt": reader._fetched_at.isoformat() if reader._fetched_at else None, - "artifacts": sorted(entries, key=lambda e: e["id"]), - } - - -LEGACY_CHECKPOINT_VERSION = 1 +LEGACY_CHECKPOINT_VERSION = 2 +# The pages the build's whole listing may read (about 135 on 2026-11-02). +LISTING_FAILS_PAGES_CAP = 600 +# How long after the merge commit that brought FAIL_INDEX a run may still have +# been triggered on the older workflow; a later cutover only reads more runs. +CUTOVER_MARGIN = timedelta(hours=1) # GitHub returns at most 1,000 results for a filtered run search, so a window # whose count reaches it is split. RUN_SEARCH_RESULTS_CAP = 1000 @@ -5371,7 +5294,13 @@ def build_legacy_fails_from_listing( "head_sha", "created_at", ) -RATE_LIMIT_SIGNS = ("rate limit", "HTTP 429", "HTTP 403", "retry-after", "Retry-After") +RATE_LIMIT_SIGNS = ( + "rate limit", + "HTTP 429", + "retry-after", + "Retry-After", + "x-ratelimit-remaining: 0", +) class _BuildStopped(Exception): @@ -5469,6 +5398,46 @@ def __call__(self, args: list[str], **kwargs: Any) -> Any: return proc +def _kept_run(run: dict[str, Any]) -> dict[str, Any]: + """The fields of a run record the build keeps: its status and origin.""" + return { + **{k: run.get(k) for k in RUN_FIELDS_KEPT}, + "repository": {"full_name": json_object(run.get("repository")).get("full_name")}, + "head_repository": { + "full_name": json_object(run.get("head_repository")).get("full_name") + }, + } + + +def cutover_of(reader: Reader, merge_sha: str) -> datetime: + """When FAIL_INDEX reached main: the committer date of `merge_sha`, a + commit main holds whose pr-verify.yml sets FAIL_INDEX, plus + `CUTOVER_MARGIN`. Raises `_Unreadable` otherwise.""" + if merge_policy.SHA_RE.fullmatch(merge_sha) is None: + raise _Unreadable("--merge-sha must be a full commit SHA") + commit = json_object( + reader._actions(["api", f"repos/{reader.repo}/commits/{merge_sha}"], "merge commit") + ) + date = _when(json_object(json_object(commit.get("commit")).get("committer")).get("date")) + if date is None: + raise _Unreadable(f"commit {merge_sha} has no committer date") + if not reader._contains(merge_sha): + raise _Unreadable(f"commit {merge_sha} is not on {reader.branch}") + workflow = json_object( + reader._actions( + ["api", f"repos/{reader.repo}/contents/{WORKFLOW_PATH}?ref={merge_sha}"], + "pr-verify.yml at the merge commit", + ) + ) + try: + text = base64.b64decode(str(workflow.get("content") or "")).decode("utf-8") + except ValueError as exc: + raise _Unreadable(f"pr-verify.yml at {merge_sha}: {exc}") from exc + if 'FAIL_INDEX: "1"' not in text: + raise _Unreadable(f"pr-verify.yml at {merge_sha} sets no FAIL_INDEX") + return date + CUTOVER_MARGIN + + def _windows(since: datetime, until: datetime) -> list[tuple[datetime, datetime]]: """Whole UTC days from `since` up to `until`, the last cut at `until`.""" out, start = [], since @@ -5518,10 +5487,13 @@ def save(self) -> None: tmp.write_text(json.dumps(self.state), encoding="utf-8") os.replace(tmp, self.checkpoint_path) - def _window(self, start: datetime, end: datetime) -> list[dict[str, Any]]: - """Every verifier run created in [start, end): each page holds the - same `total_count`, and the ids, each once, equal it.""" - created = f"{_stamp(start)}..{_stamp(end - timedelta(seconds=1))}" + def _window( + self, start: datetime, end: datetime, *, last: bool = False + ) -> list[dict[str, Any]]: + """Every verifier run created in [start, end), or [start, end] for + the `last` window, so the cutover's own second is read: each page + holds the same `total_count`, and the ids, each once, equal it.""" + created = f"{_stamp(start)}..{_stamp(end if last else end - timedelta(seconds=1))}" found: dict[int, dict[str, Any]] = {} totals: set[Any] = set() page = 1 @@ -5543,20 +5515,12 @@ def _window(self, start: datetime, end: datetime) -> list[dict[str, Any]]: if end - start <= timedelta(minutes=1): raise _Unreadable(f"{count} verifier runs created {created}") middle = start + (end - start) / 2 - return self._window(start, middle) + self._window(middle, end) + return self._window(start, middle) + self._window(middle, end, last=last) totals.add(count) for run in map(json_object, runs): if not _is_id(run.get("id"), run.get("id")): raise _Unreadable(f"a verifier run created {created} has no id") - found[run["id"]] = { - **{k: run.get(k) for k in RUN_FIELDS_KEPT}, - "repository": { - "full_name": json_object(run.get("repository")).get("full_name") - }, - "head_repository": { - "full_name": json_object(run.get("head_repository")).get("full_name") - }, - } + found[run["id"]] = _kept_run(run) if len(runs) < LISTING_PAGE_SIZE or page * LISTING_PAGE_SIZE >= count: break page += 1 @@ -5566,10 +5530,11 @@ def _window(self, start: datetime, end: datetime) -> list[dict[str, Any]]: ) return list(found.values()) - def _run(self, record: dict[str, Any]) -> list[dict[str, Any]]: + def _run(self, record: dict[str, Any]) -> dict[str, Any]: """The FAILs of one completed verifier run, from its artifacts read in - one page that must hold its `total_count`: none for a run that - uploaded a FAIL index or is of another origin.""" + one page that must hold its `total_count`, and why it holds none + (`why`: `none`, `indexed` for a run that uploaded a FAIL index, + `foreign` for one of another origin, else `kept`).""" run_id = record["id"] if record.get("status") != "completed": raise _Unreadable(f"verifier run {run_id} is {record.get('status')}") @@ -5600,15 +5565,17 @@ def _run(self, record: dict[str, Any]) -> list[dict[str, Any]]: for a, name in zip(items, names, strict=True) if FAIL_PREFIX_RE.match(name) and a["expired"] is False ] - if not fails or any(n.startswith(INDEX_PREFIX) for n in names): - return [] + if not fails: + return {"why": "none", "entries": []} + if any(n.startswith(INDEX_PREFIX) for n in names): + return {"why": "indexed", "entries": []} try: origin = self.reader._origin(record, run_id, shared=False) except _Unreadable as exc: raise _Unreadable(f"the origin of run {run_id}: {exc}") from exc if not origin: - return [] - return [ + return {"why": "foreign", "entries": []} + return {"why": "kept", "entries": [ { "id": a["id"], "name": a["name"], @@ -5620,27 +5587,107 @@ def _run(self, record: dict[str, Any]) -> list[dict[str, Any]]: "head_ref": self.reader._head_ref(run_id, a["name"], a["id"], ""), } for a in fails - ] + ]} + + def _listing_fails(self) -> list[dict[str, Any]]: + """Every unexpired FAIL-shaped artifact one whole listing shows, by + id, name and run, read page by page. No count is checked: the listing + only adds runs to read, so a page it skips hides nothing the windows + hold.""" + found: dict[Any, dict[str, Any]] = {} + page = 1 + while True: + body = json_object( + self.reader._actions( + [ + "api", + f"repos/{self.reader.repo}/actions/artifacts" + f"?per_page={LISTING_PAGE_SIZE}&page={page}", + ], + f"artifact listing page {page}", + ) + ) + items, count = body.get("artifacts"), body.get("total_count") + if not is_json_array(items) or type(count) is not int: + raise _Unreadable(f"artifact listing page {page} is malformed") + for item in map(json_object, items): + name = str(item.get("name") or "") + if FAIL_PREFIX_RE.match(name) and item.get("expired") is False: + found[item.get("id")] = { + "id": item.get("id"), + "name": name, + "run": json_object(item.get("workflow_run")).get("id"), + } + if len(items) < LISTING_PAGE_SIZE or page * LISTING_PAGE_SIZE >= count: + break + if page >= LISTING_FAILS_PAGES_CAP: + raise _Unreadable(f"the artifact listing runs past {page} pages") + page += 1 + return list(found.values()) + + def _run_record(self, run_id: int) -> dict[str, Any]: + """A run the listing named outside every window, read by id.""" + record = json_object( + self.reader._actions( + ["api", f"repos/{self.reader.repo}/actions/runs/{run_id}"], + f"run {run_id}", + ) + ) + if not _is_id(record.get("id"), run_id): + raise _Unreadable(f"run {run_id} could not be read") + return _kept_run(record) def build(self) -> dict[str, Any]: self.load() windows, runs = self.state["windows"], self.state["runs"] - for start, end in _windows(self.since, self.cutover): + spans = _windows(self.since, self.cutover) + for n, (start, end) in enumerate(spans): key = _stamp(start) if key not in windows: - windows[key] = self._window(start, end) + windows[key] = self._window(start, end, last=n == len(spans) - 1) self.save() for record in windows[key]: if str(record["id"]) not in runs: runs[str(record["id"])] = self._run(record) self.save() - entries = [e for listed in runs.values() for e in listed] + # The listing's FAILs: a run created before `since` (a re-run keeps + # its creation time, and a judgment can upload hours after it) is + # read here, so the file is the union of the windows and the listing. + if "listed" not in self.state: + self.state["listed"] = self._listing_fails() + self.save() + added = [] + for item in self.state["listed"]: + run_id = item["run"] + if not _is_id(run_id, run_id): + continue # No run to authenticate it: every judgment ignores it. + if str(run_id) not in runs: + runs[str(run_id)] = self._run(self._run_record(run_id)) + added.append(run_id) + self.save() + entries = [e for held in runs.values() for e in held["entries"]] + kept = {e["id"] for e in entries} + missing = sorted( + { + item["run"] + for item in self.state["listed"] + if _is_id(item["run"], item["run"]) + and item["id"] not in kept + and runs[str(item["run"])]["why"] not in ("indexed", "foreign") + } + ) + if missing: + raise _Unreadable( + f"the listing holds FAILs no run read holds, in runs {missing}" + ) return { "version": LEGACY_FAILS_VERSION, "repo": self.reader.repo, "cutover": self.cutover.isoformat(), "since": self.since.isoformat(), "runs": len(runs), + "listedFails": len(self.state["listed"]), + "addedFromListing": sorted(added), "artifacts": sorted(entries, key=lambda e: e["id"]), } @@ -5652,18 +5699,21 @@ def build_legacy_fails( now: datetime, checkpoint: Path, ) -> dict[str, Any]: - """The legacy FAILs file (`Reader._legacy`), built from runs, never from - the artifact listing: every pr-verify.yml run created from `since` to the - cutover, listed in one-day `created` windows (split while a window's count - reaches GitHub's 1,000-result cap) whose listed ids must equal each - window's `total_count`, which holds for a past window since runs are not - deleted inside retention; then each run's artifacts, one page that must - hold its own `total_count`. It keeps every unexpired FAIL-shaped artifact - of a run of verifier origin that uploaded no FAIL index, with its - verdict's head branch. A run not completed, a read that fails or does - not hold its count, an origin or head branch that cannot be read raise - `_Unreadable`; every finished window and run is kept in `checkpoint`, so - a stopped build resumes where it stopped.""" + """The legacy FAILs file (`Reader._legacy`), the union of two reads. The + runs: every pr-verify.yml run created from `since` to the cutover (the + cutover's own second included), listed in one-day `created` windows + (split while a window's count reaches GitHub's 1,000-result cap) whose + listed ids must equal each window's `total_count`, which holds for a past + window since runs are not deleted inside retention, then each run's + artifacts, one page that must hold its own `total_count`. The listing: + one whole artifact listing, whose FAILs' runs outside every window (a + re-run keeps its creation time, and a FAIL can upload hours after it) + are read the same way. It keeps every unexpired FAIL-shaped artifact of a + run of verifier origin that uploaded no FAIL index, with its verdict's + head branch. A run not completed, a read that fails or does not hold its + count, an origin or head branch that cannot be read, and a listed FAIL + no run read holds raise `_Unreadable`; every finished window and run is + kept in `checkpoint`, so a stopped build resumes where it stopped.""" if not since < cutover <= now: raise _Unreadable("the range must run from --since to a cutover in the past") return _PerRunBuild(reader, since, cutover, checkpoint).build() @@ -5674,8 +5724,9 @@ def _legacy_fails_main(args: argparse.Namespace, now: datetime | None) -> int: 1 refused (nothing written), 2 usage, 3 stopped on a limit (resume from the checkpoint).""" reader = Reader(args.repo, args.branch, now=now, legacy_path=None) - since, cutover = _when(args.since), _when(args.cutover) - if since is None or cutover is None: + since = _when(args.since) + asked = _when(args.cutover) if args.cutover else None + if since is None or (args.cutover and asked is None): print("legacy-fails: --since and --cutover must be times with a zone", file=sys.stderr) return 2 pacer = _Pacer( @@ -5688,16 +5739,17 @@ def _legacy_fails_main(args: argparse.Namespace, now: datetime | None) -> int: try: with _paced_gh(pacer): pacer.open_window() + cutover = cutover_of(reader, args.merge_sha) + if asked is not None: + if asked < cutover: + raise _Unreadable( + f"--cutover {asked.isoformat()} is before {cutover.isoformat()}, " + "when FAIL_INDEX reached main (with its margin)" + ) + cutover = asked built = build_legacy_fails( reader, since, cutover, now or _wall_clock(), Path(args.checkpoint) ) - if args.cross_check_listing: - listed = build_legacy_fails_from_listing(reader, cutover, now or _wall_clock()) - missing = {a["id"] for a in listed["artifacts"]} - { - a["id"] for a in built["artifacts"] - } - if missing: - raise _Unreadable(f"the listing holds FAILs the runs lack: {sorted(missing)}") except _BuildStopped as exc: pacer.report() print(f"legacy-fails: stopped, resume from {args.checkpoint}: {exc}", file=sys.stderr) @@ -5837,7 +5889,8 @@ def main(argv: list[str] | None = None, *, now: datetime | None = None) -> int: legacy.add_argument("--repo", required=True) legacy.add_argument("--branch", default="main") legacy.add_argument("--since", required=True) - legacy.add_argument("--cutover", required=True) + legacy.add_argument("--merge-sha", required=True) + legacy.add_argument("--cutover", default=None) legacy.add_argument("--out", required=True) legacy.add_argument("--checkpoint", required=True) # The owner token's 5,000 an hour is shared by every session: about one @@ -5846,7 +5899,6 @@ def main(argv: list[str] | None = None, *, now: datetime | None = None) -> int: legacy.add_argument("--window-cap", type=int, default=1800) legacy.add_argument("--min-remaining", type=int, default=2000) legacy.add_argument("--breaker-file", default=None) - legacy.add_argument("--cross-check-listing", action="store_true") args = parser.parse_args(argv) if args.command == "legacy-fails": return _legacy_fails_main(args, now) diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py index 66f3446f4a0..a910a907c9c 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py @@ -12,6 +12,7 @@ from __future__ import annotations +import base64 import contextlib import io import json @@ -21,7 +22,6 @@ import sys import tempfile import unittest -from datetime import UTC, datetime from typing import Any from unittest import mock @@ -34,10 +34,12 @@ from test_babysit_verify_evidence import ( EARLIER, HEAD, + IN_PR, NOW, PR, REPO, UNRELATED, + WORKFLOW, artifact, named, pull, @@ -252,6 +254,13 @@ def test_an_expired_legacy_fail_no_longer_counts(self) -> None: [old | {"workflow_run": {"id": 5}, "origin": True, "head_ref": "feat/x"}] ) self.assertIsNone(self.judge(self.passing()).refusal) + # Nor is it a finding of the pull request's own FAILs. + fake = self.passing() + with ( + mock.patch.object(postmerge, "gh_capture", side_effect=fake), + mock.patch.object(ve.merge_policy, "gh_capture", side_effect=fake), + ): + self.assertEqual(self.reader().fail_findings(PR), []) def test_an_index_whose_run_still_uploads_refuses(self) -> None: fake = self.passing(index(f"pr-{PR}", 3, 1), runs={3: run(3)}) @@ -294,93 +303,15 @@ def test_own_verdicts_reads_the_pull_request_and_head_names(self) -> None: self.assertEqual(fake.whole(), 0) -class LegacyFailsBuild(unittest.TestCase): - CUTOVER = datetime(2026, 10, 15, tzinfo=UTC) - - def build(self, fake: IndexGitHub) -> dict[str, Any]: - with ( - mock.patch.object(postmerge, "gh_capture", side_effect=fake), - mock.patch.object(ve.merge_policy, "gh_capture", side_effect=fake), - ): - reader = ve.Reader( - REPO, - "main", - now=NOW, - legacy_path=None, - listing_path=None, - carry_path=None, - ) - return ve.build_legacy_fails_from_listing(reader, self.CUTOVER, NOW) - - def fake(self) -> IndexGitHub: - fake = IndexGitHub( - [ - artifact(named(EARLIER, "fail"), 5), # before the index - *fail(PR, UNRELATED, 3), # indexed: found by its names - artifact(named(HEAD, "fail", 90), 6), # another workflow's - artifact(named(HEAD, "pass"), 1), - ], - { - 1: run(1), - 3: run(3), - 5: run(5), - 6: run(6, path=".github/workflows/other.yml"), - }, - {named(EARLIER, "fail"): record(sha=EARLIER, verdict="fail")}, - ) - return fake - - def test_it_holds_each_unindexed_fail_of_verifier_origin_with_its_branch( - self, - ) -> None: - built = self.build(self.fake()) - self.assertEqual( - [ - (a["name"], a["workflow_run"], a["origin"], a["head_ref"]) - for a in built["artifacts"] - ], - [(named(EARLIER, "fail"), {"id": 5}, True, "feat/x")], - ) - self.assertEqual(built["repo"], REPO) - # What it writes, the reader takes. - with tempfile.TemporaryDirectory() as tmp: - path = pathlib.Path(tmp, "legacy.json") - path.write_text(json.dumps(built)) - self.assertTrue(ve.Reader(REPO, "main", legacy_path=str(path)).index_mode()) - - def test_a_verifier_run_from_before_the_cutover_still_running_refuses(self) -> None: - fake = self.fake() - fake.running = [ - run( - 9, - status="in_progress", - conclusion=None, - created_at="2026-10-14T23:00:00Z", - ) - ] - with self.assertRaisesRegex(ve._Unreadable, "before the cutover"): - self.build(fake) - fake.running = [ - run( - 9, - status="in_progress", - conclusion=None, - created_at="2026-10-15T01:00:00Z", - ) - ] - self.build(fake) - - def test_an_unreadable_branch_refuses(self) -> None: - fake = self.fake() - fake.records.clear() - with self.assertRaises(ve._Unreadable): - self.build(fake) +MERGE_SHA = "9" * 40 +MERGED_AT = "2026-10-15T20:00:00Z" # its cutover, with the margin: 21:00 class RunsGitHub: - """GitHub as the per-run build reads it: the verifier's runs by - `created` window, each run's artifacts, the rate limit, a compare of a - run head against main, and a verdict archive by artifact id.""" + """GitHub as the per-run build reads it: the merge commit and its + pr-verify.yml, the verifier's runs by `created` window and by id, each + run's artifacts, one whole artifact listing, the rate limit, a compare of + a commit against main, and a verdict archive by artifact id.""" def __init__(self) -> None: self.runs = [ @@ -397,11 +328,17 @@ def __init__(self) -> None: 1: [artifact(named(HEAD, "pass"), 1)], 6: [artifact(named(HEAD, "fail", 90), 6)], } + # Runs outside every window, read only by id. + self.other_runs: dict[int, dict[str, Any]] = {} + # Listed artifacts no run's own read holds. + self.listed_only: list[dict[str, Any]] = [] self.records = {named(EARLIER, "fail"): record(sha=EARLIER, verdict="fail")} + self.workflow = 'env:\n FAIL_INDEX: "1"\n' self.window_total: int | None = None self.run_total: dict[int, int] = {} self.remaining = 4000 self.limit_from: int | None = None # the call that answers rate limited + self.limit_text = "gh: API rate limit exceeded (HTTP 403)" self.calls: list[str] = [] def __call__(self, args: list[str], **kwargs: Any) -> subprocess.CompletedProcess: @@ -412,14 +349,17 @@ def ok(payload: Any) -> subprocess.CompletedProcess: return subprocess.CompletedProcess(args, 0, json.dumps(payload), "") if self.limit_from is not None and len(self.calls) >= self.limit_from: - return subprocess.CompletedProcess( - args, 1, "", "gh: API rate limit exceeded (HTTP 403)" - ) + return subprocess.CompletedProcess(args, 1, "", self.limit_text) if path == "rate_limit": reset = 4_000_000_000 return ok( {"resources": {"core": {"remaining": self.remaining, "reset": reset}}} ) + if path == f"repos/{REPO}/commits/{MERGE_SHA}": + return ok({"sha": MERGE_SHA, "commit": {"committer": {"date": MERGED_AT}}}) + if path == f"repos/{REPO}/contents/{WORKFLOW}?ref={MERGE_SHA}": + content = base64.b64encode(self.workflow.encode()).decode() + return ok({"content": content, "encoding": "base64"}) window = re.fullmatch( rf"repos/{REPO}/actions/workflows/pr-verify\.yml/runs\?created=(\S+)\.\.(\S+)" r"&per_page=100&page=1", @@ -436,6 +376,13 @@ def ok(payload: Any) -> subprocess.CompletedProcess: held = self.artifacts.get(int(of_run[1]), []) total = self.run_total.get(int(of_run[1]), len(held)) return ok({"total_count": total, "artifacts": held}) + by_id = re.fullmatch(rf"repos/{REPO}/actions/runs/([0-9]+)", path) + if by_id: + return ok(self.other_runs[int(by_id[1])]) + if path == f"repos/{REPO}/actions/artifacts?per_page=100&page=1": + items = [a for held in self.artifacts.values() for a in held] + items += self.listed_only + return ok({"total_count": len(items), "artifacts": items}) if path.startswith(f"repos/{REPO}/compare/"): return ok({"status": "ahead"}) zips = re.fullmatch(rf"repos/{REPO}/actions/artifacts/([0-9]+)/zip", path) @@ -452,6 +399,12 @@ def ok(payload: Any) -> subprocess.CompletedProcess: raise AssertionError(f"unexpected gh call: {args}") +# Calls of a whole build: rate limit, merge commit, its compare, its +# pr-verify.yml; window 1, run 5, its compare and download; window 2, runs 3, +# 1 and 6; the listing page. +BUILD_CALLS = 13 + + class PerRunLegacyFails(unittest.TestCase): def setUp(self) -> None: self.tmp = tempfile.TemporaryDirectory() @@ -459,7 +412,9 @@ def setUp(self) -> None: self.out = pathlib.Path(self.tmp.name, "legacy.json") self.checkpoint = pathlib.Path(self.tmp.name, "checkpoint.json") - def run_build(self, fake: RunsGitHub, *extra: str) -> tuple[int, str]: + def run_build( + self, fake: RunsGitHub, *extra: str, pace: bool = False + ) -> tuple[int, str]: stderr = io.StringIO() with ( mock.patch.object(postmerge, "gh_capture", side_effect=fake), @@ -472,14 +427,15 @@ def run_build(self, fake: RunsGitHub, *extra: str) -> tuple[int, str]: REPO, "--since", "2026-10-14T00:00:00Z", + "--merge-sha", + MERGE_SHA, "--cutover", "2026-10-16T00:00:00Z", "--out", str(self.out), "--checkpoint", str(self.checkpoint), - "--calls-per-second", - "0", + *([] if pace else ["--calls-per-second", "0"]), *extra, ], now=NOW, @@ -495,12 +451,60 @@ def test_it_holds_each_unindexed_fail_of_a_verifier_run(self) -> None: [(a["name"], a["head_ref"], a["origin"]) for a in built["artifacts"]], [(named(EARLIER, "fail"), "feat/x", True)], ) - self.assertEqual(built["runs"], 4) - # One rate-limit read, two day windows, four runs, one compare, one download. - self.assertEqual(len(fake.calls), 9) - self.assertIn("calls=9 total=9", err) + self.assertEqual((built["runs"], built["addedFromListing"]), (4, [])) + self.assertEqual(len(fake.calls), BUILD_CALLS) + self.assertIn(f"calls={BUILD_CALLS} total={BUILD_CALLS}", err) self.assertTrue(ve.Reader(REPO, "main", legacy_path=str(self.out)).index_mode()) + def test_the_last_window_reads_the_cutovers_own_second(self) -> None: + fake = RunsGitHub() + self.run_build(fake) + self.assertIn( + f"repos/{REPO}/actions/workflows/pr-verify.yml/runs" + "?created=2026-10-15T00:00:00Z..2026-10-16T00:00:00Z&per_page=100&page=1", + fake.calls, + ) + + def test_a_fail_of_a_run_from_before_since_is_added_through_the_listing( + self, + ) -> None: + # A re-run keeps its creation time, so no window holds run 8. + fake = RunsGitHub() + fake.other_runs[8] = run(8, created_at="2026-10-13T23:00:00Z") + fake.artifacts[8] = [artifact(named(IN_PR, "fail"), 8)] + fake.records[named(IN_PR, "fail")] = record(sha=IN_PR, verdict="fail") + code, err = self.run_build(fake) + self.assertEqual(code, 0, err) + built = json.loads(self.out.read_text()) + self.assertEqual(built["addedFromListing"], [8]) + self.assertEqual( + sorted(a["name"] for a in built["artifacts"]), + sorted([named(EARLIER, "fail"), named(IN_PR, "fail")]), + ) + + def test_a_listed_fail_no_run_read_holds_refuses(self) -> None: + fake = RunsGitHub() + ghost = artifact(named(IN_PR, "fail"), 5) + ghost["id"] = 51 + fake.listed_only.append(ghost) + code, err = self.run_build(fake) + self.assertEqual(code, 1, err) + self.assertIn("the listing holds FAILs no run read holds, in runs [5]", err) + self.assertFalse(self.out.exists()) + + def test_a_cutover_before_the_merge_commit_refuses(self) -> None: + code, err = self.run_build(RunsGitHub(), "--cutover", "2026-10-15T20:30:00Z") + self.assertEqual(code, 1, err) + self.assertIn("when FAIL_INDEX reached main", err) + self.assertFalse(self.out.exists()) + + def test_a_merge_commit_without_fail_index_refuses(self) -> None: + fake = RunsGitHub() + fake.workflow = "env: {}\n" + code, err = self.run_build(fake) + self.assertEqual(code, 1, err) + self.assertIn("sets no FAIL_INDEX", err) + def test_a_window_that_misses_its_count_refuses_and_writes_nothing(self) -> None: fake = RunsGitHub() fake.window_total = 9 @@ -526,9 +530,7 @@ def test_a_run_not_completed_refuses(self) -> None: def test_a_rate_limit_stops_and_the_checkpoint_resumes(self) -> None: fake = RunsGitHub() - # Calls: rate limit, window 1, run 5, its compare and download, then - # window 2, which answers rate limited. - fake.limit_from = 6 + fake.limit_from = 9 # window 2 code, err = self.run_build(fake) self.assertEqual(code, 3, err) self.assertIn("stopped, resume from", err) @@ -548,36 +550,24 @@ def test_a_rate_limit_stops_and_the_checkpoint_resumes(self) -> None: def test_a_rate_limit_inside_a_verdict_download_still_stops(self) -> None: # The download's reader wraps every error; the stop is kept anyway. fake = RunsGitHub() - fake.limit_from = 5 + fake.limit_from = 8 code, err = self.run_build(fake) self.assertEqual(code, 3, err) self.assertEqual(json.loads(self.checkpoint.read_text())["runs"], {}) + def test_a_permission_403_refuses_and_does_not_wait(self) -> None: + fake = RunsGitHub() + fake.limit_from = 6 # run 5's artifacts + fake.limit_text = "gh: Resource not accessible by integration (HTTP 403)" + code, err = self.run_build(fake) + self.assertEqual(code, 1, err) + self.assertIn("refused, nothing written", err) + def test_calls_are_paced(self) -> None: with mock.patch.object(ve, "_listing_sleep") as slept: - stderr = io.StringIO() - with ( - mock.patch.object(postmerge, "gh_capture", side_effect=RunsGitHub()), - contextlib.redirect_stderr(stderr), - ): - ve.main( - [ - "legacy-fails", - "--repo", - REPO, - "--since", - "2026-10-14T00:00:00Z", - "--cutover", - "2026-10-16T00:00:00Z", - "--out", - str(self.out), - "--checkpoint", - str(self.checkpoint), - ], - now=NOW, - ) + self.run_build(RunsGitHub(), pace=True) # The default 0.5 calls a second: about 2 s before each call after the first. - self.assertEqual(slept.call_count, 8) + self.assertEqual(slept.call_count, BUILD_CALLS - 1) self.assertTrue(all(1.9 < c.args[0] <= 2.0 for c in slept.call_args_list)) def test_the_window_cap_stops(self) -> None: @@ -599,7 +589,7 @@ def test_the_breaker_or_low_remaining_stops_before_any_read(self) -> None: def test_a_checkpoint_for_another_range_refuses(self) -> None: self.checkpoint.write_text( - json.dumps({"version": 1, "repo": REPO, "since": "x"}) + json.dumps({"version": 2, "repo": REPO, "since": "x"}) ) code, err = self.run_build(RunsGitHub()) self.assertEqual(code, 1, err) From 696ba56ec612791d7387ccedf2fd16870d6cf7f5 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 2 Nov 2026 02:00:36 -0500 Subject: [PATCH 7/8] fix(source-control): keep earlier FAIL indexes and read re-run and in-flight runs overwrite deletes a same-named artifact before the new upload, so a failed upload on a re-run attempt would drop attempt 1's pointer. The index step now reads the run's artifacts once (holding its count) and skips every index name the run already holds, which already points to this run. legacy-fails now lists runs created in the 31 days before --since (GitHub allows a re-run up to 30 days after the initial run) with the same windowed count check, and reads each one updated since --since; the listing union stays as a second check. It again refuses on any verifier run created at or before the cutover that is still queued or running, read per status with its count, before and after the listing. Co-Authored-By: Claude Opus 5.5 --- .github/actions/upload-fail-index/action.yml | 66 +++++++++++---- .github/scripts/pr-verify/workflow.test.mjs | 34 ++++++++ .github/workflows/pr-verify.yml | 3 + .../scripts/babysit_verify_evidence.py | 80 +++++++++++++++++- .../tests/test_babysit_verify_index.py | 81 ++++++++++++++++--- 5 files changed, 235 insertions(+), 29 deletions(-) diff --git a/.github/actions/upload-fail-index/action.yml b/.github/actions/upload-fail-index/action.yml index 41544a4607e..b568d74bdb4 100644 --- a/.github/actions/upload-fail-index/action.yml +++ b/.github/actions/upload-fail-index/action.yml @@ -7,7 +7,9 @@ description: > artifact listing (babysit_verify_evidence.py `Reader._targeted`). The FAIL's upload must require this step's success: a FAIL with no index is one that reader cannot see, and a failed index upload leaves no verdict at all. More - names than slots fail the step. + names than slots fail the step. A name the run already holds (an earlier + attempt's upload) is skipped: it already points to this run, and replacing + it would delete that pointer before the new upload could fail. inputs: names: @@ -19,6 +21,9 @@ inputs: retention-days: description: The FAIL's retention, which each index keeps. required: true + github-token: + description: A token that can read this run's artifacts (actions read). + required: true runs: using: composite @@ -29,6 +34,9 @@ runs: env: NAMES: ${{ inputs.names }} RETENTION: ${{ inputs.retention-days }} + GH_TOKEN: ${{ inputs.github-token }} + REPO: ${{ github.repository }} + RUN_ID: ${{ github.run_id }} run: | case "$RETENTION" in [1-9] | [1-8][0-9] | 90) echo "retention-days=$RETENTION" >>"$GITHUB_OUTPUT" ;; @@ -42,62 +50,86 @@ runs: echo "::error::${#names[@]} FAIL index names, not 3 to 13" exit 1 fi - for i in "${!names[@]}"; do - case "${names[$i]}" in + # The names this run holds already, every attempt's, in one read + # that must hold its own count. + if ! body=$(gh api "repos/$REPO/actions/runs/$RUN_ID/artifacts?per_page=100"); then + echo "::error::the run's artifacts could not be read" + exit 1 + fi + if ! jq -e '.total_count == (.artifacts | length) and .total_count < 100' <<<"$body" >/dev/null; then + echo "::error::the run's artifacts do not hold their count" + exit 1 + fi + mapfile -t held < <(jq -r '.artifacts[].name' <<<"$body") + slot=0 + for name in "${names[@]}"; do + case "$name" in pr-verify-index-[a-z]*-[0-9a-f]*) ;; *) echo "::error::a FAIL index name no reader takes" exit 1 ;; esac - echo "n$((i + 1))=${names[$i]}" >>"$GITHUB_OUTPUT" + found= + for have in "${held[@]}"; do + if [ "$have" = "$name" ]; then found=1; fi + done + if [ -n "$found" ]; then + echo "$name is held already" + continue + fi + slot=$((slot + 1)) + echo "n$slot=$name" >>"$GITHUB_OUTPUT" done - name: Upload index 1 + if: ${{ steps.slots.outputs.n1 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n1 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } + with: { name: "${{ steps.slots.outputs.n1 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } - name: Upload index 2 + if: ${{ steps.slots.outputs.n2 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n2 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } + with: { name: "${{ steps.slots.outputs.n2 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } - name: Upload index 3 + if: ${{ steps.slots.outputs.n3 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n3 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } + with: { name: "${{ steps.slots.outputs.n3 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } - name: Upload index 4 if: ${{ steps.slots.outputs.n4 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n4 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } + with: { name: "${{ steps.slots.outputs.n4 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } - name: Upload index 5 if: ${{ steps.slots.outputs.n5 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n5 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } + with: { name: "${{ steps.slots.outputs.n5 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } - name: Upload index 6 if: ${{ steps.slots.outputs.n6 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n6 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } + with: { name: "${{ steps.slots.outputs.n6 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } - name: Upload index 7 if: ${{ steps.slots.outputs.n7 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n7 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } + with: { name: "${{ steps.slots.outputs.n7 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } - name: Upload index 8 if: ${{ steps.slots.outputs.n8 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n8 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } + with: { name: "${{ steps.slots.outputs.n8 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } - name: Upload index 9 if: ${{ steps.slots.outputs.n9 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n9 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } + with: { name: "${{ steps.slots.outputs.n9 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } - name: Upload index 10 if: ${{ steps.slots.outputs.n10 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n10 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } + with: { name: "${{ steps.slots.outputs.n10 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } - name: Upload index 11 if: ${{ steps.slots.outputs.n11 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n11 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } + with: { name: "${{ steps.slots.outputs.n11 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } - name: Upload index 12 if: ${{ steps.slots.outputs.n12 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n12 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } + with: { name: "${{ steps.slots.outputs.n12 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } - name: Upload index 13 if: ${{ steps.slots.outputs.n13 != '' }} uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: { name: "${{ steps.slots.outputs.n13 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error, overwrite: true } + with: { name: "${{ steps.slots.outputs.n13 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error } diff --git a/.github/scripts/pr-verify/workflow.test.mjs b/.github/scripts/pr-verify/workflow.test.mjs index e14133bc564..48195f6b981 100644 --- a/.github/scripts/pr-verify/workflow.test.mjs +++ b/.github/scripts/pr-verify/workflow.test.mjs @@ -114,6 +114,39 @@ const upstreamRun = (overrides = {}) => ({ }, }); +// upload-fail-index's "Split the names" step, run with a stand-in gh that +// answers the run's artifacts read with `held` (or fails), returning its exit +// status and the outputs it wrote. +const ACTION = readFileSync(new URL("../../actions/upload-fail-index/action.yml", import.meta.url), "utf8"); +const splitNames = (names, held, { total, fail = false } = {}) => { + const script = /\n {6}run: \|\n([\s\S]*?)\n {4}- name: /.exec(ACTION)[1].replace(/^ {8}/gm, ""); + const dir = tempDir("upload-fail-index-"); + const bin = path.join(dir, "bin"); + mkdirSync(bin); + const body = JSON.stringify({ total_count: total ?? held.length, artifacts: held.map((name) => ({ name })) }); + writeFileSync(path.join(bin, "gh"), fail ? "#!/bin/sh\nexit 1\n" : `#!/bin/sh\ncat <<'JSON'\n${body}\nJSON\n`, { mode: 0o755 }); + const out = path.join(dir, "out"); + writeFileSync(out, ""); + const run = spawnSync("bash", ["--noprofile", "--norc", "-eo", "pipefail", "-c", script], { + env: { PATH: `${bin}:${process.env.PATH}`, NAMES: names.join("\n"), RETENTION: "90", GITHUB_OUTPUT: out, REPO: "o/r", RUN_ID: "1", GH_TOKEN: "t" }, + encoding: "utf8", + }); + return { status: run.status, outputs: readFileSync(out, "utf8") }; +}; + +test("a FAIL index skips the names its run already holds, and never overwrites one", () => { + const names = ["pr-verify-index-pr-41", `pr-verify-index-sha-${"a".repeat(40)}`, "pr-verify-index-ref-0123456789abcdef"]; + assert.equal(splitNames(names, []).outputs, `retention-days=90\n${names.map((n, i) => `n${i + 1}=${n}`).join("\n")}\n`); + // A re-run attempt: the earlier attempt's pr index stays, and only the rest upload. + assert.equal(splitNames(names, [names[0], "pr-verify-41-x-fail"]).outputs, `retention-days=90\nn1=${names[1]}\nn2=${names[2]}\n`); + // A read that fails or does not hold its count uploads nothing. + assert.notEqual(splitNames(names, [], { fail: true }).status, 0); + assert.notEqual(splitNames(names, [names[0]], { total: 2 }).status, 0); + assert.doesNotMatch(ACTION, /overwrite/); + assert.equal((ACTION.match(/uses: actions\/upload-artifact@/g) ?? []).length, 13); + assert.equal((ACTION.match(/if: \$\{\{ steps\.slots\.outputs\.n\d+ != '' \}\}/g) ?? []).length, 13); +}); + test("the upload is each job's only artifact step and follows the written verdict", () => { for (const job of JOBS) { const uploads = STEPS[job].filter((s) => /uses: actions\/upload-artifact@/.test(s.block)); @@ -144,6 +177,7 @@ test("the upload is each job's only artifact step and follows the written verdic assert.match(failIndex, /^ {8}uses: \.\/\.base\/\.github\/actions\/upload-fail-index /m, job); assert.equal(input(failIndex, "names"), "${{ steps.verdict.outputs.index-names }}", job); assert.equal(input(failIndex, "retention-days"), "${{ steps.verdict.outputs.retention-days }}", job); + assert.equal(input(failIndex, "github-token"), "${{ github.token }}", job); assert.match( uploads[0].block, new RegExp(`^ {8}if: >-\\n {10}\\(!cancelled\\(\\) && ${indexed}\\) && steps\\.scan-upload\\.outputs\\.clean == 'true'$`, "m"), diff --git a/.github/workflows/pr-verify.yml b/.github/workflows/pr-verify.yml index fb687bf0dfb..74498fa1fef 100644 --- a/.github/workflows/pr-verify.yml +++ b/.github/workflows/pr-verify.yml @@ -1882,6 +1882,7 @@ jobs: names: ${{ steps.verdict.outputs.index-names }} path: ${{ runner.temp }}/pr-verify-verdict/fail-index.txt retention-days: ${{ steps.verdict.outputs.retention-days }} + github-token: ${{ github.token }} - name: Scan the upload for credentials id: scan-upload @@ -3703,6 +3704,7 @@ jobs: names: ${{ steps.verdict.outputs.index-names }} path: ${{ runner.temp }}/pr-verify-verdict/fail-index.txt retention-days: ${{ steps.verdict.outputs.retention-days }} + github-token: ${{ github.token }} - name: Scan the upload for credentials id: scan-upload @@ -5258,6 +5260,7 @@ jobs: names: ${{ steps.verdict.outputs.index-names }} path: ${{ runner.temp }}/pr-verify-verdict/fail-index.txt retention-days: ${{ steps.verdict.outputs.retention-days }} + github-token: ${{ github.token }} - name: Scan the upload for credentials id: scan-upload 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 f15d1253258..f18860f1d42 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 @@ -5293,7 +5293,17 @@ def _capped_close_main(args: argparse.Namespace, now: datetime | None) -> int: "head_branch", "head_sha", "created_at", + "updated_at", ) +# A run can be re-run up to 30 days after its first run +# (https://docs.github.com/en/actions/how-tos/manage-workflow-runs/re-run-workflows-and-jobs, +# "Re-running workflows and jobs", read 2026-11-02; recheck when that limit +# changes), keeping its `created_at`: so the runs created this long before +# `--since` are listed too, and each one updated since is read. A day over +# the limit covers its boundary. +RERUN_LOOKBACK = timedelta(days=31) +# The statuses of a verifier run not completed yet. +RUNNING_STATUSES = ("queued", "in_progress", "waiting", "requested", "pending") RATE_LIMIT_SIGNS = ( "rate limit", "HTTP 429", @@ -5460,6 +5470,7 @@ class _PerRunBuild: since: datetime cutover: datetime checkpoint_path: Path + lookback: timedelta = RERUN_LOOKBACK state: dict[str, Any] = field(default_factory=dict) def load(self) -> None: @@ -5468,7 +5479,9 @@ def load(self) -> None: "repo": self.reader.repo, "since": self.since.isoformat(), "cutover": self.cutover.isoformat(), + "lookback": self.lookback.total_seconds(), "windows": {}, + "lookbackWindows": {}, "runs": {}, } try: @@ -5478,7 +5491,10 @@ def load(self) -> None: return except (OSError, ValueError) as exc: raise _Unreadable(f"the checkpoint cannot be read: {exc}") from exc - if any(saved.get(k) != fresh[k] for k in ("version", "repo", "since", "cutover")): + if any( + saved.get(k) != fresh[k] + for k in ("version", "repo", "since", "cutover", "lookback") + ): raise _Unreadable("the checkpoint is for another repository, range or cutover") self.state = saved @@ -5637,9 +5653,56 @@ def _run_record(self, run_id: int) -> dict[str, Any]: raise _Unreadable(f"run {run_id} could not be read") return _kept_run(record) + def _running_check(self) -> None: + """No verifier run created before the cutover is still to finish: it + could upload a FAIL with no index after its window was read. Each + status's runs are read in one page that must hold its own + `total_count`, never kept in the checkpoint.""" + for status in RUNNING_STATUSES: + body = json_object( + self.reader._actions( + [ + "api", + f"repos/{self.reader.repo}/actions/workflows/{Path(WORKFLOW_PATH).name}" + f"/runs?status={status}&per_page={LISTING_PAGE_SIZE}", + ], + f"{status} verifier runs", + ) + ) + listed, count = body.get("workflow_runs"), body.get("total_count") + if ( + not is_json_array(listed) + or type(count) is not int + or count != len(listed) + or count >= LISTING_PAGE_SIZE + ): + raise _Unreadable(f"the {status} verifier runs do not hold their count") + for run in map(json_object, listed): + created = _when(run.get("created_at")) + if created is None or created <= self.cutover: + raise _Unreadable( + f"verifier run {run.get('id')} from before the cutover is {status}" + ) + def build(self) -> dict[str, Any]: self.load() windows, runs = self.state["windows"], self.state["runs"] + self._running_check() + # Runs created up to `lookback` before `since` and updated since: a + # re-run keeps its creation time. Each window holds its count. + early = self.state["lookbackWindows"] + for start, end in _windows(self.since - self.lookback, self.since): + key = _stamp(start) + if key not in early: + early[key] = self._window(start, end) + self.save() + for record in early[key]: + updated = _when(record.get("updated_at")) + if (updated is None or updated >= self.since) and str( + record["id"] + ) not in runs: + runs[str(record["id"])] = self._run(record) + self.save() spans = _windows(self.since, self.cutover) for n, (start, end) in enumerate(spans): key = _stamp(start) @@ -5653,9 +5716,11 @@ def build(self) -> dict[str, Any]: # The listing's FAILs: a run created before `since` (a re-run keeps # its creation time, and a judgment can upload hours after it) is # read here, so the file is the union of the windows and the listing. + self._running_check() if "listed" not in self.state: self.state["listed"] = self._listing_fails() self.save() + self._running_check() added = [] for item in self.state["listed"]: run_id = item["run"] @@ -5698,6 +5763,7 @@ def build_legacy_fails( cutover: datetime, now: datetime, checkpoint: Path, + lookback: timedelta = RERUN_LOOKBACK, ) -> dict[str, Any]: """The legacy FAILs file (`Reader._legacy`), the union of two reads. The runs: every pr-verify.yml run created from `since` to the cutover (the @@ -5716,7 +5782,7 @@ def build_legacy_fails( kept in `checkpoint`, so a stopped build resumes where it stopped.""" if not since < cutover <= now: raise _Unreadable("the range must run from --since to a cutover in the past") - return _PerRunBuild(reader, since, cutover, checkpoint).build() + return _PerRunBuild(reader, since, cutover, checkpoint, lookback).build() def _legacy_fails_main(args: argparse.Namespace, now: datetime | None) -> int: @@ -5748,7 +5814,12 @@ def _legacy_fails_main(args: argparse.Namespace, now: datetime | None) -> int: ) cutover = asked built = build_legacy_fails( - reader, since, cutover, now or _wall_clock(), Path(args.checkpoint) + reader, + since, + cutover, + now or _wall_clock(), + Path(args.checkpoint), + timedelta(days=args.rerun_lookback_days), ) except _BuildStopped as exc: pacer.report() @@ -5899,6 +5970,9 @@ def main(argv: list[str] | None = None, *, now: datetime | None = None) -> int: legacy.add_argument("--window-cap", type=int, default=1800) legacy.add_argument("--min-remaining", type=int, default=2000) legacy.add_argument("--breaker-file", default=None) + legacy.add_argument( + "--rerun-lookback-days", type=float, default=RERUN_LOOKBACK.days + ) args = parser.parse_args(argv) if args.command == "legacy-fails": return _legacy_fails_main(args, now) diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py index a910a907c9c..eeedb332caf 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py @@ -334,12 +334,15 @@ def __init__(self) -> None: self.listed_only: list[dict[str, Any]] = [] self.records = {named(EARLIER, "fail"): record(sha=EARLIER, verdict="fail")} self.workflow = 'env:\n FAIL_INDEX: "1"\n' - self.window_total: int | None = None + # A window's stated count, by its start, where it is not its runs'. + self.window_total: dict[str, int] = {} self.run_total: dict[int, int] = {} self.remaining = 4000 self.limit_from: int | None = None # the call that answers rate limited self.limit_text = "gh: API rate limit exceeded (HTTP 403)" self.calls: list[str] = [] + # Verifier runs not completed, by the workflow's status filter. + self.running: list[dict[str, Any]] = [] def __call__(self, args: list[str], **kwargs: Any) -> subprocess.CompletedProcess: path = next(a for a in args if a.startswith("repos/") or a == "rate_limit") @@ -360,6 +363,13 @@ def ok(payload: Any) -> subprocess.CompletedProcess: if path == f"repos/{REPO}/contents/{WORKFLOW}?ref={MERGE_SHA}": content = base64.b64encode(self.workflow.encode()).decode() return ok({"content": content, "encoding": "base64"}) + status = re.fullmatch( + rf"repos/{REPO}/actions/workflows/pr-verify\.yml/runs\?status=([a-z_]+)&per_page=100", + path, + ) + if status: + listed = [r for r in self.running if r["status"] == status[1]] + return ok({"total_count": len(listed), "workflow_runs": listed}) window = re.fullmatch( rf"repos/{REPO}/actions/workflows/pr-verify\.yml/runs\?created=(\S+)\.\.(\S+)" r"&per_page=100&page=1", @@ -367,7 +377,7 @@ def ok(payload: Any) -> subprocess.CompletedProcess: ) if window: listed = [r for r in self.runs if window[1] <= r["created_at"] <= window[2]] - total = len(listed) if self.window_total is None else self.window_total + total = self.window_total.get(window[1], len(listed)) return ok({"total_count": total, "workflow_runs": listed}) of_run = re.fullmatch( rf"repos/{REPO}/actions/runs/([0-9]+)/artifacts\?per_page=100", path @@ -400,9 +410,11 @@ def ok(payload: Any) -> subprocess.CompletedProcess: # Calls of a whole build: rate limit, merge commit, its compare, its -# pr-verify.yml; window 1, run 5, its compare and download; window 2, runs 3, -# 1 and 6; the listing page. -BUILD_CALLS = 13 +# pr-verify.yml (4); the five running statuses (9); 31 lookback windows (40); +# window 1, run 5, its compare and download (44); window 2, runs 3, 1 and 6 +# (48); the running statuses (53), the listing page (54), and the running +# statuses again (59). +BUILD_CALLS = 59 class PerRunLegacyFails(unittest.TestCase): @@ -482,6 +494,57 @@ def test_a_fail_of_a_run_from_before_since_is_added_through_the_listing( sorted([named(EARLIER, "fail"), named(IN_PR, "fail")]), ) + def test_a_run_created_before_since_and_updated_after_it_is_read(self) -> None: + # Run 9 was re-run after `--since`; run 7 was not, so it is not read. + fake = RunsGitHub() + fake.runs += [ + run( + 9, created_at="2026-10-01T00:00:00Z", updated_at="2026-10-14T05:00:00Z" + ), + run( + 7, created_at="2026-10-02T00:00:00Z", updated_at="2026-10-02T01:00:00Z" + ), + ] + fake.artifacts[9] = [artifact(named(IN_PR, "fail"), 9)] + fake.records[named(IN_PR, "fail")] = record(sha=IN_PR, verdict="fail") + code, err = self.run_build(fake) + self.assertEqual(code, 0, err) + built = json.loads(self.out.read_text()) + # Read through its window, not added by the listing. + self.assertEqual(built["addedFromListing"], []) + self.assertIn(named(IN_PR, "fail"), [a["name"] for a in built["artifacts"]]) + self.assertNotIn( + f"repos/{REPO}/actions/runs/7/artifacts?per_page=100", fake.calls + ) + + def test_a_lookback_window_that_misses_its_count_refuses(self) -> None: + fake = RunsGitHub() + fake.window_total["2026-10-01T00:00:00Z"] = 1 + code, err = self.run_build(fake) + self.assertEqual(code, 1, err) + self.assertIn("list 0 of [1]", err) + + def test_a_verifier_run_from_before_the_cutover_still_running_refuses(self) -> None: + fake = RunsGitHub() + fake.running = [ + run( + 9, + status="in_progress", + conclusion=None, + created_at="2026-10-15T23:00:00Z", + ) + ] + code, err = self.run_build(fake) + self.assertEqual(code, 1, err) + self.assertIn("verifier run 9 from before the cutover is in_progress", err) + self.assertFalse(self.out.exists()) + # One created after the cutover is the index's, and holds nothing up. + fake = RunsGitHub() + fake.running = [ + run(9, status="queued", conclusion=None, created_at="2026-10-16T00:00:01Z") + ] + self.assertEqual(self.run_build(fake)[0], 0) + def test_a_listed_fail_no_run_read_holds_refuses(self) -> None: fake = RunsGitHub() ghost = artifact(named(IN_PR, "fail"), 5) @@ -507,7 +570,7 @@ def test_a_merge_commit_without_fail_index_refuses(self) -> None: def test_a_window_that_misses_its_count_refuses_and_writes_nothing(self) -> None: fake = RunsGitHub() - fake.window_total = 9 + fake.window_total["2026-10-14T00:00:00Z"] = 9 code, err = self.run_build(fake) self.assertEqual(code, 1, err) self.assertIn("list 1 of [9]", err) @@ -530,7 +593,7 @@ def test_a_run_not_completed_refuses(self) -> None: def test_a_rate_limit_stops_and_the_checkpoint_resumes(self) -> None: fake = RunsGitHub() - fake.limit_from = 9 # window 2 + fake.limit_from = 45 # window 2 code, err = self.run_build(fake) self.assertEqual(code, 3, err) self.assertIn("stopped, resume from", err) @@ -550,14 +613,14 @@ def test_a_rate_limit_stops_and_the_checkpoint_resumes(self) -> None: def test_a_rate_limit_inside_a_verdict_download_still_stops(self) -> None: # The download's reader wraps every error; the stop is kept anyway. fake = RunsGitHub() - fake.limit_from = 8 + fake.limit_from = 44 # run 5's download code, err = self.run_build(fake) self.assertEqual(code, 3, err) self.assertEqual(json.loads(self.checkpoint.read_text())["runs"], {}) def test_a_permission_403_refuses_and_does_not_wait(self) -> None: fake = RunsGitHub() - fake.limit_from = 6 # run 5's artifacts + fake.limit_from = 42 # run 5's artifacts fake.limit_text = "gh: Resource not accessible by integration (HTTP 403)" code, err = self.run_build(fake) self.assertEqual(code, 1, err) From 44b849d13290b763af31bf549dc8733f6c9db77b Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 2 Nov 2026 02:51:38 -0500 Subject: [PATCH 8/8] fix(source-control): answer the #12302 review threads on the FAIL index - A run seen completed with only its index is read again, since its FAIL may have uploaded between the two reads. - A legacy-fails window holding an unfinished run refuses before it is kept, so a resume lists that window again. - The pacer keeps its reset window and call count beside the checkpoint, so a resume inside the same window keeps its cap. - The index step fails when the run already holds one of its names from an earlier attempt, so a re-run attempt writes no FAIL that would outlive the index it kept. Co-Authored-By: Claude Opus 5.5 --- .github/actions/upload-fail-index/action.yml | 9 ++- .github/scripts/pr-verify/workflow.test.mjs | 11 ++- .../scripts/babysit_verify_evidence.py | 73 ++++++++++++++----- .../tests/test_babysit_verify_index.py | 33 +++++++++ 4 files changed, 102 insertions(+), 24 deletions(-) diff --git a/.github/actions/upload-fail-index/action.yml b/.github/actions/upload-fail-index/action.yml index b568d74bdb4..62b7f1ed511 100644 --- a/.github/actions/upload-fail-index/action.yml +++ b/.github/actions/upload-fail-index/action.yml @@ -8,8 +8,9 @@ description: > upload must require this step's success: a FAIL with no index is one that reader cannot see, and a failed index upload leaves no verdict at all. More names than slots fail the step. A name the run already holds (an earlier - attempt's upload) is skipped: it already points to this run, and replacing - it would delete that pointer before the new upload could fail. + attempt's upload) fails the step too, so a re-run attempt writes no FAIL: + replacing the name would delete the earlier attempt's pointer first, and + keeping it would leave a new FAIL that outlives its index. inputs: names: @@ -75,8 +76,8 @@ runs: if [ "$have" = "$name" ]; then found=1; fi done if [ -n "$found" ]; then - echo "$name is held already" - continue + echo "::error::$name is held by an earlier attempt of this run, so this attempt writes no FAIL" + exit 1 fi slot=$((slot + 1)) echo "n$slot=$name" >>"$GITHUB_OUTPUT" diff --git a/.github/scripts/pr-verify/workflow.test.mjs b/.github/scripts/pr-verify/workflow.test.mjs index 48195f6b981..bdcb3d6e188 100644 --- a/.github/scripts/pr-verify/workflow.test.mjs +++ b/.github/scripts/pr-verify/workflow.test.mjs @@ -134,11 +134,16 @@ const splitNames = (names, held, { total, fail = false } = {}) => { return { status: run.status, outputs: readFileSync(out, "utf8") }; }; -test("a FAIL index skips the names its run already holds, and never overwrites one", () => { +test("a FAIL index never replaces or keeps a name its run already holds", () => { const names = ["pr-verify-index-pr-41", `pr-verify-index-sha-${"a".repeat(40)}`, "pr-verify-index-ref-0123456789abcdef"]; assert.equal(splitNames(names, []).outputs, `retention-days=90\n${names.map((n, i) => `n${i + 1}=${n}`).join("\n")}\n`); - // A re-run attempt: the earlier attempt's pr index stays, and only the rest upload. - assert.equal(splitNames(names, [names[0], "pr-verify-41-x-fail"]).outputs, `retention-days=90\nn1=${names[1]}\nn2=${names[2]}\n`); + // A re-run attempt: an earlier attempt's index name fails the step, so no + // new FAIL outlives that index, and nothing replaces it. + const rerun = splitNames(names, [names[0], "pr-verify-41-x-fail"]); + assert.notEqual(rerun.status, 0); + assert.doesNotMatch(rerun.outputs, /^n\d+=/m); + // Another name the run holds (the verdict's own) uploads every index. + assert.equal(splitNames(names, ["pr-verify-41-x-fail"]).outputs, `retention-days=90\n${names.map((n, i) => `n${i + 1}=${n}`).join("\n")}\n`); // A read that fails or does not hold its count uploads nothing. assert.notEqual(splitNames(names, [], { fail: true }).status, 0); assert.notEqual(splitNames(names, [names[0]], { total: 2 }).status, 0); 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 f18860f1d42..293c16ca128 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 @@ -2781,22 +2781,30 @@ def _run_artifacts(self, run_id: int, deadline: float) -> list[dict[str, Any]]: """The unexpired artifacts of a run an index artifact names, in one read that must hold its `total_count`. A run that holds an index but no FAIL-shaped name may still be uploading its FAIL: while it has not - completed, `_Unreadable`; once it has, its FAIL upload failed and - there is none.""" - read = self._api_object( - f"repos/{self.repo}/actions/runs/{run_id}/artifacts?per_page={LISTING_PAGE_SIZE}", - deadline, - ) - self.index_reads += 1 - items = read[0].get("artifacts") if isinstance(read, tuple) else None - if ( - not is_json_array(items) - or read[0].get("total_count") != len(items) - or not all(map(_listed_entry, items)) - or any(json_object(a.get("workflow_run")).get("id") != run_id for a in items) - ): - raise _Unreadable(f"the artifacts of run {run_id} could not be read") - items = [a for a in items if a["expired"] is False] + completed, `_Unreadable`; once it is seen completed, its artifacts are + read again, since the FAIL may have uploaded between the two reads, + and that read decides (no FAIL then: its FAIL upload failed).""" + + def read() -> list[dict[str, Any]]: + answer = self._api_object( + f"repos/{self.repo}/actions/runs/{run_id}/artifacts" + f"?per_page={LISTING_PAGE_SIZE}", + deadline, + ) + self.index_reads += 1 + items = answer[0].get("artifacts") if isinstance(answer, tuple) else None + if ( + not is_json_array(items) + or answer[0].get("total_count") != len(items) + or not all(map(_listed_entry, items)) + or any( + json_object(a.get("workflow_run")).get("id") != run_id for a in items + ) + ): + raise _Unreadable(f"the artifacts of run {run_id} could not be read") + return [a for a in items if a["expired"] is False] + + items = read() if not any(FAIL_PREFIX_RE.match(str(a.get("name") or "")) for a in items): record = json_object( self._actions( @@ -2806,6 +2814,7 @@ def _run_artifacts(self, run_id: int, deadline: float) -> list[dict[str, Any]]: self.index_reads += 1 if record.get("status") != "completed": raise _Unreadable(f"run {run_id} holds a FAIL index and is still running") + items = read() return items def _index_hits(self, names: Iterable[str], deadline: float) -> list[dict[str, Any]]: @@ -5331,6 +5340,9 @@ class _Pacer: window_cap: int min_remaining: int breaker_file: str | None + # Where the window's reset and calls are kept, so a resume inside the + # same reset window keeps counting toward its cap. + state_file: str | None = None calls: int = 0 window_calls: int = 0 window_reset: int | None = None @@ -5379,7 +5391,27 @@ def open_window(self) -> None: raise self._stop(f"core remaining {remaining} is under {self.min_remaining}") if self.window_reset is not None: self.report() - self.window_reset, self.window_calls = reset, 1 + kept = 0 + if self.state_file: + try: + saved = json_object(json.loads(Path(self.state_file).read_text())) + except (OSError, ValueError): + saved = {} + if saved.get("reset") == reset and type(saved.get("calls")) is int: + kept = saved["calls"] + self.window_reset, self.window_calls = reset, kept + 1 + self._keep() + if self.window_calls > self.window_cap: + raise self._stop( + f"{self.window_calls - 1} calls in the window that resets at " + f"{datetime.fromtimestamp(reset, UTC).isoformat()}, the cap" + ) + + def _keep(self) -> None: + if self.state_file and self.window_reset is not None: + Path(self.state_file).write_text( + json.dumps({"reset": self.window_reset, "calls": self.window_calls}) + ) def report(self) -> None: reset = ( @@ -5403,6 +5435,7 @@ def __call__(self, args: list[str], **kwargs: Any) -> Any: f"{datetime.fromtimestamp(self.window_reset or 0, UTC).isoformat()}, the cap" ) proc = self._raw(args, **kwargs) + self._keep() if self.limited: raise self._stop(f"rate limited: {self.limited}") return proc @@ -5544,6 +5577,11 @@ def _window( raise _Unreadable( f"the verifier runs created {created} list {len(found)} of {sorted(totals)}" ) + # Refused before the caller keeps the window, so a resume lists it + # again once the run has completed. + for record in found.values(): + if record.get("status") != "completed": + raise _Unreadable(f"verifier run {record['id']} is {record.get('status')}") return list(found.values()) def _run(self, record: dict[str, Any]) -> dict[str, Any]: @@ -5801,6 +5839,7 @@ def _legacy_fails_main(args: argparse.Namespace, now: datetime | None) -> int: args.window_cap, args.min_remaining, args.breaker_file, + f"{args.checkpoint}.window", ) try: with _paced_gh(pacer): diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py index eeedb332caf..4d61c852372 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_index.py @@ -83,6 +83,8 @@ def __init__(self, *args: Any, **kwargs: Any): super().__init__(*args, **kwargs) self.name_total: dict[str, int] = {} self.still_running: set[int] = set() + # Answers to a run's artifact reads in turn, before its artifacts. + self.run_reads: dict[int, list[list[dict[str, Any]]]] = {} # The verifier runs not completed, as the workflow's run list gives them. self.running: list[dict[str, Any]] = [] @@ -105,6 +107,9 @@ def rest(self, args, ok, missing, kwargs): # type: ignore[no-untyped-def] of_run = re.fullmatch( rf"repos/{REPO}/actions/runs/([0-9]+)/artifacts\?per_page=100", path ) + if of_run and self.run_reads.get(int(of_run[1])): + held = self.run_reads[int(of_run[1])].pop(0) + return ok({"total_count": len(held), "artifacts": held}) if of_run: held = [ a @@ -270,6 +275,14 @@ def test_an_index_whose_run_still_uploads_refuses(self) -> None: fake.still_running.clear() self.assertIsNone(self.judge(fake).refusal) + def test_a_fail_uploaded_while_its_run_completes_still_counts(self) -> None: + # The first read of run 3 holds only its index; by the time the run is + # seen completed its FAIL has uploaded. + items = fail(90, HEAD, 3) + fake = self.passing(*items, runs={3: run(3)}) + fake.run_reads[3] = [items[1:]] + self.assertEqual(self.judge(fake).refusal, ve.FAIL) + def test_a_name_read_that_fails_or_misses_its_count_refuses(self) -> None: fake = self.passing() fake.fail.add(f"name={ve.INDEX_PREFIX}sha-{HEAD}") @@ -610,6 +623,26 @@ def test_a_rate_limit_stops_and_the_checkpoint_resumes(self) -> None: ) self.assertEqual(len(json.loads(self.out.read_text())["artifacts"]), 1) + def test_a_run_unfinished_at_the_first_build_is_listed_again_on_resume( + self, + ) -> None: + fake = RunsGitHub() + fake.runs[2] = run(1, created_at="2026-10-15T02:00:00Z", status="in_progress") + code, err = self.run_build(fake) + self.assertEqual(code, 1, err) + # Run 1 completed since: the resume reads its window afresh. + code, err = self.run_build(RunsGitHub()) + self.assertEqual(code, 0, err) + + def test_a_resume_inside_the_same_reset_window_keeps_its_cap(self) -> None: + code, err = self.run_build(RunsGitHub(), "--window-cap", "4") + self.assertEqual(code, 3, err) + resumed = RunsGitHub() + code, err = self.run_build(resumed, "--window-cap", "4") + self.assertEqual(code, 3, err) + self.assertEqual(resumed.calls, ["rate_limit"]) + self.assertIn("calls in the window", err) + def test_a_rate_limit_inside_a_verdict_download_still_stops(self) -> None: # The download's reader wraps every error; the stop is kept anyway. fake = RunsGitHub()