upload: Query head oid of a PR via commits

The headRefOid field of a PR actually contains the oid of
whatever the tot of that branch name is, which may not be the
commit that is actually associated with the pr if it has been merged.
To get that correct, we have to query the actual commit list.

This handles a corner case in reordering PRs with the repo's merge
setting as rebase. Somehow this causes one of the reordered PRs to
get merged with 0 commmits, which later causes us to not re-make prs.

With this change, we'll always succeed in remaking prs if they are
erroneously closed or merged on a reorder. Avoiding having them get
closed is a follow-up change.

Topic: query1
Reviewers: aaron, brian-k

revup/github_utils.py

@@ -24,8 +24,8 @@ class PrInfo:
 
     baseRef: str
     headRef: str
-    baseRefOid: GitCommitHash
-    headRefOid: GitCommitHash
+    baseRefOid: Optional[GitCommitHash]
+    headRefOid: Optional[GitCommitHash]
     body: str
     title: str
     id: str = ""
@@ -181,12 +181,10 @@ async def query_everything(
                 state
                 url
                 baseRefName
-                headRefOid
                 body
                 title
                 isDraft
-                updatedAt
-                commits (first: 1) {{
+                baseCommit: commits(first: 1) {{
                     nodes {{
                         commit {{
                             parents (first: 1) {{
@@ -197,6 +195,13 @@ async def query_everything(
                         }}
                     }}
                 }}
+                headCommit: commits(last: 1) {{
+                    nodes {{
+                        commit {{
+                            oid
+                        }}
+                    }}
+                }}
                 reviewRequests (first: 25) {{
                     nodes {{
                         requestedReviewer {{
@@ -279,15 +284,20 @@ async def query_everything(
             for user in this_node["assignees"]["nodes"]:
                 assignees.add(user["login"])
                 assignee_ids.add(user["id"])
-            headRefOid = this_node["headRefOid"]
-            # Github's "baseRefOid" in the api field returns the ToT commit for the base ref
-            # which isn't what we want, since that commit may not exist locally. Instead
-            # we get the parent of the first commit, which is the base ref it was actually
-            # uploaded against.
+
+            # The plain headRef and baseRef fields return the latest commit id associated with
+            # that branch name which may be newer than the PR itself if it was merged. We want
+            # the ids of the commits actually last associated with the PR, which we query from
+            # the commit list. This can also mean they are None if the PR has 0 commits.
+            headRefOid = (
+                this_node["headCommit"]["nodes"][0]["commit"]["oid"]
+                if this_node["headCommit"]["nodes"]
+                else None
+            )
             baseRefOid = (
-                headRefOid
-                if not this_node["commits"]["nodes"]
-                else this_node["commits"]["nodes"][0]["commit"]["parents"]["nodes"][0]["oid"]
+                this_node["baseCommit"]["nodes"][0]["commit"]["parents"]["nodes"][0]["oid"]
+                if this_node["baseCommit"]["nodes"]
+                else None
             )
 
             comments = []

revup/topic_stack.py

@@ -328,6 +328,8 @@ async def create_patchsets_comment(
             or not self.repo_info
             or not self.fork_info
             or review.pr_info is None
+            or review.pr_info.headRefOid is None
+            or review.pr_info.baseRefOid is None
             or review.base_ref is None
         ):
             return None
@@ -748,6 +750,12 @@ async def mark_rebases(self, skip_rebase: bool) -> None:
                 # but they are both corner cases and the worst that could happen is we fail to
                 # recreate the pr (but warn anyway).
                 review.status = PrStatus.NEW
+                review.pr_info = None
+
+            if review.pr_info and (not review.pr_info.baseRefOid or not review.pr_info.headRefOid):
+                logging.warning(f"Branch {review.remote_head} was merged but has no commits!")
+                review.status = PrStatus.NEW
+                review.pr_info = None
 
             if review.pr_info is not None and review.remote_base != review.pr_info.baseRef:
                 logging.debug(
@@ -766,6 +774,9 @@ async def mark_rebases(self, skip_rebase: bool) -> None:
                 # This is a new pr, no need to check patch ids
                 review.is_pure_rebase = False
             else:
+                assert (
+                    review.pr_info.baseRefOid is not None and review.pr_info.headRefOid is not None
+                )
                 if not topic.patch_ids:
                     # Lazily load patch ids for the topic.
                     topic.patch_ids = await asyncio.gather(
@@ -993,8 +1004,10 @@ async def fetch_git_refs(self) -> None:
 
         to_fetch = set()
         for _, _, _, review in self.all_reviews_iter():
-            if review.pr_info is not None and not await self.git_ctx.is_branch_or_commit(
-                review.pr_info.headRefOid
+            if (
+                review.pr_info is not None
+                and review.pr_info.headRefOid is not None
+                and not await self.git_ctx.is_branch_or_commit(review.pr_info.headRefOid)
             ):
                 to_fetch.add(review.pr_info.headRefOid)