From bbd3d99a5f59a762bf548616a6e1eafcd15885b9 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 1 Nov 2026 00:51:47 -0400 Subject: [PATCH] fix(source-control): page the merge gate's force-push read _force_pushed_heads read one page of 100 force-push events and raised whenever totalCount was larger, so a pull request rebased more than 100 times failed its risk gate on every run and could never arm. It now follows the cursor up to 10 pages and still raises on a missing cursor, a count the pages do not reach, or a listing past the cap. Co-Authored-By: Claude Opus 5.5 --- .../fix-force-push-read-pages-1473d48c.md | 7 +++ .../babysit-prs/scripts/babysit_merge.py | 58 ++++++++++++------ .../scripts/tests/test_babysit_merge.py | 59 +++++++++++++++---- 3 files changed, 94 insertions(+), 30 deletions(-) create mode 100644 .changes/source-control/fix-force-push-read-pages-1473d48c.md diff --git a/.changes/source-control/fix-force-push-read-pages-1473d48c.md b/.changes/source-control/fix-force-push-read-pages-1473d48c.md new file mode 100644 index 00000000000..e4e83f235b0 --- /dev/null +++ b/.changes/source-control/fix-force-push-read-pages-1473d48c.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Fixed + +- The babysit merge gate reads a pull request's force pushes 100 at a time up to 1,000, so a pull request rebased more than 100 times no longer fails its risk gate with `force pushes: unread or past one page` on every run and never arms. A listing past 1,000, a missing cursor or a count the pages do not reach still raises. diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py index 1207ab918b3..5ce6abba42a 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py @@ -3337,19 +3337,26 @@ def _main_reviews( _FORCE_PUSHES_QUERY = ( - "query($o:String!,$r:String!,$n:Int!){repository(owner:$o,name:$r){" - "pullRequest(number:$n){timelineItems(first:100,itemTypes:[HEAD_REF_FORCE_PUSHED_EVENT])" - "{totalCount nodes{... on HeadRefForcePushedEvent{beforeCommit{oid}}}}}}}" + "query($o:String!,$r:String!,$n:Int!,$c:String){repository(owner:$o,name:$r){" + "pullRequest(number:$n){timelineItems(first:100,after:$c,itemTypes:[HEAD_REF_FORCE_PUSHED_EVENT])" + "{totalCount pageInfo{hasNextPage endCursor}" + "nodes{... on HeadRefForcePushedEvent{beforeCommit{oid}}}}}}}" ) +# A pull request rebased more often than this is past what the gate reads. +_FORCE_PUSH_PAGES = 10 def _force_pushed_heads(repo: str, number: int) -> list[str]: """The heads force pushes replaced, which the commits list no longer - names but whose recorded counts still hold. Any doubt raises, so the - evidence is unbound rather than missing a count.""" + names but whose recorded counts still hold, read a page of 100 at a time + up to _FORCE_PUSH_PAGES. Any doubt raises, so the evidence is unbound + rather than missing a count.""" owner, name = repo.split("/", 1) - data = gh_json_graphql( - [ + nodes: list[Any] = [] + cursor = None + total = None + for _ in range(_FORCE_PUSH_PAGES): + args = [ "api", "graphql", "-f", @@ -3360,19 +3367,32 @@ def _force_pushed_heads(repo: str, number: int) -> list[str]: f"r={name}", "-F", f"n={number}", - ], - label=f"#{number} force pushes", - ) - items = json_object( - json_object( + ] + if cursor is not None: + args += ["-f", f"c={cursor}"] + data = gh_json_graphql(args, label=f"#{number} force pushes") + items = json_object( json_object( - json_object(json_object(data).get("data")).get("repository") - ).get("pullRequest") - ).get("timelineItems") - ) - nodes = items.get("nodes") - if not is_json_array(nodes) or items.get("totalCount") != len(nodes): - raise RuntimeError(f"#{number} force pushes: unread or past one page") + json_object( + json_object(json_object(data).get("data")).get("repository") + ).get("pullRequest") + ).get("timelineItems") + ) + page = items.get("nodes") + info = json_object(items.get("pageInfo")) + if not is_json_array(page): + raise RuntimeError(f"#{number} force pushes: unread") + total = items.get("totalCount") + nodes.extend(page) + if info.get("hasNextPage") is not True: + break + cursor = info.get("endCursor") + if not isinstance(cursor, str) or not cursor: + raise RuntimeError(f"#{number} force pushes: unread") + else: + raise RuntimeError(f"#{number} force pushes: past {_FORCE_PUSH_PAGES} pages") + if total != len(nodes): + raise RuntimeError(f"#{number} force pushes: unread or incomplete") heads = [] for node in nodes: oid = json_object(json_object(node).get("beforeCommit")).get("oid") diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge.py index 5030cb12abe..376ac940746 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge.py @@ -2502,7 +2502,14 @@ def collect(repo, names, workflow, branch, env): ), ): reviews, context = merge._main_reviews( - "owner/repo", 5, HEAD, changed, None, None, None, [HEAD], + "owner/repo", + 5, + HEAD, + changed, + None, + None, + None, + [HEAD], floor=merge.babysit_risk.floor_for(), ) calls.extend(read.call_args_list) @@ -2620,32 +2627,62 @@ def test_any_other_pull_request_lists_every_lane(self) -> None: class ForcePushedHeadsTests(unittest.TestCase): """The heads force pushes replaced, whose recorded review counts still - hold; a listing that is not whole raises.""" + hold, read across pages; a listing that is not whole raises.""" - def heads(self, total: int, oids: list[Any]) -> list[str]: + @staticmethod + def page(total: int, oids: list[Any], cursor: str | None = None) -> dict: nodes = [ {"beforeCommit": None if oid is None else {"oid": oid}} for oid in oids ] - data = { + info = {"hasNextPage": cursor is not None, "endCursor": cursor} + return { "data": { "repository": { "pullRequest": { - "timelineItems": {"totalCount": total, "nodes": nodes} + "timelineItems": { + "totalCount": total, + "pageInfo": info, + "nodes": nodes, + } } } } } - with mock.patch.object(merge, "gh_json_graphql", return_value=data): - return merge._force_pushed_heads("owner/repo", 5) + + def heads(self, *pages: dict) -> tuple[list[str], mock.MagicMock]: + with mock.patch.object( + merge, "gh_json_graphql", side_effect=list(pages) + ) as read: + return merge._force_pushed_heads("owner/repo", 5), read def test_names_each_replaced_head(self) -> None: - self.assertEqual( - self.heads(3, ["a" * 40, None, "b" * 40]), ["a" * 40, "b" * 40] + heads, _ = self.heads(self.page(3, ["a" * 40, None, "b" * 40])) + self.assertEqual(heads, ["a" * 40, "b" * 40]) + + def test_reads_every_page_by_its_cursor(self) -> None: + heads, read = self.heads( + self.page(101, ["a" * 40] * 100, cursor="next"), + self.page(101, ["b" * 40]), ) + self.assertEqual(heads, ["a" * 40] * 100 + ["b" * 40]) + self.assertNotIn("c=next", read.call_args_list[0].args[0]) + self.assertIn("c=next", read.call_args_list[1].args[0]) + + def test_a_count_the_pages_do_not_reach_raises(self) -> None: + with self.assertRaises(RuntimeError): + self.heads(self.page(101, ["a" * 40])) + + def test_a_next_page_with_no_cursor_raises(self) -> None: + with self.assertRaises(RuntimeError): + self.heads(self.page(200, ["a" * 40], cursor="")) - def test_a_listing_past_one_page_raises(self) -> None: + def test_a_listing_past_the_page_cap_raises(self) -> None: + pages = [ + self.page(9999, ["a" * 40], cursor=f"c{i}") + for i in range(merge._FORCE_PUSH_PAGES) + ] with self.assertRaises(RuntimeError): - self.heads(101, ["a" * 40]) + self.heads(*pages) if __name__ == "__main__":