Merge pull request #428 from Sakib25800/retry-cmd

Add `@bors retry`

Author: Kobzol

.sqlx/query-aa9f3483df0d2ab722603300368e19a9ae88785203c49bc4622a704d9bef220d.json

@@ -129,4 +129,7 @@ pub enum BorsCommand {
     OpenTree,
     /// Set the tree closed with a priority level.
     TreeClosed(Priority),
+    /// Clear a failed auto build status from an approved PR.
+    /// This will cause the merge queue to attempt to start a new auto build and retry merging the PR again.
+    Retry,
 }

src/bors/command/mod.rs

@@ -142,6 +142,7 @@ const DEFAULT_PARSERS: &[ParserFn] = &[
     parser_info,
     parser_help,
     parser_ping,
+    parser_retry,
     parser_tree_ops,
 ];
 
@@ -168,7 +169,7 @@ fn parse_command(input: &str, parsers: &[ParserFn]) -> ParseResult {
     }
 }
 
-fn parse_parts(input: &str) -> Result<Vec<CommandPart>, CommandParseError> {
+fn parse_parts(input: &str) -> Result<Vec<CommandPart<'_>>, CommandParseError> {
     let mut parts = vec![];
     let mut seen_keys = HashSet::new();
 
@@ -411,6 +412,15 @@ fn parser_info(command: &CommandPart<'_>, _parts: &[CommandPart<'_>]) -> ParseRe
     }
 }
 
