1use anyhow::Result;
18
19use crate::queue::{FollowUp, Queue, Source, Task};
20use crate::run::{FollowupRecord, RunState};
21use crate::verdict::{Finding, ReviewVote};
22
23pub const NODE: &str = "followup";
25
26pub const MAX_FOLLOWUP_GENERATION: u32 = 2;
28
29const LINE_WINDOW: u32 = 5;
31
32#[derive(Debug, Default, PartialEq, Eq)]
34pub struct Outcome {
35 pub filed: Vec<String>,
37 pub capped: Vec<String>,
39 pub unfiled: Vec<String>,
41 pub covered: Vec<(String, String)>,
44}
45
46pub async fn after_merge(state: &mut RunState, pr_url: &str) {
49 let queue = Queue::open();
50 let release = state.config.graph.release_deputy_followups;
51 settle_deputy_tasks(state, pr_url, &queue, release);
54 let outcome = if state.config.graph.file_followups {
55 match file(state, pr_url, &queue) {
56 Ok(o) => o,
57 Err(e) => {
58 state.event(NODE, format!("follow-up filing failed: {e:#}"));
59 return;
60 }
61 }
62 } else {
63 Outcome::default()
64 };
65 if state.config.graph.file_followups && !release {
66 settle_deputy_tasks(state, pr_url, &queue, false);
67 }
68 if outcome.filed.is_empty()
69 && (state.followup_commented
70 || (state.followups.is_empty()
71 && outcome.covered.is_empty()
72 && state.deputy_followups.is_empty()))
73 {
74 return;
75 }
76 let body = comment_body(state, &outcome);
77 match crate::bump::gh_pr_comment(state, pr_url, &body).await {
78 Ok(()) => state.followup_commented = true,
79 Err(e) => state.event(NODE, format!("follow-up comment not posted: {e:#}")),
80 }
81}
82
83const SUPERSEDED: &str = "[superseded] ";
86const CAPPED: &str = "[generation cap] ";
87
88fn is_deputy_candidate(t: &Task, run: &str, pr_url: &str) -> bool {
93 t.status == crate::queue::TaskStatus::Held
94 && t.followup.is_none()
95 && matches!(&t.source, Source::Agent { run: r, node }
96 if r == run && node == crate::deputy::NODE)
97 && t.hold_reason.as_deref().is_some_and(|r| {
98 !r.starts_with(SUPERSEDED) && !r.starts_with(CAPPED) && names_pr(r, pr_url)
99 })
100}
101
102fn task_matches_finding(t: &Task, f: &Finding) -> bool {
106 let text = format!("{}\n{}", t.title, t.instruction);
107 if names_word(&text, &f.id) {
108 return true;
109 }
110 if let (Some(file), Some(line)) = (&f.file, f.line)
111 && text.match_indices(file.as_str()).any(|(i, _)| {
112 text[i + file.len()..]
113 .strip_prefix(':')
114 .map(|r| {
115 r.chars()
116 .take_while(char::is_ascii_digit)
117 .collect::<String>()
118 })
119 .and_then(|d| d.parse::<u32>().ok())
120 .is_some_and(|n| n.abs_diff(line) <= LINE_WINDOW)
121 })
122 {
123 return true;
124 }
125 let (nt, nf) = (normalize(&t.title), normalize(&f.title));
126 !nf.is_empty() && (nt == nf || (nf.split(' ').count() >= 3 && nt.contains(&nf)))
127}
128
129fn parent_generation(state: &mut RunState, queue: &Queue, origin_task: Option<&str>) -> u32 {
131 match state.followup_generation {
132 Some(g) => g,
133 None => match origin_task.map(|t| queue.get(t)) {
134 Some(Ok(t)) => {
135 let g = t.followup.map_or(0, |f| f.generation);
136 state.followup_generation = Some(g);
137 g
138 }
139 Some(Err(_)) => MAX_FOLLOWUP_GENERATION,
140 None => 0,
141 },
142 }
143}
144
145pub fn settle_deputy_tasks(state: &mut RunState, pr_url: &str, queue: &Queue, release: bool) {
165 let tasks = queue.list();
166 for t in &tasks {
168 if t.followup.as_ref().is_some_and(|f| f.run == state.id)
169 && matches!(&t.source, Source::Agent { node, .. } if node == crate::deputy::NODE)
170 && !state.deputy_followups.contains(&t.id)
171 {
172 state.deputy_followups.push(t.id.clone());
173 }
174 }
175 let cands: Vec<&Task> = tasks
176 .iter()
177 .filter(|t| is_deputy_candidate(t, &state.id, pr_url))
178 .collect();
179 if cands.is_empty() {
180 return;
181 }
182 let chosen = state
183 .reviews
184 .last()
185 .map(|r| select(r).0)
186 .unwrap_or_default();
187 let groups = group_findings(chosen);
188 let autos: Vec<&Task> = tasks
189 .iter()
190 .filter(|t| {
191 t.followup.as_ref().is_some_and(|f| f.run == state.id)
192 && matches!(&t.source, Source::Agent { node, .. } if node == NODE)
193 })
194 .collect();
195 let origin_task = state.origin.as_ref().and_then(|o| o.task.clone());
196 let mut gen_of_parent = None;
197 for cand in cands {
198 let matched: Vec<String> = groups
200 .iter()
201 .filter(|g| g.iter().any(|f| task_matches_finding(cand, f)))
202 .flat_map(|g| g.iter().map(|f| f.id.clone()))
203 .collect();
204 let rivals: Vec<&&Task> = autos
205 .iter()
206 .filter(|a| {
207 a.followup
208 .as_ref()
209 .is_some_and(|f| f.findings.iter().any(|x| matched.contains(x)))
210 })
211 .collect();
212 if release {
213 let parent_gen = *gen_of_parent
214 .get_or_insert_with(|| parent_generation(state, queue, origin_task.as_deref()));
215 if parent_gen >= MAX_FOLLOWUP_GENERATION {
216 let why = format!(
217 "{CAPPED}generation {parent_gen} reached, not released after {pr_url} merged; was: {}",
218 cand.hold_reason.as_deref().unwrap_or_default()
219 );
220 mark_held(state, queue, cand, why, pr_url, "generation cap");
221 continue;
222 }
223 }
224 if !rivals.is_empty() {
225 let mut guards = Vec::new();
230 let mut blocker: Option<&Task> = None;
231 if release {
232 for a in &rivals {
233 if a.status == crate::queue::TaskStatus::Held {
234 continue;
235 }
236 match queue.claim(&a.id) {
237 Ok(g) => guards.push((a.id.clone(), g)),
238 Err(_) => {
239 blocker = Some(a);
240 break;
241 }
242 }
243 }
244 if blocker.is_none() {
245 blocker = rivals.iter().map(|a| &***a).find(|a| {
246 !matches!(
247 queue.get(&a.id).map(|t| t.status),
248 Ok(crate::queue::TaskStatus::Queued | crate::queue::TaskStatus::Held)
249 )
250 });
251 }
252 } else {
253 blocker = Some(&***rivals.first().expect("non-empty"));
254 }
255 if let Some(a) = blocker {
256 drop(guards);
257 let why = format!(
258 "{SUPERSEDED}follow-up task {} ({:?}) already covers {} of {pr_url}; was: {}",
259 a.id,
260 a.status,
261 matched.join(", "),
262 cand.hold_reason.as_deref().unwrap_or_default()
263 );
264 mark_held(state, queue, cand, why, pr_url, "superseded");
265 continue;
266 }
267 let mut failed: Option<String> = None;
270 for (id, _guard) in &guards {
271 let why = format!(
272 "{SUPERSEDED}the owner's approval deputy filed task {} for the same finding(s) of {pr_url}",
273 cand.id
274 );
275 let held = queue.modify(id, |t| {
276 if t.status != crate::queue::TaskStatus::Queued {
277 return false;
278 }
279 t.hold_manual(Some(why));
280 true
281 });
282 match held {
283 Ok(true) => state.event(
284 NODE,
285 format!(
286 "follow-up task {id} held: superseded by deputy task {}",
287 cand.id
288 ),
289 ),
290 Ok(false) => {
291 failed = Some(id.clone());
292 break;
293 }
294 Err(e) => {
295 state.event(NODE, format!("could not hold follow-up {id}: {e:#}"));
296 failed = Some(id.clone());
297 break;
298 }
299 }
300 }
301 if let Some(id) = failed {
302 drop(guards);
303 let why = format!(
304 "{SUPERSEDED}follow-up task {id} could not be held, so it may still run for {} of {pr_url}; was: {}",
305 matched.join(", "),
306 cand.hold_reason.as_deref().unwrap_or_default()
307 );
308 mark_held(state, queue, cand, why, pr_url, "superseded");
309 continue;
310 }
311 } else if !release {
312 continue;
313 }
314 let parent_gen = gen_of_parent.expect("computed when releasing");
315 let stamp = FollowUp {
316 run: state.id.clone(),
317 origin_task: origin_task.clone(),
318 pr: pr_url.to_owned(),
319 findings: matched,
320 generation: parent_gen + 1,
321 };
322 let id = cand.id.clone();
323 let run = state.id.clone();
324 let result = queue.modify(&id, |t| {
325 if !is_deputy_candidate(t, &run, pr_url) {
326 return false;
327 }
328 t.release();
329 t.followup = Some(stamp);
330 true
331 });
332 match result {
333 Ok(true) => {
334 if !state.deputy_followups.contains(&id) {
335 state.deputy_followups.push(id.clone());
336 }
337 state.event(
338 NODE,
339 format!("released deputy follow-up task {id} after {pr_url} merged"),
340 );
341 }
342 Ok(false) => {}
343 Err(e) => state.event(NODE, format!("could not release deputy task {id}: {e:#}")),
344 }
345 }
346}
347
348fn mark_held(
350 state: &mut RunState,
351 queue: &Queue,
352 cand: &Task,
353 why: String,
354 pr_url: &str,
355 kind: &str,
356) {
357 let run = state.id.clone();
358 let reason = why.clone();
359 match queue.modify(&cand.id, |t| {
360 if !is_deputy_candidate(t, &run, pr_url) {
361 return false;
362 }
363 t.hold_reason = Some(reason);
364 true
365 }) {
366 Ok(true) => state.event(
367 NODE,
368 format!("deputy task {} left held ({kind}): {why}", cand.id),
369 ),
370 Ok(false) => {}
371 Err(e) => state.event(
372 NODE,
373 format!("could not mark deputy task {}: {e:#}", cand.id),
374 ),
375 }
376}
377
378fn select(round: &crate::run::ReviewRound) -> (Vec<Finding>, Vec<String>) {
382 let rejecters: Vec<usize> = round
383 .final_votes()
384 .into_iter()
385 .filter(|(_, _, v)| *v == ReviewVote::Reject)
386 .map(|(seat, _, _)| seat)
387 .collect();
388 let (mut chosen, mut unfiled) = (Vec::new(), Vec::new());
389 for rec in &round.reviews {
390 let rejected = rejecters.contains(&rec.reviewer);
391 for f in &rec.findings {
392 if f.severity.blocks() || rejected {
393 chosen.push(f.clone());
394 } else {
395 unfiled.push(f.id.clone());
396 }
397 }
398 }
399 (chosen, unfiled)
400}
401
402pub fn file(state: &mut RunState, pr_url: &str, queue: &Queue) -> Result<Outcome> {
406 let mut out = Outcome::default();
407 let Some(round) = state.reviews.last().cloned() else {
408 return Ok(out);
409 };
410 let (chosen, unfiled) = select(&round);
411 out.unfiled = unfiled;
412 if chosen.is_empty() {
413 return Ok(out);
414 }
415
416 let origin_task = state.origin.as_ref().and_then(|o| o.task.clone());
417 let parent_gen = parent_generation(state, queue, origin_task.as_deref());
421 if parent_gen >= MAX_FOLLOWUP_GENERATION {
422 out.capped = chosen.iter().map(|f| f.id.clone()).collect();
423 state.event(
424 NODE,
425 format!(
426 "generation cap reached ({parent_gen}); not filing follow-ups for open finding(s): {}",
427 out.capped.join(", ")
428 ),
429 );
430 return Ok(out);
431 }
432
433 let mut done: Vec<String> = state
434 .followups
435 .iter()
436 .flat_map(|r| r.findings.iter().cloned())
437 .collect();
438 for t in queue.list() {
439 if let Some(f) = t.followup
440 && f.run == state.id
441 {
442 done.extend(f.findings.iter().cloned());
443 let by_deputy =
444 matches!(&t.source, Source::Agent { node, .. } if node == crate::deputy::NODE);
445 if !by_deputy && !state.followups.iter().any(|r| r.task == t.id) {
446 state.followups.push(FollowupRecord {
447 task: t.id,
448 findings: f.findings,
449 });
450 }
451 }
452 }
453
454 let tasks = queue.list();
455 let origin_chat = state.origin_chat.clone().or_else(|| {
456 let tasks = queue.list();
457 let parent = origin_task
458 .as_deref()
459 .and_then(|id| tasks.iter().find(|t| t.id == id))?;
460 crate::consult::chat_talk_of(&tasks, parent)
461 });
462 let mut failure = None;
463 for group in group_findings(chosen) {
464 let ids: Vec<String> = group.iter().map(|f| f.id.clone()).collect();
465 let own = task_id(&state.id, &ids);
466 let covers: Vec<Option<String>> = ids
467 .iter()
468 .map(|id| covering_task(&tasks, &own, &state.id, pr_url, id))
469 .collect();
470 if covers.iter().all(Option::is_some) {
471 for (id, by) in ids.iter().zip(covers.into_iter().flatten()) {
472 state.event(
473 NODE,
474 format!("not filing follow-up for {id}: already covered by task {by}"),
475 );
476 out.covered.push((id.clone(), by));
477 }
478 continue;
479 }
480 if ids.iter().any(|id| done.contains(id)) {
481 continue;
482 }
483 let mut task = build_task(state, pr_url, origin_task.clone(), &group, parent_gen + 1);
484 task.origin_chat = origin_chat.clone();
485 match queue.create_new(&mut task) {
486 Ok(created) => {
487 if created {
488 out.filed.push(task.id.clone());
489 }
490 state.followups.push(FollowupRecord {
491 task: task.id,
492 findings: ids,
493 });
494 }
495 Err(e) => {
496 failure = Some(e);
497 break;
498 }
499 }
500 }
501 if !out.filed.is_empty() {
502 state.event(
503 NODE,
504 format!(
505 "filed {} follow-up task(s): ids {}",
506 out.filed.len(),
507 out.filed.join(", ")
508 ),
509 );
510 }
511 if let Some(e) = failure {
512 state.event(NODE, format!("follow-up filing stopped: {e:#}"));
513 }
514 Ok(out)
515}
516
517fn covering_task(tasks: &[Task], own: &str, run: &str, pr_url: &str, id: &str) -> Option<String> {
522 tasks
523 .iter()
524 .find(|t| {
525 t.id != own
526 && (t
527 .followup
528 .as_ref()
529 .is_some_and(|f| f.run == run && f.findings.iter().any(|x| x == id))
530 || (names_word(&t.instruction, id) && names_pr(&t.instruction, pr_url)))
531 })
532 .map(|t| t.id.clone())
533}
534
535fn is_name_char(c: char) -> bool {
538 c.is_ascii_alphanumeric() || matches!(c, '_' | '-')
539}
540
541fn names_word(text: &str, needle: &str) -> bool {
543 !needle.is_empty()
544 && text.match_indices(needle).any(|(i, _)| {
545 let before = text[..i].chars().next_back();
546 let after = text[i + needle.len()..].chars().next();
547 before.is_none_or(|c| !is_name_char(c)) && after.is_none_or(|c| !is_name_char(c))
548 })
549}
550
551fn names_pr(text: &str, pr_url: &str) -> bool {
554 let url = pr_url.trim_end_matches('/');
555 let digit_after = |rest: &str| rest.chars().next().is_some_and(|c| c.is_ascii_digit());
556 if !url.is_empty()
557 && text
558 .match_indices(url)
559 .any(|(i, _)| !digit_after(&text[i + url.len()..]))
560 {
561 return true;
562 }
563 let Some(n) = url
564 .rsplit('/')
565 .next()
566 .filter(|n| !n.is_empty() && n.bytes().all(|b| b.is_ascii_digit()))
567 else {
568 return false;
569 };
570 let tag = format!("#{n}");
571 text.match_indices(&tag).any(|(i, _)| {
572 let before = text[..i].chars().next_back();
573 before.is_none_or(|c| !is_name_char(c)) && !digit_after(&text[i + tag.len()..])
574 })
575}
576
577fn group_findings(mut findings: Vec<Finding>) -> Vec<Vec<Finding>> {
581 findings.sort_by(|a, b| (&a.file, a.line, &a.id).cmp(&(&b.file, b.line, &b.id)));
582 let mut groups: Vec<Vec<Finding>> = Vec::new();
583 for f in findings {
584 let hit = groups
585 .iter()
586 .position(|g| g.iter().any(|o| same_defect(o, &f)));
587 match hit {
588 Some(i) => groups[i].push(f),
589 None => groups.push(vec![f]),
590 }
591 }
592 for g in &mut groups {
593 g.sort_by(|a, b| a.id.cmp(&b.id));
594 }
595 groups
596}
597
598fn same_defect(a: &Finding, b: &Finding) -> bool {
599 match (&a.file, &b.file) {
600 (Some(x), Some(y)) => {
601 x == y
602 && match (a.line, b.line) {
603 (Some(l), Some(m)) => l.abs_diff(m) <= LINE_WINDOW,
604 (None, None) => true,
605 _ => false,
606 }
607 }
608 (None, None) => normalize(&a.title) == normalize(&b.title),
609 _ => false,
610 }
611}
612
613fn normalize(title: &str) -> String {
614 title
615 .to_lowercase()
616 .split(|c: char| !c.is_alphanumeric())
617 .filter(|w| !w.is_empty())
618 .collect::<Vec<_>>()
619 .join(" ")
620}
621
622fn task_id(run: &str, ids: &[String]) -> String {
626 let mut h: u64 = 0xcbf2_9ce4_8422_2325;
627 for b in ids.join("+").bytes() {
628 h ^= u64::from(b);
629 h = h.wrapping_mul(0x0100_0000_01b3);
630 }
631 format!("{run}-f{:05x}", h & 0xf_ffff)
632}
633
634fn location(f: &Finding) -> String {
635 match (&f.file, f.line) {
636 (Some(p), Some(l)) => format!("{p}:{l}"),
637 (Some(p), None) => p.clone(),
638 _ => "(no location)".to_owned(),
639 }
640}
641
642fn build_task(
643 state: &RunState,
644 pr_url: &str,
645 origin_task: Option<String>,
646 group: &[Finding],
647 generation: u32,
648) -> Task {
649 let ids: Vec<String> = group.iter().map(|f| f.id.clone()).collect();
650 let first = &group[0];
651 let title = format!(
652 "follow-up: {} ({})",
653 crate::queue::title_from(&first.title, 80),
654 location(first)
655 );
656 let mut s = format!(
657 "This is a follow-up to a change that was already merged: run {run} merged \
658 {pr_url} while the review finding(s) below were still open. Branch from the \
659 current main - do not use the merged branch - and fix the defect described, \
660 keeping the change small. If a finding no longer applies to current main, say so \
661 and change nothing for it.\n\n\
662 Merged run: {run}\nPull request: {pr_url}\n",
663 run = state.id,
664 );
665 if let Some(t) = &origin_task {
666 s.push_str(&format!("Original task: {t}\n"));
667 }
668 s.push_str("\n## Findings\n");
669 for f in group {
670 s.push_str(&format!(
671 "\n### {} [{:?}] {}\n\nLocation: {}\n\n{}\n",
672 f.id,
673 f.severity,
674 f.title,
675 location(f),
676 f.detail.trim()
677 ));
678 }
679 let mut task = Task::new(
680 title,
681 s,
682 state.repo.clone(),
683 Source::Agent {
684 run: state.id.clone(),
685 node: NODE.to_owned(),
686 },
687 );
688 task.id = task_id(&state.id, &ids);
689 task.solo = true;
690 task.followup = Some(FollowUp {
691 run: state.id.clone(),
692 origin_task,
693 pr: pr_url.to_owned(),
694 findings: ids,
695 generation,
696 });
697 task
698}
699
700fn comment_body(state: &RunState, out: &Outcome) -> String {
701 let mut s = format!(
702 "<!-- magi-followup run={} -->\nThis pull request merged with review findings still open.",
703 state.id
704 );
705 if state.followups.is_empty() {
706 if !out.covered.is_empty() || state.deputy_followups.is_empty() {
707 s.push_str(" Each is already covered by another task:\n");
708 }
709 } else {
710 s.push_str(" They were filed as follow-up tasks:\n\n");
711 for r in &state.followups {
712 s.push_str(&format!("- `{}`: {}\n", r.task, r.findings.join(", ")));
713 }
714 if !out.covered.is_empty() {
715 s.push_str("\nNot filed, already covered:\n");
716 }
717 }
718 if !out.covered.is_empty() {
719 s.push('\n');
720 }
721 for (id, by) in &out.covered {
722 s.push_str(&format!("- {id}: already covered by task `{by}`\n"));
723 }
724 if !state.deputy_followups.is_empty() {
725 s.push_str(
726 "\nFollow-up tasks filed by the approval deputy and released after the merge:\n\n",
727 );
728 for id in &state.deputy_followups {
729 s.push_str(&format!("- `{id}`\n"));
730 }
731 }
732 if !out.unfiled.is_empty() {
733 s.push_str(&format!(
734 "\nMinor/nit findings that were only listed, not filed: {}\n",
735 out.unfiled.join(", ")
736 ));
737 }
738 s
739}
740
741#[cfg(test)]
742mod tests {
743 use super::*;
744 use crate::config::Config;
745 use crate::queue::Queue;
746 use crate::run::ReviewRecord;
747 use crate::run::{Origin, ReviewRound, RunState};
748 use crate::verdict::{Finding, ReviewVote, Severity};
749 use std::path::PathBuf;
750
751 fn finding(id: &str, sev: Severity, file: &str, line: u32) -> Finding {
752 Finding {
753 id: id.to_owned(),
754 severity: sev,
755 file: Some(file.to_owned()),
756 line: Some(line),
757 title: format!("title {id}"),
758 detail: format!("detail of {id}"),
759 }
760 }
761
762 fn record(seat: usize, findings: Vec<Finding>, vote: ReviewVote) -> ReviewRecord {
763 ReviewRecord {
764 attempts: 0,
765 reviewer: seat,
766 agent: "alpha".to_owned(),
767 summary: String::new(),
768 findings,
769 vote: Some(vote),
770 failed: None,
771 duration_ms: 0,
772 }
773 }
774
775 fn round(reviews: Vec<ReviewRecord>) -> ReviewRound {
776 ReviewRound {
777 round: 3,
778 head: "deadbee".to_owned(),
779 verified_head: None,
780 verified_at: None,
781 reviews,
782 e2e: Vec::new(),
783 verify_retried: false,
784 e2e_deferred: false,
785 e2e_defer_reason: None,
786 fix: None,
787 blocking: 0,
788 answered: 2,
789 expected: 2,
790 clean: false,
791 progressed: false,
792 vote_split: false,
793 reconsideration: Vec::new(),
794 verdict: None,
795 }
796 }
797
798 fn merged(reviews: Vec<ReviewRecord>) -> RunState {
799 let mut s = RunState::new(
800 PathBuf::from("/repo"),
801 "main".to_owned(),
802 "abc1234".to_owned(),
803 "do it".to_owned(),
804 Config::default(),
805 );
806 s.reviews.push(round(reviews));
807 s
808 }
809
810 fn two_seats() -> Vec<ReviewRecord> {
811 vec![
812 record(
813 1,
814 vec![
815 finding("R3-1-1", Severity::Major, "src/waiter.rs", 211),
816 finding("R3-1-2", Severity::Minor, "src/a.rs", 1),
817 ],
818 ReviewVote::Reject,
819 ),
820 record(
821 2,
822 vec![
823 finding("R3-2-1", Severity::Major, "src/waiter.rs", 213),
824 finding("R3-2-2", Severity::Nit, "src/b.rs", 9),
825 ],
826 ReviewVote::Approve,
827 ),
828 ]
829 }
830
831 #[test]
832 fn one_task_per_distinct_defect_with_the_findings_in_full() {
833 let dir = tempfile::tempdir().unwrap();
834 let q = Queue::at(dir.path().join("queue"));
835 let mut s = merged(two_seats());
836 let out = file(&mut s, "https://example.invalid/o/r/pull/9", &q).unwrap();
837 assert_eq!(out.filed.len(), 2, "{out:?}");
840 assert_eq!(out.unfiled, vec!["R3-2-2".to_owned()]);
841 let tasks = q.list();
842 let t = tasks
843 .iter()
844 .find(|t| t.followup.as_ref().unwrap().findings.len() == 2)
845 .unwrap();
846 assert!(t.solo);
847 assert_eq!(t.followup.as_ref().unwrap().generation, 1);
848 assert_eq!(
849 t.source,
850 Source::Agent {
851 run: s.id.clone(),
852 node: NODE.to_owned()
853 }
854 );
855 for needle in [
856 "R3-1-1",
857 "R3-2-1",
858 "src/waiter.rs:211",
859 "detail of R3-2-1",
860 "pull/9",
861 &s.id,
862 "current main",
863 ] {
864 assert!(
865 t.instruction.contains(needle),
866 "{needle}: {}",
867 t.instruction
868 );
869 }
870 assert!(
871 s.events
872 .iter()
873 .any(|e| e.message.contains("filed 2 follow-up"))
874 );
875 assert_eq!(s.followups.len(), 2);
876 }
877
878 #[test]
879 fn filing_twice_does_not_duplicate() {
880 let dir = tempfile::tempdir().unwrap();
881 let q = Queue::at(dir.path().join("queue"));
882 let mut s = merged(two_seats());
883 file(&mut s, "u", &q).unwrap();
884 let again = file(&mut s, "u", &q).unwrap();
885 assert!(again.filed.is_empty());
886 assert_eq!(q.list().len(), 2);
887 s.followups.clear();
889 let third = file(&mut s, "u", &q).unwrap();
890 assert!(third.filed.is_empty());
891 assert_eq!(q.list().len(), 2);
892 assert_eq!(s.followups.len(), 2, "records are restored from the queue");
893 }
894
895 #[test]
896 fn a_clean_merge_files_nothing() {
897 let dir = tempfile::tempdir().unwrap();
898 let q = Queue::at(dir.path().join("queue"));
899 let mut s = merged(vec![record(
900 1,
901 vec![finding("R1-1-1", Severity::Minor, "a.rs", 1)],
902 ReviewVote::Approve,
903 )]);
904 let out = file(&mut s, "u", &q).unwrap();
905 assert!(out.filed.is_empty());
906 assert!(q.list().is_empty());
907 assert!(s.followups.is_empty());
908 }
909
910 #[test]
911 fn a_failing_queue_is_an_error_event_not_a_status_change() {
912 let dir = tempfile::tempdir().unwrap();
913 let root = dir.path().join("queue");
914 std::fs::write(&root, "not a directory").unwrap();
915 let q = Queue::at(root);
916 let mut s = merged(two_seats());
917 s.status = crate::run::RunStatus::Merged;
918 let _ = file(&mut s, "u", &q);
919 assert_eq!(s.status, crate::run::RunStatus::Merged);
920 assert!(
921 s.events
922 .iter()
923 .any(|e| e.node == NODE && e.message.contains("stopped"))
924 );
925 assert!(s.followups.is_empty());
926 }
927
928 #[test]
929 fn the_generation_cap_stops_the_chain() {
930 let dir = tempfile::tempdir().unwrap();
931 let q = Queue::at(dir.path().join("queue"));
932 let mut parent = Task::new(
933 "p".to_owned(),
934 "i".to_owned(),
935 PathBuf::from("/repo"),
936 Source::Human,
937 );
938 parent.followup = Some(FollowUp {
939 run: "r0".to_owned(),
940 origin_task: None,
941 pr: "u".to_owned(),
942 findings: vec!["R1-1-1".to_owned()],
943 generation: MAX_FOLLOWUP_GENERATION,
944 });
945 q.put(&mut parent).unwrap();
946 let mut s = merged(two_seats());
947 s.origin = Some(Origin {
948 by: crate::run::StartedBy::Operator,
949 task: Some(parent.id.clone()),
950 });
951 s.origin = Some(Origin {
952 by: crate::run::StartedBy::Operator,
953 task: Some("gone".to_owned()),
954 });
955 s.followup_generation = Some(MAX_FOLLOWUP_GENERATION);
956 let out = file(&mut s, "u", &q).unwrap();
957 assert!(out.filed.is_empty());
958 assert!(!out.capped.is_empty());
959 assert_eq!(q.list().len(), 1);
960 assert!(
961 s.events
962 .iter()
963 .any(|e| e.message.contains("generation cap"))
964 );
965 }
966
967 #[test]
968 fn a_deeper_task_files_one_generation_further() {
969 let dir = tempfile::tempdir().unwrap();
970 let q = Queue::at(dir.path().join("queue"));
971 let mut parent = Task::new(
972 "p".to_owned(),
973 "i".to_owned(),
974 PathBuf::from("/repo"),
975 Source::Human,
976 );
977 q.put(&mut parent).unwrap();
978 let mut s = merged(two_seats());
979 s.origin = Some(Origin {
980 by: crate::run::StartedBy::Operator,
981 task: Some(parent.id.clone()),
982 });
983 file(&mut s, "u", &q).unwrap();
984 let t = q.list().into_iter().find(|t| t.followup.is_some()).unwrap();
985 assert_eq!(t.followup.unwrap().origin_task, Some(parent.id));
986 }
987
988 #[test]
989 fn the_chat_is_inherited_from_the_run_even_when_the_parent_is_gone() {
990 let dir = tempfile::tempdir().unwrap();
991 let q = Queue::at(dir.path().join("queue"));
992 let mut s = merged(two_seats());
993 s.origin = Some(Origin {
994 by: crate::run::StartedBy::Operator,
995 task: Some("gone".to_owned()),
996 });
997 s.followup_generation = Some(1);
998 s.origin_chat = Some("talk-1".to_owned());
999 file(&mut s, "u", &q).unwrap();
1000 let t = q.list().into_iter().find(|t| t.followup.is_some()).unwrap();
1001 assert_eq!(t.origin_chat.as_deref(), Some("talk-1"));
1002 }
1003
1004 #[test]
1005 fn a_parent_without_the_field_passes_on_what_its_ancestry_resolves() {
1006 let dir = tempfile::tempdir().unwrap();
1007 let q = Queue::at(dir.path().join("queue"));
1008 let mut parent = Task::new(
1009 "p".to_owned(),
1010 "i".to_owned(),
1011 PathBuf::from("/repo"),
1012 Source::Agent {
1013 run: "talk-2".to_owned(),
1014 node: crate::queue::CHAT_NODE.to_owned(),
1015 },
1016 );
1017 parent.origin_chat = None;
1018 q.put(&mut parent).unwrap();
1019 let mut s = merged(two_seats());
1020 s.origin = Some(Origin {
1021 by: crate::run::StartedBy::Operator,
1022 task: Some(parent.id.clone()),
1023 });
1024 file(&mut s, "u", &q).unwrap();
1025 let t = q.list().into_iter().find(|t| t.followup.is_some()).unwrap();
1026 assert_eq!(t.origin_chat.as_deref(), Some("talk-2"));
1027 }
1028
1029 const PR: &str = "https://github.com/o/r/pull/473";
1030
1031 fn manual(text: &str) -> Task {
1032 Task::new(
1033 "manual".to_owned(),
1034 text.to_owned(),
1035 PathBuf::from("/repo"),
1036 Source::Human,
1037 )
1038 }
1039
1040 fn filed_with(text: &str) -> (Outcome, RunState, Queue, String, tempfile::TempDir) {
1042 let dir = tempfile::tempdir().unwrap();
1043 let q = Queue::at(dir.path().join("queue"));
1044 let mut t = manual(text);
1045 t.status = crate::queue::TaskStatus::Done;
1046 q.put(&mut t).unwrap();
1047 let mut s = merged(two_seats());
1048 let out = file(&mut s, PR, &q).unwrap();
1049 (out, s, q, t.id, dir)
1050 }
1051
1052 #[test]
1053 fn a_task_naming_the_pr_url_and_ids_covers_the_group() {
1054 let (out, s, _q, id, _d) = filed_with(&format!("Fix R3-1-1 and R3-2-1 from {PR}"));
1055 assert_eq!(out.filed.len(), 1, "{out:?}");
1057 assert_eq!(
1058 out.covered,
1059 vec![
1060 ("R3-1-1".to_owned(), id.clone()),
1061 ("R3-2-1".to_owned(), id.clone())
1062 ]
1063 );
1064 assert!(out.capped.is_empty());
1065 assert!(
1066 s.events
1067 .iter()
1068 .any(|e| e.message.contains("already covered"))
1069 );
1070 let body = comment_body(&s, &out);
1071 assert!(body.contains(&format!("R3-1-1: already covered by task `{id}`")));
1072 }
1073
1074 #[test]
1075 fn the_hash_form_covers_but_a_prefix_number_does_not() {
1076 let (out, ..) = filed_with("R3-1-1 R3-2-1 fixed in PR #473");
1077 assert_eq!(out.covered.len(), 2);
1078 let (out, ..) = filed_with("R3-1-1 R3-2-1 fixed in #47");
1079 assert!(out.covered.is_empty());
1080 assert_eq!(out.filed.len(), 2);
1081 let (out, ..) = filed_with("R3-1-1 R3-2-1 fixed in #4731");
1082 assert!(out.covered.is_empty());
1083 }
1084
1085 #[test]
1086 fn an_id_without_the_pr_does_not_cover() {
1087 let (out, ..) = filed_with("R3-1-1 and R3-2-1 are bad");
1088 assert!(out.covered.is_empty());
1089 assert_eq!(out.filed.len(), 2);
1090 }
1091
1092 #[test]
1093 fn an_id_prefix_does_not_cover() {
1094 let (out, ..) = filed_with(&format!("R3-1-10 R3-2-10 in {PR}"));
1095 assert!(out.covered.is_empty());
1096 }
1097
1098 #[test]
1099 fn partial_coverage_still_files() {
1100 let (out, ..) = filed_with(&format!("R3-1-1 in {PR}"));
1101 assert!(out.covered.is_empty());
1102 assert_eq!(out.filed.len(), 2);
1103 }
1104
1105 #[test]
1106 fn another_followups_findings_cover() {
1107 let dir = tempfile::tempdir().unwrap();
1108 let q = Queue::at(dir.path().join("queue"));
1109 let mut s = merged(two_seats());
1110 let mut other = manual("x");
1111 other.followup = Some(FollowUp {
1112 run: s.id.clone(),
1113 origin_task: None,
1114 pr: PR.to_owned(),
1115 findings: vec!["R3-1-1".to_owned(), "R3-2-1".to_owned()],
1116 generation: 1,
1117 });
1118 q.put(&mut other).unwrap();
1119 let out = file(&mut s, PR, &q).unwrap();
1120 assert_eq!(out.covered.len(), 2, "{out:?}");
1121 assert_eq!(out.filed.len(), 1);
1122 }
1123
1124 #[test]
1125 fn github_text_fixed_followup_comment_passes() {
1126 let state = merged(two_seats());
1127 let body = comment_body(&state, &Outcome::default());
1128 assert!(crate::github_text::check("", &body).is_empty());
1129 }
1130
1131 #[test]
1132 fn a_fully_covered_comment_lists_only_the_covered() {
1133 let (out, mut s, ..) = filed_with(&format!("R3-1-1 R3-2-1 {PR}"));
1134 s.followups.clear();
1135 let body = comment_body(&s, &out);
1136 assert!(body.contains("already covered by task"));
1137 assert!(!body.contains("filed as follow-up"));
1138 }
1139
1140 #[test]
1141 fn an_id_next_to_japanese_prose_still_covers() {
1142 let (out, ..) = filed_with("PR #473 の R3-1-1を修正、R3-2-1も対応");
1143 assert_eq!(out.covered.len(), 2, "{out:?}");
1144 }
1145
1146 fn deputy_task(run: &str, title: &str, text: &str, reason: &str) -> Task {
1149 let mut t = Task::new(
1150 title.to_owned(),
1151 text.to_owned(),
1152 PathBuf::from("/repo"),
1153 Source::Agent {
1154 run: run.to_owned(),
1155 node: crate::deputy::NODE.to_owned(),
1156 },
1157 );
1158 t.hold_manual(Some(reason.to_owned()));
1159 t
1160 }
1161
1162 fn status_of(q: &Queue, id: &str) -> crate::queue::TaskStatus {
1163 q.get(id).unwrap().status
1164 }
1165
1166 #[test]
1167 fn a_deputy_task_is_released_once_and_stamped() {
1168 let dir = tempfile::tempdir().unwrap();
1169 let q = Queue::at(dir.path().join("queue"));
1170 let mut s = merged(two_seats());
1171 s.followup_generation = Some(0);
1172 let mut t = deputy_task(
1173 &s.id,
1174 "Unrelated work",
1175 "do the thing",
1176 &format!("after {PR}"),
1177 );
1178 q.put(&mut t).unwrap();
1179 settle_deputy_tasks(&mut s, PR, &q, true);
1180 let got = q.get(&t.id).unwrap();
1181 assert_eq!(got.status, crate::queue::TaskStatus::Queued);
1182 assert!(got.hold_reason.is_none());
1183 let f = got.followup.unwrap();
1184 assert_eq!((f.run.as_str(), f.generation), (s.id.as_str(), 1));
1185 assert_eq!(s.deputy_followups, vec![t.id.clone()]);
1186 let events = s.events.len();
1187 settle_deputy_tasks(&mut s, PR, &q, true);
1189 assert_eq!(s.events.len(), events);
1190 s.deputy_followups.clear();
1191 settle_deputy_tasks(&mut s, PR, &q, true);
1192 assert_eq!(
1193 s.deputy_followups,
1194 vec![t.id.clone()],
1195 "rebuilt from the queue"
1196 );
1197 assert_eq!(status_of(&q, &t.id), crate::queue::TaskStatus::Queued);
1198 }
1199
1200 #[test]
1201 fn unrelated_held_tasks_are_never_released() {
1202 let dir = tempfile::tempdir().unwrap();
1203 let q = Queue::at(dir.path().join("queue"));
1204 let mut s = merged(two_seats());
1205 s.followup_generation = Some(0);
1206 let mut other_run = deputy_task("someone-else", "a", "b", &format!("after {PR}"));
1207 let mut other_pr = deputy_task(&s.id, "c", "d", "after https://github.com/o/r/pull/4731");
1208 let mut no_reason = deputy_task(&s.id, "e", "f", "x");
1209 no_reason.hold_reason = None;
1210 let mut human = manual("g");
1211 human.hold_manual(Some(format!("waiting for {PR}")));
1212 let mut queued = deputy_task(&s.id, "h", "i", &format!("after {PR}"));
1213 queued.release();
1214 let ids: Vec<String> = [
1215 &mut other_run,
1216 &mut other_pr,
1217 &mut no_reason,
1218 &mut human,
1219 &mut queued,
1220 ]
1221 .into_iter()
1222 .map(|t| {
1223 q.put(t).unwrap();
1224 t.id.clone()
1225 })
1226 .collect();
1227 settle_deputy_tasks(&mut s, PR, &q, true);
1228 for id in &ids[..4] {
1229 assert_eq!(status_of(&q, id), crate::queue::TaskStatus::Held, "{id}");
1230 assert!(q.get(id).unwrap().followup.is_none());
1231 }
1232 assert!(s.deputy_followups.is_empty());
1233 }
1234
1235 #[test]
1236 fn the_switch_off_leaves_the_task_held() {
1237 let dir = tempfile::tempdir().unwrap();
1238 let q = Queue::at(dir.path().join("queue"));
1239 let mut s = merged(two_seats());
1240 s.followup_generation = Some(0);
1241 let mut t = deputy_task(&s.id, "x", "y", &format!("after {PR}"));
1242 q.put(&mut t).unwrap();
1243 settle_deputy_tasks(&mut s, PR, &q, false);
1244 assert_eq!(status_of(&q, &t.id), crate::queue::TaskStatus::Held);
1245 }
1246
1247 #[test]
1248 fn the_generation_cap_applies_to_released_tasks() {
1249 let dir = tempfile::tempdir().unwrap();
1250 let q = Queue::at(dir.path().join("queue"));
1251 let mut s = merged(two_seats());
1252 s.followup_generation = Some(MAX_FOLLOWUP_GENERATION);
1253 let mut t = deputy_task(&s.id, "x", "y", &format!("after {PR}"));
1254 q.put(&mut t).unwrap();
1255 settle_deputy_tasks(&mut s, PR, &q, true);
1256 let got = q.get(&t.id).unwrap();
1257 assert_eq!(got.status, crate::queue::TaskStatus::Held);
1258 assert!(got.hold_reason.unwrap().starts_with(CAPPED));
1259 let events = s.events.len();
1260 settle_deputy_tasks(&mut s, PR, &q, true);
1261 assert_eq!(s.events.len(), events, "marked once");
1262 let mut s2 = merged(two_seats());
1263 s2.followup_generation = Some(MAX_FOLLOWUP_GENERATION - 1);
1264 let mut t2 = deputy_task(&s2.id, "x", "y", &format!("after {PR}"));
1265 q.put(&mut t2).unwrap();
1266 settle_deputy_tasks(&mut s2, PR, &q, true);
1267 assert_eq!(
1268 q.get(&t2.id).unwrap().followup.unwrap().generation,
1269 MAX_FOLLOWUP_GENERATION
1270 );
1271 }
1272
1273 #[test]
1274 fn deputy_first_the_owners_task_runs_and_the_automatic_one_is_not_filed() {
1275 let dir = tempfile::tempdir().unwrap();
1276 let q = Queue::at(dir.path().join("queue"));
1277 let mut s = merged(two_seats());
1278 s.followup_generation = Some(0);
1279 let mut t = deputy_task(
1280 &s.id,
1281 "Fix the waiter",
1282 "R3-1-1 at src/waiter.rs:212 loses replies",
1283 &format!("after {PR}"),
1284 );
1285 q.put(&mut t).unwrap();
1286 settle_deputy_tasks(&mut s, PR, &q, true);
1287 let out = file(&mut s, PR, &q).unwrap();
1288 assert_eq!(out.covered.len(), 2, "{out:?}");
1289 assert_eq!(out.filed.len(), 1, "{out:?}");
1291 let runnable: Vec<_> = q
1292 .list()
1293 .into_iter()
1294 .filter(|t| {
1295 t.status == crate::queue::TaskStatus::Queued
1296 && t.followup
1297 .as_ref()
1298 .is_some_and(|f| f.findings.iter().any(|x| x == "R3-1-1"))
1299 })
1300 .collect();
1301 assert_eq!(runnable.len(), 1);
1302 assert_eq!(runnable[0].id, t.id);
1303 }
1304
1305 #[test]
1306 fn followup_first_a_queued_automatic_task_gives_way_to_the_owners() {
1307 let dir = tempfile::tempdir().unwrap();
1308 let q = Queue::at(dir.path().join("queue"));
1309 let mut s = merged(two_seats());
1310 s.followup_generation = Some(0);
1311 file(&mut s, PR, &q).unwrap();
1312 let mut t = deputy_task(
1313 &s.id,
1314 "Waiter replies",
1315 "see R3-2-1",
1316 &format!("after {PR}"),
1317 );
1318 q.put(&mut t).unwrap();
1319 settle_deputy_tasks(&mut s, PR, &q, true);
1320 let all = q.list();
1321 let live: Vec<_> = all
1322 .iter()
1323 .filter(|t| {
1324 t.status == crate::queue::TaskStatus::Queued
1325 && t.followup
1326 .as_ref()
1327 .is_some_and(|f| f.findings.iter().any(|x| x == "R3-2-1"))
1328 })
1329 .collect();
1330 assert_eq!(live.len(), 1);
1331 assert_eq!(live[0].id, t.id);
1332 let dropped = all
1333 .iter()
1334 .find(|a| {
1335 a.id != t.id
1336 && a.followup
1337 .as_ref()
1338 .is_some_and(|f| f.findings.iter().any(|x| x == "R3-2-1"))
1339 })
1340 .unwrap();
1341 assert_eq!(dropped.status, crate::queue::TaskStatus::Held);
1342 assert!(
1343 dropped
1344 .hold_reason
1345 .as_ref()
1346 .unwrap()
1347 .starts_with(SUPERSEDED)
1348 );
1349 }
1350
1351 #[test]
1352 fn followup_first_a_finished_automatic_task_supersedes_the_deputys() {
1353 let dir = tempfile::tempdir().unwrap();
1354 let q = Queue::at(dir.path().join("queue"));
1355 let mut s = merged(two_seats());
1356 s.followup_generation = Some(0);
1357 file(&mut s, PR, &q).unwrap();
1358 for mut a in q.list() {
1359 a.status = crate::queue::TaskStatus::Done;
1360 q.put(&mut a).unwrap();
1361 }
1362 let mut t = deputy_task(&s.id, "title R3-1-1", "no ids here", &format!("after {PR}"));
1364 t.instruction = "nothing".to_owned();
1365 q.put(&mut t).unwrap();
1366 settle_deputy_tasks(&mut s, PR, &q, true);
1367 let got = q.get(&t.id).unwrap();
1368 assert_eq!(got.status, crate::queue::TaskStatus::Held);
1369 assert!(got.followup.is_none());
1370 let reason = got.hold_reason.unwrap();
1371 assert!(reason.starts_with(SUPERSEDED), "{reason}");
1372 assert!(reason.contains(PR));
1373 let events = s.events.len();
1374 settle_deputy_tasks(&mut s, PR, &q, true);
1375 assert_eq!(s.events.len(), events, "judged once");
1376 }
1377
1378 #[test]
1379 fn a_held_deputy_task_is_superseded_by_a_later_filing_when_the_switch_is_off() {
1380 let dir = tempfile::tempdir().unwrap();
1381 let q = Queue::at(dir.path().join("queue"));
1382 let mut s = merged(two_seats());
1383 s.followup_generation = Some(0);
1384 let mut t = deputy_task(&s.id, "x", "R3-1-1 here", &format!("after {PR}"));
1385 q.put(&mut t).unwrap();
1386 settle_deputy_tasks(&mut s, PR, &q, false);
1387 assert!(
1388 !q.get(&t.id)
1389 .unwrap()
1390 .hold_reason
1391 .unwrap()
1392 .starts_with(SUPERSEDED)
1393 );
1394 file(&mut s, PR, &q).unwrap();
1395 settle_deputy_tasks(&mut s, PR, &q, false);
1396 let got = q.get(&t.id).unwrap();
1397 assert_eq!(got.status, crate::queue::TaskStatus::Held);
1398 assert!(got.hold_reason.unwrap().starts_with(SUPERSEDED));
1399 }
1400
1401 #[test]
1402 fn the_comment_lists_released_deputy_tasks() {
1403 let mut s = merged(two_seats());
1404 s.deputy_followups.push("abcd-1".to_owned());
1405 let body = comment_body(&s, &Outcome::default());
1406 assert!(body.contains("`abcd-1`"));
1407 assert!(!body.contains("already covered"));
1408 assert!(crate::github_text::check("", &body).is_empty());
1409 }
1410
1411 #[test]
1412 fn a_deputy_task_over_two_automatic_ones_holds_both() {
1413 let dir = tempfile::tempdir().unwrap();
1414 let q = Queue::at(dir.path().join("queue"));
1415 let mut s = merged(two_seats());
1416 s.followup_generation = Some(0);
1417 file(&mut s, PR, &q).unwrap();
1418 let mut t = deputy_task(&s.id, "both", "R3-1-1 and R3-1-2", &format!("after {PR}"));
1419 q.put(&mut t).unwrap();
1420 settle_deputy_tasks(&mut s, PR, &q, true);
1421 assert_eq!(status_of(&q, &t.id), crate::queue::TaskStatus::Queued);
1422 let live = q
1423 .list()
1424 .into_iter()
1425 .filter(|x| x.status == crate::queue::TaskStatus::Queued)
1426 .count();
1427 assert_eq!(live, 1, "only the owner's task still runs");
1428 }
1429
1430 #[test]
1431 fn a_claimed_automatic_task_is_not_touched_and_the_deputys_stays_held() {
1432 let dir = tempfile::tempdir().unwrap();
1433 let q = Queue::at(dir.path().join("queue"));
1434 let mut s = merged(two_seats());
1435 s.followup_generation = Some(0);
1436 file(&mut s, PR, &q).unwrap();
1437 let auto = q
1438 .list()
1439 .into_iter()
1440 .find(|x| {
1441 x.followup
1442 .as_ref()
1443 .is_some_and(|f| f.findings.iter().any(|i| i == "R3-1-1"))
1444 })
1445 .unwrap();
1446 let _claim = q.claim(&auto.id).unwrap();
1447 let mut t = deputy_task(&s.id, "x", "R3-1-1", &format!("after {PR}"));
1448 q.put(&mut t).unwrap();
1449 settle_deputy_tasks(&mut s, PR, &q, true);
1450 assert_eq!(status_of(&q, &auto.id), crate::queue::TaskStatus::Queued);
1451 let got = q.get(&t.id).unwrap();
1452 assert_eq!(got.status, crate::queue::TaskStatus::Held);
1453 assert!(got.hold_reason.unwrap().starts_with(SUPERSEDED));
1454 }
1455}