Skip to content
Sign in

flagon-io/g1t

public
0

g1t/services/work/src/team_reviews.rs

562 lines23,066 bytesCodeBlame
1//! Asking a team to review a pull request (`g1t_contracts::teams`).
2//!
3//! A team is asked by a person (`update_pull` with `workspace/team` among
4//! the reviewers) or by the CODEOWNERS file (codeowners.rs). The team is
5//! kept in `team_reviewers`. With its review assignment off, everyone in
6//! it, and in its child teams, is told. With it on, [`pick`] chooses
7//! `count` people, who become reviewers themselves and are told; the rest
8//! of the team is told too only when the team says so.
9//!
10//! [`pick`] never chooses the pull request's author, `g1t`, anyone the
11//! team leaves out, or (with `skip_busy`) anyone with `busy_at` or more
12//! pull requests waiting on their review. People from the team already
13//! asked count towards `count`. Round robin puts whoever this team asked
14//! least recently first (never asked before first of all); load balance,
15//! whoever has the fewest pull requests waiting on them, then the same.
16//! Ties go by username, so the same facts always pick the same people.
17
18use std::collections::HashMap;
19
20use g1t_contracts::events::{PullEvent, TeamRequested};
21use g1t_contracts::identity::AGENT_NAME;
22use g1t_contracts::repos::RepoPath;
23use g1t_contracts::teams::{ResolveTeamsArgs, ResolvedTeam, ReviewAlgorithm, ReviewAssignment, TeamVisibility};
24use g1t_contracts::time::rfc3339;
25use g1t_contracts::work::Pull;
26use g1t_contracts::{FailureCode, Outcome, Role, User};
27use g1t_kit::now_ms;
28use serde::Deserialize;
29use worker::Result;
30
31use crate::Work;
32
33/// Someone review assignment could pick.
34#[derive(Clone, Debug, PartialEq, Eq)]
35pub(crate) struct Candidate {
36 pub username: String,
37 /// When this team last had them asked (RFC 3339), if ever.
38 pub last_asked: Option<String>,
39 /// Open pull requests waiting on their review.
40 pub waiting: u32,
41}
42
43/// Who to ask from a team: see the module's notes.
44pub(crate) fn pick(candidates: &[Candidate], settings: &ReviewAssignment, author: &str, already: &[String]) -> Vec<String> {
45 let author = author.to_lowercase();
46 let in_team_already = candidates
47 .iter()
48 .filter(|candidate| already.iter().any(|name| name.eq_ignore_ascii_case(&candidate.username)))
49 .count() as u32;
50 let wanted = settings.count.saturating_sub(in_team_already) as usize;
51 let mut eligible: Vec<&Candidate> = candidates
52 .iter()
53 .filter(|candidate| {
54 let name = candidate.username.to_lowercase();
55 name != author
56 && name != AGENT_NAME
57 && !settings.excluded.iter().any(|excluded| excluded.eq_ignore_ascii_case(&name))
58 && !already.iter().any(|had| had.eq_ignore_ascii_case(&name))
59 && !(settings.skip_busy && candidate.waiting >= settings.busy_at)
60 })
61 .collect();
62 // `None` (never asked) sorts before any time.
63 let by_turn = |a: &&Candidate, b: &&Candidate| a.last_asked.cmp(&b.last_asked).then_with(|| a.username.cmp(&b.username));
64 match settings.algorithm {
65 ReviewAlgorithm::RoundRobin => eligible.sort_by(by_turn),
66 ReviewAlgorithm::LoadBalance => eligible.sort_by(|a, b| a.waiting.cmp(&b.waiting).then_with(|| by_turn(a, b))),
67 }
68 let mut picked: Vec<String> = Vec::new();
69 for candidate in eligible {
70 if picked.len() >= wanted {
71 break;
72 }
73 if !picked.contains(&candidate.username) {
74 picked.push(candidate.username.clone());
75 }
76 }
77 picked
78}
79
80/// Who a request for `team` tells and asks, given who was picked: with
81/// assignment off, everyone (never the author or g1t); with it on, the
82/// people picked, and everyone else too when the team says so.
83pub(crate) fn told(team: &ResolvedTeam, picked: &[String], author: &str) -> TeamRequested {
84 let everyone: Vec<String> = team
85 .everyone()
86 .map(|person| person.username.clone())
87 .filter(|name| !name.eq_ignore_ascii_case(author) && name != AGENT_NAME)
88 .collect();
89 let settings = &team.review_assignment;
90 let notified = if !settings.enabled {
91 everyone
92 } else if settings.notify_team {
93 let mut all = picked.to_vec();
94 all.extend(everyone.into_iter().filter(|name| !picked.contains(name)));
95 all
96 } else {
97 picked.to_vec()
98 };
99 TeamRequested {
100 team: format!("{}/{}", team.workspace, team.slug),
101 notified,
102 assigned: if settings.enabled { picked.to_vec() } else { Vec::new() },
103 }
104}
105
106/// The most teams one pull request asks to review.
107const MAX_TEAMS: usize = 10;
108
109/// Whether `actor` may ask `team` to review: it is a team of the
110/// repository's workspace, and a secret one only for its own people and
111/// the workspace's owners.
112pub(crate) fn may_ask(team: &ResolvedTeam, actor: &User, namespace: &str) -> Result<(), String> {
113 let handle = team.handle();
114 if !team.workspace.eq_ignore_ascii_case(namespace) {
115 return Err(format!("Only teams of {namespace} can be asked to review its pull requests, and {handle} is not one."));
116 }
117 let owner = actor.role_in(&namespace.to_lowercase()) == Some(Role::Owner);
118 if team.visibility == TeamVisibility::Secret && !owner && !team.everyone().any(|person| person.id == actor.id) {
119 return Err(format!("There is no team named {handle}."));
120 }
121 Ok(())
122}
123
124/// `workspace/team`, or `@workspace/team`, as written among reviewers.
125pub(crate) fn team_name(text: &str) -> Option<String> {
126 let text = text.trim().trim_start_matches('@').to_lowercase();
127 let (workspace, slug) = text.split_once('/')?;
128 (g1t_contracts::is_valid_namespace(workspace) && g1t_contracts::teams::is_valid_slug(slug))
129 .then(|| format!("{workspace}/{slug}"))
130}
131
132#[derive(Deserialize)]
133struct LastRow {
134 username: String,
135 last: String,
136}
137
138#[derive(Deserialize)]
139struct WaitingRow {
140 username: String,
141 n: u32,
142}
143
144impl Work {
145 /// The teams `names` asks to review, checked: each exists, is the
146 /// repository's workspace's, and the actor may ask it.
147 pub(crate) async fn valid_team_reviewers(
148 &self,
149 actor: &User,
150 repo: &RepoPath,
151 pull: &Pull,
152 names: Vec<String>,
153 ) -> Result<Outcome<Vec<String>>> {
154 let mut wanted: Vec<String> = Vec::new();
155 for name in names.iter().filter_map(|name| team_name(name)) {
156 if !wanted.contains(&name) {
157 wanted.push(name);
158 }
159 }
160 if wanted.len() > MAX_TEAMS {
161 return Ok(Outcome::fail(
162 FailureCode::Invalid,
163 format!("A pull request can ask at most {MAX_TEAMS} teams to review it."),
164 ));
165 }
166 let found = self.resolve_teams(&wanted, &pull.repo_id).await?;
167 for name in &wanted {
168 let Some(team) = found.iter().find(|team| format!("{}/{}", team.workspace, team.slug) == *name) else {
169 return Ok(Outcome::fail(FailureCode::Invalid, format!("There is no team named @{name}.")));
170 };
171 // A team already asked stays, whoever asks now.
172 if pull.team_reviewers.contains(name) {
173 continue;
174 }
175 if let Err(why) = may_ask(team, actor, &repo.namespace) {
176 return Ok(Outcome::fail(FailureCode::Invalid, why));
177 }
178 }
179 Ok(Outcome::Ok(wanted))
180 }
181
182 /// Asks more people and teams to review, keeping who is asked already:
183 /// what CODEOWNERS asks for (codeowners.rs).
184 pub(crate) async fn ask_reviewers(
185 &self,
186 pull: &Pull,
187 people: Vec<String>,
188 teams: Vec<String>,
189 actor: Option<&User>,
190 code_owners: bool,
191 ) -> Result<()> {
192 let mut all_people = pull.reviewers.clone();
193 all_people.extend(people.into_iter().filter(|name| !pull.reviewers.contains(name)));
194 let mut all_teams = pull.team_reviewers.clone();
195 all_teams.extend(teams.into_iter().filter(|team| !pull.team_reviewers.contains(team)));
196 self.set_reviewers(pull, all_people, all_teams, actor, code_owners).await
197 }
198
199 /// Makes `people` and `teams` the ones asked to review `pull`: newly
200 /// asked teams pick their people (and record it), the timeline says
201 /// who was asked or no longer is, and `pull.review_requested` and
202 /// `pull.review_request_removed` say so to the inbox and webhooks.
203 /// `actor` is whoever asked; none for g1t.
204 pub(crate) async fn set_reviewers(
205 &self,
206 pull: &Pull,
207 mut people: Vec<String>,
208 teams: Vec<String>,
209 actor: Option<&User>,
210 code_owners: bool,
211 ) -> Result<()> {
212 let new_teams: Vec<String> = teams.iter().filter(|team| !pull.team_reviewers.contains(team)).cloned().collect();
213 let resolved = self.resolve_teams(&new_teams, &pull.repo_id).await?;
214 let asked = self.ask_teams(pull, &resolved, &people).await?;
215 for request in &asked {
216 for name in &request.assigned {
217 if !people.contains(name) {
218 people.push(name.clone());
219 }
220 }
221 }
222 self.db
223 .prepare("UPDATE pulls SET reviewers = ?, team_reviewers = ?, updated_at = ? WHERE id = ?")
224 .bind(&[
225 serde_json::to_string(&people)?.into(),
226 serde_json::to_string(&teams)?.into(),
227 rfc3339(now_ms()).into(),
228 pull.id.as_str().into(),
229 ])?
230 .run()
231 .await?;
232 let g1t = User {
233 id: crate::lifecycle::POLICY_ACTOR_ID.to_owned(),
234 username: crate::lifecycle::POLICY_ACTOR_NAME.to_owned(),
235 ..User::default()
236 };
237 let who = actor.unwrap_or(&g1t);
238 let handles = |names: &[String]| names.iter().map(|name| format!("@{name}")).collect::<Vec<_>>();
239 let verbs = ("requested a review from", "withdrew the request for a review from");
240 self.note_changes(&pull.repo_id, pull.number, who, &pull.reviewers, &people, verbs)
241 .await?;
242 self.note_changes(&pull.repo_id, pull.number, who, &handles(&pull.team_reviewers), &handles(&teams), verbs)
243 .await?;
244 let newly = |after: &[String], before: &[String]| -> Vec<String> {
245 after.iter().filter(|name| !before.contains(name)).cloned().collect()
246 };
247 let added = newly(&people, &pull.reviewers);
248 let removed = newly(&pull.reviewers, &people);
249 let teams_removed: Vec<TeamRequested> = newly(&pull.team_reviewers, &teams)
250 .into_iter()
251 .map(|team| TeamRequested {
252 team,
253 ..TeamRequested::default()
254 })
255 .collect();
256 let mut events: Vec<(&'static str, PullEvent)> = Vec::new();
257 if !added.is_empty() || !asked.is_empty() {
258 events.push((
259 "pull.review_requested",
260 PullEvent {
261 reviewers: Some(added),
262 teams: (!asked.is_empty()).then_some(asked),
263 code_owners,
264 ..Self::pull_event(pull)
265 },
266 ));
267 }
268 if !removed.is_empty() || !teams_removed.is_empty() {
269 events.push((
270 "pull.review_request_removed",
271 PullEvent {
272 reviewers: Some(removed),
273 teams: (!teams_removed.is_empty()).then_some(teams_removed),
274 ..Self::pull_event(pull)
275 },
276 ));
277 }
278 for (kind, data) in events {
279 self.publish_as(kind, &pull.repo_id, actor.map(|actor| actor.id.clone()), data)
280 .await?;
281 }
282 Ok(())
283 }
284
285 /// The teams named, as identity knows them, with their roles on the
286 /// repository.
287 pub(crate) async fn resolve_teams(&self, names: &[String], repo_id: &str) -> Result<Vec<ResolvedTeam>> {
288 if names.is_empty() {
289 return Ok(Vec::new());
290 }
291 g1t_kit::call(
292 &self.identity,
293 "resolve_teams",
294 &ResolveTeamsArgs {
295 teams: names.to_vec(),
296 repo_id: Some(repo_id.to_owned()),
297 asker: None,
298 },
299 )
300 .await
301 }
302
303 /// How many open pull requests wait on each person's review: they are
304 /// asked and have not given a verdict.
305 async fn waiting_on(&self, usernames: &[String]) -> Result<HashMap<String, u32>> {
306 if usernames.is_empty() {
307 return Ok(HashMap::new());
308 }
309 let rows = self
310 .db
311 .prepare(
312 "SELECT r.value AS username, count(*) AS n FROM pulls p, json_each(p.reviewers) r
313 WHERE p.status IN ('draft', 'open') AND r.value IN (SELECT value FROM json_each(?1))
314 AND NOT EXISTS (
315 SELECT 1 FROM comments c WHERE c.repo_id = p.repo_id AND c.number = p.number
316 AND lower(c.author_name) = r.value AND c.verdict IS NOT NULL
317 )
318 GROUP BY r.value",
319 )
320 .bind(&[serde_json::to_string(usernames)?.into()])?
321 .all()
322 .await?
323 .results::<WaitingRow>()?;
324 Ok(rows.into_iter().map(|row| (row.username, row.n)).collect())
325 }
326
327 /// When the team last had each person asked.
328 async fn last_asked(&self, team_id: &str) -> Result<HashMap<String, String>> {
329 let rows = self
330 .db
331 .prepare("SELECT username, max(requested_at) AS last FROM team_review_requests WHERE team_id = ? GROUP BY username")
332 .bind(&[team_id.into()])?
333 .all()
334 .await?
335 .results::<LastRow>()?;
336 Ok(rows.into_iter().map(|row| (row.username, row.last)).collect())
337 }
338
339 /// Asks each team to review `pull`: picks people where the team assigns
340 /// reviews, and records who was picked. `reviewers` are the people
341 /// already asked. Returns, per team, who is told and who was picked.
342 pub(crate) async fn ask_teams(&self, pull: &Pull, teams: &[ResolvedTeam], reviewers: &[String]) -> Result<Vec<TeamRequested>> {
343 let author = pull.owner().username.clone();
344 let mut asked = Vec::new();
345 let mut already = reviewers.to_vec();
346 for team in teams {
347 let settings = &team.review_assignment;
348 let picked = if settings.enabled {
349 let pool: Vec<String> = if settings.include_child_teams {
350 team.everyone().map(|person| person.username.clone()).collect()
351 } else {
352 team.members.iter().map(|person| person.username.clone()).collect()
353 };
354 let (last, waiting) = futures_util::future::try_join(self.last_asked(&team.id), self.waiting_on(&pool)).await?;
355 let candidates: Vec<Candidate> = pool
356 .iter()
357 .map(|name| Candidate {
358 username: name.clone(),
359 last_asked: last.get(name).cloned(),
360 waiting: waiting.get(name).copied().unwrap_or(0),
361 })
362 .collect();
363 let picked = pick(&candidates, settings, &author, &already);
364 let now = rfc3339(now_ms());
365 for name in &picked {
366 self.db
367 .prepare(
368 "INSERT INTO team_review_requests (team_id, username, repo_id, number, requested_at)
369 VALUES (?, ?, ?, ?, ?)",
370 )
371 .bind(&[
372 team.id.as_str().into(),
373 name.as_str().into(),
374 pull.repo_id.as_str().into(),
375 pull.number.into(),
376 now.as_str().into(),
377 ])?
378 .run()
379 .await?;
380 }
381 picked
382 } else {
383 Vec::new()
384 };
385 already.extend(picked.iter().cloned());
386 asked.push(told(team, &picked, &author));
387 }
388 Ok(asked)
389 }
390}
391
392#[cfg(test)]
393mod tests {
394 use super::*;
395 use g1t_contracts::teams::TeamPerson;
396
397 fn candidate(name: &str, last: Option<&str>, waiting: u32) -> Candidate {
398 Candidate {
399 username: name.to_owned(),
400 last_asked: last.map(str::to_owned),
401 waiting,
402 }
403 }
404
405 fn assigning(algorithm: ReviewAlgorithm, count: u32) -> ReviewAssignment {
406 ReviewAssignment {
407 enabled: true,
408 algorithm,
409 count,
410 ..ReviewAssignment::default()
411 }
412 }
413
414 fn team() -> Vec<Candidate> {
415 vec![
416 candidate("ana", Some("2026-10-05T10:00:00.000Z"), 1),
417 candidate("bo", None, 4),
418 candidate("cy", Some("2026-10-01T10:00:00.000Z"), 0),
419 candidate("dee", Some("2026-10-06T10:00:00.000Z"), 0),
420 candidate("eve", None, 2),
421 ]
422 }
423
424 #[test]
425 fn round_robin_asks_whoever_was_asked_least_recently() {
426 let settings = assigning(ReviewAlgorithm::RoundRobin, 3);
427 // Never asked first (by name), then the oldest.
428 assert_eq!(pick(&team(), &settings, "zed", &[]), vec!["bo", "eve", "cy"]);
429 // The same facts, the same people, every time.
430 assert_eq!(pick(&team(), &settings, "zed", &[]), pick(&team(), &settings, "zed", &[]));
431 }
432
433 #[test]
434 fn round_robin_moves_on_as_people_are_asked() {
435 let settings = assigning(ReviewAlgorithm::RoundRobin, 1);
436 let mut people = team();
437 let mut asked = Vec::new();
438 for turn in 0..5 {
439 let picked = pick(&people, &settings, "zed", &[]);
440 assert_eq!(picked.len(), 1);
441 let name = picked[0].clone();
442 let person = people.iter_mut().find(|person| person.username == name).unwrap();
443 person.last_asked = Some(format!("2026-10-07T10:00:0{turn}.000Z"));
444 asked.push(name);
445 }
446 // Everyone once before anyone twice.
447 let mut sorted = asked.clone();
448 sorted.sort();
449 assert_eq!(sorted, vec!["ana", "bo", "cy", "dee", "eve"]);
450 assert_eq!(asked, vec!["bo", "eve", "cy", "ana", "dee"]);
451 }
452
453 #[test]
454 fn load_balance_asks_whoever_has_the_least_waiting() {
455 let settings = assigning(ReviewAlgorithm::LoadBalance, 2);
456 // cy and dee have nothing waiting; cy was asked longer ago.
457 assert_eq!(pick(&team(), &settings, "zed", &[]), vec!["cy", "dee"]);
458 let settings = assigning(ReviewAlgorithm::LoadBalance, 4);
459 assert_eq!(pick(&team(), &settings, "zed", &[]), vec!["cy", "dee", "ana", "eve"]);
460 }
461
462 #[test]
463 fn the_author_g1t_and_those_left_out_are_never_asked() {
464 let mut people = team();
465 people.push(candidate("g1t", None, 0));
466 let settings = ReviewAssignment {
467 excluded: vec!["eve".into()],
468 ..assigning(ReviewAlgorithm::RoundRobin, 10)
469 };
470 let picked = pick(&people, &settings, "BO", &[]);
471 assert_eq!(picked, vec!["cy", "ana", "dee"]);
472 }
473
474 #[test]
475 fn busy_people_are_skipped_when_the_team_says_so() {
476 let settings = ReviewAssignment {
477 skip_busy: true,
478 busy_at: 2,
479 ..assigning(ReviewAlgorithm::RoundRobin, 10)
480 };
481 // bo (4) and eve (2) are busy.
482 assert_eq!(pick(&team(), &settings, "zed", &[]), vec!["cy", "ana", "dee"]);
483 }
484
485 #[test]
486 fn people_already_asked_count_towards_the_number() {
487 let settings = assigning(ReviewAlgorithm::RoundRobin, 2);
488 assert_eq!(pick(&team(), &settings, "zed", &["bo".into()]), vec!["eve"]);
489 assert!(pick(&team(), &settings, "zed", &["bo".into(), "Cy".into()]).is_empty());
490 // Someone asked who is not in the team does not count.
491 assert_eq!(pick(&team(), &settings, "zed", &["outsider".into()]), vec!["bo", "eve"]);
492 }
493
494 fn resolved(enabled: bool, notify_team: bool) -> ResolvedTeam {
495 let person = |name: &str| TeamPerson {
496 id: format!("usr_{name}"),
497 username: name.to_owned(),
498 };
499 ResolvedTeam {
500 id: "team_1".into(),
501 workspace: "acme".into(),
502 slug: "backend".into(),
503 members: vec![person("ana"), person("bo"), person("cy")],
504 child_members: vec![person("dee")],
505 review_assignment: ReviewAssignment {
506 enabled,
507 notify_team,
508 ..ReviewAssignment::default()
509 },
510 ..ResolvedTeam::default()
511 }
512 }
513
514 #[test]
515 fn a_request_tells_the_whole_team_or_the_people_picked() {
516 // Off: everyone, child teams too, never the author.
517 let all = told(&resolved(false, false), &[], "bo");
518 assert_eq!(all.team, "acme/backend");
519 assert_eq!(all.notified, vec!["ana", "cy", "dee"]);
520 assert!(all.assigned.is_empty());
521 // On: the people picked.
522 let picked = told(&resolved(true, false), &["cy".into()], "bo");
523 assert_eq!(picked.notified, vec!["cy"]);
524 assert_eq!(picked.assigned, vec!["cy"]);
525 // On, telling the team too: the picked first.
526 let both = told(&resolved(true, true), &["cy".into()], "bo");
527 assert_eq!(both.notified, vec!["cy", "ana", "dee"]);
528 }
529
530 #[test]
531 fn only_the_workspaces_teams_are_asked_and_secret_ones_by_their_people() {
532 let actor = |id: &str, role: Option<Role>| User {
533 id: id.to_owned(),
534 username: id.to_owned(),
535 workspaces: role
536 .map(|role| g1t_contracts::Membership {
537 role,
538 ..g1t_contracts::Membership::member("acme")
539 })
540 .into_iter()
541 .collect(),
542 ..User::default()
543 };
544 let mut team = resolved(false, false);
545 assert!(may_ask(&team, &actor("usr_zed", Some(Role::Member)), "acme").is_ok());
546 assert!(may_ask(&team, &actor("usr_zed", Some(Role::Member)), "globex").is_err());
547 team.visibility = TeamVisibility::Secret;
548 assert!(may_ask(&team, &actor("usr_zed", Some(Role::Member)), "acme").is_err());
549 assert!(may_ask(&team, &actor("usr_ana", Some(Role::Member)), "acme").is_ok());
550 // Through a child team too.
551 assert!(may_ask(&team, &actor("usr_dee", Some(Role::Member)), "acme").is_ok());
552 assert!(may_ask(&team, &actor("usr_zed", Some(Role::Owner)), "acme").is_ok());
553 }
554
555 #[test]
556 fn teams_are_named_workspace_slash_slug() {
557 assert_eq!(team_name("@Acme/Backend").as_deref(), Some("acme/backend"));
558 assert_eq!(team_name("acme/backend").as_deref(), Some("acme/backend"));
559 assert_eq!(team_name("ana"), None);
560 assert_eq!(team_name("acme/"), None);
561 }
562}