+/// Parses `@bors retry`
+fn parser_retry(command: &CommandPart<'_>, _parts: &[CommandPart<'_>]) -> ParseResult {
+    if let CommandPart::Bare("retry") = command {
+        Some(Ok(BorsCommand::Retry))
+    } else {
+        None
+    }
+}
+
 /// Parses `@bors treeclosed-`, `@bors treeopen` and `@bors treeclosed=<priority>`
 fn parser_tree_ops(command: &CommandPart<'_>, _parts: &[CommandPart<'_>]) -> ParseResult {
     match command {
@@ -1195,6 +1205,20 @@ for the crater",
         assert!(matches!(cmds[0], Ok(BorsCommand::Info)));
     }
 
+    #[test]
+    fn parse_retry() {
+        let cmds = parse_commands("@bors retry");
+        assert_eq!(cmds.len(), 1);
+        assert!(matches!(cmds[0], Ok(BorsCommand::Retry)));
+    }
+
+    #[test]
+    fn parse_retry_unknown_arg() {
+        let cmds = parse_commands("@bors retry xyz");
+        assert_eq!(cmds.len(), 1);
+        assert!(matches!(cmds[0], Ok(BorsCommand::Retry)));
+    }
+
     #[test]
     fn parse_try_cancel() {
         let cmds = parse_commands("@bors try cancel");

src/bors/command/parser.rs

@@ -39,6 +39,7 @@ fn format_help() -> &'static str {
         BorsCommand::SetRollupMode(_) => {}
         BorsCommand::OpenTree => {}
         BorsCommand::TreeClosed(_) => {}
+        BorsCommand::Retry => {}
     }
 
     r#"
@@ -65,6 +66,7 @@ You can use the following commands:
     - Optionally, you can specify a `<parent>` SHA with which will the PR be merged. You can specify `parent=last` to use the same parent SHA as the previous try build.
     - Optionally, you can select a comma-separated list of CI `<jobs>` to run in the try build.
 - `try cancel`: Cancel a running try build
+- `retry`: Clear a failed auto build status from an approved PR. This will cause the merge queue to attempt to start a new auto build and retry merging the PR again.
 - `info`: Get information about the current PR
 
 ## Repository management
@@ -109,6 +111,7 @@ mod tests {
                 - Optionally, you can specify a `<parent>` SHA with which will the PR be merged. You can specify `parent=last` to use the same parent SHA as the previous try build.
                 - Optionally, you can select a comma-separated list of CI `<jobs>` to run in the try build.
             - `try cancel`: Cancel a running try build
+            - `retry`: Clear a failed auto build status from an approved PR. This will cause the merge queue to attempt to start a new auto build and retry merging the PR again.
             - `info`: Get information about the current PR
 
             ## Repository management

src/bors/handlers/help.rs

@@ -15,6 +15,7 @@ use crate::bors::handlers::refresh::{
     refresh_pending_builds, reload_mergeability_status, reload_repository_config,
     reload_repository_permissions,
 };
+use crate::bors::handlers::retry::command_retry;
 use crate::bors::handlers::review::{
     command_approve, command_close_tree, command_open_tree, command_unapprove,
 };
@@ -44,6 +45,7 @@ mod labels;
 mod ping;
 mod pr_events;
 mod refresh;
+mod retry;
 mod review;
 mod trybuild;
 mod workflow;
@@ -393,6 +395,12 @@ async fn handle_comment(
                 let repo = Arc::clone(&repo);
                 let database = Arc::clone(&database);
                 let result = match command {
+                    BorsCommand::Retry => {
+                        let span = tracing::info_span!("Retry");
+                        command_retry(repo, database, pr, &comment.author, &merge_queue_tx)
+                            .instrument(span)
+                            .await
+                    }
                     BorsCommand::Approve {
                         approver,
                         priority,

src/bors/handlers/mod.rs

@@ -0,0 +1,99 @@
+use std::sync::Arc;
+
+use crate::PgDbClient;
+use crate::bors::handlers::{PullRequestData, deny_request, has_permission};
+use crate::bors::merge_queue::MergeQueueSender;
+use crate::bors::{Comment, RepositoryState};
+use crate::github::{GithubUser, PullRequestNumber};
+use crate::permissions::PermissionType;
+
+pub(super) async fn command_retry(
+    repo_state: Arc<RepositoryState>,
+    db: Arc<PgDbClient>,
+    pr: PullRequestData<'_>,
+    author: &GithubUser,
+    merge_queue_tx: &MergeQueueSender,
+) -> anyhow::Result<()> {
+    if !has_permission(&repo_state, author, pr, PermissionType::Review).await? {
+        deny_request(&repo_state, pr.number(), author, PermissionType::Review).await?;
+        return Ok(());
+    }
+
+    let pr_model = pr.db;
+
+    if pr_model.is_stalled() {
+        db.clear_auto_build(pr_model).await?;
+        merge_queue_tx.notify().await?;
+    } else {
+        notify_of_invalid_retry_state(&repo_state, pr.number()).await?;
+    }
+
+    Ok(())
+}
+
+async fn notify_of_invalid_retry_state(
+    repo: &RepositoryState,
+    pr_number: PullRequestNumber,
+) -> anyhow::Result<()> {
+    repo.client
+        .post_comment(
+            pr_number,
+            Comment::new(":exclamation: You can only retry pull requests that are approved and have a previously failed auto build".to_string())
+        )
+        .await?;
+    Ok(())
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::tests::{BorsTester, Comment, User, run_test};
+
+    #[sqlx::test]
+    async fn retry_command_insufficient_privileges(pool: sqlx::PgPool) {
+        run_test(pool, async |tester: &mut BorsTester| {
+            tester
+                .post_comment(Comment::from("@bors retry").with_author(User::unprivileged()))
+                .await?;
+            insta::assert_snapshot!(
+                tester.get_comment_text(()).await?,
+                @"@unprivileged-user: :key: Insufficient privileges: not in review users"
+            );
+            Ok(())
+        })
+        .await;
+    }
+
+    #[sqlx::test]
+    async fn retry_invalid_state_error(pool: sqlx::PgPool) {
+        run_test(pool, async |tester: &mut BorsTester| {
+            tester.post_comment(Comment::from("@bors retry")).await?;
+            insta::assert_snapshot!(
+                tester.get_comment_text(()).await?,
+                @":exclamation: You can only retry pull requests that are approved and have a previously failed auto build"
+            );
+            Ok(())
+        })
+        .await;
+    }
+
+    #[sqlx::test]
+    async fn retry_auto_build(pool: sqlx::PgPool) {
+        run_test(pool, async |tester: &mut BorsTester| {
+            tester.approve(()).await?;
+            tester.start_auto_build(()).await?;
+            tester
+                .workflow_full_failure(tester.auto_branch().await)
+                .await?;
+            tester.expect_comments((), 1).await;
+            tester.post_comment(Comment::from("@bors retry")).await?;
+            tester.wait_for_pr((), |pr| pr.auto_build.is_none()).await?;
+            tester.process_merge_queue().await;
+            insta::assert_snapshot!(
+                tester.get_comment_text(()).await?,
+                @":hourglass: Testing commit pr-1-sha with merge merge-1-pr-1..."
+            );
+            Ok(())
+        })
+        .await;
+    }
+}

src/bors/handlers/retry.rs

@@ -78,20 +78,20 @@ pub(super) async fn handle_workflow_completed(
     if let Some(running_time) = payload.running_time {
         let running_time =
             chrono::Duration::to_std(&running_time).unwrap_or(Duration::from_secs(0));
-        if let Some(min_ci_time) = repo.config.load().min_ci_time {
-            if running_time < min_ci_time {
-                tracing::warn!(
-                    "Workflow running time is less than the minimum CI duration: workflow time ({}s) < min time ({}s). Marking it as a failure",
-                    running_time.as_secs_f64(),
-                    min_ci_time.as_secs_f64()
-                );
-                payload.status = WorkflowStatus::Failure;
-                error_context = Some(format!(
-                    "A workflow was considered to be a failure because it took only `{}s`. The minimum duration for CI workflows is configured to be `{}s`.",
-                    running_time.as_secs_f64(),
-                    min_ci_time.as_secs_f64()
-                ))
-            }
+        if let Some(min_ci_time) = repo.config.load().min_ci_time
+            && running_time < min_ci_time
+        {
+            tracing::warn!(
+                "Workflow running time is less than the minimum CI duration: workflow time ({}s) < min time ({}s). Marking it as a failure",
+                running_time.as_secs_f64(),
+                min_ci_time.as_secs_f64()
+            );
+            payload.status = WorkflowStatus::Failure;
+            error_context = Some(format!(
+                "A workflow was considered to be a failure because it took only `{}s`. The minimum duration for CI workflows is configured to be `{}s`.",
+                running_time.as_secs_f64(),
+                min_ci_time.as_secs_f64()
+            ))
         }
     }
 
@@ -375,19 +375,16 @@ pub async fn cancel_build(
         .await
         .map_err(CancelBuildError::FailedToCancelWorkflows)?;
 
-    if let Some(check_run_id) = build.check_run_id {
-        if let Err(error) = client
+    if let Some(check_run_id) = build.check_run_id
+        && let Err(error) = client
             .update_check_run(
                 CheckRunId(check_run_id as u64),
                 CheckRunStatus::Completed,
                 Some(check_run_conclusion),
             )
             .await
-        {
-            tracing::error!(
-                "Could not update check run {check_run_id} for build {build:?}: {error:?}"
-            );
-        }
+    {
+        tracing::error!("Could not update check run {check_run_id} for build {build:?}: {error:?}");
     }
 
     Ok(pending_workflows)

src/bors/handlers/workflow.rs

@@ -11,7 +11,7 @@ use crate::github::PullRequestNumber;
 use crate::github::{CommitSha, GithubRepoName};
 
 use super::operations::{
-    approve_pull_request, create_build, create_pull_request, create_workflow,
+    approve_pull_request, clear_auto_build, create_build, create_pull_request, create_workflow,
     delegate_pull_request, delete_tagged_bot_comment, find_build, find_pr_by_build,
     get_nonclosed_pull_requests, get_pending_builds, get_prs_with_unknown_mergeability_state,
     get_pull_request, get_repository, get_repository_by_name, get_tagged_bot_comments,
@@ -49,6 +49,10 @@ impl PgDbClient {
         unapprove_pull_request(&self.pool, pr.id).await
     }
 
+    pub async fn clear_auto_build(&self, pr: &PullRequestModel) -> anyhow::Result<()> {
+        clear_auto_build(&self.pool, pr.id).await
+    }
+
     pub async fn set_priority(&self, pr: &PullRequestModel, priority: u32) -> anyhow::Result<()> {
         set_pr_priority(&self.pool, pr.id, priority).await
     }

src/database/client.rs

@@ -381,6 +381,15 @@ impl PullRequestModel {
             ApprovalStatus::NotApproved => None,
         }
     }
+
+    /// Approved but with a failed auto build.
+    pub fn is_stalled(&self) -> bool {
+        self.is_approved()
+            && self
+                .auto_build
+                .as_ref()
+                .is_some_and(|build| build.status.is_failure())
+    }
 }
 
 /// Describes whether a workflow is a Github Actions workflow or if it's a job from some external

src/database/mod.rs

@@ -1092,3 +1092,19 @@ pub(crate) async fn delete_tagged_bot_comment(
     })
     .await
 }
+
+pub(crate) async fn clear_auto_build(
+    executor: impl PgExecutor<'_>,
+    pr_id: i32,
+) -> anyhow::Result<()> {
+    measure_db_query("clear_pr_builds", || async {
+        sqlx::query!(
+            "UPDATE pull_request SET auto_build_id = NULL WHERE id = $1",
+            pr_id
+        )
+        .execute(executor)
+        .await?;
+        Ok(())
+    })
+    .await
+}