Skip to main content

magi/
recovery.rs

1//! Carrying an earlier attempt's evidence into the run that takes its branch over.
2//!
3//! A task whose previous run stopped on a failed verification used to reopen
4//! its branch as a context-free review-only run: the new reviewers and fixer
5//! never saw the failing command's output, the review decisions or the
6//! unresolved findings, and round 1 started from scratch.
7//!
8//! [`assess`] is pure. From the earlier run's record and the relation of the
9//! branch tip to the commit that run reviewed it decides between
10//!
11//! * [`Mode::VerifyRepair`]: the earlier panel found nothing blocking and only
12//!   verification failed, on exactly the commit that is about to be reviewed.
13//!   The fixer gets the failure first, then the review is scoped to what
14//!   changed since the reviewed commit.
15//! * [`Mode::FullReview`]: anything doubtful. The reviewers still see the
16//!   earlier context, but nothing is scoped and no approval is carried; each
17//!   [`RecoveryRecord::reasons`] entry says why.
18//!
19//! Inherited verification is evidence for a prompt, never a result: it is not
20//! copied into [`crate::run::ReviewRound`], so verification always runs again
21//! on the commit that is going to land.
22
23use serde::{Deserialize, Serialize};
24
25use crate::run::{CommandOutcome, E2eStatus, ReviewRound, RunState};
26
27fn short(sha: &str) -> &str {
28    sha.get(..8).unwrap_or(sha)
29}
30use crate::verdict::{Finding, ReviewVote};
31
32/// How many times evidence may be handed from one run to the next. Past this
33/// the successor starts as a plain full review.
34pub const MAX_GENERATION: u32 = 2;
35
36/// Longest output tail carried into a prompt, in bytes.
37pub const TAIL_MAX: usize = 6_000;
38
39/// Longest scoped diff shown to a reviewer, in bytes.
40pub const SINCE_DIFF_MAX: usize = 60_000;
41
42/// What the review after a takeover does with the earlier run.
43#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
44#[serde(rename_all = "kebab-case")]
45pub enum Mode {
46    /// One repair fix with the failure in hand, then a review scoped to the
47    /// changes since the reviewed commit.
48    VerifyRepair,
49    /// The whole patch is reviewed again; the earlier run is context only.
50    FullReview,
51}
52
53/// Which check failed.
54#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
55#[serde(rename_all = "kebab-case")]
56pub enum Source {
57    /// The round's `verify.e2e`.
58    E2e,
59    /// The final `verify.gate`.
60    Gate,
61}
62
63/// The failed verification the earlier run stopped on.
64#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
65pub struct Evidence {
66    /// Which check.
67    pub source: Source,
68    /// The command, as configured.
69    pub command: String,
70    /// Exit code, `None` on timeout or signal.
71    pub code: Option<i32>,
72    /// Scrubbed, clipped tail of its output.
73    pub output_tail: String,
74    /// The commit it was run against.
75    pub head: String,
76}
77
78/// One earlier review round, reduced to its decision.
79#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
80pub struct PriorRound {
81    /// Round number in the earlier run.
82    pub round: usize,
83    /// The commit it reviewed.
84    pub head: String,
85    /// Findings that held the merge.
86    pub blocking: usize,
87    /// Seats that answered, of `expected`.
88    pub answered: usize,
89    /// Seats the round expected.
90    pub expected: usize,
91    /// The round's verdict, when a vote was usable.
92    pub verdict: Option<ReviewVote>,
93}
94
95/// Where the one repair fix stands. Saved before the fixer is called, so a
96/// resume never repairs twice.
97#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
98#[serde(rename_all = "kebab-case")]
99pub enum Repair {
100    /// Not attempted (yet, or not applicable).
101    NotStarted,
102    /// The fixer was called from this commit and has not been settled.
103    Started {
104        /// HEAD before the call.
105        before: String,
106    },
107    /// Settled.
108    Done {
109        /// HEAD before the call.
110        before: String,
111        /// HEAD after it.
112        after: String,
113        /// The fixer's own commit or rescue commit moved HEAD.
114        committed: bool,
115        /// Why nothing was kept (the worktree was restored to `before`).
116        failed: Option<String>,
117    },
118}
119
120/// What a run inherited from the attempt it took over, and why.
121#[derive(Debug, Clone, Serialize, Deserialize)]
122pub struct RecoveryRecord {
123    /// The earlier run.
124    pub from_run: String,
125    /// How many hand-overs of evidence lie behind this one.
126    pub generation: u32,
127    /// The decision.
128    pub mode: Mode,
129    /// For [`Mode::FullReview`]: every reason a narrower recovery was not
130    /// justified. Empty for [`Mode::VerifyRepair`].
131    #[serde(default)]
132    pub reasons: Vec<String>,
133    /// The earlier run's last reviewed commit, when it had one.
134    #[serde(default)]
135    pub reviewed_head: Option<String>,
136    /// The earlier rounds' decisions.
137    #[serde(default)]
138    pub rounds: Vec<PriorRound>,
139    /// Findings the last earlier round left open.
140    #[serde(default)]
141    pub open_findings: Vec<Finding>,
142    /// The failed verification, when one was recorded.
143    #[serde(default)]
144    pub evidence: Option<Evidence>,
145    /// The task's own statement replaced the commit-log stand-in.
146    #[serde(default)]
147    pub criteria_inherited: bool,
148    /// The repair fix.
149    #[serde(default = "not_started")]
150    pub repair: Repair,
151}
152
153fn not_started() -> Repair {
154    Repair::NotStarted
155}
156
157/// How the branch tip relates to the commit the earlier run reviewed.
158#[derive(Debug, Clone, Copy, PartialEq, Eq)]
159pub enum Relation {
160    /// The same commit.
161    Same,
162    /// The reviewed commit is a proper ancestor: commits were added after it.
163    Descendant,
164    /// Not an ancestor: rebased, rewritten or unknown to this repository.
165    Diverged,
166}
167
168/// Clip `text` to its last `max` bytes on a char boundary.
169pub fn tail_of(text: &str, max: usize) -> String {
170    if text.len() <= max {
171        return text.to_owned();
172    }
173    let mut start = text.len() - max;
174    while !text.is_char_boundary(start) {
175        start += 1;
176    }
177    format!("[...]\n{}", &text[start..])
178}
179
180/// Clip `text` to its first `max` bytes on a char boundary.
181pub fn head_of(text: &str, max: usize) -> String {
182    if text.len() <= max {
183        return text.to_owned();
184    }
185    let mut end = max;
186    while !text.is_char_boundary(end) {
187        end -= 1;
188    }
189    format!("{}\n[... truncated]", &text[..end])
190}
191
192fn evidence_from(round: &ReviewRound, prior: &RunState) -> Option<Evidence> {
193    let scrub =
194        |s: &str| crate::scrub::scrub(&tail_of(s, TAIL_MAX), &crate::scrub::Identity::current());
195    let failed = |o: &&CommandOutcome| !o.ok() && !o.resource_blocked;
196    if round.e2e_status() == E2eStatus::Failed
197        && let (Some(o), Some(head)) = (round.e2e.iter().find(failed), &round.verified_head)
198    {
199        return Some(Evidence {
200            source: Source::E2e,
201            command: o.command.clone(),
202            code: o.code,
203            output_tail: scrub(&o.output_tail),
204            head: head.clone(),
205        });
206    }
207    // The gate runs on the tree the last round left; it is only evidence about
208    // that round's head while no gate fix moved the tree afterwards.
209    if prior.gate_ran
210        && prior.gate_fixes.is_empty()
211        && let Some(o) = prior.gate.iter().find(failed)
212    {
213        return Some(Evidence {
214            source: Source::Gate,
215            command: o.command.clone(),
216            code: o.code,
217            output_tail: scrub(&o.output_tail),
218            head: round.head.clone(),
219        });
220    }
221    None
222}
223
224/// The record for a run that takes `prior` over. Pure.
225///
226/// `prior` is `None` when the earlier attempt's record could not be read.
227/// `base_now` is the base commit the new run reviews against and `relation`
228/// how the branch tip stands to the earlier run's last reviewed commit.
229pub fn assess(
230    from_run: &str,
231    prior: Option<&RunState>,
232    base_now: &str,
233    relation: Relation,
234) -> RecoveryRecord {
235    let mut rec = RecoveryRecord {
236        from_run: from_run.to_owned(),
237        generation: 0,
238        mode: Mode::FullReview,
239        reasons: Vec::new(),
240        reviewed_head: None,
241        rounds: Vec::new(),
242        open_findings: Vec::new(),
243        evidence: None,
244        criteria_inherited: false,
245        repair: Repair::NotStarted,
246    };
247    let Some(prior) = prior else {
248        rec.reasons
249            .push("the earlier run's record could not be read".to_owned());
250        return rec;
251    };
252    rec.generation = prior.recovery.as_ref().map_or(0, |r| r.generation) + 1;
253    rec.rounds = prior
254        .reviews
255        .iter()
256        .map(|r| PriorRound {
257            round: r.round,
258            head: r.head.clone(),
259            blocking: r.blocking,
260            answered: r.answered,
261            expected: r.expected,
262            verdict: r.verdict,
263        })
264        .collect();
265    let Some(last) = prior.reviews.last() else {
266        rec.reasons
267            .push("the earlier run never completed a review round".to_owned());
268        return rec;
269    };
270    rec.reviewed_head = Some(last.head.clone());
271    rec.open_findings = last
272        .reviews
273        .iter()
274        .flat_map(|r| r.findings.iter().cloned())
275        .collect();
276    rec.evidence = evidence_from(last, prior);
277
278    if rec.generation > MAX_GENERATION {
279        rec.reasons.push(format!(
280            "evidence has already been handed over {MAX_GENERATION} times"
281        ));
282    }
283    if last.answered < last.expected || last.reviews.iter().any(|r| r.failed.is_some()) {
284        rec.reasons.push(format!(
285            "only {} of {} reviewer seat(s) answered the last round; a panel that did not \
286             answer is not a clean one",
287            last.answered, last.expected
288        ));
289    }
290    if last.blocking > 0 {
291        rec.reasons.push(format!(
292            "the last round still had {} blocking finding(s)",
293            last.blocking
294        ));
295    }
296    match &rec.evidence {
297        None => rec
298            .reasons
299            .push("no failed verification with a known command was recorded".to_owned()),
300        Some(e) if e.output_tail.trim().is_empty() => rec
301            .reasons
302            .push("the failed verification kept no output to act on".to_owned()),
303        Some(e) if e.head != last.head => rec.reasons.push(
304            "the failed verification was about a different commit than the one reviewed".to_owned(),
305        ),
306        Some(_) => {}
307    }
308    match relation {
309        Relation::Same => {}
310        Relation::Descendant => rec.reasons.push(
311            "commits were added after the reviewed commit; they were never reviewed".to_owned(),
312        ),
313        Relation::Diverged => rec.reasons.push(
314            "the reviewed commit is not an ancestor of the branch: it was rebased or rewritten"
315                .to_owned(),
316        ),
317    }
318    if prior.base_commit != base_now {
319        rec.reasons.push(format!(
320            "the base moved from {} to {}",
321            short(&prior.base_commit),
322            short(base_now)
323        ));
324    }
325    if rec.reasons.is_empty() {
326        rec.mode = Mode::VerifyRepair;
327    }
328    rec
329}
330
331/// Re-check a [`Mode::VerifyRepair`] record against the tree the review is
332/// about to look at (a base sync may have rebased it after the takeover was
333/// assessed). Returns true when it was downgraded. Pure.
334pub fn recheck(rec: &mut RecoveryRecord, head: &str) -> bool {
335    if rec.mode != Mode::VerifyRepair {
336        return false;
337    }
338    let Some(reviewed) = rec.reviewed_head.as_deref() else {
339        return false;
340    };
341    if reviewed == head {
342        return false;
343    }
344    rec.mode = Mode::FullReview;
345    rec.reasons.push(format!(
346        "the branch moved from {} to {} before the review started (rebased onto the base or \
347         changed); the earlier approval does not carry",
348        short(reviewed),
349        short(head)
350    ));
351    true
352}
353
354impl RecoveryRecord {
355    /// One line for the event log, `magi show` and the web.
356    pub fn summary(&self) -> String {
357        let from = crate::run::short_of(&self.from_run);
358        match self.mode {
359            Mode::VerifyRepair => format!(
360                "inherited from run {from}: reviewed commit, {} round(s), failed verification \
361                 `{}`; repair first, then a review scoped to the changes since",
362                self.rounds.len(),
363                self.evidence.as_ref().map_or("?", |e| e.command.as_str())
364            ),
365            Mode::FullReview => format!(
366                "full review after run {from}: {}",
367                if self.reasons.is_empty() {
368                    "no narrower recovery applied".to_owned()
369                } else {
370                    self.reasons.join("; ")
371                }
372            ),
373        }
374    }
375}
376
377#[cfg(test)]
378mod tests {
379    use super::*;
380    use crate::run::ReviewRecord;
381    use crate::verdict::Severity;
382
383    fn outcome(code: i32, tail: &str) -> CommandOutcome {
384        CommandOutcome {
385            command: "cargo test".to_owned(),
386            code: Some(code),
387            output_tail: tail.to_owned(),
388            duration_ms: 1,
389            resource_blocked: false,
390        }
391    }
392
393    fn seat(failed: bool, findings: Vec<Finding>) -> ReviewRecord {
394        ReviewRecord {
395            reviewer: 1,
396            agent: "a".to_owned(),
397            summary: String::new(),
398            findings,
399            vote: Some(ReviewVote::Approve),
400            failed: failed.then(|| "timed out".to_owned()),
401            duration_ms: 0,
402            attempts: 0,
403        }
404    }
405
406    fn round(head: &str, e2e: Vec<CommandOutcome>) -> ReviewRound {
407        ReviewRound {
408            round: 3,
409            head: head.to_owned(),
410            verified_head: Some(head.to_owned()),
411            verified_at: None,
412            reviews: vec![seat(false, Vec::new())],
413            e2e,
414            verify_retried: false,
415            e2e_deferred: false,
416            e2e_defer_reason: None,
417            fix: None,
418            blocking: 0,
419            answered: 1,
420            expected: 1,
421            clean: false,
422            progressed: false,
423            vote_split: false,
424            reconsideration: Vec::new(),
425            verdict: Some(ReviewVote::Approve),
426        }
427    }
428
429    fn prior(round: ReviewRound) -> RunState {
430        let mut s = RunState::new(
431            std::path::PathBuf::from("/tmp/x"),
432            "main".to_owned(),
433            "base1".to_owned(),
434            "task".to_owned(),
435            crate::config::Config::default(),
436        );
437        s.reviews.push(round);
438        s
439    }
440
441    fn failing() -> RunState {
442        prior(round("aaa111", vec![outcome(101, "1 failed: quiet_spawn")]))
443    }
444
445    #[test]
446    fn unchanged_commit_with_a_nonblocking_failed_verification_is_a_verify_repair() {
447        let r = assess("r1", Some(&failing()), "base1", Relation::Same);
448        assert_eq!(r.mode, Mode::VerifyRepair, "{:?}", r.reasons);
449        assert!(r.reasons.is_empty());
450        assert_eq!(r.generation, 1);
451        let e = r.evidence.unwrap();
452        assert_eq!(e.command, "cargo test");
453        assert_eq!(e.head, "aaa111");
454        assert!(e.output_tail.contains("quiet_spawn"));
455    }
456
457    #[test]
458    fn descendant_rebased_and_moved_base_widen_the_review() {
459        let p = failing();
460        let d = assess("r1", Some(&p), "base1", Relation::Descendant);
461        assert_eq!(d.mode, Mode::FullReview);
462        assert!(d.reasons[0].contains("never reviewed"));
463        let x = assess("r1", Some(&p), "base1", Relation::Diverged);
464        assert_eq!(x.mode, Mode::FullReview);
465        assert!(x.reasons[0].contains("rebased"));
466        let b = assess("r1", Some(&p), "base2", Relation::Same);
467        assert_eq!(b.mode, Mode::FullReview);
468        assert!(b.reasons[0].contains("base moved"));
469        // The evidence still travels as context.
470        assert!(b.evidence.is_some());
471    }
472
473    #[test]
474    fn missing_or_empty_evidence_falls_back_to_a_full_review() {
475        let none = prior(round("aaa111", Vec::new()));
476        let r = assess("r1", Some(&none), "base1", Relation::Same);
477        assert_eq!(r.mode, Mode::FullReview);
478        assert!(r.reasons[0].contains("no failed verification"));
479        let empty = prior(round("aaa111", vec![outcome(1, "  \n")]));
480        let r = assess("r1", Some(&empty), "base1", Relation::Same);
481        assert_eq!(r.mode, Mode::FullReview);
482        assert!(r.reasons[0].contains("no output"));
483        let r = assess("r1", None, "base1", Relation::Same);
484        assert_eq!(r.mode, Mode::FullReview);
485        assert!(r.reasons[0].contains("could not be read"));
486    }
487
488    #[test]
489    fn evidence_about_another_commit_is_not_reused() {
490        let mut p = failing();
491        p.reviews[0].verified_head = Some("bbb222".to_owned());
492        let r = assess("r1", Some(&p), "base1", Relation::Same);
493        assert_eq!(r.mode, Mode::FullReview);
494        assert!(r.reasons[0].contains("different commit"));
495    }
496
497    #[test]
498    fn a_timed_out_seat_or_a_blocking_finding_is_never_a_clean_panel() {
499        let mut p = failing();
500        p.reviews[0].answered = 0;
501        p.reviews[0].reviews[0] = seat(true, Vec::new());
502        let r = assess("r1", Some(&p), "base1", Relation::Same);
503        assert_eq!(r.mode, Mode::FullReview);
504        assert!(r.reasons[0].contains("did not answer"));
505
506        let mut p = failing();
507        p.reviews[0].blocking = 1;
508        p.reviews[0].reviews[0].findings = vec![Finding {
509            id: "R3-1-1".to_owned(),
510            severity: Severity::Major,
511            file: None,
512            line: None,
513            title: "t".to_owned(),
514            detail: String::new(),
515        }];
516        let r = assess("r1", Some(&p), "base1", Relation::Same);
517        assert_eq!(r.mode, Mode::FullReview);
518        assert_eq!(r.open_findings.len(), 1);
519    }
520
521    #[test]
522    fn generations_are_bounded() {
523        let mut p = failing();
524        let mut inherited = assess("r0", Some(&failing()), "base1", Relation::Same);
525        inherited.generation = MAX_GENERATION;
526        p.recovery = Some(inherited);
527        let r = assess("r1", Some(&p), "base1", Relation::Same);
528        assert_eq!(r.generation, MAX_GENERATION + 1);
529        assert_eq!(r.mode, Mode::FullReview);
530        assert!(r.reasons[0].contains("handed over"));
531    }
532
533    #[test]
534    fn a_gate_failure_counts_only_while_no_gate_fix_moved_the_tree() {
535        let mut p = prior(round("aaa111", Vec::new()));
536        p.gate_ran = true;
537        p.gate = vec![outcome(2, "gate red")];
538        let r = assess("r1", Some(&p), "base1", Relation::Same);
539        assert_eq!(r.mode, Mode::VerifyRepair);
540        assert_eq!(r.evidence.unwrap().source, Source::Gate);
541        p.gate_fixes.push(crate::run::GateFixRecord {
542            agent: "a".to_owned(),
543            failed: Vec::new(),
544            notes: String::new(),
545            committed: true,
546            error: None,
547        });
548        let r = assess("r1", Some(&p), "base1", Relation::Same);
549        assert_eq!(r.mode, Mode::FullReview);
550    }
551
552    #[test]
553    fn recheck_downgrades_a_moved_branch() {
554        let mut r = assess("r1", Some(&failing()), "base1", Relation::Same);
555        assert!(!recheck(&mut r, "aaa111"));
556        assert_eq!(r.mode, Mode::VerifyRepair);
557        assert!(recheck(&mut r, "ccc333"));
558        assert_eq!(r.mode, Mode::FullReview);
559        assert!(r.reasons[0].contains("moved"));
560    }
561
562    #[test]
563    fn tails_are_clipped_on_char_boundaries() {
564        let s = "あ".repeat(100);
565        assert!(tail_of(&s, 10).len() < 30);
566        assert!(head_of(&s, 10).starts_with('あ'));
567        assert_eq!(tail_of("ok", 10), "ok");
568    }
569}