Skip to content

Commit 7813a53

Browse files
authored
Merge pull request #2066 from Urgau/automate_mcp_acceptence
Add automatic closing of major change after their waiting period
2 parents 3a6a290 + d751177 commit 7813a53

File tree

4 files changed

+244
-2
lines changed

4 files changed

+244
-2
lines changed

src/config.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,9 @@ pub(crate) struct MajorChangeConfig {
400400
pub(crate) meeting_label: String,
401401
/// This label signals there are concern(s) about the proposal.
402402
pub(crate) concerns_label: Option<String>,
403+
/// An optional duration (in days) for the waiting period after second for the
404+
/// major change to become automaticaly accepted.
405+
pub(crate) waiting_period: Option<u16>,
403406
/// The Zulip stream ID where the messages about the status of
404407
/// the major changed should be relayed.
405408
pub(crate) zulip_stream: u64,

src/handlers.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ mod concern;
3434
pub mod docs_update;
3535
mod github_releases;
3636
mod issue_links;
37-
mod major_change;
37+
pub(crate) mod major_change;
3838
mod mentions;
3939
mod merge_conflicts;
4040
mod milestone_prs;

src/handlers/major_change.rs

Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
use std::fmt::Display;
2+
3+
use crate::jobs::Job;
14
use crate::zulip::api::Recipient;
25
use crate::{
36
config::MajorChangeConfig,
@@ -6,7 +9,10 @@ use crate::{
69
interactions::ErrorComment,
710
};
811
use anyhow::Context as _;
12+
use async_trait::async_trait;
13+
use chrono::{DateTime, Duration, Utc};
914
use parser::command::second::SecondCommand;
15+
use serde::{Deserialize, Serialize};
1016
use tracing as log;
1117

1218
#[derive(Clone, PartialEq, Eq, Debug)]
@@ -202,6 +208,9 @@ pub(super) async fn handle_input(
202208
if event.issue.labels().contains(&Label {
203209
name: config.second_label.to_string(),
204210
}) {
211+
// Re-schedule acceptence job to automaticaly close the MCP
212+
schedule_acceptence_job(ctx, config, &event.issue).await?;
213+
205214
format!("All concerns on the [associated GitHub issue]({}) have been resolved, this proposal is no longer blocked, and will be approved in 10 days if no (new) objections are raised.", event.issue.html_url)
206215
} else {
207216
format!("All concerns on the [associated GitHub issue]({}) have been resolved, this proposal is no longer blocked.", event.issue.html_url)
@@ -274,6 +283,52 @@ pub(super) async fn handle_command(
274283
false,
275284
)
276285
.await
286+
.context("unable to process second command")?;
287+
288+
// Schedule acceptence job to automaticaly close the MCP
289+
schedule_acceptence_job(ctx, config, issue).await?;
290+
291+
Ok(())
292+
}
293+
294+
async fn schedule_acceptence_job(
295+
ctx: &Context,
296+
config: &MajorChangeConfig,
297+
issue: &Issue,
298+
) -> anyhow::Result<()> {
299+
if let Some(waiting_period) = &config.waiting_period {
300+
let seconded_at = Utc::now();
301+
let accept_at = if issue.repository().full_repo_name() == "rust-lang/triagebot" {
302+
// Hack for the triagebot repo, so we can test more quickly
303+
seconded_at + Duration::minutes(5)
304+
} else {
305+
seconded_at + Duration::days((*waiting_period).into())
306+
};
307+
308+
let major_change_seconded = MajorChangeSeconded {
309+
repo: issue.repository().full_repo_name(),
310+
issue: issue.number,
311+
seconded_at,
312+
accept_at,
313+
};
314+
315+
tracing::info!(
316+
"major_change inserting to acceptence queue: {:?}",
317+
&major_change_seconded
318+
);
319+
320+
crate::db::schedule_job(
321+
&*ctx.db.get().await,
322+
MAJOR_CHANGE_ACCEPTENCE_JOB_NAME,
323+
serde_json::to_value(major_change_seconded)
324+
.context("unable to serialize the major change metadata")?,
325+
accept_at,
326+
)
327+
.await
328+
.context("failed to add the major change to the automatic acceptance queue")?;
329+
}
330+
331+
Ok(())
277332
}
278333

279334
async fn handle(
@@ -362,3 +417,183 @@ fn zulip_topic_from_issue(issue: &ZulipGitHubReference) -> String {
362417
_ => format!("{} {}", issue.title, topic_ref),
363418
}
364419
}
420+
421+
#[derive(Debug)]
422+
enum SecondedLogicError {
423+
NotYetAcceptenceTime {
424+
accept_at: DateTime<Utc>,
425+
now: DateTime<Utc>,
426+
},
427+
IssueNotReady {
428+
draft: bool,
429+
open: bool,
430+
},
431+
ConcernsLabelSet,
432+
NoMajorChangeConfig,
433+
}
434+
435+
impl std::error::Error for SecondedLogicError {}
436+
437+
impl Display for SecondedLogicError {
438+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
439+
match self {
440+
SecondedLogicError::NotYetAcceptenceTime { accept_at, now } => {
441+
write!(f, "not yet acceptence time ({accept_at} < {now})")
442+
}
443+
SecondedLogicError::IssueNotReady { draft, open } => {
444+
write!(f, "issue is not ready (draft: {draft}; open: {open})")
445+
}
446+
SecondedLogicError::ConcernsLabelSet => write!(f, "concerns label set"),
447+
SecondedLogicError::NoMajorChangeConfig => write!(f, "no `[major_change]` config"),
448+
}
449+
}
450+
}
451+
452+
#[derive(Debug, Serialize, Deserialize)]
453+
#[cfg_attr(test, derive(PartialEq, Eq, Clone))]
454+
struct MajorChangeSeconded {
455+
repo: String,
456+
issue: u64,
457+
seconded_at: DateTime<Utc>,
458+
accept_at: DateTime<Utc>,
459+
}
460+
461+
const MAJOR_CHANGE_ACCEPTENCE_JOB_NAME: &str = "major_change_acceptence";
462+
463+
pub(crate) struct MajorChangeAcceptenceJob;
464+
465+
#[async_trait]
466+
impl Job for MajorChangeAcceptenceJob {
467+
fn name(&self) -> &'static str {
468+
MAJOR_CHANGE_ACCEPTENCE_JOB_NAME
469+
}
470+
471+
async fn run(&self, ctx: &super::Context, metadata: &serde_json::Value) -> anyhow::Result<()> {
472+
let major_change: MajorChangeSeconded = serde_json::from_value(metadata.clone())
473+
.context("unable to deserialize the metadata in major change acceptence job")?;
474+
475+
let now = Utc::now();
476+
477+
match process_seconded(&ctx, &major_change, now).await {
478+
Ok(()) => {
479+
tracing::info!(
480+
"{}: major change ({:?}) as been accepted, remove from the queue",
481+
self.name(),
482+
&major_change,
483+
);
484+
}
485+
Err(err) if err.downcast_ref::<SecondedLogicError>().is_some() => {
486+
tracing::error!(
487+
"{}: major change ({:?}) has a logical error (no retry): {err}",
488+
self.name(),
489+
&major_change,
490+
);
491+
// exit job succesfully, so it's not retried
492+
}
493+
Err(err) => {
494+
tracing::error!(
495+
"{}: major change ({:?}) is in error: {err}",
496+
self.name(),
497+
&major_change,
498+
);
499+
return Err(err); // so it is retried
500+
}
501+
}
502+
503+
Ok(())
504+
}
505+
}
506+
507+
async fn process_seconded(
508+
ctx: &super::Context,
509+
major_change: &MajorChangeSeconded,
510+
now: DateTime<Utc>,
511+
) -> anyhow::Result<()> {
512+
if major_change.accept_at < now {
513+
anyhow::bail!(SecondedLogicError::NotYetAcceptenceTime {
514+
accept_at: major_change.accept_at,
515+
now
516+
});
517+
}
518+
519+
let repo = ctx
520+
.github
521+
.repository(&major_change.repo)
522+
.await
523+
.context("failed retrieving the repository informations")?;
524+
525+
let config = crate::config::get(&ctx.github, &repo)
526+
.await
527+
.context("failed to get triagebot configuration")?;
528+
529+
let config = config
530+
.major_change
531+
.as_ref()
532+
.ok_or(SecondedLogicError::NoMajorChangeConfig)?;
533+
534+
let issue = repo
535+
.get_issue(&ctx.github, major_change.issue)
536+
.await
537+
.context("unable to get the associated issue")?;
538+
539+
if issue
540+
.labels
541+
.iter()
542+
.any(|l| Some(&l.name) == config.concerns_label.as_ref())
543+
{
544+
anyhow::bail!(SecondedLogicError::ConcernsLabelSet);
545+
}
546+
547+
if !issue.is_open() || issue.draft {
548+
anyhow::bail!(SecondedLogicError::IssueNotReady {
549+
draft: issue.draft,
550+
open: issue.is_open()
551+
});
552+
}
553+
554+
if !issue.labels.iter().any(|l| l.name == config.accept_label) {
555+
// Only post the comment if the accept_label isn't set yet, we may be in a retry
556+
issue
557+
.post_comment(
558+
&ctx.github,
559+
"The final comment period is now complete, this major change is now accepted.\n\nAs the automated representative, I would like to thank the author for their work and everyone else who contributed to this major change proposal."
560+
)
561+
.await
562+
.context("unable to post the acceptance comment")?;
563+
}
564+
issue
565+
.add_labels(
566+
&ctx.github,
567+
vec![Label {
568+
name: config.accept_label.clone(),
569+
}],
570+
)
571+
.await
572+
.context("unable to add the accept label")?;
573+
issue
574+
.remove_label(&ctx.github, &config.second_label)
575+
.await
576+
.context("unable to remove the second label")?;
577+
issue
578+
.close(&ctx.github)
579+
.await
580+
.context("unable to close the issue")?;
581+
582+
Ok(())
583+
}
584+
585+
#[test]
586+
fn major_change_queue_serialize() {
587+
let original = MajorChangeSeconded {
588+
repo: "rust-lang/rust".to_string(),
589+
issue: 1245,
590+
seconded_at: Utc::now(),
591+
accept_at: Utc::now(),
592+
};
593+
594+
let value = serde_json::to_value(original.clone()).unwrap();
595+
596+
let deserialized = serde_json::from_value(value).unwrap();
597+
598+
assert_eq!(original, deserialized);
599+
}

src/jobs.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,10 @@ use cron::Schedule;
4949
use crate::handlers::pull_requests_assignment_update::PullRequestAssignmentUpdate;
5050
use crate::{
5151
db::jobs::JobSchedule,
52-
handlers::{docs_update::DocsUpdateJob, rustc_commits::RustcCommitsJob, Context},
52+
handlers::{
53+
docs_update::DocsUpdateJob, major_change::MajorChangeAcceptenceJob,
54+
rustc_commits::RustcCommitsJob, Context,
55+
},
5356
};
5457

5558
/// How often new cron-based jobs will be placed in the queue.
@@ -66,6 +69,7 @@ pub fn jobs() -> Vec<Box<dyn Job + Send + Sync>> {
6669
Box::new(DocsUpdateJob),
6770
Box::new(RustcCommitsJob),
6871
Box::new(PullRequestAssignmentUpdate),
72+
Box::new(MajorChangeAcceptenceJob),
6973
]
7074
}
7175

0 commit comments

Comments
 (0)