diff --git a/docs/file-length-enforcement-plan.md b/docs/file-length-enforcement-plan.md index 3e260438..c2e19889 100644 --- a/docs/file-length-enforcement-plan.md +++ b/docs/file-length-enforcement-plan.md @@ -311,7 +311,7 @@ Agent 委譲 (general-purpose) を活用、PR-3a の hooks-session-start 分割 #### スコープ - [`src/cli-pr-monitor/src/stages/poll/mod.rs`](../src/cli-pr-monitor/src/stages/poll/mod.rs) (1404 行) -- [`src/cli-pr-monitor/src/fix_commit.rs`](../src/cli-pr-monitor/src/fix_commit.rs) (972 行) +- [`src/cli-pr-monitor/src/fix_commit/`](../src/cli-pr-monitor/src/fix_commit/mod.rs) (972 行) `stages/poll/` は既に sub-module 化されているが `mod.rs` 自身が 1404 行。さらに sub-split が必要 (例: `poll/state_handlers.rs` / `poll/transitions.rs` 等)。 diff --git a/src/cli-pr-monitor/src/fix_commit.rs b/src/cli-pr-monitor/src/fix_commit.rs deleted file mode 100644 index 257b361b..00000000 --- a/src/cli-pr-monitor/src/fix_commit.rs +++ /dev/null @@ -1,972 +0,0 @@ -//! 分離型 fix commit の pre-create と description 生成。 -//! -//! ADR-022 例外条項 (2026-04-20): 自動生成された修正を独立した child commit として -//! 分離する場合に限り、その child commit への description 付与を許可する。 -//! 元 commit (= 人間が意図を込めた初回 PR commit) の description は改変しない。 -//! -//! pre-takt で `jj new -m "..."` により空 child を作成し、takt が `@` を amend する -//! ことで fix 内容が自動的に child commit へ入る仕組み。 - -use lib_report_formatter::Finding; - -use crate::log::log_info; -use crate::runner::{capture_commit_id, diff_at_is_empty, run_cmd_direct, JJ_CMD_TIMEOUT_SECS}; - -/// 分離型 fix commit の状態。 -/// -/// pre-takt で作成を試み、成否を型で表現する。 -/// post-takt の分岐 (re-push / abandon / 放置) で消費される。 -#[derive(Debug, Clone)] -pub(crate) enum FixCommitState { - /// 分離を行わなかった (findings なし、takt 未構成、または作成失敗) - None, - /// fix commit を pre-create 済み - Created { commit_id: String }, -} - -impl FixCommitState { - pub(crate) fn is_created(&self) -> bool { - matches!(self, Self::Created { .. }) - } -} - -/// fix commit の description を生成する。 -/// -/// ADR-022 例外の「新規 child commit への自己記述」として、 -/// - header ラベル: commit 種別を示す -/// - findings summary: 何を問題と捉え、どれを修正したかの文脈 -/// -/// の 2 段構成で返す。findings が空なら header のみ返す。 -pub(crate) fn build_fix_commit_description(pr_number: Option, findings: &[Finding]) -> String { - let header = match pr_number { - Some(n) => format!("fix(review): apply CodeRabbit fixes for #{}", n), - None => "fix(review): apply CodeRabbit fixes".to_string(), - }; - - if findings.is_empty() { - return header; - } - - let mut body = String::with_capacity(256); - body.push_str(&header); - body.push_str("\n\nResolved findings:\n"); - for f in findings { - let issue_oneline = sanitize_to_oneline(&f.issue); - body.push_str(&format!( - "- [{}] {}:{} {}\n", - f.severity, f.file, f.line, issue_oneline - )); - } - body.trim_end().to_string() -} - -/// CodeRabbit の `issue` フィールドは複数行になることがあるため、 -/// `build_fix_commit_description` のリスト項目に埋める前に単行化する。 -fn sanitize_to_oneline(input: &str) -> String { - input.split_whitespace().collect::>().join(" ") -} - -/// pre-takt で fix commit を新規作成する (`jj new -m "..."`)。 -/// -/// 成功時: `FixCommitState::Created { commit_id }` を返す。@ は空 child を指す状態。 -/// 失敗時: `FixCommitState::None` を返す (fallback = 分離なしで元の flow へフォールバック)。 -/// -/// `jj new` が成功したが `capture_commit_id` で commit id を追跡できない場合は、 -/// 作成済みの空 child が orphan にならないよう即座に abandon を試みる -/// (fail-safe: 追跡不能 child を remote に残さない)。 -pub(crate) fn create_fix_commit(pr_number: Option, findings: &[Finding]) -> FixCommitState { - let desc = build_fix_commit_description(pr_number, findings); - let (ok, output) = run_cmd_direct("jj", &["new", "-m", &desc], &[], JJ_CMD_TIMEOUT_SECS); - if !ok { - log_info(&format!( - "[action] fix commit 分離 skip: jj new 失敗: {}", - output - )); - return FixCommitState::None; - } - match capture_commit_id() { - Some(cid) => { - log_info(&format!("[state] fix commit pre-created: {}", cid)); - FixCommitState::Created { commit_id: cid } - } - None => { - log_info( - "[state] fix commit 作成後の commit id capture 失敗 (orphan child を cleanup)", - ); - try_abandon_empty_fix_commit("create_fix_commit id capture 失敗:", None); - FixCommitState::None - } - } -} - -/// 空 fix commit を安全に abandon する。 -/// -/// `commit_id` が `Some(expected)` のとき: 現在の `@` が `expected` と一致する場合のみ -/// abandon を実行する。不一致または capture 失敗時は `[warn]` を出してスキップする。 -/// `commit_id` が `None` のとき: 従来通り diff チェックのみで判定する。 -/// -/// diff あり判定失敗時は abandon をスキップ (fail-safe: 誤 abandon 防止)。 -/// -/// abandon 成功後は `reparent_at_to_pr_tip` で `@` を PR tip 直下に戻す -/// (task 6: cleanup 後の @ 孤児化を解消)。 -pub(crate) fn try_abandon_empty_fix_commit(context: &str, commit_id: Option<&str>) { - if let Some(expected) = commit_id { - match capture_commit_id().as_deref() { - Some(current) if current == expected => {} - Some(current) => { - log_info(&format!( - "[warn] {} expected={}, current={} abandon を見送り", - context, expected, current - )); - return; - } - None => { - log_info(&format!( - "[warn] {} expected={}, current= abandon を見送り", - context, expected - )); - return; - } - } - } - - if diff_at_is_empty() { - let label = commit_id.map_or_else(String::new, |id| format!(" ({})", id)); - log_info(&format!( - "[action] {} 空 fix commit を abandon{}", - context, label - )); - let (ok, out) = run_cmd_direct("jj", &["abandon"], &[], JJ_CMD_TIMEOUT_SECS); - if !ok { - log_info(&format!( - "[action] jj abandon 失敗 (手動片付け推奨): {}", - out - )); - return; - } - reparent_at_to_pr_tip(context); - } else { - log_info(&format!( - "[warn] {} fix commit に diff あり、abandon を見送り", - context - )); - } -} - -/// `@` を PR tip (単一 local bookmark の指す commit) 直下に再配置する。 -/// -/// `jj abandon` 直後の `@` は stale な空 commit の上に残ることがあり -/// (task 6 背景: PR #64 で 3 回発生)、次の `jj new` がそこに積まれる。 -/// これを解消するため、bookmark が指す PR tip を解決して `jj new -r ` で -/// `@` を PR tip の直接子に戻す。 -/// -/// 以下のケースは fail-safe でスキップする: -/// - PR tip 解決失敗 (bookmark なし / 複数 bookmark で曖昧 / 取得失敗) -/// - 既に `@-` が PR tip と一致 (redundant な空 commit を作らない) -/// - `jj new -r ` 自体の失敗 (ログのみで処理を継続) -fn reparent_at_to_pr_tip(context: &str) { - let pr_tip = match crate::stages::push_jj_bookmark::resolve_pr_tip_commit_id() { - Some(id) => id, - None => { - log_info(&format!( - "[state] {} PR tip bookmark を特定できず re-parent スキップ", - context - )); - return; - } - }; - - if parent_commit_id_is(&pr_tip) { - log_info(&format!( - "[state] {} @ は既に PR tip ({}) 直下、re-parent 不要", - context, pr_tip - )); - return; - } - - let (ok, out) = run_cmd_direct("jj", &["new", "-r", &pr_tip], &[], JJ_CMD_TIMEOUT_SECS); - if ok { - log_info(&format!( - "[action] {} @ を PR tip ({}) 直下に re-parent", - context, pr_tip - )); - } else { - log_info(&format!( - "[action] {} @ の re-parent 失敗 (手動対応): {}", - context, out - )); - } -} - -/// `default_branch..@` 範囲の `fix(review):` 空 commit を sweep して全て abandon する (順位 155、PR #174 T1-#1)。 -/// -/// 既存 `try_abandon_empty_fix_commit` が tracked な単一 fix commit (= 直近 `create_fix_commit` -/// の戻り値) のみを対象とするのに対し、本関数は PR 範囲全体を sweep して -/// **untracked な空 commit** を網羅的に拾う。PR #174 で観測した `kqvluqyv` 事例 -/// (過去 fix loop で取りこぼされた granduncle 位置の空 commit が後続 push で PR diff 汚染) の -/// 構造的予防層。 -/// -/// 実装: jj revset `empty() & description("fix(review):") & (default_branch..@)` で範囲内の -/// fix(review): 空 commit を 1 step で列挙し、change_id ベースで順次 `jj abandon` する。 -/// change_id は jj の永続識別子のため、複数 abandon で graph が rebase されても残りの id 参照は invariant。 -/// description フィルタにより `create_fix_commit` 由来のコミットのみを対象とし、他の空コミットは除外する。 -/// -/// fail-open: jj log / abandon の失敗時は warn ログのみで cleanup を継続する -/// (push を block すると fix loop 全体が止まるため、ローカル副作用は次回再走で吸収する方針)。 -pub(crate) fn sweep_empty_commits_in_pr_range(default_branch: &str) { - let revset = format!( - "empty() & description(substring:\"fix(review):\") & ({}..@)", - default_branch - ); - let (ok, out) = run_cmd_direct( - "jj", - &[ - "log", - "-r", - &revset, - "--no-graph", - "-T", - "change_id ++ \"\\n\"", - ], - &[], - JJ_CMD_TIMEOUT_SECS, - ); - if !ok { - log_info(&format!( - "[warn] sweep_empty_commits: jj log 失敗 (sweep skip): {}", - out.trim() - )); - return; - } - - let change_ids = parse_empty_change_ids(&out); - if change_ids.is_empty() { - return; - } - log_info(&format!( - "[action] sweep_empty_commits: {}..@ 範囲に fix(review): 空 commit {} 件を検出 → abandon", - default_branch, - change_ids.len() - )); - for cid in &change_ids { - let (ok, out) = run_cmd_direct("jj", &["abandon", cid], &[], JJ_CMD_TIMEOUT_SECS); - if !ok { - log_info(&format!( - "[warn] sweep_empty_commits: jj abandon {} 失敗 (継続): {}", - cid, - out.trim() - )); - continue; - } - log_info(&format!("[action] sweep_empty_commits: abandoned {}", cid)); - } -} - -/// `jj log` 出力 (1 行 1 change_id) を parse する純関数。空行と前後空白を除去する。 -fn parse_empty_change_ids(log_output: &str) -> Vec { - log_output - .lines() - .map(|l| l.trim().to_string()) - .filter(|s| !s.is_empty()) - .collect() -} - -/// `@-` (親 commit) の id が `expected` と一致するか判定する。 -/// 取得失敗時は `false` (= 不一致扱いで reparent を試行) を返す。 -fn parent_commit_id_is(expected: &str) -> bool { - let (ok, out) = run_cmd_direct( - "jj", - &["log", "-r", "@-", "--no-graph", "-T", "commit_id"], - &[], - JJ_CMD_TIMEOUT_SECS, - ); - ok && out.trim() == expected -} - -#[cfg(test)] -mod tests { - //! jj integration test の不変式パターン (PR #194 T2-#3 codified、 - //! `~/.claude/rules/common/testing.md` § "jj 操作コードの integration test pattern" と対): - //! - //! - NG: `count_empty_in_pr_range(repo_dir) == 0` 等の count-based assert - //! → jj は abandon 後に空 WC を自動生成するため、count は意図通り減らず false failure を起こす - //! - OK: `assert_descriptions_absent_in_pr_range(repo_dir, default_branch, &[target_desc])` の description-based assert - //! → 明示的に投入した description は auto-generated WC と区別できる - //! - sentinel 事前投入: 「mutation が発生していない」を assert する場合、本来残るべき commit を - //! `assert_descriptions_present_in_pr_range` で生存確認すると no-op vs no-mutation の偽陽性を防げる - - use super::*; - - fn finding(severity: &str, file: &str, line: &str, issue: &str) -> Finding { - Finding { - severity: severity.to_string(), - file: file.to_string(), - line: line.to_string(), - issue: issue.to_string(), - suggestion: String::new(), - source: "CodeRabbit".to_string(), - } - } - - #[test] - fn description_without_findings_is_header_only() { - let desc = build_fix_commit_description(Some(42), &[]); - assert_eq!(desc, "fix(review): apply CodeRabbit fixes for #42"); - } - - #[test] - fn description_without_pr_number_falls_back_to_generic_header() { - let desc = build_fix_commit_description(None, &[]); - assert_eq!(desc, "fix(review): apply CodeRabbit fixes"); - } - - #[test] - fn description_with_findings_includes_summary_block() { - let fs = vec![ - finding("Major", "src/foo.rs", "12", "null pointer"), - finding("Minor", "src/bar.rs", "34", "unused variable"), - ]; - let desc = build_fix_commit_description(Some(42), &fs); - assert!( - desc.starts_with("fix(review): apply CodeRabbit fixes for #42\n\nResolved findings:\n") - ); - assert!(desc.contains("- [Major] src/foo.rs:12 null pointer")); - assert!(desc.contains("- [Minor] src/bar.rs:34 unused variable")); - assert!(!desc.ends_with('\n')); - } - - #[test] - fn description_with_findings_without_pr_number() { - let fs = vec![finding("Major", "a.rs", "1", "issue")]; - let desc = build_fix_commit_description(None, &fs); - assert!(desc.starts_with("fix(review): apply CodeRabbit fixes\n\n")); - assert!(desc.contains("- [Major] a.rs:1 issue")); - } - - #[test] - fn description_sanitizes_multiline_issue_into_single_line() { - let fs = vec![finding( - "Major", - "src/foo.rs", - "10", - "first line\nsecond line\r\nthird line", - )]; - let desc = build_fix_commit_description(Some(1), &fs); - assert!( - desc.contains("- [Major] src/foo.rs:10 first line second line third line"), - "multi-line issue が単行化されていない: {:?}", - desc - ); - let bullet_lines: Vec<_> = desc.lines().filter(|l| l.starts_with("- ")).collect(); - assert_eq!(bullet_lines.len(), 1, "bullet は 1 行のみ: {:?}", desc); - } - - #[test] - fn sanitize_to_oneline_preserves_single_spacing_and_trims() { - assert_eq!(sanitize_to_oneline("a b\nc\td"), "a b c d"); - assert_eq!(sanitize_to_oneline(" leading "), "leading"); - assert_eq!(sanitize_to_oneline(""), ""); - } - - /// 統合: `create_fix_commit` の fail-safe cleanup 動作を確認する。 - /// - /// `capture_commit_id` 失敗を直接 inject できないため、代わりに - /// `try_abandon_empty_fix_commit(_, None)` を直接呼んで「空 child が cleanup される」 - /// 挙動 (= None 分岐が依拠する唯一の副作用) が jj で動くことを確認する。 - #[test] - #[ignore = "integration: requires jj in PATH; run via `cargo test -- --ignored --test-threads=1`"] - fn integration_try_abandon_empty_fix_commit_without_id_drops_orphan_child() { - use std::env; - use std::process::Command as StdCommand; - - let temp = tempfile::tempdir().expect("tempdir 作成失敗"); - let repo_dir = temp.path(); - - assert!(StdCommand::new("jj") - .args(["git", "init"]) - .current_dir(repo_dir) - .status() - .expect("jj git init 失敗") - .success()); - - std::fs::write(repo_dir.join("a.txt"), "x\n").expect("write failed"); - let original_msg = "feat: original"; - assert!(StdCommand::new("jj") - .args(["describe", "-m", original_msg]) - .current_dir(repo_dir) - .status() - .expect("describe") - .success()); - - assert!(StdCommand::new("jj") - .args(["new", "-m", "fix(review): orphan test"]) - .current_dir(repo_dir) - .status() - .expect("jj new") - .success()); - - let original_cwd = env::current_dir().expect("cwd"); - env::set_current_dir(repo_dir).expect("cd"); - // panic-safe cwd restore - struct CwdRestore { - original: std::path::PathBuf, - } - impl Drop for CwdRestore { - fn drop(&mut self) { - let _ = std::env::set_current_dir(&self.original); - } - } - let _guard = CwdRestore { - original: original_cwd, - }; - - try_abandon_empty_fix_commit("test:", None); - - let log_out = StdCommand::new("jj") - .args([ - "log", - "-r", - "::@", - "--no-graph", - "-T", - "description ++ \"\\n\"", - ]) - .current_dir(repo_dir) - .output() - .expect("jj log"); - let log_str = String::from_utf8_lossy(&log_out.stdout); - assert!( - !log_str.contains("fix(review): orphan test"), - "orphan child が abandon されていない: {:?}", - log_str - ); - assert!( - log_str.contains(original_msg), - "元 commit が残っていること: {:?}", - log_str - ); - } - - #[test] - fn parse_empty_change_ids_handles_empty_input() { - assert!(parse_empty_change_ids("").is_empty()); - } - - #[test] - fn parse_empty_change_ids_extracts_single_id() { - let out = "abc123def\n"; - assert_eq!(parse_empty_change_ids(out), vec!["abc123def".to_string()]); - } - - #[test] - fn parse_empty_change_ids_extracts_multiple_ids() { - let out = "abc\ndef\nghi\n"; - assert_eq!( - parse_empty_change_ids(out), - vec!["abc".to_string(), "def".to_string(), "ghi".to_string()] - ); - } - - #[test] - fn parse_empty_change_ids_skips_blank_lines_and_whitespace() { - let out = " abc \n\n \ndef\n\n"; - assert_eq!( - parse_empty_change_ids(out), - vec!["abc".to_string(), "def".to_string()] - ); - } - - fn setup_jj_repo_with_master_at_base(base_msg: &str) -> tempfile::TempDir { - use std::process::Command as StdCommand; - let temp = tempfile::tempdir().expect("tempdir 作成失敗"); - let repo_dir = temp.path(); - assert!(StdCommand::new("jj") - .args(["git", "init"]) - .current_dir(repo_dir) - .status() - .expect("jj git init") - .success()); - std::fs::write(repo_dir.join("base.txt"), "content\n").expect("write base"); - assert!(StdCommand::new("jj") - .args(["describe", "-m", base_msg]) - .current_dir(repo_dir) - .status() - .expect("describe base") - .success()); - assert!(StdCommand::new("jj") - .args(["bookmark", "create", "master", "-r", "@"]) - .current_dir(repo_dir) - .status() - .expect("bookmark master") - .success()); - temp - } - - struct CwdGuard { - original: std::path::PathBuf, - } - impl Drop for CwdGuard { - fn drop(&mut self) { - let _ = std::env::set_current_dir(&self.original); - } - } - - fn enter_repo(repo_dir: &std::path::Path) -> CwdGuard { - let original = std::env::current_dir().expect("cwd"); - std::env::set_current_dir(repo_dir).expect("cd"); - CwdGuard { original } - } - - /// `jj new -m ` で空 commit を作成する test helper。 - /// integration test での空 commit 列挙を 1 行で書けるようにする。 - fn build_jj_empty_with_description(repo_dir: &std::path::Path, description: &str) { - let status = std::process::Command::new("jj") - .args(["new", "-m", description]) - .current_dir(repo_dir) - .status() - .expect("jj new"); - assert!(status.success(), "jj new failed for: {}", description); - } - - /// `master` bookmark を `branch_name` にリネームする test helper。 - /// alternative default_branch test の前処理として利用。 - fn rename_master_bookmark(repo_dir: &std::path::Path, branch_name: &str) { - let status = std::process::Command::new("jj") - .args(["bookmark", "rename", "master", branch_name]) - .current_dir(repo_dir) - .status() - .expect("jj bookmark rename"); - assert!(status.success(), "rename master -> {}", branch_name); - } - - /// 指定 description を持つ commit が PR 範囲に残存していることを assert する。 - /// (negative case 用 = abandon されてはいけない sentinel commit の生存確認) - fn assert_descriptions_present_in_pr_range( - repo_dir: &std::path::Path, - default_branch: &str, - descriptions: &[&str], - ) { - let revset = format!("{}..@", default_branch); - let out = std::process::Command::new("jj") - .args([ - "log", - "-r", - &revset, - "--no-graph", - "-T", - "description ++ \"\\n\"", - ]) - .current_dir(repo_dir) - .output() - .expect("jj log"); - let log_str = String::from_utf8_lossy(&out.stdout); - for d in descriptions { - assert!( - log_str.contains(d), - "{:?} が残存している前提だが消えている: {:?}", - d, - log_str - ); - } - } - - fn assert_descriptions_absent_in_pr_range( - repo_dir: &std::path::Path, - default_branch: &str, - descriptions: &[&str], - ) { - let revset = format!("{}..@", default_branch); - let out = std::process::Command::new("jj") - .args([ - "log", - "-r", - &revset, - "--no-graph", - "-T", - "description ++ \"\\n\"", - ]) - .current_dir(repo_dir) - .output() - .expect("jj log"); - let log_str = String::from_utf8_lossy(&out.stdout); - for d in descriptions { - assert!( - !log_str.contains(d), - "{:?} が abandon されている前提だが残存: {:?}", - d, - log_str - ); - } - } - - fn count_empty_in_pr_range(repo_dir: &std::path::Path, default_branch: &str) -> usize { - let revset = format!("empty() & ({}..@)", default_branch); - let out = std::process::Command::new("jj") - .args([ - "log", - "-r", - &revset, - "--no-graph", - "-T", - "change_id ++ \"\\n\"", - ]) - .current_dir(repo_dir) - .output() - .expect("jj log"); - String::from_utf8_lossy(&out.stdout) - .lines() - .filter(|l| !l.trim().is_empty()) - .count() - } - - /// 統合: PR 範囲 (`..@`) に空 commit が無いとき sweep は no-op (非空 commit を保持)。 - #[test] - #[ignore = "integration: requires jj in PATH; run via `cargo test -- --ignored --test-threads=1`"] - fn integration_sweep_empty_commits_no_op_when_no_empty_in_range() { - let temp = setup_jj_repo_with_master_at_base("feat: real change"); - let repo_dir = temp.path(); - let _guard = enter_repo(repo_dir); - - sweep_empty_commits_in_pr_range("master"); - - let log_out = std::process::Command::new("jj") - .args(["log", "-r", "::@", "--no-graph", "-T", "description"]) - .current_dir(repo_dir) - .output() - .expect("jj log"); - let log_str = String::from_utf8_lossy(&log_out.stdout); - assert!( - log_str.contains("feat: real change"), - "non-empty commit が保持されていること: {:?}", - log_str - ); - } - - /// 統合: PR 範囲 (`..@`) の複数空 commit を sweep が全て abandon する。 - /// PR #174 `kqvluqyv` 事例の最小再現。 - #[test] - #[ignore = "integration: requires jj in PATH; run via `cargo test -- --ignored --test-threads=1`"] - fn integration_sweep_empty_commits_abandons_multiple_in_range() { - use std::process::Command as StdCommand; - let temp = setup_jj_repo_with_master_at_base("feat: base"); - let repo_dir = temp.path(); - - for label in &["fix(review): empty 1", "fix(review): empty 2"] { - build_jj_empty_with_description(repo_dir, label); - } - assert!( - count_empty_in_pr_range(repo_dir, "master") >= 2, - "前提: sweep 前に空 commit が 2 件以上" - ); - - let _guard = enter_repo(repo_dir); - sweep_empty_commits_in_pr_range("master"); - - assert_descriptions_absent_in_pr_range( - repo_dir, - "master", - &["fix(review): empty 1", "fix(review): empty 2"], - ); - - let master_out = StdCommand::new("jj") - .args(["log", "-r", "master", "--no-graph", "-T", "description"]) - .current_dir(repo_dir) - .output() - .expect("jj log master"); - let master_desc = String::from_utf8_lossy(&master_out.stdout); - assert!( - master_desc.contains("feat: base"), - "master commit (非空) は abandon されない: {:?}", - master_desc - ); - } - - /// 統合 (PR #194 T2-#2 variant 1): non-`fix(review):` 系の空 commit (`feat:` / `docs:` / `chore:` 等) - /// は sweep 対象外であることを assert (description filter の negative case)。 - /// fix(review): empty が混在しても誤 abandon されないことが保証される。 - #[test] - #[ignore = "integration: requires jj in PATH; run via `cargo test -- --ignored --test-threads=1`"] - fn integration_sweep_skips_non_fix_review_empty_commits() { - let temp = setup_jj_repo_with_master_at_base("feat: base"); - let repo_dir = temp.path(); - - build_jj_empty_with_description(repo_dir, "feat: empty 1"); - build_jj_empty_with_description(repo_dir, "docs: empty 2"); - build_jj_empty_with_description(repo_dir, "chore: empty 3"); - build_jj_empty_with_description(repo_dir, "fix(review): empty matched"); - - let _guard = enter_repo(repo_dir); - sweep_empty_commits_in_pr_range("master"); - - assert_descriptions_present_in_pr_range( - repo_dir, - "master", - &["feat: empty 1", "docs: empty 2", "chore: empty 3"], - ); - assert_descriptions_absent_in_pr_range(repo_dir, "master", &["fix(review): empty matched"]); - } - - /// 統合 (PR #194 T2-#2 variant 2): default_branch を `main` 等の alternative 名で - /// 指定したとき、revset がパラメータ化されて該当範囲のみ対象になることを assert。 - /// `SweepConfig.default_branch` 設定可能化の dogfood。 - #[test] - #[ignore = "integration: requires jj in PATH; run via `cargo test -- --ignored --test-threads=1`"] - fn integration_sweep_respects_alternative_default_branch() { - let temp = setup_jj_repo_with_master_at_base("feat: base"); - let repo_dir = temp.path(); - rename_master_bookmark(repo_dir, "main"); - - build_jj_empty_with_description(repo_dir, "fix(review): empty under main"); - - assert!( - count_empty_in_pr_range(repo_dir, "main") >= 1, - "前提: sweep 前に 'main' 範囲で空 commit が 1 件以上 (helper の default_branch 引数が main で機能していること)" - ); - - let _guard = enter_repo(repo_dir); - sweep_empty_commits_in_pr_range("main"); - - assert_descriptions_absent_in_pr_range(repo_dir, "main", &["fix(review): empty under main"]); - } - - /// 統合 (PR #194 T2-#2 variant 3): `fix(review):` 空 commit が 0 件のとき、 - /// 他 description の空 commit が範囲内に存在しても sweep が abandon を 1 件も発行しない - /// (description filter の早期 return path)。 - #[test] - #[ignore = "integration: requires jj in PATH; run via `cargo test -- --ignored --test-threads=1`"] - fn integration_sweep_no_op_when_only_non_fix_review_empties_present() { - let temp = setup_jj_repo_with_master_at_base("feat: base"); - let repo_dir = temp.path(); - - build_jj_empty_with_description(repo_dir, "feat: only feat empty"); - build_jj_empty_with_description(repo_dir, "docs: only docs empty"); - - let _guard = enter_repo(repo_dir); - sweep_empty_commits_in_pr_range("master"); - - assert_descriptions_present_in_pr_range( - repo_dir, - "master", - &["feat: only feat empty", "docs: only docs empty"], - ); - } - - #[test] - fn fix_commit_state_is_created_truth_table() { - assert!(!FixCommitState::None.is_created()); - assert!(FixCommitState::Created { - commit_id: "abc".into() - } - .is_created()); - } - - /// 統合: task 6 の再現 — `pnpm push` 後の空 WC の上に fix commit が - /// 作られた状態で cleanup すると、`@` が stale な空 commit に残らず、 - /// PR tip (bookmark の指す commit) 直下に自動で re-parent されることを確認する。 - /// - /// 検証対象シナリオ (PR #64 で 3 回発生): - /// - `C1 (bookmark) ← C1' (empty, from pnpm push) ← Y (fix commit, @)` - /// - takt が NoChange で Y を abandon した後、従来は `@- == C1'` に残っていた - /// - 修正後は `@- == C1` (PR tip) に戻る - #[test] - #[ignore = "integration: requires jj in PATH; run via `cargo test -- --ignored --test-threads=1`"] - fn integration_try_abandon_reparents_at_to_pr_tip_after_cleanup() { - use std::env; - use std::process::Command as StdCommand; - - let temp = tempfile::tempdir().expect("tempdir 作成失敗"); - let repo_dir = temp.path(); - - assert!(StdCommand::new("jj") - .args(["git", "init"]) - .current_dir(repo_dir) - .status() - .expect("jj git init 失敗") - .success()); - - // 1. C1: 実コンテンツを持つ commit (PR 本体に相当) - std::fs::write(repo_dir.join("a.txt"), "content\n").expect("write a.txt 失敗"); - assert!(StdCommand::new("jj") - .args(["describe", "-m", "feat: PR body"]) - .current_dir(repo_dir) - .status() - .expect("describe C1 失敗") - .success()); - let c1_id = { - let out = StdCommand::new("jj") - .args(["log", "-r", "@", "--no-graph", "-T", "commit_id"]) - .current_dir(repo_dir) - .output() - .expect("jj log C1"); - String::from_utf8_lossy(&out.stdout).trim().to_string() - }; - assert!(!c1_id.is_empty()); - - // 2. bookmark feat/task6 を C1 に作成 (PR tip として resolve される対象) - assert!(StdCommand::new("jj") - .args(["bookmark", "create", "feat/task6", "-r", "@"]) - .current_dir(repo_dir) - .status() - .expect("bookmark create 失敗") - .success()); - - // 3. C1': `pnpm push` 相当で @ を空 child に移す - assert!(StdCommand::new("jj") - .args(["new"]) - .current_dir(repo_dir) - .status() - .expect("jj new (C1') 失敗") - .success()); - - // 4. cwd を tempdir に切り替え (cli-pr-monitor helpers は cwd 依存) - let original_cwd = env::current_dir().expect("cwd 取得失敗"); - env::set_current_dir(repo_dir).expect("cd 失敗"); - struct CwdRestore { - original: std::path::PathBuf, - } - impl Drop for CwdRestore { - fn drop(&mut self) { - let _ = std::env::set_current_dir(&self.original); - } - } - let _guard = CwdRestore { - original: original_cwd, - }; - - // 5. Y: fix commit を pre-create (cli-pr-monitor の pre-takt 相当) - let fix_state = create_fix_commit(Some(64), &[]); - let fix_cid = match &fix_state { - FixCommitState::Created { commit_id } => commit_id.clone(), - _ => panic!("create_fix_commit 失敗: {:?}", fix_state), - }; - - // 6. takt no-op: ファイル変更なし → @ は空 Y のまま - - // 7. cleanup 実行: abandon + reparent - try_abandon_empty_fix_commit("test:", Some(&fix_cid)); - - // 8. 検証: @- は PR tip (C1) と一致する。stale な C1' 上に残っていない。 - let parent_id = { - let out = StdCommand::new("jj") - .args(["log", "-r", "@-", "--no-graph", "-T", "commit_id"]) - .current_dir(repo_dir) - .output() - .expect("jj log @-"); - String::from_utf8_lossy(&out.stdout).trim().to_string() - }; - assert_eq!( - parent_id, c1_id, - "@- が PR tip (bookmark feat/task6) の指す commit と一致すること: got={:?}", - parent_id - ); - - // 9. @ は空 WC (新規作成されたもの) - assert!(diff_at_is_empty(), "reparent 後の @ は空 WC"); - - // 10. bookmark は C1 から動いていない (reparent は bookmark を触らない) - let bookmark_tip = { - let out = StdCommand::new("jj") - .args(["log", "-r", "feat/task6", "--no-graph", "-T", "commit_id"]) - .current_dir(repo_dir) - .output() - .expect("jj log bookmark"); - String::from_utf8_lossy(&out.stdout).trim().to_string() - }; - assert_eq!( - bookmark_tip, c1_id, - "bookmark が動かされていないこと: got={:?}", - bookmark_tip - ); - } - - /// 統合: bookmark が複数ある場合 (stacked PR 等) は reparent をスキップし、 - /// `jj abandon` のデフォルト配置 (親の上に新規 WC) に任せる fail-safe 挙動を確認する。 - #[test] - #[ignore = "integration: requires jj in PATH; run via `cargo test -- --ignored --test-threads=1`"] - fn integration_try_abandon_skips_reparent_with_multiple_bookmarks() { - use std::env; - use std::process::Command as StdCommand; - - let temp = tempfile::tempdir().expect("tempdir 作成失敗"); - let repo_dir = temp.path(); - - assert!(StdCommand::new("jj") - .args(["git", "init"]) - .current_dir(repo_dir) - .status() - .expect("jj git init 失敗") - .success()); - - std::fs::write(repo_dir.join("a.txt"), "content\n").expect("write 失敗"); - assert!(StdCommand::new("jj") - .args(["describe", "-m", "feat: base"]) - .current_dir(repo_dir) - .status() - .expect("describe 失敗") - .success()); - - // 複数の非 trunk bookmark を作成 (ambiguous な状態) - for name in &["feat/stack-a", "feat/stack-b"] { - assert!(StdCommand::new("jj") - .args(["bookmark", "create", name, "-r", "@"]) - .current_dir(repo_dir) - .status() - .expect("bookmark create 失敗") - .success()); - } - - // 空 child (pnpm push 相当) を作成 - assert!(StdCommand::new("jj") - .args(["new"]) - .current_dir(repo_dir) - .status() - .expect("jj new 失敗") - .success()); - let c1_prime_id = { - let out = StdCommand::new("jj") - .args(["log", "-r", "@", "--no-graph", "-T", "commit_id"]) - .current_dir(repo_dir) - .output() - .expect("jj log"); - String::from_utf8_lossy(&out.stdout).trim().to_string() - }; - - let original_cwd = env::current_dir().expect("cwd 取得失敗"); - env::set_current_dir(repo_dir).expect("cd 失敗"); - struct CwdRestore { - original: std::path::PathBuf, - } - impl Drop for CwdRestore { - fn drop(&mut self) { - let _ = std::env::set_current_dir(&self.original); - } - } - let _guard = CwdRestore { - original: original_cwd, - }; - - let fix_state = create_fix_commit(Some(1), &[]); - let fix_cid = match &fix_state { - FixCommitState::Created { commit_id } => commit_id.clone(), - _ => panic!("create_fix_commit 失敗"), - }; - - try_abandon_empty_fix_commit("test:", Some(&fix_cid)); - - // 複数 bookmark なので reparent スキップ。@- は stale な C1' (fix の元親) のまま - // = jj abandon のデフォルト配置に委ねられる。 - let parent_id = { - let out = StdCommand::new("jj") - .args(["log", "-r", "@-", "--no-graph", "-T", "commit_id"]) - .current_dir(repo_dir) - .output() - .expect("jj log @-"); - String::from_utf8_lossy(&out.stdout).trim().to_string() - }; - assert_eq!( - parent_id, c1_prime_id, - "複数 bookmark 時は reparent スキップ、@- は C1' のまま: got={:?}", - parent_id - ); - } -} diff --git a/src/cli-pr-monitor/src/fix_commit/abandon.rs b/src/cli-pr-monitor/src/fix_commit/abandon.rs new file mode 100644 index 00000000..479f7888 --- /dev/null +++ b/src/cli-pr-monitor/src/fix_commit/abandon.rs @@ -0,0 +1,428 @@ +use lib_report_formatter::Finding; + +use crate::log::log_info; +use crate::runner::{capture_commit_id, diff_at_is_empty, run_cmd_direct, JJ_CMD_TIMEOUT_SECS}; + +use super::description::{build_fix_commit_description, FixCommitState}; + +/// pre-takt で fix commit を新規作成する (`jj new -m "..."`)。 +/// +/// 成功時: `FixCommitState::Created { commit_id }` を返す。@ は空 child を指す状態。 +/// 失敗時: `FixCommitState::None` を返す (fallback = 分離なしで元の flow へフォールバック)。 +/// +/// `jj new` が成功したが `capture_commit_id` で commit id を追跡できない場合は、 +/// 作成済みの空 child が orphan にならないよう即座に abandon を試みる +/// (fail-safe: 追跡不能 child を remote に残さない)。 +pub(crate) fn create_fix_commit(pr_number: Option, findings: &[Finding]) -> FixCommitState { + let desc = build_fix_commit_description(pr_number, findings); + let (ok, output) = run_cmd_direct("jj", &["new", "-m", &desc], &[], JJ_CMD_TIMEOUT_SECS); + if !ok { + log_info(&format!( + "[action] fix commit 分離 skip: jj new 失敗: {}", + output + )); + return FixCommitState::None; + } + match capture_commit_id() { + Some(cid) => { + log_info(&format!("[state] fix commit pre-created: {}", cid)); + FixCommitState::Created { commit_id: cid } + } + None => { + log_info( + "[state] fix commit 作成後の commit id capture 失敗 (orphan child を cleanup)", + ); + try_abandon_empty_fix_commit("create_fix_commit id capture 失敗:", None); + FixCommitState::None + } + } +} + +/// 空 fix commit を安全に abandon する。 +/// +/// `commit_id` が `Some(expected)` のとき: 現在の `@` が `expected` と一致する場合のみ +/// abandon を実行する。不一致または capture 失敗時は `[warn]` を出してスキップする。 +/// `commit_id` が `None` のとき: 従来通り diff チェックのみで判定する。 +/// +/// diff あり判定失敗時は abandon をスキップ (fail-safe: 誤 abandon 防止)。 +/// +/// abandon 成功後は `reparent_at_to_pr_tip` で `@` を PR tip 直下に戻す +/// (task 6: cleanup 後の @ 孤児化を解消)。 +pub(crate) fn try_abandon_empty_fix_commit(context: &str, commit_id: Option<&str>) { + if let Some(expected) = commit_id { + match capture_commit_id().as_deref() { + Some(current) if current == expected => {} + Some(current) => { + log_info(&format!( + "[warn] {} expected={}, current={} abandon を見送り", + context, expected, current + )); + return; + } + None => { + log_info(&format!( + "[warn] {} expected={}, current= abandon を見送り", + context, expected + )); + return; + } + } + } + + if diff_at_is_empty() { + let label = commit_id.map_or_else(String::new, |id| format!(" ({})", id)); + log_info(&format!( + "[action] {} 空 fix commit を abandon{}", + context, label + )); + let (ok, out) = run_cmd_direct("jj", &["abandon"], &[], JJ_CMD_TIMEOUT_SECS); + if !ok { + log_info(&format!( + "[action] jj abandon 失敗 (手動片付け推奨): {}", + out + )); + return; + } + reparent_at_to_pr_tip(context); + } else { + log_info(&format!( + "[warn] {} fix commit に diff あり、abandon を見送り", + context + )); + } +} + +/// `@` を PR tip (単一 local bookmark の指す commit) 直下に再配置する。 +/// +/// `jj abandon` 直後の `@` は stale な空 commit の上に残ることがあり +/// (task 6 背景: PR #64 で 3 回発生)、次の `jj new` がそこに積まれる。 +/// これを解消するため、bookmark が指す PR tip を解決して `jj new -r ` で +/// `@` を PR tip の直接子に戻す。 +/// +/// 以下のケースは fail-safe でスキップする: +/// - PR tip 解決失敗 (bookmark なし / 複数 bookmark で曖昧 / 取得失敗) +/// - 既に `@-` が PR tip と一致 (redundant な空 commit を作らない) +/// - `jj new -r ` 自体の失敗 (ログのみで処理を継続) +fn reparent_at_to_pr_tip(context: &str) { + let pr_tip = match crate::stages::push_jj_bookmark::resolve_pr_tip_commit_id() { + Some(id) => id, + None => { + log_info(&format!( + "[state] {} PR tip bookmark を特定できず re-parent スキップ", + context + )); + return; + } + }; + + if parent_commit_id_is(&pr_tip) { + log_info(&format!( + "[state] {} @ は既に PR tip ({}) 直下、re-parent 不要", + context, pr_tip + )); + return; + } + + let (ok, out) = run_cmd_direct("jj", &["new", "-r", &pr_tip], &[], JJ_CMD_TIMEOUT_SECS); + if ok { + log_info(&format!( + "[action] {} @ を PR tip ({}) 直下に re-parent", + context, pr_tip + )); + } else { + log_info(&format!( + "[action] {} @ の re-parent 失敗 (手動対応): {}", + context, out + )); + } +} + +/// `@-` (親 commit) の id が `expected` と一致するか判定する。 +/// 取得失敗時は `false` (= 不一致扱いで reparent を試行) を返す。 +fn parent_commit_id_is(expected: &str) -> bool { + let (ok, out) = run_cmd_direct( + "jj", + &["log", "-r", "@-", "--no-graph", "-T", "commit_id"], + &[], + JJ_CMD_TIMEOUT_SECS, + ); + ok && out.trim() == expected +} + +#[cfg(test)] +mod tests { + use super::*; + + /// 統合: `create_fix_commit` の fail-safe cleanup 動作を確認する。 + /// + /// `capture_commit_id` 失敗を直接 inject できないため、代わりに + /// `try_abandon_empty_fix_commit(_, None)` を直接呼んで「空 child が cleanup される」 + /// 挙動 (= None 分岐が依拠する唯一の副作用) が jj で動くことを確認する。 + #[test] + #[ignore = "integration: requires jj in PATH; run via `cargo test -- --ignored --test-threads=1`"] + fn integration_try_abandon_empty_fix_commit_without_id_drops_orphan_child() { + use std::env; + use std::process::Command as StdCommand; + + let temp = tempfile::tempdir().expect("tempdir 作成失敗"); + let repo_dir = temp.path(); + + assert!(StdCommand::new("jj") + .args(["git", "init"]) + .current_dir(repo_dir) + .status() + .expect("jj git init 失敗") + .success()); + + std::fs::write(repo_dir.join("a.txt"), "x\n").expect("write failed"); + let original_msg = "feat: original"; + assert!(StdCommand::new("jj") + .args(["describe", "-m", original_msg]) + .current_dir(repo_dir) + .status() + .expect("describe") + .success()); + + assert!(StdCommand::new("jj") + .args(["new", "-m", "fix(review): orphan test"]) + .current_dir(repo_dir) + .status() + .expect("jj new") + .success()); + + let original_cwd = env::current_dir().expect("cwd"); + env::set_current_dir(repo_dir).expect("cd"); + struct CwdRestore { + original: std::path::PathBuf, + } + impl Drop for CwdRestore { + fn drop(&mut self) { + let _ = std::env::set_current_dir(&self.original); + } + } + let _guard = CwdRestore { + original: original_cwd, + }; + + try_abandon_empty_fix_commit("test:", None); + + let log_out = StdCommand::new("jj") + .args([ + "log", + "-r", + "::@", + "--no-graph", + "-T", + "description ++ \"\\n\"", + ]) + .current_dir(repo_dir) + .output() + .expect("jj log"); + let log_str = String::from_utf8_lossy(&log_out.stdout); + assert!( + !log_str.contains("fix(review): orphan test"), + "orphan child が abandon されていない: {:?}", + log_str + ); + assert!( + log_str.contains(original_msg), + "元 commit が残っていること: {:?}", + log_str + ); + } + + /// 統合: task 6 の再現 — `pnpm push` 後の空 WC の上に fix commit が + /// 作られた状態で cleanup すると、`@` が stale な空 commit に残らず、 + /// PR tip (bookmark の指す commit) 直下に自動で re-parent されることを確認する。 + /// + /// 検証対象シナリオ (PR #64 で 3 回発生): + /// - `C1 (bookmark) ← C1' (empty, from pnpm push) ← Y (fix commit, @)` + /// - takt が NoChange で Y を abandon した後、従来は `@- == C1'` に残っていた + /// - 修正後は `@- == C1` (PR tip) に戻る + #[test] + #[ignore = "integration: requires jj in PATH; run via `cargo test -- --ignored --test-threads=1`"] + fn integration_try_abandon_reparents_at_to_pr_tip_after_cleanup() { + use std::env; + use std::process::Command as StdCommand; + + let temp = tempfile::tempdir().expect("tempdir 作成失敗"); + let repo_dir = temp.path(); + + assert!(StdCommand::new("jj") + .args(["git", "init"]) + .current_dir(repo_dir) + .status() + .expect("jj git init 失敗") + .success()); + + std::fs::write(repo_dir.join("a.txt"), "content\n").expect("write a.txt 失敗"); + assert!(StdCommand::new("jj") + .args(["describe", "-m", "feat: PR body"]) + .current_dir(repo_dir) + .status() + .expect("describe C1 失敗") + .success()); + let c1_id = { + let out = StdCommand::new("jj") + .args(["log", "-r", "@", "--no-graph", "-T", "commit_id"]) + .current_dir(repo_dir) + .output() + .expect("jj log C1"); + String::from_utf8_lossy(&out.stdout).trim().to_string() + }; + assert!(!c1_id.is_empty()); + + assert!(StdCommand::new("jj") + .args(["bookmark", "create", "feat/task6", "-r", "@"]) + .current_dir(repo_dir) + .status() + .expect("bookmark create 失敗") + .success()); + + assert!(StdCommand::new("jj") + .args(["new"]) + .current_dir(repo_dir) + .status() + .expect("jj new (C1') 失敗") + .success()); + + let original_cwd = env::current_dir().expect("cwd 取得失敗"); + env::set_current_dir(repo_dir).expect("cd 失敗"); + struct CwdRestore { + original: std::path::PathBuf, + } + impl Drop for CwdRestore { + fn drop(&mut self) { + let _ = std::env::set_current_dir(&self.original); + } + } + let _guard = CwdRestore { + original: original_cwd, + }; + + let fix_state = create_fix_commit(Some(64), &[]); + let fix_cid = match &fix_state { + FixCommitState::Created { commit_id } => commit_id.clone(), + _ => panic!("create_fix_commit 失敗: {:?}", fix_state), + }; + + try_abandon_empty_fix_commit("test:", Some(&fix_cid)); + + let parent_id = { + let out = StdCommand::new("jj") + .args(["log", "-r", "@-", "--no-graph", "-T", "commit_id"]) + .current_dir(repo_dir) + .output() + .expect("jj log @-"); + String::from_utf8_lossy(&out.stdout).trim().to_string() + }; + assert_eq!( + parent_id, c1_id, + "@- が PR tip (bookmark feat/task6) の指す commit と一致すること: got={:?}", + parent_id + ); + + assert!(diff_at_is_empty(), "reparent 後の @ は空 WC"); + + let bookmark_tip = { + let out = StdCommand::new("jj") + .args(["log", "-r", "feat/task6", "--no-graph", "-T", "commit_id"]) + .current_dir(repo_dir) + .output() + .expect("jj log bookmark"); + String::from_utf8_lossy(&out.stdout).trim().to_string() + }; + assert_eq!( + bookmark_tip, c1_id, + "bookmark が動かされていないこと: got={:?}", + bookmark_tip + ); + } + + /// 統合: bookmark が複数ある場合 (stacked PR 等) は reparent をスキップし、 + /// `jj abandon` のデフォルト配置 (親の上に新規 WC) に任せる fail-safe 挙動を確認する。 + #[test] + #[ignore = "integration: requires jj in PATH; run via `cargo test -- --ignored --test-threads=1`"] + fn integration_try_abandon_skips_reparent_with_multiple_bookmarks() { + use std::env; + use std::process::Command as StdCommand; + + let temp = tempfile::tempdir().expect("tempdir 作成失敗"); + let repo_dir = temp.path(); + + assert!(StdCommand::new("jj") + .args(["git", "init"]) + .current_dir(repo_dir) + .status() + .expect("jj git init 失敗") + .success()); + + std::fs::write(repo_dir.join("a.txt"), "content\n").expect("write 失敗"); + assert!(StdCommand::new("jj") + .args(["describe", "-m", "feat: base"]) + .current_dir(repo_dir) + .status() + .expect("describe 失敗") + .success()); + + for name in &["feat/stack-a", "feat/stack-b"] { + assert!(StdCommand::new("jj") + .args(["bookmark", "create", name, "-r", "@"]) + .current_dir(repo_dir) + .status() + .expect("bookmark create 失敗") + .success()); + } + + assert!(StdCommand::new("jj") + .args(["new"]) + .current_dir(repo_dir) + .status() + .expect("jj new 失敗") + .success()); + let c1_prime_id = { + let out = StdCommand::new("jj") + .args(["log", "-r", "@", "--no-graph", "-T", "commit_id"]) + .current_dir(repo_dir) + .output() + .expect("jj log"); + String::from_utf8_lossy(&out.stdout).trim().to_string() + }; + + let original_cwd = env::current_dir().expect("cwd 取得失敗"); + env::set_current_dir(repo_dir).expect("cd 失敗"); + struct CwdRestore { + original: std::path::PathBuf, + } + impl Drop for CwdRestore { + fn drop(&mut self) { + let _ = std::env::set_current_dir(&self.original); + } + } + let _guard = CwdRestore { + original: original_cwd, + }; + + let fix_state = create_fix_commit(Some(1), &[]); + let fix_cid = match &fix_state { + FixCommitState::Created { commit_id } => commit_id.clone(), + _ => panic!("create_fix_commit 失敗"), + }; + + try_abandon_empty_fix_commit("test:", Some(&fix_cid)); + + let parent_id = { + let out = StdCommand::new("jj") + .args(["log", "-r", "@-", "--no-graph", "-T", "commit_id"]) + .current_dir(repo_dir) + .output() + .expect("jj log @-"); + String::from_utf8_lossy(&out.stdout).trim().to_string() + }; + assert_eq!( + parent_id, c1_prime_id, + "複数 bookmark 時は reparent スキップ、@- は C1' のまま: got={:?}", + parent_id + ); + } +} diff --git a/src/cli-pr-monitor/src/fix_commit/description.rs b/src/cli-pr-monitor/src/fix_commit/description.rs new file mode 100644 index 00000000..0822ed1a --- /dev/null +++ b/src/cli-pr-monitor/src/fix_commit/description.rs @@ -0,0 +1,140 @@ +use lib_report_formatter::Finding; + +/// 分離型 fix commit の状態。 +/// +/// pre-takt で作成を試み、成否を型で表現する。 +/// post-takt の分岐 (re-push / abandon / 放置) で消費される。 +#[derive(Debug, Clone)] +pub(crate) enum FixCommitState { + /// 分離を行わなかった (findings なし、takt 未構成、または作成失敗) + None, + /// fix commit を pre-create 済み + Created { commit_id: String }, +} + +impl FixCommitState { + pub(crate) fn is_created(&self) -> bool { + matches!(self, Self::Created { .. }) + } +} + +/// fix commit の description を生成する。 +/// +/// ADR-022 例外の「新規 child commit への自己記述」として、 +/// - header ラベル: commit 種別を示す +/// - findings summary: 何を問題と捉え、どれを修正したかの文脈 +/// +/// の 2 段構成で返す。findings が空なら header のみ返す。 +pub(crate) fn build_fix_commit_description(pr_number: Option, findings: &[Finding]) -> String { + let header = match pr_number { + Some(n) => format!("fix(review): apply CodeRabbit fixes for #{}", n), + None => "fix(review): apply CodeRabbit fixes".to_string(), + }; + + if findings.is_empty() { + return header; + } + + let mut body = String::with_capacity(256); + body.push_str(&header); + body.push_str("\n\nResolved findings:\n"); + for f in findings { + let issue_oneline = sanitize_to_oneline(&f.issue); + body.push_str(&format!( + "- [{}] {}:{} {}\n", + f.severity, f.file, f.line, issue_oneline + )); + } + body.trim_end().to_string() +} + +/// CodeRabbit の `issue` フィールドは複数行になることがあるため、 +/// `build_fix_commit_description` のリスト項目に埋める前に単行化する。 +fn sanitize_to_oneline(input: &str) -> String { + input.split_whitespace().collect::>().join(" ") +} + +#[cfg(test)] +mod tests { + use super::*; + + fn finding(severity: &str, file: &str, line: &str, issue: &str) -> Finding { + Finding { + severity: severity.to_string(), + file: file.to_string(), + line: line.to_string(), + issue: issue.to_string(), + suggestion: String::new(), + source: "CodeRabbit".to_string(), + } + } + + #[test] + fn description_without_findings_is_header_only() { + let desc = build_fix_commit_description(Some(42), &[]); + assert_eq!(desc, "fix(review): apply CodeRabbit fixes for #42"); + } + + #[test] + fn description_without_pr_number_falls_back_to_generic_header() { + let desc = build_fix_commit_description(None, &[]); + assert_eq!(desc, "fix(review): apply CodeRabbit fixes"); + } + + #[test] + fn description_with_findings_includes_summary_block() { + let fs = vec![ + finding("Major", "src/foo.rs", "12", "null pointer"), + finding("Minor", "src/bar.rs", "34", "unused variable"), + ]; + let desc = build_fix_commit_description(Some(42), &fs); + assert!( + desc.starts_with("fix(review): apply CodeRabbit fixes for #42\n\nResolved findings:\n") + ); + assert!(desc.contains("- [Major] src/foo.rs:12 null pointer")); + assert!(desc.contains("- [Minor] src/bar.rs:34 unused variable")); + assert!(!desc.ends_with('\n')); + } + + #[test] + fn description_with_findings_without_pr_number() { + let fs = vec![finding("Major", "a.rs", "1", "issue")]; + let desc = build_fix_commit_description(None, &fs); + assert!(desc.starts_with("fix(review): apply CodeRabbit fixes\n\n")); + assert!(desc.contains("- [Major] a.rs:1 issue")); + } + + #[test] + fn description_sanitizes_multiline_issue_into_single_line() { + let fs = vec![finding( + "Major", + "src/foo.rs", + "10", + "first line\nsecond line\r\nthird line", + )]; + let desc = build_fix_commit_description(Some(1), &fs); + assert!( + desc.contains("- [Major] src/foo.rs:10 first line second line third line"), + "multi-line issue が単行化されていない: {:?}", + desc + ); + let bullet_lines: Vec<_> = desc.lines().filter(|l| l.starts_with("- ")).collect(); + assert_eq!(bullet_lines.len(), 1, "bullet は 1 行のみ: {:?}", desc); + } + + #[test] + fn sanitize_to_oneline_preserves_single_spacing_and_trims() { + assert_eq!(sanitize_to_oneline("a b\nc\td"), "a b c d"); + assert_eq!(sanitize_to_oneline(" leading "), "leading"); + assert_eq!(sanitize_to_oneline(""), ""); + } + + #[test] + fn fix_commit_state_is_created_truth_table() { + assert!(!FixCommitState::None.is_created()); + assert!(FixCommitState::Created { + commit_id: "abc".into() + } + .is_created()); + } +} diff --git a/src/cli-pr-monitor/src/fix_commit/mod.rs b/src/cli-pr-monitor/src/fix_commit/mod.rs new file mode 100644 index 00000000..ba0a6d8d --- /dev/null +++ b/src/cli-pr-monitor/src/fix_commit/mod.rs @@ -0,0 +1,16 @@ +//! 分離型 fix commit の pre-create と description 生成。 +//! +//! ADR-022 例外条項 (2026-04-20): 自動生成された修正を独立した child commit として +//! 分離する場合に限り、その child commit への description 付与を許可する。 +//! 元 commit (= 人間が意図を込めた初回 PR commit) の description は改変しない。 +//! +//! pre-takt で `jj new -m "..."` により空 child を作成し、takt が `@` を amend する +//! ことで fix 内容が自動的に child commit へ入る仕組み。 + +mod abandon; +mod description; +mod sweep; + +pub(crate) use abandon::{create_fix_commit, try_abandon_empty_fix_commit}; +pub(crate) use description::FixCommitState; +pub(crate) use sweep::sweep_empty_commits_in_pr_range; diff --git a/src/cli-pr-monitor/src/fix_commit/sweep.rs b/src/cli-pr-monitor/src/fix_commit/sweep.rs new file mode 100644 index 00000000..6c69769a --- /dev/null +++ b/src/cli-pr-monitor/src/fix_commit/sweep.rs @@ -0,0 +1,398 @@ +use crate::log::log_info; +use crate::runner::{run_cmd_direct, JJ_CMD_TIMEOUT_SECS}; + +/// `default_branch..@` 範囲の `fix(review):` 空 commit を sweep して全て abandon する (順位 155、PR #174 T1-#1)。 +/// +/// 既存 `try_abandon_empty_fix_commit` が tracked な単一 fix commit (= 直近 `create_fix_commit` +/// の戻り値) のみを対象とするのに対し、本関数は PR 範囲全体を sweep して +/// **untracked な空 commit** を網羅的に拾う。PR #174 で観測した `kqvluqyv` 事例 +/// (過去 fix loop で取りこぼされた granduncle 位置の空 commit が後続 push で PR diff 汚染) の +/// 構造的予防層。 +/// +/// 実装: jj revset `empty() & description("fix(review):") & (default_branch..@)` で範囲内の +/// fix(review): 空 commit を 1 step で列挙し、change_id ベースで順次 `jj abandon` する。 +/// change_id は jj の永続識別子のため、複数 abandon で graph が rebase されても残りの id 参照は invariant。 +/// description フィルタにより `create_fix_commit` 由来のコミットのみを対象とし、他の空コミットは除外する。 +/// +/// fail-open: jj log / abandon の失敗時は warn ログのみで cleanup を継続する +/// (push を block すると fix loop 全体が止まるため、ローカル副作用は次回再走で吸収する方針)。 +pub(crate) fn sweep_empty_commits_in_pr_range(default_branch: &str) { + let revset = format!( + "empty() & description(substring:\"fix(review):\") & ({}..@)", + default_branch + ); + let (ok, out) = run_cmd_direct( + "jj", + &[ + "log", + "-r", + &revset, + "--no-graph", + "-T", + "change_id ++ \"\\n\"", + ], + &[], + JJ_CMD_TIMEOUT_SECS, + ); + if !ok { + log_info(&format!( + "[warn] sweep_empty_commits: jj log 失敗 (sweep skip): {}", + out.trim() + )); + return; + } + + let change_ids = parse_empty_change_ids(&out); + if change_ids.is_empty() { + return; + } + log_info(&format!( + "[action] sweep_empty_commits: {}..@ 範囲に fix(review): 空 commit {} 件を検出 → abandon", + default_branch, + change_ids.len() + )); + for cid in &change_ids { + let (ok, out) = run_cmd_direct("jj", &["abandon", cid], &[], JJ_CMD_TIMEOUT_SECS); + if !ok { + log_info(&format!( + "[warn] sweep_empty_commits: jj abandon {} 失敗 (継続): {}", + cid, + out.trim() + )); + continue; + } + log_info(&format!("[action] sweep_empty_commits: abandoned {}", cid)); + } +} + +/// `jj log` 出力 (1 行 1 change_id) を parse する純関数。空行と前後空白を除去する。 +fn parse_empty_change_ids(log_output: &str) -> Vec { + log_output + .lines() + .map(|l| l.trim().to_string()) + .filter(|s| !s.is_empty()) + .collect() +} + +#[cfg(test)] +mod tests { + //! jj integration test の不変式パターン (PR #194 T2-#3 codified、 + //! `~/.claude/rules/common/testing.md` § "jj 操作コードの integration test pattern" と対): + //! + //! - NG: `count_empty_in_pr_range(repo_dir) == 0` 等の count-based assert + //! → jj は abandon 後に空 WC を自動生成するため、count は意図通り減らず false failure を起こす + //! - OK: `assert_descriptions_absent_in_pr_range(repo_dir, default_branch, &[target_desc])` の description-based assert + //! → 明示的に投入した description は auto-generated WC と区別できる + //! - sentinel 事前投入: 「mutation が発生していない」を assert する場合、本来残るべき commit を + //! `assert_descriptions_present_in_pr_range` で生存確認すると no-op vs no-mutation の偽陽性を防げる + + use super::*; + + #[test] + fn parse_empty_change_ids_handles_empty_input() { + assert!(parse_empty_change_ids("").is_empty()); + } + + #[test] + fn parse_empty_change_ids_extracts_single_id() { + let out = "abc123def\n"; + assert_eq!(parse_empty_change_ids(out), vec!["abc123def".to_string()]); + } + + #[test] + fn parse_empty_change_ids_extracts_multiple_ids() { + let out = "abc\ndef\nghi\n"; + assert_eq!( + parse_empty_change_ids(out), + vec!["abc".to_string(), "def".to_string(), "ghi".to_string()] + ); + } + + #[test] + fn parse_empty_change_ids_skips_blank_lines_and_whitespace() { + let out = " abc \n\n \ndef\n\n"; + assert_eq!( + parse_empty_change_ids(out), + vec!["abc".to_string(), "def".to_string()] + ); + } + + fn setup_jj_repo_with_master_at_base(base_msg: &str) -> tempfile::TempDir { + use std::process::Command as StdCommand; + let temp = tempfile::tempdir().expect("tempdir 作成失敗"); + let repo_dir = temp.path(); + assert!(StdCommand::new("jj") + .args(["git", "init"]) + .current_dir(repo_dir) + .status() + .expect("jj git init") + .success()); + std::fs::write(repo_dir.join("base.txt"), "content\n").expect("write base"); + assert!(StdCommand::new("jj") + .args(["describe", "-m", base_msg]) + .current_dir(repo_dir) + .status() + .expect("describe base") + .success()); + assert!(StdCommand::new("jj") + .args(["bookmark", "create", "master", "-r", "@"]) + .current_dir(repo_dir) + .status() + .expect("bookmark master") + .success()); + temp + } + + struct CwdGuard { + original: std::path::PathBuf, + } + impl Drop for CwdGuard { + fn drop(&mut self) { + let _ = std::env::set_current_dir(&self.original); + } + } + + fn enter_repo(repo_dir: &std::path::Path) -> CwdGuard { + let original = std::env::current_dir().expect("cwd"); + std::env::set_current_dir(repo_dir).expect("cd"); + CwdGuard { original } + } + + /// `jj new -m ` で空 commit を作成する test helper。 + /// integration test での空 commit 列挙を 1 行で書けるようにする。 + fn build_jj_empty_with_description(repo_dir: &std::path::Path, description: &str) { + let status = std::process::Command::new("jj") + .args(["new", "-m", description]) + .current_dir(repo_dir) + .status() + .expect("jj new"); + assert!(status.success(), "jj new failed for: {}", description); + } + + /// `master` bookmark を `branch_name` にリネームする test helper。 + /// alternative default_branch test の前処理として利用。 + fn rename_master_bookmark(repo_dir: &std::path::Path, branch_name: &str) { + let status = std::process::Command::new("jj") + .args(["bookmark", "rename", "master", branch_name]) + .current_dir(repo_dir) + .status() + .expect("jj bookmark rename"); + assert!(status.success(), "rename master -> {}", branch_name); + } + + /// 指定 description を持つ commit が PR 範囲に残存していることを assert する。 + /// (negative case 用 = abandon されてはいけない sentinel commit の生存確認) + fn assert_descriptions_present_in_pr_range( + repo_dir: &std::path::Path, + default_branch: &str, + descriptions: &[&str], + ) { + let revset = format!("{}..@", default_branch); + let out = std::process::Command::new("jj") + .args([ + "log", + "-r", + &revset, + "--no-graph", + "-T", + "description ++ \"\\n\"", + ]) + .current_dir(repo_dir) + .output() + .expect("jj log"); + let log_str = String::from_utf8_lossy(&out.stdout); + for d in descriptions { + assert!( + log_str.contains(d), + "{:?} が残存している前提だが消えている: {:?}", + d, + log_str + ); + } + } + + fn assert_descriptions_absent_in_pr_range( + repo_dir: &std::path::Path, + default_branch: &str, + descriptions: &[&str], + ) { + let revset = format!("{}..@", default_branch); + let out = std::process::Command::new("jj") + .args([ + "log", + "-r", + &revset, + "--no-graph", + "-T", + "description ++ \"\\n\"", + ]) + .current_dir(repo_dir) + .output() + .expect("jj log"); + let log_str = String::from_utf8_lossy(&out.stdout); + for d in descriptions { + assert!( + !log_str.contains(d), + "{:?} が abandon されている前提だが残存: {:?}", + d, + log_str + ); + } + } + + fn count_empty_in_pr_range(repo_dir: &std::path::Path, default_branch: &str) -> usize { + let revset = format!("empty() & ({}..@)", default_branch); + let out = std::process::Command::new("jj") + .args([ + "log", + "-r", + &revset, + "--no-graph", + "-T", + "change_id ++ \"\\n\"", + ]) + .current_dir(repo_dir) + .output() + .expect("jj log"); + String::from_utf8_lossy(&out.stdout) + .lines() + .filter(|l| !l.trim().is_empty()) + .count() + } + + /// 統合: PR 範囲 (`..@`) に空 commit が無いとき sweep は no-op (非空 commit を保持)。 + #[test] + #[ignore = "integration: requires jj in PATH; run via `cargo test -- --ignored --test-threads=1`"] + fn integration_sweep_empty_commits_no_op_when_no_empty_in_range() { + let temp = setup_jj_repo_with_master_at_base("feat: real change"); + let repo_dir = temp.path(); + let _guard = enter_repo(repo_dir); + + sweep_empty_commits_in_pr_range("master"); + + let log_out = std::process::Command::new("jj") + .args(["log", "-r", "::@", "--no-graph", "-T", "description"]) + .current_dir(repo_dir) + .output() + .expect("jj log"); + let log_str = String::from_utf8_lossy(&log_out.stdout); + assert!( + log_str.contains("feat: real change"), + "non-empty commit が保持されていること: {:?}", + log_str + ); + } + + /// 統合: PR 範囲 (`..@`) の複数空 commit を sweep が全て abandon する。 + /// PR #174 `kqvluqyv` 事例の最小再現。 + #[test] + #[ignore = "integration: requires jj in PATH; run via `cargo test -- --ignored --test-threads=1`"] + fn integration_sweep_empty_commits_abandons_multiple_in_range() { + use std::process::Command as StdCommand; + let temp = setup_jj_repo_with_master_at_base("feat: base"); + let repo_dir = temp.path(); + + for label in &["fix(review): empty 1", "fix(review): empty 2"] { + build_jj_empty_with_description(repo_dir, label); + } + assert!( + count_empty_in_pr_range(repo_dir, "master") >= 2, + "前提: sweep 前に空 commit が 2 件以上" + ); + + let _guard = enter_repo(repo_dir); + sweep_empty_commits_in_pr_range("master"); + + assert_descriptions_absent_in_pr_range( + repo_dir, + "master", + &["fix(review): empty 1", "fix(review): empty 2"], + ); + + let master_out = StdCommand::new("jj") + .args(["log", "-r", "master", "--no-graph", "-T", "description"]) + .current_dir(repo_dir) + .output() + .expect("jj log master"); + let master_desc = String::from_utf8_lossy(&master_out.stdout); + assert!( + master_desc.contains("feat: base"), + "master commit (非空) は abandon されない: {:?}", + master_desc + ); + } + + /// 統合 (PR #194 T2-#2 variant 1): non-`fix(review):` 系の空 commit (`feat:` / `docs:` / `chore:` 等) + /// は sweep 対象外であることを assert (description filter の negative case)。 + /// fix(review): empty が混在しても誤 abandon されないことが保証される。 + #[test] + #[ignore = "integration: requires jj in PATH; run via `cargo test -- --ignored --test-threads=1`"] + fn integration_sweep_skips_non_fix_review_empty_commits() { + let temp = setup_jj_repo_with_master_at_base("feat: base"); + let repo_dir = temp.path(); + + build_jj_empty_with_description(repo_dir, "feat: empty 1"); + build_jj_empty_with_description(repo_dir, "docs: empty 2"); + build_jj_empty_with_description(repo_dir, "chore: empty 3"); + build_jj_empty_with_description(repo_dir, "fix(review): empty matched"); + + let _guard = enter_repo(repo_dir); + sweep_empty_commits_in_pr_range("master"); + + assert_descriptions_present_in_pr_range( + repo_dir, + "master", + &["feat: empty 1", "docs: empty 2", "chore: empty 3"], + ); + assert_descriptions_absent_in_pr_range(repo_dir, "master", &["fix(review): empty matched"]); + } + + /// 統合 (PR #194 T2-#2 variant 2): default_branch を `main` 等の alternative 名で + /// 指定したとき、revset がパラメータ化されて該当範囲のみ対象になることを assert。 + /// `SweepConfig.default_branch` 設定可能化の dogfood。 + #[test] + #[ignore = "integration: requires jj in PATH; run via `cargo test -- --ignored --test-threads=1`"] + fn integration_sweep_respects_alternative_default_branch() { + let temp = setup_jj_repo_with_master_at_base("feat: base"); + let repo_dir = temp.path(); + rename_master_bookmark(repo_dir, "main"); + + build_jj_empty_with_description(repo_dir, "fix(review): empty under main"); + + assert!( + count_empty_in_pr_range(repo_dir, "main") >= 1, + "前提: sweep 前に 'main' 範囲で空 commit が 1 件以上 (helper の default_branch 引数が main で機能していること)" + ); + + let _guard = enter_repo(repo_dir); + sweep_empty_commits_in_pr_range("main"); + + assert_descriptions_absent_in_pr_range( + repo_dir, + "main", + &["fix(review): empty under main"], + ); + } + + /// 統合 (PR #194 T2-#2 variant 3): `fix(review):` 空 commit が 0 件のとき、 + /// 他 description の空 commit が範囲内に存在しても sweep が abandon を 1 件も発行しない + /// (description filter の早期 return path)。 + #[test] + #[ignore = "integration: requires jj in PATH; run via `cargo test -- --ignored --test-threads=1`"] + fn integration_sweep_no_op_when_only_non_fix_review_empties_present() { + let temp = setup_jj_repo_with_master_at_base("feat: base"); + let repo_dir = temp.path(); + + build_jj_empty_with_description(repo_dir, "feat: only feat empty"); + build_jj_empty_with_description(repo_dir, "docs: only docs empty"); + + let _guard = enter_repo(repo_dir); + sweep_empty_commits_in_pr_range("master"); + + assert_descriptions_present_in_pr_range( + repo_dir, + "master", + &["feat: only feat empty", "docs: only docs empty"], + ); + } +} diff --git a/src/cli-pr-monitor/src/stages/poll/iteration.rs b/src/cli-pr-monitor/src/stages/poll/iteration.rs new file mode 100644 index 00000000..99b82b51 --- /dev/null +++ b/src/cli-pr-monitor/src/stages/poll/iteration.rs @@ -0,0 +1,365 @@ +use std::time::Duration; + +use crate::classifier_runner::classify_findings; +use crate::config::{ClassifierConfig, DEFAULT_CHECK_TIMEOUT_SECS}; +use crate::log::{log_info, truncate_safe}; +use crate::runner::run_cmd_direct; +use crate::state::{ + read_state, update_state_from_check_result, write_state, CiState, CodeRabbitState, + PrMonitorState, +}; +use crate::util::{utc_now_iso8601, PrInfo}; + +use super::rate_limit::handle_rate_limit_branch; +use super::{error_poll_result, PollContext, PollResult}; + +pub(super) fn run_one_iteration(ctx: &PollContext<'_>) -> Option { + let effective_push_time = ctx.fix_push_time.unwrap_or(ctx.push_time); + let args = build_checker_args(effective_push_time, ctx.pr_info); + let result = match invoke_checker(ctx.checker, &args) { + Ok(r) => r, + Err(pr) => return Some(*pr), + }; + let mut state = build_state_for_iteration( + ctx.pr_info, + ctx.push_time, + &result, + ctx.skip_ci, + ctx.skip_coderabbit, + ); + enrich_with_classifier(&mut state, ctx.classifier_config); + log_info(&format!( + "ポーリング: action={}, summary={}", + state.action, state.summary + )); + + if state.action != "continue_monitoring" { + return Some(make_terminal_result(state, result)); + } + + if let Some(terminal) = handle_rate_limit_branch( + &mut state, + ctx.rate_limit_config, + ctx.pr_info, + ctx.review_recheck_wait_secs, + &result, + ) { + return Some(terminal); + } + + if ctx.start.elapsed() >= Duration::from_secs(ctx.max_duration) { + log_info(&format!("監視タイムアウト ({}秒)", ctx.max_duration)); + return Some(make_timeout_result(state, ctx.max_duration, result)); + } + + None +} + +fn build_checker_args(push_time: &str, pr_info: &PrInfo) -> Vec { + let mut args: Vec = vec!["--push-time".into(), push_time.into()]; + if let Some(ref repo) = pr_info.repo { + args.push("--repo".into()); + args.push(repo.clone()); + } + if let Some(pr) = pr_info.pr_number { + args.push("--pr".into()); + args.push(pr.to_string()); + } + args +} + +fn invoke_checker( + checker: &std::path::Path, + args: &[String], +) -> Result> { + let (success, output) = run_cmd_direct( + &checker.to_string_lossy(), + &[], + args, + DEFAULT_CHECK_TIMEOUT_SECS, + ); + + if !success { + log_info(&format!("checker 失敗: {}", truncate_safe(&output, 200))); + return Err(Box::new(error_poll_result(&format!( + "check-ci-coderabbit.exe 失敗: {}", + truncate_safe(&output, 200) + )))); + } + + serde_json::from_str::(&output).map_err(|e| { + log_info(&format!("JSON パース失敗: {}", e)); + Box::new(error_poll_result(&format!( + "checker 出力の JSON パース失敗: {}", + e + ))) + }) +} + +/// `PrMonitorState::new` は毎回 notified / rate_limit_retries を 0 リセットするため、 +/// 既存 state から runtime-updated な値を読み戻して 1 iteration の base state を組む。 +fn build_state_for_iteration( + pr_info: &PrInfo, + push_time: &str, + result: &serde_json::Value, + skip_ci: bool, + skip_coderabbit: bool, +) -> PrMonitorState { + let mut state = PrMonitorState::new( + pr_info.pr_number, + pr_info.repo.clone(), + push_time.to_string(), + ); + update_state_from_check_result(&mut state, result); + + if let Some(existing) = read_state() { + state.notified = existing.notified; + state.rate_limit_retries = existing.rate_limit_retries; + state.rate_limit_last_retriggered_at = existing.rate_limit_last_retriggered_at; + state.review_recheck_count = existing.review_recheck_count; + state.head_commit = existing.head_commit; + state.classified_findings = existing.classified_findings; + state.fix_push_time = existing.fix_push_time; + } + + apply_skip_handling(&mut state, skip_ci, skip_coderabbit); + state.last_checked = Some(utc_now_iso8601()); + if let Err(e) = write_state(&state) { + log_info(&format!("state 書き込み失敗 (skip 反映後、続行): {}", e)); + } + state +} + +/// classifier (ADR-038, Phase 5) で findings を enrich する。 +/// +/// `config.classifier.enabled = false` または findings が空のときは何もしない。 +/// 実行成功時は state.classified_findings を populate して state file を再書き出す。 +/// 失敗時は state.classified_findings は空のまま (caller は findings をそのまま使えばよい)。 +fn enrich_with_classifier(state: &mut PrMonitorState, config: &ClassifierConfig) { + if !config.enabled || state.findings.is_empty() { + return; + } + let classified = classify_findings(config, &state.findings); + if classified.is_empty() { + return; + } + log_info(&format!( + "classifier: {} findings を分類完了", + classified.len() + )); + state.classified_findings = classified; + if let Err(e) = write_state(state) { + log_info(&format!( + "state 書き込み失敗 (classifier enrich 後、続行): {}", + e + )); + } +} + +fn apply_skip_handling(state: &mut PrMonitorState, skip_ci: bool, skip_coderabbit: bool) { + if skip_ci { + state.ci = Some(CiState { + overall: "skipped".into(), + runs: vec![], + }); + } + if skip_coderabbit { + state.coderabbit = Some(CodeRabbitState { + review_state: "skipped".into(), + new_comments: 0, + actionable_comments: None, + unresolved_threads: None, + }); + state.findings = Vec::new(); + } + if skip_ci || skip_coderabbit { + state.action = recompute_action(state, skip_ci, skip_coderabbit); + } +} + +fn make_terminal_result(state: PrMonitorState, result: serde_json::Value) -> PollResult { + PollResult { + action: state.action, + summary: state.summary, + ci: state.ci, + coderabbit: state.coderabbit, + findings: state.findings, + check_output: Some(result), + rate_limit: state.rate_limit, + } +} + +fn make_timeout_result( + state: PrMonitorState, + max_duration: u64, + result: serde_json::Value, +) -> PollResult { + PollResult { + action: "timed_out".into(), + summary: format!("監視タイムアウト ({}秒)", max_duration), + ci: state.ci, + coderabbit: state.coderabbit, + findings: state.findings, + check_output: Some(result), + rate_limit: state.rate_limit, + } +} + +/// skip 適用後に、有効なチェックだけを見て action を再導出する +fn recompute_action(state: &PrMonitorState, skip_ci: bool, skip_coderabbit: bool) -> String { + let ci_ok = skip_ci + || state + .ci + .as_ref() + .map(|c| c.overall == "success" || c.overall == "skipped") + .unwrap_or(false); + + let cr_ok = skip_coderabbit + || state + .coderabbit + .as_ref() + .map(|c| { + c.review_state == "skipped" + || (c.new_comments == 0 && c.unresolved_threads.unwrap_or(0) == 0) + }) + .unwrap_or(false); + + let ci_pending = !skip_ci + && state + .ci + .as_ref() + .map(|c| c.overall == "pending") + .unwrap_or(true); + + let cr_pending = !skip_coderabbit + && state + .coderabbit + .as_ref() + .map(|c| c.review_state == "not_found" || c.review_state == "pending") + .unwrap_or(true); + + if ci_pending || cr_pending { + return "continue_monitoring".into(); + } + + let ci_failed = !skip_ci + && state + .ci + .as_ref() + .map(|c| c.overall == "failure") + .unwrap_or(false); + + let cr_action_required = !skip_coderabbit + && state + .coderabbit + .as_ref() + .map(|c| c.new_comments > 0 || c.unresolved_threads.unwrap_or(0) > 0) + .unwrap_or(false); + + if ci_failed { + "stop_monitoring_failure".into() + } else if cr_action_required { + "action_required".into() + } else if ci_ok && cr_ok { + "stop_monitoring_success".into() + } else { + state.action.clone() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use lib_report_formatter::Finding; + + /// PR #120 W-001 follow-up (順位 83): `enrich_with_classifier` の `!config.enabled` + /// guard を **単独で** 検証する。`findings` を非空 (= `findings.is_empty()` guard + /// 不発)、`enabled = false` (= 本 guard 発火) にして 2 つの OR guard を直交させる。 + /// + /// 検証対象 field `state.classified_findings` を sentinel で pre-populate し、 + /// 早期 return しなかった場合の代入 (`state.classified_findings = classified;`) + /// を sentinel 消失として検出する設計。空のまま渡すと「不変=空」が早期 return + /// 由来か他経路由来か判別できないため sentinel 必須。 + #[test] + fn enrich_with_classifier_skips_when_disabled() { + use crate::classifier_runner::ClassifiedFinding; + + let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); + state.findings = vec![Finding { + severity: "Major".into(), + file: "f.rs".into(), + line: "1".into(), + issue: "issue".into(), + suggestion: "fix".into(), + source: "coderabbit".into(), + }]; + let sentinel = ClassifiedFinding { + finding: Finding { + severity: "Minor".into(), + file: "sentinel.rs".into(), + line: "1".into(), + issue: "sentinel".into(), + suggestion: "must not be overwritten".into(), + source: "test".into(), + }, + action: "auto_fix".into(), + action_confidence: 0.99, + normalized_issue: None, + fallback_reason: None, + }; + state.classified_findings = vec![sentinel.clone()]; + let disabled = ClassifierConfig { + enabled: false, + ..ClassifierConfig::default() + }; + + enrich_with_classifier(&mut state, &disabled); + + assert_eq!( + state.classified_findings, + vec![sentinel], + "!config.enabled guard should early return before any mutation" + ); + } + + /// `state.findings.is_empty()` guard (`enrich_with_classifier` 2 番目の早期 return) + /// を単独で検証する。`enabled = true` (明示、= `!config.enabled` guard 不発)、 + /// `findings` 空 (= 本 guard 発火) にして他条件と直交させる。 + #[test] + fn enrich_with_classifier_skips_when_findings_empty() { + use crate::classifier_runner::ClassifiedFinding; + + let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); + assert!( + state.findings.is_empty(), + "test precondition: findings must be empty so `!enabled` guard stays unfired" + ); + let sentinel = ClassifiedFinding { + finding: Finding { + severity: "Minor".into(), + file: "sentinel.rs".into(), + line: "1".into(), + issue: "sentinel".into(), + suggestion: "must not be overwritten".into(), + source: "test".into(), + }, + action: "auto_fix".into(), + action_confidence: 0.99, + normalized_issue: None, + fallback_reason: None, + }; + state.classified_findings = vec![sentinel.clone()]; + let enabled = ClassifierConfig { + enabled: true, + ..ClassifierConfig::default() + }; + + enrich_with_classifier(&mut state, &enabled); + + assert_eq!( + state.classified_findings, + vec![sentinel], + "findings.is_empty() guard should early return before any mutation" + ); + } +} diff --git a/src/cli-pr-monitor/src/stages/poll/mod.rs b/src/cli-pr-monitor/src/stages/poll/mod.rs index ad144f4c..b6478dca 100644 --- a/src/cli-pr-monitor/src/stages/poll/mod.rs +++ b/src/cli-pr-monitor/src/stages/poll/mod.rs @@ -1,34 +1,18 @@ +mod iteration; mod rate_limit; +mod rate_limit_signal; mod review_recheck; +mod review_recheck_signal; -use rate_limit::{handle_rate_limit_branch, make_action_required_result}; use review_recheck::{finalize_initial_review_park, finalize_review_recheck_park}; -#[cfg(test)] -use rate_limit::{ - evaluate_rate_limit_shortcut, finalize_parked, finalize_posted_retrigger, format_park_signal, - format_shortcut_signal, handle_rate_limit_retry, MergeableStatus, RateLimitOutcome, -}; -#[cfg(test)] -use review_recheck::{ - compute_safe_minute_for_park_signal, format_review_park_signal, round_up_to_next_minute, - schedule_next_review_recheck_park, -}; - use lib_report_formatter::Finding; -use std::time::Duration; -use crate::classifier_runner::classify_findings; -use crate::config::{ - ClassifierConfig, Config, MonitorConfig, RateLimitConfig, DEFAULT_CHECK_TIMEOUT_SECS, -}; -use crate::log::{log_info, truncate_safe}; -use crate::runner::{checker_exe_path, run_cmd_direct}; -use crate::state::{ - read_state, update_state_from_check_result, write_state, CiState, CodeRabbitState, - PrMonitorState, RateLimitState, -}; -use crate::util::{utc_now_iso8601, PrInfo}; +use crate::config::{Config, MonitorConfig, RateLimitConfig}; +use crate::log::log_info; +use crate::runner::checker_exe_path; +use crate::state::{CiState, CodeRabbitState, PrMonitorState, RateLimitState}; +use crate::util::PrInfo; pub(crate) struct PollResult { pub(crate) action: String, @@ -53,7 +37,7 @@ pub(super) struct PollContext<'a> { pub(super) fix_push_time: Option<&'a str>, pub(super) pr_info: &'a PrInfo, pub(super) rate_limit_config: &'a RateLimitConfig, - pub(super) classifier_config: &'a ClassifierConfig, + pub(super) classifier_config: &'a crate::config::ClassifierConfig, pub(super) start: std::time::Instant, pub(super) max_duration: u64, pub(super) skip_ci: bool, @@ -109,96 +93,13 @@ pub(crate) fn run_poll_loop(full_config: &Config, pr_info: &PrInfo, is_wakeup: b return finalize_initial_review_park(&ctx); } - if let Some(terminal) = run_one_iteration(&ctx) { + if let Some(terminal) = iteration::run_one_iteration(&ctx) { return terminal; } finalize_review_recheck_park(&ctx) } -fn run_one_iteration(ctx: &PollContext<'_>) -> Option { - let effective_push_time = ctx.fix_push_time.unwrap_or(ctx.push_time); - let args = build_checker_args(effective_push_time, ctx.pr_info); - let result = match invoke_checker(ctx.checker, &args) { - Ok(r) => r, - Err(pr) => return Some(*pr), - }; - let mut state = build_state_for_iteration( - ctx.pr_info, - ctx.push_time, - &result, - ctx.skip_ci, - ctx.skip_coderabbit, - ); - enrich_with_classifier(&mut state, ctx.classifier_config); - log_info(&format!( - "ポーリング: action={}, summary={}", - state.action, state.summary - )); - - if state.action != "continue_monitoring" { - return Some(make_terminal_result(state, result)); - } - - if let Some(terminal) = handle_rate_limit_branch( - &mut state, - ctx.rate_limit_config, - ctx.pr_info, - ctx.review_recheck_wait_secs, - &result, - ) { - return Some(terminal); - } - - if ctx.start.elapsed() >= Duration::from_secs(ctx.max_duration) { - log_info(&format!("監視タイムアウト ({}秒)", ctx.max_duration)); - return Some(make_timeout_result(state, ctx.max_duration, result)); - } - - None -} - -fn build_checker_args(push_time: &str, pr_info: &PrInfo) -> Vec { - let mut args: Vec = vec!["--push-time".into(), push_time.into()]; - if let Some(ref repo) = pr_info.repo { - args.push("--repo".into()); - args.push(repo.clone()); - } - if let Some(pr) = pr_info.pr_number { - args.push("--pr".into()); - args.push(pr.to_string()); - } - args -} - -fn invoke_checker( - checker: &std::path::Path, - args: &[String], -) -> Result> { - let (success, output) = run_cmd_direct( - &checker.to_string_lossy(), - &[], - args, - DEFAULT_CHECK_TIMEOUT_SECS, - ); - - if !success { - log_info(&format!("checker 失敗: {}", truncate_safe(&output, 200))); - return Err(Box::new(error_poll_result(&format!( - "check-ci-coderabbit.exe 失敗: {}", - truncate_safe(&output, 200) - )))); - } - - serde_json::from_str::(&output).map_err(|e| { - log_info(&format!("JSON パース失敗: {}", e)); - Box::new(error_poll_result(&format!( - "checker 出力の JSON パース失敗: {}", - e - ))) - }) -} - -fn error_poll_result(summary: &str) -> PollResult { +pub(super) fn error_poll_result(summary: &str) -> PollResult { PollResult { action: "error".into(), summary: summary.into(), @@ -210,116 +111,6 @@ fn error_poll_result(summary: &str) -> PollResult { } } -/// `PrMonitorState::new` は毎回 notified / rate_limit_retries を 0 リセットするため、 -/// 既存 state から runtime-updated な値を読み戻して 1 iteration の base state を組む。 -fn build_state_for_iteration( - pr_info: &PrInfo, - push_time: &str, - result: &serde_json::Value, - skip_ci: bool, - skip_coderabbit: bool, -) -> PrMonitorState { - let mut state = PrMonitorState::new( - pr_info.pr_number, - pr_info.repo.clone(), - push_time.to_string(), - ); - update_state_from_check_result(&mut state, result); - - if let Some(existing) = read_state() { - state.notified = existing.notified; - state.rate_limit_retries = existing.rate_limit_retries; - state.rate_limit_last_retriggered_at = existing.rate_limit_last_retriggered_at; - state.review_recheck_count = existing.review_recheck_count; - state.head_commit = existing.head_commit; - state.classified_findings = existing.classified_findings; - state.fix_push_time = existing.fix_push_time; - } - - apply_skip_handling(&mut state, skip_ci, skip_coderabbit); - state.last_checked = Some(utc_now_iso8601()); - if let Err(e) = write_state(&state) { - log_info(&format!("state 書き込み失敗 (skip 反映後、続行): {}", e)); - } - state -} - -/// classifier (ADR-038, Phase 5) で findings を enrich する。 -/// -/// `config.classifier.enabled = false` または findings が空のときは何もしない。 -/// 実行成功時は state.classified_findings を populate して state file を再書き出す。 -/// 失敗時は state.classified_findings は空のまま (caller は findings をそのまま使えばよい)。 -fn enrich_with_classifier(state: &mut PrMonitorState, config: &ClassifierConfig) { - if !config.enabled || state.findings.is_empty() { - return; - } - let classified = classify_findings(config, &state.findings); - if classified.is_empty() { - return; - } - log_info(&format!( - "classifier: {} findings を分類完了", - classified.len() - )); - state.classified_findings = classified; - if let Err(e) = write_state(state) { - log_info(&format!( - "state 書き込み失敗 (classifier enrich 後、続行): {}", - e - )); - } -} - -fn apply_skip_handling(state: &mut PrMonitorState, skip_ci: bool, skip_coderabbit: bool) { - if skip_ci { - state.ci = Some(CiState { - overall: "skipped".into(), - runs: vec![], - }); - } - if skip_coderabbit { - state.coderabbit = Some(CodeRabbitState { - review_state: "skipped".into(), - new_comments: 0, - actionable_comments: None, - unresolved_threads: None, - }); - state.findings = Vec::new(); - } - if skip_ci || skip_coderabbit { - state.action = recompute_action(state, skip_ci, skip_coderabbit); - } -} - -fn make_terminal_result(state: PrMonitorState, result: serde_json::Value) -> PollResult { - PollResult { - action: state.action, - summary: state.summary, - ci: state.ci, - coderabbit: state.coderabbit, - findings: state.findings, - check_output: Some(result), - rate_limit: state.rate_limit, - } -} - -fn make_timeout_result( - state: PrMonitorState, - max_duration: u64, - result: serde_json::Value, -) -> PollResult { - PollResult { - action: "timed_out".into(), - summary: format!("監視タイムアウト ({}秒)", max_duration), - ci: state.ci, - coderabbit: state.coderabbit, - findings: state.findings, - check_output: Some(result), - rate_limit: state.rate_limit, - } -} - - /// review_recheck park / initial park の戻り値生成 helper (check_output=None)。 pub(super) fn make_park_poll_result(state: PrMonitorState) -> PollResult { PollResult { @@ -333,305 +124,12 @@ pub(super) fn make_park_poll_result(state: PrMonitorState) -> PollResult { } } -/// skip 適用後に、有効なチェックだけを見て action を再導出する -fn recompute_action(state: &PrMonitorState, skip_ci: bool, skip_coderabbit: bool) -> String { - let ci_ok = skip_ci - || state - .ci - .as_ref() - .map(|c| c.overall == "success" || c.overall == "skipped") - .unwrap_or(false); - - let cr_ok = skip_coderabbit - || state - .coderabbit - .as_ref() - .map(|c| { - c.review_state == "skipped" - || (c.new_comments == 0 && c.unresolved_threads.unwrap_or(0) == 0) - }) - .unwrap_or(false); - - let ci_pending = !skip_ci - && state - .ci - .as_ref() - .map(|c| c.overall == "pending") - .unwrap_or(true); - - let cr_pending = !skip_coderabbit - && state - .coderabbit - .as_ref() - .map(|c| c.review_state == "not_found" || c.review_state == "pending") - .unwrap_or(true); - - if ci_pending || cr_pending { - return "continue_monitoring".into(); - } - - let ci_failed = !skip_ci - && state - .ci - .as_ref() - .map(|c| c.overall == "failure") - .unwrap_or(false); - - let cr_action_required = !skip_coderabbit - && state - .coderabbit - .as_ref() - .map(|c| c.new_comments > 0 || c.unresolved_threads.unwrap_or(0) > 0) - .unwrap_or(false); - - if ci_failed { - "stop_monitoring_failure".into() - } else if cr_action_required { - "action_required".into() - } else if ci_ok && cr_ok { - "stop_monitoring_success".into() - } else { - // Fallback: keep original action - state.action.clone() - } -} - #[cfg(test)] mod tests { use super::*; - use crate::state::RateLimitState; - - #[test] - fn rate_limit_state_persists_retries_across_polls() { - // simulate state.json round-trip behavior: 1 iteration で incremented した - // retries が次 iteration で復元されることを検証 - let tmp = std::env::temp_dir().join(format!("test-rl-retries-{}.json", std::process::id())); - let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); - state.rate_limit_retries = 2; - state.rate_limit = Some(RateLimitState { - until_unix_secs: 1_735_689_600, - comment_event_time: "2026-04-30T00:00:00Z".into(), - wait_minutes: 5, - wait_seconds: 13, - }); - crate::state::write_state_to(&tmp, &state).unwrap(); - - let loaded = crate::state::read_state_from(&tmp).unwrap(); - assert_eq!(loaded.rate_limit_retries, 2); - assert_eq!( - loaded.rate_limit.as_ref().unwrap().until_unix_secs, - 1_735_689_600 - ); - - let _ = std::fs::remove_file(&tmp); - } - - #[test] - fn rate_limit_default_config_allows_retry_within_limit() { - let cfg = RateLimitConfig::default(); - assert!(cfg.auto_retry_enabled); - assert_eq!(cfg.max_retries, 3); - // 2 retries 後: 2 < 3 で auto_retry_enabled パスを通る - assert!(2 < cfg.max_retries); - // 3 retries 後: 3 >= 3 で max 到達 → action_required で抜ける - assert!(3 >= cfg.max_retries); - } - - /// 同じ rate-limit comment が iteration 跨ぎで残った場合に dedup が働くことを検証する。 - /// - /// シナリオ (advisor 発見のバグ): - /// - Iter 1: comment A, retries=0, last_retriggered=None → handle 対象 - /// - Iter 2: 同じ comment A still in PR, last_retriggered=A → 即時 retrigger を skip - /// - Iter 3: CR が新たな rate-limit comment B を投稿, last_retriggered=A != B → 再 handle 対象 - /// - /// dedup なしだと Iter 2/3 で sleep_secs=0 となり数秒で max_retries を消費する。 - #[test] - fn rate_limit_dedup_skips_repeated_comment() { - let comment_a = "2026-04-30T00:00:00Z"; - let comment_b = "2026-04-30T00:30:00Z"; - - // Iter 1: 初回 detection (last_retriggered=None) - let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); - let rl_a = RateLimitState { - until_unix_secs: 0, - comment_event_time: comment_a.into(), - wait_minutes: 5, - wait_seconds: 0, - }; - let already_handled_iter1 = state.rate_limit_last_retriggered_at.as_deref() - == Some(rl_a.comment_event_time.as_str()); - assert!( - !already_handled_iter1, - "Iter 1: 初回 detection は handle されるべき" - ); - - // Iter 1 で handle した結果を simulate - state.rate_limit_retries = 1; - state.rate_limit_last_retriggered_at = Some(comment_a.into()); - - // Iter 2: 同じ comment が PR に残っている (CR レビュー再開待ち) - let already_handled_iter2 = state.rate_limit_last_retriggered_at.as_deref() - == Some(rl_a.comment_event_time.as_str()); - assert!( - already_handled_iter2, - "Iter 2: 同じ comment は dedup で skip されるべき" - ); - - // Iter 3: CR が新たな rate-limit comment を投稿 - let rl_b = RateLimitState { - until_unix_secs: 0, - comment_event_time: comment_b.into(), - wait_minutes: 5, - wait_seconds: 0, - }; - let already_handled_iter3 = state.rate_limit_last_retriggered_at.as_deref() - == Some(rl_b.comment_event_time.as_str()); - assert!( - !already_handled_iter3, - "Iter 3: 新 comment は再度 handle 対象" - ); - } - - /// state.json round-trip で rate_limit_last_retriggered_at が persistence される。 - #[test] - fn rate_limit_last_retriggered_at_persists_across_polls() { - let tmp = - std::env::temp_dir().join(format!("test-rl-last-handled-{}.json", std::process::id())); - let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); - state.rate_limit_last_retriggered_at = Some("2026-04-30T00:00:00Z".into()); - crate::state::write_state_to(&tmp, &state).unwrap(); - - let loaded = crate::state::read_state_from(&tmp).unwrap(); - assert_eq!( - loaded.rate_limit_last_retriggered_at.as_deref(), - Some("2026-04-30T00:00:00Z") - ); - - let _ = std::fs::remove_file(&tmp); - } - - /// Bb-1: reset 時刻が未来の場合、`handle_rate_limit_retry` は Parked を返し - /// state.rate_limit_retries を変更しない (実 retry 計上は wakeup 経由で post 投稿後)。 - #[test] - fn rate_limit_retry_returns_parked_when_reset_in_future() { - let future_unix = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_secs() as i64 - + 600; - let rl = RateLimitState { - until_unix_secs: future_unix, - comment_event_time: "2026-04-30T00:00:00Z".into(), - wait_minutes: 10, - wait_seconds: 0, - }; - let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); - let pr_info = crate::util::PrInfo { - pr_number: Some(42), - repo: Some("o/r".into()), - push_time: None, - head_commit: None, - fix_push_time: None, - }; - - let outcome = handle_rate_limit_retry(&rl, &mut state, &pr_info, 3); - match outcome { - RateLimitOutcome::Parked { wakeup_at_unix } => { - assert_eq!(wakeup_at_unix, future_unix); - } - _ => panic!("expected Parked outcome for future reset, got other variant"), - } - assert_eq!(state.rate_limit_retries, 0); - assert!(state.rate_limit_last_retriggered_at.is_none()); - } - - /// Bb-1: PR 番号未確定の場合、`handle_rate_limit_retry` は Failed を返し - /// state を変更しない (caller は action_required で抜ける)。 - #[test] - fn rate_limit_retry_returns_failed_when_pr_number_missing() { - let past_unix = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_secs() as i64 - - 60; - let rl = RateLimitState { - until_unix_secs: past_unix, - comment_event_time: "2026-04-30T00:00:00Z".into(), - wait_minutes: 0, - wait_seconds: 0, - }; - let mut state = PrMonitorState::new(None, None, "t".into()); - let pr_info = crate::util::PrInfo { - pr_number: None, - repo: None, - push_time: None, - head_commit: None, - fix_push_time: None, - }; - - let outcome = handle_rate_limit_retry(&rl, &mut state, &pr_info, 3); - assert!(matches!(outcome, RateLimitOutcome::Failed(_))); - assert_eq!(state.rate_limit_retries, 0); - assert!(state.rate_limit_last_retriggered_at.is_none()); - } - - /// Bb-1: PARK signal は CronCreate 呼び出しに必要な構造化情報を含む。 - #[test] - fn format_park_signal_includes_required_fields() { - let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); - state.rate_limit_retries = 0; - let rl = RateLimitState { - until_unix_secs: 1_775_088_000, - comment_event_time: "2026-05-01T00:00:00Z".into(), - wait_minutes: 47, - wait_seconds: 0, - }; - let pr_info = crate::util::PrInfo { - pr_number: Some(42), - repo: Some("o/r".into()), - push_time: None, - head_commit: None, - fix_push_time: None, - }; - - let signal = format_park_signal(&state, &rl, &pr_info, 3); - assert!(signal.starts_with("[PR_MONITOR_PARK]")); - assert!(signal.contains("[/PR_MONITOR_PARK]")); - assert!(signal.contains("pr: 42")); - assert!(signal.contains("repo: o/r")); - assert!(signal.contains("reset_at_unix: 1775088000")); - assert!(signal.contains("wait_total_seconds: 2820")); - assert!(signal.contains("retry_count: 1")); - assert!(signal.contains("max_retries: 3")); - assert!(signal.contains("CronCreate(")); - assert!(signal.contains("durable: true")); - assert!(signal.contains("recurring: false")); - assert!(signal.contains("--monitor-only")); - } - - /// Bb-1: PR 番号 / repo が None でも format_park_signal は panic せず "?" を出す。 - #[test] - fn format_park_signal_handles_missing_pr_info() { - let state = PrMonitorState::new(None, None, "t".into()); - let rl = RateLimitState { - until_unix_secs: 1_775_088_000, - comment_event_time: "2026-05-01T00:00:00Z".into(), - wait_minutes: 5, - wait_seconds: 30, - }; - let pr_info = crate::util::PrInfo { - pr_number: None, - repo: None, - push_time: None, - head_commit: None, - fix_push_time: None, - }; - - let signal = format_park_signal(&state, &rl, &pr_info, 3); - assert!(signal.contains("pr: ?")); - assert!(signal.contains("repo: ?")); - assert!(signal.contains("wait_total_seconds: 330")); - } + use crate::config::ClassifierConfig; + use rate_limit::finalize_parked; + use review_recheck::{finalize_initial_review_park, schedule_next_review_recheck_park}; /// PR_MONITOR_STATE_FILE_OVERRIDE は process-global env var のため、 /// override 設定 / 解除を test 並行実行で race させない serial guard。 @@ -649,93 +147,6 @@ mod tests { .join("state.json") } - /// Bb-1 (T2-2): `finalize_parked` は write_state 失敗時に PARK signal emit を中止し - /// `action_required` を返却する fail-safe 経路を持つ (CodeRabbit Major #1 fix の固定化)。 - #[test] - fn finalize_parked_returns_action_required_when_write_state_fails() { - let _guard = env_override_lock(); - let bad_path = unwritable_state_path(); - std::env::set_var("PR_MONITOR_STATE_FILE_OVERRIDE", &bad_path); - - let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); - let rl = RateLimitState { - until_unix_secs: 1_775_088_000, - comment_event_time: "2026-05-01T00:00:00Z".into(), - wait_minutes: 47, - wait_seconds: 0, - }; - let pr_info = crate::util::PrInfo { - pr_number: Some(42), - repo: Some("o/r".into()), - push_time: None, - head_commit: None, - fix_push_time: None, - }; - let result = serde_json::json!({}); - - let outcome = finalize_parked(&mut state, &rl, &pr_info, 1_775_088_000, 3, &result); - - std::env::remove_var("PR_MONITOR_STATE_FILE_OVERRIDE"); - - assert_eq!( - outcome.action, "action_required", - "T2-2: write_state 失敗 → action_required で抜ける fail-safe が必要" - ); - assert!( - outcome.summary.contains("PARK signal を中止") - || outcome.summary.contains("永続化失敗"), - "summary に永続化失敗の説明が含まれること: {}", - outcome.summary - ); - } - - /// Bb-2 (T2-2): `schedule_next_review_recheck_park` は write_state 失敗時に - /// PARK signal emit を中止し `action_required` を返却する (sibling parity)。 - #[test] - fn schedule_next_review_recheck_park_returns_action_required_when_write_state_fails() { - let _guard = env_override_lock(); - let bad_path = unwritable_state_path(); - std::env::set_var("PR_MONITOR_STATE_FILE_OVERRIDE", &bad_path); - - let mut state = - PrMonitorState::new(Some(42), Some("o/r".into()), "2026-05-01T00:00:00Z".into()); - state.review_recheck_count = 1; - let checker_path = std::path::PathBuf::from("dummy-checker"); - let pr_info = crate::util::PrInfo { - pr_number: Some(42), - repo: Some("o/r".into()), - push_time: Some("2026-05-01T00:00:00Z".into()), - head_commit: None, - fix_push_time: None, - }; - let rate_limit_config = RateLimitConfig::default(); - let classifier_config = ClassifierConfig::default(); - let ctx = PollContext { - checker: &checker_path, - push_time: "2026-05-01T00:00:00Z", - fix_push_time: None, - pr_info: &pr_info, - rate_limit_config: &rate_limit_config, - classifier_config: &classifier_config, - start: std::time::Instant::now(), - max_duration: 600, - skip_ci: false, - skip_coderabbit: false, - initial_review_wait_secs: 300, - review_recheck_wait_secs: 300, - max_review_rechecks: 3, - }; - - let outcome = schedule_next_review_recheck_park(&mut state, &ctx); - - std::env::remove_var("PR_MONITOR_STATE_FILE_OVERRIDE"); - - assert_eq!( - outcome.action, "action_required", - "T2-2 sibling parity: review park も write_state 失敗 → action_required で抜けること" - ); - } - fn invoke_finalize_parked_with_bad_path(pr_info: &crate::util::PrInfo) -> PollResult { let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); let rl = RateLimitState { @@ -797,317 +208,6 @@ mod tests { finalize_initial_review_park(&ctx) } - fn seed_stale_recheck_state(tmp_path: &std::path::Path) { - let mut stale_state = - PrMonitorState::new(Some(42), Some("o/r".into()), "2026-05-01T00:00:00Z".into()); - stale_state.review_recheck_count = 3; - stale_state.action = "action_required".into(); - crate::state::write_state_to(tmp_path, &stale_state).unwrap(); - } - - /// Bb-3 (順位 55): `max_review_rechecks` の config 化が実際に PARK signal に - /// 反映されることを machine-enforce する (default 3 ではなく custom 値が出力されること)。 - #[test] - fn format_review_park_signal_uses_configured_max_rechecks() { - let state = - PrMonitorState::new(Some(42), Some("o/r".into()), "2026-05-01T00:00:00Z".into()); - let pr_info = crate::util::PrInfo { - pr_number: Some(42), - repo: Some("o/r".into()), - push_time: Some("2026-05-01T00:00:00Z".into()), - head_commit: None, - fix_push_time: None, - }; - let checker = std::path::PathBuf::from("dummy"); - let rate_limit_config = RateLimitConfig::default(); - let classifier_config = ClassifierConfig::default(); - let ctx = PollContext { - checker: &checker, - push_time: "2026-05-01T00:00:00Z", - fix_push_time: None, - pr_info: &pr_info, - rate_limit_config: &rate_limit_config, - classifier_config: &classifier_config, - start: std::time::Instant::now(), - max_duration: 600, - skip_ci: false, - skip_coderabbit: false, - initial_review_wait_secs: 120, - review_recheck_wait_secs: 240, - max_review_rechecks: 7, - }; - - let signal = format_review_park_signal(&state, &ctx); - - assert!( - signal.contains("max_rechecks: 7"), - "PARK signal に config 値 (max_rechecks: 7) が反映されること: {}", - signal - ); - assert!( - !signal.contains("max_rechecks: 3"), - "default 値 3 が hard-coded で残っていないこと: {}", - signal - ); - } - - #[test] - fn round_up_to_next_minute_keeps_value_when_seconds_already_zero() { - let aligned = 1_775_044_800; - assert_eq!(round_up_to_next_minute(aligned), aligned); - } - - #[test] - fn round_up_to_next_minute_rounds_up_when_seconds_present() { - let unaligned = 1_775_044_819; - assert_eq!(round_up_to_next_minute(unaligned), 1_775_044_860); - } - - #[test] - fn round_up_to_next_minute_rounds_up_one_second_before_next_minute() { - let one_sec_before = 1_775_044_859; - assert_eq!(round_up_to_next_minute(one_sec_before), 1_775_044_860); - } - - #[test] - fn round_up_to_next_minute_one_second_past_minute_rounds_up_to_next_full_minute() { - let one_sec_past = 1_775_044_801; - assert_eq!(round_up_to_next_minute(one_sec_past), 1_775_044_860); - } - - #[test] - fn round_up_to_next_minute_handles_zero_input_as_minute_zero() { - assert_eq!(round_up_to_next_minute(0), 0); - } - - #[test] - fn compute_safe_minute_returns_sentinel_when_input_zero() { - let (safe_unix, safe_iso) = compute_safe_minute_for_park_signal(0); - assert_eq!(safe_unix, 0); - assert_eq!(safe_iso, "?"); - } - - #[test] - fn compute_safe_minute_returns_sentinel_when_input_negative() { - let (safe_unix, safe_iso) = compute_safe_minute_for_park_signal(-1); - assert_eq!(safe_unix, 0); - assert_eq!(safe_iso, "?"); - } - - #[test] - fn compute_safe_minute_rounds_up_and_formats_iso_when_input_unaligned() { - let (safe_unix, safe_iso) = compute_safe_minute_for_park_signal(1_775_044_819); - assert_eq!(safe_unix, 1_775_044_860); - assert_eq!(safe_iso, "2026-04-01T12:01:00Z"); - } - - #[test] - fn compute_safe_minute_preserves_iso_when_input_already_aligned() { - let (safe_unix, safe_iso) = compute_safe_minute_for_park_signal(1_775_044_800); - assert_eq!(safe_unix, 1_775_044_800); - assert_eq!(safe_iso, "2026-04-01T12:00:00Z"); - } - - #[test] - fn format_review_park_signal_includes_safe_minute_iso_utc_field() { - let mut state = - PrMonitorState::new(Some(99), Some("o/r".into()), "2026-04-01T00:00:00Z".into()); - state.next_wakeup_at_unix = Some(1_775_044_819); - let pr_info = crate::util::PrInfo { - pr_number: Some(99), - repo: Some("o/r".into()), - push_time: Some("2026-04-01T00:00:00Z".into()), - head_commit: None, - fix_push_time: None, - }; - let checker = std::path::PathBuf::from("dummy"); - let rate_limit_config = RateLimitConfig::default(); - let classifier_config = ClassifierConfig::default(); - let ctx = PollContext { - checker: &checker, - push_time: "2026-04-01T00:00:00Z", - fix_push_time: None, - pr_info: &pr_info, - rate_limit_config: &rate_limit_config, - classifier_config: &classifier_config, - start: std::time::Instant::now(), - max_duration: 600, - skip_ci: false, - skip_coderabbit: false, - initial_review_wait_secs: 300, - review_recheck_wait_secs: 300, - max_review_rechecks: 3, - }; - - let signal = format_review_park_signal(&state, &ctx); - - assert!( - signal.contains("safe_minute_at_unix: 1775044860"), - "PARK signal に safe_minute_at_unix の round-UP 値が含まれること: {}", - signal - ); - assert!( - signal.contains("safe_minute_at_iso_utc: 2026-04-01T12:01:00Z"), - "PARK signal に safe_minute_at_iso_utc の round-UP ISO が含まれること: {}", - signal - ); - } - - /// CR Major #2 fix (Bb-2 PR #114 review): fresh push 経路では `finalize_initial_review_park` - /// が `review_recheck_count` を 0 に明示リセットすること。前サイクルが MAX 到達 (count=3) - /// で残った state を持ち越さないことを machine-enforce する。 - #[test] - fn finalize_initial_review_park_resets_recheck_count() { - let _guard = env_override_lock(); - let tmp_path = std::env::temp_dir().join(format!( - "pr-monitor-CR-M2-{}-state.json", - std::process::id() - )); - std::env::set_var("PR_MONITOR_STATE_FILE_OVERRIDE", &tmp_path); - seed_stale_recheck_state(&tmp_path); - - let pr_info = pr_info_for_initial_review_park_test(); - let checker = std::path::PathBuf::from("dummy"); - let rate_limit_config = RateLimitConfig::default(); - let classifier_config = ClassifierConfig::default(); - let ctx = make_default_test_ctx(&checker, &pr_info, &rate_limit_config, &classifier_config); - - let outcome = finalize_initial_review_park(&ctx); - let persisted = crate::state::read_state_from(&tmp_path).unwrap(); - - std::env::remove_var("PR_MONITOR_STATE_FILE_OVERRIDE"); - let _ = std::fs::remove_file(&tmp_path); - - assert_eq!(outcome.action, "parked_review_recheck"); - assert_eq!( - persisted.review_recheck_count, 0, - "CR Major #2: fresh push 経路で count=3 が残らず 0 にリセットされること" - ); - assert_eq!( - persisted.head_commit.as_deref(), - Some("abc1234"), - "CR Major #1: fresh push 経路で head_commit が pr_info から保存されること" - ); - } - - fn pr_info_for_initial_review_park_test() -> crate::util::PrInfo { - crate::util::PrInfo { - pr_number: Some(42), - repo: Some("o/r".into()), - push_time: Some("2026-05-01T00:00:00Z".into()), - head_commit: Some("abc1234".into()), - fix_push_time: None, - } - } - - fn make_default_test_ctx<'a>( - checker: &'a std::path::Path, - pr_info: &'a crate::util::PrInfo, - rate_limit_config: &'a RateLimitConfig, - classifier_config: &'a ClassifierConfig, - ) -> PollContext<'a> { - PollContext { - checker, - push_time: "2026-05-01T00:00:00Z", - fix_push_time: None, - pr_info, - rate_limit_config, - classifier_config, - start: std::time::Instant::now(), - max_duration: 600, - skip_ci: false, - skip_coderabbit: false, - initial_review_wait_secs: 300, - review_recheck_wait_secs: 300, - max_review_rechecks: 3, - } - } - - /// PR #120 W-001 follow-up (順位 83): `enrich_with_classifier` の `!config.enabled` - /// guard を **単独で** 検証する。`findings` を非空 (= `findings.is_empty()` guard - /// 不発)、`enabled = false` (= 本 guard 発火) にして 2 つの OR guard を直交させる。 - /// - /// 検証対象 field `state.classified_findings` を sentinel で pre-populate し、 - /// 早期 return しなかった場合の代入 (`state.classified_findings = classified;`) - /// を sentinel 消失として検出する設計。空のまま渡すと「不変=空」が早期 return - /// 由来か他経路由来か判別できないため sentinel 必須。 - #[test] - fn enrich_with_classifier_skips_when_disabled() { - use crate::classifier_runner::ClassifiedFinding; - - let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); - state.findings = vec![Finding { - severity: "Major".into(), - file: "f.rs".into(), - line: "1".into(), - issue: "issue".into(), - suggestion: "fix".into(), - source: "coderabbit".into(), - }]; - let sentinel = ClassifiedFinding { - finding: Finding { - severity: "Minor".into(), - file: "sentinel.rs".into(), - line: "1".into(), - issue: "sentinel".into(), - suggestion: "must not be overwritten".into(), - source: "test".into(), - }, - action: "auto_fix".into(), - action_confidence: 0.99, - normalized_issue: None, - fallback_reason: None, - }; - state.classified_findings = vec![sentinel.clone()]; - let disabled = ClassifierConfig { enabled: false, ..ClassifierConfig::default() }; - - enrich_with_classifier(&mut state, &disabled); - - assert_eq!( - state.classified_findings, - vec![sentinel], - "!config.enabled guard should early return before any mutation" - ); - } - - /// `state.findings.is_empty()` guard (`enrich_with_classifier` 2 番目の早期 return) - /// を単独で検証する。`enabled = true` (明示、= `!config.enabled` guard 不発)、 - /// `findings` 空 (= 本 guard 発火) にして他条件と直交させる。 - #[test] - fn enrich_with_classifier_skips_when_findings_empty() { - use crate::classifier_runner::ClassifiedFinding; - - let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); - assert!( - state.findings.is_empty(), - "test precondition: findings must be empty so `!enabled` guard stays unfired" - ); - let sentinel = ClassifiedFinding { - finding: Finding { - severity: "Minor".into(), - file: "sentinel.rs".into(), - line: "1".into(), - issue: "sentinel".into(), - suggestion: "must not be overwritten".into(), - source: "test".into(), - }, - action: "auto_fix".into(), - action_confidence: 0.99, - normalized_issue: None, - fallback_reason: None, - }; - state.classified_findings = vec![sentinel.clone()]; - let enabled = ClassifierConfig { enabled: true, ..ClassifierConfig::default() }; - - enrich_with_classifier(&mut state, &enabled); - - assert_eq!( - state.classified_findings, - vec![sentinel], - "findings.is_empty() guard should early return before any mutation" - ); - } - /// Bb-2 (T2-2) + Bb-3 follow-up: 3 つの finalize_* park sibling /// (`finalize_parked` / `schedule_next_review_recheck_park` / `finalize_initial_review_park`) /// は全て write_state 失敗で `action_required` を返す invariant を 1 テストで @@ -1154,251 +254,4 @@ mod tests { "sibling parity (review_recheck ↔ initial_review)" ); } - - fn setup_posted_retrigger_fixture() -> (PrMonitorState, RateLimitState, crate::util::PrInfo) { - let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); - state.action = "continue_monitoring".into(); - state.rate_limit_retries = 1; - let rl = RateLimitState { - until_unix_secs: 0, - comment_event_time: "2026-05-08T00:00:00Z".into(), - wait_minutes: 5, - wait_seconds: 0, - }; - let pr_info = crate::util::PrInfo { - pr_number: Some(1), - repo: Some("o/r".into()), - push_time: Some("2026-05-01T00:00:00Z".into()), - head_commit: Some("abc1234".into()), - fix_push_time: None, - }; - (state, rl, pr_info) - } - - #[test] - fn finalize_posted_retrigger_schedules_park_after_post() { - let _guard = env_override_lock(); - let tmp = tempfile::tempdir().unwrap(); - let state_path = tmp.path().join("state.json"); - std::env::set_var("PR_MONITOR_STATE_FILE_OVERRIDE", &state_path); - - let (mut state, rl, pr_info) = setup_posted_retrigger_fixture(); - let result = finalize_posted_retrigger(&mut state, &rl, &pr_info, 300, &serde_json::Value::Null); - - std::env::remove_var("PR_MONITOR_STATE_FILE_OVERRIDE"); - - let park_result = result.expect("順位 80 fix: Posted 後は必ず park を返し silent exit を防ぐ"); - assert_eq!(park_result.action, "parked_review_recheck"); - assert_eq!(state.wakeup_reason.as_deref(), Some("rate_limit_post_retrigger")); - let now_unix = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_secs() as i64; - let wakeup = state.next_wakeup_at_unix.expect("next_wakeup_at_unix が設定される"); - assert!(wakeup > now_unix && wakeup <= now_unix + 301); - assert_eq!(state.rate_limit_last_retriggered_at.as_deref(), Some("2026-05-08T00:00:00Z")); - } - - #[test] - fn finalize_posted_retrigger_action_required_when_write_state_fails() { - let _guard = env_override_lock(); - let bad_path = unwritable_state_path(); - std::env::set_var("PR_MONITOR_STATE_FILE_OVERRIDE", &bad_path); - - let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); - state.action = "continue_monitoring".into(); - let rl = RateLimitState { - until_unix_secs: 0, - comment_event_time: "2026-05-08T00:00:00Z".into(), - wait_minutes: 5, - wait_seconds: 0, - }; - let pr_info = crate::util::PrInfo { - pr_number: Some(1), - repo: Some("o/r".into()), - push_time: Some("2026-05-01T00:00:00Z".into()), - head_commit: None, - fix_push_time: None, - }; - - let result = finalize_posted_retrigger(&mut state, &rl, &pr_info, 300, &serde_json::Value::Null); - - std::env::remove_var("PR_MONITOR_STATE_FILE_OVERRIDE"); - - assert!(result.is_some()); - assert_eq!( - result.unwrap().action, - "action_required", - "write_state 失敗時は action_required で抜ける (sibling parity with finalize_parked)" - ); - } - - /// 順位 141: shortcut signal の trigger 条件 (mergeable CLEAN + unresolved 0) で true。 - #[test] - fn evaluate_rate_limit_shortcut_when_all_conditions_met() { - let m = MergeableStatus { - mergeable: "MERGEABLE".into(), - merge_state: "CLEAN".into(), - }; - let cr = crate::state::CodeRabbitState { - review_state: "approved".into(), - new_comments: 0, - actionable_comments: Some(0), - unresolved_threads: Some(0), - }; - assert!(evaluate_rate_limit_shortcut(Some(&cr), &m)); - } - - /// 順位 141: unresolved thread が残っていれば shortcut を抑止 (CR の指摘が未対応)。 - #[test] - fn evaluate_rate_limit_shortcut_blocks_when_unresolved_threads_exist() { - let m = MergeableStatus { - mergeable: "MERGEABLE".into(), - merge_state: "CLEAN".into(), - }; - let cr = crate::state::CodeRabbitState { - review_state: "commented".into(), - new_comments: 1, - actionable_comments: Some(1), - unresolved_threads: Some(1), - }; - assert!(!evaluate_rate_limit_shortcut(Some(&cr), &m)); - } - - /// 順位 141: mergeable が BLOCKED なら shortcut を抑止 (GitHub 側で merge 不可)。 - #[test] - fn evaluate_rate_limit_shortcut_blocks_when_not_mergeable() { - let m = MergeableStatus { - mergeable: "BLOCKED".into(), - merge_state: "BLOCKED".into(), - }; - assert!(!evaluate_rate_limit_shortcut(None, &m)); - } - - /// 順位 141: CR state が None (初回 review なし) でも mergeable CLEAN なら shortcut 可。 - #[test] - fn evaluate_rate_limit_shortcut_passes_when_coderabbit_none() { - let m = MergeableStatus { - mergeable: "MERGEABLE".into(), - merge_state: "CLEAN".into(), - }; - assert!(evaluate_rate_limit_shortcut(None, &m)); - } - - /// 順位 141: signal format に必須 field が全て含まれ、Claude が AskUserQuestion 化できる。 - #[test] - fn format_shortcut_signal_includes_required_fields() { - let rl = crate::state::RateLimitState { - until_unix_secs: 1_779_432_672, - comment_event_time: "2026-05-22T06:08:02Z".into(), - wait_minutes: 38, - wait_seconds: 30, - }; - let pr_info = crate::util::PrInfo { - pr_number: Some(169), - repo: Some("aloekun/claude-code-hook-test".into()), - push_time: None, - head_commit: None, - fix_push_time: None, - }; - let m = MergeableStatus { - mergeable: "MERGEABLE".into(), - merge_state: "CLEAN".into(), - }; - let sig = format_shortcut_signal(&rl, &pr_info, &m); - assert!(sig.starts_with("[RATE_LIMIT_BUT_MERGEABLE]")); - assert!(sig.contains("[/RATE_LIMIT_BUT_MERGEABLE]")); - assert!(sig.contains("pr: 169")); - assert!(sig.contains("repo: aloekun/claude-code-hook-test")); - assert!(sig.contains("rate_limit_wait_seconds: 2310")); - assert!(sig.contains("mergeable: MERGEABLE")); - assert!(sig.contains("merge_state: CLEAN")); - assert!(sig.contains("AskUserQuestion")); - } - - /// 順位 141: `fix_push_time` の write-once 不変条件 — - /// `finalize_initial_review_park` が state に既存の `fix_push_time` がある場合に - /// `ctx.fix_push_time` の値で上書きしないことを検証する。 - /// - /// `ctx.fix_push_time = Some("new_time")` (= None ではなく非 None) を使うことで、 - /// or_else 被演算子の入れ替えバグを discriminate できる。 - #[test] - fn finalize_initial_review_park_preserves_existing_fix_push_time() { - let _guard = env_override_lock(); - let tmp = tempfile::tempdir().unwrap(); - let state_path = tmp.path().join("state.json"); - std::env::set_var("PR_MONITOR_STATE_FILE_OVERRIDE", &state_path); - - let mut seeded = - PrMonitorState::new(Some(42), Some("o/r".into()), "2026-05-01T00:00:00Z".into()); - seeded.fix_push_time = Some("2026-05-22T06:06:00Z".into()); - crate::state::write_state_to(&state_path, &seeded).unwrap(); - - let pr_info = crate::util::PrInfo { - pr_number: Some(42), - repo: Some("o/r".into()), - push_time: Some("2026-05-01T00:00:00Z".into()), - head_commit: Some("abc1234".into()), - fix_push_time: None, - }; - let checker = std::path::PathBuf::from("dummy"); - let rate_limit_config = RateLimitConfig::default(); - let classifier_config = ClassifierConfig::default(); - let mut ctx = - make_default_test_ctx(&checker, &pr_info, &rate_limit_config, &classifier_config); - let ctx_fix_push_time_must_lose = "2026-05-22T06:10:00Z"; - ctx.fix_push_time = Some(ctx_fix_push_time_must_lose); - - finalize_initial_review_park(&ctx); - let persisted = crate::state::read_state_from(&state_path).unwrap(); - std::env::remove_var("PR_MONITOR_STATE_FILE_OVERRIDE"); - - assert_eq!( - persisted.fix_push_time.as_deref(), - Some("2026-05-22T06:06:00Z"), - "write-once: state に既存 fix_push_time がある場合、ctx の値で上書きしない" - ); - } - - /// 順位 141: `fix_push_time` の write-once 不変条件 — - /// `finalize_review_recheck_park` が state に既存の `fix_push_time` がある場合に - /// `ctx.fix_push_time` の値で上書きしないことを検証する。 - #[test] - fn finalize_review_recheck_park_preserves_existing_fix_push_time() { - let _guard = env_override_lock(); - let tmp = tempfile::tempdir().unwrap(); - let state_path = tmp.path().join("state.json"); - std::env::set_var("PR_MONITOR_STATE_FILE_OVERRIDE", &state_path); - - let mut seeded = - PrMonitorState::new(Some(42), Some("o/r".into()), "2026-05-01T00:00:00Z".into()); - seeded.fix_push_time = Some("2026-05-22T06:06:00Z".into()); - seeded.review_recheck_count = 0; - crate::state::write_state_to(&state_path, &seeded).unwrap(); - - let pr_info = crate::util::PrInfo { - pr_number: Some(42), - repo: Some("o/r".into()), - push_time: Some("2026-05-01T00:00:00Z".into()), - head_commit: Some("abc1234".into()), - fix_push_time: None, - }; - let checker = std::path::PathBuf::from("dummy"); - let rate_limit_config = RateLimitConfig::default(); - let classifier_config = ClassifierConfig::default(); - let mut ctx = - make_default_test_ctx(&checker, &pr_info, &rate_limit_config, &classifier_config); - let ctx_fix_push_time_must_lose = "2026-05-22T06:10:00Z"; - ctx.fix_push_time = Some(ctx_fix_push_time_must_lose); - - finalize_review_recheck_park(&ctx); - let persisted = crate::state::read_state_from(&state_path).unwrap(); - std::env::remove_var("PR_MONITOR_STATE_FILE_OVERRIDE"); - - assert_eq!( - persisted.fix_push_time.as_deref(), - Some("2026-05-22T06:06:00Z"), - "write-once: state に既存 fix_push_time がある場合、ctx の値で上書きしない" - ); - } } diff --git a/src/cli-pr-monitor/src/stages/poll/rate_limit.rs b/src/cli-pr-monitor/src/stages/poll/rate_limit.rs index 08e1bdc5..cde81aeb 100644 --- a/src/cli-pr-monitor/src/stages/poll/rate_limit.rs +++ b/src/cli-pr-monitor/src/stages/poll/rate_limit.rs @@ -2,13 +2,13 @@ //! //! - `handle_rate_limit_branch` + `dispatch_rate_limit_outcome` (branch entry) //! - `finalize_posted_retrigger` / `finalize_parked` (state finalize) -//! - `emit_shortcut_signal_if_eligible` / `fetch_mergeable_status` / -//! `evaluate_rate_limit_shortcut` / `format_shortcut_signal` (順位 141 shortcut) //! - `handle_rate_limit_retry` / `post_review_immediately` (retry logic) -//! - `format_park_signal` (rate_limit_retry PARK signal) -//! - `MergeableStatus` / `RateLimitOutcome` (DTO/enum) +//! - `RateLimitOutcome` (enum) //! - `make_max_retries_result` / `make_action_required_result` (general result builders、 //! review_recheck.rs からも参照される) +//! +//! signal 整形部分 (`format_park_signal` / shortcut signal / +//! `format_posted_retrigger_review_park_signal`) は `rate_limit_signal.rs` に分離。 use crate::config::RateLimitConfig; use crate::log::log_info; @@ -16,7 +16,10 @@ use crate::runner::run_gh_quiet; use crate::state::{write_state, PrMonitorState}; use crate::util::PrInfo; -use super::review_recheck::round_up_to_next_minute; +use super::rate_limit_signal::{ + emit_shortcut_signal_if_eligible, format_park_signal, + format_posted_retrigger_review_park_signal, +}; use super::{make_park_poll_result, PollResult}; /// rate-limit 検出 branch を集約する。 @@ -74,13 +77,9 @@ fn dispatch_rate_limit_outcome( result: &serde_json::Value, ) -> Option { match handle_rate_limit_retry(rl, state, pr_info, max_retries) { - RateLimitOutcome::Posted => finalize_posted_retrigger( - state, - rl, - pr_info, - review_recheck_wait_secs, - result, - ), + RateLimitOutcome::Posted => { + finalize_posted_retrigger(state, rl, pr_info, review_recheck_wait_secs, result) + } RateLimitOutcome::Parked { wakeup_at_unix } => Some(finalize_parked( state, rl, @@ -184,98 +183,6 @@ pub(super) fn finalize_parked( } } -/// 順位 141: rate-limit 検出 + mergeable CLEAN + 未解決 thread なしの 3 条件が揃ったとき -/// `[RATE_LIMIT_BUT_MERGEABLE]` signal を stdout に出力する shortcut path。 -fn emit_shortcut_signal_if_eligible( - state: &PrMonitorState, - rl: &crate::state::RateLimitState, - pr_info: &PrInfo, -) { - let Some(mergeable) = fetch_mergeable_status(pr_info) else { - return; - }; - if !evaluate_rate_limit_shortcut(state.coderabbit.as_ref(), &mergeable) { - return; - } - println!("{}", format_shortcut_signal(rl, pr_info, &mergeable)); -} - -/// 順位 141: PR の mergeable / mergeStateStatus を gh で取得。失敗時は None。 -fn fetch_mergeable_status(pr_info: &PrInfo) -> Option { - let pr = pr_info.pr_number?; - let pr_str = pr.to_string(); - let mut args: Vec<&str> = vec![ - "pr", - "view", - &pr_str, - "--json", - "mergeable,mergeStateStatus", - ]; - if let Some(repo) = pr_info.repo.as_deref() { - args.push("--repo"); - args.push(repo); - } - let json_str = run_gh_quiet(&args)?; - let parsed: serde_json::Value = serde_json::from_str(&json_str).ok()?; - Some(MergeableStatus { - mergeable: parsed.get("mergeable")?.as_str()?.to_string(), - merge_state: parsed.get("mergeStateStatus")?.as_str()?.to_string(), - }) -} - -/// 順位 141: mergeable + 未解決 thread の 3 条件評価を pure 関数化 (test 容易性)。 -pub(super) fn evaluate_rate_limit_shortcut( - coderabbit: Option<&crate::state::CodeRabbitState>, - mergeable: &MergeableStatus, -) -> bool { - let cr_clean = coderabbit - .map(|c| c.unresolved_threads.unwrap_or(0) == 0) - .unwrap_or(true); - mergeable.mergeable == "MERGEABLE" && mergeable.merge_state == "CLEAN" && cr_clean -} - -/// 順位 141: `[RATE_LIMIT_BUT_MERGEABLE]` signal を構築 (pure)。 -pub(super) fn format_shortcut_signal( - rl: &crate::state::RateLimitState, - pr_info: &PrInfo, - mergeable: &MergeableStatus, -) -> String { - let pr = pr_info - .pr_number - .map(|n| n.to_string()) - .unwrap_or_else(|| "?".into()); - let repo = pr_info.repo.as_deref().unwrap_or("?"); - let reset_iso = if rl.until_unix_secs > 0 { - lib_pending_file::epoch_secs_to_iso8601(rl.until_unix_secs as u64) - } else { - "?".into() - }; - let wait_total_secs = rl.wait_minutes * 60 + rl.wait_seconds; - format!( - "[RATE_LIMIT_BUT_MERGEABLE] -pr: {pr} -repo: {repo} -rate_limit_reset_at_iso_utc: {reset_iso} -rate_limit_wait_seconds: {wait_total_secs} -mergeable: {merge} -merge_state: {state} - -ACTION REQUIRED: ユーザーに以下 2 択を AskUserQuestion で問うこと: - A: 今すぐ merge する (rate-limit reset を待たない、CR 2 回目 review なしで進める) - B: reset を待って通常 auto-retry flow に乗る -[/RATE_LIMIT_BUT_MERGEABLE]", - merge = mergeable.mergeable, - state = mergeable.merge_state, - ) -} - -/// 順位 141: gh `pr view --json mergeable,mergeStateStatus` の結果を保持する DTO。 -#[derive(Debug, Clone)] -pub(crate) struct MergeableStatus { - pub(crate) mergeable: String, - pub(crate) merge_state: String, -} - fn make_max_retries_result(state: &PrMonitorState, result: &serde_json::Value) -> PollResult { let summary = format!( "CodeRabbit rate-limit が {} 回再試行後も継続。手動で `@coderabbitai review` を投稿してください", @@ -363,168 +270,128 @@ fn post_review_immediately(pr: u64, state: &mut PrMonitorState) -> RateLimitOutc RateLimitOutcome::Posted } -struct PostedRetriggerParkFields { - pr: String, - repo: String, - wakeup_unix: i64, - wakeup_iso: String, - safe_unix: i64, - safe_iso: String, - wait_secs: i64, - recheck: u32, - exe: String, - cwd: String, -} +#[cfg(test)] +mod tests { + use super::*; + use crate::state::RateLimitState; -fn collect_posted_retrigger_park_fields( - state: &PrMonitorState, - pr_info: &PrInfo, -) -> PostedRetriggerParkFields { - let pr = pr_info - .pr_number - .map(|n| n.to_string()) - .unwrap_or_else(|| "?".into()); - let repo = pr_info.repo.as_deref().unwrap_or("?").to_string(); - let wakeup_unix = state.next_wakeup_at_unix.unwrap_or(0); - let wakeup_iso = if wakeup_unix > 0 { - lib_pending_file::epoch_secs_to_iso8601(wakeup_unix as u64) - } else { - "?".into() - }; - let safe_unix = if wakeup_unix > 0 { round_up_to_next_minute(wakeup_unix) } else { 0 }; - let safe_iso = if safe_unix > 0 { - lib_pending_file::epoch_secs_to_iso8601(safe_unix as u64) - } else { - "?".into() - }; - let now_unix = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs() as i64) - .unwrap_or(0); - let wait_secs = (wakeup_unix - now_unix).max(0); - let exe = std::env::current_exe() - .map(|p| p.display().to_string()) - .unwrap_or_else(|_| "cli-pr-monitor.exe".into()); - let cwd = std::env::current_dir() - .map(|p| p.display().to_string()) - .unwrap_or_else(|_| ".".into()); - PostedRetriggerParkFields { - pr, repo, wakeup_unix, wakeup_iso, safe_unix, safe_iso, - wait_secs, recheck: state.review_recheck_count, exe, cwd, + #[test] + fn rate_limit_state_persists_retries_across_polls() { + let tmp = std::env::temp_dir().join(format!("test-rl-retries-{}.json", std::process::id())); + let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); + state.rate_limit_retries = 2; + state.rate_limit = Some(RateLimitState { + until_unix_secs: 1_735_689_600, + comment_event_time: "2026-04-30T00:00:00Z".into(), + wait_minutes: 5, + wait_seconds: 13, + }); + crate::state::write_state_to(&tmp, &state).unwrap(); + + let loaded = crate::state::read_state_from(&tmp).unwrap(); + assert_eq!(loaded.rate_limit_retries, 2); + assert_eq!( + loaded.rate_limit.as_ref().unwrap().until_unix_secs, + 1_735_689_600 + ); + + let _ = std::fs::remove_file(&tmp); } -} -/// rate-limit 解消後の `@coderabbitai review` 投稿完了 → review 待ち PARK signal を生成する。 -/// -/// `format_park_signal` (reason: rate_limit_retry, reset_at_unix 基準) とは異なり、 -/// `state.next_wakeup_at_unix` の review 待ち wakeup を基準に `reason: review_recheck` で -/// discriminate する。過去 timestamp (`rl.until_unix_secs`) を CronCreate に誤使用しない。 -pub(super) fn format_posted_retrigger_review_park_signal( - state: &PrMonitorState, - pr_info: &PrInfo, -) -> String { - let PostedRetriggerParkFields { - pr, repo, wakeup_unix, wakeup_iso, safe_unix, safe_iso, wait_secs, recheck, exe, cwd, - } = collect_posted_retrigger_park_fields(state, pr_info); - format!( - "[PR_MONITOR_PARK] -reason: review_recheck -pr: {pr} -repo: {repo} -next_wakeup_at_unix: {wakeup_unix} -next_wakeup_at_iso_utc: {wakeup_iso} -safe_minute_at_unix: {safe_unix} -safe_minute_at_iso_utc: {safe_iso} -wait_total_seconds: {wait_secs} -recheck_count: {recheck} -exe: {exe} -cwd: {cwd} - -ACTION REQUIRED: please schedule one-shot wakeup using CronCreate. - -Cron spec derivation (apply 2 orthogonal constraints sequentially): - Step 1 (round-UP, already applied at the source): use `safe_minute_at_iso_utc` - (= next_wakeup_at_iso_utc with seconds rounded UP to next full minute). - Step 2 (avoid :00 / :30 minute due to 90s pre-fire jitter): convert - `safe_minute_at_iso_utc` to LOCAL TZ, then bump the minute by +1 if it - lands on :00 or :30. Use the resulting `HH:MM` as the cron field. - Reference: ~/.claude/rules/common/development-workflow.md - § Cron スケジューリングの秒 → 分 round-UP - -CronCreate({{ - cron: \"\", - recurring: false, - durable: true, - prompt: \"Wakeup: review recheck for PR #{pr} ({repo}). cd \\\"{cwd}\\\" && \\\"{exe}\\\" --monitor-only\" -}}) -[/PR_MONITOR_PARK]" - ) -} + #[test] + fn rate_limit_default_config_allows_retry_within_limit() { + let cfg = RateLimitConfig::default(); + assert!(cfg.auto_retry_enabled); + assert_eq!(cfg.max_retries, 3); + assert!(2 < cfg.max_retries); + assert!(3 >= cfg.max_retries); + } -/// PARK signal を stdout に書き出すための pure 関数 (Bb-1)。 -pub(crate) fn format_park_signal( - state: &PrMonitorState, - rl: &crate::state::RateLimitState, - pr_info: &PrInfo, - max_retries: u32, -) -> String { - let pr = pr_info - .pr_number - .map(|n| n.to_string()) - .unwrap_or_else(|| "?".into()); - let repo = pr_info.repo.as_deref().unwrap_or("?"); - let reset_iso = if rl.until_unix_secs > 0 { - lib_pending_file::epoch_secs_to_iso8601(rl.until_unix_secs as u64) - } else { - "?".into() - }; - let wait_total_secs = rl.wait_minutes * 60 + rl.wait_seconds; - let exe = std::env::current_exe() - .map(|p| p.display().to_string()) - .unwrap_or_else(|_| "cli-pr-monitor.exe".into()); - let cwd = std::env::current_dir() - .map(|p| p.display().to_string()) - .unwrap_or_else(|_| ".".into()); - let retry_attempt = state.rate_limit_retries + 1; - - format!( - "[PR_MONITOR_PARK] -reason: rate_limit_retry -pr: {pr} -repo: {repo} -reset_at_unix: {until} -reset_at_iso_utc: {reset_iso} -wait_total_seconds: {wait_total_secs} -retry_count: {retry_attempt} -max_retries: {max_retries} -exe: {exe} -cwd: {cwd} - -ACTION REQUIRED: please schedule one-shot wakeup using CronCreate. - -CronCreate({{ - cron: \"\", - recurring: false, - durable: true, - prompt: \"Wakeup: rate-limit retry for PR #{pr} ({repo}). cd \\\"{cwd}\\\" && \\\"{exe}\\\" --monitor-only\" -}}) -[/PR_MONITOR_PARK]", - until = rl.until_unix_secs, - ) -} + /// 同じ rate-limit comment が iteration 跨ぎで残った場合に dedup が働くことを検証する。 + /// + /// シナリオ (advisor 発見のバグ): + /// - Iter 1: comment A, retries=0, last_retriggered=None → handle 対象 + /// - Iter 2: 同じ comment A still in PR, last_retriggered=A → 即時 retrigger を skip + /// - Iter 3: CR が新たな rate-limit comment B を投稿, last_retriggered=A != B → 再 handle 対象 + /// + /// dedup なしだと Iter 2/3 で sleep_secs=0 となり数秒で max_retries を消費する。 + #[test] + fn rate_limit_dedup_skips_repeated_comment() { + let comment_a = "2026-04-30T00:00:00Z"; + let comment_b = "2026-04-30T00:30:00Z"; + + let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); + let rl_a = RateLimitState { + until_unix_secs: 0, + comment_event_time: comment_a.into(), + wait_minutes: 5, + wait_seconds: 0, + }; + let already_handled_iter1 = state.rate_limit_last_retriggered_at.as_deref() + == Some(rl_a.comment_event_time.as_str()); + assert!( + !already_handled_iter1, + "Iter 1: 初回 detection は handle されるべき" + ); -#[cfg(test)] -mod tests { - use super::format_posted_retrigger_review_park_signal; - use crate::state::PrMonitorState; + state.rate_limit_retries = 1; + state.rate_limit_last_retriggered_at = Some(comment_a.into()); + + let already_handled_iter2 = state.rate_limit_last_retriggered_at.as_deref() + == Some(rl_a.comment_event_time.as_str()); + assert!( + already_handled_iter2, + "Iter 2: 同じ comment は dedup で skip されるべき" + ); + + let rl_b = RateLimitState { + until_unix_secs: 0, + comment_event_time: comment_b.into(), + wait_minutes: 5, + wait_seconds: 0, + }; + let already_handled_iter3 = state.rate_limit_last_retriggered_at.as_deref() + == Some(rl_b.comment_event_time.as_str()); + assert!( + !already_handled_iter3, + "Iter 3: 新 comment は再度 handle 対象" + ); + } + + /// state.json round-trip で rate_limit_last_retriggered_at が persistence される。 + #[test] + fn rate_limit_last_retriggered_at_persists_across_polls() { + let tmp = + std::env::temp_dir().join(format!("test-rl-last-handled-{}.json", std::process::id())); + let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); + state.rate_limit_last_retriggered_at = Some("2026-04-30T00:00:00Z".into()); + crate::state::write_state_to(&tmp, &state).unwrap(); + + let loaded = crate::state::read_state_from(&tmp).unwrap(); + assert_eq!( + loaded.rate_limit_last_retriggered_at.as_deref(), + Some("2026-04-30T00:00:00Z") + ); + + let _ = std::fs::remove_file(&tmp); + } - /// Finding #3: rate-limit retrigger 後の PARK signal が `reason: review_recheck` を使い、 - /// 過去 timestamp (`rl.until_unix_secs`) ではなく `state.next_wakeup_at_unix` を参照する。 + /// Bb-1: reset 時刻が未来の場合、`handle_rate_limit_retry` は Parked を返し + /// state.rate_limit_retries を変更しない (実 retry 計上は wakeup 経由で post 投稿後)。 #[test] - fn format_posted_retrigger_review_park_signal_uses_review_recheck_reason() { + fn rate_limit_retry_returns_parked_when_reset_in_future() { + let future_unix = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs() as i64 + + 600; + let rl = RateLimitState { + until_unix_secs: future_unix, + comment_event_time: "2026-04-30T00:00:00Z".into(), + wait_minutes: 10, + wait_seconds: 0, + }; let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); - state.next_wakeup_at_unix = Some(1_775_044_800); - state.review_recheck_count = 1; let pr_info = crate::util::PrInfo { pr_number: Some(42), repo: Some("o/r".into()), @@ -533,27 +400,189 @@ mod tests { fix_push_time: None, }; - let signal = format_posted_retrigger_review_park_signal(&state, &pr_info); + let outcome = handle_rate_limit_retry(&rl, &mut state, &pr_info, 3); + match outcome { + RateLimitOutcome::Parked { wakeup_at_unix } => { + assert_eq!(wakeup_at_unix, future_unix); + } + _ => panic!("expected Parked outcome for future reset, got other variant"), + } + assert_eq!(state.rate_limit_retries, 0); + assert!(state.rate_limit_last_retriggered_at.is_none()); + } - assert!( - signal.starts_with("[PR_MONITOR_PARK]"), - "PARK signal ヘッダが正しい形式でない: {}", - signal + /// Bb-1: PR 番号未確定の場合、`handle_rate_limit_retry` は Failed を返し + /// state を変更しない (caller は action_required で抜ける)。 + #[test] + fn rate_limit_retry_returns_failed_when_pr_number_missing() { + let past_unix = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs() as i64 + - 60; + let rl = RateLimitState { + until_unix_secs: past_unix, + comment_event_time: "2026-04-30T00:00:00Z".into(), + wait_minutes: 0, + wait_seconds: 0, + }; + let mut state = PrMonitorState::new(None, None, "t".into()); + let pr_info = crate::util::PrInfo { + pr_number: None, + repo: None, + push_time: None, + head_commit: None, + fix_push_time: None, + }; + + let outcome = handle_rate_limit_retry(&rl, &mut state, &pr_info, 3); + assert!(matches!(outcome, RateLimitOutcome::Failed(_))); + assert_eq!(state.rate_limit_retries, 0); + assert!(state.rate_limit_last_retriggered_at.is_none()); + } + + /// PR_MONITOR_STATE_FILE_OVERRIDE は process-global env var のため、 + /// override 設定 / 解除を test 並行実行で race させない serial guard。 + fn env_override_lock() -> std::sync::MutexGuard<'static, ()> { + use std::sync::{Mutex, OnceLock}; + static LOCK: OnceLock> = OnceLock::new(); + LOCK.get_or_init(|| Mutex::new(())).lock().unwrap() + } + + /// 書き込み先がディレクトリ不在のため write が必ず失敗する override path を返す。 + fn unwritable_state_path() -> std::path::PathBuf { + std::env::temp_dir() + .join(format!("pr-monitor-T2-2-{}", std::process::id())) + .join("nonexistent-dir") + .join("state.json") + } + + /// Bb-1 (T2-2): `finalize_parked` は write_state 失敗時に PARK signal emit を中止し + /// `action_required` を返却する fail-safe 経路を持つ (CodeRabbit Major #1 fix の固定化)。 + #[test] + fn finalize_parked_returns_action_required_when_write_state_fails() { + let _guard = env_override_lock(); + let bad_path = unwritable_state_path(); + std::env::set_var("PR_MONITOR_STATE_FILE_OVERRIDE", &bad_path); + + let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); + let rl = RateLimitState { + until_unix_secs: 1_775_088_000, + comment_event_time: "2026-05-01T00:00:00Z".into(), + wait_minutes: 47, + wait_seconds: 0, + }; + let pr_info = crate::util::PrInfo { + pr_number: Some(42), + repo: Some("o/r".into()), + push_time: None, + head_commit: None, + fix_push_time: None, + }; + let result = serde_json::json!({}); + + let outcome = finalize_parked(&mut state, &rl, &pr_info, 1_775_088_000, 3, &result); + + std::env::remove_var("PR_MONITOR_STATE_FILE_OVERRIDE"); + + assert_eq!( + outcome.action, "action_required", + "T2-2: write_state 失敗 → action_required で抜ける fail-safe が必要" ); assert!( - signal.contains("reason: review_recheck"), - "Finding #3: rate-limit retrigger 後も reason は review_recheck であるべき。実際: {}", - signal + outcome.summary.contains("PARK signal を中止") + || outcome.summary.contains("永続化失敗"), + "summary に永続化失敗の説明が含まれること: {}", + outcome.summary ); - assert!( - !signal.contains("reason: rate_limit_retry"), - "Finding #3: rate_limit_retry は誤った reason (rate-limit PARK と混同)。実際: {}", - signal + } + + fn setup_posted_retrigger_fixture() -> (PrMonitorState, RateLimitState, crate::util::PrInfo) { + let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); + state.action = "continue_monitoring".into(); + state.rate_limit_retries = 1; + let rl = RateLimitState { + until_unix_secs: 0, + comment_event_time: "2026-05-08T00:00:00Z".into(), + wait_minutes: 5, + wait_seconds: 0, + }; + let pr_info = crate::util::PrInfo { + pr_number: Some(1), + repo: Some("o/r".into()), + push_time: Some("2026-05-01T00:00:00Z".into()), + head_commit: Some("abc1234".into()), + fix_push_time: None, + }; + (state, rl, pr_info) + } + + #[test] + fn finalize_posted_retrigger_schedules_park_after_post() { + let _guard = env_override_lock(); + let tmp = tempfile::tempdir().unwrap(); + let state_path = tmp.path().join("state.json"); + std::env::set_var("PR_MONITOR_STATE_FILE_OVERRIDE", &state_path); + + let (mut state, rl, pr_info) = setup_posted_retrigger_fixture(); + let result = + finalize_posted_retrigger(&mut state, &rl, &pr_info, 300, &serde_json::Value::Null); + + std::env::remove_var("PR_MONITOR_STATE_FILE_OVERRIDE"); + + let park_result = + result.expect("順位 80 fix: Posted 後は必ず park を返し silent exit を防ぐ"); + assert_eq!(park_result.action, "parked_review_recheck"); + assert_eq!( + state.wakeup_reason.as_deref(), + Some("rate_limit_post_retrigger") ); - assert!( - signal.contains("next_wakeup_at_unix: 1775044800"), - "state.next_wakeup_at_unix を参照すべき (rl.until_unix_secs の過去 timestamp ではない)。実際: {}", - signal + let now_unix = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs() as i64; + let wakeup = state + .next_wakeup_at_unix + .expect("next_wakeup_at_unix が設定される"); + assert!(wakeup > now_unix && wakeup <= now_unix + 301); + assert_eq!( + state.rate_limit_last_retriggered_at.as_deref(), + Some("2026-05-08T00:00:00Z") + ); + } + + #[test] + fn finalize_posted_retrigger_action_required_when_write_state_fails() { + let _guard = env_override_lock(); + let bad_path = unwritable_state_path(); + std::env::set_var("PR_MONITOR_STATE_FILE_OVERRIDE", &bad_path); + + let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); + state.action = "continue_monitoring".into(); + let rl = RateLimitState { + until_unix_secs: 0, + comment_event_time: "2026-05-08T00:00:00Z".into(), + wait_minutes: 5, + wait_seconds: 0, + }; + let pr_info = crate::util::PrInfo { + pr_number: Some(1), + repo: Some("o/r".into()), + push_time: Some("2026-05-01T00:00:00Z".into()), + head_commit: None, + fix_push_time: None, + }; + + let result = + finalize_posted_retrigger(&mut state, &rl, &pr_info, 300, &serde_json::Value::Null); + + std::env::remove_var("PR_MONITOR_STATE_FILE_OVERRIDE"); + + assert!(result.is_some()); + assert_eq!( + result.unwrap().action, + "action_required", + "write_state 失敗時は action_required で抜ける (sibling parity with finalize_parked)" ); } } diff --git a/src/cli-pr-monitor/src/stages/poll/rate_limit_signal.rs b/src/cli-pr-monitor/src/stages/poll/rate_limit_signal.rs new file mode 100644 index 00000000..3b613db2 --- /dev/null +++ b/src/cli-pr-monitor/src/stages/poll/rate_limit_signal.rs @@ -0,0 +1,485 @@ +//! rate-limit park / shortcut signal の formatting helper +//! (PR-W2 refactor で `rate_limit.rs` から signal 整形部分を切り出し)。 +//! +//! - `emit_shortcut_signal_if_eligible` / `fetch_mergeable_status` / +//! `evaluate_rate_limit_shortcut` / `format_shortcut_signal` (順位 141 shortcut) +//! - `format_park_signal` (rate_limit_retry PARK signal) +//! - `collect_posted_retrigger_park_fields` / `format_posted_retrigger_review_park_signal` +//! (rate-limit 解消後の review 待ち PARK signal) +//! - `MergeableStatus` / `PostedRetriggerParkFields` (DTO) + +use crate::state::PrMonitorState; +use crate::util::PrInfo; + +use crate::runner::run_gh_quiet; + +use super::review_recheck_signal::round_up_to_next_minute; + +/// 順位 141: rate-limit 検出 + mergeable CLEAN + CR 全フィールドクリーンの条件が揃ったとき +/// `[RATE_LIMIT_BUT_MERGEABLE]` signal を stdout に出力する shortcut path。 +pub(super) fn emit_shortcut_signal_if_eligible( + state: &PrMonitorState, + rl: &crate::state::RateLimitState, + pr_info: &PrInfo, +) { + let Some(mergeable) = fetch_mergeable_status(pr_info) else { + return; + }; + if !evaluate_rate_limit_shortcut(state.coderabbit.as_ref(), &mergeable) { + return; + } + println!("{}", format_shortcut_signal(rl, pr_info, &mergeable)); +} + +/// 順位 141: PR の mergeable / mergeStateStatus を gh で取得。失敗時は None。 +fn fetch_mergeable_status(pr_info: &PrInfo) -> Option { + let pr = pr_info.pr_number?; + let pr_str = pr.to_string(); + let mut args: Vec<&str> = vec![ + "pr", + "view", + &pr_str, + "--json", + "mergeable,mergeStateStatus", + ]; + if let Some(repo) = pr_info.repo.as_deref() { + args.push("--repo"); + args.push(repo); + } + let json_str = run_gh_quiet(&args)?; + let parsed: serde_json::Value = serde_json::from_str(&json_str).ok()?; + Some(MergeableStatus { + mergeable: parsed.get("mergeable")?.as_str()?.to_string(), + merge_state: parsed.get("mergeStateStatus")?.as_str()?.to_string(), + }) +} + +/// 順位 141: mergeable + CR 全フィールドクリーンの条件評価を pure 関数化 (test 容易性)。 +pub(super) fn evaluate_rate_limit_shortcut( + coderabbit: Option<&crate::state::CodeRabbitState>, + mergeable: &MergeableStatus, +) -> bool { + let cr_clean = coderabbit + .map(|c| { + c.new_comments == 0 + && c.actionable_comments.unwrap_or(0) == 0 + && c.unresolved_threads.unwrap_or(0) == 0 + }) + .unwrap_or(true); + mergeable.mergeable == "MERGEABLE" && mergeable.merge_state == "CLEAN" && cr_clean +} + +/// 順位 141: `[RATE_LIMIT_BUT_MERGEABLE]` signal を構築 (pure)。 +pub(super) fn format_shortcut_signal( + rl: &crate::state::RateLimitState, + pr_info: &PrInfo, + mergeable: &MergeableStatus, +) -> String { + let pr = pr_info + .pr_number + .map(|n| n.to_string()) + .unwrap_or_else(|| "?".into()); + let repo = pr_info.repo.as_deref().unwrap_or("?"); + let reset_iso = if rl.until_unix_secs > 0 { + lib_pending_file::epoch_secs_to_iso8601(rl.until_unix_secs as u64) + } else { + "?".into() + }; + let wait_total_secs = rl.wait_minutes * 60 + rl.wait_seconds; + format!( + "[RATE_LIMIT_BUT_MERGEABLE] +pr: {pr} +repo: {repo} +rate_limit_reset_at_iso_utc: {reset_iso} +rate_limit_wait_seconds: {wait_total_secs} +mergeable: {merge} +merge_state: {state} + +ACTION REQUIRED: ユーザーに以下 2 択を AskUserQuestion で問うこと: + A: 今すぐ merge する (rate-limit reset を待たない、CR 2 回目 review なしで進める) + B: reset を待って通常 auto-retry flow に乗る +[/RATE_LIMIT_BUT_MERGEABLE]", + merge = mergeable.mergeable, + state = mergeable.merge_state, + ) +} + +/// 順位 141: gh `pr view --json mergeable,mergeStateStatus` の結果を保持する DTO。 +#[derive(Debug, Clone)] +pub(crate) struct MergeableStatus { + pub(crate) mergeable: String, + pub(crate) merge_state: String, +} + +struct PostedRetriggerParkFields { + pr: String, + repo: String, + wakeup_unix: i64, + wakeup_iso: String, + safe_unix: i64, + safe_iso: String, + wait_secs: i64, + recheck: u32, + exe: String, + cwd: String, +} + +fn collect_posted_retrigger_park_fields( + state: &PrMonitorState, + pr_info: &PrInfo, +) -> PostedRetriggerParkFields { + let pr = pr_info + .pr_number + .map(|n| n.to_string()) + .unwrap_or_else(|| "?".into()); + let repo = pr_info.repo.as_deref().unwrap_or("?").to_string(); + let wakeup_unix = state.next_wakeup_at_unix.unwrap_or(0); + let wakeup_iso = if wakeup_unix > 0 { + lib_pending_file::epoch_secs_to_iso8601(wakeup_unix as u64) + } else { + "?".into() + }; + let safe_unix = if wakeup_unix > 0 { + round_up_to_next_minute(wakeup_unix) + } else { + 0 + }; + let safe_iso = if safe_unix > 0 { + lib_pending_file::epoch_secs_to_iso8601(safe_unix as u64) + } else { + "?".into() + }; + let now_unix = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs() as i64) + .unwrap_or(0); + let wait_secs = (wakeup_unix - now_unix).max(0); + let exe = std::env::current_exe() + .map(|p| p.display().to_string()) + .unwrap_or_else(|_| "cli-pr-monitor.exe".into()); + let cwd = std::env::current_dir() + .map(|p| p.display().to_string()) + .unwrap_or_else(|_| ".".into()); + PostedRetriggerParkFields { + pr, + repo, + wakeup_unix, + wakeup_iso, + safe_unix, + safe_iso, + wait_secs, + recheck: state.review_recheck_count, + exe, + cwd, + } +} + +/// rate-limit 解消後の `@coderabbitai review` 投稿完了 → review 待ち PARK signal を生成する。 +/// +/// `format_park_signal` (reason: rate_limit_retry, reset_at_unix 基準) とは異なり、 +/// `state.next_wakeup_at_unix` の review 待ち wakeup を基準に `reason: review_recheck` で +/// discriminate する。過去 timestamp (`rl.until_unix_secs`) を CronCreate に誤使用しない。 +pub(super) fn format_posted_retrigger_review_park_signal( + state: &PrMonitorState, + pr_info: &PrInfo, +) -> String { + let PostedRetriggerParkFields { + pr, + repo, + wakeup_unix, + wakeup_iso, + safe_unix, + safe_iso, + wait_secs, + recheck, + exe, + cwd, + } = collect_posted_retrigger_park_fields(state, pr_info); + format!( + "[PR_MONITOR_PARK] +reason: review_recheck +pr: {pr} +repo: {repo} +next_wakeup_at_unix: {wakeup_unix} +next_wakeup_at_iso_utc: {wakeup_iso} +safe_minute_at_unix: {safe_unix} +safe_minute_at_iso_utc: {safe_iso} +wait_total_seconds: {wait_secs} +recheck_count: {recheck} +exe: {exe} +cwd: {cwd} + +ACTION REQUIRED: please schedule one-shot wakeup using CronCreate. + +Cron spec derivation (apply 2 orthogonal constraints sequentially): + Step 1 (round-UP, already applied at the source): use `safe_minute_at_iso_utc` + (= next_wakeup_at_iso_utc with seconds rounded UP to next full minute). + Step 2 (avoid :00 / :30 minute due to 90s pre-fire jitter): convert + `safe_minute_at_iso_utc` to LOCAL TZ, then bump the minute by +1 if it + lands on :00 or :30. Use the resulting `HH:MM` as the cron field. + Reference: ~/.claude/rules/common/development-workflow.md + § Cron スケジューリングの秒 → 分 round-UP + +CronCreate({{ + cron: \"\", + recurring: false, + durable: true, + prompt: \"Wakeup: review recheck for PR #{pr} ({repo}). cd \\\"{cwd}\\\" && \\\"{exe}\\\" --monitor-only\" +}}) +[/PR_MONITOR_PARK]" + ) +} + +/// PARK signal を stdout に書き出すための pure 関数 (Bb-1)。 +pub(crate) fn format_park_signal( + state: &PrMonitorState, + rl: &crate::state::RateLimitState, + pr_info: &PrInfo, + max_retries: u32, +) -> String { + let pr = pr_info + .pr_number + .map(|n| n.to_string()) + .unwrap_or_else(|| "?".into()); + let repo = pr_info.repo.as_deref().unwrap_or("?"); + let reset_iso = if rl.until_unix_secs > 0 { + lib_pending_file::epoch_secs_to_iso8601(rl.until_unix_secs as u64) + } else { + "?".into() + }; + let wait_total_secs = rl.wait_minutes * 60 + rl.wait_seconds; + let exe = std::env::current_exe() + .map(|p| p.display().to_string()) + .unwrap_or_else(|_| "cli-pr-monitor.exe".into()); + let cwd = std::env::current_dir() + .map(|p| p.display().to_string()) + .unwrap_or_else(|_| ".".into()); + let retry_attempt = state.rate_limit_retries + 1; + + format!( + "[PR_MONITOR_PARK] +reason: rate_limit_retry +pr: {pr} +repo: {repo} +reset_at_unix: {until} +reset_at_iso_utc: {reset_iso} +wait_total_seconds: {wait_total_secs} +retry_count: {retry_attempt} +max_retries: {max_retries} +exe: {exe} +cwd: {cwd} + +ACTION REQUIRED: please schedule one-shot wakeup using CronCreate. + +CronCreate({{ + cron: \"\", + recurring: false, + durable: true, + prompt: \"Wakeup: rate-limit retry for PR #{pr} ({repo}). cd \\\"{cwd}\\\" && \\\"{exe}\\\" --monitor-only\" +}}) +[/PR_MONITOR_PARK]", + until = rl.until_unix_secs, + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Finding #3: rate-limit retrigger 後の PARK signal が `reason: review_recheck` を使い、 + /// 過去 timestamp (`rl.until_unix_secs`) ではなく `state.next_wakeup_at_unix` を参照する。 + #[test] + fn format_posted_retrigger_review_park_signal_uses_review_recheck_reason() { + let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); + state.next_wakeup_at_unix = Some(1_775_044_800); + state.review_recheck_count = 1; + let pr_info = crate::util::PrInfo { + pr_number: Some(42), + repo: Some("o/r".into()), + push_time: None, + head_commit: None, + fix_push_time: None, + }; + + let signal = format_posted_retrigger_review_park_signal(&state, &pr_info); + + assert!( + signal.starts_with("[PR_MONITOR_PARK]"), + "PARK signal ヘッダが正しい形式でない: {}", + signal + ); + assert!( + signal.contains("reason: review_recheck"), + "Finding #3: rate-limit retrigger 後も reason は review_recheck であるべき。実際: {}", + signal + ); + assert!( + !signal.contains("reason: rate_limit_retry"), + "Finding #3: rate_limit_retry は誤った reason (rate-limit PARK と混同)。実際: {}", + signal + ); + assert!( + signal.contains("next_wakeup_at_unix: 1775044800"), + "state.next_wakeup_at_unix を参照すべき (rl.until_unix_secs の過去 timestamp ではない)。実際: {}", + signal + ); + } + + /// Bb-1: PARK signal は CronCreate 呼び出しに必要な構造化情報を含む。 + #[test] + fn format_park_signal_includes_required_fields() { + let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); + state.rate_limit_retries = 0; + let rl = crate::state::RateLimitState { + until_unix_secs: 1_775_088_000, + comment_event_time: "2026-05-01T00:00:00Z".into(), + wait_minutes: 47, + wait_seconds: 0, + }; + let pr_info = crate::util::PrInfo { + pr_number: Some(42), + repo: Some("o/r".into()), + push_time: None, + head_commit: None, + fix_push_time: None, + }; + + let signal = format_park_signal(&state, &rl, &pr_info, 3); + assert!(signal.starts_with("[PR_MONITOR_PARK]")); + assert!(signal.contains("[/PR_MONITOR_PARK]")); + assert!(signal.contains("pr: 42")); + assert!(signal.contains("repo: o/r")); + assert!(signal.contains("reset_at_unix: 1775088000")); + assert!(signal.contains("wait_total_seconds: 2820")); + assert!(signal.contains("retry_count: 1")); + assert!(signal.contains("max_retries: 3")); + assert!(signal.contains("CronCreate(")); + assert!(signal.contains("durable: true")); + assert!(signal.contains("recurring: false")); + assert!(signal.contains("--monitor-only")); + } + + /// Bb-1: PR 番号 / repo が None でも format_park_signal は panic せず "?" を出す。 + #[test] + fn format_park_signal_handles_missing_pr_info() { + let state = PrMonitorState::new(None, None, "t".into()); + let rl = crate::state::RateLimitState { + until_unix_secs: 1_775_088_000, + comment_event_time: "2026-05-01T00:00:00Z".into(), + wait_minutes: 5, + wait_seconds: 30, + }; + let pr_info = crate::util::PrInfo { + pr_number: None, + repo: None, + push_time: None, + head_commit: None, + fix_push_time: None, + }; + + let signal = format_park_signal(&state, &rl, &pr_info, 3); + assert!(signal.contains("pr: ?")); + assert!(signal.contains("repo: ?")); + assert!(signal.contains("wait_total_seconds: 330")); + } + + /// 順位 141: shortcut signal の trigger 条件 (mergeable CLEAN + unresolved 0) で true。 + #[test] + fn evaluate_rate_limit_shortcut_when_all_conditions_met() { + let m = MergeableStatus { + mergeable: "MERGEABLE".into(), + merge_state: "CLEAN".into(), + }; + let cr = crate::state::CodeRabbitState { + review_state: "approved".into(), + new_comments: 0, + actionable_comments: Some(0), + unresolved_threads: Some(0), + }; + assert!(evaluate_rate_limit_shortcut(Some(&cr), &m)); + } + + /// 順位 141: unresolved thread が残っていれば shortcut を抑止 (CR の指摘が未対応)。 + #[test] + fn evaluate_rate_limit_shortcut_blocks_when_unresolved_threads_exist() { + let m = MergeableStatus { + mergeable: "MERGEABLE".into(), + merge_state: "CLEAN".into(), + }; + let cr = crate::state::CodeRabbitState { + review_state: "commented".into(), + new_comments: 1, + actionable_comments: Some(1), + unresolved_threads: Some(1), + }; + assert!(!evaluate_rate_limit_shortcut(Some(&cr), &m)); + } + + /// 順位 141: new_comments > 0 のとき unresolved_threads が 0 でも shortcut を抑止。 + /// CR がまだコメントを処理中の状態で merge 判定を通過させない。 + #[test] + fn evaluate_rate_limit_shortcut_blocks_when_new_comments_exist() { + let m = MergeableStatus { + mergeable: "MERGEABLE".into(), + merge_state: "CLEAN".into(), + }; + let cr = crate::state::CodeRabbitState { + review_state: "commented".into(), + new_comments: 1, + actionable_comments: Some(0), + unresolved_threads: Some(0), + }; + assert!(!evaluate_rate_limit_shortcut(Some(&cr), &m)); + } + + /// 順位 141: mergeable が BLOCKED なら shortcut を抑止 (GitHub 側で merge 不可)。 + #[test] + fn evaluate_rate_limit_shortcut_blocks_when_not_mergeable() { + let m = MergeableStatus { + mergeable: "BLOCKED".into(), + merge_state: "BLOCKED".into(), + }; + assert!(!evaluate_rate_limit_shortcut(None, &m)); + } + + /// 順位 141: CR state が None (初回 review なし) でも mergeable CLEAN なら shortcut 可。 + #[test] + fn evaluate_rate_limit_shortcut_passes_when_coderabbit_none() { + let m = MergeableStatus { + mergeable: "MERGEABLE".into(), + merge_state: "CLEAN".into(), + }; + assert!(evaluate_rate_limit_shortcut(None, &m)); + } + + /// 順位 141: signal format に必須 field が全て含まれ、Claude が AskUserQuestion 化できる。 + #[test] + fn format_shortcut_signal_includes_required_fields() { + let rl = crate::state::RateLimitState { + until_unix_secs: 1_779_432_672, + comment_event_time: "2026-05-22T06:08:02Z".into(), + wait_minutes: 38, + wait_seconds: 30, + }; + let pr_info = crate::util::PrInfo { + pr_number: Some(169), + repo: Some("aloekun/claude-code-hook-test".into()), + push_time: None, + head_commit: None, + fix_push_time: None, + }; + let m = MergeableStatus { + mergeable: "MERGEABLE".into(), + merge_state: "CLEAN".into(), + }; + let sig = format_shortcut_signal(&rl, &pr_info, &m); + assert!(sig.starts_with("[RATE_LIMIT_BUT_MERGEABLE]")); + assert!(sig.contains("[/RATE_LIMIT_BUT_MERGEABLE]")); + assert!(sig.contains("pr: 169")); + assert!(sig.contains("repo: aloekun/claude-code-hook-test")); + assert!(sig.contains("rate_limit_wait_seconds: 2310")); + assert!(sig.contains("mergeable: MERGEABLE")); + assert!(sig.contains("merge_state: CLEAN")); + assert!(sig.contains("AskUserQuestion")); + } +} diff --git a/src/cli-pr-monitor/src/stages/poll/review_recheck.rs b/src/cli-pr-monitor/src/stages/poll/review_recheck.rs index 728629d7..a79c6710 100644 --- a/src/cli-pr-monitor/src/stages/poll/review_recheck.rs +++ b/src/cli-pr-monitor/src/stages/poll/review_recheck.rs @@ -1,168 +1,18 @@ //! Review recheck park 関連 (PR B refactor で `mod.rs` から切り出し)。 //! -//! - 順位 209 / 210 の安全 cron spec 生成 helper (`round_up_to_next_minute`, -//! `compute_safe_minute_for_park_signal`) -//! - 順位 209 で導入された PARK signal format (`format_review_park_signal`) //! - Bb-2 アーキで定義された review_recheck park 経路 (`finalize_initial_review_park`, //! `finalize_review_recheck_park`, `finalize_review_recheck_max_reached`, //! `schedule_next_review_recheck_park`) +//! +//! signal 整形部分 (`round_up_to_next_minute` / `compute_safe_minute_for_park_signal` / +//! `format_review_park_signal`) は `review_recheck_signal.rs` に分離。 use crate::log::log_info; use crate::state::{read_state, write_state, PrMonitorState}; -use super::{ - make_action_required_result, make_park_poll_result, PollContext, PollResult, -}; - -/// 順位 209: PARK signal の cron spec round-UP rule (= Constraint 1)。 -/// -/// `unix_secs` の秒部分が `0` でなければ次の完全な分に round-UP した unix seconds を返す。 -/// `~/.claude/rules/common/development-workflow.md` § Cron スケジューリングの秒 → 分 round-UP の -/// Constraint 1 (= scheduling minimum lead time) のみを実装。 -/// -/// Constraint 2 (= execution jitter ≤90s pre-fire / minute `:00`・`:30` 回避) は local TZ -/// awareness が必要で fractional-hour offset (例: IST +5:30) で正しく適用するには -/// AI agent consumer 側での処理が安全。本関数は UTC pure arithmetic に閉じる設計とし、 -/// PARK signal の ACTION REQUIRED block で Step 2 として AI agent に明示する。 -/// -/// 由来: PR #210 セッション (2026-06-16) で実観測した cron timing race。秒解像度 timestamp を -/// 分単位 cron に round-DOWN 変換した結果、`should_resume_wakeup` が `wakeup_at > now` で false -/// 判定 → fresh path に倒れて recheck_count が前進せず、2 回の無駄 wakeup が発生した root cause。 -pub(crate) fn round_up_to_next_minute(unix_secs: i64) -> i64 { - let sec_in_minute = unix_secs.rem_euclid(60); - if sec_in_minute == 0 { - unix_secs - } else { - unix_secs - sec_in_minute + 60 - } -} - -/// 順位 209: PARK signal 用に Constraint 1 (秒 → 分 round-UP) を適用した -/// safe minute の unix seconds と UTC ISO 8601 文字列を返す。 -/// -/// `wakeup_unix == 0` (未設定) のとき `(0, "?")` を返す sentinel 値を維持し、 -/// `format_review_park_signal` 出力の "?" plain string 互換を保つ。 -pub(super) fn compute_safe_minute_for_park_signal(wakeup_unix: i64) -> (i64, String) { - if wakeup_unix <= 0 { - return (0, "?".into()); - } - let safe_unix = round_up_to_next_minute(wakeup_unix); - let safe_iso = lib_pending_file::epoch_secs_to_iso8601(safe_unix as u64); - (safe_unix, safe_iso) -} - -struct ReviewParkSignalFields { - safe_minute_unix: i64, - safe_minute_iso_utc: String, - pr: String, - repo: String, - wakeup_unix: i64, - wakeup_iso: String, - wait_secs: i64, - exe: String, - cwd: String, - recheck: u32, - max_rechecks: u32, -} - -fn collect_review_park_fields( - state: &PrMonitorState, - ctx: &PollContext<'_>, -) -> ReviewParkSignalFields { - let pr = ctx - .pr_info - .pr_number - .map(|n| n.to_string()) - .unwrap_or_else(|| "?".into()); - let repo = ctx.pr_info.repo.clone().unwrap_or_else(|| "?".into()); - let wakeup_unix = state.next_wakeup_at_unix.unwrap_or(0); - let wakeup_iso = if wakeup_unix > 0 { - lib_pending_file::epoch_secs_to_iso8601(wakeup_unix as u64) - } else { - "?".into() - }; - let (safe_minute_unix, safe_minute_iso_utc) = - compute_safe_minute_for_park_signal(wakeup_unix); - let now_unix = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs() as i64) - .unwrap_or(0); - let wait_secs = (wakeup_unix - now_unix).max(0); - let exe = std::env::current_exe() - .map(|p| p.display().to_string()) - .unwrap_or_else(|_| "cli-pr-monitor.exe".into()); - let cwd = std::env::current_dir() - .map(|p| p.display().to_string()) - .unwrap_or_else(|_| ".".into()); - - ReviewParkSignalFields { - safe_minute_unix, - safe_minute_iso_utc, - pr, - repo, - wakeup_unix, - wakeup_iso, - wait_secs, - exe, - cwd, - recheck: state.review_recheck_count, - max_rechecks: ctx.max_review_rechecks, - } -} - -/// Bb-2: 初回 push 後の review_recheck park signal を生成する。 -/// -/// `format_park_signal` (rate_limit_retry) と同じ envelope `[PR_MONITOR_PARK]` を使い、 -/// `reason: review_recheck` で discriminate する。Claude Code 側のパーサは両 signal を -/// 同じ format で読める。 -pub(super) fn format_review_park_signal(state: &PrMonitorState, ctx: &PollContext<'_>) -> String { - let f = collect_review_park_fields(state, ctx); - format!( - "[PR_MONITOR_PARK] -reason: review_recheck -pr: {pr} -repo: {repo} -next_wakeup_at_unix: {wakeup_unix} -next_wakeup_at_iso_utc: {wakeup_iso} -safe_minute_at_unix: {safe_unix} -safe_minute_at_iso_utc: {safe_iso} -wait_total_seconds: {wait_secs} -recheck_count: {recheck} -max_rechecks: {max} -exe: {exe} -cwd: {cwd} - -ACTION REQUIRED: please schedule one-shot wakeup using CronCreate. - -Cron spec derivation (apply 2 orthogonal constraints sequentially): - Step 1 (round-UP, already applied at the source): use `safe_minute_at_iso_utc` - (= next_wakeup_at_iso_utc with seconds rounded UP to next full minute). - Step 2 (avoid :00 / :30 minute due to 90s pre-fire jitter): convert - `safe_minute_at_iso_utc` to LOCAL TZ, then bump the minute by +1 if it - lands on :00 or :30. Use the resulting `HH:MM` as the cron field. - Reference: ~/.claude/rules/common/development-workflow.md - § Cron スケジューリングの秒 → 分 round-UP - -CronCreate({{ - cron: \"\", - recurring: false, - durable: true, - prompt: \"Wakeup: review recheck for PR #{pr} ({repo}). cd \\\"{cwd}\\\" && \\\"{exe}\\\" --monitor-only\" -}}) -[/PR_MONITOR_PARK]", - pr = f.pr, - repo = f.repo, - wakeup_unix = f.wakeup_unix, - wakeup_iso = f.wakeup_iso, - safe_unix = f.safe_minute_unix, - safe_iso = f.safe_minute_iso_utc, - wait_secs = f.wait_secs, - recheck = f.recheck, - max = f.max_rechecks, - exe = f.exe, - cwd = f.cwd, - ) -} +use super::rate_limit::make_action_required_result; +use super::review_recheck_signal::format_review_park_signal; +use super::{make_park_poll_result, PollContext, PollResult}; /// Bb-2: fresh push 経路で review_recheck park を行う (checker 呼び出しなし)。 /// @@ -187,10 +37,9 @@ pub(super) fn finalize_initial_review_park(ctx: &PollContext<'_>) -> PollResult state.started_at = ctx.push_time.to_string(); state.review_recheck_count = 0; state.head_commit = ctx.pr_info.head_commit.clone(); - state.fix_push_time = ctx + state.fix_push_time = state .fix_push_time - .map(String::from) - .or(state.fix_push_time); + .or_else(|| ctx.fix_push_time.map(String::from)); let now_unix = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_secs() as i64) @@ -304,8 +153,8 @@ pub(super) fn schedule_next_review_recheck_park( #[cfg(test)] mod tests { - use super::finalize_review_recheck_max_reached; - use crate::state::PrMonitorState; + use super::*; + use crate::config::{ClassifierConfig, RateLimitConfig}; use std::sync::{Mutex, OnceLock}; fn env_lock() -> std::sync::MutexGuard<'static, ()> { @@ -315,6 +164,62 @@ mod tests { .unwrap_or_else(|e| e.into_inner()) } + /// PR_MONITOR_STATE_FILE_OVERRIDE は process-global env var のため、 + /// override 設定 / 解除を test 並行実行で race させない serial guard。 + fn env_override_lock() -> std::sync::MutexGuard<'static, ()> { + static LOCK: OnceLock> = OnceLock::new(); + LOCK.get_or_init(|| Mutex::new(())).lock().unwrap() + } + + /// 書き込み先がディレクトリ不在のため write が必ず失敗する override path を返す。 + fn unwritable_state_path() -> std::path::PathBuf { + std::env::temp_dir() + .join(format!("pr-monitor-T2-2-{}", std::process::id())) + .join("nonexistent-dir") + .join("state.json") + } + + fn seed_stale_recheck_state(tmp_path: &std::path::Path) { + let mut stale_state = + PrMonitorState::new(Some(42), Some("o/r".into()), "2026-05-01T00:00:00Z".into()); + stale_state.review_recheck_count = 3; + stale_state.action = "action_required".into(); + crate::state::write_state_to(tmp_path, &stale_state).unwrap(); + } + + fn pr_info_for_initial_review_park_test() -> crate::util::PrInfo { + crate::util::PrInfo { + pr_number: Some(42), + repo: Some("o/r".into()), + push_time: Some("2026-05-01T00:00:00Z".into()), + head_commit: Some("abc1234".into()), + fix_push_time: None, + } + } + + fn make_default_test_ctx<'a>( + checker: &'a std::path::Path, + pr_info: &'a crate::util::PrInfo, + rate_limit_config: &'a RateLimitConfig, + classifier_config: &'a ClassifierConfig, + ) -> PollContext<'a> { + PollContext { + checker, + push_time: "2026-05-01T00:00:00Z", + fix_push_time: None, + pr_info, + rate_limit_config, + classifier_config, + start: std::time::Instant::now(), + max_duration: 600, + skip_ci: false, + skip_coderabbit: false, + initial_review_wait_secs: 300, + review_recheck_wait_secs: 300, + max_review_rechecks: 3, + } + } + /// Finding #5: `finalize_review_recheck_max_reached` は `action_required` 確定後に /// 残留 wakeup fields を None にクリアする。ADR-030 invariant: /// "wakeup は parked_* action のときのみスケジュールされる"。 @@ -352,4 +257,174 @@ mod tests { "Finding #5: action が action_required に確定されること" ); } + + /// Bb-2 (T2-2): `schedule_next_review_recheck_park` は write_state 失敗時に + /// PARK signal emit を中止し `action_required` を返却する (sibling parity)。 + #[test] + fn schedule_next_review_recheck_park_returns_action_required_when_write_state_fails() { + let _guard = env_override_lock(); + let bad_path = unwritable_state_path(); + std::env::set_var("PR_MONITOR_STATE_FILE_OVERRIDE", &bad_path); + + let mut state = + PrMonitorState::new(Some(42), Some("o/r".into()), "2026-05-01T00:00:00Z".into()); + state.review_recheck_count = 1; + let checker_path = std::path::PathBuf::from("dummy-checker"); + let pr_info = crate::util::PrInfo { + pr_number: Some(42), + repo: Some("o/r".into()), + push_time: Some("2026-05-01T00:00:00Z".into()), + head_commit: None, + fix_push_time: None, + }; + let rate_limit_config = RateLimitConfig::default(); + let classifier_config = ClassifierConfig::default(); + let ctx = PollContext { + checker: &checker_path, + push_time: "2026-05-01T00:00:00Z", + fix_push_time: None, + pr_info: &pr_info, + rate_limit_config: &rate_limit_config, + classifier_config: &classifier_config, + start: std::time::Instant::now(), + max_duration: 600, + skip_ci: false, + skip_coderabbit: false, + initial_review_wait_secs: 300, + review_recheck_wait_secs: 300, + max_review_rechecks: 3, + }; + + let outcome = schedule_next_review_recheck_park(&mut state, &ctx); + + std::env::remove_var("PR_MONITOR_STATE_FILE_OVERRIDE"); + + assert_eq!( + outcome.action, "action_required", + "T2-2 sibling parity: review park も write_state 失敗 → action_required で抜けること" + ); + } + + /// CR Major #2 fix (Bb-2 PR #114 review): fresh push 経路では `finalize_initial_review_park` + /// が `review_recheck_count` を 0 に明示リセットすること。前サイクルが MAX 到達 (count=3) + /// で残った state を持ち越さないことを machine-enforce する。 + #[test] + fn finalize_initial_review_park_resets_recheck_count() { + let _guard = env_override_lock(); + let tmp_path = std::env::temp_dir().join(format!( + "pr-monitor-CR-M2-{}-state.json", + std::process::id() + )); + std::env::set_var("PR_MONITOR_STATE_FILE_OVERRIDE", &tmp_path); + seed_stale_recheck_state(&tmp_path); + + let pr_info = pr_info_for_initial_review_park_test(); + let checker = std::path::PathBuf::from("dummy"); + let rate_limit_config = RateLimitConfig::default(); + let classifier_config = ClassifierConfig::default(); + let ctx = make_default_test_ctx(&checker, &pr_info, &rate_limit_config, &classifier_config); + + let outcome = finalize_initial_review_park(&ctx); + let persisted = crate::state::read_state_from(&tmp_path).unwrap(); + + std::env::remove_var("PR_MONITOR_STATE_FILE_OVERRIDE"); + let _ = std::fs::remove_file(&tmp_path); + + assert_eq!(outcome.action, "parked_review_recheck"); + assert_eq!( + persisted.review_recheck_count, 0, + "CR Major #2: fresh push 経路で count=3 が残らず 0 にリセットされること" + ); + assert_eq!( + persisted.head_commit.as_deref(), + Some("abc1234"), + "CR Major #1: fresh push 経路で head_commit が pr_info から保存されること" + ); + } + + /// 順位 141: `fix_push_time` の write-once 不変条件 — + /// `finalize_initial_review_park` が state に既存の `fix_push_time` がある場合に + /// `ctx.fix_push_time` の値で上書きしないことを検証する。 + /// + /// `ctx.fix_push_time = Some("new_time")` (= None ではなく非 None) を使うことで、 + /// or_else 被演算子の入れ替えバグを discriminate できる。 + #[test] + fn finalize_initial_review_park_preserves_existing_fix_push_time() { + let _guard = env_override_lock(); + let tmp = tempfile::tempdir().unwrap(); + let state_path = tmp.path().join("state.json"); + std::env::set_var("PR_MONITOR_STATE_FILE_OVERRIDE", &state_path); + + let mut seeded = + PrMonitorState::new(Some(42), Some("o/r".into()), "2026-05-01T00:00:00Z".into()); + seeded.fix_push_time = Some("2026-05-22T06:06:00Z".into()); + crate::state::write_state_to(&state_path, &seeded).unwrap(); + + let pr_info = crate::util::PrInfo { + pr_number: Some(42), + repo: Some("o/r".into()), + push_time: Some("2026-05-01T00:00:00Z".into()), + head_commit: Some("abc1234".into()), + fix_push_time: None, + }; + let checker = std::path::PathBuf::from("dummy"); + let rate_limit_config = RateLimitConfig::default(); + let classifier_config = ClassifierConfig::default(); + let mut ctx = + make_default_test_ctx(&checker, &pr_info, &rate_limit_config, &classifier_config); + let ctx_fix_push_time_must_lose = "2026-05-22T06:10:00Z"; + ctx.fix_push_time = Some(ctx_fix_push_time_must_lose); + + finalize_initial_review_park(&ctx); + let persisted = crate::state::read_state_from(&state_path).unwrap(); + std::env::remove_var("PR_MONITOR_STATE_FILE_OVERRIDE"); + + assert_eq!( + persisted.fix_push_time.as_deref(), + Some("2026-05-22T06:06:00Z"), + "write-once: state に既存 fix_push_time がある場合、ctx の値で上書きしない" + ); + } + + /// 順位 141: `fix_push_time` の write-once 不変条件 — + /// `finalize_review_recheck_park` が state に既存の `fix_push_time` がある場合に + /// `ctx.fix_push_time` の値で上書きしないことを検証する。 + #[test] + fn finalize_review_recheck_park_preserves_existing_fix_push_time() { + let _guard = env_override_lock(); + let tmp = tempfile::tempdir().unwrap(); + let state_path = tmp.path().join("state.json"); + std::env::set_var("PR_MONITOR_STATE_FILE_OVERRIDE", &state_path); + + let mut seeded = + PrMonitorState::new(Some(42), Some("o/r".into()), "2026-05-01T00:00:00Z".into()); + seeded.fix_push_time = Some("2026-05-22T06:06:00Z".into()); + seeded.review_recheck_count = 0; + crate::state::write_state_to(&state_path, &seeded).unwrap(); + + let pr_info = crate::util::PrInfo { + pr_number: Some(42), + repo: Some("o/r".into()), + push_time: Some("2026-05-01T00:00:00Z".into()), + head_commit: Some("abc1234".into()), + fix_push_time: None, + }; + let checker = std::path::PathBuf::from("dummy"); + let rate_limit_config = RateLimitConfig::default(); + let classifier_config = ClassifierConfig::default(); + let mut ctx = + make_default_test_ctx(&checker, &pr_info, &rate_limit_config, &classifier_config); + let ctx_fix_push_time_must_lose = "2026-05-22T06:10:00Z"; + ctx.fix_push_time = Some(ctx_fix_push_time_must_lose); + + finalize_review_recheck_park(&ctx); + let persisted = crate::state::read_state_from(&state_path).unwrap(); + std::env::remove_var("PR_MONITOR_STATE_FILE_OVERRIDE"); + + assert_eq!( + persisted.fix_push_time.as_deref(), + Some("2026-05-22T06:06:00Z"), + "write-once: state に既存 fix_push_time がある場合、ctx の値で上書きしない" + ); + } } diff --git a/src/cli-pr-monitor/src/stages/poll/review_recheck_signal.rs b/src/cli-pr-monitor/src/stages/poll/review_recheck_signal.rs new file mode 100644 index 00000000..ecaaed77 --- /dev/null +++ b/src/cli-pr-monitor/src/stages/poll/review_recheck_signal.rs @@ -0,0 +1,313 @@ +//! Review recheck PARK signal の formatting helper +//! (PR-W2 refactor で `review_recheck.rs` から signal 整形部分を切り出し)。 +//! +//! - 順位 209 / 210 の安全 cron spec 生成 helper (`round_up_to_next_minute`, +//! `compute_safe_minute_for_park_signal`) +//! - 順位 209 で導入された PARK signal format (`format_review_park_signal`) + +use crate::state::PrMonitorState; + +use super::PollContext; + +/// 順位 209: PARK signal の cron spec round-UP rule (= Constraint 1)。 +/// +/// `unix_secs` の秒部分が `0` でなければ次の完全な分に round-UP した unix seconds を返す。 +/// `~/.claude/rules/common/development-workflow.md` § Cron スケジューリングの秒 → 分 round-UP の +/// Constraint 1 (= scheduling minimum lead time) のみを実装。 +/// +/// Constraint 2 (= execution jitter ≤90s pre-fire / minute `:00`・`:30` 回避) は local TZ +/// awareness が必要で fractional-hour offset (例: IST +5:30) で正しく適用するには +/// AI agent consumer 側での処理が安全。本関数は UTC pure arithmetic に閉じる設計とし、 +/// PARK signal の ACTION REQUIRED block で Step 2 として AI agent に明示する。 +/// +/// 由来: PR #210 セッション (2026-06-16) で実観測した cron timing race。秒解像度 timestamp を +/// 分単位 cron に round-DOWN 変換した結果、`should_resume_wakeup` が `wakeup_at > now` で false +/// 判定 → fresh path に倒れて recheck_count が前進せず、2 回の無駄 wakeup が発生した root cause。 +pub(crate) fn round_up_to_next_minute(unix_secs: i64) -> i64 { + let sec_in_minute = unix_secs.rem_euclid(60); + if sec_in_minute == 0 { + unix_secs + } else { + unix_secs - sec_in_minute + 60 + } +} + +/// 順位 209: PARK signal 用に Constraint 1 (秒 → 分 round-UP) を適用した +/// safe minute の unix seconds と UTC ISO 8601 文字列を返す。 +/// +/// `wakeup_unix == 0` (未設定) のとき `(0, "?")` を返す sentinel 値を維持し、 +/// `format_review_park_signal` 出力の "?" plain string 互換を保つ。 +fn compute_safe_minute_for_park_signal(wakeup_unix: i64) -> (i64, String) { + if wakeup_unix <= 0 { + return (0, "?".into()); + } + let safe_unix = round_up_to_next_minute(wakeup_unix); + let safe_iso = lib_pending_file::epoch_secs_to_iso8601(safe_unix as u64); + (safe_unix, safe_iso) +} + +struct ReviewParkSignalFields { + safe_minute_unix: i64, + safe_minute_iso_utc: String, + pr: String, + repo: String, + wakeup_unix: i64, + wakeup_iso: String, + wait_secs: i64, + exe: String, + cwd: String, + recheck: u32, + max_rechecks: u32, +} + +fn collect_review_park_fields( + state: &PrMonitorState, + ctx: &PollContext<'_>, +) -> ReviewParkSignalFields { + let pr = ctx + .pr_info + .pr_number + .map(|n| n.to_string()) + .unwrap_or_else(|| "?".into()); + let repo = ctx.pr_info.repo.clone().unwrap_or_else(|| "?".into()); + let wakeup_unix = state.next_wakeup_at_unix.unwrap_or(0); + let wakeup_iso = if wakeup_unix > 0 { + lib_pending_file::epoch_secs_to_iso8601(wakeup_unix as u64) + } else { + "?".into() + }; + let (safe_minute_unix, safe_minute_iso_utc) = compute_safe_minute_for_park_signal(wakeup_unix); + let now_unix = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs() as i64) + .unwrap_or(0); + let wait_secs = (wakeup_unix - now_unix).max(0); + let exe = std::env::current_exe() + .map(|p| p.display().to_string()) + .unwrap_or_else(|_| "cli-pr-monitor.exe".into()); + let cwd = std::env::current_dir() + .map(|p| p.display().to_string()) + .unwrap_or_else(|_| ".".into()); + + ReviewParkSignalFields { + safe_minute_unix, + safe_minute_iso_utc, + pr, + repo, + wakeup_unix, + wakeup_iso, + wait_secs, + exe, + cwd, + recheck: state.review_recheck_count, + max_rechecks: ctx.max_review_rechecks, + } +} + +/// Bb-2: 初回 push 後の review_recheck park signal を生成する。 +/// +/// `format_park_signal` (rate_limit_retry) と同じ envelope `[PR_MONITOR_PARK]` を使い、 +/// `reason: review_recheck` で discriminate する。Claude Code 側のパーサは両 signal を +/// 同じ format で読める。 +pub(super) fn format_review_park_signal(state: &PrMonitorState, ctx: &PollContext<'_>) -> String { + let f = collect_review_park_fields(state, ctx); + format!( + "[PR_MONITOR_PARK] +reason: review_recheck +pr: {pr} +repo: {repo} +next_wakeup_at_unix: {wakeup_unix} +next_wakeup_at_iso_utc: {wakeup_iso} +safe_minute_at_unix: {safe_unix} +safe_minute_at_iso_utc: {safe_iso} +wait_total_seconds: {wait_secs} +recheck_count: {recheck} +max_rechecks: {max} +exe: {exe} +cwd: {cwd} + +ACTION REQUIRED: please schedule one-shot wakeup using CronCreate. + +Cron spec derivation (apply 2 orthogonal constraints sequentially): + Step 1 (round-UP, already applied at the source): use `safe_minute_at_iso_utc` + (= next_wakeup_at_iso_utc with seconds rounded UP to next full minute). + Step 2 (avoid :00 / :30 minute due to 90s pre-fire jitter): convert + `safe_minute_at_iso_utc` to LOCAL TZ, then bump the minute by +1 if it + lands on :00 or :30. Use the resulting `HH:MM` as the cron field. + Reference: ~/.claude/rules/common/development-workflow.md + § Cron スケジューリングの秒 → 分 round-UP + +CronCreate({{ + cron: \"\", + recurring: false, + durable: true, + prompt: \"Wakeup: review recheck for PR #{pr} ({repo}). cd \\\"{cwd}\\\" && \\\"{exe}\\\" --monitor-only\" +}}) +[/PR_MONITOR_PARK]", + pr = f.pr, + repo = f.repo, + wakeup_unix = f.wakeup_unix, + wakeup_iso = f.wakeup_iso, + safe_unix = f.safe_minute_unix, + safe_iso = f.safe_minute_iso_utc, + wait_secs = f.wait_secs, + recheck = f.recheck, + max = f.max_rechecks, + exe = f.exe, + cwd = f.cwd, + ) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::{ClassifierConfig, RateLimitConfig}; + + #[test] + fn round_up_to_next_minute_keeps_value_when_seconds_already_zero() { + let aligned = 1_775_044_800; + assert_eq!(round_up_to_next_minute(aligned), aligned); + } + + #[test] + fn round_up_to_next_minute_rounds_up_when_seconds_present() { + let unaligned = 1_775_044_819; + assert_eq!(round_up_to_next_minute(unaligned), 1_775_044_860); + } + + #[test] + fn round_up_to_next_minute_rounds_up_one_second_before_next_minute() { + let one_sec_before = 1_775_044_859; + assert_eq!(round_up_to_next_minute(one_sec_before), 1_775_044_860); + } + + #[test] + fn round_up_to_next_minute_one_second_past_minute_rounds_up_to_next_full_minute() { + let one_sec_past = 1_775_044_801; + assert_eq!(round_up_to_next_minute(one_sec_past), 1_775_044_860); + } + + #[test] + fn round_up_to_next_minute_handles_zero_input_as_minute_zero() { + assert_eq!(round_up_to_next_minute(0), 0); + } + + #[test] + fn compute_safe_minute_returns_sentinel_when_input_zero() { + let (safe_unix, safe_iso) = compute_safe_minute_for_park_signal(0); + assert_eq!(safe_unix, 0); + assert_eq!(safe_iso, "?"); + } + + #[test] + fn compute_safe_minute_returns_sentinel_when_input_negative() { + let (safe_unix, safe_iso) = compute_safe_minute_for_park_signal(-1); + assert_eq!(safe_unix, 0); + assert_eq!(safe_iso, "?"); + } + + #[test] + fn compute_safe_minute_rounds_up_and_formats_iso_when_input_unaligned() { + let (safe_unix, safe_iso) = compute_safe_minute_for_park_signal(1_775_044_819); + assert_eq!(safe_unix, 1_775_044_860); + assert_eq!(safe_iso, "2026-04-01T12:01:00Z"); + } + + #[test] + fn compute_safe_minute_preserves_iso_when_input_already_aligned() { + let (safe_unix, safe_iso) = compute_safe_minute_for_park_signal(1_775_044_800); + assert_eq!(safe_unix, 1_775_044_800); + assert_eq!(safe_iso, "2026-04-01T12:00:00Z"); + } + + /// Bb-3 (順位 55): `max_review_rechecks` の config 化が実際に PARK signal に + /// 反映されることを machine-enforce する (default 3 ではなく custom 値が出力されること)。 + #[test] + fn format_review_park_signal_uses_configured_max_rechecks() { + let state = + PrMonitorState::new(Some(42), Some("o/r".into()), "2026-05-01T00:00:00Z".into()); + let pr_info = crate::util::PrInfo { + pr_number: Some(42), + repo: Some("o/r".into()), + push_time: Some("2026-05-01T00:00:00Z".into()), + head_commit: None, + fix_push_time: None, + }; + let checker = std::path::PathBuf::from("dummy"); + let rate_limit_config = RateLimitConfig::default(); + let classifier_config = ClassifierConfig::default(); + let ctx = PollContext { + checker: &checker, + push_time: "2026-05-01T00:00:00Z", + fix_push_time: None, + pr_info: &pr_info, + rate_limit_config: &rate_limit_config, + classifier_config: &classifier_config, + start: std::time::Instant::now(), + max_duration: 600, + skip_ci: false, + skip_coderabbit: false, + initial_review_wait_secs: 120, + review_recheck_wait_secs: 240, + max_review_rechecks: 7, + }; + + let signal = format_review_park_signal(&state, &ctx); + + assert!( + signal.contains("max_rechecks: 7"), + "PARK signal に config 値 (max_rechecks: 7) が反映されること: {}", + signal + ); + assert!( + !signal.contains("max_rechecks: 3"), + "default 値 3 が hard-coded で残っていないこと: {}", + signal + ); + } + + #[test] + fn format_review_park_signal_includes_safe_minute_iso_utc_field() { + let mut state = + PrMonitorState::new(Some(99), Some("o/r".into()), "2026-04-01T00:00:00Z".into()); + state.next_wakeup_at_unix = Some(1_775_044_819); + let pr_info = crate::util::PrInfo { + pr_number: Some(99), + repo: Some("o/r".into()), + push_time: Some("2026-04-01T00:00:00Z".into()), + head_commit: None, + fix_push_time: None, + }; + let checker = std::path::PathBuf::from("dummy"); + let rate_limit_config = RateLimitConfig::default(); + let classifier_config = ClassifierConfig::default(); + let ctx = PollContext { + checker: &checker, + push_time: "2026-04-01T00:00:00Z", + fix_push_time: None, + pr_info: &pr_info, + rate_limit_config: &rate_limit_config, + classifier_config: &classifier_config, + start: std::time::Instant::now(), + max_duration: 600, + skip_ci: false, + skip_coderabbit: false, + initial_review_wait_secs: 300, + review_recheck_wait_secs: 300, + max_review_rechecks: 3, + }; + + let signal = format_review_park_signal(&state, &ctx); + + assert!( + signal.contains("safe_minute_at_unix: 1775044860"), + "PARK signal に safe_minute_at_unix の round-UP 値が含まれること: {}", + signal + ); + assert!( + signal.contains("safe_minute_at_iso_utc: 2026-04-01T12:01:00Z"), + "PARK signal に safe_minute_at_iso_utc の round-UP ISO が含まれること: {}", + signal + ); + } +}