From 325a5a2a82fa5c786d28a5b8d43bf24fc171cd80 Mon Sep 17 00:00:00 2001 From: aloekun Date: Mon, 20 Jul 2026 19:22:54 +0900 Subject: [PATCH 1/4] =?UTF-8?q?fix(check-ci-coderabbit):=20rate-limit=20?= =?UTF-8?q?=E6=A4=9C=E7=9F=A5=E3=81=AE=20silent=20regression=20=E3=82=92?= =?UTF-8?q?=E6=A7=8B=E9=80=A0=E7=9A=84=E3=81=AB=E5=A1=9E=E3=81=90?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #307 で実発火した誤報告の修正。CodeRabbit がレート制限でレビューを開始 できなかったにもかかわらず、監視が「CodeRabbit 指摘なし」= stop_monitoring_success と報告した (人間が PR コメントを直接読むまで誰も気付けない状態だった)。 原因は 2 段構えの検知のうち後段だけが失敗し、それが fail-open だったこと: - is_rate_limit_comment ("rate limited by coderabbit.ai") は一致していた - extract_wait_time が既知 2 書式のいずれにも一致せず None - parse_rate_limit は `extract_wait_time(body)?` で早期 return するため、 **レート制限と判っているのに「制限なし」を返していた** CR は書式を 3 度目の変更をしていた: 旧: Please wait **N minutes and M seconds** 新: More reviews will be available in N minutes and M seconds 現行: **Next review available in:** **15 minutes** ← 未対応だった 修正は 2 層: 1. **構造 (本質)**: marker が一致したら待ち時間が読めなくても必ず rate-limit として 報告する。既定値 15 分で park し、`wait_time_parsed: false` を下流へ伝えて summary / ログに「書式が未知」と明示する。これにより 4 回目の書式変更が起きても silent success ではなく「park + 書式追加が必要という可視の警告」に着地する。 短すぎる既定値は self-correcting (wakeup 後に再 poll し、まだ制限中なら再 park)。 2. **書式追加**: 現行書式の regex を追加。markdown 強調 (`**`) を `\**` と `\s*` で吸収。 既存 test `rate_limit_no_match_when_no_wait_time` は**旧来の誤挙動そのもの**を 固定していたため、新契約に合わせて書き換えた (改名 + 理由を doc に明記)。 ADR-034 が「CR は format を時間経過で変更するため multi-variant 配列で対応する (PR #182/#184 で silent regression を実体観測)」と警告していた事象の再発であり、 regex を足すだけでは同じことが繰り返される。構造側を直したのはそのため。 実測: Windows で cargo test --workspace 全 pass / clippy clean。Linux (WSL) でも check-ci-coderabbit rate_limit 28/28・cli-pr-monitor state 23/23 pass。 incident 由来 fixture は PR #307 に実投稿された comment 本文を使用 (ADR-049)。 Co-Authored-By: Claude Opus 4.8 (1M context) --- src/check-ci-coderabbit/src/models.rs | 7 + src/check-ci-coderabbit/src/rate_limit.rs | 149 ++++++++++++++++-- src/cli-pr-monitor/src/stages/poll/mod.rs | 1 + .../src/stages/poll/rate_limit.rs | 26 ++- .../src/stages/poll/rate_limit_signal.rs | 3 + src/cli-pr-monitor/src/state.rs | 41 +++++ 6 files changed, 215 insertions(+), 12 deletions(-) diff --git a/src/check-ci-coderabbit/src/models.rs b/src/check-ci-coderabbit/src/models.rs index 8dbde37e..0d99518e 100644 --- a/src/check-ci-coderabbit/src/models.rs +++ b/src/check-ci-coderabbit/src/models.rs @@ -28,6 +28,13 @@ pub(crate) struct RateLimitInfo { pub(crate) comment_event_time: String, pub(crate) wait_minutes: u64, pub(crate) wait_seconds: u64, + /// 待ち時間を comment 本文から**実際に読み取れたか**。 + /// + /// `false` = rate-limit comment とは判定できたが、CR の文面が既知のどの書式にも + /// 一致せず既定値で代替したことを意味する (CR の書式変更を検知した状態)。 + /// 下流はこれを見て「wakeup 時刻は当てにならない」と扱い、書式追加が必要な事実を + /// 可視化する。ADR-034 § CR rate-limit format evolution 参照。 + pub(crate) wait_time_parsed: bool, } #[derive(Serialize, Default)] diff --git a/src/check-ci-coderabbit/src/rate_limit.rs b/src/check-ci-coderabbit/src/rate_limit.rs index fea0def5..1ae67c03 100644 --- a/src/check-ci-coderabbit/src/rate_limit.rs +++ b/src/check-ci-coderabbit/src/rate_limit.rs @@ -3,10 +3,24 @@ use crate::markers::{is_rate_limit_comment, rate_limit_event_time}; use crate::models::{GhComment, RateLimitInfo}; -/// CodeRabbit rate-limit comment を検出し、reset 時刻 (unix epoch) を返す。 -pub(crate) fn parse_rate_limit(json: &str, push_time: &str) -> Option { - let comments: Vec = serde_json::from_str(json).ok()?; - +/// 待ち時間を読み取れなかった rate-limit comment に充てる既定の待ち分数。 +/// +/// CR が書式を変えても「レート制限は起きている」という事実は marker で判っている。 +/// ここで `None` を返して**制限なし扱いにするのが最悪の失敗**なので (実観測: 監視が +/// 「CodeRabbit 指摘なし」と誤報告して success 判定した)、既定値で park させる。 +/// +/// 短すぎても実害は小さい: wakeup 後に再 poll し、まだ制限中なら再度 park する +/// (self-correcting)。過去の実観測値は 5 / 15 / 21 / 30 / 36 分。 +const FALLBACK_WAIT_MINUTES: u64 = 15; + +/// push 以降に CR が投稿した rate-limit comment のうち**最も新しいもの**を返す。 +/// +/// 新しい順で選ぶのは、CR が同一 comment を編集して待ち時間を更新するため +/// (`rate_limit_event_time` が `updated_at` を優先する理由と対)。 +fn latest_rate_limit_comment<'a>( + comments: &'a [GhComment], + push_time: &str, +) -> Option<&'a GhComment> { let mut candidates: Vec<&GhComment> = comments .iter() .filter(|c| { @@ -28,13 +42,36 @@ pub(crate) fn parse_rate_limit(json: &str, push_time: &str) -> Option Option { + let comments: Vec = serde_json::from_str(json).ok()?; + let latest = latest_rate_limit_comment(&comments, push_time)?; let body = latest.body.as_deref()?; let event_time = rate_limit_event_time(latest)?; - let (minutes, seconds) = extract_wait_time(body)?; let comment_unix = parse_iso8601_to_unix(event_time)?; + let (minutes, seconds, wait_time_parsed) = match extract_wait_time(body) { + Some((m, s)) => (m, s, true), + None => { + eprintln!( + "[check-ci] Warning: rate-limit comment を検出しましたが待ち時間の書式が未知です。\ + 既定値 {}分で park します。CR が書式を変更した可能性があるため \ + extract_wait_time に新書式の追加が必要です (ADR-034)。", + FALLBACK_WAIT_MINUTES + ); + (FALLBACK_WAIT_MINUTES, 0, false) + } + }; + let until_unix_secs = comment_unix + (minutes as i64) * 60 + (seconds as i64) + 60; Some(RateLimitInfo { @@ -42,6 +79,7 @@ pub(crate) fn parse_rate_limit(json: &str, push_time: &str) -> Option Option<(u64, u64)> { Some((m, 0)) } -/// 旧 / 新どちらかの format に一致すれば `(minutes, seconds)` を返す。旧 → 新の順で試行。 +/// 現行 format (`**Next review available in:** **N minutes**`) を抽出。 +/// PR #307 (2026-07-20) で実観測した CR の 3 番目の書式。 +/// +/// markdown の強調 (`**`) が `in:` の直後と数値の直前の両方に入るため、 +/// `\**` と `\s*` で任意個の `*` と空白を吸収する。 +pub(crate) fn extract_current_format_wait_time(body: &str) -> Option<(u64, u64)> { + let re_full = regex::Regex::new( + r"Next review available in:\**\s*\**(\d+) minutes? and \**(\d+) seconds?", + ) + .ok()?; + if let Some(caps) = re_full.captures(body) { + let m: u64 = caps.get(1)?.as_str().parse().ok()?; + let s: u64 = caps.get(2)?.as_str().parse().ok()?; + return Some((m, s)); + } + let re_min = regex::Regex::new(r"Next review available in:\**\s*\**(\d+) minutes?").ok()?; + let caps = re_min.captures(body)?; + let m: u64 = caps.get(1)?.as_str().parse().ok()?; + Some((m, 0)) +} + +/// 既知の 3 書式のいずれかに一致すれば `(minutes, seconds)` を返す。旧 → 新 → 現行の順で試行。 +/// +/// **書式を足すときは必ず実観測した本文で test を書くこと**。ここに落ちると +/// `parse_rate_limit` が既定値へフォールバックし、wakeup 時刻が実際の reset と +/// ズレる (制限なし扱いにはならないが、無駄な poll が増える)。 pub(crate) fn extract_wait_time(body: &str) -> Option<(u64, u64)> { - extract_old_format_wait_time(body).or_else(|| extract_new_format_wait_time(body)) + extract_old_format_wait_time(body) + .or_else(|| extract_new_format_wait_time(body)) + .or_else(|| extract_current_format_wait_time(body)) } /// ISO 8601 (`YYYY-MM-DDTHH:MM:SSZ` 形式) を unix epoch 秒に変換する。 @@ -239,14 +304,21 @@ mod tests { assert!(parse_rate_limit(json, "2026-04-29T00:00:00Z").is_none()); } + /// **契約変更 (PR #307 incident、2026-07-20)**: 旧実装はこのケースで `None` + /// (= rate-limit なし) を返しており、本 test はその挙動を固定していた。 + /// しかしそれこそが「CR が書式を変えると監視が誤って success を返す」原因だった + /// ため、marker 一致時は既定値付きで必ず `Some` を返す契約へ改めた。 #[test] - fn rate_limit_no_match_when_no_wait_time() { + fn rate_limit_reported_with_fallback_when_wait_time_format_is_unknown() { let json = r#"[{ "user": {"login": "coderabbitai[bot]"}, "body": "Rate limit exceeded but format is unusual", "created_at": "2026-04-30T00:00:00Z" }]"#; - assert!(parse_rate_limit(json, "2026-04-29T00:00:00Z").is_none()); + let result = parse_rate_limit(json, "2026-04-29T00:00:00Z") + .expect("marker 一致なら待ち時間が読めなくても rate-limit として報告する"); + assert!(!result.wait_time_parsed); + assert_eq!(result.wait_minutes, FALLBACK_WAIT_MINUTES); } #[test] @@ -351,6 +423,63 @@ mod tests { assert_eq!(result.wait_seconds, 0); } + /// PR #307 (2026-07-20) の incident 再現 (bad): CR が 3 番目の書式に変えたため + /// `extract_wait_time` が None になり、`parse_rate_limit` が **rate-limit なし**を + /// 返していた。結果、監視が「CodeRabbit 指摘なし」= success と誤報告した。 + /// + /// body は実際に PR #307 へ投稿された comment から採取したもの (ADR-049 の流儀: + /// incident 由来の fixture は実データを使う)。 + #[test] + fn rate_limit_detected_from_current_format_next_review_available_in() { + let json = r#"[{ + "user": {"login": "coderabbitai[bot]"}, + "body": "\n\n\n> [!WARNING]\n> ## Review limit reached\n> \n> `@aloekun`, you've reached your PR review limit, so we couldn't start this review.\n> \n> **Next review available in:** **15 minutes**\n", + "created_at": "2026-07-20T10:06:43Z" + }]"#; + let result = parse_rate_limit(json, "2026-07-20T10:06:34Z") + .expect("現行書式の rate-limit comment を検出できること (PR #307 incident)"); + assert_eq!(result.wait_minutes, 15); + assert_eq!(result.wait_seconds, 0); + assert!( + result.wait_time_parsed, + "書式を追加したので実測値として読めていること", + ); + } + + /// 構造的な回帰防止 (good): marker は一致するが待ち時間の書式が**未知**でも + /// `None` (= rate-limit なし) を返さないこと。 + /// + /// これが本 incident の本質。書式追加だけでは 4 回目の変更で同じ silent regression が + /// 再発するため、「marker 一致 → 必ず rate-limit として扱う」を契約として固定する。 + #[test] + fn unknown_wait_time_format_still_reports_rate_limit_with_fallback() { + let json = r#"[{ + "user": {"login": "coderabbitai[bot]"}, + "body": "\n\nReviews resume at some point in the future.", + "created_at": "2026-07-20T10:06:43Z" + }]"#; + let result = parse_rate_limit(json, "2026-07-20T10:00:00Z").expect( + "待ち時間が読めなくても rate-limit として報告すること (None は制限なし扱い = 誤 success の原因)", + ); + assert!( + !result.wait_time_parsed, + "既定値で代替したことを下流へ伝えること", + ); + assert_eq!(result.wait_minutes, FALLBACK_WAIT_MINUTES); + let base = parse_iso8601_to_unix("2026-07-20T10:06:43Z").unwrap(); + assert_eq!( + result.until_unix_secs, + base + (FALLBACK_WAIT_MINUTES as i64) * 60 + 60, + ); + } + + /// 現行書式の分・秒併記 variant (CR が秒を付け足した場合に備える)。 + #[test] + fn current_format_with_minutes_and_seconds() { + let body = "**Next review available in:** **3 minutes and 20 seconds**"; + assert_eq!(extract_wait_time(body), Some((3, 20))); + } + #[test] fn rate_limit_picks_latest_when_mixed_old_and_new_formats() { let json = r#"[ diff --git a/src/cli-pr-monitor/src/stages/poll/mod.rs b/src/cli-pr-monitor/src/stages/poll/mod.rs index d69dfc07..22009fc0 100644 --- a/src/cli-pr-monitor/src/stages/poll/mod.rs +++ b/src/cli-pr-monitor/src/stages/poll/mod.rs @@ -153,6 +153,7 @@ mod tests { comment_event_time: "x".into(), wait_minutes: 5, wait_seconds: 0, + wait_time_parsed: true, }; let result = serde_json::json!({}); finalize_parked( 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 2653ade4..b81b448d 100644 --- a/src/cli-pr-monitor/src/stages/poll/rate_limit.rs +++ b/src/cli-pr-monitor/src/stages/poll/rate_limit.rs @@ -173,8 +173,14 @@ pub(super) fn finalize_parked( state.wakeup_reason = Some("rate_limit_retry".into()); state.head_commit = pr_info.head_commit.clone(); state.summary = format!( - "CodeRabbit rate-limit: wakeup を {}m{}s 後に予約 (PARK signal 参照)", - rl.wait_minutes, rl.wait_seconds + "CodeRabbit rate-limit: wakeup を {}m{}s 後に予約 (PARK signal 参照){}", + rl.wait_minutes, + rl.wait_seconds, + if rl.wait_time_parsed { + "" + } else { + " ※待ち時間の書式が未知のため既定値。CR の書式変更を疑うこと" + } ); if let Err(e) = write_state_to(state_path, state) { let msg = format!("park state 永続化失敗のため PARK signal を中止 ({})。手動で `@coderabbitai review` を投稿してください", e); @@ -244,6 +250,14 @@ pub(super) fn handle_rate_limit_retry( return RateLimitOutcome::Failed("PR 番号未確定のため retrigger スキップ".into()); }; + if !rl.wait_time_parsed { + log_info( + "[rate_limit] Warning: CR の待ち時間書式が未知のため既定値を使用しています。\ + wakeup 時刻は目安です。check-ci-coderabbit の extract_wait_time に \ + 新書式の追加が必要です (ADR-034)", + ); + } + if sleep_secs > 0 { log_info(&format!( "[rate_limit] reset まで {}秒 (wait={}m{}s + 60s buffer)、Park で wakeup 要求 (retry 候補={}/{})", @@ -298,6 +312,7 @@ mod tests { comment_event_time: "2026-04-30T00:00:00Z".into(), wait_minutes: 5, wait_seconds: 13, + wait_time_parsed: true, }); crate::state::write_state_to(&tmp, &state).unwrap(); @@ -339,6 +354,7 @@ mod tests { comment_event_time: comment_a.into(), wait_minutes: 5, wait_seconds: 0, + wait_time_parsed: true, }; let already_handled_iter1 = state.rate_limit_last_retriggered_at.as_deref() == Some(rl_a.comment_event_time.as_str()); @@ -362,6 +378,7 @@ mod tests { comment_event_time: comment_b.into(), wait_minutes: 5, wait_seconds: 0, + wait_time_parsed: true, }; let already_handled_iter3 = state.rate_limit_last_retriggered_at.as_deref() == Some(rl_b.comment_event_time.as_str()); @@ -403,6 +420,7 @@ mod tests { comment_event_time: "2026-04-30T00:00:00Z".into(), wait_minutes: 10, wait_seconds: 0, + wait_time_parsed: true, }; let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); let pr_info = crate::util::PrInfo { @@ -438,6 +456,7 @@ mod tests { comment_event_time: "2026-04-30T00:00:00Z".into(), wait_minutes: 0, wait_seconds: 0, + wait_time_parsed: true, }; let mut state = PrMonitorState::new(None, None, "t".into()); let pr_info = crate::util::PrInfo { @@ -474,6 +493,7 @@ mod tests { comment_event_time: "2026-05-01T00:00:00Z".into(), wait_minutes: 47, wait_seconds: 0, + wait_time_parsed: true, }; let pr_info = crate::util::PrInfo { pr_number: Some(42), @@ -515,6 +535,7 @@ mod tests { comment_event_time: "2026-05-08T00:00:00Z".into(), wait_minutes: 5, wait_seconds: 0, + wait_time_parsed: true, }; let pr_info = crate::util::PrInfo { pr_number: Some(1), @@ -573,6 +594,7 @@ mod tests { comment_event_time: "2026-05-08T00:00:00Z".into(), wait_minutes: 5, wait_seconds: 0, + wait_time_parsed: true, }; let pr_info = crate::util::PrInfo { pr_number: Some(1), 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 index 8852f697..63811f8f 100644 --- a/src/cli-pr-monitor/src/stages/poll/rate_limit_signal.rs +++ b/src/cli-pr-monitor/src/stages/poll/rate_limit_signal.rs @@ -335,6 +335,7 @@ mod tests { comment_event_time: "2026-05-01T00:00:00Z".into(), wait_minutes: 47, wait_seconds: 0, + wait_time_parsed: true, }; let pr_info = crate::util::PrInfo { pr_number: Some(42), @@ -368,6 +369,7 @@ mod tests { comment_event_time: "2026-05-01T00:00:00Z".into(), wait_minutes: 5, wait_seconds: 30, + wait_time_parsed: true, }; let pr_info = crate::util::PrInfo { pr_number: None, @@ -460,6 +462,7 @@ mod tests { comment_event_time: "2026-05-22T06:08:02Z".into(), wait_minutes: 38, wait_seconds: 30, + wait_time_parsed: true, }; let pr_info = crate::util::PrInfo { pr_number: Some(169), diff --git a/src/cli-pr-monitor/src/state.rs b/src/cli-pr-monitor/src/state.rs index 78bcc3ba..76b9f7bc 100644 --- a/src/cli-pr-monitor/src/state.rs +++ b/src/cli-pr-monitor/src/state.rs @@ -103,6 +103,18 @@ pub(crate) struct RateLimitState { pub(crate) comment_event_time: String, pub(crate) wait_minutes: u64, pub(crate) wait_seconds: u64, + /// 待ち時間を CR の comment 本文から実際に読み取れたか (PR #307)。 + /// + /// `false` = CR が未知の書式に変えたため既定値で代替した = **wakeup 時刻は目安**。 + /// 旧 state ファイルにはこのキーが無いが、旧実装は待ち時間を読めたときしか + /// rate_limit を書き出さなかったため、既定値は `true` が事実に合う。 + #[serde(default = "wait_time_parsed_default")] + pub(crate) wait_time_parsed: bool, +} + +/// 旧 state ファイル (キー不在) の `wait_time_parsed` 既定値。 +fn wait_time_parsed_default() -> bool { + true } #[derive(Serialize, Deserialize, Clone, Debug, PartialEq)] @@ -528,6 +540,34 @@ mod tests { assert_eq!(rl.until_unix_secs, 1_735_689_600); assert_eq!(rl.wait_minutes, 5); assert_eq!(rl.wait_seconds, 13); + assert!( + rl.wait_time_parsed, + "wait_time_parsed 不在の旧 wire format は true 扱い (旧実装は読めたときだけ書き出した)", + ); + } + + /// PR #307: CR の書式変更で待ち時間を読めなかった旨が check-ci-coderabbit から + /// 監視側へ伝播すること。ここで落ちると「既定値で park した」事実が可視化されず、 + /// 書式追加が必要なことに誰も気付けない。 + #[test] + fn update_state_propagates_unparsed_wait_time_flag() { + let mut state = PrMonitorState::new(Some(1), None, "t".into()); + let result = serde_json::json!({ + "action": "continue_monitoring", + "rate_limit": { + "until_unix_secs": 1735689600_i64, + "comment_created_at": "2026-07-20T10:06:43Z", + "wait_minutes": 15, + "wait_seconds": 0, + "wait_time_parsed": false + } + }); + update_state_from_check_result(&mut state, &result); + let rl = state.rate_limit.expect("rate_limit must be populated"); + assert!( + !rl.wait_time_parsed, + "書式未知のフラグが監視側へ伝播すること", + ); } #[test] @@ -539,6 +579,7 @@ mod tests { comment_event_time: "2026-04-30T00:00:00Z".into(), wait_minutes: 5, wait_seconds: 13, + wait_time_parsed: true, }); // rate_limit field を含まない正常 polling 結果 let result = serde_json::json!({ "action": "continue_monitoring" }); From 0194a6d336723643f18f7ecd7356d441c586b623 Mon Sep 17 00:00:00 2001 From: aloekun Date: Mon, 20 Jul 2026 21:01:59 +0900 Subject: [PATCH 2/4] =?UTF-8?q?fix(pr-monitor):=20=E5=88=A4=E5=AE=9A?= =?UTF-8?q?=E6=96=87=E3=81=8C=20unresolved=5Fthreads=20=E3=82=92=E7=84=A1?= =?UTF-8?q?=E8=A6=96=E3=81=99=E3=82=8B=20fail-open=20=E3=82=92=E4=BF=AE?= =?UTF-8?q?=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #307 の recheck で実観測: 同じレポート内で「未解決スレッド2件」 「action: action_required」と表示した直後に「**判定**: 問題は見つかりませんでした」 と結論していた。人間はレポート末尾の判定行を読んで判断するため、未解決の CodeRabbit 指摘を見落とす経路になっていた。 原因は compute_verdict が result.findings の件数だけで分岐していたこと。 再チェックでは新規コメントが無いため findings は空になるが、未解決スレッドは 残り続ける。action は正しく action_required になっていたので、**判定文だけが 不完全な信号から安心させる結論を出していた**。 findings が空かつ未解決スレッドがある場合の分岐を追加した。findings がある場合は 従来どおり severity ベースの判定を優先し、未解決スレッドの文言で severity 情報を 潰さない。unresolved_threads が None (不明) の場合は 0 と同じ扱いで従来挙動を維持。 これは同日に修正した rate-limit 検知の silent regression と同じ family (不完全な信号から success を導出する fail-open) で、post-merge feedback の analyzer も独立に Tier 1 High として検出していた。 test は ADR-049 の流儀で incident 再現 (bad) + 退行防止 (good) を対で追加: - findings 空 + 未解決 2 件 → 「問題なし」と結論しないこと (incident 再現) - 未解決 0 件 → 従来どおり「問題なし」 - unresolved_threads = None → 0 と同じ扱い - findings あり + 未解決あり → severity ベース判定を優先 実測: Windows で cli-pr-monitor 262 pass (257→262、本 test 4 件 + helper 追加)。 Co-Authored-By: Claude Opus 4.8 (1M context) --- src/cli-pr-monitor/src/stages/monitor.rs | 76 ++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/src/cli-pr-monitor/src/stages/monitor.rs b/src/cli-pr-monitor/src/stages/monitor.rs index 7988bc66..238d4084 100644 --- a/src/cli-pr-monitor/src/stages/monitor.rs +++ b/src/cli-pr-monitor/src/stages/monitor.rs @@ -313,6 +313,13 @@ fn print_report(result: &crate::stages::poll::PollResult, pr_label: &str) { } } +/// レポート末尾に出す人間向けの判定文を決める。 +/// +/// **`findings` が空でも「問題なし」と断定してはならない**。CodeRabbit の未解決 +/// スレッドは `findings` には現れない (新規コメントが無い再チェックでは空になる) ため、 +/// これを見ないと「未解決 2 件」と表示した直後に「問題は見つかりませんでした」と +/// 結論する矛盾が起きる (PR #307 で実観測)。`action` は正しく action_required に +/// なっていたので、判定文だけが不完全な信号から安心させる結論を出していた。 fn compute_verdict(result: &crate::stages::poll::PollResult) -> &'static str { match result.action.as_str() { "parked_rate_limit" => { @@ -339,10 +346,18 @@ fn compute_verdict(result: &crate::stages::poll::PollResult) -> &'static str { }) .count(); + let unresolved_threads = result + .coderabbit + .as_ref() + .and_then(|cr| cr.unresolved_threads) + .unwrap_or(0); + if critical_major > 0 { "修正が必要な指摘があります" } else if !result.findings.is_empty() { "重大な問題は見つかりませんでした。軽微な改善提案があります" + } else if unresolved_threads > 0 { + "CodeRabbit の未解決スレッドが残っています。内容を確認してください" } else { "問題は見つかりませんでした" } @@ -482,6 +497,20 @@ mod tests { } } + /// `unresolved_threads` を指定できる variant (PR #307 incident の再現用)。 + fn poll_result_with_threads( + action: &str, + review_state: Option<&str>, + findings: Vec, + unresolved_threads: Option, + ) -> PollResult { + let mut r = poll_result(action, review_state, findings); + if let Some(cr) = r.coderabbit.as_mut() { + cr.unresolved_threads = unresolved_threads; + } + r + } + fn finding(severity: &str) -> Finding { Finding { severity: severity.into(), @@ -501,6 +530,53 @@ mod tests { const VERDICT_MINOR: &str = "重大な問題は見つかりませんでした。軽微な改善提案があります"; const VERDICT_CRITICAL: &str = "修正が必要な指摘があります"; + const VERDICT_UNRESOLVED: &str = + "CodeRabbit の未解決スレッドが残っています。内容を確認してください"; + + /// PR #307 incident 再現 (bad): findings が空でも未解決スレッドが残っていれば + /// 「問題は見つかりませんでした」と結論しないこと。 + /// + /// 由来: 2026-07-20 の PR #307 recheck。同じレポート内で + /// 「未解決スレッド2件」「action: action_required」と表示した直後に + /// 「**判定**: 問題は見つかりませんでした」と出していた。再チェックでは新規 + /// コメントが無く findings が空になるため、findings だけを見る判定は破綻する。 + #[test] + fn verdict_flags_unresolved_threads_even_when_findings_are_empty() { + let r = poll_result_with_threads("action_required", Some("success"), vec![], Some(2)); + assert_eq!( + compute_verdict(&r), + VERDICT_UNRESOLVED, + "未解決スレッドがあるのに「問題なし」と結論してはならない", + ); + } + + /// good: 未解決スレッドが 0 なら従来どおり「問題なし」と結論すること (退行なし)。 + #[test] + fn verdict_reports_no_problems_when_threads_are_resolved() { + let r = poll_result_with_threads("continue_monitoring", Some("success"), vec![], Some(0)); + assert_eq!(compute_verdict(&r), VERDICT_NO_PROBLEMS); + } + + /// good: `unresolved_threads` 不明 (None) は 0 と同じ扱い (従来挙動を維持)。 + #[test] + fn verdict_treats_unknown_thread_count_as_none_outstanding() { + let r = poll_result_with_threads("continue_monitoring", Some("success"), vec![], None); + assert_eq!(compute_verdict(&r), VERDICT_NO_PROBLEMS); + } + + /// good: findings があるときは従来の findings ベース判定を優先すること。 + /// 未解決スレッドの文言で severity 情報を潰さない。 + #[test] + fn verdict_prefers_findings_severity_over_unresolved_threads() { + let r = poll_result_with_threads( + "action_required", + Some("success"), + vec![finding("critical")], + Some(3), + ); + assert_eq!(compute_verdict(&r), VERDICT_CRITICAL); + } + #[test] fn verdict_park_rate_limit_takes_precedence_over_review_state() { let r = poll_result("parked_rate_limit", Some("not_found"), vec![]); From 6679b2c449f0cfcf03f8062bc335a37085874518 Mon Sep 17 00:00:00 2001 From: aloekun Date: Mon, 20 Jul 2026 21:03:31 +0900 Subject: [PATCH 3/4] =?UTF-8?q?docs(plan):=20WP-15=20=E3=81=AE=20`?= =?UTF-8?q?=E5=AE=8C=E4=BA=86`=20=E6=9D=A1=E4=BB=B6=20(1)=20=E9=81=94?= =?UTF-8?q?=E6=88=90=E3=82=92=E8=A8=98=E9=8C=B2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #307 / #308 マージ後に release-binaries.yml が master で実走し `nightly` prerelease を生成した (tarball 9.72 MB + sha256、commit 541adde1)。 公開された実物の artifact で end-to-end 検証した結果を記録する: - 認証なしの素の curl で取得 (= 「public release は gh 認証不要」の設計判断を実証) - checksum 一致 / 17 ファイル展開 (バイナリ 16 + BUILD_INFO) - file が ELF 64-bit LSB pie executable, x86-64 ... stripped を確認 - **release バイナリそのもので Linux 上の hooks が実発火** (PreToolUse が危険 コマンドを exit 2 でブロックし通常コマンドは exit 0、SessionStart が additionalContext JSON、tree-sitter の comment-lint が違反検出) あわせて初回 run が赤だった経緯 (PR #308 で修正) も記録した。WSL では修正前でも 60/60 pass して再現できず ubuntu-22.04 の CI が初めて捕捉したため、 **「WSL 検証はスケジューリング依存の race に対しては CI の代替にならない」**という 限定的な教訓として書いた。WP-16 (CI matrix) の必要性を裏づける実例になる。 過度な一般化 (「WSL は CI の代替にならない」) は避けた。post-merge feedback の analyzer が「既存メモと矛盾しうる」と指摘したとおり、WSL 検証自体は cargo test / clippy / hooks 発火の確認には有効であり、限界は race に限られる。 残る `完了` 条件 (2) は実クラウドセッションでの cloud-setup.sh 実走のみ。 Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/harness-improvement-plan.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/harness-improvement-plan.md b/docs/harness-improvement-plan.md index e23a8ea3..d1965036 100644 --- a/docs/harness-improvement-plan.md +++ b/docs/harness-improvement-plan.md @@ -74,7 +74,7 @@ Anthropic 公式のハーネスエンジニアリング指針（決定論的基 | WP-12 | 2 | 発火テレメトリ + ハーネス ROI 棚卸し | M | なし | 実装済（step1 収集層のみ: ADR-055 + lib-telemetry + 6 hook 計装。step2-3〔集計 pre-step / 卒業判定機械化〕は 28 日 warm-up 後着手のため todo 順位 307/308 へ移管） | | WP-13 | 3 | EXE_SUFFIX 抽象化 | M | なし | 実装済（build/実行 scripts を deploy-artifacts.mjs / run-artifact.mjs 経由に、settings を `/` 区切り + `{{EXE_SUFFIX}}` 化、Rust の機能的 exe 解決を EXE_SUFFIX 化。ADR-005 amendment。cargo test 全 pass・build:all/deploy:hooks/lint:docs 退行なし実測。config TOML の cmd.exe 依存は WP-15 へ。`完了` は初回 push/PR で launcher 経路の実走確認後） | | WP-14 | 3 | PowerShell 3 本の Rust 化 | S-M ×2 | なし | 実装済（3 本すべて Rust 化: fix-metrics-check→comment-lint `--fix-metrics-check` / prepare-pr-body→cli-pr-monitor サブコマンド / analyze-takt-timings→新規 cli-takt-timings crate。cargo test カバレッジ下・実データで旧 ps1 と出力一致確認。`完了` は初回 push/PR で fix step metrics-check と prepare-pr-body 経路の実走確認後） | -| WP-15 | 3 | Linux バイナリビルド + クラウド setup script | M | WP-13, 14 | 実装済（release-binaries.yml〔master push → rolling `nightly` prerelease に単一 tarball〕+ scripts/cloud-setup.sh 新設。前提として Linux 実行時に壊れる可搬性欠陥を修正: `cmd /c` 決め打ちの唯一の shell spawn 点を `shell_command`〔Windows=cmd /c / 他=sh -c〕へ集約、taskkill のみだった timeout kill に unix 分岐、cmd.exe 構文テストの OS 中立化、config の `.exe`/backslash 依存を `{{CLAUDE_DIR}}`/`{{EXE_SUFFIX}}` 展開へ。**WSL Ubuntu 24.04 で実測**: cargo test --workspace 全 pass・ignored 含め全 pass・clippy clean・hooks 実発火・push pipeline が sh -c 経路で完走。Linux 実測により lock の同時取得レース〔8 中 6 取得〕も発見・修正。`完了` は release 実生成 + 実クラウドセッションでの cloud-setup.sh 実走確認後） | +| WP-15 | 3 | Linux バイナリビルド + クラウド setup script | M | WP-13, 14 | 実装済（release-binaries.yml〔master push → rolling `nightly` prerelease に単一 tarball〕+ scripts/cloud-setup.sh 新設。前提として Linux 実行時に壊れる可搬性欠陥を修正: `cmd /c` 決め打ちの唯一の shell spawn 点を `shell_command`〔Windows=cmd /c / 他=sh -c〕へ集約、taskkill のみだった timeout kill に unix 分岐、cmd.exe 構文テストの OS 中立化、config の `.exe`/backslash 依存を `{{CLAUDE_DIR}}`/`{{EXE_SUFFIX}}` 展開へ。**WSL Ubuntu 24.04 で実測**: cargo test --workspace 全 pass・ignored 含め全 pass・clippy clean・hooks 実発火・push pipeline が sh -c 経路で完走。Linux 実測により lock の同時取得レース〔8 中 6 取得〕も発見・修正。**`完了` 条件 (1) 達成 (2026-07-20)**: PR #307 / #308 マージ後に release-binaries.yml が実走し `nightly` prerelease を生成。公開された実物の artifact を認証なし curl で取得 → checksum 一致 → 展開 → **release バイナリそのもので Linux 上の hooks 実発火**まで確認。残るは実クラウドセッションでの cloud-setup.sh 実走のみ） | | WP-16 | 3 | CI matrix（移植退行防止） | S | WP-13, 14 | 未着手 | | WP-17 | 4 | イベント駆動バックボーン完成（Phase B + routines 移行） | M | WP-09, 10, 11 | 未着手 | | WP-18 | 4 | 夜間 todo 消化ループ | M-L | WP-15, 17 | 未着手 | @@ -267,7 +267,11 @@ Anthropic 公式のハーネスエンジニアリング指針（決定論的基 > > **Linux 実測で発見した副次不具合**: `cli-pr-monitor` の lock が**同時取得**を許していた (8 スレッド中 6 つが取得)。`create_new` は atomic だが直後のファイルは空で、その窓を読んだ側が TOML parse 失敗を一律「stale」と扱って全員 takeover していた。Windows ではスケジューリング差で顕在化していなかっただけで欠陥は同じ。parse 失敗を内容で 2 分 (空 = 書き込み中 → busy / 非空の不正 = 破損 → takeover) して修正。**「Windows だけで回していると気付けない設計欠陥が実在した」= Linux 実測と WP-16 (CI matrix) の価値を裏づける実例。** > -> **`完了` 条件**: (1) 本変更が master に入り release-binaries.yml が実走して `nightly` release が生成されること、(2) 実際の claude.ai/code セッションで `cloud-setup.sh` を走らせ hooks 発火と `cargo test` 通過を確認すること。いずれも本セッションでは実施不能 (release 未生成 / クラウド環境未使用) のため `実装済` に留める。**Linux 側の未検証領域**: `#[cfg(windows)]` ガードのテスト (pump_child_io の deadlock 保護、run_cmd_capture の stdout/stderr 分離) は Linux で skip されるため、WP-16 の CI matrix で扱う。以下は当初ステップ (記録用)。 +> **`完了` 条件 (1) 達成 (2026-07-20、PR #307 / #308 マージ後)**: release-binaries.yml が master で実走し `nightly` prerelease を生成 (tarball 9.72 MB + sha256、commit `541adde1`)。**公開された実物の artifact で end-to-end 検証済み**: 認証なしの素の curl で取得 → checksum 一致 → 17 ファイル展開 (バイナリ 16 + BUILD_INFO) → `file` が `ELF 64-bit LSB pie executable, x86-64 ... stripped` を確認 → **release バイナリそのもので Linux 上の hooks が実発火** (PreToolUse が危険コマンドを exit 2 でブロックし通常コマンドは exit 0、SessionStart が additionalContext JSON を出力、tree-sitter の comment-lint が違反検出)。これにより「public release は gh 認証不要」という設計判断も実証された。 +> +> **初回実行は失敗した (PR #308 で修正)**: #307 マージ直後の初回 run は `hooks-stop-tool-call-leak` の E2E が Broken pipe で落ちて赤になった。kill-switch 経路の hook が stdin を読まずに即 return するため、親の `write_all` が Unix で EPIPE になる競合で、Windows では payload がパイプバッファに収まり顕在化していなかった。**WSL では修正前でも 60/60 pass してしまい再現できず、ubuntu-22.04 の CI が初めて捕捉した** (遅延注入で EPIPE を強制して修正前 FAIL / 修正後 PASS を実測)。**WSL 検証はスケジューリング依存の race に対しては CI の代替にならない**という限定的な教訓であり、WP-16 (CI matrix) の必要性を裏づける実例。 +> +> **残る `完了` 条件 (2)**: 実際の claude.ai/code セッションで `cloud-setup.sh` を走らせ hooks 発火と `cargo test` 通過を確認すること (クラウド環境未使用のため本セッションでは実施不能)。**Linux 側の未検証領域**: `#[cfg(windows)]` ガードのテスト (pump_child_io の deadlock 保護、run_cmd_capture の stdout/stderr 分離) は Linux で skip されるため、WP-16 の CI matrix で扱う。以下は当初ステップ (記録用)。 - **目的**: 使い捨てのクラウドセッションで 19 crate をビルドせずにハーネスを即時有効化する。 - **ステップ**: From 908f6a9bb4cfd5ebab7858dbe72a567954d97a3b Mon Sep 17 00:00:00 2001 From: aloekun Date: Mon, 20 Jul 2026 21:21:47 +0900 Subject: [PATCH 4/4] =?UTF-8?q?fix(pr-monitor):=20=E3=83=AC=E3=83=93?= =?UTF-8?q?=E3=83=A5=E3=83=BC=E6=9C=AA=E5=AE=9F=E6=96=BD=E3=82=92=20succes?= =?UTF-8?q?s=20=E3=81=A8=E8=AA=A4=E5=88=A4=E5=AE=9A=E3=81=99=E3=82=8B=20fa?= =?UTF-8?q?il-open=20=E3=82=92=E4=BF=AE=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 本 PR の先行 2 コミットだけでは**観測される症状が変わらなかった**ため追加する。 rate-limit の検出は直ったが、検出結果が action 判定で捨てられていた。 3 層構造だった: 層1: 待ち時間パース失敗 → 「制限なし」扱い (先行コミットで修正済み) 層2: cr_ok が「コメント0件」を「レビューclean」と解釈 ← 本コミット 層3: action 確定後に terminal 短絡し rate-limit 分岐に到達しない ← 本コミット 層2 が本質。CodeRabbit がレート制限でレビューを**開始できなかった**場合もコメントは 0 件になるため、`new_comments == 0 && unresolved_threads == 0` だけを見ると **レビュー未実施と clean レビューが区別できない**。そこに ci_ok が重なり stop_monitoring_success に倒れていた。 層3 は iteration.rs の `action != "continue_monitoring"` で即 terminal を返す短絡。 層2 で success が確定すると、せっかく検出した state.rate_limit を使う handle_rate_limit_branch に一度も到達せず、park も `@coderabbitai review` の 再トリガーも起きなかった。 pending 判定を `checks_still_outstanding` として切り出し、rate_limit を 「まだ結論を出せない」条件に加えた。これで continue_monitoring に倒れ、既存の park + 再トリガー経路へ流れる。skip_coderabbit 指定時は rate-limit も判断材料から 外す (skip の意味と整合)。 **実環境での検証**: 同じ PR #309・同じレート制限状態で修正前後を実測。 修正前: action=stop_monitoring_success / 「問題は見つかりませんでした」 修正後: action=parked_review_recheck / 「review 完了待ちのため wakeup を予約」 checker 単体では既に rate_limit を正しく検出できていた (wait_minutes=23, wait_time_parsed=true) ことも確認済みで、捨てていたのが監視側だと特定できている。 test は ADR-049 の流儀で incident 再現 (bad) + 退行防止 (good) を対で追加: - rate-limit 中は「コメント0件」でも continue_monitoring にすること (incident 再現) - rate-limit 無しなら従来どおり stop_monitoring_success に到達すること - skip_coderabbit = true なら rate-limit も判断材料から外すこと 実測: Windows で cli-pr-monitor 265 pass (262→265) / clippy clean。 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/stages/poll/iteration.rs | 218 +++++++++++++++--- 1 file changed, 192 insertions(+), 26 deletions(-) diff --git a/src/cli-pr-monitor/src/stages/poll/iteration.rs b/src/cli-pr-monitor/src/stages/poll/iteration.rs index 9b4b2375..da4effd3 100644 --- a/src/cli-pr-monitor/src/stages/poll/iteration.rs +++ b/src/cli-pr-monitor/src/stages/poll/iteration.rs @@ -183,25 +183,51 @@ fn enrich_with_classifier( 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![], - }); + mark_ci_skipped(state); } if skip_coderabbit { - state.coderabbit = Some(CodeRabbitState { - review_state: "skipped".into(), - new_comments: 0, - actionable_comments: None, - unresolved_threads: None, - }); - state.findings = Vec::new(); + mark_coderabbit_skipped(state); } if skip_ci || skip_coderabbit { state.action = recompute_action(state, skip_ci, skip_coderabbit); + } else if should_downgrade_rate_limited_success(state, skip_coderabbit) { + state.action = "continue_monitoring".into(); } } +fn mark_ci_skipped(state: &mut PrMonitorState) { + state.ci = Some(CiState { + overall: "skipped".into(), + runs: vec![], + }); +} + +fn mark_coderabbit_skipped(state: &mut PrMonitorState) { + state.coderabbit = Some(CodeRabbitState { + review_state: "skipped".into(), + new_comments: 0, + actionable_comments: None, + unresolved_threads: None, + }); + state.findings = Vec::new(); +} + +/// skip 未指定 (本番設定: `check_ci=true` / `check_coderabbit=true`) の既定経路で、 +/// rate-limit 中に誤って `stop_monitoring_success` へ倒れた `state.action` を検出する。 +/// +/// この経路では上の `recompute_action` 呼び出しが発火せず、`state.action` は +/// `check-ci-coderabbit::decide()` の値がそのまま残る。`decide()` は `rate_limit` を +/// 知らないため、レート制限中で「コメント0件」だと誤って `stop_monitoring_success` を +/// 返すことがある (PR #307/#309 実観測)。ここで検出した場合は呼び出し側で +/// `continue_monitoring` に差し戻し、`run_one_iteration` の terminal 短絡を回避して +/// `handle_rate_limit_branch` の park + 再トリガー経路へ流す。 +/// +/// `action_required` / `stop_monitoring_failure` は `decide()` の判定 (actionable な +/// 指摘 / CI・CR 失敗) を尊重するため対象外 (`stop_monitoring_success` のみ検出)。 +fn should_downgrade_rate_limited_success(state: &PrMonitorState, skip_coderabbit: bool) -> bool { + cr_rate_limited(state, skip_coderabbit) && state.action == "stop_monitoring_success" +} + fn make_terminal_result(state: PrMonitorState, result: serde_json::Value) -> PollResult { PollResult { action: state.action, @@ -230,6 +256,40 @@ fn make_timeout_result( } } +/// まだ結論を出せる状態にないか (= 監視継続すべきか) を判定する。 +/// +/// **rate-limit は「レビュー未実施」であって「指摘なし」ではない**。CodeRabbit が +/// レート制限でレビューを開始できなかった場合もコメントは 0 件になるため、 +/// `cr_ok` (new_comments == 0 && unresolved_threads == 0) だけを見ると +/// **レビュー未実施と clean レビューが区別できず** success に倒れる +/// (PR #307 / #309 で実観測: 制限中なのに「問題は見つかりませんでした」と報告)。 +/// +/// ここで continue_monitoring に倒すことで、呼び出し側の terminal 短絡 +/// (`action != "continue_monitoring"` で即 return) を回避し、 +/// `handle_rate_limit_branch` の park + `@coderabbitai review` 再トリガー経路へ流す。 +fn checks_still_outstanding(state: &PrMonitorState, skip_ci: bool, skip_coderabbit: bool) -> bool { + 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); + + ci_pending || cr_pending || cr_rate_limited(state, skip_coderabbit) +} + +/// `skip_coderabbit` 適用後に、CodeRabbit がレート制限中かどうかを判定する。 +fn cr_rate_limited(state: &PrMonitorState, skip_coderabbit: bool) -> bool { + !skip_coderabbit && state.rate_limit.is_some() +} + /// skip 適用後に、有効なチェックだけを見て action を再導出する fn recompute_action(state: &PrMonitorState, skip_ci: bool, skip_coderabbit: bool) -> String { let ci_ok = skip_ci @@ -249,21 +309,7 @@ fn recompute_action(state: &PrMonitorState, skip_ci: bool, skip_coderabbit: bool }) .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 { + if checks_still_outstanding(state, skip_ci, skip_coderabbit) { return "continue_monitoring".into(); } @@ -297,6 +343,126 @@ mod tests { use super::*; use lib_report_formatter::Finding; + /// CI 完了 + CodeRabbit がコメント 0 件 = 一見「clean」に見える state を作る。 + /// rate_limit を後から差し込むことで「レビュー未実施」との差だけを検証できる。 + fn settled_state() -> PrMonitorState { + let mut state = PrMonitorState::new(Some(1), None, "t".into()); + state.ci = Some(crate::state::CiState { + overall: "success".into(), + runs: vec![], + }); + state.coderabbit = Some(crate::state::CodeRabbitState { + review_state: "success".into(), + new_comments: 0, + actionable_comments: None, + unresolved_threads: Some(0), + }); + state + } + + fn rate_limit_state() -> crate::state::RateLimitState { + crate::state::RateLimitState { + until_unix_secs: 1_784_550_887, + comment_event_time: "2026-07-20T12:10:47Z".into(), + wait_minutes: 23, + wait_seconds: 0, + wait_time_parsed: true, + } + } + + /// PR #307 / #309 incident 再現 (bad): rate-limit 中は「コメント 0 件」でも + /// 結論を出さず監視を継続すること。 + /// + /// 由来: 2026-07-20。CodeRabbit がレート制限でレビューを開始できなかったのに + /// 監視が stop_monitoring_success を返した。**レビュー未実施と clean レビューが + /// どちらも「コメント 0 件」になる**ため、rate_limit を見ないと区別できない。 + /// success に倒れると呼び出し側の terminal 短絡で rate-limit 分岐に到達せず、 + /// park も `@coderabbitai review` の再トリガーも起きない。 + #[test] + fn rate_limited_review_is_not_treated_as_clean() { + let mut state = settled_state(); + state.rate_limit = Some(rate_limit_state()); + + assert!( + checks_still_outstanding(&state, false, false), + "rate-limit 中は監視継続すべき (レビュー未実施を clean と誤判定しない)", + ); + assert_eq!( + recompute_action(&state, false, false), + "continue_monitoring", + "success に倒すと terminal 短絡で park 経路に到達しない", + ); + } + + /// good: rate-limit が無ければ従来どおり success に到達すること (退行なし)。 + #[test] + fn settled_checks_without_rate_limit_still_reach_success() { + let state = settled_state(); + + assert!(!checks_still_outstanding(&state, false, false)); + assert_eq!(recompute_action(&state, false, false), "stop_monitoring_success"); + } + + /// good: CodeRabbit を skip する構成では rate-limit があっても監視を止めないこと。 + /// skip 指定は「CodeRabbit を判断材料にしない」意味なので、その中の rate-limit も + /// 判断材料から外れる。 + #[test] + fn rate_limit_is_ignored_when_coderabbit_is_skipped() { + let mut state = settled_state(); + state.rate_limit = Some(rate_limit_state()); + + assert!( + !checks_still_outstanding(&state, false, true), + "skip_coderabbit = true なら rate-limit も判断材料から外す", + ); + } + + /// SIM-NEW-iteration.rs-L259 fix (regression): skip 未指定の既定経路 + /// (本番設定 `check_ci=true` / `check_coderabbit=true` → skip_ci=false / + /// skip_coderabbit=false) では旧実装が `recompute_action` を呼ばず、 + /// `checks_still_outstanding` の rate-limit チェックが実質デッドコードだった。 + /// `apply_skip_handling` を直接呼び、`state.action` が実際に downgrade + /// されることを確認する。 + #[test] + fn apply_skip_handling_downgrades_rate_limited_success_without_skip_flags() { + let mut state = settled_state(); + state.action = "stop_monitoring_success".into(); + state.rate_limit = Some(rate_limit_state()); + + apply_skip_handling(&mut state, false, false); + + assert_eq!( + state.action, "continue_monitoring", + "skip 未指定の既定経路でも rate-limit 中は success を確定させてはいけない", + ); + } + + /// good: rate-limit があっても `action_required` (actionable な既存指摘) は + /// 上書きしないこと。stale な状態でも既にある指摘を握りつぶすべきではない。 + #[test] + fn apply_skip_handling_keeps_action_required_even_when_rate_limited() { + let mut state = settled_state(); + state.action = "action_required".into(); + state.rate_limit = Some(rate_limit_state()); + + apply_skip_handling(&mut state, false, false); + + assert_eq!(state.action, "action_required"); + } + + /// good: rate-limit があっても `stop_monitoring_failure` (CI/CR失敗) は + /// 上書きしないこと。失敗判定は decide() の優先順位を尊重する。 + #[test] + fn apply_skip_handling_keeps_failure_even_when_rate_limited() { + let mut state = settled_state(); + state.action = "stop_monitoring_failure".into(); + state.rate_limit = Some(rate_limit_state()); + + apply_skip_handling(&mut state, false, false); + + assert_eq!(state.action, "stop_monitoring_failure"); + } + /// PR #238 regression: checker が stderr に警告 (repo 検出失敗等の fail-soft ログ) /// を出しても、stdout の JSON パースが壊れないこと。修正前は stdout+stderr の /// 結合テキストをパースしており「trailing characters」で監視が停止した。