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/// Run the loop against a real pull request until it merges or the budget runs
1773/// out.
1774///
1775/// The caller decides whether landing happens at all: this is only reached when
1776/// `graph.land` is on. Returns the last observation, so the caller can report
1777/// what magi was looking at when it stopped.
1778pub async fn land(state: &mut RunState, pr_url: &str) -> Result<PrState> {
1779    let repo = state.repo.clone();
1780    let budget = state.config.graph.land_rounds;
1781    let mut round = 0usize;
1782    // Counted apart from `round`: a rebase is not a fix, and a base that
1783    // moved is not the change's fault.
1784    let mut rebases = 0usize;
1785    let mut waited = Duration::ZERO;
1786    // Comment bodies the fixer has already been shown. A comment is
1787    // outstanding until it has been handed over once; after that it is a
1788    // recorded decision, not an open question, and re-feeding it would loop the
1789    // budget away on a comment the fixer already declined with an argument.
1790    let mut shown: BTreeSet<String> = BTreeSet::new();
1791
1792    // Marks the run resumable through exactly this function, not through a
1793    // fresh competition: `RunStatus::resumable` excludes only `Merged`,
1794    // `Ready` and `Failed`, and `merge`'s own re-entry guard looks for this
1795    // status specifically to know a resumed run belongs back in `land`
1796    // rather than at a second `gh pr create`. Set on every entry - fresh or
1797    // resumed - because a resume that parked here again must keep reading
1798    // `Landing`, not whatever a first pass through `merge` left behind.
1799    state.status = RunStatus::Landing;
1800    state.event("land", format!("watching {pr_url}"));
1801    state.save()?;
1802
1803    loop {
1804        let seen = observe(&repo, pr_url).await?;
1805        let mut pr = seen.pr;
1806        pr.review_comments.retain(|c| !shown.contains(&c.body));
1807        state.pr = Some(crate::run::PrRecord {
1808            url: pr.url.clone(),
1809            number: pr.number,
1810            state: pr.state.as_str().to_owned(),
1811            checks: pr.checks.as_str().to_owned(),
1812            round,
1813            rounds: budget,
1814        });
1815        state.save()?;
1816
1817        match decide(&pr, round, budget, waited) {
1818            Step::Wait => {
1819                if waited >= WAIT_CEILING {
1820                    let why = format!(
1821                        "checks were still running after {} minutes",
1822                        WAIT_CEILING.as_secs() / 60
1823                    );
1824                    stop(state, &repo, &pr, &why).await?;
1825                    return Ok(pr);
1826                }
1827                waited += POLL;
1828                tokio::time::sleep(POLL).await;
1829            }
1830            Step::Done { merged } => {
1831                state.status = if merged {
1832                    RunStatus::Merged
1833                } else {
1834                    RunStatus::Ready
1835                };
1836                let detail = if merged {
1837                    format!("{} was merged", pr.url)
1838                } else {
1839                    format!("{} was closed without merging", pr.url)
1840                };
1841                state.merge = Some(MergeOutcome {
1842                    mode: MergeMode::Pr,
1843                    ok: merged,
1844                    detail: detail.clone(),
1845                });
1846                state.event("land", detail);
1847                state.save()?;
1848                return Ok(pr);
1849            }
1850            Step::Merge => {
1851                let subject = merge_subject(&seen.title, &state.instruction);
1852                // The owner sees the panel before the one irreversible step,
1853                // and an unanswered question is a hold: silence never merges.
1854                if state.config.graph.land_approval {
1855                    match approval_gate(state, &pr, &subject).await? {
1856                        ApprovalGate::Approved => {}
1857                        ApprovalGate::Held => {
1858                            stop(
1859                                state,
1860                                &repo,
1861                                &pr,
1862                                "the owner did not approve the merge (held or unanswered)",
1863                            )
1864                            .await?;
1865                            return Ok(pr);
1866                        }
1867                        // Filed (or still standing from an earlier visit) and
1868                        // not yet answered. Park here rather than wait: the
1869                        // question survives on disk, the daemon hands this
1870                        // run's slot to something else, and a later resume
1871                        // re-enters `land`, finds the same question, and
1872                        // either merges or stops depending on what it says
1873                        // by then.
1874                        ApprovalGate::Pending => {
1875                            state.parked = true;
1876                            state.event(
1877                                "land",
1878                                "parked awaiting merge approval - resumes once answered",
1879                            );
1880                            state.save()?;
1881                            return Ok(pr);
1882                        }
1883                    }
1884                }
1885                let argv = merge_argv(pr.number, &subject);
1886                let out = {
1887                    let merge_lock = repo_merge_lock(&repo);
1888                    let _merge_slot = merge_lock.lock().await;
1889                    gh(&repo, &argv).await?
1890                };
1891                if out.0 {
1892                    pr.state = PrLifecycle::Merged;
1893                    state.status = RunStatus::Merged;
1894                    state.merge = Some(MergeOutcome {
1895                        mode: MergeMode::Pr,
1896                        ok: true,
1897                        detail: format!("gh {}", argv.join(" ")),
1898                    });
1899                    // The last `state.pr` snapshot is whatever the poll before
1900                    // this merge observed - still `open` - and nothing below
1901                    // refreshes it from GitHub again, so the UI's round rail
1902                    // would otherwise keep animating a merged run forever.
1903                    if let Some(pr_record) = state.pr.as_mut() {
1904                        pr_record.state = pr.state.as_str().to_owned();
1905                    }
1906                    state.event("land", format!("merged {} as `{subject}`", pr.url));
1907                    state.save()?;
1908                    return Ok(pr);
1909                }
1910                let after = observe(&repo, pr_url).await.ok().map(|s| s.pr.state);
1911                if let Some(outcome) = merged_after_all(&argv, &out.1, after) {
1912                    pr.state = PrLifecycle::Merged;
1913                    state.status = RunStatus::Merged;
1914                    state.merge = Some(outcome);
1915                    if let Some(pr_record) = state.pr.as_mut() {
1916                        pr_record.state = pr.state.as_str().to_owned();
1917                    }
1918                    state.event("land", format!("merged {} as `{subject}`", pr.url));
1919                    state.save()?;
1920                    return Ok(pr);
1921                }
1922                stop(
1923                    state,
1924                    &repo,
1925                    &pr,
1926                    &format!("`gh pr merge` failed: {}", out.1),
1927                )
1928                .await?;
1929                return Ok(pr);
1930            }
1931            Step::Rebase => {
1932                // Bounded by the same budget as a fix, because a rebase that
1933                // keeps being needed means the base moves faster than this
1934                // run can land and a person should decide what to do. It
1935                // spends none of that budget: the change is not what is
1936                // wrong.
1937                if rebases >= budget {
1938                    let why = format!(
1939                        "the base moved under this branch {budget} time(s) and it still does \
1940                         not merge; rebasing again would only race it"
1941                    );
1942                    stop(state, &repo, &pr, &why).await?;
1943                    return Ok(pr);
1944                }
1945                rebases += 1;
1946                let Some(branch) = state.winner().map(|w| w.branch.clone()) else {
1947                    stop(
1948                        state,
1949                        &repo,
1950                        &pr,
1951                        "the pull request conflicts and this run has no winning branch to rebase",
1952                    )
1953                    .await?;
1954                    return Ok(pr);
1955                };
1956                let base = state.base_branch.clone();
1957                state.event(
1958                    "land",
1959                    format!("{} no longer merges; rebasing onto {base}", pr.url),
1960                );
1961                state.save()?;
1962
1963                // Onto the base as the *remote* has it: the local ref may be
1964                // behind, and rebasing onto a stale base produces a branch
1965                // that conflicts all over again.
1966                git::fetch(&repo, "origin", &base).await.ok();
1967                let scratch = state.dir().join("rebase");
1968                let onto = format!("origin/{base}");
1969                match git::rebase_branch_in_temp(&repo, &scratch, &branch, &onto).await {
1970                    Ok(None) => {
1971                        let pushed = {
1972                            let merge_lock = repo_merge_lock(&repo);
1973                            let _merge_slot = merge_lock.lock().await;
1974                            git::push_rewritten(&repo, "origin", &branch).await?
1975                        };
1976                        if !pushed.ok() {
1977                            let why = format!(
1978                                "rebased {branch} but could not push it: {}",
1979                                pushed.stderr.trim()
1980                            );
1981                            stop(state, &repo, &pr, &why).await?;
1982                            return Ok(pr);
1983                        }
1984                        state.event("land", format!("rebased {branch} onto {base}"));
1985                        state.save()?;
1986                        // The forge has to re-run its checks against the
1987                        // rebased head before anything else can be decided.
1988                        waited = Duration::ZERO;
1989                        tokio::time::sleep(POLL).await;
1990                    }
1991                    // A conflict is a decision, not a chore.
1992                    Ok(Some(conflict)) => {
1993                        let why = format!(
1994                            "{} conflicts with {base} and the rebase did not apply: {}",
1995                            pr.url,
1996                            conflict.chars().take(600).collect::<String>()
1997                        );
1998                        stop(state, &repo, &pr, &why).await?;
1999                        return Ok(pr);
2000                    }
2001                    Err(e) => {
2002                        let why = format!("could not rebase {branch} onto {base}: {e:#}");
2003                        stop(state, &repo, &pr, &why).await?;
2004                        return Ok(pr);
2005                    }
2006                }
2007            }
2008            Step::GiveUp { reason } => {
2009                stop(state, &repo, &pr, &reason).await?;
2010                return Ok(pr);
2011            }
2012            Step::Fix { reason } => {
2013                round += 1;
2014                waited = Duration::ZERO;
2015                for c in &pr.review_comments {
2016                    shown.insert(c.body.clone());
2017                }
2018                state.event("land", format!("round {round}: {reason}"));
2019                state.save()?;
2020
2021                let logs = failing_logs(&repo, &seen.failing_urls).await;
2022                let was_red = pr.checks == Checks::Red;
2023                match fix_round(state, &pr, round, budget, &reason, &logs).await? {
2024                    Fixed::Committed => {}
2025                    Fixed::Declined if was_red => {
2026                        let why = format!(
2027                            "the fixer produced no commit while {} check(s) were failing \
2028                             ({}); stopping instead of looping on an unchanged tree",
2029                            pr.failing.len(),
2030                            pr.failing.join(", ")
2031                        );
2032                        stop(state, &repo, &pr, &why).await?;
2033                        return Ok(pr);
2034                    }
2035                    // Comment-driven round with no commit: the fixer read the
2036                    // comments and changed nothing, which is a decision it is
2037                    // allowed to make. The comments are recorded as shown, so
2038                    // the next observation sees a clean pull request.
2039                    Fixed::Declined => state.event(
2040                        "land",
2041                        format!("round {round}: fixer declined the comments, nothing committed"),
2042                    ),
2043                    Fixed::Failed(why) => {
2044                        stop(state, &repo, &pr, &format!("the fix round failed: {why}")).await?;
2045                        return Ok(pr);
2046                    }
2047                }
2048                state.save()?;
2049            }
2050        }
2051    }
2052}
2053
2054/// One observation, plus the two things [`PrState`] deliberately does not carry:
2055/// the title (needed for the squash subject) and where the failing checks'
2056/// logs live.
2057struct Seen {
2058    pr: PrState,
2059    title: String,
2060    failing_urls: Vec<(String, String)>,
2061}
2062
2063/// Read the pull request: `gh pr view` for the rollup and the top-level thread,
2064/// `gh api` for the inline review comments `gh pr view` does not report.
2065async fn observe(repo: &Path, pr_url: &str) -> Result<Seen> {
2066    let view = gh(
2067        repo,
2068        &[
2069            "pr".to_owned(),
2070            "view".to_owned(),
2071            pr_url.to_owned(),
2072            "--json".to_owned(),
2073            "url,number,state,title,statusCheckRollup,reviews,comments,mergeStateStatus".to_owned(),
2074        ],
2075    )
2076    .await?;
2077    if !view.0 {
2078        bail!("gh pr view {pr_url}: {}", view.1);
2079    }
2080    let mut pr = parse_pr(&view.1)?;
2081    let raw: GhPr = serde_json::from_str(&view.1).context("re-read pull request json")?;
2082
2083    let inline = gh(
2084        repo,
2085        &[
2086            "api".to_owned(),
2087            format!("repos/{{owner}}/{{repo}}/pulls/{}/comments", pr.number),
2088        ],
2089    )
2090    .await?;
2091    if inline.0 {
2092        match parse_inline_comments(&inline.1) {
2093            Ok(mut comments) => pr.review_comments.append(&mut comments),
2094            // An unreadable inline thread must not end a landing: the rollup
2095            // and the top-level thread are still real signal.
2096            Err(e) => tracing::warn!("inline review comments unreadable: {e}"),
2097        }
2098    } else {
2099        tracing::warn!("gh api pulls/{}/comments: {}", pr.number, inline.1);
2100    }
2101
2102    let failing_urls = raw
2103        .status_check_rollup
2104        .iter()
2105        .filter(|c| c.verdict() == Verdict::Fail)
2106        .filter_map(|c| c.url().map(|u| (c.label(), u.to_owned())))
2107        .collect();
2108
2109    Ok(Seen {
2110        pr,
2111        title: raw.title,
2112        failing_urls,
2113    })
2114}
2115
2116/// What a fix round did.
2117enum Fixed {
2118    /// The fixer committed something.
2119    Committed,
2120    /// The fixer ran and chose to change nothing.
2121    Declined,
2122    /// The fixer could not run, or said nothing usable.
2123    Failed(String),
2124}
2125
2126/// Hand the failures and the comments to the fixer, then commit and push.
2127///
2128/// The fixer works in the winner's own worktree so its commits land on the
2129/// branch the pull request is built from, and it runs with `allow_write` for
2130/// the same reason.
2131async fn fix_round(
2132    state: &mut RunState,
2133    pr: &PrState,
2134    round: usize,
2135    budget: usize,
2136    reason: &str,
2137    logs: &str,
2138) -> Result<Fixed> {
2139    let winner = state
2140        .winner()
2141        .cloned()
2142        .context("landing needs a winning candidate; none is recorded on this run")?;
2143    let roles = state
2144        .config
2145        .resolve_roles()
2146        .context("resolve the roster for the fix round")?;
2147    // Same rule as the review loop: an explicitly configured fixer, otherwise
2148    // the winner's own author continuing its own conversation - the competition
2149    // is over, so its context is pure benefit.
2150    let (spec, seat_key): (AgentSpec, String) = match &roles.fixer {
2151        Some(f) if f.id != winner.agent => (f.clone(), "fix".to_owned()),
2152        _ => (
2153            state
2154                .config
2155                .agent(&winner.agent)
2156                .cloned()
2157                .unwrap_or_else(|_| roles.implementers[winner.index].clone()),
2158            format!("impl-{}", winner.label),
2159        ),
2160    };
2161
2162    let prompt = fix_prompt(state, pr, round, budget, reason, logs);
2163    let mut seat = seat_of(state, &seat_key, &spec.id);
2164    let artifacts = agent::artifacts_dir(&state.dir());
2165    let prompt = if state.config.cache_dir().is_some() {
2166        format!("{prompt}\n\n{}", prompt::build_cache_note("fix", true))
2167    } else {
2168        prompt
2169    };
2170    let out = agent::invoke(
2171        &spec,
2172        &mut seat,
2173        &Invocation {
2174            cwd: &winner.worktree,
2175            prompt: &prompt,
2176            timeout: Duration::from_secs(state.config.graph.timeout_fix),
2177            allow_write: true,
2178            sessions: state.config.graph.sessions,
2179            artifacts: &artifacts,
2180            stem: &format!("land-{round}"),
2181            run: &state.id,
2182            node: "land",
2183            cache_dir: state.config.cache_dir().as_deref(),
2184            attachments: &[],
2185        },
2186    )
2187    .await;
2188    state.seats.insert(seat.key.clone(), seat);
2189
2190    match out {
2191        Ok(o) if o.quota_exhausted() => {
2192            return Ok(Fixed::Failed(
2193                "rate limited (quota); the fixer could not run".to_owned(),
2194            ));
2195        }
2196        Ok(o) if !o.usable() => {
2197            return Ok(Fixed::Failed(format!(
2198                "the fixer produced nothing usable (exit {:?}, timed out: {})",
2199                o.exit_code, o.timed_out
2200            )));
2201        }
2202        Ok(_) => {}
2203        Err(e) => return Ok(Fixed::Failed(format!("{e:#}"))),
2204    }
2205
2206    let before = git::rev_parse(&winner.worktree, "HEAD").await?;
2207    // An agent that edited files but never committed would otherwise push
2208    // nothing and look like a refusal.
2209    if let Ok(r) = git::rescue_commit(
2210        &winner.worktree,
2211        &format!("magi: land round {round} fixes (uncommitted work)"),
2212    )
2213    .await
2214    {
2215        state.note_withheld("land", &r.withheld);
2216    }
2217    let after = git::rev_parse(&winner.worktree, "HEAD").await?;
2218    if after == before {
2219        return Ok(Fixed::Declined);
2220    }
2221
2222    let remote = state.config.merge.remote.clone();
2223    let push = git::push(&winner.worktree, &remote, &winner.branch).await?;
2224    if !push.ok() {
2225        return Ok(Fixed::Failed(format!(
2226            "pushing {} to {remote} failed: {}",
2227            winner.branch, push.stderr
2228        )));
2229    }
2230    state.event(
2231        "land",
2232        format!("round {round}: pushed a fix to {}", winner.branch),
2233    );
2234    Ok(Fixed::Committed)
2235}
2236
2237/// Fetch or create a seat, keeping its conversation across nodes.
2238fn seat_of(state: &mut RunState, key: &str, agent: &str) -> SeatState {
2239    if let Some(existing) = state.seats.get(key)
2240        && existing.agent == agent
2241    {
2242        return existing.clone();
2243    }
2244    let fresh = SeatState::new(key, agent, state.seed);
2245    state.seats.insert(key.to_owned(), fresh.clone());
2246    fresh
2247}
2248
2249/// What the fixer is told.
2250fn fix_prompt(
2251    state: &RunState,
2252    pr: &PrState,
2253    round: usize,
2254    budget: usize,
2255    reason: &str,
2256    logs: &str,
2257) -> String {
2258    let mut s = format!(
2259        "Your patch is open as a pull request and it is not landing. Land round \
2260         {round} of {budget}.\n\n\
2261         Pull request: {}\n\n\
2262         What is holding it: {reason}\n\n\
2263         # The task\n\n{}\n",
2264        pr.url, state.instruction
2265    );
2266
2267    if pr.failing.is_empty() {
2268        s.push_str("\n# Failing checks\n\n(none)\n");
2269    } else {
2270        let _ = write!(s, "\n# Failing checks\n\n- {}\n", pr.failing.join("\n- "));
2271        if logs.trim().is_empty() {
2272            s.push_str("\nNo log could be read; reproduce the failure locally.\n");
2273        } else {
2274            let _ = write!(s, "\n## Failing log tails\n\n{logs}\n");
2275        }
2276    }
2277
2278    if pr.review_comments.is_empty() {
2279        s.push_str("\n# Review comments\n\n(none)\n");
2280    } else {
2281        s.push_str("\n# Review comments\n");
2282        for c in &pr.review_comments {
2283            let where_ = match (&c.path, c.line) {
2284                (Some(p), Some(l)) => format!(" ({p}:{l})"),
2285                (Some(p), None) => format!(" ({p})"),
2286                _ => String::new(),
2287            };
2288            let _ = write!(s, "\n## {}{where_}\n\n{}\n", c.author, c.body.trim());
2289        }
2290    }
2291
2292    s.push_str(
2293        "\n# Rules\n\n\
2294         1. Fix the cause, never the symptom. Do not delete, skip, or weaken a \
2295            failing test; do not silence a lint with an allow attribute; do not \
2296            stretch a timeout to hide a race. If the check is right, the code is \
2297            wrong.\n\
2298         2. Change nothing the checks and the comments did not raise. A \
2299            drive-by refactor turns a one-line fix into a pull request that \
2300            needs reviewing again.\n\
2301         3. If a comment is wrong, say so with a checkable argument and change \
2302            nothing for it. A declined comment with a reason is a correct \
2303            outcome; a change made to appease a reviewer is not.\n\
2304         4. Commit in this worktree. magi pushes to the pull request's branch \
2305            for you; do not push, merge, or close anything yourself.\n\
2306         5. Never name yourself, your vendor, or your model, anywhere.\n\n\
2307         # Output\n\n\
2308         Say what you changed and why, and what you declined and why.",
2309    );
2310
2311    let language = &state.config.graph.language;
2312    if !(language.trim().is_empty() || language.eq_ignore_ascii_case("en")) {
2313        let _ = write!(s, "\n\nWrite all prose in {language}.");
2314    }
2315    // After the language line, so the exception is the last word on it.
2316    s.push_str(&crate::prompt::github_english(language));
2317    if let Some(overlay) = state.config.prompts.overlay("fix") {
2318        let _ = write!(s, "\n\n{overlay}");
2319    }
2320    s
2321}
2322
2323/// Failing log tails, the way the operator collects them by hand:
2324/// `gh run view --log-failed`.
2325async fn failing_logs(repo: &Path, failing: &[(String, String)]) -> String {
2326    let mut out = String::new();
2327    for (name, url) in failing.iter().take(MAX_LOGS) {
2328        let args = match (job_of(url), run_of(url)) {
2329            (Some(job), _) => vec![
2330                "run".to_owned(),
2331                "view".to_owned(),
2332                "--log-failed".to_owned(),
2333                "--job".to_owned(),
2334                job,
2335            ],
2336            (None, Some(run)) => vec![
2337                "run".to_owned(),
2338                "view".to_owned(),
2339                run,
2340                "--log-failed".to_owned(),
2341            ],
2342            // Not a GitHub Actions check - an external status has no log here.
2343            (None, None) => continue,
2344        };
2345        let (ok, body) = match gh(repo, &args).await {
2346            Ok(v) => v,
2347            Err(e) => (false, format!("{e:#}")),
2348        };
2349        if !ok && body.trim().is_empty() {
2350            continue;
2351        }
2352        let _ = write!(out, "### {name}\n\n```\n{}\n```\n\n", tail(&body, LOG_TAIL));
2353    }
2354    out
2355}
2356
2357/// Job id out of a check's `detailsUrl`
2358/// (`https://github.com/o/r/actions/runs/<run>/job/<job>`).
2359fn job_of(details_url: &str) -> Option<String> {
2360    let after = details_url.split("/job/").nth(1)?;
2361    let id: String = after.chars().take_while(char::is_ascii_digit).collect();
2362    (!id.is_empty()).then_some(id)
2363}
2364
2365/// Workflow run id out of a check's `detailsUrl`.
2366fn run_of(details_url: &str) -> Option<String> {
2367    let after = details_url.split("/actions/runs/").nth(1)?;
2368    let id: String = after.chars().take_while(char::is_ascii_digit).collect();
2369    (!id.is_empty()).then_some(id)
2370}
2371
2372/// The comment `stop` posts. Fixed English, whatever `[graph] language` says:
2373/// it lands on GitHub, not in front of the operator. Pure so a test can hold
2374/// it to that.
2375fn stop_comment(run_id: &str, why: &str) -> String {
2376    format!(
2377        "{MARKER}\nmagi stopped landing this pull request: {why}\n\n\
2378         The branch is untouched and the run is `{run_id}`. Nothing was merged."
2379    )
2380}
2381
2382/// Leave the pull request open, say why on it, and mark the run blocked.
2383///
2384/// The comment is what makes an unattended stop actionable: the operator wakes
2385/// up to a pull request that explains itself rather than to a silent queue.
2386async fn stop(state: &mut RunState, repo: &Path, pr: &PrState, why: &str) -> Result<()> {
2387    let body = stop_comment(&state.id, why);
2388    let posted = gh(
2389        repo,
2390        &[
2391            "pr".to_owned(),
2392            "comment".to_owned(),
2393            pr.number.to_string(),
2394            "--body".to_owned(),
2395            body,
2396        ],
2397    )
2398    .await;
2399    match posted {
2400        Ok((true, _)) => {}
2401        Ok((false, out)) => tracing::warn!("could not comment on {}: {out}", pr.url),
2402        Err(e) => tracing::warn!("could not comment on {}: {e:#}", pr.url),
2403    }
2404    state.status = RunStatus::Blocked;
2405    state.merge = Some(MergeOutcome {
2406        mode: MergeMode::Pr,
2407        ok: false,
2408        detail: why.to_owned(),
2409    });
2410    state.event("land", format!("stopped: {why}"));
2411    state.save()?;
2412    Ok(())
2413}
2414
2415/// Run `gh` in `repo`, returning success and the combined output.
2416///
2417/// Combined because `gh` reports a refused merge on stderr and the pull request
2418/// json on stdout, and both are evidence.
2419///
2420/// `GH_REPO` is stripped from the child's environment: every call site here
2421/// passes an explicit `cwd` (or a full pull request URL) meaning to operate
2422/// on *that* checkout's own remote, and `gh` prefers `GH_REPO` over the
2423/// checkout it is sitting in when no `--repo` flag is given. Left unset, a
2424/// `GH_REPO` the operator happens to have exported for an unrelated script
2425/// would silently redirect [`repo_slug`] (and every other cwd-scoped call
2426/// below) to a different repository than the one actually on disk - which
2427/// for the same-repo guard in [`correct_manual_merge`] would mean the check
2428/// could be made to agree with whatever repository a forged `--merged` URL
2429/// claims, defeating it entirely.
2430async fn gh(cwd: &Path, args: &[String]) -> Result<(bool, String)> {
2431    let out = tokio::process::Command::new("gh")
2432        .args(args)
2433        .current_dir(cwd)
2434        .env_remove("GH_REPO")
2435        .quiet()
2436        .stdin(std::process::Stdio::null())
2437        .output()
2438        .await
2439        .with_context(|| format!("spawn gh {}", args.join(" ")))?;
2440    let mut body = String::from_utf8_lossy(&out.stdout).into_owned();
2441    let err = String::from_utf8_lossy(&out.stderr);
2442    if body.trim().is_empty() {
2443        body = err.into_owned();
2444    } else if !err.trim().is_empty() {
2445        body.push_str(&err);
2446    }
2447    Ok((out.status.success(), body.trim().to_owned()))
2448}
2449
2450/// Verdict of one entry in the status rollup.
2451#[derive(Debug, Clone, Copy, PartialEq, Eq)]
2452enum Verdict {
2453    Pass,
2454    Fail,
2455    Pending,
2456    Unknown,
2457}
2458
2459#[derive(Debug, Deserialize)]
2460#[serde(rename_all = "camelCase")]
2461struct GhPr {
2462    #[serde(default)]
2463    url: String,
2464    #[serde(default)]
2465    number: u64,
2466    #[serde(default)]
2467    state: String,
2468    #[serde(default)]
2469    title: String,
2470    #[serde(default)]
2471    status_check_rollup: Vec<GhCheck>,
2472    /// GitHub's own verdict on whether the pull request can be merged.
2473    ///
2474    /// Worth asking for because it is the only place the *required* check set
2475    /// is applied: the rollup lists every check equally, so a repository that
2476    /// deliberately does not require `coverage` still looks red here. See
2477    /// [`Blocking`].
2478    #[serde(default)]
2479    merge_state_status: String,
2480    #[serde(default)]
2481    reviews: Vec<GhReview>,
2482    #[serde(default)]
2483    comments: Vec<GhComment>,
2484}
2485
2486/// One rollup entry. `gh` mixes two GraphQL types in this array: a `CheckRun`
2487/// has `name`/`status`/`conclusion`, while a `StatusContext` - the old commit
2488/// status API, which is how CodeRabbit reports - has `context`/`state` and no
2489/// conclusion at all.
2490#[derive(Debug, Deserialize)]
2491#[serde(rename_all = "camelCase")]
2492struct GhCheck {
2493    #[serde(default)]
2494    name: Option<String>,
2495    #[serde(default)]
2496    context: Option<String>,
2497    #[serde(default)]
2498    status: Option<String>,
2499    #[serde(default)]
2500    conclusion: Option<String>,
2501    #[serde(default)]
2502    state: Option<String>,
2503    #[serde(default)]
2504    details_url: Option<String>,
2505    #[serde(default)]
2506    target_url: Option<String>,
2507}
2508
2509impl GhCheck {
2510    /// Name to show a human and hand to the fixer.
2511    fn label(&self) -> String {
2512        self.name
2513            .clone()
2514            .or_else(|| self.context.clone())
2515            .unwrap_or_else(|| "(unnamed check)".to_owned())
2516    }
2517
2518    /// Where this check's logs live, when it has any.
2519    fn url(&self) -> Option<&str> {
2520        self.details_url
2521            .as_deref()
2522            .or(self.target_url.as_deref())
2523            .filter(|u| !u.is_empty())
2524    }
2525
2526    /// Did it pass?
2527    ///
2528    /// `SKIPPED` and `NEUTRAL` count as passed: the Claude review workflow
2529    /// skips release and bot pull requests by design, and a skip that blocked
2530    /// landing would block exactly the pull requests that need no review.
2531    /// `CANCELLED` counts as failed - a cancelled check did not pass, and
2532    /// merging over one is merging over a check that never ran.
2533    fn verdict(&self) -> Verdict {
2534        if let Some(status) = self.status.as_deref() {
2535            if !status.eq_ignore_ascii_case("COMPLETED") {
2536                return Verdict::Pending;
2537            }
2538        }
2539        let outcome = self
2540            .conclusion
2541            .as_deref()
2542            .or(self.state.as_deref())
2543            .unwrap_or("");
2544        match outcome.to_ascii_uppercase().as_str() {
2545            "SUCCESS" | "SKIPPED" | "NEUTRAL" => Verdict::Pass,
2546            "FAILURE" | "ERROR" | "TIMED_OUT" | "CANCELLED" | "STARTUP_FAILURE"
2547            | "ACTION_REQUIRED" => Verdict::Fail,
2548            "PENDING" | "EXPECTED" | "QUEUED" | "IN_PROGRESS" | "WAITING" | "REQUESTED" => {
2549                Verdict::Pending
2550            }
2551            _ => Verdict::Unknown,
2552        }
2553    }
2554}
2555
2556#[derive(Debug, Deserialize)]
2557struct GhAuthor {
2558    #[serde(default)]
2559    login: String,
2560}
2561
2562#[derive(Debug, Deserialize)]
2563struct GhReview {
2564    #[serde(default)]
2565    author: GhAuthor,
2566    #[serde(default)]
2567    body: String,
2568}
2569
2570#[derive(Debug, Deserialize)]
2571struct GhComment {
2572    #[serde(default)]
2573    author: GhAuthor,
2574    #[serde(default)]
2575    body: String,
2576}
2577
2578#[derive(Debug, Deserialize)]
2579struct GhUser {
2580    #[serde(default)]
2581    login: String,
2582}
2583
2584#[derive(Debug, Deserialize)]
2585struct GhInline {
2586    #[serde(default)]
2587    user: GhUser,
2588    #[serde(default)]
2589    path: Option<String>,
2590    #[serde(default)]
2591    line: Option<u64>,
2592    #[serde(default)]
2593    body: String,
2594}
2595
2596impl Default for GhAuthor {
2597    fn default() -> Self {
2598        Self {
2599            login: "(unknown)".to_owned(),
2600        }
2601    }
2602}
2603
2604impl Default for GhUser {
2605    fn default() -> Self {
2606        Self {
2607            login: "(unknown)".to_owned(),
2608        }
2609    }
2610}
2611
2612#[cfg(test)]
2613mod tests {
2614    use super::*;
2615    use crate::run::{Candidate, ReviewRecord, ReviewRound, Tally};
2616
2617    /// 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.
2618    const GREEN_OPEN: &str = r####"{
2619  "url": "https://github.com/yukimemi/magi/pull/10",
2620  "number": 10,
2621  "state": "OPEN",
2622  "mergeStateStatus": "CLEAN",
2623  "statusCheckRollup": [
2624    {
2625      "__typename": "CheckRun",
2626      "conclusion": "SKIPPED",
2627      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33356278334/job/99378963755",
2628      "name": "review",
2629      "status": "COMPLETED",
2630      "workflowName": "claude-review"
2631    },
2632    {
2633      "__typename": "CheckRun",
2634      "conclusion": "SUCCESS",
2635      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33356278338/job/99378963144",
2636      "name": "check (ubuntu-latest)",
2637      "status": "COMPLETED",
2638      "workflowName": "CI"
2639    },
2640    {
2641      "__typename": "CheckRun",
2642      "conclusion": "SUCCESS",
2643      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33356278338/job/99378963095",
2644      "name": "rustfmt",
2645      "status": "COMPLETED",
2646      "workflowName": "CI"
2647    },
2648    {
2649      "__typename": "StatusContext",
2650      "context": "CodeRabbit",
2651      "state": "SUCCESS",
2652      "targetUrl": ""
2653    }
2654  ],
2655  "reviews": [],
2656  "comments": [
2657    {
2658      "author": {
2659        "login": "coderabbitai"
2660      },
2661      "authorAssociation": "NONE",
2662      "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"
2663    }
2664  ]
2665}"####;
2666
2667    /// Real output for the open pull request #9 (the daily kata-apply), whose `editorconfig` check failed while everything else passed.
2668    const RED_OPEN: &str = r####"{
2669  "url": "https://github.com/yukimemi/magi/pull/9",
2670  "number": 9,
2671  "state": "OPEN",
2672  "mergeStateStatus": "UNSTABLE",
2673  "statusCheckRollup": [
2674    {
2675      "__typename": "CheckRun",
2676      "conclusion": "SUCCESS",
2677      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323744",
2678      "name": "check (ubuntu-latest)",
2679      "status": "COMPLETED",
2680      "workflowName": "CI"
2681    },
2682    {
2683      "__typename": "CheckRun",
2684      "conclusion": "SUCCESS",
2685      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323811",
2686      "name": "rustfmt",
2687      "status": "COMPLETED",
2688      "workflowName": "CI"
2689    },
2690    {
2691      "__typename": "CheckRun",
2692      "conclusion": "FAILURE",
2693      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323572",
2694      "name": "editorconfig",
2695      "status": "COMPLETED",
2696      "workflowName": "CI"
2697    },
2698    {
2699      "__typename": "StatusContext",
2700      "context": "CodeRabbit",
2701      "state": "SUCCESS",
2702      "targetUrl": ""
2703    }
2704  ],
2705  "reviews": [],
2706  "comments": [
2707    {
2708      "author": {
2709        "login": "coderabbitai"
2710      },
2711      "authorAssociation": "NONE",
2712      "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"
2713    }
2714  ]
2715}"####;
2716
2717    /// 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.
2718    const PENDING_OPEN: &str = r####"{
2719  "url": "https://github.com/yukimemi/magi/pull/9",
2720  "number": 9,
2721  "state": "OPEN",
2722  "statusCheckRollup": [
2723    {
2724      "__typename": "CheckRun",
2725      "conclusion": "SUCCESS",
2726      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323744",
2727      "name": "check (ubuntu-latest)",
2728      "status": "COMPLETED",
2729      "workflowName": "CI"
2730    },
2731    {
2732      "__typename": "CheckRun",
2733      "conclusion": "SUCCESS",
2734      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323811",
2735      "name": "rustfmt",
2736      "status": "COMPLETED",
2737      "workflowName": "CI"
2738    },
2739    {
2740      "__typename": "CheckRun",
2741      "conclusion": null,
2742      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323572",
2743      "name": "editorconfig",
2744      "status": "IN_PROGRESS",
2745      "workflowName": "CI"
2746    },
2747    {
2748      "__typename": "StatusContext",
2749      "context": "CodeRabbit",
2750      "state": "SUCCESS",
2751      "targetUrl": ""
2752    }
2753  ],
2754  "reviews": [],
2755  "comments": []
2756}"####;
2757
2758    /// Real output for pull request #16 after it was merged - the shape landing sees when a person merged underneath it.
2759    const MERGED: &str = r####"{
2760  "url": "https://github.com/yukimemi/magi/pull/16",
2761  "number": 16,
2762  "state": "MERGED",
2763  "statusCheckRollup": [
2764    {
2765      "__typename": "CheckRun",
2766      "conclusion": "SUCCESS",
2767      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33636587933/job/100268878095",
2768      "name": "check (ubuntu-latest)",
2769      "status": "COMPLETED",
2770      "workflowName": "CI"
2771    },
2772    {
2773      "__typename": "CheckRun",
2774      "conclusion": "SUCCESS",
2775      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33636587918/job/100268876427",
2776      "name": "review",
2777      "status": "COMPLETED",
2778      "workflowName": "claude-review"
2779    }
2780  ],
2781  "reviews": [],
2782  "comments": []
2783}"####;
2784
2785    /// 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.
2786    const REVIEWED_OPEN: &str = r####"{
2787  "url": "https://github.com/yukimemi/magi/pull/12",
2788  "number": 12,
2789  "state": "OPEN",
2790  "statusCheckRollup": [
2791    {
2792      "__typename": "CheckRun",
2793      "conclusion": "SUCCESS",
2794      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33571212506/job/100065355258",
2795      "name": "check (ubuntu-latest)",
2796      "status": "COMPLETED",
2797      "workflowName": "CI"
2798    },
2799    {
2800      "__typename": "CheckRun",
2801      "conclusion": "SUCCESS",
2802      "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33571212566/job/100065355810",
2803      "name": "review",
2804      "status": "COMPLETED",
2805      "workflowName": "claude-review"
2806    }
2807  ],
2808  "reviews": [
2809    {
2810      "author": {
2811        "login": "claude"
2812      },
2813      "state": "COMMENTED",
2814      "body": ""
2815    }
2816  ],
2817  "comments": [
2818    {
2819      "author": {
2820        "login": "coderabbitai"
2821      },
2822      "authorAssociation": "NONE",
2823      "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"
2824    },
2825    {
2826      "author": {
2827        "login": "claude"
2828      },
2829      "authorAssociation": "NONE",
2830      "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"
2831    }
2832  ]
2833}"####;
2834
2835    /// Real `gh api repos/{owner}/{repo}/pulls/12/comments` output: one inline finding with its file and line.
2836    const INLINE: &str = r####"[
2837  {
2838    "user": {
2839      "login": "claude[bot]"
2840    },
2841    "path": "src/graph.rs",
2842    "line": 231,
2843    "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"
2844  }
2845]"####;
2846
2847    /// CodeRabbit's real trigger notice: a checkbox, a `<details>` block, and its own "skip review" marker.
2848    const CODERABBIT_TRIGGER: &str = r####"<!-- This is an auto-generated comment: summarize by coderabbit.ai -->
2849<!-- This is an auto-generated comment: skip review by coderabbit.ai -->
2850
2851> [!IMPORTANT]
2852> - [ ] <!-- {"checkboxId":"e9bb8d72-00e8-4f67-9cb2-caf3b22574fe"} --> 🔍 Trigger review
2853> 
2854> This repository does not receive automatic reviews because it has fewer than 10 stars.
2855> 
2856> <details>
2857> <summary>⚙️ Run configuration</summary>
2858> 
2859> **Configuration used**: defaults
2860> 
2861> **Review profile**: CHILL
2862> 
2863> **Plan**: Team
2864> 
2865> **Run ID**: `c1e2a68f-87fc-4b35-9ec4-e75c7854966a`
2866> 
2867> </details>
2868
2869<!-- end of auto-generated comment: skip review by coderabbit.ai -->
2870
2871<!-- tips_start -->
2872
2873---
2874
2875Thanks 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.
2876
2877<details>
2878<summary>❤️ Share</summary>
2879
2880- [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"####;
2881
2882    /// 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.
2883    const CLAUDE_CHECKLIST: &str = r####"**Claude finished @yukimemi's task in 4m 14s** —— [View job](https://github.com/yukimemi/magi/actions/runs/33636587918)
2884
2885---
2886### Reviewing PR #16
2887
2888- [x] Read AGENTS.md conventions
2889- [x] Review `src/daemon.rs` changes
2890- [x] Review `src/main.rs` changes (new `doctor` reporting)
2891- [x] Review `src/web.rs` changes (reuse of unreadable-run count)
2892- [x] Check test coverage for new behavior
2893- [x] Run verification commands (blocked — see note)
2894- [x] Post findings"####;
2895
2896    /// The same job's real comment on pull request #12 once it had something to say.
2897    const CLAUDE_FINDING: &str = r####"**Claude finished @yukimemi's task in 3m 52s** —— [View job](https://github.com/yukimemi/magi/actions/runs/33571212566)
2898
2899---
2900### Review: `magi review <branch>` — cheap-half-only graph
2901
2902Read 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.
2903
2904**Correctness**
2905
2906- 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"####;
2907
2908    fn pr(checks: Checks, failing: &[&str], comments: usize) -> PrState {
2909        PrState {
2910            url: "https://github.com/yukimemi/magi/pull/16".to_owned(),
2911            number: 16,
2912            state: PrLifecycle::Open,
2913            checks,
2914            // These tests are about red-means-fix, so a red here is one the
2915            // forge gates on. Without saying so they would assert the new
2916            // "merge past a check nobody requires" path by accident.
2917            blocking: if matches!(checks, Checks::Red) {
2918                Blocking::Yes
2919            } else {
2920                Blocking::No
2921            },
2922            failing: failing.iter().map(|s| (*s).to_owned()).collect(),
2923            review_comments: (0..comments)
2924                .map(|i| ReviewComment {
2925                    author: "coderabbitai".to_owned(),
2926                    path: Some("src/graph.rs".to_owned()),
2927                    line: Some(231),
2928                    body: format!("finding {i}"),
2929                })
2930                .collect(),
2931        }
2932    }
2933
2934    #[test]
2935    fn a_green_pull_request_with_nothing_outstanding_parses_as_ready_to_merge() {
2936        let state = parse_pr(GREEN_OPEN).expect("green fixture parses");
2937        assert_eq!(state.number, 10);
2938        assert_eq!(state.state, PrLifecycle::Open);
2939        assert_eq!(state.checks, Checks::Green);
2940        assert!(state.failing.is_empty());
2941        assert!(
2942            state.review_comments.is_empty(),
2943            "the only comment is CodeRabbit's trigger notice: {:?}",
2944            state.review_comments
2945        );
2946        assert_eq!(decide(&state, 0, 4, Duration::ZERO), Step::Merge);
2947    }
2948
2949    #[test]
2950    fn a_failing_check_parses_as_red_and_is_named() {
2951        let state = parse_pr(RED_OPEN).expect("red fixture parses");
2952        assert_eq!(state.checks, Checks::Red);
2953        assert_eq!(state.failing, vec!["editorconfig".to_owned()]);
2954        // The captured payload says `UNSTABLE` - mergeable, with a check
2955        // nobody requires red - which is exactly the shape that had to be
2956        // merged by hand. Asserted separately, in
2957        // `a_red_check_nobody_requires_does_not_buy_a_fix_round`. What this
2958        // test is about is that a red check is *named*, so the reason a fixer
2959        // is handed says which one; so it asks the blocking question here.
2960        let mut blocking = state.clone();
2961        blocking.blocking = Blocking::Yes;
2962        match decide(&blocking, 0, 4, Duration::ZERO) {
2963            Step::Fix { reason } => {
2964                assert!(reason.contains("editorconfig"), "reason: {reason}");
2965                assert!(reason.contains("failing"), "reason: {reason}");
2966            }
2967            other => panic!("expected a fix round, got {other:?}"),
2968        }
2969    }
2970
2971    #[test]
2972    fn a_check_still_running_parses_as_pending_and_is_waited_for() {
2973        let state = parse_pr(PENDING_OPEN).expect("pending fixture parses");
2974        assert_eq!(state.checks, Checks::Pending);
2975        assert_eq!(decide(&state, 0, 4, Duration::ZERO), Step::Wait);
2976    }
2977
2978    #[test]
2979    fn a_pull_request_merged_underneath_us_is_done_rather_than_a_failure() {
2980        let state = parse_pr(MERGED).expect("merged fixture parses");
2981        assert_eq!(state.state, PrLifecycle::Merged);
2982        assert_eq!(
2983            decide(&state, 0, 4, Duration::ZERO),
2984            Step::Done { merged: true }
2985        );
2986    }
2987
2988    #[test]
2989    fn a_review_that_found_something_is_outstanding_and_holds_the_merge() {
2990        let state = parse_pr(REVIEWED_OPEN).expect("reviewed fixture parses");
2991        assert_eq!(state.checks, Checks::Green);
2992        let authors: Vec<&str> = state
2993            .review_comments
2994            .iter()
2995            .map(|c| c.author.as_str())
2996            .collect();
2997        assert_eq!(
2998            authors,
2999            vec!["claude"],
3000            "CodeRabbit's walkthrough is machinery; Claude's review is a finding"
3001        );
3002        match decide(&state, 0, 4, Duration::ZERO) {
3003            Step::Fix { reason } => assert!(reason.contains("unresolved"), "reason: {reason}"),
3004            other => panic!("expected a fix round, got {other:?}"),
3005        }
3006    }
3007
3008    #[test]
3009    fn inline_review_comments_keep_their_file_and_line() {
3010        let comments = parse_inline_comments(INLINE).expect("inline fixture parses");
3011        assert_eq!(comments.len(), 1);
3012        assert_eq!(comments[0].author, "claude[bot]");
3013        assert_eq!(comments[0].path.as_deref(), Some("src/graph.rs"));
3014        assert_eq!(comments[0].line, Some(231));
3015        assert!(comments[0].body.contains("empty"), "{}", comments[0].body);
3016    }
3017
3018    #[test]
3019    fn a_status_only_bot_comment_does_not_trigger_a_fix_round() {
3020        assert!(
3021            is_noise(CODERABBIT_TRIGGER),
3022            "CodeRabbit's trigger notice declares itself not a review"
3023        );
3024        assert!(
3025            is_noise(CLAUDE_CHECKLIST),
3026            "a progress checklist asks for nothing"
3027        );
3028        assert!(
3029            !is_noise(CLAUDE_FINDING),
3030            "a review that names a bug is input, not noise"
3031        );
3032
3033        let mut clean = pr(Checks::Green, &[], 0);
3034        clean.review_comments.push(ReviewComment {
3035            author: "coderabbitai".to_owned(),
3036            path: None,
3037            line: None,
3038            body: CODERABBIT_TRIGGER.to_owned(),
3039        });
3040        clean.review_comments.retain(|c| !is_noise(&c.body));
3041        assert_eq!(decide(&clean, 0, 4, Duration::ZERO), Step::Merge);
3042
3043        let mut found = pr(Checks::Green, &[], 0);
3044        found.review_comments.push(ReviewComment {
3045            author: "claude".to_owned(),
3046            path: None,
3047            line: None,
3048            body: CLAUDE_FINDING.to_owned(),
3049        });
3050        found.review_comments.retain(|c| !is_noise(&c.body));
3051        assert!(matches!(
3052            decide(&found, 0, 4, Duration::ZERO),
3053            Step::Fix { .. }
3054        ));
3055    }
3056
3057    #[test]
3058    fn the_policy_table_holds_for_every_combination_that_matters() {
3059        let cases: Vec<(&str, PrState, usize, usize, Duration, Step)> = vec![
3060            (
3061                "pending checks are waited for, even on the last round",
3062                pr(Checks::Pending, &[], 0),
3063                4,
3064                4,
3065                Duration::ZERO,
3066                Step::Wait,
3067            ),
3068            (
3069                "red checks are fixed",
3070                pr(Checks::Red, &["editorconfig"], 0),
3071                0,
3072                4,
3073                Duration::ZERO,
3074                Step::Fix {
3075                    reason: "1 check(s) failing: editorconfig".to_owned(),
3076                },
3077            ),
3078            (
3079                "green with comments is fixed, not merged",
3080                pr(Checks::Green, &[], 2),
3081                1,
3082                4,
3083                Duration::ZERO,
3084                Step::Fix {
3085                    reason: "checks are green but 2 review comment(s) are unresolved: coderabbitai"
3086                        .to_owned(),
3087                },
3088            ),
3089            (
3090                "green and clean merges",
3091                pr(Checks::Green, &[], 0),
3092                3,
3093                4,
3094                Duration::ZERO,
3095                Step::Merge,
3096            ),
3097            (
3098                "an unreadable rollup is waited on while the grace lasts",
3099                pr(Checks::Unknown, &[], 0),
3100                0,
3101                4,
3102                Duration::ZERO,
3103                Step::Wait,
3104            ),
3105            (
3106                "an unreadable rollup is never merged once the grace is spent",
3107                pr(Checks::Unknown, &[], 0),
3108                0,
3109                4,
3110                CHECKS_GRACE,
3111                Step::GiveUp {
3112                    reason: "no check status is readable on the pull request after 3 minute(s); \
3113                             refusing to merge on a guess"
3114                        .to_owned(),
3115                },
3116            ),
3117        ];
3118        for (what, state, round, budget, waited, want) in cases {
3119            assert_eq!(decide(&state, round, budget, waited), want, "{what}");
3120        }
3121    }
3122
3123    #[test]
3124    fn the_forge_verdict_survives_the_round_trip_from_gh() {
3125        // Read off `gh pr view --json ...,mergeStateStatus`, because a field
3126        // requested but never parsed is the kind of thing that looks wired up
3127        // and answers `Unsaid` forever.
3128        let green = parse_pr(GREEN_OPEN).expect("parse");
3129        assert_eq!(green.blocking, Blocking::No);
3130        let red = parse_pr(RED_OPEN).expect("parse");
3131        assert_eq!(
3132            red.blocking,
3133            Blocking::No,
3134            "`UNSTABLE` is mergeable: the red check is one nobody requires"
3135        );
3136        assert_eq!(red.checks, Checks::Red, "and it is still reported as red");
3137        // A payload from an older `gh` has no such field at all.
3138        let quiet =
3139            parse_pr(&GREEN_OPEN.replace("\"mergeStateStatus\": \"CLEAN\",", "")).expect("parse");
3140        assert_eq!(quiet.blocking, Blocking::Unsaid);
3141    }
3142
3143    #[test]
3144    fn a_red_check_nobody_requires_does_not_buy_a_fix_round() {
3145        // Pull request 37's only red check was `editorconfig`, failing
3146        // because the action could not fetch its own binary after
3147        // editorconfig-checker v4 renamed its release assets. The repository
3148        // does not require it. magi answered by asking a fixer to repair a
3149        // change that was fine, and the pull request had to be merged by hand.
3150        let mut nonblocking = pr(Checks::Red, &["editorconfig", "coverage"], 0);
3151        nonblocking.blocking = Blocking::No;
3152        assert_eq!(
3153            decide(&nonblocking, 0, 4, Duration::ZERO),
3154            Step::Merge,
3155            "the forge says nothing is in the way, so nothing is"
3156        );
3157
3158        // The same red, gated on: that is a fix round, as before.
3159        let mut blocking = pr(Checks::Red, &["test (ubuntu-latest)"], 0);
3160        blocking.blocking = Blocking::Yes;
3161        assert!(matches!(
3162            decide(&blocking, 0, 4, Duration::ZERO),
3163            Step::Fix { .. }
3164        ));
3165
3166        // A review comment still outranks green-enough: a non-required red
3167        // must not become a way to merge past an unanswered reviewer.
3168        let mut commented = pr(Checks::Red, &["coverage"], 1);
3169        commented.blocking = Blocking::No;
3170        assert!(matches!(
3171            decide(&commented, 0, 4, Duration::ZERO),
3172            Step::Fix { .. }
3173        ));
3174
3175        // And silence from the forge is not consent.
3176        let mut unsaid = pr(Checks::Red, &["coverage"], 0);
3177        unsaid.blocking = Blocking::Unsaid;
3178        assert!(matches!(
3179            decide(&unsaid, 0, 4, Duration::ZERO),
3180            Step::Fix { .. }
3181        ));
3182    }
3183
3184    #[test]
3185    fn a_branch_the_base_moved_under_is_rebased_not_fixed() {
3186        // Pull requests 35 and 37 were both rebased by hand: a competition
3187        // that runs for two hours against a repository merging pull requests
3188        // all day conflicts on the way in, and that is arithmetic rather
3189        // than a defect in the change.
3190        let mut conflicted = pr(Checks::Green, &[], 0);
3191        conflicted.blocking = Blocking::Conflict;
3192        assert_eq!(decide(&conflicted, 0, 4, Duration::ZERO), Step::Rebase);
3193
3194        // Decided before the checks, and even with the rounds spent: every
3195        // check on a branch that cannot land is an answer about a state that
3196        // cannot land, and a conflict is not the change's fault.
3197        let mut red = pr(Checks::Red, &["test (ubuntu-latest)"], 2);
3198        red.blocking = Blocking::Conflict;
3199        assert_eq!(decide(&red, 4, 4, Duration::ZERO), Step::Rebase);
3200
3201        // The lifecycle still wins over everything, conflict included.
3202        let mut merged = pr(Checks::Red, &[], 0);
3203        merged.blocking = Blocking::Conflict;
3204        merged.state = PrLifecycle::Merged;
3205        assert_eq!(
3206            decide(&merged, 0, 4, Duration::ZERO),
3207            Step::Done { merged: true }
3208        );
3209    }
3210
3211    #[test]
3212    fn the_forge_verdict_is_read_off_merge_state_status() {
3213        // The spellings that mean "mergeable". `UNSTABLE` is the one that
3214        // matters: mergeable, with a non-required check red or still running.
3215        for ok in ["CLEAN", "UNSTABLE", "unstable", "HAS_HOOKS"] {
3216            assert_eq!(Blocking::of(ok), Blocking::No, "{ok}");
3217            assert!(!Blocking::of(ok).stops_a_merge(), "{ok}");
3218        }
3219        assert_eq!(Blocking::of("DIRTY"), Blocking::Conflict);
3220        assert_eq!(Blocking::of("BLOCKED"), Blocking::Yes);
3221        assert_eq!(Blocking::of("BEHIND"), Blocking::Yes);
3222        // An older `gh`, or a token without the scope, says nothing - and
3223        // refusing to guess is the rule everywhere else in this module.
3224        for quiet in ["", "UNKNOWN"] {
3225            assert_eq!(Blocking::of(quiet), Blocking::Unsaid);
3226            assert!(Blocking::of(quiet).stops_a_merge());
3227        }
3228    }
3229
3230    #[test]
3231    fn a_merge_command_that_failed_after_merging_is_still_a_merge() {
3232        let argv = merge_argv(28, "fix: retry uploads on transient network errors");
3233        // The exact stderr from run ec12, in a jj-colocated repository.
3234        let jj = "could not determine current branch: failed to run git: not on any branch";
3235
3236        let landed = merged_after_all(&argv, jj, Some(PrLifecycle::Merged))
3237            .expect("the forge says merged, so it merged");
3238        assert!(landed.ok);
3239        assert!(
3240            landed.detail.contains("but the pull request is merged"),
3241            "the record must not read as a clean success: {}",
3242            landed.detail
3243        );
3244        assert!(
3245            landed.detail.contains("not on any branch"),
3246            "and it must keep what the command actually said: {}",
3247            landed.detail
3248        );
3249
3250        // A pull request still open means the merge really failed.
3251        assert!(merged_after_all(&argv, jj, Some(PrLifecycle::Open)).is_none());
3252        assert!(merged_after_all(&argv, jj, Some(PrLifecycle::Closed)).is_none());
3253        // And an unreadable answer is not evidence of success.
3254        assert!(merged_after_all(&argv, jj, None).is_none());
3255    }
3256
3257    #[test]
3258    fn a_pull_request_closed_underneath_us_is_done_and_not_merged() {
3259        let mut state = pr(Checks::Red, &["editorconfig"], 3);
3260        state.state = PrLifecycle::Closed;
3261        assert_eq!(
3262            decide(&state, 0, 4, Duration::ZERO),
3263            Step::Done { merged: false },
3264            "a human closing the pull request ends the loop, whatever CI says"
3265        );
3266    }
3267
3268    #[test]
3269    fn the_last_round_gives_up_with_a_reason_naming_what_is_still_failing() {
3270        let red = decide(
3271            &pr(Checks::Red, &["editorconfig", "test (macos)"], 0),
3272            4,
3273            4,
3274            Duration::ZERO,
3275        );
3276        match red {
3277            Step::GiveUp { reason } => {
3278                assert!(reason.contains("editorconfig"), "reason: {reason}");
3279                assert!(reason.contains("test (macos)"), "reason: {reason}");
3280                assert!(reason.contains("4 fix round(s)"), "reason: {reason}");
3281            }
3282            other => panic!("expected a give-up, got {other:?}"),
3283        }
3284
3285        let commented = decide(&pr(Checks::Green, &[], 1), 2, 2, Duration::ZERO);
3286        match commented {
3287            Step::GiveUp { reason } => {
3288                assert!(reason.contains("unresolved"), "reason: {reason}");
3289                assert!(reason.contains("2 fix round(s)"), "reason: {reason}");
3290            }
3291            other => panic!("expected a give-up, got {other:?}"),
3292        }
3293    }
3294
3295    #[test]
3296    fn the_merge_command_squashes_deletes_the_branch_and_sets_its_own_subject() {
3297        let candidate_commit = "magi: candidate A (uncommitted work)";
3298        let subject = merge_subject(candidate_commit, "add retries to the uploader");
3299        let argv = merge_argv(16, &subject);
3300
3301        assert!(argv.contains(&"--squash".to_owned()));
3302        assert!(argv.contains(&"--delete-branch".to_owned()));
3303        assert!(argv.contains(&"--subject".to_owned()));
3304        assert_eq!(
3305            argv.last().map(String::as_str),
3306            Some("add retries to the uploader"),
3307            "the subject must not be the candidate commit message"
3308        );
3309        assert_ne!(subject, candidate_commit);
3310    }
3311
3312    #[test]
3313    fn a_real_pull_request_title_is_used_as_the_squash_subject_verbatim() {
3314        assert_eq!(
3315            merge_subject("feat: a queue, an unattended loop, and a phone UI", "task"),
3316            "feat: a queue, an unattended loop, and a phone UI"
3317        );
3318        assert_eq!(
3319            merge_subject("", "# port the retry logic\n\ndetails"),
3320            "port the retry logic",
3321            "an empty title falls back to the task's first line, heading marks stripped"
3322        );
3323    }
3324
3325    #[test]
3326    fn a_failing_checks_details_url_yields_the_job_to_read_logs_from() {
3327        let url = "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323572";
3328        assert_eq!(job_of(url).as_deref(), Some("100114323572"));
3329        assert_eq!(run_of(url).as_deref(), Some("33587406996"));
3330        assert_eq!(job_of("https://coderabbit.ai/status"), None);
3331        assert_eq!(run_of(""), None);
3332    }
3333
3334    #[test]
3335    fn magis_own_stop_comment_is_never_read_back_as_a_finding() {
3336        let mut out = Vec::new();
3337        push_if_outstanding(
3338            &mut out,
3339            ReviewComment {
3340                author: "yukimemi".to_owned(),
3341                path: None,
3342                line: None,
3343                body: format!("{MARKER}\nmagi stopped landing this pull request: 1 check failing"),
3344            },
3345        );
3346        assert!(out.is_empty());
3347    }
3348
3349    /// A run with no tally, so [`RunState::winner`] is `None` and the panel
3350    /// falls back to the repository - which keeps these tests free of a
3351    /// worktree, a `git` invocation and a network.
3352    fn run_state() -> RunState {
3353        RunState::new(
3354            std::path::PathBuf::from("/repo/magi"),
3355            "main".to_owned(),
3356            "abcdef1234".to_owned(),
3357            "add retries to the uploader".to_owned(),
3358            crate::config::Config::default(),
3359        )
3360    }
3361
3362    fn green_pr() -> PrState {
3363        PrState {
3364            url: "https://github.com/yukimemi/magi/pull/42".to_owned(),
3365            number: 42,
3366            state: PrLifecycle::Open,
3367            checks: Checks::Green,
3368            // The forge sees nothing in the way unless a test says otherwise.
3369            blocking: Blocking::No,
3370            failing: Vec::new(),
3371            review_comments: vec![ReviewComment {
3372                author: "coderabbitai".to_owned(),
3373                path: Some("src/land.rs".to_owned()),
3374                line: Some(212),
3375                body: "this branch never checks the exit code".to_owned(),
3376            }],
3377        }
3378    }
3379
3380    #[test]
3381    fn github_facing_land_text_is_english_whatever_the_language() {
3382        let mut state = run_state();
3383        state.config.graph.language = "ja".to_owned();
3384        let comment = stop_comment(&state.id, "checks are still red");
3385        assert!(comment.is_ascii(), "{comment}");
3386        assert!(comment.starts_with(MARKER));
3387
3388        let p = fix_prompt(&state, &green_pr(), 1, 2, "red", "");
3389        let ja_at = p.find("Write all prose in ja").unwrap();
3390        let rule_at = p.find(crate::prompt::GITHUB_ENGLISH_HEADING).unwrap();
3391        assert!(ja_at < rule_at, "{p}");
3392        assert!(p.contains("stays in Japanese"), "{p}");
3393
3394        state.config.graph.language = "en".to_owned();
3395        let p = fix_prompt(&state, &green_pr(), 1, 2, "red", "");
3396        assert!(p.contains(crate::prompt::GITHUB_ENGLISH_HEADING), "{p}");
3397        assert!(!p.contains("does not apply"), "{p}");
3398    }
3399
3400    const NUMSTAT: &str = "12\t3\tsrc/land.rs\n40\t1\tsrc/web.rs\n-\t-\tassets/logo.png";
3401
3402    fn panel() -> String {
3403        approval_panel(
3404            &run_state(),
3405            &green_pr(),
3406            NUMSTAT,
3407            "diff --git a/src/land.rs b/src/land.rs\n@@ -1,2 +1,2 @@\n-old line\n+new line\n context",
3408            &[
3409                "land: ask before merging".to_owned(),
3410                "land: colour the diff".to_owned(),
3411            ],
3412            "feat: merge approval from the phone",
3413        )
3414    }
3415
3416    #[test]
3417    fn the_approval_panel_carries_the_whole_case_for_the_merge() {
3418        let html = panel();
3419        for needle in [
3420            "42",
3421            "main",
3422            "src/land.rs",
3423            "src/web.rs",
3424            "assets/logo.png",
3425            "feat: merge approval from the phone",
3426            "land: ask before merging",
3427            "land: colour the diff",
3428            "coderabbitai",
3429            "this branch never checks the exit code",
3430            "green",
3431        ] {
3432            assert!(html.contains(needle), "the panel must state `{needle}`");
3433        }
3434    }
3435
3436    /// A candidate whose label is `A` and has won, so [`RunState::winner`]
3437    /// resolves to it.
3438    fn winning_candidate(summary: &str) -> Candidate {
3439        Candidate {
3440            index: 0,
3441            label: 'A',
3442            agent: "opus".to_owned(),
3443            branch: "magi/x/A".to_owned(),
3444            worktree: PathBuf::from("/wt/A"),
3445            summary: summary.to_owned(),
3446            stat: String::new(),
3447            files: 1,
3448            commits: 1,
3449            empty: false,
3450            failed: None,
3451            verified_noop: None,
3452            duration_ms: 0,
3453            folded: false,
3454        }
3455    }
3456
3457    fn uncontested_tally() -> Tally {
3458        Tally {
3459            first_choice: BTreeMap::from([('A', 1)]),
3460            borda: BTreeMap::new(),
3461            winner: 'A',
3462            rankings: 1,
3463            unanimous_initial: true,
3464            deliberated: false,
3465            changed_votes: 0,
3466            unanimous_final: true,
3467            tie_break: None,
3468            judges: 1,
3469            present: 1,
3470            quorum: 1,
3471            met_quorum: true,
3472            uncontested: None,
3473        }
3474    }
3475
3476    fn review_record(reviewer: usize, agent: &str, summary: &str) -> ReviewRecord {
3477        ReviewRecord {
3478            attempts: 0,
3479            reviewer,
3480            agent: agent.to_owned(),
3481            summary: summary.to_owned(),
3482            findings: Vec::new(),
3483            vote: None,
3484            failed: None,
3485            duration_ms: 0,
3486        }
3487    }
3488
3489    fn review_round(round: usize, reviews: Vec<ReviewRecord>) -> ReviewRound {
3490        let answered = reviews.len();
3491        ReviewRound {
3492            round,
3493            head: "abc1234".to_owned(),
3494            verified_head: None,
3495            verified_at: None,
3496            reviews,
3497            e2e: Vec::new(),
3498            verify_retried: false,
3499            e2e_deferred: false,
3500            e2e_defer_reason: None,
3501            fix: None,
3502            blocking: 0,
3503            answered,
3504            expected: answered,
3505            clean: true,
3506            progressed: false,
3507            vote_split: false,
3508            reconsideration: Vec::new(),
3509            verdict: None,
3510        }
3511    }
3512
3513    #[test]
3514    fn the_approval_panel_states_the_task_verbatim_in_either_language() {
3515        let en = panel();
3516        assert!(en.contains("Task"), "{en}");
3517        assert!(en.contains("add retries to the uploader"), "{en}");
3518
3519        let mut state = run_state();
3520        state.config.graph.language = "ja".to_owned();
3521        let ja = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3522        assert!(ja.contains("タスク"), "{ja}");
3523        assert!(
3524            ja.contains("add retries to the uploader"),
3525            "the task itself is not translated: {ja}"
3526        );
3527    }
3528
3529    #[test]
3530    fn the_approval_panel_omits_what_changed_and_review_verdict_with_no_data() {
3531        // `run_state()` has no candidates, no tally and no reviews - exactly
3532        // the shape a run has before anything has judged or reviewed it, and
3533        // the panel must not print an empty box for either.
3534        let html = panel();
3535        assert!(!html.contains("What changed"), "{html}");
3536        assert!(!html.contains("Review verdict"), "{html}");
3537    }
3538
3539    #[test]
3540    fn the_approval_panel_omits_what_changed_when_the_winners_summary_is_empty() {
3541        let mut state = run_state();
3542        state.candidates = vec![winning_candidate("")];
3543        state.tally = Some(uncontested_tally());
3544        let html = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3545        assert!(
3546            !html.contains("What changed"),
3547            "an empty summary must not render an empty box: {html}"
3548        );
3549    }
3550
3551    #[test]
3552    fn the_approval_panel_shows_the_winners_own_account_in_either_language() {
3553        let mut state = run_state();
3554        state.candidates = vec![winning_candidate(
3555            "Added a retry loop around the uploader PUT call.",
3556        )];
3557        state.tally = Some(uncontested_tally());
3558        let en = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3559        assert!(en.contains("What changed"), "{en}");
3560        assert!(
3561            en.contains("Added a retry loop around the uploader PUT call."),
3562            "{en}"
3563        );
3564
3565        state.config.graph.language = "ja".to_owned();
3566        let ja = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3567        assert!(ja.contains("変更内容"), "{ja}");
3568        assert!(
3569            ja.contains("Added a retry loop around the uploader PUT call."),
3570            "{ja}"
3571        );
3572    }
3573
3574    #[test]
3575    fn the_approval_panel_shows_only_the_last_review_rounds_verdict() {
3576        let mut state = run_state();
3577        state.reviews = vec![
3578            review_round(
3579                1,
3580                vec![review_record(1, "alpha", "found a race, sent back")],
3581            ),
3582            review_round(2, vec![review_record(1, "alpha", "race is fixed, clean")]),
3583        ];
3584        let en = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3585        assert!(en.contains("Review verdict"), "{en}");
3586        assert!(en.contains("race is fixed, clean"), "{en}");
3587        assert!(
3588            !en.contains("found a race, sent back"),
3589            "only the round that actually cleared the merge should show: {en}"
3590        );
3591
3592        state.config.graph.language = "ja".to_owned();
3593        let ja = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3594        assert!(ja.contains("レビューの結論"), "{ja}");
3595        assert!(ja.contains("レビュアー"), "{ja}");
3596        assert!(ja.contains("race is fixed, clean"), "{ja}");
3597    }
3598
3599    /// The `incomplete_review = "warn"` policy (see
3600    /// `graph::Runner::review_loop`) can push a `clean` round to
3601    /// `state.reviews` while one seat's own record still has `failed: Some`
3602    /// and an empty `summary` - a seat that never answered, not one that
3603    /// answered with nothing to say.
3604    fn unanswered_review_record(reviewer: usize, agent: &str, reason: &str) -> ReviewRecord {
3605        ReviewRecord {
3606            attempts: 0,
3607            reviewer,
3608            agent: agent.to_owned(),
3609            summary: String::new(),
3610            findings: Vec::new(),
3611            vote: None,
3612            failed: Some(reason.to_owned()),
3613            duration_ms: 0,
3614        }
3615    }
3616
3617    #[test]
3618    fn the_approval_panel_never_shows_an_unanswered_seat_as_a_blank_verdict() {
3619        let mut state = run_state();
3620        state.reviews = vec![review_round(
3621            1,
3622            vec![
3623                review_record(1, "alpha", "clean, nothing to add"),
3624                unanswered_review_record(2, "beta", "timed out"),
3625            ],
3626        )];
3627        let en = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3628        assert!(en.contains("clean, nothing to add"), "{en}");
3629        assert!(
3630            en.contains("produced no answer: timed out"),
3631            "a seat that never answered must say so, not render a blank box: {en}"
3632        );
3633        assert!(
3634            !en.contains("<div style=\"white-space:pre-wrap;font-size:13px\"></div>"),
3635            "no reviewer box may be left empty: {en}"
3636        );
3637
3638        state.config.graph.language = "ja".to_owned();
3639        let ja = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3640        assert!(ja.contains("回答なし: timed out"), "{ja}");
3641    }
3642
3643    #[test]
3644    fn the_approval_panel_contains_nothing_the_frames_policy_would_block() {
3645        let html = panel();
3646        assert!(!html.contains("<script"), "no script survives the csp");
3647        assert!(!html.contains("<form"), "form-action is 'none'");
3648        let pr = green_pr();
3649        assert_eq!(
3650            html.matches("http").count(),
3651            html.matches(pr.url.as_str()).count(),
3652            "the only http url in the panel is the pull request's own link"
3653        );
3654    }
3655
3656    #[test]
3657    fn added_and_removed_diff_lines_are_distinguishable_without_colour() {
3658        let html = panel();
3659        assert!(
3660            html.contains(">+</span>"),
3661            "an added line carries a `+` in the gutter, not only a background"
3662        );
3663        assert!(
3664            html.contains(">-</span>"),
3665            "a removed line carries a `-` in the gutter, not only a background"
3666        );
3667        assert!(
3668            html.contains(">new line</span>"),
3669            "the marker is moved to the gutter, so the body is printed once without it"
3670        );
3671    }
3672
3673    #[test]
3674    fn a_diff_past_the_threshold_is_cut_with_an_honest_count() {
3675        let total = DIFF_MAX_LINES + 100;
3676        let diff: String = (0..total).map(|i| format!("+line {i}\n")).collect();
3677        let html = approval_panel(
3678            &run_state(),
3679            &green_pr(),
3680            NUMSTAT,
3681            &diff,
3682            &[],
3683            "feat: something long",
3684        );
3685        assert!(
3686            html.contains(&format!("100 of {total} diff lines omitted")),
3687            "the note must say exactly how much was cut"
3688        );
3689        assert!(html.contains(&format!("line {}", DIFF_MAX_LINES - 1)));
3690        assert!(
3691            !html.contains(&format!("line {DIFF_MAX_LINES}")),
3692            "nothing past the threshold is rendered"
3693        );
3694        assert!(
3695            html.contains("/repo/magi"),
3696            "the note says where the rest is"
3697        );
3698    }
3699
3700    #[test]
3701    fn a_path_with_html_metacharacters_is_escaped_rather_than_rendered() {
3702        let html = approval_panel(
3703            &run_state(),
3704            &green_pr(),
3705            "1\t2\tsrc/<b>&\"x\"'.rs",
3706            "",
3707            &[],
3708            "subject",
3709        );
3710        assert!(html.contains("src/&lt;b&gt;&amp;&quot;x&quot;&#39;.rs"));
3711        assert!(
3712            !html.contains("<b>"),
3713            "an agent-influenced path must never become markup"
3714        );
3715    }
3716
3717    #[tokio::test]
3718    async fn the_merge_lock_serialises_one_repository_but_never_a_different_one() {
3719        let a = std::path::PathBuf::from("/repo/a");
3720        let b = std::path::PathBuf::from("/repo/b");
3721
3722        let held = repo_merge_lock(&a).lock_owned().await;
3723
3724        // A second, concurrent land run against the *same* repository must
3725        // wait - `try_lock` fails while `held` is alive.
3726        assert!(
3727            repo_merge_lock(&a).try_lock().is_err(),
3728            "a second merge into the same repository must not proceed concurrently"
3729        );
3730
3731        // A run against a *different* repository must not be blocked by it -
3732        // this is what keeps a slow rebase or `gh pr merge` in one
3733        // repository from also stalling a land-approval resume in another.
3734        assert!(
3735            repo_merge_lock(&b).try_lock().is_ok(),
3736            "a different repository's merge lock must be independent"
3737        );
3738
3739        drop(held);
3740        assert!(
3741            repo_merge_lock(&a).try_lock().is_ok(),
3742            "the lock is released once the holder is done"
3743        );
3744    }
3745
3746    #[test]
3747    fn only_the_merge_choice_merges_and_silence_holds() {
3748        let table = [
3749            (None, Approval::Hold),
3750            (Some("merge"), Approval::Merge),
3751            (Some(" merge\n"), Approval::Merge),
3752            (Some("hold"), Approval::Hold),
3753            (Some(""), Approval::Hold),
3754            (Some("yes"), Approval::Hold),
3755        ];
3756        for (answer, want) in table {
3757            assert_eq!(
3758                approval(answer),
3759                want,
3760                "answer {answer:?} must resolve to {want:?}"
3761            );
3762        }
3763    }
3764
3765    #[tokio::test]
3766    async fn a_first_visit_to_the_merge_gate_files_a_question_and_returns_pending_at_once() {
3767        crate::run::set_home(std::env::temp_dir().join("magi-land-approval-test-home"));
3768        let mut state = run_state();
3769        state.config.graph.land_approval = true;
3770        let pr = green_pr();
3771
3772        let gate = approval_gate(&mut state, &pr, "feat: x").await.unwrap();
3773        assert_eq!(gate, ApprovalGate::Pending, "nobody has answered yet");
3774        assert!(
3775            !state.parked,
3776            "approval_gate itself never sets `parked`; only its caller does"
3777        );
3778
3779        let store = ask::Questions::open();
3780        let filed: Vec<_> = store
3781            .list()
3782            .into_iter()
3783            .filter(|q| q.run == state.id)
3784            .collect();
3785        assert_eq!(filed.len(), 1, "exactly one question is filed");
3786        assert_eq!(filed[0].node, APPROVAL_NODE);
3787        assert_eq!(filed[0].choices, vec![APPROVE.to_owned(), HOLD.to_owned()]);
3788        assert!(filed[0].status.open());
3789
3790        // A second visit - standing in for a resumed run whose slot the
3791        // daemon handed to something else while nobody had answered - must
3792        // find the same question rather than filing a second one.
3793        let again = approval_gate(&mut state, &pr, "feat: x").await.unwrap();
3794        assert_eq!(again, ApprovalGate::Pending);
3795        let still_one = store
3796            .list()
3797            .into_iter()
3798            .filter(|q| q.run == state.id)
3799            .count();
3800        assert_eq!(
3801            still_one, 1,
3802            "asking twice must not double-file the question"
3803        );
3804    }
3805
3806    #[tokio::test]
3807    async fn approving_the_existing_question_is_read_back_as_approved() {
3808        crate::run::set_home(std::env::temp_dir().join("magi-land-approval-test-home"));
3809        let mut state = run_state();
3810        state.config.graph.land_approval = true;
3811        let pr = green_pr();
3812        assert_eq!(
3813            approval_gate(&mut state, &pr, "feat: x").await.unwrap(),
3814            ApprovalGate::Pending
3815        );
3816
3817        let store = ask::Questions::open();
3818        let mut q = store
3819            .list()
3820            .into_iter()
3821            .find(|q| q.run == state.id)
3822            .expect("filed above");
3823        q.answer(ask::Answer::Choice(APPROVE.to_owned())).unwrap();
3824        store.put(&mut q).unwrap();
3825
3826        assert_eq!(
3827            approval_gate(&mut state, &pr, "feat: x").await.unwrap(),
3828            ApprovalGate::Approved
3829        );
3830    }
3831
3832    #[tokio::test]
3833    async fn holding_or_abandoning_the_existing_question_is_read_back_as_held() {
3834        crate::run::set_home(std::env::temp_dir().join("magi-land-approval-test-home"));
3835        let store = ask::Questions::open();
3836
3837        let mut held_state = run_state();
3838        held_state.config.graph.land_approval = true;
3839        let pr = green_pr();
3840        approval_gate(&mut held_state, &pr, "feat: x")
3841            .await
3842            .unwrap();
3843        let mut q = store
3844            .list()
3845            .into_iter()
3846            .find(|q| q.run == held_state.id)
3847            .expect("filed above");
3848        q.answer(ask::Answer::Choice(HOLD.to_owned())).unwrap();
3849        store.put(&mut q).unwrap();
3850        assert_eq!(
3851            approval_gate(&mut held_state, &pr, "feat: x")
3852                .await
3853                .unwrap(),
3854            ApprovalGate::Held
3855        );
3856
3857        let mut abandoned_state = run_state();
3858        abandoned_state.config.graph.land_approval = true;
3859        approval_gate(&mut abandoned_state, &pr, "feat: x")
3860            .await
3861            .unwrap();
3862        let mut q = store
3863            .list()
3864            .into_iter()
3865            .find(|q| q.run == abandoned_state.id)
3866            .expect("filed above");
3867        q.abandon("no answer within the timeout");
3868        store.put(&mut q).unwrap();
3869        assert_eq!(
3870            approval_gate(&mut abandoned_state, &pr, "feat: x")
3871                .await
3872                .unwrap(),
3873            ApprovalGate::Held,
3874            "silence must never merge"
3875        );
3876    }
3877
3878    #[test]
3879    fn the_diffstat_table_is_ordered_by_churn_with_binaries_last() {
3880        let rows = parse_numstat(NUMSTAT);
3881        assert_eq!(
3882            rows.iter().map(|r| r.path.as_str()).collect::<Vec<_>>(),
3883            ["src/web.rs", "src/land.rs", "assets/logo.png"]
3884        );
3885        assert_eq!(rows[2].added, None, "a binary file has no line counts");
3886    }
3887    #[test]
3888    fn the_approval_speaks_the_language_the_repository_is_configured_for() {
3889        // Reported from a real run: the merge question arrived in English on a
3890        // repository with `language = "ja"`. magi's own strings have to follow
3891        // that setting too - "it is a literal in Rust" is not an answer.
3892        let mut state = run_state();
3893        state.config.graph.language = "ja".to_owned();
3894        let pr = green_pr();
3895        let commits = ["c1".to_owned()];
3896
3897        let ja = approval_panel(&state, &pr, "3\t1\tsrc/a.rs", "+ x", &commits, "feat: x");
3898        assert!(ja.contains("lang=\"ja\""), "the document must declare it");
3899        assert!(ja.contains("squash されるコミット"), "{ja}");
3900        assert!(ja.contains("レビューコメント"), "{ja}");
3901        assert!(ja.contains("差分"), "{ja}");
3902        assert!(
3903            !ja.contains("Commits being squashed"),
3904            "no English left over"
3905        );
3906
3907        let w = words("ja");
3908        assert!(w.approval_summary(17, "feat: x").contains("マージ"));
3909        assert!(
3910            w.approval_detail("http://x/1", "main", "feat: x")
3911                .contains("パネル")
3912        );
3913
3914        // The evidence itself is language-neutral and must survive either way.
3915        assert!(ja.contains("src/a.rs"), "the diffstat is not prose");
3916        assert!(ja.contains("feat: x"), "nor is the merge subject");
3917
3918        // English stays the default, and a language magi cannot check falls
3919        // back to it rather than shipping a guess.
3920        state.config.graph.language = "en".to_owned();
3921        let en = approval_panel(&state, &pr, "3\t1\tsrc/a.rs", "+ x", &commits, "feat: x");
3922        assert!(en.contains("Commits being squashed"), "{en}");
3923        assert_eq!(words("Klingon").html_lang, "en");
3924    }
3925
3926    /// A `gh pr list` result naming exactly one pull request whose base and
3927    /// merge time both fit the run is exactly the case
3928    /// [`find_external_merge`] exists to act on.
3929    #[test]
3930    fn pick_merged_pr_picks_the_unique_match() {
3931        let json = r#"[
3932            {"url": "https://github.com/o/r/pull/42", "number": 42,
3933             "mergedAt": "2026-09-20T10:00:00Z", "baseRefName": "main"}
3934        ]"#;
3935        let created_at: Timestamp = "2026-09-19T00:00:00Z".parse().unwrap();
3936        let found = pick_merged_pr(json, "main", created_at)
3937            .expect("valid json")
3938            .expect("one unambiguous match");
3939        assert_eq!(found.url, "https://github.com/o/r/pull/42");
3940        assert_eq!(found.number, 42);
3941    }
3942
3943    /// Two candidates surviving the filter is exactly as uninformative as
3944    /// zero — a branch name can be reused across runs — so neither is
3945    /// preferred over the other and nothing is recorded automatically.
3946    #[test]
3947    fn pick_merged_pr_refuses_when_more_than_one_candidate_survives() {
3948        let json = r#"[
3949            {"url": "https://github.com/o/r/pull/42", "number": 42,
3950             "mergedAt": "2026-09-20T10:00:00Z", "baseRefName": "main"},
3951            {"url": "https://github.com/o/r/pull/43", "number": 43,
3952             "mergedAt": "2026-09-21T10:00:00Z", "baseRefName": "main"}
3953        ]"#;
3954        let created_at: Timestamp = "2026-09-19T00:00:00Z".parse().unwrap();
3955        assert_eq!(pick_merged_pr(json, "main", created_at).unwrap(), None);
3956    }
3957
3958    /// A pull request that targets a different base branch cannot be this
3959    /// run's, whatever its head branch is named — a reused branch name from
3960    /// an unrelated task must not be recorded as this run's merge.
3961    #[test]
3962    fn pick_merged_pr_ignores_a_different_base_branch() {
3963        let json = r#"[
3964            {"url": "https://github.com/o/r/pull/42", "number": 42,
3965             "mergedAt": "2026-09-20T10:00:00Z", "baseRefName": "release"}
3966        ]"#;
3967        let created_at: Timestamp = "2026-09-19T00:00:00Z".parse().unwrap();
3968        assert_eq!(pick_merged_pr(json, "main", created_at).unwrap(), None);
3969    }
3970
3971    /// A pull request merged before this run was even created cannot be this
3972    /// run's winner, no matter how its head branch is spelled.
3973    #[test]
3974    fn pick_merged_pr_ignores_a_merge_that_predates_the_run() {
3975        let json = r#"[
3976            {"url": "https://github.com/o/r/pull/42", "number": 42,
3977             "mergedAt": "2026-09-18T10:00:00Z", "baseRefName": "main"}
3978        ]"#;
3979        let created_at: Timestamp = "2026-09-19T00:00:00Z".parse().unwrap();
3980        assert_eq!(pick_merged_pr(json, "main", created_at).unwrap(), None);
3981    }
3982
3983    #[test]
3984    fn slug_of_pr_url_reads_host_owner_and_repo() {
3985        assert_eq!(
3986            slug_of_pr_url("https://github.com/yukimemi/shun/pull/272").as_deref(),
3987            Some("github.com/yukimemi/shun")
3988        );
3989    }
3990
3991    #[test]
3992    fn slug_of_pr_url_refuses_a_url_with_no_pull_segment() {
3993        assert_eq!(slug_of_pr_url("https://github.com/yukimemi/shun"), None);
3994        assert_eq!(slug_of_pr_url("not a url at all"), None);
3995        assert_eq!(slug_of_pr_url("https://github.com"), None);
3996    }
3997
3998    #[test]
3999    fn slug_of_repo_url_reads_host_owner_and_repo() {
4000        assert_eq!(
4001            slug_of_repo_url("https://github.com/yukimemi/magi").as_deref(),
4002            Some("github.com/yukimemi/magi")
4003        );
4004        assert_eq!(slug_of_repo_url("https://github.com"), None);
4005    }
4006
4007    #[test]
4008    fn ensure_same_repo_accepts_a_matching_slug_regardless_of_case() {
4009        ensure_same_repo("github.com/yukimemi/magi", "GitHub.Com/YukiMemi/Magi")
4010            .expect("same repo, different case");
4011    }
4012
4013    /// The shun/8c75 incident: an id-less `--merged` picked this repository's
4014    /// own in-progress run and rewrote its status from a pull request in a
4015    /// completely different repository. This is the guard that must catch
4016    /// that even when an explicit (but wrong) id is given.
4017    #[test]
4018    fn ensure_same_repo_refuses_a_different_repo() {
4019        let err =
4020            ensure_same_repo("github.com/yukimemi/magi", "github.com/yukimemi/shun").unwrap_err();
4021        let msg = format!("{err:#}");
4022        assert!(msg.contains("github.com/yukimemi/magi"), "{msg}");
4023        assert!(msg.contains("github.com/yukimemi/shun"), "{msg}");
4024    }
4025
4026    /// Same owner/repo on two different forge hosts (a GitHub Enterprise
4027    /// instance mirroring a `github.com` repository's name, say) must not be
4028    /// treated as the same repository just because the trailing path
4029    /// matches.
4030    #[test]
4031    fn ensure_same_repo_refuses_the_same_slug_on_a_different_host() {
4032        let err = ensure_same_repo(
4033            "github.com/yukimemi/magi",
4034            "github.example.com/yukimemi/magi",
4035        )
4036        .unwrap_err();
4037        let msg = format!("{err:#}");
4038        assert!(msg.contains("github.com/yukimemi/magi"), "{msg}");
4039        assert!(msg.contains("github.example.com/yukimemi/magi"), "{msg}");
4040    }
4041
4042    /// No winner decided yet means there is no branch to ask GitHub about at
4043    /// all — `find_external_merge` must return `None` without ever spawning
4044    /// `gh`, which this proves by never providing a real repository to spawn
4045    /// it in.
4046    #[tokio::test]
4047    async fn find_external_merge_returns_none_without_a_winner() {
4048        let state = RunState::new(
4049            PathBuf::from("/no/such/repo"),
4050            "main".to_owned(),
4051            "0000000000000000000000000000000000000000".to_owned(),
4052            "irrelevant".to_owned(),
4053            crate::config::Config::default(),
4054        );
4055        assert_eq!(find_external_merge(&state).await.unwrap(), None);
4056    }
4057}