From 80f20f468397624846c1a2ac4c7a3690a4861935 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 1 Nov 2026 11:15:52 -0500 Subject: [PATCH 1/6] fix(source-control): check near-expiry entries on an exact-count verdict listing The pr-verify verdict reader accepted a listing whose entry count equals the first page's total_count, and re-read entries near expiry only on an over-count. An entry GitHub deleted early could stand in for a FAIL a whole listing skipped at a page boundary, or that a stale carry lacked, so the count matched and the FAIL was left out. _incremental now runs _near_expiry whenever a kept entry expires within EARLY_DELETION_LEAD, on an exact count too, under the existing per-listing caps. _near_expiry reports whether it settled every such entry; an exact count it cannot settle (a cap, an unbracketed group, an odd id answer) fails the listing instead of being accepted, and a 5xx that outlasts its waits stays transient. Co-Authored-By: Claude Opus 5.5 --- ...erify-listing-exact-count-fail-e7080d66.md | 7 + .../scripts/babysit_verify_evidence.py | 84 ++++++---- .../tests/test_babysit_verify_evidence.py | 152 +++++++++++++++++- 3 files changed, 206 insertions(+), 37 deletions(-) create mode 100644 .changes/source-control/fix-verify-listing-exact-count-fail-e7080d66.md diff --git a/.changes/source-control/fix-verify-listing-exact-count-fail-e7080d66.md b/.changes/source-control/fix-verify-listing-exact-count-fail-e7080d66.md new file mode 100644 index 00000000000..be4054572a2 --- /dev/null +++ b/.changes/source-control/fix-verify-listing-exact-count-fail-e7080d66.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Fixed + +- **babysit-prs**: the pr-verify verdict reader re-reads the pages around kept artifact-listing entries near expiry even when the listing holds exactly GitHub's live count. An entry GitHub deleted early could stand in for a FAIL the listing skipped, so the count matched and the FAIL was left out. A count those reads cannot settle now refuses instead of allowing; a 5xx that outlasts its waits stays transient. 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 be91281c10e..a0851bcec1d 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 @@ -152,15 +152,18 @@ `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`. 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 +`expires_at`, in no expiry order (`EARLY_DELETION_LEAD`), so whenever a kept +entry expires within it the pages around the kept entries near expiry are 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 +total and at most `NEAR_EXPIRY_PAGE_CAP` a listing. They are read on an exact +count too: an entry deleted early can stand in for a FAIL the pages skipped. +An over-count those pages do not settle goes to the one whole listing, an +exact count they do not settle fails the listing, and a page or id read still +failing on a 5xx after `LISTING_WAITS` fails it 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 the artifacts whose `expires_at` had passed by the response's `Date`. Three incremental listings that day each held exactly that count and every artifact @@ -398,19 +401,21 @@ def verdict_name(name: str) -> re.Match[str] | None: # (judgment on that data). EARLY_DELETION_LEAD = timedelta(hours=2) # The most pages `Reader._near_expiry` reads for one listing, its incremental -# read and top-up together; past it the over-count stands and goes to the one -# whole listing. On the 2026-11-01 carry behind #11903's refusal it read 12 -# pages; twice that (judgment on that data), at `LISTING_PAGE_SECONDS` a page, -# stays well inside verify-cap.mjs's 120 s limit and a fifth of that day's -# 133-page whole listing. +# read and top-up together; past it an over-count stands and goes to the one +# whole listing, and an exact count fails the listing. On the 2026-11-01 +# carry behind #11903's refusal it read 12 pages; twice that (judgment on +# that data), at `LISTING_PAGE_SECONDS` a page, 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 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. +# behind #11903's refusal; the largest (judgment on that data). Past it an +# over-count stands and an exact count fails the listing. 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 @@ -1554,8 +1559,10 @@ def _incremental( 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). 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 + the live count, or any result while a kept entry is near expiry, has + the ids around its kept entries near expiry decided by the pages that + hold them now (`_near_expiry`); an exact count they leave unsettled + gives None, since an entry deleted early may stand in for a FAIL. The result must hold exactly the first page's `total_count` artifacts, GitHub's live count: fewer gives `COUNT_MISSED`, which `_current_listing` reads again on the same base after a wait, and more, @@ -1625,16 +1632,25 @@ def _incremental( listing = sorted([*fresh.values(), *kept], key=lambda a: -a["id"]) regions = 0 over = len(listing) > total - if over: + limit = server_now + EARLY_DELETION_LEAD + near = any( + a["expires_at"] is not None and _when(a["expires_at"]) <= limit + for a in kept + ) + if over or near: # GitHub can delete an artifact before its expires_at: read again - # the pages around the kept entries near expiry. - self._over_counted = True + # the pages around the kept entries near expiry. An exact count + # can hide one there standing in for a FAIL the pages skipped. + self._over_counted |= over reread = self._near_expiry( - listing, len(fresh), len(listing) - total, server_now, deadline + listing, len(fresh), max(0, len(listing) - total), server_now, deadline ) if reread is None: return None - listing, regions = reread + listing, regions, settled = reread + if not over and not settled: + # Unsettled, an exact count may still lack a FAIL. + return None if len(listing) == total: return listing # Fewer than the live count: the pages skipped an artifact. More: the @@ -1654,8 +1670,9 @@ def _incremental( file=sys.stderr, flush=True, ) - # Once the pages near expiry were read, no wait can settle the count. - return COUNT_OVER if over else COUNT_MISSED + # Once the pages near expiry were read on an over-count, no wait can + # settle it. + return COUNT_OVER if over or len(listing) > total else COUNT_MISSED def _near_expiry( self, @@ -1664,11 +1681,12 @@ def _near_expiry( surplus: int, server_now: datetime, deadline: float, - ) -> tuple[list[dict[str, Any]], int] | None: + ) -> tuple[list[dict[str, Any]], int, bool] | 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 page - read or an id read that stays transient past its waits. + pages that hold them now, the pages read, and whether every such + entry was decided; 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 @@ -1683,7 +1701,7 @@ def _near_expiry( 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 that is not transient leaves - `listing` as it was.""" + `listing` as it was, unsettled.""" size = LISTING_PAGE_SIZE limit = server_now + EARLY_DELETION_LEAD near = [ @@ -1710,7 +1728,7 @@ def _near_expiry( ended = False while True: if self._near_expiry_reads >= NEAR_EXPIRY_PAGE_CAP: - return listing, reads + return listing, reads, False read = self._incremental_page(page, deadline) reads += 1 self._near_expiry_reads += 1 @@ -1734,7 +1752,7 @@ def _near_expiry( downs -= 1 page += 1 if not seen or (not ended and min(seen) > bottom): - return listing, reads + return listing, reads, False spans.append((0 if ended else min(seen), max(seen))) decided.update(seen) gone: set[int] = set() @@ -1744,16 +1762,16 @@ def _near_expiry( ): continue if self._artifact_reads >= NEAR_EXPIRY_ID_READ_CAP: - return listing, reads + return listing, reads, False self._artifact_reads += 1 answer = self._artifact_gone(a["id"], deadline) if answer is None: # A transient answer fails the read, as a lost page does. - return None if self._page_transient else (listing, reads) + return None if self._page_transient else (listing, reads, False) 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 + return sorted(merged.values(), key=lambda a: -a["id"]), reads, True def _artifact_gone(self, artifact_id: int, deadline: float) -> bool | None: """Whether GitHub no longer holds the artifact: True on a 404 or an 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 f75639534f5..ba800ab5d1f 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 @@ -4169,6 +4169,8 @@ 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`. + # The top-up's count is exact, but 400 is near expiry, so it reads + # page 3 too and refuses as transient: nothing noted for the job. base = self.tail((500, 30), (400, 40)) self.write_carry(base) fake = self.fake(base=[a for a in base if a["id"] != 500]) @@ -4184,11 +4186,14 @@ def page(args: list[str], **kwargs: Any) -> Any: mock.patch.object(ve, "gh_api_with_headers", side_effect=page), ): reader = ve.Reader(REPO, "main", now=NOW) - reader.evidence(PR, HEAD, pull()) + evidence = reader.evidence(PR, HEAD, pull()) self.assertEqual(reader.listing_mode, "full") self.assertEqual(fake.full_listings(), 1) waits = [c.args[0] for c in self.sleep.call_args_list] - self.assertEqual(waits, list(ve.LISTING_WAITS)) + self.assertEqual(waits, list(ve.LISTING_WAITS) * 2) + self.assertEqual(evidence.refusal, "verify:unreadable") + self.assertIn("could not be checked against", evidence.detail or "") + self.assertFalse(self.failed.exists()) 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 @@ -4342,13 +4347,16 @@ def test_the_region_walk_stops_at_its_upward_bound(self) -> None: 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 - # (page 1) holds the count, and no INCREMENTAL_WAITS. + # (page 1) holds the count, and no INCREMENTAL_WAITS. That top-up + # keeps 500, near expiry, with the cap spent: an exact count it + # cannot settle may lack a FAIL, so the gate refuses. base = self.tail((500, 30), (400, 20)) self.write_carry(base) fake = self.fake(base=[a for a in base if a["id"] != 400]) with mock.patch.object(ve, "NEAR_EXPIRY_PAGE_CAP", 1): reader, evidence = self.judge(fake) - self.assertIsNone(evidence.refusal, evidence.detail) + self.assertEqual(evidence.refusal, "verify:unreadable") + self.assertIn("could not be checked against", evidence.detail or "") self.assertEqual(reader.listing_mode, "full") self.assertEqual(fake.full_listings(), 1) # Page 1, one capped region page, then the top-up's page 1. @@ -5200,6 +5208,142 @@ def test_the_refresh_before_an_allow_holds_the_live_count_too(self) -> None: ) +class AnExactCountChecksItsEntriesNearExpiry(unittest.TestCase): + """A listing that holds exactly the live count can still lack a FAIL: a + kept entry GitHub deleted early (`EARLY_DELETION_LEAD`) stands in for it. + So kept entries near expiry are decided by the pages that hold them now + and by their own reads even when the count matches, a count those reads + do not settle is refused, and a listing with none near expiry reads + nothing more.""" + + setUp = TheListingBuildsOnACarriedOne.setUp + write_carry = TheListingBuildsOnACarriedOne.write_carry + fake = TheListingBuildsOnACarriedOne.fake + judge = TheListingBuildsOnACarriedOne.judge + + def entry(self, artifact_id: int, expires: timedelta | None = None) -> Any: + """An old gate cursor, expiring `expires` after the runner's clock.""" + old = self.base_at - timedelta(days=2) + return dated( + "pr-merge-gate-cursor", + 3, + artifact_id, + old, + expires=None if expires is None else LISTED_AT + expires, + ) + + def fail(self, artifact_id: int) -> Any: + """An earlier head's FAIL of this pull request, two days old.""" + return dated( + named(EARLIER, "fail"), 2, artifact_id, self.base_at - timedelta(days=2) + ) + + @staticmethod + def by_id(fake: FakeGitHub) -> list[list[str]]: + """The reads of one artifact by id (`_artifact_gone`).""" + by_id = re.compile(r"/actions/artifacts/[0-9]+$") + return [c for c in fake.calls if by_id.search(c[-1])] + + def skipped(self) -> FakeGitHub: + """No carry: the whole listing skips FAIL 650 at a page boundary and + holds 600, expiring in 30 minutes, which GitHub then deletes early: + the union holds exactly the live count, with 600 in the FAIL's place.""" + dropping = self.entry(600, timedelta(minutes=30)) + fake = self.fake(self.fail(650), dropping) + fake.full_skips = [{650}] + 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] + return fake + + def test_a_whole_listing_that_skips_a_fail_beside_an_early_deletion( + self, + ) -> None: + # The top-up's page 1 (1000, 900) keeps 800, 700 and 600: five, the + # live count. 600 is near expiry, so page 3 is read (650 alone), + # bringing the FAIL back, and 600, which it omits, answers 404. + fake = self.skipped() + reader, evidence = self.judge(fake) + self.assertEqual(evidence.refusal, "verify:earlier-fail", evidence.detail) + self.assertEqual(reader.listing_mode, "full") + self.assertEqual(fake.full_listings(), 1) + self.assertEqual(len(self.by_id(fake)), 1) + ids = [a["id"] for a in json.loads(self.saved.read_text())["artifacts"]] + self.assertEqual(ids, [1000, 900, 800, 700, 650]) + + def test_a_stale_carry_whose_early_deletion_hides_a_missed_fail(self) -> None: + # The carry lacks FAIL 750 and still holds 600, which GitHub deleted + # 30 minutes before its expiry: five against a live count of five. + # Page 3 (700) does not reach 750, but 600's own read answers 404, + # so the count is one short and the whole listing holds the FAIL. + self.write_carry([*self.base, self.entry(600, timedelta(minutes=30))]) + fake = self.fake(self.fail(750)) + reader, evidence = self.judge(fake) + self.assertEqual(evidence.refusal, "verify:earlier-fail", evidence.detail) + self.assertEqual(reader.listing_mode, "full") + self.assertEqual(fake.full_listings(), 1) + self.assertTrue(self.by_id(fake)) + ids = [a["id"] for a in json.loads(self.saved.read_text())["artifacts"]] + self.assertEqual(ids, [1000, 900, 800, 750, 700]) + + def test_an_exact_count_with_none_near_expiry_reads_nothing_more(self) -> None: + # 600 expires three hours on, past EARLY_DELETION_LEAD: page 1 alone. + self.write_carry([*self.base, self.entry(600, timedelta(hours=3))]) + fake = self.fake(self.entry(600, timedelta(hours=3))) + reader, evidence = self.judge(fake) + self.assertIsNone(evidence.refusal, evidence.detail) + self.assertEqual(reader.listing_mode, "incremental") + self.assertEqual((fake.pages(), fake.full_listings()), (1, 0)) + self.assertEqual(self.by_id(fake), []) + + def test_an_exact_count_the_pages_near_expiry_cannot_settle_refuses( + self, + ) -> None: + # 600, live and expiring in 30 minutes, is on page 3: read, it settles + # the count. With no page left under the cap, neither the incremental + # read nor the whole listing's top-up can, and the gate refuses. + for cap, refusal, mode, full in ( + (ve.NEAR_EXPIRY_PAGE_CAP, None, "incremental", 0), + (0, "verify:unreadable", "full", 1), + ): + with self.subTest(cap=cap): + for path in (self.saved, self.failed): + path.unlink(missing_ok=True) + near = self.entry(600, timedelta(minutes=30)) + self.write_carry([*self.base, near]) + fake = self.fake(near) + with mock.patch.object(ve, "NEAR_EXPIRY_PAGE_CAP", cap): + reader, evidence = self.judge(fake) + self.assertEqual(evidence.refusal, refusal, evidence.detail) + self.assertEqual(reader.listing_mode, mode) + self.assertEqual(fake.full_listings(), full) + self.assertEqual(self.by_id(fake), []) + if refusal: + self.assertIn("could not be checked against", evidence.detail) + self.assertTrue(self.failed.exists()) + + def test_a_5xx_on_an_exact_count_id_read_refuses_as_transient(self) -> None: + # The skipped listing, with 600's read by id answering 502 through its + # waits: the Reader refuses, nothing is noted for the job, and the + # next Reader, the blip over, judges the FAIL. + fake = self.skipped() + fake.fail = {"artifacts/600"} + 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") + self.assertEqual(len(self.by_id(fake)), 1 + len(ve.LISTING_WAITS)) + self.assertFalse(self.failed.exists()) + fake.fail = set() + _, again = self.judge(fake) + self.assertEqual(again.refusal, "verify:earlier-fail", again.detail) + + class TheColdReadHeadroomCoversTheListingCaps(unittest.TestCase): """ci-status holds a cold reader unless GITHUB_TOKEN's pool has one cold read's worst case per queue entry (pr-require-checks.yml), which it works From 3170ed133a94fefdbbf16ba8181862de04eb0f43 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 1 Nov 2026 12:13:36 -0500 Subject: [PATCH 2/6] fix(source-control): leave short counts alone and skip waits after a near-expiry drop Read the pages near expiry only on an exact count or an over-count: a short count is never kept, so on main it waited and read again with no extra reads, while 80f20f46 re-read those pages on every attempt and, once the shared cap ran out, refused a count main would retry. An exact count those pages leave short now goes straight to a whole listing (COUNT_OVER) from the incremental stage: the base still holds what they dropped, so each wait re-read the same pages and spent the cap the whole listing's top-up needs. A top-up's short count still goes to another whole listing. Rename the test helper that shadowed TestCase.fail, which made assertIn, assertIsNone and list assertEqual pass on failure in that class, and align the docstrings with the returns. Co-Authored-By: Claude Opus 5.5 --- .../scripts/babysit_verify_evidence.py | 52 +++++++++++-------- .../tests/test_babysit_verify_evidence.py | 42 +++++++++++++-- 2 files changed, 68 insertions(+), 26 deletions(-) diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py index a0851bcec1d..58b668608be 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 @@ -152,15 +152,17 @@ `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`. GitHub can delete an artifact up to an hour before its -`expires_at`, in no expiry order (`EARLY_DELETION_LEAD`), so whenever a kept -entry expires within it the pages around the kept entries near expiry are -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. They are read on an exact -count too: an entry deleted early can stand in for a FAIL the pages skipped. -An over-count those pages do not settle goes to the one whole listing, an -exact count they do not settle fails the listing, and a page or id read still -failing on a 5xx after `LISTING_WAITS` fails it as transient. Such a listing +`expires_at`, in no expiry order (`EARLY_DELETION_LEAD`), so when a listing +holds the live count or more and a kept entry expires within it, the pages +around the kept entries near expiry are 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. They are read on an exact count too: an entry deleted early can stand +in for a FAIL the pages skipped. An over-count those pages do not settle goes +to the one whole listing, an exact count they do not settle fails the +listing, a count they leave short goes to a whole listing with no wait, and a +page or id read still failing on a 5xx after `LISTING_WAITS` fails it 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 @@ -444,8 +446,9 @@ class _CountMissed: """`Reader._incremental`'s answer when its pages read whole but do not hold the live count: `COUNT_MISSED` (fewer), the one case read again on the same base, or `COUNT_OVER` (more, or any count once the pages near - expiry were read again), which goes to one whole listing, since no wait - removes an entry GitHub no longer counts.""" + expiry were read again, but a top-up's exact count left short), which + goes to one whole listing, since no wait removes an entry GitHub no + longer counts.""" # Fewer than the live count: the pages skipped an artifact, which another @@ -1559,16 +1562,17 @@ def _incremental( 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). A result that holds more than - the live count, or any result while a kept entry is near expiry, has + the live count, or exactly it while a kept entry is near expiry, has the ids around its kept entries near expiry decided by the pages that hold them now (`_near_expiry`); an exact count they leave unsettled gives None, since an entry deleted early may stand in for a FAIL. The result must hold exactly the first page's `total_count` artifacts, GitHub's live count: fewer gives `COUNT_MISSED`, which - `_current_listing` reads again on the same base after a wait, and more, - or any count the pages near expiry left unsettled, gives `COUNT_OVER`, - which goes to one whole listing, since no wait removes an entry GitHub - no longer counts; either prints one + `_current_listing` reads again on the same base after a wait, and + more, or any count once the pages near expiry were read (a top-up's + exact count left short aside, which another whole listing can + restore), gives `COUNT_OVER`, which goes to one whole listing, since + no wait removes an entry GitHub no longer counts; either prints one `LISTING_MISS_PREFIX` line on stderr naming `stage`, the counts and the pages read. No base, an entry or page of another shape, a missing `Date`, or any failed read gives None (`_page_transient` says whether @@ -1633,7 +1637,7 @@ def _incremental( regions = 0 over = len(listing) > total limit = server_now + EARLY_DELETION_LEAD - near = any( + near = len(listing) == total and any( a["expires_at"] is not None and _when(a["expires_at"]) <= limit for a in kept ) @@ -1643,12 +1647,12 @@ def _incremental( # can hide one there standing in for a FAIL the pages skipped. self._over_counted |= over reread = self._near_expiry( - listing, len(fresh), max(0, len(listing) - total), server_now, deadline + listing, len(fresh), len(listing) - total, server_now, deadline ) if reread is None: return None listing, regions, settled = reread - if not over and not settled: + if near and not settled: # Unsettled, an exact count may still lack a FAIL. return None if len(listing) == total: @@ -1670,9 +1674,13 @@ def _incremental( file=sys.stderr, flush=True, ) - # Once the pages near expiry were read on an over-count, no wait can - # settle it. - return COUNT_OVER if over or len(listing) > total else COUNT_MISSED + # Once the pages near expiry were read, no wait on this base settles + # the count: the base still holds what they dropped. A top-up's exact + # count left short goes to another whole listing, which can restore a + # skip outside those pages. + if over or len(listing) > total or (near and stage != "top-up"): + return COUNT_OVER + return COUNT_MISSED def _near_expiry( self, 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 ba800ab5d1f..10ebfc84e42 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 @@ -5232,7 +5232,7 @@ def entry(self, artifact_id: int, expires: timedelta | None = None) -> Any: expires=None if expires is None else LISTED_AT + expires, ) - def fail(self, artifact_id: int) -> Any: + def earlier_fail(self, artifact_id: int) -> Any: """An earlier head's FAIL of this pull request, two days old.""" return dated( named(EARLIER, "fail"), 2, artifact_id, self.base_at - timedelta(days=2) @@ -5249,7 +5249,7 @@ def skipped(self) -> FakeGitHub: holds 600, expiring in 30 minutes, which GitHub then deletes early: the union holds exactly the live count, with 600 in the FAIL's place.""" dropping = self.entry(600, timedelta(minutes=30)) - fake = self.fake(self.fail(650), dropping) + fake = self.fake(self.earlier_fail(650), dropping) fake.full_skips = [{650}] listed = fake.listed @@ -5281,16 +5281,50 @@ def test_a_stale_carry_whose_early_deletion_hides_a_missed_fail(self) -> None: # 30 minutes before its expiry: five against a live count of five. # Page 3 (700) does not reach 750, but 600's own read answers 404, # so the count is one short and the whole listing holds the FAIL. + # No wait on this base could restore it: no INCREMENTAL_WAITS. self.write_carry([*self.base, self.entry(600, timedelta(minutes=30))]) - fake = self.fake(self.fail(750)) + fake = self.fake(self.earlier_fail(750)) reader, evidence = self.judge(fake) self.assertEqual(evidence.refusal, "verify:earlier-fail", evidence.detail) self.assertEqual(reader.listing_mode, "full") self.assertEqual(fake.full_listings(), 1) - self.assertTrue(self.by_id(fake)) + self.assertEqual(len(self.by_id(fake)), 1) + self.assertEqual(self.sleep.call_args_list, []) ids = [a["id"] for a in json.loads(self.saved.read_text())["artifacts"]] self.assertEqual(ids, [1000, 900, 800, 750, 700]) + def test_a_dropped_entry_does_not_spend_the_top_ups_page_cap(self) -> None: + # As above, with 650 live and near expiry too: the incremental read + # takes pages 3 and 4, and the whole listing's top-up, which keeps + # 650, takes page 3 again: three of a cap of six. Re-read after each + # wait on the same base, the incremental read would take all six and + # leave the top-up unsettled. + near = self.entry(650, timedelta(minutes=30)) + self.write_carry([*self.base, near, self.entry(600, timedelta(minutes=30))]) + fake = self.fake(self.earlier_fail(750), near) + with mock.patch.object(ve, "NEAR_EXPIRY_PAGE_CAP", 6): + reader, evidence = self.judge(fake) + self.assertEqual(evidence.refusal, "verify:earlier-fail", evidence.detail) + self.assertEqual(reader.listing_mode, "full") + self.assertEqual(self.sleep.call_args_list, []) + self.assertEqual(reader._near_expiry_reads, 3) + self.assertFalse(self.failed.exists()) + + def test_a_short_count_reads_no_pages_near_expiry(self) -> None: + # The carry lacks FAIL 750 and 600 is live, near expiry: five of six. + # A short count is never kept, so it waits and reads again as before; + # only the whole listing's exact top-up reads 600's page. + near = self.entry(600, timedelta(minutes=30)) + self.write_carry([*self.base, near]) + fake = self.fake(self.earlier_fail(750), near) + reader, evidence = self.judge(fake) + self.assertEqual(evidence.refusal, "verify:earlier-fail", evidence.detail) + self.assertEqual(reader.listing_mode, "full") + waits = [c.args[0] for c in self.sleep.call_args_list] + self.assertEqual(waits, list(ve.INCREMENTAL_WAITS)) + self.assertEqual(reader._near_expiry_reads, 1) + self.assertEqual(self.by_id(fake), []) + def test_an_exact_count_with_none_near_expiry_reads_nothing_more(self) -> None: # 600 expires three hours on, past EARLY_DELETION_LEAD: page 1 alone. self.write_carry([*self.base, self.entry(600, timedelta(hours=3))]) From 244e885128026c1391a28afd621b6b33e9ec42c0 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 1 Nov 2026 12:58:35 -0500 Subject: [PATCH 3/6] test(source-control): pin the top-up short count and the free over-count slot The ready-gate code review found two branches no test held: a top-up's exact count left short must read another whole listing, and an exact count must not take the job's over-count slot. Each test fails under the mutant that breaks its branch. Reword the COUNT_OVER docstrings to match the returns. Co-Authored-By: Claude Opus 5.5 --- .../scripts/babysit_verify_evidence.py | 9 ++++--- .../tests/test_babysit_verify_evidence.py | 27 ++++++++++++++----- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py index 16400418309..084a5b5ca54 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 @@ -454,8 +454,8 @@ class _CountMissed: """`Reader._incremental`'s answer when its pages read whole but do not hold the live count: `COUNT_MISSED` (fewer), the one case read again on the same base, or `COUNT_OVER` (more, or any count once the pages near - expiry were read again, but a top-up's exact count left short), which - goes to one whole listing, since no wait removes an entry GitHub no + expiry were read again, except a top-up's exact count left short), which + goes to the whole listings, since no wait removes an entry GitHub no longer counts.""" @@ -1579,8 +1579,9 @@ def _incremental( `_current_listing` reads again on the same base after a wait, and more, or any count once the pages near expiry were read (a top-up's exact count left short aside, which another whole listing can - restore), gives `COUNT_OVER`, which goes to one whole listing, since - no wait removes an entry GitHub no longer counts; either prints one + restore), gives `COUNT_OVER`, which goes to the whole listings (one, + after an over-count), since no wait removes an entry GitHub no + longer counts; either prints one `LISTING_MISS_PREFIX` line on stderr naming `stage`, the counts and the pages read. No base, an entry or page of another shape, a missing `Date`, or any failed read gives None (`_page_transient` says whether 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 732ea05ffd4..9dba7a7619b 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 @@ -5318,13 +5318,14 @@ def by_id(fake: FakeGitHub) -> list[list[str]]: by_id = re.compile(r"/actions/artifacts/[0-9]+$") return [c for c in fake.calls if by_id.search(c[-1])] - def skipped(self) -> FakeGitHub: - """No carry: the whole listing skips FAIL 650 at a page boundary and - holds 600, expiring in 30 minutes, which GitHub then deletes early: - the union holds exactly the live count, with 600 in the FAIL's place.""" + def skipped(self, fail: int = 650) -> FakeGitHub: + """No carry: the first whole listing skips FAIL `fail` at a page + boundary and holds 600, expiring in 30 minutes, which GitHub then + deletes early: the union holds exactly the live count, with 600 in + the FAIL's place.""" dropping = self.entry(600, timedelta(minutes=30)) - fake = self.fake(self.earlier_fail(650), dropping) - fake.full_skips = [{650}] + fake = self.fake(self.earlier_fail(fail), dropping) + fake.full_skips = [{fail}] listed = fake.listed def drops_during_the_listing() -> list[dict[str, Any]]: @@ -5364,9 +5365,23 @@ def test_a_stale_carry_whose_early_deletion_hides_a_missed_fail(self) -> None: self.assertEqual(fake.full_listings(), 1) self.assertEqual(len(self.by_id(fake)), 1) self.assertEqual(self.sleep.call_args_list, []) + # An exact count is not an over-count: the job's slot stays free. + overcount = pathlib.Path(f"{self.saved}{ve.OVERCOUNT_FALLBACK_SUFFIX}") + self.assertFalse(overcount.exists()) ids = [a["id"] for a in json.loads(self.saved.read_text())["artifacts"]] self.assertEqual(ids, [1000, 900, 800, 750, 700]) + def test_a_top_up_left_short_reads_another_whole_listing(self) -> None: + # The first whole listing skips FAIL 850, far from 600's page: the + # top-up keeps 600, whose read answers 404, and is one short. A + # second whole listing, not a refusal, restores the FAIL. + fake = self.skipped(850) + reader, evidence = self.judge(fake) + self.assertEqual(evidence.refusal, "verify:earlier-fail", evidence.detail) + self.assertEqual(reader.listing_mode, "full") + self.assertEqual(fake.full_listings(), 2) + self.assertFalse(self.failed.exists()) + def test_a_dropped_entry_does_not_spend_the_top_ups_page_cap(self) -> None: # As above, with 650 live and near expiry too: the incremental read # takes pages 3 and 4, and the whole listing's top-up, which keeps From db621263dab82c3bd21822968b87a9de38fb9118 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 1 Nov 2026 13:12:05 -0500 Subject: [PATCH 4/6] fix(source-control): leave a cap-only near-expiry refusal unnoted for the job An exact count the near-expiry reads leave unsettled only because NEAR_EXPIRY_PAGE_CAP or NEAR_EXPIRY_ID_READ_CAP ran out still refuses the read, but as transient: no .failed note, so a later Reader with fresh caps can list and settle it. _near_expiry now returns why an entry stayed undecided (near-expiry-cap, unbracketed, id-answer), and every unsettled exact count prints a babysit-listing-miss line naming that cause. Co-Authored-By: Claude Opus 5.5 --- ...erify-listing-exact-count-fail-e7080d66.md | 2 +- .../scripts/babysit_verify_evidence.py | 63 ++++++++++++------- .../tests/test_babysit_verify_evidence.py | 53 ++++++++++------ 3 files changed, 76 insertions(+), 42 deletions(-) diff --git a/.changes/source-control/fix-verify-listing-exact-count-fail-e7080d66.md b/.changes/source-control/fix-verify-listing-exact-count-fail-e7080d66.md index be4054572a2..030273f2463 100644 --- a/.changes/source-control/fix-verify-listing-exact-count-fail-e7080d66.md +++ b/.changes/source-control/fix-verify-listing-exact-count-fail-e7080d66.md @@ -4,4 +4,4 @@ bump: patch ### Fixed -- **babysit-prs**: the pr-verify verdict reader re-reads the pages around kept artifact-listing entries near expiry even when the listing holds exactly GitHub's live count. An entry GitHub deleted early could stand in for a FAIL the listing skipped, so the count matched and the FAIL was left out. A count those reads cannot settle now refuses instead of allowing; a 5xx that outlasts its waits stays transient. +- **babysit-prs**: the pr-verify verdict reader re-reads the pages around kept artifact-listing entries near expiry even when the listing holds exactly GitHub's live count. An entry GitHub deleted early could stand in for a FAIL the listing skipped, so the count matched and the FAIL was left out. A count those reads cannot settle now refuses instead of allowing; a 5xx that outlasts its waits, or a count left unsettled only by the near-expiry read caps, refuses as transient and is not noted for the job, and the latter prints a `babysit-listing-miss` line with `cause` `near-expiry-cap`. 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 084a5b5ca54..3b94a8726b9 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 @@ -160,7 +160,8 @@ listing. They are read on an exact count too: an entry deleted early can stand in for a FAIL the pages skipped. An over-count those pages do not settle goes to the one whole listing, an exact count they do not settle fails the -listing, a count they leave short goes to a whole listing with no wait, and a +listing (as transient when only `NEAR_EXPIRY_PAGE_CAP` or +`NEAR_EXPIRY_ID_READ_CAP` left it so, so a later Reader may list), a count they leave short goes to a whole listing with no wait, and a page or id read still failing on a 5xx after `LISTING_WAITS` fails it as transient. Such a listing could hide a FAIL by leaving it out, so it is kept only when it holds exactly @@ -424,6 +425,9 @@ def verdict_name(name: str) -> re.Match[str] | None: # 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 `cause` a `LISTING_MISS_PREFIX` line names when the caps above alone +# leave an exact count unsettled: refused, but not noted for the job. +NEAR_EXPIRY_CAP_CAUSE = "near-expiry-cap" # 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 @@ -1263,6 +1267,9 @@ class Reader: # The last `_incremental_page` that read nothing failed on a 5xx, a # network error or a body cut short, after its waits. _page_transient: bool = False + # Whether `_incremental`'s last None was an exact count the near-expiry + # caps alone left unsettled (`NEAR_EXPIRY_CAP_CAUSE`). + _caps_spent: bool = False # This listing's pages held more than the live count, so it takes one # whole listing at most. _over_counted: bool = False @@ -1541,9 +1548,10 @@ def _whole_listings(self, budget: float) -> list[dict[str, Any]]: else: cause = "never held" noun = "listing" if reads == 1 else "listings" - # A top-up page lost to a 5xx or network error is transient: not - # noted for the job (`_note_failed_listing`). - transient = listing is None and self._page_transient + # A top-up page lost to a 5xx or network error, or an exact count left + # unsettled only by the near-expiry caps, is transient: not noted for + # the job (`_note_failed_listing`), so a later Reader can list again. + transient = listing is None and (self._page_transient or self._caps_spent) raise (_Transient if transient else _Unreadable)( f"repository artifacts: {reads} whole {noun} {cause} " "GitHub's live artifact count" @@ -1587,8 +1595,10 @@ def _incremental( `Date`, or any failed read gives None (`_page_transient` says whether the failed read was a 5xx or network error). `base` replaces `_incremental_base` and `deadline` its own - `LISTING_TIMEOUT_SECONDS` (`_complete_listing`'s top-up).""" - self._page_transient = False + `LISTING_TIMEOUT_SECONDS` (`_complete_listing`'s top-up). + An exact count left unsettled prints the same line with its `cause` + and sets `_caps_spent` when only the caps left it so.""" + self._page_transient = self._caps_spent = False if base is not None and not all(_listed_entry(a) for a in base[1]): return None base = base or self._incremental_base() @@ -1660,14 +1670,7 @@ def _incremental( ) if reread is None: return None - listing, regions, settled = reread - if near and not settled: - # Unsettled, an exact count may still lack a FAIL. - return None - if len(listing) == total: - return listing - # Fewer than the live count: the pages skipped an artifact. More: the - # base holds one GitHub no longer lists below the pages read. + listing, regions, unsettled = reread miss = { "stage": stage, "listed": len(listing), @@ -1678,6 +1681,21 @@ def _incremental( "region_pages": regions, "base_age_seconds": round((server_now - base_at).total_seconds()), } + if near and unsettled: + # Unsettled, an exact count may still lack a FAIL. Refused only + # for want of the caps, it is not noted for the job. + self._caps_spent = unsettled == NEAR_EXPIRY_CAP_CAUSE + miss["cause"] = unsettled + print( + f"{LISTING_MISS_PREFIX} {json.dumps(miss, sort_keys=True)}", + file=sys.stderr, + flush=True, + ) + return None + if len(listing) == total: + return listing + # Fewer than the live count: the pages skipped an artifact. More: the + # base holds one GitHub no longer lists below the pages read. print( f"{LISTING_MISS_PREFIX} {json.dumps(miss, sort_keys=True)}", file=sys.stderr, @@ -1698,11 +1716,12 @@ def _near_expiry( surplus: int, server_now: datetime, deadline: float, - ) -> tuple[list[dict[str, Any]], int, bool] | None: + ) -> tuple[list[dict[str, Any]], int, str | None] | 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, the pages read, and whether every such - entry was decided; None for a failed page read or an id read that + pages that hold them now, the pages read, and None when every such + entry was decided, else why not (`NEAR_EXPIRY_CAP_CAUSE`, "unbracketed" + or "id-answer"); 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. @@ -1745,7 +1764,7 @@ def _near_expiry( ended = False while True: if self._near_expiry_reads >= NEAR_EXPIRY_PAGE_CAP: - return listing, reads, False + return listing, reads, NEAR_EXPIRY_CAP_CAUSE read = self._incremental_page(page, deadline) reads += 1 self._near_expiry_reads += 1 @@ -1769,7 +1788,7 @@ def _near_expiry( downs -= 1 page += 1 if not seen or (not ended and min(seen) > bottom): - return listing, reads, False + return listing, reads, "unbracketed" spans.append((0 if ended else min(seen), max(seen))) decided.update(seen) gone: set[int] = set() @@ -1779,16 +1798,16 @@ def _near_expiry( ): continue if self._artifact_reads >= NEAR_EXPIRY_ID_READ_CAP: - return listing, reads, False + return listing, reads, NEAR_EXPIRY_CAP_CAUSE self._artifact_reads += 1 answer = self._artifact_gone(a["id"], deadline) if answer is None: # A transient answer fails the read, as a lost page does. - return None if self._page_transient else (listing, reads, False) + return None if self._page_transient else (listing, reads, "id-answer") 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, True + return sorted(merged.values(), key=lambda a: -a["id"]), reads, None def _artifact_gone(self, artifact_id: int, deadline: float) -> bool | None: """Whether GitHub no longer holds the artifact: True on a 404 or an 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 9dba7a7619b..95d37bda8c7 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 @@ -5429,26 +5429,41 @@ def test_an_exact_count_the_pages_near_expiry_cannot_settle_refuses( ) -> None: # 600, live and expiring in 30 minutes, is on page 3: read, it settles # the count. With no page left under the cap, neither the incremental - # read nor the whole listing's top-up can, and the gate refuses. - for cap, refusal, mode, full in ( - (ve.NEAR_EXPIRY_PAGE_CAP, None, "incremental", 0), - (0, "verify:unreadable", "full", 1), + # read nor the whole listing's top-up can, and the gate refuses. The + # caps alone left it unsettled, so nothing is noted for the job and + # the next Reader, its caps fresh, settles the count and allows. + near = self.entry(600, timedelta(minutes=30)) + self.write_carry([*self.base, near]) + fake = self.fake(near) + stderr = io.StringIO() + with ( + mock.patch.object(ve, "NEAR_EXPIRY_PAGE_CAP", 0), + contextlib.redirect_stderr(stderr), ): - with self.subTest(cap=cap): - for path in (self.saved, self.failed): - path.unlink(missing_ok=True) - near = self.entry(600, timedelta(minutes=30)) - self.write_carry([*self.base, near]) - fake = self.fake(near) - with mock.patch.object(ve, "NEAR_EXPIRY_PAGE_CAP", cap): - reader, evidence = self.judge(fake) - self.assertEqual(evidence.refusal, refusal, evidence.detail) - self.assertEqual(reader.listing_mode, mode) - self.assertEqual(fake.full_listings(), full) - self.assertEqual(self.by_id(fake), []) - if refusal: - self.assertIn("could not be checked against", evidence.detail) - self.assertTrue(self.failed.exists()) + reader, evidence = self.judge(fake) + self.assertEqual(evidence.refusal, "verify:unreadable", evidence.detail) + self.assertIn("could not be checked against", evidence.detail or "") + self.assertEqual(reader.listing_mode, "full") + self.assertEqual(fake.full_listings(), 1) + self.assertEqual(self.by_id(fake), []) + self.assertFalse(self.failed.exists()) + misses = [ + json.loads(line.split(" ", 1)[1]) + for line in stderr.getvalue().splitlines() + if line.startswith(ve.LISTING_MISS_PREFIX) + ] + self.assertEqual( + [(m["stage"], m.get("cause")) for m in misses], + [ + ("incremental", ve.NEAR_EXPIRY_CAP_CAUSE), + ("top-up", ve.NEAR_EXPIRY_CAP_CAUSE), + ], + ) + fake = self.fake(near) + reader, evidence = self.judge(fake) + self.assertIsNone(evidence.refusal, evidence.detail) + self.assertEqual(reader.listing_mode, "incremental") + self.assertEqual(fake.full_listings(), 0) def test_a_5xx_on_an_exact_count_id_read_refuses_as_transient(self) -> None: # The skipped listing, with 600's read by id answering 502 through its From 12251a6b877cc7de04ed8b88ef152cc14fbf3371 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 1 Nov 2026 14:24:04 -0500 Subject: [PATCH 5/6] fix(source-control): refuse a near-expiry deadline overrun as transient listing_timeout sizes the whole listings' deadline for LISTING_READS listings and their top-ups only. Exact-count top-ups now spend it on near-expiry page and id reads, so a short top-up's next whole listing, or a later region page, can run out of time; that failure was non-transient and noted for the job, so every later Reader refused. When the deadline runs out after this listing's near-expiry reads began (in _read_listing, a region page or an id read), the read is refused as transient, never allowed, writes no .failed note, and prints a babysit-listing-miss line with cause near-expiry-deadline. listing_timeout and cold_cost are unchanged. Co-Authored-By: Claude Opus 5.5 --- ...erify-listing-exact-count-fail-e7080d66.md | 2 +- .../scripts/babysit_verify_evidence.py | 90 +++++++++++++++---- .../tests/test_babysit_verify_evidence.py | 50 +++++++++++ 3 files changed, 126 insertions(+), 16 deletions(-) diff --git a/.changes/source-control/fix-verify-listing-exact-count-fail-e7080d66.md b/.changes/source-control/fix-verify-listing-exact-count-fail-e7080d66.md index 030273f2463..c4786d0b1d9 100644 --- a/.changes/source-control/fix-verify-listing-exact-count-fail-e7080d66.md +++ b/.changes/source-control/fix-verify-listing-exact-count-fail-e7080d66.md @@ -4,4 +4,4 @@ bump: patch ### Fixed -- **babysit-prs**: the pr-verify verdict reader re-reads the pages around kept artifact-listing entries near expiry even when the listing holds exactly GitHub's live count. An entry GitHub deleted early could stand in for a FAIL the listing skipped, so the count matched and the FAIL was left out. A count those reads cannot settle now refuses instead of allowing; a 5xx that outlasts its waits, or a count left unsettled only by the near-expiry read caps, refuses as transient and is not noted for the job, and the latter prints a `babysit-listing-miss` line with `cause` `near-expiry-cap`. +- **babysit-prs**: the pr-verify verdict reader re-reads the pages around kept artifact-listing entries near expiry even when the listing holds exactly GitHub's live count. An entry GitHub deleted early could stand in for a FAIL the listing skipped, so the count matched and the FAIL was left out. A count those reads cannot settle now refuses instead of allowing; a 5xx that outlasts its waits, a count left unsettled only by the near-expiry read caps, or a listing whose time runs out after those reads began, refuses as transient and is not noted for the job; the last two print a `babysit-listing-miss` line with `cause` `near-expiry-cap` or `near-expiry-deadline`. 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 3b94a8726b9..e5582a12b03 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 @@ -428,6 +428,9 @@ def verdict_name(name: str) -> re.Match[str] | None: # The `cause` a `LISTING_MISS_PREFIX` line names when the caps above alone # leave an exact count unsettled: refused, but not noted for the job. NEAR_EXPIRY_CAP_CAUSE = "near-expiry-cap" +# The `cause` when the listing's deadline runs out after its near-expiry reads +# began, which `listing_timeout` does not size for: refused, but not noted. +NEAR_EXPIRY_DEADLINE_CAUSE = "near-expiry-deadline" # 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 @@ -1267,9 +1270,10 @@ class Reader: # The last `_incremental_page` that read nothing failed on a 5xx, a # network error or a body cut short, after its waits. _page_transient: bool = False - # Whether `_incremental`'s last None was an exact count the near-expiry - # caps alone left unsettled (`NEAR_EXPIRY_CAP_CAUSE`). - _caps_spent: bool = False + # Whether `_incremental`'s last None came from the near-expiry caps or the + # deadline running out after near-expiry reads began + # (`NEAR_EXPIRY_CAP_CAUSE`, `NEAR_EXPIRY_DEADLINE_CAUSE`). + _near_expiry_spent: bool = False # This listing's pages held more than the live count, so it takes one # whole listing at most. _over_counted: bool = False @@ -1524,7 +1528,14 @@ def _whole_listings(self, budget: float) -> list[dict[str, Any]]: reads += 1 read_at = _wall_clock() self._whole_paged = True - payload = self._read_listing(deadline, budget) + try: + payload = self._read_listing(deadline, budget) + except _Unreadable as exc: + if isinstance(exc, _Transient) or not self._near_expiry_out_of_time( + deadline, "whole-listing", reads + ): + raise + raise _Transient(str(exc)) from exc deadline = self._read_deadline for item in flatten_paginated_items( payload, "repository artifacts", items_key="artifacts" @@ -1548,10 +1559,13 @@ def _whole_listings(self, budget: float) -> list[dict[str, Any]]: else: cause = "never held" noun = "listing" if reads == 1 else "listings" - # A top-up page lost to a 5xx or network error, or an exact count left - # unsettled only by the near-expiry caps, is transient: not noted for - # the job (`_note_failed_listing`), so a later Reader can list again. - transient = listing is None and (self._page_transient or self._caps_spent) + # A top-up page lost to a 5xx or network error, or a count left + # unsettled only by the near-expiry caps or the time they took, is + # transient: not noted for the job (`_note_failed_listing`), so a later + # Reader can list again. + transient = listing is None and ( + self._page_transient or self._near_expiry_spent + ) raise (_Transient if transient else _Unreadable)( f"repository artifacts: {reads} whole {noun} {cause} " "GitHub's live artifact count" @@ -1597,8 +1611,9 @@ def _incremental( `base` replaces `_incremental_base` and `deadline` its own `LISTING_TIMEOUT_SECONDS` (`_complete_listing`'s top-up). An exact count left unsettled prints the same line with its `cause` - and sets `_caps_spent` when only the caps left it so.""" - self._page_transient = self._caps_spent = False + and sets `_near_expiry_spent` when only the caps, or the deadline + running out during those reads, left it so.""" + self._page_transient = self._near_expiry_spent = False if base is not None and not all(_listed_entry(a) for a in base[1]): return None base = base or self._incremental_base() @@ -1654,6 +1669,7 @@ def _incremental( ] listing = sorted([*fresh.values(), *kept], key=lambda a: -a["id"]) regions = 0 + unsettled: str | None = None over = len(listing) > total limit = server_now + EARLY_DELETION_LEAD near = len(listing) == total and any( @@ -1681,10 +1697,13 @@ def _incremental( "region_pages": regions, "base_age_seconds": round((server_now - base_at).total_seconds()), } - if near and unsettled: + if unsettled == NEAR_EXPIRY_DEADLINE_CAUSE or (near and unsettled): # Unsettled, an exact count may still lack a FAIL. Refused only - # for want of the caps, it is not noted for the job. - self._caps_spent = unsettled == NEAR_EXPIRY_CAP_CAUSE + # for want of the caps or their time, it is not noted for the job. + self._near_expiry_spent = unsettled in ( + NEAR_EXPIRY_CAP_CAUSE, + NEAR_EXPIRY_DEADLINE_CAUSE, + ) miss["cause"] = unsettled print( f"{LISTING_MISS_PREFIX} {json.dumps(miss, sort_keys=True)}", @@ -1769,7 +1788,7 @@ def _near_expiry( reads += 1 self._near_expiry_reads += 1 if read is None: - return None + return self._out_of_time(listing, reads, deadline) items = read[0].get("artifacts") if not is_json_array(items) or not all(map(_listed_entry, items)): return None @@ -1803,12 +1822,53 @@ def _near_expiry( answer = self._artifact_gone(a["id"], deadline) if answer is None: # A transient answer fails the read, as a lost page does. - return None if self._page_transient else (listing, reads, "id-answer") + if self._page_transient: + return None + return self._out_of_time(listing, reads, deadline) or ( + listing, + reads, + "id-answer", + ) 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, None + def _out_of_time( + self, listing: list[dict[str, Any]], reads: int, deadline: float + ) -> tuple[list[dict[str, Any]], int, str] | None: + """`_near_expiry`'s answer when a read gave nothing: unsettled for + `NEAR_EXPIRY_DEADLINE_CAUSE` when the deadline ran out (a 5xx past its + waits stays transient), else None.""" + if self._page_transient or deadline - _listing_clock() > 0: + return None + return listing, reads, NEAR_EXPIRY_DEADLINE_CAUSE + + def _near_expiry_out_of_time( + self, deadline: float | None, stage: str, reads: int + ) -> bool: + """Whether a whole listing that failed did so for want of time after + this listing's near-expiry reads began, which `listing_timeout` does + not size for: then it prints a `LISTING_MISS_PREFIX` line with + `NEAR_EXPIRY_DEADLINE_CAUSE`, and is refused as transient.""" + if ( + deadline is None + or deadline - _listing_clock() > 0 + or not (self._near_expiry_reads or self._artifact_reads) + ): + return False + miss = { + "stage": stage, + "whole_listings": reads, + "cause": NEAR_EXPIRY_DEADLINE_CAUSE, + } + print( + f"{LISTING_MISS_PREFIX} {json.dumps(miss, sort_keys=True)}", + file=sys.stderr, + flush=True, + ) + return True + 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, with 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 95d37bda8c7..7de135ae692 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 @@ -5465,6 +5465,56 @@ def test_an_exact_count_the_pages_near_expiry_cannot_settle_refuses( self.assertEqual(reader.listing_mode, "incremental") self.assertEqual(fake.full_listings(), 0) + def test_near_expiry_reads_out_of_the_listings_time_are_not_noted( + self, + ) -> None: + # The skipped listing, each read taking `step` seconds of the whole + # listings' 60 s (`listing_timeout` for five artifacts), the caps out + # of reach. The first top-up reads 600's page and id and comes out + # short; at 15 s a read the second whole listing finds no time left, + # at 10 s the second top-up's page near expiry does. Either way the + # time ran out after near-expiry reads began: refused, not noted. + for step, stage in ((15.0, "whole-listing"), (10.0, "top-up")): + with self.subTest(step=step): + self.failed.unlink(missing_ok=True) + fake = self.skipped(850) + now = [0.0] + + def listing(args: list[str], **kwargs: Any) -> Any: + now[0] += step + return fake(args, **kwargs) + + def page(args: list[str], **kwargs: Any) -> Any: + now[0] += step + return fake(args, **kwargs), {"date": DATE} + + stderr = io.StringIO() + with ( + mock.patch.object(ve, "NEAR_EXPIRY_PAGE_CAP", 1000), + mock.patch.object(ve, "NEAR_EXPIRY_ID_READ_CAP", 1000), + mock.patch.object(ve, "_elapsed", return_value=0.0), + mock.patch.object( + ve, "_listing_clock", side_effect=lambda: now[0] + ), + mock.patch.object(postmerge, "gh_capture", side_effect=listing), + mock.patch.object(ve, "gh_api_with_headers", side_effect=page), + contextlib.redirect_stderr(stderr), + ): + evidence = ve.Reader(REPO, "main", now=NOW).evidence( + PR, HEAD, pull() + ) + self.assertEqual(evidence.refusal, "verify:unreadable") + self.assertFalse(self.failed.exists()) + causes = [ + (m["stage"], m.get("cause")) + for m in ( + json.loads(line.split(" ", 1)[1]) + for line in stderr.getvalue().splitlines() + if line.startswith(ve.LISTING_MISS_PREFIX) + ) + ] + self.assertIn((stage, ve.NEAR_EXPIRY_DEADLINE_CAUSE), causes) + def test_a_5xx_on_an_exact_count_id_read_refuses_as_transient(self) -> None: # The skipped listing, with 600's read by id answering 502 through its # waits: the Reader refuses, nothing is noted for the job, and the From 5fcddd3557a0d620fc3d075638820f9425f44f2b Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 1 Nov 2026 14:59:39 -0500 Subject: [PATCH 6/6] fix(source-control): refuse a top-up page out of time as transient A top-up's own page that ran out of the shared deadline after an earlier top-up's near-expiry reads returned a bare None with no transient flag, so the whole listings raised _Unreadable and noted the failure for the job. Classify it with _near_expiry_out_of_time like the whole-listing case: it stays a refusal, refused as transient, with no job-wide .failed note. The near-expiry deadline subtest gains a 12 s step that lands the timeout on the second top-up's page 1; it fails on the previous head. Co-Authored-By: Claude Opus 5.5 --- .../scripts/babysit_verify_evidence.py | 18 +++++++++++------- .../tests/test_babysit_verify_evidence.py | 16 +++++++++++----- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py index 8fe50e02fca..ff39ac71a11 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 @@ -1563,11 +1563,14 @@ def _whole_listings(self, budget: float) -> list[dict[str, Any]]: cause = "never held" noun = "listing" if reads == 1 else "listings" # A top-up page lost to a 5xx or network error, or a count left - # unsettled only by the near-expiry caps or the time they took, is - # transient: not noted for the job (`_note_failed_listing`), so a later - # Reader can list again. + # unsettled only by the near-expiry caps or the time they took, or a + # top-up page out of time after near-expiry reads began, is transient: + # not noted for the job (`_note_failed_listing`), so a later Reader can + # list again. transient = listing is None and ( - self._page_transient or self._near_expiry_spent + self._page_transient + or self._near_expiry_spent + or self._near_expiry_out_of_time(deadline, "top-up", reads) ) raise (_Transient if transient else _Unreadable)( f"repository artifacts: {reads} whole {noun} {cause} " @@ -1850,9 +1853,10 @@ def _out_of_time( def _near_expiry_out_of_time( self, deadline: float | None, stage: str, reads: int ) -> bool: - """Whether a whole listing that failed did so for want of time after - this listing's near-expiry reads began, which `listing_timeout` does - not size for: then it prints a `LISTING_MISS_PREFIX` line with + """Whether a whole listing or top-up page that failed did so for want + of time after this listing's near-expiry reads began, which + `listing_timeout` does not size for: then it prints a + `LISTING_MISS_PREFIX` line with `NEAR_EXPIRY_DEADLINE_CAUSE`, and is refused as transient.""" if ( deadline is 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 4a7a1eb1fc9..2fcbe4d8f94 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 @@ -5472,9 +5472,15 @@ def test_near_expiry_reads_out_of_the_listings_time_are_not_noted( # listings' 60 s (`listing_timeout` for five artifacts), the caps out # of reach. The first top-up reads 600's page and id and comes out # short; at 15 s a read the second whole listing finds no time left, - # at 10 s the second top-up's page near expiry does. Either way the - # time ran out after near-expiry reads began: refused, not noted. - for step, stage in ((15.0, "whole-listing"), (10.0, "top-up")): + # at 12 s the second top-up's page 1 does, and at 10 s its page near + # expiry does. Each way the time ran out after near-expiry reads + # began: refused, not noted. `whole` marks the miss line printed for + # a failed whole listing or top-up page, not a region read. + for step, stage, whole in ( + (15.0, "whole-listing", True), + (12.0, "top-up", True), + (10.0, "top-up", False), + ): with self.subTest(step=step): self.failed.unlink(missing_ok=True) fake = self.skipped(850) @@ -5506,14 +5512,14 @@ def page(args: list[str], **kwargs: Any) -> Any: self.assertEqual(evidence.refusal, "verify:unreadable") self.assertFalse(self.failed.exists()) causes = [ - (m["stage"], m.get("cause")) + (m["stage"], m.get("cause"), "whole_listings" in m) for m in ( json.loads(line.split(" ", 1)[1]) for line in stderr.getvalue().splitlines() if line.startswith(ve.LISTING_MISS_PREFIX) ) ] - self.assertIn((stage, ve.NEAR_EXPIRY_DEADLINE_CAUSE), causes) + self.assertIn((stage, ve.NEAR_EXPIRY_DEADLINE_CAUSE, whole), causes) def test_a_5xx_on_an_exact_count_id_read_refuses_as_transient(self) -> None: # The skipped listing, with 600's read by id answering 502 through its