From 7fb447a92be4d67856972785a133d25c7591c0f3 Mon Sep 17 00:00:00 2001
From: Peter Jaszkowiak <p.jaszkow@gmail.com>
Date: Fri, 15 Sep 2023 21:10:39 -0600
Subject: [PATCH 1/6] no-merges: match titles instead of labels

also don't re-add labels if they're manually removed

labels are not always set atomically when opening a PR
example: rust-lang/miri#3059
---
 src/config.rs             |  4 ++--
 src/handlers/no_merges.rs | 42 ++++++++++++++++++++++++++++-----------
 2 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/src/config.rs b/src/config.rs
index 38e81fce..3386c32a 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -105,9 +105,9 @@ impl AssignConfig {
 
 #[derive(PartialEq, Eq, Debug, serde::Deserialize)]
 pub(crate) struct NoMergesConfig {
-    /// No action will be taken on PRs with these labels.
+    /// No action will be taken on PRs with these substrings in the title.
     #[serde(default)]
-    pub(crate) exclude_labels: Vec<String>,
+    pub(crate) exclude_titles: Vec<String>,
     /// Set these labels on the PR when merge commits are detected.
     #[serde(default)]
     pub(crate) labels: Vec<String>,
diff --git a/src/handlers/no_merges.rs b/src/handlers/no_merges.rs
index e90008cd..9856c871 100644
--- a/src/handlers/no_merges.rs
+++ b/src/handlers/no_merges.rs
@@ -48,11 +48,13 @@ pub(super) async fn parse_input(
         return Ok(None);
     }
 
-    // Don't trigger if the PR has any of the excluded labels.
-    for label in event.issue.labels() {
-        if config.exclude_labels.contains(&label.name) {
-            return Ok(None);
-        }
+    // Don't trigger if the PR has any of the excluded title segments.
+    if config
+        .exclude_titles
+        .iter()
+        .any(|s| event.issue.title.contains(s))
+    {
+        return Ok(None);
     }
 
     let mut merge_commits = HashSet::new();
@@ -70,12 +72,11 @@ pub(super) async fn parse_input(
         }
     }
 
-    let input = NoMergesInput { merge_commits };
-    Ok(if input.merge_commits.is_empty() {
-        None
-    } else {
-        Some(input)
-    })
+    if merge_commits.is_empty() {
+        return Ok(None);
+    }
+
+    Ok(Some(NoMergesInput { merge_commits }))
 }
 
 const DEFAULT_MESSAGE: &str = "
@@ -102,6 +103,7 @@ pub(super) async fn handle_input(
     let mut client = ctx.db.get().await;
     let mut state: IssueData<'_, NoMergesState> =
         IssueData::load(&mut client, &event.issue, NO_MERGES_KEY).await?;
+    let first_time = state.data.mentioned_merge_commits.is_empty();
 
     let mut message = config
         .message
@@ -109,7 +111,7 @@ pub(super) async fn handle_input(
         .unwrap_or(DEFAULT_MESSAGE)
         .to_string();
 
-    let since_last_posted = if state.data.mentioned_merge_commits.is_empty() {
+    let since_last_posted = if first_time {
         ""
     } else {
         " (since this message was last posted)"
@@ -132,6 +134,22 @@ pub(super) async fn handle_input(
     }
 
     if should_send {
+        if !first_time {
+            // Check if the labels are still set.
+            // Otherwise, they were probably removed manually.
+            let any_removed = config.labels.iter().any(|label| {
+                // No label on the issue matches.
+                event.issue.labels().iter().all(|l| &l.name != label)
+            });
+
+            if any_removed {
+                // Assume it was a false positive, so don't
+                // re-add the labels or send a message this time.
+                state.save().await?;
+                return Ok(());
+            }
+        }
+
         // Set labels
         let labels = config
             .labels

From 559e1d37b23c113d7d5574bf708ca6e4a489a742 Mon Sep 17 00:00:00 2001
From: Maybe Waffle <waffle.lapkin@gmail.com>
Date: Sun, 15 Oct 2023 12:48:24 +0200
Subject: [PATCH 2/6] Make `IssuesAction` into a dataful enum, to remove
 unwraps

---
 src/github.rs                | 22 ++++++++++++++++------
 src/handlers/autolabel.rs    |  4 ++--
 src/handlers/major_change.rs | 19 +++----------------
 src/handlers/notify_zulip.rs | 18 ++++++++++--------
 4 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/src/github.rs b/src/github.rs
index 496a214e..f46b8d55 100644
--- a/src/github.rs
+++ b/src/github.rs
@@ -911,7 +911,7 @@ pub struct IssueCommentEvent {
 }
 
 #[derive(PartialEq, Eq, Debug, serde::Deserialize)]
-#[serde(rename_all = "snake_case")]
+#[serde(rename_all = "snake_case", tag = "action")]
 pub enum IssuesAction {
     Opened,
     Edited,
@@ -923,13 +923,22 @@ pub enum IssuesAction {
     Reopened,
     Assigned,
     Unassigned,
-    Labeled,
-    Unlabeled,
+    Labeled {
+        /// The label added from the issue
+        label: Label,
+    },
+    Unlabeled {
+        /// The label removed from the issue
+        label: Label,
+    },
     Locked,
     Unlocked,
     Milestoned,
     Demilestoned,
-    ReviewRequested,
+    ReviewRequested {
+        /// The person requested to review the pull request
+        requested_reviewer: User,
+    },
     ReviewRequestRemoved,
     ReadyForReview,
     Synchronize,
@@ -940,13 +949,14 @@ pub enum IssuesAction {
 
 #[derive(Debug, serde::Deserialize)]
 pub struct IssuesEvent {
+    #[serde(flatten)]
     pub action: IssuesAction,
     #[serde(alias = "pull_request")]
     pub issue: Issue,
     pub changes: Option<Changes>,
     pub repository: Repository,
-    /// Some if action is IssuesAction::Labeled, for example
-    pub label: Option<Label>,
+    /// The GitHub user that triggered the event.
+    pub sender: User,
 }
 
 #[derive(Debug, serde::Deserialize)]
diff --git a/src/handlers/autolabel.rs b/src/handlers/autolabel.rs
index df17fd99..2a14a9d0 100644
--- a/src/handlers/autolabel.rs
+++ b/src/handlers/autolabel.rs
@@ -95,9 +95,9 @@ pub(super) async fn parse_input(
         }
     }
 
-    if event.action == IssuesAction::Labeled {
+    if let IssuesAction::Labeled { label } = &event.action {
         let mut autolabels = Vec::new();
-        let applied_label = &event.label.as_ref().expect("label").name;
+        let applied_label = &label.name;
 
         'outer: for (label, config) in config.get_by_trigger(applied_label) {
             let exclude_patterns: Vec<glob::Pattern> = config
diff --git a/src/handlers/major_change.rs b/src/handlers/major_change.rs
index e1a57603..48148397 100644
--- a/src/handlers/major_change.rs
+++ b/src/handlers/major_change.rs
@@ -55,11 +55,7 @@ pub(super) async fn parse_input(
     }
 
     // If we were labeled with accepted, then issue that event
-    if event.action == IssuesAction::Labeled
-        && event
-            .label
-            .as_ref()
-            .map_or(false, |l| l.name == config.accept_label)
+    if matches!(&event.action, IssuesAction::Labeled { label } if label.name == config.accept_label)
     {
         return Ok(Some(Invocation::AcceptedProposal));
     }
@@ -70,17 +66,8 @@ pub(super) async fn parse_input(
     // We want to treat reopened issues as new proposals but if the
     // issue is freshly opened, we only want to trigger once;
     // currently we do so on the label event.
-    if (event.action == IssuesAction::Reopened
-        && event
-            .issue
-            .labels()
-            .iter()
-            .any(|l| l.name == enabling_label))
-        || (event.action == IssuesAction::Labeled
-            && event
-                .label
-                .as_ref()
-                .map_or(false, |l| l.name == enabling_label))
+    if matches!(event.action, IssuesAction::Reopened if event.issue.labels().iter().any(|l| l.name == enabling_label))
+        || matches!(&event.action, IssuesAction::Labeled { label } if label.name == enabling_label)
     {
         return Ok(Some(Invocation::NewProposal));
     }
diff --git a/src/handlers/notify_zulip.rs b/src/handlers/notify_zulip.rs
index dc3b0d22..87d0a018 100644
--- a/src/handlers/notify_zulip.rs
+++ b/src/handlers/notify_zulip.rs
@@ -31,9 +31,9 @@ pub(super) async fn parse_input(
         None => return Ok(None),
     };
 
-    match event.action {
-        IssuesAction::Labeled | IssuesAction::Unlabeled => {
-            let applied_label = event.label.as_ref().expect("label").clone();
+    match &event.action {
+        IssuesAction::Labeled { label } | IssuesAction::Unlabeled { label } => {
+            let applied_label = label.clone();
             Ok(config
                 .labels
                 .get(&applied_label.name)
@@ -60,14 +60,16 @@ fn parse_label_change_input(
     }
 
     match event.action {
-        IssuesAction::Labeled if config.message_on_add.is_some() => Some(NotifyZulipInput {
+        IssuesAction::Labeled { .. } if config.message_on_add.is_some() => Some(NotifyZulipInput {
             notification_type: NotificationType::Labeled,
             label,
         }),
-        IssuesAction::Unlabeled if config.message_on_remove.is_some() => Some(NotifyZulipInput {
-            notification_type: NotificationType::Unlabeled,
-            label,
-        }),
+        IssuesAction::Unlabeled { .. } if config.message_on_remove.is_some() => {
+            Some(NotifyZulipInput {
+                notification_type: NotificationType::Unlabeled,
+                label,
+            })
+        }
         _ => None,
     }
 }

From 7a0a7728d965d8a66eef1d9717f196ff32e9f7dd Mon Sep 17 00:00:00 2001
From: Maybe Waffle <waffle.lapkin@gmail.com>
Date: Sun, 15 Oct 2023 12:51:21 +0200
Subject: [PATCH 3/6] Add a handler for requested reviews

---
 src/config.rs                    |  8 +++++++
 src/handlers.rs                  | 15 ++++++++++++
 src/handlers/review_requested.rs | 40 ++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+)
 create mode 100644 src/handlers/review_requested.rs

diff --git a/src/config.rs b/src/config.rs
index 38e81fce..2235c1ad 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -30,6 +30,7 @@ pub(crate) struct Config {
     pub(crate) notify_zulip: Option<NotifyZulipConfig>,
     pub(crate) github_releases: Option<GitHubReleasesConfig>,
     pub(crate) review_submitted: Option<ReviewSubmittedConfig>,
+    pub(crate) review_requested: Option<ReviewRequestedConfig>,
     pub(crate) shortcut: Option<ShortcutConfig>,
     pub(crate) note: Option<NoteConfig>,
     pub(crate) mentions: Option<MentionsConfig>,
@@ -253,6 +254,12 @@ pub(crate) struct ReviewSubmittedConfig {
     pub(crate) reviewed_label: String,
 }
 
+#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
+pub(crate) struct ReviewRequestedConfig {
+    pub(crate) remove_labels: Vec<String>,
+    pub(crate) add_labels: Vec<String>,
+}
+
 pub(crate) async fn get(
     gh: &GithubClient,
     repo: &Repository,
@@ -421,6 +428,7 @@ mod tests {
                 notify_zulip: None,
                 github_releases: None,
                 review_submitted: None,
+                review_requested: None,
                 mentions: None,
                 no_merges: None,
             }
diff --git a/src/handlers.rs b/src/handlers.rs
index da39943b..9415c606 100644
--- a/src/handlers.rs
+++ b/src/handlers.rs
@@ -41,6 +41,7 @@ mod notify_zulip;
 mod ping;
 mod prioritize;
 mod relabel;
+mod review_requested;
 mod review_submitted;
 mod rfc_helper;
 pub mod rustc_commits;
@@ -107,6 +108,20 @@ pub async fn handle(ctx: &Context, event: &Event) -> Vec<HandlerError> {
         }
     }
 
+    if let Some(config) = config
+        .as_ref()
+        .ok()
+        .and_then(|c| c.review_requested.as_ref())
+    {
+        if let Err(e) = review_requested::handle(ctx, event, config).await {
+            log::error!(
+                "failed to process event {:?} with review_requested handler: {:?}",
+                event,
+                e
+            )
+        }
+    }
+
     if let Some(ghr_config) = config
         .as_ref()
         .ok()
diff --git a/src/handlers/review_requested.rs b/src/handlers/review_requested.rs
new file mode 100644
index 00000000..d09e65df
--- /dev/null
+++ b/src/handlers/review_requested.rs
@@ -0,0 +1,40 @@
+use crate::config::ReviewRequestedConfig;
+use crate::github::{Event, IssuesAction, IssuesEvent, Label};
+use crate::handlers::Context;
+
+pub(crate) async fn handle(
+    ctx: &Context,
+    event: &Event,
+    config: &ReviewRequestedConfig,
+) -> anyhow::Result<()> {
+    // PR author requests a review from one of the assignees
+    if let Event::Issue(IssuesEvent {
+        action,
+        issue,
+        sender,
+        ..
+    }) = event
+    {
+        if let IssuesAction::ReviewRequested { requested_reviewer } = action {
+            if *sender == issue.user && issue.assignees.contains(requested_reviewer) {
+                issue
+                    .add_labels(
+                        &ctx.github,
+                        config
+                            .add_labels
+                            .iter()
+                            .cloned()
+                            .map(|name| Label { name })
+                            .collect(),
+                    )
+                    .await?;
+
+                for label in &config.remove_labels {
+                    issue.remove_label(&ctx.github, label).await?;
+                }
+            }
+        }
+    }
+
+    Ok(())
+}

From 3233df1f32276461528826ee711e3f6c9b7d16d9 Mon Sep 17 00:00:00 2001
From: Maybe Waffle <waffle.lapkin@gmail.com>
Date: Sun, 15 Oct 2023 14:03:40 +0200
Subject: [PATCH 4/6] Allow configuring bot name

---
 .env.sample             |  6 ++++++
 src/handlers/assign.rs  | 16 +++++++++++-----
 src/handlers/glacier.rs |  4 ++--
 src/main.rs             |  5 ++++-
 4 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/.env.sample b/.env.sample
index 405d97dd..d79667d3 100644
--- a/.env.sample
+++ b/.env.sample
@@ -5,3 +5,9 @@ GITHUB_WEBHOOK_SECRET=MUST_BE_CONFIGURED
 # for logging, refer to this document: https://rust-lang-nursery.github.io/rust-cookbook/development_tools/debugging/config_log.html
 # `RUSTC_LOG` is not required to run the application, but it makes local development easier
 # RUST_LOG=MUST_BE_CONFIGURED
+
+# If you are running a bot on non-rustbot account,
+# this allows to configure that username which the bot will respond to.
+# For example write blahblahblah here, if you want for this bot to 
+# respond to @blahblahblah claim.
+# TRIAGEBOT_USERNAME=CAN_BE_CONFIGURED
diff --git a/src/handlers/assign.rs b/src/handlers/assign.rs
index 2f36dfdc..8f0529f2 100644
--- a/src/handlers/assign.rs
+++ b/src/handlers/assign.rs
@@ -47,8 +47,8 @@ minimum review times lag, PR authors and assigned reviewers should ensure that t
 label (`S-waiting-on-review` and `S-waiting-on-author`) stays updated, invoking these commands \
 when appropriate:
 
-- `@rustbot author`: the review is finished, PR author should check the comments and take action accordingly
-- `@rustbot review`: the author is ready for a review, this PR will be queued again in the reviewer's queue";
+- `@{bot} author`: the review is finished, PR author should check the comments and take action accordingly
+- `@{bot} review`: the author is ready for a review, this PR will be queued again in the reviewer's queue";
 
 const WELCOME_WITH_REVIEWER: &str = "@{assignee} (or someone else)";
 
@@ -56,7 +56,7 @@ const WELCOME_WITHOUT_REVIEWER: &str = "@Mark-Simulacrum (NB. this repo may be m
 
 const RETURNING_USER_WELCOME_MESSAGE: &str = "r? @{assignee}
 
-(rustbot has picked a reviewer for you, use r? to override)";
+({bot} has picked a reviewer for you, use r? to override)";
 
 const RETURNING_USER_WELCOME_MESSAGE_NO_REVIEWER: &str =
     "@{author}: no appropriate reviewer found, use r? to override";
@@ -141,12 +141,18 @@ pub(super) async fn handle_input(
             let mut welcome = NEW_USER_WELCOME_MESSAGE.replace("{who}", &who_text);
             if let Some(contrib) = &config.contributing_url {
                 welcome.push_str("\n\n");
-                welcome.push_str(&CONTRIBUTION_MESSAGE.replace("{contributing_url}", contrib));
+                welcome.push_str(
+                    &CONTRIBUTION_MESSAGE
+                        .replace("{contributing_url}", contrib)
+                        .replace("{bot}", &ctx.username),
+                );
             }
             Some(welcome)
         } else if !from_comment {
             let welcome = match &assignee {
-                Some(assignee) => RETURNING_USER_WELCOME_MESSAGE.replace("{assignee}", assignee),
+                Some(assignee) => RETURNING_USER_WELCOME_MESSAGE
+                    .replace("{assignee}", assignee)
+                    .replace("{bot}", &ctx.username),
                 None => RETURNING_USER_WELCOME_MESSAGE_NO_REVIEWER
                     .replace("{author}", &event.issue.user.login),
             };
diff --git a/src/handlers/glacier.rs b/src/handlers/glacier.rs
index b8930aaf..941c9edf 100644
--- a/src/handlers/glacier.rs
+++ b/src/handlers/glacier.rs
@@ -33,7 +33,7 @@ pub(super) async fn handle_command(
 
     let octocrab = &ctx.octocrab;
 
-    let fork = octocrab.repos("rustbot", "glacier");
+    let fork = octocrab.repos(&ctx.username, "glacier");
     let base = octocrab.repos("rust-lang", "glacier");
 
     let master = base
@@ -65,7 +65,7 @@ pub(super) async fn handle_command(
         .pulls("rust-lang", "glacier")
         .create(
             format!("ICE - rust-lang/rust#{}", number),
-            format!("rustbot:triagebot-ice-{}", number),
+            format!("{}:triagebot-ice-{}", ctx.username, number),
             "master",
         )
         .body(format!(
diff --git a/src/main.rs b/src/main.rs
index 92fda321..0a041385 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -246,7 +246,10 @@ async fn run_server(addr: SocketAddr) -> anyhow::Result<()> {
         .build()
         .expect("Failed to build octograb.");
     let ctx = Arc::new(Context {
-        username: String::from("rustbot"),
+        username: std::env::var("TRIAGEBOT_USERNAME").or_else(|err| match err {
+            std::env::VarError::NotPresent => Ok("rustbot".to_owned()),
+            err => Err(err),
+        })?,
         db: pool,
         github: gh,
         octocrab: oc,

From 9563afffd8952b4daedbc4cf44afc0bfad79b493 Mon Sep 17 00:00:00 2001
From: Maybe Waffle <waffle.lapkin@gmail.com>
Date: Mon, 16 Oct 2023 00:19:05 +0200
Subject: [PATCH 5/6] Un-ad-hoc `review_requested` handler

Use the `issue_handlers!` instead
---
 src/handlers.rs                  | 15 +------
 src/handlers/review_requested.rs | 73 +++++++++++++++++++-------------
 2 files changed, 44 insertions(+), 44 deletions(-)

diff --git a/src/handlers.rs b/src/handlers.rs
index 9415c606..7e4a04f9 100644
--- a/src/handlers.rs
+++ b/src/handlers.rs
@@ -108,20 +108,6 @@ pub async fn handle(ctx: &Context, event: &Event) -> Vec<HandlerError> {
         }
     }
 
-    if let Some(config) = config
-        .as_ref()
-        .ok()
-        .and_then(|c| c.review_requested.as_ref())
-    {
-        if let Err(e) = review_requested::handle(ctx, event, config).await {
-            log::error!(
-                "failed to process event {:?} with review_requested handler: {:?}",
-                event,
-                e
-            )
-        }
-    }
-
     if let Some(ghr_config) = config
         .as_ref()
         .ok()
@@ -179,6 +165,7 @@ issue_handlers! {
     mentions,
     no_merges,
     notify_zulip,
+    review_requested,
 }
 
 macro_rules! command_handlers {
diff --git a/src/handlers/review_requested.rs b/src/handlers/review_requested.rs
index d09e65df..2d38b9e3 100644
--- a/src/handlers/review_requested.rs
+++ b/src/handlers/review_requested.rs
@@ -1,39 +1,52 @@
 use crate::config::ReviewRequestedConfig;
-use crate::github::{Event, IssuesAction, IssuesEvent, Label};
+use crate::github::{IssuesAction, IssuesEvent, Label};
 use crate::handlers::Context;
 
-pub(crate) async fn handle(
+pub(crate) struct ReviewRequestedInput {}
+
+pub(crate) async fn parse_input(
+    _ctx: &Context,
+    event: &IssuesEvent,
+    _config: Option<&ReviewRequestedConfig>,
+) -> Result<Option<ReviewRequestedInput>, String> {
+    // PR author requests a review from one of the assignees
+
+    let IssuesAction::ReviewRequested { requested_reviewer } = &event.action else {
+        return Ok(None);
+    };
+
+    if event.sender != event.issue.user {
+        return Ok(None);
+    }
+
+    if !event.issue.assignees.contains(requested_reviewer) {
+        return Ok(None);
+    }
+
+    Ok(Some(ReviewRequestedInput {}))
+}
+
+pub(crate) async fn handle_input(
     ctx: &Context,
-    event: &Event,
     config: &ReviewRequestedConfig,
+    event: &IssuesEvent,
+    ReviewRequestedInput {}: ReviewRequestedInput,
 ) -> anyhow::Result<()> {
-    // PR author requests a review from one of the assignees
-    if let Event::Issue(IssuesEvent {
-        action,
-        issue,
-        sender,
-        ..
-    }) = event
-    {
-        if let IssuesAction::ReviewRequested { requested_reviewer } = action {
-            if *sender == issue.user && issue.assignees.contains(requested_reviewer) {
-                issue
-                    .add_labels(
-                        &ctx.github,
-                        config
-                            .add_labels
-                            .iter()
-                            .cloned()
-                            .map(|name| Label { name })
-                            .collect(),
-                    )
-                    .await?;
-
-                for label in &config.remove_labels {
-                    issue.remove_label(&ctx.github, label).await?;
-                }
-            }
-        }
+    event
+        .issue
+        .add_labels(
+            &ctx.github,
+            config
+                .add_labels
+                .iter()
+                .cloned()
+                .map(|name| Label { name })
+                .collect(),
+        )
+        .await?;
+
+    for label in &config.remove_labels {
+        event.issue.remove_label(&ctx.github, label).await?;
     }
 
     Ok(())

From 74f2ee16fb1102ca5d0d4eb22f42c924383f819c Mon Sep 17 00:00:00 2001
From: Eric Huss <eric@huss.org>
Date: Thu, 19 Oct 2023 08:55:35 -0700
Subject: [PATCH 6/6] Review requested: don't error if not configured

---
 src/handlers/review_requested.rs | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/handlers/review_requested.rs b/src/handlers/review_requested.rs
index 2d38b9e3..3f5db5a4 100644
--- a/src/handlers/review_requested.rs
+++ b/src/handlers/review_requested.rs
@@ -7,10 +7,14 @@ pub(crate) struct ReviewRequestedInput {}
 pub(crate) async fn parse_input(
     _ctx: &Context,
     event: &IssuesEvent,
-    _config: Option<&ReviewRequestedConfig>,
+    config: Option<&ReviewRequestedConfig>,
 ) -> Result<Option<ReviewRequestedInput>, String> {
     // PR author requests a review from one of the assignees
 
+    if config.is_none() {
+        return Ok(None);
+    }
+
     let IssuesAction::ReviewRequested { requested_reviewer } = &event.action else {
         return Ok(None);
     };