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            let takeover = format!(
232                "{SUPERSEDED}the owner's approval deputy filed task {} for the same finding(s) of {pr_url}",
233                cand.id
234            );
235            // Held by an earlier pass of this same takeover that could not
236            // finish (or be rolled back): it is ours to restore on failure.
237            let mut carried: Vec<String> = Vec::new();
238            if release {
239                for a in &rivals {
240                    let ours = a.status == crate::queue::TaskStatus::Held
241                        && a.hold_reason.as_deref() == Some(takeover.as_str());
242                    if a.status == crate::queue::TaskStatus::Held && !ours {
243                        continue;
244                    }
245                    match queue.claim(&a.id) {
246                        Ok(g) => {
247                            if ours {
248                                carried.push(a.id.clone());
249                            }
250                            guards.push((a.id.clone(), g));
251                        }
252                        Err(_) => {
253                            blocker = Some(a);
254                            break;
255                        }
256                    }
257                }
258                if blocker.is_none() {
259                    blocker = rivals.iter().map(|a| &***a).find(|a| {
260                        !matches!(
261                            queue.get(&a.id).map(|t| t.status),
262                            Ok(crate::queue::TaskStatus::Queued | crate::queue::TaskStatus::Held)
263                        )
264                    });
265                }
266            } else {
267                blocker = Some(&***rivals.first().expect("non-empty"));
268            }
269            if let Some(a) = blocker {
270                // A hold left by an interrupted takeover is not covered by
271                // the blocker: give it back before superseding the deputy.
272                let mut stuck = false;
273                for r in &rivals {
274                    let held_by_us = queue.get(&r.id).is_ok_and(|t| {
275                        t.status == crate::queue::TaskStatus::Held
276                            && t.hold_reason.as_deref() == Some(takeover.as_str())
277                    });
278                    if !held_by_us {
279                        continue;
280                    }
281                    let _own;
282                    if !guards.iter().any(|(g, _)| g == &r.id) {
283                        match queue.claim(&r.id) {
284                            Ok(g) => _own = g,
285                            Err(_) => {
286                                stuck = true;
287                                continue;
288                            }
289                        }
290                    }
291                    stuck |= !restore_takeover_hold(state, queue, &r.id, &takeover, &cand.id);
292                }
293                drop(guards);
294                if stuck {
295                    continue;
296                }
297                let why = format!(
298                    "{SUPERSEDED}follow-up task {} ({:?}) already covers {} of {pr_url}; was: {}",
299                    a.id,
300                    a.status,
301                    matched.join(", "),
302                    cand.hold_reason.as_deref().unwrap_or_default()
303                );
304                mark_held(state, queue, cand, why, pr_url, "superseded");
305                continue;
306            }
307            // The owner's task wins; the idle automatic ones are held. Only
308            // when every one is confirmed held is the deputy task released.
309            let mut failed: Option<String> = None;
310            // What each held task looked like before, for a rollback.
311            let mut done: Vec<(String, Task, String)> = Vec::new();
312            for (id, _guard) in &guards {
313                if carried.contains(id) {
314                    continue;
315                }
316                let why = takeover.clone();
317                let mut before: Option<Task> = None;
318                let held = queue.modify(id, |t| {
319                    if t.status != crate::queue::TaskStatus::Queued {
320                        return false;
321                    }
322                    before = Some(t.clone());
323                    t.hold_manual(Some(why.clone()));
324                    true
325                });
326                match held {
327                    Ok(true) => {
328                        done.extend(before.map(|b| (id.clone(), b, why)));
329                        state.event(
330                            NODE,
331                            format!(
332                                "follow-up task {id} held: superseded by deputy task {}",
333                                cand.id
334                            ),
335                        );
336                    }
337                    Ok(false) => {
338                        failed = Some(id.clone());
339                        break;
340                    }
341                    Err(e) => {
342                        state.event(NODE, format!("could not hold follow-up {id}: {e:#}"));
343                        failed = Some(id.clone());
344                        break;
345                    }
346                }
347            }
348            if let Some(id) = failed {
349                // Give back what was already held, still under its claim, so
350                // the automatic path keeps covering every finding. Only a
351                // task still carrying our hold is touched.
352                let mut stuck = false;
353                for tid in &carried {
354                    stuck |= !restore_takeover_hold(state, queue, tid, &takeover, &cand.id);
355                }
356                for (tid, before, why) in done.into_iter().rev() {
357                    let back = queue.modify(&tid, |t| {
358                        if t.status != crate::queue::TaskStatus::Held
359                            || t.hold_reason.as_deref() != Some(why.as_str())
360                        {
361                            return false;
362                        }
363                        *t = before;
364                        true
365                    });
366                    match back {
367                        Ok(true) => state.event(
368                            NODE,
369                            format!(
370                                "follow-up task {tid} restored: deputy task {} could not take it over",
371                                cand.id
372                            ),
373                        ),
374                        Ok(false) => {}
375                        Err(e) => {
376                            stuck = true;
377                            state.event(
378                                NODE,
379                                format!(
380                                    "rollback of {tid} failed, deputy task {} left as a candidate for the next settle: {e:#}",
381                                    cand.id
382                                ),
383                            );
384                        }
385                    }
386                }
387                drop(guards);
388                if stuck {
389                    continue;
390                }
391                let why = format!(
392                    "{SUPERSEDED}follow-up task {id} could not be held, so it may still run for {} of {pr_url}; was: {}",
393                    matched.join(", "),
394                    cand.hold_reason.as_deref().unwrap_or_default()
395                );
396                mark_held(state, queue, cand, why, pr_url, "superseded");
397                continue;
398            }
399        } else if !release {
400            continue;
401        }
402        let parent_gen = gen_of_parent.expect("computed when releasing");
403        let stamp = FollowUp {
404            run: state.id.clone(),
405            origin_task: origin_task.clone(),
406            pr: pr_url.to_owned(),
407            findings: matched,
408            generation: parent_gen + 1,
409        };
410        let id = cand.id.clone();
411        let run = state.id.clone();
412        let result = queue.modify(&id, |t| {
413            if !is_deputy_candidate(t, &run, pr_url) {
414                return false;
415            }
416            t.release();
417            t.followup = Some(stamp);
418            true
419        });
420        match result {
421            Ok(true) => {
422                if !state.deputy_followups.contains(&id) {
423                    state.deputy_followups.push(id.clone());
424                }
425                state.event(
426                    NODE,
427                    format!("released deputy follow-up task {id} after {pr_url} merged"),
428                );
429            }
430            Ok(false) => {}
431            Err(e) => state.event(NODE, format!("could not release deputy task {id}: {e:#}")),
432        }
433    }
434}
435
436/// Give a follow-up task held by an interrupted takeover back to the
437/// automatic path. `false` when the write failed (the caller keeps the deputy
438/// task a candidate); a task no longer carrying `takeover` is left alone.
439fn restore_takeover_hold(
440    state: &mut RunState,
441    queue: &Queue,
442    tid: &str,
443    takeover: &str,
444    cand_id: &str,
445) -> bool {
446    let back = queue.modify(tid, |t| {
447        if t.status != crate::queue::TaskStatus::Held || t.hold_reason.as_deref() != Some(takeover)
448        {
449            return false;
450        }
451        // No snapshot survives a restart; an automatic follow-up is filed
452        // Queued with nothing else set.
453        t.status = crate::queue::TaskStatus::Queued;
454        t.held_at = None;
455        t.hold_reason = None;
456        t.hold_source = None;
457        true
458    });
459    match back {
460        Ok(true) => {
461            state.event(
462                NODE,
463                format!(
464                    "follow-up task {tid} restored: deputy task {cand_id} could not take it over"
465                ),
466            );
467            true
468        }
469        Ok(false) => true,
470        Err(e) => {
471            state.event(
472                NODE,
473                format!(
474                    "rollback of {tid} failed, deputy task {cand_id} left as a candidate for the next settle: {e:#}"
475                ),
476            );
477            false
478        }
479    }
480}
481
482/// Rewrite a still-candidate deputy task's hold reason, leaving it held.
483fn mark_held(
484    state: &mut RunState,
485    queue: &Queue,
486    cand: &Task,
487    why: String,
488    pr_url: &str,
489    kind: &str,
490) {
491    let run = state.id.clone();
492    let reason = why.clone();
493    match queue.modify(&cand.id, |t| {
494        if !is_deputy_candidate(t, &run, pr_url) {
495            return false;
496        }
497        t.hold_reason = Some(reason);
498        true
499    }) {
500        Ok(true) => state.event(
501            NODE,
502            format!("deputy task {} left held ({kind}): {why}", cand.id),
503        ),
504        Ok(false) => {}
505        Err(e) => state.event(
506            NODE,
507            format!("could not mark deputy task {}: {e:#}", cand.id),
508        ),
509    }
510}
511
512/// Findings the last review round leaves to be followed up, and the ids of
513/// those only listed: every Major-or-above finding, plus all findings of a
514/// seat whose final vote was reject.
515fn select(round: &crate::run::ReviewRound) -> (Vec<Finding>, Vec<String>) {
516    let rejecters: Vec<usize> = round
517        .final_votes()
518        .into_iter()
519        .filter(|(_, _, v)| *v == ReviewVote::Reject)
520        .map(|(seat, _, _)| seat)
521        .collect();
522    let (mut chosen, mut unfiled) = (Vec::new(), Vec::new());
523    for rec in &round.reviews {
524        let rejected = rejecters.contains(&rec.reviewer);
525        for f in &rec.findings {
526            if f.severity.blocks() || rejected {
527                chosen.push(f.clone());
528            } else {
529                unfiled.push(f.id.clone());
530            }
531        }
532    }
533    (chosen, unfiled)
534}
535
536/// The deterministic half of [`after_merge`]: select, group, and file into
537/// `queue`. Records the event and the run's own bookkeeping; the caller
538/// saves the state.
539pub fn file(state: &mut RunState, pr_url: &str, queue: &Queue) -> Result<Outcome> {
540    let mut out = Outcome::default();
541    let Some(round) = state.reviews.last().cloned() else {
542        return Ok(out);
543    };
544    let (chosen, unfiled) = select(&round);
545    out.unfiled = unfiled;
546    if chosen.is_empty() {
547        return Ok(out);
548    }
549
550    let origin_task = state.origin.as_ref().and_then(|o| o.task.clone());
551    // The depth recorded when the run started wins; the queue is only a
552    // fallback for runs that predate it, and an unknown depth is read as
553    // unbounded-until-proven-otherwise only when there is no origin task.
554    let parent_gen = parent_generation(state, queue, origin_task.as_deref());
555    if parent_gen >= MAX_FOLLOWUP_GENERATION {
556        out.capped = chosen.iter().map(|f| f.id.clone()).collect();
557        state.event(
558            NODE,
559            format!(
560                "generation cap reached ({parent_gen}); not filing follow-ups for open finding(s): {}",
561                out.capped.join(", ")
562            ),
563        );
564        return Ok(out);
565    }
566
567    let mut done: Vec<String> = state
568        .followups
569        .iter()
570        .flat_map(|r| r.findings.iter().cloned())
571        .collect();
572    for t in queue.list() {
573        if let Some(f) = t.followup
574            && f.run == state.id
575        {
576            done.extend(f.findings.iter().cloned());
577            let by_deputy =
578                matches!(&t.source, Source::Agent { node, .. } if node == crate::deputy::NODE);
579            if !by_deputy && !state.followups.iter().any(|r| r.task == t.id) {
580                state.followups.push(FollowupRecord {
581                    task: t.id,
582                    findings: f.findings,
583                });
584            }
585        }
586    }
587
588    let tasks = queue.list();
589    let origin_chat = state.origin_chat.clone().or_else(|| {
590        let tasks = queue.list();
591        let parent = origin_task
592            .as_deref()
593            .and_then(|id| tasks.iter().find(|t| t.id == id))?;
594        crate::consult::chat_talk_of(&tasks, parent)
595    });
596    let mut failure = None;
597    for group in group_findings(chosen) {
598        let ids: Vec<String> = group.iter().map(|f| f.id.clone()).collect();
599        let own = task_id(&state.id, &ids);
600        let covers: Vec<Option<String>> = ids
601            .iter()
602            .map(|id| covering_task(&tasks, &own, &state.id, pr_url, id))
603            .collect();
604        if covers.iter().all(Option::is_some) {
605            for (id, by) in ids.iter().zip(covers.into_iter().flatten()) {
606                state.event(
607                    NODE,
608                    format!("not filing follow-up for {id}: already covered by task {by}"),
609                );
610                out.covered.push((id.clone(), by));
611            }
612            continue;
613        }
614        if ids.iter().any(|id| done.contains(id)) {
615            continue;
616        }
617        let mut task = build_task(state, pr_url, origin_task.clone(), &group, parent_gen + 1);
618        task.origin_chat = origin_chat.clone();
619        match queue.create_new(&mut task) {
620            Ok(created) => {
621                if created {
622                    out.filed.push(task.id.clone());
623                }
624                state.followups.push(FollowupRecord {
625                    task: task.id,
626                    findings: ids,
627                });
628            }
629            Err(e) => {
630                failure = Some(e);
631                break;
632            }
633        }
634    }
635    if !out.filed.is_empty() {
636        state.event(
637            NODE,
638            format!(
639                "filed {} follow-up task(s): ids {}",
640                out.filed.len(),
641                out.filed.join(", ")
642            ),
643        );
644    }
645    if let Some(e) = failure {
646        state.event(NODE, format!("follow-up filing stopped: {e:#}"));
647    }
648    Ok(out)
649}
650
651/// The id of a task, other than `own`, that already covers finding `id` of
652/// run `run` merged as `pr_url`, whatever its status. Concrete identifiers
653/// only: a follow-up of the same run listing the finding, or an instruction
654/// naming both the finding id and the pull request (URL or `#<number>`).
655fn covering_task(tasks: &[Task], own: &str, run: &str, pr_url: &str, id: &str) -> Option<String> {
656    tasks
657        .iter()
658        .find(|t| {
659            t.id != own
660                && (t
661                    .followup
662                    .as_ref()
663                    .is_some_and(|f| f.run == run && f.findings.iter().any(|x| x == id))
664                    || (names_word(&t.instruction, id) && names_pr(&t.instruction, pr_url)))
665        })
666        .map(|t| t.id.clone())
667}
668
669/// ASCII only: an id is often followed directly by prose in another script
670/// (`R3-1-1を修正`), which must not read as part of the id.
671fn is_name_char(c: char) -> bool {
672    c.is_ascii_alphanumeric() || matches!(c, '_' | '-')
673}
674
675/// `needle` in `text` with no name character on either side.
676fn names_word(text: &str, needle: &str) -> bool {
677    !needle.is_empty()
678        && text.match_indices(needle).any(|(i, _)| {
679            let before = text[..i].chars().next_back();
680            let after = text[i + needle.len()..].chars().next();
681            before.is_none_or(|c| !is_name_char(c)) && after.is_none_or(|c| !is_name_char(c))
682        })
683}
684
685/// Does `text` name the pull request by its URL or its `#<number>` form,
686/// with no further digit (or name character before `#`) so `#47` is not `#473`?
687fn names_pr(text: &str, pr_url: &str) -> bool {
688    let url = pr_url.trim_end_matches('/');
689    let digit_after = |rest: &str| rest.chars().next().is_some_and(|c| c.is_ascii_digit());
690    if !url.is_empty()
691        && text
692            .match_indices(url)
693            .any(|(i, _)| !digit_after(&text[i + url.len()..]))
694    {
695        return true;
696    }
697    let Some(n) = url
698        .rsplit('/')
699        .next()
700        .filter(|n| !n.is_empty() && n.bytes().all(|b| b.is_ascii_digit()))
701    else {
702        return false;
703    };
704    let tag = format!("#{n}");
705    text.match_indices(&tag).any(|(i, _)| {
706        let before = text[..i].chars().next_back();
707        before.is_none_or(|c| !is_name_char(c)) && !digit_after(&text[i + tag.len()..])
708    })
709}
710
711/// Group findings that are the same defect: same file with lines within
712/// [`LINE_WINDOW`] of each other, or, with no file, the same normalized
713/// title. Deterministic, so a re-run groups identically.
714fn group_findings(mut findings: Vec<Finding>) -> Vec<Vec<Finding>> {
715    findings.sort_by(|a, b| (&a.file, a.line, &a.id).cmp(&(&b.file, b.line, &b.id)));
716    let mut groups: Vec<Vec<Finding>> = Vec::new();
717    for f in findings {
718        let hit = groups
719            .iter()
720            .position(|g| g.iter().any(|o| same_defect(o, &f)));
721        match hit {
722            Some(i) => groups[i].push(f),
723            None => groups.push(vec![f]),
724        }
725    }
726    for g in &mut groups {
727        g.sort_by(|a, b| a.id.cmp(&b.id));
728    }
729    groups
730}
731
732fn same_defect(a: &Finding, b: &Finding) -> bool {
733    match (&a.file, &b.file) {
734        (Some(x), Some(y)) => {
735            x == y
736                && match (a.line, b.line) {
737                    (Some(l), Some(m)) => l.abs_diff(m) <= LINE_WINDOW,
738                    (None, None) => true,
739                    _ => false,
740                }
741        }
742        (None, None) => normalize(&a.title) == normalize(&b.title),
743        _ => false,
744    }
745}
746
747fn normalize(title: &str) -> String {
748    title
749        .to_lowercase()
750        .split(|c: char| !c.is_alphanumeric())
751        .filter(|w| !w.is_empty())
752        .collect::<Vec<_>>()
753        .join(" ")
754}
755
756/// Deterministic id: the run id plus a hash of the group's finding ids, so
757/// two passes name the same task and [`Queue::create_new`] can refuse the
758/// second. Ends in a `-` segment like every id, so `queue::short` works.
759fn task_id(run: &str, ids: &[String]) -> String {
760    let mut h: u64 = 0xcbf2_9ce4_8422_2325;
761    for b in ids.join("+").bytes() {
762        h ^= u64::from(b);
763        h = h.wrapping_mul(0x0100_0000_01b3);
764    }
765    format!("{run}-f{:05x}", h & 0xf_ffff)
766}
767
768fn location(f: &Finding) -> String {
769    match (&f.file, f.line) {
770        (Some(p), Some(l)) => format!("{p}:{l}"),
771        (Some(p), None) => p.clone(),
772        _ => "(no location)".to_owned(),
773    }
774}
775
776fn build_task(
777    state: &RunState,
778    pr_url: &str,
779    origin_task: Option<String>,
780    group: &[Finding],
781    generation: u32,
782) -> Task {
783    let ids: Vec<String> = group.iter().map(|f| f.id.clone()).collect();
784    let first = &group[0];
785    let title = format!(
786        "follow-up: {} ({})",
787        crate::queue::title_from(&first.title, 80),
788        location(first)
789    );
790    let mut s = format!(
791        "This is a follow-up to a change that was already merged: run {run} merged \
792         {pr_url} while the review finding(s) below were still open. Branch from the \
793         current main - do not use the merged branch - and fix the defect described, \
794         keeping the change small. If a finding no longer applies to current main, say so \
795         and change nothing for it.\n\n\
796         Merged run: {run}\nPull request: {pr_url}\n",
797        run = state.id,
798    );
799    if let Some(t) = &origin_task {
800        s.push_str(&format!("Original task: {t}\n"));
801    }
802    s.push_str("\n## Findings\n");
803    for f in group {
804        s.push_str(&format!(
805            "\n### {} [{:?}] {}\n\nLocation: {}\n\n{}\n",
806            f.id,
807            f.severity,
808            f.title,
809            location(f),
810            f.detail.trim()
811        ));
812    }
813    let mut task = Task::new(
814        title,
815        s,
816        state.repo.clone(),
817        Source::Agent {
818            run: state.id.clone(),
819            node: NODE.to_owned(),
820        },
821    );
822    task.id = task_id(&state.id, &ids);
823    task.solo = true;
824    task.followup = Some(FollowUp {
825        run: state.id.clone(),
826        origin_task,
827        pr: pr_url.to_owned(),
828        findings: ids,
829        generation,
830    });
831    task
832}
833
834fn comment_body(state: &RunState, out: &Outcome) -> String {
835    let mut s = format!(
836        "<!-- magi-followup run={} -->\nThis pull request merged with review findings still open.",
837        state.id
838    );
839    if state.followups.is_empty() {
840        if !out.covered.is_empty() || state.deputy_followups.is_empty() {
841            s.push_str(" Each is already covered by another task:\n");
842        }
843    } else {
844        s.push_str(" They were filed as follow-up tasks:\n\n");
845        for r in &state.followups {
846            s.push_str(&format!("- `{}`: {}\n", r.task, r.findings.join(", ")));
847        }
848        if !out.covered.is_empty() {
849            s.push_str("\nNot filed, already covered:\n");
850        }
851    }
852    if !out.covered.is_empty() {
853        s.push('\n');
854    }
855    for (id, by) in &out.covered {
856        s.push_str(&format!("- {id}: already covered by task `{by}`\n"));
857    }
858    if !state.deputy_followups.is_empty() {
859        s.push_str(
860            "\nFollow-up tasks filed by the approval deputy and released after the merge:\n\n",
861        );
862        for id in &state.deputy_followups {
863            s.push_str(&format!("- `{id}`\n"));
864        }
865    }
866    if !out.unfiled.is_empty() {
867        s.push_str(&format!(
868            "\nMinor/nit findings that were only listed, not filed: {}\n",
869            out.unfiled.join(", ")
870        ));
871    }
872    s
873}
874
875#[cfg(test)]
876mod tests {
877    use super::*;
878    use crate::config::Config;
879    use crate::queue::Queue;
880    use crate::run::ReviewRecord;
881    use crate::run::{Origin, ReviewRound, RunState};
882    use crate::verdict::{Finding, ReviewVote, Severity};
883    use std::path::PathBuf;
884
885    fn finding(id: &str, sev: Severity, file: &str, line: u32) -> Finding {
886        Finding {
887            id: id.to_owned(),
888            severity: sev,
889            file: Some(file.to_owned()),
890            line: Some(line),
891            title: format!("title {id}"),
892            detail: format!("detail of {id}"),
893        }
894    }
895
896    fn record(seat: usize, findings: Vec<Finding>, vote: ReviewVote) -> ReviewRecord {
897        ReviewRecord {
898            attempts: 0,
899            reviewer: seat,
900            agent: "alpha".to_owned(),
901            summary: String::new(),
902            findings,
903            vote: Some(vote),
904            failed: None,
905            duration_ms: 0,
906        }
907    }
908
909    fn round(reviews: Vec<ReviewRecord>) -> ReviewRound {
910        ReviewRound {
911            round: 3,
912            head: "deadbee".to_owned(),
913            verified_head: None,
914            verified_at: None,
915            reviews,
916            e2e: Vec::new(),
917            verify_retried: false,
918            e2e_deferred: false,
919            e2e_defer_reason: None,
920            fix: None,
921            blocking: 0,
922            answered: 2,
923            expected: 2,
924            clean: false,
925            progressed: false,
926            vote_split: false,
927            reconsideration: Vec::new(),
928            verdict: None,
929        }
930    }
931
932    fn merged(reviews: Vec<ReviewRecord>) -> RunState {
933        let mut s = RunState::new(
934            PathBuf::from("/repo"),
935            "main".to_owned(),
936            "abc1234".to_owned(),
937            "do it".to_owned(),
938            Config::default(),
939        );
940        s.reviews.push(round(reviews));
941        s
942    }
943
944    fn two_seats() -> Vec<ReviewRecord> {
945        vec![
946            record(
947                1,
948                vec![
949                    finding("R3-1-1", Severity::Major, "src/waiter.rs", 211),
950                    finding("R3-1-2", Severity::Minor, "src/a.rs", 1),
951                ],
952                ReviewVote::Reject,
953            ),
954            record(
955                2,
956                vec![
957                    finding("R3-2-1", Severity::Major, "src/waiter.rs", 213),
958                    finding("R3-2-2", Severity::Nit, "src/b.rs", 9),
959                ],
960                ReviewVote::Approve,
961            ),
962        ]
963    }
964
965    #[test]
966    fn one_task_per_distinct_defect_with_the_findings_in_full() {
967        let dir = tempfile::tempdir().unwrap();
968        let q = Queue::at(dir.path().join("queue"));
969        let mut s = merged(two_seats());
970        let out = file(&mut s, "https://example.invalid/o/r/pull/9", &q).unwrap();
971        // waiter.rs pair is one defect; seat 1 rejected so its Minor is filed
972        // too; seat 2's Nit is only listed.
973        assert_eq!(out.filed.len(), 2, "{out:?}");
974        assert_eq!(out.unfiled, vec!["R3-2-2".to_owned()]);
975        let tasks = q.list();
976        let t = tasks
977            .iter()
978            .find(|t| t.followup.as_ref().unwrap().findings.len() == 2)
979            .unwrap();
980        assert!(t.solo);
981        assert_eq!(t.followup.as_ref().unwrap().generation, 1);
982        assert_eq!(
983            t.source,
984            Source::Agent {
985                run: s.id.clone(),
986                node: NODE.to_owned()
987            }
988        );
989        for needle in [
990            "R3-1-1",
991            "R3-2-1",
992            "src/waiter.rs:211",
993            "detail of R3-2-1",
994            "pull/9",
995            &s.id,
996            "current main",
997        ] {
998            assert!(
999                t.instruction.contains(needle),
1000                "{needle}: {}",
1001                t.instruction
1002            );
1003        }
1004        assert!(
1005            s.events
1006                .iter()
1007                .any(|e| e.message.contains("filed 2 follow-up"))
1008        );
1009        assert_eq!(s.followups.len(), 2);
1010    }
1011
1012    #[test]
1013    fn filing_twice_does_not_duplicate() {
1014        let dir = tempfile::tempdir().unwrap();
1015        let q = Queue::at(dir.path().join("queue"));
1016        let mut s = merged(two_seats());
1017        file(&mut s, "u", &q).unwrap();
1018        let again = file(&mut s, "u", &q).unwrap();
1019        assert!(again.filed.is_empty());
1020        assert_eq!(q.list().len(), 2);
1021        // Even with the run's own record lost, the queue says what exists.
1022        s.followups.clear();
1023        let third = file(&mut s, "u", &q).unwrap();
1024        assert!(third.filed.is_empty());
1025        assert_eq!(q.list().len(), 2);
1026        assert_eq!(s.followups.len(), 2, "records are restored from the queue");
1027    }
1028
1029    #[test]
1030    fn a_clean_merge_files_nothing() {
1031        let dir = tempfile::tempdir().unwrap();
1032        let q = Queue::at(dir.path().join("queue"));
1033        let mut s = merged(vec![record(
1034            1,
1035            vec![finding("R1-1-1", Severity::Minor, "a.rs", 1)],
1036            ReviewVote::Approve,
1037        )]);
1038        let out = file(&mut s, "u", &q).unwrap();
1039        assert!(out.filed.is_empty());
1040        assert!(q.list().is_empty());
1041        assert!(s.followups.is_empty());
1042    }
1043
1044    #[test]
1045    fn a_failing_queue_is_an_error_event_not_a_status_change() {
1046        let dir = tempfile::tempdir().unwrap();
1047        let root = dir.path().join("queue");
1048        std::fs::write(&root, "not a directory").unwrap();
1049        let q = Queue::at(root);
1050        let mut s = merged(two_seats());
1051        s.status = crate::run::RunStatus::Merged;
1052        let _ = file(&mut s, "u", &q);
1053        assert_eq!(s.status, crate::run::RunStatus::Merged);
1054        assert!(
1055            s.events
1056                .iter()
1057                .any(|e| e.node == NODE && e.message.contains("stopped"))
1058        );
1059        assert!(s.followups.is_empty());
1060    }
1061
1062    #[test]
1063    fn the_generation_cap_stops_the_chain() {
1064        let dir = tempfile::tempdir().unwrap();
1065        let q = Queue::at(dir.path().join("queue"));
1066        let mut parent = Task::new(
1067            "p".to_owned(),
1068            "i".to_owned(),
1069            PathBuf::from("/repo"),
1070            Source::Human,
1071        );
1072        parent.followup = Some(FollowUp {
1073            run: "r0".to_owned(),
1074            origin_task: None,
1075            pr: "u".to_owned(),
1076            findings: vec!["R1-1-1".to_owned()],
1077            generation: MAX_FOLLOWUP_GENERATION,
1078        });
1079        q.put(&mut parent).unwrap();
1080        let mut s = merged(two_seats());
1081        s.origin = Some(Origin {
1082            by: crate::run::StartedBy::Operator,
1083            task: Some(parent.id.clone()),
1084        });
1085        s.origin = Some(Origin {
1086            by: crate::run::StartedBy::Operator,
1087            task: Some("gone".to_owned()),
1088        });
1089        s.followup_generation = Some(MAX_FOLLOWUP_GENERATION);
1090        let out = file(&mut s, "u", &q).unwrap();
1091        assert!(out.filed.is_empty());
1092        assert!(!out.capped.is_empty());
1093        assert_eq!(q.list().len(), 1);
1094        assert!(
1095            s.events
1096                .iter()
1097                .any(|e| e.message.contains("generation cap"))
1098        );
1099    }
1100
1101    #[test]
1102    fn a_deeper_task_files_one_generation_further() {
1103        let dir = tempfile::tempdir().unwrap();
1104        let q = Queue::at(dir.path().join("queue"));
1105        let mut parent = Task::new(
1106            "p".to_owned(),
1107            "i".to_owned(),
1108            PathBuf::from("/repo"),
1109            Source::Human,
1110        );
1111        q.put(&mut parent).unwrap();
1112        let mut s = merged(two_seats());
1113        s.origin = Some(Origin {
1114            by: crate::run::StartedBy::Operator,
1115            task: Some(parent.id.clone()),
1116        });
1117        file(&mut s, "u", &q).unwrap();
1118        let t = q.list().into_iter().find(|t| t.followup.is_some()).unwrap();
1119        assert_eq!(t.followup.unwrap().origin_task, Some(parent.id));
1120    }
1121
1122    #[test]
1123    fn the_chat_is_inherited_from_the_run_even_when_the_parent_is_gone() {
1124        let dir = tempfile::tempdir().unwrap();
1125        let q = Queue::at(dir.path().join("queue"));
1126        let mut s = merged(two_seats());
1127        s.origin = Some(Origin {
1128            by: crate::run::StartedBy::Operator,
1129            task: Some("gone".to_owned()),
1130        });
1131        s.followup_generation = Some(1);
1132        s.origin_chat = Some("talk-1".to_owned());
1133        file(&mut s, "u", &q).unwrap();
1134        let t = q.list().into_iter().find(|t| t.followup.is_some()).unwrap();
1135        assert_eq!(t.origin_chat.as_deref(), Some("talk-1"));
1136    }
1137
1138    #[test]
1139    fn a_parent_without_the_field_passes_on_what_its_ancestry_resolves() {
1140        let dir = tempfile::tempdir().unwrap();
1141        let q = Queue::at(dir.path().join("queue"));
1142        let mut parent = Task::new(
1143            "p".to_owned(),
1144            "i".to_owned(),
1145            PathBuf::from("/repo"),
1146            Source::Agent {
1147                run: "talk-2".to_owned(),
1148                node: crate::queue::CHAT_NODE.to_owned(),
1149            },
1150        );
1151        parent.origin_chat = None;
1152        q.put(&mut parent).unwrap();
1153        let mut s = merged(two_seats());
1154        s.origin = Some(Origin {
1155            by: crate::run::StartedBy::Operator,
1156            task: Some(parent.id.clone()),
1157        });
1158        file(&mut s, "u", &q).unwrap();
1159        let t = q.list().into_iter().find(|t| t.followup.is_some()).unwrap();
1160        assert_eq!(t.origin_chat.as_deref(), Some("talk-2"));
1161    }
1162
1163    const PR: &str = "https://github.com/o/r/pull/473";
1164
1165    fn manual(text: &str) -> Task {
1166        Task::new(
1167            "manual".to_owned(),
1168            text.to_owned(),
1169            PathBuf::from("/repo"),
1170            Source::Human,
1171        )
1172    }
1173
1174    /// File follow-ups with one manual task already in the queue.
1175    fn filed_with(text: &str) -> (Outcome, RunState, Queue, String, tempfile::TempDir) {
1176        let dir = tempfile::tempdir().unwrap();
1177        let q = Queue::at(dir.path().join("queue"));
1178        let mut t = manual(text);
1179        t.status = crate::queue::TaskStatus::Done;
1180        q.put(&mut t).unwrap();
1181        let mut s = merged(two_seats());
1182        let out = file(&mut s, PR, &q).unwrap();
1183        (out, s, q, t.id, dir)
1184    }
1185
1186    #[test]
1187    fn a_task_naming_the_pr_url_and_ids_covers_the_group() {
1188        let (out, s, _q, id, _d) = filed_with(&format!("Fix R3-1-1 and R3-2-1 from {PR}"));
1189        // the waiter.rs group is covered; the other seat-1 Minor is still filed
1190        assert_eq!(out.filed.len(), 1, "{out:?}");
1191        assert_eq!(
1192            out.covered,
1193            vec![
1194                ("R3-1-1".to_owned(), id.clone()),
1195                ("R3-2-1".to_owned(), id.clone())
1196            ]
1197        );
1198        assert!(out.capped.is_empty());
1199        assert!(
1200            s.events
1201                .iter()
1202                .any(|e| e.message.contains("already covered"))
1203        );
1204        let body = comment_body(&s, &out);
1205        assert!(body.contains(&format!("R3-1-1: already covered by task `{id}`")));
1206    }
1207
1208    #[test]
1209    fn the_hash_form_covers_but_a_prefix_number_does_not() {
1210        let (out, ..) = filed_with("R3-1-1 R3-2-1 fixed in PR #473");
1211        assert_eq!(out.covered.len(), 2);
1212        let (out, ..) = filed_with("R3-1-1 R3-2-1 fixed in #47");
1213        assert!(out.covered.is_empty());
1214        assert_eq!(out.filed.len(), 2);
1215        let (out, ..) = filed_with("R3-1-1 R3-2-1 fixed in #4731");
1216        assert!(out.covered.is_empty());
1217    }
1218
1219    #[test]
1220    fn an_id_without_the_pr_does_not_cover() {
1221        let (out, ..) = filed_with("R3-1-1 and R3-2-1 are bad");
1222        assert!(out.covered.is_empty());
1223        assert_eq!(out.filed.len(), 2);
1224    }
1225
1226    #[test]
1227    fn an_id_prefix_does_not_cover() {
1228        let (out, ..) = filed_with(&format!("R3-1-10 R3-2-10 in {PR}"));
1229        assert!(out.covered.is_empty());
1230    }
1231
1232    #[test]
1233    fn partial_coverage_still_files() {
1234        let (out, ..) = filed_with(&format!("R3-1-1 in {PR}"));
1235        assert!(out.covered.is_empty());
1236        assert_eq!(out.filed.len(), 2);
1237    }
1238
1239    #[test]
1240    fn another_followups_findings_cover() {
1241        let dir = tempfile::tempdir().unwrap();
1242        let q = Queue::at(dir.path().join("queue"));
1243        let mut s = merged(two_seats());
1244        let mut other = manual("x");
1245        other.followup = Some(FollowUp {
1246            run: s.id.clone(),
1247            origin_task: None,
1248            pr: PR.to_owned(),
1249            findings: vec!["R3-1-1".to_owned(), "R3-2-1".to_owned()],
1250            generation: 1,
1251        });
1252        q.put(&mut other).unwrap();
1253        let out = file(&mut s, PR, &q).unwrap();
1254        assert_eq!(out.covered.len(), 2, "{out:?}");
1255        assert_eq!(out.filed.len(), 1);
1256    }
1257
1258    #[test]
1259    fn github_text_fixed_followup_comment_passes() {
1260        let state = merged(two_seats());
1261        let body = comment_body(&state, &Outcome::default());
1262        assert!(crate::github_text::check("", &body).is_empty());
1263    }
1264
1265    #[test]
1266    fn a_fully_covered_comment_lists_only_the_covered() {
1267        let (out, mut s, ..) = filed_with(&format!("R3-1-1 R3-2-1 {PR}"));
1268        s.followups.clear();
1269        let body = comment_body(&s, &out);
1270        assert!(body.contains("already covered by task"));
1271        assert!(!body.contains("filed as follow-up"));
1272    }
1273
1274    #[test]
1275    fn an_id_next_to_japanese_prose_still_covers() {
1276        let (out, ..) = filed_with("PR #473 の R3-1-1を修正、R3-2-1も対応");
1277        assert_eq!(out.covered.len(), 2, "{out:?}");
1278    }
1279
1280    // ---- deputy tasks released after the merge ----
1281
1282    fn deputy_task(run: &str, title: &str, text: &str, reason: &str) -> Task {
1283        let mut t = Task::new(
1284            title.to_owned(),
1285            text.to_owned(),
1286            PathBuf::from("/repo"),
1287            Source::Agent {
1288                run: run.to_owned(),
1289                node: crate::deputy::NODE.to_owned(),
1290            },
1291        );
1292        t.hold_manual(Some(reason.to_owned()));
1293        t
1294    }
1295
1296    fn status_of(q: &Queue, id: &str) -> crate::queue::TaskStatus {
1297        q.get(id).unwrap().status
1298    }
1299
1300    #[test]
1301    fn a_deputy_task_is_released_once_and_stamped() {
1302        let dir = tempfile::tempdir().unwrap();
1303        let q = Queue::at(dir.path().join("queue"));
1304        let mut s = merged(two_seats());
1305        s.followup_generation = Some(0);
1306        let mut t = deputy_task(
1307            &s.id,
1308            "Unrelated work",
1309            "do the thing",
1310            &format!("after {PR}"),
1311        );
1312        q.put(&mut t).unwrap();
1313        settle_deputy_tasks(&mut s, PR, &q, true);
1314        let got = q.get(&t.id).unwrap();
1315        assert_eq!(got.status, crate::queue::TaskStatus::Queued);
1316        assert!(got.hold_reason.is_none());
1317        let f = got.followup.unwrap();
1318        assert_eq!((f.run.as_str(), f.generation), (s.id.as_str(), 1));
1319        assert_eq!(s.deputy_followups, vec![t.id.clone()]);
1320        let events = s.events.len();
1321        // A second pass, or a restart that lost the run's record, changes nothing.
1322        settle_deputy_tasks(&mut s, PR, &q, true);
1323        assert_eq!(s.events.len(), events);
1324        s.deputy_followups.clear();
1325        settle_deputy_tasks(&mut s, PR, &q, true);
1326        assert_eq!(
1327            s.deputy_followups,
1328            vec![t.id.clone()],
1329            "rebuilt from the queue"
1330        );
1331        assert_eq!(status_of(&q, &t.id), crate::queue::TaskStatus::Queued);
1332    }
1333
1334    #[test]
1335    fn unrelated_held_tasks_are_never_released() {
1336        let dir = tempfile::tempdir().unwrap();
1337        let q = Queue::at(dir.path().join("queue"));
1338        let mut s = merged(two_seats());
1339        s.followup_generation = Some(0);
1340        let mut other_run = deputy_task("someone-else", "a", "b", &format!("after {PR}"));
1341        let mut other_pr = deputy_task(&s.id, "c", "d", "after https://github.com/o/r/pull/4731");
1342        let mut no_reason = deputy_task(&s.id, "e", "f", "x");
1343        no_reason.hold_reason = None;
1344        let mut human = manual("g");
1345        human.hold_manual(Some(format!("waiting for {PR}")));
1346        let mut queued = deputy_task(&s.id, "h", "i", &format!("after {PR}"));
1347        queued.release();
1348        let ids: Vec<String> = [
1349            &mut other_run,
1350            &mut other_pr,
1351            &mut no_reason,
1352            &mut human,
1353            &mut queued,
1354        ]
1355        .into_iter()
1356        .map(|t| {
1357            q.put(t).unwrap();
1358            t.id.clone()
1359        })
1360        .collect();
1361        settle_deputy_tasks(&mut s, PR, &q, true);
1362        for id in &ids[..4] {
1363            assert_eq!(status_of(&q, id), crate::queue::TaskStatus::Held, "{id}");
1364            assert!(q.get(id).unwrap().followup.is_none());
1365        }
1366        assert!(s.deputy_followups.is_empty());
1367    }
1368
1369    #[test]
1370    fn the_switch_off_leaves_the_task_held() {
1371        let dir = tempfile::tempdir().unwrap();
1372        let q = Queue::at(dir.path().join("queue"));
1373        let mut s = merged(two_seats());
1374        s.followup_generation = Some(0);
1375        let mut t = deputy_task(&s.id, "x", "y", &format!("after {PR}"));
1376        q.put(&mut t).unwrap();
1377        settle_deputy_tasks(&mut s, PR, &q, false);
1378        assert_eq!(status_of(&q, &t.id), crate::queue::TaskStatus::Held);
1379    }
1380
1381    #[test]
1382    fn the_generation_cap_applies_to_released_tasks() {
1383        let dir = tempfile::tempdir().unwrap();
1384        let q = Queue::at(dir.path().join("queue"));
1385        let mut s = merged(two_seats());
1386        s.followup_generation = Some(MAX_FOLLOWUP_GENERATION);
1387        let mut t = deputy_task(&s.id, "x", "y", &format!("after {PR}"));
1388        q.put(&mut t).unwrap();
1389        settle_deputy_tasks(&mut s, PR, &q, true);
1390        let got = q.get(&t.id).unwrap();
1391        assert_eq!(got.status, crate::queue::TaskStatus::Held);
1392        assert!(got.hold_reason.unwrap().starts_with(CAPPED));
1393        let events = s.events.len();
1394        settle_deputy_tasks(&mut s, PR, &q, true);
1395        assert_eq!(s.events.len(), events, "marked once");
1396        let mut s2 = merged(two_seats());
1397        s2.followup_generation = Some(MAX_FOLLOWUP_GENERATION - 1);
1398        let mut t2 = deputy_task(&s2.id, "x", "y", &format!("after {PR}"));
1399        q.put(&mut t2).unwrap();
1400        settle_deputy_tasks(&mut s2, PR, &q, true);
1401        assert_eq!(
1402            q.get(&t2.id).unwrap().followup.unwrap().generation,
1403            MAX_FOLLOWUP_GENERATION
1404        );
1405    }
1406
1407    #[test]
1408    fn deputy_first_the_owners_task_runs_and_the_automatic_one_is_not_filed() {
1409        let dir = tempfile::tempdir().unwrap();
1410        let q = Queue::at(dir.path().join("queue"));
1411        let mut s = merged(two_seats());
1412        s.followup_generation = Some(0);
1413        let mut t = deputy_task(
1414            &s.id,
1415            "Fix the waiter",
1416            "R3-1-1 at src/waiter.rs:212 loses replies",
1417            &format!("after {PR}"),
1418        );
1419        q.put(&mut t).unwrap();
1420        settle_deputy_tasks(&mut s, PR, &q, true);
1421        let out = file(&mut s, PR, &q).unwrap();
1422        assert_eq!(out.covered.len(), 2, "{out:?}");
1423        // Only the seat-1 Minor group is filed; the waiter.rs defect has one task.
1424        assert_eq!(out.filed.len(), 1, "{out:?}");
1425        let runnable: Vec<_> = q
1426            .list()
1427            .into_iter()
1428            .filter(|t| {
1429                t.status == crate::queue::TaskStatus::Queued
1430                    && t.followup
1431                        .as_ref()
1432                        .is_some_and(|f| f.findings.iter().any(|x| x == "R3-1-1"))
1433            })
1434            .collect();
1435        assert_eq!(runnable.len(), 1);
1436        assert_eq!(runnable[0].id, t.id);
1437    }
1438
1439    #[test]
1440    fn followup_first_a_queued_automatic_task_gives_way_to_the_owners() {
1441        let dir = tempfile::tempdir().unwrap();
1442        let q = Queue::at(dir.path().join("queue"));
1443        let mut s = merged(two_seats());
1444        s.followup_generation = Some(0);
1445        file(&mut s, PR, &q).unwrap();
1446        let mut t = deputy_task(
1447            &s.id,
1448            "Waiter replies",
1449            "see R3-2-1",
1450            &format!("after {PR}"),
1451        );
1452        q.put(&mut t).unwrap();
1453        settle_deputy_tasks(&mut s, PR, &q, true);
1454        let all = q.list();
1455        let live: Vec<_> = all
1456            .iter()
1457            .filter(|t| {
1458                t.status == crate::queue::TaskStatus::Queued
1459                    && t.followup
1460                        .as_ref()
1461                        .is_some_and(|f| f.findings.iter().any(|x| x == "R3-2-1"))
1462            })
1463            .collect();
1464        assert_eq!(live.len(), 1);
1465        assert_eq!(live[0].id, t.id);
1466        let dropped = all
1467            .iter()
1468            .find(|a| {
1469                a.id != t.id
1470                    && a.followup
1471                        .as_ref()
1472                        .is_some_and(|f| f.findings.iter().any(|x| x == "R3-2-1"))
1473            })
1474            .unwrap();
1475        assert_eq!(dropped.status, crate::queue::TaskStatus::Held);
1476        assert!(
1477            dropped
1478                .hold_reason
1479                .as_ref()
1480                .unwrap()
1481                .starts_with(SUPERSEDED)
1482        );
1483    }
1484
1485    #[test]
1486    fn followup_first_a_finished_automatic_task_supersedes_the_deputys() {
1487        let dir = tempfile::tempdir().unwrap();
1488        let q = Queue::at(dir.path().join("queue"));
1489        let mut s = merged(two_seats());
1490        s.followup_generation = Some(0);
1491        file(&mut s, PR, &q).unwrap();
1492        for mut a in q.list() {
1493            a.status = crate::queue::TaskStatus::Done;
1494            q.put(&mut a).unwrap();
1495        }
1496        // Matched by the finding's title, not its id.
1497        let mut t = deputy_task(&s.id, "title R3-1-1", "no ids here", &format!("after {PR}"));
1498        t.instruction = "nothing".to_owned();
1499        q.put(&mut t).unwrap();
1500        settle_deputy_tasks(&mut s, PR, &q, true);
1501        let got = q.get(&t.id).unwrap();
1502        assert_eq!(got.status, crate::queue::TaskStatus::Held);
1503        assert!(got.followup.is_none());
1504        let reason = got.hold_reason.unwrap();
1505        assert!(reason.starts_with(SUPERSEDED), "{reason}");
1506        assert!(reason.contains(PR));
1507        let events = s.events.len();
1508        settle_deputy_tasks(&mut s, PR, &q, true);
1509        assert_eq!(s.events.len(), events, "judged once");
1510    }
1511
1512    #[test]
1513    fn a_held_deputy_task_is_superseded_by_a_later_filing_when_the_switch_is_off() {
1514        let dir = tempfile::tempdir().unwrap();
1515        let q = Queue::at(dir.path().join("queue"));
1516        let mut s = merged(two_seats());
1517        s.followup_generation = Some(0);
1518        let mut t = deputy_task(&s.id, "x", "R3-1-1 here", &format!("after {PR}"));
1519        q.put(&mut t).unwrap();
1520        settle_deputy_tasks(&mut s, PR, &q, false);
1521        assert!(
1522            !q.get(&t.id)
1523                .unwrap()
1524                .hold_reason
1525                .unwrap()
1526                .starts_with(SUPERSEDED)
1527        );
1528        file(&mut s, PR, &q).unwrap();
1529        settle_deputy_tasks(&mut s, PR, &q, false);
1530        let got = q.get(&t.id).unwrap();
1531        assert_eq!(got.status, crate::queue::TaskStatus::Held);
1532        assert!(got.hold_reason.unwrap().starts_with(SUPERSEDED));
1533    }
1534
1535    #[test]
1536    fn the_comment_lists_released_deputy_tasks() {
1537        let mut s = merged(two_seats());
1538        s.deputy_followups.push("abcd-1".to_owned());
1539        let body = comment_body(&s, &Outcome::default());
1540        assert!(body.contains("`abcd-1`"));
1541        assert!(!body.contains("already covered"));
1542        assert!(crate::github_text::check("", &body).is_empty());
1543    }
1544
1545    #[test]
1546    fn a_deputy_task_over_two_automatic_ones_holds_both() {
1547        let dir = tempfile::tempdir().unwrap();
1548        let q = Queue::at(dir.path().join("queue"));
1549        let mut s = merged(two_seats());
1550        s.followup_generation = Some(0);
1551        file(&mut s, PR, &q).unwrap();
1552        let mut t = deputy_task(&s.id, "both", "R3-1-1 and R3-1-2", &format!("after {PR}"));
1553        q.put(&mut t).unwrap();
1554        settle_deputy_tasks(&mut s, PR, &q, true);
1555        assert_eq!(status_of(&q, &t.id), crate::queue::TaskStatus::Queued);
1556        let live = q
1557            .list()
1558            .into_iter()
1559            .filter(|x| x.status == crate::queue::TaskStatus::Queued)
1560            .count();
1561        assert_eq!(live, 1, "only the owner's task still runs");
1562    }
1563
1564    #[test]
1565    fn a_claimed_automatic_task_is_not_touched_and_the_deputys_stays_held() {
1566        let dir = tempfile::tempdir().unwrap();
1567        let q = Queue::at(dir.path().join("queue"));
1568        let mut s = merged(two_seats());
1569        s.followup_generation = Some(0);
1570        file(&mut s, PR, &q).unwrap();
1571        let auto = q
1572            .list()
1573            .into_iter()
1574            .find(|x| {
1575                x.followup
1576                    .as_ref()
1577                    .is_some_and(|f| f.findings.iter().any(|i| i == "R3-1-1"))
1578            })
1579            .unwrap();
1580        let _claim = q.claim(&auto.id).unwrap();
1581        let mut t = deputy_task(&s.id, "x", "R3-1-1", &format!("after {PR}"));
1582        q.put(&mut t).unwrap();
1583        settle_deputy_tasks(&mut s, PR, &q, true);
1584        assert_eq!(status_of(&q, &auto.id), crate::queue::TaskStatus::Queued);
1585        let got = q.get(&t.id).unwrap();
1586        assert_eq!(got.status, crate::queue::TaskStatus::Held);
1587        assert!(got.hold_reason.unwrap().starts_with(SUPERSEDED));
1588    }
1589
1590    #[test]
1591    fn a_failed_hold_restores_the_automatic_tasks_already_held() {
1592        let dir = tempfile::tempdir().unwrap();
1593        let q = Queue::at(dir.path().join("queue"));
1594        let mut s = merged(two_seats());
1595        s.followup_generation = Some(0);
1596        file(&mut s, PR, &q).unwrap();
1597        let autos = q.list();
1598        assert_eq!(autos.len(), 2);
1599        let mut t = deputy_task(&s.id, "both", "R3-1-1 and R3-1-2", &format!("after {PR}"));
1600        q.put(&mut t).unwrap();
1601        // Make the save of the second automatic task fail.
1602        let last = autos.last().unwrap().id.clone();
1603        let tmp = dir.path().join("queue").join(format!("{last}.json.tmp"));
1604        std::fs::create_dir_all(&tmp).unwrap();
1605        settle_deputy_tasks(&mut s, PR, &q, true);
1606        for a in &autos {
1607            let got = q.get(&a.id).unwrap();
1608            assert_eq!(got.status, crate::queue::TaskStatus::Queued, "{}", a.id);
1609            assert_eq!(got.attempts, a.attempts);
1610        }
1611        let got = q.get(&t.id).unwrap();
1612        assert_eq!(got.status, crate::queue::TaskStatus::Held);
1613        assert!(got.hold_reason.unwrap().starts_with(SUPERSEDED));
1614    }
1615
1616    #[test]
1617    fn a_hold_left_by_an_earlier_failed_pass_is_restored_on_the_next_failure() {
1618        let dir = tempfile::tempdir().unwrap();
1619        let q = Queue::at(dir.path().join("queue"));
1620        let mut s = merged(two_seats());
1621        s.followup_generation = Some(0);
1622        file(&mut s, PR, &q).unwrap();
1623        let autos = q.list();
1624        let mut t = deputy_task(&s.id, "both", "R3-1-1 and R3-1-2", &format!("after {PR}"));
1625        q.put(&mut t).unwrap();
1626        // The first auto task is left held by an earlier, interrupted pass.
1627        let why = format!(
1628            "{SUPERSEDED}the owner's approval deputy filed task {} for the same finding(s) of {PR}",
1629            t.id
1630        );
1631        q.modify(&autos[0].id, |x| {
1632            x.hold_manual(Some(why));
1633            true
1634        })
1635        .unwrap();
1636        let tmp = dir
1637            .path()
1638            .join("queue")
1639            .join(format!("{}.json.tmp", autos[1].id));
1640        std::fs::create_dir_all(&tmp).unwrap();
1641        settle_deputy_tasks(&mut s, PR, &q, true);
1642        for a in &autos {
1643            let got = q.get(&a.id).unwrap();
1644            assert_eq!(got.status, crate::queue::TaskStatus::Queued, "{}", a.id);
1645            assert!(got.hold_reason.is_none());
1646        }
1647        assert!(
1648            q.get(&t.id)
1649                .unwrap()
1650                .hold_reason
1651                .unwrap()
1652                .starts_with(SUPERSEDED)
1653        );
1654    }
1655
1656    #[test]
1657    fn a_blocked_takeover_restores_a_hold_left_by_an_earlier_pass() {
1658        let dir = tempfile::tempdir().unwrap();
1659        let q = Queue::at(dir.path().join("queue"));
1660        let mut s = merged(two_seats());
1661        s.followup_generation = Some(0);
1662        file(&mut s, PR, &q).unwrap();
1663        let autos = q.list();
1664        let mut t = deputy_task(&s.id, "both", "R3-1-1 and R3-1-2", &format!("after {PR}"));
1665        q.put(&mut t).unwrap();
1666        let why = format!(
1667            "{SUPERSEDED}the owner's approval deputy filed task {} for the same finding(s) of {PR}",
1668            t.id
1669        );
1670        q.modify(&autos[0].id, |x| {
1671            x.hold_manual(Some(why));
1672            true
1673        })
1674        .unwrap();
1675        let _claim = q.claim(&autos[1].id).unwrap();
1676        settle_deputy_tasks(&mut s, PR, &q, true);
1677        assert_eq!(
1678            status_of(&q, &autos[0].id),
1679            crate::queue::TaskStatus::Queued
1680        );
1681        assert!(
1682            q.get(&t.id)
1683                .unwrap()
1684                .hold_reason
1685                .unwrap()
1686                .starts_with(SUPERSEDED)
1687        );
1688    }
1689}