1use 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
59pub const POLL: Duration = Duration::from_secs(30);
65
66pub const WAIT_CEILING: Duration = Duration::from_secs(45 * 60);
72
73pub const CHECKS_GRACE: Duration = Duration::from_secs(3 * 60);
85
86const LOG_TAIL: usize = 4_000;
89
90const MAX_LOGS: usize = 3;
93
94pub const MARKER: &str = "<!-- magi:land -->";
100
101const NOT_A_REVIEW: [&str; 3] = [
110 "skip review by coderabbit.ai",
111 "summarize by coderabbit.ai",
112 "<!-- tips_start -->",
113];
114
115#[derive(Debug, Clone, Copy, PartialEq, Eq)]
117pub enum PrLifecycle {
118 Open,
120 Merged,
122 Closed,
124}
125
126#[derive(Debug, Clone, Copy, PartialEq, Eq)]
128pub enum Checks {
129 Pending,
131 Green,
134 Red,
136 Unknown,
138}
139
140impl PrLifecycle {
141 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 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#[derive(Debug, Clone, PartialEq, Eq)]
165pub struct ReviewComment {
166 pub author: String,
168 pub path: Option<String>,
170 pub line: Option<u64>,
172 pub body: String,
174}
175
176#[derive(Debug, Clone, PartialEq, Eq)]
178pub struct PrState {
179 pub url: String,
181 pub number: u64,
183 pub state: PrLifecycle,
185 pub checks: Checks,
187 pub failing: Vec<String>,
189 pub review_comments: Vec<ReviewComment>,
191 pub blocking: Blocking,
193}
194
195#[derive(Debug, Clone, Copy, PartialEq, Eq)]
208pub enum Blocking {
209 No,
211 Yes,
213 Conflict,
215 Unsaid,
219}
220
221impl Blocking {
222 fn of(raw: &str) -> Self {
224 match raw.to_ascii_uppercase().as_str() {
225 "CLEAN" | "UNSTABLE" | "HAS_HOOKS" => Self::No,
228 "DIRTY" => Self::Conflict,
229 "" | "UNKNOWN" => Self::Unsaid,
230 _ => Self::Yes,
232 }
233 }
234
235 #[must_use]
237 pub fn stops_a_merge(self) -> bool {
238 !matches!(self, Self::No)
239 }
240}
241
242#[derive(Debug, Clone, PartialEq, Eq)]
244pub enum Step {
245 Wait,
247 Rebase,
255 Fix {
257 reason: String,
259 },
260 Merge,
262 Done {
264 merged: bool,
266 },
267 GiveUp {
269 reason: String,
271 },
272}
273
274pub(crate) fn merged_after_all(
296 argv: &[String],
297 stderr: &str,
298 after: Option<PrLifecycle>,
299) -> Option<MergeOutcome> {
300 if after? != PrLifecycle::Merged {
301 return None;
302 }
303 Some(MergeOutcome {
304 mode: MergeMode::Pr,
305 ok: true,
306 detail: format!(
307 "gh {} (the command reported `{}`, but the pull request is merged)",
308 argv.join(" "),
309 stderr.trim()
310 ),
311 })
312}
313
314pub fn decide(pr: &PrState, round: usize, budget: usize, waited: Duration) -> Step {
334 match pr.state {
335 PrLifecycle::Merged => return Step::Done { merged: true },
336 PrLifecycle::Closed => return Step::Done { merged: false },
337 PrLifecycle::Open => {}
338 }
339
340 if pr.blocking == Blocking::Conflict {
343 return Step::Rebase;
344 }
345
346 let spent = round >= budget;
347 match pr.checks {
348 Checks::Pending => Step::Wait,
349 Checks::Unknown if waited < CHECKS_GRACE => Step::Wait,
350 Checks::Unknown => Step::GiveUp {
351 reason: format!(
352 "no check status is readable on the pull request after {} minute(s); \
353 refusing to merge on a guess",
354 CHECKS_GRACE.as_secs() / 60
355 ),
356 },
357 Checks::Red if !pr.blocking.stops_a_merge() && pr.review_comments.is_empty() => Step::Merge,
364 Checks::Red => {
365 let what = format!(
366 "{} check(s) failing: {}",
367 pr.failing.len(),
368 pr.failing.join(", ")
369 );
370 if spent {
371 Step::GiveUp {
372 reason: format!("{what} — still red after {budget} fix round(s)"),
373 }
374 } else {
375 Step::Fix { reason: what }
376 }
377 }
378 Checks::Green if pr.review_comments.is_empty() => Step::Merge,
379 Checks::Green => {
380 let what = format!(
381 "checks are green but {} review comment(s) are unresolved: {}",
382 pr.review_comments.len(),
383 authors(&pr.review_comments)
384 );
385 if spent {
386 Step::GiveUp {
387 reason: format!("{what} — still unresolved after {budget} fix round(s)"),
388 }
389 } else {
390 Step::Fix { reason: what }
391 }
392 }
393 }
394}
395
396fn authors(comments: &[ReviewComment]) -> String {
398 let mut seen: Vec<&str> = Vec::new();
399 for c in comments {
400 if !seen.contains(&c.author.as_str()) {
401 seen.push(&c.author);
402 }
403 }
404 seen.join(", ")
405}
406
407pub fn merge_argv(number: u64, subject: &str) -> Vec<String> {
411 vec![
412 "pr".to_owned(),
413 "merge".to_owned(),
414 number.to_string(),
415 "--squash".to_owned(),
416 "--delete-branch".to_owned(),
417 "--subject".to_owned(),
418 subject.to_owned(),
419 ]
420}
421
422pub fn merge_subject(pr_title: &str, instruction: &str) -> String {
429 let title = pr_title.trim();
430 if !title.is_empty() && !title.starts_with("magi: candidate") {
431 return title.to_owned();
432 }
433 let first = instruction
434 .lines()
435 .map(str::trim)
436 .find(|l| !l.is_empty())
437 .unwrap_or("magi: land the winning candidate");
438 first.trim_start_matches(['#', ' ']).to_owned()
439}
440
441pub const APPROVE: &str = "merge";
443
444pub const HOLD: &str = "hold";
446
447pub const APPROVAL_NODE: &str = "land-approval";
453
454pub const DIFF_MAX_LINES: usize = 400;
462
463#[derive(Debug, Clone, Copy, PartialEq, Eq)]
465pub enum Approval {
466 Merge,
468 Hold,
470}
471
472pub fn approval(answer: Option<&str>) -> Approval {
480 match answer {
481 Some(a) if a.trim().eq_ignore_ascii_case(APPROVE) => Approval::Merge,
482 _ => Approval::Hold,
483 }
484}
485
486#[derive(Debug, Clone, Copy, PartialEq, Eq)]
488enum ApprovalGate {
489 Approved,
491 Held,
494 Pending,
496}
497
498fn esc(s: &str) -> String {
508 let mut out = String::with_capacity(s.len());
509 for c in s.chars() {
510 match c {
511 '&' => out.push_str("&"),
512 '<' => out.push_str("<"),
513 '>' => out.push_str(">"),
514 '"' => out.push_str("""),
515 '\'' => out.push_str("'"),
516 _ => out.push(c),
517 }
518 }
519 out
520}
521
522#[derive(Debug, Clone, PartialEq, Eq)]
524struct StatRow {
525 path: String,
526 added: Option<u64>,
528 removed: Option<u64>,
529}
530
531impl StatRow {
532 fn churn(&self) -> u64 {
535 self.added.unwrap_or(0) + self.removed.unwrap_or(0)
536 }
537}
538
539fn parse_numstat(numstat: &str) -> Vec<StatRow> {
545 let mut rows: Vec<StatRow> = numstat
546 .lines()
547 .filter_map(|line| {
548 let mut parts = line.splitn(3, '\t');
549 let added = parts.next()?.trim();
550 let removed = parts.next()?.trim();
551 let path = parts.next()?.trim();
552 if path.is_empty() {
553 return None;
554 }
555 Some(StatRow {
556 path: path.to_owned(),
557 added: added.parse().ok(),
558 removed: removed.parse().ok(),
559 })
560 })
561 .collect();
562 rows.sort_by(|a, b| b.churn().cmp(&a.churn()).then_with(|| a.path.cmp(&b.path)));
565 rows
566}
567
568fn diff_row(line: &str) -> (&'static str, &'static str, &str) {
577 if line.starts_with("+++") || line.starts_with("---") {
578 (" ", "color:#57606a;font-weight:600", line)
579 } else if let Some(body) = line.strip_prefix('+') {
580 ("+", "background:#e6ffec;color:#0a3622", body)
581 } else if let Some(body) = line.strip_prefix('-') {
582 ("-", "background:#ffebe9;color:#5c1a17", body)
583 } else if line.starts_with("@@") {
584 ("~", "background:#eef2ff;color:#3730a3", line)
585 } else if let Some(body) = line.strip_prefix(' ') {
586 (" ", "", body)
587 } else {
588 (" ", "color:#57606a;font-weight:600", line)
589 }
590}
591
592struct Words {
601 html_lang: &'static str,
602 task: &'static str,
603 what_changed: &'static str,
604 review_verdict: &'static str,
605 reviewer: &'static str,
606 reviewer_no_answer: &'static str,
607 checks: &'static str,
608 nothing_failing: &'static str,
609 files_changed: &'static str,
610 commits: &'static str,
611 no_commits: &'static str,
612 comments: &'static str,
613 no_comments: &'static str,
614 diff: &'static str,
615 truncated: &'static str,
616 lands_as: &'static str,
617}
618
619const EN: Words = Words {
620 html_lang: "en",
621 task: "Task",
622 what_changed: "What changed",
623 review_verdict: "Review verdict",
624 reviewer: "Reviewer",
625 reviewer_no_answer: "produced no answer",
626 checks: "Checks",
627 nothing_failing: "Nothing failing.",
628 files_changed: "file(s) changed",
629 commits: "Commits being squashed",
630 no_commits: "No commit subjects could be read from the branch.",
631 comments: "Review comments",
632 no_comments: "Nothing outstanding at this observation.",
633 diff: "Diff",
634 truncated: "Truncated",
635 lands_as: "They land as one commit titled",
636};
637
638const JA: Words = Words {
639 html_lang: "ja",
640 task: "タスク",
641 what_changed: "変更内容",
642 review_verdict: "レビューの結論",
643 reviewer: "レビュアー",
644 reviewer_no_answer: "回答なし",
645 checks: "チェック",
646 nothing_failing: "失敗しているものはありません。",
647 files_changed: "ファイル変更",
648 commits: "squash されるコミット",
649 no_commits: "ブランチからコミット件名を読めませんでした。",
650 comments: "レビューコメント",
651 no_comments: "この時点で未対応のものはありません。",
652 diff: "差分",
653 truncated: "省略",
654 lands_as: "これらは次の件名の1コミットとして入ります:",
655};
656
657impl Words {
658 fn lands_as_tail(&self) -> &'static str {
662 if self.html_lang == "ja" {
663 "。この件名も承認の対象です。"
664 } else {
665 ", which you are approving too."
666 }
667 }
668
669 fn approval_summary(&self, number: u64, subject: &str) -> String {
671 if self.html_lang == "ja" {
672 format!("プルリクエスト #{number} をマージ: {subject}")
673 } else {
674 format!("merge pull request #{number}: {subject}")
675 }
676 }
677
678 fn approval_detail(&self, url: &str, base: &str, subject: &str) -> String {
680 if self.html_lang == "ja" {
681 format!(
682 "{url} はチェックが緑で、`{base}` へ `{subject}` として squash \
683 できる状態です。差分の要約・パッチ・squash されるコミットは\
684 下のパネルにあります。"
685 )
686 } else {
687 format!(
688 "{url} is green and ready to squash into `{base}` as `{subject}`. \
689 The panel holds the diffstat, the patch and the commits being squashed."
690 )
691 }
692 }
693
694 fn truncated_note(
696 &self,
697 omitted: usize,
698 total: usize,
699 shown: usize,
700 where_: &str,
701 base: &str,
702 head: &str,
703 ) -> String {
704 if self.html_lang == "ja" {
705 format!(
706 "先頭 {shown} 行のあと、差分 {total} 行のうち {omitted} 行を省略しました。\
707 全体は <code>{where_}</code>(<code>git diff {base}...{head}</code>)と\
708 プルリクエストにあります。"
709 )
710 } else {
711 format!(
712 "{omitted} of {total} diff lines omitted after the first {shown}. \
713 The whole patch is in <code>{where_}</code> \
714 (<code>git diff {base}...{head}</code>) and on the pull request."
715 )
716 }
717 }
718}
719
720fn words(language: &str) -> &'static Words {
723 let l = language.trim();
724 if l.eq_ignore_ascii_case("ja")
725 || l.eq_ignore_ascii_case("jp")
726 || l.eq_ignore_ascii_case("japanese")
727 || l.eq_ignore_ascii_case("日本語")
728 {
729 &JA
730 } else {
731 &EN
732 }
733}
734
735pub fn approval_panel(
747 state: &RunState,
748 pr: &PrState,
749 diffstat: &str,
750 diff: &str,
751 commits: &[String],
752 subject: &str,
753) -> String {
754 let rows = parse_numstat(diffstat);
755 let w = words(&state.config.graph.language);
756 let mut h = String::with_capacity(4_096 + diff.len().min(200_000));
757
758 let _ = writeln!(
759 h,
760 "<!doctype html>\n<html lang=\"{}\">\n<head>\n<meta charset=\"utf-8\">\n\
761 <meta name=\"viewport\" content=\"width=device-width, initial-scale=1\">",
762 w.html_lang
763 );
764 let _ = writeln!(
765 h,
766 "<title>merge #{} — {}</title>\n</head>",
767 pr.number,
768 esc(subject)
769 );
770 h.push_str(
771 "<body style=\"margin:0;padding:12px;font:15px/1.5 -apple-system,\
772 'Segoe UI',system-ui,sans-serif;color:#1f2328;background:#fff;\
773 word-break:break-word\">\n",
774 );
775
776 let _ = writeln!(
778 h,
779 "<h1 style=\"margin:0 0 4px;font-size:19px\">Merge #{} into \
780 <code style=\"background:#f6f8fa;padding:1px 4px;border-radius:4px\">{}</code></h1>\n\
781 <p style=\"margin:0 0 4px;font-size:17px;font-weight:600\">{}</p>\n\
782 <p style=\"margin:0 0 12px;font-size:13px;color:#57606a\">squash merge · run {} · \
783 <a href=\"{}\" style=\"color:#0969da\">{}</a></p>",
784 pr.number,
785 esc(&state.base_branch),
786 esc(subject),
787 esc(&state.id),
788 esc(&pr.url),
789 esc(&pr.url),
790 );
791
792 let _ = writeln!(
795 h,
796 "<h2 style=\"margin:16px 0 6px;font-size:15px\">{}</h2>\n\
797 <p style=\"margin:0;font-size:13px;white-space:pre-wrap\">{}</p>",
798 w.task,
799 esc(&state.instruction)
800 );
801
802 if let Some(summary) = state
804 .winner()
805 .map(|c| c.summary.as_str())
806 .filter(|s| !s.is_empty())
807 {
808 let _ = writeln!(
809 h,
810 "<h2 style=\"margin:16px 0 6px;font-size:15px\">{}</h2>\n\
811 <p style=\"margin:0;font-size:13px;white-space:pre-wrap\">{}</p>",
812 w.what_changed,
813 esc(summary)
814 );
815 }
816
817 if let Some(round) = state.reviews.last() {
820 let _ = writeln!(
821 h,
822 "<h2 style=\"margin:16px 0 6px;font-size:15px\">{}</h2>",
823 w.review_verdict
824 );
825 for r in &round.reviews {
826 let body = match &r.failed {
833 Some(reason) => format!("{}: {}", w.reviewer_no_answer, esc(reason)),
834 None => esc(&r.summary),
835 };
836 let _ = writeln!(
837 h,
838 "<div style=\"margin:0 0 8px;padding:8px;background:#f6f8fa;\
839 border-radius:6px\">\
840 <div style=\"font-size:12px;color:#57606a\">{} {} · {}</div>\
841 <div style=\"white-space:pre-wrap;font-size:13px\">{}</div></div>",
842 w.reviewer,
843 r.reviewer,
844 esc(&r.agent),
845 body,
846 );
847 }
848 }
849
850 let _ = writeln!(
851 h,
852 "<h2 style=\"margin:16px 0 6px;font-size:15px\">{}: {}</h2>",
853 w.checks,
854 esc(pr.checks.as_str())
855 );
856 if pr.failing.is_empty() {
857 let _ = writeln!(
858 h,
859 "<p style=\"margin:0;font-size:13px;color:#57606a\">{}</p>",
860 w.nothing_failing
861 );
862 } else {
863 h.push_str("<ul style=\"margin:0;padding-left:20px;font-size:13px\">\n");
864 for f in &pr.failing {
865 let _ = writeln!(h, "<li>{}</li>", esc(f));
866 }
867 h.push_str("</ul>\n");
868 }
869
870 let _ = writeln!(
873 h,
874 "<h2 style=\"margin:16px 0 6px;font-size:15px\">{} {}</h2>",
875 rows.len(),
876 w.files_changed
877 );
878 h.push_str(
879 "<table style=\"width:100%;border-collapse:collapse;font-size:13px\">\n\
880 <thead><tr>\
881 <th style=\"text-align:left;border-bottom:1px solid #d0d7de;padding:4px 2px\">file</th>\
882 <th style=\"text-align:right;border-bottom:1px solid #d0d7de;padding:4px 2px\">added</th>\
883 <th style=\"text-align:right;border-bottom:1px solid #d0d7de;padding:4px 2px\">removed\
884 </th></tr></thead>\n<tbody>\n",
885 );
886 let mut total_added = 0u64;
887 let mut total_removed = 0u64;
888 for r in &rows {
889 total_added += r.added.unwrap_or(0);
890 total_removed += r.removed.unwrap_or(0);
891 let cell = |n: Option<u64>| match n {
892 Some(n) => n.to_string(),
893 None => "bin".to_owned(),
894 };
895 let _ = writeln!(
896 h,
897 "<tr>\
898 <td style=\"padding:4px 2px;border-bottom:1px solid #eaeef2;\
899 font-family:ui-monospace,monospace\">{}</td>\
900 <td style=\"padding:4px 2px;border-bottom:1px solid #eaeef2;text-align:right;\
901 color:#0a3622\">{}</td>\
902 <td style=\"padding:4px 2px;border-bottom:1px solid #eaeef2;text-align:right;\
903 color:#5c1a17\">{}</td></tr>",
904 esc(&r.path),
905 cell(r.added),
906 cell(r.removed),
907 );
908 }
909 let _ = writeln!(
910 h,
911 "</tbody>\n<tfoot><tr style=\"font-weight:600\">\
912 <td style=\"padding:4px 2px\">total</td>\
913 <td style=\"padding:4px 2px;text-align:right\">{total_added}</td>\
914 <td style=\"padding:4px 2px;text-align:right\">{total_removed}</td>\
915 </tr></tfoot>\n</table>"
916 );
917
918 let _ = writeln!(
920 h,
921 "<h2 style=\"margin:16px 0 6px;font-size:15px\">{}</h2>",
922 w.commits
923 );
924 if commits.is_empty() {
925 h.push_str(&format!(
926 "<p style=\"margin:0;font-size:13px;color:#57606a\">{}</p>\n",
927 w.no_commits
928 ));
929 } else {
930 h.push_str("<ol style=\"margin:0;padding-left:20px;font-size:13px\">\n");
931 for c in commits {
932 let _ = writeln!(h, "<li>{}</li>", esc(c));
933 }
934 h.push_str("</ol>\n");
935 }
936 let _ = writeln!(
937 h,
938 "<p style=\"margin:8px 0 0;font-size:13px\">{} <strong>{}</strong>{}</p>",
939 w.lands_as,
940 esc(subject),
941 w.lands_as_tail()
942 );
943
944 let _ = writeln!(
946 h,
947 "<h2 style=\"margin:16px 0 6px;font-size:15px\">{}</h2>",
948 w.comments
949 );
950 if pr.review_comments.is_empty() {
951 h.push_str(&format!(
952 "<p style=\"margin:0;font-size:13px;color:#57606a\">{}</p>\n",
953 w.no_comments
954 ));
955 } else {
956 for c in &pr.review_comments {
957 let anchor = match (&c.path, c.line) {
958 (Some(p), Some(l)) => format!("{p}:{l}"),
959 (Some(p), None) => p.clone(),
960 _ => "pull request thread".to_owned(),
961 };
962 let _ = writeln!(
963 h,
964 "<div style=\"margin:0 0 8px;padding:8px;background:#f6f8fa;border-radius:6px\">\
965 <div style=\"font-size:12px;color:#57606a\">{} · {}</div>\
966 <div style=\"white-space:pre-wrap;font-size:13px\">{}</div></div>",
967 esc(&c.author),
968 esc(&anchor),
969 esc(&tail(&c.body, 800)),
970 );
971 }
972 }
973
974 let total = diff.lines().count();
976 let shown = total.min(DIFF_MAX_LINES);
977 let _ = writeln!(
978 h,
979 "<h2 style=\"margin:16px 0 6px;font-size:15px\">{}</h2>",
980 w.diff
981 );
982 h.push_str(
983 "<div style=\"font:12px/1.45 ui-monospace,SFMono-Regular,Menlo,monospace;\
984 border:1px solid #d0d7de;border-radius:6px;overflow-x:auto\">\n",
985 );
986 for line in diff.lines().take(shown) {
987 let (gutter, style, body) = diff_row(line);
988 let _ = writeln!(
989 h,
990 "<div style=\"display:flex;{style}\">\
991 <span style=\"flex:0 0 1.4em;text-align:center;user-select:none;\
992 border-right:1px solid #d0d7de\">{gutter}</span>\
993 <span style=\"white-space:pre;padding-left:6px\">{}</span></div>",
994 esc(body),
995 );
996 }
997 h.push_str("</div>\n");
998 if total > shown {
999 let omitted = total - shown;
1000 let head = state.winner().map_or("HEAD", |w| w.branch.as_str());
1001 let where_ = state.winner().map_or_else(
1002 || state.repo.display().to_string(),
1003 |w| w.worktree.display().to_string(),
1004 );
1005 let _ = writeln!(
1006 h,
1007 "<p style=\"margin:8px 0 0;padding:8px;background:#fff8c5;border-radius:6px;\
1008 font-size:13px\">{}: {}</p>",
1009 w.truncated,
1010 w.truncated_note(
1011 omitted,
1012 total,
1013 shown,
1014 &esc(&where_),
1015 &esc(&state.base_branch),
1016 &esc(head),
1017 ),
1018 );
1019 }
1020
1021 h.push_str("</body>\n</html>\n");
1022 h
1023}
1024
1025async fn approval_gate(state: &mut RunState, pr: &PrState, subject: &str) -> Result<ApprovalGate> {
1045 let store = ask::Questions::open();
1046 let existing = store
1047 .list()
1048 .into_iter()
1049 .filter(|q| q.run == state.id && q.node == APPROVAL_NODE)
1050 .max_by(|a, b| a.id.cmp(&b.id));
1051
1052 let q = match existing {
1053 Some(q) => q,
1054 None => {
1055 let (worktree, head) = match state.winner() {
1056 Some(w) => (w.worktree.clone(), w.branch.clone()),
1057 None => (state.repo.clone(), "HEAD".to_owned()),
1058 };
1059 let base = state.base_branch.clone();
1060 let range = format!("{base}...{head}");
1061 let numstat = git::git_raw(&worktree, &["diff", "--numstat", "-M", &range])
1065 .await
1066 .map(|o| o.stdout)
1067 .unwrap_or_default();
1068 let diff = git::diff(&worktree, &base, &head).await.unwrap_or_default();
1069 let commits: Vec<String> = git::git_raw(
1070 &worktree,
1071 &[
1072 "log",
1073 "--reverse",
1074 "--format=%s",
1075 &format!("{base}..{head}"),
1076 ],
1077 )
1078 .await
1079 .map(|o| o.stdout)
1080 .unwrap_or_default()
1081 .lines()
1082 .filter(|l| !l.trim().is_empty())
1083 .map(str::to_owned)
1084 .collect();
1085
1086 let w = words(&state.config.graph.language);
1087 let html = approval_panel(state, pr, &numstat, &diff, &commits, subject);
1088 let mut fresh = ask::Question::new(
1089 state.id.clone(),
1090 APPROVAL_NODE.to_owned(),
1091 "land".to_owned(),
1092 w.approval_summary(pr.number, subject),
1093 w.approval_detail(&pr.url, &base, subject),
1094 vec![APPROVE.to_owned(), HOLD.to_owned()],
1095 );
1096 store
1097 .put_panel(&mut fresh, &html, &[])
1098 .context("write the merge approval panel")?;
1099 store
1100 .put(&mut fresh)
1101 .context("file the merge approval question")?;
1102 state.event(
1103 "land",
1104 format!("asking for merge approval ({})", fresh.short()),
1105 );
1106 state.save()?;
1107 if let Err(e) = ask::notify(&state.config.notify, &fresh).await {
1108 tracing::warn!(
1112 "could not notify about merge approval question {}: {e:#} - \
1113 the web UI is the only surface for it now",
1114 fresh.short()
1115 );
1116 }
1117 fresh
1118 }
1119 };
1120
1121 Ok(match q.status {
1122 ask::QuestionStatus::Open => ApprovalGate::Pending,
1123 ask::QuestionStatus::Abandoned => ApprovalGate::Held,
1127 ask::QuestionStatus::Answered => match approval(q.resolution().as_deref()) {
1131 Approval::Merge => ApprovalGate::Approved,
1132 Approval::Hold => ApprovalGate::Held,
1133 },
1134 })
1135}
1136
1137pub fn parse_pr(json: &str) -> Result<PrState> {
1140 let raw: GhPr = serde_json::from_str(json).context("parse `gh pr view --json ...` output")?;
1141 let state = match raw.state.to_ascii_uppercase().as_str() {
1142 "OPEN" => PrLifecycle::Open,
1143 "MERGED" => PrLifecycle::Merged,
1144 "CLOSED" => PrLifecycle::Closed,
1145 other => bail!("unknown pull request state `{other}`"),
1146 };
1147
1148 let mut failing = Vec::new();
1149 let mut pending = false;
1150 let mut unknown = false;
1151 for check in &raw.status_check_rollup {
1152 match check.verdict() {
1153 Verdict::Pass => {}
1154 Verdict::Pending => pending = true,
1155 Verdict::Fail => failing.push(check.label()),
1156 Verdict::Unknown => unknown = true,
1157 }
1158 }
1159 let checks = if raw.status_check_rollup.is_empty() {
1160 Checks::Unknown
1161 } else if pending {
1162 Checks::Pending
1163 } else if !failing.is_empty() {
1164 Checks::Red
1165 } else if unknown {
1166 Checks::Unknown
1167 } else {
1168 Checks::Green
1169 };
1170
1171 let mut review_comments = Vec::new();
1172 for r in raw.reviews {
1173 push_if_outstanding(
1174 &mut review_comments,
1175 ReviewComment {
1176 author: r.author.login,
1177 path: None,
1178 line: None,
1179 body: r.body,
1180 },
1181 );
1182 }
1183 for c in raw.comments {
1184 push_if_outstanding(
1185 &mut review_comments,
1186 ReviewComment {
1187 author: c.author.login,
1188 path: None,
1189 line: None,
1190 body: c.body,
1191 },
1192 );
1193 }
1194
1195 Ok(PrState {
1196 url: raw.url,
1197 number: raw.number,
1198 state,
1199 checks,
1200 failing,
1201 review_comments,
1202 blocking: Blocking::of(&raw.merge_state_status),
1203 })
1204}
1205
1206pub async fn lifecycle(repo: &Path, pr_url: &str) -> Result<PrLifecycle> {
1216 let view = gh(
1217 repo,
1218 &[
1219 "pr".to_owned(),
1220 "view".to_owned(),
1221 pr_url.to_owned(),
1222 "--json".to_owned(),
1223 "state".to_owned(),
1224 ],
1225 )
1226 .await?;
1227 if !view.0 {
1228 bail!("gh pr view {pr_url}: {}", view.1);
1229 }
1230 Ok(parse_pr(&view.1)?.state)
1234}
1235
1236#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
1240pub struct ExternalMerge {
1241 pub url: String,
1243 pub number: u64,
1245}
1246
1247#[derive(Debug, Deserialize)]
1248#[serde(rename_all = "camelCase")]
1249struct GhMergedPr {
1250 url: String,
1251 number: u64,
1252 merged_at: String,
1253 base_ref_name: String,
1254}
1255
1256fn pick_merged_pr(
1270 json: &str,
1271 base_branch: &str,
1272 created_at: Timestamp,
1273) -> Result<Option<ExternalMerge>> {
1274 let raw: Vec<GhMergedPr> =
1275 serde_json::from_str(json).context("parse `gh pr list ... --json ...` output")?;
1276 let mut matches: Vec<ExternalMerge> = Vec::new();
1277 for pr in raw {
1278 if pr.base_ref_name != base_branch {
1279 continue;
1280 }
1281 let Ok(merged_at) = pr.merged_at.parse::<Timestamp>() else {
1282 continue;
1283 };
1284 if merged_at < created_at {
1285 continue;
1286 }
1287 matches.push(ExternalMerge {
1288 url: pr.url,
1289 number: pr.number,
1290 });
1291 }
1292 if matches.len() == 1 {
1293 Ok(matches.pop())
1294 } else {
1295 Ok(None)
1296 }
1297}
1298
1299pub async fn find_external_merge(state: &RunState) -> Result<Option<ExternalMerge>> {
1312 let Some(winner) = state.winner() else {
1313 return Ok(None);
1314 };
1315 let branch = winner.branch.clone();
1316 let out = gh(
1317 &state.repo,
1318 &[
1319 "pr".to_owned(),
1320 "list".to_owned(),
1321 "--head".to_owned(),
1322 branch.clone(),
1323 "--state".to_owned(),
1324 "merged".to_owned(),
1325 "--json".to_owned(),
1326 "url,number,mergedAt,baseRefName".to_owned(),
1327 ],
1328 )
1329 .await?;
1330 if !out.0 {
1331 bail!("gh pr list --head {branch}: {}", out.1);
1332 }
1333 pick_merged_pr(&out.1, &state.base_branch, state.created_at)
1334}
1335
1336pub async fn branch_is_ancestor(repo: &Path, branch: &str, base_branch: &str) -> Result<bool> {
1348 let out = tokio::process::Command::new("git")
1349 .args(["merge-base", "--is-ancestor", branch, base_branch])
1350 .current_dir(repo)
1351 .quiet()
1352 .stdin(std::process::Stdio::null())
1353 .output()
1354 .await
1355 .context("spawn git merge-base --is-ancestor")?;
1356 Ok(out.status.success())
1357}
1358
1359fn forge_slug(url: &str) -> Option<(String, &str)> {
1367 let rest = url.rsplit("://").next()?;
1368 let (host, path) = rest.split_once('/')?;
1369 if host.is_empty() {
1370 return None;
1371 }
1372 Some((host.to_ascii_lowercase(), path))
1373}
1374
1375pub(crate) fn slug_of_pr_url(url: &str) -> Option<String> {
1383 let (host, path) = forge_slug(url)?;
1384 let mut segments = path.split('/');
1385 let owner = segments.next()?;
1386 let repo = segments.next()?;
1387 let kind = segments.next()?;
1388 if owner.is_empty() || repo.is_empty() || kind != "pull" {
1389 return None;
1390 }
1391 Some(format!("{host}/{owner}/{repo}"))
1392}
1393
1394fn slug_of_repo_url(url: &str) -> Option<String> {
1398 let (host, path) = forge_slug(url)?;
1399 let mut segments = path.split('/');
1400 let owner = segments.next()?;
1401 let repo = segments.next()?;
1402 if owner.is_empty() || repo.is_empty() {
1403 return None;
1404 }
1405 Some(format!("{host}/{owner}/{repo}"))
1406}
1407
1408pub(crate) fn ensure_same_repo(run_repo_slug: &str, pr_repo_slug: &str) -> Result<()> {
1429 if run_repo_slug.eq_ignore_ascii_case(pr_repo_slug) {
1430 return Ok(());
1431 }
1432 bail!(
1433 "refusing to correct this run: it is recorded against {run_repo_slug}, but the pull \
1434 request URL belongs to {pr_repo_slug} - pass the run id whose repository the URL \
1435 actually belongs to (or, if `origin` is a fork opened against a different upstream, \
1436 verify by hand before treating this as a false positive)"
1437 );
1438}
1439
1440async fn repo_slug(repo: &Path) -> Result<String> {
1451 let out = gh(
1452 repo,
1453 &[
1454 "repo".to_owned(),
1455 "view".to_owned(),
1456 "--json".to_owned(),
1457 "url".to_owned(),
1458 ],
1459 )
1460 .await?;
1461 if !out.0 {
1462 bail!("gh repo view --json url: {}", out.1);
1463 }
1464 #[derive(Debug, Deserialize)]
1465 struct GhRepo {
1466 url: String,
1467 }
1468 let parsed: GhRepo = serde_json::from_str(&out.1)
1469 .with_context(|| format!("parse `gh repo view` output: {}", out.1))?;
1470 slug_of_repo_url(&parsed.url)
1471 .with_context(|| format!("could not parse a host/owner/repo out of {}", parsed.url))
1472}
1473
1474pub async fn correct_manual_merge(
1521 state: &mut RunState,
1522 url: &str,
1523) -> Result<(RunStatus, RunStatus)> {
1524 let Some(pr_slug) = slug_of_pr_url(url) else {
1525 bail!(
1526 "could not parse an owner/repo out of {url}; refusing to guess which repository \
1527 this pull request belongs to"
1528 );
1529 };
1530 let run_slug = repo_slug(&state.repo).await?;
1531 ensure_same_repo(&run_slug, &pr_slug)?;
1532 correct_merge(state, url).await
1533}
1534
1535pub(crate) async fn correct_confirmed_external_merge(
1546 state: &mut RunState,
1547 url: &str,
1548) -> Result<(RunStatus, RunStatus)> {
1549 correct_merge(state, url).await
1550}
1551
1552async fn correct_merge(state: &mut RunState, url: &str) -> Result<(RunStatus, RunStatus)> {
1553 match lifecycle(&state.repo, url).await? {
1554 PrLifecycle::Merged => {}
1555 other => bail!(
1556 "{url} is {}, not merged; refusing to record {} as merged on a guess",
1557 other.as_str(),
1558 state.id
1559 ),
1560 }
1561 let before = state.status;
1562 if let Err(e) = land(state, url).await {
1563 state.status = RunStatus::Blocked;
1570 state.event("fold", format!("manual-merge correction failed: {e:#}"));
1571 state.save()?;
1572 return Err(e).context(format!("confirming the merge of {url}"));
1573 }
1574 state.event(
1575 "fold",
1576 "operator recorded this pull request as a manual merge; this run never \
1577 re-entered `land`, so `bump::after_merge` did not run for it - a release \
1578 bump this change might warrant has to be filed by hand",
1579 );
1580 state.save()?;
1581 Ok((before, state.status))
1582}
1583
1584pub fn parse_inline_comments(json: &str) -> Result<Vec<ReviewComment>> {
1591 let raw: Vec<GhInline> =
1592 serde_json::from_str(json).context("parse `gh api .../pulls/<n>/comments` output")?;
1593 let mut out = Vec::new();
1594 for c in raw {
1595 push_if_outstanding(
1596 &mut out,
1597 ReviewComment {
1598 author: c.user.login,
1599 path: c.path,
1600 line: c.line,
1601 body: c.body,
1602 },
1603 );
1604 }
1605 Ok(out)
1606}
1607
1608fn push_if_outstanding(out: &mut Vec<ReviewComment>, comment: ReviewComment) {
1614 if comment.body.trim().is_empty() || comment.body.contains(MARKER) {
1615 return;
1616 }
1617 if comment.path.is_none() && is_noise(&comment.body) {
1618 return;
1619 }
1620 out.push(comment);
1621}
1622
1623pub fn is_noise(body: &str) -> bool {
1641 if NOT_A_REVIEW.iter().any(|m| body.contains(m)) {
1642 return true;
1643 }
1644 let mut content = false;
1645 for line in strip_blocks(body).lines() {
1646 let line = unquote(line);
1647 if line.is_empty() || is_checklist(line) || is_decoration(line) || is_banner(line) {
1648 continue;
1649 }
1650 content = true;
1651 break;
1652 }
1653 !content
1654}
1655
1656fn strip_blocks(body: &str) -> String {
1658 let mut out = String::with_capacity(body.len());
1659 let mut rest = body;
1660 loop {
1661 let open = ["<!--", "<details>"]
1662 .iter()
1663 .filter_map(|tag| rest.find(tag).map(|i| (i, *tag)))
1664 .min_by_key(|(i, _)| *i);
1665 let Some((at, tag)) = open else {
1666 out.push_str(rest);
1667 return out;
1668 };
1669 out.push_str(&rest[..at]);
1670 let after = &rest[at + tag.len()..];
1671 let close = if tag == "<!--" { "-->" } else { "</details>" };
1672 match after.find(close) {
1673 Some(end) => rest = &after[end + close.len()..],
1674 None => return out,
1676 }
1677 }
1678}
1679
1680fn unquote(line: &str) -> &str {
1682 let mut s = line.trim();
1683 while let Some(rest) = s.strip_prefix('>') {
1684 s = rest.trim_start();
1685 }
1686 s.trim()
1687}
1688
1689fn is_checklist(line: &str) -> bool {
1691 let rest = line
1692 .strip_prefix("- ")
1693 .or_else(|| line.strip_prefix("* "))
1694 .unwrap_or("");
1695 let rest = rest.trim_start();
1696 matches!(
1697 rest.get(..3),
1698 Some("[ ]") | Some("[x]") | Some("[X]") | Some("[*]")
1699 )
1700}
1701
1702fn is_decoration(line: &str) -> bool {
1704 line.starts_with('#')
1705 || line.starts_with("[!")
1706 || (line.len() >= 3 && line.chars().all(|c| matches!(c, '-' | '=' | '*' | '_')))
1707}
1708
1709fn is_banner(line: &str) -> bool {
1716 let plain = drop_spans(line, "**", "**");
1717 let plain = if plain.contains("](") {
1718 drop_spans(&plain, "[", ")")
1719 } else {
1720 plain
1721 };
1722 !plain.chars().any(char::is_alphanumeric)
1723}
1724
1725fn drop_spans(s: &str, open: &str, close: &str) -> String {
1729 let mut out = String::with_capacity(s.len());
1730 let mut rest = s;
1731 while let Some(at) = rest.find(open) {
1732 out.push_str(&rest[..at]);
1733 let after = &rest[at + open.len()..];
1734 match after.find(close) {
1735 Some(end) => rest = &after[end + close.len()..],
1736 None => return out,
1737 }
1738 }
1739 out.push_str(rest);
1740 out
1741}
1742
1743fn repo_merge_lock(repo: &Path) -> Arc<tokio::sync::Mutex<()>> {
1761 static LOCKS: std::sync::LazyLock<
1762 std::sync::Mutex<BTreeMap<PathBuf, Arc<tokio::sync::Mutex<()>>>>,
1763 > = std::sync::LazyLock::new(|| std::sync::Mutex::new(BTreeMap::new()));
1764 LOCKS
1765 .lock()
1766 .unwrap_or_else(std::sync::PoisonError::into_inner)
1767 .entry(repo.to_path_buf())
1768 .or_insert_with(|| Arc::new(tokio::sync::Mutex::new(())))
1769 .clone()
1770}
1771
1772pub async fn land(state: &mut RunState, pr_url: &str) -> Result<PrState> {
1779 let repo = state.repo.clone();
1780 let budget = state.config.graph.land_rounds;
1781 let mut round = 0usize;
1782 let mut rebases = 0usize;
1785 let mut waited = Duration::ZERO;
1786 let mut shown: BTreeSet<String> = BTreeSet::new();
1791
1792 state.status = RunStatus::Landing;
1800 state.event("land", format!("watching {pr_url}"));
1801 state.save()?;
1802
1803 loop {
1804 let seen = observe(&repo, pr_url).await?;
1805 let mut pr = seen.pr;
1806 pr.review_comments.retain(|c| !shown.contains(&c.body));
1807 state.pr = Some(crate::run::PrRecord {
1808 url: pr.url.clone(),
1809 number: pr.number,
1810 state: pr.state.as_str().to_owned(),
1811 checks: pr.checks.as_str().to_owned(),
1812 round,
1813 rounds: budget,
1814 });
1815 state.save()?;
1816
1817 match decide(&pr, round, budget, waited) {
1818 Step::Wait => {
1819 if waited >= WAIT_CEILING {
1820 let why = format!(
1821 "checks were still running after {} minutes",
1822 WAIT_CEILING.as_secs() / 60
1823 );
1824 stop(state, &repo, &pr, &why).await?;
1825 return Ok(pr);
1826 }
1827 waited += POLL;
1828 tokio::time::sleep(POLL).await;
1829 }
1830 Step::Done { merged } => {
1831 state.status = if merged {
1832 RunStatus::Merged
1833 } else {
1834 RunStatus::Ready
1835 };
1836 let detail = if merged {
1837 format!("{} was merged", pr.url)
1838 } else {
1839 format!("{} was closed without merging", pr.url)
1840 };
1841 state.merge = Some(MergeOutcome {
1842 mode: MergeMode::Pr,
1843 ok: merged,
1844 detail: detail.clone(),
1845 });
1846 state.event("land", detail);
1847 state.save()?;
1848 return Ok(pr);
1849 }
1850 Step::Merge => {
1851 let subject = merge_subject(&seen.title, &state.instruction);
1852 if state.config.graph.land_approval {
1855 match approval_gate(state, &pr, &subject).await? {
1856 ApprovalGate::Approved => {}
1857 ApprovalGate::Held => {
1858 stop(
1859 state,
1860 &repo,
1861 &pr,
1862 "the owner did not approve the merge (held or unanswered)",
1863 )
1864 .await?;
1865 return Ok(pr);
1866 }
1867 ApprovalGate::Pending => {
1875 state.parked = true;
1876 state.event(
1877 "land",
1878 "parked awaiting merge approval - resumes once answered",
1879 );
1880 state.save()?;
1881 return Ok(pr);
1882 }
1883 }
1884 }
1885 let argv = merge_argv(pr.number, &subject);
1886 let out = {
1887 let merge_lock = repo_merge_lock(&repo);
1888 let _merge_slot = merge_lock.lock().await;
1889 gh(&repo, &argv).await?
1890 };
1891 if out.0 {
1892 pr.state = PrLifecycle::Merged;
1893 state.status = RunStatus::Merged;
1894 state.merge = Some(MergeOutcome {
1895 mode: MergeMode::Pr,
1896 ok: true,
1897 detail: format!("gh {}", argv.join(" ")),
1898 });
1899 if let Some(pr_record) = state.pr.as_mut() {
1904 pr_record.state = pr.state.as_str().to_owned();
1905 }
1906 state.event("land", format!("merged {} as `{subject}`", pr.url));
1907 state.save()?;
1908 return Ok(pr);
1909 }
1910 let after = observe(&repo, pr_url).await.ok().map(|s| s.pr.state);
1911 if let Some(outcome) = merged_after_all(&argv, &out.1, after) {
1912 pr.state = PrLifecycle::Merged;
1913 state.status = RunStatus::Merged;
1914 state.merge = Some(outcome);
1915 if let Some(pr_record) = state.pr.as_mut() {
1916 pr_record.state = pr.state.as_str().to_owned();
1917 }
1918 state.event("land", format!("merged {} as `{subject}`", pr.url));
1919 state.save()?;
1920 return Ok(pr);
1921 }
1922 stop(
1923 state,
1924 &repo,
1925 &pr,
1926 &format!("`gh pr merge` failed: {}", out.1),
1927 )
1928 .await?;
1929 return Ok(pr);
1930 }
1931 Step::Rebase => {
1932 if rebases >= budget {
1938 let why = format!(
1939 "the base moved under this branch {budget} time(s) and it still does \
1940 not merge; rebasing again would only race it"
1941 );
1942 stop(state, &repo, &pr, &why).await?;
1943 return Ok(pr);
1944 }
1945 rebases += 1;
1946 let Some(branch) = state.winner().map(|w| w.branch.clone()) else {
1947 stop(
1948 state,
1949 &repo,
1950 &pr,
1951 "the pull request conflicts and this run has no winning branch to rebase",
1952 )
1953 .await?;
1954 return Ok(pr);
1955 };
1956 let base = state.base_branch.clone();
1957 state.event(
1958 "land",
1959 format!("{} no longer merges; rebasing onto {base}", pr.url),
1960 );
1961 state.save()?;
1962
1963 git::fetch(&repo, "origin", &base).await.ok();
1967 let scratch = state.dir().join("rebase");
1968 let onto = format!("origin/{base}");
1969 match git::rebase_branch_in_temp(&repo, &scratch, &branch, &onto).await {
1970 Ok(None) => {
1971 let pushed = {
1972 let merge_lock = repo_merge_lock(&repo);
1973 let _merge_slot = merge_lock.lock().await;
1974 git::push_rewritten(&repo, "origin", &branch).await?
1975 };
1976 if !pushed.ok() {
1977 let why = format!(
1978 "rebased {branch} but could not push it: {}",
1979 pushed.stderr.trim()
1980 );
1981 stop(state, &repo, &pr, &why).await?;
1982 return Ok(pr);
1983 }
1984 state.event("land", format!("rebased {branch} onto {base}"));
1985 state.save()?;
1986 waited = Duration::ZERO;
1989 tokio::time::sleep(POLL).await;
1990 }
1991 Ok(Some(conflict)) => {
1993 let why = format!(
1994 "{} conflicts with {base} and the rebase did not apply: {}",
1995 pr.url,
1996 conflict.chars().take(600).collect::<String>()
1997 );
1998 stop(state, &repo, &pr, &why).await?;
1999 return Ok(pr);
2000 }
2001 Err(e) => {
2002 let why = format!("could not rebase {branch} onto {base}: {e:#}");
2003 stop(state, &repo, &pr, &why).await?;
2004 return Ok(pr);
2005 }
2006 }
2007 }
2008 Step::GiveUp { reason } => {
2009 stop(state, &repo, &pr, &reason).await?;
2010 return Ok(pr);
2011 }
2012 Step::Fix { reason } => {
2013 round += 1;
2014 waited = Duration::ZERO;
2015 for c in &pr.review_comments {
2016 shown.insert(c.body.clone());
2017 }
2018 state.event("land", format!("round {round}: {reason}"));
2019 state.save()?;
2020
2021 let logs = failing_logs(&repo, &seen.failing_urls).await;
2022 let was_red = pr.checks == Checks::Red;
2023 match fix_round(state, &pr, round, budget, &reason, &logs).await? {
2024 Fixed::Committed => {}
2025 Fixed::Declined if was_red => {
2026 let why = format!(
2027 "the fixer produced no commit while {} check(s) were failing \
2028 ({}); stopping instead of looping on an unchanged tree",
2029 pr.failing.len(),
2030 pr.failing.join(", ")
2031 );
2032 stop(state, &repo, &pr, &why).await?;
2033 return Ok(pr);
2034 }
2035 Fixed::Declined => state.event(
2040 "land",
2041 format!("round {round}: fixer declined the comments, nothing committed"),
2042 ),
2043 Fixed::Failed(why) => {
2044 stop(state, &repo, &pr, &format!("the fix round failed: {why}")).await?;
2045 return Ok(pr);
2046 }
2047 }
2048 state.save()?;
2049 }
2050 }
2051 }
2052}
2053
2054struct Seen {
2058 pr: PrState,
2059 title: String,
2060 failing_urls: Vec<(String, String)>,
2061}
2062
2063async fn observe(repo: &Path, pr_url: &str) -> Result<Seen> {
2066 let view = gh(
2067 repo,
2068 &[
2069 "pr".to_owned(),
2070 "view".to_owned(),
2071 pr_url.to_owned(),
2072 "--json".to_owned(),
2073 "url,number,state,title,statusCheckRollup,reviews,comments,mergeStateStatus".to_owned(),
2074 ],
2075 )
2076 .await?;
2077 if !view.0 {
2078 bail!("gh pr view {pr_url}: {}", view.1);
2079 }
2080 let mut pr = parse_pr(&view.1)?;
2081 let raw: GhPr = serde_json::from_str(&view.1).context("re-read pull request json")?;
2082
2083 let inline = gh(
2084 repo,
2085 &[
2086 "api".to_owned(),
2087 format!("repos/{{owner}}/{{repo}}/pulls/{}/comments", pr.number),
2088 ],
2089 )
2090 .await?;
2091 if inline.0 {
2092 match parse_inline_comments(&inline.1) {
2093 Ok(mut comments) => pr.review_comments.append(&mut comments),
2094 Err(e) => tracing::warn!("inline review comments unreadable: {e}"),
2097 }
2098 } else {
2099 tracing::warn!("gh api pulls/{}/comments: {}", pr.number, inline.1);
2100 }
2101
2102 let failing_urls = raw
2103 .status_check_rollup
2104 .iter()
2105 .filter(|c| c.verdict() == Verdict::Fail)
2106 .filter_map(|c| c.url().map(|u| (c.label(), u.to_owned())))
2107 .collect();
2108
2109 Ok(Seen {
2110 pr,
2111 title: raw.title,
2112 failing_urls,
2113 })
2114}
2115
2116enum Fixed {
2118 Committed,
2120 Declined,
2122 Failed(String),
2124}
2125
2126async fn fix_round(
2132 state: &mut RunState,
2133 pr: &PrState,
2134 round: usize,
2135 budget: usize,
2136 reason: &str,
2137 logs: &str,
2138) -> Result<Fixed> {
2139 let winner = state
2140 .winner()
2141 .cloned()
2142 .context("landing needs a winning candidate; none is recorded on this run")?;
2143 let roles = state
2144 .config
2145 .resolve_roles()
2146 .context("resolve the roster for the fix round")?;
2147 let (spec, seat_key): (AgentSpec, String) = match &roles.fixer {
2151 Some(f) if f.id != winner.agent => (f.clone(), "fix".to_owned()),
2152 _ => (
2153 state
2154 .config
2155 .agent(&winner.agent)
2156 .cloned()
2157 .unwrap_or_else(|_| roles.implementers[winner.index].clone()),
2158 format!("impl-{}", winner.label),
2159 ),
2160 };
2161
2162 let prompt = fix_prompt(state, pr, round, budget, reason, logs);
2163 let mut seat = seat_of(state, &seat_key, &spec.id);
2164 let artifacts = agent::artifacts_dir(&state.dir());
2165 let prompt = if state.config.cache_dir().is_some() {
2166 format!("{prompt}\n\n{}", prompt::build_cache_note("fix", true))
2167 } else {
2168 prompt
2169 };
2170 let out = agent::invoke(
2171 &spec,
2172 &mut seat,
2173 &Invocation {
2174 cwd: &winner.worktree,
2175 prompt: &prompt,
2176 timeout: Duration::from_secs(state.config.graph.timeout_fix),
2177 allow_write: true,
2178 sessions: state.config.graph.sessions,
2179 artifacts: &artifacts,
2180 stem: &format!("land-{round}"),
2181 run: &state.id,
2182 node: "land",
2183 cache_dir: state.config.cache_dir().as_deref(),
2184 attachments: &[],
2185 },
2186 )
2187 .await;
2188 state.seats.insert(seat.key.clone(), seat);
2189
2190 match out {
2191 Ok(o) if o.quota_exhausted() => {
2192 return Ok(Fixed::Failed(
2193 "rate limited (quota); the fixer could not run".to_owned(),
2194 ));
2195 }
2196 Ok(o) if !o.usable() => {
2197 return Ok(Fixed::Failed(format!(
2198 "the fixer produced nothing usable (exit {:?}, timed out: {})",
2199 o.exit_code, o.timed_out
2200 )));
2201 }
2202 Ok(_) => {}
2203 Err(e) => return Ok(Fixed::Failed(format!("{e:#}"))),
2204 }
2205
2206 let before = git::rev_parse(&winner.worktree, "HEAD").await?;
2207 if let Ok(r) = git::rescue_commit(
2210 &winner.worktree,
2211 &format!("magi: land round {round} fixes (uncommitted work)"),
2212 )
2213 .await
2214 {
2215 state.note_withheld("land", &r.withheld);
2216 }
2217 let after = git::rev_parse(&winner.worktree, "HEAD").await?;
2218 if after == before {
2219 return Ok(Fixed::Declined);
2220 }
2221
2222 let remote = state.config.merge.remote.clone();
2223 let push = git::push(&winner.worktree, &remote, &winner.branch).await?;
2224 if !push.ok() {
2225 return Ok(Fixed::Failed(format!(
2226 "pushing {} to {remote} failed: {}",
2227 winner.branch, push.stderr
2228 )));
2229 }
2230 state.event(
2231 "land",
2232 format!("round {round}: pushed a fix to {}", winner.branch),
2233 );
2234 Ok(Fixed::Committed)
2235}
2236
2237fn seat_of(state: &mut RunState, key: &str, agent: &str) -> SeatState {
2239 if let Some(existing) = state.seats.get(key)
2240 && existing.agent == agent
2241 {
2242 return existing.clone();
2243 }
2244 let fresh = SeatState::new(key, agent, state.seed);
2245 state.seats.insert(key.to_owned(), fresh.clone());
2246 fresh
2247}
2248
2249fn fix_prompt(
2251 state: &RunState,
2252 pr: &PrState,
2253 round: usize,
2254 budget: usize,
2255 reason: &str,
2256 logs: &str,
2257) -> String {
2258 let mut s = format!(
2259 "Your patch is open as a pull request and it is not landing. Land round \
2260 {round} of {budget}.\n\n\
2261 Pull request: {}\n\n\
2262 What is holding it: {reason}\n\n\
2263 # The task\n\n{}\n",
2264 pr.url, state.instruction
2265 );
2266
2267 if pr.failing.is_empty() {
2268 s.push_str("\n# Failing checks\n\n(none)\n");
2269 } else {
2270 let _ = write!(s, "\n# Failing checks\n\n- {}\n", pr.failing.join("\n- "));
2271 if logs.trim().is_empty() {
2272 s.push_str("\nNo log could be read; reproduce the failure locally.\n");
2273 } else {
2274 let _ = write!(s, "\n## Failing log tails\n\n{logs}\n");
2275 }
2276 }
2277
2278 if pr.review_comments.is_empty() {
2279 s.push_str("\n# Review comments\n\n(none)\n");
2280 } else {
2281 s.push_str("\n# Review comments\n");
2282 for c in &pr.review_comments {
2283 let where_ = match (&c.path, c.line) {
2284 (Some(p), Some(l)) => format!(" ({p}:{l})"),
2285 (Some(p), None) => format!(" ({p})"),
2286 _ => String::new(),
2287 };
2288 let _ = write!(s, "\n## {}{where_}\n\n{}\n", c.author, c.body.trim());
2289 }
2290 }
2291
2292 s.push_str(
2293 "\n# Rules\n\n\
2294 1. Fix the cause, never the symptom. Do not delete, skip, or weaken a \
2295 failing test; do not silence a lint with an allow attribute; do not \
2296 stretch a timeout to hide a race. If the check is right, the code is \
2297 wrong.\n\
2298 2. Change nothing the checks and the comments did not raise. A \
2299 drive-by refactor turns a one-line fix into a pull request that \
2300 needs reviewing again.\n\
2301 3. If a comment is wrong, say so with a checkable argument and change \
2302 nothing for it. A declined comment with a reason is a correct \
2303 outcome; a change made to appease a reviewer is not.\n\
2304 4. Commit in this worktree. magi pushes to the pull request's branch \
2305 for you; do not push, merge, or close anything yourself.\n\
2306 5. Never name yourself, your vendor, or your model, anywhere.\n\n\
2307 # Output\n\n\
2308 Say what you changed and why, and what you declined and why.",
2309 );
2310
2311 let language = &state.config.graph.language;
2312 if !(language.trim().is_empty() || language.eq_ignore_ascii_case("en")) {
2313 let _ = write!(s, "\n\nWrite all prose in {language}.");
2314 }
2315 s.push_str(&crate::prompt::github_english(language));
2317 if let Some(overlay) = state.config.prompts.overlay("fix") {
2318 let _ = write!(s, "\n\n{overlay}");
2319 }
2320 s
2321}
2322
2323async fn failing_logs(repo: &Path, failing: &[(String, String)]) -> String {
2326 let mut out = String::new();
2327 for (name, url) in failing.iter().take(MAX_LOGS) {
2328 let args = match (job_of(url), run_of(url)) {
2329 (Some(job), _) => vec![
2330 "run".to_owned(),
2331 "view".to_owned(),
2332 "--log-failed".to_owned(),
2333 "--job".to_owned(),
2334 job,
2335 ],
2336 (None, Some(run)) => vec![
2337 "run".to_owned(),
2338 "view".to_owned(),
2339 run,
2340 "--log-failed".to_owned(),
2341 ],
2342 (None, None) => continue,
2344 };
2345 let (ok, body) = match gh(repo, &args).await {
2346 Ok(v) => v,
2347 Err(e) => (false, format!("{e:#}")),
2348 };
2349 if !ok && body.trim().is_empty() {
2350 continue;
2351 }
2352 let _ = write!(out, "### {name}\n\n```\n{}\n```\n\n", tail(&body, LOG_TAIL));
2353 }
2354 out
2355}
2356
2357fn job_of(details_url: &str) -> Option<String> {
2360 let after = details_url.split("/job/").nth(1)?;
2361 let id: String = after.chars().take_while(char::is_ascii_digit).collect();
2362 (!id.is_empty()).then_some(id)
2363}
2364
2365fn run_of(details_url: &str) -> Option<String> {
2367 let after = details_url.split("/actions/runs/").nth(1)?;
2368 let id: String = after.chars().take_while(char::is_ascii_digit).collect();
2369 (!id.is_empty()).then_some(id)
2370}
2371
2372fn stop_comment(run_id: &str, why: &str) -> String {
2376 format!(
2377 "{MARKER}\nmagi stopped landing this pull request: {why}\n\n\
2378 The branch is untouched and the run is `{run_id}`. Nothing was merged."
2379 )
2380}
2381
2382async fn stop(state: &mut RunState, repo: &Path, pr: &PrState, why: &str) -> Result<()> {
2387 let body = stop_comment(&state.id, why);
2388 let posted = gh(
2389 repo,
2390 &[
2391 "pr".to_owned(),
2392 "comment".to_owned(),
2393 pr.number.to_string(),
2394 "--body".to_owned(),
2395 body,
2396 ],
2397 )
2398 .await;
2399 match posted {
2400 Ok((true, _)) => {}
2401 Ok((false, out)) => tracing::warn!("could not comment on {}: {out}", pr.url),
2402 Err(e) => tracing::warn!("could not comment on {}: {e:#}", pr.url),
2403 }
2404 state.status = RunStatus::Blocked;
2405 state.merge = Some(MergeOutcome {
2406 mode: MergeMode::Pr,
2407 ok: false,
2408 detail: why.to_owned(),
2409 });
2410 state.event("land", format!("stopped: {why}"));
2411 state.save()?;
2412 Ok(())
2413}
2414
2415async fn gh(cwd: &Path, args: &[String]) -> Result<(bool, String)> {
2431 let out = tokio::process::Command::new("gh")
2432 .args(args)
2433 .current_dir(cwd)
2434 .env_remove("GH_REPO")
2435 .quiet()
2436 .stdin(std::process::Stdio::null())
2437 .output()
2438 .await
2439 .with_context(|| format!("spawn gh {}", args.join(" ")))?;
2440 let mut body = String::from_utf8_lossy(&out.stdout).into_owned();
2441 let err = String::from_utf8_lossy(&out.stderr);
2442 if body.trim().is_empty() {
2443 body = err.into_owned();
2444 } else if !err.trim().is_empty() {
2445 body.push_str(&err);
2446 }
2447 Ok((out.status.success(), body.trim().to_owned()))
2448}
2449
2450#[derive(Debug, Clone, Copy, PartialEq, Eq)]
2452enum Verdict {
2453 Pass,
2454 Fail,
2455 Pending,
2456 Unknown,
2457}
2458
2459#[derive(Debug, Deserialize)]
2460#[serde(rename_all = "camelCase")]
2461struct GhPr {
2462 #[serde(default)]
2463 url: String,
2464 #[serde(default)]
2465 number: u64,
2466 #[serde(default)]
2467 state: String,
2468 #[serde(default)]
2469 title: String,
2470 #[serde(default)]
2471 status_check_rollup: Vec<GhCheck>,
2472 #[serde(default)]
2479 merge_state_status: String,
2480 #[serde(default)]
2481 reviews: Vec<GhReview>,
2482 #[serde(default)]
2483 comments: Vec<GhComment>,
2484}
2485
2486#[derive(Debug, Deserialize)]
2491#[serde(rename_all = "camelCase")]
2492struct GhCheck {
2493 #[serde(default)]
2494 name: Option<String>,
2495 #[serde(default)]
2496 context: Option<String>,
2497 #[serde(default)]
2498 status: Option<String>,
2499 #[serde(default)]
2500 conclusion: Option<String>,
2501 #[serde(default)]
2502 state: Option<String>,
2503 #[serde(default)]
2504 details_url: Option<String>,
2505 #[serde(default)]
2506 target_url: Option<String>,
2507}
2508
2509impl GhCheck {
2510 fn label(&self) -> String {
2512 self.name
2513 .clone()
2514 .or_else(|| self.context.clone())
2515 .unwrap_or_else(|| "(unnamed check)".to_owned())
2516 }
2517
2518 fn url(&self) -> Option<&str> {
2520 self.details_url
2521 .as_deref()
2522 .or(self.target_url.as_deref())
2523 .filter(|u| !u.is_empty())
2524 }
2525
2526 fn verdict(&self) -> Verdict {
2534 if let Some(status) = self.status.as_deref() {
2535 if !status.eq_ignore_ascii_case("COMPLETED") {
2536 return Verdict::Pending;
2537 }
2538 }
2539 let outcome = self
2540 .conclusion
2541 .as_deref()
2542 .or(self.state.as_deref())
2543 .unwrap_or("");
2544 match outcome.to_ascii_uppercase().as_str() {
2545 "SUCCESS" | "SKIPPED" | "NEUTRAL" => Verdict::Pass,
2546 "FAILURE" | "ERROR" | "TIMED_OUT" | "CANCELLED" | "STARTUP_FAILURE"
2547 | "ACTION_REQUIRED" => Verdict::Fail,
2548 "PENDING" | "EXPECTED" | "QUEUED" | "IN_PROGRESS" | "WAITING" | "REQUESTED" => {
2549 Verdict::Pending
2550 }
2551 _ => Verdict::Unknown,
2552 }
2553 }
2554}
2555
2556#[derive(Debug, Deserialize)]
2557struct GhAuthor {
2558 #[serde(default)]
2559 login: String,
2560}
2561
2562#[derive(Debug, Deserialize)]
2563struct GhReview {
2564 #[serde(default)]
2565 author: GhAuthor,
2566 #[serde(default)]
2567 body: String,
2568}
2569
2570#[derive(Debug, Deserialize)]
2571struct GhComment {
2572 #[serde(default)]
2573 author: GhAuthor,
2574 #[serde(default)]
2575 body: String,
2576}
2577
2578#[derive(Debug, Deserialize)]
2579struct GhUser {
2580 #[serde(default)]
2581 login: String,
2582}
2583
2584#[derive(Debug, Deserialize)]
2585struct GhInline {
2586 #[serde(default)]
2587 user: GhUser,
2588 #[serde(default)]
2589 path: Option<String>,
2590 #[serde(default)]
2591 line: Option<u64>,
2592 #[serde(default)]
2593 body: String,
2594}
2595
2596impl Default for GhAuthor {
2597 fn default() -> Self {
2598 Self {
2599 login: "(unknown)".to_owned(),
2600 }
2601 }
2602}
2603
2604impl Default for GhUser {
2605 fn default() -> Self {
2606 Self {
2607 login: "(unknown)".to_owned(),
2608 }
2609 }
2610}
2611
2612#[cfg(test)]
2613mod tests {
2614 use super::*;
2615 use crate::run::{Candidate, ReviewRecord, ReviewRound, Tally};
2616
2617 const GREEN_OPEN: &str = r####"{
2619 "url": "https://github.com/yukimemi/magi/pull/10",
2620 "number": 10,
2621 "state": "OPEN",
2622 "mergeStateStatus": "CLEAN",
2623 "statusCheckRollup": [
2624 {
2625 "__typename": "CheckRun",
2626 "conclusion": "SKIPPED",
2627 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33356278334/job/99378963755",
2628 "name": "review",
2629 "status": "COMPLETED",
2630 "workflowName": "claude-review"
2631 },
2632 {
2633 "__typename": "CheckRun",
2634 "conclusion": "SUCCESS",
2635 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33356278338/job/99378963144",
2636 "name": "check (ubuntu-latest)",
2637 "status": "COMPLETED",
2638 "workflowName": "CI"
2639 },
2640 {
2641 "__typename": "CheckRun",
2642 "conclusion": "SUCCESS",
2643 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33356278338/job/99378963095",
2644 "name": "rustfmt",
2645 "status": "COMPLETED",
2646 "workflowName": "CI"
2647 },
2648 {
2649 "__typename": "StatusContext",
2650 "context": "CodeRabbit",
2651 "state": "SUCCESS",
2652 "targetUrl": ""
2653 }
2654 ],
2655 "reviews": [],
2656 "comments": [
2657 {
2658 "author": {
2659 "login": "coderabbitai"
2660 },
2661 "authorAssociation": "NONE",
2662 "body": "<!-- This is an auto-generated comment: summarize by coderabbit.ai -->\n<!-- This is an auto-generated comment: skip review by coderabbit.ai -->\n\n> [!IMPORTANT]\n> - [ ] <!-- {\"checkboxId\":\"e9bb8d72-00e8-4f67-9cb2-caf3b22574fe\"} --> 🔍 Trigger review\n> \n> This repository does not receive automatic reviews because it has fewer than 10 stars.\n> \n> <details>\n> <summary>⚙️ Run configuration</summary>\n> \n> **Configuration used**: defaults\n> \n> **Review profile**: CHILL\n> \n> **Plan**: Pro Plus\n> \n> **Run ID**: `78e70bf3-c5a0-4269-a96c-2afb2dba7eff`\n> \n> </details>\n\n<!-- end of auto-generated comment: skip review by coderabbit.ai -->\n\n<!-- tips_start -->\n\n---\n\nThanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=yukimemi/magi&utm_content=10)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.\n\n<details>\n<summary>❤️ Share</summary>\n\n- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%2"
2663 }
2664 ]
2665}"####;
2666
2667 const RED_OPEN: &str = r####"{
2669 "url": "https://github.com/yukimemi/magi/pull/9",
2670 "number": 9,
2671 "state": "OPEN",
2672 "mergeStateStatus": "UNSTABLE",
2673 "statusCheckRollup": [
2674 {
2675 "__typename": "CheckRun",
2676 "conclusion": "SUCCESS",
2677 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323744",
2678 "name": "check (ubuntu-latest)",
2679 "status": "COMPLETED",
2680 "workflowName": "CI"
2681 },
2682 {
2683 "__typename": "CheckRun",
2684 "conclusion": "SUCCESS",
2685 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323811",
2686 "name": "rustfmt",
2687 "status": "COMPLETED",
2688 "workflowName": "CI"
2689 },
2690 {
2691 "__typename": "CheckRun",
2692 "conclusion": "FAILURE",
2693 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323572",
2694 "name": "editorconfig",
2695 "status": "COMPLETED",
2696 "workflowName": "CI"
2697 },
2698 {
2699 "__typename": "StatusContext",
2700 "context": "CodeRabbit",
2701 "state": "SUCCESS",
2702 "targetUrl": ""
2703 }
2704 ],
2705 "reviews": [],
2706 "comments": [
2707 {
2708 "author": {
2709 "login": "coderabbitai"
2710 },
2711 "authorAssociation": "NONE",
2712 "body": "<!-- This is an auto-generated comment: summarize by coderabbit.ai -->\n<!-- This is an auto-generated comment: skip review by coderabbit.ai -->\n\n> [!IMPORTANT]\n> - [ ] <!-- {\"checkboxId\":\"e9bb8d72-00e8-4f67-9cb2-caf3b22574fe\"} --> 🔍 Trigger review\n> \n> This repository does not receive automatic reviews because it has fewer than 10 stars.\n> \n> <details>\n> <summary>⚙️ Run configuration</summary>\n> \n> **Configuration used**: defaults\n> \n> **Review profile**: CHILL\n> \n> **Plan**: Team\n> \n> **Run ID**: `91e0dc24-6040-4c3d-92c6-f7d2b542523d`\n> \n> </details>\n\n<!-- end of auto-generated comment: skip review by coderabbit.ai -->\n\n<!-- tips_start -->\n\n---\n\nThanks for using [CodeRabbit](https://coderab"
2713 }
2714 ]
2715}"####;
2716
2717 const PENDING_OPEN: &str = r####"{
2719 "url": "https://github.com/yukimemi/magi/pull/9",
2720 "number": 9,
2721 "state": "OPEN",
2722 "statusCheckRollup": [
2723 {
2724 "__typename": "CheckRun",
2725 "conclusion": "SUCCESS",
2726 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323744",
2727 "name": "check (ubuntu-latest)",
2728 "status": "COMPLETED",
2729 "workflowName": "CI"
2730 },
2731 {
2732 "__typename": "CheckRun",
2733 "conclusion": "SUCCESS",
2734 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323811",
2735 "name": "rustfmt",
2736 "status": "COMPLETED",
2737 "workflowName": "CI"
2738 },
2739 {
2740 "__typename": "CheckRun",
2741 "conclusion": null,
2742 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323572",
2743 "name": "editorconfig",
2744 "status": "IN_PROGRESS",
2745 "workflowName": "CI"
2746 },
2747 {
2748 "__typename": "StatusContext",
2749 "context": "CodeRabbit",
2750 "state": "SUCCESS",
2751 "targetUrl": ""
2752 }
2753 ],
2754 "reviews": [],
2755 "comments": []
2756}"####;
2757
2758 const MERGED: &str = r####"{
2760 "url": "https://github.com/yukimemi/magi/pull/16",
2761 "number": 16,
2762 "state": "MERGED",
2763 "statusCheckRollup": [
2764 {
2765 "__typename": "CheckRun",
2766 "conclusion": "SUCCESS",
2767 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33636587933/job/100268878095",
2768 "name": "check (ubuntu-latest)",
2769 "status": "COMPLETED",
2770 "workflowName": "CI"
2771 },
2772 {
2773 "__typename": "CheckRun",
2774 "conclusion": "SUCCESS",
2775 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33636587918/job/100268876427",
2776 "name": "review",
2777 "status": "COMPLETED",
2778 "workflowName": "claude-review"
2779 }
2780 ],
2781 "reviews": [],
2782 "comments": []
2783}"####;
2784
2785 const REVIEWED_OPEN: &str = r####"{
2787 "url": "https://github.com/yukimemi/magi/pull/12",
2788 "number": 12,
2789 "state": "OPEN",
2790 "statusCheckRollup": [
2791 {
2792 "__typename": "CheckRun",
2793 "conclusion": "SUCCESS",
2794 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33571212506/job/100065355258",
2795 "name": "check (ubuntu-latest)",
2796 "status": "COMPLETED",
2797 "workflowName": "CI"
2798 },
2799 {
2800 "__typename": "CheckRun",
2801 "conclusion": "SUCCESS",
2802 "detailsUrl": "https://github.com/yukimemi/magi/actions/runs/33571212566/job/100065355810",
2803 "name": "review",
2804 "status": "COMPLETED",
2805 "workflowName": "claude-review"
2806 }
2807 ],
2808 "reviews": [
2809 {
2810 "author": {
2811 "login": "claude"
2812 },
2813 "state": "COMMENTED",
2814 "body": ""
2815 }
2816 ],
2817 "comments": [
2818 {
2819 "author": {
2820 "login": "coderabbitai"
2821 },
2822 "authorAssociation": "NONE",
2823 "body": "<!-- This is an auto-generated comment: summarize by coderabbit.ai -->\n<!-- This is an auto-generated comment: skip review by coderabbit.ai -->\n\n> [!IMPORTANT]\n> - [ ] <!-- {\"checkboxId\":\"e9bb8d72-00e8-4f67-9cb2-caf3b22574fe\"} --> 🔍 Trigger review\n> \n> This repository does not receive automatic reviews because it has fewer than 10 stars.\n> \n> <details>\n> <summary>⚙️ Run configuration</summary>\n> \n> **Configuration used**: defaults\n> \n> **Review profile**: CHILL\n> \n> **Plan**: Team\n> \n> **Run ID**: `72058bf3-b7df-41d9-8e4d-a06a31be4a26`\n> \n> </details>\n\n<!-- end of auto-generated comment: skip review by coderabbit.ai -->\n\n<!-- tips_start -->\n\n---\n\nThanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=yukimemi/magi&utm_content=12)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.\n\n<details>\n<summa"
2824 },
2825 {
2826 "author": {
2827 "login": "claude"
2828 },
2829 "authorAssociation": "NONE",
2830 "body": "**Claude finished @yukimemi's task in 3m 52s** —— [View job](https://github.com/yukimemi/magi/actions/runs/33571212566)\n\n---\n### Review: `magi review <branch>` — cheap-half-only graph\n\nRead through `src/graph.rs`, `src/main.rs`, `src/prompt.rs`, and the new/edited tests, and traced the claimed degeneration (`prep` short-circuits on a non-empty candidate list, `implement` skips because `commits != 0`, `judge`/`vote` skip on `viable().len() == 1`, `tally` skips because it's pre-set, `fold_losers` has no losers) against the actual code — it holds up. CI (`cargo make check`) is green on this PR.\n\n**Correctness**\n\n- One real bug, flagged inline on `src/graph.rs:1255`: the fixer-agent fallback (`self.roles.implementers[winner.index].clone()`) is unreachable in the normal graph (a real candidate's `winner.agent` always resolves via `config.agent(...)`), but a review-only run's `winner.agent` is always the `\"(existing branch)\"` sentinel, so this fallback now runs on *every* review-only fix that has no dedicated `[roles] fixer`. `graph.candidates` has no lower-bound validation, so a `magi.toml` tuned for review-only use (`candidates = 0`, plausible given this PR's own cost rationale) would panic with an out-of-bounds index the first time a"
2831 }
2832 ]
2833}"####;
2834
2835 const INLINE: &str = r####"[
2837 {
2838 "user": {
2839 "login": "claude[bot]"
2840 },
2841 "path": "src/graph.rs",
2842 "line": 231,
2843 "body": "Minor edge case: unlike `implement()` (which sets `c.empty = commits == 0 || patch.trim().is_empty()`, `src/graph.rs:472`), the seeded review-only candidate always sets `empty: false` once `commits > 0` is confirmed, without checking whether the diff itself is actually empty (e.g. a commit immediately followed by a revert nets zero file changes). Such a branch would pass `Runner::review`'s validation and proceed into a review round with an empty patch, where `implement()`'s equivalent path would"
2844 }
2845]"####;
2846
2847 const CODERABBIT_TRIGGER: &str = r####"<!-- This is an auto-generated comment: summarize by coderabbit.ai -->
2849<!-- This is an auto-generated comment: skip review by coderabbit.ai -->
2850
2851> [!IMPORTANT]
2852> - [ ] <!-- {"checkboxId":"e9bb8d72-00e8-4f67-9cb2-caf3b22574fe"} --> 🔍 Trigger review
2853>
2854> This repository does not receive automatic reviews because it has fewer than 10 stars.
2855>
2856> <details>
2857> <summary>⚙️ Run configuration</summary>
2858>
2859> **Configuration used**: defaults
2860>
2861> **Review profile**: CHILL
2862>
2863> **Plan**: Team
2864>
2865> **Run ID**: `c1e2a68f-87fc-4b35-9ec4-e75c7854966a`
2866>
2867> </details>
2868
2869<!-- end of auto-generated comment: skip review by coderabbit.ai -->
2870
2871<!-- tips_start -->
2872
2873---
2874
2875Thanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=yukimemi/magi&utm_content=16)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
2876
2877<details>
2878<summary>❤️ Share</summary>
2879
2880- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20off"####;
2881
2882 const CLAUDE_CHECKLIST: &str = r####"**Claude finished @yukimemi's task in 4m 14s** —— [View job](https://github.com/yukimemi/magi/actions/runs/33636587918)
2884
2885---
2886### Reviewing PR #16
2887
2888- [x] Read AGENTS.md conventions
2889- [x] Review `src/daemon.rs` changes
2890- [x] Review `src/main.rs` changes (new `doctor` reporting)
2891- [x] Review `src/web.rs` changes (reuse of unreadable-run count)
2892- [x] Check test coverage for new behavior
2893- [x] Run verification commands (blocked — see note)
2894- [x] Post findings"####;
2895
2896 const CLAUDE_FINDING: &str = r####"**Claude finished @yukimemi's task in 3m 52s** —— [View job](https://github.com/yukimemi/magi/actions/runs/33571212566)
2898
2899---
2900### Review: `magi review <branch>` — cheap-half-only graph
2901
2902Read through `src/graph.rs`, `src/main.rs`, `src/prompt.rs`, and the new/edited tests, and traced the claimed degeneration (`prep` short-circuits on a non-empty candidate list, `implement` skips because `commits != 0`, `judge`/`vote` skip on `viable().len() == 1`, `tally` skips because it's pre-set, `fold_losers` has no losers) against the actual code — it holds up. CI (`cargo make check`) is green on this PR.
2903
2904**Correctness**
2905
2906- One real bug, flagged inline on `src/graph.rs:1255`: the fixer-agent fallback (`self.roles.implementers[winner.index].clone()`) is unreachable in the normal graph (a real candidate's `winner.agent` always resolves via `config.agent(...)`), but a review-only run's `winner.agent` is always the `"(existing branch)"` sentinel, so this fallback now runs on *every* review-only fix that has no dedicated `[roles] fixer`. `graph.candidates` has no lower-bound validation, so a `magi.toml` tuned for review-only use (`candidates = 0`, plausible given this PR's own cost rationale) would panic with an out-of-bounds index the first time a"####;
2907
2908 fn pr(checks: Checks, failing: &[&str], comments: usize) -> PrState {
2909 PrState {
2910 url: "https://github.com/yukimemi/magi/pull/16".to_owned(),
2911 number: 16,
2912 state: PrLifecycle::Open,
2913 checks,
2914 blocking: if matches!(checks, Checks::Red) {
2918 Blocking::Yes
2919 } else {
2920 Blocking::No
2921 },
2922 failing: failing.iter().map(|s| (*s).to_owned()).collect(),
2923 review_comments: (0..comments)
2924 .map(|i| ReviewComment {
2925 author: "coderabbitai".to_owned(),
2926 path: Some("src/graph.rs".to_owned()),
2927 line: Some(231),
2928 body: format!("finding {i}"),
2929 })
2930 .collect(),
2931 }
2932 }
2933
2934 #[test]
2935 fn a_green_pull_request_with_nothing_outstanding_parses_as_ready_to_merge() {
2936 let state = parse_pr(GREEN_OPEN).expect("green fixture parses");
2937 assert_eq!(state.number, 10);
2938 assert_eq!(state.state, PrLifecycle::Open);
2939 assert_eq!(state.checks, Checks::Green);
2940 assert!(state.failing.is_empty());
2941 assert!(
2942 state.review_comments.is_empty(),
2943 "the only comment is CodeRabbit's trigger notice: {:?}",
2944 state.review_comments
2945 );
2946 assert_eq!(decide(&state, 0, 4, Duration::ZERO), Step::Merge);
2947 }
2948
2949 #[test]
2950 fn a_failing_check_parses_as_red_and_is_named() {
2951 let state = parse_pr(RED_OPEN).expect("red fixture parses");
2952 assert_eq!(state.checks, Checks::Red);
2953 assert_eq!(state.failing, vec!["editorconfig".to_owned()]);
2954 let mut blocking = state.clone();
2961 blocking.blocking = Blocking::Yes;
2962 match decide(&blocking, 0, 4, Duration::ZERO) {
2963 Step::Fix { reason } => {
2964 assert!(reason.contains("editorconfig"), "reason: {reason}");
2965 assert!(reason.contains("failing"), "reason: {reason}");
2966 }
2967 other => panic!("expected a fix round, got {other:?}"),
2968 }
2969 }
2970
2971 #[test]
2972 fn a_check_still_running_parses_as_pending_and_is_waited_for() {
2973 let state = parse_pr(PENDING_OPEN).expect("pending fixture parses");
2974 assert_eq!(state.checks, Checks::Pending);
2975 assert_eq!(decide(&state, 0, 4, Duration::ZERO), Step::Wait);
2976 }
2977
2978 #[test]
2979 fn a_pull_request_merged_underneath_us_is_done_rather_than_a_failure() {
2980 let state = parse_pr(MERGED).expect("merged fixture parses");
2981 assert_eq!(state.state, PrLifecycle::Merged);
2982 assert_eq!(
2983 decide(&state, 0, 4, Duration::ZERO),
2984 Step::Done { merged: true }
2985 );
2986 }
2987
2988 #[test]
2989 fn a_review_that_found_something_is_outstanding_and_holds_the_merge() {
2990 let state = parse_pr(REVIEWED_OPEN).expect("reviewed fixture parses");
2991 assert_eq!(state.checks, Checks::Green);
2992 let authors: Vec<&str> = state
2993 .review_comments
2994 .iter()
2995 .map(|c| c.author.as_str())
2996 .collect();
2997 assert_eq!(
2998 authors,
2999 vec!["claude"],
3000 "CodeRabbit's walkthrough is machinery; Claude's review is a finding"
3001 );
3002 match decide(&state, 0, 4, Duration::ZERO) {
3003 Step::Fix { reason } => assert!(reason.contains("unresolved"), "reason: {reason}"),
3004 other => panic!("expected a fix round, got {other:?}"),
3005 }
3006 }
3007
3008 #[test]
3009 fn inline_review_comments_keep_their_file_and_line() {
3010 let comments = parse_inline_comments(INLINE).expect("inline fixture parses");
3011 assert_eq!(comments.len(), 1);
3012 assert_eq!(comments[0].author, "claude[bot]");
3013 assert_eq!(comments[0].path.as_deref(), Some("src/graph.rs"));
3014 assert_eq!(comments[0].line, Some(231));
3015 assert!(comments[0].body.contains("empty"), "{}", comments[0].body);
3016 }
3017
3018 #[test]
3019 fn a_status_only_bot_comment_does_not_trigger_a_fix_round() {
3020 assert!(
3021 is_noise(CODERABBIT_TRIGGER),
3022 "CodeRabbit's trigger notice declares itself not a review"
3023 );
3024 assert!(
3025 is_noise(CLAUDE_CHECKLIST),
3026 "a progress checklist asks for nothing"
3027 );
3028 assert!(
3029 !is_noise(CLAUDE_FINDING),
3030 "a review that names a bug is input, not noise"
3031 );
3032
3033 let mut clean = pr(Checks::Green, &[], 0);
3034 clean.review_comments.push(ReviewComment {
3035 author: "coderabbitai".to_owned(),
3036 path: None,
3037 line: None,
3038 body: CODERABBIT_TRIGGER.to_owned(),
3039 });
3040 clean.review_comments.retain(|c| !is_noise(&c.body));
3041 assert_eq!(decide(&clean, 0, 4, Duration::ZERO), Step::Merge);
3042
3043 let mut found = pr(Checks::Green, &[], 0);
3044 found.review_comments.push(ReviewComment {
3045 author: "claude".to_owned(),
3046 path: None,
3047 line: None,
3048 body: CLAUDE_FINDING.to_owned(),
3049 });
3050 found.review_comments.retain(|c| !is_noise(&c.body));
3051 assert!(matches!(
3052 decide(&found, 0, 4, Duration::ZERO),
3053 Step::Fix { .. }
3054 ));
3055 }
3056
3057 #[test]
3058 fn the_policy_table_holds_for_every_combination_that_matters() {
3059 let cases: Vec<(&str, PrState, usize, usize, Duration, Step)> = vec![
3060 (
3061 "pending checks are waited for, even on the last round",
3062 pr(Checks::Pending, &[], 0),
3063 4,
3064 4,
3065 Duration::ZERO,
3066 Step::Wait,
3067 ),
3068 (
3069 "red checks are fixed",
3070 pr(Checks::Red, &["editorconfig"], 0),
3071 0,
3072 4,
3073 Duration::ZERO,
3074 Step::Fix {
3075 reason: "1 check(s) failing: editorconfig".to_owned(),
3076 },
3077 ),
3078 (
3079 "green with comments is fixed, not merged",
3080 pr(Checks::Green, &[], 2),
3081 1,
3082 4,
3083 Duration::ZERO,
3084 Step::Fix {
3085 reason: "checks are green but 2 review comment(s) are unresolved: coderabbitai"
3086 .to_owned(),
3087 },
3088 ),
3089 (
3090 "green and clean merges",
3091 pr(Checks::Green, &[], 0),
3092 3,
3093 4,
3094 Duration::ZERO,
3095 Step::Merge,
3096 ),
3097 (
3098 "an unreadable rollup is waited on while the grace lasts",
3099 pr(Checks::Unknown, &[], 0),
3100 0,
3101 4,
3102 Duration::ZERO,
3103 Step::Wait,
3104 ),
3105 (
3106 "an unreadable rollup is never merged once the grace is spent",
3107 pr(Checks::Unknown, &[], 0),
3108 0,
3109 4,
3110 CHECKS_GRACE,
3111 Step::GiveUp {
3112 reason: "no check status is readable on the pull request after 3 minute(s); \
3113 refusing to merge on a guess"
3114 .to_owned(),
3115 },
3116 ),
3117 ];
3118 for (what, state, round, budget, waited, want) in cases {
3119 assert_eq!(decide(&state, round, budget, waited), want, "{what}");
3120 }
3121 }
3122
3123 #[test]
3124 fn the_forge_verdict_survives_the_round_trip_from_gh() {
3125 let green = parse_pr(GREEN_OPEN).expect("parse");
3129 assert_eq!(green.blocking, Blocking::No);
3130 let red = parse_pr(RED_OPEN).expect("parse");
3131 assert_eq!(
3132 red.blocking,
3133 Blocking::No,
3134 "`UNSTABLE` is mergeable: the red check is one nobody requires"
3135 );
3136 assert_eq!(red.checks, Checks::Red, "and it is still reported as red");
3137 let quiet =
3139 parse_pr(&GREEN_OPEN.replace("\"mergeStateStatus\": \"CLEAN\",", "")).expect("parse");
3140 assert_eq!(quiet.blocking, Blocking::Unsaid);
3141 }
3142
3143 #[test]
3144 fn a_red_check_nobody_requires_does_not_buy_a_fix_round() {
3145 let mut nonblocking = pr(Checks::Red, &["editorconfig", "coverage"], 0);
3151 nonblocking.blocking = Blocking::No;
3152 assert_eq!(
3153 decide(&nonblocking, 0, 4, Duration::ZERO),
3154 Step::Merge,
3155 "the forge says nothing is in the way, so nothing is"
3156 );
3157
3158 let mut blocking = pr(Checks::Red, &["test (ubuntu-latest)"], 0);
3160 blocking.blocking = Blocking::Yes;
3161 assert!(matches!(
3162 decide(&blocking, 0, 4, Duration::ZERO),
3163 Step::Fix { .. }
3164 ));
3165
3166 let mut commented = pr(Checks::Red, &["coverage"], 1);
3169 commented.blocking = Blocking::No;
3170 assert!(matches!(
3171 decide(&commented, 0, 4, Duration::ZERO),
3172 Step::Fix { .. }
3173 ));
3174
3175 let mut unsaid = pr(Checks::Red, &["coverage"], 0);
3177 unsaid.blocking = Blocking::Unsaid;
3178 assert!(matches!(
3179 decide(&unsaid, 0, 4, Duration::ZERO),
3180 Step::Fix { .. }
3181 ));
3182 }
3183
3184 #[test]
3185 fn a_branch_the_base_moved_under_is_rebased_not_fixed() {
3186 let mut conflicted = pr(Checks::Green, &[], 0);
3191 conflicted.blocking = Blocking::Conflict;
3192 assert_eq!(decide(&conflicted, 0, 4, Duration::ZERO), Step::Rebase);
3193
3194 let mut red = pr(Checks::Red, &["test (ubuntu-latest)"], 2);
3198 red.blocking = Blocking::Conflict;
3199 assert_eq!(decide(&red, 4, 4, Duration::ZERO), Step::Rebase);
3200
3201 let mut merged = pr(Checks::Red, &[], 0);
3203 merged.blocking = Blocking::Conflict;
3204 merged.state = PrLifecycle::Merged;
3205 assert_eq!(
3206 decide(&merged, 0, 4, Duration::ZERO),
3207 Step::Done { merged: true }
3208 );
3209 }
3210
3211 #[test]
3212 fn the_forge_verdict_is_read_off_merge_state_status() {
3213 for ok in ["CLEAN", "UNSTABLE", "unstable", "HAS_HOOKS"] {
3216 assert_eq!(Blocking::of(ok), Blocking::No, "{ok}");
3217 assert!(!Blocking::of(ok).stops_a_merge(), "{ok}");
3218 }
3219 assert_eq!(Blocking::of("DIRTY"), Blocking::Conflict);
3220 assert_eq!(Blocking::of("BLOCKED"), Blocking::Yes);
3221 assert_eq!(Blocking::of("BEHIND"), Blocking::Yes);
3222 for quiet in ["", "UNKNOWN"] {
3225 assert_eq!(Blocking::of(quiet), Blocking::Unsaid);
3226 assert!(Blocking::of(quiet).stops_a_merge());
3227 }
3228 }
3229
3230 #[test]
3231 fn a_merge_command_that_failed_after_merging_is_still_a_merge() {
3232 let argv = merge_argv(28, "fix: retry uploads on transient network errors");
3233 let jj = "could not determine current branch: failed to run git: not on any branch";
3235
3236 let landed = merged_after_all(&argv, jj, Some(PrLifecycle::Merged))
3237 .expect("the forge says merged, so it merged");
3238 assert!(landed.ok);
3239 assert!(
3240 landed.detail.contains("but the pull request is merged"),
3241 "the record must not read as a clean success: {}",
3242 landed.detail
3243 );
3244 assert!(
3245 landed.detail.contains("not on any branch"),
3246 "and it must keep what the command actually said: {}",
3247 landed.detail
3248 );
3249
3250 assert!(merged_after_all(&argv, jj, Some(PrLifecycle::Open)).is_none());
3252 assert!(merged_after_all(&argv, jj, Some(PrLifecycle::Closed)).is_none());
3253 assert!(merged_after_all(&argv, jj, None).is_none());
3255 }
3256
3257 #[test]
3258 fn a_pull_request_closed_underneath_us_is_done_and_not_merged() {
3259 let mut state = pr(Checks::Red, &["editorconfig"], 3);
3260 state.state = PrLifecycle::Closed;
3261 assert_eq!(
3262 decide(&state, 0, 4, Duration::ZERO),
3263 Step::Done { merged: false },
3264 "a human closing the pull request ends the loop, whatever CI says"
3265 );
3266 }
3267
3268 #[test]
3269 fn the_last_round_gives_up_with_a_reason_naming_what_is_still_failing() {
3270 let red = decide(
3271 &pr(Checks::Red, &["editorconfig", "test (macos)"], 0),
3272 4,
3273 4,
3274 Duration::ZERO,
3275 );
3276 match red {
3277 Step::GiveUp { reason } => {
3278 assert!(reason.contains("editorconfig"), "reason: {reason}");
3279 assert!(reason.contains("test (macos)"), "reason: {reason}");
3280 assert!(reason.contains("4 fix round(s)"), "reason: {reason}");
3281 }
3282 other => panic!("expected a give-up, got {other:?}"),
3283 }
3284
3285 let commented = decide(&pr(Checks::Green, &[], 1), 2, 2, Duration::ZERO);
3286 match commented {
3287 Step::GiveUp { reason } => {
3288 assert!(reason.contains("unresolved"), "reason: {reason}");
3289 assert!(reason.contains("2 fix round(s)"), "reason: {reason}");
3290 }
3291 other => panic!("expected a give-up, got {other:?}"),
3292 }
3293 }
3294
3295 #[test]
3296 fn the_merge_command_squashes_deletes_the_branch_and_sets_its_own_subject() {
3297 let candidate_commit = "magi: candidate A (uncommitted work)";
3298 let subject = merge_subject(candidate_commit, "add retries to the uploader");
3299 let argv = merge_argv(16, &subject);
3300
3301 assert!(argv.contains(&"--squash".to_owned()));
3302 assert!(argv.contains(&"--delete-branch".to_owned()));
3303 assert!(argv.contains(&"--subject".to_owned()));
3304 assert_eq!(
3305 argv.last().map(String::as_str),
3306 Some("add retries to the uploader"),
3307 "the subject must not be the candidate commit message"
3308 );
3309 assert_ne!(subject, candidate_commit);
3310 }
3311
3312 #[test]
3313 fn a_real_pull_request_title_is_used_as_the_squash_subject_verbatim() {
3314 assert_eq!(
3315 merge_subject("feat: a queue, an unattended loop, and a phone UI", "task"),
3316 "feat: a queue, an unattended loop, and a phone UI"
3317 );
3318 assert_eq!(
3319 merge_subject("", "# port the retry logic\n\ndetails"),
3320 "port the retry logic",
3321 "an empty title falls back to the task's first line, heading marks stripped"
3322 );
3323 }
3324
3325 #[test]
3326 fn a_failing_checks_details_url_yields_the_job_to_read_logs_from() {
3327 let url = "https://github.com/yukimemi/magi/actions/runs/33587406996/job/100114323572";
3328 assert_eq!(job_of(url).as_deref(), Some("100114323572"));
3329 assert_eq!(run_of(url).as_deref(), Some("33587406996"));
3330 assert_eq!(job_of("https://coderabbit.ai/status"), None);
3331 assert_eq!(run_of(""), None);
3332 }
3333
3334 #[test]
3335 fn magis_own_stop_comment_is_never_read_back_as_a_finding() {
3336 let mut out = Vec::new();
3337 push_if_outstanding(
3338 &mut out,
3339 ReviewComment {
3340 author: "yukimemi".to_owned(),
3341 path: None,
3342 line: None,
3343 body: format!("{MARKER}\nmagi stopped landing this pull request: 1 check failing"),
3344 },
3345 );
3346 assert!(out.is_empty());
3347 }
3348
3349 fn run_state() -> RunState {
3353 RunState::new(
3354 std::path::PathBuf::from("/repo/magi"),
3355 "main".to_owned(),
3356 "abcdef1234".to_owned(),
3357 "add retries to the uploader".to_owned(),
3358 crate::config::Config::default(),
3359 )
3360 }
3361
3362 fn green_pr() -> PrState {
3363 PrState {
3364 url: "https://github.com/yukimemi/magi/pull/42".to_owned(),
3365 number: 42,
3366 state: PrLifecycle::Open,
3367 checks: Checks::Green,
3368 blocking: Blocking::No,
3370 failing: Vec::new(),
3371 review_comments: vec![ReviewComment {
3372 author: "coderabbitai".to_owned(),
3373 path: Some("src/land.rs".to_owned()),
3374 line: Some(212),
3375 body: "this branch never checks the exit code".to_owned(),
3376 }],
3377 }
3378 }
3379
3380 #[test]
3381 fn github_facing_land_text_is_english_whatever_the_language() {
3382 let mut state = run_state();
3383 state.config.graph.language = "ja".to_owned();
3384 let comment = stop_comment(&state.id, "checks are still red");
3385 assert!(comment.is_ascii(), "{comment}");
3386 assert!(comment.starts_with(MARKER));
3387
3388 let p = fix_prompt(&state, &green_pr(), 1, 2, "red", "");
3389 let ja_at = p.find("Write all prose in ja").unwrap();
3390 let rule_at = p.find(crate::prompt::GITHUB_ENGLISH_HEADING).unwrap();
3391 assert!(ja_at < rule_at, "{p}");
3392 assert!(p.contains("stays in Japanese"), "{p}");
3393
3394 state.config.graph.language = "en".to_owned();
3395 let p = fix_prompt(&state, &green_pr(), 1, 2, "red", "");
3396 assert!(p.contains(crate::prompt::GITHUB_ENGLISH_HEADING), "{p}");
3397 assert!(!p.contains("does not apply"), "{p}");
3398 }
3399
3400 const NUMSTAT: &str = "12\t3\tsrc/land.rs\n40\t1\tsrc/web.rs\n-\t-\tassets/logo.png";
3401
3402 fn panel() -> String {
3403 approval_panel(
3404 &run_state(),
3405 &green_pr(),
3406 NUMSTAT,
3407 "diff --git a/src/land.rs b/src/land.rs\n@@ -1,2 +1,2 @@\n-old line\n+new line\n context",
3408 &[
3409 "land: ask before merging".to_owned(),
3410 "land: colour the diff".to_owned(),
3411 ],
3412 "feat: merge approval from the phone",
3413 )
3414 }
3415
3416 #[test]
3417 fn the_approval_panel_carries_the_whole_case_for_the_merge() {
3418 let html = panel();
3419 for needle in [
3420 "42",
3421 "main",
3422 "src/land.rs",
3423 "src/web.rs",
3424 "assets/logo.png",
3425 "feat: merge approval from the phone",
3426 "land: ask before merging",
3427 "land: colour the diff",
3428 "coderabbitai",
3429 "this branch never checks the exit code",
3430 "green",
3431 ] {
3432 assert!(html.contains(needle), "the panel must state `{needle}`");
3433 }
3434 }
3435
3436 fn winning_candidate(summary: &str) -> Candidate {
3439 Candidate {
3440 index: 0,
3441 label: 'A',
3442 agent: "opus".to_owned(),
3443 branch: "magi/x/A".to_owned(),
3444 worktree: PathBuf::from("/wt/A"),
3445 summary: summary.to_owned(),
3446 stat: String::new(),
3447 files: 1,
3448 commits: 1,
3449 empty: false,
3450 failed: None,
3451 verified_noop: None,
3452 duration_ms: 0,
3453 folded: false,
3454 }
3455 }
3456
3457 fn uncontested_tally() -> Tally {
3458 Tally {
3459 first_choice: BTreeMap::from([('A', 1)]),
3460 borda: BTreeMap::new(),
3461 winner: 'A',
3462 rankings: 1,
3463 unanimous_initial: true,
3464 deliberated: false,
3465 changed_votes: 0,
3466 unanimous_final: true,
3467 tie_break: None,
3468 judges: 1,
3469 present: 1,
3470 quorum: 1,
3471 met_quorum: true,
3472 uncontested: None,
3473 }
3474 }
3475
3476 fn review_record(reviewer: usize, agent: &str, summary: &str) -> ReviewRecord {
3477 ReviewRecord {
3478 attempts: 0,
3479 reviewer,
3480 agent: agent.to_owned(),
3481 summary: summary.to_owned(),
3482 findings: Vec::new(),
3483 vote: None,
3484 failed: None,
3485 duration_ms: 0,
3486 }
3487 }
3488
3489 fn review_round(round: usize, reviews: Vec<ReviewRecord>) -> ReviewRound {
3490 let answered = reviews.len();
3491 ReviewRound {
3492 round,
3493 head: "abc1234".to_owned(),
3494 verified_head: None,
3495 verified_at: None,
3496 reviews,
3497 e2e: Vec::new(),
3498 verify_retried: false,
3499 e2e_deferred: false,
3500 e2e_defer_reason: None,
3501 fix: None,
3502 blocking: 0,
3503 answered,
3504 expected: answered,
3505 clean: true,
3506 progressed: false,
3507 vote_split: false,
3508 reconsideration: Vec::new(),
3509 verdict: None,
3510 }
3511 }
3512
3513 #[test]
3514 fn the_approval_panel_states_the_task_verbatim_in_either_language() {
3515 let en = panel();
3516 assert!(en.contains("Task"), "{en}");
3517 assert!(en.contains("add retries to the uploader"), "{en}");
3518
3519 let mut state = run_state();
3520 state.config.graph.language = "ja".to_owned();
3521 let ja = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3522 assert!(ja.contains("タスク"), "{ja}");
3523 assert!(
3524 ja.contains("add retries to the uploader"),
3525 "the task itself is not translated: {ja}"
3526 );
3527 }
3528
3529 #[test]
3530 fn the_approval_panel_omits_what_changed_and_review_verdict_with_no_data() {
3531 let html = panel();
3535 assert!(!html.contains("What changed"), "{html}");
3536 assert!(!html.contains("Review verdict"), "{html}");
3537 }
3538
3539 #[test]
3540 fn the_approval_panel_omits_what_changed_when_the_winners_summary_is_empty() {
3541 let mut state = run_state();
3542 state.candidates = vec![winning_candidate("")];
3543 state.tally = Some(uncontested_tally());
3544 let html = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3545 assert!(
3546 !html.contains("What changed"),
3547 "an empty summary must not render an empty box: {html}"
3548 );
3549 }
3550
3551 #[test]
3552 fn the_approval_panel_shows_the_winners_own_account_in_either_language() {
3553 let mut state = run_state();
3554 state.candidates = vec![winning_candidate(
3555 "Added a retry loop around the uploader PUT call.",
3556 )];
3557 state.tally = Some(uncontested_tally());
3558 let en = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3559 assert!(en.contains("What changed"), "{en}");
3560 assert!(
3561 en.contains("Added a retry loop around the uploader PUT call."),
3562 "{en}"
3563 );
3564
3565 state.config.graph.language = "ja".to_owned();
3566 let ja = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3567 assert!(ja.contains("変更内容"), "{ja}");
3568 assert!(
3569 ja.contains("Added a retry loop around the uploader PUT call."),
3570 "{ja}"
3571 );
3572 }
3573
3574 #[test]
3575 fn the_approval_panel_shows_only_the_last_review_rounds_verdict() {
3576 let mut state = run_state();
3577 state.reviews = vec![
3578 review_round(
3579 1,
3580 vec![review_record(1, "alpha", "found a race, sent back")],
3581 ),
3582 review_round(2, vec![review_record(1, "alpha", "race is fixed, clean")]),
3583 ];
3584 let en = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3585 assert!(en.contains("Review verdict"), "{en}");
3586 assert!(en.contains("race is fixed, clean"), "{en}");
3587 assert!(
3588 !en.contains("found a race, sent back"),
3589 "only the round that actually cleared the merge should show: {en}"
3590 );
3591
3592 state.config.graph.language = "ja".to_owned();
3593 let ja = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3594 assert!(ja.contains("レビューの結論"), "{ja}");
3595 assert!(ja.contains("レビュアー"), "{ja}");
3596 assert!(ja.contains("race is fixed, clean"), "{ja}");
3597 }
3598
3599 fn unanswered_review_record(reviewer: usize, agent: &str, reason: &str) -> ReviewRecord {
3605 ReviewRecord {
3606 attempts: 0,
3607 reviewer,
3608 agent: agent.to_owned(),
3609 summary: String::new(),
3610 findings: Vec::new(),
3611 vote: None,
3612 failed: Some(reason.to_owned()),
3613 duration_ms: 0,
3614 }
3615 }
3616
3617 #[test]
3618 fn the_approval_panel_never_shows_an_unanswered_seat_as_a_blank_verdict() {
3619 let mut state = run_state();
3620 state.reviews = vec![review_round(
3621 1,
3622 vec![
3623 review_record(1, "alpha", "clean, nothing to add"),
3624 unanswered_review_record(2, "beta", "timed out"),
3625 ],
3626 )];
3627 let en = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3628 assert!(en.contains("clean, nothing to add"), "{en}");
3629 assert!(
3630 en.contains("produced no answer: timed out"),
3631 "a seat that never answered must say so, not render a blank box: {en}"
3632 );
3633 assert!(
3634 !en.contains("<div style=\"white-space:pre-wrap;font-size:13px\"></div>"),
3635 "no reviewer box may be left empty: {en}"
3636 );
3637
3638 state.config.graph.language = "ja".to_owned();
3639 let ja = approval_panel(&state, &green_pr(), NUMSTAT, "", &[], "feat: x");
3640 assert!(ja.contains("回答なし: timed out"), "{ja}");
3641 }
3642
3643 #[test]
3644 fn the_approval_panel_contains_nothing_the_frames_policy_would_block() {
3645 let html = panel();
3646 assert!(!html.contains("<script"), "no script survives the csp");
3647 assert!(!html.contains("<form"), "form-action is 'none'");
3648 let pr = green_pr();
3649 assert_eq!(
3650 html.matches("http").count(),
3651 html.matches(pr.url.as_str()).count(),
3652 "the only http url in the panel is the pull request's own link"
3653 );
3654 }
3655
3656 #[test]
3657 fn added_and_removed_diff_lines_are_distinguishable_without_colour() {
3658 let html = panel();
3659 assert!(
3660 html.contains(">+</span>"),
3661 "an added line carries a `+` in the gutter, not only a background"
3662 );
3663 assert!(
3664 html.contains(">-</span>"),
3665 "a removed line carries a `-` in the gutter, not only a background"
3666 );
3667 assert!(
3668 html.contains(">new line</span>"),
3669 "the marker is moved to the gutter, so the body is printed once without it"
3670 );
3671 }
3672
3673 #[test]
3674 fn a_diff_past_the_threshold_is_cut_with_an_honest_count() {
3675 let total = DIFF_MAX_LINES + 100;
3676 let diff: String = (0..total).map(|i| format!("+line {i}\n")).collect();
3677 let html = approval_panel(
3678 &run_state(),
3679 &green_pr(),
3680 NUMSTAT,
3681 &diff,
3682 &[],
3683 "feat: something long",
3684 );
3685 assert!(
3686 html.contains(&format!("100 of {total} diff lines omitted")),
3687 "the note must say exactly how much was cut"
3688 );
3689 assert!(html.contains(&format!("line {}", DIFF_MAX_LINES - 1)));
3690 assert!(
3691 !html.contains(&format!("line {DIFF_MAX_LINES}")),
3692 "nothing past the threshold is rendered"
3693 );
3694 assert!(
3695 html.contains("/repo/magi"),
3696 "the note says where the rest is"
3697 );
3698 }
3699
3700 #[test]
3701 fn a_path_with_html_metacharacters_is_escaped_rather_than_rendered() {
3702 let html = approval_panel(
3703 &run_state(),
3704 &green_pr(),
3705 "1\t2\tsrc/<b>&\"x\"'.rs",
3706 "",
3707 &[],
3708 "subject",
3709 );
3710 assert!(html.contains("src/<b>&"x"'.rs"));
3711 assert!(
3712 !html.contains("<b>"),
3713 "an agent-influenced path must never become markup"
3714 );
3715 }
3716
3717 #[tokio::test]
3718 async fn the_merge_lock_serialises_one_repository_but_never_a_different_one() {
3719 let a = std::path::PathBuf::from("/repo/a");
3720 let b = std::path::PathBuf::from("/repo/b");
3721
3722 let held = repo_merge_lock(&a).lock_owned().await;
3723
3724 assert!(
3727 repo_merge_lock(&a).try_lock().is_err(),
3728 "a second merge into the same repository must not proceed concurrently"
3729 );
3730
3731 assert!(
3735 repo_merge_lock(&b).try_lock().is_ok(),
3736 "a different repository's merge lock must be independent"
3737 );
3738
3739 drop(held);
3740 assert!(
3741 repo_merge_lock(&a).try_lock().is_ok(),
3742 "the lock is released once the holder is done"
3743 );
3744 }
3745
3746 #[test]
3747 fn only_the_merge_choice_merges_and_silence_holds() {
3748 let table = [
3749 (None, Approval::Hold),
3750 (Some("merge"), Approval::Merge),
3751 (Some(" merge\n"), Approval::Merge),
3752 (Some("hold"), Approval::Hold),
3753 (Some(""), Approval::Hold),
3754 (Some("yes"), Approval::Hold),
3755 ];
3756 for (answer, want) in table {
3757 assert_eq!(
3758 approval(answer),
3759 want,
3760 "answer {answer:?} must resolve to {want:?}"
3761 );
3762 }
3763 }
3764
3765 #[tokio::test]
3766 async fn a_first_visit_to_the_merge_gate_files_a_question_and_returns_pending_at_once() {
3767 crate::run::set_home(std::env::temp_dir().join("magi-land-approval-test-home"));
3768 let mut state = run_state();
3769 state.config.graph.land_approval = true;
3770 let pr = green_pr();
3771
3772 let gate = approval_gate(&mut state, &pr, "feat: x").await.unwrap();
3773 assert_eq!(gate, ApprovalGate::Pending, "nobody has answered yet");
3774 assert!(
3775 !state.parked,
3776 "approval_gate itself never sets `parked`; only its caller does"
3777 );
3778
3779 let store = ask::Questions::open();
3780 let filed: Vec<_> = store
3781 .list()
3782 .into_iter()
3783 .filter(|q| q.run == state.id)
3784 .collect();
3785 assert_eq!(filed.len(), 1, "exactly one question is filed");
3786 assert_eq!(filed[0].node, APPROVAL_NODE);
3787 assert_eq!(filed[0].choices, vec![APPROVE.to_owned(), HOLD.to_owned()]);
3788 assert!(filed[0].status.open());
3789
3790 let again = approval_gate(&mut state, &pr, "feat: x").await.unwrap();
3794 assert_eq!(again, ApprovalGate::Pending);
3795 let still_one = store
3796 .list()
3797 .into_iter()
3798 .filter(|q| q.run == state.id)
3799 .count();
3800 assert_eq!(
3801 still_one, 1,
3802 "asking twice must not double-file the question"
3803 );
3804 }
3805
3806 #[tokio::test]
3807 async fn approving_the_existing_question_is_read_back_as_approved() {
3808 crate::run::set_home(std::env::temp_dir().join("magi-land-approval-test-home"));
3809 let mut state = run_state();
3810 state.config.graph.land_approval = true;
3811 let pr = green_pr();
3812 assert_eq!(
3813 approval_gate(&mut state, &pr, "feat: x").await.unwrap(),
3814 ApprovalGate::Pending
3815 );
3816
3817 let store = ask::Questions::open();
3818 let mut q = store
3819 .list()
3820 .into_iter()
3821 .find(|q| q.run == state.id)
3822 .expect("filed above");
3823 q.answer(ask::Answer::Choice(APPROVE.to_owned())).unwrap();
3824 store.put(&mut q).unwrap();
3825
3826 assert_eq!(
3827 approval_gate(&mut state, &pr, "feat: x").await.unwrap(),
3828 ApprovalGate::Approved
3829 );
3830 }
3831
3832 #[tokio::test]
3833 async fn holding_or_abandoning_the_existing_question_is_read_back_as_held() {
3834 crate::run::set_home(std::env::temp_dir().join("magi-land-approval-test-home"));
3835 let store = ask::Questions::open();
3836
3837 let mut held_state = run_state();
3838 held_state.config.graph.land_approval = true;
3839 let pr = green_pr();
3840 approval_gate(&mut held_state, &pr, "feat: x")
3841 .await
3842 .unwrap();
3843 let mut q = store
3844 .list()
3845 .into_iter()
3846 .find(|q| q.run == held_state.id)
3847 .expect("filed above");
3848 q.answer(ask::Answer::Choice(HOLD.to_owned())).unwrap();
3849 store.put(&mut q).unwrap();
3850 assert_eq!(
3851 approval_gate(&mut held_state, &pr, "feat: x")
3852 .await
3853 .unwrap(),
3854 ApprovalGate::Held
3855 );
3856
3857 let mut abandoned_state = run_state();
3858 abandoned_state.config.graph.land_approval = true;
3859 approval_gate(&mut abandoned_state, &pr, "feat: x")
3860 .await
3861 .unwrap();
3862 let mut q = store
3863 .list()
3864 .into_iter()
3865 .find(|q| q.run == abandoned_state.id)
3866 .expect("filed above");
3867 q.abandon("no answer within the timeout");
3868 store.put(&mut q).unwrap();
3869 assert_eq!(
3870 approval_gate(&mut abandoned_state, &pr, "feat: x")
3871 .await
3872 .unwrap(),
3873 ApprovalGate::Held,
3874 "silence must never merge"
3875 );
3876 }
3877
3878 #[test]
3879 fn the_diffstat_table_is_ordered_by_churn_with_binaries_last() {
3880 let rows = parse_numstat(NUMSTAT);
3881 assert_eq!(
3882 rows.iter().map(|r| r.path.as_str()).collect::<Vec<_>>(),
3883 ["src/web.rs", "src/land.rs", "assets/logo.png"]
3884 );
3885 assert_eq!(rows[2].added, None, "a binary file has no line counts");
3886 }
3887 #[test]
3888 fn the_approval_speaks_the_language_the_repository_is_configured_for() {
3889 let mut state = run_state();
3893 state.config.graph.language = "ja".to_owned();
3894 let pr = green_pr();
3895 let commits = ["c1".to_owned()];
3896
3897 let ja = approval_panel(&state, &pr, "3\t1\tsrc/a.rs", "+ x", &commits, "feat: x");
3898 assert!(ja.contains("lang=\"ja\""), "the document must declare it");
3899 assert!(ja.contains("squash されるコミット"), "{ja}");
3900 assert!(ja.contains("レビューコメント"), "{ja}");
3901 assert!(ja.contains("差分"), "{ja}");
3902 assert!(
3903 !ja.contains("Commits being squashed"),
3904 "no English left over"
3905 );
3906
3907 let w = words("ja");
3908 assert!(w.approval_summary(17, "feat: x").contains("マージ"));
3909 assert!(
3910 w.approval_detail("http://x/1", "main", "feat: x")
3911 .contains("パネル")
3912 );
3913
3914 assert!(ja.contains("src/a.rs"), "the diffstat is not prose");
3916 assert!(ja.contains("feat: x"), "nor is the merge subject");
3917
3918 state.config.graph.language = "en".to_owned();
3921 let en = approval_panel(&state, &pr, "3\t1\tsrc/a.rs", "+ x", &commits, "feat: x");
3922 assert!(en.contains("Commits being squashed"), "{en}");
3923 assert_eq!(words("Klingon").html_lang, "en");
3924 }
3925
3926 #[test]
3930 fn pick_merged_pr_picks_the_unique_match() {
3931 let json = r#"[
3932 {"url": "https://github.com/o/r/pull/42", "number": 42,
3933 "mergedAt": "2026-09-20T10:00:00Z", "baseRefName": "main"}
3934 ]"#;
3935 let created_at: Timestamp = "2026-09-19T00:00:00Z".parse().unwrap();
3936 let found = pick_merged_pr(json, "main", created_at)
3937 .expect("valid json")
3938 .expect("one unambiguous match");
3939 assert_eq!(found.url, "https://github.com/o/r/pull/42");
3940 assert_eq!(found.number, 42);
3941 }
3942
3943 #[test]
3947 fn pick_merged_pr_refuses_when_more_than_one_candidate_survives() {
3948 let json = r#"[
3949 {"url": "https://github.com/o/r/pull/42", "number": 42,
3950 "mergedAt": "2026-09-20T10:00:00Z", "baseRefName": "main"},
3951 {"url": "https://github.com/o/r/pull/43", "number": 43,
3952 "mergedAt": "2026-09-21T10:00:00Z", "baseRefName": "main"}
3953 ]"#;
3954 let created_at: Timestamp = "2026-09-19T00:00:00Z".parse().unwrap();
3955 assert_eq!(pick_merged_pr(json, "main", created_at).unwrap(), None);
3956 }
3957
3958 #[test]
3962 fn pick_merged_pr_ignores_a_different_base_branch() {
3963 let json = r#"[
3964 {"url": "https://github.com/o/r/pull/42", "number": 42,
3965 "mergedAt": "2026-09-20T10:00:00Z", "baseRefName": "release"}
3966 ]"#;
3967 let created_at: Timestamp = "2026-09-19T00:00:00Z".parse().unwrap();
3968 assert_eq!(pick_merged_pr(json, "main", created_at).unwrap(), None);
3969 }
3970
3971 #[test]
3974 fn pick_merged_pr_ignores_a_merge_that_predates_the_run() {
3975 let json = r#"[
3976 {"url": "https://github.com/o/r/pull/42", "number": 42,
3977 "mergedAt": "2026-09-18T10:00:00Z", "baseRefName": "main"}
3978 ]"#;
3979 let created_at: Timestamp = "2026-09-19T00:00:00Z".parse().unwrap();
3980 assert_eq!(pick_merged_pr(json, "main", created_at).unwrap(), None);
3981 }
3982
3983 #[test]
3984 fn slug_of_pr_url_reads_host_owner_and_repo() {
3985 assert_eq!(
3986 slug_of_pr_url("https://github.com/yukimemi/shun/pull/272").as_deref(),
3987 Some("github.com/yukimemi/shun")
3988 );
3989 }
3990
3991 #[test]
3992 fn slug_of_pr_url_refuses_a_url_with_no_pull_segment() {
3993 assert_eq!(slug_of_pr_url("https://github.com/yukimemi/shun"), None);
3994 assert_eq!(slug_of_pr_url("not a url at all"), None);
3995 assert_eq!(slug_of_pr_url("https://github.com"), None);
3996 }
3997
3998 #[test]
3999 fn slug_of_repo_url_reads_host_owner_and_repo() {
4000 assert_eq!(
4001 slug_of_repo_url("https://github.com/yukimemi/magi").as_deref(),
4002 Some("github.com/yukimemi/magi")
4003 );
4004 assert_eq!(slug_of_repo_url("https://github.com"), None);
4005 }
4006
4007 #[test]
4008 fn ensure_same_repo_accepts_a_matching_slug_regardless_of_case() {
4009 ensure_same_repo("github.com/yukimemi/magi", "GitHub.Com/YukiMemi/Magi")
4010 .expect("same repo, different case");
4011 }
4012
4013 #[test]
4018 fn ensure_same_repo_refuses_a_different_repo() {
4019 let err =
4020 ensure_same_repo("github.com/yukimemi/magi", "github.com/yukimemi/shun").unwrap_err();
4021 let msg = format!("{err:#}");
4022 assert!(msg.contains("github.com/yukimemi/magi"), "{msg}");
4023 assert!(msg.contains("github.com/yukimemi/shun"), "{msg}");
4024 }
4025
4026 #[test]
4031 fn ensure_same_repo_refuses_the_same_slug_on_a_different_host() {
4032 let err = ensure_same_repo(
4033 "github.com/yukimemi/magi",
4034 "github.example.com/yukimemi/magi",
4035 )
4036 .unwrap_err();
4037 let msg = format!("{err:#}");
4038 assert!(msg.contains("github.com/yukimemi/magi"), "{msg}");
4039 assert!(msg.contains("github.example.com/yukimemi/magi"), "{msg}");
4040 }
4041
4042 #[tokio::test]
4047 async fn find_external_merge_returns_none_without_a_winner() {
4048 let state = RunState::new(
4049 PathBuf::from("/no/such/repo"),
4050 "main".to_owned(),
4051 "0000000000000000000000000000000000000000".to_owned(),
4052 "irrelevant".to_owned(),
4053 crate::config::Config::default(),
4054 );
4055 assert_eq!(find_external_merge(&state).await.unwrap(), None);
4056 }
4057}