[kyle-sexton]

No related issue: incident fix for the verdict reader's artifact listing (#12205's merge-queue ejection, GITHUB_TOKEN breaker trips)

Summary

ci-status merge_group run 41374462603 (job 125052787044, #12205) spent core=562. The verdict reader made 134 whole-listing page calls (repositories/{n}/actions/artifacts), 24 single-page calls (repos/{repo}/actions/artifacts) and 4 id reads. It then refused verify:unreadable: repository artifacts: 1 whole listing never held GitHub's live artifact count, and the merge queue ejected #12205. pr-merge's enable-auto-merge job 125053997338 (run 41375133667) did the same for #12203: 293 whole-listing pages, 40 single pages and 2 id reads in one invocation.

Both jobs' babysit-listing-miss lines show the same start:

Job	Incremental read	Whole listings
ci-status 125052787044	listed 13493 vs live 13496 after region_pages 11	1 (over-count cap); top-up 13488 vs 13493, region_pages 0: refused
enable-auto-merge 125053997338	listed 13480 vs live 13481 after region_pages 11	3; top-ups short by 2 and 1, then near-expiry-deadline: refused

In both jobs, the near-expiry re-read dropped every entry GitHub had deleted early and left the count 1 to 3 short. The reader sent that short count straight to a whole listing of all ~13,500 artifacts (#12020: "a wait would re-read the same pages and spend the cap the top-up needs"). The re-read drops an id only on a 404 or an expired record, so a count it leaves short is a page-boundary skip in the 6 fresh pages (#10486). A read after a wait restores such a skip, as it already does for any other short incremental count.

The ci-status step that printed the refusal is named Fail a lanes App pull request its body or a member holds, and its error annotation carried no category. A reader of the run therefore saw a body hold.

Fix

Why not per-run reads (the proposed design). That design is rejected on evidence:

• pr-verify runs cannot be found from a PR head. They are workflow_run runs on main. One gated read of actions/workflows/pr-verify.yml/runs?per_page=3 shows head_sha 00f9939 (main's tip), head_branch main and display_title "pr-verify" on every run, with total_count 15,449. ?head_sha=<pr head> returns nothing, and pr-verify.yml sets no run-name.
• Per-run reads would break guarantees that need every pr-verify artifact in the repository, as babysit_verify_evidence.py's docstring states them:
  • a FAIL at this head under any PR number;
  • FAILs of other PRs for the same issue, which count toward the max_fails cap (_elsewhere);
  • an untied FAIL whose head branch cannot be read, which blocks every PR;
  • FAILs at heads gone from pulls/<n>/commits, tied by the branch digest.
• Cost for N candidates: with no head filter, per-run reads would mean listing every pr-verify run (~155 pages at 15,449 runs) plus one artifacts page per run. For any N that costs more than one whole artifact listing (~135 pages), and it still breaks the guarantees above.
• Counter A (workflows/pr-verify.yml/runs): +0 calls. Counter B (runs/{id}/artifacts): +0 calls. Neither read was added.

Why the whole listing exists (history from git).

• feat(source-control): add the non-author pr-verify verdict and pr-merge verdict reader #7704 created the reader: "a FAIL at the head SHA, under any PR number", earlier heads and the same branch, and an untied FAIL blocks every PR. Its docstring says the listing covers "every artifact, and filtered on the name here: the API's name filter matches whole names only".
• fix(source-control): re-read the pages near expiry when the verdict listing over-counts #11921's "Options weighed" rejected reading only what the reader needs: "The REST name parameter matches exact names only ... exact names cannot find every FAIL the cross-PR max_fails count needs." It also added the near-expiry re-read and "one whole listing per over-count".
• fix(source-control): stop the verdict reader's repeated cold artifact listings #11926 sent an over-count to one whole listing and then a refusal. Its reason: during GitHub's early deletions on 2026-11-01, repeated whole listings exhausted the pool.
• fix(source-control): check near-expiry entries on an exact-count verdict listing #12020 added the near-expiry check on an exact count. It sent a count those pages leave short to a whole listing without waiting, so that the incremental reads would not spend the top-up's cap.

This change. Changes to babysit_verify_evidence.py:

• When the pages near expiry leave an incremental count short, the reader returns COUNT_MISSED instead of COUNT_OVER. The count is then read again on the same base after INCREMENTAL_WAITS. Each read faces the same exact-count and near-expiry check as before, so no read can allow a listing the old code refused.
• The retry happens only while two conditions hold. First, the near-expiry page and id caps left must cover another read like this one. Second, under a caller's limit (DEADLINE_ENV), the time left must still fit this read, the longest wait and one whole listing. Otherwise the whole listing comes at once, as before. In practice this allows one retry for an 11-page region under the 24-page cap.
• _complete_listing resets the near-expiry caps, so the whole listings' top-ups get caps of their own. This answers fix(source-control): check near-expiry entries on an exact-count verdict listing #12020's starvation concern. Worst case: cold_cost in pr-require-checks.yml now counts both caps twice. That is 1,681 → 1,739 at 7,522 artifacts and 2,079 → 2,137 at 10,450 artifacts. The comment, scripts/ci-lanes-pr-hold.test.sh and TheColdReadHeadroomCoversTheListingCaps are updated to match.

Kept unchanged: every FAIL rule, the trusted-producer checks (pr-verify on main, this repository, not forks), the near-expiry rules from #12020 and #11921, bounded loops (same INCREMENTAL_READS, same caps per stage), failing closed on any unreadable read, and #11921's one whole listing per over-count.

Defect 2 (the recorded category). merge-holds.mjs now writes every verifier hold (verdictHold, linkHold, evidenceHold) as ::error title=pr-verify hold::#N: ... (VERIFIER_TITLE). Body holds, member vetoes and hold paths keep plain ::error::. Body-hold behavior and exit codes are unchanged.

Consumers of the step name were checked: resolve-pr.mjs HOLD_STEP/heldOnly, reconcile.test.mjs, api-budget-coverage.test.mjs and ci-lanes-pr-hold.test.sh. The step name is kept, so they keep working. heldOnly judges only pull_request runs, which carry no verifier lines.

On a verifier-only hold, the module now also prints #N: no body hold, no member veto and no hold path. and still exits 1. The workflow greps that line only after exit 0.

events.log is the local hub's file, not part of this repository. To use the new category, its classifier must key on the annotation title. That is a follow-up for the hub owner.

Verification

• test_babysit_verify_evidence.py: 424 passed, 356 subtests. Each test was run against origin/main's reader by swapping in only the module:

Test	origin/main reader	this branch
a skip after the pages near expiry is read again after a wait (enable-auto-merge's shape)	fail (full != incremental)	pass
an over-count left short by a skip is read again after a wait (ci-status's shape: no whole listing, no .overcount-fallback, no .failed)	fail	pass
a skip a caller's limit cannot wait for takes a whole listing	pass (coverage); fails when the time guard is mutated off	pass
a skip the caps left cannot read again takes a whole listing	pass (coverage)	pass
the pages near expiry stop at their cap (changed: the top-up's own cap now settles and allows; it refused before)	fail	pass
a dropped entry does not spend the top-up's page cap (changed: the incremental read now waits twice; the top-up keeps its own cap)	fail	pass
a stale carry whose early deletion hides a missed FAIL (changed: the futile retries cost 2 waits and 2 more id reads, then the whole listing finds the FAIL)	fail	pass
the cold-read headroom covers the listing caps (changed: caps counted twice)	n/a (reads the workflow)	pass

An exact count that stays unsettled still refuses, pinned by test_an_exact_count_the_pages_near_expiry_cannot_settle_refuses (cap 0).

• merge-holds.test.mjs: 66 pass. Against origin/main's module, the three title tests fail, including the new test "the reader's verify:unreadable is a pr-verify hold beside a body hold".
• scripts/ci-lanes-pr-hold.test.sh: PASS=300 FAIL=0. scripts/ci-hold-merge-group.test.sh: PASS=82.
• enable-auto-merge.test.mjs: 195 pass. resolve-pr.test.mjs: 143 pass. reconcile.test.mjs: 143 pass. api-budget-coverage.test.mjs: 4 pass. test_merge_holds_sync.py + test_verify_rules_sync.py: 39 pass. test_babysit_verify_answers.py, test_babysit_merge_skip.py, test_babysit_merge_unattended.py: 351 pass. Suites were run one at a time.
• check-lane-coverage.sh --check, check-pipefail-grep-q.sh, actionlint on pr-require-checks.yml, lint-shell-filter.test.sh (16 pass) and ruff: clean.
• The brief's five tests map to this design as follows:
  • verdict via a per-run path: not applicable (design rejected);
  • a FAIL on an earlier rerun of the same head, and artifacts from another workflow or repository ignored: existing suites, unchanged;
  • a listing whose count moves no longer refuses: the two new skip tests;
  • recategorized refusal: the new merge-holds test.
• evidence was not run locally. It cannot reproduce a carry inside an early-deletion window, and it costs owner-token calls. After merge, verify with the api-routes annotations of merge_group ci-status and enable-auto-merge jobs.

Per-run call delta (R34), derived from the two jobs' miss lines (not re-run):

Caller	Before	After, when one retry settles
ci-status merge_group (job 125052787044)	134 whole-listing pages + 24 single pages + 4 id reads, then refused	24 + one retry (~6 fresh + ~11 region pages + ≤4 id reads) ≈ 45; 0 whole-listing pages; no ejection
enable-auto-merge (job 125053997338)	293 whole-listing pages + 40 single pages + 2 id reads, then refused	≈ 40 + ~20; 0 whole-listing pages
either, when the short count is a carry gap rather than a skip	as before	as before, plus up to 2 futile retries (≤ 2 × (13 pages + region pages + id reads), inside the caps) and their 15 s and 30 s waits, then the same whole listing

UNVERIFIED: whether those shortfalls were fresh-page skips or gaps in the carry. The miss lines do not tell the two apart. A carry is a listing saved after it held the live count, so a skip is the likelier cause.

Residual risks:

• When the retries do not settle, fix(source-control): re-read the pages near expiry when the verdict listing over-counts #11921's one whole listing per over-count still applies. ci-status's path (over-count, one whole listing, short top-up, refusal) is then unchanged. Whether a short top-up after an over-count deserves a second whole listing is an open choice for the reader's owner.
• verify-cap.mjs sets BABYSIT_VERIFY_NO_INCREMENTAL_WAITS=1 on its fallback after a missed deadline, so that path reads once and gains nothing from the retry.
• With the caps reset per stage, a full-budget timeout of the first whole listing that follows incremental near-expiry reads is now noted .failed for the job, where it used to be transient. fix(source-control): stop the verdict reader's repeated cold artifact listings #11926's rule notes a full-budget timeout.

This PR changes hold paths (babysit_verify_evidence.py, merge-holds.mjs, pr-require-checks.yml), so a member's approval is required. It is not armed.

Related

• test(sql-server): check the write tier in the integration script #12205's ejection: run 41374462603, check run 125052787044
• enable-auto-merge evidence: run 41375133667 (job 125053997338)
• fix(source-control): stop the verdict reader's repeated cold artifact listings #11926, fix(source-control): re-read the pages near expiry when the verdict listing over-counts #11921 and fix(source-control): check near-expiry entries on an exact-count verdict listing #12020: the listing rules this change adjusts. feat(source-control): add the non-author pr-verify verdict and pr-merge verdict reader #7704: the reader's cross-PR FAIL rules. fix(source-control): the verdict reader's full artifact listing skips live artifacts at page boundaries, FAILs included #10486: page-boundary skips.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-02T03:06:53.992584Z	e704fa1	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

claude-security-review reviewed e704fa1b5b6bbc473e949a63b291fe625ba74663: read 9 of 9 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #12233 at e704fa1b

I found no security issues in this change. I read every hunk of all 9 changed files. Three of them are trusted paths and merge-hold controls: merge-holds.mjs, babysit_verify_evidence.py and pr-require-checks.yml. For each, I read the code around the change and the gates it affects.

merge-holds.mjs: the merge hold is not weakened

• Verifier holds (verdictHold, linkHold, evidenceHold) now go into a separate verifier array and print as ::error title=pr-verify hold::…. The exit code is reasons.length || verifier.length ? 1 : 0 (L1720), so a verifier hold still fails the step.
• When only a verifier hold applies, the module now also prints #N: no body hold, no member veto and no hold path. The one consumer of that line, pr-require-checks.yml L6602-L6609, checks the exit code first and exits 1 on any non-zero code. It greps the line only after exit 0, so the extra line cannot release a hold.
• resolve-pr.mjs heldOnly treats a run as person-held only when every error line matches personHoldLine (L271-L275). A verifier hold line never matches that pattern, with or without the title, so that check still fails closed.
• The annotation title is a constant. Reason text still goes through escape(), so pull request text cannot inject workflow commands.

babysit_verify_evidence.py: the reader still accepts no listing it refused before

• Every listing it returns must still hold exactly the live total_count and pass the near-expiry checks (L1716-L1731).
• The change only reroutes a short incremental count. It now returns COUNT_MISSED, so the reader tries again after a wait, instead of COUNT_OVER, which went straight to a whole listing. The retry is limited by the remaining caps and the caller's deadline (L1749-L1765). The over-count path still returns COUNT_OVER.
• _complete_listing now resets the near-expiry caps (L1493). That gives the top-up more of its budget, but it does not loosen the rule for dropping an entry: an id is still dropped only when its own read answers 404 or shows it expired. Reads stay bounded per stage, and an unsettled exact count still refuses.
• The unchanged checks still hold: pr-verify must run on main, artifacts must come from this repository, and forks are excluded.

pr-require-checks.yml: the headroom check gets stricter

• cold_cost now counts the near-expiry caps twice. That raises the GITHUB_TOKEN headroom a cold read needs (for example 1,681 → 1,739), so the check is stricter.

Other checks

• Owner throughput exemption: none of the named friction holds is removed.
• No instruction surface, plugin manifest, token scope, trusted-actor list or kill-switch file changed.
• Embedded text: the PR description contains no directive aimed at this lane.
• Spend: the extra retries and waits use the GitHub Actions API rate budget (GITHUB_TOKEN core requests) and runner time only. Nothing is billed.

critical-findings: 0

[github-actions]

claude-review reviewed e704fa1b5b6bbc473e949a63b291fe625ba74663: read 9 of 9 changed file(s); 1 finding(s), 0 as inline comments.

Code review: #12233 (head e704fa1)

I read every hunk of the 9 changed files. I also read the surrounding Reader code in babysit_verify_evidence.py (_current_listing, _complete_listing, _whole_listings, _incremental, _near_expiry, listing_cap), the main function in merge-holds.mjs, and the step in pr-require-checks.yml that reads merge-holds' output.

Reader change (babysit_verify_evidence.py).

• The new branch keeps the old top-up behaviour: an over-count gives COUNT_OVER, and an exact count left short gives COUNT_MISSED.
• On the incremental stage, a count left short after the near-expiry pages now returns COUNT_MISSED. That happens only while the remaining caps cover another read of the same size, and, under a caller's DEADLINE_ENV, while the time left fits that read, the longest wait and a whole listing. Otherwise it returns COUNT_OVER.
• If a retry still overruns a cap, the exact-count case returns None, which also goes to the whole listing. No path allows a listing that the old code refused.
• _over_counted is still OR-ed in on the first read, so fix(source-control): re-read the pages near expiry when the verdict listing over-counts #11921's rule of one whole listing per over-count holds.
• Resetting the caps in _complete_listing also changes _near_expiry_out_of_time. A whole-listing timeout that follows only incremental near-expiry reads is no longer transient. The PR lists this as a residual risk. The whole listing's budget is recomputed after the incremental stage, so the new classification is defensible.

merge-holds change. Verifier holds now print as ::error title=pr-verify hold::, and the exit code stays 1 whenever any hold is present. On a verifier-only hold, the module now also prints the "no body hold" line. That is safe: pr-require-checks.yml#L6602-L6609 prints the output and exits 1 on a nonzero exit before it greps for that line.

Finding (cross-file, on an unchanged line):

• Stale cold-read formula in the pr-pipeline convention doc. docs/conventions/pr-pipeline/README.md#L2021-L2024 still says a cold read's worst case includes "at most 24 pages near expiry read again and 34 artifacts read by id: 1,681 at 7,522 artifacts and 264 FAILs".
  • This PR changes cold_cost at pr-require-checks.yml#L6512 to 2 * (24 + 34), which gives 1,739. The workflow comment, ci-lanes-pr-hold.test.sh, TheColdReadHeadroomCoversTheListingCaps and the changelog fragment were all updated, and the PR edits this README at L2674. This paragraph was missed, so the convention doc now gives an operator the old headroom.
  • Fix: count the caps twice (once for the incremental reads, once for the whole listings' top-ups) and change the figure to 1,739.

Not a finding.

• The extra incremental retries and their 15 s and 30 s waits spend GitHub API requests and runner time inside existing caps. The PR's figures show them replacing much larger whole listings. Nothing here is billed.
• Path-scoped rules matching the changed files (ruff-pin, skill-bodies-state-current-rules, cost-claims): no violations. The request counts in the docs are API budgets, not prices.
• No closing issue: the PR says it is an incident fix. Prior findings: none, 0 rounds, cap 3, not past the cap.

Verification. All test results in the PR body are author-claimed and unverified. This lane has no shell and ran nothing.