[kyle-sexton]

No related issue: verify:unreadable when the artifact listing outgrows verify-cap's limit (#11903)

Summary

pr-feedback-dispatch run 41287325439 refused #11903 with verify:unreadable. The verdict reader's cheap incremental listing (7 pages) missed GitHub's live total_count three times, each time holding more artifacts than the count (13415 vs 13384, 13416 vs 13384, 13424 vs 13390). It then fell back to a full listing, which needed 134 pages and 141 s, more than verify-cap.mjs's 120 s limit.

Cause: on 2026-11-01 GitHub deleted artifacts up to about an hour before their expires_at, and not in expiry order. The carried base still held them, so the incremental result over-counted.

• The carried listing behind that run (fetched 07:51:30Z) held 234 artifacts, all expiring 09:20 to 09:55Z, that three later full listings lacked. The 60 of them read by id at 09:37Z each answered 404.
• A full listing at 08:55Z already lacked ones expiring up to 59 minutes later.
• A reader-verified listing at 09:13Z still held 8 entries expiring 09:43 to 09:54Z, so the deletions do not follow expiry order.
• The cause is UNVERIFIED. 2026-11-01 was the US DST change. This looks like GitHub behavior, not ours; whether to raise it upstream is the owner's call.

#11926 (merged into this branch) handles the same over-count by taking one whole listing and then refusing, and notes the failure for the job. This PR runs a bounded re-read of the pages near expiry before that fallback, so the common case settles incrementally.

Artifact census (two full listings with duplicates removed: 13168 artifacts):

Family	Count	Retention
pr-verify pass-notes	2666	7, 30 or 90 days
pr-verify fail	510	90 days
pr-verify pass	466	7, 30 or 90 days
everything else	9526	1 to 90 days

• The reader needs only the pr-verify families, about 28% of the total.
• Every family is retention-bounded. The 30-day passes predate perf(pr-pipeline): expire pr-verify PASS artifacts after 7 days #11288 and age out by about 2026-11-30.

Options weighed:

