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::{BTreeMap, BTreeSet};
42use std::fmt::Write as _;
43use std::path::{Path, PathBuf};
44use std::sync::Arc;
45use std::time::Duration;
46
47use anyhow::{Context as _, Result, bail};
48use jiff::Timestamp;
49use serde::{Deserialize, Serialize};
50
51use crate::agent::{self, Invocation, SeatState};
52use crate::ask;
53use crate::config::{AgentSpec, MergeMode};
54use crate::git;
55use crate::proc::Quiet as _;
56use crate::prompt;
57use crate::run::{MergeOutcome, RunState, RunStatus, tail};
58
59/// How often the pull request is re-read while its checks are still running.
60///
61/// Thirty seconds: a CI matrix takes minutes, so anything shorter is spent
62/// entirely on `gh` invocations, and anything much longer adds latency to every
63/// single round of a loop that already waits for agents.
64pub const POLL: Duration = Duration::from_secs(30);
65
66/// How long one wait may last before landing gives up on the checks finishing.
67///
68/// A workflow that has not settled in forty-five minutes is stuck on a runner
69/// queue, a missing approval, or a hung job - none of which more polling fixes,
70/// and all of which a person needs to see.
71pub const WAIT_CEILING: Duration = Duration::from_secs(45 * 60);
72
73/// How long the checks may stay unreadable before landing gives up on them.
74///
75/// GitHub registers a workflow run some seconds after the branch is pushed, so
76/// immediately after a pull request is opened "no checks" and "no CI in this
77/// repository" look identical. Measured on run 01c2: magi opened pull request
78/// 22, read `unknown` four seconds later, refused to merge on a guess and
79/// marked the run blocked - and every check on that pull request was green
80/// minutes afterwards, with the whole competition then re-run from scratch for
81/// a task that was already finished. Three minutes is well past the observed
82/// registration delay and still bounded, so a repository that genuinely has no
83/// checks costs one three-minute wait and then says so.
84pub const CHECKS_GRACE: Duration = Duration::from_secs(3 * 60);
85
86/// Bytes of failing log kept per check. The fixer needs the assertion and the
87/// frame around it, not the forty thousand lines of `cargo` output above it.
88const LOG_TAIL: usize = 4_000;
89
90/// Failing checks whose logs are fetched. Beyond a handful the failures share a
91/// cause, and fetching each one costs a `gh` round trip.
92const MAX_LOGS: usize = 3;
93
94/// Marker carried by every comment magi posts on a pull request.
95///
96/// Without it magi's own "still failing" comment is indistinguishable from a
97/// reviewer's, and the next observation would hand magi's own prose to the
98/// fixer as a finding.
99pub const MARKER: &str = "<!-- magi:land -->";
100
101/// Markers a bot puts in a comment to say that the comment is not a review.
102///
103/// CodeRabbit labels its own machinery in HTML comments - the trigger notice,
104/// the walkthrough summary, the "thanks for using" footer - and its actual
105/// findings arrive as *inline* review comments with a path and a line. Taking
106/// the bot at its word is more honest than guessing from prose, and it is the
107/// difference between a fix round that has something to fix and one that asks
108/// an agent to act on a quota notice.
109const NOT_A_REVIEW: [&str; 3] = [
110    "skip review by coderabbit.ai",
111    "summarize by coderabbit.ai",
112    "<!-- tips_start -->",
113];
114
115/// Where a pull request is in its life.
116#[derive(Debug, Clone, Copy, PartialEq, Eq)]
117pub enum PrLifecycle {
118    /// Still ours to land.
119    Open,
120    /// Already merged, by us or by a person.
121    Merged,
122    /// Closed without merging.
123    Closed,
124}
125
126/// The aggregate verdict of a pull request's checks.
127#[derive(Debug, Clone, Copy, PartialEq, Eq)]
128pub enum Checks {
129    /// At least one check has not finished.
130    Pending,
131    /// Every check passed (a skipped check counts as passed: the review
132    /// workflow skips release and bot pull requests by design).
133    Green,
134    /// At least one check finished without passing.
135    Red,
136    /// Nothing readable - no rollup at all, or a status magi does not know.
137    Unknown,
138}
139
140impl PrLifecycle {
141    /// Stable lower-case name, as the API and the reports spell it.
142    pub fn as_str(self) -> &'static str {
143        match self {
144            Self::Open => "open",
145            Self::Merged => "merged",
146            Self::Closed => "closed",
147        }
148    }
149}
150
151impl Checks {
152    /// Stable lower-case name, as the API and the reports spell it.
153    pub fn as_str(self) -> &'static str {
154        match self {
155            Self::Pending => "pending",
156            Self::Green => "green",
157            Self::Red => "red",
158            Self::Unknown => "unknown",
159        }
160    }
161}
162
163/// One outstanding review comment, human or bot.
164#[derive(Debug, Clone, PartialEq, Eq)]
165pub struct ReviewComment {
166    /// Login of whoever wrote it.
167    pub author: String,
168    /// File it was left on, for inline review comments.
169    pub path: Option<String>,
170    /// Line it was left on, when the comment is inline and still anchored.
171    pub line: Option<u64>,
172    /// The comment itself, as written.
173    pub body: String,
174}
175
176/// One observation of a pull request.
177#[derive(Debug, Clone, PartialEq, Eq)]
178pub struct PrState {
179    /// Pull request url, as `gh` reports it.
180    pub url: String,
181    /// Pull request number.
182    pub number: u64,
183    /// Open, merged, or closed.
184    pub state: PrLifecycle,
185    /// Aggregate check verdict.
186    pub checks: Checks,
187    /// Names of the checks that finished without passing.
188    pub failing: Vec<String>,
189    /// Comments that still want an answer, human and bot.
190    pub review_comments: Vec<ReviewComment>,
191    /// Whether the forge itself considers the failures blocking.
192    pub blocking: Blocking,
193}
194
195/// Whether a failing check actually stands between the pull request and
196/// `main`, according to the forge.
197///
198/// The rollup lists every check equally, so `coverage` going red on a
199/// repository that deliberately does not require it looked exactly like a
200/// broken build - and magi answered by spending a fix round on a change that
201/// was fine. Pull request 37 had to be merged by hand for that reason: the
202/// only red check was `editorconfig`, which was failing because the *action*
203/// could not fetch its own binary, and which the repository does not require.
204///
205/// `mergeStateStatus` is where GitHub applies the required-check set, so it
206/// is the one field that can tell the difference.
207#[derive(Debug, Clone, Copy, PartialEq, Eq)]
208pub enum Blocking {
209    /// Required checks are satisfied and the branch merges cleanly.
210    No,
211    /// Something required is failing or missing.
212    Yes,
213    /// The branch no longer merges: the base moved under it.
214    Conflict,
215    /// The forge did not say - an older `gh`, or a token without the scope.
216    /// Treated as `Yes`, because refusing to guess is the rule everywhere
217    /// else in this module.
218    Unsaid,
219}
220
221impl Blocking {
222    /// Read `mergeStateStatus`, which is upper-case in `gh`'s output.
223    fn of(raw: &str) -> Self {
224        match raw.to_ascii_uppercase().as_str() {
225            // Mergeable. `UNSTABLE` is the interesting one: mergeable, with a
226            // non-required check failing or still running.
227            "CLEAN" | "UNSTABLE" | "HAS_HOOKS" => Self::No,
228            "DIRTY" => Self::Conflict,
229            "" | "UNKNOWN" => Self::Unsaid,
230            // BLOCKED, BEHIND, DRAFT: something has to change first.
231            _ => Self::Yes,
232        }
233    }
234
235    /// Does this stand between the pull request and the base branch?
236    #[must_use]
237    pub fn stops_a_merge(self) -> bool {
238        !matches!(self, Self::No)
239    }
240}
241
242/// What the loop decided to do next. Pure, so the policy is testable.
243#[derive(Debug, Clone, PartialEq, Eq)]
244pub enum Step {
245    /// Checks are still running; re-read the pull request after [`POLL`].
246    Wait,
247    /// The base moved and the branch no longer merges: rebase it.
248    ///
249    /// Not a fix round. Nothing is wrong with the change - a competition
250    /// that runs for two hours against a repository merging pull requests
251    /// all day conflicts on the way in, and that is arithmetic rather than a
252    /// defect. Pull requests 35 and 37 were both rebased by hand for exactly
253    /// this.
254    Rebase,
255    /// Red checks or unresolved comments; run a fix round.
256    Fix {
257        /// What is unhappy, in one line, for the run log and the fix prompt.
258        reason: String,
259    },
260    /// Green and nothing outstanding; merge it.
261    Merge,
262    /// The pull request left our hands.
263    Done {
264        /// Did it land, or was it closed?
265        merged: bool,
266    },
267    /// Stop and leave the pull request to a person.
268    GiveUp {
269        /// Why magi stopped, in one line.
270        reason: String,
271    },
272}
273
274/// The outcome to record when `gh pr merge` exits non-zero, given what the
275/// pull request looked like immediately afterwards.
276///
277/// `gh pr merge` merges server-side first and only then does local work -
278/// deleting the branch, switching back to a base branch - so a non-zero exit
279/// does not mean the merge did not happen. In a jj-colocated repository it
280/// reliably does not mean that: git HEAD is detached, and `--delete-branch`
281/// ends with "could not determine current branch: not on any branch" *after*
282/// the merge has landed. Run ec12 merged pull request 28 into `main` and
283/// recorded `ok: false`, and its task was held waiting for a merge that was
284/// already done.
285///
286/// So the forge is asked, and its answer wins - the same authority [`decide`]
287/// gives the pull request's own state over everything else. The recorded
288/// detail carries both facts, because "the command failed and the merge
289/// happened anyway" is exactly what someone reading the run later needs to
290/// know.
291///
292/// `None` means the merge really did not happen, including when the pull
293/// request could not be read at all: an unreadable answer is not evidence of
294/// success.
295pub(crate) fn merged_after_all(
296    argv: &[String],
297    stderr: &str,
298    after: Option<PrLifecycle>,
299) -> Option<MergeOutcome> {
300    if after? != PrLifecycle::Merged {
301        return None;
302    }
303    Some(MergeOutcome {
304        mode: MergeMode::Pr,
305        ok: true,
306        detail: format!(
307            "gh {} (the command reported `{}`, but the pull request is merged)",
308            argv.join(" "),
309            stderr.trim()
310        ),
311    })
312}
313
314/// Decide the next step. No I/O.
315///
316/// `round` counts the fix rounds already spent, so `round == budget` means the
317/// budget is gone. A wait never spends a round: waiting is free, and a slow CI
318/// must not consume the allowance meant for actual fixes.
319///
320/// The order of the tests is the policy:
321///
322/// 1. **The pull request's own state wins.** A merge or a close that happened
323///    underneath us is the end of the story regardless of what the checks say.
324/// 2. **Pending beats red.** A check that is still running may yet fail, and one
325///    fix round that addresses every failure is cheaper than two that each
326///    address half - the fix pushes and restarts the whole suite anyway.
327/// 3. **Comments outrank green.** An unresolved comment holds the merge even
328///    when CI is happy; that is what a review is for.
329/// 4. **Unreadable is not absent.** Checks that cannot be read yet are waited
330///    on for [`CHECKS_GRACE`], because a pull request opened a moment ago has
331///    not been given its workflow runs yet. Past the grace they are treated as
332///    genuinely missing and magi stops rather than merge on a guess.
333pub fn decide(pr: &PrState, round: usize, budget: usize, waited: Duration) -> Step {
334    match pr.state {
335        PrLifecycle::Merged => return Step::Done { merged: true },
336        PrLifecycle::Closed => return Step::Done { merged: false },
337        PrLifecycle::Open => {}
338    }
339
340    // Before the checks: every check on a branch that cannot land is an
341    // answer about a state that cannot land.
342    if pr.blocking == Blocking::Conflict {
343        return Step::Rebase;
344    }
345
346    let spent = round >= budget;
347    match pr.checks {
348        Checks::Pending => Step::Wait,
349        Checks::Unknown if waited < CHECKS_GRACE => Step::Wait,
350        Checks::Unknown => Step::GiveUp {
351            reason: format!(
352                "no check status is readable on the pull request after {} minute(s); \
353                 refusing to merge on a guess",
354                CHECKS_GRACE.as_secs() / 60
355            ),
356        },
357        // Red, but the forge says it does not stand in the way: the failing
358        // checks are ones this repository chose not to require. Spending a fix
359        // round on them asks an agent to repair something nobody is gating on
360        // - and pull request 37's only red check was an *action* that could
361        // not fetch its own binary. Merge, and name them so the record is
362        // honest about what was red when it landed.
363        Checks::Red if !pr.blocking.stops_a_merge() && pr.review_comments.is_empty() => Step::Merge,
364        Checks::Red => {
365            let what = format!(
366                "{} check(s) failing: {}",
367                pr.failing.len(),
368                pr.failing.join(", ")
369            );
370            if spent {
371                Step::GiveUp {
372                    reason: format!("{what} — still red after {budget} fix round(s)"),
373                }
374            } else {
375                Step::Fix { reason: what }
376            }
377        }
378        Checks::Green if pr.review_comments.is_empty() => Step::Merge,
379        Checks::Green => {
380            let what = format!(
381                "checks are green but {} review comment(s) are unresolved: {}",
382                pr.review_comments.len(),
383                authors(&pr.review_comments)
384            );
385            if spent {
386                Step::GiveUp {
387                    reason: format!("{what} — still unresolved after {budget} fix round(s)"),
388                }
389            } else {
390                Step::Fix { reason: what }
391            }
392        }
393    }
394}
395
396/// Distinct comment authors, in the order they first appear.
397fn authors(comments: &[ReviewComment]) -> String {
398    let mut seen: Vec<&str> = Vec::new();
399    for c in comments {
400        if !seen.contains(&c.author.as_str()) {
401            seen.push(&c.author);
402        }
403    }
404    seen.join(", ")
405}
406
407/// The argv magi merges with, minus the program name.
408///
409/// `--subject` is the point of this function existing: see the module docs.
410pub fn merge_argv(number: u64, subject: &str) -> Vec<String> {
411    vec![
412        "pr".to_owned(),
413        "merge".to_owned(),
414        number.to_string(),
415        "--squash".to_owned(),
416        "--delete-branch".to_owned(),
417        "--subject".to_owned(),
418        subject.to_owned(),
419    ]
420}
421
422/// The squash subject to merge under.
423///
424/// The pull request title, unless it is empty or is a candidate branch's commit
425/// subject that leaked into the title - in which case the task's own first line
426/// is used, because `magi: candidate A (uncommitted work)` in `main` tells a
427/// reader nothing about what landed.
428pub fn merge_subject(pr_title: &str, instruction: &str) -> String {
429    let title = pr_title.trim();
430    if !title.is_empty() && !title.starts_with("magi: candidate") {
431        return title.to_owned();
432    }
433    let first = instruction
434        .lines()
435        .map(str::trim)
436        .find(|l| !l.is_empty())
437        .unwrap_or("magi: land the winning candidate");
438    first.trim_start_matches(['#', ' ']).to_owned()
439}
440
441/// The choice that lets the merge happen, verbatim as the owner taps it.
442pub const APPROVE: &str = "merge";
443
444/// The choice that leaves the pull request open.
445pub const HOLD: &str = "hold";
446
447/// Graph node recorded on the approval question.
448///
449/// The phone keys its high-stakes card off this rather than off the choice
450/// strings, so renaming a button cannot silently downgrade the card that
451/// guards the one irreversible action magi takes.
452pub const APPROVAL_NODE: &str = "land-approval";
453
454/// Unified diff lines carried in the panel before it is truncated.
455///
456/// Four hundred: the panel is read on a 390px phone, where a diff line often
457/// wraps to two rows, so this is already a few thousand rows of scrolling -
458/// past that nobody is reading, and the bytes still count against the panel's
459/// 8 MiB cap. A larger diff is not hidden: the note says how many lines were
460/// cut and which worktree holds the whole patch.
461pub const DIFF_MAX_LINES: usize = 400;
462
463/// What the owner's answer to the approval question means.
464#[derive(Debug, Clone, Copy, PartialEq, Eq)]
465pub enum Approval {
466    /// The owner said [`APPROVE`]. Merge.
467    Merge,
468    /// Anything else, including silence. Leave the pull request open.
469    Hold,
470}
471
472/// Read the owner's answer, where `None` is an unanswered question.
473///
474/// Silence is a hold. A timed-out question means the owner never saw it or
475/// never decided, and defaulting an irreversible merge to "yes" would make this
476/// gate worse than no gate at all: it would merge unattended while claiming to
477/// have asked. Only the exact [`APPROVE`] choice merges, so an answer this
478/// function does not recognise holds too.
479pub fn approval(answer: Option<&str>) -> Approval {
480    match answer {
481        Some(a) if a.trim().eq_ignore_ascii_case(APPROVE) => Approval::Merge,
482        _ => Approval::Hold,
483    }
484}
485
486/// What [`approval_gate`] found on one check of the owner's merge decision.
487#[derive(Debug, Clone, Copy, PartialEq, Eq)]
488enum ApprovalGate {
489    /// The owner said [`APPROVE`]. Merge.
490    Approved,
491    /// The owner said anything else, the question timed out, or it was
492    /// closed with no decision recorded.
493    Held,
494    /// Filed and still waiting - the caller parks rather than blocking on it.
495    Pending,
496}
497
498/// Escape text for HTML, including both quote characters.
499///
500/// Every string in the panel is agent-influenced: a branch name, a file path, a
501/// commit subject, a review comment. The sandboxed frame stops such text from
502/// *running*, but it does not stop a `<` from ending the document early or a
503/// `"` from ending an attribute and inventing a new one - the panel would then
504/// render a lie, or not render at all. Both quotes are escaped because the same
505/// function is used inside attributes, where remembering which quote style the
506/// caller used is one mistake away from an injected attribute.
507fn esc(s: &str) -> String {
508    let mut out = String::with_capacity(s.len());
509    for c in s.chars() {
510        match c {
511            '&' => out.push_str("&amp;"),
512            '<' => out.push_str("&lt;"),
513            '>' => out.push_str("&gt;"),
514            '"' => out.push_str("&quot;"),
515            '\'' => out.push_str("&#39;"),
516            _ => out.push(c),
517        }
518    }
519    out
520}
521
522/// One row of the diffstat table.
523#[derive(Debug, Clone, PartialEq, Eq)]
524struct StatRow {
525    path: String,
526    /// `None` for a binary file, which `git` reports as `-`.
527    added: Option<u64>,
528    removed: Option<u64>,
529}
530
531impl StatRow {
532    /// Lines touched, for sorting. A binary file counts as zero rather than as
533    /// unknown, which puts it at the bottom where it needs no attention.
534    fn churn(&self) -> u64 {
535        self.added.unwrap_or(0) + self.removed.unwrap_or(0)
536    }
537}
538
539/// Parse `git diff --numstat` into rows, biggest churn first.
540///
541/// `--numstat` and not `--stat`: the `+++---` bar in `--stat` is *scaled* to the
542/// terminal width, so counting its characters would print fabricated numbers in
543/// the one table an operator approves an irreversible action from.
544fn parse_numstat(numstat: &str) -> Vec<StatRow> {
545    let mut rows: Vec<StatRow> = numstat
546        .lines()
547        .filter_map(|line| {
548            let mut parts = line.splitn(3, '\t');
549            let added = parts.next()?.trim();
550            let removed = parts.next()?.trim();
551            let path = parts.next()?.trim();
552            if path.is_empty() {
553                return None;
554            }
555            Some(StatRow {
556                path: path.to_owned(),
557                added: added.parse().ok(),
558                removed: removed.parse().ok(),
559            })
560        })
561        .collect();
562    // Path breaks the tie so the same change always renders the same table; an
563    // operator comparing two panels should not see rows shuffle.
564    rows.sort_by(|a, b| b.churn().cmp(&a.churn()).then_with(|| a.path.cmp(&b.path)));
565    rows
566}
567
568/// How one diff line is shown: a gutter character, a style, and the body to
569/// print - which is the line minus its marker, so the marker appears exactly
570/// once, in the gutter.
571///
572/// The gutter is why this exists at all. The operator may be colour blind, or
573/// reading in sunlight with the screen dimmed, so an added line is never
574/// distinguished by its background alone: `+` and `-` sit in a fixed column,
575/// the same mark they already read in a terminal.
576fn diff_row(line: &str) -> (&'static str, &'static str, &str) {
577    if line.starts_with("+++") || line.starts_with("---") {
578        (" ", "color:#57606a;font-weight:600", line)
579    } else if let Some(body) = line.strip_prefix('+') {
580        ("+", "background:#e6ffec;color:#0a3622", body)
581    } else if let Some(body) = line.strip_prefix('-') {
582        ("-", "background:#ffebe9;color:#5c1a17", body)
583    } else if line.starts_with("@@") {
584        ("~", "background:#eef2ff;color:#3730a3", line)
585    } else if let Some(body) = line.strip_prefix(' ') {
586        (" ", "", body)
587    } else {
588        (" ", "color:#57606a;font-weight:600", line)
589    }
590}
591
592/// The handful of words the approval panel says in its own voice.
593///
594/// magi's own text, not an agent's, so `[graph] language` has to reach it too:
595/// the operator asked why the merge question spoke English on a repository
596/// configured for Japanese, and "because that string is a literal in Rust" is
597/// not an answer. Only the languages magi can actually check are translated;
598/// anything else falls back to English rather than shipping a guess, and that
599/// fallback is deliberate.
600struct Words {
601    html_lang: &'static str,
602    task: &'static str,
603    what_changed: &'static str,
604    review_verdict: &'static str,
605    reviewer: &'static str,
606    reviewer_no_answer: &'static str,
607    checks: &'static str,
608    nothing_failing: &'static str,
609    files_changed: &'static str,
610    commits: &'static str,
611    no_commits: &'static str,
612    comments: &'static str,
613    no_comments: &'static str,
614    diff: &'static str,
615    truncated: &'static str,
616    lands_as: &'static str,
617}
618
619const EN: Words = Words {
620    html_lang: "en",
621    task: "Task",
622    what_changed: "What changed",
623    review_verdict: "Review verdict",
624    reviewer: "Reviewer",
625    reviewer_no_answer: "produced no answer",
626    checks: "Checks",
627    nothing_failing: "Nothing failing.",
628    files_changed: "file(s) changed",
629    commits: "Commits being squashed",
630    no_commits: "No commit subjects could be read from the branch.",
631    comments: "Review comments",
632    no_comments: "Nothing outstanding at this observation.",
633    diff: "Diff",
634    truncated: "Truncated",
635    lands_as: "They land as one commit titled",
636};
637
638const JA: Words = Words {
639    html_lang: "ja",
640    task: "タスク",
641    what_changed: "変更内容",
642    review_verdict: "レビューの結論",
643    reviewer: "レビュアー",
644    reviewer_no_answer: "回答なし",
645    checks: "チェック",
646    nothing_failing: "失敗しているものはありません。",
647    files_changed: "ファイル変更",
648    commits: "squash されるコミット",
649    no_commits: "ブランチからコミット件名を読めませんでした。",
650    comments: "レビューコメント",
651    no_comments: "この時点で未対応のものはありません。",
652    diff: "差分",
653    truncated: "省略",
654    lands_as: "これらは次の件名の1コミットとして入ります:",
655};
656
657impl Words {
658    /// The clause after the merge subject. Split out because word order moves:
659    /// Japanese puts the subject before the verb, so a shared template with a
660    /// hole in the middle would read as machine translation.
661    fn lands_as_tail(&self) -> &'static str {
662        if self.html_lang == "ja" {
663            "。この件名も承認の対象です。"
664        } else {
665            ", which you are approving too."
666        }
667    }
668
669    /// The question's own one-line summary, which is what a phone shows first.
670    fn approval_summary(&self, number: u64, subject: &str) -> String {
671        if self.html_lang == "ja" {
672            format!("プルリクエスト #{number} をマージ: {subject}")
673        } else {
674            format!("merge pull request #{number}: {subject}")
675        }
676    }
677
678    /// The body under the summary, above the panel.
679    fn approval_detail(&self, url: &str, base: &str, subject: &str) -> String {
680        if self.html_lang == "ja" {
681            format!(
682                "{url} はチェックが緑で、`{base}` へ `{subject}` として squash \
683                 できる状態です。差分の要約・パッチ・squash されるコミットは\
684                 下のパネルにあります。"
685            )
686        } else {
687            format!(
688                "{url} is green and ready to squash into `{base}` as `{subject}`. \
689                 The panel holds the diffstat, the patch and the commits being squashed."
690            )
691        }
692    }
693
694    /// The truncation note, written whole in each language for the same reason.
695    fn truncated_note(
696        &self,
697        omitted: usize,
698        total: usize,
699        shown: usize,
700        where_: &str,
701        base: &str,
702        head: &str,
703    ) -> String {
704        if self.html_lang == "ja" {
705            format!(
706                "先頭 {shown} 行のあと、差分 {total} 行のうち {omitted} 行を省略しました。\
707                 全体は <code>{where_}</code>(<code>git diff {base}...{head}</code>)と\
708                 プルリクエストにあります。"
709            )
710        } else {
711            format!(
712                "{omitted} of {total} diff lines omitted after the first {shown}. \
713                 The whole patch is in <code>{where_}</code> \
714                 (<code>git diff {base}...{head}</code>) and on the pull request."
715            )
716        }
717    }
718}
719
720/// Pick the panel's language. Codes and names both, because `[graph] language`
721/// has always accepted either.
722fn words(language: &str) -> &'static Words {
723    let l = language.trim();
724    if l.eq_ignore_ascii_case("ja")
725        || l.eq_ignore_ascii_case("jp")
726        || l.eq_ignore_ascii_case("japanese")
727        || l.eq_ignore_ascii_case("日本語")
728    {
729        &JA
730    } else {
731        &EN
732    }
733}
734
735/// The approval panel's html: what is about to land, and the evidence for it.
736///
737/// Pure, so the whole document is asserted in tests without `gh`, without a
738/// network and without a repository. The caller gathers `diffstat`
739/// (`git diff --numstat`), `diff` (the unified patch), `commits` (the subjects
740/// being squashed) and `subject` (what the squash will be called) from the
741/// winner's worktree.
742///
743/// It emits no `<script>`, no `<form>` and no remote url, because the frame's
744/// content security policy blocks all three: anything of the sort here would be
745/// dead markup that misleads the next reader into thinking it works.
746pub fn approval_panel(
747    state: &RunState,
748    pr: &PrState,
749    diffstat: &str,
750    diff: &str,
751    commits: &[String],
752    subject: &str,
753) -> String {
754    let rows = parse_numstat(diffstat);
755    let w = words(&state.config.graph.language);
756    let mut h = String::with_capacity(4_096 + diff.len().min(200_000));
757
758    let _ = writeln!(
759        h,
760        "<!doctype html>\n<html lang=\"{}\">\n<head>\n<meta charset=\"utf-8\">\n\
761         <meta name=\"viewport\" content=\"width=device-width, initial-scale=1\">",
762        w.html_lang
763    );
764    let _ = writeln!(
765        h,
766        "<title>merge #{} — {}</title>\n</head>",
767        pr.number,
768        esc(subject)
769    );
770    h.push_str(
771        "<body style=\"margin:0;padding:12px;font:15px/1.5 -apple-system,\
772         'Segoe UI',system-ui,sans-serif;color:#1f2328;background:#fff;\
773         word-break:break-word\">\n",
774    );
775
776    // The decision, in the words the operator is approving.
777    let _ = writeln!(
778        h,
779        "<h1 style=\"margin:0 0 4px;font-size:19px\">Merge #{} into \
780         <code style=\"background:#f6f8fa;padding:1px 4px;border-radius:4px\">{}</code></h1>\n\
781         <p style=\"margin:0 0 4px;font-size:17px;font-weight:600\">{}</p>\n\
782         <p style=\"margin:0 0 12px;font-size:13px;color:#57606a\">squash merge · run {} · \
783         <a href=\"{}\" style=\"color:#0969da\">{}</a></p>",
784        pr.number,
785        esc(&state.base_branch),
786        esc(subject),
787        esc(&state.id),
788        esc(&pr.url),
789        esc(&pr.url),
790    );
791
792    // The task, verbatim: the operator's own words for what was asked, so the
793    // panel does not make them reconstruct the request from a diffstat.
794    let _ = writeln!(
795        h,
796        "<h2 style=\"margin:16px 0 6px;font-size:15px\">{}</h2>\n\
797         <p style=\"margin:0;font-size:13px;white-space:pre-wrap\">{}</p>",
798        w.task,
799        esc(&state.instruction)
800    );
801
802    // The winner's own account of what it did and why, when there is one.
803    if let Some(summary) = state
804        .winner()
805        .map(|c| c.summary.as_str())
806        .filter(|s| !s.is_empty())
807    {
808        let _ = writeln!(
809            h,
810            "<h2 style=\"margin:16px 0 6px;font-size:15px\">{}</h2>\n\
811             <p style=\"margin:0;font-size:13px;white-space:pre-wrap\">{}</p>",
812            w.what_changed,
813            esc(summary)
814        );
815    }
816
817    // The verdict from the round that actually cleared this for merge - the
818    // last one, since only that round's word is still standing.
819    if let Some(round) = state.reviews.last() {
820        let _ = writeln!(
821            h,
822            "<h2 style=\"margin:16px 0 6px;font-size:15px\">{}</h2>",
823            w.review_verdict
824        );
825        for r in &round.reviews {
826            // A seat the review loop counted as answered has real prose in
827            // `summary`; one it counted against `incomplete` (see
828            // `graph::Runner::review_loop`) never produced any and left it
829            // empty - which must not be read back as a blank verdict, since
830            // an empty box here looks like "nothing to say" rather than
831            // "never answered".
832            let body = match &r.failed {
833                Some(reason) => format!("{}: {}", w.reviewer_no_answer, esc(reason)),
834                None => esc(&r.summary),
835            };
836            let _ = writeln!(
837                h,
838                "<div style=\"margin:0 0 8px;padding:8px;background:#f6f8fa;\
839                 border-radius:6px\">\
840                 <div style=\"font-size:12px;color:#57606a\">{} {} · {}</div>\
841                 <div style=\"white-space:pre-wrap;font-size:13px\">{}</div></div>",
842                w.reviewer,
843                r.reviewer,
844                esc(&r.agent),
845                body,
846            );
847        }
848    }
849
850    let _ = writeln!(
851        h,
852        "<h2 style=\"margin:16px 0 6px;font-size:15px\">{}: {}</h2>",
853        w.checks,
854        esc(pr.checks.as_str())
855    );
856    if pr.failing.is_empty() {
857        let _ = writeln!(
858            h,
859            "<p style=\"margin:0;font-size:13px;color:#57606a\">{}</p>",
860            w.nothing_failing
861        );
862    } else {
863        h.push_str("<ul style=\"margin:0;padding-left:20px;font-size:13px\">\n");
864        for f in &pr.failing {
865            let _ = writeln!(h, "<li>{}</li>", esc(f));
866        }
867        h.push_str("</ul>\n");
868    }
869
870    // Diffstat as a real table, so a phone reads what moved without scrolling
871    // sideways through a terminal bar chart.
872    let _ = writeln!(
873        h,
874        "<h2 style=\"margin:16px 0 6px;font-size:15px\">{} {}</h2>",
875        rows.len(),
876        w.files_changed
877    );
878    h.push_str(
879        "<table style=\"width:100%;border-collapse:collapse;font-size:13px\">\n\
880         <thead><tr>\
881         <th style=\"text-align:left;border-bottom:1px solid #d0d7de;padding:4px 2px\">file</th>\
882         <th style=\"text-align:right;border-bottom:1px solid #d0d7de;padding:4px 2px\">added</th>\
883         <th style=\"text-align:right;border-bottom:1px solid #d0d7de;padding:4px 2px\">removed\
884         </th></tr></thead>\n<tbody>\n",
885    );
886    let mut total_added = 0u64;
887    let mut total_removed = 0u64;
888    for r in &rows {
889        total_added += r.added.unwrap_or(0);
890        total_removed += r.removed.unwrap_or(0);
891        let cell = |n: Option<u64>| match n {
892            Some(n) => n.to_string(),
893            None => "bin".to_owned(),
894        };
895        let _ = writeln!(
896            h,
897            "<tr>\
898             <td style=\"padding:4px 2px;border-bottom:1px solid #eaeef2;\
899             font-family:ui-monospace,monospace\">{}</td>\
900             <td style=\"padding:4px 2px;border-bottom:1px solid #eaeef2;text-align:right;\
901             color:#0a3622\">{}</td>\
902             <td style=\"padding:4px 2px;border-bottom:1px solid #eaeef2;text-align:right;\
903             color:#5c1a17\">{}</td></tr>",
904            esc(&r.path),
905            cell(r.added),
906            cell(r.removed),
907        );
908    }
909    let _ = writeln!(
910        h,
911        "</tbody>\n<tfoot><tr style=\"font-weight:600\">\
912         <td style=\"padding:4px 2px\">total</td>\
913         <td style=\"padding:4px 2px;text-align:right\">{total_added}</td>\
914         <td style=\"padding:4px 2px;text-align:right\">{total_removed}</td>\
915         </tr></tfoot>\n</table>"
916    );
917
918    // The commits being squashed, and the subject that replaces them.
919    let _ = writeln!(
920        h,
921        "<h2 style=\"margin:16px 0 6px;font-size:15px\">{}</h2>",
922        w.commits
923    );
924    if commits.is_empty() {
925        h.push_str(&format!(
926            "<p style=\"margin:0;font-size:13px;color:#57606a\">{}</p>\n",
927            w.no_commits
928        ));
929    } else {
930        h.push_str("<ol style=\"margin:0;padding-left:20px;font-size:13px\">\n");
931        for c in commits {
932            let _ = writeln!(h, "<li>{}</li>", esc(c));
933        }
934        h.push_str("</ol>\n");
935    }
936    let _ = writeln!(
937        h,
938        "<p style=\"margin:8px 0 0;font-size:13px\">{} <strong>{}</strong>{}</p>",
939        w.lands_as,
940        esc(subject),
941        w.lands_as_tail()
942    );
943
944    // The review comments that shaped this branch, and who asked for them.
945    let _ = writeln!(
946        h,
947        "<h2 style=\"margin:16px 0 6px;font-size:15px\">{}</h2>",
948        w.comments
949    );
950    if pr.review_comments.is_empty() {
951        h.push_str(&format!(
952            "<p style=\"margin:0;font-size:13px;color:#57606a\">{}</p>\n",
953            w.no_comments
954        ));
955    } else {
956        for c in &pr.review_comments {
957            let anchor = match (&c.path, c.line) {
958                (Some(p), Some(l)) => format!("{p}:{l}"),
959                (Some(p), None) => p.clone(),
960                _ => "pull request thread".to_owned(),
961            };
962            let _ = writeln!(
963                h,
964                "<div style=\"margin:0 0 8px;padding:8px;background:#f6f8fa;border-radius:6px\">\
965                 <div style=\"font-size:12px;color:#57606a\">{} · {}</div>\
966                 <div style=\"white-space:pre-wrap;font-size:13px\">{}</div></div>",
967                esc(&c.author),
968                esc(&anchor),
969                esc(&tail(&c.body, 800)),
970            );
971        }
972    }
973
974    // The patch itself.
975    let total = diff.lines().count();
976    let shown = total.min(DIFF_MAX_LINES);
977    let _ = writeln!(
978        h,
979        "<h2 style=\"margin:16px 0 6px;font-size:15px\">{}</h2>",
980        w.diff
981    );
982    h.push_str(
983        "<div style=\"font:12px/1.45 ui-monospace,SFMono-Regular,Menlo,monospace;\
984         border:1px solid #d0d7de;border-radius:6px;overflow-x:auto\">\n",
985    );
986    for line in diff.lines().take(shown) {
987        let (gutter, style, body) = diff_row(line);
988        let _ = writeln!(
989            h,
990            "<div style=\"display:flex;{style}\">\
991             <span style=\"flex:0 0 1.4em;text-align:center;user-select:none;\
992             border-right:1px solid #d0d7de\">{gutter}</span>\
993             <span style=\"white-space:pre;padding-left:6px\">{}</span></div>",
994            esc(body),
995        );
996    }
997    h.push_str("</div>\n");
998    if total > shown {
999        let omitted = total - shown;
1000        let head = state.winner().map_or("HEAD", |w| w.branch.as_str());
1001        let where_ = state.winner().map_or_else(
1002            || state.repo.display().to_string(),
1003            |w| w.worktree.display().to_string(),
1004        );
1005        let _ = writeln!(
1006            h,
1007            "<p style=\"margin:8px 0 0;padding:8px;background:#fff8c5;border-radius:6px;\
1008             font-size:13px\">{}: {}</p>",
1009            w.truncated,
1010            w.truncated_note(
1011                omitted,
1012                total,
1013                shown,
1014                &esc(&where_),
1015                &esc(&state.base_branch),
1016                &esc(head),
1017            ),
1018        );
1019    }
1020
1021    h.push_str("</body>\n</html>\n");
1022    h
1023}
1024
1025/// Ask the owner before merging, with the whole case attached as a panel.
1026///
1027/// The evidence is gathered from the winner's own worktree with the `git` CLI,
1028/// never from the network, so a phone on a slow link gets the diff magi is
1029/// looking at rather than a link it has to go and open.
1030///
1031/// Never blocks. `land` used to sit inside [`ask::ask_and_wait`]'s poll loop
1032/// for up to a day right here, which held the whole run's task claim - and
1033/// the daemon's one slot with it - for exactly as long as the owner took to
1034/// notice their phone. [`ApprovalGate::Pending`] is the answer that lets the
1035/// caller park the run and hand the slot back instead: the question is on
1036/// disk either way, so nothing about the wait itself changes, only who is
1037/// blocked on it.
1038///
1039/// Idempotent across resumes: called again for a run already waiting on its
1040/// own question, this finds that question by [`crate::ask::Questions::list`]
1041/// rather than filing a second one - asking twice would double the
1042/// notification for one decision, and leave the first question's panel an
1043/// orphan nobody's answer ever reaches.
1044async fn approval_gate(state: &mut RunState, pr: &PrState, subject: &str) -> Result<ApprovalGate> {
1045    let store = ask::Questions::open();
1046    let existing = store
1047        .list()
1048        .into_iter()
1049        .filter(|q| q.run == state.id && q.node == APPROVAL_NODE)
1050        .max_by(|a, b| a.id.cmp(&b.id));
1051
1052    let q = match existing {
1053        Some(q) => q,
1054        None => {
1055            let (worktree, head) = match state.winner() {
1056                Some(w) => (w.worktree.clone(), w.branch.clone()),
1057                None => (state.repo.clone(), "HEAD".to_owned()),
1058            };
1059            let base = state.base_branch.clone();
1060            let range = format!("{base}...{head}");
1061            // A failed `git` must not decide the merge: the panel degrades to
1062            // less evidence and the owner still chooses. Merging because the
1063            // diff could not be read would be the worst of both.
1064            let numstat = git::git_raw(&worktree, &["diff", "--numstat", "-M", &range])
1065                .await
1066                .map(|o| o.stdout)
1067                .unwrap_or_default();
1068            let diff = git::diff(&worktree, &base, &head).await.unwrap_or_default();
1069            let commits: Vec<String> = git::git_raw(
1070                &worktree,
1071                &[
1072                    "log",
1073                    "--reverse",
1074                    "--format=%s",
1075                    &format!("{base}..{head}"),
1076                ],
1077            )
1078            .await
1079            .map(|o| o.stdout)
1080            .unwrap_or_default()
1081            .lines()
1082            .filter(|l| !l.trim().is_empty())
1083            .map(str::to_owned)
1084            .collect();
1085
1086            let w = words(&state.config.graph.language);
1087            let html = approval_panel(state, pr, &numstat, &diff, &commits, subject);
1088            let mut fresh = ask::Question::new(
1089                state.id.clone(),
1090                APPROVAL_NODE.to_owned(),
1091                "land".to_owned(),
1092                w.approval_summary(pr.number, subject),
1093                w.approval_detail(&pr.url, &base, subject),
1094                vec![APPROVE.to_owned(), HOLD.to_owned()],
1095            );
1096            store
1097                .put_panel(&mut fresh, &html, &[])
1098                .context("write the merge approval panel")?;
1099            store
1100                .put(&mut fresh)
1101                .context("file the merge approval question")?;
1102            state.event(
1103                "land",
1104                format!("asking for merge approval ({})", fresh.short()),
1105            );
1106            state.save()?;
1107            if let Err(e) = ask::notify(&state.config.notify, &fresh).await {
1108                // A broken webhook is not a reason to lose the merge: the
1109                // question is already on disk and the web UI already shows
1110                // it, so the operator still has a way in.
1111                tracing::warn!(
1112                    "could not notify about merge approval question {}: {e:#} - \
1113                     the web UI is the only surface for it now",
1114                    fresh.short()
1115                );
1116            }
1117            fresh
1118        }
1119    };
1120
1121    Ok(match q.status {
1122        ask::QuestionStatus::Open => ApprovalGate::Pending,
1123        // Nobody answered before `state.config.graph.answer_timeout` passed,
1124        // or the question was closed with no decision recorded underneath
1125        // this run - either way there is nothing left to wait on.
1126        ask::QuestionStatus::Abandoned => ApprovalGate::Held,
1127        // The merge gate does not speak `--thread`: an owner who talked back
1128        // instead of choosing never reaches `Answered`, so this arm only
1129        // ever sees an actual decision.
1130        ask::QuestionStatus::Answered => match approval(q.resolution().as_deref()) {
1131            Approval::Merge => ApprovalGate::Approved,
1132            Approval::Hold => ApprovalGate::Held,
1133        },
1134    })
1135}
1136
1137/// Parse `gh pr view --json url,number,state,statusCheckRollup,reviews,comments`
1138/// output into a [`PrState`]. No I/O.
1139pub fn parse_pr(json: &str) -> Result<PrState> {
1140    let raw: GhPr = serde_json::from_str(json).context("parse `gh pr view --json ...` output")?;
1141    let state = match raw.state.to_ascii_uppercase().as_str() {
1142        "OPEN" => PrLifecycle::Open,
1143        "MERGED" => PrLifecycle::Merged,
1144        "CLOSED" => PrLifecycle::Closed,
1145        other => bail!("unknown pull request state `{other}`"),
1146    };
1147
1148    let mut failing = Vec::new();
1149    let mut pending = false;
1150    let mut unknown = false;
1151    for check in &raw.status_check_rollup {
1152        match check.verdict() {
1153            Verdict::Pass => {}
1154            Verdict::Pending => pending = true,
1155            Verdict::Fail => failing.push(check.label()),
1156            Verdict::Unknown => unknown = true,
1157        }
1158    }
1159    let checks = if raw.status_check_rollup.is_empty() {
1160        Checks::Unknown
1161    } else if pending {
1162        Checks::Pending
1163    } else if !failing.is_empty() {
1164        Checks::Red
1165    } else if unknown {
1166        Checks::Unknown
1167    } else {
1168        Checks::Green
1169    };
1170
1171    let mut review_comments = Vec::new();
1172    for r in raw.reviews {
1173        push_if_outstanding(
1174            &mut review_comments,
1175            ReviewComment {
1176                author: r.author.login,
1177                path: None,
1178                line: None,
1179                body: r.body,
1180            },
1181        );
1182    }
1183    for c in raw.comments {
1184        push_if_outstanding(
1185            &mut review_comments,
1186            ReviewComment {
1187                author: c.author.login,
1188                path: None,
1189                line: None,
1190                body: c.body,
1191            },
1192        );
1193    }
1194
1195    Ok(PrState {
1196        url: raw.url,
1197        number: raw.number,
1198        state,
1199        checks,
1200        failing,
1201        review_comments,
1202        blocking: Blocking::of(&raw.merge_state_status),
1203    })
1204}
1205
1206/// Read just a pull request's lifecycle state - open, merged, or closed -
1207/// with none of the checks/reviews/comments [`land`] itself needs to decide
1208/// what to do next.
1209///
1210/// For a caller that only ever wants one fact and must not risk anything
1211/// else: `magi fold --merged` uses this to confirm a URL the operator hands
1212/// it is actually a merged pull request *before* touching a run's state, so a
1213/// typo or a still-open PR fails loudly instead of quietly recording a merge
1214/// that never happened.
1215pub async fn lifecycle(repo: &Path, pr_url: &str) -> Result<PrLifecycle> {
1216    let view = gh(
1217        repo,
1218        &[
1219            "pr".to_owned(),
1220            "view".to_owned(),
1221            pr_url.to_owned(),
1222            "--json".to_owned(),
1223            "state".to_owned(),
1224        ],
1225    )
1226    .await?;
1227    if !view.0 {
1228        bail!("gh pr view {pr_url}: {}", view.1);
1229    }
1230    // `parse_pr` reads every other field of `GhPr` as its serde default
1231    // (empty string, empty vec, zero) when this narrower `--json` selection
1232    // does not carry them - harmless, since only `.state` is read back.
1233    Ok(parse_pr(&view.1)?.state)
1234}
1235
1236/// A pull request the operator merged outside of `land::land`'s own loop,
1237/// found by asking GitHub about the run's own winning branch rather than
1238/// requiring the operator to go and find the URL themselves.
1239#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
1240pub struct ExternalMerge {
1241    /// The pull request's URL, ready to hand to [`correct_manual_merge`].
1242    pub url: String,
1243    /// The pull request's number.
1244    pub number: u64,
1245}
1246
1247#[derive(Debug, Deserialize)]
1248#[serde(rename_all = "camelCase")]
1249struct GhMergedPr {
1250    url: String,
1251    number: u64,
1252    merged_at: String,
1253    base_ref_name: String,
1254}
1255
1256/// Pure half of [`find_external_merge`]: given the raw `gh pr list --head
1257/// <branch> --state merged --json url,number,mergedAt,baseRefName` output,
1258/// decide whether exactly one of the pull requests it lists could actually
1259/// be *this* run's.
1260///
1261/// A branch name alone does not prove it: [`RunState::branch_for`] derives it
1262/// from the run's own short id, so a collision with some other, unrelated
1263/// task's merged pull request from a same-named branch is rare but not
1264/// impossible once branches are deleted and ids run out. Filtering on
1265/// `base_ref_name` (the branch this run actually targets) and `merged_at`
1266/// (which cannot predate the run itself) rules that case out. More than one
1267/// survivor is exactly as uninformative as zero — something this run cannot
1268/// tell apart from another — so only a unique survivor is returned.
1269fn pick_merged_pr(
1270    json: &str,
1271    base_branch: &str,
1272    created_at: Timestamp,
1273) -> Result<Option<ExternalMerge>> {
1274    let raw: Vec<GhMergedPr> =
1275        serde_json::from_str(json).context("parse `gh pr list ... --json ...` output")?;
1276    let mut matches: Vec<ExternalMerge> = Vec::new();
1277    for pr in raw {
1278        if pr.base_ref_name != base_branch {
1279            continue;
1280        }
1281        let Ok(merged_at) = pr.merged_at.parse::<Timestamp>() else {
1282            continue;
1283        };
1284        if merged_at < created_at {
1285            continue;
1286        }
1287        matches.push(ExternalMerge {
1288            url: pr.url,
1289            number: pr.number,
1290        });
1291    }
1292    if matches.len() == 1 {
1293        Ok(matches.pop())
1294    } else {
1295        Ok(None)
1296    }
1297}
1298
1299/// Ask GitHub whether this run's winning candidate branch was actually merged
1300/// somewhere `land::land`'s own loop never saw — the gap `magi fold
1301/// --merged` exists to close, minus the operator having to find the URL by
1302/// hand.
1303///
1304/// `Ok(None)` covers every case where nothing can be said with confidence: no
1305/// winner decided yet (nothing to check a branch for), no merged pull request
1306/// found, or [`pick_merged_pr`] found more than one candidate and would not
1307/// guess between them. Never wired to a weaker, URL-less signal like
1308/// [`branch_is_ancestor`] — a caller wanting that has to ask for it
1309/// separately, precisely because it cannot drive an automatic correction on
1310/// its own (see that function's own doc).
1311pub async fn find_external_merge(state: &RunState) -> Result<Option<ExternalMerge>> {
1312    let Some(winner) = state.winner() else {
1313        return Ok(None);
1314    };
1315    let branch = winner.branch.clone();
1316    let out = gh(
1317        &state.repo,
1318        &[
1319            "pr".to_owned(),
1320            "list".to_owned(),
1321            "--head".to_owned(),
1322            branch.clone(),
1323            "--state".to_owned(),
1324            "merged".to_owned(),
1325            "--json".to_owned(),
1326            "url,number,mergedAt,baseRefName".to_owned(),
1327        ],
1328    )
1329    .await?;
1330    if !out.0 {
1331        bail!("gh pr list --head {branch}: {}", out.1);
1332    }
1333    pick_merged_pr(&out.1, &state.base_branch, state.created_at)
1334}
1335
1336/// Whether `branch` is, right now, an ancestor of `base_branch` in the local
1337/// git graph — the weaker, URL-less signal that a branch landed somewhere.
1338///
1339/// Deliberately never consulted by [`find_external_merge`]: a base branch
1340/// that has moved since the run started can make an old, abandoned branch
1341/// look like an ancestor of the *current* base for reasons that have nothing
1342/// to do with a merge (a later commit that happens to supersede it, an
1343/// unrelated squash), and there is no pull request URL here to confirm
1344/// against or to land through anyway. Its only honest use is a weaker
1345/// notice — "this looks merged, go check" — never an automatic rewrite of
1346/// `status`/`merge`.
1347pub async fn branch_is_ancestor(repo: &Path, branch: &str, base_branch: &str) -> Result<bool> {
1348    let out = tokio::process::Command::new("git")
1349        .args(["merge-base", "--is-ancestor", branch, base_branch])
1350        .current_dir(repo)
1351        .quiet()
1352        .stdin(std::process::Stdio::null())
1353        .output()
1354        .await
1355        .context("spawn git merge-base --is-ancestor")?;
1356    Ok(out.status.success())
1357}
1358
1359/// Parse `host/owner/repo` out of a forge URL, with no network access.
1360///
1361/// The host is part of the slug, not discarded: `owner/repo` alone would
1362/// treat `github.example.com/o/r` and `github.com/o/r` as the same
1363/// repository, which is exactly the mix-up the same-repo guard exists to
1364/// catch. Returns `None` for anything that doesn't have a `<host>/<path>`
1365/// shape at all.
1366fn forge_slug(url: &str) -> Option<(String, &str)> {
1367    let rest = url.rsplit("://").next()?;
1368    let (host, path) = rest.split_once('/')?;
1369    if host.is_empty() {
1370        return None;
1371    }
1372    Some((host.to_ascii_lowercase(), path))
1373}
1374
1375/// Parse `host/owner/repo` out of a GitHub pull request URL, with no network
1376/// access - the first half of the same-repo guard [`correct_manual_merge`]
1377/// applies before it writes anything.
1378///
1379/// Returns `None` for anything that does not look like
1380/// `https://<host>/<owner>/<repo>/pull/<n>`, which the caller treats as
1381/// fail-closed: a URL this cannot make sense of refuses rather than guesses.
1382pub(crate) fn slug_of_pr_url(url: &str) -> Option<String> {
1383    let (host, path) = forge_slug(url)?;
1384    let mut segments = path.split('/');
1385    let owner = segments.next()?;
1386    let repo = segments.next()?;
1387    let kind = segments.next()?;
1388    if owner.is_empty() || repo.is_empty() || kind != "pull" {
1389        return None;
1390    }
1391    Some(format!("{host}/{owner}/{repo}"))
1392}
1393
1394/// Parse `host/owner/repo` out of a plain repository URL (no `/pull/<n>`
1395/// suffix), the shape `gh repo view --json url` returns - the other half of
1396/// the same-repo guard, matched against [`slug_of_pr_url`]'s output.
1397fn slug_of_repo_url(url: &str) -> Option<String> {
1398    let (host, path) = forge_slug(url)?;
1399    let mut segments = path.split('/');
1400    let owner = segments.next()?;
1401    let repo = segments.next()?;
1402    if owner.is_empty() || repo.is_empty() {
1403        return None;
1404    }
1405    Some(format!("{host}/{owner}/{repo}"))
1406}
1407
1408/// Refuse to correct a run against a pull request from a different
1409/// repository than the one it is recorded against.
1410///
1411/// This is the guard the shun/8c75 incident argued for: an operator ran
1412/// `magi fold --merged <shun PR url>` meaning to correct an old `Blocked` run
1413/// in a different repository, omitted the run id, and the id defaulted to
1414/// this machine's most recently created run - an unrelated, still-in-progress
1415/// run in a completely different repository - which then had its `status`
1416/// rewritten to `merged` from a pull request it had nothing to do with.
1417/// `correct_manual_merge` now requires an explicit id (see `magi fold`'s own
1418/// CLI help), but a mistyped or stale id could still name a run in a
1419/// different repository than the one the URL belongs to, so this checks that
1420/// independently rather than trusting the id alone.
1421///
1422/// Comparison is case-insensitive - GitHub owner/repo names are - and a
1423/// mismatch names both slugs rather than just refusing, so an operator whose
1424/// local checkout's `origin` is a fork of the repository the pull request was
1425/// opened against (a legitimate setup this cannot tell apart from a genuine
1426/// mix-up) can judge for themselves rather than being blocked with no way to
1427/// see why.
1428pub(crate) fn ensure_same_repo(run_repo_slug: &str, pr_repo_slug: &str) -> Result<()> {
1429    if run_repo_slug.eq_ignore_ascii_case(pr_repo_slug) {
1430        return Ok(());
1431    }
1432    bail!(
1433        "refusing to correct this run: it is recorded against {run_repo_slug}, but the pull \
1434         request URL belongs to {pr_repo_slug} - pass the run id whose repository the URL \
1435         actually belongs to (or, if `origin` is a fork opened against a different upstream, \
1436         verify by hand before treating this as a false positive)"
1437    );
1438}
1439
1440/// Ask the forge which `host/owner/repo` a local checkout's `origin` remote
1441/// actually resolves to, for the same-repo guard in [`correct_manual_merge`].
1442///
1443/// Asking `gh` rather than parsing `git remote -v` locally is deliberate: it
1444/// normalizes case, SSH vs. HTTPS remotes, and a renamed or transferred
1445/// repository the same way GitHub itself would recognize it, so the
1446/// comparison in [`ensure_same_repo`] is against the same canonical slug on
1447/// both sides. Reads `url` rather than `nameWithOwner` so the host is part of
1448/// the answer too - `nameWithOwner` alone cannot tell a `github.com` repo from
1449/// a same-named one on a GitHub Enterprise host.
1450async fn repo_slug(repo: &Path) -> Result<String> {
1451    let out = gh(
1452        repo,
1453        &[
1454            "repo".to_owned(),
1455            "view".to_owned(),
1456            "--json".to_owned(),
1457            "url".to_owned(),
1458        ],
1459    )
1460    .await?;
1461    if !out.0 {
1462        bail!("gh repo view --json url: {}", out.1);
1463    }
1464    #[derive(Debug, Deserialize)]
1465    struct GhRepo {
1466        url: String,
1467    }
1468    let parsed: GhRepo = serde_json::from_str(&out.1)
1469        .with_context(|| format!("parse `gh repo view` output: {}", out.1))?;
1470    slug_of_repo_url(&parsed.url)
1471        .with_context(|| format!("could not parse a host/owner/repo out of {}", parsed.url))
1472}
1473
1474/// Confirm `url` is actually a merged pull request, then rewrite `state`'s
1475/// `status` and `merge` exactly as the automatic land loop (`land::land`)
1476/// would have written them had magi opened and merged this pull request
1477/// itself.
1478///
1479/// This is `magi fold --merged`'s whole implementation, and also what the
1480/// web `fold-merged` route calls once it has a URL in hand — an operator
1481/// recovery path for a merge magi could not finish on its own: a PR title too
1482/// long for the GraphQL mutation, `gh pr create` unreachable, a stale token -
1483/// closed by hand with a pull request magi never opened and so never
1484/// recorded. Reusing `land::land` rather than writing `status`/`merge`
1485/// directly keeps this one authoritative: a merged pull request decides
1486/// `Step::Done { merged: true }` on the very first read, before any of
1487/// `land`'s own checks/fix/rebase machinery can run, which is what makes it
1488/// safe to call here even though this pull request was never magi's own.
1489///
1490/// [`ensure_same_repo`] is checked before anything else: a pull request from
1491/// a different repository than the one `state` is recorded against is
1492/// refused outright, regardless of its lifecycle. This is the guard for a
1493/// URL an *operator* hands in - the CLI or the web route - where a stale or
1494/// mistyped run id could otherwise get corrected from an unrelated
1495/// repository's pull request (see the shun/8c75 incident in `magi fold`'s own
1496/// CLI help). The automatic janitor sweep (`clean::reconcile_external_merges`)
1497/// goes through [`correct_confirmed_external_merge`] instead, which skips
1498/// this check: its `url` was never operator-supplied, it comes from
1499/// [`find_external_merge`] querying `gh` from inside `state.repo` itself, so
1500/// it is already guaranteed to name a pull request in that same repository -
1501/// re-deriving and re-checking the repository here would only be a second
1502/// `gh repo view` call that can fail for reasons that have nothing to do with
1503/// correctness (a rate limit, a network blip), turning a self-heal that would
1504/// otherwise have succeeded into a run left `Blocked` for another pass.
1505///
1506/// [`lifecycle`] is checked next and separately so a mistyped or still-open
1507/// URL fails loudly without writing anything, rather than handing an open
1508/// pull request to the full autonomous loop by accident.
1509///
1510/// Correcting `status` this way does not run `bump::after_merge`
1511/// (`src/bump.rs`): that call is made only from `graph::Runner::run_land`,
1512/// which this path never goes through. A release version bump the change
1513/// might have earned is therefore not filed automatically and has to be
1514/// requested by hand - recorded as an event on the run so the gap is visible
1515/// to whoever reads it later, not just wherever this was called from.
1516///
1517/// Returns the status before and after, so every caller (CLI, janitor, web
1518/// route) can build its own log line or response from the same pair rather
1519/// than each re-deriving it.
1520pub async fn correct_manual_merge(
1521    state: &mut RunState,
1522    url: &str,
1523) -> Result<(RunStatus, RunStatus)> {
1524    let Some(pr_slug) = slug_of_pr_url(url) else {
1525        bail!(
1526            "could not parse an owner/repo out of {url}; refusing to guess which repository \
1527             this pull request belongs to"
1528        );
1529    };
1530    let run_slug = repo_slug(&state.repo).await?;
1531    ensure_same_repo(&run_slug, &pr_slug)?;
1532    correct_merge(state, url).await
1533}
1534
1535/// The janitor's own entry point into the same correction
1536/// [`correct_manual_merge`] performs for an operator-supplied URL, minus the
1537/// same-repo guard - see that function's own doc for why skipping it here is
1538/// safe rather than a hole: [`clean::reconcile_external_merges`] only ever
1539/// calls this with a `url` [`find_external_merge`] already found by querying
1540/// `state.repo`'s own remote, so the guard could never do anything here but
1541/// fail on its own transient errors.
1542///
1543/// [`crate::clean`] is the only caller; `pub(crate)` rather than private only
1544/// because it lives in a different module.
1545pub(crate) async fn correct_confirmed_external_merge(
1546    state: &mut RunState,
1547    url: &str,
1548) -> Result<(RunStatus, RunStatus)> {
1549    correct_merge(state, url).await
1550}
1551
1552async fn correct_merge(state: &mut RunState, url: &str) -> Result<(RunStatus, RunStatus)> {
1553    match lifecycle(&state.repo, url).await? {
1554        PrLifecycle::Merged => {}
1555        other => bail!(
1556            "{url} is {}, not merged; refusing to record {} as merged on a guess",
1557            other.as_str(),
1558            state.id
1559        ),
1560    }
1561    let before = state.status;
1562    if let Err(e) = land(state, url).await {
1563        // `land` sets `status` to `Landing` and saves before its first read
1564        // of the pull request — see its own doc — so a failure here (a
1565        // transient `gh` hiccup between the two forge reads this function
1566        // makes) can leave the run stuck on that in-between value with
1567        // nothing left driving it. Land it on the same terminal shape an
1568        // automated `land` failure lands on instead of leaving it stuck.
1569        state.status = RunStatus::Blocked;
1570        state.event("fold", format!("manual-merge correction failed: {e:#}"));
1571        state.save()?;
1572        return Err(e).context(format!("confirming the merge of {url}"));
1573    }
1574    state.event(
1575        "fold",
1576        "operator recorded this pull request as a manual merge; this run never \
1577         re-entered `land`, so `bump::after_merge` did not run for it - a release \
1578         bump this change might warrant has to be filed by hand",
1579    );
1580    state.save()?;
1581    Ok((before, state.status))
1582}
1583
1584/// Parse `gh api repos/{owner}/{repo}/pulls/<n>/comments` into inline review
1585/// comments. No I/O.
1586///
1587/// `gh pr view` does not surface inline comments, and inline is exactly where
1588/// both review bots put their findings - a landing loop that read only the
1589/// top-level thread would never see the thing it is supposed to fix.
1590pub fn parse_inline_comments(json: &str) -> Result<Vec<ReviewComment>> {
1591    let raw: Vec<GhInline> =
1592        serde_json::from_str(json).context("parse `gh api .../pulls/<n>/comments` output")?;
1593    let mut out = Vec::new();
1594    for c in raw {
1595        push_if_outstanding(
1596            &mut out,
1597            ReviewComment {
1598                author: c.user.login,
1599                path: c.path,
1600                line: c.line,
1601                body: c.body,
1602            },
1603        );
1604    }
1605    Ok(out)
1606}
1607
1608/// Keep a comment only when it asks for something.
1609///
1610/// An inline comment always does: it names a file and a line. A top-level
1611/// comment is dropped when it is empty, when it is magi's own, or when it is
1612/// [noise](is_noise).
1613fn push_if_outstanding(out: &mut Vec<ReviewComment>, comment: ReviewComment) {
1614    if comment.body.trim().is_empty() || comment.body.contains(MARKER) {
1615        return;
1616    }
1617    if comment.path.is_none() && is_noise(&comment.body) {
1618        return;
1619    }
1620    out.push(comment);
1621}
1622
1623/// Is this comment body machinery rather than a finding?
1624///
1625/// Two tests, both structural, because guessing from prose is how a "looks
1626/// good to me" turns into a fix round:
1627///
1628/// 1. The bot said so - the body carries one of the [`NOT_A_REVIEW`] markers
1629///    with which CodeRabbit labels its trigger notice, its walkthrough, and its
1630///    footer.
1631/// 2. It asks for nothing - once HTML comments, `<details>` blocks, headings,
1632///    horizontal rules, and the bot's own status banner are removed, every
1633///    remaining line is a task-list item. That is exactly the shape of the
1634///    comment the Claude review job posts while it is still working.
1635///
1636/// Anything else is input, including bot prose. A bot that writes a paragraph
1637/// has said something, and the fix prompt tells the fixer it may decline a
1638/// comment with an argument - a wasted sentence in a prompt is cheaper than a
1639/// missed finding.
1640pub fn is_noise(body: &str) -> bool {
1641    if NOT_A_REVIEW.iter().any(|m| body.contains(m)) {
1642        return true;
1643    }
1644    let mut content = false;
1645    for line in strip_blocks(body).lines() {
1646        let line = unquote(line);
1647        if line.is_empty() || is_checklist(line) || is_decoration(line) || is_banner(line) {
1648            continue;
1649        }
1650        content = true;
1651        break;
1652    }
1653    !content
1654}
1655
1656/// Remove HTML comments and collapsed `<details>` blocks.
1657fn strip_blocks(body: &str) -> String {
1658    let mut out = String::with_capacity(body.len());
1659    let mut rest = body;
1660    loop {
1661        let open = ["<!--", "<details>"]
1662            .iter()
1663            .filter_map(|tag| rest.find(tag).map(|i| (i, *tag)))
1664            .min_by_key(|(i, _)| *i);
1665        let Some((at, tag)) = open else {
1666            out.push_str(rest);
1667            return out;
1668        };
1669        out.push_str(&rest[..at]);
1670        let after = &rest[at + tag.len()..];
1671        let close = if tag == "<!--" { "-->" } else { "</details>" };
1672        match after.find(close) {
1673            Some(end) => rest = &after[end + close.len()..],
1674            // Unterminated: the rest of the body is inside the block.
1675            None => return out,
1676        }
1677    }
1678}
1679
1680/// Strip blockquote markers, which both bots wrap their callouts in.
1681fn unquote(line: &str) -> &str {
1682    let mut s = line.trim();
1683    while let Some(rest) = s.strip_prefix('>') {
1684        s = rest.trim_start();
1685    }
1686    s.trim()
1687}
1688
1689/// `- [ ]` / `- [x]`, in any of the bullet styles GitHub renders.
1690fn is_checklist(line: &str) -> bool {
1691    let rest = line
1692        .strip_prefix("- ")
1693        .or_else(|| line.strip_prefix("* "))
1694        .unwrap_or("");
1695    let rest = rest.trim_start();
1696    matches!(
1697        rest.get(..3),
1698        Some("[ ]") | Some("[x]") | Some("[X]") | Some("[*]")
1699    )
1700}
1701
1702/// A heading, a horizontal rule, or a callout tag - shape, never content.
1703fn is_decoration(line: &str) -> bool {
1704    line.starts_with('#')
1705        || line.starts_with("[!")
1706        || (line.len() >= 3 && line.chars().all(|c| matches!(c, '-' | '=' | '*' | '_')))
1707}
1708
1709/// A line that is nothing but emphasis and links.
1710///
1711/// Both review jobs open with a status banner
1712/// (`**Claude finished ... in 4m 14s** —— [View job](url)`). It reads as prose
1713/// to a line-based test and asks for nothing, so it is measured the same way a
1714/// heading is: strip the markup, and if no word survives, it was decoration.
1715fn is_banner(line: &str) -> bool {
1716    let plain = drop_spans(line, "**", "**");
1717    let plain = if plain.contains("](") {
1718        drop_spans(&plain, "[", ")")
1719    } else {
1720        plain
1721    };
1722    !plain.chars().any(char::is_alphanumeric)
1723}
1724
1725/// Remove every `open` .. `close` span, including the delimiters. An
1726/// unterminated span swallows the rest of the input, which is what a reader
1727/// sees too.
1728fn drop_spans(s: &str, open: &str, close: &str) -> String {
1729    let mut out = String::with_capacity(s.len());
1730    let mut rest = s;
1731    while let Some(at) = rest.find(open) {
1732        out.push_str(&rest[..at]);
1733        let after = &rest[at + open.len()..];
1734        match after.find(close) {
1735            Some(end) => rest = &after[end + close.len()..],
1736            None => return out,
1737        }
1738    }
1739    out.push_str(rest);
1740    out
1741}
1742
1743/// The lock that keeps at most one run per repository actually moving the
1744/// base branch at a time: a rebase push, or `gh pr merge`.
1745///
1746/// Deliberately narrow. Everything else in [`land`]'s loop - watching CI,
1747/// running a fix round in the winner's own worktree, waiting on the owner's
1748/// approval - touches nothing a *different* run in the same repository could
1749/// collide with, and holding a lock across any of that would serialise one
1750/// run's CI wait (up to [`WAIT_CEILING`]) against another run's land-approval
1751/// resume, which is precisely the "must not wait on another task" property
1752/// the daemon's slot-freeing exists to give a resume. Only the two moments
1753/// that actually write to the shared base branch need mutual exclusion, and
1754/// both are brief.
1755///
1756/// One entry per repository, each its own `tokio::sync::Mutex`, so two
1757/// different repositories' runs never wait on each other. The outer
1758/// `std::sync::Mutex` guards only the map itself, held long enough to find or
1759/// insert an entry and clone its `Arc`, never across an `.await`.
1760fn repo_merge_lock(repo: &Path) -> Arc<tokio::sync::Mutex<()>> {
1761    static LOCKS: std::sync::LazyLock<
1762        std::sync::Mutex<BTreeMap<PathBuf, Arc<tokio::sync::Mutex<()>>>>,
1763    > = std::sync::LazyLock::new(|| std::sync::Mutex::new(BTreeMap::new()));
1764    LOCKS
1765        .lock()
1766        .unwrap_or_else(std::sync::PoisonError::into_inner)
1767        .entry(repo.to_path_buf())
1768        .or_insert_with(|| Arc::new(tokio::sync::Mutex::new(())))
1769        .clone()
1770}
1771
1772/// `owner/repo` out of a pull request url, falling back to the checkout's
1773/// directory name when the url is not the usual `host/owner/repo/pull/N`.
1774fn repo_label(repo: &Path, pr_url: &str) -> String {
1775    let parts: Vec<&str> = pr_url.split('/').collect();
1776    if let Some(at) = parts.iter().rposition(|p| *p == "pull")
1777        && at >= 2
1778        && !parts[at - 1].is_empty()
1779        && !parts[at - 2].is_empty()
1780    {
1781        return format!("{}/{}", parts[at - 2], parts[at - 1]);
1782    }
1783    repo.file_name()
1784        .map(|n| n.to_string_lossy().into_owned())
1785        .unwrap_or_default()
1786}
1787
1788/// The operator-facing sentence for a merge that went ahead with red checks,
1789/// or `None` when the checks were not red. Judged on `checks`, not on
1790/// `failing`, which can be non-empty on a green observation.
1791fn red_merge_summary(repo_name: &str, pr: &PrState) -> Option<String> {
1792    (pr.checks == Checks::Red).then(|| {
1793        format!(
1794            "Merged {repo_name} PR #{} with red checks: {} ({})",
1795            pr.number,
1796            if pr.failing.is_empty() {
1797                "(none named)".to_owned()
1798            } else {
1799                pr.failing.join(", ")
1800            },
1801            pr.url
1802        )
1803    })
1804}
1805
1806/// After a merge that succeeded: record which checks were red and tell the
1807/// operator. The decision to merge is already made; this only makes it audible.
1808/// Best-effort - a broken notifier never fails the run.
1809async fn announce_red_merge(state: &mut RunState, pr: &PrState) {
1810    let repo_name = repo_label(&state.repo, &pr.url);
1811    let Some(summary) = red_merge_summary(&repo_name, pr) else {
1812        return;
1813    };
1814    if let Some(rec) = state.pr.as_mut() {
1815        rec.red_at_merge = pr.failing.clone();
1816    }
1817    state.event("land", summary.clone());
1818    crate::notices::raise(crate::notices::merged_red(&state.id, &summary));
1819    if let Err(e) = ask::notify_text(&state.config.notify, &state.id, &summary).await {
1820        tracing::warn!("could not notify about a merge with red checks: {e:#}");
1821    }
1822}
1823
1824/// Run the loop against a real pull request until it merges or the budget runs
1825/// out.
1826///
1827/// The caller decides whether landing happens at all: this is only reached when
1828/// `graph.land` is on. Returns the last observation, so the caller can report
1829/// what magi was looking at when it stopped.
1830pub async fn land(state: &mut RunState, pr_url: &str) -> Result<PrState> {
1831    let repo = state.repo.clone();
1832    let budget = state.config.graph.land_rounds;
1833    let mut round = 0usize;
1834    // Counted apart from `round`: a rebase is not a fix, and a base that
1835    // moved is not the change's fault.
1836    let mut rebases = 0usize;
1837    let mut waited = Duration::ZERO;
1838    // Comment bodies the fixer has already been shown. A comment is
1839    // outstanding until it has been handed over once; after that it is a
1840    // recorded decision, not an open question, and re-feeding it would loop the
1841    // budget away on a comment the fixer already declined with an argument.
1842    let mut shown: BTreeSet<String> = BTreeSet::new();
1843
1844    // Marks the run resumable through exactly this function, not through a
1845    // fresh competition: `RunStatus::resumable` excludes only `Merged`,
1846    // `Ready` and `Failed`, and `merge`'s own re-entry guard looks for this
1847    // status specifically to know a resumed run belongs back in `land`
1848    // rather than at a second `gh pr create`. Set on every entry - fresh or
1849    // resumed - because a resume that parked here again must keep reading
1850    // `Landing`, not whatever a first pass through `merge` left behind.
1851    state.status = RunStatus::Landing;
1852    state.event("land", format!("watching {pr_url}"));
1853    state.save()?;
1854
1855    loop {
1856        let seen = observe(&repo, pr_url).await?;
1857        let mut pr = seen.pr;
1858        pr.review_comments.retain(|c| !shown.contains(&c.body));
1859        state.pr = Some(crate::run::PrRecord {
1860            url: pr.url.clone(),
1861            number: pr.number,
1862            state: pr.state.as_str().to_owned(),
1863            checks: pr.checks.as_str().to_owned(),
1864            round,
1865            rounds: budget,
1866            red_at_merge: Vec::new(),
1867        });
1868        state.save()?;
1869
1870        match decide(&pr, round, budget, waited) {
1871            Step::Wait => {
1872                if waited >= WAIT_CEILING {
1873                    let why = format!(
1874                        "checks were still running after {} minutes",
1875                        WAIT_CEILING.as_secs() / 60
1876                    );
1877                    stop(state, &repo, &pr, &why).await?;
1878                    return Ok(pr);
1879                }
1880                waited += POLL;
1881                tokio::time::sleep(POLL).await;
1882            }
1883            Step::Done { merged } => {
1884                state.status = if merged {
1885                    RunStatus::Merged
1886                } else {
1887                    RunStatus::Ready
1888                };
1889                let detail = if merged {
1890                    format!("{} was merged", pr.url)
1891                } else {
1892                    format!("{} was closed without merging", pr.url)
1893                };
1894                state.merge = Some(MergeOutcome {
1895                    mode: MergeMode::Pr,
1896                    ok: merged,
1897                    detail: detail.clone(),
1898                });
1899                state.event("land", detail);
1900                state.save()?;
1901                return Ok(pr);
1902            }
1903            Step::Merge => {
1904                let subject = merge_subject(&seen.title, &state.instruction);
1905                // The owner sees the panel before the one irreversible step,
1906                // and an unanswered question is a hold: silence never merges.
1907                if state.config.graph.land_approval {
1908                    match approval_gate(state, &pr, &subject).await? {
1909                        ApprovalGate::Approved => {}
1910                        ApprovalGate::Held => {
1911                            stop(
1912                                state,
1913                                &repo,
1914                                &pr,
1915                                "the owner did not approve the merge (held or unanswered)",
1916                            )
1917                            .await?;
1918                            return Ok(pr);
1919                        }
1920                        // Filed (or still standing from an earlier visit) and
1921                        // not yet answered. Park here rather than wait: the
1922                        // question survives on disk, the daemon hands this
1923                        // run's slot to something else, and a later resume
1924                        // re-enters `land`, finds the same question, and
1925                        // either merges or stops depending on what it says
1926                        // by then.
1927                        ApprovalGate::Pending => {
1928                            state.parked = true;
1929                            state.event(
1930                                "land",
1931                                "parked awaiting merge approval - resumes once answered",
1932                            );
1933                            state.save()?;
1934                            return Ok(pr);
1935                        }
1936                    }
1937                }
1938                let argv = merge_argv(pr.number, &subject);
1939                let out = {
1940                    let merge_lock = repo_merge_lock(&repo);
1941                    let _merge_slot = merge_lock.lock().await;
1942                    gh(&repo, &argv).await?
1943                };
1944                if out.0 {
1945                    pr.state = PrLifecycle::Merged;
1946                    state.status = RunStatus::Merged;
1947                    state.merge = Some(MergeOutcome {
1948                        mode: MergeMode::Pr,
1949                        ok: true,
1950                        detail: format!("gh {}", argv.join(" ")),
1951                    });
1952                    // The last `state.pr` snapshot is whatever the poll before
1953                    // this merge observed - still `open` - and nothing below
1954                    // refreshes it from GitHub again, so the UI's round rail
1955                    // would otherwise keep animating a merged run forever.
1956                    if let Some(pr_record) = state.pr.as_mut() {
1957                        pr_record.state = pr.state.as_str().to_owned();
1958                    }
1959                    state.event("land", format!("merged {} as `{subject}`", pr.url));
1960                    announce_red_merge(state, &pr).await;
1961                    state.save()?;
1962                    return Ok(pr);
1963                }
1964                let after = observe(&repo, pr_url).await.ok().map(|s| s.pr.state);
1965                if let Some(outcome) = merged_after_all(&argv, &out.1, after) {
1966                    pr.state = PrLifecycle::Merged;
1967                    state.status = RunStatus::Merged;
1968                    state.merge = Some(outcome);
1969                    if let Some(pr_record) = state.pr.as_mut() {
1970                        pr_record.state = pr.state.as_str().to_owned();
1971                    }
1972                    state.event("land", format!("merged {} as `{subject}`", pr.url));
1973                    announce_red_merge(state, &pr).await;
1974                    state.save()?;
1975                    return Ok(pr);
1976                }
1977                stop(
1978                    state,
1979                    &repo,
1980                    &pr,
1981                    &format!("`gh pr merge` failed: {}", out.1),
1982                )
1983                .await?;
1984                return Ok(pr);
1985            }
1986            Step::Rebase => {
1987                // Bounded by the same budget as a fix, because a rebase that
1988                // keeps being needed means the base moves faster than this
1989                // run can land and a person should decide what to do. It
1990                // spends none of that budget: the change is not what is
1991                // wrong.
1992                if rebases >= budget {
1993                    let why = format!(
1994                        "the base moved under this branch {budget} time(s) and it still does \
1995                         not merge; rebasing again would only race it"
1996                    );
1997                    stop(state, &repo, &pr, &why).await?;
1998                    return Ok(pr);
1999                }
2000                rebases += 1;
2001                let Some(branch) = state.winner().map(|w| w.branch.clone()) else {
2002                    stop(
2003                        state,
2004                        &repo,
2005                        &pr,
2006                        "the pull request conflicts and this run has no winning branch to rebase",
2007                    )
2008                    .await?;
2009                    return Ok(pr);
2010                };
2011                let base = state.base_branch.clone();
2012                state.event(
2013                    "land",
2014                    format!("{} no longer merges; rebasing onto {base}", pr.url),
2015                );
2016                state.save()?;
2017
2018                // Onto the base as the *remote* has it: the local ref may be
2019                // behind, and rebasing onto a stale base produces a branch
2020                // that conflicts all over again.
2021                git::fetch(&repo, "origin", &base).await.ok();
2022                let scratch = state.dir().join("rebase");
2023                let onto = format!("origin/{base}");
2024                match git::rebase_branch_in_temp(&repo, &scratch, &branch, &onto).await {
2025                    Ok(None) => {
2026                        let pushed = {
2027                            let merge_lock = repo_merge_lock(&repo);
2028                            let _merge_slot = merge_lock.lock().await;
2029                            git::push_rewritten(&repo, "origin", &branch).await?
2030                        };
2031                        if !pushed.ok() {
2032                            let why = format!(
2033                                "rebased {branch} but could not push it: {}",
2034                                pushed.stderr.trim()
2035                            );
2036                            stop(state, &repo, &pr, &why).await?;
2037                            return Ok(pr);
2038                        }
2039                        state.event("land", format!("rebased {branch} onto {base}"));
2040                        state.save()?;
2041                        // The forge has to re-run its checks against the
2042                        // rebased head before anything else can be decided.
2043                        waited = Duration::ZERO;
2044                        tokio::time::sleep(POLL).await;
2045                    }
2046                    // A conflict is a decision, not a chore.
2047                    Ok(Some(conflict)) => {
2048                        let why = format!(
2049                            "{} conflicts with {base} and the rebase did not apply: {}",
2050                            pr.url,
2051                            conflict.chars().take(600).collect::<String>()
2052                        );
2053                        stop(state, &repo, &pr, &why).await?;
2054                        return Ok(pr);
2055                    }
2056                    Err(e) => {
2057                        let why = format!("could not rebase {branch} onto {base}: {e:#}");
2058                        stop(state, &repo, &pr, &why).await?;
2059                        return Ok(pr);
2060                    }
2061                }
2062            }
2063            Step::GiveUp { reason } => {
2064                stop(state, &repo, &pr, &reason).await?;
2065                return Ok(pr);
2066            }
2067            Step::Fix { reason } => {
2068                round += 1;
2069                waited = Duration::ZERO;
2070                for c in &pr.review_comments {
2071                    shown.insert(c.body.clone());
2072                }
2073                state.event("land", format!("round {round}: {reason}"));
2074                state.save()?;
2075
2076                let logs = failing_logs(&repo, &seen.failing_urls).await;
2077                let was_red = pr.checks == Checks::Red;
2078                match fix_round(state, &pr, round, budget, &reason, &logs).await? {
2079                    Fixed::Committed => {}
2080                    Fixed::Declined if was_red => {
2081                        let why = format!(
2082                            "the fixer produced no commit while {} check(s) were failing \
2083                             ({}); stopping instead of looping on an unchanged tree",
2084                            pr.failing.len(),
2085                            pr.failing.join(", ")
2086                        );
2087                        stop(state, &repo, &pr, &why).await?;
2088                        return Ok(pr);
2089                    }
2090                    // Comment-driven round with no commit: the fixer read the
2091                    // comments and changed nothing, which is a decision it is
2092                    // allowed to make. The comments are recorded as shown, so
2093                    // the next observation sees a clean pull request.
2094                    Fixed::Declined => state.event(
2095                        "land",
2096                        format!("round {round}: fixer declined the comments, nothing committed"),
2097                    ),
2098                    Fixed::Failed(why) => {
2099                        stop(state, &repo, &pr, &format!("the fix round failed: {why}")).await?;
2100                        return Ok(pr);
2101                    }
2102                }
2103                state.save()?;
2104            }
2105        }
2106    }
2107}
2108
2109/// One observation, plus the two things [`PrState`] deliberately does not carry:
2110/// the title (needed for the squash subject) and where the failing checks'
2111/// logs live.
2112struct Seen {
2113    pr: PrState,
2114    title: String,
2115    failing_urls: Vec<(String, String)>,
2116}
2117
2118/// Read the pull request: `gh pr view` for the rollup and the top-level thread,
2119/// `gh api` for the inline review comments `gh pr view` does not report.
2120async fn observe(repo: &Path, pr_url: &str) -> Result<Seen> {
2121    let view = gh(
2122        repo,
2123        &[
2124            "pr".to_owned(),
2125            "view".to_owned(),
2126            pr_url.to_owned(),
2127            "--json".to_owned(),
2128            "url,number,state,title,statusCheckRollup,reviews,comments,mergeStateStatus".to_owned(),
2129        ],
2130    )
2131    .await?;
2132    if !view.0 {
2133        bail!("gh pr view {pr_url}: {}", view.1);
2134    }
2135    let mut pr = parse_pr(&view.1)?;
2136    let raw: GhPr = serde_json::from_str(&view.1).context("re-read pull request json")?;
2137
2138    let inline = gh(
2139        repo,
2140        &[
2141            "api".to_owned(),
2142            format!("repos/{{owner}}/{{repo}}/pulls/{}/comments", pr.number),
2143        ],
2144    )
2145    .await?;
2146    if inline.0 {
2147        match parse_inline_comments(&inline.1) {
2148            Ok(mut comments) => pr.review_comments.append(&mut comments),
2149            // An unreadable inline thread must not end a landing: the rollup
2150            // and the top-level thread are still real signal.
2151            Err(e) => tracing::warn!("inline review comments unreadable: {e}"),
2152        }
2153    } else {
2154        tracing::warn!("gh api pulls/{}/comments: {}", pr.number, inline.1);
2155    }
2156
2157    let failing_urls = raw
2158        .status_check_rollup
2159        .iter()
2160        .filter(|c| c.verdict() == Verdict::Fail)
2161        .filter_map(|c| c.url().map(|u| (c.label(), u.to_owned())))
2162        .collect();
2163
2164    Ok(Seen {
2165        pr,
2166        title: raw.title,
2167        failing_urls,
2168    })
2169}
2170
2171/// What a fix round did.
2172enum Fixed {
2173    /// The fixer committed something.
2174    Committed,
2175    /// The fixer ran and chose to change nothing.
2176    Declined,
2177    /// The fixer could not run, or said nothing usable.
2178    Failed(String),
2179}
2180
2181/// Hand the failures and the comments to the fixer, then commit and push.
2182///
2183/// The fixer works in the winner's own worktree so its commits land on the
2184/// branch the pull request is built from, and it runs with `allow_write` for
2185/// the same reason.
2186async fn fix_round(
2187    state: &mut RunState,
2188    pr: &PrState,
2189    round: usize,
2190    budget: usize,
2191    reason: &str,
2192    logs: &str,
2193) -> Result<Fixed> {
2194    let winner = state
2195        .winner()
2196        .cloned()
2197        .context("landing needs a winning candidate; none is recorded on this run")?;
2198    let roles = state
2199        .config
2200        .resolve_roles()
2201        .context("resolve the roster for the fix round")?;
2202    // Same rule as the review loop: an explicitly configured fixer, otherwise
2203    // the winner's own author continuing its own conversation - the competition
2204    // is over, so its context is pure benefit.
2205    let (spec, seat_key): (AgentSpec, String) = match &roles.fixer {
2206        Some(f) if f.id != winner.agent => (f.clone(), "fix".to_owned()),
2207        _ => (
2208            state
2209                .config
2210                .agent(&winner.agent)
2211                .cloned()
2212                .unwrap_or_else(|_| roles.implementers[winner.index].clone()),
2213            format!("impl-{}", winner.label),
2214        ),
2215    };
2216
2217    let prompt = fix_prompt(state, pr, round, budget, reason, logs);
2218    let mut seat = seat_of(state, &seat_key, &spec.id);
2219    let artifacts = agent::artifacts_dir(&state.dir());
2220    let prompt = if state.config.cache_dir().is_some() {
2221        format!("{prompt}\n\n{}", prompt::build_cache_note("fix", true))
2222    } else {
2223        prompt
2224    };
2225    let out = agent::invoke(
2226        &spec,
2227        &mut seat,
2228        &Invocation {
2229            cwd: &winner.worktree,
2230            prompt: &prompt,
2231            timeout: Duration::from_secs(state.config.graph.timeout_fix),
2232            allow_write: true,
2233            sessions: state.config.graph.sessions,
2234            artifacts: &artifacts,
2235            stem: &format!("land-{round}"),
2236            run: &state.id,
2237            node: "land",
2238            cache_dir: state.config.cache_dir().as_deref(),
2239            attachments: &[],
2240        },
2241    )
2242    .await;
2243    state.seats.insert(seat.key.clone(), seat);
2244
2245    match out {
2246        Ok(o) if o.quota_exhausted() => {
2247            return Ok(Fixed::Failed(
2248                "rate limited (quota); the fixer could not run".to_owned(),
2249            ));
2250        }
2251        Ok(o) if !o.usable() => {
2252            return Ok(Fixed::Failed(format!(
2253                "the fixer produced nothing usable (exit {:?}, timed out: {})",
2254                o.exit_code, o.timed_out
2255            )));
2256        }
2257        Ok(_) => {}
2258        Err(e) => return Ok(Fixed::Failed(format!("{e:#}"))),
2259    }
2260
2261    let before = git::rev_parse(&winner.worktree, "HEAD").await?;
2262    // An agent that edited files but never committed would otherwise push
2263    // nothing and look like a refusal.
2264    if let Ok(r) = git::rescue_commit(
2265        &winner.worktree,
2266        &format!("magi: land round {round} fixes (uncommitted work)"),
2267    )
2268    .await
2269    {
2270        state.note_withheld("land", &r.withheld);
2271    }
2272    let after = git::rev_parse(&winner.worktree, "HEAD").await?;
2273    if after == before {
2274        return Ok(Fixed::Declined);
2275    }
2276
2277    let remote = state.config.merge.remote.clone();
2278    let push = git::push(&winner.worktree, &remote, &winner.branch).await?;
2279    if !push.ok() {
2280        return Ok(Fixed::Failed(format!(
2281            "pushing {} to {remote} failed: {}",
2282            winner.branch, push.stderr
2283        )));
2284    }
2285    state.event(
2286        "land",
2287        format!("round {round}: pushed a fix to {}", winner.branch),
2288    );
2289    Ok(Fixed::Committed)
2290}
2291
2292/// Fetch or create a seat, keeping its conversation across nodes.
2293fn seat_of(state: &mut RunState, key: &str, agent: &str) -> SeatState {
2294    if let Some(existing) = state.seats.get(key)
2295        && existing.agent == agent
2296    {
2297        return existing.clone();
2298    }
2299    let fresh = SeatState::new(key, agent, state.seed);
2300    state.seats.insert(key.to_owned(), fresh.clone());
2301    fresh
2302}
2303
2304/// What the fixer is told.
2305fn fix_prompt(
2306    state: &RunState,
2307    pr: &PrState,
2308    round: usize,
2309    budget: usize,
2310    reason: &str,
2311    logs: &str,
2312) -> String {
2313    let mut s = format!(
2314        "Your patch is open as a pull request and it is not landing. Land round \
2315         {round} of {budget}.\n\n\
2316         Pull request: {}\n\n\
2317         What is holding it: {reason}\n\n\
2318         # The task\n\n{}\n",
2319        pr.url, state.instruction
2320    );
2321
2322    if pr.failing.is_empty() {
2323        s.push_str("\n# Failing checks\n\n(none)\n");
2324    } else {
2325        let _ = write!(s, "\n# Failing checks\n\n- {}\n", pr.failing.join("\n- "));
2326        if logs.trim().is_empty() {
2327            s.push_str("\nNo log could be read; reproduce the failure locally.\n");
2328        } else {
2329            let _ = write!(s, "\n## Failing log tails\n\n{logs}\n");
2330        }
2331    }
2332
2333    if pr.review_comments.is_empty() {
2334        s.push_str("\n# Review comments\n\n(none)\n");
2335    } else {
2336        s.push_str("\n# Review comments\n");
2337        for c in &pr.review_comments {
2338            let where_ = match (&c.path, c.line) {
2339                (Some(p), Some(l)) => format!(" ({p}:{l})"),
2340                (Some(p), None) => format!(" ({p})"),
2341                _ => String::new(),
2342            };
2343            let _ = write!(s, "\n## {}{where_}\n\n{}\n", c.author, c.body.trim());
2344        }
2345    }
2346
2347    s.push_str(
2348        "\n# Rules\n\n\
2349         1. Fix the cause, never the symptom. Do not delete, skip, or weaken a \
2350            failing test; do not silence a lint with an allow attribute; do not \
2351            stretch a timeout to hide a race. If the check is right, the code is \
2352            wrong.\n\
2353         2. Change nothing the checks and the comments did not raise. A \
2354            drive-by refactor turns a one-line fix into a pull request that \
2355            needs reviewing again.\n\
2356         3. If a comment is wrong, say so with a checkable argument and change \
2357            nothing for it. A declined comment with a reason is a correct \
2358            outcome; a change made to appease a reviewer is not.\n\
2359         4. Commit in this worktree. magi pushes to the pull request's branch \
2360            for you; do not push, merge, or close anything yourself.\n\
2361         5. Never name yourself, your vendor, or your model, anywhere.\n\n\
2362         # Output\n\n\
2363         Say what you changed and why, and what you declined and why.",
2364    );
2365
2366    let language = &state.config.graph.language;
2367    if !(language.trim().is_empty() || language.eq_ignore_ascii_case("en")) {
2368        let _ = write!(s, "\n\nWrite all prose in {language}.");
2369    }
2370    // After the language line, so the exception is the last word on it.
2371    s.push_str(&crate::prompt::github_english(language));
2372    if let Some(overlay) = state.config.prompts.overlay("fix") {
2373        let _ = write!(s, "\n\n{overlay}");
2374    }
2375    s
2376}
2377
2378/// Failing log tails, the way the operator collects them by hand:
2379/// `gh run view --log-failed`.
2380async fn failing_logs(repo: &Path, failing: &[(String, String)]) -> String {
2381    let mut out = String::new();
2382    for (name, url) in failing.iter().take(MAX_LOGS) {
2383        let args = match (job_of(url), run_of(url)) {
2384            (Some(job), _) => vec![
2385                "run".to_owned(),
2386                "view".to_owned(),
2387                "--log-failed".to_owned(),
2388                "--job".to_owned(),
2389                job,
2390            ],
2391            (None, Some(run)) => vec![
2392                "run".to_owned(),
2393                "view".to_owned(),
2394                run,
2395                "--log-failed".to_owned(),
2396            ],
2397            // Not a GitHub Actions check - an external status has no log here.
2398            (None, None) => continue,
2399        };
2400        let (ok, body) = match gh(repo, &args).await {
2401            Ok(v) => v,
2402            Err(e) => (false, format!("{e:#}")),
2403        };
2404        if !ok && body.trim().is_empty() {
2405            continue;
2406        }
2407        let _ = write!(out, "### {name}\n\n```\n{}\n```\n\n", tail(&body, LOG_TAIL));
2408    }
2409    out
2410}
2411
2412/// Job id out of a check's `detailsUrl`
2413/// (`https://github.com/o/r/actions/runs/<run>/job/<job>`).
2414fn job_of(details_url: &str) -> Option<String> {
2415    let after = details_url.split("/job/").nth(1)?;
2416    let id: String = after.chars().take_while(char::is_ascii_digit).collect();
2417    (!id.is_empty()).then_some(id)
2418}
2419
2420/// Workflow run id out of a check's `detailsUrl`.
2421fn run_of(details_url: &str) -> Option<String> {
2422    let after = details_url.split("/actions/runs/").nth(1)?;
2423    let id: String = after.chars().take_while(char::is_ascii_digit).collect();
2424    (!id.is_empty()).then_some(id)
2425}
2426
2427/// The comment `stop` posts. Fixed English, whatever `[graph] language` says:
2428/// it lands on GitHub, not in front of the operator. Pure so a test can hold
2429/// it to that.
2430fn stop_comment(run_id: &str, why: &str) -> String {
2431    format!(
2432        "{MARKER}\nmagi stopped landing this pull request: {why}\n\n\
2433         The branch is untouched and the run is `{run_id}`. Nothing was merged."
2434    )
2435}
2436
2437/// Leave the pull request open, say why on it, and mark the run blocked.
2438///
2439/// The comment is what makes an unattended stop actionable: the operator wakes
2440/// up to a pull request that explains itself rather than to a silent queue.
2441async fn stop(state: &mut RunState, repo: &Path, pr: &PrState, why: &str) -> Result<()> {
2442    let body = stop_comment(&state.id, why);
2443    let posted = gh(
2444        repo,
2445        &[
2446            "pr".to_owned(),
2447            "comment".to_owned(),
2448            pr.number.to_string(),
2449            "--body".to_owned(),
2450            body,
2451        ],
2452    )
2453    .await;
2454    match posted {
2455        Ok((true, _)) => {}
2456        Ok((false, out)) => tracing::warn!("could not comment on {}: {out}", pr.url),
2457        Err(e) => tracing::warn!("could not comment on {}: {e:#}", pr.url),
2458    }
2459    state.status = RunStatus::Blocked;
2460    state.merge = Some(MergeOutcome {
2461        mode: MergeMode::Pr,
2462        ok: false,
2463        detail: why.to_owned(),
2464    });
2465    state.event("land", format!("stopped: {why}"));
2466    state.save()?;
2467    Ok(())
2468}
2469
2470/// Run `gh` in `repo`, returning success and the combined output.
2471///
2472/// Combined because `gh` reports a refused merge on stderr and the pull request
2473/// json on stdout, and both are evidence.
2474///
2475/// `GH_REPO` is stripped from the child's environment: every call site here
2476/// passes an explicit `cwd` (or a full pull request URL) meaning to operate
2477/// on *that* checkout's own remote, and `gh` prefers `GH_REPO` over the
2478/// checkout it is sitting in when no `--repo` flag is given. Left unset, a
2479/// `GH_REPO` the operator happens to have exported for an unrelated script
2480/// would silently redirect [`repo_slug`] (and every other cwd-scoped call
2481/// below) to a different repository than the one actually on disk - which
2482/// for the same-repo guard in [`correct_manual_merge`] would mean the check
2483/// could be made to agree with whatever repository a forged `--merged` URL
2484/// claims, defeating it entirely.
2485async fn gh(cwd: &Path, args: &[String]) -> Result<(bool, String)> {
2486    let out = tokio::process::Command::new("gh")
2487        .args(args)
2488        .current_dir(cwd)
2489        .env_remove("GH_REPO")
2490        .quiet()
2491        .stdin(std::process::Stdio::null())
2492        .output()
2493        .await
2494        .with_context(|| format!("spawn gh {}", args.join(" ")))?;
2495    let mut body = String::from_utf8_lossy(&out.stdout).into_owned();
2496    let err = String::from_utf8_lossy(&out.stderr);
2497    if body.trim().is_empty() {
2498        body = err.into_owned();
2499    } else if !err.trim().is_empty() {
2500        body.push_str(&err);
2501    }
2502    Ok((out.status.success(), body.trim().to_owned()))
2503}
2504
2505/// Verdict of one entry in the status rollup.
2506#[derive(Debug, Clone, Copy, PartialEq, Eq)]
2507enum Verdict {
2508    Pass,
2509    Fail,
2510    Pending,
2511    Unknown,
2512}
2513
2514#[derive(Debug, Deserialize)]
2515#[serde(rename_all = "camelCase")]
2516struct GhPr {
2517    #[serde(default)]
2518    url: String,
2519    #[serde(default)]
2520    number: u64,
2521    #[serde(default)]
2522    state: String,
2523    #[serde(default)]
2524    title: String,
2525    #[serde(default)]
2526    status_check_rollup: Vec<GhCheck>,
2527    /// GitHub's own verdict on whether the pull request can be merged.
2528    ///
2529    /// Worth asking for because it is the only place the *required* check set
2530    /// is applied: the rollup lists every check equally, so a repository that
2531    /// deliberately does not require `coverage` still looks red here. See
2532    /// [`Blocking`].
2533    #[serde(default)]
2534    merge_state_status: String,
2535    #[serde(default)]
2536    reviews: Vec<GhReview>,
2537    #[serde(default)]
2538    comments: Vec<GhComment>,
2539}
2540
2541/// One rollup entry. `gh` mixes two GraphQL types in this array: a `CheckRun`
2542/// has `name`/`status`/`conclusion`, while a `StatusContext` - the old commit
2543/// status API, which is how CodeRabbit reports - has `context`/`state` and no
2544/// conclusion at all.
2545#[derive(Debug, Deserialize)]
2546#[serde(rename_all = "camelCase")]
2547struct GhCheck {
2548    #[serde(default)]
2549    name: Option<String>,
2550    #[serde(default)]
2551    context: Option<String>,
2552    #[serde(default)]
2553    status: Option<String>,
2554    #[serde(default)]
2555    conclusion: Option<String>,
2556    #[serde(default)]
2557    state: Option<String>,
2558    #[serde(default)]
2559    details_url: Option<String>,
2560    #[serde(default)]
2561    target_url: Option<String>,
2562}
2563
2564impl GhCheck {
2565    /// Name to show a human and hand to the fixer.
2566    fn label(&self) -> String {
2567        self.name
2568            .clone()
2569            .or_else(|| self.context.clone())
2570            .unwrap_or_else(|| "(unnamed check)".to_owned())
2571    }
2572
2573    /// Where this check's logs live, when it has any.
2574    fn url(&self) -> Option<&str> {
2575        self.details_url
2576            .as_deref()
2577            .or(self.target_url.as_deref())
2578            .filter(|u| !u.is_empty())
2579    }
2580
2581    /// Did it pass?
2582    ///
2583    /// `SKIPPED` and `NEUTRAL` count as passed: the Claude review workflow
2584    /// skips release and bot pull requests by design, and a skip that blocked
2585    /// landing would block exactly the pull requests that need no review.
2586    /// `CANCELLED` counts as failed - a cancelled check did not pass, and
2587    /// merging over one is merging over a check that never ran.
2588    fn verdict(&self) -> Verdict {
2589        if let Some(status) = self.status.as_deref() {
2590            if !status.eq_ignore_ascii_case("COMPLETED") {
2591                return Verdict::Pending;
2592            }
2593        }
2594        let outcome = self
2595            .conclusion
2596            .as_deref()
2597            .or(self.state.as_deref())
2598            .unwrap_or("");
2599        match outcome.to_ascii_uppercase().as_str() {
2600            "SUCCESS" | "SKIPPED" | "NEUTRAL" => Verdict::Pass,
2601            "FAILURE" | "ERROR" | "TIMED_OUT" | "CANCELLED" | "STARTUP_FAILURE"
2602            | "ACTION_REQUIRED" => Verdict::Fail,
2603            "PENDING" | "EXPECTED" | "QUEUED" | "IN_PROGRESS" | "WAITING" | "REQUESTED" => {
2604                Verdict::Pending
2605            }
2606            _ => Verdict::Unknown,
2607        }
2608    }
2609}
2610
2611#[derive(Debug, Deserialize)]
2612struct GhAuthor {
2613    #[serde(default)]
2614    login: String,
2615}
2616
2617#[derive(Debug, Deserialize)]
2618struct GhReview {
2619    #[serde(default)]
2620    author: GhAuthor,
2621    #[serde(default)]
2622    body: String,
2623}
2624
2625#[derive(Debug, Deserialize)]
2626struct GhComment {
2627    #[serde(default)]
2628    author: GhAuthor,
2629    #[serde(default)]
2630    body: String,
2631}
2632
2633#[derive(Debug, Deserialize)]
2634struct GhUser {
2635    #[serde(default)]
2636    login: String,
2637}
2638
2639#[derive(Debug, Deserialize)]
2640struct GhInline {
2641    #[serde(default)]
2642    user: GhUser,
2643    #[serde(default)]
2644    path: Option<String>,
2645    #[serde(default)]
2646    line: Option<u64>,
2647    #[serde(default)]
2648    body: String,
2649}
2650
2651impl Default for GhAuthor {
2652    fn default() -> Self {
2653        Self {
2654            login: "(unknown)".to_owned(),
2655        }
2656    }
2657}
2658
2659impl Default for GhUser {
2660    fn default() -> Self {
2661        Self {
2662            login: "(unknown)".to_owned(),
2663        }
2664    }
2665}
2666
2667#[cfg(test)]
2668mod tests {
2669    use super::*;
2670    use crate::run::{Candidate, ReviewRecord, ReviewRound, Tally};
2671
2672    /// 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.
2673    const GREEN_OPEN: &str = r####"{
2674  "url": "https://github.com/yukimemi/magi/pull/10",
2675  "number": 10,
2676  "state": "OPEN",
2677  "mergeStateStatus": "CLEAN",
2678  "statusCheckRollup": [
2679    {
2680      "__typename": "CheckRun",
2681      "conclusion": "SKIPPED",
2682      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33356278334/job/99378963755",
2683      "name": "review",
2684      "status": "COMPLETED",
2685      "workflowName": "claude-review"
2686    },
2687    {
2688      "__typename": "CheckRun",
2689      "conclusion": "SUCCESS",
2690      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33356278338/job/99378963144",
2691      "name": "check (ubuntu-latest)",
2692      "status": "COMPLETED",
2693      "workflowName": "CI"
2694    },
2695    {
2696      "__typename": "CheckRun",
2697      "conclusion": "SUCCESS",
2698      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33356278338/job/99378963095",
2699      "name": "rustfmt",
2700      "status": "COMPLETED",
2701      "workflowName": "CI"
2702    },
2703    {
2704      "__typename": "StatusContext",
2705      "context": "CodeRabbit",
2706      "state": "SUCCESS",
2707      "targetUrl": ""
2708    }
2709  ],
2710  "reviews": [],
2711  "comments": [
2712    {
2713      "author": {
2714        "login": "coderabbitai"
2715      },
2716      "authorAssociation": "NONE",
2717      "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"
2718    }
2719  ]
2720}"####;
2721
2722    /// Real output for the open pull request #9 (the daily kata-apply), whose `editorconfig` check failed while everything else passed.
2723    const RED_OPEN: &str = r####"{
2724  "url": "https://github.com/yukimemi/magi/pull/9",
2725  "number": 9,
2726  "state": "OPEN",
2727  "mergeStateStatus": "UNSTABLE",
2728  "statusCheckRollup": [
2729    {
2730      "__typename": "CheckRun",
2731      "conclusion": "SUCCESS",
2732      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323744",
2733      "name": "check (ubuntu-latest)",
2734      "status": "COMPLETED",
2735      "workflowName": "CI"
2736    },
2737    {
2738      "__typename": "CheckRun",
2739      "conclusion": "SUCCESS",
2740      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323811",
2741      "name": "rustfmt",
2742      "status": "COMPLETED",
2743      "workflowName": "CI"
2744    },
2745    {
2746      "__typename": "CheckRun",
2747      "conclusion": "FAILURE",
2748      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323572",
2749      "name": "editorconfig",
2750      "status": "COMPLETED",
2751      "workflowName": "CI"
2752    },
2753    {
2754      "__typename": "StatusContext",
2755      "context": "CodeRabbit",
2756      "state": "SUCCESS",
2757      "targetUrl": ""
2758    }
2759  ],
2760  "reviews": [],
2761  "comments": [
2762    {
2763      "author": {
2764        "login": "coderabbitai"
2765      },
2766      "authorAssociation": "NONE",
2767      "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"
2768    }
2769  ]
2770}"####;
2771
2772    /// 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.
2773    const PENDING_OPEN: &str = r####"{
2774  "url": "https://github.com/yukimemi/magi/pull/9",
2775  "number": 9,
2776  "state": "OPEN",
2777  "statusCheckRollup": [
2778    {
2779      "__typename": "CheckRun",
2780      "conclusion": "SUCCESS",
2781      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323744",
2782      "name": "check (ubuntu-latest)",
2783      "status": "COMPLETED",
2784      "workflowName": "CI"
2785    },
2786    {
2787      "__typename": "CheckRun",
2788      "conclusion": "SUCCESS",
2789      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323811",
2790      "name": "rustfmt",
2791      "status": "COMPLETED",
2792      "workflowName": "CI"
2793    },
2794    {
2795      "__typename": "CheckRun",
2796      "conclusion": null,
2797      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323572",
2798      "name": "editorconfig",
2799      "status": "IN_PROGRESS",
2800      "workflowName": "CI"
2801    },
2802    {
2803      "__typename": "StatusContext",
2804      "context": "CodeRabbit",
2805      "state": "SUCCESS",
2806      "targetUrl": ""
2807    }
2808  ],
2809  "reviews": [],
2810  "comments": []
2811}"####;
2812
2813    /// Real output for pull request #16 after it was merged - the shape landing sees when a person merged underneath it.
2814    const MERGED: &str = r####"{
2815  "url": "https://github.com/yukimemi/magi/pull/16",
2816  "number": 16,
2817  "state": "MERGED",
2818  "statusCheckRollup": [
2819    {
2820      "__typename": "CheckRun",
2821      "conclusion": "SUCCESS",
2822      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33636587933/job/100268878095",
2823      "name": "check (ubuntu-latest)",
2824      "status": "COMPLETED",
2825      "workflowName": "CI"
2826    },
2827    {
2828      "__typename": "CheckRun",
2829      "conclusion": "SUCCESS",
2830      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33636587918/job/100268876427",
2831      "name": "review",
2832      "status": "COMPLETED",
2833      "workflowName": "claude-review"
2834    }
2835  ],
2836  "reviews": [],
2837  "comments": []
2838}"####;
2839
2840    /// 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.
2841    const REVIEWED_OPEN: &str = r####"{
2842  "url": "https://github.com/yukimemi/magi/pull/12",
2843  "number": 12,
2844  "state": "OPEN",
2845  "statusCheckRollup": [
2846    {
2847      "__typename": "CheckRun",
2848      "conclusion": "SUCCESS",
2849      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33571212506/job/100065355258",
2850      "name": "check (ubuntu-latest)",
2851      "status": "COMPLETED",
2852      "workflowName": "CI"
2853    },
2854    {
2855      "__typename": "CheckRun",
2856      "conclusion": "SUCCESS",
2857      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33571212566/job/100065355810",
2858      "name": "review",
2859      "status": "COMPLETED",
2860      "workflowName": "claude-review"
2861    }
2862  ],
2863  "reviews": [
2864    {
2865      "author": {
2866        "login": "claude"
2867      },
2868      "state": "COMMENTED",
2869      "body": ""
2870    }
2871  ],
2872  "comments": [
2873    {
2874      "author": {
2875        "login": "coderabbitai"
2876      },
2877      "authorAssociation": "NONE",
2878      "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"
2879    },
2880    {
2881      "author": {
2882        "login": "claude"
2883      },
2884      "authorAssociation": "NONE",
2885      "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"
2886    }
2887  ]
2888}"####;
2889
2890    /// Real `gh api repos/{owner}/{repo}/pulls/12/comments` output: one inline finding with its file and line.
2891    const INLINE: &str = r####"[
2892  {
2893    "user": {
2894      "login": "claude[bot]"
2895    },
2896    "path": "src/graph.rs",
2897    "line": 231,
2898    "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"
2899  }
2900]"####;
2901
2902    /// CodeRabbit's real trigger notice: a checkbox, a `<details>` block, and its own "skip review" marker.
2903    const CODERABBIT_TRIGGER: &str = r####"<!-- This is an auto-generated comment: summarize by coderabbit.ai -->
2904<!-- This is an auto-generated comment: skip review by coderabbit.ai -->
2905
2906> [!IMPORTANT]
2907> - [ ] <!-- {"checkboxId":"e9bb8d72-00e8-4f67-9cb2-caf3b22574fe"} --> 🔍 Trigger review
2908> 
2909> This repository does not receive automatic reviews because it has fewer than 10 stars.
2910> 
2911> <details>
2912> <summary>⚙️ Run configuration</summary>
2913> 
2914> **Configuration used**: defaults
2915> 
2916> **Review profile**: CHILL
2917> 
2918> **Plan**: Team
2919> 
2920> **Run ID**: `c1e2a68f-87fc-4b35-9ec4-e75c7854966a`
2921> 
2922> </details>
2923
2924<!-- end of auto-generated comment: skip review by coderabbit.ai -->
2925
2926<!-- tips_start -->
2927
2928---
2929
2930Thanks 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.
2931
2932<details>
2933<summary>❤️ Share</summary>
2934
2935- [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"####;
2936
2937    /// 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.
2938    const CLAUDE_CHECKLIST: &str = r####"**Claude finished @yukimemi's task in 4m 14s** —— [View job](https://github.com/yukimemi/magi/actions/runs/33636587918)
2939
2940---
2941### Reviewing PR #16
2942
2943- [x] Read AGENTS.md conventions
2944- [x] Review `src/daemon.rs` changes
2945- [x] Review `src/main.rs` changes (new `doctor` reporting)
2946- [x] Review `src/web.rs` changes (reuse of unreadable-run count)
2947- [x] Check test coverage for new behavior
2948- [x] Run verification commands (blocked — see note)
2949- [x] Post findings"####;
2950
2951    /// The same job's real comment on pull request #12 once it had something to say.
2952    const CLAUDE_FINDING: &str = r####"**Claude finished @yukimemi's task in 3m 52s** —— [View job](https://github.com/yukimemi/magi/actions/runs/33571212566)
2953
2954---
2955### Review: `magi review <branch>` — cheap-half-only graph
2956
2957Read 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.
2958
2959**Correctness**
2960
2961- 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"####;
2962
2963    fn pr(checks: Checks, failing: &[&str], comments: usize) -> PrState {
2964        PrState {
2965            url: "https://github.com/yukimemi/magi/pull/16".to_owned(),
2966            number: 16,
2967            state: PrLifecycle::Open,
2968            checks,
2969            // These tests are about red-means-fix, so a red here is one the
2970            // forge gates on. Without saying so they would assert the new
2971            // "merge past a check nobody requires" path by accident.
2972            blocking: if matches!(checks, Checks::Red) {
2973                Blocking::Yes
2974            } else {
2975                Blocking::No
2976            },
2977            failing: failing.iter().map(|s| (*s).to_owned()).collect(),
2978            review_comments: (0..comments)
2979                .map(|i| ReviewComment {
2980                    author: "coderabbitai".to_owned(),
2981                    path: Some("src/graph.rs".to_owned()),
2982                    line: Some(231),
2983                    body: format!("finding {i}"),
2984                })
2985                .collect(),
2986        }
2987    }
2988
2989    #[test]
2990    fn a_green_pull_request_with_nothing_outstanding_parses_as_ready_to_merge() {
2991        let state = parse_pr(GREEN_OPEN).expect("green fixture parses");
2992        assert_eq!(state.number, 10);
2993        assert_eq!(state.state, PrLifecycle::Open);
2994        assert_eq!(state.checks, Checks::Green);
2995        assert!(state.failing.is_empty());
2996        assert!(
2997            state.review_comments.is_empty(),
2998            "the only comment is CodeRabbit's trigger notice: {:?}",
2999            state.review_comments
3000        );
3001        assert_eq!(decide(&state, 0, 4, Duration::ZERO), Step::Merge);
3002    }
3003
3004    #[test]
3005    fn a_failing_check_parses_as_red_and_is_named() {
3006        let state = parse_pr(RED_OPEN).expect("red fixture parses");
3007        assert_eq!(state.checks, Checks::Red);
3008        assert_eq!(state.failing, vec!["editorconfig".to_owned()]);
3009        // The captured payload says `UNSTABLE` - mergeable, with a check
3010        // nobody requires red - which is exactly the shape that had to be
3011        // merged by hand. Asserted separately, in
3012        // `a_red_check_nobody_requires_does_not_buy_a_fix_round`. What this
3013        // test is about is that a red check is *named*, so the reason a fixer
3014        // is handed says which one; so it asks the blocking question here.
3015        let mut blocking = state.clone();
3016        blocking.blocking = Blocking::Yes;
3017        match decide(&blocking, 0, 4, Duration::ZERO) {
3018            Step::Fix { reason } => {
3019                assert!(reason.contains("editorconfig"), "reason: {reason}");
3020                assert!(reason.contains("failing"), "reason: {reason}");
3021            }
3022            other => panic!("expected a fix round, got {other:?}"),
3023        }
3024    }
3025
3026    #[test]
3027    fn a_check_still_running_parses_as_pending_and_is_waited_for() {
3028        let state = parse_pr(PENDING_OPEN).expect("pending fixture parses");
3029        assert_eq!(state.checks, Checks::Pending);
3030        assert_eq!(decide(&state, 0, 4, Duration::ZERO), Step::Wait);
3031    }
3032
3033    #[test]
3034    fn a_pull_request_merged_underneath_us_is_done_rather_than_a_failure() {
3035        let state = parse_pr(MERGED).expect("merged fixture parses");
3036        assert_eq!(state.state, PrLifecycle::Merged);
3037        assert_eq!(
3038            decide(&state, 0, 4, Duration::ZERO),
3039            Step::Done { merged: true }
3040        );
3041    }
3042
3043    #[test]
3044    fn a_review_that_found_something_is_outstanding_and_holds_the_merge() {
3045        let state = parse_pr(REVIEWED_OPEN).expect("reviewed fixture parses");
3046        assert_eq!(state.checks, Checks::Green);
3047        let authors: Vec<&str> = state
3048            .review_comments
3049            .iter()
3050            .map(|c| c.author.as_str())
3051            .collect();
3052        assert_eq!(
3053            authors,
3054            vec!["claude"],
3055            "CodeRabbit's walkthrough is machinery; Claude's review is a finding"
3056        );
3057        match decide(&state, 0, 4, Duration::ZERO) {
3058            Step::Fix { reason } => assert!(reason.contains("unresolved"), "reason: {reason}"),
3059            other => panic!("expected a fix round, got {other:?}"),
3060        }
3061    }
3062
3063    #[test]
3064    fn inline_review_comments_keep_their_file_and_line() {
3065        let comments = parse_inline_comments(INLINE).expect("inline fixture parses");
3066        assert_eq!(comments.len(), 1);
3067        assert_eq!(comments[0].author, "claude[bot]");
3068        assert_eq!(comments[0].path.as_deref(), Some("src/graph.rs"));
3069        assert_eq!(comments[0].line, Some(231));
3070        assert!(comments[0].body.contains("empty"), "{}", comments[0].body);
3071    }
3072
3073    #[test]
3074    fn a_status_only_bot_comment_does_not_trigger_a_fix_round() {
3075        assert!(
3076            is_noise(CODERABBIT_TRIGGER),
3077            "CodeRabbit's trigger notice declares itself not a review"
3078        );
3079        assert!(
3080            is_noise(CLAUDE_CHECKLIST),
3081            "a progress checklist asks for nothing"
3082        );
3083        assert!(
3084            !is_noise(CLAUDE_FINDING),
3085            "a review that names a bug is input, not noise"
3086        );
3087
3088        let mut clean = pr(Checks::Green, &[], 0);
3089        clean.review_comments.push(ReviewComment {
3090            author: "coderabbitai".to_owned(),
3091            path: None,
3092            line: None,
3093            body: CODERABBIT_TRIGGER.to_owned(),
3094        });
3095        clean.review_comments.retain(|c| !is_noise(&c.body));
3096        assert_eq!(decide(&clean, 0, 4, Duration::ZERO), Step::Merge);
3097
3098        let mut found = pr(Checks::Green, &[], 0);
3099        found.review_comments.push(ReviewComment {
3100            author: "claude".to_owned(),
3101            path: None,
3102            line: None,
3103            body: CLAUDE_FINDING.to_owned(),
3104        });
3105        found.review_comments.retain(|c| !is_noise(&c.body));
3106        assert!(matches!(
3107            decide(&found, 0, 4, Duration::ZERO),
3108            Step::Fix { .. }
3109        ));
3110    }
3111
3112    #[test]
3113    fn the_policy_table_holds_for_every_combination_that_matters() {
3114        let cases: Vec<(&str, PrState, usize, usize, Duration, Step)> = vec![
3115            (
3116                "pending checks are waited for, even on the last round",
3117                pr(Checks::Pending, &[], 0),
3118                4,
3119                4,
3120                Duration::ZERO,
3121                Step::Wait,
3122            ),
3123            (
3124                "red checks are fixed",
3125                pr(Checks::Red, &["editorconfig"], 0),
3126                0,
3127                4,
3128                Duration::ZERO,
3129                Step::Fix {
3130                    reason: "1 check(s) failing: editorconfig".to_owned(),
3131                },
3132            ),
3133            (
3134                "green with comments is fixed, not merged",
3135                pr(Checks::Green, &[], 2),
3136                1,
3137                4,
3138                Duration::ZERO,
3139                Step::Fix {
3140                    reason: "checks are green but 2 review comment(s) are unresolved: coderabbitai"
3141                        .to_owned(),
3142                },
3143            ),
3144            (
3145                "green and clean merges",
3146                pr(Checks::Green, &[], 0),
3147                3,
3148                4,
3149                Duration::ZERO,
3150                Step::Merge,
3151            ),
3152            (
3153                "an unreadable rollup is waited on while the grace lasts",
3154                pr(Checks::Unknown, &[], 0),
3155                0,
3156                4,
3157                Duration::ZERO,
3158                Step::Wait,
3159            ),
3160            (
3161                "an unreadable rollup is never merged once the grace is spent",
3162                pr(Checks::Unknown, &[], 0),
3163                0,
3164                4,
3165                CHECKS_GRACE,
3166                Step::GiveUp {
3167                    reason: "no check status is readable on the pull request after 3 minute(s); \
3168                             refusing to merge on a guess"
3169                        .to_owned(),
3170                },
3171            ),
3172        ];
3173        for (what, state, round, budget, waited, want) in cases {
3174            assert_eq!(decide(&state, round, budget, waited), want, "{what}");
3175        }
3176    }
3177
3178    #[test]
3179    fn the_forge_verdict_survives_the_round_trip_from_gh() {
3180        // Read off `gh pr view --json ...,mergeStateStatus`, because a field
3181        // requested but never parsed is the kind of thing that looks wired up
3182        // and answers `Unsaid` forever.
3183        let green = parse_pr(GREEN_OPEN).expect("parse");
3184        assert_eq!(green.blocking, Blocking::No);
3185        let red = parse_pr(RED_OPEN).expect("parse");
3186        assert_eq!(
3187            red.blocking,
3188            Blocking::No,
3189            "`UNSTABLE` is mergeable: the red check is one nobody requires"
3190        );
3191        assert_eq!(red.checks, Checks::Red, "and it is still reported as red");
3192        // A payload from an older `gh` has no such field at all.
3193        let quiet =
3194            parse_pr(&GREEN_OPEN.replace("\"mergeStateStatus\": \"CLEAN\",", "")).expect("parse");
3195        assert_eq!(quiet.blocking, Blocking::Unsaid);
3196    }
3197
3198    #[test]
3199    fn a_red_check_nobody_requires_does_not_buy_a_fix_round() {
3200        // Pull request 37's only red check was `editorconfig`, failing
3201        // because the action could not fetch its own binary after
3202        // editorconfig-checker v4 renamed its release assets. The repository
3203        // does not require it. magi answered by asking a fixer to repair a
3204        // change that was fine, and the pull request had to be merged by hand.
3205        let mut nonblocking = pr(Checks::Red, &["editorconfig", "coverage"], 0);
3206        nonblocking.blocking = Blocking::No;
3207        assert_eq!(
3208            decide(&nonblocking, 0, 4, Duration::ZERO),
3209            Step::Merge,
3210            "the forge says nothing is in the way, so nothing is"
3211        );
3212
3213        // The same red, gated on: that is a fix round, as before.
3214        let mut blocking = pr(Checks::Red, &["test (ubuntu-latest)"], 0);
3215        blocking.blocking = Blocking::Yes;
3216        assert!(matches!(
3217            decide(&blocking, 0, 4, Duration::ZERO),
3218            Step::Fix { .. }
3219        ));
3220
3221        // A review comment still outranks green-enough: a non-required red
3222        // must not become a way to merge past an unanswered reviewer.
3223        let mut commented = pr(Checks::Red, &["coverage"], 1);
3224        commented.blocking = Blocking::No;
3225        assert!(matches!(
3226            decide(&commented, 0, 4, Duration::ZERO),
3227            Step::Fix { .. }
3228        ));
3229
3230        // And silence from the forge is not consent.
3231        let mut unsaid = pr(Checks::Red, &["coverage"], 0);
3232        unsaid.blocking = Blocking::Unsaid;
3233        assert!(matches!(
3234            decide(&unsaid, 0, 4, Duration::ZERO),
3235            Step::Fix { .. }
3236        ));
3237    }
3238
3239    #[test]
3240    fn a_red_merge_is_announced_with_every_failing_check_and_a_green_one_is_not() {
3241        let mut red = pr(Checks::Red, &["test (windows-latest)", "coverage"], 0);
3242        red.blocking = Blocking::No;
3243        assert_eq!(
3244            decide(&red, 0, 4, Duration::ZERO),
3245            Step::Merge,
3246            "announcing must not change the decision"
3247        );
3248        let said = red_merge_summary("yukimemi/magi", &red).expect("red merge is announced");
3249        assert!(said.contains("yukimemi/magi"), "{said}");
3250        assert!(said.contains("#16"), "{said}");
3251        assert!(
3252            said.contains("https://github.com/yukimemi/magi/pull/16"),
3253            "{said}"
3254        );
3255        assert!(
3256            said.contains("test (windows-latest)") && said.contains("coverage"),
3257            "{said}"
3258        );
3259
3260        // `failing` can be left over on a green observation; only `checks` counts.
3261        let green = pr(Checks::Green, &["stale"], 0);
3262        assert_eq!(red_merge_summary("yukimemi/magi", &green), None);
3263    }
3264
3265    #[test]
3266    fn the_repo_label_comes_from_the_pull_request_url() {
3267        let p = Path::new("/tmp/checkout");
3268        assert_eq!(
3269            repo_label(p, "https://github.com/yukimemi/magi/pull/16"),
3270            "yukimemi/magi"
3271        );
3272        assert_eq!(repo_label(p, "not a url"), "checkout");
3273    }
3274
3275    #[test]
3276    fn a_branch_the_base_moved_under_is_rebased_not_fixed() {
3277        // Pull requests 35 and 37 were both rebased by hand: a competition
3278        // that runs for two hours against a repository merging pull requests
3279        // all day conflicts on the way in, and that is arithmetic rather
3280        // than a defect in the change.
3281        let mut conflicted = pr(Checks::Green, &[], 0);
3282        conflicted.blocking = Blocking::Conflict;
3283        assert_eq!(decide(&conflicted, 0, 4, Duration::ZERO), Step::Rebase);
3284
3285        // Decided before the checks, and even with the rounds spent: every
3286        // check on a branch that cannot land is an answer about a state that
3287        // cannot land, and a conflict is not the change's fault.
3288        let mut red = pr(Checks::Red, &["test (ubuntu-latest)"], 2);
3289        red.blocking = Blocking::Conflict;
3290        assert_eq!(decide(&red, 4, 4, Duration::ZERO), Step::Rebase);
3291
3292        // The lifecycle still wins over everything, conflict included.
3293        let mut merged = pr(Checks::Red, &[], 0);
3294        merged.blocking = Blocking::Conflict;
3295        merged.state = PrLifecycle::Merged;
3296        assert_eq!(
3297            decide(&merged, 0, 4, Duration::ZERO),
3298            Step::Done { merged: true }
3299        );
3300    }
3301
3302    #[test]
3303    fn the_forge_verdict_is_read_off_merge_state_status() {
3304        // The spellings that mean "mergeable". `UNSTABLE` is the one that
3305        // matters: mergeable, with a non-required check red or still running.
3306        for ok in ["CLEAN", "UNSTABLE", "unstable", "HAS_HOOKS"] {
3307            assert_eq!(Blocking::of(ok), Blocking::No, "{ok}");
3308            assert!(!Blocking::of(ok).stops_a_merge(), "{ok}");
3309        }
3310        assert_eq!(Blocking::of("DIRTY"), Blocking::Conflict);
3311        assert_eq!(Blocking::of("BLOCKED"), Blocking::Yes);
3312        assert_eq!(Blocking::of("BEHIND"), Blocking::Yes);
3313        // An older `gh`, or a token without the scope, says nothing - and
3314        // refusing to guess is the rule everywhere else in this module.
3315        for quiet in ["", "UNKNOWN"] {
3316            assert_eq!(Blocking::of(quiet), Blocking::Unsaid);
3317            assert!(Blocking::of(quiet).stops_a_merge());
3318        }
3319    }
3320
3321    #[test]
3322    fn a_merge_command_that_failed_after_merging_is_still_a_merge() {
3323        let argv = merge_argv(28, "fix: retry uploads on transient network errors");
3324        // The exact stderr from run ec12, in a jj-colocated repository.
3325        let jj = "could not determine current branch: failed to run git: not on any branch";
3326
3327        let landed = merged_after_all(&argv, jj, Some(PrLifecycle::Merged))
3328            .expect("the forge says merged, so it merged");
3329        assert!(landed.ok);
3330        assert!(
3331            landed.detail.contains("but the pull request is merged"),
3332            "the record must not read as a clean success: {}",
3333            landed.detail
3334        );
3335        assert!(
3336            landed.detail.contains("not on any branch"),
3337            "and it must keep what the command actually said: {}",
3338            landed.detail
3339        );
3340
3341        // A pull request still open means the merge really failed.
3342        assert!(merged_after_all(&argv, jj, Some(PrLifecycle::Open)).is_none());
3343        assert!(merged_after_all(&argv, jj, Some(PrLifecycle::Closed)).is_none());
3344        // And an unreadable answer is not evidence of success.
3345        assert!(merged_after_all(&argv, jj, None).is_none());
3346    }
3347
3348    #[test]
3349    fn a_pull_request_closed_underneath_us_is_done_and_not_merged() {
3350        let mut state = pr(Checks::Red, &["editorconfig"], 3);
3351        state.state = PrLifecycle::Closed;
3352        assert_eq!(
3353            decide(&state, 0, 4, Duration::ZERO),
3354            Step::Done { merged: false },
3355            "a human closing the pull request ends the loop, whatever CI says"
3356        );
3357    }
3358
3359    #[test]
3360    fn the_last_round_gives_up_with_a_reason_naming_what_is_still_failing() {
3361        let red = decide(
3362            &pr(Checks::Red, &["editorconfig", "test (macos)"], 0),
3363            4,
3364            4,
3365            Duration::ZERO,
3366        );
3367        match red {
3368            Step::GiveUp { reason } => {
3369                assert!(reason.contains("editorconfig"), "reason: {reason}");
3370                assert!(reason.contains("test (macos)"), "reason: {reason}");
3371                assert!(reason.contains("4 fix round(s)"), "reason: {reason}");
3372            }
3373            other => panic!("expected a give-up, got {other:?}"),
3374        }
3375
3376        let commented = decide(&pr(Checks::Green, &[], 1), 2, 2, Duration::ZERO);
3377        match commented {
3378            Step::GiveUp { reason } => {
3379                assert!(reason.contains("unresolved"), "reason: {reason}");
3380                assert!(reason.contains("2 fix round(s)"), "reason: {reason}");
3381            }
3382            other => panic!("expected a give-up, got {other:?}"),
3383        }
3384    }
3385
3386    #[test]
3387    fn the_merge_command_squashes_deletes_the_branch_and_sets_its_own_subject() {
3388        let candidate_commit = "magi: candidate A (uncommitted work)";
3389        let subject = merge_subject(candidate_commit, "add retries to the uploader");
3390        let argv = merge_argv(16, &subject);
3391
3392        assert!(argv.contains(&"--squash".to_owned()));
3393        assert!(argv.contains(&"--delete-branch".to_owned()));
3394        assert!(argv.contains(&"--subject".to_owned()));
3395        assert_eq!(
3396            argv.last().map(String::as_str),
3397            Some("add retries to the uploader"),
3398            "the subject must not be the candidate commit message"
3399        );
3400        assert_ne!(subject, candidate_commit);
3401    }
3402
3403    #[test]
3404    fn a_real_pull_request_title_is_used_as_the_squash_subject_verbatim() {
3405        assert_eq!(
3406            merge_subject("feat: a queue, an unattended loop, and a phone UI", "task"),
3407            "feat: a queue, an unattended loop, and a phone UI"
3408        );
3409        assert_eq!(
3410            merge_subject("", "# port the retry logic\n\ndetails"),
3411            "port the retry logic",
3412            "an empty title falls back to the task's first line, heading marks stripped"
3413        );
3414    }
3415
3416    #[test]
3417    fn a_failing_checks_details_url_yields_the_job_to_read_logs_from() {
3418        let url = "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323572";
3419        assert_eq!(job_of(url).as_deref(), Some("100114323572"));
3420        assert_eq!(run_of(url).as_deref(), Some("33587406996"));
3421        assert_eq!(job_of("https://coderabbit.ai/status"), None);
3422        assert_eq!(run_of(""), None);
3423    }
3424
3425    #[test]
3426    fn magis_own_stop_comment_is_never_read_back_as_a_finding() {
3427        let mut out = Vec::new();
3428        push_if_outstanding(
3429            &mut out,
3430            ReviewComment {
3431                author: "yukimemi".to_owned(),
3432                path: None,
3433                line: None,
3434                body: format!("{MARKER}\nmagi stopped landing this pull request: 1 check failing"),
3435            },
3436        );
3437        assert!(out.is_empty());
3438    }
3439
3440    /// A run with no tally, so [`RunState::winner`] is `None` and the panel
3441    /// falls back to the repository - which keeps these tests free of a
3442    /// worktree, a `git` invocation and a network.
3443    fn run_state() -> RunState {
3444        RunState::new(
3445            std::path::PathBuf::from("/repo/magi"),
3446            "main".to_owned(),
3447            "abcdef1234".to_owned(),
3448            "add retries to the uploader".to_owned(),
3449            crate::config::Config::default(),
3450        )
3451    }
3452
3453    fn green_pr() -> PrState {
3454        PrState {
3455            url: "https://github.com/yukimemi/magi/pull/42".to_owned(),
3456            number: 42,
3457            state: PrLifecycle::Open,
3458            checks: Checks::Green,
3459            // The forge sees nothing in the way unless a test says otherwise.
3460            blocking: Blocking::No,
3461            failing: Vec::new(),
3462            review_comments: vec![ReviewComment {
3463                author: "coderabbitai".to_owned(),
3464                path: Some("src/land.rs".to_owned()),
3465                line: Some(212),
3466                body: "this branch never checks the exit code".to_owned(),
3467            }],
3468        }
3469    }
3470
3471    #[test]
3472    fn github_facing_land_text_is_english_whatever_the_language() {
3473        let mut state = run_state();
3474        state.config.graph.language = "ja".to_owned();
3475        let comment = stop_comment(&state.id, "checks are still red");
3476        assert!(comment.is_ascii(), "{comment}");
3477        assert!(comment.starts_with(MARKER));
3478
3479        let p = fix_prompt(&state, &green_pr(), 1, 2, "red", "");
3480        let ja_at = p.find("Write all prose in ja").unwrap();
3481        let rule_at = p.find(crate::prompt::GITHUB_ENGLISH_HEADING).unwrap();
3482        assert!(ja_at < rule_at, "{p}");
3483        assert!(p.contains("stays in Japanese"), "{p}");
3484
3485        state.config.graph.language = "en".to_owned();
3486        let p = fix_prompt(&state, &green_pr(), 1, 2, "red", "");
3487        assert!(p.contains(crate::prompt::GITHUB_ENGLISH_HEADING), "{p}");
3488        assert!(!p.contains("does not apply"), "{p}");
3489    }
3490
3491    const NUMSTAT: &str = "12\t3\tsrc/land.rs\n40\t1\tsrc/web.rs\n-\t-\tassets/logo.png";
3492
3493    fn panel() -> String {
3494        approval_panel(
3495            &run_state(),
3496            &green_pr(),
3497            NUMSTAT,
3498            "diff --git a/src/land.rs b/src/land.rs\n@@ -1,2 +1,2 @@\n-old line\n+new line\n context",
3499            &[
3500                "land: ask before merging".to_owned(),
3501                "land: colour the diff".to_owned(),
3502            ],
3503            "feat: merge approval from the phone",
3504        )
3505    }
3506
3507    #[test]
3508    fn the_approval_panel_carries_the_whole_case_for_the_merge() {
3509        let html = panel();
3510        for needle in [
3511            "42",
3512            "main",
3513            "src/land.rs",
3514            "src/web.rs",
3515            "assets/logo.png",
3516            "feat: merge approval from the phone",
3517            "land: ask before merging",
3518            "land: colour the diff",
3519            "coderabbitai",
3520            "this branch never checks the exit code",
3521            "green",
3522        ] {
3523            assert!(html.contains(needle), "the panel must state `{needle}`");
3524        }
3525    }
3526
3527    /// A candidate whose label is `A` and has won, so [`RunState::winner`]
3528    /// resolves to it.
3529    fn winning_candidate(summary: &str) -> Candidate {
3530        Candidate {
3531            index: 0,
3532            label: 'A',
3533            agent: "opus".to_owned(),
3534            branch: "magi/x/A".to_owned(),
3535            worktree: PathBuf::from("/wt/A"),
3536            summary: summary.to_owned(),
3537            stat: String::new(),
3538            files: 1,
3539            commits: 1,
3540            empty: false,
3541            failed: None,
3542            verified_noop: None,
3543            duration_ms: 0,
3544            folded: false,
3545        }
3546    }
3547
3548    fn uncontested_tally() -> Tally {
3549        Tally {
3550            first_choice: BTreeMap::from([('A', 1)]),
3551            borda: BTreeMap::new(),
3552            winner: 'A',
3553            rankings: 1,
3554            unanimous_initial: true,
3555            deliberated: false,
3556            changed_votes: 0,
3557            unanimous_final: true,
3558            tie_break: None,
3559            judges: 1,
3560            present: 1,
3561            quorum: 1,
3562            met_quorum: true,
3563            uncontested: None,
3564        }
3565    }
3566
3567    fn review_record(reviewer: usize, agent: &str, summary: &str) -> ReviewRecord {
3568        ReviewRecord {
3569            attempts: 0,
3570            reviewer,
3571            agent: agent.to_owned(),
3572            summary: summary.to_owned(),
3573            findings: Vec::new(),
3574            vote: None,
3575            failed: None,
3576            duration_ms: 0,
3577        }
3578    }
3579
3580    fn review_round(round: usize, reviews: Vec<ReviewRecord>) -> ReviewRound {
3581        let answered = reviews.len();
3582        ReviewRound {
3583            round,
3584            head: "abc1234".to_owned(),
3585            verified_head: None,
3586            verified_at: None,
3587            reviews,
3588            e2e: Vec::new(),
3589            verify_retried: false,
3590            e2e_deferred: false,
3591            e2e_defer_reason: None,
3592            fix: None,
3593            blocking: 0,
3594            answered,
3595            expected: answered,
3596            clean: true,
3597            progressed: false,
3598            vote_split: false,
3599            reconsideration: Vec::new(),
3600            verdict: None,
3601        }
3602    }
3603
3604    #[test]
3605    fn the_approval_panel_states_the_task_verbatim_in_either_language() {
3606        let en = panel();
3607        assert!(en.contains("Task"), "{en}");
3608        assert!(en.contains("add retries to the uploader"), "{en}");
3609
3610        let mut state = run_state();
3611        state.config.graph.language = "ja".to_owned();
3612        let ja = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3613        assert!(ja.contains("タスク"), "{ja}");
3614        assert!(
3615            ja.contains("add retries to the uploader"),
3616            "the task itself is not translated: {ja}"
3617        );
3618    }
3619
3620    #[test]
3621    fn the_approval_panel_omits_what_changed_and_review_verdict_with_no_data() {
3622        // `run_state()` has no candidates, no tally and no reviews - exactly
3623        // the shape a run has before anything has judged or reviewed it, and
3624        // the panel must not print an empty box for either.
3625        let html = panel();
3626        assert!(!html.contains("What changed"), "{html}");
3627        assert!(!html.contains("Review verdict"), "{html}");
3628    }
3629
3630    #[test]
3631    fn the_approval_panel_omits_what_changed_when_the_winners_summary_is_empty() {
3632        let mut state = run_state();
3633        state.candidates = vec![winning_candidate("")];
3634        state.tally = Some(uncontested_tally());
3635        let html = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3636        assert!(
3637            !html.contains("What changed"),
3638            "an empty summary must not render an empty box: {html}"
3639        );
3640    }
3641
3642    #[test]
3643    fn the_approval_panel_shows_the_winners_own_account_in_either_language() {
3644        let mut state = run_state();
3645        state.candidates = vec![winning_candidate(
3646            "Added a retry loop around the uploader PUT call.",
3647        )];
3648        state.tally = Some(uncontested_tally());
3649        let en = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3650        assert!(en.contains("What changed"), "{en}");
3651        assert!(
3652            en.contains("Added a retry loop around the uploader PUT call."),
3653            "{en}"
3654        );
3655
3656        state.config.graph.language = "ja".to_owned();
3657        let ja = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3658        assert!(ja.contains("変更内容"), "{ja}");
3659        assert!(
3660            ja.contains("Added a retry loop around the uploader PUT call."),
3661            "{ja}"
3662        );
3663    }
3664
3665    #[test]
3666    fn the_approval_panel_shows_only_the_last_review_rounds_verdict() {
3667        let mut state = run_state();
3668        state.reviews = vec![
3669            review_round(
3670                1,
3671                vec![review_record(1, "alpha", "found a race, sent back")],
3672            ),
3673            review_round(2, vec![review_record(1, "alpha", "race is fixed, clean")]),
3674        ];
3675        let en = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3676        assert!(en.contains("Review verdict"), "{en}");
3677        assert!(en.contains("race is fixed, clean"), "{en}");
3678        assert!(
3679            !en.contains("found a race, sent back"),
3680            "only the round that actually cleared the merge should show: {en}"
3681        );
3682
3683        state.config.graph.language = "ja".to_owned();
3684        let ja = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3685        assert!(ja.contains("レビューの結論"), "{ja}");
3686        assert!(ja.contains("レビュアー"), "{ja}");
3687        assert!(ja.contains("race is fixed, clean"), "{ja}");
3688    }
3689
3690    /// The `incomplete_review = "warn"` policy (see
3691    /// `graph::Runner::review_loop`) can push a `clean` round to
3692    /// `state.reviews` while one seat's own record still has `failed: Some`
3693    /// and an empty `summary` - a seat that never answered, not one that
3694    /// answered with nothing to say.
3695    fn unanswered_review_record(reviewer: usize, agent: &str, reason: &str) -> ReviewRecord {
3696        ReviewRecord {
3697            attempts: 0,
3698            reviewer,
3699            agent: agent.to_owned(),
3700            summary: String::new(),
3701            findings: Vec::new(),
3702            vote: None,
3703            failed: Some(reason.to_owned()),
3704            duration_ms: 0,
3705        }
3706    }
3707
3708    #[test]
3709    fn the_approval_panel_never_shows_an_unanswered_seat_as_a_blank_verdict() {
3710        let mut state = run_state();
3711        state.reviews = vec![review_round(
3712            1,
3713            vec![
3714                review_record(1, "alpha", "clean, nothing to add"),
3715                unanswered_review_record(2, "beta", "timed out"),
3716            ],
3717        )];
3718        let en = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3719        assert!(en.contains("clean, nothing to add"), "{en}");
3720        assert!(
3721            en.contains("produced no answer: timed out"),
3722            "a seat that never answered must say so, not render a blank box: {en}"
3723        );
3724        assert!(
3725            !en.contains("<div style=\"white-space:pre-wrap;font-size:13px\"></div>"),
3726            "no reviewer box may be left empty: {en}"
3727        );
3728
3729        state.config.graph.language = "ja".to_owned();
3730        let ja = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3731        assert!(ja.contains("回答なし: timed out"), "{ja}");
3732    }
3733
3734    #[test]
3735    fn the_approval_panel_contains_nothing_the_frames_policy_would_block() {
3736        let html = panel();
3737        assert!(!html.contains("<script"), "no script survives the csp");
3738        assert!(!html.contains("<form"), "form-action is 'none'");
3739        let pr = green_pr();
3740        assert_eq!(
3741            html.matches("http").count(),
3742            html.matches(pr.url.as_str()).count(),
3743            "the only http url in the panel is the pull request's own link"
3744        );
3745    }
3746
3747    #[test]
3748    fn added_and_removed_diff_lines_are_distinguishable_without_colour() {
3749        let html = panel();
3750        assert!(
3751            html.contains(">+</span>"),
3752            "an added line carries a `+` in the gutter, not only a background"
3753        );
3754        assert!(
3755            html.contains(">-</span>"),
3756            "a removed line carries a `-` in the gutter, not only a background"
3757        );
3758        assert!(
3759            html.contains(">new line</span>"),
3760            "the marker is moved to the gutter, so the body is printed once without it"
3761        );
3762    }
3763
3764    #[test]
3765    fn a_diff_past_the_threshold_is_cut_with_an_honest_count() {
3766        let total = DIFF_MAX_LINES + 100;
3767        let diff: String = (0..total).map(|i| format!("+line {i}\n")).collect();
3768        let html = approval_panel(
3769            &run_state(),
3770            &green_pr(),
3771            NUMSTAT,
3772            &diff,
3773            &[],
3774            "feat: something long",
3775        );
3776        assert!(
3777            html.contains(&format!("100 of {total} diff lines omitted")),
3778            "the note must say exactly how much was cut"
3779        );
3780        assert!(html.contains(&format!("line {}", DIFF_MAX_LINES - 1)));
3781        assert!(
3782            !html.contains(&format!("line {DIFF_MAX_LINES}")),
3783            "nothing past the threshold is rendered"
3784        );
3785        assert!(
3786            html.contains("/repo/magi"),
3787            "the note says where the rest is"
3788        );
3789    }
3790
3791    #[test]
3792    fn a_path_with_html_metacharacters_is_escaped_rather_than_rendered() {
3793        let html = approval_panel(
3794            &run_state(),
3795            &green_pr(),
3796            "1\t2\tsrc/<b>&\"x\"'.rs",
3797            "",
3798            &[],
3799            "subject",
3800        );
3801        assert!(html.contains("src/&lt;b&gt;&amp;&quot;x&quot;&#39;.rs"));
3802        assert!(
3803            !html.contains("<b>"),
3804            "an agent-influenced path must never become markup"
3805        );
3806    }
3807
3808    #[tokio::test]
3809    async fn the_merge_lock_serialises_one_repository_but_never_a_different_one() {
3810        let a = std::path::PathBuf::from("/repo/a");
3811        let b = std::path::PathBuf::from("/repo/b");
3812
3813        let held = repo_merge_lock(&a).lock_owned().await;
3814
3815        // A second, concurrent land run against the *same* repository must
3816        // wait - `try_lock` fails while `held` is alive.
3817        assert!(
3818            repo_merge_lock(&a).try_lock().is_err(),
3819            "a second merge into the same repository must not proceed concurrently"
3820        );
3821
3822        // A run against a *different* repository must not be blocked by it -
3823        // this is what keeps a slow rebase or `gh pr merge` in one
3824        // repository from also stalling a land-approval resume in another.
3825        assert!(
3826            repo_merge_lock(&b).try_lock().is_ok(),
3827            "a different repository's merge lock must be independent"
3828        );
3829
3830        drop(held);
3831        assert!(
3832            repo_merge_lock(&a).try_lock().is_ok(),
3833            "the lock is released once the holder is done"
3834        );
3835    }
3836
3837    #[test]
3838    fn only_the_merge_choice_merges_and_silence_holds() {
3839        let table = [
3840            (None, Approval::Hold),
3841            (Some("merge"), Approval::Merge),
3842            (Some(" merge\n"), Approval::Merge),
3843            (Some("hold"), Approval::Hold),
3844            (Some(""), Approval::Hold),
3845            (Some("yes"), Approval::Hold),
3846        ];
3847        for (answer, want) in table {
3848            assert_eq!(
3849                approval(answer),
3850                want,
3851                "answer {answer:?} must resolve to {want:?}"
3852            );
3853        }
3854    }
3855
3856    #[tokio::test]
3857    async fn a_first_visit_to_the_merge_gate_files_a_question_and_returns_pending_at_once() {
3858        crate::run::set_home(std::env::temp_dir().join("magi-land-approval-test-home"));
3859        let mut state = run_state();
3860        state.config.graph.land_approval = true;
3861        let pr = green_pr();
3862
3863        let gate = approval_gate(&mut state, &pr, "feat: x").await.unwrap();
3864        assert_eq!(gate, ApprovalGate::Pending, "nobody has answered yet");
3865        assert!(
3866            !state.parked,
3867            "approval_gate itself never sets `parked`; only its caller does"
3868        );
3869
3870        let store = ask::Questions::open();
3871        let filed: Vec<_> = store
3872            .list()
3873            .into_iter()
3874            .filter(|q| q.run == state.id)
3875            .collect();
3876        assert_eq!(filed.len(), 1, "exactly one question is filed");
3877        assert_eq!(filed[0].node, APPROVAL_NODE);
3878        assert_eq!(filed[0].choices, vec![APPROVE.to_owned(), HOLD.to_owned()]);
3879        assert!(filed[0].status.open());
3880
3881        // A second visit - standing in for a resumed run whose slot the
3882        // daemon handed to something else while nobody had answered - must
3883        // find the same question rather than filing a second one.
3884        let again = approval_gate(&mut state, &pr, "feat: x").await.unwrap();
3885        assert_eq!(again, ApprovalGate::Pending);
3886        let still_one = store
3887            .list()
3888            .into_iter()
3889            .filter(|q| q.run == state.id)
3890            .count();
3891        assert_eq!(
3892            still_one, 1,
3893            "asking twice must not double-file the question"
3894        );
3895    }
3896
3897    #[tokio::test]
3898    async fn approving_the_existing_question_is_read_back_as_approved() {
3899        crate::run::set_home(std::env::temp_dir().join("magi-land-approval-test-home"));
3900        let mut state = run_state();
3901        state.config.graph.land_approval = true;
3902        let pr = green_pr();
3903        assert_eq!(
3904            approval_gate(&mut state, &pr, "feat: x").await.unwrap(),
3905            ApprovalGate::Pending
3906        );
3907
3908        let store = ask::Questions::open();
3909        let mut q = store
3910            .list()
3911            .into_iter()
3912            .find(|q| q.run == state.id)
3913            .expect("filed above");
3914        q.answer(ask::Answer::Choice(APPROVE.to_owned())).unwrap();
3915        store.put(&mut q).unwrap();
3916
3917        assert_eq!(
3918            approval_gate(&mut state, &pr, "feat: x").await.unwrap(),
3919            ApprovalGate::Approved
3920        );
3921    }
3922
3923    #[tokio::test]
3924    async fn holding_or_abandoning_the_existing_question_is_read_back_as_held() {
3925        crate::run::set_home(std::env::temp_dir().join("magi-land-approval-test-home"));
3926        let store = ask::Questions::open();
3927
3928        let mut held_state = run_state();
3929        held_state.config.graph.land_approval = true;
3930        let pr = green_pr();
3931        approval_gate(&mut held_state, &pr, "feat: x")
3932            .await
3933            .unwrap();
3934        let mut q = store
3935            .list()
3936            .into_iter()
3937            .find(|q| q.run == held_state.id)
3938            .expect("filed above");
3939        q.answer(ask::Answer::Choice(HOLD.to_owned())).unwrap();
3940        store.put(&mut q).unwrap();
3941        assert_eq!(
3942            approval_gate(&mut held_state, &pr, "feat: x")
3943                .await
3944                .unwrap(),
3945            ApprovalGate::Held
3946        );
3947
3948        let mut abandoned_state = run_state();
3949        abandoned_state.config.graph.land_approval = true;
3950        approval_gate(&mut abandoned_state, &pr, "feat: x")
3951            .await
3952            .unwrap();
3953        let mut q = store
3954            .list()
3955            .into_iter()
3956            .find(|q| q.run == abandoned_state.id)
3957            .expect("filed above");
3958        q.abandon("no answer within the timeout");
3959        store.put(&mut q).unwrap();
3960        assert_eq!(
3961            approval_gate(&mut abandoned_state, &pr, "feat: x")
3962                .await
3963                .unwrap(),
3964            ApprovalGate::Held,
3965            "silence must never merge"
3966        );
3967    }
3968
3969    #[test]
3970    fn the_diffstat_table_is_ordered_by_churn_with_binaries_last() {
3971        let rows = parse_numstat(NUMSTAT);
3972        assert_eq!(
3973            rows.iter().map(|r| r.path.as_str()).collect::<Vec<_>>(),
3974            ["src/web.rs", "src/land.rs", "assets/logo.png"]
3975        );
3976        assert_eq!(rows[2].added, None, "a binary file has no line counts");
3977    }
3978    #[test]
3979    fn the_approval_speaks_the_language_the_repository_is_configured_for() {
3980        // Reported from a real run: the merge question arrived in English on a
3981        // repository with `language = "ja"`. magi's own strings have to follow
3982        // that setting too - "it is a literal in Rust" is not an answer.
3983        let mut state = run_state();
3984        state.config.graph.language = "ja".to_owned();
3985        let pr = green_pr();
3986        let commits = ["c1".to_owned()];
3987
3988        let ja = approval_panel(&state, &pr, "3\t1\tsrc/a.rs", "+ x", &commits, "feat: x");
3989        assert!(ja.contains("lang=\"ja\""), "the document must declare it");
3990        assert!(ja.contains("squash されるコミット"), "{ja}");
3991        assert!(ja.contains("レビューコメント"), "{ja}");
3992        assert!(ja.contains("差分"), "{ja}");
3993        assert!(
3994            !ja.contains("Commits being squashed"),
3995            "no English left over"
3996        );
3997
3998        let w = words("ja");
3999        assert!(w.approval_summary(17, "feat: x").contains("マージ"));
4000        assert!(
4001            w.approval_detail("http://x/1", "main", "feat: x")
4002                .contains("パネル")
4003        );
4004
4005        // The evidence itself is language-neutral and must survive either way.
4006        assert!(ja.contains("src/a.rs"), "the diffstat is not prose");
4007        assert!(ja.contains("feat: x"), "nor is the merge subject");
4008
4009        // English stays the default, and a language magi cannot check falls
4010        // back to it rather than shipping a guess.
4011        state.config.graph.language = "en".to_owned();
4012        let en = approval_panel(&state, &pr, "3\t1\tsrc/a.rs", "+ x", &commits, "feat: x");
4013        assert!(en.contains("Commits being squashed"), "{en}");
4014        assert_eq!(words("Klingon").html_lang, "en");
4015    }
4016
4017    /// A `gh pr list` result naming exactly one pull request whose base and
4018    /// merge time both fit the run is exactly the case
4019    /// [`find_external_merge`] exists to act on.
4020    #[test]
4021    fn pick_merged_pr_picks_the_unique_match() {
4022        let json = r#"[
4023            {"url": "https://github.com/o/r/pull/42", "number": 42,
4024             "mergedAt": "2026-09-20T10:00:00Z", "baseRefName": "main"}
4025        ]"#;
4026        let created_at: Timestamp = "2026-09-19T00:00:00Z".parse().unwrap();
4027        let found = pick_merged_pr(json, "main", created_at)
4028            .expect("valid json")
4029            .expect("one unambiguous match");
4030        assert_eq!(found.url, "https://github.com/o/r/pull/42");
4031        assert_eq!(found.number, 42);
4032    }
4033
4034    /// Two candidates surviving the filter is exactly as uninformative as
4035    /// zero — a branch name can be reused across runs — so neither is
4036    /// preferred over the other and nothing is recorded automatically.
4037    #[test]
4038    fn pick_merged_pr_refuses_when_more_than_one_candidate_survives() {
4039        let json = r#"[
4040            {"url": "https://github.com/o/r/pull/42", "number": 42,
4041             "mergedAt": "2026-09-20T10:00:00Z", "baseRefName": "main"},
4042            {"url": "https://github.com/o/r/pull/43", "number": 43,
4043             "mergedAt": "2026-09-21T10:00:00Z", "baseRefName": "main"}
4044        ]"#;
4045        let created_at: Timestamp = "2026-09-19T00:00:00Z".parse().unwrap();
4046        assert_eq!(pick_merged_pr(json, "main", created_at).unwrap(), None);
4047    }
4048
4049    /// A pull request that targets a different base branch cannot be this
4050    /// run's, whatever its head branch is named — a reused branch name from
4051    /// an unrelated task must not be recorded as this run's merge.
4052    #[test]
4053    fn pick_merged_pr_ignores_a_different_base_branch() {
4054        let json = r#"[
4055            {"url": "https://github.com/o/r/pull/42", "number": 42,
4056             "mergedAt": "2026-09-20T10:00:00Z", "baseRefName": "release"}
4057        ]"#;
4058        let created_at: Timestamp = "2026-09-19T00:00:00Z".parse().unwrap();
4059        assert_eq!(pick_merged_pr(json, "main", created_at).unwrap(), None);
4060    }
4061
4062    /// A pull request merged before this run was even created cannot be this
4063    /// run's winner, no matter how its head branch is spelled.
4064    #[test]
4065    fn pick_merged_pr_ignores_a_merge_that_predates_the_run() {
4066        let json = r#"[
4067            {"url": "https://github.com/o/r/pull/42", "number": 42,
4068             "mergedAt": "2026-09-18T10:00:00Z", "baseRefName": "main"}
4069        ]"#;
4070        let created_at: Timestamp = "2026-09-19T00:00:00Z".parse().unwrap();
4071        assert_eq!(pick_merged_pr(json, "main", created_at).unwrap(), None);
4072    }
4073
4074    #[test]
4075    fn slug_of_pr_url_reads_host_owner_and_repo() {
4076        assert_eq!(
4077            slug_of_pr_url("https://github.com/yukimemi/shun/pull/272").as_deref(),
4078            Some("github.com/yukimemi/shun")
4079        );
4080    }
4081
4082    #[test]
4083    fn slug_of_pr_url_refuses_a_url_with_no_pull_segment() {
4084        assert_eq!(slug_of_pr_url("https://github.com/yukimemi/shun"), None);
4085        assert_eq!(slug_of_pr_url("not a url at all"), None);
4086        assert_eq!(slug_of_pr_url("https://github.com"), None);
4087    }
4088
4089    #[test]
4090    fn slug_of_repo_url_reads_host_owner_and_repo() {
4091        assert_eq!(
4092            slug_of_repo_url("https://github.com/yukimemi/magi").as_deref(),
4093            Some("github.com/yukimemi/magi")
4094        );
4095        assert_eq!(slug_of_repo_url("https://github.com"), None);
4096    }
4097
4098    #[test]
4099    fn ensure_same_repo_accepts_a_matching_slug_regardless_of_case() {
4100        ensure_same_repo("github.com/yukimemi/magi", "GitHub.Com/YukiMemi/Magi")
4101            .expect("same repo, different case");
4102    }
4103
4104    /// The shun/8c75 incident: an id-less `--merged` picked this repository's
4105    /// own in-progress run and rewrote its status from a pull request in a
4106    /// completely different repository. This is the guard that must catch
4107    /// that even when an explicit (but wrong) id is given.
4108    #[test]
4109    fn ensure_same_repo_refuses_a_different_repo() {
4110        let err =
4111            ensure_same_repo("github.com/yukimemi/magi", "github.com/yukimemi/shun").unwrap_err();
4112        let msg = format!("{err:#}");
4113        assert!(msg.contains("github.com/yukimemi/magi"), "{msg}");
4114        assert!(msg.contains("github.com/yukimemi/shun"), "{msg}");
4115    }
4116
4117    /// Same owner/repo on two different forge hosts (a GitHub Enterprise
4118    /// instance mirroring a `github.com` repository's name, say) must not be
4119    /// treated as the same repository just because the trailing path
4120    /// matches.
4121    #[test]
4122    fn ensure_same_repo_refuses_the_same_slug_on_a_different_host() {
4123        let err = ensure_same_repo(
4124            "github.com/yukimemi/magi",
4125            "github.example.com/yukimemi/magi",
4126        )
4127        .unwrap_err();
4128        let msg = format!("{err:#}");
4129        assert!(msg.contains("github.com/yukimemi/magi"), "{msg}");
4130        assert!(msg.contains("github.example.com/yukimemi/magi"), "{msg}");
4131    }
4132
4133    /// No winner decided yet means there is no branch to ask GitHub about at
4134    /// all — `find_external_merge` must return `None` without ever spawning
4135    /// `gh`, which this proves by never providing a real repository to spawn
4136    /// it in.
4137    #[tokio::test]
4138    async fn find_external_merge_returns_none_without_a_winner() {
4139        let state = RunState::new(
4140            PathBuf::from("/no/such/repo"),
4141            "main".to_owned(),
4142            "0000000000000000000000000000000000000000".to_owned(),
4143            "irrelevant".to_owned(),
4144            crate::config::Config::default(),
4145        );
4146        assert_eq!(find_external_merge(&state).await.unwrap(), None);
4147    }
4148}