Skip to content

Commit 04a2c13

Browse files
committed
Do not trust GitHub's PR state after the PR has been already merged
1 parent edf49e3 commit 04a2c13

2 files changed

Lines changed: 38 additions & 1 deletion

File tree

src/bors/merge_queue.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1900,4 +1900,32 @@ also include this pls"
19001900
})
19011901
.await;
19021902
}
1903+
1904+
#[sqlx::test]
1905+
async fn github_reopens_merged_pr(pool: sqlx::PgPool) {
1906+
// Sometimes, GitHub does not notice that a pull request was actually merged, and it still
1907+
// claims that it is open.
1908+
// We should not ever reopen a PR that was already merged in our DB though, as that can
1909+
// break merge queue invariants.
1910+
// We thus simulate what happens when we run the PR refresh job while GitHub changes its
1911+
// mind.
1912+
// See https://github.com/rust-lang/rust/pull/154327 and
1913+
// https://rust-lang.zulipchat.com/#narrow/channel/242791-t-infra/topic/Bors.20posting.20success.20message.20a.20dozen.20times/with/593477072
1914+
run_test(pool, async |ctx: &mut BorsTester| {
1915+
ctx.approve(()).await?;
1916+
ctx.start_and_finish_auto_build(()).await?;
1917+
1918+
ctx.pr(()).await.expect_status(PullRequestStatus::Merged);
1919+
1920+
// Simulate GitHub thinking that the PR has been reopened
1921+
ctx.edit_pr((), |pr| pr.reopen()).await?;
1922+
// Refresh PRs, which must not reopen the PR in the DB
1923+
ctx.refresh_prs().await;
1924+
1925+
ctx.pr(()).await.expect_status(PullRequestStatus::Merged);
1926+
1927+
Ok(())
1928+
})
1929+
.await;
1930+
}
19031931
}

src/database/operations.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,16 @@ pub(crate) async fn upsert_pull_request(
151151
WHEN $9 = true THEN true
152152
ELSE pull_request.mergeable_state_is_stale
153153
END,
154-
status = $10
154+
-- The merged state is final, there is no going back from it.
155+
-- Sometimes, GitHub can return inconsistent data, and claim that a merged PR
156+
-- is open again.
157+
-- We do not ever want to revert a merged state in the DB, as that could
158+
-- break some invariants in the merge queue.
159+
status =
160+
CASE
161+
WHEN pull_request.status = 'merged' THEN pull_request.status
162+
ELSE $10
163+
END
155164
RETURNING *
156165
)
157166
SELECT

0 commit comments

Comments
 (0)