From 27379dec5e341b01b218ce424e03bf59db2cec22 Mon Sep 17 00:00:00 2001 From: aloekun Date: Tue, 5 May 2026 18:43:20 +0900 Subject: [PATCH 1/2] =?UTF-8?q?feat(cli-pr-monitor):=20rate-limit=20retry?= =?UTF-8?q?=20=E3=82=92=20CronCreate=20park=20=E3=83=A2=E3=83=87=E3=83=AB?= =?UTF-8?q?=E3=81=AB=E7=A7=BB=E8=A1=8C=20(Bundle=20b=20PR-1,=20=E9=A0=86?= =?UTF-8?q?=E4=BD=8D=2053)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #104 で 47 分 rate-limit を実観測した致命点 (max_duration_secs=600s cap で auto-retry できず action_required 通知 → ユーザー手動介入が必要) への対策。 cli-pr-monitor が同プロセス内で std::thread::sleep する設計を廃止し、reset 時刻を state.next_wakeup_at_unix に保存して PARK signal を stdout に emit して exit するモデルに切り替える。Claude Code 側が PARK signal を読み CronCreate (durable: true) で wakeup を予約、reset 後に cli-pr-monitor.exe --monitor-only が再 invoke されて @coderabbitai review が自動投稿される。 ADR-030 (post-merge-feedback) で実証済の責務分離パターン (機械的 Rust + Claude Code 定期チェックの分離) を CR rate-limit に適用する 3 例目。CronCreate には 時間制約がない (60 分 cap は別ツール ScheduleWakeup の制約) ため、47 分でも 90 分でも cron で任意時刻予約可能。durable: true で .claude/scheduled_tasks.json に永続化され session 跨ぎを native サポート。 主な変更: - PrMonitorState に next_wakeup_at_unix / wakeup_reason を追加 - handle_rate_limit_retry を RateLimitOutcome enum 返却に refactor (Posted / Parked { wakeup_at_unix } / Failed) - reset 時刻が未来 → Parked → state 更新 + PARK signal emit + parked_rate_limit action で early return - reset 時刻が過去 → 従来通り即時 @coderabbitai review 投稿 (Posted) - max_duration_secs > sleep_secs の予算チェック撤廃 (sleep 自体が廃止) - print_report を parked verdict 対応に拡張 - 副次的 refactor (touch-trigger ratchet 適用): run_poll_loop を PollContext + run_one_iteration 等 11 helper に分割、print_report を compute_verdict + print_findings_table に分割 PARK signal フォーマット (stdout): [PR_MONITOR_PARK] pr / repo / reset_at_unix / reset_at_iso_utc / wait_total_seconds / retry_count / max_retries / exe / cwd + CronCreate({...}) 呼び出しテンプレート (cron 式は Claude Code が local TZ で計算、durable: true / recurring: false / prompt にコマンドを埋め込み) [/PR_MONITOR_PARK] 既知の制約 (PR description / Bb-2/Bb-3 で対応予定): - 単一 state file 設計は維持 (複数 PR 並行時の collision は Bb-2/Bb-3 で扱う) - session 終了時の wakeup catch-up は Bb-3 (SessionStart hook 拡張) で対応 - 過渡期: Bb-1 land 後 Bb-3 land 前は AI 離席中の wakeup スキップが残る (durable: true で再起動時 fire-on-idle するため致命的ではない) 設計根拠は docs/coderabbit-monitoring-efficiency.md の「着手決定 (2026-05-05)」 セクションを参照。 Tests: - 125 passed / 0 failed (cargo test -p cli-pr-monitor) - 新規テスト 6 件: state serde (3) + RateLimitOutcome 経路 (2) + format_park_signal (2) - cargo clippy -D warnings clean --- docs/coderabbit-monitoring-efficiency.md | 46 ++ src/cli-pr-monitor/src/stages/monitor.rs | 83 +-- src/cli-pr-monitor/src/stages/poll.rs | 763 +++++++++++++++-------- src/cli-pr-monitor/src/state.rs | 53 ++ 4 files changed, 638 insertions(+), 307 deletions(-) diff --git a/docs/coderabbit-monitoring-efficiency.md b/docs/coderabbit-monitoring-efficiency.md index 8d209fb3..58de931b 100644 --- a/docs/coderabbit-monitoring-efficiency.md +++ b/docs/coderabbit-monitoring-efficiency.md @@ -10,6 +10,52 @@ --- +## 着手決定 (2026-05-05) + +### 採用案 + +**Bundle b (CR auto-monitoring core、3 PR 構成) を採用**。`max_duration_secs` 拡大による sleep 延長案 (案 C) は **却下**。 + +### 着手順 / 設計方針 + +- **着手は Bb-1 (順位 53) から** +- **CronCreate は `durable: true` で利用** — `.claude/scheduled_tasks.json` に永続化され、Claude Code session 再起動を跨ぐ +- **PR description で ADR-030 の責務分離パターン (L1 floor + L2 recovery) を引用** — post-merge-feedback で実証済の「機械的 Rust + Claude Code 定期チェック」分離原則を CR rate-limit に適用 (3 例目)。新 ADR で本パターン適用を明文化する想定 + +### 設計判断時に確認した CronCreate の重要事実 (今後の再分析を防ぐため記録) + +- 標準 5 フィールド cron 構文 (`MM HH DoM Mon DoW`) — **時間制約なし**。「60 分上限」は **別ツール `ScheduleWakeup` (`/loop` 動的モード) の `clamped to [60, 3600]` 制約** であり、CronCreate には適用されない +- One-shot (`recurring: false`) で任意の reset 時刻に 1 度だけ wakeup 可能 → 47 分でも 90 分でも cron で直接予約可能 +- `durable: true` で session 跨ぎが native サポート — 旧 ADR-009 設計の「session 終了で wakeup 発火しない」問題は durable 化で解消可能 (Bb-3 SessionStart catch-up は補完層として位置づけ) +- recurring task は 7 日 auto-expire (rate-limit context では十分長い) + +### ADR-018 との整合性 + +ADR-018 が廃止したのは「同一プロセス内で 4 段間接連携 (daemon → state file → CronCreate → Claude → skill) する旧設計」であり、**プロセス分離 + 責務分離原則そのものは ADR-018 流儀と整合**。Bundle b は ADR-030 の L1+L2 パターン (機械的 Rust 状態管理 + Claude Code 定期チェックの分離) を踏襲する。 + +### 案 C を却下した理由 + +- **`max_duration_secs` 1h cap 延長は短期的には有効だが、Claude Code session 終了時の silent loss を解消できない** — cli-pr-monitor は Claude Code Bash tool の child process として動作するため、session 終了で sleep 中の subprocess が kill される +- **CronCreate `durable: true` は session 跨ぎを native サポート** — 案 C 単体より UX が確実に良い +- 90+ 分 rate-limit に対する構造的限界 (案 C は cap 超過で fail、案 A は cron で任意時刻予約可能) +- 「30 分ごとに確認 → 残時間再計算で reschedule」chain pattern は CronCreate native 機能で実現可能 (ユーザー指摘、本決定の根拠の一つ) + +### 進捗 + +- ✅ **Bb-1 (順位 53) — 実装完了 (未 PR / 未マージ、2026-05-05)**: `cli-pr-monitor` の rate-limit retry を CronCreate park モデルに切り替え。 + - `PrMonitorState` に `next_wakeup_at_unix` / `wakeup_reason` を追加 + - `handle_rate_limit_retry` を `RateLimitOutcome` enum 返却に refactor (Posted / Parked / Failed) + - reset 時刻が未来の場合は sleep せず park: state を更新し PARK signal (CronCreate 呼び出しテンプレート + repo / pr / reset_at_unix / reset_at_iso_utc / wait_total_seconds / exe / cwd を含む) を stdout に emit して `parked_rate_limit` action で early return + - reset 時刻が過去の場合は従来通り即時 `@coderabbitai review` 投稿 (Posted) + - `max_duration_secs > sleep_secs` の予算チェックを撤廃 (sleep そのものが廃止) + - `print_report` を parked 対応 verdict に拡張 + - 副次的 refactor (touch-trigger ratchet 適用): `run_poll_loop` を `PollContext` + `run_one_iteration` 等の helper に分割、`print_report` を `compute_verdict` + `print_findings_table` に分割 + - 全 125 テスト pass、clippy clean、release ビルド済 (`.claude/cli-pr-monitor.exe` 配置済) +- ⏳ **Bb-2 (順位 54)** — 未着手 (Bb-1 land 後) +- ⏳ **Bb-3 (順位 55)** — 未着手 (Bb-1 / Bb-2 land 後) + +--- + ## 現状の課題 - **CodeRabbit 無課金**: 1 時間あたり 3 reviews 上限 diff --git a/src/cli-pr-monitor/src/stages/monitor.rs b/src/cli-pr-monitor/src/stages/monitor.rs index bee67f60..82aa88e3 100644 --- a/src/cli-pr-monitor/src/stages/monitor.rs +++ b/src/cli-pr-monitor/src/stages/monitor.rs @@ -159,7 +159,6 @@ fn print_report(result: &crate::stages::poll::PollResult, pr_label: &str) { .as_ref() .map(|c| c.overall.as_str()) .unwrap_or("unknown"); - let cr_comments = result .coderabbit .as_ref() @@ -171,7 +170,27 @@ fn print_report(result: &crate::stages::poll::PollResult, pr_label: &str) { .and_then(|c| c.unresolved_threads) .unwrap_or(0); - let findings_count = result.findings.len(); + println!(); + println!("## Review Report ({})", pr_label); + println!(); + println!( + "CI: {} | CodeRabbit: 新規コメント{}件, 未解決スレッド{}件", + ci_status, cr_comments, cr_threads + ); + println!("action: {} | summary: {}", result.action, result.summary); + println!(); + println!("**判定**: {}", compute_verdict(result)); + + if !result.findings.is_empty() { + print_findings_table(&result.findings); + } +} + +fn compute_verdict(result: &crate::stages::poll::PollResult) -> &'static str { + if result.action == "parked_rate_limit" { + return "CodeRabbit rate-limit のため wakeup を予約 (上記 PARK signal 参照)"; + } + let critical_major = result .findings .iter() @@ -181,48 +200,34 @@ fn print_report(result: &crate::stages::poll::PollResult, pr_label: &str) { }) .count(); - // 判定 - let verdict = if critical_major > 0 { + if critical_major > 0 { "修正が必要な指摘があります" - } else if findings_count > 0 { + } else if !result.findings.is_empty() { "重大な問題は見つかりませんでした。軽微な改善提案があります" } else { "問題は見つかりませんでした" - }; + } +} - // 統合レポート形式 (post-pr-create-review-check スキルと同一フォーマット) +fn print_findings_table(findings: &[lib_report_formatter::Finding]) { println!(); - println!("## Review Report ({})", pr_label); - println!(); - println!( - "CI: {} | CodeRabbit: 新規コメント{}件, 未解決スレッド{}件", - ci_status, cr_comments, cr_threads - ); - println!("action: {} | summary: {}", result.action, result.summary); - println!(); - println!("**判定**: {}", verdict); - - if findings_count > 0 { - println!(); - println!("| # | Source | Severity | File (Line) | Issue | Suggestion |"); - println!("|---|--------|----------|-------------|-------|------------|"); - for (i, f) in result.findings.iter().enumerate() { - // suggestion を 80 文字 (char 単位) で切り詰め (UTF-8 安全) - let suggestion = if f.suggestion.chars().count() > 80 { - format!("{}...", truncate_safe(&f.suggestion, 77)) - } else { - f.suggestion.clone() - }; - println!( - "| {} | {} | {} | {} ({}) | {} | {} |", - i + 1, - f.source, - f.severity, - f.file, - f.line, - f.issue, - suggestion - ); - } + println!("| # | Source | Severity | File (Line) | Issue | Suggestion |"); + println!("|---|--------|----------|-------------|-------|------------|"); + for (i, f) in findings.iter().enumerate() { + let suggestion = if f.suggestion.chars().count() > 80 { + format!("{}...", truncate_safe(&f.suggestion, 77)) + } else { + f.suggestion.clone() + }; + println!( + "| {} | {} | {} | {} ({}) | {} | {} |", + i + 1, + f.source, + f.severity, + f.file, + f.line, + f.issue, + suggestion + ); } } diff --git a/src/cli-pr-monitor/src/stages/poll.rs b/src/cli-pr-monitor/src/stages/poll.rs index c5a0dbbd..d43fc973 100644 --- a/src/cli-pr-monitor/src/stages/poll.rs +++ b/src/cli-pr-monitor/src/stages/poll.rs @@ -24,14 +24,21 @@ pub(crate) struct PollResult { pub(crate) rate_limit: Option, } +struct PollContext<'a> { + checker: &'a std::path::Path, + push_time: &'a str, + pr_info: &'a PrInfo, + rate_limit_config: &'a RateLimitConfig, + start: std::time::Instant, + max_duration: u64, + skip_ci: bool, + skip_coderabbit: bool, +} + /// in-process 同期ポーリングループ (daemon.rs の同期版) pub(crate) fn run_poll_loop(full_config: &Config, pr_info: &PrInfo) -> PollResult { let config: &MonitorConfig = &full_config.monitor; - let rate_limit_config: &RateLimitConfig = &full_config.rate_limit; let poll_interval = config.poll_interval_secs; - let max_duration = config.max_duration_secs; - let skip_ci = !config.check_ci; - let skip_coderabbit = !config.check_coderabbit; let checker = checker_exe_path(); if !checker.exists() { @@ -39,297 +46,411 @@ pub(crate) fn run_poll_loop(full_config: &Config, pr_info: &PrInfo) -> PollResul "check-ci-coderabbit.exe が見つかりません: {}", checker.display() )); - return PollResult { - action: "error".into(), - summary: "check-ci-coderabbit.exe が見つかりません".into(), - ci: None, - coderabbit: None, - findings: Vec::new(), - check_output: None, - rate_limit: None, - }; + return error_poll_result("check-ci-coderabbit.exe が見つかりません"); } - let push_time = pr_info - .push_time - .as_deref() - .unwrap_or("1970-01-01T00:00:00Z"); - - let start = std::time::Instant::now(); + let ctx = PollContext { + checker: &checker, + push_time: pr_info + .push_time + .as_deref() + .unwrap_or("1970-01-01T00:00:00Z"), + pr_info, + rate_limit_config: &full_config.rate_limit, + start: std::time::Instant::now(), + max_duration: config.max_duration_secs, + skip_ci: !config.check_ci, + skip_coderabbit: !config.check_coderabbit, + }; loop { - // Build checker arguments - let mut checker_args: Vec = vec!["--push-time".to_string(), push_time.to_string()]; - if let Some(ref repo) = pr_info.repo { - checker_args.push("--repo".to_string()); - checker_args.push(repo.clone()); - } - if let Some(pr) = pr_info.pr_number { - checker_args.push("--pr".to_string()); - checker_args.push(pr.to_string()); + if let Some(terminal) = run_one_iteration(&ctx) { + return terminal; } + std::thread::sleep(Duration::from_secs(poll_interval)); + } +} - // Run check-ci-coderabbit.exe - let (success, output) = run_cmd_direct( - &checker.to_string_lossy(), - &[], - &checker_args, - DEFAULT_CHECK_TIMEOUT_SECS, - ); +fn run_one_iteration(ctx: &PollContext<'_>) -> Option { + let args = build_checker_args(ctx.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, + ); + log_info(&format!( + "ポーリング: action={}, summary={}", + state.action, state.summary + )); - if !success { - log_info(&format!("checker 失敗: {}", truncate_safe(&output, 200))); - return PollResult { - action: "error".into(), - summary: format!( - "check-ci-coderabbit.exe 失敗: {}", - truncate_safe(&output, 200) - ), - ci: None, - coderabbit: None, - findings: Vec::new(), - check_output: None, - rate_limit: None, - }; - } + if state.action != "continue_monitoring" { + return Some(make_terminal_result(state, result)); + } - let result = match serde_json::from_str::(&output) { - Ok(r) => r, - Err(e) => { - log_info(&format!("JSON パース失敗: {}", e)); - return PollResult { - action: "error".into(), - summary: format!("checker 出力の JSON パース失敗: {}", e), - ci: None, - coderabbit: None, - findings: Vec::new(), - check_output: None, - rate_limit: None, - }; - } - }; + if let Some(terminal) = + handle_rate_limit_branch(&mut state, ctx.rate_limit_config, ctx.pr_info, &result) + { + return Some(terminal); + } - // Update state from check result - 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); - - // `PrMonitorState::new` は毎回 notified=false / rate_limit_retries=0 で初期化するため、 - // 既存 state から runtime-updated な値を読み戻す。新規セッションでは - // start_monitoring 冒頭で init_state により reset 済み。 - 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; - } + 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)); + } - // Skip handling: skipped なチェックを成功扱いにした後、action を再計算する - 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); - } + 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 { + PollResult { + action: "error".into(), + summary: summary.into(), + ci: None, + coderabbit: None, + findings: Vec::new(), + check_output: None, + rate_limit: None, + } +} + +/// `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.last_checked = Some(utc_now_iso8601()); + apply_skip_handling(&mut state, skip_ci, skip_coderabbit); + state.last_checked = Some(utc_now_iso8601()); + let _ = write_state(&state); + state +} - // Write state for debug/observability - let _ = write_state(&state); +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, + } +} + +/// rate-limit 検出 branch を集約する。 +/// +/// dedup: 同一 rate-limit comment は iteration を跨いで残るため `comment_event_time` +/// で dedup する。dedup なしでは即時 retrigger を秒単位で繰り返し max_retries を浪費する。 +/// CR が新たな rate-limit comment を投稿すると event_time が変わり再 handle 対象になる。 +fn handle_rate_limit_branch( + state: &mut PrMonitorState, + rate_limit_config: &RateLimitConfig, + pr_info: &PrInfo, + result: &serde_json::Value, +) -> Option { + let rl = state.rate_limit.clone()?; + let already_handled = + state.rate_limit_last_retriggered_at.as_deref() == Some(rl.comment_event_time.as_str()); + + if already_handled { log_info(&format!( - "ポーリング: action={}, summary={}", - state.action, state.summary + "[rate_limit] 同じ rate-limit comment ({}) は処理済み、retrigger スキップ", + rl.comment_event_time )); + return None; + } - // Terminal action -> return result - if state.action != "continue_monitoring" { - return 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, - }; - } + if state.rate_limit_retries >= rate_limit_config.max_retries { + log_info(&format!( + "[rate_limit] max_retries={} 到達、自動 retry を停止", + rate_limit_config.max_retries + )); + return Some(make_max_retries_result(state, result)); + } - // Rate-limit 自動 retry (PR #89 T2-1) - // - // dedup: 同一の rate-limit comment は iteration を跨いで PR コメント一覧に残るため - // `comment_event_time` で dedup しないと、毎回 sleep_secs=0 で即時 retrigger を繰り返し - // 数秒で max_retries を消費してしまう。CR が新たな rate-limit comment を投稿した時点で - // created_at が変わり再度 retrigger 対象になる。 - if let Some(rl) = state.rate_limit.clone() { - let already_handled = state.rate_limit_last_retriggered_at.as_deref() - == Some(rl.comment_event_time.as_str()); - - if already_handled { - log_info(&format!( - "[rate_limit] 同じ rate-limit comment ({}) は処理済み、retrigger スキップ", - rl.comment_event_time -)); - // 通常 polling cadence で待機する (CR レビュー完了を待つ) - } else if rate_limit_config.auto_retry_enabled - && state.rate_limit_retries < rate_limit_config.max_retries - { - let elapsed = start.elapsed().as_secs(); - let remaining_monitor_secs = max_duration.saturating_sub(elapsed); - - if let Err(e) = handle_rate_limit_retry( - &rl, - &mut state, - pr_info, - rate_limit_config.max_retries, - remaining_monitor_secs, - ) { - // 失敗時は dedup を更新せず action_required で抜ける。 - // last_retriggered_at が未更新のため、次セッションで同 comment を - // 再 trigger 試行できる (本セッションでは budget 超過/post 失敗のため停止)。 - log_info(&format!("[rate_limit] retrigger 失敗: {}", e)); - return PollResult { - action: "action_required".into(), - summary: format!( - "rate-limit 自動 retry 失敗 ({})。手動で `@coderabbitai review` を投稿してください", - e - ), - ci: state.ci, - coderabbit: state.coderabbit, - findings: state.findings, - check_output: Some(result), - rate_limit: state.rate_limit, - }; - } - state.rate_limit_last_retriggered_at = Some(rl.comment_event_time.clone()); - // state 永続化失敗時は dedup / max_retries が壊れる可能性があるため、 - // 自動 retry を停止し action_required で抜ける (重複投稿リスクを回避)。 - if let Err(e) = write_state(&state) { - log_info(&format!( - "[rate_limit] retrigger 後の state 永続化失敗、自動 retry を停止: {}", - e - )); - return PollResult { - action: "action_required".into(), - summary: format!( - "rate-limit retry 後の state 永続化に失敗 ({})。手動で `@coderabbitai review` の重複投稿に注意してください", - e - ), - ci: state.ci, - coderabbit: state.coderabbit, - findings: state.findings, - check_output: Some(result), - rate_limit: state.rate_limit, - }; - } - continue; // skip 通常 sleep、次 iteration で fresh polling - } else if state.rate_limit_retries >= rate_limit_config.max_retries { - log_info(&format!( - "[rate_limit] max_retries={} 到達、自動 retry を停止 (action_required で抜ける)", - rate_limit_config.max_retries - )); - return PollResult { - action: "action_required".into(), - summary: format!( - "CodeRabbit rate-limit が {} 回再試行後も継続。手動で `@coderabbitai review` を投稿してください", - state.rate_limit_retries - ), - ci: state.ci, - coderabbit: state.coderabbit, - findings: state.findings, - check_output: Some(result), - rate_limit: state.rate_limit, - }; - } - } + if !rate_limit_config.auto_retry_enabled { + return None; + } + + dispatch_rate_limit_outcome(state, &rl, pr_info, rate_limit_config.max_retries, result) +} - // Timeout check - if start.elapsed() >= Duration::from_secs(max_duration) { - log_info(&format!("監視タイムアウト ({}秒)", max_duration)); - return 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, - }; +fn dispatch_rate_limit_outcome( + state: &mut PrMonitorState, + rl: &crate::state::RateLimitState, + pr_info: &PrInfo, + max_retries: u32, + result: &serde_json::Value, +) -> Option { + match handle_rate_limit_retry(rl, state, pr_info, max_retries) { + RateLimitOutcome::Posted => finalize_posted_retrigger(state, rl, result), + RateLimitOutcome::Parked { wakeup_at_unix } => Some(finalize_parked( + state, + rl, + pr_info, + wakeup_at_unix, + max_retries, + result, + )), + RateLimitOutcome::Failed(e) => { + log_info(&format!("[rate_limit] retrigger 失敗: {}", e)); + Some(make_action_required_result( + state, + result, + &format!( + "rate-limit 自動 retry 失敗 ({})。手動で `@coderabbitai review` を投稿してください", + e + ), + )) } + } +} - // Sleep before next poll - std::thread::sleep(Duration::from_secs(poll_interval)); +fn finalize_posted_retrigger( + state: &mut PrMonitorState, + rl: &crate::state::RateLimitState, + result: &serde_json::Value, +) -> Option { + state.rate_limit_last_retriggered_at = Some(rl.comment_event_time.clone()); + if let Err(e) = write_state(state) { + log_info(&format!( + "[rate_limit] retrigger 後の state 永続化失敗、自動 retry を停止: {}", + e + )); + return Some(make_action_required_result( + state, + result, + &format!( + "rate-limit retry 後の state 永続化に失敗 ({})。手動で `@coderabbitai review` の重複投稿に注意してください", + e + ), + )); } + None } -/// rate-limit reset まで sleep し、`@coderabbitai review` を post する。 +fn finalize_parked( + state: &mut PrMonitorState, + rl: &crate::state::RateLimitState, + pr_info: &PrInfo, + wakeup_at_unix: i64, + max_retries: u32, + result: &serde_json::Value, +) -> PollResult { + state.action = "parked_rate_limit".into(); + state.next_wakeup_at_unix = Some(wakeup_at_unix); + state.wakeup_reason = Some("rate_limit_retry".into()); + state.summary = format!( + "CodeRabbit rate-limit: wakeup を {}m{}s 後に予約 (PARK signal 参照)", + rl.wait_minutes, rl.wait_seconds + ); + if let Err(e) = write_state(state) { + log_info(&format!("[rate_limit] park state 永続化失敗: {}", e)); + } + let signal = format_park_signal(state, rl, pr_info, max_retries); + println!("{}", signal); + + PollResult { + action: state.action.clone(), + summary: state.summary.clone(), + ci: state.ci.clone(), + coderabbit: state.coderabbit.clone(), + findings: state.findings.clone(), + check_output: Some(result.clone()), + rate_limit: state.rate_limit.clone(), + } +} + +fn make_max_retries_result(state: &PrMonitorState, result: &serde_json::Value) -> PollResult { + let summary = format!( + "CodeRabbit rate-limit が {} 回再試行後も継続。手動で `@coderabbitai review` を投稿してください", + state.rate_limit_retries + ); + make_action_required_result(state, result, &summary) +} + +fn make_action_required_result( + state: &PrMonitorState, + result: &serde_json::Value, + summary: &str, +) -> PollResult { + PollResult { + action: "action_required".into(), + summary: summary.into(), + ci: state.ci.clone(), + coderabbit: state.coderabbit.clone(), + findings: state.findings.clone(), + check_output: Some(result.clone()), + rate_limit: state.rate_limit.clone(), + } +} + +/// `handle_rate_limit_retry` の outcome 種別 (Bb-1, Bundle b PR-1)。 /// -/// 成功時のみ `Ok(())` を返し、`state.rate_limit_retries` をインクリメントする。 -/// 失敗ケース (PR 番号未確定 / sleep が監視残り予算超過 / gh post 失敗) は -/// state を変更せず `Err` を返す。caller は `last_retriggered_at` を更新せず、 -/// 同 comment を未処理のまま残すこと。 +/// rate-limit 検出時の振る舞いは sleep 廃止 + park 化に切り替わった: /// -/// `until_unix_secs <= now` の場合は sleep をスキップして即時 retrigger -/// (= 過去の rate-limit comment を発見、既にリセット済み)。 +/// - `Posted`: reset 時刻が既に過去 (`sleep_secs <= 0`) のため、その場で +/// `@coderabbitai review` を投稿し `rate_limit_retries` をインクリメント。 +/// caller は polling を継続する (現状挙動と同じ)。 +/// - `Parked`: reset 時刻が未来。同プロセス内で sleep せず、caller に「state に +/// `next_wakeup_at_unix` を保存し PARK signal を stdout に出して終端 action で +/// exit せよ」と通知する。実 wakeup は CronCreate (`durable: true`) 経由で +/// `cli-pr-monitor.exe --monitor-only` を再 invoke する流れ (ADR-030 L1+L2 を踏襲)。 +/// - `Failed`: PR 番号未確定 / gh post 失敗。caller は state を更新せず +/// action_required で抜ける。 +pub(crate) enum RateLimitOutcome { + Posted, + Parked { wakeup_at_unix: i64 }, + Failed(String), +} + +/// rate-limit 検出時の outcome を返す。 +/// +/// `until_unix_secs > now`: park (sleep しない、caller が wakeup 予約を依頼) +/// `until_unix_secs <= now`: その場で `@coderabbitai review` を投稿 fn handle_rate_limit_retry( rl: &crate::state::RateLimitState, state: &mut PrMonitorState, pr_info: &PrInfo, max_retries: u32, - remaining_monitor_secs: u64, -) -> Result<(), String> { +) -> RateLimitOutcome { let now_unix = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_secs() as i64) .unwrap_or(0); let sleep_secs = (rl.until_unix_secs - now_unix).max(0) as u64; - // 監視残り時間を超える sleep は実施しない (max_duration を素通りさせない) - if sleep_secs > remaining_monitor_secs { - return Err(format!( - "rate-limit sleep ({}s) > 監視残り予算 ({}s)", - sleep_secs, remaining_monitor_secs - )); - } - - let pr = pr_info - .pr_number - .ok_or_else(|| "PR 番号未確定のため retrigger スキップ".to_string())?; + let Some(pr) = pr_info.pr_number else { + return RateLimitOutcome::Failed("PR 番号未確定のため retrigger スキップ".into()); + }; if sleep_secs > 0 { log_info(&format!( - "[rate_limit] reset まで sleep {}秒 (wait={}m{}s + 60s buffer、retry={}/{})", + "[rate_limit] reset まで {}秒 (wait={}m{}s + 60s buffer)、Park で wakeup 要求 (retry 候補={}/{})", sleep_secs, rl.wait_minutes, rl.wait_seconds, state.rate_limit_retries + 1, max_retries )); - std::thread::sleep(Duration::from_secs(sleep_secs)); - } else { - log_info(&format!( - "[rate_limit] reset 時刻は既に過去、即時 retrigger (retry={})", - state.rate_limit_retries + 1 - )); + return RateLimitOutcome::Parked { + wakeup_at_unix: rl.until_unix_secs, + }; } + post_review_immediately(pr, state) +} + +/// `sleep_secs <= 0` 経路の即時 `@coderabbitai review` 投稿を担う helper。 +fn post_review_immediately(pr: u64, state: &mut PrMonitorState) -> RateLimitOutcome { + log_info(&format!( + "[rate_limit] reset 時刻は既に過去、即時 retrigger (retry={})", + state.rate_limit_retries + 1 + )); + let pr_str = pr.to_string(); if run_gh_quiet(&["pr", "comment", &pr_str, "--body", "@coderabbitai review"]).is_none() { - return Err(format!("gh pr comment 投稿失敗 (PR #{})", pr)); + return RateLimitOutcome::Failed(format!("gh pr comment 投稿失敗 (PR #{})", pr)); } log_info(&format!( @@ -339,8 +460,63 @@ fn handle_rate_limit_retry( )); state.rate_limit_retries += 1; - // rate_limit field は次の polling iteration で再 detect されるためここでは clear しない。 - Ok(()) + RateLimitOutcome::Posted +} + +/// PARK signal を stdout に書き出すための pure 関数 (Bb-1)。 +/// +/// Claude Code が stdout を読み、含まれる `CronCreate(...)` 呼び出しテンプレートに +/// 従って `durable: true` で wakeup を予約する想定。`reset_at_iso_utc` は UTC な +/// ため、cron 式は Claude が local timezone に変換して計算する (chrono 依存を +/// 避けるため Rust 側では cron 式を組まない)。 +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] +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, + ) } /// skip 適用後に、有効なチェックだけを見て action を再導出する @@ -520,15 +696,15 @@ mod tests { let _ = std::fs::remove_file(&tmp); } - /// Finding 2 (PR #97 round 3): sleep が監視残り予算を超える場合、 - /// `handle_rate_limit_retry` は Err を返し state を変更しない。 + /// Bb-1: reset 時刻が未来の場合、`handle_rate_limit_retry` は Parked を返し + /// state.rate_limit_retries を変更しない (実 retry 計上は wakeup 経由で post 投稿後)。 #[test] - fn rate_limit_retry_returns_err_when_sleep_exceeds_budget() { + 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; // 10 分後 + + 600; let rl = RateLimitState { until_unix_secs: future_unix, comment_event_time: "2026-04-30T00:00:00Z".into(), @@ -542,28 +718,26 @@ mod tests { push_time: None, }; - // remaining=60s なのに sleep=600s 必要 → Err - let result = handle_rate_limit_retry(&rl, &mut state, &pr_info, 3, 60); - assert!(result.is_err()); - let err_msg = result.unwrap_err(); - assert!( - err_msg.contains("監視残り予算"), - "Err message に budget 不足の説明が含まれるべき: {}", - err_msg - ); - // state は変更されない (retries 0 のまま、last_retriggered_at 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()); } - /// Finding 3 (PR #97 round 3): PR 番号未確定の場合、Err を返し state を変更しない。 + /// Bb-1: PR 番号未確定の場合、`handle_rate_limit_retry` は Failed を返し + /// state を変更しない (caller は action_required で抜ける)。 #[test] - fn rate_limit_retry_returns_err_when_pr_number_missing() { + 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; // 1 分前 (sleep_secs=0 経路) + - 60; let rl = RateLimitState { until_unix_secs: past_unix, comment_event_time: "2026-04-30T00:00:00Z".into(), @@ -577,10 +751,63 @@ mod tests { push_time: None, }; - let result = handle_rate_limit_retry(&rl, &mut state, &pr_info, 3, 1000); - assert!(result.is_err()); - // state は変更されない + 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, + }; + + 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, + }; + + 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")); + } } diff --git a/src/cli-pr-monitor/src/state.rs b/src/cli-pr-monitor/src/state.rs index 359ab2bc..fd43d1c7 100644 --- a/src/cli-pr-monitor/src/state.rs +++ b/src/cli-pr-monitor/src/state.rs @@ -32,6 +32,20 @@ pub(crate) struct PrMonitorState { /// CR が新たな rate-limit comment を投稿したら event_time が変わり再度 retrigger 対象になる。 #[serde(default, skip_serializing_if = "Option::is_none")] pub(crate) rate_limit_last_retriggered_at: Option, + /// 次回 wakeup の予定時刻 (unix epoch 秒)。Bb-1 (Bundle b PR-1) で導入。 + /// + /// rate-limit 等で長時間待機が必要な場合、cli-pr-monitor は同プロセス内で sleep せず + /// この field に reset 時刻を保存して exit する。Claude Code 側が stdout の + /// PARK signal を読み、CronCreate (`durable: true`) で wakeup を予約する。 + /// wakeup 発火時に `cli-pr-monitor.exe --monitor-only` が再 invoke される。 + #[serde(default, skip_serializing_if = "Option::is_none")] + pub(crate) next_wakeup_at_unix: Option, + /// wakeup の理由ラベル (e.g. "rate_limit_retry"). Bb-1 で導入。 + /// + /// 将来 Bb-2 (review 完了待ち) や Bb-3 (SessionStart catch-up) で複数の wakeup + /// 経路を識別するための discriminator。 + #[serde(default, skip_serializing_if = "Option::is_none")] + pub(crate) wakeup_reason: Option, } /// check-ci-coderabbit から伝播する rate-limit 制御情報 @@ -91,6 +105,8 @@ impl PrMonitorState { rate_limit: None, rate_limit_retries: 0, rate_limit_last_retriggered_at: None, + next_wakeup_at_unix: None, + wakeup_reason: None, } } } @@ -214,6 +230,8 @@ mod tests { rate_limit: None, rate_limit_retries: 0, rate_limit_last_retriggered_at: None, + next_wakeup_at_unix: None, + wakeup_reason: None, }; let json = serde_json::to_string(&state).unwrap(); @@ -221,6 +239,41 @@ mod tests { assert_eq!(state, deserialized); } + #[test] + fn state_serialize_roundtrip_with_wakeup_fields() { + let mut state = + PrMonitorState::new(Some(42), Some("o/r".into()), "2026-05-05T12:00:00Z".into()); + state.next_wakeup_at_unix = Some(1_775_088_000); + state.wakeup_reason = Some("rate_limit_retry".into()); + + let json = serde_json::to_string(&state).unwrap(); + assert!(json.contains("next_wakeup_at_unix")); + assert!(json.contains("rate_limit_retry")); + + let deserialized: PrMonitorState = serde_json::from_str(&json).unwrap(); + assert_eq!(state, deserialized); + assert_eq!(deserialized.next_wakeup_at_unix, Some(1_775_088_000)); + assert_eq!( + deserialized.wakeup_reason.as_deref(), + Some("rate_limit_retry") + ); + } + + #[test] + fn state_omits_wakeup_fields_when_none() { + let state = PrMonitorState::new(Some(1), None, "t".into()); + let json = serde_json::to_string(&state).unwrap(); + assert!(!json.contains("next_wakeup_at_unix")); + assert!(!json.contains("wakeup_reason")); + } + + #[test] + fn state_default_wakeup_fields_are_none() { + let state = PrMonitorState::new(Some(1), None, "t".into()); + assert!(state.next_wakeup_at_unix.is_none()); + assert!(state.wakeup_reason.is_none()); + } + #[test] fn state_write_read_roundtrip() { let tmp = From 43df40dbccb00ec835dfac3b207c0cd451316879 Mon Sep 17 00:00:00 2001 From: aloekun Date: Tue, 5 May 2026 19:02:55 +0900 Subject: [PATCH 2/2] fix(review): apply CodeRabbit fixes for #113 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolved findings: - [Major] src/cli-pr-monitor/src/stages/poll.rs:351 state 永続化に失敗しても PARK を発行すると durable な再 park ループになります - [Major] src/cli-pr-monitor/src/stages/poll.rs:495 CronCreate prompt のコマンドは path をクォートしないと Windows で壊れます - [Major] src/cli-pr-monitor/src/stages/poll.rs:515 One-shot cron は ISO 8601 形式で秒精度をサポートするため、5 フィールド cron への変換を避ける --- src/cli-pr-monitor/src/stages/poll.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/cli-pr-monitor/src/stages/poll.rs b/src/cli-pr-monitor/src/stages/poll.rs index d43fc973..7031281c 100644 --- a/src/cli-pr-monitor/src/stages/poll.rs +++ b/src/cli-pr-monitor/src/stages/poll.rs @@ -345,7 +345,8 @@ fn finalize_parked( rl.wait_minutes, rl.wait_seconds ); if let Err(e) = write_state(state) { - log_info(&format!("[rate_limit] park state 永続化失敗: {}", e)); + let msg = format!("park state 永続化失敗のため PARK signal を中止 ({})。手動で `@coderabbitai review` を投稿してください", e); + return make_action_required_result(state, result, &msg); } let signal = format_park_signal(state, rl, pr_info, max_retries); println!("{}", signal); @@ -467,8 +468,8 @@ fn post_review_immediately(pr: u64, state: &mut PrMonitorState) -> RateLimitOutc /// /// Claude Code が stdout を読み、含まれる `CronCreate(...)` 呼び出しテンプレートに /// 従って `durable: true` で wakeup を予約する想定。`reset_at_iso_utc` は UTC な -/// ため、cron 式は Claude が local timezone に変換して計算する (chrono 依存を -/// 避けるため Rust 側では cron 式を組まない)。 +/// ため、CronCreate の cron 値は Claude が local timezone に変換した ISO 8601 形式で +/// 設定する (chrono 依存を避けるため Rust 側では変換しない)。 pub(crate) fn format_park_signal( state: &PrMonitorState, rl: &crate::state::RateLimitState, @@ -509,10 +510,10 @@ cwd: {cwd} ACTION REQUIRED: please schedule one-shot wakeup using CronCreate. CronCreate({{ - cron: , + cron: \"\", recurring: false, durable: true, - prompt: \"Wakeup: rate-limit retry for PR #{pr} ({repo}). cd {cwd} && {exe} --monitor-only\" + prompt: \"Wakeup: rate-limit retry for PR #{pr} ({repo}). cd \\\"{cwd}\\\" && \\\"{exe}\\\" --monitor-only\" }}) [/PR_MONITOR_PARK]", until = rl.until_unix_secs,