From eb14f44fab9fcc6abf9b14fae8c49a21c5be0b88 Mon Sep 17 00:00:00 2001 From: gustrb Date: Tue, 4 Nov 2025 09:55:07 -0300 Subject: [PATCH 1/4] feat: Don't allow the merging of PRs when they don't have a clean state --- src/bors/comment.rs | 6 ++++++ src/bors/handlers/review.rs | 10 ++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/bors/comment.rs b/src/bors/comment.rs index fc3aa5fc..f73fc588 100644 --- a/src/bors/comment.rs +++ b/src/bors/comment.rs @@ -276,6 +276,12 @@ Hint: Remove **{keyword}** from this PR's title when it is ready for review. )) } +pub fn approve_non_clean_pr() -> Comment { + Comment::new(format!( + r":clipboard: Looks like this PR is not ready to be merged, ignoring approval." + )) +} + pub fn approve_blocking_labels_present(blocking_labels: &[&str]) -> Comment { let labels = blocking_labels .iter() diff --git a/src/bors/handlers/review.rs b/src/bors/handlers/review.rs index 497674e9..0564c481 100644 --- a/src/bors/handlers/review.rs +++ b/src/bors/handlers/review.rs @@ -4,8 +4,9 @@ use crate::bors::RepositoryState; use crate::bors::command::RollupMode; use crate::bors::command::{Approver, CommandPrefix}; use crate::bors::comment::{ - approve_blocking_labels_present, approve_non_open_pr_comment, approve_wip_title, - approved_comment, delegate_comment, delegate_try_builds_comment, unapprove_non_open_pr_comment, + approve_blocking_labels_present, approve_non_clean_pr, approve_non_open_pr_comment, + approve_wip_title, approved_comment, delegate_comment, delegate_try_builds_comment, + unapprove_non_open_pr_comment, }; use crate::bors::handlers::labels::handle_label_trigger; use crate::bors::handlers::workflow::{AutoBuildCancelReason, maybe_cancel_auto_build}; @@ -20,6 +21,7 @@ use crate::github::LabelTrigger; use crate::github::{GithubUser, PullRequestNumber}; use crate::permissions::PermissionType; use crate::{BorsContext, PgDbClient}; +use octocrab::models::pulls::MergeableState; /// Approve a pull request. /// A pull request can only be approved by a user of sufficient authority. @@ -81,6 +83,10 @@ async fn check_pr_approval_validity( return Ok(Some(approve_non_open_pr_comment())); } + if !matches!(pr.github.mergeable_state, MergeableState::Clean) { + return Ok(Some(approve_non_clean_pr())); + } + // Check WIP title let title = pr.github.title.to_lowercase(); if let Some(wip_kw) = WIP_KEYWORDS.iter().find(|kw| title.contains(*kw)) { From 51b61f6e96e74f854302a480f0dcaaecbf8085ed Mon Sep 17 00:00:00 2001 From: gustrb Date: Tue, 4 Nov 2025 10:34:48 -0300 Subject: [PATCH 2/4] chore: Check for matching in dirty PRs --- src/bors/handlers/review.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bors/handlers/review.rs b/src/bors/handlers/review.rs index 0564c481..2b8de52c 100644 --- a/src/bors/handlers/review.rs +++ b/src/bors/handlers/review.rs @@ -83,7 +83,7 @@ async fn check_pr_approval_validity( return Ok(Some(approve_non_open_pr_comment())); } - if !matches!(pr.github.mergeable_state, MergeableState::Clean) { + if matches!(pr.github.mergeable_state, MergeableState::Dirty) { return Ok(Some(approve_non_clean_pr())); } From 26a4e8c61bbf8ef76f966a3d7add25a02d171e47 Mon Sep 17 00:00:00 2001 From: gustrb Date: Tue, 4 Nov 2025 10:41:41 -0300 Subject: [PATCH 3/4] refactor: stop using useless format --- src/bors/comment.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/bors/comment.rs b/src/bors/comment.rs index f73fc588..faa87f21 100644 --- a/src/bors/comment.rs +++ b/src/bors/comment.rs @@ -277,9 +277,7 @@ Hint: Remove **{keyword}** from this PR's title when it is ready for review. } pub fn approve_non_clean_pr() -> Comment { - Comment::new(format!( - r":clipboard: Looks like this PR is not ready to be merged, ignoring approval." - )) + Comment::new(r":clipboard: Looks like this PR is not ready to be merged,.".to_string()) } pub fn approve_blocking_labels_present(blocking_labels: &[&str]) -> Comment { From d57b20632c1a05a983f799c032c7c3b13b5af3b2 Mon Sep 17 00:00:00 2001 From: gustrb Date: Tue, 4 Nov 2025 16:25:09 -0300 Subject: [PATCH 4/4] test: add unit tests --- src/bors/comment.rs | 4 +++- src/bors/handlers/review.rs | 14 ++++++++++++++ src/tests/mocks/pull_request.rs | 4 ++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/bors/comment.rs b/src/bors/comment.rs index faa87f21..eedd624a 100644 --- a/src/bors/comment.rs +++ b/src/bors/comment.rs @@ -277,7 +277,9 @@ Hint: Remove **{keyword}** from this PR's title when it is ready for review. } pub fn approve_non_clean_pr() -> Comment { - Comment::new(r":clipboard: Looks like this PR is not ready to be merged,.".to_string()) + Comment::new( + r":clipboard: Looks like this PR is not ready to be merged, ignoring approval.".to_string(), + ) } pub fn approve_blocking_labels_present(blocking_labels: &[&str]) -> Comment { diff --git a/src/bors/handlers/review.rs b/src/bors/handlers/review.rs index 2b8de52c..300675eb 100644 --- a/src/bors/handlers/review.rs +++ b/src/bors/handlers/review.rs @@ -1220,6 +1220,20 @@ mod tests { .await; } + #[sqlx::test] + async fn approve_dirty_pr(pool: sqlx::PgPool) { + run_test(pool, async |tester: &mut BorsTester| { + tester.edit_pr((), |pr| pr.dirty_pull_request()).await?; + tester.post_comment("@bors r+").await?; + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @r" + :clipboard: Looks like this PR is not ready to be merged, ignoring approval. + "); + tester.get_pr_copy(()).await.expect_unapproved(); + Ok(()) + }) + .await; + } + #[sqlx::test] async fn approve_closed_pr(pool: sqlx::PgPool) { run_test(pool, async |tester: &mut BorsTester| { diff --git a/src/tests/mocks/pull_request.rs b/src/tests/mocks/pull_request.rs index c75ac731..ee322144 100644 --- a/src/tests/mocks/pull_request.rs +++ b/src/tests/mocks/pull_request.rs @@ -206,6 +206,10 @@ impl PullRequest { self.status = PullRequestStatus::Draft; } + pub fn dirty_pull_request(&mut self) { + self.mergeable_state = MergeableState::Dirty; + } + pub fn add_comment_to_history(&mut self, comment: Comment) { self.comment_history.push(comment); }