Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changes/source-control/fix-force-push-read-pages-1473d48c.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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__":
Expand Down
Loading