Skip to main content

magi/
followup.rs

1//! Follow-up tasks for the findings a merged run left open.
2//!
3//! A run can hand off after its review budget is spent with findings still
4//! open (and, with `land_approval` off, merge unattended). Those findings
5//! live in the pull request body, which nobody reads after the merge. This
6//! module files them as queue tasks instead, once the merge is confirmed.
7//!
8//! What is filed: every Major-or-above finding of the last review round, and
9//! every finding of a seat whose final vote was reject (a vote carries no
10//! per-finding reasoning, so the seat's Minor/Nit findings come along). Other
11//! Minor/Nit findings are only listed, in the pull-request comment.
12//!
13//! Best-effort by construction, like `bump::after_merge`: [`after_merge`]
14//! returns `()`, records every failure as a run event and never touches
15//! `status`.
16
17use anyhow::Result;
18
19use crate::queue::{FollowUp, Queue, Source, Task};
20use crate::run::{FollowupRecord, RunState};
21use crate::verdict::{Finding, ReviewVote};
22
23/// Node name carried by [`Source::Agent`] on a filed follow-up.
24pub const NODE: &str = "followup";
25
26/// A task of this generation (or deeper) does not file follow-ups of its own.
27pub const MAX_FOLLOWUP_GENERATION: u32 = 2;
28
29/// Two findings in one file this many lines apart are one defect.
30const LINE_WINDOW: u32 = 5;
31
32/// What one pass did.
33#[derive(Debug, Default, PartialEq, Eq)]
34pub struct Outcome {
35    /// Task ids created by this pass.
36    pub filed: Vec<String>,
37    /// Findings selected but not filed (cap reached), as ids.
38    pub capped: Vec<String>,
39    /// Minor/Nit findings of the last round that were only listed.
40    pub unfiled: Vec<String>,
41    /// Findings not filed because another task already covers them, as
42    /// `(finding id, covering task id)`. Recomputed from the queue each pass.
43    pub covered: Vec<(String, String)>,
44}
45
46/// File follow-ups for a run that just merged, and comment on the pull
47/// request. Never fails: problems become run events.
48pub 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    // The deputy's tasks are settled first and again after filing, so the
52    // outcome is the same whichever side got to the queue first.
53    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
83/// Reason prefixes that mark a deputy task this module already judged and
84/// left held. Both stop a second pass from rewriting the reason again.
85const SUPERSEDED: &str = "[superseded] ";
86const CAPPED: &str = "[generation cap] ";
87
88/// A task the merge approval's deputy filed for run `run` that is still held
89/// for pull request `pr_url`: the only kind [`settle_deputy_tasks`] touches.
90/// Concrete identifiers only - the filer is the deputy seat of this run and
91/// the hold reason names the pull request - never text similarity alone.
92fn 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
102/// Does deputy task `t` talk about finding `f`: its id as a word, a
103/// `file:line` within [`LINE_WINDOW`] of the finding, or the finding's title
104/// (normalized; contained only when long enough not to match by accident).
105fn 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
129/// The follow-up depth of the task the run served; see [`file`].
130fn 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
145/// Reconcile the follow-up tasks the merge approval's deputy filed (held,
146/// reason naming this pull request) with the automatic ones, once the merge
147/// is confirmed. Idempotent, and the same whichever side reached the queue
148/// first:
149///
150/// - a deputy task that covers a finding an automatic follow-up already
151///   covers (same finding, same file within [`LINE_WINDOW`] lines, or the same
152///   normalized title) never runs twice: if the automatic one is queued the
153///   owner's task wins and the automatic one is held with the reason, else
154///   the deputy task stays held with a `[superseded]` reason. Nothing is
155///   deleted;
156/// - otherwise, with `release`, the task is released in the same queue write
157///   that stamps it with a [`FollowUp`] (so it is released once, counts
158///   toward the generation cap, and covers its findings for [`file`]);
159/// - a task at the generation cap stays held with a `[generation cap]`
160///   reason.
161///
162/// Only [`is_deputy_candidate`] tasks are touched, re-checked under the
163/// queue's lock. Best-effort: failures are run events.
164pub fn settle_deputy_tasks(state: &mut RunState, pr_url: &str, queue: &Queue, release: bool) {
165    let tasks = queue.list();
166    // The queue is the record: restore what a crash after the write lost.
167    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        // Findings the task covers, widened to whole defect groups.
199        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            // Hold every idle rival under its claim, so the daemon cannot be
226            // between reading it and starting it. Any rival that is not
227            // idle, or cannot be claimed, means the automatic path owns the
228            // finding and the deputy task is the one left held.
229            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            // The owner's task wins; the idle automatic ones are held. Only
268            // when every one is confirmed held is the deputy task released.
269            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
348/// Rewrite a still-candidate deputy task's hold reason, leaving it held.
349fn 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
378/// Findings the last review round leaves to be followed up, and the ids of
379/// those only listed: every Major-or-above finding, plus all findings of a
380/// seat whose final vote was reject.
381fn 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
402/// The deterministic half of [`after_merge`]: select, group, and file into
403/// `queue`. Records the event and the run's own bookkeeping; the caller
404/// saves the state.
405pub 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    // The depth recorded when the run started wins; the queue is only a
418    // fallback for runs that predate it, and an unknown depth is read as
419    // unbounded-until-proven-otherwise only when there is no origin task.
420    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
517/// The id of a task, other than `own`, that already covers finding `id` of
518/// run `run` merged as `pr_url`, whatever its status. Concrete identifiers
519/// only: a follow-up of the same run listing the finding, or an instruction
520/// naming both the finding id and the pull request (URL or `#<number>`).
521fn 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
535/// ASCII only: an id is often followed directly by prose in another script
536/// (`R3-1-1を修正`), which must not read as part of the id.
537fn is_name_char(c: char) -> bool {
538    c.is_ascii_alphanumeric() || matches!(c, '_' | '-')
539}
540
541/// `needle` in `text` with no name character on either side.
542fn 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
551/// Does `text` name the pull request by its URL or its `#<number>` form,
552/// with no further digit (or name character before `#`) so `#47` is not `#473`?
553fn 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
577/// Group findings that are the same defect: same file with lines within
578/// [`LINE_WINDOW`] of each other, or, with no file, the same normalized
579/// title. Deterministic, so a re-run groups identically.
580fn 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
622/// Deterministic id: the run id plus a hash of the group's finding ids, so
623/// two passes name the same task and [`Queue::create_new`] can refuse the
624/// second. Ends in a `-` segment like every id, so `queue::short` works.
625fn 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        // waiter.rs pair is one defect; seat 1 rejected so its Minor is filed
838        // too; seat 2's Nit is only listed.
839        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        // Even with the run's own record lost, the queue says what exists.
888        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    /// File follow-ups with one manual task already in the queue.
1041    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        // the waiter.rs group is covered; the other seat-1 Minor is still filed
1056        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    // ---- deputy tasks released after the merge ----
1147
1148    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        // A second pass, or a restart that lost the run's record, changes nothing.
1188        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        // Only the seat-1 Minor group is filed; the waiter.rs defect has one task.
1290        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        // Matched by the finding's title, not its id.
1363        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}