Skip to main content

magi/
land.rs

1//! Landing the winner: watch the pull request, fix what it complains about,
2//! and merge it.
3//!
4//! Opening the pull request used to be where magi stopped and the operator
5//! started: watch the checks, read what the review bots found, push a fix,
6//! wait again, merge. That loop is mechanical, it takes an hour of wall-clock
7//! time per pull request, and doing it by hand six times in one session is how
8//! a queue that drains unattended stops being unattended. So it lives here.
9//!
10//! # Shape
11//!
12//! [`PrState`] is one observation of a pull request and [`decide`] is the whole
13//! policy as a *pure* function of it. Nothing in [`decide`] talks to `gh`,
14//! which is what makes "green with an unresolved comment is a fix, not a merge"
15//! an assertion in a test rather than a claim in a comment. [`land`] is the
16//! only part that performs I/O: observe, decide, act, repeat.
17//!
18//! # What it refuses to do
19//!
20//! Merging is the one irreversible thing magi can do to a repository, so the
21//! loop is built to stop rather than to guess:
22//!
23//! * A red pull request is never merged. When the budget runs out the pull
24//!   request is left open with a comment naming what is still failing, because
25//!   a magi that force-merges a red pull request is worse than one that stops.
26//! * A pull request whose checks cannot be read at all (`gh` reported no
27//!   rollup) is not merged either. Landing is for repositories with CI; with no
28//!   signal there is nothing to be green.
29//! * A pull request a human merged or closed underneath us is
30//!   [`Step::Done`] - the person won, and their decision is not an error.
31//!
32//! # Why `--subject` is not optional
33//!
34//! A candidate branch holds one commit whose subject is
35//! `magi: candidate A (uncommitted work)`, and `gh pr merge --squash` prefers a
36//! single commit's message over the pull request title. Merging without
37//! [`merge_argv`]'s explicit `--subject` therefore writes a `main` history that
38//! says nothing about what landed. `AGENTS.md` records the trap; this module is
39//! where it is prevented.
40
41use std::collections::BTreeSet;
42use std::fmt::Write as _;
43use std::path::Path;
44use std::time::Duration;
45
46use anyhow::{Context as _, Result, bail};
47use serde::Deserialize;
48
49use crate::agent::{self, Invocation, SeatState};
50use crate::ask;
51use crate::config::{AgentSpec, MergeMode};
52use crate::git;
53use crate::proc::Quiet as _;
54use crate::prompt;
55use crate::run::{MergeOutcome, RunState, RunStatus, tail};
56
57/// How often the pull request is re-read while its checks are still running.
58///
59/// Thirty seconds: a CI matrix takes minutes, so anything shorter is spent
60/// entirely on `gh` invocations, and anything much longer adds latency to every
61/// single round of a loop that already waits for agents.
62pub const POLL: Duration = Duration::from_secs(30);
63
64/// How long one wait may last before landing gives up on the checks finishing.
65///
66/// A workflow that has not settled in forty-five minutes is stuck on a runner
67/// queue, a missing approval, or a hung job - none of which more polling fixes,
68/// and all of which a person needs to see.
69pub const WAIT_CEILING: Duration = Duration::from_secs(45 * 60);
70
71/// How long the checks may stay unreadable before landing gives up on them.
72///
73/// GitHub registers a workflow run some seconds after the branch is pushed, so
74/// immediately after a pull request is opened "no checks" and "no CI in this
75/// repository" look identical. Measured on run 01c2: magi opened pull request
76/// 22, read `unknown` four seconds later, refused to merge on a guess and
77/// marked the run blocked - and every check on that pull request was green
78/// minutes afterwards, with the whole competition then re-run from scratch for
79/// a task that was already finished. Three minutes is well past the observed
80/// registration delay and still bounded, so a repository that genuinely has no
81/// checks costs one three-minute wait and then says so.
82pub const CHECKS_GRACE: Duration = Duration::from_secs(3 * 60);
83
84/// Bytes of failing log kept per check. The fixer needs the assertion and the
85/// frame around it, not the forty thousand lines of `cargo` output above it.
86const LOG_TAIL: usize = 4_000;
87
88/// Failing checks whose logs are fetched. Beyond a handful the failures share a
89/// cause, and fetching each one costs a `gh` round trip.
90const MAX_LOGS: usize = 3;
91
92/// Marker carried by every comment magi posts on a pull request.
93///
94/// Without it magi's own "still failing" comment is indistinguishable from a
95/// reviewer's, and the next observation would hand magi's own prose to the
96/// fixer as a finding.
97pub const MARKER: &str = "<!-- magi:land -->";
98
99/// Markers a bot puts in a comment to say that the comment is not a review.
100///
101/// CodeRabbit labels its own machinery in HTML comments - the trigger notice,
102/// the walkthrough summary, the "thanks for using" footer - and its actual
103/// findings arrive as *inline* review comments with a path and a line. Taking
104/// the bot at its word is more honest than guessing from prose, and it is the
105/// difference between a fix round that has something to fix and one that asks
106/// an agent to act on a quota notice.
107const NOT_A_REVIEW: [&str; 3] = [
108    "skip review by coderabbit.ai",
109    "summarize by coderabbit.ai",
110    "<!-- tips_start -->",
111];
112
113/// Where a pull request is in its life.
114#[derive(Debug, Clone, Copy, PartialEq, Eq)]
115pub enum PrLifecycle {
116    /// Still ours to land.
117    Open,
118    /// Already merged, by us or by a person.
119    Merged,
120    /// Closed without merging.
121    Closed,
122}
123
124/// The aggregate verdict of a pull request's checks.
125#[derive(Debug, Clone, Copy, PartialEq, Eq)]
126pub enum Checks {
127    /// At least one check has not finished.
128    Pending,
129    /// Every check passed (a skipped check counts as passed: the review
130    /// workflow skips release and bot pull requests by design).
131    Green,
132    /// At least one check finished without passing.
133    Red,
134    /// Nothing readable - no rollup at all, or a status magi does not know.
135    Unknown,
136}
137
138impl PrLifecycle {
139    /// Stable lower-case name, as the API and the reports spell it.
140    pub fn as_str(self) -> &'static str {
141        match self {
142            Self::Open => "open",
143            Self::Merged => "merged",
144            Self::Closed => "closed",
145        }
146    }
147}
148
149impl Checks {
150    /// Stable lower-case name, as the API and the reports spell it.
151    pub fn as_str(self) -> &'static str {
152        match self {
153            Self::Pending => "pending",
154            Self::Green => "green",
155            Self::Red => "red",
156            Self::Unknown => "unknown",
157        }
158    }
159}
160
161/// One outstanding review comment, human or bot.
162#[derive(Debug, Clone, PartialEq, Eq)]
163pub struct ReviewComment {
164    /// Login of whoever wrote it.
165    pub author: String,
166    /// File it was left on, for inline review comments.
167    pub path: Option<String>,
168    /// Line it was left on, when the comment is inline and still anchored.
169    pub line: Option<u64>,
170    /// The comment itself, as written.
171    pub body: String,
172}
173
174/// One observation of a pull request.
175#[derive(Debug, Clone, PartialEq, Eq)]
176pub struct PrState {
177    /// Pull request url, as `gh` reports it.
178    pub url: String,
179    /// Pull request number.
180    pub number: u64,
181    /// Open, merged, or closed.
182    pub state: PrLifecycle,
183    /// Aggregate check verdict.
184    pub checks: Checks,
185    /// Names of the checks that finished without passing.
186    pub failing: Vec<String>,
187    /// Comments that still want an answer, human and bot.
188    pub review_comments: Vec<ReviewComment>,
189    /// Whether the forge itself considers the failures blocking.
190    pub blocking: Blocking,
191}
192
193/// Whether a failing check actually stands between the pull request and
194/// `main`, according to the forge.
195///
196/// The rollup lists every check equally, so `coverage` going red on a
197/// repository that deliberately does not require it looked exactly like a
198/// broken build - and magi answered by spending a fix round on a change that
199/// was fine. Pull request 37 had to be merged by hand for that reason: the
200/// only red check was `editorconfig`, which was failing because the *action*
201/// could not fetch its own binary, and which the repository does not require.
202///
203/// `mergeStateStatus` is where GitHub applies the required-check set, so it
204/// is the one field that can tell the difference.
205#[derive(Debug, Clone, Copy, PartialEq, Eq)]
206pub enum Blocking {
207    /// Required checks are satisfied and the branch merges cleanly.
208    No,
209    /// Something required is failing or missing.
210    Yes,
211    /// The branch no longer merges: the base moved under it.
212    Conflict,
213    /// The forge did not say - an older `gh`, or a token without the scope.
214    /// Treated as `Yes`, because refusing to guess is the rule everywhere
215    /// else in this module.
216    Unsaid,
217}
218
219impl Blocking {
220    /// Read `mergeStateStatus`, which is upper-case in `gh`'s output.
221    fn of(raw: &str) -> Self {
222        match raw.to_ascii_uppercase().as_str() {
223            // Mergeable. `UNSTABLE` is the interesting one: mergeable, with a
224            // non-required check failing or still running.
225            "CLEAN" | "UNSTABLE" | "HAS_HOOKS" => Self::No,
226            "DIRTY" => Self::Conflict,
227            "" | "UNKNOWN" => Self::Unsaid,
228            // BLOCKED, BEHIND, DRAFT: something has to change first.
229            _ => Self::Yes,
230        }
231    }
232
233    /// Does this stand between the pull request and the base branch?
234    #[must_use]
235    pub fn stops_a_merge(self) -> bool {
236        !matches!(self, Self::No)
237    }
238}
239
240/// What the loop decided to do next. Pure, so the policy is testable.
241#[derive(Debug, Clone, PartialEq, Eq)]
242pub enum Step {
243    /// Checks are still running; re-read the pull request after [`POLL`].
244    Wait,
245    /// The base moved and the branch no longer merges: rebase it.
246    ///
247    /// Not a fix round. Nothing is wrong with the change - a competition
248    /// that runs for two hours against a repository merging pull requests
249    /// all day conflicts on the way in, and that is arithmetic rather than a
250    /// defect. Pull requests 35 and 37 were both rebased by hand for exactly
251    /// this.
252    Rebase,
253    /// Red checks or unresolved comments; run a fix round.
254    Fix {
255        /// What is unhappy, in one line, for the run log and the fix prompt.
256        reason: String,
257    },
258    /// Green and nothing outstanding; merge it.
259    Merge,
260    /// The pull request left our hands.
261    Done {
262        /// Did it land, or was it closed?
263        merged: bool,
264    },
265    /// Stop and leave the pull request to a person.
266    GiveUp {
267        /// Why magi stopped, in one line.
268        reason: String,
269    },
270}
271
272/// The outcome to record when `gh pr merge` exits non-zero, given what the
273/// pull request looked like immediately afterwards.
274///
275/// `gh pr merge` merges server-side first and only then does local work -
276/// deleting the branch, switching back to a base branch - so a non-zero exit
277/// does not mean the merge did not happen. In a jj-colocated repository it
278/// reliably does not mean that: git HEAD is detached, and `--delete-branch`
279/// ends with "could not determine current branch: not on any branch" *after*
280/// the merge has landed. Run ec12 merged pull request 28 into `main` and
281/// recorded `ok: false`, and its task was held waiting for a merge that was
282/// already done.
283///
284/// So the forge is asked, and its answer wins - the same authority [`decide`]
285/// gives the pull request's own state over everything else. The recorded
286/// detail carries both facts, because "the command failed and the merge
287/// happened anyway" is exactly what someone reading the run later needs to
288/// know.
289///
290/// `None` means the merge really did not happen, including when the pull
291/// request could not be read at all: an unreadable answer is not evidence of
292/// success.
293fn merged_after_all(
294    argv: &[String],
295    stderr: &str,
296    after: Option<PrLifecycle>,
297) -> Option<MergeOutcome> {
298    if after? != PrLifecycle::Merged {
299        return None;
300    }
301    Some(MergeOutcome {
302        mode: MergeMode::Pr,
303        ok: true,
304        detail: format!(
305            "gh {} (the command reported `{}`, but the pull request is merged)",
306            argv.join(" "),
307            stderr.trim()
308        ),
309    })
310}
311
312/// Decide the next step. No I/O.
313///
314/// `round` counts the fix rounds already spent, so `round == budget` means the
315/// budget is gone. A wait never spends a round: waiting is free, and a slow CI
316/// must not consume the allowance meant for actual fixes.
317///
318/// The order of the tests is the policy:
319///
320/// 1. **The pull request's own state wins.** A merge or a close that happened
321///    underneath us is the end of the story regardless of what the checks say.
322/// 2. **Pending beats red.** A check that is still running may yet fail, and one
323///    fix round that addresses every failure is cheaper than two that each
324///    address half - the fix pushes and restarts the whole suite anyway.
325/// 3. **Comments outrank green.** An unresolved comment holds the merge even
326///    when CI is happy; that is what a review is for.
327/// 4. **Unreadable is not absent.** Checks that cannot be read yet are waited
328///    on for [`CHECKS_GRACE`], because a pull request opened a moment ago has
329///    not been given its workflow runs yet. Past the grace they are treated as
330///    genuinely missing and magi stops rather than merge on a guess.
331pub fn decide(pr: &PrState, round: usize, budget: usize, waited: Duration) -> Step {
332    match pr.state {
333        PrLifecycle::Merged => return Step::Done { merged: true },
334        PrLifecycle::Closed => return Step::Done { merged: false },
335        PrLifecycle::Open => {}
336    }
337
338    // Before the checks: every check on a branch that cannot land is an
339    // answer about a state that cannot land.
340    if pr.blocking == Blocking::Conflict {
341        return Step::Rebase;
342    }
343
344    let spent = round >= budget;
345    match pr.checks {
346        Checks::Pending => Step::Wait,
347        Checks::Unknown if waited < CHECKS_GRACE => Step::Wait,
348        Checks::Unknown => Step::GiveUp {
349            reason: format!(
350                "no check status is readable on the pull request after {} minute(s); \
351                 refusing to merge on a guess",
352                CHECKS_GRACE.as_secs() / 60
353            ),
354        },
355        // Red, but the forge says it does not stand in the way: the failing
356        // checks are ones this repository chose not to require. Spending a fix
357        // round on them asks an agent to repair something nobody is gating on
358        // - and pull request 37's only red check was an *action* that could
359        // not fetch its own binary. Merge, and name them so the record is
360        // honest about what was red when it landed.
361        Checks::Red if !pr.blocking.stops_a_merge() && pr.review_comments.is_empty() => Step::Merge,
362        Checks::Red => {
363            let what = format!(
364                "{} check(s) failing: {}",
365                pr.failing.len(),
366                pr.failing.join(", ")
367            );
368            if spent {
369                Step::GiveUp {
370                    reason: format!("{what} — still red after {budget} fix round(s)"),
371                }
372            } else {
373                Step::Fix { reason: what }
374            }
375        }
376        Checks::Green if pr.review_comments.is_empty() => Step::Merge,
377        Checks::Green => {
378            let what = format!(
379                "checks are green but {} review comment(s) are unresolved: {}",
380                pr.review_comments.len(),
381                authors(&pr.review_comments)
382            );
383            if spent {
384                Step::GiveUp {
385                    reason: format!("{what} — still unresolved after {budget} fix round(s)"),
386                }
387            } else {
388                Step::Fix { reason: what }
389            }
390        }
391    }
392}
393
394/// Distinct comment authors, in the order they first appear.
395fn authors(comments: &[ReviewComment]) -> String {
396    let mut seen: Vec<&str> = Vec::new();
397    for c in comments {
398        if !seen.contains(&c.author.as_str()) {
399            seen.push(&c.author);
400        }
401    }
402    seen.join(", ")
403}
404
405/// The argv magi merges with, minus the program name.
406///
407/// `--subject` is the point of this function existing: see the module docs.
408pub fn merge_argv(number: u64, subject: &str) -> Vec<String> {
409    vec![
410        "pr".to_owned(),
411        "merge".to_owned(),
412        number.to_string(),
413        "--squash".to_owned(),
414        "--delete-branch".to_owned(),
415        "--subject".to_owned(),
416        subject.to_owned(),
417    ]
418}
419
420/// The squash subject to merge under.
421///
422/// The pull request title, unless it is empty or is a candidate branch's commit
423/// subject that leaked into the title - in which case the task's own first line
424/// is used, because `magi: candidate A (uncommitted work)` in `main` tells a
425/// reader nothing about what landed.
426pub fn merge_subject(pr_title: &str, instruction: &str) -> String {
427    let title = pr_title.trim();
428    if !title.is_empty() && !title.starts_with("magi: candidate") {
429        return title.to_owned();
430    }
431    let first = instruction
432        .lines()
433        .map(str::trim)
434        .find(|l| !l.is_empty())
435        .unwrap_or("magi: land the winning candidate");
436    first.trim_start_matches(['#', ' ']).to_owned()
437}
438
439/// The choice that lets the merge happen, verbatim as the owner taps it.
440pub const APPROVE: &str = "merge";
441
442/// The choice that leaves the pull request open.
443pub const HOLD: &str = "hold";
444
445/// Graph node recorded on the approval question.
446///
447/// The phone keys its high-stakes card off this rather than off the choice
448/// strings, so renaming a button cannot silently downgrade the card that
449/// guards the one irreversible action magi takes.
450pub const APPROVAL_NODE: &str = "land-approval";
451
452/// Unified diff lines carried in the panel before it is truncated.
453///
454/// Four hundred: the panel is read on a 390px phone, where a diff line often
455/// wraps to two rows, so this is already a few thousand rows of scrolling -
456/// past that nobody is reading, and the bytes still count against the panel's
457/// 8 MiB cap. A larger diff is not hidden: the note says how many lines were
458/// cut and which worktree holds the whole patch.
459pub const DIFF_MAX_LINES: usize = 400;
460
461/// What the owner's answer to the approval question means.
462#[derive(Debug, Clone, Copy, PartialEq, Eq)]
463pub enum Approval {
464    /// The owner said [`APPROVE`]. Merge.
465    Merge,
466    /// Anything else, including silence. Leave the pull request open.
467    Hold,
468}
469
470/// Read the owner's answer, where `None` is an unanswered question.
471///
472/// Silence is a hold. A timed-out question means the owner never saw it or
473/// never decided, and defaulting an irreversible merge to "yes" would make this
474/// gate worse than no gate at all: it would merge unattended while claiming to
475/// have asked. Only the exact [`APPROVE`] choice merges, so an answer this
476/// function does not recognise holds too.
477pub fn approval(answer: Option<&str>) -> Approval {
478    match answer {
479        Some(a) if a.trim().eq_ignore_ascii_case(APPROVE) => Approval::Merge,
480        _ => Approval::Hold,
481    }
482}
483
484/// Escape text for HTML, including both quote characters.
485///
486/// Every string in the panel is agent-influenced: a branch name, a file path, a
487/// commit subject, a review comment. The sandboxed frame stops such text from
488/// *running*, but it does not stop a `<` from ending the document early or a
489/// `"` from ending an attribute and inventing a new one - the panel would then
490/// render a lie, or not render at all. Both quotes are escaped because the same
491/// function is used inside attributes, where remembering which quote style the
492/// caller used is one mistake away from an injected attribute.
493fn esc(s: &str) -> String {
494    let mut out = String::with_capacity(s.len());
495    for c in s.chars() {
496        match c {
497            '&' => out.push_str("&amp;"),
498            '<' => out.push_str("&lt;"),
499            '>' => out.push_str("&gt;"),
500            '"' => out.push_str("&quot;"),
501            '\'' => out.push_str("&#39;"),
502            _ => out.push(c),
503        }
504    }
505    out
506}
507
508/// One row of the diffstat table.
509#[derive(Debug, Clone, PartialEq, Eq)]
510struct StatRow {
511    path: String,
512    /// `None` for a binary file, which `git` reports as `-`.
513    added: Option<u64>,
514    removed: Option<u64>,
515}
516
517impl StatRow {
518    /// Lines touched, for sorting. A binary file counts as zero rather than as
519    /// unknown, which puts it at the bottom where it needs no attention.
520    fn churn(&self) -> u64 {
521        self.added.unwrap_or(0) + self.removed.unwrap_or(0)
522    }
523}
524
525/// Parse `git diff --numstat` into rows, biggest churn first.
526///
527/// `--numstat` and not `--stat`: the `+++---` bar in `--stat` is *scaled* to the
528/// terminal width, so counting its characters would print fabricated numbers in
529/// the one table an operator approves an irreversible action from.
530fn parse_numstat(numstat: &str) -> Vec<StatRow> {
531    let mut rows: Vec<StatRow> = numstat
532        .lines()
533        .filter_map(|line| {
534            let mut parts = line.splitn(3, '\t');
535            let added = parts.next()?.trim();
536            let removed = parts.next()?.trim();
537            let path = parts.next()?.trim();
538            if path.is_empty() {
539                return None;
540            }
541            Some(StatRow {
542                path: path.to_owned(),
543                added: added.parse().ok(),
544                removed: removed.parse().ok(),
545            })
546        })
547        .collect();
548    // Path breaks the tie so the same change always renders the same table; an
549    // operator comparing two panels should not see rows shuffle.
550    rows.sort_by(|a, b| b.churn().cmp(&a.churn()).then_with(|| a.path.cmp(&b.path)));
551    rows
552}
553
554/// How one diff line is shown: a gutter character, a style, and the body to
555/// print - which is the line minus its marker, so the marker appears exactly
556/// once, in the gutter.
557///
558/// The gutter is why this exists at all. The operator may be colour blind, or
559/// reading in sunlight with the screen dimmed, so an added line is never
560/// distinguished by its background alone: `+` and `-` sit in a fixed column,
561/// the same mark they already read in a terminal.
562fn diff_row(line: &str) -> (&'static str, &'static str, &str) {
563    if line.starts_with("+++") || line.starts_with("---") {
564        (" ", "color:#57606a;font-weight:600", line)
565    } else if let Some(body) = line.strip_prefix('+') {
566        ("+", "background:#e6ffec;color:#0a3622", body)
567    } else if let Some(body) = line.strip_prefix('-') {
568        ("-", "background:#ffebe9;color:#5c1a17", body)
569    } else if line.starts_with("@@") {
570        ("~", "background:#eef2ff;color:#3730a3", line)
571    } else if let Some(body) = line.strip_prefix(' ') {
572        (" ", "", body)
573    } else {
574        (" ", "color:#57606a;font-weight:600", line)
575    }
576}
577
578/// The handful of words the approval panel says in its own voice.
579///
580/// magi's own text, not an agent's, so `[graph] language` has to reach it too:
581/// the operator asked why the merge question spoke English on a repository
582/// configured for Japanese, and "because that string is a literal in Rust" is
583/// not an answer. Only the languages magi can actually check are translated;
584/// anything else falls back to English rather than shipping a guess, and that
585/// fallback is deliberate.
586struct Words {
587    html_lang: &'static str,
588    checks: &'static str,
589    nothing_failing: &'static str,
590    files_changed: &'static str,
591    commits: &'static str,
592    no_commits: &'static str,
593    comments: &'static str,
594    no_comments: &'static str,
595    diff: &'static str,
596    truncated: &'static str,
597    lands_as: &'static str,
598}
599
600const EN: Words = Words {
601    html_lang: "en",
602    checks: "Checks",
603    nothing_failing: "Nothing failing.",
604    files_changed: "file(s) changed",
605    commits: "Commits being squashed",
606    no_commits: "No commit subjects could be read from the branch.",
607    comments: "Review comments",
608    no_comments: "Nothing outstanding at this observation.",
609    diff: "Diff",
610    truncated: "Truncated",
611    lands_as: "They land as one commit titled",
612};
613
614const JA: Words = Words {
615    html_lang: "ja",
616    checks: "チェック",
617    nothing_failing: "失敗しているものはありません。",
618    files_changed: "ファイル変更",
619    commits: "squash されるコミット",
620    no_commits: "ブランチからコミット件名を読めませんでした。",
621    comments: "レビューコメント",
622    no_comments: "この時点で未対応のものはありません。",
623    diff: "差分",
624    truncated: "省略",
625    lands_as: "これらは次の件名の1コミットとして入ります:",
626};
627
628impl Words {
629    /// The clause after the merge subject. Split out because word order moves:
630    /// Japanese puts the subject before the verb, so a shared template with a
631    /// hole in the middle would read as machine translation.
632    fn lands_as_tail(&self) -> &'static str {
633        if self.html_lang == "ja" {
634            "。この件名も承認の対象です。"
635        } else {
636            ", which you are approving too."
637        }
638    }
639
640    /// The question's own one-line summary, which is what a phone shows first.
641    fn approval_summary(&self, number: u64, subject: &str) -> String {
642        if self.html_lang == "ja" {
643            format!("プルリクエスト #{number} をマージ: {subject}")
644        } else {
645            format!("merge pull request #{number}: {subject}")
646        }
647    }
648
649    /// The body under the summary, above the panel.
650    fn approval_detail(&self, url: &str, base: &str, subject: &str) -> String {
651        if self.html_lang == "ja" {
652            format!(
653                "{url} はチェックが緑で、`{base}` へ `{subject}` として squash \
654                 できる状態です。差分の要約・パッチ・squash されるコミットは\
655                 下のパネルにあります。"
656            )
657        } else {
658            format!(
659                "{url} is green and ready to squash into `{base}` as `{subject}`. \
660                 The panel holds the diffstat, the patch and the commits being squashed."
661            )
662        }
663    }
664
665    /// The truncation note, written whole in each language for the same reason.
666    fn truncated_note(
667        &self,
668        omitted: usize,
669        total: usize,
670        shown: usize,
671        where_: &str,
672        base: &str,
673        head: &str,
674    ) -> String {
675        if self.html_lang == "ja" {
676            format!(
677                "先頭 {shown} 行のあと、差分 {total} 行のうち {omitted} 行を省略しました。\
678                 全体は <code>{where_}</code>(<code>git diff {base}...{head}</code>)と\
679                 プルリクエストにあります。"
680            )
681        } else {
682            format!(
683                "{omitted} of {total} diff lines omitted after the first {shown}. \
684                 The whole patch is in <code>{where_}</code> \
685                 (<code>git diff {base}...{head}</code>) and on the pull request."
686            )
687        }
688    }
689}
690
691/// Pick the panel's language. Codes and names both, because `[graph] language`
692/// has always accepted either.
693fn words(language: &str) -> &'static Words {
694    let l = language.trim();
695    if l.eq_ignore_ascii_case("ja")
696        || l.eq_ignore_ascii_case("jp")
697        || l.eq_ignore_ascii_case("japanese")
698        || l.eq_ignore_ascii_case("日本語")
699    {
700        &JA
701    } else {
702        &EN
703    }
704}
705
706/// The approval panel's html: what is about to land, and the evidence for it.
707///
708/// Pure, so the whole document is asserted in tests without `gh`, without a
709/// network and without a repository. The caller gathers `diffstat`
710/// (`git diff --numstat`), `diff` (the unified patch), `commits` (the subjects
711/// being squashed) and `subject` (what the squash will be called) from the
712/// winner's worktree.
713///
714/// It emits no `<script>`, no `<form>` and no remote url, because the frame's
715/// content security policy blocks all three: anything of the sort here would be
716/// dead markup that misleads the next reader into thinking it works.
717pub fn approval_panel(
718    state: &RunState,
719    pr: &PrState,
720    diffstat: &str,
721    diff: &str,
722    commits: &[String],
723    subject: &str,
724) -> String {
725    let rows = parse_numstat(diffstat);
726    let w = words(&state.config.graph.language);
727    let mut h = String::with_capacity(4_096 + diff.len().min(200_000));
728
729    let _ = writeln!(
730        h,
731        "<!doctype html>\n<html lang=\"{}\">\n<head>\n<meta charset=\"utf-8\">\n\
732         <meta name=\"viewport\" content=\"width=device-width, initial-scale=1\">",
733        w.html_lang
734    );
735    let _ = writeln!(
736        h,
737        "<title>merge #{} — {}</title>\n</head>",
738        pr.number,
739        esc(subject)
740    );
741    h.push_str(
742        "<body style=\"margin:0;padding:12px;font:15px/1.5 -apple-system,\
743         'Segoe UI',system-ui,sans-serif;color:#1f2328;background:#fff;\
744         word-break:break-word\">\n",
745    );
746
747    // The decision, in the words the operator is approving.
748    let _ = writeln!(
749        h,
750        "<h1 style=\"margin:0 0 4px;font-size:19px\">Merge #{} into \
751         <code style=\"background:#f6f8fa;padding:1px 4px;border-radius:4px\">{}</code></h1>\n\
752         <p style=\"margin:0 0 4px;font-size:17px;font-weight:600\">{}</p>\n\
753         <p style=\"margin:0 0 12px;font-size:13px;color:#57606a\">squash merge · run {} · \
754         <a href=\"{}\" style=\"color:#0969da\">{}</a></p>",
755        pr.number,
756        esc(&state.base_branch),
757        esc(subject),
758        esc(&state.id),
759        esc(&pr.url),
760        esc(&pr.url),
761    );
762
763    let _ = writeln!(
764        h,
765        "<h2 style=\"margin:16px 0 6px;font-size:15px\">{}: {}</h2>",
766        w.checks,
767        esc(pr.checks.as_str())
768    );
769    if pr.failing.is_empty() {
770        let _ = writeln!(
771            h,
772            "<p style=\"margin:0;font-size:13px;color:#57606a\">{}</p>",
773            w.nothing_failing
774        );
775    } else {
776        h.push_str("<ul style=\"margin:0;padding-left:20px;font-size:13px\">\n");
777        for f in &pr.failing {
778            let _ = writeln!(h, "<li>{}</li>", esc(f));
779        }
780        h.push_str("</ul>\n");
781    }
782
783    // Diffstat as a real table, so a phone reads what moved without scrolling
784    // sideways through a terminal bar chart.
785    let _ = writeln!(
786        h,
787        "<h2 style=\"margin:16px 0 6px;font-size:15px\">{} {}</h2>",
788        rows.len(),
789        w.files_changed
790    );
791    h.push_str(
792        "<table style=\"width:100%;border-collapse:collapse;font-size:13px\">\n\
793         <thead><tr>\
794         <th style=\"text-align:left;border-bottom:1px solid #d0d7de;padding:4px 2px\">file</th>\
795         <th style=\"text-align:right;border-bottom:1px solid #d0d7de;padding:4px 2px\">added</th>\
796         <th style=\"text-align:right;border-bottom:1px solid #d0d7de;padding:4px 2px\">removed\
797         </th></tr></thead>\n<tbody>\n",
798    );
799    let mut total_added = 0u64;
800    let mut total_removed = 0u64;
801    for r in &rows {
802        total_added += r.added.unwrap_or(0);
803        total_removed += r.removed.unwrap_or(0);
804        let cell = |n: Option<u64>| match n {
805            Some(n) => n.to_string(),
806            None => "bin".to_owned(),
807        };
808        let _ = writeln!(
809            h,
810            "<tr>\
811             <td style=\"padding:4px 2px;border-bottom:1px solid #eaeef2;\
812             font-family:ui-monospace,monospace\">{}</td>\
813             <td style=\"padding:4px 2px;border-bottom:1px solid #eaeef2;text-align:right;\
814             color:#0a3622\">{}</td>\
815             <td style=\"padding:4px 2px;border-bottom:1px solid #eaeef2;text-align:right;\
816             color:#5c1a17\">{}</td></tr>",
817            esc(&r.path),
818            cell(r.added),
819            cell(r.removed),
820        );
821    }
822    let _ = writeln!(
823        h,
824        "</tbody>\n<tfoot><tr style=\"font-weight:600\">\
825         <td style=\"padding:4px 2px\">total</td>\
826         <td style=\"padding:4px 2px;text-align:right\">{total_added}</td>\
827         <td style=\"padding:4px 2px;text-align:right\">{total_removed}</td>\
828         </tr></tfoot>\n</table>"
829    );
830
831    // The commits being squashed, and the subject that replaces them.
832    let _ = writeln!(
833        h,
834        "<h2 style=\"margin:16px 0 6px;font-size:15px\">{}</h2>",
835        w.commits
836    );
837    if commits.is_empty() {
838        h.push_str(&format!(
839            "<p style=\"margin:0;font-size:13px;color:#57606a\">{}</p>\n",
840            w.no_commits
841        ));
842    } else {
843        h.push_str("<ol style=\"margin:0;padding-left:20px;font-size:13px\">\n");
844        for c in commits {
845            let _ = writeln!(h, "<li>{}</li>", esc(c));
846        }
847        h.push_str("</ol>\n");
848    }
849    let _ = writeln!(
850        h,
851        "<p style=\"margin:8px 0 0;font-size:13px\">{} <strong>{}</strong>{}</p>",
852        w.lands_as,
853        esc(subject),
854        w.lands_as_tail()
855    );
856
857    // The review comments that shaped this branch, and who asked for them.
858    let _ = writeln!(
859        h,
860        "<h2 style=\"margin:16px 0 6px;font-size:15px\">{}</h2>",
861        w.comments
862    );
863    if pr.review_comments.is_empty() {
864        h.push_str(&format!(
865            "<p style=\"margin:0;font-size:13px;color:#57606a\">{}</p>\n",
866            w.no_comments
867        ));
868    } else {
869        for c in &pr.review_comments {
870            let anchor = match (&c.path, c.line) {
871                (Some(p), Some(l)) => format!("{p}:{l}"),
872                (Some(p), None) => p.clone(),
873                _ => "pull request thread".to_owned(),
874            };
875            let _ = writeln!(
876                h,
877                "<div style=\"margin:0 0 8px;padding:8px;background:#f6f8fa;border-radius:6px\">\
878                 <div style=\"font-size:12px;color:#57606a\">{} · {}</div>\
879                 <div style=\"white-space:pre-wrap;font-size:13px\">{}</div></div>",
880                esc(&c.author),
881                esc(&anchor),
882                esc(&tail(&c.body, 800)),
883            );
884        }
885    }
886
887    // The patch itself.
888    let total = diff.lines().count();
889    let shown = total.min(DIFF_MAX_LINES);
890    let _ = writeln!(
891        h,
892        "<h2 style=\"margin:16px 0 6px;font-size:15px\">{}</h2>",
893        w.diff
894    );
895    h.push_str(
896        "<div style=\"font:12px/1.45 ui-monospace,SFMono-Regular,Menlo,monospace;\
897         border:1px solid #d0d7de;border-radius:6px;overflow-x:auto\">\n",
898    );
899    for line in diff.lines().take(shown) {
900        let (gutter, style, body) = diff_row(line);
901        let _ = writeln!(
902            h,
903            "<div style=\"display:flex;{style}\">\
904             <span style=\"flex:0 0 1.4em;text-align:center;user-select:none;\
905             border-right:1px solid #d0d7de\">{gutter}</span>\
906             <span style=\"white-space:pre;padding-left:6px\">{}</span></div>",
907            esc(body),
908        );
909    }
910    h.push_str("</div>\n");
911    if total > shown {
912        let omitted = total - shown;
913        let head = state.winner().map_or("HEAD", |w| w.branch.as_str());
914        let where_ = state.winner().map_or_else(
915            || state.repo.display().to_string(),
916            |w| w.worktree.display().to_string(),
917        );
918        let _ = writeln!(
919            h,
920            "<p style=\"margin:8px 0 0;padding:8px;background:#fff8c5;border-radius:6px;\
921             font-size:13px\">{}: {}</p>",
922            w.truncated,
923            w.truncated_note(
924                omitted,
925                total,
926                shown,
927                &esc(&where_),
928                &esc(&state.base_branch),
929                &esc(head),
930            ),
931        );
932    }
933
934    h.push_str("</body>\n</html>\n");
935    h
936}
937
938/// Ask the owner before merging, with the whole case attached as a panel.
939///
940/// The evidence is gathered from the winner's own worktree with the `git` CLI,
941/// never from the network, so a phone on a slow link gets the diff magi is
942/// looking at rather than a link it has to go and open.
943async fn request_approval(state: &mut RunState, pr: &PrState, subject: &str) -> Result<Approval> {
944    let (worktree, head) = match state.winner() {
945        Some(w) => (w.worktree.clone(), w.branch.clone()),
946        None => (state.repo.clone(), "HEAD".to_owned()),
947    };
948    let base = state.base_branch.clone();
949    let range = format!("{base}...{head}");
950    // A failed `git` must not decide the merge: the panel degrades to less
951    // evidence and the owner still chooses. Merging because the diff could not
952    // be read would be the worst of both.
953    let numstat = git::git_raw(&worktree, &["diff", "--numstat", "-M", &range])
954        .await
955        .map(|o| o.stdout)
956        .unwrap_or_default();
957    let diff = git::diff(&worktree, &base, &head).await.unwrap_or_default();
958    let commits: Vec<String> = git::git_raw(
959        &worktree,
960        &[
961            "log",
962            "--reverse",
963            "--format=%s",
964            &format!("{base}..{head}"),
965        ],
966    )
967    .await
968    .map(|o| o.stdout)
969    .unwrap_or_default()
970    .lines()
971    .filter(|l| !l.trim().is_empty())
972    .map(str::to_owned)
973    .collect();
974
975    let w = words(&state.config.graph.language);
976    let html = approval_panel(state, pr, &numstat, &diff, &commits, subject);
977    let store = ask::Questions::open();
978    let mut q = ask::Question::new(
979        state.id.clone(),
980        APPROVAL_NODE.to_owned(),
981        "land".to_owned(),
982        w.approval_summary(pr.number, subject),
983        w.approval_detail(&pr.url, &base, subject),
984        vec![APPROVE.to_owned(), HOLD.to_owned()],
985    );
986    store
987        .put_panel(&mut q, &html, &[])
988        .context("write the merge approval panel")?;
989    state.event("land", format!("asking for merge approval ({})", q.short()));
990    state.save()?;
991
992    let timeout = Duration::from_secs(state.config.graph.answer_timeout);
993    let said = ask::ask_and_wait(&mut q, &store, &state.config.notify, timeout).await?;
994    Ok(approval(said.as_deref()))
995}
996
997/// Parse `gh pr view --json url,number,state,statusCheckRollup,reviews,comments`
998/// output into a [`PrState`]. No I/O.
999pub fn parse_pr(json: &str) -> Result<PrState> {
1000    let raw: GhPr = serde_json::from_str(json).context("parse `gh pr view --json ...` output")?;
1001    let state = match raw.state.to_ascii_uppercase().as_str() {
1002        "OPEN" => PrLifecycle::Open,
1003        "MERGED" => PrLifecycle::Merged,
1004        "CLOSED" => PrLifecycle::Closed,
1005        other => bail!("unknown pull request state `{other}`"),
1006    };
1007
1008    let mut failing = Vec::new();
1009    let mut pending = false;
1010    let mut unknown = false;
1011    for check in &raw.status_check_rollup {
1012        match check.verdict() {
1013            Verdict::Pass => {}
1014            Verdict::Pending => pending = true,
1015            Verdict::Fail => failing.push(check.label()),
1016            Verdict::Unknown => unknown = true,
1017        }
1018    }
1019    let checks = if raw.status_check_rollup.is_empty() {
1020        Checks::Unknown
1021    } else if pending {
1022        Checks::Pending
1023    } else if !failing.is_empty() {
1024        Checks::Red
1025    } else if unknown {
1026        Checks::Unknown
1027    } else {
1028        Checks::Green
1029    };
1030
1031    let mut review_comments = Vec::new();
1032    for r in raw.reviews {
1033        push_if_outstanding(
1034            &mut review_comments,
1035            ReviewComment {
1036                author: r.author.login,
1037                path: None,
1038                line: None,
1039                body: r.body,
1040            },
1041        );
1042    }
1043    for c in raw.comments {
1044        push_if_outstanding(
1045            &mut review_comments,
1046            ReviewComment {
1047                author: c.author.login,
1048                path: None,
1049                line: None,
1050                body: c.body,
1051            },
1052        );
1053    }
1054
1055    Ok(PrState {
1056        url: raw.url,
1057        number: raw.number,
1058        state,
1059        checks,
1060        failing,
1061        review_comments,
1062        blocking: Blocking::of(&raw.merge_state_status),
1063    })
1064}
1065
1066/// Parse `gh api repos/{owner}/{repo}/pulls/<n>/comments` into inline review
1067/// comments. No I/O.
1068///
1069/// `gh pr view` does not surface inline comments, and inline is exactly where
1070/// both review bots put their findings - a landing loop that read only the
1071/// top-level thread would never see the thing it is supposed to fix.
1072pub fn parse_inline_comments(json: &str) -> Result<Vec<ReviewComment>> {
1073    let raw: Vec<GhInline> =
1074        serde_json::from_str(json).context("parse `gh api .../pulls/<n>/comments` output")?;
1075    let mut out = Vec::new();
1076    for c in raw {
1077        push_if_outstanding(
1078            &mut out,
1079            ReviewComment {
1080                author: c.user.login,
1081                path: c.path,
1082                line: c.line,
1083                body: c.body,
1084            },
1085        );
1086    }
1087    Ok(out)
1088}
1089
1090/// Keep a comment only when it asks for something.
1091///
1092/// An inline comment always does: it names a file and a line. A top-level
1093/// comment is dropped when it is empty, when it is magi's own, or when it is
1094/// [noise](is_noise).
1095fn push_if_outstanding(out: &mut Vec<ReviewComment>, comment: ReviewComment) {
1096    if comment.body.trim().is_empty() || comment.body.contains(MARKER) {
1097        return;
1098    }
1099    if comment.path.is_none() && is_noise(&comment.body) {
1100        return;
1101    }
1102    out.push(comment);
1103}
1104
1105/// Is this comment body machinery rather than a finding?
1106///
1107/// Two tests, both structural, because guessing from prose is how a "looks
1108/// good to me" turns into a fix round:
1109///
1110/// 1. The bot said so - the body carries one of the [`NOT_A_REVIEW`] markers
1111///    with which CodeRabbit labels its trigger notice, its walkthrough, and its
1112///    footer.
1113/// 2. It asks for nothing - once HTML comments, `<details>` blocks, headings,
1114///    horizontal rules, and the bot's own status banner are removed, every
1115///    remaining line is a task-list item. That is exactly the shape of the
1116///    comment the Claude review job posts while it is still working.
1117///
1118/// Anything else is input, including bot prose. A bot that writes a paragraph
1119/// has said something, and the fix prompt tells the fixer it may decline a
1120/// comment with an argument - a wasted sentence in a prompt is cheaper than a
1121/// missed finding.
1122pub fn is_noise(body: &str) -> bool {
1123    if NOT_A_REVIEW.iter().any(|m| body.contains(m)) {
1124        return true;
1125    }
1126    let mut content = false;
1127    for line in strip_blocks(body).lines() {
1128        let line = unquote(line);
1129        if line.is_empty() || is_checklist(line) || is_decoration(line) || is_banner(line) {
1130            continue;
1131        }
1132        content = true;
1133        break;
1134    }
1135    !content
1136}
1137
1138/// Remove HTML comments and collapsed `<details>` blocks.
1139fn strip_blocks(body: &str) -> String {
1140    let mut out = String::with_capacity(body.len());
1141    let mut rest = body;
1142    loop {
1143        let open = ["<!--", "<details>"]
1144            .iter()
1145            .filter_map(|tag| rest.find(tag).map(|i| (i, *tag)))
1146            .min_by_key(|(i, _)| *i);
1147        let Some((at, tag)) = open else {
1148            out.push_str(rest);
1149            return out;
1150        };
1151        out.push_str(&rest[..at]);
1152        let after = &rest[at + tag.len()..];
1153        let close = if tag == "<!--" { "-->" } else { "</details>" };
1154        match after.find(close) {
1155            Some(end) => rest = &after[end + close.len()..],
1156            // Unterminated: the rest of the body is inside the block.
1157            None => return out,
1158        }
1159    }
1160}
1161
1162/// Strip blockquote markers, which both bots wrap their callouts in.
1163fn unquote(line: &str) -> &str {
1164    let mut s = line.trim();
1165    while let Some(rest) = s.strip_prefix('>') {
1166        s = rest.trim_start();
1167    }
1168    s.trim()
1169}
1170
1171/// `- [ ]` / `- [x]`, in any of the bullet styles GitHub renders.
1172fn is_checklist(line: &str) -> bool {
1173    let rest = line
1174        .strip_prefix("- ")
1175        .or_else(|| line.strip_prefix("* "))
1176        .unwrap_or("");
1177    let rest = rest.trim_start();
1178    matches!(
1179        rest.get(..3),
1180        Some("[ ]") | Some("[x]") | Some("[X]") | Some("[*]")
1181    )
1182}
1183
1184/// A heading, a horizontal rule, or a callout tag - shape, never content.
1185fn is_decoration(line: &str) -> bool {
1186    line.starts_with('#')
1187        || line.starts_with("[!")
1188        || (line.len() >= 3 && line.chars().all(|c| matches!(c, '-' | '=' | '*' | '_')))
1189}
1190
1191/// A line that is nothing but emphasis and links.
1192///
1193/// Both review jobs open with a status banner
1194/// (`**Claude finished ... in 4m 14s** —— [View job](url)`). It reads as prose
1195/// to a line-based test and asks for nothing, so it is measured the same way a
1196/// heading is: strip the markup, and if no word survives, it was decoration.
1197fn is_banner(line: &str) -> bool {
1198    let plain = drop_spans(line, "**", "**");
1199    let plain = if plain.contains("](") {
1200        drop_spans(&plain, "[", ")")
1201    } else {
1202        plain
1203    };
1204    !plain.chars().any(char::is_alphanumeric)
1205}
1206
1207/// Remove every `open` .. `close` span, including the delimiters. An
1208/// unterminated span swallows the rest of the input, which is what a reader
1209/// sees too.
1210fn drop_spans(s: &str, open: &str, close: &str) -> String {
1211    let mut out = String::with_capacity(s.len());
1212    let mut rest = s;
1213    while let Some(at) = rest.find(open) {
1214        out.push_str(&rest[..at]);
1215        let after = &rest[at + open.len()..];
1216        match after.find(close) {
1217            Some(end) => rest = &after[end + close.len()..],
1218            None => return out,
1219        }
1220    }
1221    out.push_str(rest);
1222    out
1223}
1224
1225/// Run the loop against a real pull request until it merges or the budget runs
1226/// out.
1227///
1228/// The caller decides whether landing happens at all: this is only reached when
1229/// `graph.land` is on. Returns the last observation, so the caller can report
1230/// what magi was looking at when it stopped.
1231pub async fn land(state: &mut RunState, pr_url: &str) -> Result<PrState> {
1232    let repo = state.repo.clone();
1233    let budget = state.config.graph.land_rounds;
1234    let mut round = 0usize;
1235    // Counted apart from `round`: a rebase is not a fix, and a base that
1236    // moved is not the change's fault.
1237    let mut rebases = 0usize;
1238    let mut waited = Duration::ZERO;
1239    // Comment bodies the fixer has already been shown. A comment is
1240    // outstanding until it has been handed over once; after that it is a
1241    // recorded decision, not an open question, and re-feeding it would loop the
1242    // budget away on a comment the fixer already declined with an argument.
1243    let mut shown: BTreeSet<String> = BTreeSet::new();
1244
1245    state.event("land", format!("watching {pr_url}"));
1246    state.save()?;
1247
1248    loop {
1249        let seen = observe(&repo, pr_url).await?;
1250        let mut pr = seen.pr;
1251        pr.review_comments.retain(|c| !shown.contains(&c.body));
1252        state.pr = Some(crate::run::PrRecord {
1253            url: pr.url.clone(),
1254            number: pr.number,
1255            state: pr.state.as_str().to_owned(),
1256            checks: pr.checks.as_str().to_owned(),
1257            round,
1258            rounds: budget,
1259        });
1260        state.save()?;
1261
1262        match decide(&pr, round, budget, waited) {
1263            Step::Wait => {
1264                if waited >= WAIT_CEILING {
1265                    let why = format!(
1266                        "checks were still running after {} minutes",
1267                        WAIT_CEILING.as_secs() / 60
1268                    );
1269                    stop(state, &repo, &pr, &why).await?;
1270                    return Ok(pr);
1271                }
1272                waited += POLL;
1273                tokio::time::sleep(POLL).await;
1274            }
1275            Step::Done { merged } => {
1276                state.status = if merged {
1277                    RunStatus::Merged
1278                } else {
1279                    RunStatus::Ready
1280                };
1281                let detail = if merged {
1282                    format!("{} was merged", pr.url)
1283                } else {
1284                    format!("{} was closed without merging", pr.url)
1285                };
1286                state.merge = Some(MergeOutcome {
1287                    mode: MergeMode::Pr,
1288                    ok: merged,
1289                    detail: detail.clone(),
1290                });
1291                state.event("land", detail);
1292                state.save()?;
1293                return Ok(pr);
1294            }
1295            Step::Merge => {
1296                let subject = merge_subject(&seen.title, &state.instruction);
1297                // The owner sees the panel before the one irreversible step,
1298                // and an unanswered question is a hold: silence never merges.
1299                if state.config.graph.land_approval
1300                    && request_approval(state, &pr, &subject).await? == Approval::Hold
1301                {
1302                    stop(
1303                        state,
1304                        &repo,
1305                        &pr,
1306                        "the owner did not approve the merge (held or unanswered)",
1307                    )
1308                    .await?;
1309                    return Ok(pr);
1310                }
1311                let argv = merge_argv(pr.number, &subject);
1312                let out = gh(&repo, &argv).await?;
1313                if out.0 {
1314                    state.status = RunStatus::Merged;
1315                    state.merge = Some(MergeOutcome {
1316                        mode: MergeMode::Pr,
1317                        ok: true,
1318                        detail: format!("gh {}", argv.join(" ")),
1319                    });
1320                    state.event("land", format!("merged {} as `{subject}`", pr.url));
1321                    state.save()?;
1322                    pr.state = PrLifecycle::Merged;
1323                    return Ok(pr);
1324                }
1325                let after = observe(&repo, pr_url).await.ok().map(|s| s.pr.state);
1326                if let Some(outcome) = merged_after_all(&argv, &out.1, after) {
1327                    state.status = RunStatus::Merged;
1328                    state.merge = Some(outcome);
1329                    state.event("land", format!("merged {} as `{subject}`", pr.url));
1330                    state.save()?;
1331                    pr.state = PrLifecycle::Merged;
1332                    return Ok(pr);
1333                }
1334                stop(
1335                    state,
1336                    &repo,
1337                    &pr,
1338                    &format!("`gh pr merge` failed: {}", out.1),
1339                )
1340                .await?;
1341                return Ok(pr);
1342            }
1343            Step::Rebase => {
1344                // Bounded by the same budget as a fix, because a rebase that
1345                // keeps being needed means the base moves faster than this
1346                // run can land and a person should decide what to do. It
1347                // spends none of that budget: the change is not what is
1348                // wrong.
1349                if rebases >= budget {
1350                    let why = format!(
1351                        "the base moved under this branch {budget} time(s) and it still does \
1352                         not merge; rebasing again would only race it"
1353                    );
1354                    stop(state, &repo, &pr, &why).await?;
1355                    return Ok(pr);
1356                }
1357                rebases += 1;
1358                let Some(branch) = state.winner().map(|w| w.branch.clone()) else {
1359                    stop(
1360                        state,
1361                        &repo,
1362                        &pr,
1363                        "the pull request conflicts and this run has no winning branch to rebase",
1364                    )
1365                    .await?;
1366                    return Ok(pr);
1367                };
1368                let base = state.base_branch.clone();
1369                state.event(
1370                    "land",
1371                    format!("{} no longer merges; rebasing onto {base}", pr.url),
1372                );
1373                state.save()?;
1374
1375                // Onto the base as the *remote* has it: the local ref may be
1376                // behind, and rebasing onto a stale base produces a branch
1377                // that conflicts all over again.
1378                git::fetch(&repo, "origin", &base).await.ok();
1379                let scratch = state.dir().join("rebase");
1380                let onto = format!("origin/{base}");
1381                match git::rebase_branch_in_temp(&repo, &scratch, &branch, &onto).await {
1382                    Ok(None) => {
1383                        let pushed = git::push_rewritten(&repo, "origin", &branch).await?;
1384                        if !pushed.ok() {
1385                            let why = format!(
1386                                "rebased {branch} but could not push it: {}",
1387                                pushed.stderr.trim()
1388                            );
1389                            stop(state, &repo, &pr, &why).await?;
1390                            return Ok(pr);
1391                        }
1392                        state.event("land", format!("rebased {branch} onto {base}"));
1393                        state.save()?;
1394                        // The forge has to re-run its checks against the
1395                        // rebased head before anything else can be decided.
1396                        waited = Duration::ZERO;
1397                        tokio::time::sleep(POLL).await;
1398                    }
1399                    // A conflict is a decision, not a chore.
1400                    Ok(Some(conflict)) => {
1401                        let why = format!(
1402                            "{} conflicts with {base} and the rebase did not apply: {}",
1403                            pr.url,
1404                            conflict.chars().take(600).collect::<String>()
1405                        );
1406                        stop(state, &repo, &pr, &why).await?;
1407                        return Ok(pr);
1408                    }
1409                    Err(e) => {
1410                        let why = format!("could not rebase {branch} onto {base}: {e:#}");
1411                        stop(state, &repo, &pr, &why).await?;
1412                        return Ok(pr);
1413                    }
1414                }
1415            }
1416            Step::GiveUp { reason } => {
1417                stop(state, &repo, &pr, &reason).await?;
1418                return Ok(pr);
1419            }
1420            Step::Fix { reason } => {
1421                round += 1;
1422                waited = Duration::ZERO;
1423                for c in &pr.review_comments {
1424                    shown.insert(c.body.clone());
1425                }
1426                state.event("land", format!("round {round}: {reason}"));
1427                state.save()?;
1428
1429                let logs = failing_logs(&repo, &seen.failing_urls).await;
1430                let was_red = pr.checks == Checks::Red;
1431                match fix_round(state, &pr, round, budget, &reason, &logs).await? {
1432                    Fixed::Committed => {}
1433                    Fixed::Declined if was_red => {
1434                        let why = format!(
1435                            "the fixer produced no commit while {} check(s) were failing; \
1436                             stopping instead of looping on an unchanged tree",
1437                            pr.failing.len()
1438                        );
1439                        stop(state, &repo, &pr, &why).await?;
1440                        return Ok(pr);
1441                    }
1442                    // Comment-driven round with no commit: the fixer read the
1443                    // comments and changed nothing, which is a decision it is
1444                    // allowed to make. The comments are recorded as shown, so
1445                    // the next observation sees a clean pull request.
1446                    Fixed::Declined => state.event(
1447                        "land",
1448                        format!("round {round}: fixer declined the comments, nothing committed"),
1449                    ),
1450                    Fixed::Failed(why) => {
1451                        stop(state, &repo, &pr, &format!("the fix round failed: {why}")).await?;
1452                        return Ok(pr);
1453                    }
1454                }
1455                state.save()?;
1456            }
1457        }
1458    }
1459}
1460
1461/// One observation, plus the two things [`PrState`] deliberately does not carry:
1462/// the title (needed for the squash subject) and where the failing checks'
1463/// logs live.
1464struct Seen {
1465    pr: PrState,
1466    title: String,
1467    failing_urls: Vec<(String, String)>,
1468}
1469
1470/// Read the pull request: `gh pr view` for the rollup and the top-level thread,
1471/// `gh api` for the inline review comments `gh pr view` does not report.
1472async fn observe(repo: &Path, pr_url: &str) -> Result<Seen> {
1473    let view = gh(
1474        repo,
1475        &[
1476            "pr".to_owned(),
1477            "view".to_owned(),
1478            pr_url.to_owned(),
1479            "--json".to_owned(),
1480            "url,number,state,title,statusCheckRollup,reviews,comments,mergeStateStatus".to_owned(),
1481        ],
1482    )
1483    .await?;
1484    if !view.0 {
1485        bail!("gh pr view {pr_url}: {}", view.1);
1486    }
1487    let mut pr = parse_pr(&view.1)?;
1488    let raw: GhPr = serde_json::from_str(&view.1).context("re-read pull request json")?;
1489
1490    let inline = gh(
1491        repo,
1492        &[
1493            "api".to_owned(),
1494            format!("repos/{{owner}}/{{repo}}/pulls/{}/comments", pr.number),
1495        ],
1496    )
1497    .await?;
1498    if inline.0 {
1499        match parse_inline_comments(&inline.1) {
1500            Ok(mut comments) => pr.review_comments.append(&mut comments),
1501            // An unreadable inline thread must not end a landing: the rollup
1502            // and the top-level thread are still real signal.
1503            Err(e) => tracing::warn!("inline review comments unreadable: {e}"),
1504        }
1505    } else {
1506        tracing::warn!("gh api pulls/{}/comments: {}", pr.number, inline.1);
1507    }
1508
1509    let failing_urls = raw
1510        .status_check_rollup
1511        .iter()
1512        .filter(|c| c.verdict() == Verdict::Fail)
1513        .filter_map(|c| c.url().map(|u| (c.label(), u.to_owned())))
1514        .collect();
1515
1516    Ok(Seen {
1517        pr,
1518        title: raw.title,
1519        failing_urls,
1520    })
1521}
1522
1523/// What a fix round did.
1524enum Fixed {
1525    /// The fixer committed something.
1526    Committed,
1527    /// The fixer ran and chose to change nothing.
1528    Declined,
1529    /// The fixer could not run, or said nothing usable.
1530    Failed(String),
1531}
1532
1533/// Hand the failures and the comments to the fixer, then commit and push.
1534///
1535/// The fixer works in the winner's own worktree so its commits land on the
1536/// branch the pull request is built from, and it runs with `allow_write` for
1537/// the same reason.
1538async fn fix_round(
1539    state: &mut RunState,
1540    pr: &PrState,
1541    round: usize,
1542    budget: usize,
1543    reason: &str,
1544    logs: &str,
1545) -> Result<Fixed> {
1546    let winner = state
1547        .winner()
1548        .cloned()
1549        .context("landing needs a winning candidate; none is recorded on this run")?;
1550    let roles = state
1551        .config
1552        .resolve_roles()
1553        .context("resolve the roster for the fix round")?;
1554    // Same rule as the review loop: an explicitly configured fixer, otherwise
1555    // the winner's own author continuing its own conversation - the competition
1556    // is over, so its context is pure benefit.
1557    let (spec, seat_key): (AgentSpec, String) = match &roles.fixer {
1558        Some(f) if f.id != winner.agent => (f.clone(), "fix".to_owned()),
1559        _ => (
1560            state
1561                .config
1562                .agent(&winner.agent)
1563                .cloned()
1564                .unwrap_or_else(|_| roles.implementers[winner.index].clone()),
1565            format!("impl-{}", winner.label),
1566        ),
1567    };
1568
1569    let prompt = fix_prompt(state, pr, round, budget, reason, logs);
1570    let mut seat = seat_of(state, &seat_key, &spec.id);
1571    let artifacts = agent::artifacts_dir(&state.dir());
1572    let prompt = if state.config.cache_dir().is_some() {
1573        format!("{prompt}\n\n{}", prompt::build_cache_note())
1574    } else {
1575        prompt
1576    };
1577    let out = agent::invoke(
1578        &spec,
1579        &mut seat,
1580        &Invocation {
1581            cwd: &winner.worktree,
1582            prompt: &prompt,
1583            timeout: Duration::from_secs(state.config.graph.timeout_fix),
1584            allow_write: true,
1585            sessions: state.config.graph.sessions,
1586            artifacts: &artifacts,
1587            stem: &format!("land-{round}"),
1588            run: &state.id,
1589            node: "land",
1590            cache_dir: state.config.cache_dir().as_deref(),
1591        },
1592    )
1593    .await;
1594    state.seats.insert(seat.key.clone(), seat);
1595
1596    match out {
1597        Ok(o) if o.quota_exhausted() => {
1598            return Ok(Fixed::Failed(
1599                "rate limited (quota); the fixer could not run".to_owned(),
1600            ));
1601        }
1602        Ok(o) if !o.usable() => {
1603            return Ok(Fixed::Failed(format!(
1604                "the fixer produced nothing usable (exit {:?}, timed out: {})",
1605                o.exit_code, o.timed_out
1606            )));
1607        }
1608        Ok(_) => {}
1609        Err(e) => return Ok(Fixed::Failed(format!("{e:#}"))),
1610    }
1611
1612    let before = git::rev_parse(&winner.worktree, "HEAD").await?;
1613    // An agent that edited files but never committed would otherwise push
1614    // nothing and look like a refusal.
1615    git::commit_all(
1616        &winner.worktree,
1617        &format!("magi: land round {round} fixes (uncommitted work)"),
1618    )
1619    .await
1620    .ok();
1621    let after = git::rev_parse(&winner.worktree, "HEAD").await?;
1622    if after == before {
1623        return Ok(Fixed::Declined);
1624    }
1625
1626    let remote = state.config.merge.remote.clone();
1627    let push = git::push(&winner.worktree, &remote, &winner.branch).await?;
1628    if !push.ok() {
1629        return Ok(Fixed::Failed(format!(
1630            "pushing {} to {remote} failed: {}",
1631            winner.branch, push.stderr
1632        )));
1633    }
1634    state.event(
1635        "land",
1636        format!("round {round}: pushed a fix to {}", winner.branch),
1637    );
1638    Ok(Fixed::Committed)
1639}
1640
1641/// Fetch or create a seat, keeping its conversation across nodes.
1642fn seat_of(state: &mut RunState, key: &str, agent: &str) -> SeatState {
1643    if let Some(existing) = state.seats.get(key)
1644        && existing.agent == agent
1645    {
1646        return existing.clone();
1647    }
1648    let fresh = SeatState::new(key, agent, state.seed);
1649    state.seats.insert(key.to_owned(), fresh.clone());
1650    fresh
1651}
1652
1653/// What the fixer is told.
1654fn fix_prompt(
1655    state: &RunState,
1656    pr: &PrState,
1657    round: usize,
1658    budget: usize,
1659    reason: &str,
1660    logs: &str,
1661) -> String {
1662    let mut s = format!(
1663        "Your patch is open as a pull request and it is not landing. Land round \
1664         {round} of {budget}.\n\n\
1665         Pull request: {}\n\n\
1666         What is holding it: {reason}\n\n\
1667         # The task\n\n{}\n",
1668        pr.url, state.instruction
1669    );
1670
1671    if pr.failing.is_empty() {
1672        s.push_str("\n# Failing checks\n\n(none)\n");
1673    } else {
1674        let _ = write!(s, "\n# Failing checks\n\n- {}\n", pr.failing.join("\n- "));
1675        if logs.trim().is_empty() {
1676            s.push_str("\nNo log could be read; reproduce the failure locally.\n");
1677        } else {
1678            let _ = write!(s, "\n## Failing log tails\n\n{logs}\n");
1679        }
1680    }
1681
1682    if pr.review_comments.is_empty() {
1683        s.push_str("\n# Review comments\n\n(none)\n");
1684    } else {
1685        s.push_str("\n# Review comments\n");
1686        for c in &pr.review_comments {
1687            let where_ = match (&c.path, c.line) {
1688                (Some(p), Some(l)) => format!(" ({p}:{l})"),
1689                (Some(p), None) => format!(" ({p})"),
1690                _ => String::new(),
1691            };
1692            let _ = write!(s, "\n## {}{where_}\n\n{}\n", c.author, c.body.trim());
1693        }
1694    }
1695
1696    s.push_str(
1697        "\n# Rules\n\n\
1698         1. Fix the cause, never the symptom. Do not delete, skip, or weaken a \
1699            failing test; do not silence a lint with an allow attribute; do not \
1700            stretch a timeout to hide a race. If the check is right, the code is \
1701            wrong.\n\
1702         2. Change nothing the checks and the comments did not raise. A \
1703            drive-by refactor turns a one-line fix into a pull request that \
1704            needs reviewing again.\n\
1705         3. If a comment is wrong, say so with a checkable argument and change \
1706            nothing for it. A declined comment with a reason is a correct \
1707            outcome; a change made to appease a reviewer is not.\n\
1708         4. Commit in this worktree. magi pushes to the pull request's branch \
1709            for you; do not push, merge, or close anything yourself.\n\
1710         5. Never name yourself, your vendor, or your model, anywhere.\n\n\
1711         # Output\n\n\
1712         Say what you changed and why, and what you declined and why.",
1713    );
1714
1715    let language = &state.config.graph.language;
1716    if !(language.trim().is_empty() || language.eq_ignore_ascii_case("en")) {
1717        let _ = write!(s, "\n\nWrite all prose in {language}.");
1718    }
1719    if let Some(overlay) = state.config.prompts.overlay("fix") {
1720        let _ = write!(s, "\n\n{overlay}");
1721    }
1722    s
1723}
1724
1725/// Failing log tails, the way the operator collects them by hand:
1726/// `gh run view --log-failed`.
1727async fn failing_logs(repo: &Path, failing: &[(String, String)]) -> String {
1728    let mut out = String::new();
1729    for (name, url) in failing.iter().take(MAX_LOGS) {
1730        let args = match (job_of(url), run_of(url)) {
1731            (Some(job), _) => vec![
1732                "run".to_owned(),
1733                "view".to_owned(),
1734                "--log-failed".to_owned(),
1735                "--job".to_owned(),
1736                job,
1737            ],
1738            (None, Some(run)) => vec![
1739                "run".to_owned(),
1740                "view".to_owned(),
1741                run,
1742                "--log-failed".to_owned(),
1743            ],
1744            // Not a GitHub Actions check - an external status has no log here.
1745            (None, None) => continue,
1746        };
1747        let (ok, body) = match gh(repo, &args).await {
1748            Ok(v) => v,
1749            Err(e) => (false, format!("{e:#}")),
1750        };
1751        if !ok && body.trim().is_empty() {
1752            continue;
1753        }
1754        let _ = write!(out, "### {name}\n\n```\n{}\n```\n\n", tail(&body, LOG_TAIL));
1755    }
1756    out
1757}
1758
1759/// Job id out of a check's `detailsUrl`
1760/// (`https://github.com/o/r/actions/runs/<run>/job/<job>`).
1761fn job_of(details_url: &str) -> Option<String> {
1762    let after = details_url.split("/job/").nth(1)?;
1763    let id: String = after.chars().take_while(char::is_ascii_digit).collect();
1764    (!id.is_empty()).then_some(id)
1765}
1766
1767/// Workflow run id out of a check's `detailsUrl`.
1768fn run_of(details_url: &str) -> Option<String> {
1769    let after = details_url.split("/actions/runs/").nth(1)?;
1770    let id: String = after.chars().take_while(char::is_ascii_digit).collect();
1771    (!id.is_empty()).then_some(id)
1772}
1773
1774/// Leave the pull request open, say why on it, and mark the run blocked.
1775///
1776/// The comment is what makes an unattended stop actionable: the operator wakes
1777/// up to a pull request that explains itself rather than to a silent queue.
1778async fn stop(state: &mut RunState, repo: &Path, pr: &PrState, why: &str) -> Result<()> {
1779    let body = format!(
1780        "{MARKER}\nmagi stopped landing this pull request: {why}\n\n\
1781         The branch is untouched and the run is `{}`. Nothing was merged.",
1782        state.id
1783    );
1784    let posted = gh(
1785        repo,
1786        &[
1787            "pr".to_owned(),
1788            "comment".to_owned(),
1789            pr.number.to_string(),
1790            "--body".to_owned(),
1791            body,
1792        ],
1793    )
1794    .await;
1795    match posted {
1796        Ok((true, _)) => {}
1797        Ok((false, out)) => tracing::warn!("could not comment on {}: {out}", pr.url),
1798        Err(e) => tracing::warn!("could not comment on {}: {e:#}", pr.url),
1799    }
1800    state.status = RunStatus::Blocked;
1801    state.merge = Some(MergeOutcome {
1802        mode: MergeMode::Pr,
1803        ok: false,
1804        detail: why.to_owned(),
1805    });
1806    state.event("land", format!("stopped: {why}"));
1807    state.save()?;
1808    Ok(())
1809}
1810
1811/// Run `gh` in `repo`, returning success and the combined output.
1812///
1813/// Combined because `gh` reports a refused merge on stderr and the pull request
1814/// json on stdout, and both are evidence.
1815async fn gh(cwd: &Path, args: &[String]) -> Result<(bool, String)> {
1816    let out = tokio::process::Command::new("gh")
1817        .args(args)
1818        .current_dir(cwd)
1819        .quiet()
1820        .stdin(std::process::Stdio::null())
1821        .output()
1822        .await
1823        .with_context(|| format!("spawn gh {}", args.join(" ")))?;
1824    let mut body = String::from_utf8_lossy(&out.stdout).into_owned();
1825    let err = String::from_utf8_lossy(&out.stderr);
1826    if body.trim().is_empty() {
1827        body = err.into_owned();
1828    } else if !err.trim().is_empty() {
1829        body.push_str(&err);
1830    }
1831    Ok((out.status.success(), body.trim().to_owned()))
1832}
1833
1834/// Verdict of one entry in the status rollup.
1835#[derive(Debug, Clone, Copy, PartialEq, Eq)]
1836enum Verdict {
1837    Pass,
1838    Fail,
1839    Pending,
1840    Unknown,
1841}
1842
1843#[derive(Debug, Deserialize)]
1844#[serde(rename_all = "camelCase")]
1845struct GhPr {
1846    #[serde(default)]
1847    url: String,
1848    #[serde(default)]
1849    number: u64,
1850    #[serde(default)]
1851    state: String,
1852    #[serde(default)]
1853    title: String,
1854    #[serde(default)]
1855    status_check_rollup: Vec<GhCheck>,
1856    /// GitHub's own verdict on whether the pull request can be merged.
1857    ///
1858    /// Worth asking for because it is the only place the *required* check set
1859    /// is applied: the rollup lists every check equally, so a repository that
1860    /// deliberately does not require `coverage` still looks red here. See
1861    /// [`Blocking`].
1862    #[serde(default)]
1863    merge_state_status: String,
1864    #[serde(default)]
1865    reviews: Vec<GhReview>,
1866    #[serde(default)]
1867    comments: Vec<GhComment>,
1868}
1869
1870/// One rollup entry. `gh` mixes two GraphQL types in this array: a `CheckRun`
1871/// has `name`/`status`/`conclusion`, while a `StatusContext` - the old commit
1872/// status API, which is how CodeRabbit reports - has `context`/`state` and no
1873/// conclusion at all.
1874#[derive(Debug, Deserialize)]
1875#[serde(rename_all = "camelCase")]
1876struct GhCheck {
1877    #[serde(default)]
1878    name: Option<String>,
1879    #[serde(default)]
1880    context: Option<String>,
1881    #[serde(default)]
1882    status: Option<String>,
1883    #[serde(default)]
1884    conclusion: Option<String>,
1885    #[serde(default)]
1886    state: Option<String>,
1887    #[serde(default)]
1888    details_url: Option<String>,
1889    #[serde(default)]
1890    target_url: Option<String>,
1891}
1892
1893impl GhCheck {
1894    /// Name to show a human and hand to the fixer.
1895    fn label(&self) -> String {
1896        self.name
1897            .clone()
1898            .or_else(|| self.context.clone())
1899            .unwrap_or_else(|| "(unnamed check)".to_owned())
1900    }
1901
1902    /// Where this check's logs live, when it has any.
1903    fn url(&self) -> Option<&str> {
1904        self.details_url
1905            .as_deref()
1906            .or(self.target_url.as_deref())
1907            .filter(|u| !u.is_empty())
1908    }
1909
1910    /// Did it pass?
1911    ///
1912    /// `SKIPPED` and `NEUTRAL` count as passed: the Claude review workflow
1913    /// skips release and bot pull requests by design, and a skip that blocked
1914    /// landing would block exactly the pull requests that need no review.
1915    /// `CANCELLED` counts as failed - a cancelled check did not pass, and
1916    /// merging over one is merging over a check that never ran.
1917    fn verdict(&self) -> Verdict {
1918        if let Some(status) = self.status.as_deref() {
1919            if !status.eq_ignore_ascii_case("COMPLETED") {
1920                return Verdict::Pending;
1921            }
1922        }
1923        let outcome = self
1924            .conclusion
1925            .as_deref()
1926            .or(self.state.as_deref())
1927            .unwrap_or("");
1928        match outcome.to_ascii_uppercase().as_str() {
1929            "SUCCESS" | "SKIPPED" | "NEUTRAL" => Verdict::Pass,
1930            "FAILURE" | "ERROR" | "TIMED_OUT" | "CANCELLED" | "STARTUP_FAILURE"
1931            | "ACTION_REQUIRED" => Verdict::Fail,
1932            "PENDING" | "EXPECTED" | "QUEUED" | "IN_PROGRESS" | "WAITING" | "REQUESTED" => {
1933                Verdict::Pending
1934            }
1935            _ => Verdict::Unknown,
1936        }
1937    }
1938}
1939
1940#[derive(Debug, Deserialize)]
1941struct GhAuthor {
1942    #[serde(default)]
1943    login: String,
1944}
1945
1946#[derive(Debug, Deserialize)]
1947struct GhReview {
1948    #[serde(default)]
1949    author: GhAuthor,
1950    #[serde(default)]
1951    body: String,
1952}
1953
1954#[derive(Debug, Deserialize)]
1955struct GhComment {
1956    #[serde(default)]
1957    author: GhAuthor,
1958    #[serde(default)]
1959    body: String,
1960}
1961
1962#[derive(Debug, Deserialize)]
1963struct GhUser {
1964    #[serde(default)]
1965    login: String,
1966}
1967
1968#[derive(Debug, Deserialize)]
1969struct GhInline {
1970    #[serde(default)]
1971    user: GhUser,
1972    #[serde(default)]
1973    path: Option<String>,
1974    #[serde(default)]
1975    line: Option<u64>,
1976    #[serde(default)]
1977    body: String,
1978}
1979
1980impl Default for GhAuthor {
1981    fn default() -> Self {
1982        Self {
1983            login: "(unknown)".to_owned(),
1984        }
1985    }
1986}
1987
1988impl Default for GhUser {
1989    fn default() -> Self {
1990        Self {
1991            login: "(unknown)".to_owned(),
1992        }
1993    }
1994}
1995
1996#[cfg(test)]
1997mod tests {
1998    use super::*;
1999
2000    /// Real `gh pr view` output for the open pull request #10 (Renovate's apm bump), trimmed to four checks and its one comment. Every check passed or was skipped by the review workflow, and the only comment is CodeRabbit's trigger notice.
2001    const GREEN_OPEN: &str = r####"{
2002  "url": "https://github.com/yukimemi/magi/pull/10",
2003  "number": 10,
2004  "state": "OPEN",
2005  "mergeStateStatus": "CLEAN",
2006  "statusCheckRollup": [
2007    {
2008      "__typename": "CheckRun",
2009      "conclusion": "SKIPPED",
2010      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33356278334/job/99378963755",
2011      "name": "review",
2012      "status": "COMPLETED",
2013      "workflowName": "claude-review"
2014    },
2015    {
2016      "__typename": "CheckRun",
2017      "conclusion": "SUCCESS",
2018      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33356278338/job/99378963144",
2019      "name": "check (ubuntu-latest)",
2020      "status": "COMPLETED",
2021      "workflowName": "CI"
2022    },
2023    {
2024      "__typename": "CheckRun",
2025      "conclusion": "SUCCESS",
2026      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33356278338/job/99378963095",
2027      "name": "rustfmt",
2028      "status": "COMPLETED",
2029      "workflowName": "CI"
2030    },
2031    {
2032      "__typename": "StatusContext",
2033      "context": "CodeRabbit",
2034      "state": "SUCCESS",
2035      "targetUrl": ""
2036    }
2037  ],
2038  "reviews": [],
2039  "comments": [
2040    {
2041      "author": {
2042        "login": "coderabbitai"
2043      },
2044      "authorAssociation": "NONE",
2045      "body": "<!-- This is an auto-generated comment: summarize by coderabbit.ai -->\n<!-- This is an auto-generated comment: skip review by coderabbit.ai -->\n\n> [!IMPORTANT]\n> - [ ] <!-- {\"checkboxId\":\"e9bb8d72-00e8-4f67-9cb2-caf3b22574fe\"} --> 🔍 Trigger review\n> \n> This repository does not receive automatic reviews because it has fewer than 10 stars.\n> \n> <details>\n> <summary>⚙️ Run configuration</summary>\n> \n> **Configuration used**: defaults\n> \n> **Review profile**: CHILL\n> \n> **Plan**: Pro Plus\n> \n> **Run ID**: `78e70bf3-c5a0-4269-a96c-2afb2dba7eff`\n> \n> </details>\n\n<!-- end of auto-generated comment: skip review by coderabbit.ai -->\n\n<!-- tips_start -->\n\n---\n\nThanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=yukimemi/magi&utm_content=10)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.\n\n<details>\n<summary>❤️ Share</summary>\n\n- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%2"
2046    }
2047  ]
2048}"####;
2049
2050    /// Real output for the open pull request #9 (the daily kata-apply), whose `editorconfig` check failed while everything else passed.
2051    const RED_OPEN: &str = r####"{
2052  "url": "https://github.com/yukimemi/magi/pull/9",
2053  "number": 9,
2054  "state": "OPEN",
2055  "mergeStateStatus": "UNSTABLE",
2056  "statusCheckRollup": [
2057    {
2058      "__typename": "CheckRun",
2059      "conclusion": "SUCCESS",
2060      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323744",
2061      "name": "check (ubuntu-latest)",
2062      "status": "COMPLETED",
2063      "workflowName": "CI"
2064    },
2065    {
2066      "__typename": "CheckRun",
2067      "conclusion": "SUCCESS",
2068      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323811",
2069      "name": "rustfmt",
2070      "status": "COMPLETED",
2071      "workflowName": "CI"
2072    },
2073    {
2074      "__typename": "CheckRun",
2075      "conclusion": "FAILURE",
2076      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323572",
2077      "name": "editorconfig",
2078      "status": "COMPLETED",
2079      "workflowName": "CI"
2080    },
2081    {
2082      "__typename": "StatusContext",
2083      "context": "CodeRabbit",
2084      "state": "SUCCESS",
2085      "targetUrl": ""
2086    }
2087  ],
2088  "reviews": [],
2089  "comments": [
2090    {
2091      "author": {
2092        "login": "coderabbitai"
2093      },
2094      "authorAssociation": "NONE",
2095      "body": "<!-- This is an auto-generated comment: summarize by coderabbit.ai -->\n<!-- This is an auto-generated comment: skip review by coderabbit.ai -->\n\n> [!IMPORTANT]\n> - [ ] <!-- {\"checkboxId\":\"e9bb8d72-00e8-4f67-9cb2-caf3b22574fe\"} --> 🔍 Trigger review\n> \n> This repository does not receive automatic reviews because it has fewer than 10 stars.\n> \n> <details>\n> <summary>⚙️ Run configuration</summary>\n> \n> **Configuration used**: defaults\n> \n> **Review profile**: CHILL\n> \n> **Plan**: Team\n> \n> **Run ID**: `91e0dc24-6040-4c3d-92c6-f7d2b542523d`\n> \n> </details>\n\n<!-- end of auto-generated comment: skip review by coderabbit.ai -->\n\n<!-- tips_start -->\n\n---\n\nThanks for using [CodeRabbit](https://coderab"
2096    }
2097  ]
2098}"####;
2099
2100    /// Pull request #9's real payload with its `editorconfig` check rewound to the `IN_PROGRESS` / `conclusion: null` pair `gh` reports while a job is still in flight.
2101    const PENDING_OPEN: &str = r####"{
2102  "url": "https://github.com/yukimemi/magi/pull/9",
2103  "number": 9,
2104  "state": "OPEN",
2105  "statusCheckRollup": [
2106    {
2107      "__typename": "CheckRun",
2108      "conclusion": "SUCCESS",
2109      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323744",
2110      "name": "check (ubuntu-latest)",
2111      "status": "COMPLETED",
2112      "workflowName": "CI"
2113    },
2114    {
2115      "__typename": "CheckRun",
2116      "conclusion": "SUCCESS",
2117      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323811",
2118      "name": "rustfmt",
2119      "status": "COMPLETED",
2120      "workflowName": "CI"
2121    },
2122    {
2123      "__typename": "CheckRun",
2124      "conclusion": null,
2125      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323572",
2126      "name": "editorconfig",
2127      "status": "IN_PROGRESS",
2128      "workflowName": "CI"
2129    },
2130    {
2131      "__typename": "StatusContext",
2132      "context": "CodeRabbit",
2133      "state": "SUCCESS",
2134      "targetUrl": ""
2135    }
2136  ],
2137  "reviews": [],
2138  "comments": []
2139}"####;
2140
2141    /// Real output for pull request #16 after it was merged - the shape landing sees when a person merged underneath it.
2142    const MERGED: &str = r####"{
2143  "url": "https://github.com/yukimemi/magi/pull/16",
2144  "number": 16,
2145  "state": "MERGED",
2146  "statusCheckRollup": [
2147    {
2148      "__typename": "CheckRun",
2149      "conclusion": "SUCCESS",
2150      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33636587933/job/100268878095",
2151      "name": "check (ubuntu-latest)",
2152      "status": "COMPLETED",
2153      "workflowName": "CI"
2154    },
2155    {
2156      "__typename": "CheckRun",
2157      "conclusion": "SUCCESS",
2158      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33636587918/job/100268876427",
2159      "name": "review",
2160      "status": "COMPLETED",
2161      "workflowName": "claude-review"
2162    }
2163  ],
2164  "reviews": [],
2165  "comments": []
2166}"####;
2167
2168    /// Pull request #12's real payload - a green pull request carrying CodeRabbit's walkthrough and a Claude review that found a real bug - rewound to the `OPEN` state it was in when that review was posted.
2169    const REVIEWED_OPEN: &str = r####"{
2170  "url": "https://github.com/yukimemi/magi/pull/12",
2171  "number": 12,
2172  "state": "OPEN",
2173  "statusCheckRollup": [
2174    {
2175      "__typename": "CheckRun",
2176      "conclusion": "SUCCESS",
2177      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33571212506/job/100065355258",
2178      "name": "check (ubuntu-latest)",
2179      "status": "COMPLETED",
2180      "workflowName": "CI"
2181    },
2182    {
2183      "__typename": "CheckRun",
2184      "conclusion": "SUCCESS",
2185      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33571212566/job/100065355810",
2186      "name": "review",
2187      "status": "COMPLETED",
2188      "workflowName": "claude-review"
2189    }
2190  ],
2191  "reviews": [
2192    {
2193      "author": {
2194        "login": "claude"
2195      },
2196      "state": "COMMENTED",
2197      "body": ""
2198    }
2199  ],
2200  "comments": [
2201    {
2202      "author": {
2203        "login": "coderabbitai"
2204      },
2205      "authorAssociation": "NONE",
2206      "body": "<!-- This is an auto-generated comment: summarize by coderabbit.ai -->\n<!-- This is an auto-generated comment: skip review by coderabbit.ai -->\n\n> [!IMPORTANT]\n> - [ ] <!-- {\"checkboxId\":\"e9bb8d72-00e8-4f67-9cb2-caf3b22574fe\"} --> 🔍 Trigger review\n> \n> This repository does not receive automatic reviews because it has fewer than 10 stars.\n> \n> <details>\n> <summary>⚙️ Run configuration</summary>\n> \n> **Configuration used**: defaults\n> \n> **Review profile**: CHILL\n> \n> **Plan**: Team\n> \n> **Run ID**: `72058bf3-b7df-41d9-8e4d-a06a31be4a26`\n> \n> </details>\n\n<!-- end of auto-generated comment: skip review by coderabbit.ai -->\n\n<!-- tips_start -->\n\n---\n\nThanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=yukimemi/magi&utm_content=12)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.\n\n<details>\n<summa"
2207    },
2208    {
2209      "author": {
2210        "login": "claude"
2211      },
2212      "authorAssociation": "NONE",
2213      "body": "**Claude finished @yukimemi's task in 3m 52s** —— [View job](https://github.com/yukimemi/magi/actions/runs/33571212566)\n\n---\n### Review: `magi review <branch>` — cheap-half-only graph\n\nRead through `src/graph.rs`, `src/main.rs`, `src/prompt.rs`, and the new/edited tests, and traced the claimed degeneration (`prep` short-circuits on a non-empty candidate list, `implement` skips because `commits != 0`, `judge`/`vote` skip on `viable().len() == 1`, `tally` skips because it's pre-set, `fold_losers` has no losers) against the actual code — it holds up. CI (`cargo make check`) is green on this PR.\n\n**Correctness**\n\n- One real bug, flagged inline on `src/graph.rs:1255`: the fixer-agent fallback (`self.roles.implementers[winner.index].clone()`) is unreachable in the normal graph (a real candidate's `winner.agent` always resolves via `config.agent(...)`), but a review-only run's `winner.agent` is always the `\"(existing branch)\"` sentinel, so this fallback now runs on *every* review-only fix that has no dedicated `[roles] fixer`. `graph.candidates` has no lower-bound validation, so a `magi.toml` tuned for review-only use (`candidates = 0`, plausible given this PR's own cost rationale) would panic with an out-of-bounds index the first time a"
2214    }
2215  ]
2216}"####;
2217
2218    /// Real `gh api repos/{owner}/{repo}/pulls/12/comments` output: one inline finding with its file and line.
2219    const INLINE: &str = r####"[
2220  {
2221    "user": {
2222      "login": "claude[bot]"
2223    },
2224    "path": "src/graph.rs",
2225    "line": 231,
2226    "body": "Minor edge case: unlike `implement()` (which sets `c.empty = commits == 0 || patch.trim().is_empty()`, `src/graph.rs:472`), the seeded review-only candidate always sets `empty: false` once `commits > 0` is confirmed, without checking whether the diff itself is actually empty (e.g. a commit immediately followed by a revert nets zero file changes). Such a branch would pass `Runner::review`'s validation and proceed into a review round with an empty patch, where `implement()`'s equivalent path would"
2227  }
2228]"####;
2229
2230    /// CodeRabbit's real trigger notice: a checkbox, a `<details>` block, and its own "skip review" marker.
2231    const CODERABBIT_TRIGGER: &str = r####"<!-- This is an auto-generated comment: summarize by coderabbit.ai -->
2232<!-- This is an auto-generated comment: skip review by coderabbit.ai -->
2233
2234> [!IMPORTANT]
2235> - [ ] <!-- {"checkboxId":"e9bb8d72-00e8-4f67-9cb2-caf3b22574fe"} --> 🔍 Trigger review
2236> 
2237> This repository does not receive automatic reviews because it has fewer than 10 stars.
2238> 
2239> <details>
2240> <summary>⚙️ Run configuration</summary>
2241> 
2242> **Configuration used**: defaults
2243> 
2244> **Review profile**: CHILL
2245> 
2246> **Plan**: Team
2247> 
2248> **Run ID**: `c1e2a68f-87fc-4b35-9ec4-e75c7854966a`
2249> 
2250> </details>
2251
2252<!-- end of auto-generated comment: skip review by coderabbit.ai -->
2253
2254<!-- tips_start -->
2255
2256---
2257
2258Thanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=yukimemi/magi&utm_content=16)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
2259
2260<details>
2261<summary>❤️ Share</summary>
2262
2263- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20off"####;
2264
2265    /// The Claude review job's real comment while it is still working: a heading and a task list, and nothing that asks for a change.
2266    const CLAUDE_CHECKLIST: &str = r####"**Claude finished @yukimemi's task in 4m 14s** —— [View job](https://github.com/yukimemi/magi/actions/runs/33636587918)
2267
2268---
2269### Reviewing PR #16
2270
2271- [x] Read AGENTS.md conventions
2272- [x] Review `src/daemon.rs` changes
2273- [x] Review `src/main.rs` changes (new `doctor` reporting)
2274- [x] Review `src/web.rs` changes (reuse of unreadable-run count)
2275- [x] Check test coverage for new behavior
2276- [x] Run verification commands (blocked — see note)
2277- [x] Post findings"####;
2278
2279    /// The same job's real comment on pull request #12 once it had something to say.
2280    const CLAUDE_FINDING: &str = r####"**Claude finished @yukimemi's task in 3m 52s** —— [View job](https://github.com/yukimemi/magi/actions/runs/33571212566)
2281
2282---
2283### Review: `magi review <branch>` — cheap-half-only graph
2284
2285Read through `src/graph.rs`, `src/main.rs`, `src/prompt.rs`, and the new/edited tests, and traced the claimed degeneration (`prep` short-circuits on a non-empty candidate list, `implement` skips because `commits != 0`, `judge`/`vote` skip on `viable().len() == 1`, `tally` skips because it's pre-set, `fold_losers` has no losers) against the actual code — it holds up. CI (`cargo make check`) is green on this PR.
2286
2287**Correctness**
2288
2289- One real bug, flagged inline on `src/graph.rs:1255`: the fixer-agent fallback (`self.roles.implementers[winner.index].clone()`) is unreachable in the normal graph (a real candidate's `winner.agent` always resolves via `config.agent(...)`), but a review-only run's `winner.agent` is always the `"(existing branch)"` sentinel, so this fallback now runs on *every* review-only fix that has no dedicated `[roles] fixer`. `graph.candidates` has no lower-bound validation, so a `magi.toml` tuned for review-only use (`candidates = 0`, plausible given this PR's own cost rationale) would panic with an out-of-bounds index the first time a"####;
2290
2291    fn pr(checks: Checks, failing: &[&str], comments: usize) -> PrState {
2292        PrState {
2293            url: "https://github.com/yukimemi/magi/pull/16".to_owned(),
2294            number: 16,
2295            state: PrLifecycle::Open,
2296            checks,
2297            // These tests are about red-means-fix, so a red here is one the
2298            // forge gates on. Without saying so they would assert the new
2299            // "merge past a check nobody requires" path by accident.
2300            blocking: if matches!(checks, Checks::Red) {
2301                Blocking::Yes
2302            } else {
2303                Blocking::No
2304            },
2305            failing: failing.iter().map(|s| (*s).to_owned()).collect(),
2306            review_comments: (0..comments)
2307                .map(|i| ReviewComment {
2308                    author: "coderabbitai".to_owned(),
2309                    path: Some("src/graph.rs".to_owned()),
2310                    line: Some(231),
2311                    body: format!("finding {i}"),
2312                })
2313                .collect(),
2314        }
2315    }
2316
2317    #[test]
2318    fn a_green_pull_request_with_nothing_outstanding_parses_as_ready_to_merge() {
2319        let state = parse_pr(GREEN_OPEN).expect("green fixture parses");
2320        assert_eq!(state.number, 10);
2321        assert_eq!(state.state, PrLifecycle::Open);
2322        assert_eq!(state.checks, Checks::Green);
2323        assert!(state.failing.is_empty());
2324        assert!(
2325            state.review_comments.is_empty(),
2326            "the only comment is CodeRabbit's trigger notice: {:?}",
2327            state.review_comments
2328        );
2329        assert_eq!(decide(&state, 0, 4, Duration::ZERO), Step::Merge);
2330    }
2331
2332    #[test]
2333    fn a_failing_check_parses_as_red_and_is_named() {
2334        let state = parse_pr(RED_OPEN).expect("red fixture parses");
2335        assert_eq!(state.checks, Checks::Red);
2336        assert_eq!(state.failing, vec!["editorconfig".to_owned()]);
2337        // The captured payload says `UNSTABLE` - mergeable, with a check
2338        // nobody requires red - which is exactly the shape that had to be
2339        // merged by hand. Asserted separately, in
2340        // `a_red_check_nobody_requires_does_not_buy_a_fix_round`. What this
2341        // test is about is that a red check is *named*, so the reason a fixer
2342        // is handed says which one; so it asks the blocking question here.
2343        let mut blocking = state.clone();
2344        blocking.blocking = Blocking::Yes;
2345        match decide(&blocking, 0, 4, Duration::ZERO) {
2346            Step::Fix { reason } => {
2347                assert!(reason.contains("editorconfig"), "reason: {reason}");
2348                assert!(reason.contains("failing"), "reason: {reason}");
2349            }
2350            other => panic!("expected a fix round, got {other:?}"),
2351        }
2352    }
2353
2354    #[test]
2355    fn a_check_still_running_parses_as_pending_and_is_waited_for() {
2356        let state = parse_pr(PENDING_OPEN).expect("pending fixture parses");
2357        assert_eq!(state.checks, Checks::Pending);
2358        assert_eq!(decide(&state, 0, 4, Duration::ZERO), Step::Wait);
2359    }
2360
2361    #[test]
2362    fn a_pull_request_merged_underneath_us_is_done_rather_than_a_failure() {
2363        let state = parse_pr(MERGED).expect("merged fixture parses");
2364        assert_eq!(state.state, PrLifecycle::Merged);
2365        assert_eq!(
2366            decide(&state, 0, 4, Duration::ZERO),
2367            Step::Done { merged: true }
2368        );
2369    }
2370
2371    #[test]
2372    fn a_review_that_found_something_is_outstanding_and_holds_the_merge() {
2373        let state = parse_pr(REVIEWED_OPEN).expect("reviewed fixture parses");
2374        assert_eq!(state.checks, Checks::Green);
2375        let authors: Vec<&str> = state
2376            .review_comments
2377            .iter()
2378            .map(|c| c.author.as_str())
2379            .collect();
2380        assert_eq!(
2381            authors,
2382            vec!["claude"],
2383            "CodeRabbit's walkthrough is machinery; Claude's review is a finding"
2384        );
2385        match decide(&state, 0, 4, Duration::ZERO) {
2386            Step::Fix { reason } => assert!(reason.contains("unresolved"), "reason: {reason}"),
2387            other => panic!("expected a fix round, got {other:?}"),
2388        }
2389    }
2390
2391    #[test]
2392    fn inline_review_comments_keep_their_file_and_line() {
2393        let comments = parse_inline_comments(INLINE).expect("inline fixture parses");
2394        assert_eq!(comments.len(), 1);
2395        assert_eq!(comments[0].author, "claude[bot]");
2396        assert_eq!(comments[0].path.as_deref(), Some("src/graph.rs"));
2397        assert_eq!(comments[0].line, Some(231));
2398        assert!(comments[0].body.contains("empty"), "{}", comments[0].body);
2399    }
2400
2401    #[test]
2402    fn a_status_only_bot_comment_does_not_trigger_a_fix_round() {
2403        assert!(
2404            is_noise(CODERABBIT_TRIGGER),
2405            "CodeRabbit's trigger notice declares itself not a review"
2406        );
2407        assert!(
2408            is_noise(CLAUDE_CHECKLIST),
2409            "a progress checklist asks for nothing"
2410        );
2411        assert!(
2412            !is_noise(CLAUDE_FINDING),
2413            "a review that names a bug is input, not noise"
2414        );
2415
2416        let mut clean = pr(Checks::Green, &[], 0);
2417        clean.review_comments.push(ReviewComment {
2418            author: "coderabbitai".to_owned(),
2419            path: None,
2420            line: None,
2421            body: CODERABBIT_TRIGGER.to_owned(),
2422        });
2423        clean.review_comments.retain(|c| !is_noise(&c.body));
2424        assert_eq!(decide(&clean, 0, 4, Duration::ZERO), Step::Merge);
2425
2426        let mut found = pr(Checks::Green, &[], 0);
2427        found.review_comments.push(ReviewComment {
2428            author: "claude".to_owned(),
2429            path: None,
2430            line: None,
2431            body: CLAUDE_FINDING.to_owned(),
2432        });
2433        found.review_comments.retain(|c| !is_noise(&c.body));
2434        assert!(matches!(
2435            decide(&found, 0, 4, Duration::ZERO),
2436            Step::Fix { .. }
2437        ));
2438    }
2439
2440    #[test]
2441    fn the_policy_table_holds_for_every_combination_that_matters() {
2442        let cases: Vec<(&str, PrState, usize, usize, Duration, Step)> = vec![
2443            (
2444                "pending checks are waited for, even on the last round",
2445                pr(Checks::Pending, &[], 0),
2446                4,
2447                4,
2448                Duration::ZERO,
2449                Step::Wait,
2450            ),
2451            (
2452                "red checks are fixed",
2453                pr(Checks::Red, &["editorconfig"], 0),
2454                0,
2455                4,
2456                Duration::ZERO,
2457                Step::Fix {
2458                    reason: "1 check(s) failing: editorconfig".to_owned(),
2459                },
2460            ),
2461            (
2462                "green with comments is fixed, not merged",
2463                pr(Checks::Green, &[], 2),
2464                1,
2465                4,
2466                Duration::ZERO,
2467                Step::Fix {
2468                    reason: "checks are green but 2 review comment(s) are unresolved: coderabbitai"
2469                        .to_owned(),
2470                },
2471            ),
2472            (
2473                "green and clean merges",
2474                pr(Checks::Green, &[], 0),
2475                3,
2476                4,
2477                Duration::ZERO,
2478                Step::Merge,
2479            ),
2480            (
2481                "an unreadable rollup is waited on while the grace lasts",
2482                pr(Checks::Unknown, &[], 0),
2483                0,
2484                4,
2485                Duration::ZERO,
2486                Step::Wait,
2487            ),
2488            (
2489                "an unreadable rollup is never merged once the grace is spent",
2490                pr(Checks::Unknown, &[], 0),
2491                0,
2492                4,
2493                CHECKS_GRACE,
2494                Step::GiveUp {
2495                    reason: "no check status is readable on the pull request after 3 minute(s); \
2496                             refusing to merge on a guess"
2497                        .to_owned(),
2498                },
2499            ),
2500        ];
2501        for (what, state, round, budget, waited, want) in cases {
2502            assert_eq!(decide(&state, round, budget, waited), want, "{what}");
2503        }
2504    }
2505
2506    #[test]
2507    fn the_forge_verdict_survives_the_round_trip_from_gh() {
2508        // Read off `gh pr view --json ...,mergeStateStatus`, because a field
2509        // requested but never parsed is the kind of thing that looks wired up
2510        // and answers `Unsaid` forever.
2511        let green = parse_pr(GREEN_OPEN).expect("parse");
2512        assert_eq!(green.blocking, Blocking::No);
2513        let red = parse_pr(RED_OPEN).expect("parse");
2514        assert_eq!(
2515            red.blocking,
2516            Blocking::No,
2517            "`UNSTABLE` is mergeable: the red check is one nobody requires"
2518        );
2519        assert_eq!(red.checks, Checks::Red, "and it is still reported as red");
2520        // A payload from an older `gh` has no such field at all.
2521        let quiet =
2522            parse_pr(&GREEN_OPEN.replace("\"mergeStateStatus\": \"CLEAN\",", "")).expect("parse");
2523        assert_eq!(quiet.blocking, Blocking::Unsaid);
2524    }
2525
2526    #[test]
2527    fn a_red_check_nobody_requires_does_not_buy_a_fix_round() {
2528        // Pull request 37's only red check was `editorconfig`, failing
2529        // because the action could not fetch its own binary after
2530        // editorconfig-checker v4 renamed its release assets. The repository
2531        // does not require it. magi answered by asking a fixer to repair a
2532        // change that was fine, and the pull request had to be merged by hand.
2533        let mut nonblocking = pr(Checks::Red, &["editorconfig", "coverage"], 0);
2534        nonblocking.blocking = Blocking::No;
2535        assert_eq!(
2536            decide(&nonblocking, 0, 4, Duration::ZERO),
2537            Step::Merge,
2538            "the forge says nothing is in the way, so nothing is"
2539        );
2540
2541        // The same red, gated on: that is a fix round, as before.
2542        let mut blocking = pr(Checks::Red, &["test (ubuntu-latest)"], 0);
2543        blocking.blocking = Blocking::Yes;
2544        assert!(matches!(
2545            decide(&blocking, 0, 4, Duration::ZERO),
2546            Step::Fix { .. }
2547        ));
2548
2549        // A review comment still outranks green-enough: a non-required red
2550        // must not become a way to merge past an unanswered reviewer.
2551        let mut commented = pr(Checks::Red, &["coverage"], 1);
2552        commented.blocking = Blocking::No;
2553        assert!(matches!(
2554            decide(&commented, 0, 4, Duration::ZERO),
2555            Step::Fix { .. }
2556        ));
2557
2558        // And silence from the forge is not consent.
2559        let mut unsaid = pr(Checks::Red, &["coverage"], 0);
2560        unsaid.blocking = Blocking::Unsaid;
2561        assert!(matches!(
2562            decide(&unsaid, 0, 4, Duration::ZERO),
2563            Step::Fix { .. }
2564        ));
2565    }
2566
2567    #[test]
2568    fn a_branch_the_base_moved_under_is_rebased_not_fixed() {
2569        // Pull requests 35 and 37 were both rebased by hand: a competition
2570        // that runs for two hours against a repository merging pull requests
2571        // all day conflicts on the way in, and that is arithmetic rather
2572        // than a defect in the change.
2573        let mut conflicted = pr(Checks::Green, &[], 0);
2574        conflicted.blocking = Blocking::Conflict;
2575        assert_eq!(decide(&conflicted, 0, 4, Duration::ZERO), Step::Rebase);
2576
2577        // Decided before the checks, and even with the rounds spent: every
2578        // check on a branch that cannot land is an answer about a state that
2579        // cannot land, and a conflict is not the change's fault.
2580        let mut red = pr(Checks::Red, &["test (ubuntu-latest)"], 2);
2581        red.blocking = Blocking::Conflict;
2582        assert_eq!(decide(&red, 4, 4, Duration::ZERO), Step::Rebase);
2583
2584        // The lifecycle still wins over everything, conflict included.
2585        let mut merged = pr(Checks::Red, &[], 0);
2586        merged.blocking = Blocking::Conflict;
2587        merged.state = PrLifecycle::Merged;
2588        assert_eq!(
2589            decide(&merged, 0, 4, Duration::ZERO),
2590            Step::Done { merged: true }
2591        );
2592    }
2593
2594    #[test]
2595    fn the_forge_verdict_is_read_off_merge_state_status() {
2596        // The spellings that mean "mergeable". `UNSTABLE` is the one that
2597        // matters: mergeable, with a non-required check red or still running.
2598        for ok in ["CLEAN", "UNSTABLE", "unstable", "HAS_HOOKS"] {
2599            assert_eq!(Blocking::of(ok), Blocking::No, "{ok}");
2600            assert!(!Blocking::of(ok).stops_a_merge(), "{ok}");
2601        }
2602        assert_eq!(Blocking::of("DIRTY"), Blocking::Conflict);
2603        assert_eq!(Blocking::of("BLOCKED"), Blocking::Yes);
2604        assert_eq!(Blocking::of("BEHIND"), Blocking::Yes);
2605        // An older `gh`, or a token without the scope, says nothing - and
2606        // refusing to guess is the rule everywhere else in this module.
2607        for quiet in ["", "UNKNOWN"] {
2608            assert_eq!(Blocking::of(quiet), Blocking::Unsaid);
2609            assert!(Blocking::of(quiet).stops_a_merge());
2610        }
2611    }
2612
2613    #[test]
2614    fn a_merge_command_that_failed_after_merging_is_still_a_merge() {
2615        let argv = merge_argv(28, "Merge magi run ec12 (candidate B)");
2616        // The exact stderr from run ec12, in a jj-colocated repository.
2617        let jj = "could not determine current branch: failed to run git: not on any branch";
2618
2619        let landed = merged_after_all(&argv, jj, Some(PrLifecycle::Merged))
2620            .expect("the forge says merged, so it merged");
2621        assert!(landed.ok);
2622        assert!(
2623            landed.detail.contains("but the pull request is merged"),
2624            "the record must not read as a clean success: {}",
2625            landed.detail
2626        );
2627        assert!(
2628            landed.detail.contains("not on any branch"),
2629            "and it must keep what the command actually said: {}",
2630            landed.detail
2631        );
2632
2633        // A pull request still open means the merge really failed.
2634        assert!(merged_after_all(&argv, jj, Some(PrLifecycle::Open)).is_none());
2635        assert!(merged_after_all(&argv, jj, Some(PrLifecycle::Closed)).is_none());
2636        // And an unreadable answer is not evidence of success.
2637        assert!(merged_after_all(&argv, jj, None).is_none());
2638    }
2639
2640    #[test]
2641    fn a_pull_request_closed_underneath_us_is_done_and_not_merged() {
2642        let mut state = pr(Checks::Red, &["editorconfig"], 3);
2643        state.state = PrLifecycle::Closed;
2644        assert_eq!(
2645            decide(&state, 0, 4, Duration::ZERO),
2646            Step::Done { merged: false },
2647            "a human closing the pull request ends the loop, whatever CI says"
2648        );
2649    }
2650
2651    #[test]
2652    fn the_last_round_gives_up_with_a_reason_naming_what_is_still_failing() {
2653        let red = decide(
2654            &pr(Checks::Red, &["editorconfig", "test (macos)"], 0),
2655            4,
2656            4,
2657            Duration::ZERO,
2658        );
2659        match red {
2660            Step::GiveUp { reason } => {
2661                assert!(reason.contains("editorconfig"), "reason: {reason}");
2662                assert!(reason.contains("test (macos)"), "reason: {reason}");
2663                assert!(reason.contains("4 fix round(s)"), "reason: {reason}");
2664            }
2665            other => panic!("expected a give-up, got {other:?}"),
2666        }
2667
2668        let commented = decide(&pr(Checks::Green, &[], 1), 2, 2, Duration::ZERO);
2669        match commented {
2670            Step::GiveUp { reason } => {
2671                assert!(reason.contains("unresolved"), "reason: {reason}");
2672                assert!(reason.contains("2 fix round(s)"), "reason: {reason}");
2673            }
2674            other => panic!("expected a give-up, got {other:?}"),
2675        }
2676    }
2677
2678    #[test]
2679    fn the_merge_command_squashes_deletes_the_branch_and_sets_its_own_subject() {
2680        let candidate_commit = "magi: candidate A (uncommitted work)";
2681        let subject = merge_subject(candidate_commit, "add retries to the uploader");
2682        let argv = merge_argv(16, &subject);
2683
2684        assert!(argv.contains(&"--squash".to_owned()));
2685        assert!(argv.contains(&"--delete-branch".to_owned()));
2686        assert!(argv.contains(&"--subject".to_owned()));
2687        assert_eq!(
2688            argv.last().map(String::as_str),
2689            Some("add retries to the uploader"),
2690            "the subject must not be the candidate commit message"
2691        );
2692        assert_ne!(subject, candidate_commit);
2693    }
2694
2695    #[test]
2696    fn a_real_pull_request_title_is_used_as_the_squash_subject_verbatim() {
2697        assert_eq!(
2698            merge_subject("feat: a queue, an unattended loop, and a phone UI", "task"),
2699            "feat: a queue, an unattended loop, and a phone UI"
2700        );
2701        assert_eq!(
2702            merge_subject("", "# port the retry logic\n\ndetails"),
2703            "port the retry logic",
2704            "an empty title falls back to the task's first line, heading marks stripped"
2705        );
2706    }
2707
2708    #[test]
2709    fn a_failing_checks_details_url_yields_the_job_to_read_logs_from() {
2710        let url = "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323572";
2711        assert_eq!(job_of(url).as_deref(), Some("100114323572"));
2712        assert_eq!(run_of(url).as_deref(), Some("33587406996"));
2713        assert_eq!(job_of("https://coderabbit.ai/status"), None);
2714        assert_eq!(run_of(""), None);
2715    }
2716
2717    #[test]
2718    fn magis_own_stop_comment_is_never_read_back_as_a_finding() {
2719        let mut out = Vec::new();
2720        push_if_outstanding(
2721            &mut out,
2722            ReviewComment {
2723                author: "yukimemi".to_owned(),
2724                path: None,
2725                line: None,
2726                body: format!("{MARKER}\nmagi stopped landing this pull request: 1 check failing"),
2727            },
2728        );
2729        assert!(out.is_empty());
2730    }
2731
2732    /// A run with no tally, so [`RunState::winner`] is `None` and the panel
2733    /// falls back to the repository - which keeps these tests free of a
2734    /// worktree, a `git` invocation and a network.
2735    fn run_state() -> RunState {
2736        RunState::new(
2737            std::path::PathBuf::from("/repo/magi"),
2738            "main".to_owned(),
2739            "abcdef1234".to_owned(),
2740            "add retries to the uploader".to_owned(),
2741            crate::config::Config::default(),
2742        )
2743    }
2744
2745    fn green_pr() -> PrState {
2746        PrState {
2747            url: "https://github.com/yukimemi/magi/pull/42".to_owned(),
2748            number: 42,
2749            state: PrLifecycle::Open,
2750            checks: Checks::Green,
2751            // The forge sees nothing in the way unless a test says otherwise.
2752            blocking: Blocking::No,
2753            failing: Vec::new(),
2754            review_comments: vec![ReviewComment {
2755                author: "coderabbitai".to_owned(),
2756                path: Some("src/land.rs".to_owned()),
2757                line: Some(212),
2758                body: "this branch never checks the exit code".to_owned(),
2759            }],
2760        }
2761    }
2762
2763    const NUMSTAT: &str = "12\t3\tsrc/land.rs\n40\t1\tsrc/web.rs\n-\t-\tassets/logo.png";
2764
2765    fn panel() -> String {
2766        approval_panel(
2767            &run_state(),
2768            &green_pr(),
2769            NUMSTAT,
2770            "diff --git a/src/land.rs b/src/land.rs\n@@ -1,2 +1,2 @@\n-old line\n+new line\n context",
2771            &[
2772                "land: ask before merging".to_owned(),
2773                "land: colour the diff".to_owned(),
2774            ],
2775            "feat: merge approval from the phone",
2776        )
2777    }
2778
2779    #[test]
2780    fn the_approval_panel_carries_the_whole_case_for_the_merge() {
2781        let html = panel();
2782        for needle in [
2783            "42",
2784            "main",
2785            "src/land.rs",
2786            "src/web.rs",
2787            "assets/logo.png",
2788            "feat: merge approval from the phone",
2789            "land: ask before merging",
2790            "land: colour the diff",
2791            "coderabbitai",
2792            "this branch never checks the exit code",
2793            "green",
2794        ] {
2795            assert!(html.contains(needle), "the panel must state `{needle}`");
2796        }
2797    }
2798
2799    #[test]
2800    fn the_approval_panel_contains_nothing_the_frames_policy_would_block() {
2801        let html = panel();
2802        assert!(!html.contains("<script"), "no script survives the csp");
2803        assert!(!html.contains("<form"), "form-action is 'none'");
2804        let pr = green_pr();
2805        assert_eq!(
2806            html.matches("http").count(),
2807            html.matches(pr.url.as_str()).count(),
2808            "the only http url in the panel is the pull request's own link"
2809        );
2810    }
2811
2812    #[test]
2813    fn added_and_removed_diff_lines_are_distinguishable_without_colour() {
2814        let html = panel();
2815        assert!(
2816            html.contains(">+</span>"),
2817            "an added line carries a `+` in the gutter, not only a background"
2818        );
2819        assert!(
2820            html.contains(">-</span>"),
2821            "a removed line carries a `-` in the gutter, not only a background"
2822        );
2823        assert!(
2824            html.contains(">new line</span>"),
2825            "the marker is moved to the gutter, so the body is printed once without it"
2826        );
2827    }
2828
2829    #[test]
2830    fn a_diff_past_the_threshold_is_cut_with_an_honest_count() {
2831        let total = DIFF_MAX_LINES + 100;
2832        let diff: String = (0..total).map(|i| format!("+line {i}\n")).collect();
2833        let html = approval_panel(
2834            &run_state(),
2835            &green_pr(),
2836            NUMSTAT,
2837            &diff,
2838            &[],
2839            "feat: something long",
2840        );
2841        assert!(
2842            html.contains(&format!("100 of {total} diff lines omitted")),
2843            "the note must say exactly how much was cut"
2844        );
2845        assert!(html.contains(&format!("line {}", DIFF_MAX_LINES - 1)));
2846        assert!(
2847            !html.contains(&format!("line {DIFF_MAX_LINES}")),
2848            "nothing past the threshold is rendered"
2849        );
2850        assert!(
2851            html.contains("/repo/magi"),
2852            "the note says where the rest is"
2853        );
2854    }
2855
2856    #[test]
2857    fn a_path_with_html_metacharacters_is_escaped_rather_than_rendered() {
2858        let html = approval_panel(
2859            &run_state(),
2860            &green_pr(),
2861            "1\t2\tsrc/<b>&\"x\"'.rs",
2862            "",
2863            &[],
2864            "subject",
2865        );
2866        assert!(html.contains("src/&lt;b&gt;&amp;&quot;x&quot;&#39;.rs"));
2867        assert!(
2868            !html.contains("<b>"),
2869            "an agent-influenced path must never become markup"
2870        );
2871    }
2872
2873    #[test]
2874    fn only_the_merge_choice_merges_and_silence_holds() {
2875        let table = [
2876            (None, Approval::Hold),
2877            (Some("merge"), Approval::Merge),
2878            (Some(" merge\n"), Approval::Merge),
2879            (Some("hold"), Approval::Hold),
2880            (Some(""), Approval::Hold),
2881            (Some("yes"), Approval::Hold),
2882        ];
2883        for (answer, want) in table {
2884            assert_eq!(
2885                approval(answer),
2886                want,
2887                "answer {answer:?} must resolve to {want:?}"
2888            );
2889        }
2890    }
2891
2892    #[test]
2893    fn the_diffstat_table_is_ordered_by_churn_with_binaries_last() {
2894        let rows = parse_numstat(NUMSTAT);
2895        assert_eq!(
2896            rows.iter().map(|r| r.path.as_str()).collect::<Vec<_>>(),
2897            ["src/web.rs", "src/land.rs", "assets/logo.png"]
2898        );
2899        assert_eq!(rows[2].added, None, "a binary file has no line counts");
2900    }
2901    #[test]
2902    fn the_approval_speaks_the_language_the_repository_is_configured_for() {
2903        // Reported from a real run: the merge question arrived in English on a
2904        // repository with `language = "ja"`. magi's own strings have to follow
2905        // that setting too - "it is a literal in Rust" is not an answer.
2906        let mut state = run_state();
2907        state.config.graph.language = "ja".to_owned();
2908        let pr = green_pr();
2909        let commits = ["c1".to_owned()];
2910
2911        let ja = approval_panel(&state, &pr, "3\t1\tsrc/a.rs", "+ x", &commits, "feat: x");
2912        assert!(ja.contains("lang=\"ja\""), "the document must declare it");
2913        assert!(ja.contains("squash されるコミット"), "{ja}");
2914        assert!(ja.contains("レビューコメント"), "{ja}");
2915        assert!(ja.contains("差分"), "{ja}");
2916        assert!(
2917            !ja.contains("Commits being squashed"),
2918            "no English left over"
2919        );
2920
2921        let w = words("ja");
2922        assert!(w.approval_summary(17, "feat: x").contains("マージ"));
2923        assert!(
2924            w.approval_detail("http://x/1", "main", "feat: x")
2925                .contains("パネル")
2926        );
2927
2928        // The evidence itself is language-neutral and must survive either way.
2929        assert!(ja.contains("src/a.rs"), "the diffstat is not prose");
2930        assert!(ja.contains("feat: x"), "nor is the merge subject");
2931
2932        // English stays the default, and a language magi cannot check falls
2933        // back to it rather than shipping a guess.
2934        state.config.graph.language = "en".to_owned();
2935        let en = approval_panel(&state, &pr, "3\t1\tsrc/a.rs", "+ x", &commits, "feat: x");
2936        assert!(en.contains("Commits being squashed"), "{en}");
2937        assert_eq!(words("Klingon").html_lang, "en");
2938    }
2939}