diff --git a/docs/todo-summary.md b/docs/todo-summary.md index 920a752f..3f9d6f9a 100644 --- a/docs/todo-summary.md +++ b/docs/todo-summary.md @@ -86,9 +86,6 @@ | 205 | 💎 Tier 3 | **`~/.claude/rules/common/git-workflow.md` に jj auto-snapshot onboarding rule 追記 (PR #201 post-merge-feedback T3-4 採用)** | todo10.md | XS | なし (PR #201 で prior session の docs commit 199-202 と本セッションの impl 146 が auto-snapshot で混入し bundle 化に収束した実観測、jj Operations section に「Auto-snapshot の理解と logical separation」sub-section 追加、`jj new -m` を **作業開始時** に実行する正しいフロー明文化、派生プロジェクトへ global 経由で自動波及、`feedback_global_config_backup` 適用必須) | | 206 | 💎 Tier 3 | **`~/.claude/rules/common/development-workflow.md` § 1. Plan First に「todo*.md 分割時の todo-summary.md 同一 commit 更新」checklist 追加 (PR #204 post-merge-feedback T3-1 採用)** | todo10.md | S | なし (PR #133 + #153 + #204 の 3 PR 連続観測で multi-file artifact split 時の永続 index 更新漏れが Frequency Medium 閾値到達、3 step checklist (分割エントリ列挙 / sed 一括 file 列更新 / 同一 commit) を § 1. Plan First の Codification 重複確認 step 直後に配置、coding-style.md § Cross-File Reference Lifecycle の具体化事例として cite、派生プロジェクト (techbook-ledger / auto-review-fix-vc) へ global 経由で自動波及、`feedback_global_config_backup` 適用必須) | | 207 | 💎 Tier 3 | **`~/.claude/rules/common/patterns.md` § Experimental Feature 設計時の参照必須 に「mechanical lint は ADR-039 scope 外」境界 case 追加 (PR #204 post-merge-feedback T3-2 採用)** | todo10.md | S | なし (PR #204 で project-local adr-039 § 1.b 追加した知見の global 投影、派生プロジェクトでの同型 over-application 防止、4 条件 (non-blocking / 決定論 / scope 限定 / recovery hint 明確) + 該当例 (file-length / file-size lint) と非該当例 (post-merge-feedback / weekly-review / local-llm) を境界 case として明示、順位 200/202/205 と同 pattern = project-local 知見の global codification、`feedback_global_config_backup` 適用必須) | -| 208 | 🔧 Tier 2 | **`cli-pr-monitor` の CR walkthrough body parse で "No actionable comments" 検出 (PR #210 post-merge-feedback T1-1 採用)** | todo10.md | S | なし (PR #210 セッションで CR clean 状態が walkthrough body にのみ書かれ formal Review object 不在 → monitor の `parse_actionable_comments` が None を返し recheck loop に入る事象を実観測、memory `feedback_coderabbit_no_actionable_merge_signal.md` の手動 workaround を機械化、`decide()` に新分岐 + 3 fixture test (clean / rate-limit overlay / formal review) で defense in depth、Effort S + Adoption Risk None) | -| 209 | 🔧 Tier 2 | **PARK signal 出力を分単位 round-UP に変更 — Cron 秒解像度ミスマッチ修正 (PR #210 post-merge-feedback T2-1 採用)** | todo10.md | S | なし (PR #210 セッションで cron timing race を実観測 = monitor の秒解像度 `next_wakeup_at_unix` を AI agent が round-DOWN cron spec に変換 → state.next_wakeup 未到達で fresh path 再ループ、PARK signal に `next_wakeup_safe_minute_local` + `cron_spec_recommended` field 追加で構造的防御、post-pr-monitor が動くすべての PR で再発しうる horizontal リスク、順位 210 (rule codify) と相補) | -| 210 | 💎 Tier 3 | **`~/.claude/rules/common/development-workflow.md` + MEMORY.md に「Cron 秒 → 分 round-UP」ルール codify (PR #210 post-merge-feedback T3-1 採用)** | todo10.md | XS | なし (PR #210 セッションの cron timing race 即効ワークアラウンドを global rule + memory file として永続化、順位 209 (signal 改善) land までのブリッジ + land 後の補完層、ScheduleWakeup や手動 cron でも横展開可能、`~/.claude/rules/common/` 配下のため techbook-ledger / auto-review-fix-vc に自動波及、`feedback_global_config_backup` 適用必須) | | 211 | 💎 Tier 3 | **`~/.claude/rules/common/testing.md` に「単複・閾値・時制で出力形式が変わる関数は N=0 / N=1 / N≥2 の 3 境界 variant 必須」guideline 追加 (PR #210 post-merge-feedback T3-2 採用)** | todo10.md | XS | なし (PR #210 で `drain_pipe_capped_reporting_n_plus_1_truncates_one_appends_summary` が当初 `"1 lines truncated"` 誤期待値で takt-fix auto-fix された実観測、N-1/N/N+1 境界値と直交する「N=1 単複境界 + ゼロ近傍」次元の guideline 化、順位 110 pure function test pattern と相補、派生プロジェクトに global 経由で自動波及、`feedback_global_config_backup` 適用必須) | **戦略**: Tier 1 を 2〜3 セッションで片付け → Tier 2 で ADR-032 の前提 + rate-limit + convergence cost 削減を進める → Tier 3 で ADR-032 を land + ドキュメント整備。Tier 4-5 は cleanup / 外部展開で daily efficiency への直接効果は小さい。 diff --git a/docs/todo10.md b/docs/todo10.md index 431ae672..678385e7 100644 --- a/docs/todo10.md +++ b/docs/todo10.md @@ -432,122 +432,6 @@ ADR-039 (Experimental Feature 標準パターン) は「behavior の妥当性が --- -### `cli-pr-monitor` の CR walkthrough body parse で "No actionable comments" 検出 (PR #210 post-merge-feedback T1-1 採用) - -> **動機**: PR #210 セッションで実観測した検出 gap = CodeRabbit が "No actionable comments were generated in the recent review. 🎉" を walkthrough comment body にのみ出力し、formal Review object を投稿しない場合、monitor の `parse_actionable_comments` ([src/check-ci-coderabbit/src/main.rs:825-852](../src/check-ci-coderabbit/src/main.rs#L825-L852)) は reviews 配列が空のため None を返し、`coderabbit = null` のまま recheck loop に入る。memory `feedback_coderabbit_no_actionable_merge_signal.md` で「AI 判断で walkthrough body を手動確認」と手当てしていたが、機械化可能。 -> -> **本タスクの位置づけ**: PR #210 post-merge-feedback Tier 1 #1 採用 (Severity Medium / Frequency Medium / Effort S / Adoption Risk None、2026-06-16 ユーザー承認)。analyzer rationale: 「memory file が既に存在 = 過去 PR でも同症状。Effort S かつ Adoption Risk None のため採用候補。現状のワークアラウンド (手動 body 確認) を機械化できる」。 -> -> **参照**: `.claude/feedback-reports/210.md` Tier 1 #1、memory `feedback_coderabbit_no_actionable_merge_signal.md`、PR #210 セッションログ (12:00-12:13 JST の wakeup loop 観測)。 - -#### 設計決定 (案) - -- **検出対象**: PR の `issues/N/comments` から CodeRabbit walkthrough comment (= `coderabbitai[bot]` 投稿 + body 先頭が ``) を取得し、body 内 substring `"No actionable comments were generated in the recent review."` を検出 -- **判定統合**: `check-ci-coderabbit` の `decide()` (`src/check-ci-coderabbit/src/main.rs:881`) で `cr.review_state == "not_found"` かつ walkthrough body に clean marker 検出時は新 review_state `"clean"` で `(success, stop_monitoring_success)` を返す -- **rate-limit overlay との整合**: memory `project_coderabbit_rate_limit_overlay` の通り walkthrough body は rate-limit 中も書き換わるため、clean marker と rate-limit marker の **両方** を check し前者を優先する設計 -- **fixture test**: 3 variant fixture を testing.md pattern で追加 (clean marker あり / rate-limit overlay 状態 / 通常 review 投稿状態) - -#### 作業計画 - -- [ ] `check-ci-coderabbit` に `parse_walkthrough_clean_marker(comments_json) -> bool` helper 追加 -- [ ] `decide()` に「review_state == "not_found" && walkthrough_clean」分岐追加で `(success, stop_monitoring_success)` -- [ ] 3 fixture test 追加 (memory `feedback_test_dry_antipattern` 準拠で独立 setup) -- [ ] `cli-pr-monitor` の poll/monitor stage で新分岐の statename を park signal summary に伝播 -- [ ] 本エントリ削除 + todo-summary.md 行削除 - -#### 完了基準 - -- monitor が CR clean 状態を walkthrough body から自動検出し、recheck loop を終了 -- memory `feedback_coderabbit_no_actionable_merge_signal.md` を「機械化済 (順位 208 land)」として更新 or 削除 -- 3 fixture test (clean / rate-limit / formal review) が独立 setup で pass - -#### 詰まっている箇所 - -なし。Effort S、既存 `decide()` の分岐追加 + fixture test、PR diff < 200 行見込み。 - ---- - -### PARK signal 出力を分単位 round-UP に変更 — Cron 秒解像度ミスマッチ修正 (PR #210 post-merge-feedback T2-1 採用) - -> **動機**: PR #210 セッションで実観測した cron timing race = monitor が `next_wakeup_at_unix` を **秒解像度** で出力 (例: `2026-06-16T03:03:19Z`) するが、CronCreate は **5-field 分解像度** + 最大 30s jitter という制約。Claude / AI agent が `12:03:19` を round-DOWN して `cron: "3 12"` と指定すると 12:03:00 ± jitter で発火 → state.next_wakeup_at_unix (12:03:19) より早期 → `should_resume_wakeup` ([src/cli-pr-monitor/src/stages/monitor.rs:253-267](../src/cli-pr-monitor/src/stages/monitor.rs#L253-L267)) が `false` → fresh path → recheck_count 0 のまま空回り。本 PR では 2 回連続の無駄 wakeup が発生した。 -> -> **本タスクの位置づけ**: PR #210 post-merge-feedback Tier 2 #1 採用 (Severity Medium / Frequency Medium / Effort S / Adoption Risk None、2026-06-16 ユーザー承認)。analyzer rationale: 「CronCreate cron は分単位 + 最大 30s jitter という制約を Signal 側が明示していなかったことが根本原因。post-pr-monitor が動くすべての PR で再発しうる」。 -> -> **参照**: `.claude/feedback-reports/210.md` Tier 2 #1、PR #210 セッションログ (12:00-12:13 JST、2 回の無駄 wakeup 実観測)、順位 210 (round-UP rule codify) と相補。 - -#### 設計決定 (案) - -- **PARK signal format 変更**: 既存 - ```text - next_wakeup_at_unix: 1781578999 - next_wakeup_at_iso_utc: 2026-06-16T03:03:19Z - ``` - に加えて新 field を追加: - ```text - next_wakeup_safe_minute_local: 2026-06-16T12:04 (= round-UP to next full minute) - cron_spec_recommended: "4 12 16 6 *" - ``` -- **round-UP ロジック**: `next_wakeup_at_unix` を local timezone の `HH:MM` に変換し、秒部分が `00` でなければ次の分にインクリメント。`00` ちょうどなら現分のまま (jitter -90s 前倒し制約があるが許容範囲) -- **PARK signal の指示文も更新**: `ACTION REQUIRED` block の例示を round-UP 後の値で書き換え (現在は「``」と曖昧) -- **後方互換**: 既存 field は維持、新 field は追加のみ。AI agent が新 field を優先使用する想定 - -#### 作業計画 - -- [ ] `cli-pr-monitor` の PARK signal 生成箇所 (`format_review_park_signal` 等) を特定し新 field 追加 -- [ ] `next_minute_round_up(unix_seconds) -> (year, month, day, hour, minute)` pure helper 関数を切り出し test -- [ ] PARK signal の "ACTION REQUIRED" 例示文を「`cron_spec_recommended` を直接コピペで使用してください」に書き換え -- [ ] 統合 test で 03:03:19Z → "12:04 JST" 変換、03:03:00Z → "12:03 JST" 維持を検証 -- [ ] 本エントリ削除 + todo-summary.md 行削除 - -#### 完了基準 - -- AI agent が PARK signal から直接 cron spec を取得でき、round-DOWN 罠が構造的に防げる -- timezone 跨ぎ + 月跨ぎ + うるう年含む test が pass -- 順位 210 (round-UP rule codify) との重複は補完 (signal 側 + rule 側の二重防御) を許容 - -#### 詰まっている箇所 - -なし。Effort S、pure 変換 helper + signal format 追加のみ、PR diff < 150 行見込み。 - ---- - -### `~/.claude/rules/common/development-workflow.md` + MEMORY.md に「Cron 秒 → 分 round-UP」ルール codify (PR #210 post-merge-feedback T3-1 採用) - -> **動機**: PR #210 セッションで実観測した cron timing race の **即効ワークアラウンド** を rule として codify。順位 209 (PARK signal 改善) が land するまでのブリッジ、land 後も補完ルールとして残る。秒単位 timestamp から cron spec を生成する場面は post-pr-monitor だけでなく ScheduleWakeup や手動 cron 設定でも発生しうるため、global rule として横展開価値あり。 -> -> **本タスクの位置づけ**: PR #210 post-merge-feedback Tier 3 #1 採用 (Severity Medium / Frequency Medium / Effort XS / Adoption Risk None、2026-06-16 ユーザー承認)。analyzer rationale: 「Cron タイミング競合の即効ワークアラウンド。T2-1 (signal 出力改善) が採用されるまでのブリッジルールとして機能し、採用後は補完ルールとして残る」。 -> -> **参照**: `.claude/feedback-reports/210.md` Tier 3 #1、PR #210 セッションログ、順位 209 (PARK signal round-UP) と相補。 - -#### 設計決定 (案) - -- **追加先 1**: `~/.claude/rules/common/development-workflow.md` § 背景タスクの待機方針 (polling 禁止) の直後に新 sub-section「Cron スケジューリングの秒 → 分 round-UP」を追加 -- **追加先 2**: `MEMORY.md` の Feedback section に `feedback_cron_round_up_for_second_timestamps.md` ポインター 1 行を追加 -- **rule 内容例**: 「秒単位 unix timestamp / ISO 8601 を CronCreate の `cron` field に変換するときは、秒部分が `00` でなければ **次の完全な分** に round-UP する。例: `12:03:19` → `cron: "4 12 ..."` (`"3 12"` は早期発火で race 条件発生)」 -- **由来 cite**: PR #210 セッション (2026-06-16) の実観測 (2 回の無駄 wakeup) -- **派生プロジェクト波及**: `~/.claude/rules/common/` 配下のため techbook-ledger / auto-review-fix-vc に自動 - -#### 作業計画 - -- [ ] `~/.claude/` snapshot 取得 (memory `feedback_global_config_backup` per) -- [ ] `~/.claude/rules/common/development-workflow.md` に新 sub-section 追記 (10 行程度) -- [ ] `feedback_cron_round_up_for_second_timestamps.md` 作成 (memory file) -- [ ] `MEMORY.md` Feedback section に 1 行ポインター追加 -- [ ] markdownlint clean -- [ ] 本エントリ削除 + todo-summary.md 行削除 - -#### 完了基準 - -- AI agent が秒単位 timestamp から cron spec を生成する際、round-UP ルールが MEMORY.md / development-workflow.md 双方で参照可能 -- 派生プロジェクト (techbook-ledger / auto-review-fix-vc) に global rule として自動波及 -- 順位 209 (PARK signal 改善) が land したら本 rule を「補完層 (signal 側が一次防御)」と位置付け更新 - -#### 詰まっている箇所 - -なし。Effort XS、global rules + memory への docs 追記のみ、`feedback_global_config_backup` snapshot を忘れない。 - ---- - ### `~/.claude/rules/common/testing.md` に「単複・閾値・時制で出力形式が変わる関数は N=0 / N=1 / N≥2 の 3 境界 variant 必須」guideline 追加 (PR #210 post-merge-feedback T3-2 採用) > **動機**: PR #210 で `drain_pipe_capped_reporting_n_plus_1_truncates_one_appends_summary` test が当初 `"1 lines truncated"` (= 単数で複数形を使用) を期待値として誤って書いてしまい、takt-fix iter 1 → iter 2 で auto-fix された実観測。境界値テスト (N=N+1) を書いたが「N=1 のとき出力形式が変わる」 (single → no `s`) という単複境界を忘れた。一般化すれば「数値に応じて出力形式が変化する関数」全般に共通する盲点。 diff --git a/src/check-ci-coderabbit/src/main.rs b/src/check-ci-coderabbit/src/main.rs index df704163..6f7896be 100644 --- a/src/check-ci-coderabbit/src/main.rs +++ b/src/check-ci-coderabbit/src/main.rs @@ -189,6 +189,11 @@ struct CodeRabbitStatus { new_comments: usize, actionable_comments: Option, unresolved_threads: Option, + /// 順位 208: CR walkthrough comment body に `WALKTHROUGH_CLEAN_MARKER` + /// (= "No actionable comments were generated in the recent review.") を検出した場合 true。 + /// formal Review object が無い場合でも clean 判定を可能にし、recheck loop を終了させる。 + #[serde(default)] + walkthrough_clean: bool, } // ─── gh CLI 出力パースモデル ─── @@ -260,6 +265,15 @@ struct GhPullComment { /// - "rate limited by coderabbit.ai" (新 format、HTML マーカー、2026-05 観測) const RATE_LIMIT_MARKERS: &[&str] = &["Rate limit exceeded", "rate limited by coderabbit.ai"]; +/// 順位 208: CR walkthrough comment が clean 判定を示すときに body 内に出力する marker。 +const WALKTHROUGH_CLEAN_MARKER: &str = + "No actionable comments were generated in the recent review."; + +/// 順位 208: CR walkthrough comment body の先頭 marker。auto-generated header の +/// 識別に使い、PR ユーザーが手動投稿した類似コメントを誤検出しないようにする。 +const WALKTHROUGH_HEADER_MARKER: &str = + ""; + fn is_rate_limit_comment(c: &GhComment) -> bool { c.body .as_deref() @@ -396,6 +410,59 @@ fn parse_new_comments(json: &str, push_time: &str) -> usize { .count() } +/// 順位 208: CR walkthrough comment body から clean marker +/// (= `WALKTHROUGH_CLEAN_MARKER`) を検出する。 +/// +/// 検出条件 (全て満たすときのみ true): +/// - 投稿者が `coderabbitai[bot]` +/// - body 先頭付近に `WALKTHROUGH_HEADER_MARKER` を含む (= auto-generated walkthrough header) +/// - body が rate-limit comment ではない (rate-limit overlay 時は body が書き換わるため +/// project memory `project_coderabbit_rate_limit_overlay.md` 参照) +/// - body に `WALKTHROUGH_CLEAN_MARKER` を含む +/// - event_time (= updated_at fallback created_at) が `push_time` 以降 +/// +/// formal Review object が無い (= `review_state == "not_found"`) 場合でも本シグナルが +/// true なら `decide()` で `(complete, stop_monitoring_success)` を返し、PR #210/#211 で +/// 発生した recheck loop を構造的に終了させる (= memory `feedback_coderabbit_no_actionable_merge_signal.md` +/// の手動 workaround を機械化)。 +fn parse_walkthrough_clean_marker(json: &str, push_time: &str) -> bool { + let comments: Vec = serde_json::from_str(json).unwrap_or_else(|e| { + eprintln!( + "[check-ci-coderabbit] walkthrough JSON パースエラー: {}", + e + ); + vec![] + }); + + comments.iter().any(|c| is_clean_walkthrough_comment(c, push_time)) +} + +/// 順位 208: 単一 comment が CR walkthrough の clean marker を持つか判定する pure helper。 +/// `parse_walkthrough_clean_marker` を decompose し、`collect_review_park_fields` 等の +/// 関数長 50 行ガイドライン (順位 48) を維持する。 +fn is_clean_walkthrough_comment(c: &GhComment, push_time: &str) -> bool { + let is_coderabbit = c + .user + .as_ref() + .and_then(|u| u.login.as_deref()) + .map(|l| l == "coderabbitai[bot]") + .unwrap_or(false); + if !is_coderabbit { + return false; + } + let after_push_time = rate_limit_event_time(c) + .map(|t| t >= push_time) + .unwrap_or(false); + if !after_push_time { + return false; + } + if is_rate_limit_comment(c) { + return false; + } + let body = c.body.as_deref().unwrap_or(""); + body.contains(WALKTHROUGH_HEADER_MARKER) && body.contains(WALKTHROUGH_CLEAN_MARKER) +} + /// rate-limit comment の reset 計算に使うタイムスタンプを返す。 /// /// `updated_at` (CR が wait 時間を更新した編集時刻) を優先し、未設定なら `created_at`。 @@ -877,14 +944,21 @@ fn parse_unresolved_threads(json: &str) -> Option { // ─── 判定ロジック ─── -/// CI と CodeRabbit の状態から (status, action) を決定する +/// CI と CodeRabbit の状態から `(status, action)` を決定する。 +/// +/// 判定優先順位 (上から): +/// 1. CI failure → error / stop_monitoring_failure +/// 2. walkthrough_clean かつ unresolved_threads 無し → complete / stop_monitoring_success +/// (順位 208: formal Review object 不在でも walkthrough body の clean marker を信頼) +/// 3. review_state == not_found かつ has_actionable → action_required +/// 4. CI pending or CR pending → continue_monitoring +/// 5. review_state failure/error → stop_monitoring_failure +/// 6. has_actionable → action_required +/// 7. それ以外 → complete / stop_monitoring_success fn decide(ci: &CiStatus, cr: &CodeRabbitStatus) -> (String, String) { - // CI が失敗 → 即座に報告 if ci.overall == "failure" { return ("error".to_string(), "stop_monitoring_failure".to_string()); } - - // コメント/スレッドの集計 (review_state に関わらず先に計算) let has_unresolved = cr.unresolved_threads.map(|n| n > 0).unwrap_or(false); let effective_new = if let Some(actionable) = cr.actionable_comments { std::cmp::max(cr.new_comments, actionable) @@ -892,33 +966,26 @@ fn decide(ci: &CiStatus, cr: &CodeRabbitStatus) -> (String, String) { cr.new_comments }; let has_actionable = effective_new > 0 || has_unresolved; - - // CodeRabbit の review_state が not_found でもコメント/スレッドがあれば対応が必要 - // (commit status は未投稿でも inline comments は先に投稿されるケースがある) + if cr.walkthrough_clean && !has_unresolved { + return ( + "complete".to_string(), + "stop_monitoring_success".to_string(), + ); + } if cr.review_state == "not_found" && has_actionable { return ("action_required".to_string(), "action_required".to_string()); } - - // CI が pending (runs 空 = no_ci は "pending" ではなく CI チェックをスキップ) let ci_pending = ci.overall == "pending" && !ci.runs.is_empty(); - // CodeRabbit がまだレビュー中 or 未検出 (コメントもない) let cr_pending = cr.review_state == "pending" || cr.review_state == "not_found"; - if ci_pending || cr_pending { return ("pending".to_string(), "continue_monitoring".to_string()); } - - // CodeRabbit がエラー if cr.review_state == "failure" || cr.review_state == "error" { return ("error".to_string(), "stop_monitoring_failure".to_string()); } - - // コメント/スレッドがある → 対応が必要 if has_actionable { return ("action_required".to_string(), "action_required".to_string()); } - - // すべて OK ( "complete".to_string(), "stop_monitoring_success".to_string(), @@ -1138,6 +1205,7 @@ fn run_check(args: CliArgs) -> CheckResult { let comments_json = run_gh(&["api", &format!("repos/{}/issues/{}/comments", repo, pr_str)]) .unwrap_or_else(|_| "[]".to_string()); let new_comments = parse_new_comments(&comments_json, &args.push_time); + let walkthrough_clean = parse_walkthrough_clean_marker(&comments_json, &args.push_time); let rate_limit = parse_rate_limit(&comments_json, &args.push_time); // 4. Actionable comments クロスチェック @@ -1167,6 +1235,7 @@ fn run_check(args: CliArgs) -> CheckResult { new_comments, actionable_comments: actionable, unresolved_threads: unresolved, + walkthrough_clean, }; // 6. インラインレビューコメントを Finding に変換 @@ -1563,7 +1632,83 @@ mod tests { assert_eq!(parse_coderabbit_status(json), "success"); } - // --- parse_new_comments --- + #[test] + fn walkthrough_clean_detected_when_marker_present_with_header() { + let json = r#"[ + {"user": {"login": "coderabbitai[bot]"}, + "body": "\nNo actionable comments were generated in the recent review. 🎉", + "created_at": "2026-04-01T12:30:00Z"} + ]"#; + assert!(parse_walkthrough_clean_marker( + json, + "2026-04-01T12:00:00Z" + )); + } + + #[test] + fn walkthrough_clean_skipped_when_rate_limit_overlay_present() { + let json = r#"[ + {"user": {"login": "coderabbitai[bot]"}, + "body": "\nrate limited by coderabbit.ai\nMore reviews will be available in 1 minute and 30 seconds.\nNo actionable comments were generated in the recent review.", + "created_at": "2026-04-01T12:30:00Z"} + ]"#; + assert!(!parse_walkthrough_clean_marker( + json, + "2026-04-01T12:00:00Z" + )); + } + + #[test] + fn walkthrough_clean_skipped_when_marker_missing() { + let json = r#"[ + {"user": {"login": "coderabbitai[bot]"}, + "body": "\nReview summary: 3 changes detected.\n## Walkthrough\n...", + "created_at": "2026-04-01T12:30:00Z"} + ]"#; + assert!(!parse_walkthrough_clean_marker( + json, + "2026-04-01T12:00:00Z" + )); + } + + #[test] + fn walkthrough_clean_skipped_when_header_missing_to_avoid_user_post_false_positive() { + let json = r#"[ + {"user": {"login": "humanreviewer"}, + "body": "Quoting CR: No actionable comments were generated in the recent review.", + "created_at": "2026-04-01T12:30:00Z"} + ]"#; + assert!(!parse_walkthrough_clean_marker( + json, + "2026-04-01T12:00:00Z" + )); + } + + #[test] + fn walkthrough_clean_skipped_when_coderabbitai_post_lacks_header_marker() { + let json = r#"[ + {"user": {"login": "coderabbitai[bot]"}, + "body": "Plain text without header.\nNo actionable comments were generated in the recent review.", + "created_at": "2026-04-01T12:30:00Z"} + ]"#; + assert!(!parse_walkthrough_clean_marker( + json, + "2026-04-01T12:00:00Z" + )); + } + + #[test] + fn walkthrough_clean_skipped_when_event_time_before_push_time() { + let json = r#"[ + {"user": {"login": "coderabbitai[bot]"}, + "body": "\nNo actionable comments were generated in the recent review.", + "created_at": "2026-04-01T11:00:00Z"} + ]"#; + assert!(!parse_walkthrough_clean_marker( + json, + "2026-04-01T12:00:00Z" + )); + } #[test] fn comments_filters_by_time() { @@ -1804,6 +1949,7 @@ mod tests { new_comments: 2, actionable_comments: None, unresolved_threads: Some(0), + walkthrough_clean: false, }; let (status, action) = decide(&ci, &cr); assert_eq!(status, "action_required"); @@ -1821,6 +1967,7 @@ mod tests { new_comments: 0, actionable_comments: None, unresolved_threads: Some(3), + walkthrough_clean: false, }; let (status, action) = decide(&ci, &cr); assert_eq!(status, "action_required"); @@ -1838,6 +1985,7 @@ mod tests { new_comments: 0, actionable_comments: Some(3), // レビュー本文では3件、コメントAPIでは0件 unresolved_threads: Some(0), + walkthrough_clean: false, }; let (status, action) = decide(&ci, &cr); assert_eq!(status, "action_required"); @@ -1855,6 +2003,7 @@ mod tests { new_comments: 0, actionable_comments: Some(0), unresolved_threads: Some(0), + walkthrough_clean: false, }; let (status, action) = decide(&ci, &cr); assert_eq!(status, "complete"); @@ -1888,6 +2037,7 @@ mod tests { new_comments: 0, actionable_comments: Some(3), unresolved_threads: Some(3), + walkthrough_clean: false, }; let (status, action) = decide(&ci, &cr); assert_eq!(status, "action_required"); @@ -1906,6 +2056,7 @@ mod tests { new_comments: 0, actionable_comments: Some(0), unresolved_threads: Some(0), + walkthrough_clean: false, }; let (status, action) = decide(&ci, &cr); assert_eq!(status, "complete"); @@ -1928,7 +2079,41 @@ mod tests { assert_eq!(action, "continue_monitoring"); } - // --- build_summary --- + #[test] + fn decide_walkthrough_clean_returns_complete_when_no_unresolved_threads() { + let ci = CiStatus { + overall: "success".to_string(), + runs: vec![], + }; + let cr = CodeRabbitStatus { + review_state: "not_found".to_string(), + new_comments: 1, + walkthrough_clean: true, + unresolved_threads: Some(0), + ..Default::default() + }; + let (status, action) = decide(&ci, &cr); + assert_eq!(status, "complete"); + assert_eq!(action, "stop_monitoring_success"); + } + + #[test] + fn decide_walkthrough_clean_does_not_override_unresolved_threads() { + let ci = CiStatus { + overall: "success".to_string(), + runs: vec![], + }; + let cr = CodeRabbitStatus { + review_state: "not_found".to_string(), + new_comments: 1, + walkthrough_clean: true, + unresolved_threads: Some(2), + ..Default::default() + }; + let (status, action) = decide(&ci, &cr); + assert_eq!(status, "action_required"); + assert_eq!(action, "action_required"); + } #[test] fn summary_all_clean() { @@ -1941,6 +2126,7 @@ mod tests { new_comments: 0, actionable_comments: Some(0), unresolved_threads: Some(0), + walkthrough_clean: false, }; let summary = build_summary(&ci, &cr); assert!(summary.contains("CI成功")); @@ -1973,6 +2159,7 @@ mod tests { new_comments: 2, actionable_comments: Some(3), unresolved_threads: Some(1), + walkthrough_clean: false, }; let summary = build_summary(&ci, &cr); assert!(summary.contains("新規指摘3件")); diff --git a/src/cli-pr-monitor/src/stages/poll.rs b/src/cli-pr-monitor/src/stages/poll.rs index 1d26cb53..88856ca4 100644 --- a/src/cli-pr-monitor/src/stages/poll.rs +++ b/src/cli-pr-monitor/src/stages/poll.rs @@ -724,7 +724,46 @@ CronCreate({{ ) } +/// 順位 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, @@ -752,6 +791,8 @@ fn collect_review_park_fields( } 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) @@ -765,6 +806,8 @@ fn collect_review_park_fields( .unwrap_or_else(|_| ".".into()); ReviewParkSignalFields { + safe_minute_unix, + safe_minute_iso_utc, pr, repo, wakeup_unix, @@ -791,6 +834,8 @@ 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} @@ -799,8 +844,17 @@ 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: \"\", + cron: \"\", recurring: false, durable: true, prompt: \"Wakeup: review recheck for PR #{pr} ({repo}). cd \\\"{cwd}\\\" && \\\"{exe}\\\" --monitor-only\" @@ -810,6 +864,8 @@ CronCreate({{ 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, @@ -1481,6 +1537,108 @@ mod tests { ); } + #[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 する。