diff --git a/.changes/pr-pipeline/fix-verify-listing-early-deletion-9c4e2a17.md b/.changes/pr-pipeline/fix-verify-listing-early-deletion-9c4e2a17.md new file mode 100644 index 00000000000..a969bc8131a --- /dev/null +++ b/.changes/pr-pipeline/fix-verify-listing-early-deletion-9c4e2a17.md @@ -0,0 +1,8 @@ +--- +bump: patch +--- + +### Fixed + +- 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 new file mode 100644 index 00000000000..79e218e1db2 --- /dev/null +++ b/.changes/source-control/fix-verify-listing-early-deletion-5b1e9c3a.md @@ -0,0 +1,28 @@ +--- +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 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 + 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 f222a54e9df..22c02a5f954 100644 --- a/.github/workflows/pr-require-checks.yml +++ b/.github/workflows/pr-require-checks.yml @@ -6390,13 +6390,16 @@ 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, 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), 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 + 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 @@ -6438,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))) + 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 3df8c071754..22ac9d0c589 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -2007,7 +2007,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, 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 @@ -2648,16 +2649,25 @@ hand-started run of main's pr-merge.yml uploaded, which `.github/scripts/pr-merg fetches as the head-ref map is fetched): it reads pages newest id first down past that listing's highest id until an artifact created two hours before that listing was read, keeps that listing's 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`. Any other count, a missing, older or +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`). 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 before it and checked by a top-up from the first page, since one can skip an artifact at a page boundary while an earlier one expires (#10486); after three whole listings that never hold the live count the candidate refuses `verify:unreadable`, and after the first when its top-up holds more -than the count or does not read, which another whole listing cannot cure. A listing that fails once +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. 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 09b626615b7..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 @@ -131,11 +131,14 @@ listings read before it, and checked by a top-up read from the first page (`Reader._complete_listing`); after `LISTING_READS` whole listings that never hold the live count it refuses `verify:unreadable`, and after the first when -its top-up holds more than the count or does not read, which reading again -cannot cure. The refresh before an allow reads the same way. A listing from +its top-up holds more than the count or does not read, or the incremental +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 +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 @@ -148,8 +151,15 @@ 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, 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 the artifacts whose `expires_at` had passed by the response's `Date`. Three @@ -377,6 +387,31 @@ 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 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. +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. +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 @@ -403,8 +438,9 @@ def no_incremental_waits() -> bool: 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), which goes straight to one whole - listing, since no wait removes an entry GitHub no longer counts.""" + 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.""" # Fewer than the live count: the pages skipped an artifact, which another @@ -425,6 +461,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, 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 @@ -1191,6 +1247,16 @@ 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 + # This listing's pages held more than the live count, so it takes one + # whole listing at most. + _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) @@ -1268,6 +1334,8 @@ def _current_listing(self) -> list[dict[str, Any]]: self._listing_error = failed raise _Unreadable(failed) fetched_at = _wall_clock() + 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, @@ -1288,13 +1356,21 @@ def _current_listing(self) -> list[dict[str, Any]]: self.listing_mode = "incremental" if not isinstance(listing, list): self.listing_mode = "full" + 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 @@ -1393,6 +1469,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: @@ -1416,7 +1496,8 @@ def _whole_listings(self, budget: float) -> list[dict[str, Any]]: deadline: float | None = None listing: list[dict[str, Any]] | _CountMissed | None = None reads = 0 - while reads < LISTING_READS: + # An over-count takes one whole listing: another only adds ids. + while reads < (1 if self._over_counted else LISTING_READS): reads += 1 read_at = _wall_clock() self._whole_paged = True @@ -1472,12 +1553,15 @@ 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: fewer gives `COUNT_MISSED`, which - `_current_listing` reads again on the same base after a wait, and more - gives `COUNT_OVER`, which goes straight 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 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 `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 @@ -1539,6 +1623,18 @@ 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 + over = len(listing) > total + if over: + # GitHub can delete an artifact before its expires_at: read again + # the pages around the kept entries near expiry. + self._over_counted = True + 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 @@ -1550,6 +1646,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( @@ -1557,16 +1654,145 @@ def _incremental( file=sys.stderr, flush=True, ) - return COUNT_OVER if len(listing) > total else COUNT_MISSED + # Once the pages near expiry were read, no wait can settle the count. + return COUNT_OVER if over else 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 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 + `surplus` fewer entries above it, a page up at a time (at most + `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 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 that is not transient 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: + if self._near_expiry_reads >= NEAR_EXPIRY_PAGE_CAP: + return listing, reads + read = self._incremental_page(page, deadline) + reads += 1 + self._near_expiry_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) + 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: + # 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 + 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, 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 + ): + return None + return record["expired"] 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() @@ -1574,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(), ) @@ -1593,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 @@ -1620,6 +1844,50 @@ def _failed_listing(self) -> str | None: return None return text or "repository artifacts: an earlier listing in this job failed" + 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 + try: + with path.open("x", encoding="utf-8"): + pass + except FileExistsError: + return False + except OSError: + 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 91b5fe077a6..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 @@ -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: @@ -4090,6 +4094,348 @@ 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 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), 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): + 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 + 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), reads) + + 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 + # (page 1) holds the count, and no INCREMENTAL_WAITS. + 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(reader.listing_mode, "full") + self.assertEqual(fake.full_listings(), 1) + # Page 1, one capped region page, then the top-up's page 1. + 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_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: + # GitHub dropped 500 early, which the region's pages settle, and 300, + # 90 days from expiry, which no page near expiry reaches. The whole + # listing skips 700 at a page boundary, so its top-up is one short: + # another whole listing would restore 700, but an over-count takes + # one a job, and the refusal is noted for the job's later Readers. + base = self.tail((500, 30)) + self.write_carry(base) + fake = self.fake(base=[a for a in base if a["id"] not in (500, 300)]) + fake.full_skips = [{700}] + reader, evidence = self.judge(fake) + self.assertEqual(evidence.refusal, "verify:unreadable") + self.assertIn("1 whole listing never held", evidence.detail or "") + self.assertEqual(reader.listing_mode, "full") + self.assertEqual(fake.full_listings(), 1) + self.assertEqual(self.sleep.call_args_list, []) + self.assertIn("1 whole listing never held", self.failed.read_text()) + listed = (fake.pages(), fake.full_listings()) + _, again = self.judge(fake) + self.assertEqual(again.refusal, "verify:unreadable") + self.assertEqual((fake.pages(), fake.full_listings()), listed) + 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. @@ -4335,6 +4681,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, }, ) @@ -4376,13 +4723,14 @@ def test_a_live_count_no_listing_can_hold_refuses(self) -> None: self.assertEqual(fake.full_listings(), reads) self.assertFalse(self.saved.exists()) - def skewed(self) -> FakeGitHub: + def skewed(self, lead: timedelta = timedelta(minutes=30)) -> FakeGitHub: """GitHub as on 2026-11-01 08:00-09:00Z: its count drops artifacts up to an hour before the `expires_at` it lists, and stamps uploads an hour ahead of the runner's clock. The carry still holds 600 and 610, which GitHub already dropped; 650 is listed by the whole listing, then - dropped before its top-up, both with `expires_at` past GitHub's Date.""" - soon = LISTED_AT + timedelta(minutes=30) + dropped before its top-up, all three expiring `lead` past GitHub's + Date.""" + soon = LISTED_AT + lead old = self.base_at - timedelta(days=2) dropped = [ dated("pr-merge-gate-cursor", 3, i, old, expires=soon) for i in (600, 610) @@ -4401,9 +4749,27 @@ def drops_during_the_listing() -> list[dict[str, Any]]: fake.listed = drops_during_the_listing # type: ignore[method-assign] return fake - def test_a_count_github_drops_early_takes_one_whole_listing_a_job(self) -> None: + def test_a_count_github_drops_early_is_settled_by_the_pages_near_expiry( + self, + ) -> None: + # 600 and 610 expire within EARLY_DELETION_LEAD, so the pages around + # them decide them: no whole listing, nothing noted for the job. fake = self.skewed() reader, evidence = self.judge(fake) + self.assertIsNone(evidence.refusal, evidence.detail) + self.assertEqual(reader.listing_mode, "incremental") + self.assertEqual(fake.full_listings(), 0) + self.assertFalse(self.failed.exists()) + ids = [a["id"] for a in json.loads(self.saved.read_text())["artifacts"]] + self.assertEqual(ids, [1100, 1000, 900, 800, 700, 650]) + + def test_a_count_the_pages_near_expiry_miss_takes_one_whole_listing_a_job( + self, + ) -> None: + # Dropped three hours before expiry, past EARLY_DELETION_LEAD: no page + # near expiry is read, and the over-count goes to one whole listing. + fake = self.skewed(timedelta(hours=3)) + reader, evidence = self.judge(fake) self.assertEqual(evidence.refusal, "verify:unreadable") self.assertIn("held more artifacts than", evidence.detail or "") self.assertEqual(reader.listing_mode, "full") @@ -4495,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. @@ -4810,7 +5206,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` - or `LISTING_TOP_UP_PAGES` 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 @@ -4847,14 +5244,16 @@ 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 + + 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,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,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 ab2aeee6d7b..fdf78a442df 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, 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) = 1623, so 1623 x 1 + 0.1 x min(limit, 1000) = 1723 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 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 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 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 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) = 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 + 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: 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: 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"