From 1abf65d105ecb26f3ad5463a0bad103a18aa3627 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 29 Oct 2026 04:41:10 +0000 Subject: [PATCH] merge_freeze (main): read lists one bounded page per gh call, not --paginate Port of feat-cmux-next's #26647 to main. The agents' gh quota shim refuses `gh api --paginate` (it can't be charged), so gh-merge-green's freeze read exited 2 and refused every merge into main from an agent session. gh_items now asks for per_page=100&page=N, one call each, stops at a short page and fails closed past 30 pages. Co-Authored-By: Claude Opus 5.5 Agent-Session: https://cmux.com/deeplink/agent?id=aid-7a592ef92c66f080e3cc --- scripts/ci/merge_freeze.py | 33 ++++++++++++++++------ tests/test_merge_freeze.py | 57 +++++++++++++++++++++++++++++++++----- 2 files changed, 75 insertions(+), 15 deletions(-) diff --git a/scripts/ci/merge_freeze.py b/scripts/ci/merge_freeze.py index ed9eb51a80e4..b4706d1e0f63 100644 --- a/scripts/ci/merge_freeze.py +++ b/scripts/ci/merge_freeze.py @@ -154,10 +154,27 @@ def gh(*args: str, write: bool = False) -> str: return subprocess.run(["gh", *args], check=True, capture_output=True, text=True, env=env).stdout -def gh_items(endpoint: str) -> list[dict]: - """Every item of a paginated list endpoint (one JSON object per line: older gh has no --slurp).""" - out = gh("api", "--paginate", endpoint, "--jq", ".[] | @json") - return [json.loads(line) for line in out.splitlines() if line.strip()] +# One `gh api` call per page (see gh_items); a list longer than MAX_PAGES pages fails closed. +PER_PAGE = 100 +MAX_PAGES = 30 + + +def gh_items(endpoint: str, max_pages: int = MAX_PAGES) -> list[dict]: + """Every item of a paginated list endpoint, one page per `gh api` call. Not `gh api --paginate`: + that is one invocation making any number of requests, which the gh quota shim can't charge and + refuses, and a refused read left every merge with an unknown freeze state. A list longer than + max_pages pages raises (status 2): a list cut short never reads as "no freeze".""" + sep = "&" if "?" in endpoint else "?" + items: list[dict] = [] + for page in range(1, max_pages + 1): + batch = json.loads(gh("api", f"{endpoint}{sep}per_page={PER_PAGE}&page={page}")) + if not isinstance(batch, list): + raise TypeError(f"{endpoint} page {page} is not a list") + items.extend(batch) + if len(batch) < PER_PAGE: + return items + raise ValueError(f"{endpoint} has more than {max_pages} pages of {PER_PAGE}; " + "refusing to read a list cut short as complete") def read_json_list(path: str) -> list[dict]: @@ -240,7 +257,7 @@ def open_freezes(repo: str, issues_file: str | None = None, if i.get("state", "open") == "open" and LABEL in {label.get("name") for label in i.get("labels", [])}] else: - issues = gh_items(f"repos/{repo}/issues?state=open&labels={LABEL}&per_page=100") + issues = gh_items(f"repos/{repo}/issues?state=open&labels={LABEL}") freezes = [] for issue in issues: if "pull_request" in issue: @@ -254,7 +271,7 @@ def open_freezes(repo: str, issues_file: str | None = None, labeled_at, edited_at = issue.get("labeled_at"), issue.get("last_edited_at") else: labeler, labeled_at = last_labeler( - gh_items(f"repos/{repo}/issues/{freeze.number}/events?per_page=100"), LABEL) + gh_items(f"repos/{repo}/issues/{freeze.number}/events"), LABEL) role = repo_role(repo, labeler) if labeler else None edited_at = last_edited(repo, freeze.number) check_authority(freeze, labeler, role, labeled_at, edited_at) @@ -361,7 +378,7 @@ def main(argv: list[str] | None = None) -> int: files = [line.strip() for line in handle if line.strip()] else: # Both ends of a rename: moving a file out of a frozen directory touches it too. - entries = gh_items(f"repos/{args.repo}/pulls/{args.pr}/files?per_page=100") + entries = gh_items(f"repos/{args.repo}/pulls/{args.pr}/files") files = [e["filename"] for e in entries] + [e["previous_filename"] for e in entries if e.get("previous_filename")] # The files API stops at 3000 files; an incomplete list can't clear a freeze. @@ -372,7 +389,7 @@ def main(argv: list[str] | None = None) -> int: raise ValueError(f"GitHub listed {len(entries)} of {expected} changed files") if any(applies(f, args.base, now) for f in freezes): events = (read_json_list(args.events_file) if args.events_file is not None - else gh_items(f"repos/{args.repo}/issues/{args.pr}/events?per_page=100")) + else gh_items(f"repos/{args.repo}/issues/{args.pr}/events")) labeler = ok_labeler(events) # TypeError, AttributeError: GitHub answered with an unexpected shape (a null issue or a missing # timestamp, say). Still unknown, not frozen. diff --git a/tests/test_merge_freeze.py b/tests/test_merge_freeze.py index 2cfb19e8179d..077b09ebba37 100644 --- a/tests/test_merge_freeze.py +++ b/tests/test_merge_freeze.py @@ -218,8 +218,6 @@ def __call__(self, args, check=False, capture_output=False, text=False, env=None raise subprocess.CalledProcessError(1, args, "", "gh: API rate limit exceeded for installation (HTTP 403)") for prefix, value in self.responses.items(): if endpoint.startswith(prefix): - if "--jq" in args: # gh_items: one JSON object per line - return subprocess.CompletedProcess(args, 0, "".join(json.dumps(v) + "\n" for v in value), "") return subprocess.CompletedProcess(args, 0, json.dumps(value), "") raise AssertionError(f"unexpected gh call {args}") @@ -240,7 +238,7 @@ def test_no_open_freeze_reads_only_the_freeze_list(self): fake = FakeGh({self.ISSUES: []}) status, _, calls = self.run_check(fake, "--changed-files", "3") self.assertEqual(status, 0) - self.assertEqual(calls, [f"{self.ISSUES}&per_page=100"]) # was: + pulls/42/files and pulls/42 + self.assertEqual(calls, [f"{self.ISSUES}&per_page=100&page=1"]) # was: + pulls/42/files and pulls/42 def test_a_freeze_on_another_branch_reads_no_pull_request(self): frozen = {**issue(body=body(extra="branch: main")), "state": "open"} @@ -251,13 +249,13 @@ def test_a_freeze_on_another_branch_reads_no_pull_request(self): self.assertEqual(status, 0) self.assertFalse([c for c in calls if "/pulls/" in c or "/issues/42/" in c], calls) # A freeze that cannot apply to this base costs no authority reads either (GITHUB_TOKEN budget). - self.assertEqual(calls, [f"{self.ISSUES}&per_page=100"]) + self.assertEqual(calls, [f"{self.ISSUES}&per_page=100&page=1"]) def test_a_lapsed_freeze_reads_only_the_freeze_list(self): lapsed = {**issue(body=body(expires="2020-01-01T00:00Z")), "state": "open"} status, _, calls = self.run_check(FakeGh({self.ISSUES: [lapsed]})) self.assertEqual(status, 0) - self.assertEqual(calls, [f"{self.ISSUES}&per_page=100"]) + self.assertEqual(calls, [f"{self.ISSUES}&per_page=100&page=1"]) def applicable(self, files): return FakeGh({ @@ -275,7 +273,7 @@ def test_an_applicable_freeze_still_blocks_using_the_events_file_count(self): status, out, calls = self.run_check(fake, "--changed-files", "1") self.assertEqual(status, 1) self.assertIn("#7", out) - self.assertIn(f"repos/{self.REPO}/pulls/42/files?per_page=100", calls) + self.assertIn(f"repos/{self.REPO}/pulls/42/files?per_page=100&page=1", calls) self.assertNotIn(f"repos/{self.REPO}/pulls/42", calls) # the count came from the event def test_a_cut_short_file_list_is_still_status_2(self): @@ -328,7 +326,7 @@ def gh(*args): calls.append(args) if len(calls) <= failures: raise freeze.subprocess.CalledProcessError(1, ["gh", *args], stderr=error) - return "" + return "[]" original_gh, original_sleep = freeze.gh, freeze.time.sleep freeze.gh, freeze.time.sleep = gh, sleeps.append @@ -368,5 +366,50 @@ def test_without_a_retry_a_rate_limited_read_is_status_3_at_once(self): status, sleeps, _ = self.rate_limited_check(failures=1, retry="0") self.assertEqual((status, sleeps), (3, [])) +class PaginationTests(unittest.TestCase): + """The gh quota shim refuses `gh api --paginate` (it can't be charged), so lists are read one + page per call, with an explicit per_page and a page cap that fails closed.""" + + def fake_gh(self, total): + calls = [] + + def gh(*args): + calls.append(args) + self.assertNotIn("--paginate", args) + query = dict(part.split("=", 1) for part in args[-1].split("?", 1)[1].split("&")) + self.assertEqual(query["per_page"], str(freeze.PER_PAGE)) + start = (int(query["page"]) - 1) * freeze.PER_PAGE + return json.dumps([{"filename": f"f{i}"} for i in range(start, min(total, start + freeze.PER_PAGE))]) + return gh, calls + + def with_gh(self, gh): + original = freeze.gh + freeze.gh = gh + self.addCleanup(setattr, freeze, "gh", original) + + def test_each_page_is_one_bounded_call_and_a_short_page_ends_the_list(self): + gh, calls = self.fake_gh(250) + self.with_gh(gh) + items = freeze.gh_items("repos/x/y/pulls/1/files") + self.assertEqual(len(items), 250) + self.assertEqual([c[1] for c in calls], [f"repos/x/y/pulls/1/files?per_page=100&page={n}" for n in (1, 2, 3)]) + gh, calls = self.fake_gh(3) + self.with_gh(gh) + freeze.gh_items("repos/x/y/issues?state=open&labels=merge-freeze") + self.assertEqual(calls[0][1], "repos/x/y/issues?state=open&labels=merge-freeze&per_page=100&page=1") + + def test_a_list_past_the_page_cap_fails_closed(self): + gh, calls = self.fake_gh(freeze.PER_PAGE * freeze.MAX_PAGES) + self.with_gh(gh) + with self.assertRaisesRegex(ValueError, "more than 30 pages"): + freeze.gh_items("repos/x/y/pulls/1/files") + self.assertEqual(len(calls), freeze.MAX_PAGES) + + def test_the_script_never_asks_gh_to_paginate(self): + source = (ROOT / "scripts/ci/merge_freeze.py").read_text() + self.assertNotIn('"--paginate"', source) + + + if __name__ == "__main__": unittest.main()