From 22f7bbc31f694dba93b7627e5fa691ee05d109a1 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 1 Nov 2026 04:58:03 -0500 Subject: [PATCH 1/5] fix(source-control): re-read the pages near expiry when the verdict listing over-counts GitHub deleted carried artifacts up to about an hour before their expires_at on 2026-11-01, so the verdict reader's incremental listing held more than the live total_count, missed three times, and fell back to a full listing that no longer fits verify-cap's 120 s limit (verify:unreadable on #11903). When the incremental result over-counts, read again only the pages around the kept entries expiring within EARLY_DELETION_LEAD and let them decide the ids they span; the exact-count check is unchanged. Co-Authored-By: Claude Opus 5.5 --- ...-verify-listing-early-deletion-5b1e9c3a.md | 7 ++ .../scripts/babysit_verify_evidence.py | 109 +++++++++++++++++- .../tests/test_babysit_verify_evidence.py | 107 +++++++++++++++++ 3 files changed, 220 insertions(+), 3 deletions(-) create mode 100644 .changes/source-control/fix-verify-listing-early-deletion-5b1e9c3a.md diff --git a/.changes/source-control/fix-verify-listing-early-deletion-5b1e9c3a.md b/.changes/source-control/fix-verify-listing-early-deletion-5b1e9c3a.md new file mode 100644 index 00000000000..9694ca5cd38 --- /dev/null +++ b/.changes/source-control/fix-verify-listing-early-deletion-5b1e9c3a.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Fixed + +- The verdict gate reader (`babysit_verify_evidence.py`) no longer falls back to listing every artifact when GitHub has deleted carried artifacts before their `expires_at`. When the incremental listing holds more than GitHub's live count, it reads again only the pages around the carried entries that expire within `EARLY_DELETION_LEAD` and lets them decide those ids, keeping the exact-count check. On the carried listing behind the `verify:unreadable` refusal of #11903, the reader now finishes incrementally in 21 page reads and 15.9 s, where it refused after 28 page reads and 64.6 s under verify-cap's 120 s limit. Each `babysit-listing-miss` line now also reports `region_pages`. 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 0b6424c5f52..533fb138edb 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 @@ -144,8 +144,12 @@ newest id first, down past the base's highest id until an artifact created `LISTING_OVERLAP` before the base was read, and keeps the base's entries below the lowest id those pages hold, less those expired or past `expires_at` by the -first page's `Date`. Such a listing could hide a FAIL by leaving it out, so it -is kept only when it holds exactly the first page's `total_count`, GitHub's +first page's `Date`. GitHub can delete an artifact up to an hour before its +`expires_at`, in no expiry order (`EARLY_DELETION_LEAD`), so when that holds +more than the live count the pages around the kept entries near expiry are +read again and decide every id they span (`Reader._near_expiry`), at a few +pages a group whatever the repository's total. Such a listing could hide a +FAIL by leaving it out, so it is kept only when it holds exactly the first page's `total_count`, GitHub's live count of the repository's artifacts. Across 17 first-page reads on 2026-10-27 that count moved by exactly the uploads since the last read less the artifacts whose `expires_at` had passed by the response's `Date`. Three @@ -372,6 +376,16 @@ def verdict_name(name: str) -> re.Match[str] | None: # for the whole listing that day (judgment on that data). LISTING_CARRY_MAX_AGE = timedelta(hours=3) LISTING_PAGE_SIZE = 100 +# How far ahead of its `expires_at` GitHub may already have deleted a listed +# artifact. On 2026-11-01 a carried listing held 234 artifacts, all expiring +# 09:20 to 09:55Z, that three later listings lacked; the 60 of them read by id +# at 09:37Z each answered 404. A listing at 08:55Z already lacked ones +# expiring up to 59 minutes later, not in expiry order (an 09:13Z listing +# still held 8 of them). The 2026-10-27 counts in this module's docstring +# showed none. The cause is unverified; recheck on a `babysit-listing-miss` +# line whose `listed` exceeds `total_count`. Twice the largest lead seen +# (judgment on that data). +EARLY_DELETION_LEAD = timedelta(hours=2) # The waits between incremental reads on one base. A count that misses the # live total can stay off for longer than an immediate re-read takes, while a # page boundary settles; bounded waits let it settle before the reader pages @@ -1393,7 +1407,9 @@ def _incremental( base holds there and they do not (deleted, or an overwrite's old copy) included; below it the base's entries stand, less each one expired or past its `expires_at` by the first page's `Date` (one with - no `expires_at` stands unless expired). The + no `expires_at` stands unless expired). A result that holds more than + the live count has the ids around its kept entries near expiry + decided by the pages that hold them now (`_near_expiry`). The result must hold exactly the first page's `total_count` artifacts, GitHub's live count: any other count gives `COUNT_MISSED`, and one `LISTING_MISS_PREFIX` line on stderr names `stage`, the counts and @@ -1455,6 +1471,16 @@ def _incremental( and (a["expires_at"] is None or _when(a["expires_at"]) > server_now) ] listing = sorted([*fresh.values(), *kept], key=lambda a: -a["id"]) + regions = 0 + if len(listing) > total: + # GitHub can delete an artifact before its expires_at: read again + # the pages around the kept entries near expiry. + reread = self._near_expiry( + listing, len(fresh), len(listing) - total, server_now, deadline + ) + if reread is None: + return None + listing, regions = reread if len(listing) == total: return listing # Fewer than the live count: the pages skipped an artifact. More: the @@ -1466,6 +1492,7 @@ def _incremental( "fresh": len(fresh), "kept": len(kept), "pages": page, + "region_pages": regions, "base_age_seconds": round((server_now - base_at).total_seconds()), } print( @@ -1475,6 +1502,82 @@ def _incremental( ) return COUNT_MISSED + def _near_expiry( + self, + listing: list[dict[str, Any]], + start: int, + surplus: int, + server_now: datetime, + deadline: float, + ) -> tuple[list[dict[str, Any]], int] | None: + """`listing` with the ids around its entries from `start` on that + expire within `EARLY_DELETION_LEAD` of `server_now` decided by the + pages that hold them now, and the pages read; None for a failed read. + + Such entries are grouped where they lie within a page of each other. + Each group is read from the page its top entry would be on with + `surplus` fewer entries above it, a page up at a time (at most + `surplus` pages) until a page reaches its top id, then down until a + page reaches its bottom id or ends the listing; those pages decide + every id from the lowest to the highest they hold. A group it cannot + bracket within those pages leaves `listing` as it was.""" + size = LISTING_PAGE_SIZE + limit = server_now + EARLY_DELETION_LEAD + near = [ + i + for i in range(start, len(listing)) + if listing[i]["expires_at"] is not None + and server_now < _when(listing[i]["expires_at"]) <= limit + ] + groups: list[list[int]] = [] + for i in near: + if groups and i - groups[-1][1] <= size: + groups[-1][1] = i + else: + groups.append([i, i]) + decided: dict[int, dict[str, Any]] = {} + spans: list[tuple[int, int]] = [] + reads = 0 + for first, last in groups: + top, bottom = listing[first]["id"], listing[last]["id"] + page = max(0, first - surplus) // size + 1 + ups = 1 + surplus // size + 1 + downs = (last - first) // size + 2 + surplus // size + 1 + seen: dict[int, dict[str, Any]] = {} + ended = False + while True: + read = self._incremental_page(page, deadline) + reads += 1 + if read is None: + return None + items = read[0].get("artifacts") + if not is_json_array(items) or not all(map(_listed_entry, items)): + return None + if not seen and (not items or max(a["id"] for a in items) < top): + if page == 1 or ups == 0: + break + ups -= 1 + page -= 1 + continue + seen.update((a["id"], a) for a in items) + if len(items) < size: + ended = True + break + if min(a["id"] for a in items) <= bottom or downs == 0: + break + downs -= 1 + page += 1 + if not seen or (not ended and min(seen) > bottom): + return listing, reads + spans.append((0 if ended else min(seen), max(seen))) + decided.update(seen) + merged = { + a["id"]: a + for a in listing + if not any(low <= a["id"] <= high for low, high in spans) + } | decided + return sorted(merged.values(), key=lambda a: -a["id"]), reads + def _incremental_page( self, page: int, deadline: float ) -> tuple[dict[str, Any], dict[str, str]] | None: diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py index 130aae1ff89..5fca01ecc26 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py @@ -4076,6 +4076,112 @@ def test_a_carried_entry_the_pages_no_longer_hold_is_dropped(self) -> None: ids = [a["id"] for a in json.loads(self.saved.read_text())["artifacts"]] self.assertEqual(ids, [1050, 900, 800, 700]) + def tail(self, *expiring: tuple[int, int]) -> list[dict[str, Any]]: + """The base with four more old artifacts (600 to 300) below it, each + `(id, minutes)` among them expiring that many minutes after GitHub's + `Date`.""" + soon = dict(expiring) + old = self.base_at - timedelta(hours=3) + return [ + *self.base, + *( + dated( + "pr-merge-gate-cursor", + 3, + i, + old, + expires=LISTED_AT + timedelta(minutes=soon[i]) + if i in soon + else None, + ) + for i in (600, 500, 400, 300) + ), + ] + + def test_a_carried_entry_deleted_before_its_expiry_is_read_again(self) -> None: + # GitHub deleted 500 half an hour before its expires_at, as it did on + # 2026-11-01; 400 expires 40 minutes on and stands. The base holds both, + # so first page and base hold one more than the live count. The pages + # around the entries near expiry decide them instead of a full listing. + base = self.tail((500, 30), (400, 40)) + self.write_carry(base) + fake = self.fake(base=[a for a in base if a["id"] != 500]) + reader, evidence = self.judge(fake) + self.assertIsNone(evidence.refusal) + self.assertEqual(reader.listing_mode, "incremental") + self.assertEqual(fake.full_listings(), 0) + # Page 1 (1000, 900) reaches the base; page 3 (600, 400) brackets 500 + # and 400, the entries near expiry. + self.assertEqual(fake.pages(), 2) + ids = [a["id"] for a in json.loads(self.saved.read_text())["artifacts"]] + self.assertEqual(ids, [1000, 900, 800, 700, 600, 400, 300]) + + def test_an_early_deletion_out_of_expiry_order_is_read_again(self) -> None: + # 400 expires before 500 but GitHub deleted only 400: the pages, not + # the expiry order, decide which entry went. + base = self.tail((500, 30), (400, 20)) + self.write_carry(base) + fake = self.fake(base=[a for a in base if a["id"] != 400]) + reader, _ = self.judge(fake) + self.assertEqual(reader.listing_mode, "incremental") + self.assertEqual(fake.full_listings(), 0) + # Page 3 (600, 500) does not reach 400; page 4 (300) ends the listing. + self.assertEqual(fake.pages(), 3) + ids = [a["id"] for a in json.loads(self.saved.read_text())["artifacts"]] + self.assertEqual(ids, [1000, 900, 800, 700, 600, 500, 300]) + + def test_a_fail_a_forged_carry_drops_beside_an_early_deletion_counts( + self, + ) -> None: + # A forged carry leaves out the FAIL 450 GitHub lists between 500 and + # 400, both deleted early: the region's pages bring the FAIL back. + base = self.tail((500, 30), (400, 40)) + self.write_carry(base) + fail = dated(named(EARLIER, "fail"), 2, 450, self.base_at - timedelta(days=2)) + live = [a for a in base if a["id"] not in (500, 400)] + fake = self.fake(fail, base=live) + reader, evidence = self.judge(fake) + self.assertEqual(evidence.refusal, "verify:earlier-fail") + self.assertEqual(reader.listing_mode, "incremental") + self.assertEqual(fake.full_listings(), 0) + ids = [a["id"] for a in json.loads(self.saved.read_text())["artifacts"]] + self.assertEqual(ids, [1000, 900, 800, 700, 600, 450, 300]) + + def test_a_failed_read_of_the_pages_near_expiry_lists_every_page(self) -> None: + # Page 3, the one around 500 and 400, answers a 502 on every read: + # past the page's own retries (`LISTING_WAITS`), like any failed + # incremental read, every page is listed with no `INCREMENTAL_WAITS`. + base = self.tail((500, 30), (400, 40)) + self.write_carry(base) + fake = self.fake(base=[a for a in base if a["id"] != 500]) + + def page(args: list[str], **kwargs: Any) -> Any: + if any(a.endswith("&page=3") for a in args): + failed = subprocess.CompletedProcess(args, 1, "", "gh: boom (HTTP 502)") + return failed, {} + return fake(args, **kwargs), {"date": DATE} + + with ( + mock.patch.object(postmerge, "gh_capture", side_effect=fake), + mock.patch.object(ve, "gh_api_with_headers", side_effect=page), + ): + reader = ve.Reader(REPO, "main", now=NOW) + reader.evidence(PR, HEAD, pull()) + self.assertEqual(reader.listing_mode, "full") + self.assertEqual(fake.full_listings(), 1) + waits = [c.args[0] for c in self.sleep.call_args_list] + self.assertEqual(waits, list(ve.LISTING_WAITS)) + + def test_a_deletion_far_from_expiry_still_lists_every_page(self) -> None: + # 300 expires in 90 days but GitHub no longer lists it, and nothing + # in the base is near expiry: no region to read, so every page. + base = self.tail() + self.write_carry(base) + fake = self.fake(base=[a for a in base if a["id"] != 300]) + reader, _ = self.judge(fake) + self.assertEqual(reader.listing_mode, "full") + self.assertEqual(fake.full_listings(), 1) + def test_a_count_other_than_githubs_lists_every_page(self) -> None: # A forged carry drops an old FAIL (600): the live count holds it, so # the merged listing is one short and every page is read. @@ -4321,6 +4427,7 @@ def test_each_miss_prints_its_counts_on_stderr(self) -> None: "fresh": 4, "kept": 1, "pages": 2, + "region_pages": 0, "base_age_seconds": 605, }, ) From e0addf33f8332a5d8d07d92ccfdcf7817d8eb43e Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 1 Nov 2026 07:00:22 -0500 Subject: [PATCH 2/5] fix(source-control): take one over-count whole listing a job and count the re-read in cold_cost - An incremental over-count that takes its whole listing notes it beside the saved listing (`.overcount-fallback`), whatever that listing's outcome, created exclusively so parallel processes take one. A later Reader of the job whose pages over-count refuses verify:unreadable without another whole listing. #11926's `.failed` note and its transient and caller-limit rules are unchanged. - pr-require-checks.yml cold_cost adds NEAR_EXPIRY_PAGE_CAP (24): the worst cold read is three short incremental reads, two whole listings with short top-ups and a third whose top-up over-counts and runs the re-read. 1,623 -> 1,647 at 7,522 artifacts and 264 FAILs, 2,021 -> 2,045 at 10,450 and 300; the comment, the Python and shell tests and the pr-pipeline README cap list follow. Co-Authored-By: Claude Opus 5.5 --- ...-verify-listing-early-deletion-9c4e2a17.md | 3 +- ...-verify-listing-early-deletion-5b1e9c3a.md | 2 +- .github/workflows/pr-require-checks.yml | 14 ++++---- docs/conventions/pr-pipeline/README.md | 6 ++-- .../scripts/babysit_verify_evidence.py | 33 ++++++++++++++++++- .../tests/test_babysit_verify_evidence.py | 31 ++++++++++++++--- scripts/ci-lanes-pr-hold.test.sh | 29 ++++++++-------- 7 files changed, 89 insertions(+), 29 deletions(-) diff --git a/.changes/pr-pipeline/fix-verify-listing-early-deletion-9c4e2a17.md b/.changes/pr-pipeline/fix-verify-listing-early-deletion-9c4e2a17.md index 92fdc4ded7a..c7c1d09c02c 100644 --- a/.changes/pr-pipeline/fix-verify-listing-early-deletion-9c4e2a17.md +++ b/.changes/pr-pipeline/fix-verify-listing-early-deletion-9c4e2a17.md @@ -4,4 +4,5 @@ bump: patch ### Fixed -- The verdict reader's carried listing first re-reads the pages around entries near expiry, up to a page cap, when it holds more than GitHub's live count. That over-count then takes at most one whole listing. +- The verdict reader's carried listing first re-reads the pages around entries near expiry, up to a page cap, when it holds more than GitHub's live count. That over-count then takes at most one whole listing a job. +- The merge queue's cold-read headroom check (`cold_cost` in pr-require-checks.yml) counts those re-read pages, up to 24 more requests a cold read: 1,647 at 7,522 artifacts and 264 FAILs. diff --git a/.changes/source-control/fix-verify-listing-early-deletion-5b1e9c3a.md b/.changes/source-control/fix-verify-listing-early-deletion-5b1e9c3a.md index ee184608f9d..dabd8f618dd 100644 --- a/.changes/source-control/fix-verify-listing-early-deletion-5b1e9c3a.md +++ b/.changes/source-control/fix-verify-listing-early-deletion-5b1e9c3a.md @@ -4,4 +4,4 @@ bump: patch ### Fixed -- The verdict gate reader (`babysit_verify_evidence.py`) settles an over-count from early deletion before it falls back to a whole listing. GitHub can delete artifacts before their `expires_at`. When the incremental listing holds more than GitHub's live count, the reader first re-reads only the pages around the carried entries that expire within `EARLY_DELETION_LEAD`, and lets those pages decide the ids. It reads at most `NEAR_EXPIRY_PAGE_CAP` (24) such pages per listing. A count those pages leave unsettled, above or below the live count, goes to the single whole listing added in #11926, which refuses `verify:unreadable` when its top-up misses the count. An over-count now takes that one whole listing even when its top-up comes up short, and the refusal is noted for the job's later readers. On the carried listing behind the `verify:unreadable` refusal of #11903, the reader finished incrementally in 21 page reads and 15.9 s. With #11926 alone, that over-count goes to a whole listing of about 133 pages, past verify-cap's 120 s limit, so it refuses (derived from the measured page count, not re-run). Each `babysit-listing-miss` line also reports `region_pages`. +- The verdict gate reader (`babysit_verify_evidence.py`) settles an over-count from early deletion before it falls back to a whole listing. GitHub can delete artifacts before their `expires_at`. When the incremental listing holds more than GitHub's live count, the reader first re-reads only the pages around the carried entries that expire within `EARLY_DELETION_LEAD`, and lets those pages decide the ids. It reads at most `NEAR_EXPIRY_PAGE_CAP` (24) such pages per listing. A count those pages leave unsettled, above or below the live count, goes to the single whole listing added in #11926, which refuses `verify:unreadable` when its top-up misses the count. An over-count now takes that one whole listing even when its top-up comes up short, and the refusal is noted for the job's later readers. The over-count's whole listing is also noted beside the saved listing (`.overcount-fallback`), whatever its outcome, so a later reader in the same job whose pages over-count refuses `verify:unreadable` without another whole listing. On the carried listing behind the `verify:unreadable` refusal of #11903, the reader finished incrementally in 21 page reads and 15.9 s. With #11926 alone, that over-count goes to a whole listing of about 133 pages, past verify-cap's 120 s limit, so it refuses (derived from the measured page count, not re-run). Each `babysit-listing-miss` line also reports `region_pages`. diff --git a/.github/workflows/pr-require-checks.yml b/.github/workflows/pr-require-checks.yml index f222a54e9df..8c582542555 100644 --- a/.github/workflows/pr-require-checks.yml +++ b/.github/workflows/pr-require-checks.yml @@ -6390,13 +6390,15 @@ jobs: # most 13 pages each, LISTING_CARRY_MAX_AGE), then one count read # and LISTING_READS (3) whole listings, each up to # 1 + len(LISTING_WAITS) (3) attempts that page from the first, - # and a top-up after each (LISTING_TOP_UP_PAGES, 7). A whole + # and a top-up after each (LISTING_TOP_UP_PAGES, 7), plus at most + # NEAR_EXPIRY_PAGE_CAP (24) pages near expiry read again when a + # count is over. A whole # listing pages ceil(total_count / 100), plus one for uploads while # it pages, so a cold read costs at most - # 3 x FAILs + 3 x 13 + 1 + 3 x (3 x pages + 7), plus the pages of - # this step's own listing, which every entry the queue builds at - # once also reads, cold_cost below: 1,623 at 7,522 artifacts and - # 264 FAILs, 2,021 at 10,450 and 300. + # 3 x FAILs + 3 x 13 + 1 + 3 x (3 x pages + 7) + 24, plus the pages + # of this step's own listing, which every entry the queue builds at + # once also reads, cold_cost below: 1,647 at 7,522 artifacts and + # 264 FAILs, 2,045 at 10,450 and 300. # test_babysit_verify_evidence runs these lines against those caps # at several sizes. It covers this path only: an # entry with a carried run cache but no carried listing pays the @@ -6438,7 +6440,7 @@ jobs: exit 1 fi listing_pages=$(((total + 99) / 100 + 1)) - cold_cost=$((listing_pages + 3 * fails + 3 * 13 + 1 + 3 * (3 * listing_pages + 7))) + cold_cost=$((listing_pages + 3 * fails + 3 * 13 + 1 + 3 * (3 * listing_pages + 7) + 24)) rate=$(read_api "the GITHUB_TOKEN rate limit" rate_limit) || rate= if ! remaining=$(jq -er '.resources.core.remaining | numbers | floor' <<<"$rate" 2>/dev/null) || ! limit=$(jq -er '.resources.core.limit | numbers | floor' <<<"$rate" 2>/dev/null) || diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index e8ab6a17f04..ed7d979dc72 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -2002,7 +2002,8 @@ at once goes cold together, so the step first lists every artifact page once and starts only when the token's core pool then has at least one cold read's worst case under the listing's caps (the step's own listing, 3 reads per listed FAIL, 3 incremental reads of at most 13 pages each, one count read, and 3 whole listings of up to 3 attempts of `ceil(total_count / 100) + 1` -pages, each with a top-up of at most 7: 1,623 at 7,522 artifacts and 264 FAILs) times +pages, each with a top-up of at most 7, and at most 24 pages near expiry read again: 1,647 at 7,522 +artifacts and 264 FAILs) times `max_entries_to_build`, plus the read floor (`limits.api` `read-floor` of the planned limit, inlined in the job env), left. An entry with a carried run cache but no carried listing pays the listing's part @@ -2655,7 +2656,8 @@ live count the candidate refuses `verify:unreadable`, and after the first when i than the count or does not read, or the pages read before it held more, which another whole listing cannot cure. A listing that fails once it began a whole listing is noted beside the saved listing, and every later reader in the job -refuses on that note without paging. +refuses on that note without paging. So is the one whole listing an over-count takes, whatever its +outcome: a later reader in the job whose pages over-count refuses without another. The lane token holds no Actions permission, so pr-merge.yml grants its job `actions: read` and passes the job's `GITHUB_TOKEN` as `BABYSIT_VERIFY_GH_TOKEN`; the reader uses it as `GH_TOKEN` for the artifact listing, the run records, the download of a verdict.json (by the listed artifact's id, one request where `gh run diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py index ada1ac1c834..8053ac96ec5 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 @@ -135,7 +135,9 @@ read before it held more, which reading again cannot cure. The refresh before an allow reads the same way. A listing from the API that fails once it began a whole listing is noted beside `LISTING_ENV`'s file (`FAILED_LISTING_SUFFIX`), and every later Reader of the job refuses on it -without paging. +without paging. The whole listing an over-count takes is noted there too +(`OVERCOUNT_FALLBACK_SUFFIX`), whatever its outcome, and a later Reader of the +job whose pages over-count refuses without another. 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 @@ -449,6 +451,11 @@ class _CountMissed: # candidate, each paging three whole listings, 825 to 1,015 calls a run on # 2026-11-01). FAILED_LISTING_SUFFIX = ".failed" +# Beside `LISTING_ENV`'s file, written when an incremental over-count takes its +# whole listing, whatever that listing's outcome: a later Reader of the job +# whose pages over-count again refuses without another whole listing, so an +# over-count costs one whole listing a job. +OVERCOUNT_FALLBACK_SUFFIX = ".overcount-fallback" # The whole listings `Reader._complete_listing` reads before it refuses. Of # three whole listings read on 2026-10-27, two skipped live artifacts at page # boundaries (1 and 3, a FAIL in each), and each skipped artifact was in every @@ -1318,6 +1325,13 @@ def _current_listing(self) -> list[dict[str, Any]]: self.listing_mode = "incremental" if not isinstance(listing, list): self.listing_mode = "full" + if self._over_counted and not self._take_overcount_fallback(): + self._listing_error = ( + "repository artifacts: the pages held more than GitHub's live " + "artifact count, and this job already took its whole listing " + "for that" + ) + raise _Unreadable(self._listing_error) try: listing = self._complete_listing() except (_Unreadable, RuntimeError) as exc: @@ -1748,6 +1762,23 @@ def _failed_listing(self) -> str | None: return None return text or "repository artifacts: an earlier listing in this job failed" + def _take_overcount_fallback(self) -> bool: + """Whether this Reader may take the job's one whole listing for an + over-count (`OVERCOUNT_FALLBACK_SUFFIX`), noting it taken. With no + `listing_path` there is no job to note it for, so it may; a failed + write leaves the next Reader to take it too.""" + if not self.listing_path: + return True + path = Path(f"{self.listing_path}{OVERCOUNT_FALLBACK_SUFFIX}") + try: + with path.open("x", encoding="utf-8") as note: + note.write("taken") + except FileExistsError: + return False + except OSError: + pass + return True + def _note_failed_listing(self, error: str) -> None: """Best effort: a failed write leaves the next Reader to list. A refusal for want of one caller's time (`DEADLINE_UNREADABLE`) is not diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py index c2a44875121..b6ecfa694c3 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py @@ -4212,6 +4212,28 @@ def test_the_pages_near_expiry_stop_at_their_cap(self) -> None: self.assertEqual(fake.pages(), 3) self.assertEqual(self.sleep.call_args_list, []) + def test_two_readers_that_over_count_take_one_whole_listing_a_job( + self, + ) -> None: + # GitHub dropped 300, 90 days from expiry, so both Readers' pages over + # the carry hold one more than the live count. The first takes the + # whole listing, which holds the count, and allows. The second, its + # saved listing gone (as past its age), refuses without another. + base = self.tail() + self.write_carry(base) + fake = self.fake(base=[a for a in base if a["id"] != 300]) + _, first = self.judge(fake) + self.assertIsNone(first.refusal, first.detail) + self.assertEqual(fake.full_listings(), 1) + self.saved.unlink() + reader, second = self.judge(fake) + self.assertEqual(second.refusal, "verify:unreadable") + self.assertIn("already took its whole listing", second.detail or "") + self.assertEqual(reader.listing_mode, "full") + self.assertEqual(fake.full_listings(), 1) + # Not a failed listing: #11926's note stays unwritten. + self.assertFalse(self.failed.exists()) + def test_an_over_count_the_pages_near_expiry_leave_takes_one_whole_listing( self, ) -> None: @@ -4976,7 +4998,7 @@ class TheColdReadHeadroomCoversTheListingCaps(unittest.TestCase): out from the live listing: its `total_count` and the FAILs it lists. The workflow's figure must cover the reader's listing caps at any listing size, so raising `INCREMENTAL_READS`, `LISTING_READS`, `LISTING_WAITS` - or `LISTING_TOP_UP_PAGES` without it fails here. The other terms are the + `LISTING_TOP_UP_PAGES` or `NEAR_EXPIRY_PAGE_CAP` without it fails here. The other terms are the workflow comment's: 3 reads a FAIL (its run, a compare and a download), at most 13 pages an incremental read, the count read before the whole listings, one page more than the live count fills, and the step's own @@ -5013,14 +5035,15 @@ def test_the_workflow_cost_is_the_capped_worst_case_at_any_size(self) -> None: + ve.INCREMENTAL_READS * 13 + 1 + ve.LISTING_READS * (attempts * pages + ve.LISTING_TOP_UP_PAGES) + + ve.NEAR_EXPIRY_PAGE_CAP ) self.assertEqual(self.cold_cost(total, fails), worst) def test_the_cost_of_two_listings_differs_by_hand(self) -> None: # 10,450 artifacts (106 pages) and 300 FAILs against 7,522 (77) and - # 264: 2,021 against 1,623, worked by hand in ci-lanes-pr-hold.test.sh. - self.assertEqual(self.cold_cost(10_450, 300), 2_021) - self.assertEqual(self.cold_cost(7_522, 264), 1_623) + # 264: 2,045 against 1,647, worked by hand in ci-lanes-pr-hold.test.sh. + self.assertEqual(self.cold_cost(10_450, 300), 2_045) + self.assertEqual(self.cold_cost(7_522, 264), 1_647) # --- The --authority pr-merge path ------------------------------------------------ diff --git a/scripts/ci-lanes-pr-hold.test.sh b/scripts/ci-lanes-pr-hold.test.sh index ab2aeee6d7b..b6664192561 100755 --- a/scripts/ci-lanes-pr-hold.test.sh +++ b/scripts/ci-lanes-pr-hold.test.sh @@ -873,31 +873,32 @@ STUB_NO_LISTING=1 STUB_EVIDENCE="$(jq -nc --arg head "$SHA" '{refusal: null, hea # plus the read floor left. A cold read's cost comes from the live listing # (STUB_TOTAL artifacts, STUB_LISTED_FAILS FAILs): 3 reads a FAIL, 3 incremental # reads of 13 pages, the count read, and 3 whole listings of 3 attempts of -# ceil(total / 100) + 1 pages, each with a 7-page top-up. By default 7,522 +# ceil(total / 100) + 1 pages, each with a 7-page top-up, and 24 pages near +# expiry read again. By default 7,522 # artifacts (77 pages) and 264 FAILs: 77 for the step's own listing, plus -# 792 + 39 + 1 + 3 x (3 x 77 + 7) = 1623, so 1623 x 1 + 0.1 x min(limit, 1000) = 1723 here. +# 792 + 39 + 1 + 3 x (3 x 77 + 7) + 24 = 1647, so 1647 x 1 + 0.1 x min(limit, 1000) = 1747 here. rate() { jq -nc --argjson remaining "$1" '{resources: {core: {limit: 5000, remaining: $remaining, reset: 1793030000}}}'; } -STUB_COLD=1 STUB_RATE="$(rate 1723)" expect "in the merge queue, a cold reader with the headroom for it reads" 0 3 "#7: no pr-verify FAIL hold: the reader allows this head." +STUB_COLD=1 STUB_RATE="$(rate 1747)" expect "in the merge queue, a cold reader with the headroom for it reads" 0 3 "#7: no pr-verify FAIL hold: the reader allows this head." if [[ "$(readers)" -eq 1 ]]; then ok "a cold reader with headroom is started"; else fail "a cold reader with headroom ran $(readers) times"; fi -STUB_COLD=1 STUB_RATE="$(rate 1722)" expect "in the merge queue, a cold reader without the headroom holds without reading" 1 3 "::error::#7 is held: no carried run cache from pr-merge's last run, and GITHUB_TOKEN has 1722 requests left, under the 1723 a cold read needs" +STUB_COLD=1 STUB_RATE="$(rate 1746)" expect "in the merge queue, a cold reader without the headroom holds without reading" 1 3 "::error::#7 is held: no carried run cache from pr-merge's last run, and GITHUB_TOKEN has 1746 requests left, under the 1747 a cold read needs" if [[ "$(readers)" -eq 0 ]]; then ok "a cold reader without headroom is never started"; else fail "a cold reader without headroom ran: '$(grep '^reader ' "$TMP_ROOT/log")'"; fi STUB_COLD=1 STUB_RATE='' expect "in the merge queue, a cold reader whose rate limit does not read holds" 1 3 "::error::#7 is held: no carried run cache from pr-merge's last run, and GITHUB_TOKEN's remaining requests could not be read" if [[ "$(readers)" -eq 0 ]]; then ok "a cold reader with an unread rate limit is never started"; else fail "a cold reader with an unread rate limit ran"; fi STUB_COLD=1 STUB_RATE='{"resources":{"core":{"limit":5000}}}' expect "in the merge queue, a rate limit with no remaining count holds a cold reader" 1 3 "GITHUB_TOKEN's remaining requests could not be read" # 10,450 artifacts (105 pages, as on 2026-10-30, so 106) and 300 FAILs: -# 106 + 900 + 39 + 1 + 3 x (3 x 106 + 7) = 2021, plus the floor's 100. -STUB_COLD=1 STUB_TOTAL=10450 STUB_LISTED_FAILS=300 STUB_RATE="$(rate 2121)" expect "in the merge queue, a cold read's cost grows with the live listing" 0 3 "cold reads for 1 queue entries need 2121" -STUB_COLD=1 STUB_TOTAL=10450 STUB_LISTED_FAILS=300 STUB_RATE="$(rate 2120)" expect "in the merge queue, a larger live listing holds a cold reader the old cost would start" 1 3 "GITHUB_TOKEN has 2120 requests left, under the 2121 a cold read needs (2021 for each of the 1 entries" -# 150 artifacts (2 pages, so 3) and 10 FAILs: 3 + 30 + 39 + 1 + 3 x (3 x 3 + 7) = 121. -STUB_COLD=1 STUB_TOTAL=150 STUB_LISTED_FAILS=10 STUB_RATE="$(rate 221)" expect "in the merge queue, a small live listing needs only its own cost" 0 3 "cold reads for 1 queue entries need 221" +# 106 + 900 + 39 + 1 + 3 x (3 x 106 + 7) + 24 = 2045, plus the floor's 100. +STUB_COLD=1 STUB_TOTAL=10450 STUB_LISTED_FAILS=300 STUB_RATE="$(rate 2145)" expect "in the merge queue, a cold read's cost grows with the live listing" 0 3 "cold reads for 1 queue entries need 2145" +STUB_COLD=1 STUB_TOTAL=10450 STUB_LISTED_FAILS=300 STUB_RATE="$(rate 2144)" expect "in the merge queue, a larger live listing holds a cold reader the old cost would start" 1 3 "GITHUB_TOKEN has 2144 requests left, under the 2145 a cold read needs (2045 for each of the 1 entries" +# 150 artifacts (2 pages, so 3) and 10 FAILs: 3 + 30 + 39 + 1 + 3 x (3 x 3 + 7) + 24 = 145. +STUB_COLD=1 STUB_TOTAL=150 STUB_LISTED_FAILS=10 STUB_RATE="$(rate 245)" expect "in the merge queue, a small live listing needs only its own cost" 0 3 "cold reads for 1 queue entries need 245" STUB_COLD=1 STUB_TOTAL='' STUB_RATE="$(rate 9999)" expect "in the merge queue, a live listing that does not read holds a cold reader" 1 3 "::error::#7 is held: no carried run cache from pr-merge's last run, and the artifact listing could not be read" if [[ "$(readers)" -eq 0 ]]; then ok "a cold reader with an unread listing is never started"; else fail "a cold reader with an unread listing ran"; fi # Every entry the queue builds at once goes cold together on a missing cache, -# so the need scales with max_entries_to_build: 1623 x 5 + 100 = 8215. -STUB_COLD=1 STUB_RULES=5 STUB_RATE="$(rate 8215)" expect "in the merge queue, a cold reader reads with headroom for every entry the queue builds" 0 3 "#7: no pr-verify FAIL hold: the reader allows this head." -STUB_COLD=1 STUB_RULES=5 STUB_RATE="$(rate 8214)" expect "in the merge queue, a cold reader holds under the headroom of every entry the queue builds" 1 3 "GITHUB_TOKEN has 8214 requests left, under the 8215 a cold read needs" -STUB_COLD=1 STUB_RULES=2 STUB_RATE="$(rate 3345)" expect "in the merge queue, the cold headroom scales with max_entries_to_build" 1 3 "under the 3346 a cold read needs" -STUB_COLD=1 STUB_RULES='' STUB_RATE="$(rate 8215)" expect "in the merge queue, a merge queue rule that does not read holds a cold reader" 1 3 "the merge queue's max_entries_to_build could not be read" +# so the need scales with max_entries_to_build: 1647 x 5 + 100 = 8335. +STUB_COLD=1 STUB_RULES=5 STUB_RATE="$(rate 8335)" expect "in the merge queue, a cold reader reads with headroom for every entry the queue builds" 0 3 "#7: no pr-verify FAIL hold: the reader allows this head." +STUB_COLD=1 STUB_RULES=5 STUB_RATE="$(rate 8334)" expect "in the merge queue, a cold reader holds under the headroom of every entry the queue builds" 1 3 "GITHUB_TOKEN has 8334 requests left, under the 8335 a cold read needs" +STUB_COLD=1 STUB_RULES=2 STUB_RATE="$(rate 3393)" expect "in the merge queue, the cold headroom scales with max_entries_to_build" 1 3 "under the 3394 a cold read needs" +STUB_COLD=1 STUB_RULES='' STUB_RATE="$(rate 8335)" expect "in the merge queue, a merge queue rule that does not read holds a cold reader" 1 3 "the merge queue's max_entries_to_build could not be read" if [[ "$(readers)" -eq 0 ]]; then ok "a cold reader with an unread queue rule is never started"; else fail "a cold reader with an unread queue rule ran"; fi STUB_CARRIED='{"carried-run-record":{},"carried-fail-head-ref":{},"carried-on-branch":{}}' expect "in the merge queue, a carried cache with every section empty is cold" 1 3 "::error::#7 is held: no carried run cache from pr-merge's last run, and GITHUB_TOKEN's remaining requests could not be read" STUB_CARRIED='not json' expect "in the merge queue, a carried cache that does not parse is cold" 1 3 "GITHUB_TOKEN's remaining requests could not be read" From 5f649282a373bc18cb608010db7a09dd9a6e3fef Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 1 Nov 2026 07:16:59 -0500 Subject: [PATCH 3/5] fix(source-control): address PR review findings on the near-expiry re-read - An id inside a region span that the region pages omit is dropped only when its own read answers 404 or an expired record; a live record keeps it, and any other answer leaves the listing over, so it takes the one whole listing. Pages read one at a time skip the boundary entry when an artifact above it is deleted between reads (#10486); a skipped live FAIL plus a leftover surplus entry no longer passes the exact count. This covers the incremental and top-up paths, both through _near_expiry. NEAR_EXPIRY_ID_READ_CAP (34, the largest surplus behind #11903) bounds the id reads; cold_cost adds it (1,681 at 7,522 artifacts, 264 FAILs). - `.overcount-fallback` is created only when the over-count's whole listing starts paging, and holds its outcome: a refusal before paging leaves it free, and a later over-count's refusal keeps the DEADLINE_UNREADABLE prefix when that listing ran out of a caller's time, so verify-cap.mjs still skips the round. - The upward walk's docstring states the code's bound, surplus // LISTING_PAGE_SIZE + 2 pages; two tests reach the climb and the ups == 0 cutoff. Co-Authored-By: Claude Opus 5.5 --- ...-verify-listing-early-deletion-9c4e2a17.md | 4 +- ...-verify-listing-early-deletion-5b1e9c3a.md | 2 +- .github/workflows/pr-require-checks.yml | 13 +- docs/conventions/pr-pipeline/README.md | 15 +- .../scripts/babysit_verify_evidence.py | 165 +++++++++++++---- .../tests/test_babysit_verify_evidence.py | 168 +++++++++++++++++- scripts/ci-lanes-pr-hold.test.sh | 28 +-- 7 files changed, 331 insertions(+), 64 deletions(-) diff --git a/.changes/pr-pipeline/fix-verify-listing-early-deletion-9c4e2a17.md b/.changes/pr-pipeline/fix-verify-listing-early-deletion-9c4e2a17.md index c7c1d09c02c..a969bc8131a 100644 --- a/.changes/pr-pipeline/fix-verify-listing-early-deletion-9c4e2a17.md +++ b/.changes/pr-pipeline/fix-verify-listing-early-deletion-9c4e2a17.md @@ -4,5 +4,5 @@ bump: patch ### Fixed -- The verdict reader's carried listing first re-reads the pages around entries near expiry, up to a page cap, when it holds more than GitHub's live count. That over-count then takes at most one whole listing a job. -- The merge queue's cold-read headroom check (`cold_cost` in pr-require-checks.yml) counts those re-read pages, up to 24 more requests a cold read: 1,647 at 7,522 artifacts and 264 FAILs. +- The verdict reader's carried listing first re-reads the pages around entries near expiry when it holds more than GitHub's live count. That re-read has a page cap and an id-read cap, and drops an omitted entry only on a 404. The over-count then takes at most one whole listing a job. +- The merge queue's cold-read headroom check (`cold_cost` in pr-require-checks.yml) now counts those reads, up to 58 more requests a cold read: 1,681 at 7,522 artifacts and 264 FAILs. diff --git a/.changes/source-control/fix-verify-listing-early-deletion-5b1e9c3a.md b/.changes/source-control/fix-verify-listing-early-deletion-5b1e9c3a.md index dabd8f618dd..56fa78ca72d 100644 --- a/.changes/source-control/fix-verify-listing-early-deletion-5b1e9c3a.md +++ b/.changes/source-control/fix-verify-listing-early-deletion-5b1e9c3a.md @@ -4,4 +4,4 @@ bump: patch ### Fixed -- The verdict gate reader (`babysit_verify_evidence.py`) settles an over-count from early deletion before it falls back to a whole listing. GitHub can delete artifacts before their `expires_at`. When the incremental listing holds more than GitHub's live count, the reader first re-reads only the pages around the carried entries that expire within `EARLY_DELETION_LEAD`, and lets those pages decide the ids. It reads at most `NEAR_EXPIRY_PAGE_CAP` (24) such pages per listing. A count those pages leave unsettled, above or below the live count, goes to the single whole listing added in #11926, which refuses `verify:unreadable` when its top-up misses the count. An over-count now takes that one whole listing even when its top-up comes up short, and the refusal is noted for the job's later readers. The over-count's whole listing is also noted beside the saved listing (`.overcount-fallback`), whatever its outcome, so a later reader in the same job whose pages over-count refuses `verify:unreadable` without another whole listing. On the carried listing behind the `verify:unreadable` refusal of #11903, the reader finished incrementally in 21 page reads and 15.9 s. With #11926 alone, that over-count goes to a whole listing of about 133 pages, past verify-cap's 120 s limit, so it refuses (derived from the measured page count, not re-run). Each `babysit-listing-miss` line also reports `region_pages`. +- The verdict gate reader (`babysit_verify_evidence.py`) settles an over-count from early deletion before it falls back to a whole listing. GitHub can delete artifacts before their `expires_at`. When the incremental listing or a whole listing's top-up holds more than GitHub's live count, the reader first re-reads only the pages around the carried entries that expire within `EARLY_DELETION_LEAD`. Those pages add every id they hold. An id inside their span that they omit is dropped only when its own read answers 404, because pages read one at a time can skip a live entry at a boundary when an artifact above it is deleted. The re-read takes at most `NEAR_EXPIRY_PAGE_CAP` (24) pages and `NEAR_EXPIRY_ID_READ_CAP` (34) id reads per listing. A count it leaves unsettled, or an id read with any answer other than 200 or 404, goes to the single whole listing added in #11926, which refuses `verify:unreadable` when its top-up misses the count. An over-count now takes that whole listing at most once a job, even when its top-up comes up short. Once that listing starts paging, it is noted beside the saved listing (`.overcount-fallback`) with its outcome. A later reader in the job whose pages over-count then refuses without another whole listing, keeping the caller-limit prefix when that listing ran out of a caller's time. A refusal before paging leaves the listing to the next reader. On the carried listing behind the `verify:unreadable` refusal of #11903, the re-read finished incrementally in 21 page reads and 15.9 s, measured before the per-id reads were added. With #11926 alone, that over-count goes to a whole listing of about 133 pages, past verify-cap's 120 s limit, so it refuses (derived from the measured page count, not re-run). Each `babysit-listing-miss` line also reports `region_pages`. diff --git a/.github/workflows/pr-require-checks.yml b/.github/workflows/pr-require-checks.yml index 8c582542555..22c02a5f954 100644 --- a/.github/workflows/pr-require-checks.yml +++ b/.github/workflows/pr-require-checks.yml @@ -6392,13 +6392,14 @@ jobs: # 1 + len(LISTING_WAITS) (3) attempts that page from the first, # and a top-up after each (LISTING_TOP_UP_PAGES, 7), plus at most # NEAR_EXPIRY_PAGE_CAP (24) pages near expiry read again when a - # count is over. A whole + # count is over, and NEAR_EXPIRY_ID_READ_CAP (34) artifacts read + # by id that those pages omit. A whole # listing pages ceil(total_count / 100), plus one for uploads while # it pages, so a cold read costs at most - # 3 x FAILs + 3 x 13 + 1 + 3 x (3 x pages + 7) + 24, plus the pages - # of this step's own listing, which every entry the queue builds at - # once also reads, cold_cost below: 1,647 at 7,522 artifacts and - # 264 FAILs, 2,045 at 10,450 and 300. + # 3 x FAILs + 3 x 13 + 1 + 3 x (3 x pages + 7) + 24 + 34, plus the + # pages of this step's own listing, which every entry the queue + # builds at once also reads, cold_cost below: 1,681 at 7,522 + # artifacts and 264 FAILs, 2,079 at 10,450 and 300. # test_babysit_verify_evidence runs these lines against those caps # at several sizes. It covers this path only: an # entry with a carried run cache but no carried listing pays the @@ -6440,7 +6441,7 @@ jobs: exit 1 fi listing_pages=$(((total + 99) / 100 + 1)) - cold_cost=$((listing_pages + 3 * fails + 3 * 13 + 1 + 3 * (3 * listing_pages + 7) + 24)) + cold_cost=$((listing_pages + 3 * fails + 3 * 13 + 1 + 3 * (3 * listing_pages + 7) + 24 + 34)) rate=$(read_api "the GITHUB_TOKEN rate limit" rate_limit) || rate= if ! remaining=$(jq -er '.resources.core.remaining | numbers | floor' <<<"$rate" 2>/dev/null) || ! limit=$(jq -er '.resources.core.limit | numbers | floor' <<<"$rate" 2>/dev/null) || diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index ed7d979dc72..bde453e1fc4 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -2002,8 +2002,8 @@ at once goes cold together, so the step first lists every artifact page once and starts only when the token's core pool then has at least one cold read's worst case under the listing's caps (the step's own listing, 3 reads per listed FAIL, 3 incremental reads of at most 13 pages each, one count read, and 3 whole listings of up to 3 attempts of `ceil(total_count / 100) + 1` -pages, each with a top-up of at most 7, and at most 24 pages near expiry read again: 1,647 at 7,522 -artifacts and 264 FAILs) times +pages, each with a top-up of at most 7, at most 24 pages near expiry read again and 34 artifacts read by id: +1,681 at 7,522 artifacts and 264 FAILs) times `max_entries_to_build`, plus the read floor (`limits.api` `read-floor` of the planned limit, inlined in the job env), left. An entry with a carried run cache but no carried listing pays the listing's part @@ -2646,7 +2646,9 @@ highest id until an artifact created two hours before that listing was read, kee entries below the lowest id those pages hold, less those expired by the first page's `Date`, and is kept only when it holds exactly that page's `total_count`. A count above it first reads again the pages around kept entries near expiry, up to a page cap, since GitHub can delete an artifact up to an hour -before its `expires_at` (`EARLY_DELETION_LEAD`, `NEAR_EXPIRY_PAGE_CAP`). Any count still other than it, a missing, older or +before its `expires_at` (`EARLY_DELETION_LEAD`, `NEAR_EXPIRY_PAGE_CAP`). Those pages can skip an +entry at a boundary, so an entry they omit is dropped only when its own read answers 404, up to an +id-read cap (`NEAR_EXPIRY_ID_READ_CAP`). Any count still other than it, a missing, older or malformed base, or any failed read lists every page, so a carried listing that leaves out a FAIL lists every page too (the rule and its basis: `babysit_verify_evidence.py`, "Carried reads"). A whole listing is held to the same count: deduplicated by id, added to the whole listings read @@ -2656,8 +2658,11 @@ live count the candidate refuses `verify:unreadable`, and after the first when i than the count or does not read, or the pages read before it held more, which another whole listing cannot cure. A listing that fails once it began a whole listing is noted beside the saved listing, and every later reader in the job -refuses on that note without paging. So is the one whole listing an over-count takes, whatever its -outcome: a later reader in the job whose pages over-count refuses without another. +refuses on that note without paging. An over-count is the exception to that note's limits: once the +one whole listing an over-count takes starts paging, a later reader in the job whose pages +over-count refuses without another, even when that listing failed transiently or on a caller's +limit (then with the caller-limit prefix). A refusal before paging leaves that listing to the next +reader. The lane token holds no Actions permission, so pr-merge.yml grants its job `actions: read` and passes the job's `GITHUB_TOKEN` as `BABYSIT_VERIFY_GH_TOKEN`; the reader uses it as `GH_TOKEN` for the artifact listing, the run records, the download of a verdict.json (by the listed artifact's id, one request where `gh run diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py index 8053ac96ec5..e6c66335ab2 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 @@ -136,8 +136,9 @@ the API that fails once it began a whole listing is noted beside `LISTING_ENV`'s file (`FAILED_LISTING_SUFFIX`), and every later Reader of the job refuses on it without paging. The whole listing an over-count takes is noted there too -(`OVERCOUNT_FALLBACK_SUFFIX`), whatever its outcome, and a later Reader of the -job whose pages over-count refuses without another. +once it starts paging (`OVERCOUNT_FALLBACK_SUFFIX`), and a later Reader of the +job whose pages over-count refuses without another, whatever that listing's +outcome. 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 @@ -402,6 +403,12 @@ def verdict_name(name: str) -> re.Match[str] | None: # stays well inside verify-cap.mjs's 120 s limit and a fifth of that day's # 133-page whole listing. NEAR_EXPIRY_PAGE_CAP = 24 +# The most artifacts `Reader._near_expiry` reads by id for one listing, its +# incremental read and top-up together: each id its pages did not show is +# dropped only on a 404. It drops at most the listing's surplus over the live +# count, which was 31 to 34 in the three over-counts behind #11903's refusal; +# the largest (judgment on that data). Past it the over-count stands. +NEAR_EXPIRY_ID_READ_CAP = 34 # The waits between incremental reads on one base. A count that misses the # live total can stay off for longer than an immediate re-read takes, while a # page boundary settles; bounded waits let it settle before the reader pages @@ -451,11 +458,26 @@ class _CountMissed: # candidate, each paging three whole listings, 825 to 1,015 calls a run on # 2026-11-01). FAILED_LISTING_SUFFIX = ".failed" -# Beside `LISTING_ENV`'s file, written when an incremental over-count takes its -# whole listing, whatever that listing's outcome: a later Reader of the job -# whose pages over-count again refuses without another whole listing, so an -# over-count costs one whole listing a job. +# Beside `LISTING_ENV`'s file, created when an incremental over-count's whole +# listing starts paging and then holding that listing's outcome: a later Reader +# of the job whose pages over-count again refuses without another whole +# listing, whatever that outcome (a transient or caller-limit failure +# included, unlike `FAILED_LISTING_SUFFIX`), so an over-count costs one whole +# listing a job. A refusal before paging leaves it uncreated. OVERCOUNT_FALLBACK_SUFFIX = ".overcount-fallback" + + +def _overcount_refusal(outcome: str) -> str: + """A later Reader's refusal once the job's over-count whole listing was + taken, starting with `DEADLINE_UNREADABLE` when that listing ran out of a + caller's time, so verify-cap.mjs skips the round as it did for that one.""" + refusal = ( + "repository artifacts: the pages held more than GitHub's live " + "artifact count, and this job already took its whole listing for that" + ) + if outcome.startswith(DEADLINE_UNREADABLE): + return f"{DEADLINE_UNREADABLE}: {refusal} ({outcome})" + return refusal # The whole listings `Reader._complete_listing` reads before it refuses. Of # three whole listings read on 2026-10-27, two skipped live artifacts at page # boundaries (1 and 3, a FAIL in each), and each skipped artifact was in every @@ -1227,6 +1249,11 @@ class Reader: _over_counted: bool = False # The pages `_near_expiry` read for this listing (`NEAR_EXPIRY_PAGE_CAP`). _near_expiry_reads: int = 0 + # The ids `_near_expiry` read for this listing (`NEAR_EXPIRY_ID_READ_CAP`). + _artifact_reads: int = 0 + # This Reader took the job's over-count whole listing + # (`OVERCOUNT_FALLBACK_SUFFIX`) and records its outcome there. + _overcount_mine: bool = False _runs: dict[int, str] = field(default_factory=dict) _started: dict[int, Any] = field(default_factory=dict) _created: dict[int, Any] = field(default_factory=dict) @@ -1304,7 +1331,8 @@ def _current_listing(self) -> list[dict[str, Any]]: self._listing_error = failed raise _Unreadable(failed) fetched_at = _wall_clock() - self._over_counted, self._near_expiry_reads = False, 0 + self._over_counted, self._overcount_mine = False, False + self._near_expiry_reads = self._artifact_reads = 0 base = self._incremental_base() listing: list[dict[str, Any]] | _CountMissed | None = COUNT_MISSED # Only pages that read whole but missed the live count are read again, @@ -1325,20 +1353,21 @@ def _current_listing(self) -> list[dict[str, Any]]: self.listing_mode = "incremental" if not isinstance(listing, list): self.listing_mode = "full" - if self._over_counted and not self._take_overcount_fallback(): - self._listing_error = ( - "repository artifacts: the pages held more than GitHub's live " - "artifact count, and this job already took its whole listing " - "for that" - ) + taken = self._overcount_taken() if self._over_counted else None + if taken is not None: + self._listing_error = _overcount_refusal(taken) raise _Unreadable(self._listing_error) try: listing = self._complete_listing() except (_Unreadable, RuntimeError) as exc: self._listing_error = str(exc) + if self._overcount_mine: + self._note_overcount(self._listing_error) if self._whole_paged and not isinstance(exc, _Transient): self._note_failed_listing(self._listing_error) raise _Unreadable(self._listing_error) from exc + if self._overcount_mine: + self._note_overcount("listed") self._artifacts, self._fetched_at = listing, fetched_at self._base = (fetched_at, listing) self._saved = self._own = False @@ -1437,6 +1466,10 @@ def _complete_listing(self) -> list[dict[str, Any]]: f"a page, past {bound}, so none is started" ) full = listing_timeout(total, LISTING_TIMEOUT_SECONDS) + # The job's over-count whole listing is taken only as paging starts: + # a refusal before it leaves the next Reader free to list. + if self._over_counted and not self._claim_overcount(): + raise _Unreadable(_overcount_refusal(self._overcount_taken() or "")) try: return self._whole_listings(budget) except (_Unreadable, RuntimeError) as exc: @@ -1636,11 +1669,16 @@ def _near_expiry( Such entries are grouped where they lie within a page of each other. Each group is read from the page its top entry would be on with `surplus` fewer entries above it, a page up at a time (at most - `surplus` pages) until a page reaches its top id, then down until a - page reaches its bottom id or ends the listing; those pages decide - every id from the lowest to the highest they hold. A group it cannot - bracket within those pages, or a read past `NEAR_EXPIRY_PAGE_CAP` for - this listing, leaves `listing` as it was.""" + `surplus // LISTING_PAGE_SIZE + 2` pages above that one) until a page + reaches its top id, then down until a page reaches its bottom id or + ends the listing; those pages add every id they hold. An id of + `listing` from the lowest to the highest they hold that they do not + show is dropped only when its own read answers 404 + (`_artifact_gone`): pages read one at a time skip the entry at a + boundary when an artifact above it is deleted between reads (#10486). + A group it cannot bracket within those pages, a read past + `NEAR_EXPIRY_PAGE_CAP` or `NEAR_EXPIRY_ID_READ_CAP` for this listing, + or an id read with any other answer leaves `listing` as it was.""" size = LISTING_PAGE_SIZE limit = server_now + EARLY_DELETION_LEAD near = [ @@ -1694,13 +1732,49 @@ def _near_expiry( return listing, reads spans.append((0 if ended else min(seen), max(seen))) decided.update(seen) - merged = { - a["id"]: a - for a in listing - if not any(low <= a["id"] <= high for low, high in spans) - } | decided + gone: set[int] = set() + for a in listing: + if a["id"] in decided or not any( + low <= a["id"] <= high for low, high in spans + ): + continue + if self._artifact_reads >= NEAR_EXPIRY_ID_READ_CAP: + return listing, reads + self._artifact_reads += 1 + answer = self._artifact_gone(a["id"], deadline) + if answer is None: + return listing, reads + if answer: + gone.add(a["id"]) + merged = {a["id"]: a for a in listing if a["id"] not in gone} | decided return sorted(merged.values(), key=lambda a: -a["id"]), reads + def _artifact_gone(self, artifact_id: int, deadline: float) -> bool | None: + """Whether GitHub no longer holds the artifact: True on a 404 or an + expired record, False on a live one, None for any other answer.""" + left = deadline - _listing_clock() + if left <= 0: + return None + try: + proc, _ = gh_api_with_headers( + ["api", f"repos/{self.repo}/actions/artifacts/{artifact_id}"], + timeout_seconds=left, + env_overrides=self._env(), + ) + except (RuntimeError, ValueError, OSError): + return None + if proc.returncode != 0: + return True if gh_http_status(proc.stderr or "") == 404 else None + try: + record = json_object(json.loads(proc.stdout)) + except ValueError: + return None + if record.get("id") != artifact_id or not isinstance( + record.get("expired"), bool + ): + return None + return record["expired"] + def _incremental_page( self, page: int, deadline: float ) -> tuple[dict[str, Any], dict[str, str]] | None: @@ -1762,23 +1836,50 @@ def _failed_listing(self) -> str | None: return None return text or "repository artifacts: an earlier listing in this job failed" - def _take_overcount_fallback(self) -> bool: - """Whether this Reader may take the job's one whole listing for an - over-count (`OVERCOUNT_FALLBACK_SUFFIX`), noting it taken. With no - `listing_path` there is no job to note it for, so it may; a failed - write leaves the next Reader to take it too.""" + def _overcount_path(self) -> Path | None: + """Where the job's over-count whole listing is noted beside + `listing_path` (`OVERCOUNT_FALLBACK_SUFFIX`).""" if not self.listing_path: + return None + return Path(f"{self.listing_path}{OVERCOUNT_FALLBACK_SUFFIX}") + + def _overcount_taken(self) -> str | None: + """How the job's over-count whole listing ended ("" while it pages), + or None when no Reader of the job has taken it.""" + path = self._overcount_path() + if path is None: + return None + try: + return path.read_text(encoding="utf-8").strip() + except OSError: + return None + + def _claim_overcount(self) -> bool: + """Whether this Reader may take the job's one whole listing for an + over-count, noting it taken. With no `listing_path` there is no job + to note it for, so it may; a failed write leaves the next Reader to + take it too.""" + path = self._overcount_path() + if path is None: return True - path = Path(f"{self.listing_path}{OVERCOUNT_FALLBACK_SUFFIX}") try: - with path.open("x", encoding="utf-8") as note: - note.write("taken") + with path.open("x", encoding="utf-8"): + pass except FileExistsError: return False except OSError: - pass + return True + self._overcount_mine = True return True + def _note_overcount(self, outcome: str) -> None: + """Best effort: how the job's over-count whole listing ended, so a + later Reader's refusal carries a caller-limit refusal's prefix.""" + path = self._overcount_path() + if path is not None: + with contextlib.suppress(OSError): + path.write_text(outcome, encoding="utf-8") + def _note_failed_listing(self, error: str) -> None: """Best effort: a failed write leaves the next Reader to list. A refusal for want of one caller's time (`DEADLINE_UNREADABLE`) is not diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py index b6ecfa694c3..0f8f3c367f4 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py @@ -462,6 +462,10 @@ def rest( args, 1, b"", "gh: Not Found (HTTP 404)" ) return subprocess.CompletedProcess(args, 0, zipped(self.records[name]), "") + if found := re.fullmatch(rf"repos/{REPO}/actions/artifacts/([0-9]+)", path): + # One artifact by id, as GitHub's "Get an artifact" answers. + held = [a for a in self.artifacts if a.get("id") == int(found[1])] + return ok(held[0]) if held else missing() prefix = f"repos/{REPO}/actions/artifacts?per_page=" if path.startswith(prefix) and "&page=" in path: if self.page_fail: @@ -4196,6 +4200,125 @@ def test_a_deletion_far_from_expiry_still_lists_every_page(self) -> None: self.assertEqual(reader.listing_mode, "full") self.assertEqual(fake.full_listings(), 1) + def page_numbers(self, fake: FakeGitHub) -> list[int]: + """The single pages read, in order.""" + return [ + int(a.rsplit("&page=", 1)[1]) + for c in fake.calls + for a in c + if "&page=" in a + ] + + def test_a_deletion_between_region_pages_keeps_the_fail_it_shifts( + self, + ) -> None: + # The carry holds 1000 to 300 and FAIL 450. By page 1 GitHub dropped + # 800 (90 days from expiry) and 500 (expiring in 30 minutes): live + # count 7, the listing 9. The re-read around 500 takes page 2 (700, + # 600); 600 is deleted; page 3 is then (400, 300), so 450 is in + # neither read. Dropping it would leave 7, the live count, with the + # FAIL gone: it is read by id (live) and kept, so the count stays + # over and the one whole listing judges the FAIL. + old = self.base_at - timedelta(hours=3) + near = LISTED_AT + timedelta(minutes=30) + fail = dated(named(EARLIER, "fail"), 2, 450, self.base_at - timedelta(days=2)) + carry = [ + *self.base, + dated("pr-merge-gate-cursor", 3, 600, old), + dated("pr-merge-gate-cursor", 3, 500, old, expires=near), + fail, + *(dated("pr-merge-gate-cursor", 3, i, old) for i in (400, 300)), + ] + self.write_carry(carry) + fake = self.fake(base=[a for a in carry if a["id"] not in (800, 500)]) + shifted = [False] + + def page(args: list[str], **kwargs: Any) -> Any: + answer = fake(args, **kwargs) + if not shifted[0] and any(a.endswith("&page=2") for a in args): + shifted[0] = True + fake.artifacts = [a for a in fake.artifacts if a["id"] != 600] + return answer, {"date": DATE} + + with ( + mock.patch.object(postmerge, "gh_capture", side_effect=fake), + mock.patch.object(ve, "gh_api_with_headers", side_effect=page), + ): + reader = ve.Reader(REPO, "main", now=NOW) + evidence = reader.evidence(PR, HEAD, pull()) + self.assertEqual(self.page_numbers(fake)[:3], [1, 2, 3]) + self.assertEqual(evidence.refusal, "verify:earlier-fail", evidence.detail) + self.assertEqual(reader.listing_mode, "full") + self.assertEqual(fake.full_listings(), 1) + + def test_an_id_the_region_pages_omit_is_dropped_only_on_a_404(self) -> None: + # GitHub deleted 500 early; the region's page shows it gone. A 404 + # for 500 drops it (incremental); a 502, or no id reads left, keeps + # it, and the over-count takes the one whole listing. + for label, fail, cap, mode in ( + ("404", set(), ve.NEAR_EXPIRY_ID_READ_CAP, "incremental"), + ("502", {f"artifacts/{500}"}, ve.NEAR_EXPIRY_ID_READ_CAP, "full"), + ("cap", set(), 0, "full"), + ): + with self.subTest(label): + for path in (self.saved, self.failed): + path.unlink(missing_ok=True) + pathlib.Path(f"{self.saved}{ve.OVERCOUNT_FALLBACK_SUFFIX}").unlink( + missing_ok=True + ) + base = self.tail((500, 30), (400, 40)) + self.write_carry(base) + fake = self.fake(base=[a for a in base if a["id"] != 500]) + fake.fail = fail + with mock.patch.object(ve, "NEAR_EXPIRY_ID_READ_CAP", cap): + reader, evidence = self.judge(fake) + self.assertIsNone(evidence.refusal, evidence.detail) + self.assertEqual(reader.listing_mode, mode) + by_id = [c for c in fake.calls if c[-1].endswith("/artifacts/500")] + self.assertEqual(len(by_id), 0 if cap == 0 else 1) + + def climbing(self, missing: int) -> tuple[ve.Reader, FakeGitHub]: + """A carry whose phantoms above the near-expiry 500 outnumber the + surplus, since `missing` live artifacts below it are not carried: the + re-read starts below 500's page and walks up.""" + old = self.base_at - timedelta(hours=3) + near = LISTED_AT + timedelta(minutes=30) + phantoms = (850, 750, 650, 550, 450) if missing == 4 else ( + 875, 850, 825, 775, 750, 650, 550 + ) + held = [ + *self.base, + dated("pr-merge-gate-cursor", 3, 600, old), + dated("pr-merge-gate-cursor", 3, 500, old, expires=near), + *(dated("pr-merge-gate-cursor", 3, i, old) for i in (400, 300)), + ] + self.write_carry( + [*held, *(dated("pr-merge-gate-cursor", 3, i, old) for i in phantoms)] + ) + below = [250, 240, 230, 220, 210, 200][:missing] + fake = self.fake( + base=[*held, *(dated("pr-merge-gate-cursor", 3, i, old) for i in below)] + ) + reader, _ = self.judge(fake) + return reader, fake + + def test_the_region_walks_up_to_the_page_that_holds_its_top(self) -> None: + # Surplus 1, 500 at listing index 9: the start page is 5 (250, 240), + # page 4 (400, 300) is still below 500, page 3 (600, 500) holds it. + reader, fake = self.climbing(4) + self.assertEqual(self.page_numbers(fake), [1, 5, 4, 3]) + self.assertEqual(reader.listing_mode, "incremental") + self.assertEqual(fake.full_listings(), 0) + + def test_the_region_walk_stops_at_its_upward_bound(self) -> None: + # Surplus 1, 500 at listing index 12: the start page is 6, and the + # walk may climb surplus // 2 + 2 = 2 pages, to 4, short of page 3: + # the group stays unbracketed and the over-count takes a whole listing. + reader, fake = self.climbing(6) + self.assertEqual(self.page_numbers(fake)[:4], [1, 6, 5, 4]) + self.assertEqual(reader.listing_mode, "full") + self.assertEqual(fake.full_listings(), 1) + def test_the_pages_near_expiry_stop_at_their_cap(self) -> None: # Out of expiry order, the region takes two pages (3 and 4). Capped # at one, the count stays over: one whole listing, whose top-up @@ -4234,6 +4357,41 @@ def test_two_readers_that_over_count_take_one_whole_listing_a_job( # Not a failed listing: #11926's note stays unwritten. self.assertFalse(self.failed.exists()) + def test_an_over_count_refused_before_paging_leaves_the_job_its_listing( + self, + ) -> None: + # pr-feedback-dispatch's order: PR A's Reader over-counts under + # verify-cap's limit, and its whole listing does not fit, so it + # refuses before paging. PR B's Reader, with the time, takes it. + base = self.tail() + self.write_carry(base) + fake = self.fake(base=[a for a in base if a["id"] != 300]) + first = self.against_the_clock(fake, 0.0, limit="20") + self.assertEqual(first.refusal, "verify:unreadable") + self.assertTrue(first.detail.startswith(ve.DEADLINE_UNREADABLE)) + self.assertEqual(fake.full_listings(), 0) + second = self.against_the_clock(fake, 0.0) + self.assertIsNone(second.refusal, second.detail) + self.assertEqual(fake.full_listings(), 1) + + def test_an_over_count_listing_out_of_the_callers_time_refuses_so_later( + self, + ) -> None: + # A's whole listing pages, then its top-up runs out of the caller's + # time. B's Reader takes no second whole listing, and its refusal + # keeps the caller-limit prefix, so verify-cap skips B's round too. + base = self.tail() + self.write_carry(base) + fake = self.fake(base=[a for a in base if a["id"] != 300]) + first = self.against_the_clock(fake, 50.0, limit="60") + self.assertTrue(first.detail.startswith(ve.DEADLINE_UNREADABLE)) + self.assertEqual(fake.full_listings(), 1) + second = self.against_the_clock(fake, 0.0) + self.assertEqual(second.refusal, "verify:unreadable") + self.assertTrue(second.detail.startswith(ve.DEADLINE_UNREADABLE)) + self.assertIn("already took its whole listing", second.detail) + self.assertEqual(fake.full_listings(), 1) + def test_an_over_count_the_pages_near_expiry_leave_takes_one_whole_listing( self, ) -> None: @@ -4998,7 +5156,8 @@ class TheColdReadHeadroomCoversTheListingCaps(unittest.TestCase): out from the live listing: its `total_count` and the FAILs it lists. The workflow's figure must cover the reader's listing caps at any listing size, so raising `INCREMENTAL_READS`, `LISTING_READS`, `LISTING_WAITS` - `LISTING_TOP_UP_PAGES` or `NEAR_EXPIRY_PAGE_CAP` without it fails here. The other terms are the + `LISTING_TOP_UP_PAGES`, `NEAR_EXPIRY_PAGE_CAP` or + `NEAR_EXPIRY_ID_READ_CAP` without it fails here. The other terms are the workflow comment's: 3 reads a FAIL (its run, a compare and a download), at most 13 pages an incremental read, the count read before the whole listings, one page more than the live count fills, and the step's own @@ -5036,14 +5195,15 @@ def test_the_workflow_cost_is_the_capped_worst_case_at_any_size(self) -> None: + 1 + ve.LISTING_READS * (attempts * pages + ve.LISTING_TOP_UP_PAGES) + ve.NEAR_EXPIRY_PAGE_CAP + + ve.NEAR_EXPIRY_ID_READ_CAP ) self.assertEqual(self.cold_cost(total, fails), worst) def test_the_cost_of_two_listings_differs_by_hand(self) -> None: # 10,450 artifacts (106 pages) and 300 FAILs against 7,522 (77) and - # 264: 2,045 against 1,647, worked by hand in ci-lanes-pr-hold.test.sh. - self.assertEqual(self.cold_cost(10_450, 300), 2_045) - self.assertEqual(self.cold_cost(7_522, 264), 1_647) + # 264: 2,079 against 1,681, worked by hand in ci-lanes-pr-hold.test.sh. + self.assertEqual(self.cold_cost(10_450, 300), 2_079) + self.assertEqual(self.cold_cost(7_522, 264), 1_681) # --- The --authority pr-merge path ------------------------------------------------ diff --git a/scripts/ci-lanes-pr-hold.test.sh b/scripts/ci-lanes-pr-hold.test.sh index b6664192561..fdf78a442df 100755 --- a/scripts/ci-lanes-pr-hold.test.sh +++ b/scripts/ci-lanes-pr-hold.test.sh @@ -874,31 +874,31 @@ STUB_NO_LISTING=1 STUB_EVIDENCE="$(jq -nc --arg head "$SHA" '{refusal: null, hea # (STUB_TOTAL artifacts, STUB_LISTED_FAILS FAILs): 3 reads a FAIL, 3 incremental # reads of 13 pages, the count read, and 3 whole listings of 3 attempts of # ceil(total / 100) + 1 pages, each with a 7-page top-up, and 24 pages near -# expiry read again. By default 7,522 +# expiry read again, and 34 artifacts read by id. By default 7,522 # artifacts (77 pages) and 264 FAILs: 77 for the step's own listing, plus -# 792 + 39 + 1 + 3 x (3 x 77 + 7) + 24 = 1647, so 1647 x 1 + 0.1 x min(limit, 1000) = 1747 here. +# 792 + 39 + 1 + 3 x (3 x 77 + 7) + 24 + 34 = 1681, so 1681 x 1 + 0.1 x min(limit, 1000) = 1781 here. rate() { jq -nc --argjson remaining "$1" '{resources: {core: {limit: 5000, remaining: $remaining, reset: 1793030000}}}'; } -STUB_COLD=1 STUB_RATE="$(rate 1747)" expect "in the merge queue, a cold reader with the headroom for it reads" 0 3 "#7: no pr-verify FAIL hold: the reader allows this head." +STUB_COLD=1 STUB_RATE="$(rate 1781)" expect "in the merge queue, a cold reader with the headroom for it reads" 0 3 "#7: no pr-verify FAIL hold: the reader allows this head." if [[ "$(readers)" -eq 1 ]]; then ok "a cold reader with headroom is started"; else fail "a cold reader with headroom ran $(readers) times"; fi -STUB_COLD=1 STUB_RATE="$(rate 1746)" expect "in the merge queue, a cold reader without the headroom holds without reading" 1 3 "::error::#7 is held: no carried run cache from pr-merge's last run, and GITHUB_TOKEN has 1746 requests left, under the 1747 a cold read needs" +STUB_COLD=1 STUB_RATE="$(rate 1780)" expect "in the merge queue, a cold reader without the headroom holds without reading" 1 3 "::error::#7 is held: no carried run cache from pr-merge's last run, and GITHUB_TOKEN has 1780 requests left, under the 1781 a cold read needs" if [[ "$(readers)" -eq 0 ]]; then ok "a cold reader without headroom is never started"; else fail "a cold reader without headroom ran: '$(grep '^reader ' "$TMP_ROOT/log")'"; fi STUB_COLD=1 STUB_RATE='' expect "in the merge queue, a cold reader whose rate limit does not read holds" 1 3 "::error::#7 is held: no carried run cache from pr-merge's last run, and GITHUB_TOKEN's remaining requests could not be read" if [[ "$(readers)" -eq 0 ]]; then ok "a cold reader with an unread rate limit is never started"; else fail "a cold reader with an unread rate limit ran"; fi STUB_COLD=1 STUB_RATE='{"resources":{"core":{"limit":5000}}}' expect "in the merge queue, a rate limit with no remaining count holds a cold reader" 1 3 "GITHUB_TOKEN's remaining requests could not be read" # 10,450 artifacts (105 pages, as on 2026-10-30, so 106) and 300 FAILs: -# 106 + 900 + 39 + 1 + 3 x (3 x 106 + 7) + 24 = 2045, plus the floor's 100. -STUB_COLD=1 STUB_TOTAL=10450 STUB_LISTED_FAILS=300 STUB_RATE="$(rate 2145)" expect "in the merge queue, a cold read's cost grows with the live listing" 0 3 "cold reads for 1 queue entries need 2145" -STUB_COLD=1 STUB_TOTAL=10450 STUB_LISTED_FAILS=300 STUB_RATE="$(rate 2144)" expect "in the merge queue, a larger live listing holds a cold reader the old cost would start" 1 3 "GITHUB_TOKEN has 2144 requests left, under the 2145 a cold read needs (2045 for each of the 1 entries" -# 150 artifacts (2 pages, so 3) and 10 FAILs: 3 + 30 + 39 + 1 + 3 x (3 x 3 + 7) + 24 = 145. -STUB_COLD=1 STUB_TOTAL=150 STUB_LISTED_FAILS=10 STUB_RATE="$(rate 245)" expect "in the merge queue, a small live listing needs only its own cost" 0 3 "cold reads for 1 queue entries need 245" +# 106 + 900 + 39 + 1 + 3 x (3 x 106 + 7) + 24 + 34 = 2079, plus the floor's 100. +STUB_COLD=1 STUB_TOTAL=10450 STUB_LISTED_FAILS=300 STUB_RATE="$(rate 2179)" expect "in the merge queue, a cold read's cost grows with the live listing" 0 3 "cold reads for 1 queue entries need 2179" +STUB_COLD=1 STUB_TOTAL=10450 STUB_LISTED_FAILS=300 STUB_RATE="$(rate 2178)" expect "in the merge queue, a larger live listing holds a cold reader the old cost would start" 1 3 "GITHUB_TOKEN has 2178 requests left, under the 2179 a cold read needs (2079 for each of the 1 entries" +# 150 artifacts (2 pages, so 3) and 10 FAILs: 3 + 30 + 39 + 1 + 3 x (3 x 3 + 7) + 24 + 34 = 179. +STUB_COLD=1 STUB_TOTAL=150 STUB_LISTED_FAILS=10 STUB_RATE="$(rate 279)" expect "in the merge queue, a small live listing needs only its own cost" 0 3 "cold reads for 1 queue entries need 279" STUB_COLD=1 STUB_TOTAL='' STUB_RATE="$(rate 9999)" expect "in the merge queue, a live listing that does not read holds a cold reader" 1 3 "::error::#7 is held: no carried run cache from pr-merge's last run, and the artifact listing could not be read" if [[ "$(readers)" -eq 0 ]]; then ok "a cold reader with an unread listing is never started"; else fail "a cold reader with an unread listing ran"; fi # Every entry the queue builds at once goes cold together on a missing cache, -# so the need scales with max_entries_to_build: 1647 x 5 + 100 = 8335. -STUB_COLD=1 STUB_RULES=5 STUB_RATE="$(rate 8335)" expect "in the merge queue, a cold reader reads with headroom for every entry the queue builds" 0 3 "#7: no pr-verify FAIL hold: the reader allows this head." -STUB_COLD=1 STUB_RULES=5 STUB_RATE="$(rate 8334)" expect "in the merge queue, a cold reader holds under the headroom of every entry the queue builds" 1 3 "GITHUB_TOKEN has 8334 requests left, under the 8335 a cold read needs" -STUB_COLD=1 STUB_RULES=2 STUB_RATE="$(rate 3393)" expect "in the merge queue, the cold headroom scales with max_entries_to_build" 1 3 "under the 3394 a cold read needs" -STUB_COLD=1 STUB_RULES='' STUB_RATE="$(rate 8335)" expect "in the merge queue, a merge queue rule that does not read holds a cold reader" 1 3 "the merge queue's max_entries_to_build could not be read" +# so the need scales with max_entries_to_build: 1681 x 5 + 100 = 8505. +STUB_COLD=1 STUB_RULES=5 STUB_RATE="$(rate 8505)" expect "in the merge queue, a cold reader reads with headroom for every entry the queue builds" 0 3 "#7: no pr-verify FAIL hold: the reader allows this head." +STUB_COLD=1 STUB_RULES=5 STUB_RATE="$(rate 8504)" expect "in the merge queue, a cold reader holds under the headroom of every entry the queue builds" 1 3 "GITHUB_TOKEN has 8504 requests left, under the 8505 a cold read needs" +STUB_COLD=1 STUB_RULES=2 STUB_RATE="$(rate 3461)" expect "in the merge queue, the cold headroom scales with max_entries_to_build" 1 3 "under the 3462 a cold read needs" +STUB_COLD=1 STUB_RULES='' STUB_RATE="$(rate 8505)" expect "in the merge queue, a merge queue rule that does not read holds a cold reader" 1 3 "the merge queue's max_entries_to_build could not be read" if [[ "$(readers)" -eq 0 ]]; then ok "a cold reader with an unread queue rule is never started"; else fail "a cold reader with an unread queue rule ran"; fi STUB_CARRIED='{"carried-run-record":{},"carried-fail-head-ref":{},"carried-on-branch":{}}' expect "in the merge queue, a carried cache with every section empty is cold" 1 3 "::error::#7 is held: no carried run cache from pr-merge's last run, and GITHUB_TOKEN's remaining requests could not be read" STUB_CARRIED='not json' expect "in the merge queue, a carried cache that does not parse is cold" 1 3 "GITHUB_TOKEN's remaining requests could not be read" From bf8fd47951dba0f421d762a0e780b21b1b3cff06 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 1 Nov 2026 09:31:36 -0500 Subject: [PATCH 4/5] docs(source-control): wrap the verify-listing changelog fragment The fragment held one line over the 1,800 bytes the main-run review reads whole, so babysit rated the PR merge risk HIGH (long-line-review). Wrap it into three list items under 100 columns; the text is unchanged. Co-Authored-By: Claude Opus 5.5 --- ...-verify-listing-early-deletion-5b1e9c3a.md | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/.changes/source-control/fix-verify-listing-early-deletion-5b1e9c3a.md b/.changes/source-control/fix-verify-listing-early-deletion-5b1e9c3a.md index 56fa78ca72d..f51382eea98 100644 --- a/.changes/source-control/fix-verify-listing-early-deletion-5b1e9c3a.md +++ b/.changes/source-control/fix-verify-listing-early-deletion-5b1e9c3a.md @@ -4,4 +4,23 @@ bump: patch ### Fixed -- The verdict gate reader (`babysit_verify_evidence.py`) settles an over-count from early deletion before it falls back to a whole listing. GitHub can delete artifacts before their `expires_at`. When the incremental listing or a whole listing's top-up holds more than GitHub's live count, the reader first re-reads only the pages around the carried entries that expire within `EARLY_DELETION_LEAD`. Those pages add every id they hold. An id inside their span that they omit is dropped only when its own read answers 404, because pages read one at a time can skip a live entry at a boundary when an artifact above it is deleted. The re-read takes at most `NEAR_EXPIRY_PAGE_CAP` (24) pages and `NEAR_EXPIRY_ID_READ_CAP` (34) id reads per listing. A count it leaves unsettled, or an id read with any answer other than 200 or 404, goes to the single whole listing added in #11926, which refuses `verify:unreadable` when its top-up misses the count. An over-count now takes that whole listing at most once a job, even when its top-up comes up short. Once that listing starts paging, it is noted beside the saved listing (`.overcount-fallback`) with its outcome. A later reader in the job whose pages over-count then refuses without another whole listing, keeping the caller-limit prefix when that listing ran out of a caller's time. A refusal before paging leaves the listing to the next reader. On the carried listing behind the `verify:unreadable` refusal of #11903, the re-read finished incrementally in 21 page reads and 15.9 s, measured before the per-id reads were added. With #11926 alone, that over-count goes to a whole listing of about 133 pages, past verify-cap's 120 s limit, so it refuses (derived from the measured page count, not re-run). Each `babysit-listing-miss` line also reports `region_pages`. +- The verdict gate reader (`babysit_verify_evidence.py`) settles an over-count from early deletion + before it falls back to a whole listing. GitHub can delete artifacts before their `expires_at`. + When the incremental listing or a whole listing's top-up holds more than GitHub's live count, the + reader first re-reads only the pages around the carried entries that expire within + `EARLY_DELETION_LEAD`. Those pages add every id they hold. An id inside their span that they omit + is dropped only when its own read answers 404, because pages read one at a time can skip a live + entry at a boundary when an artifact above it is deleted. The re-read takes at most + `NEAR_EXPIRY_PAGE_CAP` (24) pages and `NEAR_EXPIRY_ID_READ_CAP` (34) id reads per listing. A count + it leaves unsettled, or an id read with any answer other than 200 or 404, goes to the single whole + listing added in #11926, which refuses `verify:unreadable` when its top-up misses the count. +- An over-count now takes that whole listing at most once a job, even when its top-up comes up + short. Once that listing starts paging, it is noted beside the saved listing + (`.overcount-fallback`) with its outcome. A later reader in the job whose pages over-count then + refuses without another whole listing, keeping the caller-limit prefix when that listing ran out + of a caller's time. A refusal before paging leaves the listing to the next reader. +- On the carried listing behind the `verify:unreadable` refusal of #11903, the re-read finished + incrementally in 21 page reads and 15.9 s, measured before the per-id reads were added. With + #11926 alone, that over-count goes to a whole listing of about 133 pages, past verify-cap's 120 s + limit, so it refuses (derived from the measured page count, not re-run). Each + `babysit-listing-miss` line also reports `region_pages`. From 47538c70d4dec7df2497424a926148648ccccd4f Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 1 Nov 2026 09:49:42 -0500 Subject: [PATCH 5/5] fix(source-control): retry a near-expiry id read on a 5xx and fail it as transient A 502 on the by-id read of a near-expiry artifact during a whole listing's top-up left the listing unchanged, so the top-up gave COUNT_OVER and the job-wide .failed note was written. The id read now shares the page read's retry over LISTING_WAITS (_api_object); a read still transient after them makes _near_expiry return None, so _whole_listings raises _Transient and nothing is noted for the job. Align the 404-only text with the code, which also drops an expired record, and note that the near-expiry caps reset with each listing. Co-Authored-By: Claude Opus 5.5 --- ...-verify-listing-early-deletion-5b1e9c3a.md | 12 ++- .../scripts/babysit_verify_evidence.py | 94 ++++++++++--------- .../tests/test_babysit_verify_evidence.py | 68 ++++++++++++-- 3 files changed, 117 insertions(+), 57 deletions(-) diff --git a/.changes/source-control/fix-verify-listing-early-deletion-5b1e9c3a.md b/.changes/source-control/fix-verify-listing-early-deletion-5b1e9c3a.md index f51382eea98..79e218e1db2 100644 --- a/.changes/source-control/fix-verify-listing-early-deletion-5b1e9c3a.md +++ b/.changes/source-control/fix-verify-listing-early-deletion-5b1e9c3a.md @@ -9,11 +9,13 @@ bump: patch When the incremental listing or a whole listing's top-up holds more than GitHub's live count, the reader first re-reads only the pages around the carried entries that expire within `EARLY_DELETION_LEAD`. Those pages add every id they hold. An id inside their span that they omit - is dropped only when its own read answers 404, because pages read one at a time can skip a live - entry at a boundary when an artifact above it is deleted. The re-read takes at most - `NEAR_EXPIRY_PAGE_CAP` (24) pages and `NEAR_EXPIRY_ID_READ_CAP` (34) id reads per listing. A count - it leaves unsettled, or an id read with any answer other than 200 or 404, goes to the single whole - listing added in #11926, which refuses `verify:unreadable` when its top-up misses the count. + is dropped only when its own read answers 404 or shows it expired, because pages read one at a + time can skip a live entry at a boundary when an artifact above it is deleted. The re-read takes + at most `NEAR_EXPIRY_PAGE_CAP` (24) pages and `NEAR_EXPIRY_ID_READ_CAP` (34) id reads per listing. + A count it leaves unsettled, or an id read with any other answer, goes to the single whole listing + added in #11926, which refuses `verify:unreadable` when its top-up misses the count. An id read + that answers a 5xx is read again after each of `LISTING_WAITS`, as a page is; one still failing + then fails the read as transient, so a top-up refuses without noting a failed listing for the job. - An over-count now takes that whole listing at most once a job, even when its top-up comes up short. Once that listing starts paging, it is noted beside the saved listing (`.overcount-fallback`) with its outcome. A later reader in the job whose pages over-count then 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 36583d439f2..be91281c10e 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 @@ -154,10 +154,11 @@ first page's `Date`. GitHub can delete an artifact up to an hour before its `expires_at`, in no expiry order (`EARLY_DELETION_LEAD`), so when that holds more than the live count the pages around the kept entries near expiry are -read again and decide every id they span (`Reader._near_expiry`), at a few -pages a group whatever the repository's total and at most -`NEAR_EXPIRY_PAGE_CAP` a listing; a count those pages do not settle goes to -the one whole listing. Such a listing could hide a FAIL by leaving it out, so +read again and decide every id they span, an id they omit by its own read +(`Reader._near_expiry`), at a few pages a group whatever the repository's +total and at most `NEAR_EXPIRY_PAGE_CAP` a listing; a count those pages do not +settle goes to the one whole listing, and a page or id read still failing on a +5xx after `LISTING_WAITS` fails the listing as transient. Such a listing could hide a FAIL by leaving it out, so it is kept only when it holds exactly the first page's `total_count`, GitHub's live count of the repository's artifacts. Across 17 first-page reads on 2026-10-27 that count moved by exactly the uploads since the last read less @@ -405,9 +406,11 @@ def verdict_name(name: str) -> re.Match[str] | None: NEAR_EXPIRY_PAGE_CAP = 24 # The most artifacts `Reader._near_expiry` reads by id for one listing, its # incremental read and top-up together: each id its pages did not show is -# dropped only on a 404. It drops at most the listing's surplus over the live -# count, which was 31 to 34 in the three over-counts behind #11903's refusal; -# the largest (judgment on that data). Past it the over-count stands. +# dropped only on a 404 or an expired record. It drops at most the listing's +# surplus over the live count, which was 31 to 34 in the three over-counts +# behind #11903's refusal; the largest (judgment on that data). Past it the +# over-count stands. Both caps reset with each listing the Reader reads, as +# after `refresh()`; pr-require-checks.yml's cold_cost counts them once. NEAR_EXPIRY_ID_READ_CAP = 34 # The waits between incremental reads on one base. A count that misses the # live total can stay off for longer than an immediate re-read takes, while a @@ -1664,7 +1667,8 @@ def _near_expiry( ) -> tuple[list[dict[str, Any]], int] | None: """`listing` with the ids around its entries from `start` on that expire within `EARLY_DELETION_LEAD` of `server_now` decided by the - pages that hold them now, and the pages read; None for a failed read. + pages that hold them now, and the pages read; None for a failed page + read or an id read that stays transient past its waits. Such entries are grouped where they lie within a page of each other. Each group is read from the page its top entry would be on with @@ -1673,12 +1677,13 @@ def _near_expiry( reaches its top id, then down until a page reaches its bottom id or ends the listing; those pages add every id they hold. An id of `listing` from the lowest to the highest they hold that they do not - show is dropped only when its own read answers 404 - (`_artifact_gone`): pages read one at a time skip the entry at a - boundary when an artifact above it is deleted between reads (#10486). + show is dropped only when its own read answers 404 or shows it + expired (`_artifact_gone`): pages read one at a time skip the entry at + a boundary when an artifact above it is deleted between reads (#10486). A group it cannot bracket within those pages, a read past `NEAR_EXPIRY_PAGE_CAP` or `NEAR_EXPIRY_ID_READ_CAP` for this listing, - or an id read with any other answer leaves `listing` as it was.""" + or an id read with any other answer that is not transient leaves + `listing` as it was.""" size = LISTING_PAGE_SIZE limit = server_now + EARLY_DELETION_LEAD near = [ @@ -1743,7 +1748,8 @@ def _near_expiry( self._artifact_reads += 1 answer = self._artifact_gone(a["id"], deadline) if answer is None: - return listing, reads + # A transient answer fails the read, as a lost page does. + return None if self._page_transient else (listing, reads) if answer: gone.add(a["id"]) merged = {a["id"]: a for a in listing if a["id"] not in gone} | decided @@ -1751,24 +1757,15 @@ def _near_expiry( def _artifact_gone(self, artifact_id: int, deadline: float) -> bool | None: """Whether GitHub no longer holds the artifact: True on a 404 or an - expired record, False on a live one, None for any other answer.""" - left = deadline - _listing_clock() - if left <= 0: - return None - try: - proc, _ = gh_api_with_headers( - ["api", f"repos/{self.repo}/actions/artifacts/{artifact_id}"], - timeout_seconds=left, - env_overrides=self._env(), - ) - except (RuntimeError, ValueError, OSError): - return None - if proc.returncode != 0: - return True if gh_http_status(proc.stderr or "") == 404 else None - try: - record = json_object(json.loads(proc.stdout)) - except ValueError: - return None + expired record, False on a live one, None for any other answer, with + `_page_transient` set when it was transient past its waits + (`_api_object`).""" + read = self._api_object( + f"repos/{self.repo}/actions/artifacts/{artifact_id}", deadline + ) + if not isinstance(read, tuple): + return True if read == 404 else None + record = read[0] if record.get("id") != artifact_id or not isinstance( record.get("expired"), bool ): @@ -1778,11 +1775,24 @@ def _artifact_gone(self, artifact_id: int, deadline: float) -> bool | None: def _incremental_page( self, page: int, deadline: float ) -> tuple[dict[str, Any], dict[str, str]] | None: - """One incremental page's body and headers, read again after + """One incremental page's body and headers (`_api_object`); None for + any failure.""" + read = self._api_object( + f"repos/{self.repo}/actions/artifacts" + f"?per_page={LISTING_PAGE_SIZE}&page={page}", + deadline, + ) + return read if isinstance(read, tuple) else None + + def _api_object( + self, path: str, deadline: float + ) -> tuple[dict[str, Any], dict[str, str]] | int | None: + """The JSON object at `path` and its headers, read again after `LISTING_WAITS` while time is left when the read fails transiently - (`_transient`) or its body is cut short; None for any other failure, - or once the waits or the time run out, with `_page_transient` set - when the last read was one of those.""" + (`_transient`) or its body is cut short; the HTTP status of any other + failed answer (None without one), or None once the waits or the time + run out, with `_page_transient` set when the last read was one of + those.""" self._page_transient = False for wait in (*LISTING_WAITS, None): left = deadline - _listing_clock() @@ -1790,11 +1800,7 @@ def _incremental_page( return None try: proc, headers = gh_api_with_headers( - [ - "api", - f"repos/{self.repo}/actions/artifacts" - f"?per_page={LISTING_PAGE_SIZE}&page={page}", - ], + ["api", path], timeout_seconds=left, env_overrides=self._env(), ) @@ -1809,9 +1815,11 @@ def _incremental_page( else: self._page_transient = False return body, headers - elif not _transient((proc.stderr or "").strip()): - self._page_transient = False - return None + else: + detail = (proc.stderr or "").strip() + if not _transient(detail): + self._page_transient = False + return gh_http_status(detail) self._page_transient = True if wait is None or deadline - _listing_clock() - wait <= 0: return None diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py index 0f8f3c367f4..f75639534f5 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_verify_evidence.py @@ -4253,12 +4253,15 @@ def page(args: list[str], **kwargs: Any) -> Any: def test_an_id_the_region_pages_omit_is_dropped_only_on_a_404(self) -> None: # GitHub deleted 500 early; the region's page shows it gone. A 404 - # for 500 drops it (incremental); a 502, or no id reads left, keeps - # it, and the over-count takes the one whole listing. - for label, fail, cap, mode in ( - ("404", set(), ve.NEAR_EXPIRY_ID_READ_CAP, "incremental"), - ("502", {f"artifacts/{500}"}, ve.NEAR_EXPIRY_ID_READ_CAP, "full"), - ("cap", set(), 0, "full"), + # for 500 drops it (incremental), after a 502 too; a 502 through the + # read's waits fails the incremental read, and no id reads left keeps + # 500: either way the over-count takes the one whole listing. + attempts = 1 + len(ve.LISTING_WAITS) + for label, fail, cap, mode, reads in ( + ("404", set(), ve.NEAR_EXPIRY_ID_READ_CAP, "incremental", 1), + ("502-404", set(), ve.NEAR_EXPIRY_ID_READ_CAP, "incremental", 2), + ("502", {"artifacts/500"}, ve.NEAR_EXPIRY_ID_READ_CAP, "full", attempts), + ("cap", set(), 0, "full", 0), ): with self.subTest(label): for path in (self.saved, self.failed): @@ -4270,12 +4273,29 @@ def test_an_id_the_region_pages_omit_is_dropped_only_on_a_404(self) -> None: self.write_carry(base) fake = self.fake(base=[a for a in base if a["id"] != 500]) fake.fail = fail - with mock.patch.object(ve, "NEAR_EXPIRY_ID_READ_CAP", cap): - reader, evidence = self.judge(fake) + blipped = [label != "502-404"] + + def page(args: list[str], **kwargs: Any) -> Any: + if not blipped[0] and args[-1].endswith("/artifacts/500"): + blipped[0] = True + fake.calls.append(args) + failed = subprocess.CompletedProcess( + args, 1, "", "gh: Bad Gateway (HTTP 502)" + ) + return failed, {} + return fake(args, **kwargs), {"date": DATE} + + with ( + mock.patch.object(ve, "NEAR_EXPIRY_ID_READ_CAP", cap), + mock.patch.object(postmerge, "gh_capture", side_effect=fake), + mock.patch.object(ve, "gh_api_with_headers", side_effect=page), + ): + reader = ve.Reader(REPO, "main", now=NOW) + evidence = reader.evidence(PR, HEAD, pull()) self.assertIsNone(evidence.refusal, evidence.detail) self.assertEqual(reader.listing_mode, mode) by_id = [c for c in fake.calls if c[-1].endswith("/artifacts/500")] - self.assertEqual(len(by_id), 0 if cap == 0 else 1) + self.assertEqual(len(by_id), reads) def climbing(self, missing: int) -> tuple[ve.Reader, FakeGitHub]: """A carry whose phantoms above the near-expiry 500 outnumber the @@ -4841,6 +4861,36 @@ def test_a_top_up_lost_to_a_5xx_is_not_noted_for_the_job(self) -> None: self.assertIsNone(evidence.refusal, evidence.detail) self.assertEqual(fake.full_listings(), 2) + def test_a_5xx_on_a_top_up_id_read_is_not_noted_for_the_job(self) -> None: + # No carry, so a whole listing, which lists 650 (expiring in 30 + # minutes); GitHub then drops it early. The top-up's page 1 stops at + # 1000, keeping 650, one over the live count; the page near expiry + # omits it, and its read by id answers 502 through its waits: this + # Reader refuses, transient, and the next one, the blip over, lists. + old, soon = self.base_at - timedelta(days=2), LISTED_AT + timedelta(minutes=30) + dropping = dated("pr-merge-gate-cursor", 3, 650, old, expires=soon) + fake = self.fake(dropping) + listed = fake.listed + + def drops_during_the_listing() -> list[dict[str, Any]]: + items = listed() + fake.artifacts = [a for a in fake.artifacts if a is not dropping] + return items + + fake.listed = drops_during_the_listing # type: ignore[method-assign] + fake.fail = {"artifacts/650"} + reader, evidence = self.judge(fake) + self.assertEqual(evidence.refusal, "verify:unreadable") + self.assertIn("could not be checked against", evidence.detail or "") + self.assertEqual(reader.listing_mode, "full") + by_id = [c for c in fake.calls if c[-1].endswith("/artifacts/650")] + self.assertEqual(len(by_id), 1 + len(ve.LISTING_WAITS)) + self.assertFalse(self.failed.exists()) + fake.fail = set() + _, again = self.judge(fake) + self.assertIsNone(again.refusal, again.detail) + self.assertEqual(fake.full_listings(), 2) + def test_a_top_up_read_after_a_5xx_with_no_date_is_noted(self) -> None: # The top-up's page 1 answers 502 once, then reads with no `Date`: # the failure is the missing header, not the 502, so it is noted.