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