From be65c102fc07b456cfe4162c06792585dc168803 Mon Sep 17 00:00:00 2001 From: nxsaken Date: Wed, 29 Jul 2026 21:07:52 +0400 Subject: [PATCH 1/9] Rewrite goals ping job --- src/bin/project_goals.rs | 6 +- src/handlers/project_goals.rs | 1019 +++++++++++++++++++++++++-------- src/jobs.rs | 7 + src/zulip.rs | 3 +- src/zulip/commands.rs | 4 +- 5 files changed, 775 insertions(+), 264 deletions(-) diff --git a/src/bin/project_goals.rs b/src/bin/project_goals.rs index c2542e4bd..0c7b5200c 100644 --- a/src/bin/project_goals.rs +++ b/src/bin/project_goals.rs @@ -10,11 +10,8 @@ struct Opt { #[arg(long)] dry_run: bool, - /// Goals with an updated within this threshold will not be pinged. + /// Goals updated within this threshold (in days) will not be pinged. days_threshold: i64, - - /// A string like "on Sep-5" when the update blog post will be written. - next_meeting_date: String, } #[tokio::main(flavor = "current_thread")] @@ -32,7 +29,6 @@ async fn main() -> anyhow::Result<()> { &team_api, opt.dry_run, opt.days_threshold, - &opt.next_meeting_date, ) .await?; diff --git a/src/handlers/project_goals.rs b/src/handlers/project_goals.rs index 9941f0dad..f2fd794af 100644 --- a/src/handlers/project_goals.rs +++ b/src/handlers/project_goals.rs @@ -1,203 +1,749 @@ -use super::Context; -use crate::github::{ - self, GitHubUser, GithubClient, IssueCommentAction, IssueCommentEvent, IssuesAction, - IssuesEvent, +use crate::{ + github::{ + self, Event, GithubClient, Issue, IssueCommentAction, IssueCommentEvent, IssuesAction, + IssuesEvent, + }, + handlers::Context, + jobs::Job, + team_data::TeamClient, + zulip::{MessageApiRequest, api::Recipient, client::ZulipClient}, }; -use crate::github::{Event, Issue}; -use crate::jobs::Job; -use crate::team_data::TeamClient; -use crate::zulip::api::Recipient; -use crate::zulip::client::ZulipClient; -use anyhow::Context as _; use async_trait::async_trait; -use chrono::{Datelike, NaiveDate, Utc}; -use tracing::{self as log}; +use chrono::{DateTime, Datelike, Duration, NaiveDate, Utc}; +use itertools::Itertools; +use std::collections::BTreeMap; +use tracing as log; -const MAX_ZULIP_TOPIC: usize = 60; const RUST_PROJECT_GOALS_REPO: &str = "rust-lang/rust-project-goals"; -const GOALS_STREAM: u64 = 435_869; // #project-goals const C_TRACKING_ISSUE: &str = "C-tracking-issue"; -fn message( - zulip_owners: &str, - days: &str, - issue_number: u64, - issue_title: &str, - next_update: &str, -) -> String { +const GOALS_STREAM: u64 = 435_869; // #project-goals +const TRIAGEBOT_TOPIC: &str = "Triagebot reports"; +const MAX_ZULIP_TOPIC: usize = 60; + +#[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] +struct ZulipId(u64); + +impl ZulipId { + fn mention(self, muted: bool) -> String { + if muted { + format!("@_**|{}**", self.0) + } else { + format!("@**|{}**", self.0) + } + } +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] +struct GhUsername<'gh>(&'gh str); + +impl<'gh> GhUsername<'gh> { + fn link(self) -> String { + format!("[@{login}](https://github.com/{login})", login = self.0,) + } + + fn team_file_link(self) -> String { + format!( + "[@{login}](https://github.com/rust-lang/team/tree/main/people/{login}.toml)", + login = self.0, + ) + } +} + +#[derive(Clone, Copy, Debug)] +struct Owner<'gh> { + github: GhUsername<'gh>, + zulip: Option, +} + +impl<'gh> Owner<'gh> { + async fn resolve( + team: &TeamClient, + github_id: u64, + username: &'gh str, + ) -> anyhow::Result { + let zulip = team.github_to_zulip_id(github_id).await?.map(ZulipId); + Ok(Self { + github: GhUsername(username), + zulip, + }) + } + + fn display_mention(self, muted: bool) -> String { + match self.zulip { + Some(zulip_id) => zulip_id.mention(muted), + None => self.github.link(), + } + } +} + +#[derive(Clone, Debug)] +struct Owners<'gh>(Vec>); + +fn join_mentions(mentions: Vec) -> String { + match mentions.as_slice() { + [] => "(none assigned)".to_owned(), + [owner] => owner.clone(), + [first, second] => format!("{first} and {second}"), + [rest @ .., last] => { + format!("{}, and {last}", rest.iter().join(", ")) + } + } +} + +impl<'gh> Owners<'gh> { + async fn resolve(team: &TeamClient, issue: &'gh Issue) -> anyhow::Result { + let mut owners = Vec::with_capacity(issue.assignees.len()); + for assignee in &issue.assignees { + owners.push(Owner::resolve(team, assignee.id, &assignee.login).await?); + } + Ok(Self(owners)) + } + + fn is_empty(&self) -> bool { + self.0.is_empty() + } + + fn has_multiple(&self) -> bool { + self.0.len() > 1 + } + + fn reachable(&self) -> impl Iterator + '_ { + self.0.iter().copied().filter_map(|owner| owner.zulip) + } + + fn unreachable(&self) -> impl Iterator> + '_ { + self.0 + .iter() + .copied() + .filter_map(|owner| owner.zulip.is_none().then_some(owner.github)) + } + + fn all_mentions(&self, muted: bool) -> String { + join_mentions( + self.0 + .iter() + .copied() + .map(|o| o.display_mention(muted)) + .collect_vec(), + ) + } + + fn reachable_mentions(&self) -> Option { + let reachables = self.reachable().map(|id| id.mention(true)).collect_vec(); + if reachables.is_empty() { + None + } else { + Some(join_mentions(reachables)) + } + } + + fn unreachable_team_links(&self) -> String { + join_mentions( + self.unreachable() + .map(GhUsername::team_file_link) + .collect_vec(), + ) + } +} + +#[derive(Clone, Copy, Debug)] +enum LastUpdate { + Never, + DaysAgo(i64), +} + +impl LastUpdate { + fn description(self) -> String { + match self { + Self::Never => "no updates so far".to_owned(), + Self::DaysAgo(days) => format!("last update was {days} days ago"), + } + } +} + +#[derive(Copy, Clone, Debug)] +struct Reminder<'gh> { + issue: u64, + title: &'gh str, + last_update: LastUpdate, +} + +struct EvaluatedReminder<'gh> { + reminder: Reminder<'gh>, + requires_update: bool, + invalid_schedule_reason: Option, +} + +impl<'gh> Reminder<'gh> { + fn from_issue(issue: &'gh Issue, days_since_last_update: i64) -> Self { + let last_update = if issue.comments.unwrap_or(0) <= 1 { + LastUpdate::Never + } else { + LastUpdate::DaysAgo(days_since_last_update) + }; + Self { + issue: issue.number, + title: &issue.title, + last_update, + } + } + + fn list_item(&self) -> String { + format!( + "+ *{title}* (goals#{issue}) — {last_update}", + title = self.title, + issue = self.issue, + last_update = self.last_update.description(), + ) + } + + fn reference(&self) -> String { + format!( + "*{title}* (goals#{issue})", + title = self.title, + issue = self.issue, + ) + } +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd)] +enum CustomSchedule { + /// Runs on every job invocation (every week). + Weekly = 0, + /// Runs during even-numbered ISO weeks. + Biweekly0 = 1, + /// Runs during odd-numbered ISO weeks. + Biweekly1 = 2, + /// Runs on the first job invocation of each month. + Monthly = 3, +} + +#[derive(Clone, Debug)] +enum Schedule { + Default, + Custom(CustomSchedule), + Invalid { + fallback: CustomSchedule, + reason: String, + }, +} + +impl Schedule { + fn from_issue(issue: &Issue) -> Self { + const PING_FREQUENCY_LABELS: &[(&str, CustomSchedule)] = &[ + ("P-weekly", CustomSchedule::Weekly), + ("P-biweekly-0", CustomSchedule::Biweekly0), + ("P-biweekly-1", CustomSchedule::Biweekly1), + ("P-monthly", CustomSchedule::Monthly), + ]; + + match PING_FREQUENCY_LABELS + .iter() + .filter_map(|&(label, value)| { + issue + .labels + .iter() + .any(|l| l.name == label) + .then_some((label, value)) + }) + .collect::>() + .as_slice() + { + [] => Self::Default, + [(_, frequency)] => Self::Custom(*frequency), + multiple => { + let (fallback_label, fallback) = multiple + .iter() + .copied() + .min_by_key(|(_, schedule)| *schedule) + .expect("multiple contains at least two schedules"); + + Self::Invalid { + fallback, + reason: format!( + "multiple frequency labels are set: {}; falling back to `{fallback_label}`", + multiple.iter().map(|&(label, _)| label).join(", "), + ), + } + } + } + } +} + +fn latest_biweekly_due_date(today: NaiveDate, parity: bool) -> NaiveDate { + if today.iso_week().week() % 2 == parity as u32 { + today + } else { + today - Duration::weeks(1) + } +} + +impl CustomSchedule { + fn latest_due_date(self, today: NaiveDate) -> NaiveDate { + match self { + Self::Weekly => today, + Self::Biweekly0 => latest_biweekly_due_date(today, false), + Self::Biweekly1 => latest_biweekly_due_date(today, true), + Self::Monthly => { + let weeks_since_first_run = today.day0() / 7; + today - Duration::weeks(i64::from(weeks_since_first_run)) + } + } + } +} + +#[derive(Clone, Debug)] +struct MultipleOwners<'gh> { + goal: Reminder<'gh>, + owners: Owners<'gh>, +} + +#[derive(Clone, Debug)] +struct InvalidSchedule<'gh> { + goal: Reminder<'gh>, + reason: String, +} + +#[derive(Default)] +struct ReminderErrors<'gh> { + unowned: Vec>, + multiply_owned: Vec>, + missing_zulip: Vec>, + invalid_schedules: Vec>, +} + +impl ReminderErrors<'_> { + fn is_empty(&self) -> bool { + self.unowned.is_empty() + && self.multiply_owned.is_empty() + && self.missing_zulip.is_empty() + && self.invalid_schedules.is_empty() + } +} + +#[derive(Default)] +struct ReminderPlan<'gh> { + goals_by_owner: BTreeMap>>, + errors: ReminderErrors<'gh>, +} + +impl<'gh> ReminderPlan<'gh> { + fn add_invalid_schedule(&mut self, goal: Reminder<'gh>, reason: String) { + self.errors + .invalid_schedules + .push(InvalidSchedule { goal, reason }); + } + + fn add_goal(&mut self, goal: Reminder<'gh>, owners: Owners<'gh>) { + if owners.is_empty() { + self.errors.unowned.push(goal); + return; + } + + let goal_with_owners = MultipleOwners { + goal: goal.clone(), + owners: owners.clone(), + }; + + if owners.has_multiple() { + self.errors.multiply_owned.push(goal_with_owners.clone()); + } + + if owners.unreachable().next().is_some() { + self.errors.missing_zulip.push(goal_with_owners); + } + + for owner in owners.reachable() { + self.goals_by_owner + .entry(owner) + .or_default() + .push(goal.clone()); + } + } +} + +fn owner_message(owner: ZulipId, goals: &[Reminder<'_>]) -> String { format!( r#" -Dear {zulip_owners}, it's been {days} days since the last update to your goal *{issue_title}*. +Hi {owner}! + +This is a reminder to post an update on your goals: + +{goals} + +Some questions to guide you (you don't have to follow this format): + ++ What has happened since your last update? ++ Are there any relevant PRs, issues, docs, or discussions to link? ++ Are you blocked on any issue, PR, teams? ++ Do you need help or feedback? Where should people look? ++ What do you plan to work on before the next update? -We will begin drafting the next blog post collecting goal updates {next_update}. +Even if there's little to say, a brief message provides reassurance that the goal is still alive. -Please comment on the github tracking issue goals#{issue_number} before then. Thanks! <3 +Please leave your updates as comments on the tracking issues. Thanks! <3 +"#, + owner = owner.mention(false), + goals = goals.iter().map(Reminder::list_item).join("\n"), + ) +} + +fn unowned_errors(goals: &[Reminder<'_>]) -> String { + let goals = goals + .iter() + .map(|goal| format!("+ {goal}", goal = goal.reference())) + .join("\n"); -Here is a suggested template for updates (feel free to drop the items that don't apply): + format!( + r#" +The following goals have no owner assigned: -* **Key developments:** *What has happened since the last time. It's perfectly ok to list "nothing" if that's the truth, we know people get busy.* -* **Blockers:** *List any Rust teams you are waiting on and what you are waiting for.* -* **Help wanted:** *Are there places where you are looking for contribution or feedback from the broader community?* +{goals} + +Please assign an owner and reach out to them! "# ) } -pub struct ProjectGoalsUpdateJob; +fn multiple_owner_errors(goals: &[MultipleOwners<'_>]) -> String { + let goals = goals + .iter() + .map(|entry| { + format!( + "+ {goal} — assigned to {owners}", + goal = entry.goal.reference(), + owners = entry.owners.all_mentions(true), + ) + }) + .join("\n"); -#[async_trait] -impl Job for ProjectGoalsUpdateJob { - fn name(&self) -> &'static str { - "project_goals_update_job" + format!( + r#" +The following goals have more than one owner assigned: + +{goals} + +A goal should have exactly one owner. All owners with a Zulip account were still notified separately. +"# + ) +} + +fn missing_zulip_errors(goals: &[MultipleOwners<'_>]) -> String { + let goals = goals + .iter() + .map(|entry| { + format!( + "+ {goal} — missing Zulip account: {unreachable}\n {notified}", + goal = entry.goal.reference(), + unreachable = entry.owners.unreachable_team_links(), + notified = match entry.owners.reachable_mentions() { + None => "No existing owner was notified on Zulip.".to_owned(), + Some(owners) => format!("{owners} got notified on Zulip."), + } + ) + }) + .join("\n"); + + format!( + r#" +The following goal owners were not pinged because they don't have a Zulip account specified in the `team` repo: + +{goals} + +Please make sure to register their `zulip-id` and reach out to them! +"# + ) +} + +fn invalid_schedule_errors(errors: &[InvalidSchedule<'_>]) -> String { + let errors = errors + .iter() + .map(|error| format!("+ {} — {}", error.goal.reference(), error.reason,)) + .join("\n"); + + format!( + r#" +The following goals have invalid ping-schedule labels: + +{errors} + +Use exactly one frequency label (`P-weekly`, `P-biweekly-0`, `P-biweekly-1`, or `P-monthly`). +"# + ) +} + +fn error_message(errors: &ReminderErrors<'_>) -> String { + let mut sections = Vec::new(); + + if !errors.unowned.is_empty() { + sections.push(unowned_errors(&errors.unowned)); + } + if !errors.multiply_owned.is_empty() { + sections.push(multiple_owner_errors(&errors.multiply_owned)); + } + if !errors.missing_zulip.is_empty() { + sections.push(missing_zulip_errors(&errors.missing_zulip)); + } + if !errors.invalid_schedules.is_empty() { + sections.push(invalid_schedule_errors(&errors.invalid_schedules)); } - async fn run(&self, ctx: &super::Context, _metadata: &serde_json::Value) -> anyhow::Result<()> { - ping_project_goals_owners_automatically(&ctx.github, &ctx.zulip, &ctx.team).await + format!( + r#" +Hi @*T-goals*! + +{} +"#, + sections.iter().join("\n\n---\n\n"), + ) +} + +fn default_update_required(issue: &Issue, now: DateTime, days_threshold: i64) -> bool { + let comments = issue.comments.unwrap_or(0); + let days_since_last_update = (now - issue.updated_at).num_days(); + + days_since_last_update >= days_threshold || comments <= 1 +} + +fn scheduled_update_required( + issue: &Issue, + now: DateTime, + schedule: CustomSchedule, +) -> bool { + let due_date = schedule.latest_due_date(now.date_naive()); + let has_real_update = issue.comments.unwrap_or(0) > 1; + let updated_after_due_date = issue.updated_at.date_naive() >= due_date; + + !has_real_update || !updated_after_due_date +} + +fn evaluate<'gh>( + issue: &'gh Issue, + now: DateTime, + days_threshold: i64, +) -> EvaluatedReminder<'gh> { + let days_since_last_update = (now - issue.updated_at).num_days(); + + log::debug!( + "issue #{}: days_since_last_comment = {} days, comments = {}", + issue.number, + days_since_last_update, + issue.comments.unwrap_or(0), + ); + + let (requires_update, invalid_schedule_reason) = match Schedule::from_issue(issue) { + Schedule::Default => ( + default_update_required(issue, now, days_threshold), + None, + ), + Schedule::Custom(schedule) => (scheduled_update_required(issue, now, schedule), None), + Schedule::Invalid { fallback, reason } => ( + scheduled_update_required(issue, now, fallback), + Some(reason), + ), + }; + + EvaluatedReminder { + reminder: Reminder::from_issue(issue, days_since_last_update), + requires_update, + invalid_schedule_reason, } } -/// Returns true if the user with the given github id is allowed to ping all group people -/// and do other "project group adminstrative" tasks. -pub async fn check_project_goal_acl(team_client: &TeamClient, gh_id: u64) -> anyhow::Result { - const GOALS_TEAM: &str = "goals"; +async fn build_plan<'gh>( + issues: &'gh [Issue], + team: &TeamClient, + days_threshold: i64, +) -> anyhow::Result> { + let now = Utc::now(); + let mut plan = ReminderPlan::default(); - let team = match team_client.get_team(GOALS_TEAM).await { - Ok(Some(team)) => team, - Ok(None) => { - log::info!("team ({GOALS_TEAM}) failed to resolve to a known team"); - return Ok(false); + for issue in issues { + let evaluation = evaluate(issue, now, days_threshold); + + if let Some(reason) = evaluation.invalid_schedule_reason { + plan.add_invalid_schedule(evaluation.reminder, reason); } - Err(err) => { - log::error!("team ({GOALS_TEAM}) failed to resolve to a known team: {err:?}"); - return Ok(false); + + if !evaluation.requires_update { + continue; } - }; - Ok(team - .members - .into_iter() - .any(|member| member.github_id == gh_id)) + let owners = Owners::resolve(team, issue).await?; + plan.add_goal(evaluation.reminder, owners); + } + + Ok(plan) } -async fn ping_project_goals_owners_automatically( - gh: &GithubClient, +async fn send_dm( zulip: &ZulipClient, - team_api: &TeamClient, + owner: ZulipId, + content: &str, + dry_run: bool, ) -> anyhow::Result<()> { - // Predicted schedule is to author a blog post on the 3rd week of the month. - // We start pinging when the month starts until we see an update in this month - // or the last 7 days of previous month. - // - // Therefore, we compute: - // * Days since start of this month -- threshold will be this number of days + 7. - // * Date of the 3rd Monday in the month -- this will be the next update (e.g., `on Sep-5`). - let now = Utc::now(); + if dry_run { + log::debug!("(DRY) Would send DM to user {}: {}", owner.0, content,); + return Ok(()); + } - // We want to ping people unless they've written an update since the last week of the previous month. - let days_threshold = now.day() + 7; + MessageApiRequest { + recipient: Recipient::Private { + id: owner.0, + email: "", + }, + content, + } + .send(zulip) + .await?; - // Format the 3rd Monday of the month, e.g. "on Sep-5", for inclusion. - let third_monday = - NaiveDate::from_weekday_of_month_opt(now.year(), now.month(), chrono::Weekday::Mon, 3) - .unwrap() - .format("on %b-%d") - .to_string(); + Ok(()) +} + +async fn send_triagebot_topic( + zulip: &ZulipClient, + content: &str, + dry_run: bool, +) -> anyhow::Result<()> { + if dry_run { + log::debug!( + "(DRY) Would send to topic {GOALS_STREAM}>{TRIAGEBOT_TOPIC}: {}", + content, + ); + return Ok(()); + } - ping_project_goals_owners( - gh, + MessageApiRequest { + recipient: Recipient::Stream { + id: GOALS_STREAM, + topic: TRIAGEBOT_TOPIC, + }, + content, + } + .send(zulip) + .await?; + + Ok(()) +} + +async fn execute_plan( + zulip: &ZulipClient, + plan: ReminderPlan<'_>, + dry_run: bool, +) -> anyhow::Result<()> { + let mut total_owners = 0; + let total_goals = plan + .goals_by_owner + .values() + .flatten() + .map(|goal| goal.issue) + .unique() + .count(); + let mut total_errors = 0; + + for (owner, goals) in plan.goals_by_owner { + send_dm(zulip, owner, &owner_message(owner, &goals), dry_run).await?; + total_owners += 1; + } + + if !plan.errors.is_empty() { + send_triagebot_topic(zulip, &error_message(&plan.errors), dry_run).await?; + + total_errors += plan.errors.unowned.len() + + plan.errors.multiply_owned.len() + + plan.errors.missing_zulip.len() + + plan.errors.invalid_schedules.len(); + } + + send_triagebot_topic( zulip, - team_api, - false, - i64::from(days_threshold), - &third_monday, + &format!( + r#" +Weekly run finished. + +{total_owners} owners have been notified about {total_goals} goals. + +{total_errors} errors happened in the process. + +Until next week! <3 + "# + ), + dry_run, ) - .await + .await?; + + Ok(()) +} + +fn is_tracking_issue(issue: &Issue) -> bool { + issue + .labels + .iter() + .any(|label| label.name == C_TRACKING_ISSUE) +} + +async fn tracking_issues(gh: &GithubClient) -> anyhow::Result> { + gh.repository(RUST_PROJECT_GOALS_REPO) + .await? + .get_issues( + gh, + &github::issue_query::Query { + filters: vec![("state", "open"), ("is", "issue")], + include_labels: vec![C_TRACKING_ISSUE], + exclude_labels: vec![], + }, + ) + .await } -/// Sends a ping message to all project goal owners if -/// they have not posted an update in the last `days_threshold` days. -/// -/// `next_update` is a human readable description of when the next update -/// will be drafted (e.g., `"on Sep 5"`). pub async fn ping_project_goals_owners( gh: &GithubClient, zulip: &ZulipClient, - team_client: &TeamClient, + team: &TeamClient, dry_run: bool, days_threshold: i64, - next_update: &str, ) -> anyhow::Result<()> { - let goals_repo = gh.repository(RUST_PROJECT_GOALS_REPO).await?; - - let tracking_issues_query = github::issue_query::Query { - filters: vec![("state", "open"), ("is", "issue")], - include_labels: vec!["C-tracking-issue"], - exclude_labels: vec![], - }; - let issues = goals_repo - .get_issues(gh, &tracking_issues_query) - .await - .with_context(|| "Unable to get issues.")?; - - for issue in issues { - let comments = issue.comments.unwrap_or(0); - - // Find the time of the last comment posted. - let days_since_last_comment = (Utc::now() - issue.updated_at).num_days(); + let issues = tracking_issues(gh).await?; + let plan = build_plan(&issues, team, days_threshold).await?; + execute_plan(zulip, plan, dry_run).await +} - // Start pinging 3 weeks after the last update. - // As a special case, if the last update was within a day of creation, that means no initial update, so ping anyway. - log::debug!( - "issue #{}: days_since_last_comment = {} days, number of comments = {}", - issue.number, - days_since_last_comment, - comments, - ); - if days_since_last_comment < days_threshold && comments > 1 { - continue; - } +pub struct ProjectGoalsUpdateJob; - let zulip_topic_name = zulip_topic_name(&issue); - let Some(zulip_owners) = zulip_owners(team_client, &issue).await? else { - log::debug!("no owners assigned"); - continue; - }; +#[async_trait] +impl Job for ProjectGoalsUpdateJob { + fn name(&self) -> &'static str { + "project_goals_update_job" + } - let message = message( - &zulip_owners, - &if comments <= 1 { - "∞".to_string() - } else { - days_since_last_comment.to_string() - }, - issue.number, - &issue.title, - next_update, - ); + async fn run(&self, ctx: &Context, _metadata: &serde_json::Value) -> anyhow::Result<()> { + let now = Utc::now(); + let days_threshold = i64::from(now.day() + 7); - let zulip_req = crate::zulip::MessageApiRequest { - recipient: Recipient::Stream { - id: GOALS_STREAM, - topic: &zulip_topic_name, - }, - content: &message, - }; + ping_project_goals_owners(&ctx.github, &ctx.zulip, &ctx.team, false, days_threshold).await + } +} - log::debug!("zulip_topic_name = {zulip_topic_name:#?}"); - log::debug!("message = {message:#?}"); +/// Returns true if the GitHub user is part of the Goals team. +pub async fn is_goals_member(team_client: &TeamClient, github_id: u64) -> anyhow::Result { + const GOALS_TEAM: &str = "goals"; - if dry_run { - eprintln!(); - eprintln!("-- Dry Run ------------------------------------"); - eprintln!("Would send to {zulip_topic_name}: {}", zulip_req.content); - } else { - zulip_req.send(zulip).await?; + let team = match team_client.get_team(GOALS_TEAM).await? { + Some(team) => team, + None => { + log::info!("team ({GOALS_TEAM}) failed to resolve to a known team"); + return Ok(false); } - } + }; - Ok(()) + Ok(team + .members + .into_iter() + .any(|member| member.github_id == github_id)) } -fn zulip_topic_name(issue: &Issue) -> String { +fn goal_zulip_topic(issue: &Issue) -> String { let goal_number = format!("(goals#{})", issue.number); let mut title = String::new(); for word in issue.title.split_whitespace() { @@ -212,38 +758,77 @@ fn zulip_topic_name(issue: &Issue) -> String { title } -async fn zulip_owners(team_client: &TeamClient, issue: &Issue) -> anyhow::Result> { - use std::fmt::Write; - - Ok(match &issue.assignees[..] { - [] => None, - [string0] => Some(owner_string(team_client, string0).await?), - [string0, string1] => Some(format!( - "{} and {}", - owner_string(team_client, string0).await?, - owner_string(team_client, string1).await? - )), - [string0 @ .., string1] => { - let mut out = String::new(); - for s in string0 { - write!(out, "{}, ", owner_string(team_client, s).await?).unwrap(); - } - write!(out, "{}, ", owner_string(team_client, string1).await?).unwrap(); - Some(out) - } - }) +async fn create_goal_topic(issue: &Issue, ctx: &Context) -> anyhow::Result<()> { + if !is_tracking_issue(issue) { + return Ok(()); + } + + let owners = Owners::resolve(&ctx.team, issue).await?; + let topic = goal_zulip_topic(issue); + let content = format!( + "Goal *{title}* (goals#{number}) has been accepted. It's owned by {owners}.", + title = issue.title, + number = issue.number, + owners = owners.all_mentions(false), + ); + + MessageApiRequest { + recipient: Recipient::Stream { + id: GOALS_STREAM, + topic: &topic, + }, + content: &content, + } + .send(&ctx.zulip) + .await?; + + Ok(()) } -async fn owner_string(team_api: &TeamClient, assignee: &GitHubUser) -> anyhow::Result { - if let Some(zulip_id) = team_api.github_to_zulip_id(assignee.id).await? { - Ok(format!("@**|{zulip_id}**")) - } else { - // No zulip-id? Fallback to github user name. - Ok(format!( - "@{login} ([register your zulip-id here to get a real ping!](https://github.com/rust-lang/team/tree/master/people/{login}.toml))", - login = assignee.login, - )) +fn quote_fence(text: &str) -> String { + let mut ticks = "````".to_owned(); + + while text.contains(&ticks) { + ticks.push('`'); } + + ticks +} + +async fn echo_comment_to_zulip( + issue: &Issue, + comment: &github::Comment, + ctx: &Context, +) -> anyhow::Result<()> { + if !is_tracking_issue(issue) { + return Ok(()); + } + + let author = Owner::resolve(&ctx.team, comment.user.id, &comment.user.login).await?; + let text = &comment.body; + + let content = format!( + "[Comment posted]({url}) on goals#{number} by {author}:\n\ + {ticks}quote\n\ + {text}\n\ + {ticks}", + url = comment.html_url, + number = issue.number, + author = author.display_mention(false), + ticks = quote_fence(&text), + ); + + MessageApiRequest { + recipient: Recipient::Stream { + id: GOALS_STREAM, + topic: &goal_zulip_topic(issue), + }, + content: &content, + } + .send(&ctx.zulip) + .await?; + + Ok(()) } pub async fn handle(ctx: &Context, event: &Event) -> anyhow::Result<()> { @@ -252,93 +837,19 @@ pub async fn handle(ctx: &Context, event: &Event) -> anyhow::Result<()> { } match event { - // When a new issue is opened that is tagged as a tracking issue, - // automatically create a Zulip topic for it and post a comment to the issue. Event::Issue(IssuesEvent { action: IssuesAction::Opened, issue, .. - }) => { - if !issue.labels.iter().any(|l| l.name == C_TRACKING_ISSUE) { - return Ok(()); - } - let zulip_topic_name = zulip_topic_name(issue); - let zulip_owners = zulip_owners(&ctx.team, issue).await?; - let zulip_owners = zulip_owners.as_deref().unwrap_or("(no owners assigned)"); - let title = &issue.title; - let goalnum = issue.number; - let zulip_req = crate::zulip::MessageApiRequest { - recipient: Recipient::Stream { - id: GOALS_STREAM, - topic: &zulip_topic_name, - }, - content: &format!( - r"New tracking issue goals#{goalnum}.\n* Goal title: {title}\n* Goal owners: {zulip_owners}" - ), - }; - zulip_req.send(&ctx.zulip).await?; - Ok(()) - } + }) => create_goal_topic(issue, ctx).await, - // When a new comment is posted on a tracking issue, post it to Zulip. Event::IssueComment(IssueCommentEvent { - action, + action: IssueCommentAction::Created, issue, comment, .. - }) => { - // Only comments on tracking issues should be forwarded to Zulip. - if !issue.labels.iter().any(|l| l.name == C_TRACKING_ISSUE) { - return Ok(()); - } - - let number = issue.number; - let action_str = match action { - IssueCommentAction::Created => "posted", - - // Don't spam for updates, deletes - _ => return Ok(()), - }; - let zulip_topic_name = zulip_topic_name(issue); - let url = &comment.html_url; - let text = &comment.body; - let zulip_author = owner_string(&ctx.team, &comment.user).await?; - - let mut ticks = "````".to_string(); - while text.contains(&ticks) { - ticks.push('`'); - } - - match action { - IssueCommentAction::Created | IssueCommentAction::Edited => { - let zulip_req = crate::zulip::MessageApiRequest { - recipient: Recipient::Stream { - id: GOALS_STREAM, - topic: &zulip_topic_name, - }, - content: &format!( - "[Comment {action_str}]({url}) on goals#{number} by {zulip_author}:\n\ - {ticks}quote\n\ - {text}\n\ - {ticks}" - ), - }; - zulip_req.send(&ctx.zulip).await?; - } + }) => echo_comment_to_zulip(issue, comment, ctx).await, - IssueCommentAction::Deleted - | IssueCommentAction::Pinned - | IssueCommentAction::Unpinned => { - // Do we really care? - } - } - - Ok(()) - } - - _ => { - /* No action for other cases */ - Ok(()) - } + _ => Ok(()), } } diff --git a/src/jobs.rs b/src/jobs.rs index e2d23c104..de302ef45 100644 --- a/src/jobs.rs +++ b/src/jobs.rs @@ -50,6 +50,7 @@ use std::str::FromStr; use async_trait::async_trait; use cron::Schedule; +use crate::handlers::project_goals::ProjectGoalsUpdateJob; use crate::handlers::pull_requests_assignment_update::PullRequestAssignmentUpdate; use crate::{ db::jobs::JobSchedule, @@ -107,6 +108,12 @@ pub fn default_jobs() -> Vec { schedule: Schedule::from_str("* */15 * * * * *").unwrap(), metadata: serde_json::Value::Null, }, + JobSchedule { + name: ProjectGoalsUpdateJob.name(), + // Around 9am Pacific time on every Monday. + schedule: Schedule::from_str("0 00 17 * * Mon *").unwrap(), + metadata: serde_json::Value::Null, + }, ] } diff --git a/src/zulip.rs b/src/zulip.rs index 16fcf1b9d..3f013b596 100644 --- a/src/zulip.rs +++ b/src/zulip.rs @@ -638,7 +638,7 @@ async fn ping_goals_cmd( message: &Message, args: &PingGoalsArgs, ) -> anyhow::Result> { - if project_goals::check_project_goal_acl(&ctx.team, gh_id).await? { + if project_goals::is_goals_member(&ctx.team, gh_id).await? { let args = args.clone(); let message = message.clone(); tokio::spawn(async move { @@ -648,7 +648,6 @@ async fn ping_goals_cmd( &ctx.team, false, args.threshold as i64, - &format!("on {}", args.next_update), ) .await; diff --git a/src/zulip/commands.rs b/src/zulip/commands.rs index 64785e3f2..cceae91c9 100644 --- a/src/zulip/commands.rs +++ b/src/zulip/commands.rs @@ -191,10 +191,8 @@ pub enum StreamCommand { #[derive(clap::Parser, Debug, PartialEq, Clone)] pub struct PingGoalsArgs { - /// Number of days before an update is considered stale + /// Goals updated within this threshold (in days) will not be pinged. pub threshold: u64, - /// Date of next update - pub next_update: String, } /// Backport release channels From 55ce9a3fac7cffdd054a8bd3e499bcc631db66b3 Mon Sep 17 00:00:00 2001 From: nxsaken Date: Thu, 30 Jul 2026 01:42:01 +0400 Subject: [PATCH 2/9] Add job to list --- src/jobs.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/jobs.rs b/src/jobs.rs index de302ef45..85b714b34 100644 --- a/src/jobs.rs +++ b/src/jobs.rs @@ -78,6 +78,7 @@ pub fn jobs() -> Vec> { Box::new(MajorChangeAcceptanceJob), Box::new(GithubRateLimitLoggingJob), Box::new(AddReviewChangesSinceLinkJob), + Box::new(ProjectGoalsUpdateJob), ] } From 94c0b60091db07c15d0901ae4e726a6246836fe2 Mon Sep 17 00:00:00 2001 From: nxsaken Date: Thu, 30 Jul 2026 01:45:06 +0400 Subject: [PATCH 3/9] Change to meta channel --- src/handlers/project_goals.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/handlers/project_goals.rs b/src/handlers/project_goals.rs index f2fd794af..60a593b8a 100644 --- a/src/handlers/project_goals.rs +++ b/src/handlers/project_goals.rs @@ -18,6 +18,7 @@ const RUST_PROJECT_GOALS_REPO: &str = "rust-lang/rust-project-goals"; const C_TRACKING_ISSUE: &str = "C-tracking-issue"; const GOALS_STREAM: u64 = 435_869; // #project-goals +const GOALS_META_STREAM: u64 = 478_266; // #project-goals/meta const TRIAGEBOT_TOPIC: &str = "Triagebot reports"; const MAX_ZULIP_TOPIC: usize = 60; @@ -616,7 +617,7 @@ async fn send_triagebot_topic( MessageApiRequest { recipient: Recipient::Stream { - id: GOALS_STREAM, + id: GOALS_META_STREAM, topic: TRIAGEBOT_TOPIC, }, content, From a5ffdfbce0f4fe081c043efc1ca51259b5bbb125 Mon Sep 17 00:00:00 2001 From: nxsaken Date: Thu, 30 Jul 2026 01:48:26 +0400 Subject: [PATCH 4/9] Remove previous scheduling --- src/bin/project_goals.rs | 12 +--------- src/handlers/project_goals.rs | 44 ++++++++--------------------------- src/zulip.rs | 9 +------ 3 files changed, 12 insertions(+), 53 deletions(-) diff --git a/src/bin/project_goals.rs b/src/bin/project_goals.rs index 0c7b5200c..cc85c8bcd 100644 --- a/src/bin/project_goals.rs +++ b/src/bin/project_goals.rs @@ -9,9 +9,6 @@ struct Opt { /// If specified, no messages are sent. #[arg(long)] dry_run: bool, - - /// Goals updated within this threshold (in days) will not be pinged. - days_threshold: i64, } #[tokio::main(flavor = "current_thread")] @@ -23,14 +20,7 @@ async fn main() -> anyhow::Result<()> { let gh = GithubClient::new_from_env(); let zulip = ZulipClient::new_from_env(); let team_api = TeamClient::new_from_env(); - project_goals::ping_project_goals_owners( - &gh, - &zulip, - &team_api, - opt.dry_run, - opt.days_threshold, - ) - .await?; + project_goals::ping_project_goals_owners(&gh, &zulip, &team_api, opt.dry_run).await?; Ok(()) } diff --git a/src/handlers/project_goals.rs b/src/handlers/project_goals.rs index 60a593b8a..2901b14e9 100644 --- a/src/handlers/project_goals.rs +++ b/src/handlers/project_goals.rs @@ -500,18 +500,7 @@ Hi @*T-goals*! ) } -fn default_update_required(issue: &Issue, now: DateTime, days_threshold: i64) -> bool { - let comments = issue.comments.unwrap_or(0); - let days_since_last_update = (now - issue.updated_at).num_days(); - - days_since_last_update >= days_threshold || comments <= 1 -} - -fn scheduled_update_required( - issue: &Issue, - now: DateTime, - schedule: CustomSchedule, -) -> bool { +fn update_required(issue: &Issue, now: DateTime, schedule: CustomSchedule) -> bool { let due_date = schedule.latest_due_date(now.date_naive()); let has_real_update = issue.comments.unwrap_or(0) > 1; let updated_after_due_date = issue.updated_at.date_naive() >= due_date; @@ -519,11 +508,7 @@ fn scheduled_update_required( !has_real_update || !updated_after_due_date } -fn evaluate<'gh>( - issue: &'gh Issue, - now: DateTime, - days_threshold: i64, -) -> EvaluatedReminder<'gh> { +fn evaluate<'gh>(issue: &'gh Issue, now: DateTime) -> EvaluatedReminder<'gh> { let days_since_last_update = (now - issue.updated_at).num_days(); log::debug!( @@ -534,15 +519,11 @@ fn evaluate<'gh>( ); let (requires_update, invalid_schedule_reason) = match Schedule::from_issue(issue) { - Schedule::Default => ( - default_update_required(issue, now, days_threshold), - None, - ), - Schedule::Custom(schedule) => (scheduled_update_required(issue, now, schedule), None), - Schedule::Invalid { fallback, reason } => ( - scheduled_update_required(issue, now, fallback), - Some(reason), - ), + Schedule::Default => (update_required(issue, now, CustomSchedule::Biweekly0), None), + Schedule::Custom(schedule) => (update_required(issue, now, schedule), None), + Schedule::Invalid { fallback, reason } => { + (update_required(issue, now, fallback), Some(reason)) + } }; EvaluatedReminder { @@ -555,13 +536,12 @@ fn evaluate<'gh>( async fn build_plan<'gh>( issues: &'gh [Issue], team: &TeamClient, - days_threshold: i64, ) -> anyhow::Result> { let now = Utc::now(); let mut plan = ReminderPlan::default(); for issue in issues { - let evaluation = evaluate(issue, now, days_threshold); + let evaluation = evaluate(issue, now); if let Some(reason) = evaluation.invalid_schedule_reason { plan.add_invalid_schedule(evaluation.reminder, reason); @@ -703,10 +683,9 @@ pub async fn ping_project_goals_owners( zulip: &ZulipClient, team: &TeamClient, dry_run: bool, - days_threshold: i64, ) -> anyhow::Result<()> { let issues = tracking_issues(gh).await?; - let plan = build_plan(&issues, team, days_threshold).await?; + let plan = build_plan(&issues, team).await?; execute_plan(zulip, plan, dry_run).await } @@ -719,10 +698,7 @@ impl Job for ProjectGoalsUpdateJob { } async fn run(&self, ctx: &Context, _metadata: &serde_json::Value) -> anyhow::Result<()> { - let now = Utc::now(); - let days_threshold = i64::from(now.day() + 7); - - ping_project_goals_owners(&ctx.github, &ctx.zulip, &ctx.team, false, days_threshold).await + ping_project_goals_owners(&ctx.github, &ctx.zulip, &ctx.team, false).await } } diff --git a/src/zulip.rs b/src/zulip.rs index 3f013b596..c9f8b25ba 100644 --- a/src/zulip.rs +++ b/src/zulip.rs @@ -642,14 +642,7 @@ async fn ping_goals_cmd( let args = args.clone(); let message = message.clone(); tokio::spawn(async move { - let res = ping_project_goals_owners( - &ctx.github, - &ctx.zulip, - &ctx.team, - false, - args.threshold as i64, - ) - .await; + let res = ping_project_goals_owners(&ctx.github, &ctx.zulip, &ctx.team, false).await; let status = match res { Ok(_res) => "OK".to_string(), From 71f372c9bfd8b762b0743e928cf9c4a29e5ef8b0 Mon Sep 17 00:00:00 2001 From: nxsaken Date: Thu, 30 Jul 2026 17:58:07 +0400 Subject: [PATCH 5/9] Query open goal tracking issues with the last comment metadata --- src/github/queries/mod.rs | 1 + src/github/queries/open_goal_issues.rs | 203 +++++++++++++++++++++++++ 2 files changed, 204 insertions(+) create mode 100644 src/github/queries/open_goal_issues.rs diff --git a/src/github/queries/mod.rs b/src/github/queries/mod.rs index a1bfa2f81..36e8b009b 100644 --- a/src/github/queries/mod.rs +++ b/src/github/queries/mod.rs @@ -1,5 +1,6 @@ pub(crate) mod issue_with_comments; pub(crate) mod least_recently_reviewed; +pub(crate) mod open_goal_issues; pub(crate) mod recent_commits; pub(crate) mod user_comments_in_org; pub(crate) mod user_contributions; diff --git a/src/github/queries/open_goal_issues.rs b/src/github/queries/open_goal_issues.rs new file mode 100644 index 000000000..8d8708fc8 --- /dev/null +++ b/src/github/queries/open_goal_issues.rs @@ -0,0 +1,203 @@ +use anyhow::Context; + +use crate::github::GithubClient; + +const ORG: &str = "rust-lang"; +const REPO: &str = "rust-project-goals"; +const LABEL: &str = "C-tracking-issue"; + +pub struct GoalIssue { + pub number: u64, + pub title: String, + pub assignees: Vec, + pub created_at: chrono::DateTime, + pub labels: Vec, + pub last_comment: Option, +} + +pub struct LastGoalComment { + pub created_at: chrono::DateTime, + pub author: Option, +} + +#[derive(serde::Deserialize)] +struct GraphQlConnection { + nodes: Vec>, + #[serde(rename = "pageInfo")] + page_info: GraphQlPageInfo, +} + +#[derive(serde::Deserialize)] +struct GraphQlPageInfo { + #[serde(rename = "hasNextPage")] + has_next_page: bool, + #[serde(rename = "endCursor")] + end_cursor: Option, +} + +#[derive(serde::Deserialize)] +struct GraphQlIssue { + number: u64, + title: String, + #[serde(rename = "createdAt")] + created_at: chrono::DateTime, + assignees: GraphQlNodes, + labels: Option>, + comments: GraphQlNodes, +} + +#[derive(serde::Deserialize)] +struct GraphQlNodes { + nodes: Vec>, +} + +#[derive(serde::Deserialize)] +struct GraphQlUser { + login: String, +} + +#[derive(serde::Deserialize)] +struct GraphQlLabel { + name: String, +} + +#[derive(serde::Deserialize)] +struct GraphQlComment { + #[serde(rename = "createdAt")] + created_at: chrono::DateTime, + author: Option, +} + +impl From for GoalIssue { + fn from(issue: GraphQlIssue) -> Self { + let assignees = issue + .assignees + .nodes + .into_iter() + .flatten() + .map(|assignee| assignee.login) + .collect(); + + let labels = issue + .labels + .map(|labels| { + labels + .nodes + .into_iter() + .flatten() + .map(|label| label.name) + .collect() + }) + .unwrap_or_default(); + + let last_comment = issue + .comments + .nodes + .into_iter() + .flatten() + .next() + .map(|comment| LastGoalComment { + created_at: comment.created_at, + author: comment.author.map(|author| author.login), + }); + + Self { + number: issue.number, + title: issue.title, + assignees, + created_at: issue.created_at, + labels, + last_comment, + } + } +} + +impl GithubClient { + /// Get every open tracking issue in `rust-lang/rust-project-goals`, + /// including the latest comment's date and author. + pub async fn open_goal_issues(&self) -> anyhow::Result> { + let mut cursor = None::; + let mut issues = Vec::new(); + + loop { + let mut response = self + .graphql_query( + r#" +query ( + $owner: String! + $repo: String! + $label: String! + $cursor: String +) { + repository(owner: $owner, name: $repo) { + issues( + first: 100 + after: $cursor + states: [OPEN] + labels: [$label] + orderBy: { + field: CREATED_AT + direction: ASC + } + ) { + nodes { + number + title + createdAt + assignees(first: 100) { + nodes { + login + } + } + labels(first: 100) { + nodes { + name + } + } + comments(last: 1) { + nodes { + createdAt + author { + login + } + } + } + } + pageInfo { + hasNextPage + endCursor + } + } + } +} +"#, + serde_json::json!({ + "owner": ORG, + "repo": REPO, + "label": LABEL, + "cursor": cursor.as_deref(), + }), + ) + .await + .context("failed to fetch goal issues")?; + + let page = response + .pointer_mut("/data/repository/issues") + .context("data.repository.issues is missing from response")? + .take(); + + let page: GraphQlConnection = + serde_json::from_value(page).context("failed to deserialize page")?; + + issues.extend(page.nodes.into_iter().flatten().map(GoalIssue::from)); + + if !page.page_info.has_next_page { + break; + } + + cursor = page.page_info.end_cursor; + } + + Ok(issues) + } +} From 6e61a9c7ed1ddb29f238172f78e0ea77dbc30071 Mon Sep 17 00:00:00 2001 From: nxsaken Date: Fri, 31 Jul 2026 01:45:24 +0400 Subject: [PATCH 6/9] Improve scheduling and messaging, run job on Thursdays --- src/handlers/project_goals.rs | 896 ++++++++++++++++++++-------------- src/jobs.rs | 4 +- 2 files changed, 541 insertions(+), 359 deletions(-) diff --git a/src/handlers/project_goals.rs b/src/handlers/project_goals.rs index 2901b14e9..7cd778ee3 100644 --- a/src/handlers/project_goals.rs +++ b/src/handlers/project_goals.rs @@ -1,7 +1,7 @@ use crate::{ github::{ self, Event, GithubClient, Issue, IssueCommentAction, IssueCommentEvent, IssuesAction, - IssuesEvent, + IssuesEvent, queries::open_goal_issues::GoalIssue, }, handlers::Context, jobs::Job, @@ -9,19 +9,34 @@ use crate::{ zulip::{MessageApiRequest, api::Recipient, client::ZulipClient}, }; use async_trait::async_trait; -use chrono::{DateTime, Datelike, Duration, NaiveDate, Utc}; +use chrono::{DateTime, Datelike, Duration, NaiveDate, Utc, Weekday}; use itertools::Itertools; use std::collections::BTreeMap; use tracing as log; const RUST_PROJECT_GOALS_REPO: &str = "rust-lang/rust-project-goals"; -const C_TRACKING_ISSUE: &str = "C-tracking-issue"; +const GOALS_TEAM: &str = "goals"; + +const FIRST_REPORT_GRACE_DAYS: i64 = 7; +const REPORT_LABELS: &[(&str, Period)] = &[ + ("R-every-week", Period::EveryWeek), + ("R-every-2-weeks", Period::Every2Weeks), + ("R-every-4-weeks", Period::Every4Weeks), +]; const GOALS_STREAM: u64 = 435_869; // #project-goals const GOALS_META_STREAM: u64 = 478_266; // #project-goals/meta -const TRIAGEBOT_TOPIC: &str = "Triagebot reports"; +const TRIAGEBOT_TOPIC: &str = "triagebot reports"; const MAX_ZULIP_TOPIC: usize = 60; +// Keep these in sync with src/jobs.rs +const JOB_WEEKDAY: Weekday = Weekday::Thu; +const JOB_UTC_HOUR: u32 = 14; +const JOB_UTC_MINUTE: u32 = 0; + +// Arbitrary date to keep cycles anchored. +const EPOCH: NaiveDate = NaiveDate::from_ymd_opt(1970, 1, 1).unwrap(); + #[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] struct ZulipId(u64); @@ -40,10 +55,10 @@ struct GhUsername<'gh>(&'gh str); impl<'gh> GhUsername<'gh> { fn link(self) -> String { - format!("[@{login}](https://github.com/{login})", login = self.0,) + format!("[@{login}](https://github.com/{login})", login = self.0) } - fn team_file_link(self) -> String { + fn team_link(self) -> String { format!( "[@{login}](https://github.com/rust-lang/team/tree/main/people/{login}.toml)", login = self.0, @@ -51,52 +66,81 @@ impl<'gh> GhUsername<'gh> { } } +#[derive(Clone, Copy, Debug)] +enum OwnerContact { + Reachable(ZulipId), + MissingZulipId, + MissingTeamEntry, +} + #[derive(Clone, Copy, Debug)] struct Owner<'gh> { github: GhUsername<'gh>, - zulip: Option, + contact: OwnerContact, } impl<'gh> Owner<'gh> { - async fn resolve( + async fn resolve_goal(team: &TeamClient, username: &'gh str) -> anyhow::Result { + Ok(Self { + github: GhUsername(username), + contact: match team.get_gh_id_from_username(username).await? { + Some(gh_id) => match team.github_to_zulip_id(gh_id).await? { + Some(zulip_id) => OwnerContact::Reachable(ZulipId(zulip_id)), + None => OwnerContact::MissingZulipId, + }, + None => OwnerContact::MissingTeamEntry, + }, + }) + } + + async fn resolve_event( team: &TeamClient, - github_id: u64, + gh_id: u64, username: &'gh str, ) -> anyhow::Result { - let zulip = team.github_to_zulip_id(github_id).await?.map(ZulipId); Ok(Self { github: GhUsername(username), - zulip, + contact: match team.github_to_zulip_id(gh_id).await? { + Some(zulip_id) => OwnerContact::Reachable(ZulipId(zulip_id)), + None => OwnerContact::MissingZulipId, + }, }) } fn display_mention(self, muted: bool) -> String { - match self.zulip { - Some(zulip_id) => zulip_id.mention(muted), - None => self.github.link(), + match self.contact { + OwnerContact::Reachable(zulip_id) => zulip_id.mention(muted), + OwnerContact::MissingZulipId | OwnerContact::MissingTeamEntry => self.github.link(), } } } -#[derive(Clone, Debug)] -struct Owners<'gh>(Vec>); - -fn join_mentions(mentions: Vec) -> String { - match mentions.as_slice() { - [] => "(none assigned)".to_owned(), +fn join_mentions(mentions: Vec) -> Option { + let joined = match mentions.as_slice() { + [] => return None, [owner] => owner.clone(), [first, second] => format!("{first} and {second}"), - [rest @ .., last] => { - format!("{}, and {last}", rest.iter().join(", ")) - } - } + [rest @ .., last] => format!("{}, and {last}", rest.iter().join(", ")), + }; + Some(joined) } +#[derive(Debug)] +struct Owners<'gh>(Vec>); + impl<'gh> Owners<'gh> { - async fn resolve(team: &TeamClient, issue: &'gh Issue) -> anyhow::Result { + async fn resolve_goal(team: &TeamClient, issue: &'gh GoalIssue) -> anyhow::Result { + let mut owners = Vec::with_capacity(issue.assignees.len()); + for username in &issue.assignees { + owners.push(Owner::resolve_goal(team, username).await?); + } + Ok(Self(owners)) + } + + async fn resolve_event(team: &TeamClient, issue: &'gh Issue) -> anyhow::Result { let mut owners = Vec::with_capacity(issue.assignees.len()); for assignee in &issue.assignees { - owners.push(Owner::resolve(team, assignee.id, &assignee.login).await?); + owners.push(Owner::resolve_event(team, assignee.id, &assignee.login).await?); } Ok(Self(owners)) } @@ -109,217 +153,306 @@ impl<'gh> Owners<'gh> { self.0.len() > 1 } - fn reachable(&self) -> impl Iterator + '_ { - self.0.iter().copied().filter_map(|owner| owner.zulip) + fn has_missing_zulip_id(&self) -> bool { + self.0 + .iter() + .any(|owner| matches!(owner.contact, OwnerContact::MissingZulipId)) } - fn unreachable(&self) -> impl Iterator> + '_ { + fn has_missing_team_entry(&self) -> bool { self.0 .iter() - .copied() - .filter_map(|owner| owner.zulip.is_none().then_some(owner.github)) + .any(|owner| matches!(owner.contact, OwnerContact::MissingTeamEntry)) + } + + fn has_problem(&self) -> bool { + self.has_multiple() || self.has_missing_zulip_id() || self.has_missing_team_entry() } - fn all_mentions(&self, muted: bool) -> String { + fn reachable(&self) -> impl Iterator + '_ { + self.0.iter().filter_map(|owner| match owner.contact { + OwnerContact::Reachable(zulip_id) => Some(zulip_id), + OwnerContact::MissingZulipId | OwnerContact::MissingTeamEntry => None, + }) + } + + fn missing_zulip_ids(&self) -> impl Iterator> + '_ { + self.0.iter().filter_map(|owner| { + matches!(owner.contact, OwnerContact::MissingZulipId).then_some(owner.github) + }) + } + + fn missing_team_entries(&self) -> impl Iterator> + '_ { + self.0.iter().filter_map(|owner| { + matches!(owner.contact, OwnerContact::MissingTeamEntry).then_some(owner.github) + }) + } + + fn all_mentions(&self, muted: bool) -> Option { join_mentions( self.0 .iter() .copied() - .map(|o| o.display_mention(muted)) + .map(|owner| owner.display_mention(muted)) .collect_vec(), ) } fn reachable_mentions(&self) -> Option { - let reachables = self.reachable().map(|id| id.mention(true)).collect_vec(); - if reachables.is_empty() { - None - } else { - Some(join_mentions(reachables)) - } + join_mentions(self.reachable().map(|id| id.mention(true)).collect_vec()) + } + + fn missing_zulip_team_links(&self) -> Option { + join_mentions( + self.missing_zulip_ids() + .map(GhUsername::team_link) + .collect_vec(), + ) } - fn unreachable_team_links(&self) -> String { + fn missing_team_entry_links(&self) -> Option { join_mentions( - self.unreachable() - .map(GhUsername::team_file_link) + self.missing_team_entries() + .map(GhUsername::link) .collect_vec(), ) } } -#[derive(Clone, Copy, Debug)] -enum LastUpdate { - Never, - DaysAgo(i64), +#[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd)] +enum Period { + /// Start a new reporting period every week. + EveryWeek = 0, + /// Start a new reporting period every 2 weeks. + Every2Weeks = 1, + /// Start a new reporting period every 4 weeks. + Every4Weeks = 2, } -impl LastUpdate { - fn description(self) -> String { +impl Period { + fn weeks(self) -> i64 { match self { - Self::Never => "no updates so far".to_owned(), - Self::DaysAgo(days) => format!("last update was {days} days ago"), + Self::EveryWeek => 1, + Self::Every2Weeks => 2, + Self::Every4Weeks => 4, } } -} - -#[derive(Copy, Clone, Debug)] -struct Reminder<'gh> { - issue: u64, - title: &'gh str, - last_update: LastUpdate, -} -struct EvaluatedReminder<'gh> { - reminder: Reminder<'gh>, - requires_update: bool, - invalid_schedule_reason: Option, -} - -impl<'gh> Reminder<'gh> { - fn from_issue(issue: &'gh Issue, days_since_last_update: i64) -> Self { - let last_update = if issue.comments.unwrap_or(0) <= 1 { - LastUpdate::Never - } else { - LastUpdate::DaysAgo(days_since_last_update) - }; - Self { - issue: issue.number, - title: &issue.title, - last_update, + fn adjective(self) -> &'static str { + match self { + Self::EveryWeek => "weekly", + Self::Every2Weeks => "biweekly", + Self::Every4Weeks => "4-week", } } - fn list_item(&self) -> String { - format!( - "+ *{title}* (goals#{issue}) — {last_update}", - title = self.title, - issue = self.issue, - last_update = self.last_update.description(), - ) + fn start(self, today: NaiveDate) -> NaiveDate { + let days_until_job = + (JOB_WEEKDAY.num_days_from_monday() + 7 - EPOCH.weekday().num_days_from_monday()) % 7; + + let anchor = EPOCH + Duration::days(i64::from(days_until_job)); + + let weeks_since_anchor = today.signed_duration_since(anchor).num_weeks(); + let period_weeks = self.weeks(); + let periods_since_anchor = weeks_since_anchor.div_euclid(period_weeks); + + anchor + Duration::weeks(periods_since_anchor * period_weeks) } - fn reference(&self) -> String { - format!( - "*{title}* (goals#{issue})", - title = self.title, - issue = self.issue, - ) + fn next(self, period_start: NaiveDate) -> NaiveDate { + period_start + Duration::weeks(self.weeks()) } } -#[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd)] -enum CustomSchedule { - /// Runs on every job invocation (every week). - Weekly = 0, - /// Runs during even-numbered ISO weeks. - Biweekly0 = 1, - /// Runs during odd-numbered ISO weeks. - Biweekly1 = 2, - /// Runs on the first job invocation of each month. - Monthly = 3, -} - -#[derive(Clone, Debug)] -enum Schedule { - Default, - Custom(CustomSchedule), - Invalid { - fallback: CustomSchedule, - reason: String, - }, +#[derive(Debug)] +struct Schedule { + period: Period, + conflict: Option, } impl Schedule { - fn from_issue(issue: &Issue) -> Self { - const PING_FREQUENCY_LABELS: &[(&str, CustomSchedule)] = &[ - ("P-weekly", CustomSchedule::Weekly), - ("P-biweekly-0", CustomSchedule::Biweekly0), - ("P-biweekly-1", CustomSchedule::Biweekly1), - ("P-monthly", CustomSchedule::Monthly), - ]; - - match PING_FREQUENCY_LABELS + fn from_issue(issue: &GoalIssue) -> Self { + let selected = REPORT_LABELS .iter() - .filter_map(|&(label, value)| { + .filter_map(|&(label, period)| { issue .labels .iter() - .any(|l| l.name == label) - .then_some((label, value)) + .any(|issue_label| issue_label == label) + .then_some((label, period)) }) - .collect::>() - .as_slice() - { - [] => Self::Default, - [(_, frequency)] => Self::Custom(*frequency), + .collect_vec(); + + match selected.as_slice() { + [] => Self { + period: Period::Every4Weeks, + conflict: None, + }, + [(_, period)] => Self { + period: *period, + conflict: None, + }, multiple => { - let (fallback_label, fallback) = multiple + let (minimal_label, minimal) = multiple .iter() .copied() - .min_by_key(|(_, schedule)| *schedule) + .min_by_key(|(_, period)| *period) .expect("multiple contains at least two schedules"); - - Self::Invalid { - fallback, - reason: format!( - "multiple frequency labels are set: {}; falling back to `{fallback_label}`", - multiple.iter().map(|&(label, _)| label).join(", "), - ), + Self { + period: minimal, + conflict: Some(format!( + "{} (`{minimal_label}` was used)", + multiple + .iter() + .map(|(label, _)| format!("`{label}`")) + .join(", "), + )), } } } } } -fn latest_biweekly_due_date(today: NaiveDate, parity: bool) -> NaiveDate { - if today.iso_week().week() % 2 == parity as u32 { - today - } else { - today - Duration::weeks(1) - } +#[derive(Clone, Copy, Debug)] +struct Goal<'gh> { + issue: u64, + title: &'gh str, + created_at: DateTime, + last_comment_at: Option>, } -impl CustomSchedule { - fn latest_due_date(self, today: NaiveDate) -> NaiveDate { - match self { - Self::Weekly => today, - Self::Biweekly0 => latest_biweekly_due_date(today, false), - Self::Biweekly1 => latest_biweekly_due_date(today, true), - Self::Monthly => { - let weeks_since_first_run = today.day0() / 7; - today - Duration::weeks(i64::from(weeks_since_first_run)) +impl<'gh> Goal<'gh> { + fn from_issue(issue: &'gh GoalIssue) -> Self { + Self { + issue: issue.number, + title: &issue.title, + created_at: issue.created_at, + last_comment_at: issue.last_comment.as_ref().map(|c| c.created_at), + } + } + + fn link(&self) -> String { + format!("goals#{number}", number = self.issue) + } + + fn named_link(&self) -> String { + format!( + "**{title}** (goals#{number})", + title = self.title, + number = self.issue + ) + } + + fn latest_update(&self) -> String { + match self.last_comment_at { + None => format!( + "goal started: {} (no updates so far)", + display_datetime(self.created_at) + ), + Some(dt) => { + format!("latest update: {}", display_datetime(dt)) } } } } -#[derive(Clone, Debug)] -struct MultipleOwners<'gh> { - goal: Reminder<'gh>, +fn display_job_date(date: NaiveDate) -> String { + format!( + "", + date.format("%Y-%m-%d"), + ) +} + +fn display_datetime(date: DateTime) -> String { + format!("", date.format("%Y-%m-%dT%H:%M%:z"),) +} + +#[derive(Clone, Copy, Debug)] +struct Reminder<'gh> { + goal: Goal<'gh>, + period: Period, + period_start: NaiveDate, + next_deadline: NaiveDate, + _is_new_period: bool, +} + +impl<'gh> Reminder<'gh> { + fn from_issue(issue: &'gh GoalIssue, now: DateTime) -> (Self, Option) { + let schedule = Schedule::from_issue(issue); + let today = now.date_naive(); + let period_start = schedule.period.start(today); + ( + Self { + goal: Goal::from_issue(issue), + period: schedule.period, + period_start, + next_deadline: schedule.period.next(period_start), + _is_new_period: period_start == today, + }, + schedule.conflict, + ) + } + + fn is_required(&self, now: DateTime) -> bool { + // Give new goals a grace period before reminders begin. + let grace_end = self.goal.created_at + Duration::days(FIRST_REPORT_GRACE_DAYS); + + if now < grace_end { + return false; + } + + self.goal + .last_comment_at + .is_none_or(|d| d.date_naive() < self.period_start) + } + + fn list_item(&self) -> String { + format!( + "+ {goal}\n - {latest}\n - next *{period}* cycle starts {next}", + goal = self.goal.named_link(), + latest = self.goal.latest_update(), + next = display_job_date(self.next_deadline), + period = self.period.adjective(), + ) + } +} + +#[derive(Debug)] +struct OwnershipProblem<'gh> { + goal: Goal<'gh>, owners: Owners<'gh>, } -#[derive(Clone, Debug)] -struct InvalidSchedule<'gh> { - goal: Reminder<'gh>, +#[derive(Debug)] +struct PeriodConflict<'gh> { + goal: Goal<'gh>, reason: String, } #[derive(Default)] struct ReminderErrors<'gh> { - unowned: Vec>, - multiply_owned: Vec>, - missing_zulip: Vec>, - invalid_schedules: Vec>, + unowned: Vec>, + ownership: Vec>, + schedule: Vec>, } impl ReminderErrors<'_> { fn is_empty(&self) -> bool { - self.unowned.is_empty() - && self.multiply_owned.is_empty() - && self.missing_zulip.is_empty() - && self.invalid_schedules.is_empty() + self.unowned.is_empty() && self.ownership.is_empty() && self.schedule.is_empty() + } + + fn count(&self) -> usize { + self.unowned.len() + + self.schedule.len() + + self + .ownership + .iter() + .map(|problem| { + problem.owners.has_multiple() as usize + + problem.owners.has_missing_zulip_id() as usize + + problem.owners.has_missing_team_entry() as usize + }) + .sum::() } } @@ -330,36 +463,25 @@ struct ReminderPlan<'gh> { } impl<'gh> ReminderPlan<'gh> { - fn add_invalid_schedule(&mut self, goal: Reminder<'gh>, reason: String) { - self.errors - .invalid_schedules - .push(InvalidSchedule { goal, reason }); + fn add_conflicts(&mut self, goal: Goal<'gh>, reason: String) { + self.errors.schedule.push(PeriodConflict { goal, reason }); } - fn add_goal(&mut self, goal: Reminder<'gh>, owners: Owners<'gh>) { + fn add_goal(&mut self, reminder: Reminder<'gh>, owners: Owners<'gh>) { if owners.is_empty() { - self.errors.unowned.push(goal); + self.errors.unowned.push(reminder.goal); return; } - let goal_with_owners = MultipleOwners { - goal: goal.clone(), - owners: owners.clone(), - }; - - if owners.has_multiple() { - self.errors.multiply_owned.push(goal_with_owners.clone()); - } - - if owners.unreachable().next().is_some() { - self.errors.missing_zulip.push(goal_with_owners); + for owner in owners.reachable() { + self.goals_by_owner.entry(owner).or_default().push(reminder); } - for owner in owners.reachable() { - self.goals_by_owner - .entry(owner) - .or_default() - .push(goal.clone()); + if owners.has_problem() { + self.errors.ownership.push(OwnershipProblem { + goal: reminder.goal, + owners, + }); } } } @@ -369,190 +491,206 @@ fn owner_message(owner: ZulipId, goals: &[Reminder<'_>]) -> String { r#" Hi {owner}! -This is a reminder to post an update on your goals: +This is your reminder to post updates for the following goals: {goals} -Some questions to guide you (you don't have to follow this format): +Some questions to guide you (you don't have to follow this): + What has happened since your last update? + Are there any relevant PRs, issues, docs, or discussions to link? -+ Are you blocked on any issue, PR, teams? ++ Are you blocked on any issue, PR, or team? + Do you need help or feedback? Where should people look? + What do you plan to work on before the next update? Even if there's little to say, a brief message provides reassurance that the goal is still alive. Please leave your updates as comments on the tracking issues. Thanks! <3 + +--- + +*Note: Two- and four-week goals are pinged weekly until an update is posted for the current reporting period.* + +*By default, the reporting period is 4 weeks. If you'd like to post updates more often, you can override the period per goal by labeling the issue with `R-every-week`, `R-every-2-weeks`, or `R-every-4-weeks`.* "#, owner = owner.mention(false), goals = goals.iter().map(Reminder::list_item).join("\n"), ) } -fn unowned_errors(goals: &[Reminder<'_>]) -> String { - let goals = goals - .iter() - .map(|goal| format!("+ {goal}", goal = goal.reference())) - .join("\n"); - +fn unowned_errors(goals: &[Goal<'_>]) -> String { format!( r#" The following goals have no owner assigned: -{goals} +{unowned} Please assign an owner and reach out to them! -"# +"#, + unowned = goals + .iter() + .map(|g| format!("+ {}", g.named_link())) + .join("\n") ) } -fn multiple_owner_errors(goals: &[MultipleOwners<'_>]) -> String { - let goals = goals - .iter() - .map(|entry| { - format!( - "+ {goal} — assigned to {owners}", - goal = entry.goal.reference(), - owners = entry.owners.all_mentions(true), - ) - }) - .join("\n"); - +fn multiple_owner_warnings(problems: &[OwnershipProblem<'_>]) -> String { format!( r#" The following goals have more than one owner assigned: -{goals} +{multiple_owner} A goal should have exactly one owner. All owners with a Zulip account were still notified separately. -"# +"#, + multiple_owner = problems + .iter() + .filter(|p| p.owners.has_multiple()) + .map(|p| { + format!( + "+ {goal}: {owners}", + goal = p.goal.link(), + owners = p.owners.all_mentions(true).expect("has multiple"), + ) + }) + .join("\n") ) } -fn missing_zulip_errors(goals: &[MultipleOwners<'_>]) -> String { - let goals = goals - .iter() - .map(|entry| { - format!( - "+ {goal} — missing Zulip account: {unreachable}\n {notified}", - goal = entry.goal.reference(), - unreachable = entry.owners.unreachable_team_links(), - notified = match entry.owners.reachable_mentions() { - None => "No existing owner was notified on Zulip.".to_owned(), - Some(owners) => format!("{owners} got notified on Zulip."), - } - ) - }) - .join("\n"); - +fn missing_zulip_errors(problems: &[OwnershipProblem<'_>]) -> String { format!( r#" The following goal owners were not pinged because they don't have a Zulip account specified in the `team` repo: -{goals} +{missing_zulip} Please make sure to register their `zulip-id` and reach out to them! -"# +"#, + missing_zulip = problems + .iter() + .filter(|p| p.owners.has_missing_zulip_id()) + .map(|p| { + format!( + "+ {goal}: {unreachable}\n - {notified}", + goal = p.goal.link(), + unreachable = p + .owners + .missing_zulip_team_links() + .expect("has missing Zulip ID"), + notified = match p.owners.reachable_mentions() { + None => "Nobody was notified on Zulip.".to_owned(), + Some(owners) => format!("{owners} got notified on Zulip."), + } + ) + }) + .join("\n") ) } -fn invalid_schedule_errors(errors: &[InvalidSchedule<'_>]) -> String { - let errors = errors - .iter() - .map(|error| format!("+ {} — {}", error.goal.reference(), error.reason,)) - .join("\n"); +fn missing_team_entry_warnings(problems: &[OwnershipProblem<'_>]) -> String { + format!( + r#" +The following assignees could not be found in the `team` repo, so Triagebot could not look up their Zulip accounts: + +{missing_team_entries} + +Please check the assignee usernames and their entries in the `team` repo. +"#, + missing_team_entries = problems + .iter() + .filter(|p| p.owners.has_missing_team_entry()) + .map(|p| { + format!( + "+ {goal}: {owners}\n - {notified}", + goal = p.goal.link(), + owners = p + .owners + .missing_team_entry_links() + .expect("has missing team entry"), + notified = match p.owners.reachable_mentions() { + None => "Nobody was notified on Zulip.".to_owned(), + Some(owners) => format!("{owners} got notified on Zulip."), + }, + ) + }) + .join("\n") + ) +} +fn schedule_warnings(conflicts: &[PeriodConflict<'_>]) -> String { format!( r#" -The following goals have invalid ping-schedule labels: +The following goals have conflicting reporting period labels: -{errors} +{conflicts} -Use exactly one frequency label (`P-weekly`, `P-biweekly-0`, `P-biweekly-1`, or `P-monthly`). -"# +Unlabeled goals use the default period of 4 weeks. +"#, + conflicts = conflicts + .iter() + .map(|e| format!("+ {}: {}", e.goal.link(), e.reason)) + .join("\n") ) } -fn error_message(errors: &ReminderErrors<'_>) -> String { +fn error_sections(errors: &ReminderErrors<'_>) -> String { let mut sections = Vec::new(); if !errors.unowned.is_empty() { sections.push(unowned_errors(&errors.unowned)); } - if !errors.multiply_owned.is_empty() { - sections.push(multiple_owner_errors(&errors.multiply_owned)); + if errors.ownership.iter().any(|p| p.owners.has_multiple()) { + sections.push(multiple_owner_warnings(&errors.ownership)); } - if !errors.missing_zulip.is_empty() { - sections.push(missing_zulip_errors(&errors.missing_zulip)); + if errors + .ownership + .iter() + .any(|p| p.owners.has_missing_zulip_id()) + { + sections.push(missing_zulip_errors(&errors.ownership)); } - if !errors.invalid_schedules.is_empty() { - sections.push(invalid_schedule_errors(&errors.invalid_schedules)); + if errors + .ownership + .iter() + .any(|p| p.owners.has_missing_team_entry()) + { + sections.push(missing_team_entry_warnings(&errors.ownership)); } - - format!( - r#" -Hi @*T-goals*! - -{} -"#, - sections.iter().join("\n\n---\n\n"), - ) -} - -fn update_required(issue: &Issue, now: DateTime, schedule: CustomSchedule) -> bool { - let due_date = schedule.latest_due_date(now.date_naive()); - let has_real_update = issue.comments.unwrap_or(0) > 1; - let updated_after_due_date = issue.updated_at.date_naive() >= due_date; - - !has_real_update || !updated_after_due_date -} - -fn evaluate<'gh>(issue: &'gh Issue, now: DateTime) -> EvaluatedReminder<'gh> { - let days_since_last_update = (now - issue.updated_at).num_days(); - - log::debug!( - "issue #{}: days_since_last_comment = {} days, comments = {}", - issue.number, - days_since_last_update, - issue.comments.unwrap_or(0), - ); - - let (requires_update, invalid_schedule_reason) = match Schedule::from_issue(issue) { - Schedule::Default => (update_required(issue, now, CustomSchedule::Biweekly0), None), - Schedule::Custom(schedule) => (update_required(issue, now, schedule), None), - Schedule::Invalid { fallback, reason } => { - (update_required(issue, now, fallback), Some(reason)) - } - }; - - EvaluatedReminder { - reminder: Reminder::from_issue(issue, days_since_last_update), - requires_update, - invalid_schedule_reason, + if !errors.schedule.is_empty() { + sections.push(schedule_warnings(&errors.schedule)); } + + sections.iter().join("\n\n---\n\n") } async fn build_plan<'gh>( - issues: &'gh [Issue], + issues: &'gh [GoalIssue], team: &TeamClient, + now: DateTime, ) -> anyhow::Result> { - let now = Utc::now(); let mut plan = ReminderPlan::default(); for issue in issues { - let evaluation = evaluate(issue, now); + let (reminder, conflict) = Reminder::from_issue(issue, now); + + log::debug!( + "issue #{}: period_start = {}, next_deadline = {}, last_comment = {:?}", + issue.number, + reminder.period_start, + reminder.next_deadline, + issue.last_comment.as_ref().map(|c| c.created_at), + ); - if let Some(reason) = evaluation.invalid_schedule_reason { - plan.add_invalid_schedule(evaluation.reminder, reason); + if let Some(conflict) = conflict { + plan.add_conflicts(reminder.goal, conflict); } - if !evaluation.requires_update { + if !reminder.is_required(now) { continue; } - let owners = Owners::resolve(team, issue).await?; - plan.add_goal(evaluation.reminder, owners); + let owners = Owners::resolve_goal(team, issue).await?; + plan.add_goal(reminder, owners); } Ok(plan) @@ -565,7 +703,7 @@ async fn send_dm( dry_run: bool, ) -> anyhow::Result<()> { if dry_run { - log::debug!("(DRY) Would send DM to user {}: {}", owner.0, content,); + log::debug!("(DRY) Would send DM to user {}: {}", owner.0, content); return Ok(()); } @@ -589,7 +727,7 @@ async fn send_triagebot_topic( ) -> anyhow::Result<()> { if dry_run { log::debug!( - "(DRY) Would send to topic {GOALS_STREAM}>{TRIAGEBOT_TOPIC}: {}", + "(DRY) Would send to topic {GOALS_META_STREAM}>{TRIAGEBOT_TOPIC}: {}", content, ); return Ok(()); @@ -608,48 +746,105 @@ async fn send_triagebot_topic( Ok(()) } +#[derive(Default)] +struct PeriodCounts { + weekly: usize, + every_2_weeks: usize, + every_4_weeks: usize, +} + +impl PeriodCounts { + fn from_reminders<'gh>(reminders: impl Iterator>) -> Self { + let mut counts = Self::default(); + + for reminder in reminders { + match reminder.period { + Period::EveryWeek => counts.weekly += 1, + Period::Every2Weeks => counts.every_2_weeks += 1, + Period::Every4Weeks => counts.every_4_weeks += 1, + } + } + + counts + } + + fn total(&self) -> usize { + self.weekly + self.every_2_weeks + self.every_4_weeks + } +} + +fn report( + errors: &ReminderErrors<'_>, + total_owners: usize, + counts: &PeriodCounts, + today: NaiveDate, +) -> String { + let next_week = Period::EveryWeek.next(Period::EveryWeek.start(today)); + let next_2_weeks = Period::Every2Weeks.next(Period::Every2Weeks.start(today)); + let next_4_weeks = Period::Every4Weeks.next(Period::Every4Weeks.start(today)); + + let error_summary = if errors.is_empty() { + "No errors happened in the process.".to_owned() + } else { + format!( + "{count} errors happened in the process.\n\n---\n\n{details}", + count = errors.count(), + details = error_sections(errors), + ) + }; + + format!( + r#" +Hi @*T-goals*! + +Weekly run finished. + +{total_owners} owners were notified about {total_goals} goals: ++ Weekly reports: {weekly} (next cycle: {next_week}) ++ Biweekly reports: {every_2_weeks} (next cycle: {next_2_weeks}) ++ Four-week reports: {every_4_weeks} (next cycle: {next_4_weeks}) + +{error_summary} + +Until next week! <3 +"#, + total_goals = counts.total(), + weekly = counts.weekly, + every_2_weeks = counts.every_2_weeks, + every_4_weeks = counts.every_4_weeks, + next_week = display_job_date(next_week), + next_2_weeks = display_job_date(next_2_weeks), + next_4_weeks = display_job_date(next_4_weeks), + ) +} + async fn execute_plan( zulip: &ZulipClient, plan: ReminderPlan<'_>, + today: NaiveDate, dry_run: bool, ) -> anyhow::Result<()> { - let mut total_owners = 0; - let total_goals = plan - .goals_by_owner - .values() - .flatten() - .map(|goal| goal.issue) - .unique() - .count(); - let mut total_errors = 0; - - for (owner, goals) in plan.goals_by_owner { - send_dm(zulip, owner, &owner_message(owner, &goals), dry_run).await?; - total_owners += 1; - } - - if !plan.errors.is_empty() { - send_triagebot_topic(zulip, &error_message(&plan.errors), dry_run).await?; + let ReminderPlan { + goals_by_owner, + errors, + } = plan; + + let total_owners = goals_by_owner.len(); + let counts = PeriodCounts::from_reminders( + goals_by_owner + .values() + .flatten() + .copied() + .unique_by(|reminder| reminder.goal.issue), + ); - total_errors += plan.errors.unowned.len() - + plan.errors.multiply_owned.len() - + plan.errors.missing_zulip.len() - + plan.errors.invalid_schedules.len(); + for (owner, goals) in goals_by_owner { + send_dm(zulip, owner, &owner_message(owner, &goals), dry_run).await?; } send_triagebot_topic( zulip, - &format!( - r#" -Weekly run finished. - -{total_owners} owners have been notified about {total_goals} goals. - -{total_errors} errors happened in the process. - -Until next week! <3 - "# - ), + &report(&errors, total_owners, &counts, today), dry_run, ) .await?; @@ -657,36 +852,16 @@ Until next week! <3 Ok(()) } -fn is_tracking_issue(issue: &Issue) -> bool { - issue - .labels - .iter() - .any(|label| label.name == C_TRACKING_ISSUE) -} - -async fn tracking_issues(gh: &GithubClient) -> anyhow::Result> { - gh.repository(RUST_PROJECT_GOALS_REPO) - .await? - .get_issues( - gh, - &github::issue_query::Query { - filters: vec![("state", "open"), ("is", "issue")], - include_labels: vec![C_TRACKING_ISSUE], - exclude_labels: vec![], - }, - ) - .await -} - pub async fn ping_project_goals_owners( gh: &GithubClient, zulip: &ZulipClient, team: &TeamClient, dry_run: bool, ) -> anyhow::Result<()> { - let issues = tracking_issues(gh).await?; - let plan = build_plan(&issues, team).await?; - execute_plan(zulip, plan, dry_run).await + let now = Utc::now(); + let issues = gh.open_goal_issues().await?; + let plan = build_plan(&issues, team, now).await?; + execute_plan(zulip, plan, now.date_naive(), dry_run).await } pub struct ProjectGoalsUpdateJob; @@ -704,8 +879,6 @@ impl Job for ProjectGoalsUpdateJob { /// Returns true if the GitHub user is part of the Goals team. pub async fn is_goals_member(team_client: &TeamClient, github_id: u64) -> anyhow::Result { - const GOALS_TEAM: &str = "goals"; - let team = match team_client.get_team(GOALS_TEAM).await? { Some(team) => team, None => { @@ -735,18 +908,27 @@ fn goal_zulip_topic(issue: &Issue) -> String { title } +fn is_tracking_issue(issue: &Issue) -> bool { + issue + .labels + .iter() + .any(|label| label.name == "C-tracking-issue") +} + async fn create_goal_topic(issue: &Issue, ctx: &Context) -> anyhow::Result<()> { if !is_tracking_issue(issue) { return Ok(()); } - let owners = Owners::resolve(&ctx.team, issue).await?; + let owners = Owners::resolve_event(&ctx.team, issue).await?; let topic = goal_zulip_topic(issue); let content = format!( "Goal *{title}* (goals#{number}) has been accepted. It's owned by {owners}.", title = issue.title, number = issue.number, - owners = owners.all_mentions(false), + owners = owners + .all_mentions(false) + .unwrap_or_else(|| "nobody (@*T-goals* should fix this)".to_owned()), ); MessageApiRequest { @@ -781,7 +963,7 @@ async fn echo_comment_to_zulip( return Ok(()); } - let author = Owner::resolve(&ctx.team, comment.user.id, &comment.user.login).await?; + let author = Owner::resolve_event(&ctx.team, comment.user.id, &comment.user.login).await?; let text = &comment.body; let content = format!( @@ -792,7 +974,7 @@ async fn echo_comment_to_zulip( url = comment.html_url, number = issue.number, author = author.display_mention(false), - ticks = quote_fence(&text), + ticks = quote_fence(text), ); MessageApiRequest { diff --git a/src/jobs.rs b/src/jobs.rs index 85b714b34..294016fb9 100644 --- a/src/jobs.rs +++ b/src/jobs.rs @@ -111,8 +111,8 @@ pub fn default_jobs() -> Vec { }, JobSchedule { name: ProjectGoalsUpdateJob.name(), - // Around 9am Pacific time on every Monday. - schedule: Schedule::from_str("0 00 17 * * Mon *").unwrap(), + // Around 6/7am Pacific time on every Thursday. + schedule: Schedule::from_str("0 00 14 * * Thu *").unwrap(), metadata: serde_json::Value::Null, }, ] From 3a832a66338db40b97f2da0b06545ea2961bf36d Mon Sep 17 00:00:00 2001 From: nxsaken Date: Fri, 31 Jul 2026 01:55:27 +0400 Subject: [PATCH 7/9] Remove binary and zulip command --- src/bin/project_goals.rs | 26 ------------------- src/zulip.rs | 54 ++-------------------------------------- src/zulip/commands.rs | 10 -------- 3 files changed, 2 insertions(+), 88 deletions(-) delete mode 100644 src/bin/project_goals.rs diff --git a/src/bin/project_goals.rs b/src/bin/project_goals.rs deleted file mode 100644 index cc85c8bcd..000000000 --- a/src/bin/project_goals.rs +++ /dev/null @@ -1,26 +0,0 @@ -use clap::Parser; -use triagebot::team_data::TeamClient; -use triagebot::zulip::client::ZulipClient; -use triagebot::{github::GithubClient, handlers::project_goals}; - -/// A basic example -#[derive(Parser, Debug)] -struct Opt { - /// If specified, no messages are sent. - #[arg(long)] - dry_run: bool, -} - -#[tokio::main(flavor = "current_thread")] -async fn main() -> anyhow::Result<()> { - dotenvy::dotenv().ok(); - tracing_subscriber::fmt::init(); - - let opt = Opt::parse(); - let gh = GithubClient::new_from_env(); - let zulip = ZulipClient::new_from_env(); - let team_api = TeamClient::new_from_env(); - project_goals::ping_project_goals_owners(&gh, &zulip, &team_api, opt.dry_run).await?; - - Ok(()) -} diff --git a/src/zulip.rs b/src/zulip.rs index c9f8b25ba..f44348613 100644 --- a/src/zulip.rs +++ b/src/zulip.rs @@ -14,14 +14,13 @@ use crate::github::{self, PullRequestNumber, Repository}; use crate::handlers::Context; use crate::handlers::docs_update::docs_update; use crate::handlers::pr_tracking::{ReviewerWorkqueue, get_assigned_prs}; -use crate::handlers::project_goals::{self, ping_project_goals_owners}; use crate::interactions::ErrorComment; use crate::utils::pluralize; use crate::zulip::api::{MessageApiResponse, Recipient}; use crate::zulip::client::ZulipClient; use crate::zulip::commands::{ - BackportChannelArgs, BackportVerbArgs, ChatCommand, IssuePrio, LookupCmd, PingGoalsArgs, - StreamCommand, WorkqueueCmd, WorkqueueLimit, parse_cli, + BackportChannelArgs, BackportVerbArgs, ChatCommand, IssuePrio, LookupCmd, StreamCommand, + WorkqueueCmd, WorkqueueLimit, parse_cli, }; use anyhow::{Context as _, format_err}; use axum::Json; @@ -266,9 +265,6 @@ async fn handle_command<'a>( id, } => unlock_cmd(&ctx, gh_id, organization, repo, *id).await, ChatCommand::Work(cmd) => workqueue_commands(&ctx, gh_id, cmd).await, - ChatCommand::PingGoals(args) => { - ping_goals_cmd(ctx.clone(), gh_id, message_data, args).await - } ChatCommand::DocsUpdate => trigger_docs_update(&ctx.zulip, message_data), ChatCommand::UserInfo { username, @@ -358,9 +354,6 @@ async fn handle_command<'a>( .await .map_err(|e| format_err!("Failed to await at this time: {e:?}")) } - StreamCommand::PingGoals(args) => { - ping_goals_cmd(ctx, gh_id, message_data, &args).await - } StreamCommand::DocsUpdate => trigger_docs_update(&ctx.zulip, message_data), StreamCommand::Backport { verb, @@ -632,48 +625,6 @@ async fn assign_issue_prio( Ok(None) } -async fn ping_goals_cmd( - ctx: Arc, - gh_id: u64, - message: &Message, - args: &PingGoalsArgs, -) -> anyhow::Result> { - if project_goals::is_goals_member(&ctx.team, gh_id).await? { - let args = args.clone(); - let message = message.clone(); - tokio::spawn(async move { - let res = ping_project_goals_owners(&ctx.github, &ctx.zulip, &ctx.team, false).await; - - let status = match res { - Ok(_res) => "OK".to_string(), - Err(err) => { - tracing::error!("ping_project_goals_owners: {err:?}"); - format!("ERROR\n\n```\n{err:#?}\n```\n") - } - }; - - let res = MessageApiRequest { - recipient: message.sender_to_recipient(), - content: &format!("End pinging project groups owners: {status}"), - } - .send(&ctx.zulip) - .await; - - if let Err(err) = res { - tracing::error!( - "error sending project goals ping reply: {err:?} for status: {status}" - ); - } - }); - - Ok(Some("Started pinging project groups owners...".to_string())) - } else { - Err(format_err!( - "That command is only permitted for those running the project-goal program.", - )) - } -} - /// Unlock a specific issue in our managed repos. /// This command can only be used by team members. async fn unlock_cmd( @@ -1195,7 +1146,6 @@ enum ImpersonationMode { fn get_cmd_impersonation_mode(cmd: &ChatCommand) -> ImpersonationMode { match cmd { ChatCommand::DocsUpdate - | ChatCommand::PingGoals(_) | ChatCommand::UserInfo { .. } | ChatCommand::TeamStats { .. } | ChatCommand::Unlock { .. } diff --git a/src/zulip/commands.rs b/src/zulip/commands.rs index cceae91c9..771d15304 100644 --- a/src/zulip/commands.rs +++ b/src/zulip/commands.rs @@ -27,8 +27,6 @@ pub enum ChatCommand { /// Issue or pull-request number to unlock. id: PullRequestNumber, }, - /// Ping project goal owners. - PingGoals(PingGoalsArgs), /// Update docs DocsUpdate, /// Show recent GitHub activity of a user. @@ -149,8 +147,6 @@ pub enum StreamCommand { EndMeeting, /// Read a document. Read, - /// Ping project goal owners. - PingGoals(PingGoalsArgs), /// Update docs. DocsUpdate, /// Accept or decline a backport. @@ -189,12 +185,6 @@ pub enum StreamCommand { }, } -#[derive(clap::Parser, Debug, PartialEq, Clone)] -pub struct PingGoalsArgs { - /// Goals updated within this threshold (in days) will not be pinged. - pub threshold: u64, -} - /// Backport release channels #[derive(Clone, clap::ValueEnum, Debug, PartialEq)] pub enum BackportChannelArgs { From 7699ccdc7adc037853fdfad91d836934475933cd Mon Sep 17 00:00:00 2001 From: nxsaken Date: Fri, 31 Jul 2026 15:30:39 +0400 Subject: [PATCH 8/9] Do not mention author in the echoed comment --- src/handlers/project_goals.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/handlers/project_goals.rs b/src/handlers/project_goals.rs index 7cd778ee3..3a943c5af 100644 --- a/src/handlers/project_goals.rs +++ b/src/handlers/project_goals.rs @@ -973,7 +973,7 @@ async fn echo_comment_to_zulip( {ticks}", url = comment.html_url, number = issue.number, - author = author.display_mention(false), + author = author.display_mention(true), ticks = quote_fence(text), ); From 969ef64b068aa7baa508bfbe5caf64f34a7a3c12 Mon Sep 17 00:00:00 2001 From: nxsaken Date: Thu, 6 Aug 2026 19:32:57 +0400 Subject: [PATCH 9/9] Address feedback (document stuff, make names clearer, remove unused code) --- src/github/queries/open_goal_issues.rs | 8 +- src/handlers/project_goals.rs | 101 ++++++++++++++++--------- src/jobs.rs | 6 +- 3 files changed, 68 insertions(+), 47 deletions(-) diff --git a/src/github/queries/open_goal_issues.rs b/src/github/queries/open_goal_issues.rs index 8d8708fc8..4927ceac4 100644 --- a/src/github/queries/open_goal_issues.rs +++ b/src/github/queries/open_goal_issues.rs @@ -17,7 +17,6 @@ pub struct GoalIssue { pub struct LastGoalComment { pub created_at: chrono::DateTime, - pub author: Option, } #[derive(serde::Deserialize)] @@ -65,7 +64,6 @@ struct GraphQlLabel { struct GraphQlComment { #[serde(rename = "createdAt")] created_at: chrono::DateTime, - author: Option, } impl From for GoalIssue { @@ -98,7 +96,6 @@ impl From for GoalIssue { .next() .map(|comment| LastGoalComment { created_at: comment.created_at, - author: comment.author.map(|author| author.login), }); Self { @@ -114,7 +111,7 @@ impl From for GoalIssue { impl GithubClient { /// Get every open tracking issue in `rust-lang/rust-project-goals`, - /// including the latest comment's date and author. + /// including the latest comment's date. pub async fn open_goal_issues(&self) -> anyhow::Result> { let mut cursor = None::; let mut issues = Vec::new(); @@ -157,9 +154,6 @@ query ( comments(last: 1) { nodes { createdAt - author { - login - } } } } diff --git a/src/handlers/project_goals.rs b/src/handlers/project_goals.rs index 3a943c5af..b472d1a59 100644 --- a/src/handlers/project_goals.rs +++ b/src/handlers/project_goals.rs @@ -29,12 +29,16 @@ const GOALS_META_STREAM: u64 = 478_266; // #project-goals/meta const TRIAGEBOT_TOPIC: &str = "triagebot reports"; const MAX_ZULIP_TOPIC: usize = 60; -// Keep these in sync with src/jobs.rs +/// The weekday of the job execution (must match [`crate::jobs`]). const JOB_WEEKDAY: Weekday = Weekday::Thu; +/// The UTC hour of the job execution (must match [`crate::jobs`]). const JOB_UTC_HOUR: u32 = 14; +/// The UTC minute of the job execution (must match [`crate::jobs`]). const JOB_UTC_MINUTE: u32 = 0; -// Arbitrary date to keep cycles anchored. +/// An arbitrary date to keep reporting periods anchored. +/// +/// The phase of the biweekly and 4-week periods depends on this day. const EPOCH: NaiveDate = NaiveDate::from_ymd_opt(1970, 1, 1).unwrap(); #[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] @@ -80,7 +84,7 @@ struct Owner<'gh> { } impl<'gh> Owner<'gh> { - async fn resolve_goal(team: &TeamClient, username: &'gh str) -> anyhow::Result { + async fn from_username(team: &TeamClient, username: &'gh str) -> anyhow::Result { Ok(Self { github: GhUsername(username), contact: match team.get_gh_id_from_username(username).await? { @@ -93,7 +97,7 @@ impl<'gh> Owner<'gh> { }) } - async fn resolve_event( + async fn from_id_and_username( team: &TeamClient, gh_id: u64, username: &'gh str, @@ -132,7 +136,7 @@ impl<'gh> Owners<'gh> { async fn resolve_goal(team: &TeamClient, issue: &'gh GoalIssue) -> anyhow::Result { let mut owners = Vec::with_capacity(issue.assignees.len()); for username in &issue.assignees { - owners.push(Owner::resolve_goal(team, username).await?); + owners.push(Owner::from_username(team, username).await?); } Ok(Self(owners)) } @@ -140,7 +144,7 @@ impl<'gh> Owners<'gh> { async fn resolve_event(team: &TeamClient, issue: &'gh Issue) -> anyhow::Result { let mut owners = Vec::with_capacity(issue.assignees.len()); for assignee in &issue.assignees { - owners.push(Owner::resolve_event(team, assignee.id, &assignee.login).await?); + owners.push(Owner::from_id_and_username(team, assignee.id, &assignee.login).await?); } Ok(Self(owners)) } @@ -219,6 +223,10 @@ impl<'gh> Owners<'gh> { } } +/// Every goal has its own reporting schedule. +/// This is how often the goal owner is prompted to author an update. +/// +/// This is set via a label (see [`REPORT_LABELS`]) on the goal's tracking issue. #[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd)] enum Period { /// Start a new reporting period every week. @@ -246,20 +254,37 @@ impl Period { } } + /// Returns the starting date of the period that includes this day. + /// + /// Every reporting period begins on a [`JOB_WEEKDAY`] + /// and lasts [`Period::weeks`], depending on the goal. + /// + /// Biweekly and 4-week cycles are aligned to [`EPOCH`]. fn start(self, today: NaiveDate) -> NaiveDate { - let days_until_job = - (JOB_WEEKDAY.num_days_from_monday() + 7 - EPOCH.weekday().num_days_from_monday()) % 7; + // Depending on the chosen date, `EPOCH` may not fall on the `JOB_WEEKDAY`. + // `days_until_job` is needed to calculate an anchor from the `EPOCH` that + // falls on the `JOB_WEEKDAY` and can be used to compute the relevant dates. + let epoch_weekday = i64::from(EPOCH.weekday().num_days_from_monday()); + let job_weekday = i64::from(JOB_WEEKDAY.num_days_from_monday()); + let days_until_job = (job_weekday - epoch_weekday).rem_euclid(7); - let anchor = EPOCH + Duration::days(i64::from(days_until_job)); + // Dates are computed relative to this date. + let anchor = EPOCH + Duration::days(days_until_job); + // The number of full weeks since the anchor. let weeks_since_anchor = today.signed_duration_since(anchor).num_weeks(); let period_weeks = self.weeks(); + // The number of full periods that passed since the anchor date. let periods_since_anchor = weeks_since_anchor.div_euclid(period_weeks); + // The number of weeks since the anchor, quantized to the period. + let weeks_since_anchor = periods_since_anchor * period_weeks; - anchor + Duration::weeks(periods_since_anchor * period_weeks) + anchor + Duration::weeks(weeks_since_anchor) } - fn next(self, period_start: NaiveDate) -> NaiveDate { + /// Returns the starting date of the next period, + /// i.e. this period's starting date plus the duration of a period. + fn next_start(self, period_start: NaiveDate) -> NaiveDate { period_start + Duration::weeks(self.weeks()) } } @@ -331,10 +356,13 @@ impl<'gh> Goal<'gh> { } } + /// Returns a string representing an issue in the `rust-lang/rust-project-goals` repo. + /// Zulip recognizes strings like `goals#123` and turns them into links. fn link(&self) -> String { format!("goals#{number}", number = self.issue) } + /// Same as [`Goal::link`], but also includes the goal title. fn named_link(&self) -> String { format!( "**{title}** (goals#{number})", @@ -370,10 +398,14 @@ fn display_datetime(date: DateTime) -> String { #[derive(Clone, Copy, Debug)] struct Reminder<'gh> { goal: Goal<'gh>, + /// The reporting period of this goal. period: Period, + /// The start date of this goal's current reporting period. period_start: NaiveDate, - next_deadline: NaiveDate, - _is_new_period: bool, + /// The start date of this goal's next reporting period. + /// + /// (This acts as a deadline for the current update.) + next_period_start: NaiveDate, } impl<'gh> Reminder<'gh> { @@ -386,8 +418,7 @@ impl<'gh> Reminder<'gh> { goal: Goal::from_issue(issue), period: schedule.period, period_start, - next_deadline: schedule.period.next(period_start), - _is_new_period: period_start == today, + next_period_start: schedule.period.next_start(period_start), }, schedule.conflict, ) @@ -411,7 +442,7 @@ impl<'gh> Reminder<'gh> { "+ {goal}\n - {latest}\n - next *{period}* cycle starts {next}", goal = self.goal.named_link(), latest = self.goal.latest_update(), - next = display_job_date(self.next_deadline), + next = display_job_date(self.next_period_start), period = self.period.adjective(), ) } @@ -677,7 +708,7 @@ async fn build_plan<'gh>( "issue #{}: period_start = {}, next_deadline = {}, last_comment = {:?}", issue.number, reminder.period_start, - reminder.next_deadline, + reminder.next_period_start, issue.last_comment.as_ref().map(|c| c.created_at), ); @@ -696,28 +727,23 @@ async fn build_plan<'gh>( Ok(plan) } -async fn send_dm( - zulip: &ZulipClient, - owner: ZulipId, - content: &str, - dry_run: bool, -) -> anyhow::Result<()> { +async fn send_dm(zulip: &ZulipClient, owner: ZulipId, content: &str, dry_run: bool) { if dry_run { log::debug!("(DRY) Would send DM to user {}: {}", owner.0, content); - return Ok(()); + return; } - MessageApiRequest { + let req = MessageApiRequest { recipient: Recipient::Private { id: owner.0, email: "", }, content, - } - .send(zulip) - .await?; + }; - Ok(()) + if let Err(err) = req.send(zulip).await { + log::error!("failed to send a DM on Zulip: {err}") + } } async fn send_triagebot_topic( @@ -779,9 +805,9 @@ fn report( counts: &PeriodCounts, today: NaiveDate, ) -> String { - let next_week = Period::EveryWeek.next(Period::EveryWeek.start(today)); - let next_2_weeks = Period::Every2Weeks.next(Period::Every2Weeks.start(today)); - let next_4_weeks = Period::Every4Weeks.next(Period::Every4Weeks.start(today)); + let next_week = Period::EveryWeek.next_start(Period::EveryWeek.start(today)); + let next_2_weeks = Period::Every2Weeks.next_start(Period::Every2Weeks.start(today)); + let next_4_weeks = Period::Every4Weeks.next_start(Period::Every4Weeks.start(today)); let error_summary = if errors.is_empty() { "No errors happened in the process.".to_owned() @@ -839,7 +865,7 @@ async fn execute_plan( ); for (owner, goals) in goals_by_owner { - send_dm(zulip, owner, &owner_message(owner, &goals), dry_run).await?; + send_dm(zulip, owner, &owner_message(owner, &goals), dry_run).await; } send_triagebot_topic( @@ -864,12 +890,12 @@ pub async fn ping_project_goals_owners( execute_plan(zulip, plan, now.date_naive(), dry_run).await } -pub struct ProjectGoalsUpdateJob; +pub struct PingProjectGoalsOwnersJob; #[async_trait] -impl Job for ProjectGoalsUpdateJob { +impl Job for PingProjectGoalsOwnersJob { fn name(&self) -> &'static str { - "project_goals_update_job" + "ping_project_goal_owners_job" } async fn run(&self, ctx: &Context, _metadata: &serde_json::Value) -> anyhow::Result<()> { @@ -963,7 +989,8 @@ async fn echo_comment_to_zulip( return Ok(()); } - let author = Owner::resolve_event(&ctx.team, comment.user.id, &comment.user.login).await?; + let author = + Owner::from_id_and_username(&ctx.team, comment.user.id, &comment.user.login).await?; let text = &comment.body; let content = format!( diff --git a/src/jobs.rs b/src/jobs.rs index 294016fb9..7c31fd2cf 100644 --- a/src/jobs.rs +++ b/src/jobs.rs @@ -50,7 +50,7 @@ use std::str::FromStr; use async_trait::async_trait; use cron::Schedule; -use crate::handlers::project_goals::ProjectGoalsUpdateJob; +use crate::handlers::project_goals::PingProjectGoalsOwnersJob; use crate::handlers::pull_requests_assignment_update::PullRequestAssignmentUpdate; use crate::{ db::jobs::JobSchedule, @@ -78,7 +78,7 @@ pub fn jobs() -> Vec> { Box::new(MajorChangeAcceptanceJob), Box::new(GithubRateLimitLoggingJob), Box::new(AddReviewChangesSinceLinkJob), - Box::new(ProjectGoalsUpdateJob), + Box::new(PingProjectGoalsOwnersJob), ] } @@ -110,7 +110,7 @@ pub fn default_jobs() -> Vec { metadata: serde_json::Value::Null, }, JobSchedule { - name: ProjectGoalsUpdateJob.name(), + name: PingProjectGoalsOwnersJob.name(), // Around 6/7am Pacific time on every Thursday. schedule: Schedule::from_str("0 00 14 * * Thu *").unwrap(), metadata: serde_json::Value::Null,