Merge pull request #752 from Kobzol/github-resiliency

Do not trust GitHub's PR state after the PR has been already merged

Author: Kobzol

…27fbcf7a0940a76df84d6871c698e400c4c.json …ad091880bcfad13a93bc77a3513d59d9975.json.sqlx/query-27babf080874701c1035b6827734227fbcf7a0940a76df84d6871c698e400c4c.json renamed to .sqlx/query-24ff010e2ff0ba612d8d4fa8e68d9ad091880bcfad13a93bc77a3513d59d9975.json .sqlx/query-27babf080874701c1035b6827734227fbcf7a0940a76df84d6871c698e400c4c.json renamed to .sqlx/query-24ff010e2ff0ba612d8d4fa8e68d9ad091880bcfad13a93bc77a3513d59d9975.json

@@ -1900,4 +1900,32 @@ also include this pls"
         })
         .await;
     }
+
+    #[sqlx::test]
+    async fn github_reopens_merged_pr(pool: sqlx::PgPool) {
+        // Sometimes, GitHub does not notice that a pull request was actually merged, and it still
+        // claims that it is open.
+        // We should not ever reopen a PR that was already merged in our DB though, as that can
+        // break merge queue invariants.
+        // We thus simulate what happens when we run the PR refresh job while GitHub changes its
+        // mind.
+        // See https://github.com/rust-lang/rust/pull/154327 and
+        // https://rust-lang.zulipchat.com/#narrow/channel/242791-t-infra/topic/Bors.20posting.20success.20message.20a.20dozen.20times/with/593477072
+        run_test(pool, async |ctx: &mut BorsTester| {
+            ctx.approve(()).await?;
+            ctx.start_and_finish_auto_build(()).await?;
+
+            ctx.pr(()).await.expect_status(PullRequestStatus::Merged);
+
+            // Simulate GitHub thinking that the PR has been reopened
+            ctx.edit_pr((), |pr| pr.reopen()).await?;
+            // Refresh PRs, which must not reopen the PR in the DB
+            ctx.refresh_prs().await;
+
+            ctx.pr(()).await.expect_status(PullRequestStatus::Merged);
+
+            Ok(())
+        })
+        .await;
+    }
 }

src/bors/merge_queue.rs

@@ -151,7 +151,16 @@ pub(crate) async fn upsert_pull_request(
                             WHEN $9 = true THEN true
                             ELSE pull_request.mergeable_state_is_stale
                           END,
-                    status = $10
+                    -- The merged state is final, there is no going back from it.
+                    -- Sometimes, GitHub can return inconsistent data, and claim that a merged PR
+                    -- is open again.
+                    -- We do not ever want to revert a merged state in the DB, as that could
+                    -- break some invariants in the merge queue.
+                    status =
+                        CASE
+                            WHEN pull_request.status = 'merged' THEN pull_request.status
+                            ELSE $10
+                        END
                 RETURNING *
             )
             SELECT