1use std::collections::BTreeSet;
42use std::fmt::Write as _;
43use std::path::Path;
44use std::time::Duration;
45
46use anyhow::{Context as _, Result, bail};
47use serde::Deserialize;
48
49use crate::agent::{self, Invocation, SeatState};
50use crate::ask;
51use crate::config::{AgentSpec, MergeMode};
52use crate::git;
53use crate::proc::Quiet as _;
54use crate::prompt;
55use crate::run::{MergeOutcome, RunState, RunStatus, tail};
56
57pub const POLL: Duration = Duration::from_secs(30);
63
64pub const WAIT_CEILING: Duration = Duration::from_secs(45 * 60);
70
71pub const CHECKS_GRACE: Duration = Duration::from_secs(3 * 60);
83
84const LOG_TAIL: usize = 4_000;
87
88const MAX_LOGS: usize = 3;
91
92pub const MARKER: &str = "<!-- magi:land -->";
98
99const NOT_A_REVIEW: [&str; 3] = [
108 "skip review by coderabbit.ai",
109 "summarize by coderabbit.ai",
110 "<!-- tips_start -->",
111];
112
113#[derive(Debug, Clone, Copy, PartialEq, Eq)]
115pub enum PrLifecycle {
116 Open,
118 Merged,
120 Closed,
122}
123
124#[derive(Debug, Clone, Copy, PartialEq, Eq)]
126pub enum Checks {
127 Pending,
129 Green,
132 Red,
134 Unknown,
136}
137
138impl PrLifecycle {
139 pub fn as_str(self) -> &'static str {
141 match self {
142 Self::Open => "open",
143 Self::Merged => "merged",
144 Self::Closed => "closed",
145 }
146 }
147}
148
149impl Checks {
150 pub fn as_str(self) -> &'static str {
152 match self {
153 Self::Pending => "pending",
154 Self::Green => "green",
155 Self::Red => "red",
156 Self::Unknown => "unknown",
157 }
158 }
159}
160
161#[derive(Debug, Clone, PartialEq, Eq)]
163pub struct ReviewComment {
164 pub author: String,
166 pub path: Option<String>,
168 pub line: Option<u64>,
170 pub body: String,
172}
173
174#[derive(Debug, Clone, PartialEq, Eq)]
176pub struct PrState {
177 pub url: String,
179 pub number: u64,
181 pub state: PrLifecycle,
183 pub checks: Checks,
185 pub failing: Vec<String>,
187 pub review_comments: Vec<ReviewComment>,
189 pub blocking: Blocking,
191}
192
193#[derive(Debug, Clone, Copy, PartialEq, Eq)]
206pub enum Blocking {
207 No,
209 Yes,
211 Conflict,
213 Unsaid,
217}
218
219impl Blocking {
220 fn of(raw: &str) -> Self {
222 match raw.to_ascii_uppercase().as_str() {
223 "CLEAN" | "UNSTABLE" | "HAS_HOOKS" => Self::No,
226 "DIRTY" => Self::Conflict,
227 "" | "UNKNOWN" => Self::Unsaid,
228 _ => Self::Yes,
230 }
231 }
232
233 #[must_use]
235 pub fn stops_a_merge(self) -> bool {
236 !matches!(self, Self::No)
237 }
238}
239
240#[derive(Debug, Clone, PartialEq, Eq)]
242pub enum Step {
243 Wait,
245 Rebase,
253 Fix {
255 reason: String,
257 },
258 Merge,
260 Done {
262 merged: bool,
264 },
265 GiveUp {
267 reason: String,
269 },
270}
271
272fn merged_after_all(
294 argv: &[String],
295 stderr: &str,
296 after: Option<PrLifecycle>,
297) -> Option<MergeOutcome> {
298 if after? != PrLifecycle::Merged {
299 return None;
300 }
301 Some(MergeOutcome {
302 mode: MergeMode::Pr,
303 ok: true,
304 detail: format!(
305 "gh {} (the command reported `{}`, but the pull request is merged)",
306 argv.join(" "),
307 stderr.trim()
308 ),
309 })
310}
311
312pub fn decide(pr: &PrState, round: usize, budget: usize, waited: Duration) -> Step {
332 match pr.state {
333 PrLifecycle::Merged => return Step::Done { merged: true },
334 PrLifecycle::Closed => return Step::Done { merged: false },
335 PrLifecycle::Open => {}
336 }
337
338 if pr.blocking == Blocking::Conflict {
341 return Step::Rebase;
342 }
343
344 let spent = round >= budget;
345 match pr.checks {
346 Checks::Pending => Step::Wait,
347 Checks::Unknown if waited < CHECKS_GRACE => Step::Wait,
348 Checks::Unknown => Step::GiveUp {
349 reason: format!(
350 "no check status is readable on the pull request after {} minute(s); \
351 refusing to merge on a guess",
352 CHECKS_GRACE.as_secs() / 60
353 ),
354 },
355 Checks::Red if !pr.blocking.stops_a_merge() && pr.review_comments.is_empty() => Step::Merge,
362 Checks::Red => {
363 let what = format!(
364 "{} check(s) failing: {}",
365 pr.failing.len(),
366 pr.failing.join(", ")
367 );
368 if spent {
369 Step::GiveUp {
370 reason: format!("{what} — still red after {budget} fix round(s)"),
371 }
372 } else {
373 Step::Fix { reason: what }
374 }
375 }
376 Checks::Green if pr.review_comments.is_empty() => Step::Merge,
377 Checks::Green => {
378 let what = format!(
379 "checks are green but {} review comment(s) are unresolved: {}",
380 pr.review_comments.len(),
381 authors(&pr.review_comments)
382 );
383 if spent {
384 Step::GiveUp {
385 reason: format!("{what} — still unresolved after {budget} fix round(s)"),
386 }
387 } else {
388 Step::Fix { reason: what }
389 }
390 }
391 }
392}
393
394fn authors(comments: &[ReviewComment]) -> String {
396 let mut seen: Vec<&str> = Vec::new();
397 for c in comments {
398 if !seen.contains(&c.author.as_str()) {
399 seen.push(&c.author);
400 }
401 }
402 seen.join(", ")
403}
404
405pub fn merge_argv(number: u64, subject: &str) -> Vec<String> {
409 vec![
410 "pr".to_owned(),
411 "merge".to_owned(),
412 number.to_string(),
413 "--squash".to_owned(),
414 "--delete-branch".to_owned(),
415 "--subject".to_owned(),
416 subject.to_owned(),
417 ]
418}
419
420pub fn merge_subject(pr_title: &str, instruction: &str) -> String {
427 let title = pr_title.trim();
428 if !title.is_empty() && !title.starts_with("magi: candidate") {
429 return title.to_owned();
430 }
431 let first = instruction
432 .lines()
433 .map(str::trim)
434 .find(|l| !l.is_empty())
435 .unwrap_or("magi: land the winning candidate");
436 first.trim_start_matches(['#', ' ']).to_owned()
437}
438
439pub const APPROVE: &str = "merge";
441
442pub const HOLD: &str = "hold";
444
445pub const APPROVAL_NODE: &str = "land-approval";
451
452pub const DIFF_MAX_LINES: usize = 400;
460
461#[derive(Debug, Clone, Copy, PartialEq, Eq)]
463pub enum Approval {
464 Merge,
466 Hold,
468}
469
470pub fn approval(answer: Option<&str>) -> Approval {
478 match answer {
479 Some(a) if a.trim().eq_ignore_ascii_case(APPROVE) => Approval::Merge,
480 _ => Approval::Hold,
481 }
482}
483
484fn esc(s: &str) -> String {
494 let mut out = String::with_capacity(s.len());
495 for c in s.chars() {
496 match c {
497 '&' => out.push_str("&"),
498 '<' => out.push_str("<"),
499 '>' => out.push_str(">"),
500 '"' => out.push_str("""),
501 '\'' => out.push_str("'"),
502 _ => out.push(c),
503 }
504 }
505 out
506}
507
508#[derive(Debug, Clone, PartialEq, Eq)]
510struct StatRow {
511 path: String,
512 added: Option<u64>,
514 removed: Option<u64>,
515}
516
517impl StatRow {
518 fn churn(&self) -> u64 {
521 self.added.unwrap_or(0) + self.removed.unwrap_or(0)
522 }
523}
524
525fn parse_numstat(numstat: &str) -> Vec<StatRow> {
531 let mut rows: Vec<StatRow> = numstat
532 .lines()
533 .filter_map(|line| {
534 let mut parts = line.splitn(3, '\t');
535 let added = parts.next()?.trim();
536 let removed = parts.next()?.trim();
537 let path = parts.next()?.trim();
538 if path.is_empty() {
539 return None;
540 }
541 Some(StatRow {
542 path: path.to_owned(),
543 added: added.parse().ok(),
544 removed: removed.parse().ok(),
545 })
546 })
547 .collect();
548 rows.sort_by(|a, b| b.churn().cmp(&a.churn()).then_with(|| a.path.cmp(&b.path)));
551 rows
552}
553
554fn diff_row(line: &str) -> (&'static str, &'static str, &str) {
563 if line.starts_with("+++") || line.starts_with("---") {
564 (" ", "color:#57606a;font-weight:600", line)
565 } else if let Some(body) = line.strip_prefix('+') {
566 ("+", "background:#e6ffec;color:#0a3622", body)
567 } else if let Some(body) = line.strip_prefix('-') {
568 ("-", "background:#ffebe9;color:#5c1a17", body)
569 } else if line.starts_with("@@") {
570 ("~", "background:#eef2ff;color:#3730a3", line)
571 } else if let Some(body) = line.strip_prefix(' ') {
572 (" ", "", body)
573 } else {
574 (" ", "color:#57606a;font-weight:600", line)
575 }
576}
577
578struct Words {
587 html_lang: &'static str,
588 checks: &'static str,
589 nothing_failing: &'static str,
590 files_changed: &'static str,
591 commits: &'static str,
592 no_commits: &'static str,
593 comments: &'static str,
594 no_comments: &'static str,
595 diff: &'static str,
596 truncated: &'static str,
597 lands_as: &'static str,
598}
599
600const EN: Words = Words {
601 html_lang: "en",
602 checks: "Checks",
603 nothing_failing: "Nothing failing.",
604 files_changed: "file(s) changed",
605 commits: "Commits being squashed",
606 no_commits: "No commit subjects could be read from the branch.",
607 comments: "Review comments",
608 no_comments: "Nothing outstanding at this observation.",
609 diff: "Diff",
610 truncated: "Truncated",
611 lands_as: "They land as one commit titled",
612};
613
614const JA: Words = Words {
615 html_lang: "ja",
616 checks: "チェック",
617 nothing_failing: "失敗しているものはありません。",
618 files_changed: "ファイル変更",
619 commits: "squash されるコミット",
620 no_commits: "ブランチからコミット件名を読めませんでした。",
621 comments: "レビューコメント",
622 no_comments: "この時点で未対応のものはありません。",
623 diff: "差分",
624 truncated: "省略",
625 lands_as: "これらは次の件名の1コミットとして入ります:",
626};
627
628impl Words {
629 fn lands_as_tail(&self) -> &'static str {
633 if self.html_lang == "ja" {
634 "。この件名も承認の対象です。"
635 } else {
636 ", which you are approving too."
637 }
638 }
639
640 fn approval_summary(&self, number: u64, subject: &str) -> String {
642 if self.html_lang == "ja" {
643 format!("プルリクエスト #{number} をマージ: {subject}")
644 } else {
645 format!("merge pull request #{number}: {subject}")
646 }
647 }
648
649 fn approval_detail(&self, url: &str, base: &str, subject: &str) -> String {
651 if self.html_lang == "ja" {
652 format!(
653 "{url} はチェックが緑で、`{base}` へ `{subject}` として squash \
654 できる状態です。差分の要約・パッチ・squash されるコミットは\
655 下のパネルにあります。"
656 )
657 } else {
658 format!(
659 "{url} is green and ready to squash into `{base}` as `{subject}`. \
660 The panel holds the diffstat, the patch and the commits being squashed."
661 )
662 }
663 }
664
665 fn truncated_note(
667 &self,
668 omitted: usize,
669 total: usize,
670 shown: usize,
671 where_: &str,
672 base: &str,
673 head: &str,
674 ) -> String {
675 if self.html_lang == "ja" {
676 format!(
677 "先頭 {shown} 行のあと、差分 {total} 行のうち {omitted} 行を省略しました。\
678 全体は <code>{where_}</code>(<code>git diff {base}...{head}</code>)と\
679 プルリクエストにあります。"
680 )
681 } else {
682 format!(
683 "{omitted} of {total} diff lines omitted after the first {shown}. \
684 The whole patch is in <code>{where_}</code> \
685 (<code>git diff {base}...{head}</code>) and on the pull request."
686 )
687 }
688 }
689}
690
691fn words(language: &str) -> &'static Words {
694 let l = language.trim();
695 if l.eq_ignore_ascii_case("ja")
696 || l.eq_ignore_ascii_case("jp")
697 || l.eq_ignore_ascii_case("japanese")
698 || l.eq_ignore_ascii_case("日本語")
699 {
700 &JA
701 } else {
702 &EN
703 }
704}
705
706pub fn approval_panel(
718 state: &RunState,
719 pr: &PrState,
720 diffstat: &str,
721 diff: &str,
722 commits: &[String],
723 subject: &str,
724) -> String {
725 let rows = parse_numstat(diffstat);
726 let w = words(&state.config.graph.language);
727 let mut h = String::with_capacity(4_096 + diff.len().min(200_000));
728
729 let _ = writeln!(
730 h,
731 "<!doctype html>\n<html lang=\"{}\">\n<head>\n<meta charset=\"utf-8\">\n\
732 <meta name=\"viewport\" content=\"width=device-width, initial-scale=1\">",
733 w.html_lang
734 );
735 let _ = writeln!(
736 h,
737 "<title>merge #{} — {}</title>\n</head>",
738 pr.number,
739 esc(subject)
740 );
741 h.push_str(
742 "<body style=\"margin:0;padding:12px;font:15px/1.5 -apple-system,\
743 'Segoe UI',system-ui,sans-serif;color:#1f2328;background:#fff;\
744 word-break:break-word\">\n",
745 );
746
747 let _ = writeln!(
749 h,
750 "<h1 style=\"margin:0 0 4px;font-size:19px\">Merge #{} into \
751 <code style=\"background:#f6f8fa;padding:1px 4px;border-radius:4px\">{}</code></h1>\n\
752 <p style=\"margin:0 0 4px;font-size:17px;font-weight:600\">{}</p>\n\
753 <p style=\"margin:0 0 12px;font-size:13px;color:#57606a\">squash merge · run {} · \
754 <a href=\"{}\" style=\"color:#0969da\">{}</a></p>",
755 pr.number,
756 esc(&state.base_branch),
757 esc(subject),
758 esc(&state.id),
759 esc(&pr.url),
760 esc(&pr.url),
761 );
762
763 let _ = writeln!(
764 h,
765 "<h2 style=\"margin:16px 0 6px;font-size:15px\">{}: {}</h2>",
766 w.checks,
767 esc(pr.checks.as_str())
768 );
769 if pr.failing.is_empty() {
770 let _ = writeln!(
771 h,
772 "<p style=\"margin:0;font-size:13px;color:#57606a\">{}</p>",
773 w.nothing_failing
774 );
775 } else {
776 h.push_str("<ul style=\"margin:0;padding-left:20px;font-size:13px\">\n");
777 for f in &pr.failing {
778 let _ = writeln!(h, "<li>{}</li>", esc(f));
779 }
780 h.push_str("</ul>\n");
781 }
782
783 let _ = writeln!(
786 h,
787 "<h2 style=\"margin:16px 0 6px;font-size:15px\">{} {}</h2>",
788 rows.len(),
789 w.files_changed
790 );
791 h.push_str(
792 "<table style=\"width:100%;border-collapse:collapse;font-size:13px\">\n\
793 <thead><tr>\
794 <th style=\"text-align:left;border-bottom:1px solid #d0d7de;padding:4px 2px\">file</th>\
795 <th style=\"text-align:right;border-bottom:1px solid #d0d7de;padding:4px 2px\">added</th>\
796 <th style=\"text-align:right;border-bottom:1px solid #d0d7de;padding:4px 2px\">removed\
797 </th></tr></thead>\n<tbody>\n",
798 );
799 let mut total_added = 0u64;
800 let mut total_removed = 0u64;
801 for r in &rows {
802 total_added += r.added.unwrap_or(0);
803 total_removed += r.removed.unwrap_or(0);
804 let cell = |n: Option<u64>| match n {
805 Some(n) => n.to_string(),
806 None => "bin".to_owned(),
807 };
808 let _ = writeln!(
809 h,
810 "<tr>\
811 <td style=\"padding:4px 2px;border-bottom:1px solid #eaeef2;\
812 font-family:ui-monospace,monospace\">{}</td>\
813 <td style=\"padding:4px 2px;border-bottom:1px solid #eaeef2;text-align:right;\
814 color:#0a3622\">{}</td>\
815 <td style=\"padding:4px 2px;border-bottom:1px solid #eaeef2;text-align:right;\
816 color:#5c1a17\">{}</td></tr>",
817 esc(&r.path),
818 cell(r.added),
819 cell(r.removed),
820 );
821 }
822 let _ = writeln!(
823 h,
824 "</tbody>\n<tfoot><tr style=\"font-weight:600\">\
825 <td style=\"padding:4px 2px\">total</td>\
826 <td style=\"padding:4px 2px;text-align:right\">{total_added}</td>\
827 <td style=\"padding:4px 2px;text-align:right\">{total_removed}</td>\
828 </tr></tfoot>\n</table>"
829 );
830
831 let _ = writeln!(
833 h,
834 "<h2 style=\"margin:16px 0 6px;font-size:15px\">{}</h2>",
835 w.commits
836 );
837 if commits.is_empty() {
838 h.push_str(&format!(
839 "<p style=\"margin:0;font-size:13px;color:#57606a\">{}</p>\n",
840 w.no_commits
841 ));
842 } else {
843 h.push_str("<ol style=\"margin:0;padding-left:20px;font-size:13px\">\n");
844 for c in commits {
845 let _ = writeln!(h, "<li>{}</li>", esc(c));
846 }
847 h.push_str("</ol>\n");
848 }
849 let _ = writeln!(
850 h,
851 "<p style=\"margin:8px 0 0;font-size:13px\">{} <strong>{}</strong>{}</p>",
852 w.lands_as,
853 esc(subject),
854 w.lands_as_tail()
855 );
856
857 let _ = writeln!(
859 h,
860 "<h2 style=\"margin:16px 0 6px;font-size:15px\">{}</h2>",
861 w.comments
862 );
863 if pr.review_comments.is_empty() {
864 h.push_str(&format!(
865 "<p style=\"margin:0;font-size:13px;color:#57606a\">{}</p>\n",
866 w.no_comments
867 ));
868 } else {
869 for c in &pr.review_comments {
870 let anchor = match (&c.path, c.line) {
871 (Some(p), Some(l)) => format!("{p}:{l}"),
872 (Some(p), None) => p.clone(),
873 _ => "pull request thread".to_owned(),
874 };
875 let _ = writeln!(
876 h,
877 "<div style=\"margin:0 0 8px;padding:8px;background:#f6f8fa;border-radius:6px\">\
878 <div style=\"font-size:12px;color:#57606a\">{} · {}</div>\
879 <div style=\"white-space:pre-wrap;font-size:13px\">{}</div></div>",
880 esc(&c.author),
881 esc(&anchor),
882 esc(&tail(&c.body, 800)),
883 );
884 }
885 }
886
887 let total = diff.lines().count();
889 let shown = total.min(DIFF_MAX_LINES);
890 let _ = writeln!(
891 h,
892 "<h2 style=\"margin:16px 0 6px;font-size:15px\">{}</h2>",
893 w.diff
894 );
895 h.push_str(
896 "<div style=\"font:12px/1.45 ui-monospace,SFMono-Regular,Menlo,monospace;\
897 border:1px solid #d0d7de;border-radius:6px;overflow-x:auto\">\n",
898 );
899 for line in diff.lines().take(shown) {
900 let (gutter, style, body) = diff_row(line);
901 let _ = writeln!(
902 h,
903 "<div style=\"display:flex;{style}\">\
904 <span style=\"flex:0 0 1.4em;text-align:center;user-select:none;\
905 border-right:1px solid #d0d7de\">{gutter}</span>\
906 <span style=\"white-space:pre;padding-left:6px\">{}</span></div>",
907 esc(body),
908 );
909 }
910 h.push_str("</div>\n");
911 if total > shown {
912 let omitted = total - shown;
913 let head = state.winner().map_or("HEAD", |w| w.branch.as_str());
914 let where_ = state.winner().map_or_else(
915 || state.repo.display().to_string(),
916 |w| w.worktree.display().to_string(),
917 );
918 let _ = writeln!(
919 h,
920 "<p style=\"margin:8px 0 0;padding:8px;background:#fff8c5;border-radius:6px;\
921 font-size:13px\">{}: {}</p>",
922 w.truncated,
923 w.truncated_note(
924 omitted,
925 total,
926 shown,
927 &esc(&where_),
928 &esc(&state.base_branch),
929 &esc(head),
930 ),
931 );
932 }
933
934 h.push_str("</body>\n</html>\n");
935 h
936}
937
938async fn request_approval(state: &mut RunState, pr: &PrState, subject: &str) -> Result<Approval> {
944 let (worktree, head) = match state.winner() {
945 Some(w) => (w.worktree.clone(), w.branch.clone()),
946 None => (state.repo.clone(), "HEAD".to_owned()),
947 };
948 let base = state.base_branch.clone();
949 let range = format!("{base}...{head}");
950 let numstat = git::git_raw(&worktree, &["diff", "--numstat", "-M", &range])
954 .await
955 .map(|o| o.stdout)
956 .unwrap_or_default();
957 let diff = git::diff(&worktree, &base, &head).await.unwrap_or_default();
958 let commits: Vec<String> = git::git_raw(
959 &worktree,
960 &[
961 "log",
962 "--reverse",
963 "--format=%s",
964 &format!("{base}..{head}"),
965 ],
966 )
967 .await
968 .map(|o| o.stdout)
969 .unwrap_or_default()
970 .lines()
971 .filter(|l| !l.trim().is_empty())
972 .map(str::to_owned)
973 .collect();
974
975 let w = words(&state.config.graph.language);
976 let html = approval_panel(state, pr, &numstat, &diff, &commits, subject);
977 let store = ask::Questions::open();
978 let mut q = ask::Question::new(
979 state.id.clone(),
980 APPROVAL_NODE.to_owned(),
981 "land".to_owned(),
982 w.approval_summary(pr.number, subject),
983 w.approval_detail(&pr.url, &base, subject),
984 vec![APPROVE.to_owned(), HOLD.to_owned()],
985 );
986 store
987 .put_panel(&mut q, &html, &[])
988 .context("write the merge approval panel")?;
989 state.event("land", format!("asking for merge approval ({})", q.short()));
990 state.save()?;
991
992 let timeout = Duration::from_secs(state.config.graph.answer_timeout);
993 let said = ask::ask_and_wait(&mut q, &store, &state.config.notify, timeout).await?;
994 Ok(approval(said.as_deref()))
995}
996
997pub fn parse_pr(json: &str) -> Result<PrState> {
1000 let raw: GhPr = serde_json::from_str(json).context("parse `gh pr view --json ...` output")?;
1001 let state = match raw.state.to_ascii_uppercase().as_str() {
1002 "OPEN" => PrLifecycle::Open,
1003 "MERGED" => PrLifecycle::Merged,
1004 "CLOSED" => PrLifecycle::Closed,
1005 other => bail!("unknown pull request state `{other}`"),
1006 };
1007
1008 let mut failing = Vec::new();
1009 let mut pending = false;
1010 let mut unknown = false;
1011 for check in &raw.status_check_rollup {
1012 match check.verdict() {
1013 Verdict::Pass => {}
1014 Verdict::Pending => pending = true,
1015 Verdict::Fail => failing.push(check.label()),
1016 Verdict::Unknown => unknown = true,
1017 }
1018 }
1019 let checks = if raw.status_check_rollup.is_empty() {
1020 Checks::Unknown
1021 } else if pending {
1022 Checks::Pending
1023 } else if !failing.is_empty() {
1024 Checks::Red
1025 } else if unknown {
1026 Checks::Unknown
1027 } else {
1028 Checks::Green
1029 };
1030
1031 let mut review_comments = Vec::new();
1032 for r in raw.reviews {
1033 push_if_outstanding(
1034 &mut review_comments,
1035 ReviewComment {
1036 author: r.author.login,
1037 path: None,
1038 line: None,
1039 body: r.body,
1040 },
1041 );
1042 }
1043 for c in raw.comments {
1044 push_if_outstanding(
1045 &mut review_comments,
1046 ReviewComment {
1047 author: c.author.login,
1048 path: None,
1049 line: None,
1050 body: c.body,
1051 },
1052 );
1053 }
1054
1055 Ok(PrState {
1056 url: raw.url,
1057 number: raw.number,
1058 state,
1059 checks,
1060 failing,
1061 review_comments,
1062 blocking: Blocking::of(&raw.merge_state_status),
1063 })
1064}
1065
1066pub fn parse_inline_comments(json: &str) -> Result<Vec<ReviewComment>> {
1073 let raw: Vec<GhInline> =
1074 serde_json::from_str(json).context("parse `gh api .../pulls/<n>/comments` output")?;
1075 let mut out = Vec::new();
1076 for c in raw {
1077 push_if_outstanding(
1078 &mut out,
1079 ReviewComment {
1080 author: c.user.login,
1081 path: c.path,
1082 line: c.line,
1083 body: c.body,
1084 },
1085 );
1086 }
1087 Ok(out)
1088}
1089
1090fn push_if_outstanding(out: &mut Vec<ReviewComment>, comment: ReviewComment) {
1096 if comment.body.trim().is_empty() || comment.body.contains(MARKER) {
1097 return;
1098 }
1099 if comment.path.is_none() && is_noise(&comment.body) {
1100 return;
1101 }
1102 out.push(comment);
1103}
1104
1105pub fn is_noise(body: &str) -> bool {
1123 if NOT_A_REVIEW.iter().any(|m| body.contains(m)) {
1124 return true;
1125 }
1126 let mut content = false;
1127 for line in strip_blocks(body).lines() {
1128 let line = unquote(line);
1129 if line.is_empty() || is_checklist(line) || is_decoration(line) || is_banner(line) {
1130 continue;
1131 }
1132 content = true;
1133 break;
1134 }
1135 !content
1136}
1137
1138fn strip_blocks(body: &str) -> String {
1140 let mut out = String::with_capacity(body.len());
1141 let mut rest = body;
1142 loop {
1143 let open = ["<!--", "<details>"]
1144 .iter()
1145 .filter_map(|tag| rest.find(tag).map(|i| (i, *tag)))
1146 .min_by_key(|(i, _)| *i);
1147 let Some((at, tag)) = open else {
1148 out.push_str(rest);
1149 return out;
1150 };
1151 out.push_str(&rest[..at]);
1152 let after = &rest[at + tag.len()..];
1153 let close = if tag == "<!--" { "-->" } else { "</details>" };
1154 match after.find(close) {
1155 Some(end) => rest = &after[end + close.len()..],
1156 None => return out,
1158 }
1159 }
1160}
1161
1162fn unquote(line: &str) -> &str {
1164 let mut s = line.trim();
1165 while let Some(rest) = s.strip_prefix('>') {
1166 s = rest.trim_start();
1167 }
1168 s.trim()
1169}
1170
1171fn is_checklist(line: &str) -> bool {
1173 let rest = line
1174 .strip_prefix("- ")
1175 .or_else(|| line.strip_prefix("* "))
1176 .unwrap_or("");
1177 let rest = rest.trim_start();
1178 matches!(
1179 rest.get(..3),
1180 Some("[ ]") | Some("[x]") | Some("[X]") | Some("[*]")
1181 )
1182}
1183
1184fn is_decoration(line: &str) -> bool {
1186 line.starts_with('#')
1187 || line.starts_with("[!")
1188 || (line.len() >= 3 && line.chars().all(|c| matches!(c, '-' | '=' | '*' | '_')))
1189}
1190
1191fn is_banner(line: &str) -> bool {
1198 let plain = drop_spans(line, "**", "**");
1199 let plain = if plain.contains("](") {
1200 drop_spans(&plain, "[", ")")
1201 } else {
1202 plain
1203 };
1204 !plain.chars().any(char::is_alphanumeric)
1205}
1206
1207fn drop_spans(s: &str, open: &str, close: &str) -> String {
1211 let mut out = String::with_capacity(s.len());
1212 let mut rest = s;
1213 while let Some(at) = rest.find(open) {
1214 out.push_str(&rest[..at]);
1215 let after = &rest[at + open.len()..];
1216 match after.find(close) {
1217 Some(end) => rest = &after[end + close.len()..],
1218 None => return out,
1219 }
1220 }
1221 out.push_str(rest);
1222 out
1223}
1224
1225pub async fn land(state: &mut RunState, pr_url: &str) -> Result<PrState> {
1232 let repo = state.repo.clone();
1233 let budget = state.config.graph.land_rounds;
1234 let mut round = 0usize;
1235 let mut rebases = 0usize;
1238 let mut waited = Duration::ZERO;
1239 let mut shown: BTreeSet<String> = BTreeSet::new();
1244
1245 state.event("land", format!("watching {pr_url}"));
1246 state.save()?;
1247
1248 loop {
1249 let seen = observe(&repo, pr_url).await?;
1250 let mut pr = seen.pr;
1251 pr.review_comments.retain(|c| !shown.contains(&c.body));
1252 state.pr = Some(crate::run::PrRecord {
1253 url: pr.url.clone(),
1254 number: pr.number,
1255 state: pr.state.as_str().to_owned(),
1256 checks: pr.checks.as_str().to_owned(),
1257 round,
1258 rounds: budget,
1259 });
1260 state.save()?;
1261
1262 match decide(&pr, round, budget, waited) {
1263 Step::Wait => {
1264 if waited >= WAIT_CEILING {
1265 let why = format!(
1266 "checks were still running after {} minutes",
1267 WAIT_CEILING.as_secs() / 60
1268 );
1269 stop(state, &repo, &pr, &why).await?;
1270 return Ok(pr);
1271 }
1272 waited += POLL;
1273 tokio::time::sleep(POLL).await;
1274 }
1275 Step::Done { merged } => {
1276 state.status = if merged {
1277 RunStatus::Merged
1278 } else {
1279 RunStatus::Ready
1280 };
1281 let detail = if merged {
1282 format!("{} was merged", pr.url)
1283 } else {
1284 format!("{} was closed without merging", pr.url)
1285 };
1286 state.merge = Some(MergeOutcome {
1287 mode: MergeMode::Pr,
1288 ok: merged,
1289 detail: detail.clone(),
1290 });
1291 state.event("land", detail);
1292 state.save()?;
1293 return Ok(pr);
1294 }
1295 Step::Merge => {
1296 let subject = merge_subject(&seen.title, &state.instruction);
1297 if state.config.graph.land_approval
1300 && request_approval(state, &pr, &subject).await? == Approval::Hold
1301 {
1302 stop(
1303 state,
1304 &repo,
1305 &pr,
1306 "the owner did not approve the merge (held or unanswered)",
1307 )
1308 .await?;
1309 return Ok(pr);
1310 }
1311 let argv = merge_argv(pr.number, &subject);
1312 let out = gh(&repo, &argv).await?;
1313 if out.0 {
1314 state.status = RunStatus::Merged;
1315 state.merge = Some(MergeOutcome {
1316 mode: MergeMode::Pr,
1317 ok: true,
1318 detail: format!("gh {}", argv.join(" ")),
1319 });
1320 state.event("land", format!("merged {} as `{subject}`", pr.url));
1321 state.save()?;
1322 pr.state = PrLifecycle::Merged;
1323 return Ok(pr);
1324 }
1325 let after = observe(&repo, pr_url).await.ok().map(|s| s.pr.state);
1326 if let Some(outcome) = merged_after_all(&argv, &out.1, after) {
1327 state.status = RunStatus::Merged;
1328 state.merge = Some(outcome);
1329 state.event("land", format!("merged {} as `{subject}`", pr.url));
1330 state.save()?;
1331 pr.state = PrLifecycle::Merged;
1332 return Ok(pr);
1333 }
1334 stop(
1335 state,
1336 &repo,
1337 &pr,
1338 &format!("`gh pr merge` failed: {}", out.1),
1339 )
1340 .await?;
1341 return Ok(pr);
1342 }
1343 Step::Rebase => {
1344 if rebases >= budget {
1350 let why = format!(
1351 "the base moved under this branch {budget} time(s) and it still does \
1352 not merge; rebasing again would only race it"
1353 );
1354 stop(state, &repo, &pr, &why).await?;
1355 return Ok(pr);
1356 }
1357 rebases += 1;
1358 let Some(branch) = state.winner().map(|w| w.branch.clone()) else {
1359 stop(
1360 state,
1361 &repo,
1362 &pr,
1363 "the pull request conflicts and this run has no winning branch to rebase",
1364 )
1365 .await?;
1366 return Ok(pr);
1367 };
1368 let base = state.base_branch.clone();
1369 state.event(
1370 "land",
1371 format!("{} no longer merges; rebasing onto {base}", pr.url),
1372 );
1373 state.save()?;
1374
1375 git::fetch(&repo, "origin", &base).await.ok();
1379 let scratch = state.dir().join("rebase");
1380 let onto = format!("origin/{base}");
1381 match git::rebase_branch_in_temp(&repo, &scratch, &branch, &onto).await {
1382 Ok(None) => {
1383 let pushed = git::push_rewritten(&repo, "origin", &branch).await?;
1384 if !pushed.ok() {
1385 let why = format!(
1386 "rebased {branch} but could not push it: {}",
1387 pushed.stderr.trim()
1388 );
1389 stop(state, &repo, &pr, &why).await?;
1390 return Ok(pr);
1391 }
1392 state.event("land", format!("rebased {branch} onto {base}"));
1393 state.save()?;
1394 waited = Duration::ZERO;
1397 tokio::time::sleep(POLL).await;
1398 }
1399 Ok(Some(conflict)) => {
1401 let why = format!(
1402 "{} conflicts with {base} and the rebase did not apply: {}",
1403 pr.url,
1404 conflict.chars().take(600).collect::<String>()
1405 );
1406 stop(state, &repo, &pr, &why).await?;
1407 return Ok(pr);
1408 }
1409 Err(e) => {
1410 let why = format!("could not rebase {branch} onto {base}: {e:#}");
1411 stop(state, &repo, &pr, &why).await?;
1412 return Ok(pr);
1413 }
1414 }
1415 }
1416 Step::GiveUp { reason } => {
1417 stop(state, &repo, &pr, &reason).await?;
1418 return Ok(pr);
1419 }
1420 Step::Fix { reason } => {
1421 round += 1;
1422 waited = Duration::ZERO;
1423 for c in &pr.review_comments {
1424 shown.insert(c.body.clone());
1425 }
1426 state.event("land", format!("round {round}: {reason}"));
1427 state.save()?;
1428
1429 let logs = failing_logs(&repo, &seen.failing_urls).await;
1430 let was_red = pr.checks == Checks::Red;
1431 match fix_round(state, &pr, round, budget, &reason, &logs).await? {
1432 Fixed::Committed => {}
1433 Fixed::Declined if was_red => {
1434 let why = format!(
1435 "the fixer produced no commit while {} check(s) were failing; \
1436 stopping instead of looping on an unchanged tree",
1437 pr.failing.len()
1438 );
1439 stop(state, &repo, &pr, &why).await?;
1440 return Ok(pr);
1441 }
1442 Fixed::Declined => state.event(
1447 "land",
1448 format!("round {round}: fixer declined the comments, nothing committed"),
1449 ),
1450 Fixed::Failed(why) => {
1451 stop(state, &repo, &pr, &format!("the fix round failed: {why}")).await?;
1452 return Ok(pr);
1453 }
1454 }
1455 state.save()?;
1456 }
1457 }
1458 }
1459}
1460
1461struct Seen {
1465 pr: PrState,
1466 title: String,
1467 failing_urls: Vec<(String, String)>,
1468}
1469
1470async fn observe(repo: &Path, pr_url: &str) -> Result<Seen> {
1473 let view = gh(
1474 repo,
1475 &[
1476 "pr".to_owned(),
1477 "view".to_owned(),
1478 pr_url.to_owned(),
1479 "--json".to_owned(),
1480 "url,number,state,title,statusCheckRollup,reviews,comments,mergeStateStatus".to_owned(),
1481 ],
1482 )
1483 .await?;
1484 if !view.0 {
1485 bail!("gh pr view {pr_url}: {}", view.1);
1486 }
1487 let mut pr = parse_pr(&view.1)?;
1488 let raw: GhPr = serde_json::from_str(&view.1).context("re-read pull request json")?;
1489
1490 let inline = gh(
1491 repo,
1492 &[
1493 "api".to_owned(),
1494 format!("repos/{{owner}}/{{repo}}/pulls/{}/comments", pr.number),
1495 ],
1496 )
1497 .await?;
1498 if inline.0 {
1499 match parse_inline_comments(&inline.1) {
1500 Ok(mut comments) => pr.review_comments.append(&mut comments),
1501 Err(e) => tracing::warn!("inline review comments unreadable: {e}"),
1504 }
1505 } else {
1506 tracing::warn!("gh api pulls/{}/comments: {}", pr.number, inline.1);
1507 }
1508
1509 let failing_urls = raw
1510 .status_check_rollup
1511 .iter()
1512 .filter(|c| c.verdict() == Verdict::Fail)
1513 .filter_map(|c| c.url().map(|u| (c.label(), u.to_owned())))
1514 .collect();
1515
1516 Ok(Seen {
1517 pr,
1518 title: raw.title,
1519 failing_urls,
1520 })
1521}
1522
1523enum Fixed {
1525 Committed,
1527 Declined,
1529 Failed(String),
1531}
1532
1533async fn fix_round(
1539 state: &mut RunState,
1540 pr: &PrState,
1541 round: usize,
1542 budget: usize,
1543 reason: &str,
1544 logs: &str,
1545) -> Result<Fixed> {
1546 let winner = state
1547 .winner()
1548 .cloned()
1549 .context("landing needs a winning candidate; none is recorded on this run")?;
1550 let roles = state
1551 .config
1552 .resolve_roles()
1553 .context("resolve the roster for the fix round")?;
1554 let (spec, seat_key): (AgentSpec, String) = match &roles.fixer {
1558 Some(f) if f.id != winner.agent => (f.clone(), "fix".to_owned()),
1559 _ => (
1560 state
1561 .config
1562 .agent(&winner.agent)
1563 .cloned()
1564 .unwrap_or_else(|_| roles.implementers[winner.index].clone()),
1565 format!("impl-{}", winner.label),
1566 ),
1567 };
1568
1569 let prompt = fix_prompt(state, pr, round, budget, reason, logs);
1570 let mut seat = seat_of(state, &seat_key, &spec.id);
1571 let artifacts = agent::artifacts_dir(&state.dir());
1572 let prompt = if state.config.cache_dir().is_some() {
1573 format!("{prompt}\n\n{}", prompt::build_cache_note())
1574 } else {
1575 prompt
1576 };
1577 let out = agent::invoke(
1578 &spec,
1579 &mut seat,
1580 &Invocation {
1581 cwd: &winner.worktree,
1582 prompt: &prompt,
1583 timeout: Duration::from_secs(state.config.graph.timeout_fix),
1584 allow_write: true,
1585 sessions: state.config.graph.sessions,
1586 artifacts: &artifacts,
1587 stem: &format!("land-{round}"),
1588 run: &state.id,
1589 node: "land",
1590 cache_dir: state.config.cache_dir().as_deref(),
1591 },
1592 )
1593 .await;
1594 state.seats.insert(seat.key.clone(), seat);
1595
1596 match out {
1597 Ok(o) if o.quota_exhausted() => {
1598 return Ok(Fixed::Failed(
1599 "rate limited (quota); the fixer could not run".to_owned(),
1600 ));
1601 }
1602 Ok(o) if !o.usable() => {
1603 return Ok(Fixed::Failed(format!(
1604 "the fixer produced nothing usable (exit {:?}, timed out: {})",
1605 o.exit_code, o.timed_out
1606 )));
1607 }
1608 Ok(_) => {}
1609 Err(e) => return Ok(Fixed::Failed(format!("{e:#}"))),
1610 }
1611
1612 let before = git::rev_parse(&winner.worktree, "HEAD").await?;
1613 git::commit_all(
1616 &winner.worktree,
1617 &format!("magi: land round {round} fixes (uncommitted work)"),
1618 )
1619 .await
1620 .ok();
1621 let after = git::rev_parse(&winner.worktree, "HEAD").await?;
1622 if after == before {
1623 return Ok(Fixed::Declined);
1624 }
1625
1626 let remote = state.config.merge.remote.clone();
1627 let push = git::push(&winner.worktree, &remote, &winner.branch).await?;
1628 if !push.ok() {
1629 return Ok(Fixed::Failed(format!(
1630 "pushing {} to {remote} failed: {}",
1631 winner.branch, push.stderr
1632 )));
1633 }
1634 state.event(
1635 "land",
1636 format!("round {round}: pushed a fix to {}", winner.branch),
1637 );
1638 Ok(Fixed::Committed)
1639}
1640
1641fn seat_of(state: &mut RunState, key: &str, agent: &str) -> SeatState {
1643 if let Some(existing) = state.seats.get(key)
1644 && existing.agent == agent
1645 {
1646 return existing.clone();
1647 }
1648 let fresh = SeatState::new(key, agent, state.seed);
1649 state.seats.insert(key.to_owned(), fresh.clone());
1650 fresh
1651}
1652
1653fn fix_prompt(
1655 state: &RunState,
1656 pr: &PrState,
1657 round: usize,
1658 budget: usize,
1659 reason: &str,
1660 logs: &str,
1661) -> String {
1662 let mut s = format!(
1663 "Your patch is open as a pull request and it is not landing. Land round \
1664 {round} of {budget}.\n\n\
1665 Pull request: {}\n\n\
1666 What is holding it: {reason}\n\n\
1667 # The task\n\n{}\n",
1668 pr.url, state.instruction
1669 );
1670
1671 if pr.failing.is_empty() {
1672 s.push_str("\n# Failing checks\n\n(none)\n");
1673 } else {
1674 let _ = write!(s, "\n# Failing checks\n\n- {}\n", pr.failing.join("\n- "));
1675 if logs.trim().is_empty() {
1676 s.push_str("\nNo log could be read; reproduce the failure locally.\n");
1677 } else {
1678 let _ = write!(s, "\n## Failing log tails\n\n{logs}\n");
1679 }
1680 }
1681
1682 if pr.review_comments.is_empty() {
1683 s.push_str("\n# Review comments\n\n(none)\n");
1684 } else {
1685 s.push_str("\n# Review comments\n");
1686 for c in &pr.review_comments {
1687 let where_ = match (&c.path, c.line) {
1688 (Some(p), Some(l)) => format!(" ({p}:{l})"),
1689 (Some(p), None) => format!(" ({p})"),
1690 _ => String::new(),
1691 };
1692 let _ = write!(s, "\n## {}{where_}\n\n{}\n", c.author, c.body.trim());
1693 }
1694 }
1695
1696 s.push_str(
1697 "\n# Rules\n\n\
1698 1. Fix the cause, never the symptom. Do not delete, skip, or weaken a \
1699 failing test; do not silence a lint with an allow attribute; do not \
1700 stretch a timeout to hide a race. If the check is right, the code is \
1701 wrong.\n\
1702 2. Change nothing the checks and the comments did not raise. A \
1703 drive-by refactor turns a one-line fix into a pull request that \
1704 needs reviewing again.\n\
1705 3. If a comment is wrong, say so with a checkable argument and change \
1706 nothing for it. A declined comment with a reason is a correct \
1707 outcome; a change made to appease a reviewer is not.\n\
1708 4. Commit in this worktree. magi pushes to the pull request's branch \
1709 for you; do not push, merge, or close anything yourself.\n\
1710 5. Never name yourself, your vendor, or your model, anywhere.\n\n\
1711 # Output\n\n\
1712 Say what you changed and why, and what you declined and why.",
1713 );
1714
1715 let language = &state.config.graph.language;
1716 if !(language.trim().is_empty() || language.eq_ignore_ascii_case("en")) {
1717 let _ = write!(s, "\n\nWrite all prose in {language}.");
1718 }
1719 if let Some(overlay) = state.config.prompts.overlay("fix") {
1720 let _ = write!(s, "\n\n{overlay}");
1721 }
1722 s
1723}
1724
1725async fn failing_logs(repo: &Path, failing: &[(String, String)]) -> String {
1728 let mut out = String::new();
1729 for (name, url) in failing.iter().take(MAX_LOGS) {
1730 let args = match (job_of(url), run_of(url)) {
1731 (Some(job), _) => vec![
1732 "run".to_owned(),
1733 "view".to_owned(),
1734 "--log-failed".to_owned(),
1735 "--job".to_owned(),
1736 job,
1737 ],
1738 (None, Some(run)) => vec![
1739 "run".to_owned(),
1740 "view".to_owned(),
1741 run,
1742 "--log-failed".to_owned(),
1743 ],
1744 (None, None) => continue,
1746 };
1747 let (ok, body) = match gh(repo, &args).await {
1748 Ok(v) => v,
1749 Err(e) => (false, format!("{e:#}")),
1750 };
1751 if !ok && body.trim().is_empty() {
1752 continue;
1753 }
1754 let _ = write!(out, "### {name}\n\n```\n{}\n```\n\n", tail(&body, LOG_TAIL));
1755 }
1756 out
1757}
1758
1759fn job_of(details_url: &str) -> Option<String> {
1762 let after = details_url.split("/job/").nth(1)?;
1763 let id: String = after.chars().take_while(char::is_ascii_digit).collect();
1764 (!id.is_empty()).then_some(id)
1765}
1766
1767fn run_of(details_url: &str) -> Option<String> {
1769 let after = details_url.split("/actions/runs/").nth(1)?;
1770 let id: String = after.chars().take_while(char::is_ascii_digit).collect();
1771 (!id.is_empty()).then_some(id)
1772}
1773
1774async fn stop(state: &mut RunState, repo: &Path, pr: &PrState, why: &str) -> Result<()> {
1779 let body = format!(
1780 "{MARKER}\nmagi stopped landing this pull request: {why}\n\n\
1781 The branch is untouched and the run is `{}`. Nothing was merged.",
1782 state.id
1783 );
1784 let posted = gh(
1785 repo,
1786 &[
1787 "pr".to_owned(),
1788 "comment".to_owned(),
1789 pr.number.to_string(),
1790 "--body".to_owned(),
1791 body,
1792 ],
1793 )
1794 .await;
1795 match posted {
1796 Ok((true, _)) => {}
1797 Ok((false, out)) => tracing::warn!("could not comment on {}: {out}", pr.url),
1798 Err(e) => tracing::warn!("could not comment on {}: {e:#}", pr.url),
1799 }
1800 state.status = RunStatus::Blocked;
1801 state.merge = Some(MergeOutcome {
1802 mode: MergeMode::Pr,
1803 ok: false,
1804 detail: why.to_owned(),
1805 });
1806 state.event("land", format!("stopped: {why}"));
1807 state.save()?;
1808 Ok(())
1809}
1810
1811async fn gh(cwd: &Path, args: &[String]) -> Result<(bool, String)> {
1816 let out = tokio::process::Command::new("gh")
1817 .args(args)
1818 .current_dir(cwd)
1819 .quiet()
1820 .stdin(std::process::Stdio::null())
1821 .output()
1822 .await
1823 .with_context(|| format!("spawn gh {}", args.join(" ")))?;
1824 let mut body = String::from_utf8_lossy(&out.stdout).into_owned();
1825 let err = String::from_utf8_lossy(&out.stderr);
1826 if body.trim().is_empty() {
1827 body = err.into_owned();
1828 } else if !err.trim().is_empty() {
1829 body.push_str(&err);
1830 }
1831 Ok((out.status.success(), body.trim().to_owned()))
1832}
1833
1834#[derive(Debug, Clone, Copy, PartialEq, Eq)]
1836enum Verdict {
1837 Pass,
1838 Fail,
1839 Pending,
1840 Unknown,
1841}
1842
1843#[derive(Debug, Deserialize)]
1844#[serde(rename_all = "camelCase")]
1845struct GhPr {
1846 #[serde(default)]
1847 url: String,
1848 #[serde(default)]
1849 number: u64,
1850 #[serde(default)]
1851 state: String,
1852 #[serde(default)]
1853 title: String,
1854 #[serde(default)]
1855 status_check_rollup: Vec<GhCheck>,
1856 #[serde(default)]
1863 merge_state_status: String,
1864 #[serde(default)]
1865 reviews: Vec<GhReview>,
1866 #[serde(default)]
1867 comments: Vec<GhComment>,
1868}
1869
1870#[derive(Debug, Deserialize)]
1875#[serde(rename_all = "camelCase")]
1876struct GhCheck {
1877 #[serde(default)]
1878 name: Option<String>,
1879 #[serde(default)]
1880 context: Option<String>,
1881 #[serde(default)]
1882 status: Option<String>,
1883 #[serde(default)]
1884 conclusion: Option<String>,
1885 #[serde(default)]
1886 state: Option<String>,
1887 #[serde(default)]
1888 details_url: Option<String>,
1889 #[serde(default)]
1890 target_url: Option<String>,
1891}
1892
1893impl GhCheck {
1894 fn label(&self) -> String {
1896 self.name
1897 .clone()
1898 .or_else(|| self.context.clone())
1899 .unwrap_or_else(|| "(unnamed check)".to_owned())
1900 }
1901
1902 fn url(&self) -> Option<&str> {
1904 self.details_url
1905 .as_deref()
1906 .or(self.target_url.as_deref())
1907 .filter(|u| !u.is_empty())
1908 }
1909
1910 fn verdict(&self) -> Verdict {
1918 if let Some(status) = self.status.as_deref() {
1919 if !status.eq_ignore_ascii_case("COMPLETED") {
1920 return Verdict::Pending;
1921 }
1922 }
1923 let outcome = self
1924 .conclusion
1925 .as_deref()
1926 .or(self.state.as_deref())
1927 .unwrap_or("");
1928 match outcome.to_ascii_uppercase().as_str() {
1929 "SUCCESS" | "SKIPPED" | "NEUTRAL" => Verdict::Pass,
1930 "FAILURE" | "ERROR" | "TIMED_OUT" | "CANCELLED" | "STARTUP_FAILURE"
1931 | "ACTION_REQUIRED" => Verdict::Fail,
1932 "PENDING" | "EXPECTED" | "QUEUED" | "IN_PROGRESS" | "WAITING" | "REQUESTED" => {
1933 Verdict::Pending
1934 }
1935 _ => Verdict::Unknown,
1936 }
1937 }
1938}
1939
1940#[derive(Debug, Deserialize)]
1941struct GhAuthor {
1942 #[serde(default)]
1943 login: String,
1944}
1945
1946#[derive(Debug, Deserialize)]
1947struct GhReview {
1948 #[serde(default)]
1949 author: GhAuthor,
1950 #[serde(default)]
1951 body: String,
1952}
1953
1954#[derive(Debug, Deserialize)]
1955struct GhComment {
1956 #[serde(default)]
1957 author: GhAuthor,
1958 #[serde(default)]
1959 body: String,
1960}
1961
1962#[derive(Debug, Deserialize)]
1963struct GhUser {
1964 #[serde(default)]
1965 login: String,
1966}
1967
1968#[derive(Debug, Deserialize)]
1969struct GhInline {
1970 #[serde(default)]
1971 user: GhUser,
1972 #[serde(default)]
1973 path: Option<String>,
1974 #[serde(default)]
1975 line: Option<u64>,
1976 #[serde(default)]
1977 body: String,
1978}
1979
1980impl Default for GhAuthor {
1981 fn default() -> Self {
1982 Self {
1983 login: "(unknown)".to_owned(),
1984 }
1985 }
1986}
1987
1988impl Default for GhUser {
1989 fn default() -> Self {
1990 Self {
1991 login: "(unknown)".to_owned(),
1992 }
1993 }
1994}
1995
1996#[cfg(test)]
1997mod tests {
1998 use super::*;
1999
2000 const GREEN_OPEN: &str = r####"{
2002 "url": "https://github.com/yukimemi/magi/pull/10",
2003 "number": 10,
2004 "state": "OPEN",
2005 "mergeStateStatus": "CLEAN",
2006 "statusCheckRollup": [
2007 {
2008 "__typename": "CheckRun",
2009 "conclusion": "SKIPPED",
2010 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33356278334/job/99378963755",
2011 "name": "review",
2012 "status": "COMPLETED",
2013 "workflowName": "claude-review"
2014 },
2015 {
2016 "__typename": "CheckRun",
2017 "conclusion": "SUCCESS",
2018 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33356278338/job/99378963144",
2019 "name": "check (ubuntu-latest)",
2020 "status": "COMPLETED",
2021 "workflowName": "CI"
2022 },
2023 {
2024 "__typename": "CheckRun",
2025 "conclusion": "SUCCESS",
2026 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33356278338/job/99378963095",
2027 "name": "rustfmt",
2028 "status": "COMPLETED",
2029 "workflowName": "CI"
2030 },
2031 {
2032 "__typename": "StatusContext",
2033 "context": "CodeRabbit",
2034 "state": "SUCCESS",
2035 "targetUrl": ""
2036 }
2037 ],
2038 "reviews": [],
2039 "comments": [
2040 {
2041 "author": {
2042 "login": "coderabbitai"
2043 },
2044 "authorAssociation": "NONE",
2045 "body": "<!-- This is an auto-generated comment: summarize by coderabbit.ai -->\n<!-- This is an auto-generated comment: skip review by coderabbit.ai -->\n\n> [!IMPORTANT]\n> - [ ] <!-- {\"checkboxId\":\"e9bb8d72-00e8-4f67-9cb2-caf3b22574fe\"} --> 🔍 Trigger review\n> \n> This repository does not receive automatic reviews because it has fewer than 10 stars.\n> \n> <details>\n> <summary>⚙️ Run configuration</summary>\n> \n> **Configuration used**: defaults\n> \n> **Review profile**: CHILL\n> \n> **Plan**: Pro Plus\n> \n> **Run ID**: `78e70bf3-c5a0-4269-a96c-2afb2dba7eff`\n> \n> </details>\n\n<!-- end of auto-generated comment: skip review by coderabbit.ai -->\n\n<!-- tips_start -->\n\n---\n\nThanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=yukimemi/magi&utm_content=10)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.\n\n<details>\n<summary>❤️ Share</summary>\n\n- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%2"
2046 }
2047 ]
2048}"####;
2049
2050 const RED_OPEN: &str = r####"{
2052 "url": "https://github.com/yukimemi/magi/pull/9",
2053 "number": 9,
2054 "state": "OPEN",
2055 "mergeStateStatus": "UNSTABLE",
2056 "statusCheckRollup": [
2057 {
2058 "__typename": "CheckRun",
2059 "conclusion": "SUCCESS",
2060 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323744",
2061 "name": "check (ubuntu-latest)",
2062 "status": "COMPLETED",
2063 "workflowName": "CI"
2064 },
2065 {
2066 "__typename": "CheckRun",
2067 "conclusion": "SUCCESS",
2068 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323811",
2069 "name": "rustfmt",
2070 "status": "COMPLETED",
2071 "workflowName": "CI"
2072 },
2073 {
2074 "__typename": "CheckRun",
2075 "conclusion": "FAILURE",
2076 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323572",
2077 "name": "editorconfig",
2078 "status": "COMPLETED",
2079 "workflowName": "CI"
2080 },
2081 {
2082 "__typename": "StatusContext",
2083 "context": "CodeRabbit",
2084 "state": "SUCCESS",
2085 "targetUrl": ""
2086 }
2087 ],
2088 "reviews": [],
2089 "comments": [
2090 {
2091 "author": {
2092 "login": "coderabbitai"
2093 },
2094 "authorAssociation": "NONE",
2095 "body": "<!-- This is an auto-generated comment: summarize by coderabbit.ai -->\n<!-- This is an auto-generated comment: skip review by coderabbit.ai -->\n\n> [!IMPORTANT]\n> - [ ] <!-- {\"checkboxId\":\"e9bb8d72-00e8-4f67-9cb2-caf3b22574fe\"} --> 🔍 Trigger review\n> \n> This repository does not receive automatic reviews because it has fewer than 10 stars.\n> \n> <details>\n> <summary>⚙️ Run configuration</summary>\n> \n> **Configuration used**: defaults\n> \n> **Review profile**: CHILL\n> \n> **Plan**: Team\n> \n> **Run ID**: `91e0dc24-6040-4c3d-92c6-f7d2b542523d`\n> \n> </details>\n\n<!-- end of auto-generated comment: skip review by coderabbit.ai -->\n\n<!-- tips_start -->\n\n---\n\nThanks for using [CodeRabbit](https://coderab"
2096 }
2097 ]
2098}"####;
2099
2100 const PENDING_OPEN: &str = r####"{
2102 "url": "https://github.com/yukimemi/magi/pull/9",
2103 "number": 9,
2104 "state": "OPEN",
2105 "statusCheckRollup": [
2106 {
2107 "__typename": "CheckRun",
2108 "conclusion": "SUCCESS",
2109 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323744",
2110 "name": "check (ubuntu-latest)",
2111 "status": "COMPLETED",
2112 "workflowName": "CI"
2113 },
2114 {
2115 "__typename": "CheckRun",
2116 "conclusion": "SUCCESS",
2117 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323811",
2118 "name": "rustfmt",
2119 "status": "COMPLETED",
2120 "workflowName": "CI"
2121 },
2122 {
2123 "__typename": "CheckRun",
2124 "conclusion": null,
2125 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323572",
2126 "name": "editorconfig",
2127 "status": "IN_PROGRESS",
2128 "workflowName": "CI"
2129 },
2130 {
2131 "__typename": "StatusContext",
2132 "context": "CodeRabbit",
2133 "state": "SUCCESS",
2134 "targetUrl": ""
2135 }
2136 ],
2137 "reviews": [],
2138 "comments": []
2139}"####;
2140
2141 const MERGED: &str = r####"{
2143 "url": "https://github.com/yukimemi/magi/pull/16",
2144 "number": 16,
2145 "state": "MERGED",
2146 "statusCheckRollup": [
2147 {
2148 "__typename": "CheckRun",
2149 "conclusion": "SUCCESS",
2150 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33636587933/job/100268878095",
2151 "name": "check (ubuntu-latest)",
2152 "status": "COMPLETED",
2153 "workflowName": "CI"
2154 },
2155 {
2156 "__typename": "CheckRun",
2157 "conclusion": "SUCCESS",
2158 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33636587918/job/100268876427",
2159 "name": "review",
2160 "status": "COMPLETED",
2161 "workflowName": "claude-review"
2162 }
2163 ],
2164 "reviews": [],
2165 "comments": []
2166}"####;
2167
2168 const REVIEWED_OPEN: &str = r####"{
2170 "url": "https://github.com/yukimemi/magi/pull/12",
2171 "number": 12,
2172 "state": "OPEN",
2173 "statusCheckRollup": [
2174 {
2175 "__typename": "CheckRun",
2176 "conclusion": "SUCCESS",
2177 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33571212506/job/100065355258",
2178 "name": "check (ubuntu-latest)",
2179 "status": "COMPLETED",
2180 "workflowName": "CI"
2181 },
2182 {
2183 "__typename": "CheckRun",
2184 "conclusion": "SUCCESS",
2185 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33571212566/job/100065355810",
2186 "name": "review",
2187 "status": "COMPLETED",
2188 "workflowName": "claude-review"
2189 }
2190 ],
2191 "reviews": [
2192 {
2193 "author": {
2194 "login": "claude"
2195 },
2196 "state": "COMMENTED",
2197 "body": ""
2198 }
2199 ],
2200 "comments": [
2201 {
2202 "author": {
2203 "login": "coderabbitai"
2204 },
2205 "authorAssociation": "NONE",
2206 "body": "<!-- This is an auto-generated comment: summarize by coderabbit.ai -->\n<!-- This is an auto-generated comment: skip review by coderabbit.ai -->\n\n> [!IMPORTANT]\n> - [ ] <!-- {\"checkboxId\":\"e9bb8d72-00e8-4f67-9cb2-caf3b22574fe\"} --> 🔍 Trigger review\n> \n> This repository does not receive automatic reviews because it has fewer than 10 stars.\n> \n> <details>\n> <summary>⚙️ Run configuration</summary>\n> \n> **Configuration used**: defaults\n> \n> **Review profile**: CHILL\n> \n> **Plan**: Team\n> \n> **Run ID**: `72058bf3-b7df-41d9-8e4d-a06a31be4a26`\n> \n> </details>\n\n<!-- end of auto-generated comment: skip review by coderabbit.ai -->\n\n<!-- tips_start -->\n\n---\n\nThanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=yukimemi/magi&utm_content=12)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.\n\n<details>\n<summa"
2207 },
2208 {
2209 "author": {
2210 "login": "claude"
2211 },
2212 "authorAssociation": "NONE",
2213 "body": "**Claude finished @yukimemi's task in 3m 52s** —— [View job](https://github.com/yukimemi/magi/actions/runs/33571212566)\n\n---\n### Review: `magi review <branch>` — cheap-half-only graph\n\nRead through `src/graph.rs`, `src/main.rs`, `src/prompt.rs`, and the new/edited tests, and traced the claimed degeneration (`prep` short-circuits on a non-empty candidate list, `implement` skips because `commits != 0`, `judge`/`vote` skip on `viable().len() == 1`, `tally` skips because it's pre-set, `fold_losers` has no losers) against the actual code — it holds up. CI (`cargo make check`) is green on this PR.\n\n**Correctness**\n\n- One real bug, flagged inline on `src/graph.rs:1255`: the fixer-agent fallback (`self.roles.implementers[winner.index].clone()`) is unreachable in the normal graph (a real candidate's `winner.agent` always resolves via `config.agent(...)`), but a review-only run's `winner.agent` is always the `\"(existing branch)\"` sentinel, so this fallback now runs on *every* review-only fix that has no dedicated `[roles] fixer`. `graph.candidates` has no lower-bound validation, so a `magi.toml` tuned for review-only use (`candidates = 0`, plausible given this PR's own cost rationale) would panic with an out-of-bounds index the first time a"
2214 }
2215 ]
2216}"####;
2217
2218 const INLINE: &str = r####"[
2220 {
2221 "user": {
2222 "login": "claude[bot]"
2223 },
2224 "path": "src/graph.rs",
2225 "line": 231,
2226 "body": "Minor edge case: unlike `implement()` (which sets `c.empty = commits == 0 || patch.trim().is_empty()`, `src/graph.rs:472`), the seeded review-only candidate always sets `empty: false` once `commits > 0` is confirmed, without checking whether the diff itself is actually empty (e.g. a commit immediately followed by a revert nets zero file changes). Such a branch would pass `Runner::review`'s validation and proceed into a review round with an empty patch, where `implement()`'s equivalent path would"
2227 }
2228]"####;
2229
2230 const CODERABBIT_TRIGGER: &str = r####"<!-- This is an auto-generated comment: summarize by coderabbit.ai -->
2232<!-- This is an auto-generated comment: skip review by coderabbit.ai -->
2233
2234> [!IMPORTANT]
2235> - [ ] <!-- {"checkboxId":"e9bb8d72-00e8-4f67-9cb2-caf3b22574fe"} --> 🔍 Trigger review
2236>
2237> This repository does not receive automatic reviews because it has fewer than 10 stars.
2238>
2239> <details>
2240> <summary>⚙️ Run configuration</summary>
2241>
2242> **Configuration used**: defaults
2243>
2244> **Review profile**: CHILL
2245>
2246> **Plan**: Team
2247>
2248> **Run ID**: `c1e2a68f-87fc-4b35-9ec4-e75c7854966a`
2249>
2250> </details>
2251
2252<!-- end of auto-generated comment: skip review by coderabbit.ai -->
2253
2254<!-- tips_start -->
2255
2256---
2257
2258Thanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=yukimemi/magi&utm_content=16)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
2259
2260<details>
2261<summary>❤️ Share</summary>
2262
2263- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20off"####;
2264
2265 const CLAUDE_CHECKLIST: &str = r####"**Claude finished @yukimemi's task in 4m 14s** —— [View job](https://github.com/yukimemi/magi/actions/runs/33636587918)
2267
2268---
2269### Reviewing PR #16
2270
2271- [x] Read AGENTS.md conventions
2272- [x] Review `src/daemon.rs` changes
2273- [x] Review `src/main.rs` changes (new `doctor` reporting)
2274- [x] Review `src/web.rs` changes (reuse of unreadable-run count)
2275- [x] Check test coverage for new behavior
2276- [x] Run verification commands (blocked — see note)
2277- [x] Post findings"####;
2278
2279 const CLAUDE_FINDING: &str = r####"**Claude finished @yukimemi's task in 3m 52s** —— [View job](https://github.com/yukimemi/magi/actions/runs/33571212566)
2281
2282---
2283### Review: `magi review <branch>` — cheap-half-only graph
2284
2285Read through `src/graph.rs`, `src/main.rs`, `src/prompt.rs`, and the new/edited tests, and traced the claimed degeneration (`prep` short-circuits on a non-empty candidate list, `implement` skips because `commits != 0`, `judge`/`vote` skip on `viable().len() == 1`, `tally` skips because it's pre-set, `fold_losers` has no losers) against the actual code — it holds up. CI (`cargo make check`) is green on this PR.
2286
2287**Correctness**
2288
2289- One real bug, flagged inline on `src/graph.rs:1255`: the fixer-agent fallback (`self.roles.implementers[winner.index].clone()`) is unreachable in the normal graph (a real candidate's `winner.agent` always resolves via `config.agent(...)`), but a review-only run's `winner.agent` is always the `"(existing branch)"` sentinel, so this fallback now runs on *every* review-only fix that has no dedicated `[roles] fixer`. `graph.candidates` has no lower-bound validation, so a `magi.toml` tuned for review-only use (`candidates = 0`, plausible given this PR's own cost rationale) would panic with an out-of-bounds index the first time a"####;
2290
2291 fn pr(checks: Checks, failing: &[&str], comments: usize) -> PrState {
2292 PrState {
2293 url: "https://github.com/yukimemi/magi/pull/16".to_owned(),
2294 number: 16,
2295 state: PrLifecycle::Open,
2296 checks,
2297 blocking: if matches!(checks, Checks::Red) {
2301 Blocking::Yes
2302 } else {
2303 Blocking::No
2304 },
2305 failing: failing.iter().map(|s| (*s).to_owned()).collect(),
2306 review_comments: (0..comments)
2307 .map(|i| ReviewComment {
2308 author: "coderabbitai".to_owned(),
2309 path: Some("src/graph.rs".to_owned()),
2310 line: Some(231),
2311 body: format!("finding {i}"),
2312 })
2313 .collect(),
2314 }
2315 }
2316
2317 #[test]
2318 fn a_green_pull_request_with_nothing_outstanding_parses_as_ready_to_merge() {
2319 let state = parse_pr(GREEN_OPEN).expect("green fixture parses");
2320 assert_eq!(state.number, 10);
2321 assert_eq!(state.state, PrLifecycle::Open);
2322 assert_eq!(state.checks, Checks::Green);
2323 assert!(state.failing.is_empty());
2324 assert!(
2325 state.review_comments.is_empty(),
2326 "the only comment is CodeRabbit's trigger notice: {:?}",
2327 state.review_comments
2328 );
2329 assert_eq!(decide(&state, 0, 4, Duration::ZERO), Step::Merge);
2330 }
2331
2332 #[test]
2333 fn a_failing_check_parses_as_red_and_is_named() {
2334 let state = parse_pr(RED_OPEN).expect("red fixture parses");
2335 assert_eq!(state.checks, Checks::Red);
2336 assert_eq!(state.failing, vec!["editorconfig".to_owned()]);
2337 let mut blocking = state.clone();
2344 blocking.blocking = Blocking::Yes;
2345 match decide(&blocking, 0, 4, Duration::ZERO) {
2346 Step::Fix { reason } => {
2347 assert!(reason.contains("editorconfig"), "reason: {reason}");
2348 assert!(reason.contains("failing"), "reason: {reason}");
2349 }
2350 other => panic!("expected a fix round, got {other:?}"),
2351 }
2352 }
2353
2354 #[test]
2355 fn a_check_still_running_parses_as_pending_and_is_waited_for() {
2356 let state = parse_pr(PENDING_OPEN).expect("pending fixture parses");
2357 assert_eq!(state.checks, Checks::Pending);
2358 assert_eq!(decide(&state, 0, 4, Duration::ZERO), Step::Wait);
2359 }
2360
2361 #[test]
2362 fn a_pull_request_merged_underneath_us_is_done_rather_than_a_failure() {
2363 let state = parse_pr(MERGED).expect("merged fixture parses");
2364 assert_eq!(state.state, PrLifecycle::Merged);
2365 assert_eq!(
2366 decide(&state, 0, 4, Duration::ZERO),
2367 Step::Done { merged: true }
2368 );
2369 }
2370
2371 #[test]
2372 fn a_review_that_found_something_is_outstanding_and_holds_the_merge() {
2373 let state = parse_pr(REVIEWED_OPEN).expect("reviewed fixture parses");
2374 assert_eq!(state.checks, Checks::Green);
2375 let authors: Vec<&str> = state
2376 .review_comments
2377 .iter()
2378 .map(|c| c.author.as_str())
2379 .collect();
2380 assert_eq!(
2381 authors,
2382 vec!["claude"],
2383 "CodeRabbit's walkthrough is machinery; Claude's review is a finding"
2384 );
2385 match decide(&state, 0, 4, Duration::ZERO) {
2386 Step::Fix { reason } => assert!(reason.contains("unresolved"), "reason: {reason}"),
2387 other => panic!("expected a fix round, got {other:?}"),
2388 }
2389 }
2390
2391 #[test]
2392 fn inline_review_comments_keep_their_file_and_line() {
2393 let comments = parse_inline_comments(INLINE).expect("inline fixture parses");
2394 assert_eq!(comments.len(), 1);
2395 assert_eq!(comments[0].author, "claude[bot]");
2396 assert_eq!(comments[0].path.as_deref(), Some("src/graph.rs"));
2397 assert_eq!(comments[0].line, Some(231));
2398 assert!(comments[0].body.contains("empty"), "{}", comments[0].body);
2399 }
2400
2401 #[test]
2402 fn a_status_only_bot_comment_does_not_trigger_a_fix_round() {
2403 assert!(
2404 is_noise(CODERABBIT_TRIGGER),
2405 "CodeRabbit's trigger notice declares itself not a review"
2406 );
2407 assert!(
2408 is_noise(CLAUDE_CHECKLIST),
2409 "a progress checklist asks for nothing"
2410 );
2411 assert!(
2412 !is_noise(CLAUDE_FINDING),
2413 "a review that names a bug is input, not noise"
2414 );
2415
2416 let mut clean = pr(Checks::Green, &[], 0);
2417 clean.review_comments.push(ReviewComment {
2418 author: "coderabbitai".to_owned(),
2419 path: None,
2420 line: None,
2421 body: CODERABBIT_TRIGGER.to_owned(),
2422 });
2423 clean.review_comments.retain(|c| !is_noise(&c.body));
2424 assert_eq!(decide(&clean, 0, 4, Duration::ZERO), Step::Merge);
2425
2426 let mut found = pr(Checks::Green, &[], 0);
2427 found.review_comments.push(ReviewComment {
2428 author: "claude".to_owned(),
2429 path: None,
2430 line: None,
2431 body: CLAUDE_FINDING.to_owned(),
2432 });
2433 found.review_comments.retain(|c| !is_noise(&c.body));
2434 assert!(matches!(
2435 decide(&found, 0, 4, Duration::ZERO),
2436 Step::Fix { .. }
2437 ));
2438 }
2439
2440 #[test]
2441 fn the_policy_table_holds_for_every_combination_that_matters() {
2442 let cases: Vec<(&str, PrState, usize, usize, Duration, Step)> = vec![
2443 (
2444 "pending checks are waited for, even on the last round",
2445 pr(Checks::Pending, &[], 0),
2446 4,
2447 4,
2448 Duration::ZERO,
2449 Step::Wait,
2450 ),
2451 (
2452 "red checks are fixed",
2453 pr(Checks::Red, &["editorconfig"], 0),
2454 0,
2455 4,
2456 Duration::ZERO,
2457 Step::Fix {
2458 reason: "1 check(s) failing: editorconfig".to_owned(),
2459 },
2460 ),
2461 (
2462 "green with comments is fixed, not merged",
2463 pr(Checks::Green, &[], 2),
2464 1,
2465 4,
2466 Duration::ZERO,
2467 Step::Fix {
2468 reason: "checks are green but 2 review comment(s) are unresolved: coderabbitai"
2469 .to_owned(),
2470 },
2471 ),
2472 (
2473 "green and clean merges",
2474 pr(Checks::Green, &[], 0),
2475 3,
2476 4,
2477 Duration::ZERO,
2478 Step::Merge,
2479 ),
2480 (
2481 "an unreadable rollup is waited on while the grace lasts",
2482 pr(Checks::Unknown, &[], 0),
2483 0,
2484 4,
2485 Duration::ZERO,
2486 Step::Wait,
2487 ),
2488 (
2489 "an unreadable rollup is never merged once the grace is spent",
2490 pr(Checks::Unknown, &[], 0),
2491 0,
2492 4,
2493 CHECKS_GRACE,
2494 Step::GiveUp {
2495 reason: "no check status is readable on the pull request after 3 minute(s); \
2496 refusing to merge on a guess"
2497 .to_owned(),
2498 },
2499 ),
2500 ];
2501 for (what, state, round, budget, waited, want) in cases {
2502 assert_eq!(decide(&state, round, budget, waited), want, "{what}");
2503 }
2504 }
2505
2506 #[test]
2507 fn the_forge_verdict_survives_the_round_trip_from_gh() {
2508 let green = parse_pr(GREEN_OPEN).expect("parse");
2512 assert_eq!(green.blocking, Blocking::No);
2513 let red = parse_pr(RED_OPEN).expect("parse");
2514 assert_eq!(
2515 red.blocking,
2516 Blocking::No,
2517 "`UNSTABLE` is mergeable: the red check is one nobody requires"
2518 );
2519 assert_eq!(red.checks, Checks::Red, "and it is still reported as red");
2520 let quiet =
2522 parse_pr(&GREEN_OPEN.replace("\"mergeStateStatus\": \"CLEAN\",", "")).expect("parse");
2523 assert_eq!(quiet.blocking, Blocking::Unsaid);
2524 }
2525
2526 #[test]
2527 fn a_red_check_nobody_requires_does_not_buy_a_fix_round() {
2528 let mut nonblocking = pr(Checks::Red, &["editorconfig", "coverage"], 0);
2534 nonblocking.blocking = Blocking::No;
2535 assert_eq!(
2536 decide(&nonblocking, 0, 4, Duration::ZERO),
2537 Step::Merge,
2538 "the forge says nothing is in the way, so nothing is"
2539 );
2540
2541 let mut blocking = pr(Checks::Red, &["test (ubuntu-latest)"], 0);
2543 blocking.blocking = Blocking::Yes;
2544 assert!(matches!(
2545 decide(&blocking, 0, 4, Duration::ZERO),
2546 Step::Fix { .. }
2547 ));
2548
2549 let mut commented = pr(Checks::Red, &["coverage"], 1);
2552 commented.blocking = Blocking::No;
2553 assert!(matches!(
2554 decide(&commented, 0, 4, Duration::ZERO),
2555 Step::Fix { .. }
2556 ));
2557
2558 let mut unsaid = pr(Checks::Red, &["coverage"], 0);
2560 unsaid.blocking = Blocking::Unsaid;
2561 assert!(matches!(
2562 decide(&unsaid, 0, 4, Duration::ZERO),
2563 Step::Fix { .. }
2564 ));
2565 }
2566
2567 #[test]
2568 fn a_branch_the_base_moved_under_is_rebased_not_fixed() {
2569 let mut conflicted = pr(Checks::Green, &[], 0);
2574 conflicted.blocking = Blocking::Conflict;
2575 assert_eq!(decide(&conflicted, 0, 4, Duration::ZERO), Step::Rebase);
2576
2577 let mut red = pr(Checks::Red, &["test (ubuntu-latest)"], 2);
2581 red.blocking = Blocking::Conflict;
2582 assert_eq!(decide(&red, 4, 4, Duration::ZERO), Step::Rebase);
2583
2584 let mut merged = pr(Checks::Red, &[], 0);
2586 merged.blocking = Blocking::Conflict;
2587 merged.state = PrLifecycle::Merged;
2588 assert_eq!(
2589 decide(&merged, 0, 4, Duration::ZERO),
2590 Step::Done { merged: true }
2591 );
2592 }
2593
2594 #[test]
2595 fn the_forge_verdict_is_read_off_merge_state_status() {
2596 for ok in ["CLEAN", "UNSTABLE", "unstable", "HAS_HOOKS"] {
2599 assert_eq!(Blocking::of(ok), Blocking::No, "{ok}");
2600 assert!(!Blocking::of(ok).stops_a_merge(), "{ok}");
2601 }
2602 assert_eq!(Blocking::of("DIRTY"), Blocking::Conflict);
2603 assert_eq!(Blocking::of("BLOCKED"), Blocking::Yes);
2604 assert_eq!(Blocking::of("BEHIND"), Blocking::Yes);
2605 for quiet in ["", "UNKNOWN"] {
2608 assert_eq!(Blocking::of(quiet), Blocking::Unsaid);
2609 assert!(Blocking::of(quiet).stops_a_merge());
2610 }
2611 }
2612
2613 #[test]
2614 fn a_merge_command_that_failed_after_merging_is_still_a_merge() {
2615 let argv = merge_argv(28, "Merge magi run ec12 (candidate B)");
2616 let jj = "could not determine current branch: failed to run git: not on any branch";
2618
2619 let landed = merged_after_all(&argv, jj, Some(PrLifecycle::Merged))
2620 .expect("the forge says merged, so it merged");
2621 assert!(landed.ok);
2622 assert!(
2623 landed.detail.contains("but the pull request is merged"),
2624 "the record must not read as a clean success: {}",
2625 landed.detail
2626 );
2627 assert!(
2628 landed.detail.contains("not on any branch"),
2629 "and it must keep what the command actually said: {}",
2630 landed.detail
2631 );
2632
2633 assert!(merged_after_all(&argv, jj, Some(PrLifecycle::Open)).is_none());
2635 assert!(merged_after_all(&argv, jj, Some(PrLifecycle::Closed)).is_none());
2636 assert!(merged_after_all(&argv, jj, None).is_none());
2638 }
2639
2640 #[test]
2641 fn a_pull_request_closed_underneath_us_is_done_and_not_merged() {
2642 let mut state = pr(Checks::Red, &["editorconfig"], 3);
2643 state.state = PrLifecycle::Closed;
2644 assert_eq!(
2645 decide(&state, 0, 4, Duration::ZERO),
2646 Step::Done { merged: false },
2647 "a human closing the pull request ends the loop, whatever CI says"
2648 );
2649 }
2650
2651 #[test]
2652 fn the_last_round_gives_up_with_a_reason_naming_what_is_still_failing() {
2653 let red = decide(
2654 &pr(Checks::Red, &["editorconfig", "test (macos)"], 0),
2655 4,
2656 4,
2657 Duration::ZERO,
2658 );
2659 match red {
2660 Step::GiveUp { reason } => {
2661 assert!(reason.contains("editorconfig"), "reason: {reason}");
2662 assert!(reason.contains("test (macos)"), "reason: {reason}");
2663 assert!(reason.contains("4 fix round(s)"), "reason: {reason}");
2664 }
2665 other => panic!("expected a give-up, got {other:?}"),
2666 }
2667
2668 let commented = decide(&pr(Checks::Green, &[], 1), 2, 2, Duration::ZERO);
2669 match commented {
2670 Step::GiveUp { reason } => {
2671 assert!(reason.contains("unresolved"), "reason: {reason}");
2672 assert!(reason.contains("2 fix round(s)"), "reason: {reason}");
2673 }
2674 other => panic!("expected a give-up, got {other:?}"),
2675 }
2676 }
2677
2678 #[test]
2679 fn the_merge_command_squashes_deletes_the_branch_and_sets_its_own_subject() {
2680 let candidate_commit = "magi: candidate A (uncommitted work)";
2681 let subject = merge_subject(candidate_commit, "add retries to the uploader");
2682 let argv = merge_argv(16, &subject);
2683
2684 assert!(argv.contains(&"--squash".to_owned()));
2685 assert!(argv.contains(&"--delete-branch".to_owned()));
2686 assert!(argv.contains(&"--subject".to_owned()));
2687 assert_eq!(
2688 argv.last().map(String::as_str),
2689 Some("add retries to the uploader"),
2690 "the subject must not be the candidate commit message"
2691 );
2692 assert_ne!(subject, candidate_commit);
2693 }
2694
2695 #[test]
2696 fn a_real_pull_request_title_is_used_as_the_squash_subject_verbatim() {
2697 assert_eq!(
2698 merge_subject("feat: a queue, an unattended loop, and a phone UI", "task"),
2699 "feat: a queue, an unattended loop, and a phone UI"
2700 );
2701 assert_eq!(
2702 merge_subject("", "# port the retry logic\n\ndetails"),
2703 "port the retry logic",
2704 "an empty title falls back to the task's first line, heading marks stripped"
2705 );
2706 }
2707
2708 #[test]
2709 fn a_failing_checks_details_url_yields_the_job_to_read_logs_from() {
2710 let url = "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323572";
2711 assert_eq!(job_of(url).as_deref(), Some("100114323572"));
2712 assert_eq!(run_of(url).as_deref(), Some("33587406996"));
2713 assert_eq!(job_of("https://coderabbit.ai/status"), None);
2714 assert_eq!(run_of(""), None);
2715 }
2716
2717 #[test]
2718 fn magis_own_stop_comment_is_never_read_back_as_a_finding() {
2719 let mut out = Vec::new();
2720 push_if_outstanding(
2721 &mut out,
2722 ReviewComment {
2723 author: "yukimemi".to_owned(),
2724 path: None,
2725 line: None,
2726 body: format!("{MARKER}\nmagi stopped landing this pull request: 1 check failing"),
2727 },
2728 );
2729 assert!(out.is_empty());
2730 }
2731
2732 fn run_state() -> RunState {
2736 RunState::new(
2737 std::path::PathBuf::from("/repo/magi"),
2738 "main".to_owned(),
2739 "abcdef1234".to_owned(),
2740 "add retries to the uploader".to_owned(),
2741 crate::config::Config::default(),
2742 )
2743 }
2744
2745 fn green_pr() -> PrState {
2746 PrState {
2747 url: "https://github.com/yukimemi/magi/pull/42".to_owned(),
2748 number: 42,
2749 state: PrLifecycle::Open,
2750 checks: Checks::Green,
2751 blocking: Blocking::No,
2753 failing: Vec::new(),
2754 review_comments: vec![ReviewComment {
2755 author: "coderabbitai".to_owned(),
2756 path: Some("src/land.rs".to_owned()),
2757 line: Some(212),
2758 body: "this branch never checks the exit code".to_owned(),
2759 }],
2760 }
2761 }
2762
2763 const NUMSTAT: &str = "12\t3\tsrc/land.rs\n40\t1\tsrc/web.rs\n-\t-\tassets/logo.png";
2764
2765 fn panel() -> String {
2766 approval_panel(
2767 &run_state(),
2768 &green_pr(),
2769 NUMSTAT,
2770 "diff --git a/src/land.rs b/src/land.rs\n@@ -1,2 +1,2 @@\n-old line\n+new line\n context",
2771 &[
2772 "land: ask before merging".to_owned(),
2773 "land: colour the diff".to_owned(),
2774 ],
2775 "feat: merge approval from the phone",
2776 )
2777 }
2778
2779 #[test]
2780 fn the_approval_panel_carries_the_whole_case_for_the_merge() {
2781 let html = panel();
2782 for needle in [
2783 "42",
2784 "main",
2785 "src/land.rs",
2786 "src/web.rs",
2787 "assets/logo.png",
2788 "feat: merge approval from the phone",
2789 "land: ask before merging",
2790 "land: colour the diff",
2791 "coderabbitai",
2792 "this branch never checks the exit code",
2793 "green",
2794 ] {
2795 assert!(html.contains(needle), "the panel must state `{needle}`");
2796 }
2797 }
2798
2799 #[test]
2800 fn the_approval_panel_contains_nothing_the_frames_policy_would_block() {
2801 let html = panel();
2802 assert!(!html.contains("<script"), "no script survives the csp");
2803 assert!(!html.contains("<form"), "form-action is 'none'");
2804 let pr = green_pr();
2805 assert_eq!(
2806 html.matches("http").count(),
2807 html.matches(pr.url.as_str()).count(),
2808 "the only http url in the panel is the pull request's own link"
2809 );
2810 }
2811
2812 #[test]
2813 fn added_and_removed_diff_lines_are_distinguishable_without_colour() {
2814 let html = panel();
2815 assert!(
2816 html.contains(">+</span>"),
2817 "an added line carries a `+` in the gutter, not only a background"
2818 );
2819 assert!(
2820 html.contains(">-</span>"),
2821 "a removed line carries a `-` in the gutter, not only a background"
2822 );
2823 assert!(
2824 html.contains(">new line</span>"),
2825 "the marker is moved to the gutter, so the body is printed once without it"
2826 );
2827 }
2828
2829 #[test]
2830 fn a_diff_past_the_threshold_is_cut_with_an_honest_count() {
2831 let total = DIFF_MAX_LINES + 100;
2832 let diff: String = (0..total).map(|i| format!("+line {i}\n")).collect();
2833 let html = approval_panel(
2834 &run_state(),
2835 &green_pr(),
2836 NUMSTAT,
2837 &diff,
2838 &[],
2839 "feat: something long",
2840 );
2841 assert!(
2842 html.contains(&format!("100 of {total} diff lines omitted")),
2843 "the note must say exactly how much was cut"
2844 );
2845 assert!(html.contains(&format!("line {}", DIFF_MAX_LINES - 1)));
2846 assert!(
2847 !html.contains(&format!("line {DIFF_MAX_LINES}")),
2848 "nothing past the threshold is rendered"
2849 );
2850 assert!(
2851 html.contains("/repo/magi"),
2852 "the note says where the rest is"
2853 );
2854 }
2855
2856 #[test]
2857 fn a_path_with_html_metacharacters_is_escaped_rather_than_rendered() {
2858 let html = approval_panel(
2859 &run_state(),
2860 &green_pr(),
2861 "1\t2\tsrc/<b>&\"x\"'.rs",
2862 "",
2863 &[],
2864 "subject",
2865 );
2866 assert!(html.contains("src/<b>&"x"'.rs"));
2867 assert!(
2868 !html.contains("<b>"),
2869 "an agent-influenced path must never become markup"
2870 );
2871 }
2872
2873 #[test]
2874 fn only_the_merge_choice_merges_and_silence_holds() {
2875 let table = [
2876 (None, Approval::Hold),
2877 (Some("merge"), Approval::Merge),
2878 (Some(" merge\n"), Approval::Merge),
2879 (Some("hold"), Approval::Hold),
2880 (Some(""), Approval::Hold),
2881 (Some("yes"), Approval::Hold),
2882 ];
2883 for (answer, want) in table {
2884 assert_eq!(
2885 approval(answer),
2886 want,
2887 "answer {answer:?} must resolve to {want:?}"
2888 );
2889 }
2890 }
2891
2892 #[test]
2893 fn the_diffstat_table_is_ordered_by_churn_with_binaries_last() {
2894 let rows = parse_numstat(NUMSTAT);
2895 assert_eq!(
2896 rows.iter().map(|r| r.path.as_str()).collect::<Vec<_>>(),
2897 ["src/web.rs", "src/land.rs", "assets/logo.png"]
2898 );
2899 assert_eq!(rows[2].added, None, "a binary file has no line counts");
2900 }
2901 #[test]
2902 fn the_approval_speaks_the_language_the_repository_is_configured_for() {
2903 let mut state = run_state();
2907 state.config.graph.language = "ja".to_owned();
2908 let pr = green_pr();
2909 let commits = ["c1".to_owned()];
2910
2911 let ja = approval_panel(&state, &pr, "3\t1\tsrc/a.rs", "+ x", &commits, "feat: x");
2912 assert!(ja.contains("lang=\"ja\""), "the document must declare it");
2913 assert!(ja.contains("squash されるコミット"), "{ja}");
2914 assert!(ja.contains("レビューコメント"), "{ja}");
2915 assert!(ja.contains("差分"), "{ja}");
2916 assert!(
2917 !ja.contains("Commits being squashed"),
2918 "no English left over"
2919 );
2920
2921 let w = words("ja");
2922 assert!(w.approval_summary(17, "feat: x").contains("マージ"));
2923 assert!(
2924 w.approval_detail("http://x/1", "main", "feat: x")
2925 .contains("パネル")
2926 );
2927
2928 assert!(ja.contains("src/a.rs"), "the diffstat is not prose");
2930 assert!(ja.contains("feat: x"), "nor is the merge subject");
2931
2932 state.config.graph.language = "en".to_owned();
2935 let en = approval_panel(&state, &pr, "3\t1\tsrc/a.rs", "+ x", &commits, "feat: x");
2936 assert!(en.contains("Commits being squashed"), "{en}");
2937 assert_eq!(words("Klingon").html_lang, "en");
2938 }
2939}