diff --git a/.sqlx/query-27babf080874701c1035b6827734227fbcf7a0940a76df84d6871c698e400c4c.json b/.sqlx/query-24ff010e2ff0ba612d8d4fa8e68d9ad091880bcfad13a93bc77a3513d59d9975.json similarity index 75% rename from .sqlx/query-27babf080874701c1035b6827734227fbcf7a0940a76df84d6871c698e400c4c.json rename to .sqlx/query-24ff010e2ff0ba612d8d4fa8e68d9ad091880bcfad13a93bc77a3513d59d9975.json index e0a31448..ac6eb8dd 100644 --- a/.sqlx/query-27babf080874701c1035b6827734227fbcf7a0940a76df84d6871c698e400c4c.json +++ b/.sqlx/query-24ff010e2ff0ba612d8d4fa8e68d9ad091880bcfad13a93bc77a3513d59d9975.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n WITH upserted_pr AS (\n INSERT INTO pull_request (\n repository,\n number,\n title,\n author,\n assignees,\n head_branch,\n base_branch,\n mergeable_state,\n mergeable_state_is_stale,\n status\n )\n VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)\n ON CONFLICT (repository, number)\n DO UPDATE SET\n title = $3,\n author = $4,\n assignees = $5,\n head_branch = $6,\n base_branch = $7,\n -- Note that we do NOT update mergeable_state here, it is updated explicitly elsewhere,\n -- to properly handle unmergeability notifications.\n mergeable_state_is_stale =\n -- Only set the stale flag here, but do not clear it!\n -- This is important for the mergeability queue to work properly, so that we can\n -- detect whether something is stale or not.\n CASE\n WHEN $9 = true THEN true\n ELSE pull_request.mergeable_state_is_stale\n END,\n status = $10\n RETURNING *\n )\n SELECT\n pr.id,\n pr.repository as \"repository: GithubRepoName\",\n pr.number as \"number!: i64\",\n pr.title,\n pr.author,\n pr.assignees as \"assignees: Assignees\",\n (\n pr.approved_by,\n pr.approved_sha\n ) AS \"approval_status!: ApprovalStatus\",\n pr.status as \"status: PullRequestStatus\",\n pr.priority,\n pr.rollup as \"rollup: RollupMode\",\n pr.delegated_permission as \"delegated_permission: DelegatedPermission\",\n pr.head_branch,\n pr.base_branch,\n pr.mergeable_state as \"mergeable_state: MergeableState\",\n pr.mergeable_state_is_stale,\n pr.created_at as \"created_at: DateTime\",\n try_build AS \"try_build: BuildModel\",\n auto_build AS \"auto_build: BuildModel\"\n FROM upserted_pr as pr\n LEFT JOIN build AS try_build ON pr.try_build_id = try_build.id\n LEFT JOIN build AS auto_build ON pr.auto_build_id = auto_build.id\n ", + "query": "\n WITH upserted_pr AS (\n INSERT INTO pull_request (\n repository,\n number,\n title,\n author,\n assignees,\n head_branch,\n base_branch,\n mergeable_state,\n mergeable_state_is_stale,\n status\n )\n VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)\n ON CONFLICT (repository, number)\n DO UPDATE SET\n title = $3,\n author = $4,\n assignees = $5,\n head_branch = $6,\n base_branch = $7,\n -- Note that we do NOT update mergeable_state here, it is updated explicitly elsewhere,\n -- to properly handle unmergeability notifications.\n mergeable_state_is_stale =\n -- Only set the stale flag here, but do not clear it!\n -- This is important for the mergeability queue to work properly, so that we can\n -- detect whether something is stale or not.\n CASE\n WHEN $9 = true THEN true\n ELSE pull_request.mergeable_state_is_stale\n END,\n -- The merged state is final, there is no going back from it.\n -- Sometimes, GitHub can return inconsistent data, and claim that a merged PR\n -- is open again.\n -- We do not ever want to revert a merged state in the DB, as that could\n -- break some invariants in the merge queue.\n status =\n CASE\n WHEN pull_request.status = 'merged' THEN pull_request.status\n ELSE $10\n END\n RETURNING *\n )\n SELECT\n pr.id,\n pr.repository as \"repository: GithubRepoName\",\n pr.number as \"number!: i64\",\n pr.title,\n pr.author,\n pr.assignees as \"assignees: Assignees\",\n (\n pr.approved_by,\n pr.approved_sha\n ) AS \"approval_status!: ApprovalStatus\",\n pr.status as \"status: PullRequestStatus\",\n pr.priority,\n pr.rollup as \"rollup: RollupMode\",\n pr.delegated_permission as \"delegated_permission: DelegatedPermission\",\n pr.head_branch,\n pr.base_branch,\n pr.mergeable_state as \"mergeable_state: MergeableState\",\n pr.mergeable_state_is_stale,\n pr.created_at as \"created_at: DateTime\",\n try_build AS \"try_build: BuildModel\",\n auto_build AS \"auto_build: BuildModel\"\n FROM upserted_pr as pr\n LEFT JOIN build AS try_build ON pr.try_build_id = try_build.id\n LEFT JOIN build AS auto_build ON pr.auto_build_id = auto_build.id\n ", "describe": { "columns": [ { @@ -225,5 +225,5 @@ true ] }, - "hash": "27babf080874701c1035b6827734227fbcf7a0940a76df84d6871c698e400c4c" + "hash": "24ff010e2ff0ba612d8d4fa8e68d9ad091880bcfad13a93bc77a3513d59d9975" } diff --git a/src/bors/merge_queue.rs b/src/bors/merge_queue.rs index 18946524..60824255 100644 --- a/src/bors/merge_queue.rs +++ b/src/bors/merge_queue.rs @@ -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; + } } diff --git a/src/database/operations.rs b/src/database/operations.rs index 702e2e15..87f95da9 100644 --- a/src/database/operations.rs +++ b/src/database/operations.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