• Shorter retention for families the reader does not need. This trims only new uploads, and cost still grows with the total. Rejected as the fix.
• Reading only what the reader needs. The REST name parameter matches exact names only (https://docs.github.com/en/rest/actions/artifacts#list-artifacts-for-a-repository). FAILs carry other PR numbers, other heads and issue suffixes, so exact names cannot find every FAIL the cross-PR max_fails count needs. Rejected.
• Scheduled deletion. Needs a counted bulk-delete plan, and does not remove the over-count. Rejected.
• Raising the time limit. Only defers the failure. Rejected.
• Picked: make the cheap incremental path survive early deletion, ahead of fix(source-control): stop the verdict reader's repeated cold artifact listings #11926's fallback. Its cost scales with uploads since the base and with expiries in the next two hours, not with the total.

Fix

Changes to babysit_verify_evidence.py, compared with main after #11926:

• Order on an over-count. When the merged incremental listing, or a whole listing's top-up, holds more than total_count, Reader._incremental first re-reads only the pages around kept entries that expire within EARLY_DELETION_LEAD (2 h, twice the largest lead seen). Only when that cannot settle the count does it return COUNT_OVER, which goes to fix(source-control): stop the verdict reader's repeated cold artifact listings #11926's single whole listing. That listing refuses when its top-up misses the count.
• Drop an id only on a 404. The region pages add every id they hold. An id of the listing inside their span that they omit is dropped only when GET repos/{repo}/actions/artifacts/{id} answers 404 or an expired record. A live record keeps it. Any other answer (5xx, a timeout, a failed read, an odd body) leaves the listing unchanged, so it ends in COUNT_OVER. Pages read one at a time skip the entry at a boundary when an artifact above it is deleted between reads (fix(source-control): the verdict reader's full artifact listing skips live artifacts at page boundaries, FAILs included #10486). Without this rule, a skipped live FAIL plus a leftover surplus entry could match the count. The rule sits in _near_expiry, so it covers both the incremental path and the whole-listing top-up path.
• Caps.
  • NEAR_EXPIRY_PAGE_CAP = 24 pages per listing, shared by the incremental read and the top-up. Basis: the re-read took 12 pages on the 2026-11-01 carry behind feat(pr-pipeline): name merge.rung values merge and merge-reviewed #11903's refusal, and the cap is twice that (judgment on that data).
  • NEAR_EXPIRY_ID_READ_CAP = 34 id reads per listing. Basis: the re-read drops at most the listing's surplus over the live count. That surplus was 31, 32 and 34 in the three over-counts behind feat(pr-pipeline): name merge.rung values merge and merge-reviewed #11903's refusal; the cap is the largest (judgment on that data).
  • Past either cap the listing stands unchanged, so the over-count goes to the fallback.
• Upward walk. A group's walk climbs at most surplus // LISTING_PAGE_SIZE + 2 pages above its start page. The docstring now says so, instead of "surplus pages".
• Fail-closed. Once the re-read has run, any count other than total_count returns COUNT_OVER, whether it is above or below. No INCREMENTAL_WAITS retry follows, and the listing never allows: it goes to the one whole listing, then refuses.
• One whole listing per over-count. _over_counted limits the whole-listing loop to one read when the pages over-counted. fix(source-control): stop the verdict reader's repeated cold artifact listings #11926 stopped after one only when the top-up over-counted; a top-up that came up short after an over-count still read up to LISTING_READS (3) whole listings.
• Once per job, and how it differs from fix(source-control): stop the verdict reader's repeated cold artifact listings #11926's .failed note. .overcount-fallback beside BABYSIT_VERIFY_LISTING is created (exclusively) only when the over-count's whole listing starts paging, after _complete_listing's budget and count checks pass. A refusal before paging leaves it free for the next Reader. Once it exists, a later Reader in the job whose pages over-count refuses verify:unreadable without another whole listing. That holds even when the first whole listing failed transiently or on a caller's limit, which .failed would not note: an over-count is the exception to fix(source-control): stop the verdict reader's repeated cold artifact listings #11926's rule. When the first listing ran out of a caller's time, the marker records that, and the later refusal carries the DEADLINE_UNREADABLE prefix, so verify-cap.mjs still skips that round. The .failed note and its transient and caller-limit rules are otherwise unchanged.
• Sidecar lifetime. The .failed and .overcount-fallback sidecars are shared by a job's Readers on purpose. That is what makes the rules per job, for example across pr-merge's merge-job candidates. The rm -f "$BABYSIT_VERIFY_LISTING" in pr-require-checks.yml leaves them in place. That step runs one reader per job, and BABYSIT_VERIFY_LISTING is set only in that step (pr-require-checks.yml:6195), under the job's own runner.temp, so no earlier sidecar can exist there.
• Log line. Each babysit-listing-miss line gains region_pages. Its readers (verify-cap.mjs, enable-auto-merge.mjs) match only the prefix.
• Docs. The rule text in docs/conventions/pr-pipeline/README.md and the module docstring now describe the order, the 404 rule and the marker.

No reader rule changed, so resolve-pr.mjs and test_verify_rules_sync.py are untouched.

Not done: the re-read still spends its page budget in group order rather than nearest the surplus first. Ordering by surplus needs a guess at which group holds the surplus, and the caps keep the result fail-closed either way.

Verification

Listing-only harness, run against the live API with the carried listing behind the refusal and verify-cap's 120 s limit, before #11926 merged and before the per-id reads were added. Calls are listing GET requests:

Code	Mode	Page reads	Seconds	Result
main before #11926	full fallback	28 (3 reads of 9 pages + count)	64.6 / 64.8	refused: 13239 artifacts in 133 pages need 140 s
this branch's re-read	incremental	21 (9 fresh + 12 near-expiry)	15.9	exact count, 13251 artifacts

The harness was not re-run against main with #11926, or with the per-id reads.

API calls per pass (R34), relative to main with #11926:

• Count matches: +0.
• Over-count the re-read settles (the 2026-11-01 case): about +12 page reads (measured), plus one id read per entry the pages omit. That is at most the surplus, 31 to 34 there, so about +43 to +46 calls (derived). In exchange, fix(source-control): stop the verdict reader's repeated cold artifact listings #11926's whole listing is skipped. That listing is about 133 pages plus a top-up and then refuses, or for a caller with verify-cap's limit, it refuses after the count read (derived from the measured page count, not re-run).
• Over-count the re-read cannot settle: at most +58 calls (24 pages + 34 id reads), then the same one whole listing.
• Over-count whose whole listing's top-up comes up short: −2 whole listings (3 → 1), about −266 pages at 133 pages a listing (derived).
• A later Reader in the same job that over-counts: with fix(source-control): stop the verdict reader's repeated cold artifact listings #11926, up to another whole listing per Reader. Now 0 whole listings once the marker is taken.
• Cold-read budget: the worst case is +58 calls. The worst path is three short incremental reads, then two whole listings with short top-ups, then a third whole listing whose top-up over-counts and runs the re-read. cold_cost in pr-require-checks.yml is now 3·FAILs + 3·13 + 1 + 3·(3·pages + 7) + 24 + 34. That moves 1,623 → 1,681 at 7,522 artifacts and 264 FAILs, and 2,021 → 2,079 at 10,450 and 300. The comment, TheColdReadHeadroomCoversTheListingCaps, scripts/ci-lanes-pr-hold.test.sh and the pr-pipeline README cap list are updated to match.
• After merge, re-measure from the api-calls annotations.

Tests in test_babysit_verify_evidence.py, run against an earlier reader by restoring only the module (git restore --source=<ref>), then against the branch's reader:

Test	Earlier reader	Branch
an early deletion read again	main: fail (full != incremental)	pass
deletion out of expiry order	main: fail (full != incremental)	pass
a forged carry's FAIL beside an early deletion still counts	main: fail (full != incremental)	pass
a failed near-expiry read lists every page	main: fail (no LISTING_WAITS)	pass
#11926's skewed scenario within the lead settles incrementally	main: fail (refuses verify:unreadable)	pass
an over-count the re-read leaves, top-up short, takes one whole listing a job and notes it	main: fail (allows after a second whole listing)	pass
#11926's skewed scenario past the lead takes one whole listing a job	main: pass	pass
a deletion far from expiry lists every page	main: pass	pass
the re-read stops at its page cap, then one whole listing	n/a (no cap on main)	pass
two Readers in one job both over-count: one whole listing	762e5f2: fail (second Reader allows after a second whole listing)	pass
a deletion between region pages 2 and 3 shifts FAIL 450 out of both reads; 800 is a leftover surplus (poteto's case)	e0addf3: fail (allows, FAIL dropped, count 7 accepted)	pass (verify:earlier-fail after the one whole listing)
an over-count refused before paging leaves the job its whole listing	e0addf3: fail (B refuses "already took")	pass
an over-count listing out of the caller's time makes a later over-count refuse with the caller-limit prefix	e0addf3: fail (no prefix)	pass
an omitted id is dropped only on a 404; a 502 or no id reads left keeps it	n/a (no per-id read at e0addf3)	pass
the region walks up two pages to its top	e0addf3: pass (coverage only)	pass
the region walk stops at its upward bound, then one whole listing	e0addf3: pass (coverage only)	pass

• scripts/run-plugin-tests.sh plugins/source-control/skills/babysit-prs/scripts/engine.test.sh: 3831 tests OK, ruff clean, including the cold_cost test.
• scripts/ci-lanes-pr-hold.test.sh: PASS=295 FAIL=0.
• scripts/run-ruff.sh check on both changed .py files: clean.
• No pr-unblock or pr-verify node file changed in this PR.

UNVERIFIED observations from this work:

• A local evidence run for feat(pr-pipeline): name merge.rung values merge and merge-reviewed #11903 kept judging for about 19 minutes after its listing finished, until stopped.
• 3 of 7 artifact page bodies came back cut short at about 09:35Z.

Related

• feat(pr-pipeline): name merge.rung values merge and merge-reviewed #11903 (the refused pull request), run 41287325439.
• fix(source-control): stop the verdict reader's repeated cold artifact listings #11926, the one-whole-listing-then-refuse fallback this PR now runs behind.
• Still open: the full-listing fallback still grows with the total artifact count (about 1 s a page). This PR makes the fallback rare, not cheaper.

🤖 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-01T10:39:45.374748Z	1397dfd	Draft marked ready

ℹ️ 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-review: 1 inline finding(s) on 1397dfd73741a4e0fc3ccda3ca9155d41a3e3767; the summary is in this lane's pull request comment.

claude-lane: claude-review

[github-actions]

claude-review reviewed 5f649282a373bc18cb608010db7a09dd9a6e3fef: read 7 of 7 changed file(s); 0 finding(s), 0 as inline comments.

Incremental review: e0addf3 → 5f64928 (no findings)

Scope: I read every hunk of all 7 files in .claude-lane/incremental.diff. For context I also read _current_listing, _complete_listing, _whole_listings, _incremental and _near_expiry in the head file, and the FakeGitHub fail and missing handling in the tests.

What changed and what I checked

• Per-id 404 rule (_near_expiry and _artifact_gone). An id inside a region's span that the region pages omit is now dropped only when it reads back as 404 or as an expired record. Every other outcome leaves the listing unchanged and returns COUNT_OVER: a live record, a non-404 error, a failed read, a malformed body, the deadline running out, or the id-read cap. That covers rate-limit 403s and 5xx answers too. This closes the earlier case where a FAIL skipped at a boundary, plus a leftover surplus entry, could match total_count. The rule is in _near_expiry, so it covers both the incremental path and the top-up path. test_a_deletion_between_region_pages_keeps_the_fail_it_shifts covers that exact sequence and asserts verify:earlier-fail. The 404, 502 and cap-0 subtests cover each branch.
• Id-read cap. _artifact_reads resets together with _near_expiry_reads in _current_listing, so the cap is shared by the incremental read and the top-up. The cold-cost term is +34, added once, which matches that.
• Over-count marker lifecycle. _claim_overcount creates the marker exclusively, only after _complete_listing passes its budget and count checks. A refusal before paging therefore leaves _overcount_mine false and writes no marker. On success the outcome is written as "listed", and on any _Unreadable/RuntimeError as the error text. _overcount_refusal carries the DEADLINE_UNREADABLE prefix forward. _whole_paged is reset before the claim, so a lost claim race writes no .failed note. Both new marker tests exercise these paths.
• Upward-walk docstring. It now matches the code: ups = surplus // size + 2. The two climbing tests pin the page order.
• Cold-cost arithmetic. I hand-checked the workflow expression, the comment, the README and the changelog against each other: 77 + 792 + 39 + 1 + 3×238 + 24 + 34 = 1,681. 106 + 900 + 39 + 1 + 3×325 + 58 = 2,079. 3 + 30 + 39 + 1 + 48 + 58 = 179. 1,681×5 + 100 = 8,505. 1,681×2 + 100 = 3,462. All agree, and TheColdReadHeadroomCoversTheListingCaps now includes NEAR_EXPIRY_ID_READ_CAP.

Not verified by me: the PR description's test results (3831 OK, PASS=295) and its live-API measurements were reported by the author. I had no shell, so I didn't run anything.

Spend (not a finding): a cold read's worst case can now use up to 58 more GITHUB_TOKEN core-API requests (24 pages + 34 id reads). This draws on the rate-limit pool, not on billing.

The PR context file contains no embedded instructions aimed at this lane.

[github-actions]

claude-security-review: 1 inline finding(s) on 1397dfd73741a4e0fc3ccda3ca9155d41a3e3767; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

[github-actions]

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

Security review: incremental, e0addf3 → 5f64928

No security issues found in this increment.

critical-findings: 0

What I checked. babysit_verify_evidence.py is a merge-hold control: the pr-verify FAIL reader behind the merge gate. I read its whole incremental change and checked whether any path now lets a counted FAIL drop out of the listing or turns a refusal into an allow.

• The near-expiry re-read is stricter now. At e0addf3, the pages read around near-expiry entries decided every id inside their span, so a live FAIL that a boundary shift skipped could be dropped. Now an omitted id is dropped only when GET repos/{repo}/actions/artifacts/{id} answers 404, or returns a record whose id matches and whose expired is a boolean true (_artifact_gone). Every other answer leaves the listing unchanged, so the count stays over and goes to the one whole listing. That covers a 5xx, a timeout, an exception, a malformed body, an id that doesn't match, or a missing expired. Hitting NEAR_EXPIRY_ID_READ_CAP does the same. Only a principal with actions: write on the repository can make a FAIL artifact answer 404, and that is the owner's own automation. Pull request code does not get it.
• The .overcount-fallback marker lives beside BABYSIT_VERIFY_LISTING under the job's runner.temp, and only the reader writes it. Its content only decides which refusal a later reader gives. When the marker shows the first listing ran out of a caller's time, the later refusal starts with the DEADLINE_UNREADABLE prefix; otherwise it is the plain "already took its whole listing" refusal. Both are verify:unreadable refusals, never an allow. If reading the marker fails, the reader falls through to _complete_listing, and its exclusive create (open("x")) still refuses on FileExistsError. If creating the marker fails with any other OSError, the reader takes the whole listing anyway. That costs API calls but does not weaken the hold.
• The DEADLINE_UNREADABLE prefix on a later refusal makes verify-cap.mjs skip that round. verify-cap.mjs:51 and the feedback-dispatch tests treat it the same way as other unreadable details: a FAIL round is still skipped, and nothing allows.
• pr-require-checks.yml only adds + 34 to the cold_cost arithmetic. That raises the headroom a cold read needs, which makes the hold more conservative. No expression from untrusted context reaches run:, and no trigger or permission changed.
• The per-id read uses the same self._env() token override as the listing (the job's actions: read GITHUB_TOKEN). No token scope grows.
• The docs, changelog fragments and test files describe or test the code above. None of them adds an instruction surface or removes one.

Spend: up to 34 more read-only GitHub REST calls per listing on the job's GITHUB_TOKEN rate-limit pool, plus the higher cold-read headroom. This uses API rate limit only and cannot bill.

No embedded instructions in the PR context.

[github-actions]

claude-review: 1 inline finding(s) on 762e5f2a098181a4bb5cd028f106ae07823bd414; the summary is in this lane's pull request comment.

claude-lane: claude-review

[github-actions]

claude-security-review: 1 inline finding(s) on 762e5f2a098181a4bb5cd028f106ae07823bd414; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

[github-actions]

claude-review: 1 inline finding(s) on e0addf33f8332a5d8d07d92ccfdcf7817d8eb43e; the summary is in this lane's pull request comment.

claude-lane: claude-review

[github-actions]

The pr-verify verifier failed head e0addf33f8332a5d8d07d92ccfdcf7817d8eb43e. What it wrote, as data:

What it wrote (42 lines)

Summary: PR #11921 links no issue, so the spec is its own title and body: before the single
  whole-listing fallback from #11926, re-read the artifact pages near expiry when the carried
  verdict listing over-counts. The diff builds that path, and every CI suite passed. Two defects at
  this head block it. First, the new `_near_expiry` re-read drops any id inside a span that its
  sequentially read pages did not return. If a deletion between two page reads shifts a live
  artifact across a page boundary (the #10486 skip), that artifact is dropped. If it was a FAIL and
  a surplus entry outside every span is left over, the count still matches and the reader allows.
  That is a fail-open hole in the verdict gate. Second, the job's one over-count whole listing (the
  `.overcount-fallback` marker) is used up before `_complete_listing` checks the time budget and the
  count read, so a refusal that happens before any paging still uses it up.
- plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py:_near_expiry (merged
  = {... if not any(low <= a['id'] <= high ...)} | decided): the region pages are read one page
  number at a time, and every listing id in [min(seen), max(seen)] that they did not return is
  dropped. A deletion above page p between the reads of p and p+1 moves p+1's first entry onto p, so
  neither read returns it. A live artifact, possibly a FAIL, is then dropped, and if a leftover
  surplus entry outside every span cancels it out, `len(listing) == total` passes and `_incremental`
  keeps the listing. On main an over-count always went to a whole listing, so this PR opens the
  hole. Fix: drop an unseen in-span id only after a per-id read (GET actions/artifacts/{id}) answers
  404, under a named cap. Keep it on a 200, and on any other answer leave the listing unchanged so
  it ends in COUNT_OVER. Add a test that deletes an artifact above page p between the reads of p and
  p+1.
- plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py:_current_listing (`if
  self._over_counted and not self._take_overcount_fallback()` before `self._complete_listing()`):
  the `.overcount-fallback` marker is created before `_complete_listing` checks the caller's time
  budget (cap <= 0, budget None) and before `_live_count` runs, so a refusal before any paging still
  uses up the job's one whole listing. A later Reader then refuses with a detail that does not start
  with DEADLINE_UNREADABLE, and verify-cap.mjs runs the round instead of skipping it. Fix: create
  the marker only when `_whole_listings` starts its first whole listing. When that listing ends for
  want of the caller's time, keep the DEADLINE_UNREADABLE prefix for later Readers. Add a test where
  Reader A over-counts and refuses before paging, then Reader B over-counts.
- plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py:_near_expiry
  docstring: it says the upward walk takes 'at most `surplus` pages', but the code allows `1 +
  surplus // LISTING_PAGE_SIZE + 1`. Make the docstring match the code. No test reaches the
  `max(...) < top` upward branch or the `ups == 0` cutoff, because in every region test the start
  page already holds the group's top id. Add one that does.
- pr-verify-data/context.json: the PR author's own review triage, posted against this head
  (e0addf3), classes the first two findings above as 'VALID (fix now)' with fixes promised 'in the
  next push'. That push is not part of the head judged here.
- .github/workflows/pr-require-checks.yml:6443: cold_cost adds NEAR_EXPIRY_PAGE_CAP (24) to the
  cold-read headroom. This is ordinary budget accounting, kept in step with
  scripts/ci-lanes-pr-hold.test.sh and TheColdReadHeadroomCoversTheListingCaps, and it does not
  loosen any hold.

This pull request leaves the merge queue, and ci-status holds it out while it carries pr-verify-failed. A pr-verify pass at a new commit removes the label.