From 75de431af362c2085e6930956efcf0ede235daec Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 09:34:52 -0400 Subject: [PATCH] fix(source-control): give the verify reader's artifact listing its own timeout The listing's pages share one timeout, gh's 60 s per-call default. At 7,522 artifacts (76 pages at 0.7 to 1 s each) a cold read ran out on page 66 and refused #10125 as verify:unreadable (run 40451037349). The listing now gets 300 s, and its 5xx retries still fit inside it. Co-Authored-By: Claude Opus 5.5 --- .../fix-verify-listing-budget-7d2e41c9.md | 7 +++ .../scripts/babysit_verify_evidence.py | 21 +++++--- .../tests/test_babysit_verify_evidence.py | 49 ++++++++++++++++--- 3 files changed, 63 insertions(+), 14 deletions(-) create mode 100644 .changes/source-control/fix-verify-listing-budget-7d2e41c9.md diff --git a/.changes/source-control/fix-verify-listing-budget-7d2e41c9.md b/.changes/source-control/fix-verify-listing-budget-7d2e41c9.md new file mode 100644 index 0000000000..b8ca417bfd --- /dev/null +++ b/.changes/source-control/fix-verify-listing-budget-7d2e41c9.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Fixed + +- `babysit_verify_evidence.py` reads the repository's artifact listing under its own 300 s timeout (`LISTING_TIMEOUT_SECONDS`) instead of gh's 60 s per-call default. The listing's pages share one timeout, and at 7,522 artifacts (76 pages at 0.7 to 1 s each) a cold read ran out on page 66 and refused the pull request as `verify:unreadable`. Retries after a 5xx still fit inside that one timeout. 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 f008e2f237..97af97f0e1 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 @@ -96,8 +96,8 @@ can only lack later uploads, so a refusal on it stands; before the gate's own candidate is allowed on it, babysit_merge.py calls `refresh()` and judges it again on a listing of its own. A listing that got a 5xx or a fast network -error is read again after each of `LISTING_WAITS`, every read inside one gh -timeout in all; any other failure, a timeout, a 429 or a rate-limit 403 +error is read again after each of `LISTING_WAITS`, every read inside +`LISTING_TIMEOUT_SECONDS` in all; any other failure, a timeout, a 429 or a rate-limit 403 included, is not. A failed listing is never saved, and is kept as well, so one outage refuses every entry of the gate run without paging again: it holds the whole queue rather than merging blind. `evidence --shared-listing` @@ -146,7 +146,6 @@ from babysit_gh import ( flatten_paginated_items, gh_http_status, - gh_timeout_seconds, pull_request_commits_once, ) from babysit_util import is_json_array, is_json_object, json_array, json_object @@ -215,6 +214,12 @@ def verdict_name(name: str) -> re.Match[str] | None: COMMITS_CAP = 250 # The waits before the second and third listing reads (judgment). LISTING_WAITS = (2.0, 4.0) +# The listing's pages share one timeout (babysit_gh._run_pages), so it grows +# with the artifact count, not gh's per-call default: 7,522 artifacts took 76 +# pages at 0.7 to 1 s a page on 2026-10-27, past that default's 60 s. 300 s +# reads about 300 pages at 1 s a page, inside every reader's step timeout +# (judgment). +LISTING_TIMEOUT_SECONDS = 300.0 # The oldest listing a judgment uses, saved or in memory (see the bound), # unless `evidence --shared-listing` lifts it (`Reader.listing_max_age`). LISTING_MAX_AGE = run_cache.MAX_AGE @@ -859,17 +864,17 @@ def _save(self, listing: list[dict[str, Any]], fetched_at: datetime) -> None: def _read_listing(self) -> Any: """The listing payload, read again after a 5xx or a fast network - error while time is left: every read and wait fits in the one gh - timeout a single read would get, so a retry adds no time to the run's - worst case.""" + error while time is left: every page, read and wait fits in + `LISTING_TIMEOUT_SECONDS`, so a retry adds no time to the run's worst + case.""" args = [ "api", f"repos/{self.repo}/actions/artifacts?per_page=100", "--paginate", "--slurp", ] - deadline = _listing_clock() + gh_timeout_seconds() - timeout: float | None = None + deadline = _listing_clock() + LISTING_TIMEOUT_SECONDS + timeout = LISTING_TIMEOUT_SECONDS for wait in LISTING_WAITS: try: return self._actions(args, "repository artifacts", timeout) 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 b47accfab8..477dcd6c1e 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 @@ -41,6 +41,7 @@ sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent)) sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent.parent)) +import babysit_gh as gh import babysit_merge as merge import babysit_merge_policy as policy import babysit_postmerge as postmerge @@ -2814,11 +2815,11 @@ def test_a_persistent_transient_failure_refuses_after_three_reads(self) -> None: self.assertEqual(sleeps, [2.0, 4.0]) def timed(self, clock: list[float]) -> FlakyListing: - """Three 502s, the clock reading `clock` in turn, a 60s gh timeout.""" + """Three 502s, the clock reading `clock` in turn, a 60s listing timeout.""" fake = FlakyListing(["gh: Bad Gateway (HTTP 502)"] * 3) with ( mock.patch.object(ve, "_listing_clock", side_effect=clock), - mock.patch.object(ve, "gh_timeout_seconds", return_value=60.0), + mock.patch.object(ve, "LISTING_TIMEOUT_SECONDS", 60.0), ): self.judge(fake) return fake @@ -2826,21 +2827,21 @@ def timed(self, clock: list[float]) -> FlakyListing: def test_the_retries_share_one_read_timeout(self) -> None: # Deadline 0 + 60; each retry gets what is left after its wait ends. fake = self.timed([0.0, 1.0, 3.0, 20.0, 24.0]) - self.assertEqual(fake.timeouts, [None, 60.0 - 3.0, 60.0 - 24.0]) + self.assertEqual(fake.timeouts, [60.0, 60.0 - 3.0, 60.0 - 24.0]) def test_a_late_sleep_shortens_the_retry_timeout(self) -> None: # The 2s sleep returned at 30s: the retry gets 30s, not 57s. fake = self.timed([0.0, 1.0, 30.0, 59.0]) - self.assertEqual(fake.timeouts, [None, 30.0]) + self.assertEqual(fake.timeouts, [60.0, 30.0]) def test_no_retry_when_the_sleep_ends_past_the_deadline(self) -> None: fake = self.timed([0.0, 1.0, 61.0]) - self.assertEqual(fake.timeouts, [None]) + self.assertEqual(fake.timeouts, [60.0]) self.assertEqual(fake.listings(), 1) def test_no_retry_once_the_timeout_is_spent(self) -> None: fake = self.timed([0.0, 58.5]) - self.assertEqual(fake.timeouts, [None]) + self.assertEqual(fake.timeouts, [60.0]) self.assertEqual(fake.listings(), 1) def test_any_other_failure_is_not_read_again(self) -> None: @@ -2853,6 +2854,41 @@ def test_any_other_failure_is_not_read_again(self) -> None: self.assertEqual(sleeps, []) +class AListingOfManyPagesReadsWhole(unittest.TestCase): + """The pages share one timeout (babysit_gh._run_pages): 76 pages at 1 s a + page, as ci-status read 7,522 artifacts on 2026-10-27, outlast gh's 60 s + default and must still read whole.""" + + PAGES = 76 + + def test_76_pages_at_one_second_each_read_whole(self) -> None: + clock = [0.0] + sent: list[str] = [] + + def send(args, *, timeout_seconds, env_overrides, binary): + self.assertGreater(timeout_seconds, 0) + clock[0] += 1.0 + sent.append(args[1]) + page = len(sent) + link = ( + f"; rel="next"' + if page < self.PAGES + else "" + ) + body = json.dumps({"total_count": 7522, "artifacts": [{"id": page}]}) + return subprocess.CompletedProcess(args, 0, body, ""), {"link": link} + + with ( + mock.patch.object(gh, "_send", side_effect=send), + mock.patch.object(gh.time, "monotonic", side_effect=lambda: clock[0]), + mock.patch.object(ve, "_listing_clock", side_effect=lambda: clock[0]), + ): + pages = ve.Reader(REPO, "main", now=NOW)._read_listing() + self.assertEqual(len(pages), self.PAGES) + self.assertEqual(len(sent), self.PAGES) + + # --- The --authority pr-merge path ------------------------------------------------ @@ -3506,6 +3542,7 @@ def config(self, value: str | None) -> policy.PipelineConfig: "version: 1\nmerge:\n rung: C2\n authority: pr-merge\n" " agent-identity: agent-lanes[bot]\n" + line ) + def file_at(owner: str, name: str, path: str, sha: str) -> str | None: return text if path == policy.CONFIG_PATH else None