diff --git a/.claude/hooks-config.toml b/.claude/hooks-config.toml index 44702cb9..89432258 100644 --- a/.claude/hooks-config.toml +++ b/.claude/hooks-config.toml @@ -328,24 +328,9 @@ name = "post_merge_feedback" type = "ai" prompt = "post-merge-feedback" -# ─── Post-PR Monitor: daemon + state file ─── -# -# pnpm create-pr / pnpm push 実行時に daemon がバックグラウンドで起動し、 -# check-ci-coderabbit.exe を定期ポーリングして結果を -# .claude/pr-monitor-state.json に書き出す。 -# Claude は CronCreate で state file を cat し、action に従って行動する。 -# CronCreate は UX 最適化レイヤーであり、コア機能ではない。 -# フォールバック: cat .claude/pr-monitor-state.json で手動確認可能。 - -[post_pr_monitor] -enabled = true -poll_interval_secs = 120 # CronCreate のポーリング間隔（秒、2分） -max_duration_secs = 600 # 最大監視時間（秒、10分） -check_ci = true # GitHub Actions の監視 -check_coderabbit = true # CodeRabbit レビューの監視 -# trigger_patterns をコメントアウトまたは未設定 → デフォルトトリガー有効 -# trigger_patterns = ["gh pr create", "git push", "jj git push", "pnpm push"] # 明示指定する場合 -# trigger_patterns = [] # 空配列 = 全トリガー無効化（モニタリング停止） +# (旧 [post_pr_monitor] セクションは WP-17 PR 3 で撤去 — daemon + CronCreate ポーリング +# 時代 (ADR-009) の残骸で、どのコードも読んでいなかった。現行の PR 監視設定は +# リポジトリルートの pr-monitor-config.toml が正) # ─── 発火テレメトリ収集層 (WP-12 step 1、ADR-055、試験運用) ─── # @@ -402,8 +387,10 @@ trend_months = 6 # レポートが遡る月数 (月別表 / 発火 # hooks-stop-tool-call-leak/prompt-recovery = leak 回収 UserPromptSubmit 経路 (warn、ADR-061) # jj-op-verify = hooks-post-tool-jj-op-verify (operation 未記録 warn) # hooks-stop-quality = Stop 品質ゲート (block) -# pr_monitor_catchup / reaper / staleness / workspace_stale / weekly_review_reminder / +# reaper / staleness / workspace_stale / weekly_review_reminder / # monthly_review_reminder = hooks-session-start の nudge 群 (warn) +# (pr_monitor_catchup は WP-17 PR 3 で nudge ごと撤去 — 過去の firings jsonl には残るが +# 集計対象の現役 id ではない) [telemetry_report.registry] hook_ids = [ "file-length", @@ -411,7 +398,6 @@ hook_ids = [ "hooks-stop-tool-call-leak/prompt-recovery", "jj-op-verify", "hooks-stop-quality", - "pr_monitor_catchup", "reaper", "staleness", "workspace_stale", diff --git a/docs/adr/adr-018-pr-monitor-takt-migration.md b/docs/adr/adr-018-pr-monitor-takt-migration.md index 6ddcbfaf..18341e20 100644 --- a/docs/adr/adr-018-pr-monitor-takt-migration.md +++ b/docs/adr/adr-018-pr-monitor-takt-migration.md @@ -207,3 +207,49 @@ PR #120 (cli-finding-classifier 統合) の dogfood で観測された **「rate - PR #120 (Phase 5 land): rate-limit auto-retry の Posted 経路 silent exit 観測の根拠 - 本 PR (順位 80 fix + 順位 82 ADR update + §A-2 P-4 ledger): 本 ADR 追記の land + +## 追記 (2026-08-03): park モデルの廃止 — single-shot 化 (WP-17 PR 3) + +### 決定 + +追記 (2026-05-06) の Bundle b で再導入した CronCreate park モデルを廃止し、cli-pr-monitor を +**single-shot モデル** (1 回 check して必ず terminal 報告で終了) へ移行する。 + +- 未確定 (review 未完) は terminal `pending_review`、rate-limit reset 待ちは terminal + `rate_limited` として報告する。どちらも「後続は GitHub Actions 経路が処理」を明示する。 +- state の wakeup fields (`next_wakeup_at_unix` / `wakeup_reason` / `review_recheck_count`)、 + `[PR_MONITOR_PARK]` envelope、`[review_recheck]` config を撤去。旧 state / config は + unknown field として無視される (前方互換テストで固定)。 +- hooks-session-start の pr_monitor catch-up nudge (Bb-3) も撤去 — 「park 失効の救済」という + 役割ごと dead code になった。 +- **維持したもの**: 同一 PR + 同一 head での state 継続 (`should_continue_state`)。これは + park の付随物ではなく時刻窓アンカー (`--push-time` の基準) であり、落とすと再実行のたびに + 新着判定窓がリセットされる。rate-limit の retry 上限 / comment dedup も同様に維持。 + +### 根拠 + +1. **wakeup はローカルセッションの寿命に依存する**。PR #237 でセッション終了による + CronCreate 失効 = 監視の取りこぼしを実観測した (Bb-3 の catch-up nudge はその症状への + 救済層であって、根治ではなかった)。 +2. **GitHub Actions 経路が常設になった** (WP-17、[ADR-067](adr-067-phase-b-unattended-fix-push.md))。 + CodeRabbit のレビュー到着・後続コメント (レート制限中の再開を含む) がそのままイベント + トリガーになるため、ローカルの時限 wakeup は冗長。 +3. 常時性の担保を「ローカルの工夫 (park + catch-up)」から「常設インフラ (Actions)」へ移すのは + ハーネス改善計画 (2) 自律実行の常時性の主旨そのもの。 + +### ADR-064 検証残の移し替え (計画書の着手前決定 2) + +- (a) 「監視が success で終わらず park すること」の実観測 → **moot として閉じる** + (park 機構ごと消滅)。single-shot の同等保証は `rate_limited` / `pending_review` の + terminal 報告で、unit test により固定済み。 +- (b) 「レポート判定文が保留を出すこと」→ ローカル側は `verdict_for_unsettled_review` の + 判定文として維持 (テスト固定済み)。**GitHub Actions 経路での同等保証は同経路の検証残** + として引き継ぐ (ADR-064 ステータス欄も同旨に更新)。 + +### 影響 + +- `pnpm push` / `pnpm create-pr` チェイン末尾の監視は「その時点の状態報告」になる。レビュー + 未着なら pending_review で終わり、後続はユーザーの手動再実行 (`--monitor-only`) か + Actions 経路のコメントで把握する。 +- 本 ADR の追記 (2026-05-06) の park 設計・順位 80 fix (PARK signal 保証) は歴史記録として + 残す。現行実装の正は本追記。 diff --git a/docs/adr/adr-034-coderabbit-auto-monitoring.md b/docs/adr/adr-034-coderabbit-auto-monitoring.md index 63efa9b2..7817d4f1 100644 --- a/docs/adr/adr-034-coderabbit-auto-monitoring.md +++ b/docs/adr/adr-034-coderabbit-auto-monitoring.md @@ -215,6 +215,8 @@ Bundle b (Bb-1/Bb-2/Bb-3、PR #113-115、2026-05-05〜06 land) は本 ADR とは ### Sub-PR 2 の構成変化 (Bundle b land 後) +> **2026-08-03 注記 (WP-17 PR 3)**: 下表の CronCreate park モデルは [ADR-018](adr-018-pr-monitor-takt-migration.md) 追記 (2026-08-03) で**廃止済み** (single-shot 化)。rate-limit の即時 retrigger と dedup は single-shot モデルでも維持されているが、park / wakeup を前提とする記述は歴史記録として読むこと。 + | component | 旧 (本 ADR 当初) | 新 (Bundle b land 後) | |---|---|---| | rate-limit auto-retry 実装 (順位 42) | Sub-PR 2 で着手予定 | ✅ **Bb-1 (PR #113) で land 済**。CronCreate park モデルで `[rate_limit] reset 後の即時 retrigger → @coderabbitai review 自動投稿` が PR #115 dogfood で実証 | diff --git a/docs/adr/adr-064-monitor-success-positive-evidence.md b/docs/adr/adr-064-monitor-success-positive-evidence.md index ef922b8e..d14afe55 100644 --- a/docs/adr/adr-064-monitor-success-positive-evidence.md +++ b/docs/adr/adr-064-monitor-success-positive-evidence.md @@ -5,9 +5,15 @@ 採用 (2026-07-21 実装、2026-08-01 ADR 化) > 本 ADR は 2026-07-04 策定のハーネス改善計画の WP-15 追補 (旧 PR #309 全破棄 → ゼロ再構築) -> として実装・検証された決定群の永続記録である。**検証残**: CR レート制限の自然発生時に -> (a) 監視が success で終わらず park すること、(b) レポート判定文が保留を出すこと、の実観測 -> (この経路はレート制限の自然発生時にしか実測できない)。 +> として実装・検証された決定群の永続記録である。 +> +> **検証残の移し替え (2026-08-03、WP-17 PR 3)**: 旧検証残は「CR レート制限の自然発生時に +> (a) 監視が success で終わらず park すること、(b) レポート判定文が保留を出すこと」だった。 +> park モデルの廃止 ([ADR-018](adr-018-pr-monitor-takt-migration.md) 追記 2026-08-03) に伴い +> (a) は **moot として閉じた** (single-shot の同等保証 = terminal `rate_limited` / +> `pending_review` 報告は unit test で固定済み)。(b) のローカル側は判定文テストで固定済み、 +> **GitHub Actions 経路での同等保証 (レート制限中の分析コメントが保留を明示すること) は +> 同経路の検証残**として引き継ぐ。 ## コンテキスト diff --git a/docs/harness-improvement-plan.md b/docs/harness-improvement-plan.md index c5c50084..d81a5c9d 100644 --- a/docs/harness-improvement-plan.md +++ b/docs/harness-improvement-plan.md @@ -87,10 +87,11 @@ Anthropic 公式のハーネスエンジニアリング指針（決定論的基 - 現状: observe 期間（2026-07-12〜08-01、fix step 実行 5 回）で誤検知ゼロを確認し、2026-08-01 に `mode = "enforce"` へ昇格済（ADR-054 の dogfood 記録参照）。 - 残作業: enforce で 3〜5 PR（fix step 発生ベース）誤検知ゼロを確認したら本採用（ADR-054 の status 更新）。判定基準・kill-switch は ADR-054 を参照。 -### WP-15 追補残: レート制限 park の実観測 +### WP-15 追補残: レート制限時の保留保証（GitHub Actions 経路） - 現状: PR 監視の陽性証拠 gate は実装・incident 実データでの単体実測済み（ADR-064）。 -- 残作業: 実 push/PR サイクルで CodeRabbit レート制限が自然発生した際に (a) 監視が success で終わらず park すること、(b) レポート判定文が保留を出すこと、を実観測したら完了（ADR-064 ステータス欄の検証残。この経路は自然発生時にしか実測できない）。 +- **2026-08-04 更新（WP-17 PR 3）**: 旧残作業のうち (a)「監視が success で終わらず **park** すること」は、park モデルの廃止（[ADR-018](adr/adr-018-pr-monitor-takt-migration.md) 追記 2026-08-03）で **moot として終了**。single-shot モデルでの同等保証は terminal `rate_limited` 報告で、unit test により固定済み。 +- 残作業: (b)「レポート判定文が保留を出すこと」の **GitHub Actions 経路での実観測**。CodeRabbit レート制限が自然発生した際に、Phase A の分析コメントが「レビュー未実施のため保留」を明示することを確認したら完了（この経路は自然発生時にしか実測できない）。ローカル側の判定文は `verdict_for_unsettled_review` のテストで固定済み。 ### WP-16 残: CI matrix の実走観測と required check 化 @@ -202,13 +203,15 @@ jj log -r 'ylkowqkp | unksnyts | mxzwmsyp | lwpktvpm | lqxzpvuw | utpvkwql | rxv - 段 2: `claude/` prefix のテストブランチで docs 指摘のある PR を作り、allow 経路（gate exit 0 → workflow step が push）と deny 経路（variable 削除で次 run から job skip）を観測。config 層の deny（master 調達の `autonomy-config.toml`）が実走で確認できるのはこの段が最初。あわせて run log の `[PHASE_B_ACTOR]` / `[PHASE_B_ACTOR_UNRESOLVED]` マーカーで **coderabbitai[bot] の permission 解決結果**を確認する — bot は collaborator ではない可能性が高く、その場合 `pull_request_review` 経路の Phase B は恒久 deny（fail-closed で危険はないが、`issue_comment` = walkthrough 経路だけが生きる形になる）。deny なら actor gate への bot allowlist 追加を follow-up として判断する（内容側は決定論著者フィルタが守っているため、追加しても多層防御は崩れない）。 - 結果を ADR-067 の検証記録と ADR-066 / ADR-068 の bounded lifetime 観測へ記帳する。 -#### WP-17 PR 3: wakeup 機構（CronCreate 系）の廃止（旧ステップ 3） +#### WP-17 PR 3: wakeup 機構（CronCreate 系）の廃止（旧ステップ 3） — 実施中（本 PR） - 廃止対象: cli-pr-monitor の CronCreate park モデル（ADR-018 追記の Bundle b で再導入。PR #237 で失効事例を観測済み）。 - park / wakeup 経路: state の `next_wakeup_at_unix` / `wakeup_reason`、monitor stage の wakeup invocation、`[PR_MONITOR_PARK]` envelope 出力。 - hooks-session-start の pr_monitor catch-up nudge（park 失効の救済層。機構ごと dead code になるため撤去）。 - 代替: PR イベント（レート制限中の再開含む）は GitHub Actions 経路（Phase A/B）が引き受ける。CodeRabbit の後続コメント / レビュー到着がそのままトリガーになるため、ローカルの時限 wakeup は不要。 - 記録: ADR-018 amendment を起票し、着手前決定 2 の ADR-064 検証残移し替え（(a) moot / (b) Actions 経路へ引き継ぎ）を amendment と ADR-064 ステータス欄の両方に記載。ADR-034 の CronCreate 参照も同 PR で整合を取る。 +- 実装メモ（本 PR で確定した設計判断）: 時刻窓アンカーの state 継続（`should_continue_state` = 同一 PR + 同一 head なら `started_at` / `fix_push_time` を維持）は park の付随物ではないため**残した**。落とすと手動再実行のたびに `--push-time` が「今」へリセットされ、push 後に届いた CR コメントが新着判定から漏れる。rate-limit の retry 上限 / comment dedup も同様に維持。 +- 本 PR の PR がそのまま**スモーク段 1 の観測対象**を兼ねる（variable 再設定済みの状態で、非 `claude/` PR に対する fix job の prefix deny をマージ済み master 版 workflow で確認する）。 #### WP-17 PR 4: weekly-review の cloud routine 移行（旧ステップ 2） diff --git a/docs/todo18.md b/docs/todo18.md index 4a0c3dfd..d375ad58 100644 --- a/docs/todo18.md +++ b/docs/todo18.md @@ -15,7 +15,7 @@ > > **本タスクの位置づけ**: PR #214 post-merge-feedback Tier 3 #1 採用 (Severity Medium / Frequency Medium / Effort S / Adoption Risk None、2026-06-20 ユーザー承認)。analyzer rationale: 「PR #214 の `review_recheck.rs` で positive pattern として land (lines 185–187)。同型コード (`review_recheck_count`, `head_commit` 上書き) との一貫性がある確立されたパターン。Frequency Medium = cli-pr-monitor には複数の state machine があり再発確実。Effort S、Adoption Risk None → ✅ 採用候補と判定」。pre-push:simplicity + pre-push:security の独立 2 ソース検出。 > -> **参照**: `.claude/feedback-reports/214.md` Tier 3 #1、[src/cli-pr-monitor/src/stages/poll/review_recheck.rs:177-193](../src/cli-pr-monitor/src/stages/poll/review_recheck.rs#L177-L193) (現行 `finalize_initial_review_park` の defensive reset block)、memory `feedback_no_unenforced_rules.md` (enforcement 要件)、memory `feedback_global_config_backup.md` (snapshot 必須)。 +> **参照**: `.claude/feedback-reports/214.md` Tier 3 #1、旧 `src/cli-pr-monitor/src/stages/poll/review_recheck.rs` の `finalize_initial_review_park` defensive reset block (**WP-17 PR 3 の park モデル廃止でファイルごと削除済み** — pattern の実例は PR #214 の diff を参照。rule 化する価値は削除後も変わらない: 同型の state machine は `finalize_pending_review` / iteration の state 継承等に現存)、memory `feedback_no_unenforced_rules.md` (enforcement 要件)、memory `feedback_global_config_backup.md` (snapshot 必須)。 > > **実行優先度**: 💎 **Tier 3** — Effort S。global rules への docs 追記 ~30 行で完結、`feedback_global_config_backup` snapshot を忘れない。 diff --git a/pr-monitor-config.toml b/pr-monitor-config.toml index 409f64f9..0d43df22 100644 --- a/pr-monitor-config.toml +++ b/pr-monitor-config.toml @@ -6,17 +6,12 @@ [monitor] enabled = true -max_duration_secs = 600 check_ci = true check_coderabbit = true +# (旧 max_duration_secs は WP-17 PR 3 で廃止 — single-shot 化によりループ上限が消滅) -# review 完了待ち park 制御 (Bb-3 順位 55)。 -# CodeRabbit walkthrough 確認後、review 完了をポーリングする CronCreate 経路の -# 待機秒数と最大再チェック回数。コメントアウトで全てデフォルト (5 分 / 5 分 / 3 回) を使用。 -# [review_recheck] -# initial_review_wait_secs = 300 -# review_recheck_wait_secs = 300 -# max_review_rechecks = 3 +# (旧 [review_recheck] セクションは WP-17 PR 3 で廃止。park/wakeup モデルの撤去に伴い +# 設定対象が消滅した。旧 config に残っていても unknown section として無視される) [takt] workflow = "post-pr-review" diff --git a/src/check-ci-coderabbit/src/main.rs b/src/check-ci-coderabbit/src/main.rs index 25ccfc3f..5e935b46 100644 --- a/src/check-ci-coderabbit/src/main.rs +++ b/src/check-ci-coderabbit/src/main.rs @@ -2,7 +2,8 @@ //! //! push / PR 作成後に CI (GitHub Actions) と CodeRabbit レビューの状態を //! gh CLI 経由で取得し、構造化 JSON を stdout に出力する。 -//! CronCreate のポーリングジョブから定期的に呼び出される想定。 +//! cli-pr-monitor の single-shot check から呼び出される (WP-17 PR 3 で +//! CronCreate ポーリング前提を廃止)。 //! //! 使い方: //! check-ci-coderabbit.exe --push-time "2026-04-01T12:00:00Z" [--repo owner/repo] [--pr 42] diff --git a/src/cli-pr-monitor/src/config.rs b/src/cli-pr-monitor/src/config.rs index 3bf99bd9..52142e0b 100644 --- a/src/cli-pr-monitor/src/config.rs +++ b/src/cli-pr-monitor/src/config.rs @@ -3,7 +3,6 @@ use std::path::{Path, PathBuf}; use crate::log::log_info; -pub(crate) const DEFAULT_MAX_DURATION: u64 = 600; pub(crate) const DEFAULT_STEP_TIMEOUT_SECS: u64 = 300; pub(crate) const DEFAULT_CHECK_TIMEOUT_SECS: u64 = 60; @@ -17,8 +16,6 @@ pub(crate) struct Config { #[serde(default)] pub(crate) rate_limit: RateLimitConfig, #[serde(default)] - pub(crate) review_recheck: ReviewRecheckConfig, - #[serde(default)] pub(crate) classifier: ClassifierConfig, } @@ -26,8 +23,6 @@ pub(crate) struct Config { pub(crate) struct MonitorConfig { #[serde(default = "default_enabled")] pub(crate) enabled: bool, - #[serde(default = "default_max_duration")] - pub(crate) max_duration_secs: u64, #[serde(default = "default_check_ci")] pub(crate) check_ci: bool, #[serde(default = "default_check_coderabbit")] @@ -37,9 +32,6 @@ pub(crate) struct MonitorConfig { fn default_enabled() -> bool { true } -fn default_max_duration() -> u64 { - DEFAULT_MAX_DURATION -} fn default_check_ci() -> bool { true } @@ -51,7 +43,6 @@ impl Default for MonitorConfig { fn default() -> Self { Self { enabled: default_enabled(), - max_duration_secs: default_max_duration(), check_ci: default_check_ci(), check_coderabbit: default_check_coderabbit(), } @@ -165,85 +156,6 @@ impl Default for RateLimitConfig { } } -/// review 完了待ち park 制御 (Bb-3 順位 55) -/// -/// CodeRabbit walkthrough 確認後、review 完了をポーリングする CronCreate 経路の -/// 待機秒数と最大再チェック回数を制御する。 -/// 旧 hard-coded const (poll.rs INITIAL_REVIEW_WAIT_SECS / REVIEW_RECHECK_WAIT_SECS / -/// MAX_REVIEW_RECHECKS) を config 化したもの。 -#[derive(Deserialize, Clone)] -pub(crate) struct ReviewRecheckConfig { - /// fresh push 経路 (initial park) の wait 秒数 - #[serde(default = "default_initial_review_wait_secs")] - pub(crate) initial_review_wait_secs: u64, - /// wakeup 経路 (continue_monitoring) で次回 wakeup までの wait 秒数 - #[serde(default = "default_review_recheck_wait_secs")] - pub(crate) review_recheck_wait_secs: u64, - /// recheck 上限。到達後は action_required で抜ける - #[serde(default = "default_max_review_rechecks")] - pub(crate) max_review_rechecks: u32, -} - -fn default_initial_review_wait_secs() -> u64 { - 300 -} -fn default_review_recheck_wait_secs() -> u64 { - 300 -} -fn default_max_review_rechecks() -> u32 { - 3 -} - -impl Default for ReviewRecheckConfig { - fn default() -> Self { - Self { - initial_review_wait_secs: default_initial_review_wait_secs(), - review_recheck_wait_secs: default_review_recheck_wait_secs(), - max_review_rechecks: default_max_review_rechecks(), - } - } -} - -/// `wait_secs` の実用的な上限 (1 年 = 31,536,000 秒)。 -/// -/// PR #115 CR Major #2 採用: poll.rs が `now_unix + wait_secs as i64` を計算するため、 -/// `wait_secs` を `i64::MAX` ぎりぎりまで許容すると `now_unix (~1.78e9 in 2026)` との -/// 加算で確実に算術 overflow し、release build では負の wakeup_at にラップする。 -/// 1 年 = 3.15e7 << i64::MAX = 9.22e18 で `now_unix + 1年` は overflow しない。 -/// CronCreate の auto-expire は 7 日のため、1 年は user 編集の上限として十分な余裕を持つ。 -/// -/// Future-proof 根拠 (順位 77): year 2100 baseline (`now_unix ~ 4.1e9`) でも -/// `4.1e9 + 3.15e7` = 4.13e9 << i64::MAX (9.22e18) で safety margin が -/// 約 9 桁 (2.2e9 倍) ある。`review_recheck_sanitize_max_safe_boundary` test -/// で 2026 + 2100 baseline 両方を machine-enforce。 -const MAX_SAFE_WAIT_SECS: u64 = 365 * 24 * 60 * 60; - -impl ReviewRecheckConfig { - /// 異常値 (0 / 実用域超過) をデフォルト値にフォールバックする。 - /// - /// PR #115 CR Major #1 / #2 採用: 防御的 input validation。 - /// poll.rs が `wait_secs as i64` を `now_unix + wait` に加算するため、`wait_secs == 0` - /// は wakeup を即時化、`max_review_rechecks == 0` は recheck を瞬時に max 到達させる、 - /// `wait_secs > MAX_SAFE_WAIT_SECS` (1 年) は `now_unix + wait` の i64 加算で overflow して - /// wakeup_at が破損する。これらを `load_config` 経路で defensively 修正する - /// (config が user 編集可能な system boundary のため、CLAUDE.md - /// "ALWAYS validate at system boundaries" 原則に従う)。 - fn sanitize(mut self) -> Self { - if self.initial_review_wait_secs == 0 || self.initial_review_wait_secs > MAX_SAFE_WAIT_SECS - { - self.initial_review_wait_secs = default_initial_review_wait_secs(); - } - if self.review_recheck_wait_secs == 0 || self.review_recheck_wait_secs > MAX_SAFE_WAIT_SECS - { - self.review_recheck_wait_secs = default_review_recheck_wait_secs(); - } - if self.max_review_rechecks == 0 { - self.max_review_rechecks = default_max_review_rechecks(); - } - self - } -} - /// CodeRabbit findings をローカル LLM (Ollama) で classify する設定 (ADR-038、Phase 5)。 /// /// `cli-finding-classifier.exe` を subprocess invoke し、`Vec` を @@ -333,10 +245,7 @@ pub(crate) fn load_config() -> Config { } }; match toml::from_str::(&content) { - Ok(mut config) => { - config.review_recheck = config.review_recheck.sanitize(); - config - } + Ok(config) => config, Err(e) => { log_info(&format!( "pr-monitor-config.toml パースエラー (デフォルト使用): {}", @@ -367,7 +276,6 @@ extra_args = ["--pipeline", "--skip-git"] "#; let config: Config = toml::from_str(toml_str).unwrap(); assert!(config.monitor.enabled); - assert_eq!(config.monitor.max_duration_secs, 900); assert!(config.monitor.check_ci); assert!(!config.monitor.check_coderabbit); @@ -377,9 +285,9 @@ extra_args = ["--pipeline", "--skip-git"] assert_eq!(takt.extra_args.as_ref().unwrap().len(), 2); } - /// Bb-3: 旧 `poll_interval_secs` フィールド (Bb-2 で未使用化、Bb-3 で削除) - /// が残った既存 config を読み込む際に、unknown field でパースエラーにならず - /// 無視されることを確認する後方互換テスト。 + /// 旧フィールドが残った既存 config を読み込む際に、unknown field でパース + /// エラーにならず無視されることを確認する後方互換テスト (poll_interval_secs は + /// Bb-3 で、max_duration_secs は WP-17 PR 3 で削除)。 #[test] fn config_ignores_legacy_poll_interval_secs() { let toml_str = r#" @@ -390,7 +298,6 @@ max_duration_secs = 900 "#; let config: Config = toml::from_str(toml_str).unwrap(); assert!(config.monitor.enabled); - assert_eq!(config.monitor.max_duration_secs, 900); } #[test] @@ -409,7 +316,6 @@ enabled = true let toml_str = "[monitor]\n"; let config: Config = toml::from_str(toml_str).unwrap(); assert!(config.monitor.enabled); - assert_eq!(config.monitor.max_duration_secs, DEFAULT_MAX_DURATION); } #[test] @@ -532,17 +438,11 @@ max_retries = 5 assert_eq!(config.rate_limit.max_retries, 5); } + /// PR 3 (wakeup 廃止): 旧 config file に残る `[review_recheck]` セクションは + /// unknown field として無視され、パースが失敗しないこと (park モデル時代の + /// pr-monitor-config.toml との前方互換)。 #[test] - fn config_review_recheck_defaults() { - let toml_str = "[monitor]\n"; - let config: Config = toml::from_str(toml_str).unwrap(); - assert_eq!(config.review_recheck.initial_review_wait_secs, 300); - assert_eq!(config.review_recheck.review_recheck_wait_secs, 300); - assert_eq!(config.review_recheck.max_review_rechecks, 3); - } - - #[test] - fn config_review_recheck_custom() { + fn config_with_removed_review_recheck_section_still_parses() { let toml_str = r#" [monitor] @@ -552,70 +452,7 @@ review_recheck_wait_secs = 900 max_review_rechecks = 5 "#; let config: Config = toml::from_str(toml_str).unwrap(); - assert_eq!(config.review_recheck.initial_review_wait_secs, 600); - assert_eq!(config.review_recheck.review_recheck_wait_secs, 900); - assert_eq!(config.review_recheck.max_review_rechecks, 5); - } - - /// PR #115 CR Major #1: `max_review_rechecks=0` は recheck を瞬時に max 到達させ - /// 機能を無効化するため、デフォルト値にフォールバックする。 - #[test] - fn review_recheck_sanitize_replaces_zero_max_review_rechecks() { - let cfg = ReviewRecheckConfig { - initial_review_wait_secs: 100, - review_recheck_wait_secs: 200, - max_review_rechecks: 0, - } - .sanitize(); - assert_eq!( - cfg.max_review_rechecks, 3, - "0 はデフォルト 3 にフォールバック" - ); - assert_eq!(cfg.initial_review_wait_secs, 100, "他フィールドは不変"); - assert_eq!(cfg.review_recheck_wait_secs, 200, "他フィールドは不変"); - } - - /// PR #115 CR Major #1: `wait_secs=0` は wakeup を即時化しスケジューリング意図を失うため、 - /// デフォルト値にフォールバックする。 - #[test] - fn review_recheck_sanitize_replaces_zero_wait_secs() { - let cfg = ReviewRecheckConfig { - initial_review_wait_secs: 0, - review_recheck_wait_secs: 0, - max_review_rechecks: 5, - } - .sanitize(); - assert_eq!(cfg.initial_review_wait_secs, 300); - assert_eq!(cfg.review_recheck_wait_secs, 300); - assert_eq!(cfg.max_review_rechecks, 5, "他フィールドは不変"); - } - - /// PR #115 CR Major #2: `wait_secs > MAX_SAFE_WAIT_SECS` (1 年) は poll.rs の - /// `now_unix + wait as i64` 加算で算術 overflow するため、デフォルト値に - /// フォールバックする。`u64::MAX` / `i64::MAX as u64` 等の極端値も対象。 - #[test] - fn review_recheck_sanitize_replaces_unrealistic_wait_secs() { - let cfg = ReviewRecheckConfig { - initial_review_wait_secs: u64::MAX, - review_recheck_wait_secs: i64::MAX as u64, - max_review_rechecks: 3, - } - .sanitize(); - assert_eq!(cfg.initial_review_wait_secs, 300); - assert_eq!(cfg.review_recheck_wait_secs, 300); - } - - #[test] - fn review_recheck_sanitize_keeps_valid_values_unchanged() { - let cfg = ReviewRecheckConfig { - initial_review_wait_secs: 600, - review_recheck_wait_secs: 900, - max_review_rechecks: 5, - } - .sanitize(); - assert_eq!(cfg.initial_review_wait_secs, 600); - assert_eq!(cfg.review_recheck_wait_secs, 900); - assert_eq!(cfg.max_review_rechecks, 5); + assert!(config.monitor.enabled); } #[test] @@ -646,155 +483,4 @@ timeout_secs = 60 assert_eq!(config.classifier.timeout_secs, 60); } - /// PR #115 CR Major #2: 1 年 (MAX_SAFE_WAIT_SECS) ぎりぎりは valid、 - /// 1 年 + 1 秒は default に置換される境界値を machine-enforce する。 - /// 加えて、`now_unix + sanitize 後の値 < i64::MAX` invariant が成立することを assert。 - #[test] - fn review_recheck_sanitize_max_safe_boundary() { - let cfg_at_limit = ReviewRecheckConfig { - initial_review_wait_secs: MAX_SAFE_WAIT_SECS, - review_recheck_wait_secs: MAX_SAFE_WAIT_SECS, - max_review_rechecks: 1, - } - .sanitize(); - assert_eq!( - cfg_at_limit.initial_review_wait_secs, MAX_SAFE_WAIT_SECS, - "1 年ジャストは valid" - ); - - let cfg_over_limit = ReviewRecheckConfig { - initial_review_wait_secs: MAX_SAFE_WAIT_SECS + 1, - review_recheck_wait_secs: MAX_SAFE_WAIT_SECS + 1, - max_review_rechecks: 1, - } - .sanitize(); - assert_eq!( - cfg_over_limit.initial_review_wait_secs, 300, - "1 年 + 1 秒は default にフォールバック" - ); - - let now_unix_2026: i64 = 1_800_000_000; - let safe_sum = now_unix_2026.checked_add(cfg_at_limit.initial_review_wait_secs as i64); - assert!( - safe_sum.is_some(), - "sanitize 後の値は now_unix + wait で overflow しない (CR Major #2 invariant)" - ); - - let now_unix_2100: i64 = 4_100_000_000; - let safe_sum_2100 = - now_unix_2100.checked_add(cfg_at_limit.initial_review_wait_secs as i64); - assert!( - safe_sum_2100.is_some(), - "year 2100 baseline でも overflow しない (順位 77: future-proof)" - ); - assert!( - safe_sum_2100.unwrap() < i64::MAX, - "year 2100 baseline で safety margin が残る" - ); - } - - #[test] - fn cross_module_overflow_safety_at_max_boundary() { - let cfg = ReviewRecheckConfig { - initial_review_wait_secs: MAX_SAFE_WAIT_SECS, - review_recheck_wait_secs: MAX_SAFE_WAIT_SECS, - max_review_rechecks: 1, - } - .sanitize(); - - let now_unix: i64 = 1_800_000_000; - let initial_park_sum = now_unix.checked_add(cfg.initial_review_wait_secs as i64); - let recheck_park_sum = now_unix.checked_add(cfg.review_recheck_wait_secs as i64); - - assert!( - initial_park_sum.is_some(), - "finalize_initial_review_park の next_wakeup_at_unix 加算が overflow しない" - ); - assert!( - recheck_park_sum.is_some(), - "schedule_next_review_recheck_park の next_wakeup_at_unix 加算が overflow しない" - ); - assert!( - initial_park_sum.unwrap() > now_unix, - "next_wakeup_at_unix が past に巻き戻らない" - ); - assert!( - recheck_park_sum.unwrap() > now_unix, - "next_wakeup_at_unix が past に巻き戻らない" - ); - } - - #[test] - fn cross_module_overflow_safety_with_zero_input_uses_default() { - let cfg = ReviewRecheckConfig { - initial_review_wait_secs: 0, - review_recheck_wait_secs: 0, - max_review_rechecks: 1, - } - .sanitize(); - - assert_eq!( - cfg.initial_review_wait_secs, 300, - "0 入力は sanitize で default 300s に置換" - ); - assert_eq!(cfg.review_recheck_wait_secs, 300); - - let now_unix: i64 = 1_800_000_000; - let park_sum = now_unix - .checked_add(cfg.initial_review_wait_secs as i64) - .unwrap(); - assert_eq!(park_sum, now_unix + 300); - } - - #[test] - fn cross_module_overflow_safety_with_u64_max_input_uses_default() { - let cfg = ReviewRecheckConfig { - initial_review_wait_secs: u64::MAX, - review_recheck_wait_secs: u64::MAX, - max_review_rechecks: 1, - } - .sanitize(); - - assert_eq!( - cfg.initial_review_wait_secs, 300, - "u64::MAX 入力は sanitize で default 300s に置換" - ); - assert_eq!(cfg.review_recheck_wait_secs, 300); - - let now_unix: i64 = 1_800_000_000; - let park_sum = now_unix - .checked_add(cfg.initial_review_wait_secs as i64) - .unwrap(); - assert!(park_sum < i64::MAX); - } - - #[test] - fn cross_module_overflow_safety_negative_test_large_unsanitized_value_overflows() { - let unsanitized_wait: u64 = i64::MAX as u64; - let now_unix: i64 = 1_800_000_000; - let unsafe_sum = now_unix.checked_add(unsanitized_wait as i64); - - assert!( - unsafe_sum.is_none(), - "sanitize なしで i64::MAX 近傍の wait を直接 cast すると positive overflow する (sanitize の必要性の裏付け、PR #115 CR Major #2 の再現)" - ); - } - - #[test] - fn cross_module_overflow_safety_negative_test_u64_max_wraps_to_minus_one() { - let unsanitized_wait: u64 = u64::MAX; - let now_unix: i64 = 1_800_000_000; - let wrapped = unsanitized_wait as i64; - - assert_eq!( - wrapped, -1, - "u64::MAX as i64 は -1 (two's complement)、checked_add は overflow しないが過去時刻になる" - ); - - let result = now_unix.checked_add(wrapped).unwrap(); - assert!( - result < now_unix, - "u64::MAX を経由した wakeup_at は過去にずれる (= silent corruption、sanitize で防止)" - ); - } } diff --git a/src/cli-pr-monitor/src/main.rs b/src/cli-pr-monitor/src/main.rs index c7bf7ade..6943bf3d 100644 --- a/src/cli-pr-monitor/src/main.rs +++ b/src/cli-pr-monitor/src/main.rs @@ -1,14 +1,18 @@ //! Post-PR Monitor //! //! PR 作成と CI/CodeRabbit 監視を一貫して行うスタンドアロン CLI。 -//! Bb-2 で single-iteration + CronCreate park モデルに移行。 +//! WP-17 PR 3 で single-shot モデルへ移行 (旧 Bb-2 の CronCreate park モデルは廃止 — +//! ADR-018 amendment)。checker を 1 回呼び、terminal action か「未確定 (pending_review / +//! rate_limited)」の報告で必ず終了する。未確定 PR の後続イベントは GitHub Actions 経路 +//! (pr-monitor workflow の Phase A/B) が常時処理し、ローカルの時限 wakeup は使わない。 //! //! モード: -//! デフォルト (PR 作成): gh pr create → 初回 review_recheck park → (wakeup で) takt 分析 +//! デフォルト (PR 作成): gh pr create → single-shot check → 報告 //! pnpm create-pr -- --title "..." --body "..." //! -//! --monitor-only: PR が存在すれば single-iteration check → (wakeup なら) park / 終端 -//! pnpm push 完了後および CronCreate wakeup でチェインで呼ばれる +//! --monitor-only: PR が存在すれば single-shot check → 報告 +//! pnpm push 完了後にチェインで呼ばれる。手動再実行も可 (同一 PR + 同一 head なら +//! state の時刻窓アンカーを継続する) //! //! --mark-notified: state file の notified フラグを true にする //! Claude が結果を処理した後に呼ばれる @@ -17,7 +21,7 @@ //! --prepare-pr-body-cleanup: `.tmp-pr-body.md` を削除する (旧 prepare-pr-body.ps1、WP-14) //! //! 終了コード: -//! 0 - 正常終了 (park 含む、PARK signal は stdout に出力済) +//! 0 - 正常終了 (pending_review / rate_limited の保留報告を含む) //! 1 - gh pr create 失敗 (PR 作成モードのみ) / prepare-pr-body の入力空・IO 失敗 mod classifier_runner; diff --git a/src/cli-pr-monitor/src/stages/monitor.rs b/src/cli-pr-monitor/src/stages/monitor.rs index 1dcc108b..f5ffb3a2 100644 --- a/src/cli-pr-monitor/src/stages/monitor.rs +++ b/src/cli-pr-monitor/src/stages/monitor.rs @@ -17,13 +17,16 @@ pub(crate) fn start_monitoring(pr_info: &PrInfo) -> i32 { start_monitoring_inner(pr_info, false) } -/// Bb-2: wakeup invocation 用 (state リセットを skip し前回の next_wakeup_at_unix / -/// review_recheck_count を保持したまま single-iteration check を実行する)。 -pub(crate) fn start_monitoring_wakeup(pr_info: &PrInfo) -> i32 { +/// 同一 PR + 同一 head への連続 invocation 用 (WP-17 PR 3)。 +/// +/// state をリセットせず、時刻窓アンカー (started_at / fix_push_time) と +/// rate_limit_retries 等の累積値を維持したまま single-shot check を実行する。 +/// 旧 wakeup invocation の後継だが、時限スケジューリングは伴わない。 +pub(crate) fn start_monitoring_continuing(pr_info: &PrInfo) -> i32 { start_monitoring_inner(pr_info, true) } -fn start_monitoring_inner(pr_info: &PrInfo, is_wakeup: bool) -> i32 { +fn start_monitoring_inner(pr_info: &PrInfo, continue_state: bool) -> i32 { let config = load_config(); if !config.monitor.enabled { log_info("監視は設定で無効化されています"); @@ -40,9 +43,9 @@ fn start_monitoring_inner(pr_info: &PrInfo, is_wakeup: bool) -> i32 { .map(|n| format!("PR #{}", n)) .unwrap_or_else(|| "PR".to_string()); - init_or_resume_state(pr_info, is_wakeup, &pr_label); + init_or_continue_state(pr_info, continue_state, &pr_label); - let poll_result = run_poll_loop(&config, pr_info, is_wakeup); + let poll_result = run_poll_loop(&config, pr_info); log_info(&format!( "ポーリング完了: action={}, summary={}", poll_result.action, poll_result.summary @@ -85,9 +88,9 @@ fn try_acquire_monitor_lock() -> AcquireResult { } } -fn init_or_resume_state(pr_info: &PrInfo, is_wakeup: bool, pr_label: &str) { - if is_wakeup { - log_info(&format!("{} の監視を再開 (wakeup)", pr_label)); +fn init_or_continue_state(pr_info: &PrInfo, continue_state: bool, pr_label: &str) { + if continue_state { + log_info(&format!("{} の監視を継続 (既存 state の時刻窓を維持)", pr_label)); return; } log_info(&format!("{} の監視を開始", pr_label)); @@ -208,15 +211,15 @@ pub(crate) fn run_monitor_only() -> i32 { log_info("監視のみモード (既存 PR 検出)"); - if let Some(resume_push_time) = detect_wakeup_resume(&pr_info) { + if let Some(resume_push_time) = detect_state_continuity(&pr_info) { log_info(&format!( - "[wakeup] 前回 park の next_wakeup_at_unix が経過 → state を継続 (started_at={})", + "[state] 既存 state と同一 PR / head → 時刻窓を継続 (started_at={})", resume_push_time )); pr_info.push_time = Some(resume_push_time.clone()); pr_info.fix_push_time = resume_fix_push_time_or_started_at(&resume_push_time, &state_file_path()); - start_monitoring_wakeup(&pr_info) + start_monitoring_continuing(&pr_info) } else { let now = utc_now_iso8601(); pr_info.push_time = Some(now.clone()); @@ -236,42 +239,36 @@ fn resume_fix_push_time_or_started_at( .or_else(|| Some(started_at_fallback.to_string())) } -/// Bb-2: 既存 state file が「自分の PR / repo / head commit の wakeup 待ち」かを判定し、 -/// 該当すれば push_time として継続用 ISO 8601 (state.started_at) を返す。 +/// 既存 state file が「同一 PR / repo / head commit の続き」かを判定し、 +/// 該当すれば push_time として継続用 ISO 8601 (state.started_at) を返す (WP-17 PR 3)。 +/// +/// 旧 `detect_wakeup_resume` の後継。wakeup 時刻の経過条件は wakeup 廃止に伴い落としたが、 +/// **時刻窓アンカーの継続判定は残す** — これを落とすと手動再実行のたびに `--push-time` が +/// 「今」になり、push 後〜再実行の間に届いた CR コメントが新着判定から漏れる。 /// -/// CR Major #1 fix (Bb-2 PR #114 review): 同一 PR でも新 commit が push されれば head_commit -/// が変わるため、stored vs current head 一致も check する。head 不一致なら fresh push 扱い。 -fn detect_wakeup_resume(pr_info: &PrInfo) -> Option { +/// CR Major #1 fix (Bb-2 PR #114 review) 由来の head 一致条件は維持: 同一 PR でも新 commit が +/// push されれば head_commit が変わるため、stored vs current head 一致も check する。 +/// head 不一致なら fresh 初期化扱い。 +fn detect_state_continuity(pr_info: &PrInfo) -> Option { let state = read_state_from(&state_file_path())?; - let now_unix = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs() as i64) - .unwrap_or(0); - if !should_resume_wakeup(&state, pr_info, now_unix) { + if !should_continue_state(&state, pr_info) { return None; } Some(state.started_at) } -/// CR Major #1 fix: detect_wakeup_resume の判定 invariant を pure に分離してテスト可能にする。 +/// detect_state_continuity の判定 invariant を pure に分離してテスト可能にする。 /// -/// resume 条件 (全て true): +/// 継続条件 (全て true): /// 1. state.pr == pr_info.pr_number AND state.repo == pr_info.repo -/// 2. state.next_wakeup_at_unix が Some かつ now を経過 -/// 3. state.head_commit が Some かつ pr_info.head_commit と一致 +/// 2. state.head_commit が Some かつ pr_info.head_commit と一致 /// -/// 1 つでも不一致なら resume せず fresh push 経路に倒す。legacy state (head_commit None) は -/// 自動的に 3 で False になり安全側 (fresh push) に倒れる。 -fn should_resume_wakeup(state: &PrMonitorState, pr_info: &PrInfo, now_unix: i64) -> bool { +/// 1 つでも不一致なら継続せず fresh 初期化に倒す。legacy state (head_commit None) は +/// 自動的に 2 で False になり安全側 (fresh 初期化) に倒れる。 +fn should_continue_state(state: &PrMonitorState, pr_info: &PrInfo) -> bool { if state.pr != pr_info.pr_number || state.repo != pr_info.repo { return false; } - let Some(wakeup_at) = state.next_wakeup_at_unix else { - return false; - }; - if wakeup_at > now_unix { - return false; - } match (state.head_commit.as_deref(), pr_info.head_commit.as_deref()) { (Some(stored), Some(current)) => stored == current, _ => false, @@ -333,14 +330,16 @@ fn compute_verdict(result: &crate::stages::poll::PollResult) -> String { /// [`verdict_for_findings`] の断定文 (「問題は見つかりませんでした」等) を出せる。 fn verdict_for_unsettled_review(result: &crate::stages::poll::PollResult) -> Option { match result.action.as_str() { - "parked_rate_limit" => { + "rate_limited" => { return Some( - "CodeRabbit rate-limit のため wakeup を予約 (上記 PARK signal 参照)".to_string(), + "CodeRabbit rate-limit 中でレビュー未実施のため、判定を保留します (後続は GitHub Actions 経路が処理)" + .to_string(), ); } - "parked_review_recheck" => { + "pending_review" => { return Some( - "review 完了待ちのため wakeup を予約 (上記 PARK signal 参照)".to_string(), + "review 未確定のため判定を保留します (後続は GitHub Actions 経路が処理)" + .to_string(), ); } _ => {} @@ -430,76 +429,56 @@ mod tests { } } - fn make_park_state(pr: u64, repo: &str, wakeup_at: i64, head: Option<&str>) -> PrMonitorState { + fn make_state(pr: u64, repo: &str, head: Option<&str>) -> PrMonitorState { let mut s = PrMonitorState::new(Some(pr), Some(repo.into()), "t".into()); - s.next_wakeup_at_unix = Some(wakeup_at); - s.wakeup_reason = Some("review_recheck".into()); s.head_commit = head.map(String::from); s } #[test] - fn should_resume_wakeup_true_when_pr_repo_head_match_and_due() { - let state = make_park_state(42, "o/r", 100, Some("abc1234")); + fn should_continue_state_true_when_pr_repo_head_match() { + let state = make_state(42, "o/r", Some("abc1234")); let pr_info = make_pr_info(42, "o/r", Some("abc1234")); - assert!(should_resume_wakeup(&state, &pr_info, 200)); + assert!(should_continue_state(&state, &pr_info)); } #[test] - fn should_resume_wakeup_false_when_head_differs() { - let state = make_park_state(42, "o/r", 100, Some("abc1234")); + fn should_continue_state_false_when_head_differs() { + let state = make_state(42, "o/r", Some("abc1234")); let pr_info = make_pr_info(42, "o/r", Some("def5678")); assert!( - !should_resume_wakeup(&state, &pr_info, 200), - "CR Major #1: head 不一致なら fresh push 経路に倒す" + !should_continue_state(&state, &pr_info), + "CR Major #1 由来: head 不一致なら fresh 初期化に倒す (stale な時刻窓を持ち込まない)" ); } #[test] - fn should_resume_wakeup_false_when_state_head_missing() { - let state = make_park_state(42, "o/r", 100, None); + fn should_continue_state_false_when_state_head_missing() { + let state = make_state(42, "o/r", None); let pr_info = make_pr_info(42, "o/r", Some("abc1234")); assert!( - !should_resume_wakeup(&state, &pr_info, 200), - "legacy state (head_commit None) は安全側で fresh push 扱い" + !should_continue_state(&state, &pr_info), + "legacy state (head_commit None) は安全側で fresh 初期化扱い" ); } #[test] - fn should_resume_wakeup_false_when_pr_info_head_missing() { - let state = make_park_state(42, "o/r", 100, Some("abc1234")); + fn should_continue_state_false_when_pr_info_head_missing() { + let state = make_state(42, "o/r", Some("abc1234")); let pr_info = make_pr_info(42, "o/r", None); assert!( - !should_resume_wakeup(&state, &pr_info, 200), - "current head 取得失敗時は安全側で fresh push 扱い" + !should_continue_state(&state, &pr_info), + "current head 取得失敗時は安全側で fresh 初期化扱い" ); } #[test] - fn should_resume_wakeup_false_when_pr_or_repo_differs() { - let state = make_park_state(42, "o/r", 100, Some("abc1234")); + fn should_continue_state_false_when_pr_or_repo_differs() { + let state = make_state(42, "o/r", Some("abc1234")); let other_pr = make_pr_info(99, "o/r", Some("abc1234")); let other_repo = make_pr_info(42, "x/y", Some("abc1234")); - assert!(!should_resume_wakeup(&state, &other_pr, 200)); - assert!(!should_resume_wakeup(&state, &other_repo, 200)); - } - - #[test] - fn should_resume_wakeup_false_when_wakeup_in_future() { - let state = make_park_state(42, "o/r", 1000, Some("abc1234")); - let pr_info = make_pr_info(42, "o/r", Some("abc1234")); - assert!( - !should_resume_wakeup(&state, &pr_info, 100), - "next_wakeup_at_unix が未来ならまだ resume しない" - ); - } - - #[test] - fn should_resume_wakeup_false_when_next_wakeup_unset() { - let mut state = make_park_state(42, "o/r", 100, Some("abc1234")); - state.next_wakeup_at_unix = None; - let pr_info = make_pr_info(42, "o/r", Some("abc1234")); - assert!(!should_resume_wakeup(&state, &pr_info, 200)); + assert!(!should_continue_state(&state, &other_pr)); + assert!(!should_continue_state(&state, &other_repo)); } use crate::stages::poll::PollResult; @@ -538,24 +517,25 @@ mod tests { } } - const VERDICT_PARK_RATE_LIMIT: &str = - "CodeRabbit rate-limit のため wakeup を予約 (上記 PARK signal 参照)"; - const VERDICT_PARK_REVIEW: &str = "review 完了待ちのため wakeup を予約 (上記 PARK signal 参照)"; + const VERDICT_RATE_LIMITED: &str = + "CodeRabbit rate-limit 中でレビュー未実施のため、判定を保留します (後続は GitHub Actions 経路が処理)"; + const VERDICT_PENDING_REVIEW: &str = + "review 未確定のため判定を保留します (後続は GitHub Actions 経路が処理)"; const VERDICT_REVIEW_PENDING: &str = "CodeRabbit review が未完了のため、判定を保留します"; const VERDICT_NO_PROBLEMS: &str = "問題は見つかりませんでした"; const VERDICT_MINOR: &str = "重大な問題は見つかりませんでした。軽微な改善提案があります"; const VERDICT_CRITICAL: &str = "修正が必要な指摘があります"; #[test] - fn verdict_park_rate_limit_takes_precedence_over_review_state() { - let r = poll_result("parked_rate_limit", Some("not_found"), vec![]); - assert_eq!(compute_verdict(&r), VERDICT_PARK_RATE_LIMIT); + fn verdict_rate_limited_takes_precedence_over_review_state() { + let r = poll_result("rate_limited", Some("not_found"), vec![]); + assert_eq!(compute_verdict(&r), VERDICT_RATE_LIMITED); } #[test] - fn verdict_park_review_recheck_takes_precedence_over_findings() { - let r = poll_result("parked_review_recheck", Some("not_found"), vec![finding("critical")]); - assert_eq!(compute_verdict(&r), VERDICT_PARK_REVIEW); + fn verdict_pending_review_takes_precedence_over_findings() { + let r = poll_result("pending_review", Some("not_found"), vec![finding("critical")]); + assert_eq!(compute_verdict(&r), VERDICT_PENDING_REVIEW); } #[test] diff --git a/src/cli-pr-monitor/src/stages/poll/iteration.rs b/src/cli-pr-monitor/src/stages/poll/iteration.rs index 9b4b2375..6c6256a4 100644 --- a/src/cli-pr-monitor/src/stages/poll/iteration.rs +++ b/src/cli-pr-monitor/src/stages/poll/iteration.rs @@ -1,5 +1,4 @@ use std::path::Path; -use std::time::Duration; use crate::classifier_runner::classify_findings; use crate::config::{ClassifierConfig, DEFAULT_CHECK_TIMEOUT_SECS}; @@ -39,22 +38,12 @@ pub(super) fn run_one_iteration(ctx: &PollContext<'_>) -> Option { return Some(make_terminal_result(state, result)); } - if let Some(terminal) = handle_rate_limit_branch( - &mut state, - ctx.rate_limit_config, - ctx.pr_info, - ctx.review_recheck_wait_secs, - &result, - ctx.state_path, - ) { + if let Some(terminal) = + handle_rate_limit_branch(&mut state, ctx.rate_limit_config, ctx.pr_info, &result, ctx.state_path) + { return Some(terminal); } - if ctx.start.elapsed() >= Duration::from_secs(ctx.max_duration) { - log_info(&format!("監視タイムアウト ({}秒)", ctx.max_duration)); - return Some(make_timeout_result(state, ctx.max_duration, result)); - } - None } @@ -137,7 +126,6 @@ fn build_state_for_iteration( state.notified = existing.notified; state.rate_limit_retries = existing.rate_limit_retries; state.rate_limit_last_retriggered_at = existing.rate_limit_last_retriggered_at; - state.review_recheck_count = existing.review_recheck_count; state.head_commit = existing.head_commit; state.classified_findings = existing.classified_findings; state.fix_push_time = existing.fix_push_time; @@ -214,22 +202,6 @@ fn make_terminal_result(state: PrMonitorState, result: serde_json::Value) -> Pol } } -fn make_timeout_result( - state: PrMonitorState, - max_duration: u64, - result: serde_json::Value, -) -> PollResult { - PollResult { - action: "timed_out".into(), - summary: format!("監視タイムアウト ({}秒)", max_duration), - ci: state.ci, - coderabbit: state.coderabbit, - findings: state.findings, - check_output: Some(result), - rate_limit: state.rate_limit, - } -} - /// skip 適用後に、有効なチェックだけを見て action を再導出する fn recompute_action(state: &PrMonitorState, skip_ci: bool, skip_coderabbit: bool) -> String { let ci_ok = skip_ci diff --git a/src/cli-pr-monitor/src/stages/poll/mod.rs b/src/cli-pr-monitor/src/stages/poll/mod.rs index d69dfc07..77265741 100644 --- a/src/cli-pr-monitor/src/stages/poll/mod.rs +++ b/src/cli-pr-monitor/src/stages/poll/mod.rs @@ -1,17 +1,12 @@ mod iteration; mod rate_limit; -mod rate_limit_signal; -mod review_recheck; -mod review_recheck_signal; - -use review_recheck::{finalize_initial_review_park, finalize_review_recheck_park}; use lib_report_formatter::Finding; use crate::config::{Config, MonitorConfig, RateLimitConfig}; use crate::log::log_info; use crate::runner::checker_exe_path; -use crate::state::{CiState, CodeRabbitState, PrMonitorState, RateLimitState}; +use crate::state::{read_state_from, write_state_to, CiState, CodeRabbitState, RateLimitState}; use crate::util::PrInfo; pub(crate) struct PollResult { @@ -40,27 +35,18 @@ pub(super) struct PollContext<'a> { pub(super) pr_info: &'a PrInfo, pub(super) rate_limit_config: &'a RateLimitConfig, pub(super) classifier_config: &'a crate::config::ClassifierConfig, - pub(super) start: std::time::Instant, - pub(super) max_duration: u64, pub(super) skip_ci: bool, pub(super) skip_coderabbit: bool, - /// fresh push 経路 (initial park) の wait 秒数 (Bb-3 順位 55: config 由来) - pub(super) initial_review_wait_secs: u64, - /// wakeup 経路で次回 wakeup までの wait 秒数 (Bb-3 順位 55: config 由来) - pub(super) review_recheck_wait_secs: u64, - /// recheck 上限 (Bb-3 順位 55: config 由来) - pub(super) max_review_rechecks: u32, } -/// single-iteration check + park-or-terminate モデル (Bb-2)。 -/// -/// `is_wakeup=false` (fresh push): checker は呼ばず、即 `initial_review_wait_secs` 後の -/// wakeup を予約して exit する (CR review 開始前の wasteful API call を回避、todo5.md spec)。 +/// single-shot check モデル (WP-17 PR 3、ADR-018 amendment)。 /// -/// `is_wakeup=true` (CronCreate からの再 invoke): 1 回 checker を呼び、結果に応じて -/// (a) terminal action / (b) rate-limit park (Bb-1) / (c) review_recheck park (Bb-2) -/// のいずれかで return する。 -pub(crate) fn run_poll_loop(full_config: &Config, pr_info: &PrInfo, is_wakeup: bool) -> PollResult { +/// checker を 1 回呼び、terminal action か「未確定 (pending)」の 2 択で必ず return する。 +/// 旧 park モデル (Bb-1/Bb-2: 未確定なら state に wakeup 時刻を書き PARK signal を出して +/// CronCreate 再 invoke を待つ) は廃止した — PR イベントの後続処理は GitHub Actions 経路 +/// (pr-monitor workflow の Phase A/B) が常時引き受け、ローカルの時限 wakeup は不要。 +/// PR #237 で観測した「セッション終了による wakeup 失効 = 監視の取りこぼし」も機構ごと消える。 +pub(crate) fn run_poll_loop(full_config: &Config, pr_info: &PrInfo) -> PollResult { let config: &MonitorConfig = &full_config.monitor; let checker = checker_exe_path(); @@ -84,39 +70,46 @@ pub(crate) fn run_poll_loop(full_config: &Config, pr_info: &PrInfo, is_wakeup: b pr_info, rate_limit_config: &full_config.rate_limit, classifier_config: &full_config.classifier, - start: std::time::Instant::now(), - max_duration: config.max_duration_secs, skip_ci: !config.check_ci, skip_coderabbit: !config.check_coderabbit, - initial_review_wait_secs: full_config.review_recheck.initial_review_wait_secs, - review_recheck_wait_secs: full_config.review_recheck.review_recheck_wait_secs, - max_review_rechecks: full_config.review_recheck.max_review_rechecks, }; - if !is_wakeup { - return finalize_initial_review_park(&ctx); - } - if let Some(terminal) = iteration::run_one_iteration(&ctx) { return terminal; } - finalize_review_recheck_park(&ctx) + finalize_pending_review(&ctx) } -pub(super) 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, +/// checker が未確定 (continue_monitoring 相当) を返した場合の terminal 化。 +/// +/// 旧 park モデルではここで wakeup を予約したが、single-shot モデルでは +/// 「未確定である」ことを loud に報告して終了する (ADR-064 の陽性証拠原則: +/// pending を silent success に見せない)。後続の PR イベント (レビュー到着 / +/// CodeRabbit コメント) は GitHub Actions 経路が処理する。再確認したい場合は +/// `cli-pr-monitor --monitor-only` を再実行すればよい (時刻窓は state.started_at +/// アンカーで継続する)。 +fn finalize_pending_review(ctx: &PollContext<'_>) -> PollResult { + let mut state = read_state_from(ctx.state_path).unwrap_or_else(|| { + crate::state::PrMonitorState::new( + ctx.pr_info.pr_number, + ctx.pr_info.repo.clone(), + ctx.push_time.to_string(), + ) + }); + state.action = "pending_review".into(); + state.summary = + "review 未確定。後続の PR イベントは GitHub Actions 経路 (pr-monitor workflow) が処理" + .to_string(); + state.record_head_commit(ctx.pr_info.head_commit.as_deref()); + state.fix_push_time = state + .fix_push_time + .or_else(|| ctx.fix_push_time.map(String::from)); + if let Err(e) = write_state_to(ctx.state_path, &state) { + log_info(&format!( + "state 書き込み失敗 (pending_review 確定後、続行): {}", + e + )); } -} - -/// review_recheck park / initial park の戻り値生成 helper (check_output=None)。 -pub(super) fn make_park_poll_result(state: PrMonitorState) -> PollResult { PollResult { action: state.action, summary: state.summary, @@ -128,140 +121,124 @@ pub(super) fn make_park_poll_result(state: PrMonitorState) -> PollResult { } } +pub(super) 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, + } +} + #[cfg(test)] mod tests { use super::*; use crate::config::ClassifierConfig; - use rate_limit::finalize_parked; - use review_recheck::{finalize_initial_review_park, schedule_next_review_recheck_park}; - - /// 書き込み先がディレクトリ不在のため write が必ず失敗する path を返す。 - fn unwritable_state_path() -> std::path::PathBuf { - std::env::temp_dir() - .join(format!("pr-monitor-T2-2-{}", std::process::id())) - .join("nonexistent-dir") - .join("state.json") - } - - fn invoke_finalize_parked_with_bad_path( - pr_info: &crate::util::PrInfo, - state_path: &std::path::Path, - ) -> PollResult { - let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); - let rl = RateLimitState { - until_unix_secs: 1_775_088_000, - comment_event_time: "x".into(), - wait_minutes: 5, - wait_seconds: 0, - }; - let result = serde_json::json!({}); - finalize_parked( - &mut state, - &rl, - pr_info, - 1_775_088_000, - 3, - &result, - state_path, - ) - } - - fn invoke_review_park_with_bad_path( - pr_info: &crate::util::PrInfo, - state_path: &std::path::Path, - ) -> PollResult { - let mut state = - PrMonitorState::new(Some(1), Some("o/r".into()), "2026-05-01T00:00:00Z".into()); - state.review_recheck_count = 1; - let checker_path = std::path::PathBuf::from("dummy"); - let rate_limit_config = RateLimitConfig::default(); - let classifier_config = ClassifierConfig::default(); - let ctx = PollContext { - checker: &checker_path, - state_path, - push_time: "2026-05-01T00:00:00Z", - fix_push_time: None, - 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, - }; - schedule_next_review_recheck_park(&mut state, &ctx) - } - - fn invoke_finalize_initial_review_park_with_bad_path( - pr_info: &crate::util::PrInfo, - state_path: &std::path::Path, - ) -> PollResult { - let checker_path = std::path::PathBuf::from("dummy"); - let rate_limit_config = RateLimitConfig::default(); - let classifier_config = ClassifierConfig::default(); - let ctx = PollContext { - checker: &checker_path, + use crate::state::PrMonitorState; + + fn make_ctx<'a>( + checker: &'a std::path::Path, + state_path: &'a std::path::Path, + pr_info: &'a crate::util::PrInfo, + rate_limit_config: &'a RateLimitConfig, + classifier_config: &'a ClassifierConfig, + ) -> PollContext<'a> { + PollContext { + checker, state_path, push_time: "2026-05-01T00:00:00Z", fix_push_time: None, pr_info, - rate_limit_config: &rate_limit_config, - classifier_config: &classifier_config, - start: std::time::Instant::now(), - max_duration: 600, + rate_limit_config, + classifier_config, skip_ci: false, skip_coderabbit: false, - initial_review_wait_secs: 300, - review_recheck_wait_secs: 300, - max_review_rechecks: 3, - }; - finalize_initial_review_park(&ctx) + } } - /// Bb-2 (T2-2) + Bb-3 follow-up: 3 つの finalize_* park sibling - /// (`finalize_parked` / `schedule_next_review_recheck_park` / `finalize_initial_review_park`) - /// は全て write_state 失敗で `action_required` を返す invariant を 1 テストで - /// machine-enforce する。新 finalize_* 関数を追加する際、本テストが落ちて - /// invariant 維持を強制する。 + /// PR 3 (wakeup 廃止): 未確定時は park ではなく terminal な `pending_review` を返し、 + /// state にも同 action が永続化されること。summary は GitHub Actions 経路への + /// 引き継ぎを明示する (ADR-064: pending を silent success に見せない)。 #[test] - fn finalize_park_siblings_have_symmetric_write_state_handling() { - let bad_path = unwritable_state_path(); + fn finalize_pending_review_returns_terminal_pending_action() { + let tmp = tempfile::tempdir().unwrap(); + let state_path = tmp.path().join("state.json"); + let mut seeded = + PrMonitorState::new(Some(42), Some("o/r".into()), "2026-05-01T00:00:00Z".into()); + seeded.fix_push_time = Some("2026-05-01T00:05:00Z".into()); + crate::state::write_state_to(&state_path, &seeded).unwrap(); let pr_info = crate::util::PrInfo { - pr_number: Some(1), + pr_number: Some(42), repo: Some("o/r".into()), push_time: Some("2026-05-01T00:00:00Z".into()), - head_commit: None, + head_commit: Some("abc1234".into()), fix_push_time: None, }; + let checker = std::path::PathBuf::from("dummy"); + let rate_limit_config = RateLimitConfig::default(); + let classifier_config = ClassifierConfig::default(); + let ctx = make_ctx( + &checker, + &state_path, + &pr_info, + &rate_limit_config, + &classifier_config, + ); - let outcome_rate_limit = invoke_finalize_parked_with_bad_path(&pr_info, &bad_path); - let outcome_review = invoke_review_park_with_bad_path(&pr_info, &bad_path); - let outcome_initial = - invoke_finalize_initial_review_park_with_bad_path(&pr_info, &bad_path); + let outcome = finalize_pending_review(&ctx); + let persisted = crate::state::read_state_from(&state_path).unwrap(); - assert_eq!( - outcome_rate_limit.action, "action_required", - "finalize_parked: write_state 失敗 → action_required" - ); - assert_eq!( - outcome_review.action, "action_required", - "schedule_next_review_recheck_park: write_state 失敗 → action_required" + assert_eq!(outcome.action, "pending_review"); + assert!( + outcome.summary.contains("GitHub Actions"), + "summary は後続処理の引き継ぎ先を明示すること: {}", + outcome.summary ); + assert_eq!(persisted.action, "pending_review"); assert_eq!( - outcome_initial.action, "action_required", - "finalize_initial_review_park: write_state 失敗 → action_required" + persisted.head_commit.as_deref(), + Some("abc1234"), + "head_commit は state 継続判定 (should_continue_state) 用に保存されること" ); assert_eq!( - outcome_rate_limit.action, outcome_review.action, - "sibling parity (rate_limit ↔ review_recheck)" + persisted.fix_push_time.as_deref(), + Some("2026-05-01T00:05:00Z"), + "write-once: 既存 fix_push_time を上書きしないこと (順位 141)" ); - assert_eq!( - outcome_review.action, outcome_initial.action, - "sibling parity (review_recheck ↔ initial_review)" + } + + /// state 書き込み失敗でも panic せず pending_review を返すこと (fail-open: + /// 監視は助言層であり、state 永続化失敗で監視結果自体を失わない)。 + #[test] + fn finalize_pending_review_survives_write_failure() { + let bad_path = std::env::temp_dir() + .join(format!("pr-monitor-pr3-{}", std::process::id())) + .join("nonexistent-dir") + .join("state.json"); + + let pr_info = crate::util::PrInfo { + pr_number: Some(1), + repo: Some("o/r".into()), + push_time: Some("2026-05-01T00:00:00Z".into()), + head_commit: None, + fix_push_time: None, + }; + let checker = std::path::PathBuf::from("dummy"); + let rate_limit_config = RateLimitConfig::default(); + let classifier_config = ClassifierConfig::default(); + let ctx = make_ctx( + &checker, + &bad_path, + &pr_info, + &rate_limit_config, + &classifier_config, ); + + let outcome = finalize_pending_review(&ctx); + assert_eq!(outcome.action, "pending_review"); } } 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..4dcc78b5 100644 --- a/src/cli-pr-monitor/src/stages/poll/rate_limit.rs +++ b/src/cli-pr-monitor/src/stages/poll/rate_limit.rs @@ -1,14 +1,19 @@ -//! rate-limit 検出 branch と関連 helper (PR B refactor で `mod.rs` から切り出し)。 +//! rate-limit 検出 branch と関連 helper。 //! //! - `handle_rate_limit_branch` + `dispatch_rate_limit_outcome` (branch entry) -//! - `finalize_posted_retrigger` / `finalize_parked` (state finalize) //! - `handle_rate_limit_retry` / `post_review_immediately` (retry logic) //! - `RateLimitOutcome` (enum) -//! - `make_max_retries_result` / `make_action_required_result` (general result builders、 -//! review_recheck.rs からも参照される) +//! - `make_max_retries_result` / `make_action_required_result` (general result builders) +//! - `emit_shortcut_signal_if_eligible` / `fetch_mergeable_status` / +//! `evaluate_rate_limit_shortcut` / `format_shortcut_signal` (順位 141 shortcut。旧 +//! `rate_limit_signal.rs` の PARK signal 整形部分は park モデルごと撤去したが、この +//! shortcut は park の付随物ではなく「rate-limit 中でも既に mergeable なら即 merge を +//! 選べる」独立機能のため、terminal 化した `finalize_waiting_reset` に引き続き残す) //! -//! signal 整形部分 (`format_park_signal` / shortcut signal / -//! `format_posted_retrigger_review_park_signal`) は `rate_limit_signal.rs` に分離。 +//! WP-17 PR 3 (wakeup 廃止) で park モデルを撤去した。旧実装は reset 時刻が未来の場合に +//! state へ wakeup を書き PARK signal を出していたが、single-shot モデルでは +//! 「rate-limit 中である」ことを terminal に報告して終了する (`finalize_waiting_reset`)。 +//! reset 後の再レビューは CodeRabbit の後続イベント → GitHub Actions 経路が処理する。 use std::path::Path; @@ -18,22 +23,17 @@ use crate::runner::run_gh_quiet; use crate::state::{write_state_to, PrMonitorState}; use crate::util::PrInfo; -use super::rate_limit_signal::{ - emit_shortcut_signal_if_eligible, format_park_signal, - format_posted_retrigger_review_park_signal, -}; -use super::{make_park_poll_result, PollResult}; +use super::PollResult; /// rate-limit 検出 branch を集約する。 /// -/// dedup: 同一 rate-limit comment は iteration を跨いで残るため `comment_event_time` -/// で dedup する。dedup なしでは即時 retrigger を秒単位で繰り返し max_retries を浪費する。 +/// dedup: 同一 rate-limit comment は invocation を跨いで残るため `comment_event_time` +/// で dedup する。dedup なしでは即時 retrigger を繰り返し max_retries を浪費する。 /// CR が新たな rate-limit comment を投稿すると event_time が変わり再 handle 対象になる。 pub(super) fn handle_rate_limit_branch( state: &mut PrMonitorState, rate_limit_config: &RateLimitConfig, pr_info: &PrInfo, - review_recheck_wait_secs: u64, result: &serde_json::Value, state_path: &Path, ) -> Option { @@ -46,7 +46,9 @@ pub(super) fn handle_rate_limit_branch( "[rate_limit] 同じ rate-limit comment ({}) は処理済み、retrigger スキップ", rl.comment_event_time )); - return None; + return Some(finalize_waiting_reset( + state, &rl, pr_info, result, state_path, + )); } if state.rate_limit_retries >= rate_limit_config.max_retries { @@ -58,84 +60,63 @@ pub(super) fn handle_rate_limit_branch( } if !rate_limit_config.auto_retry_enabled { - return None; + return Some(finalize_waiting_reset( + state, &rl, pr_info, result, state_path, + )); } - dispatch_rate_limit_outcome( - state, - &rl, - pr_info, - rate_limit_config.max_retries, - review_recheck_wait_secs, - result, - state_path, - ) + Some(dispatch_rate_limit_outcome( + state, &rl, pr_info, result, state_path, + )) } fn dispatch_rate_limit_outcome( state: &mut PrMonitorState, rl: &crate::state::RateLimitState, pr_info: &PrInfo, - max_retries: u32, - review_recheck_wait_secs: u64, result: &serde_json::Value, state_path: &Path, -) -> Option { - match handle_rate_limit_retry(rl, state, pr_info, max_retries) { - RateLimitOutcome::Posted => finalize_posted_retrigger( - state, - rl, - pr_info, - review_recheck_wait_secs, - result, - state_path, - ), - RateLimitOutcome::Parked { wakeup_at_unix } => Some(finalize_parked( - state, - rl, - pr_info, - wakeup_at_unix, - max_retries, - result, - state_path, - )), +) -> PollResult { + match handle_rate_limit_retry(rl, state, pr_info) { + RateLimitOutcome::Posted => { + finalize_posted_retrigger(state, rl, pr_info, result, state_path) + } + RateLimitOutcome::WaitingReset => { + finalize_waiting_reset(state, rl, pr_info, result, state_path) + } RateLimitOutcome::Failed(e) => { log_info(&format!("[rate_limit] retrigger 失敗: {}", e)); - Some(make_action_required_result( + make_action_required_result( state, result, &format!( "rate-limit 自動 retry 失敗 ({})。手動で `@coderabbitai review` を投稿してください", e ), - )) + ) } } } -pub(super) fn finalize_posted_retrigger( +/// retrigger を投稿した後の terminal 化。 +/// +/// 旧実装は「retrigger 後の review 完了待ち」を park していたが (順位 80 fix)、 +/// single-shot モデルでは retrigger 済みであることを報告して終了する。 +/// 再レビュー到着は GitHub Actions 経路が処理する。silent exit ではない点は +/// 旧実装と同じ (ADR-064)。 +fn finalize_posted_retrigger( state: &mut PrMonitorState, rl: &crate::state::RateLimitState, pr_info: &PrInfo, - review_recheck_wait_secs: u64, result: &serde_json::Value, state_path: &Path, -) -> Option { +) -> PollResult { state.rate_limit_last_retriggered_at = Some(rl.comment_event_time.clone()); - - let now_unix = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs() as i64) - .unwrap_or(0); - let park_at_unix = now_unix + review_recheck_wait_secs as i64; - - state.action = "parked_review_recheck".into(); - state.next_wakeup_at_unix = Some(park_at_unix); - state.wakeup_reason = Some("rate_limit_post_retrigger".into()); - state.head_commit = pr_info.head_commit.clone(); + state.record_head_commit(pr_info.head_commit.as_deref()); + state.action = "pending_review".into(); state.summary = format!( - "rate-limit retrigger 後の review 完了待ちを {}s 後に予約 (順位 80 fix: silent exit 防止)", - review_recheck_wait_secs + "rate-limit へ retrigger を投稿 (retry={}/state 参照)。review 再実行の後続は GitHub Actions 経路が処理", + state.rate_limit_retries ); if let Err(e) = write_state_to(state_path, state) { @@ -143,47 +124,80 @@ pub(super) fn finalize_posted_retrigger( "[rate_limit] retrigger 後の state 永続化失敗、自動 retry を停止: {}", e )); - return Some(make_action_required_result( + return make_action_required_result( state, result, &format!( "rate-limit retry 後の state 永続化に失敗 ({})。手動で `@coderabbitai review` の重複投稿に注意してください", e ), - )); + ); } - let signal = format_posted_retrigger_review_park_signal(state, pr_info); - 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(), + } +} - Some(make_park_poll_result(state.clone())) +/// reset 時刻が未来 (= いま retrigger しても弾かれる) 場合の terminal 化。 +/// +/// 「rate-limit 中でレビュー未実施」を loud に報告して終了する (ADR-064 (b): +/// レポート判定文の保留保証)。reset 後の再レビューは CodeRabbit の後続イベント → +/// GitHub Actions 経路が処理し、ローカルの時限 wakeup は使わない (WP-17 PR 3)。 +/// +/// state 書き込みの成否に関わらず、順位 141 の mergeable shortcut 判定 +/// (`emit_shortcut_signal_if_eligible`) は実行する — shortcut は「今すぐ merge するか」 +/// を stdout 経由でユーザーに問う独立した通知であり、この terminal 化自体の成否には +/// 依存しない。 +/// +/// **state 書き込み失敗時は fail-open** (log のみで続行し、通常どおり `rate_limited` を +/// 返す) — このパスは `finalize_posted_retrigger` と異なり `@coderabbitai review` 投稿 +/// のような副作用を伴わないため、書き込み失敗を理由に checker 呼び出し済みの判定結果を +/// 破棄する必要がない (`finalize_pending_review` (mod.rs) と同じ fail-open 方針)。 +/// head_commit の継続性 (`should_continue_state`) が失われるリスクは残るが、次回 +/// `--monitor-only` 再実行時に fresh 初期化へ倒れるだけで、`finalize_posted_retrigger` の +/// 二重投稿リスクのような不可逆な問題にはならない (SIM-NEW-rate_limit-L158)。 +fn finalize_waiting_reset( + state: &mut PrMonitorState, + rl: &crate::state::RateLimitState, + pr_info: &PrInfo, + result: &serde_json::Value, + state_path: &Path, +) -> PollResult { + finalize_waiting_reset_with(state, rl, pr_info, result, state_path, fetch_mergeable_status) } -pub(super) fn finalize_parked( +/// [`finalize_waiting_reset`] の本体。mergeable 取得を注入可能にして shell (gh) 層を +/// テストから外す (CodeRabbit #353: テストが実 `gh` を起動していた。 +/// `verify_diff_covers_pr_range` (cli-push-runner) と同じ注入の流儀)。 +fn finalize_waiting_reset_with( state: &mut PrMonitorState, rl: &crate::state::RateLimitState, pr_info: &PrInfo, - wakeup_at_unix: i64, - max_retries: u32, result: &serde_json::Value, state_path: &Path, + fetch_mergeable: impl FnOnce(&PrInfo) -> Option, ) -> 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.head_commit = pr_info.head_commit.clone(); + state.action = "rate_limited".into(); state.summary = format!( - "CodeRabbit rate-limit: wakeup を {}m{}s 後に予約 (PARK signal 参照)", + "CodeRabbit rate-limit 中 (残り約 {}m{}s)。reset 後の再レビューは GitHub Actions 経路が処理", rl.wait_minutes, rl.wait_seconds ); + state.record_head_commit(pr_info.head_commit.as_deref()); if let Err(e) = write_state_to(state_path, state) { - let msg = format!("park state 永続化失敗のため PARK signal を中止 ({})。手動で `@coderabbitai review` を投稿してください", e); - return make_action_required_result(state, result, &msg); + log_info(&format!( + "state 書き込み失敗 (rate_limited 確定後、続行): {}", + e + )); } - let signal = format_park_signal(state, rl, pr_info, max_retries); - println!("{}", signal); - emit_shortcut_signal_if_eligible(state, rl, pr_info); + emit_shortcut_signal_if_eligible(state, rl, pr_info, fetch_mergeable); PollResult { action: state.action.clone(), @@ -196,6 +210,106 @@ pub(super) fn finalize_parked( } } +/// 順位 141: rate-limit 検出 + mergeable CLEAN + CR 全フィールドクリーンの条件が揃ったとき +/// `[RATE_LIMIT_BUT_MERGEABLE]` signal を stdout に出力する shortcut path。 +/// +/// gh への問い合わせが失敗する / 条件を満たさない場合は何も出力しない (fail-safe: 通常の +/// `rate_limited` terminal 報告のみで完結し、shortcut 不在は機能低下であって障害ではない)。 +fn emit_shortcut_signal_if_eligible( + state: &PrMonitorState, + rl: &crate::state::RateLimitState, + pr_info: &PrInfo, + fetch_mergeable: impl FnOnce(&PrInfo) -> Option, +) { + let Some(mergeable) = fetch_mergeable(pr_info) else { + return; + }; + if !evaluate_rate_limit_shortcut(state.coderabbit.as_ref(), &mergeable) { + return; + } + println!("{}", format_shortcut_signal(rl, pr_info, &mergeable)); +} + +/// 順位 141: PR の mergeable / mergeStateStatus を gh で取得。失敗時は None。 +fn fetch_mergeable_status(pr_info: &PrInfo) -> Option { + let pr = pr_info.pr_number?; + let pr_str = pr.to_string(); + let mut args: Vec<&str> = vec![ + "pr", + "view", + &pr_str, + "--json", + "mergeable,mergeStateStatus", + ]; + if let Some(repo) = pr_info.repo.as_deref() { + args.push("--repo"); + args.push(repo); + } + let json_str = run_gh_quiet(&args)?; + let parsed: serde_json::Value = serde_json::from_str(&json_str).ok()?; + Some(MergeableStatus { + mergeable: parsed.get("mergeable")?.as_str()?.to_string(), + merge_state: parsed.get("mergeStateStatus")?.as_str()?.to_string(), + }) +} + +/// 順位 141: mergeable + CR 全フィールドクリーンの条件評価を pure 関数化 (test 容易性)。 +fn evaluate_rate_limit_shortcut( + coderabbit: Option<&crate::state::CodeRabbitState>, + mergeable: &MergeableStatus, +) -> bool { + let cr_clean = coderabbit + .map(|c| { + c.new_comments == 0 + && c.actionable_comments.unwrap_or(0) == 0 + && c.unresolved_threads.unwrap_or(0) == 0 + }) + .unwrap_or(true); + mergeable.mergeable == "MERGEABLE" && mergeable.merge_state == "CLEAN" && cr_clean +} + +/// 順位 141: `[RATE_LIMIT_BUT_MERGEABLE]` signal を構築 (pure)。 +fn format_shortcut_signal( + rl: &crate::state::RateLimitState, + pr_info: &PrInfo, + mergeable: &MergeableStatus, +) -> String { + let pr = pr_info + .pr_number + .map(|n| n.to_string()) + .unwrap_or_else(|| "?".into()); + let repo = pr_info.repo.as_deref().unwrap_or("?"); + let reset_iso = if rl.until_unix_secs > 0 { + lib_pending_file::epoch_secs_to_iso8601(rl.until_unix_secs as u64) + } else { + "?".into() + }; + let wait_total_secs = rl.wait_minutes * 60 + rl.wait_seconds; + format!( + "[RATE_LIMIT_BUT_MERGEABLE] +pr: {pr} +repo: {repo} +rate_limit_reset_at_iso_utc: {reset_iso} +rate_limit_wait_seconds: {wait_total_secs} +mergeable: {merge} +merge_state: {state} + +ACTION REQUIRED: ユーザーに以下 2 択を AskUserQuestion で問うこと: + A: 今すぐ merge する (rate-limit reset を待たない、CR 2 回目 review なしで進める) + B: reset 後の再レビュー到着を待つ (後続は GitHub Actions 経路のコメント、または reset 後の --monitor-only 再実行で把握する) +[/RATE_LIMIT_BUT_MERGEABLE]", + merge = mergeable.mergeable, + state = mergeable.merge_state, + ) +} + +/// 順位 141: gh `pr view --json mergeable,mergeStateStatus` の結果を保持する DTO。 +#[derive(Debug, Clone)] +struct MergeableStatus { + mergeable: String, + merge_state: String, +} + fn make_max_retries_result(state: &PrMonitorState, result: &serde_json::Value) -> PollResult { let summary = format!( "CodeRabbit rate-limit が {} 回再試行後も継続。手動で `@coderabbitai review` を投稿してください", @@ -220,10 +334,12 @@ pub(super) fn make_action_required_result( } } -/// `handle_rate_limit_retry` の outcome 種別 (Bb-1, Bundle b PR-1)。 +/// `handle_rate_limit_retry` の outcome 種別。 pub(crate) enum RateLimitOutcome { + /// 即時 retrigger を投稿した (reset 時刻は既に過去だった)。 Posted, - Parked { wakeup_at_unix: i64 }, + /// reset 時刻が未来のため何もしない (terminal な rate_limited 報告へ)。 + WaitingReset, Failed(String), } @@ -232,7 +348,6 @@ pub(super) fn handle_rate_limit_retry( rl: &crate::state::RateLimitState, state: &mut PrMonitorState, pr_info: &PrInfo, - max_retries: u32, ) -> RateLimitOutcome { let now_unix = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) @@ -246,16 +361,10 @@ pub(super) fn handle_rate_limit_retry( if sleep_secs > 0 { log_info(&format!( - "[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 + "[rate_limit] reset まで {}秒 (wait={}m{}s + 60s buffer)、rate_limited として終了", + sleep_secs, rl.wait_minutes, rl.wait_seconds )); - return RateLimitOutcome::Parked { - wakeup_at_unix: rl.until_unix_secs, - }; + return RateLimitOutcome::WaitingReset; } post_review_immediately(pr, state) @@ -284,318 +393,5 @@ fn post_review_immediately(pr: u64, state: &mut PrMonitorState) -> RateLimitOutc } #[cfg(test)] -mod tests { - use super::*; - use crate::state::RateLimitState; - - #[test] - fn rate_limit_state_persists_retries_across_polls() { - let tmp = std::env::temp_dir().join(format!("test-rl-retries-{}.json", std::process::id())); - let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); - state.rate_limit_retries = 2; - state.rate_limit = Some(RateLimitState { - until_unix_secs: 1_735_689_600, - comment_event_time: "2026-04-30T00:00:00Z".into(), - wait_minutes: 5, - wait_seconds: 13, - }); - crate::state::write_state_to(&tmp, &state).unwrap(); - - let loaded = crate::state::read_state_from(&tmp).unwrap(); - assert_eq!(loaded.rate_limit_retries, 2); - assert_eq!( - loaded.rate_limit.as_ref().unwrap().until_unix_secs, - 1_735_689_600 - ); - - let _ = std::fs::remove_file(&tmp); - } - - #[test] - fn rate_limit_default_config_allows_retry_within_limit() { - let cfg = RateLimitConfig::default(); - assert!(cfg.auto_retry_enabled); - assert_eq!(cfg.max_retries, 3); - assert!(2 < cfg.max_retries); - assert!(3 >= cfg.max_retries); - } - - /// 同じ rate-limit comment が iteration 跨ぎで残った場合に dedup が働くことを検証する。 - /// - /// シナリオ (advisor 発見のバグ): - /// - Iter 1: comment A, retries=0, last_retriggered=None → handle 対象 - /// - Iter 2: 同じ comment A still in PR, last_retriggered=A → 即時 retrigger を skip - /// - Iter 3: CR が新たな rate-limit comment B を投稿, last_retriggered=A != B → 再 handle 対象 - /// - /// dedup なしだと Iter 2/3 で sleep_secs=0 となり数秒で max_retries を消費する。 - #[test] - fn rate_limit_dedup_skips_repeated_comment() { - let comment_a = "2026-04-30T00:00:00Z"; - let comment_b = "2026-04-30T00:30:00Z"; - - let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); - let rl_a = RateLimitState { - until_unix_secs: 0, - comment_event_time: comment_a.into(), - wait_minutes: 5, - wait_seconds: 0, - }; - let already_handled_iter1 = state.rate_limit_last_retriggered_at.as_deref() - == Some(rl_a.comment_event_time.as_str()); - assert!( - !already_handled_iter1, - "Iter 1: 初回 detection は handle されるべき" - ); - - state.rate_limit_retries = 1; - state.rate_limit_last_retriggered_at = Some(comment_a.into()); - - let already_handled_iter2 = state.rate_limit_last_retriggered_at.as_deref() - == Some(rl_a.comment_event_time.as_str()); - assert!( - already_handled_iter2, - "Iter 2: 同じ comment は dedup で skip されるべき" - ); - - let rl_b = RateLimitState { - until_unix_secs: 0, - comment_event_time: comment_b.into(), - wait_minutes: 5, - wait_seconds: 0, - }; - let already_handled_iter3 = state.rate_limit_last_retriggered_at.as_deref() - == Some(rl_b.comment_event_time.as_str()); - assert!( - !already_handled_iter3, - "Iter 3: 新 comment は再度 handle 対象" - ); - } - - /// state.json round-trip で rate_limit_last_retriggered_at が persistence される。 - #[test] - fn rate_limit_last_retriggered_at_persists_across_polls() { - let tmp = - std::env::temp_dir().join(format!("test-rl-last-handled-{}.json", std::process::id())); - let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); - state.rate_limit_last_retriggered_at = Some("2026-04-30T00:00:00Z".into()); - crate::state::write_state_to(&tmp, &state).unwrap(); - - let loaded = crate::state::read_state_from(&tmp).unwrap(); - assert_eq!( - loaded.rate_limit_last_retriggered_at.as_deref(), - Some("2026-04-30T00:00:00Z") - ); - - let _ = std::fs::remove_file(&tmp); - } - - /// Bb-1: reset 時刻が未来の場合、`handle_rate_limit_retry` は Parked を返し - /// state.rate_limit_retries を変更しない (実 retry 計上は wakeup 経由で post 投稿後)。 - #[test] - fn rate_limit_retry_returns_parked_when_reset_in_future() { - let future_unix = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_secs() as i64 - + 600; - let rl = RateLimitState { - until_unix_secs: future_unix, - comment_event_time: "2026-04-30T00:00:00Z".into(), - wait_minutes: 10, - wait_seconds: 0, - }; - let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); - let pr_info = crate::util::PrInfo { - pr_number: Some(42), - repo: Some("o/r".into()), - push_time: None, - head_commit: None, - fix_push_time: None, - }; - - let outcome = handle_rate_limit_retry(&rl, &mut state, &pr_info, 3); - match outcome { - RateLimitOutcome::Parked { wakeup_at_unix } => { - assert_eq!(wakeup_at_unix, future_unix); - } - _ => panic!("expected Parked outcome for future reset, got other variant"), - } - assert_eq!(state.rate_limit_retries, 0); - assert!(state.rate_limit_last_retriggered_at.is_none()); - } - - /// Bb-1: PR 番号未確定の場合、`handle_rate_limit_retry` は Failed を返し - /// state を変更しない (caller は action_required で抜ける)。 - #[test] - fn rate_limit_retry_returns_failed_when_pr_number_missing() { - let past_unix = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_secs() as i64 - - 60; - let rl = RateLimitState { - until_unix_secs: past_unix, - comment_event_time: "2026-04-30T00:00:00Z".into(), - wait_minutes: 0, - wait_seconds: 0, - }; - let mut state = PrMonitorState::new(None, None, "t".into()); - let pr_info = crate::util::PrInfo { - pr_number: None, - repo: None, - push_time: None, - head_commit: None, - fix_push_time: None, - }; - - let outcome = handle_rate_limit_retry(&rl, &mut state, &pr_info, 3); - assert!(matches!(outcome, RateLimitOutcome::Failed(_))); - assert_eq!(state.rate_limit_retries, 0); - assert!(state.rate_limit_last_retriggered_at.is_none()); - } - - /// 書き込み先がディレクトリ不在のため write が必ず失敗する path を返す。 - fn unwritable_state_path() -> std::path::PathBuf { - std::env::temp_dir() - .join(format!("pr-monitor-T2-2-{}", std::process::id())) - .join("nonexistent-dir") - .join("state.json") - } - - /// Bb-1 (T2-2): `finalize_parked` は write_state 失敗時に PARK signal emit を中止し - /// `action_required` を返却する fail-safe 経路を持つ (CodeRabbit Major #1 fix の固定化)。 - #[test] - fn finalize_parked_returns_action_required_when_write_state_fails() { - let bad_path = unwritable_state_path(); - - let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); - let rl = RateLimitState { - until_unix_secs: 1_775_088_000, - comment_event_time: "2026-05-01T00:00:00Z".into(), - wait_minutes: 47, - wait_seconds: 0, - }; - let pr_info = crate::util::PrInfo { - pr_number: Some(42), - repo: Some("o/r".into()), - push_time: None, - head_commit: None, - fix_push_time: None, - }; - let result = serde_json::json!({}); - - let outcome = finalize_parked( - &mut state, - &rl, - &pr_info, - 1_775_088_000, - 3, - &result, - &bad_path, - ); - - assert_eq!( - outcome.action, "action_required", - "T2-2: write_state 失敗 → action_required で抜ける fail-safe が必要" - ); - assert!( - outcome.summary.contains("PARK signal を中止") - || outcome.summary.contains("永続化失敗"), - "summary に永続化失敗の説明が含まれること: {}", - outcome.summary - ); - } - - fn setup_posted_retrigger_fixture() -> (PrMonitorState, RateLimitState, crate::util::PrInfo) { - let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); - state.action = "continue_monitoring".into(); - state.rate_limit_retries = 1; - let rl = RateLimitState { - until_unix_secs: 0, - comment_event_time: "2026-05-08T00:00:00Z".into(), - wait_minutes: 5, - wait_seconds: 0, - }; - let pr_info = crate::util::PrInfo { - pr_number: Some(1), - repo: Some("o/r".into()), - push_time: Some("2026-05-01T00:00:00Z".into()), - head_commit: Some("abc1234".into()), - fix_push_time: None, - }; - (state, rl, pr_info) - } - - #[test] - fn finalize_posted_retrigger_schedules_park_after_post() { - let tmp = tempfile::tempdir().unwrap(); - let state_path = tmp.path().join("state.json"); - - let (mut state, rl, pr_info) = setup_posted_retrigger_fixture(); - let result = finalize_posted_retrigger( - &mut state, - &rl, - &pr_info, - 300, - &serde_json::Value::Null, - &state_path, - ); - - let park_result = - result.expect("順位 80 fix: Posted 後は必ず park を返し silent exit を防ぐ"); - assert_eq!(park_result.action, "parked_review_recheck"); - assert_eq!( - state.wakeup_reason.as_deref(), - Some("rate_limit_post_retrigger") - ); - let now_unix = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_secs() as i64; - let wakeup = state - .next_wakeup_at_unix - .expect("next_wakeup_at_unix が設定される"); - assert!(wakeup > now_unix && wakeup <= now_unix + 301); - assert_eq!( - state.rate_limit_last_retriggered_at.as_deref(), - Some("2026-05-08T00:00:00Z") - ); - } - - #[test] - fn finalize_posted_retrigger_action_required_when_write_state_fails() { - let bad_path = unwritable_state_path(); - - let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); - state.action = "continue_monitoring".into(); - let rl = RateLimitState { - until_unix_secs: 0, - comment_event_time: "2026-05-08T00:00:00Z".into(), - wait_minutes: 5, - wait_seconds: 0, - }; - let pr_info = crate::util::PrInfo { - pr_number: Some(1), - repo: Some("o/r".into()), - push_time: Some("2026-05-01T00:00:00Z".into()), - head_commit: None, - fix_push_time: None, - }; - - let result = finalize_posted_retrigger( - &mut state, - &rl, - &pr_info, - 300, - &serde_json::Value::Null, - &bad_path, - ); - - assert!(result.is_some()); - assert_eq!( - result.unwrap().action, - "action_required", - "write_state 失敗時は action_required で抜ける (sibling parity with finalize_parked)" - ); - } -} +#[path = "rate_limit/tests.rs"] +mod tests; diff --git a/src/cli-pr-monitor/src/stages/poll/rate_limit/tests.rs b/src/cli-pr-monitor/src/stages/poll/rate_limit/tests.rs new file mode 100644 index 00000000..04679edf --- /dev/null +++ b/src/cli-pr-monitor/src/stages/poll/rate_limit/tests.rs @@ -0,0 +1,543 @@ +use super::*; +use crate::state::RateLimitState; + +/// pr_number: None の `PrInfo` — `emit_shortcut_signal_if_eligible` を早期 return させ、 +/// テストから実 gh CLI 呼び出し (ネットワーク依存) を発生させないための fixture。 +fn pr_info_without_shortcut() -> crate::util::PrInfo { + crate::util::PrInfo { + pr_number: None, + repo: None, + push_time: None, + head_commit: None, + fix_push_time: None, + } +} + +#[test] +fn rate_limit_state_persists_retries_across_polls() { + let tmp = std::env::temp_dir().join(format!("test-rl-retries-{}.json", std::process::id())); + let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); + state.rate_limit_retries = 2; + state.rate_limit = Some(RateLimitState { + until_unix_secs: 1_735_689_600, + comment_event_time: "2026-04-30T00:00:00Z".into(), + wait_minutes: 5, + wait_seconds: 13, + }); + crate::state::write_state_to(&tmp, &state).unwrap(); + + let loaded = crate::state::read_state_from(&tmp).unwrap(); + assert_eq!(loaded.rate_limit_retries, 2); + assert_eq!( + loaded.rate_limit.as_ref().unwrap().until_unix_secs, + 1_735_689_600 + ); + + let _ = std::fs::remove_file(&tmp); +} + +#[test] +fn rate_limit_default_config_allows_retry_within_limit() { + let cfg = RateLimitConfig::default(); + assert!(cfg.auto_retry_enabled); + assert_eq!(cfg.max_retries, 3); +} + +/// 同じ rate-limit comment が invocation 跨ぎで残った場合に dedup が働くことを検証する。 +#[test] +fn rate_limit_dedup_skips_repeated_comment() { + let comment_a = "2026-04-30T00:00:00Z"; + let comment_b = "2026-04-30T00:30:00Z"; + + let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); + let rl_a = RateLimitState { + until_unix_secs: 0, + comment_event_time: comment_a.into(), + wait_minutes: 5, + wait_seconds: 0, + }; + let already_handled_iter1 = state.rate_limit_last_retriggered_at.as_deref() + == Some(rl_a.comment_event_time.as_str()); + assert!(!already_handled_iter1, "初回 detection は handle されるべき"); + + state.rate_limit_retries = 1; + state.rate_limit_last_retriggered_at = Some(comment_a.into()); + + let already_handled_iter2 = state.rate_limit_last_retriggered_at.as_deref() + == Some(rl_a.comment_event_time.as_str()); + assert!( + already_handled_iter2, + "同じ comment は dedup で skip されるべき" + ); + + let rl_b = RateLimitState { + until_unix_secs: 0, + comment_event_time: comment_b.into(), + wait_minutes: 5, + wait_seconds: 0, + }; + let already_handled_iter3 = state.rate_limit_last_retriggered_at.as_deref() + == Some(rl_b.comment_event_time.as_str()); + assert!(!already_handled_iter3, "新 comment は再度 handle 対象"); +} + +/// state.json round-trip で rate_limit_last_retriggered_at が persistence される。 +#[test] +fn rate_limit_last_retriggered_at_persists_across_polls() { + let tmp = + std::env::temp_dir().join(format!("test-rl-last-handled-{}.json", std::process::id())); + let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); + state.rate_limit_last_retriggered_at = Some("2026-04-30T00:00:00Z".into()); + crate::state::write_state_to(&tmp, &state).unwrap(); + + let loaded = crate::state::read_state_from(&tmp).unwrap(); + assert_eq!( + loaded.rate_limit_last_retriggered_at.as_deref(), + Some("2026-04-30T00:00:00Z") + ); + + let _ = std::fs::remove_file(&tmp); +} + +/// PR 3 (wakeup 廃止): reset 時刻が未来の場合、`handle_rate_limit_retry` は +/// WaitingReset を返し state.rate_limit_retries を変更しない (旧 Parked 相当。 +/// wakeup 時刻は返さない — park しないため不要)。 +#[test] +fn rate_limit_retry_returns_waiting_reset_when_reset_in_future() { + let future_unix = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs() as i64 + + 600; + let rl = RateLimitState { + until_unix_secs: future_unix, + comment_event_time: "2026-04-30T00:00:00Z".into(), + wait_minutes: 10, + wait_seconds: 0, + }; + let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); + let pr_info = crate::util::PrInfo { + pr_number: Some(42), + repo: Some("o/r".into()), + push_time: None, + head_commit: None, + fix_push_time: None, + }; + + let outcome = handle_rate_limit_retry(&rl, &mut state, &pr_info); + assert!(matches!(outcome, RateLimitOutcome::WaitingReset)); + assert_eq!(state.rate_limit_retries, 0); + assert!(state.rate_limit_last_retriggered_at.is_none()); +} + +/// PR 番号未確定の場合、`handle_rate_limit_retry` は Failed を返し +/// state を変更しない (caller は action_required で抜ける)。 +#[test] +fn rate_limit_retry_returns_failed_when_pr_number_missing() { + let past_unix = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs() as i64 + - 60; + let rl = RateLimitState { + until_unix_secs: past_unix, + comment_event_time: "2026-04-30T00:00:00Z".into(), + wait_minutes: 0, + wait_seconds: 0, + }; + let mut state = PrMonitorState::new(None, None, "t".into()); + let pr_info = crate::util::PrInfo { + pr_number: None, + repo: None, + push_time: None, + head_commit: None, + fix_push_time: None, + }; + + let outcome = handle_rate_limit_retry(&rl, &mut state, &pr_info); + assert!(matches!(outcome, RateLimitOutcome::Failed(_))); + assert_eq!(state.rate_limit_retries, 0); + assert!(state.rate_limit_last_retriggered_at.is_none()); +} + +/// PR 3 (wakeup 廃止): reset 未来の rate-limit は terminal な `rate_limited` action で +/// 報告される。summary は保留 (レビュー未実施) と後続の引き継ぎ先を明示する +/// (ADR-064 (b): レポート判定文の保留保証を Actions 経路へ引き継ぐまでの間も維持)。 +#[test] +fn finalize_waiting_reset_reports_rate_limited_terminal() { + let tmp = tempfile::tempdir().unwrap(); + let state_path = tmp.path().join("state.json"); + let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); + state.rate_limit = Some(RateLimitState { + until_unix_secs: 9_999_999_999, + comment_event_time: "2026-08-03T00:00:00Z".into(), + wait_minutes: 47, + wait_seconds: 10, + }); + let rl = state.rate_limit.clone().unwrap(); + let pr_info = pr_info_without_shortcut(); + + let outcome = finalize_waiting_reset_with( + &mut state, + &rl, + &pr_info, + &serde_json::Value::Null, + &state_path, + |_| None, + ); + + assert_eq!(outcome.action, "rate_limited"); + assert!( + outcome.summary.contains("rate-limit 中"), + "レビュー未実施であることを明示: {}", + outcome.summary + ); + assert!( + outcome.summary.contains("GitHub Actions"), + "後続の引き継ぎ先を明示: {}", + outcome.summary + ); + assert!( + outcome.rate_limit.is_some(), + "rate_limit を伝播し caller (monitor.rs) が takt invoke を skip できること (#C-3)" + ); + let persisted = crate::state::read_state_from(&state_path).unwrap(); + assert_eq!(persisted.action, "rate_limited"); +} + +/// state-continuity-drop 回帰 (SIM-NEW-rate_limit-L154): `finalize_waiting_reset` は +/// state.head_commit を pr_info から persist する。これが欠けると +/// `should_continue_state` (monitor.rs) が次回 `--monitor-only` 再実行時に +/// legacy state (head_commit None) 扱いで fresh 初期化に倒れ、push_time / +/// fix_push_time が「今」にリセットされて間に届いた CR コメントを取りこぼす。 +#[test] +fn finalize_waiting_reset_persists_head_commit() { + let tmp = tempfile::tempdir().unwrap(); + let state_path = tmp.path().join("state.json"); + let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); + state.rate_limit = Some(RateLimitState { + until_unix_secs: 9_999_999_999, + comment_event_time: "2026-08-03T00:00:00Z".into(), + wait_minutes: 47, + wait_seconds: 10, + }); + let rl = state.rate_limit.clone().unwrap(); + let pr_info = crate::util::PrInfo { + pr_number: Some(42), + repo: Some("o/r".into()), + push_time: None, + head_commit: Some("deadbeef".into()), + fix_push_time: None, + }; + + let outcome = finalize_waiting_reset_with( + &mut state, + &rl, + &pr_info, + &serde_json::Value::Null, + &state_path, + |_| None, + ); + + assert_eq!(outcome.action, "rate_limited"); + assert_eq!(state.head_commit.as_deref(), Some("deadbeef")); + let persisted = crate::state::read_state_from(&state_path).unwrap(); + assert_eq!( + persisted.head_commit.as_deref(), + Some("deadbeef"), + "head_commit が persist されないと should_continue_state が次回 fresh 初期化に倒れる" + ); +} + +/// fail-open 回帰 (SIM-NEW-rate_limit-L158): state 書き込み失敗でも +/// `finalize_waiting_reset` は `action_required` に倒れず通常どおり +/// `rate_limited` を返す (`finalize_pending_review` と同じ fail-open 方針。 +/// 副作用を伴わないこの経路は checker 判定を書き込み失敗で破棄しない)。 +#[test] +fn finalize_waiting_reset_survives_write_failure() { + let bad_path = std::env::temp_dir() + .join(format!("test-rl-waiting-reset-fail-{}", std::process::id())) + .join("nonexistent-dir") + .join("state.json"); + let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); + state.rate_limit = Some(RateLimitState { + until_unix_secs: 9_999_999_999, + comment_event_time: "2026-08-03T00:00:00Z".into(), + wait_minutes: 47, + wait_seconds: 10, + }); + let rl = state.rate_limit.clone().unwrap(); + let pr_info = pr_info_without_shortcut(); + + let outcome = finalize_waiting_reset_with( + &mut state, + &rl, + &pr_info, + &serde_json::Value::Null, + &bad_path, + |_| None, + ); + + assert_eq!( + outcome.action, "rate_limited", + "state 書き込み失敗時も action_required に倒れず rate_limited を維持すること" + ); +} + +/// state-continuity-drop 回帰 (SIM-NEW-rate_limit-L154): `finalize_posted_retrigger` も +/// 同様に head_commit を persist する (retrigger 投稿後の継続判定も同じ invariant に従う)。 +#[test] +fn finalize_posted_retrigger_persists_head_commit() { + let tmp = tempfile::tempdir().unwrap(); + let state_path = tmp.path().join("state.json"); + let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); + let rl = RateLimitState { + until_unix_secs: 0, + comment_event_time: "2026-08-03T00:00:00Z".into(), + wait_minutes: 0, + wait_seconds: 0, + }; + let pr_info = crate::util::PrInfo { + pr_number: Some(42), + repo: Some("o/r".into()), + push_time: None, + head_commit: Some("cafef00d".into()), + fix_push_time: None, + }; + + let outcome = + finalize_posted_retrigger(&mut state, &rl, &pr_info, &serde_json::Value::Null, &state_path); + + assert_eq!(outcome.action, "pending_review"); + assert_eq!(state.head_commit.as_deref(), Some("cafef00d")); + let persisted = crate::state::read_state_from(&state_path).unwrap(); + assert_eq!( + persisted.head_commit.as_deref(), + Some("cafef00d"), + "head_commit が persist されないと should_continue_state が次回 fresh 初期化に倒れる" + ); +} + +/// fail-closed 回帰 (SIM-NEW-rate_limit-L122): retrigger 投稿後の state 書き込み失敗は +/// `finalize_waiting_reset` と異なり `action_required` に倒れる — この経路は +/// `@coderabbitai review` 投稿という副作用を伴うため (L148-165 のコメント参照)、 +/// state 永続化の成否で重複投稿を防ぐ必要がある (`finalize_waiting_reset` の +/// fail-open とは対照的な fail-closed 方針)。 +#[test] +fn finalize_posted_retrigger_action_required_when_write_state_fails() { + let bad_path = std::env::temp_dir() + .join(format!( + "test-rl-posted-retrigger-fail-{}", + std::process::id() + )) + .join("nonexistent-dir") + .join("state.json"); + let mut state = PrMonitorState::new(Some(1), Some("o/r".into()), "t".into()); + let rl = RateLimitState { + until_unix_secs: 0, + comment_event_time: "2026-08-03T00:00:00Z".into(), + wait_minutes: 5, + wait_seconds: 0, + }; + let pr_info = crate::util::PrInfo { + pr_number: Some(1), + repo: Some("o/r".into()), + push_time: None, + head_commit: None, + fix_push_time: None, + }; + + let outcome = finalize_posted_retrigger( + &mut state, + &rl, + &pr_info, + &serde_json::Value::Null, + &bad_path, + ); + + assert_eq!( + outcome.action, "action_required", + "state 永続化失敗時は fail-closed で action_required に倒れること (重複投稿防止)" + ); + assert!( + outcome.summary.contains("重複投稿"), + "手動介入時に重複投稿への注意を促す文言が必要: {}", + outcome.summary + ); +} + +/// dedup 済み (同一 comment 処理済み) の rate-limit も terminal な rate_limited に +/// 落ちること — 旧実装はここで None を返し park に流れていた。 +#[test] +fn handle_rate_limit_branch_terminalizes_already_handled_comment() { + let tmp = tempfile::tempdir().unwrap(); + let state_path = tmp.path().join("state.json"); + let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); + state.rate_limit = Some(RateLimitState { + until_unix_secs: 9_999_999_999, + comment_event_time: "2026-08-03T00:00:00Z".into(), + wait_minutes: 5, + wait_seconds: 0, + }); + state.rate_limit_last_retriggered_at = Some("2026-08-03T00:00:00Z".into()); + let pr_info = pr_info_without_shortcut(); + + let outcome = handle_rate_limit_branch( + &mut state, + &RateLimitConfig::default(), + &pr_info, + &serde_json::Value::Null, + &state_path, + ); + + let terminal = outcome.expect("rate-limit active なら必ず terminal を返す (park しない)"); + assert_eq!(terminal.action, "rate_limited"); +} + +/// 順位 141: shortcut signal の trigger 条件 (mergeable CLEAN + unresolved 0) で true。 +#[test] +fn evaluate_rate_limit_shortcut_when_all_conditions_met() { + let m = MergeableStatus { + mergeable: "MERGEABLE".into(), + merge_state: "CLEAN".into(), + }; + let cr = crate::state::CodeRabbitState { + review_state: "approved".into(), + new_comments: 0, + actionable_comments: Some(0), + unresolved_threads: Some(0), + }; + assert!(evaluate_rate_limit_shortcut(Some(&cr), &m)); +} + +/// 順位 141: unresolved thread が残っていれば shortcut を抑止 (CR の指摘が未対応)。 +#[test] +fn evaluate_rate_limit_shortcut_blocks_when_unresolved_threads_exist() { + let m = MergeableStatus { + mergeable: "MERGEABLE".into(), + merge_state: "CLEAN".into(), + }; + let cr = crate::state::CodeRabbitState { + review_state: "commented".into(), + new_comments: 1, + actionable_comments: Some(1), + unresolved_threads: Some(1), + }; + assert!(!evaluate_rate_limit_shortcut(Some(&cr), &m)); +} + +/// 順位 141: new_comments > 0 のとき unresolved_threads が 0 でも shortcut を抑止。 +/// CR がまだコメントを処理中の状態で merge 判定を通過させない。 +#[test] +fn evaluate_rate_limit_shortcut_blocks_when_new_comments_exist() { + let m = MergeableStatus { + mergeable: "MERGEABLE".into(), + merge_state: "CLEAN".into(), + }; + let cr = crate::state::CodeRabbitState { + review_state: "commented".into(), + new_comments: 1, + actionable_comments: Some(0), + unresolved_threads: Some(0), + }; + assert!(!evaluate_rate_limit_shortcut(Some(&cr), &m)); +} + +/// 順位 141: mergeable が BLOCKED なら shortcut を抑止 (GitHub 側で merge 不可)。 +#[test] +fn evaluate_rate_limit_shortcut_blocks_when_not_mergeable() { + let m = MergeableStatus { + mergeable: "BLOCKED".into(), + merge_state: "BLOCKED".into(), + }; + assert!(!evaluate_rate_limit_shortcut(None, &m)); +} + +/// 順位 141: CR state が None (初回 review なし) でも mergeable CLEAN なら shortcut 可。 +#[test] +fn evaluate_rate_limit_shortcut_passes_when_coderabbit_none() { + let m = MergeableStatus { + mergeable: "MERGEABLE".into(), + merge_state: "CLEAN".into(), + }; + assert!(evaluate_rate_limit_shortcut(None, &m)); +} + +/// 順位 141: signal format に必須 field が全て含まれ、Claude が AskUserQuestion 化できる。 +#[test] +fn format_shortcut_signal_includes_required_fields() { + let rl = RateLimitState { + until_unix_secs: 1_779_432_672, + comment_event_time: "2026-05-22T06:08:02Z".into(), + wait_minutes: 38, + wait_seconds: 30, + }; + let pr_info = crate::util::PrInfo { + pr_number: Some(169), + repo: Some("aloekun/claude-code-hook-test".into()), + push_time: None, + head_commit: None, + fix_push_time: None, + }; + let m = MergeableStatus { + mergeable: "MERGEABLE".into(), + merge_state: "CLEAN".into(), + }; + let sig = format_shortcut_signal(&rl, &pr_info, &m); + assert!(sig.starts_with("[RATE_LIMIT_BUT_MERGEABLE]")); + assert!(sig.contains("[/RATE_LIMIT_BUT_MERGEABLE]")); + assert!(sig.contains("pr: 169")); + assert!(sig.contains("repo: aloekun/claude-code-hook-test")); + assert!(sig.contains("rate_limit_wait_seconds: 2310")); + assert!(sig.contains("mergeable: MERGEABLE")); + assert!(sig.contains("merge_state: CLEAN")); + assert!(sig.contains("AskUserQuestion")); +} + +/// CodeRabbit #353 (注入の正しさ): `finalize_waiting_reset_with` は注入された +/// fetcher が `Some(CLEAN)` を返し CR 側もクリーンなら shortcut 判定に到達する。 +/// +/// `|_| None` を渡すテストだけでは「注入したものが実際に使われているか」を +/// 判別できない (fetcher を無視する実装でも通ってしまう)。ここでは呼び出し自体を +/// フラグで観測し、注入経路が生きていることを固定する。 +#[test] +fn finalize_waiting_reset_with_uses_injected_fetcher() { + use std::cell::Cell; + + let tmp = tempfile::tempdir().unwrap(); + let state_path = tmp.path().join("state.json"); + let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); + state.rate_limit = Some(RateLimitState { + until_unix_secs: 9_999_999_999, + comment_event_time: "2026-08-03T00:00:00Z".into(), + wait_minutes: 5, + wait_seconds: 0, + }); + let rl = state.rate_limit.clone().unwrap(); + let pr_info = crate::util::PrInfo { + pr_number: Some(42), + repo: Some("o/r".into()), + push_time: None, + head_commit: Some("deadbeef".into()), + fix_push_time: None, + }; + + let called = Cell::new(false); + let outcome = finalize_waiting_reset_with( + &mut state, + &rl, + &pr_info, + &serde_json::Value::Null, + &state_path, + |_| { + called.set(true); + None + }, + ); + + assert!( + called.get(), + "注入した fetcher が呼ばれること (呼ばれないなら gh 層が迂回されていない疑い)" + ); + assert_eq!(outcome.action, "rate_limited"); +} 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 deleted file mode 100644 index 8852f697..00000000 --- a/src/cli-pr-monitor/src/stages/poll/rate_limit_signal.rs +++ /dev/null @@ -1,485 +0,0 @@ -//! rate-limit park / shortcut signal の formatting helper -//! (PR-W2 refactor で `rate_limit.rs` から signal 整形部分を切り出し)。 -//! -//! - `emit_shortcut_signal_if_eligible` / `fetch_mergeable_status` / -//! `evaluate_rate_limit_shortcut` / `format_shortcut_signal` (順位 141 shortcut) -//! - `format_park_signal` (rate_limit_retry PARK signal) -//! - `collect_posted_retrigger_park_fields` / `format_posted_retrigger_review_park_signal` -//! (rate-limit 解消後の review 待ち PARK signal) -//! - `MergeableStatus` / `PostedRetriggerParkFields` (DTO) - -use crate::state::PrMonitorState; -use crate::util::PrInfo; - -use crate::runner::run_gh_quiet; - -use super::review_recheck_signal::round_up_to_next_minute; - -/// 順位 141: rate-limit 検出 + mergeable CLEAN + CR 全フィールドクリーンの条件が揃ったとき -/// `[RATE_LIMIT_BUT_MERGEABLE]` signal を stdout に出力する shortcut path。 -pub(super) fn emit_shortcut_signal_if_eligible( - state: &PrMonitorState, - rl: &crate::state::RateLimitState, - pr_info: &PrInfo, -) { - let Some(mergeable) = fetch_mergeable_status(pr_info) else { - return; - }; - if !evaluate_rate_limit_shortcut(state.coderabbit.as_ref(), &mergeable) { - return; - } - println!("{}", format_shortcut_signal(rl, pr_info, &mergeable)); -} - -/// 順位 141: PR の mergeable / mergeStateStatus を gh で取得。失敗時は None。 -fn fetch_mergeable_status(pr_info: &PrInfo) -> Option { - let pr = pr_info.pr_number?; - let pr_str = pr.to_string(); - let mut args: Vec<&str> = vec![ - "pr", - "view", - &pr_str, - "--json", - "mergeable,mergeStateStatus", - ]; - if let Some(repo) = pr_info.repo.as_deref() { - args.push("--repo"); - args.push(repo); - } - let json_str = run_gh_quiet(&args)?; - let parsed: serde_json::Value = serde_json::from_str(&json_str).ok()?; - Some(MergeableStatus { - mergeable: parsed.get("mergeable")?.as_str()?.to_string(), - merge_state: parsed.get("mergeStateStatus")?.as_str()?.to_string(), - }) -} - -/// 順位 141: mergeable + CR 全フィールドクリーンの条件評価を pure 関数化 (test 容易性)。 -pub(super) fn evaluate_rate_limit_shortcut( - coderabbit: Option<&crate::state::CodeRabbitState>, - mergeable: &MergeableStatus, -) -> bool { - let cr_clean = coderabbit - .map(|c| { - c.new_comments == 0 - && c.actionable_comments.unwrap_or(0) == 0 - && c.unresolved_threads.unwrap_or(0) == 0 - }) - .unwrap_or(true); - mergeable.mergeable == "MERGEABLE" && mergeable.merge_state == "CLEAN" && cr_clean -} - -/// 順位 141: `[RATE_LIMIT_BUT_MERGEABLE]` signal を構築 (pure)。 -pub(super) fn format_shortcut_signal( - rl: &crate::state::RateLimitState, - pr_info: &PrInfo, - mergeable: &MergeableStatus, -) -> String { - let pr = pr_info - .pr_number - .map(|n| n.to_string()) - .unwrap_or_else(|| "?".into()); - let repo = pr_info.repo.as_deref().unwrap_or("?"); - let reset_iso = if rl.until_unix_secs > 0 { - lib_pending_file::epoch_secs_to_iso8601(rl.until_unix_secs as u64) - } else { - "?".into() - }; - let wait_total_secs = rl.wait_minutes * 60 + rl.wait_seconds; - format!( - "[RATE_LIMIT_BUT_MERGEABLE] -pr: {pr} -repo: {repo} -rate_limit_reset_at_iso_utc: {reset_iso} -rate_limit_wait_seconds: {wait_total_secs} -mergeable: {merge} -merge_state: {state} - -ACTION REQUIRED: ユーザーに以下 2 択を AskUserQuestion で問うこと: - A: 今すぐ merge する (rate-limit reset を待たない、CR 2 回目 review なしで進める) - B: reset を待って通常 auto-retry flow に乗る -[/RATE_LIMIT_BUT_MERGEABLE]", - merge = mergeable.mergeable, - state = mergeable.merge_state, - ) -} - -/// 順位 141: gh `pr view --json mergeable,mergeStateStatus` の結果を保持する DTO。 -#[derive(Debug, Clone)] -pub(crate) struct MergeableStatus { - pub(crate) mergeable: String, - pub(crate) merge_state: String, -} - -struct PostedRetriggerParkFields { - pr: String, - repo: String, - wakeup_unix: i64, - wakeup_iso: String, - safe_unix: i64, - safe_iso: String, - wait_secs: i64, - recheck: u32, - exe: String, - cwd: String, -} - -fn collect_posted_retrigger_park_fields( - state: &PrMonitorState, - pr_info: &PrInfo, -) -> PostedRetriggerParkFields { - let pr = pr_info - .pr_number - .map(|n| n.to_string()) - .unwrap_or_else(|| "?".into()); - let repo = pr_info.repo.as_deref().unwrap_or("?").to_string(); - let wakeup_unix = state.next_wakeup_at_unix.unwrap_or(0); - let wakeup_iso = if wakeup_unix > 0 { - lib_pending_file::epoch_secs_to_iso8601(wakeup_unix as u64) - } else { - "?".into() - }; - let safe_unix = if wakeup_unix > 0 { - round_up_to_next_minute(wakeup_unix) - } else { - 0 - }; - let safe_iso = if safe_unix > 0 { - lib_pending_file::epoch_secs_to_iso8601(safe_unix as u64) - } else { - "?".into() - }; - let now_unix = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs() as i64) - .unwrap_or(0); - let wait_secs = (wakeup_unix - now_unix).max(0); - let exe = std::env::current_exe() - .map(|p| p.display().to_string()) - .unwrap_or_else(|_| format!("cli-pr-monitor{}", std::env::consts::EXE_SUFFIX)); - let cwd = std::env::current_dir() - .map(|p| p.display().to_string()) - .unwrap_or_else(|_| ".".into()); - PostedRetriggerParkFields { - pr, - repo, - wakeup_unix, - wakeup_iso, - safe_unix, - safe_iso, - wait_secs, - recheck: state.review_recheck_count, - exe, - cwd, - } -} - -/// rate-limit 解消後の `@coderabbitai review` 投稿完了 → review 待ち PARK signal を生成する。 -/// -/// `format_park_signal` (reason: rate_limit_retry, reset_at_unix 基準) とは異なり、 -/// `state.next_wakeup_at_unix` の review 待ち wakeup を基準に `reason: review_recheck` で -/// discriminate する。過去 timestamp (`rl.until_unix_secs`) を CronCreate に誤使用しない。 -pub(super) fn format_posted_retrigger_review_park_signal( - state: &PrMonitorState, - pr_info: &PrInfo, -) -> String { - let PostedRetriggerParkFields { - pr, - repo, - wakeup_unix, - wakeup_iso, - safe_unix, - safe_iso, - wait_secs, - recheck, - exe, - cwd, - } = collect_posted_retrigger_park_fields(state, pr_info); - format!( - "[PR_MONITOR_PARK] -reason: review_recheck -pr: {pr} -repo: {repo} -next_wakeup_at_unix: {wakeup_unix} -next_wakeup_at_iso_utc: {wakeup_iso} -safe_minute_at_unix: {safe_unix} -safe_minute_at_iso_utc: {safe_iso} -wait_total_seconds: {wait_secs} -recheck_count: {recheck} -exe: {exe} -cwd: {cwd} - -ACTION REQUIRED: please schedule one-shot wakeup using CronCreate. - -Cron spec derivation (apply 2 orthogonal constraints sequentially): - Step 1 (round-UP, already applied at the source): use `safe_minute_at_iso_utc` - (= next_wakeup_at_iso_utc with seconds rounded UP to next full minute). - Step 2 (avoid :00 / :30 minute due to 90s pre-fire jitter): convert - `safe_minute_at_iso_utc` to LOCAL TZ, then bump the minute by +1 if it - lands on :00 or :30. Use the resulting `HH:MM` as the cron field. - Reference: ~/.claude/rules/common/development-workflow.md - § Cron スケジューリングの秒 → 分 round-UP - -CronCreate({{ - cron: \"\", - recurring: false, - durable: true, - prompt: \"Wakeup: review recheck for PR #{pr} ({repo}). cd \\\"{cwd}\\\" && \\\"{exe}\\\" --monitor-only\" -}}) -[/PR_MONITOR_PARK]" - ) -} - -/// PARK signal を stdout に書き出すための pure 関数 (Bb-1)。 -pub(crate) fn format_park_signal( - state: &PrMonitorState, - rl: &crate::state::RateLimitState, - pr_info: &PrInfo, - max_retries: u32, -) -> String { - let pr = pr_info - .pr_number - .map(|n| n.to_string()) - .unwrap_or_else(|| "?".into()); - let repo = pr_info.repo.as_deref().unwrap_or("?"); - let reset_iso = if rl.until_unix_secs > 0 { - lib_pending_file::epoch_secs_to_iso8601(rl.until_unix_secs as u64) - } else { - "?".into() - }; - let wait_total_secs = rl.wait_minutes * 60 + rl.wait_seconds; - let exe = std::env::current_exe() - .map(|p| p.display().to_string()) - .unwrap_or_else(|_| format!("cli-pr-monitor{}", std::env::consts::EXE_SUFFIX)); - let cwd = std::env::current_dir() - .map(|p| p.display().to_string()) - .unwrap_or_else(|_| ".".into()); - let retry_attempt = state.rate_limit_retries + 1; - - format!( - "[PR_MONITOR_PARK] -reason: rate_limit_retry -pr: {pr} -repo: {repo} -reset_at_unix: {until} -reset_at_iso_utc: {reset_iso} -wait_total_seconds: {wait_total_secs} -retry_count: {retry_attempt} -max_retries: {max_retries} -exe: {exe} -cwd: {cwd} - -ACTION REQUIRED: please schedule one-shot wakeup using CronCreate. - -CronCreate({{ - cron: \"\", - recurring: false, - durable: true, - prompt: \"Wakeup: rate-limit retry for PR #{pr} ({repo}). cd \\\"{cwd}\\\" && \\\"{exe}\\\" --monitor-only\" -}}) -[/PR_MONITOR_PARK]", - until = rl.until_unix_secs, - ) -} - -#[cfg(test)] -mod tests { - use super::*; - - /// Finding #3: rate-limit retrigger 後の PARK signal が `reason: review_recheck` を使い、 - /// 過去 timestamp (`rl.until_unix_secs`) ではなく `state.next_wakeup_at_unix` を参照する。 - #[test] - fn format_posted_retrigger_review_park_signal_uses_review_recheck_reason() { - let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); - state.next_wakeup_at_unix = Some(1_775_044_800); - state.review_recheck_count = 1; - let pr_info = crate::util::PrInfo { - pr_number: Some(42), - repo: Some("o/r".into()), - push_time: None, - head_commit: None, - fix_push_time: None, - }; - - let signal = format_posted_retrigger_review_park_signal(&state, &pr_info); - - assert!( - signal.starts_with("[PR_MONITOR_PARK]"), - "PARK signal ヘッダが正しい形式でない: {}", - signal - ); - assert!( - signal.contains("reason: review_recheck"), - "Finding #3: rate-limit retrigger 後も reason は review_recheck であるべき。実際: {}", - signal - ); - assert!( - !signal.contains("reason: rate_limit_retry"), - "Finding #3: rate_limit_retry は誤った reason (rate-limit PARK と混同)。実際: {}", - signal - ); - assert!( - signal.contains("next_wakeup_at_unix: 1775044800"), - "state.next_wakeup_at_unix を参照すべき (rl.until_unix_secs の過去 timestamp ではない)。実際: {}", - signal - ); - } - - /// Bb-1: PARK signal は CronCreate 呼び出しに必要な構造化情報を含む。 - #[test] - fn format_park_signal_includes_required_fields() { - let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); - state.rate_limit_retries = 0; - let rl = crate::state::RateLimitState { - until_unix_secs: 1_775_088_000, - comment_event_time: "2026-05-01T00:00:00Z".into(), - wait_minutes: 47, - wait_seconds: 0, - }; - let pr_info = crate::util::PrInfo { - pr_number: Some(42), - repo: Some("o/r".into()), - push_time: None, - head_commit: None, - fix_push_time: None, - }; - - let signal = format_park_signal(&state, &rl, &pr_info, 3); - assert!(signal.starts_with("[PR_MONITOR_PARK]")); - assert!(signal.contains("[/PR_MONITOR_PARK]")); - assert!(signal.contains("pr: 42")); - assert!(signal.contains("repo: o/r")); - assert!(signal.contains("reset_at_unix: 1775088000")); - assert!(signal.contains("wait_total_seconds: 2820")); - assert!(signal.contains("retry_count: 1")); - assert!(signal.contains("max_retries: 3")); - assert!(signal.contains("CronCreate(")); - assert!(signal.contains("durable: true")); - assert!(signal.contains("recurring: false")); - assert!(signal.contains("--monitor-only")); - } - - /// Bb-1: PR 番号 / repo が None でも format_park_signal は panic せず "?" を出す。 - #[test] - fn format_park_signal_handles_missing_pr_info() { - let state = PrMonitorState::new(None, None, "t".into()); - let rl = crate::state::RateLimitState { - until_unix_secs: 1_775_088_000, - comment_event_time: "2026-05-01T00:00:00Z".into(), - wait_minutes: 5, - wait_seconds: 30, - }; - let pr_info = crate::util::PrInfo { - pr_number: None, - repo: None, - push_time: None, - head_commit: None, - fix_push_time: None, - }; - - let signal = format_park_signal(&state, &rl, &pr_info, 3); - assert!(signal.contains("pr: ?")); - assert!(signal.contains("repo: ?")); - assert!(signal.contains("wait_total_seconds: 330")); - } - - /// 順位 141: shortcut signal の trigger 条件 (mergeable CLEAN + unresolved 0) で true。 - #[test] - fn evaluate_rate_limit_shortcut_when_all_conditions_met() { - let m = MergeableStatus { - mergeable: "MERGEABLE".into(), - merge_state: "CLEAN".into(), - }; - let cr = crate::state::CodeRabbitState { - review_state: "approved".into(), - new_comments: 0, - actionable_comments: Some(0), - unresolved_threads: Some(0), - }; - assert!(evaluate_rate_limit_shortcut(Some(&cr), &m)); - } - - /// 順位 141: unresolved thread が残っていれば shortcut を抑止 (CR の指摘が未対応)。 - #[test] - fn evaluate_rate_limit_shortcut_blocks_when_unresolved_threads_exist() { - let m = MergeableStatus { - mergeable: "MERGEABLE".into(), - merge_state: "CLEAN".into(), - }; - let cr = crate::state::CodeRabbitState { - review_state: "commented".into(), - new_comments: 1, - actionable_comments: Some(1), - unresolved_threads: Some(1), - }; - assert!(!evaluate_rate_limit_shortcut(Some(&cr), &m)); - } - - /// 順位 141: new_comments > 0 のとき unresolved_threads が 0 でも shortcut を抑止。 - /// CR がまだコメントを処理中の状態で merge 判定を通過させない。 - #[test] - fn evaluate_rate_limit_shortcut_blocks_when_new_comments_exist() { - let m = MergeableStatus { - mergeable: "MERGEABLE".into(), - merge_state: "CLEAN".into(), - }; - let cr = crate::state::CodeRabbitState { - review_state: "commented".into(), - new_comments: 1, - actionable_comments: Some(0), - unresolved_threads: Some(0), - }; - assert!(!evaluate_rate_limit_shortcut(Some(&cr), &m)); - } - - /// 順位 141: mergeable が BLOCKED なら shortcut を抑止 (GitHub 側で merge 不可)。 - #[test] - fn evaluate_rate_limit_shortcut_blocks_when_not_mergeable() { - let m = MergeableStatus { - mergeable: "BLOCKED".into(), - merge_state: "BLOCKED".into(), - }; - assert!(!evaluate_rate_limit_shortcut(None, &m)); - } - - /// 順位 141: CR state が None (初回 review なし) でも mergeable CLEAN なら shortcut 可。 - #[test] - fn evaluate_rate_limit_shortcut_passes_when_coderabbit_none() { - let m = MergeableStatus { - mergeable: "MERGEABLE".into(), - merge_state: "CLEAN".into(), - }; - assert!(evaluate_rate_limit_shortcut(None, &m)); - } - - /// 順位 141: signal format に必須 field が全て含まれ、Claude が AskUserQuestion 化できる。 - #[test] - fn format_shortcut_signal_includes_required_fields() { - let rl = crate::state::RateLimitState { - until_unix_secs: 1_779_432_672, - comment_event_time: "2026-05-22T06:08:02Z".into(), - wait_minutes: 38, - wait_seconds: 30, - }; - let pr_info = crate::util::PrInfo { - pr_number: Some(169), - repo: Some("aloekun/claude-code-hook-test".into()), - push_time: None, - head_commit: None, - fix_push_time: None, - }; - let m = MergeableStatus { - mergeable: "MERGEABLE".into(), - merge_state: "CLEAN".into(), - }; - let sig = format_shortcut_signal(&rl, &pr_info, &m); - assert!(sig.starts_with("[RATE_LIMIT_BUT_MERGEABLE]")); - assert!(sig.contains("[/RATE_LIMIT_BUT_MERGEABLE]")); - assert!(sig.contains("pr: 169")); - assert!(sig.contains("repo: aloekun/claude-code-hook-test")); - assert!(sig.contains("rate_limit_wait_seconds: 2310")); - assert!(sig.contains("mergeable: MERGEABLE")); - assert!(sig.contains("merge_state: CLEAN")); - assert!(sig.contains("AskUserQuestion")); - } -} diff --git a/src/cli-pr-monitor/src/stages/poll/review_recheck.rs b/src/cli-pr-monitor/src/stages/poll/review_recheck.rs deleted file mode 100644 index 727fa43d..00000000 --- a/src/cli-pr-monitor/src/stages/poll/review_recheck.rs +++ /dev/null @@ -1,420 +0,0 @@ -//! Review recheck park 関連 (PR B refactor で `mod.rs` から切り出し)。 -//! -//! - Bb-2 アーキで定義された review_recheck park 経路 (`finalize_initial_review_park`, -//! `finalize_review_recheck_park`, `finalize_review_recheck_max_reached`, -//! `schedule_next_review_recheck_park`) -//! -//! signal 整形部分 (`round_up_to_next_minute` / `compute_safe_minute_for_park_signal` / -//! `format_review_park_signal`) は `review_recheck_signal.rs` に分離。 - -use std::path::Path; - -use crate::log::log_info; -use crate::state::{read_state_from, write_state_to, PrMonitorState}; - -use super::rate_limit::make_action_required_result; -use super::review_recheck_signal::format_review_park_signal; -use super::{make_park_poll_result, PollContext, PollResult}; - -/// Bb-2: fresh push 経路で review_recheck park を行う (checker 呼び出しなし)。 -/// -/// 動機: push 直後は CR がまだ review を開始していない可能性が高く、即 check は wasteful。 -/// `initial_review_wait_secs` 後に wakeup を予約 → 1 回 check という 2-step フローに分離する。 -/// -/// CR Major #2 fix (Bb-2 PR #114 review): 既存 state に残った `review_recheck_count` を -/// fresh push では 0 に明示リセット (前サイクルが MAX 到達等で残った count が新 push に -/// 持ち越されると summary "(initial wait, recheck=0/N)" と PARK signal の recheck_count が -/// 食い違い、最悪 max 到達状態で park される)。 -/// CR Major #1 fix: head_commit を state に保存し detect_wakeup_resume の比較対象とする。 -pub(super) fn finalize_initial_review_park(ctx: &PollContext<'_>) -> PollResult { - let mut state = read_state_from(ctx.state_path).unwrap_or_else(|| { - PrMonitorState::new( - ctx.pr_info.pr_number, - ctx.pr_info.repo.clone(), - ctx.push_time.to_string(), - ) - }); - state.pr = ctx.pr_info.pr_number; - state.repo = ctx.pr_info.repo.clone(); - state.started_at = ctx.push_time.to_string(); - state.review_recheck_count = 0; - state.head_commit = ctx.pr_info.head_commit.clone(); - state.fix_push_time = state - .fix_push_time - .or_else(|| ctx.fix_push_time.map(String::from)); - let now_unix = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs() as i64) - .unwrap_or(0); - state.next_wakeup_at_unix = Some(now_unix + ctx.initial_review_wait_secs as i64); - state.wakeup_reason = Some("review_recheck".into()); - state.action = "parked_review_recheck".into(); - state.summary = format!( - "review check を {}s 後に予約 (initial wait, recheck=0/{})", - ctx.initial_review_wait_secs, ctx.max_review_rechecks - ); - - if let Err(e) = write_state_to(ctx.state_path, &state) { - log_info(&format!( - "[review_recheck] initial park state 永続化失敗、action_required で抜ける: {}", - e - )); - return make_action_required_result( - &state, - &serde_json::Value::Null, - &format!("review park の state 永続化失敗 ({})。手動確認が必要", e), - ); - } - - println!("{}", format_review_park_signal(&state, ctx)); - make_park_poll_result(state) -} - -/// Bb-2: wakeup 経路の review_recheck park (checker check 後に continue_monitoring の場合)。 -/// -/// `review_recheck_count` をインクリメントし、`max_review_rechecks` 到達なら -/// `action_required` で抜ける (review が想定時間内に未完了を通知)。 -/// 未到達なら `review_recheck_wait_secs` 後の wakeup を予約して return。 -pub(super) fn finalize_review_recheck_park(ctx: &PollContext<'_>) -> PollResult { - let mut state = read_state_from(ctx.state_path).unwrap_or_else(|| { - PrMonitorState::new( - ctx.pr_info.pr_number, - ctx.pr_info.repo.clone(), - ctx.push_time.to_string(), - ) - }); - state.review_recheck_count += 1; - state.fix_push_time = state - .fix_push_time - .or_else(|| ctx.fix_push_time.map(String::from)); - - if state.review_recheck_count >= ctx.max_review_rechecks { - return finalize_review_recheck_max_reached( - &mut state, - ctx.state_path, - ctx.max_review_rechecks, - ); - } - - schedule_next_review_recheck_park(&mut state, ctx) -} - -fn finalize_review_recheck_max_reached( - state: &mut PrMonitorState, - state_path: &Path, - max_review_rechecks: u32, -) -> PollResult { - log_info(&format!( - "[review_recheck] max {} 回到達、action_required で抜ける", - max_review_rechecks - )); - let summary = format!( - "review が想定時間内に完了せず ({} recheck 後)。手動で PR を確認してください", - state.review_recheck_count - ); - state.action = "action_required".into(); - state.summary = summary.clone(); - state.next_wakeup_at_unix = None; - state.wakeup_reason = None; - if let Err(e) = write_state_to(state_path, state) { - log_info(&format!( - "state 書き込み失敗 (action_required 確定後、続行): {}", - e - )); - } - make_action_required_result(state, &serde_json::Value::Null, &summary) -} - -pub(super) fn schedule_next_review_recheck_park( - state: &mut PrMonitorState, - ctx: &PollContext<'_>, -) -> PollResult { - let now_unix = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs() as i64) - .unwrap_or(0); - state.next_wakeup_at_unix = Some(now_unix + ctx.review_recheck_wait_secs as i64); - state.wakeup_reason = Some("review_recheck".into()); - state.action = "parked_review_recheck".into(); - state.head_commit = ctx.pr_info.head_commit.clone(); - state.summary = format!( - "review check を {}s 後に予約 (recheck={}/{})", - ctx.review_recheck_wait_secs, state.review_recheck_count, ctx.max_review_rechecks - ); - - if let Err(e) = write_state_to(ctx.state_path, state) { - log_info(&format!( - "[review_recheck] park state 永続化失敗、action_required で抜ける: {}", - e - )); - return make_action_required_result( - state, - &serde_json::Value::Null, - &format!("review park の state 永続化失敗 ({})。手動確認が必要", e), - ); - } - - println!("{}", format_review_park_signal(state, ctx)); - make_park_poll_result(state.clone()) -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::config::{ClassifierConfig, RateLimitConfig}; - - /// 書き込み先がディレクトリ不在のため write が必ず失敗する path を返す。 - fn unwritable_state_path() -> std::path::PathBuf { - std::env::temp_dir() - .join(format!("pr-monitor-T2-2-{}", std::process::id())) - .join("nonexistent-dir") - .join("state.json") - } - - fn seed_stale_recheck_state(tmp_path: &std::path::Path) { - let mut stale_state = - PrMonitorState::new(Some(42), Some("o/r".into()), "2026-05-01T00:00:00Z".into()); - stale_state.review_recheck_count = 3; - stale_state.action = "action_required".into(); - crate::state::write_state_to(tmp_path, &stale_state).unwrap(); - } - - fn pr_info_for_initial_review_park_test() -> crate::util::PrInfo { - crate::util::PrInfo { - pr_number: Some(42), - repo: Some("o/r".into()), - push_time: Some("2026-05-01T00:00:00Z".into()), - head_commit: Some("abc1234".into()), - fix_push_time: None, - } - } - - fn make_default_test_ctx<'a>( - checker: &'a std::path::Path, - state_path: &'a std::path::Path, - pr_info: &'a crate::util::PrInfo, - rate_limit_config: &'a RateLimitConfig, - classifier_config: &'a ClassifierConfig, - ) -> PollContext<'a> { - PollContext { - checker, - state_path, - push_time: "2026-05-01T00:00:00Z", - fix_push_time: None, - pr_info, - rate_limit_config, - classifier_config, - start: std::time::Instant::now(), - max_duration: 600, - skip_ci: false, - skip_coderabbit: false, - initial_review_wait_secs: 300, - review_recheck_wait_secs: 300, - max_review_rechecks: 3, - } - } - - /// Finding #5: `finalize_review_recheck_max_reached` は `action_required` 確定後に - /// 残留 wakeup fields を None にクリアする。ADR-030 invariant: - /// "wakeup は parked_* action のときのみスケジュールされる"。 - #[test] - fn finalize_review_recheck_max_reached_clears_wakeup_fields() { - let tmp = tempfile::tempdir().unwrap(); - let state_path = tmp.path().join("state.json"); - - let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); - let stale_wakeup_unix: i64 = 9_999_999_999; - let stale_wakeup_reason = "review_recheck"; - state.next_wakeup_at_unix = Some(stale_wakeup_unix); - state.wakeup_reason = Some(stale_wakeup_reason.into()); - state.review_recheck_count = 3; - state.action = "parked_review_recheck".into(); - - finalize_review_recheck_max_reached(&mut state, &state_path, 3); - - assert!( - state.next_wakeup_at_unix.is_none(), - "Finding #5: action_required 確定時に next_wakeup_at_unix が None にクリアされること。実際: {:?}", - state.next_wakeup_at_unix - ); - assert!( - state.wakeup_reason.is_none(), - "Finding #5: action_required 確定時に wakeup_reason が None にクリアされること。実際: {:?}", - state.wakeup_reason - ); - assert_eq!( - state.action, "action_required", - "Finding #5: action が action_required に確定されること" - ); - } - - /// Bb-2 (T2-2): `schedule_next_review_recheck_park` は write_state 失敗時に - /// PARK signal emit を中止し `action_required` を返却する (sibling parity)。 - #[test] - fn schedule_next_review_recheck_park_returns_action_required_when_write_state_fails() { - let bad_path = unwritable_state_path(); - - let mut state = - PrMonitorState::new(Some(42), Some("o/r".into()), "2026-05-01T00:00:00Z".into()); - state.review_recheck_count = 1; - let checker_path = std::path::PathBuf::from("dummy-checker"); - let pr_info = crate::util::PrInfo { - pr_number: Some(42), - repo: Some("o/r".into()), - push_time: Some("2026-05-01T00:00:00Z".into()), - head_commit: None, - fix_push_time: None, - }; - let rate_limit_config = RateLimitConfig::default(); - let classifier_config = ClassifierConfig::default(); - let ctx = PollContext { - checker: &checker_path, - state_path: &bad_path, - push_time: "2026-05-01T00:00:00Z", - fix_push_time: None, - pr_info: &pr_info, - rate_limit_config: &rate_limit_config, - classifier_config: &classifier_config, - start: std::time::Instant::now(), - max_duration: 600, - skip_ci: false, - skip_coderabbit: false, - initial_review_wait_secs: 300, - review_recheck_wait_secs: 300, - max_review_rechecks: 3, - }; - - let outcome = schedule_next_review_recheck_park(&mut state, &ctx); - - assert_eq!( - outcome.action, "action_required", - "T2-2 sibling parity: review park も write_state 失敗 → action_required で抜けること" - ); - } - - /// CR Major #2 fix (Bb-2 PR #114 review): fresh push 経路では `finalize_initial_review_park` - /// が `review_recheck_count` を 0 に明示リセットすること。前サイクルが MAX 到達 (count=3) - /// で残った state を持ち越さないことを machine-enforce する。 - #[test] - fn finalize_initial_review_park_resets_recheck_count() { - let tmp = tempfile::tempdir().unwrap(); - let state_path = tmp.path().join("state.json"); - seed_stale_recheck_state(&state_path); - - let pr_info = pr_info_for_initial_review_park_test(); - let checker = std::path::PathBuf::from("dummy"); - let rate_limit_config = RateLimitConfig::default(); - let classifier_config = ClassifierConfig::default(); - let ctx = make_default_test_ctx( - &checker, - &state_path, - &pr_info, - &rate_limit_config, - &classifier_config, - ); - - let outcome = finalize_initial_review_park(&ctx); - let persisted = crate::state::read_state_from(&state_path).unwrap(); - - assert_eq!(outcome.action, "parked_review_recheck"); - assert_eq!( - persisted.review_recheck_count, 0, - "CR Major #2: fresh push 経路で count=3 が残らず 0 にリセットされること" - ); - assert_eq!( - persisted.head_commit.as_deref(), - Some("abc1234"), - "CR Major #1: fresh push 経路で head_commit が pr_info から保存されること" - ); - } - - /// 順位 141: `fix_push_time` の write-once 不変条件 — - /// `finalize_initial_review_park` が state に既存の `fix_push_time` がある場合に - /// `ctx.fix_push_time` の値で上書きしないことを検証する。 - /// - /// `ctx.fix_push_time = Some("new_time")` (= None ではなく非 None) を使うことで、 - /// or_else 被演算子の入れ替えバグを discriminate できる。 - #[test] - fn finalize_initial_review_park_preserves_existing_fix_push_time() { - let tmp = tempfile::tempdir().unwrap(); - let state_path = tmp.path().join("state.json"); - - let mut seeded = - PrMonitorState::new(Some(42), Some("o/r".into()), "2026-05-01T00:00:00Z".into()); - seeded.fix_push_time = Some("2026-05-22T06:06:00Z".into()); - crate::state::write_state_to(&state_path, &seeded).unwrap(); - - let pr_info = crate::util::PrInfo { - pr_number: Some(42), - repo: Some("o/r".into()), - push_time: Some("2026-05-01T00:00:00Z".into()), - head_commit: Some("abc1234".into()), - fix_push_time: None, - }; - let checker = std::path::PathBuf::from("dummy"); - let rate_limit_config = RateLimitConfig::default(); - let classifier_config = ClassifierConfig::default(); - let mut ctx = make_default_test_ctx( - &checker, - &state_path, - &pr_info, - &rate_limit_config, - &classifier_config, - ); - let ctx_fix_push_time_must_lose = "2026-05-22T06:10:00Z"; - ctx.fix_push_time = Some(ctx_fix_push_time_must_lose); - - finalize_initial_review_park(&ctx); - let persisted = crate::state::read_state_from(&state_path).unwrap(); - - assert_eq!( - persisted.fix_push_time.as_deref(), - Some("2026-05-22T06:06:00Z"), - "write-once: state に既存 fix_push_time がある場合、ctx の値で上書きしない" - ); - } - - /// 順位 141: `fix_push_time` の write-once 不変条件 — - /// `finalize_review_recheck_park` が state に既存の `fix_push_time` がある場合に - /// `ctx.fix_push_time` の値で上書きしないことを検証する。 - #[test] - fn finalize_review_recheck_park_preserves_existing_fix_push_time() { - let tmp = tempfile::tempdir().unwrap(); - let state_path = tmp.path().join("state.json"); - - let mut seeded = - PrMonitorState::new(Some(42), Some("o/r".into()), "2026-05-01T00:00:00Z".into()); - seeded.fix_push_time = Some("2026-05-22T06:06:00Z".into()); - seeded.review_recheck_count = 0; - crate::state::write_state_to(&state_path, &seeded).unwrap(); - - let pr_info = crate::util::PrInfo { - pr_number: Some(42), - repo: Some("o/r".into()), - push_time: Some("2026-05-01T00:00:00Z".into()), - head_commit: Some("abc1234".into()), - fix_push_time: None, - }; - let checker = std::path::PathBuf::from("dummy"); - let rate_limit_config = RateLimitConfig::default(); - let classifier_config = ClassifierConfig::default(); - let mut ctx = make_default_test_ctx( - &checker, - &state_path, - &pr_info, - &rate_limit_config, - &classifier_config, - ); - let ctx_fix_push_time_must_lose = "2026-05-22T06:10:00Z"; - ctx.fix_push_time = Some(ctx_fix_push_time_must_lose); - - finalize_review_recheck_park(&ctx); - let persisted = crate::state::read_state_from(&state_path).unwrap(); - - assert_eq!( - persisted.fix_push_time.as_deref(), - Some("2026-05-22T06:06:00Z"), - "write-once: state に既存 fix_push_time がある場合、ctx の値で上書きしない" - ); - } -} diff --git a/src/cli-pr-monitor/src/stages/poll/review_recheck_signal.rs b/src/cli-pr-monitor/src/stages/poll/review_recheck_signal.rs deleted file mode 100644 index 0faaa0aa..00000000 --- a/src/cli-pr-monitor/src/stages/poll/review_recheck_signal.rs +++ /dev/null @@ -1,315 +0,0 @@ -//! Review recheck PARK signal の formatting helper -//! (PR-W2 refactor で `review_recheck.rs` から signal 整形部分を切り出し)。 -//! -//! - 順位 209 / 210 の安全 cron spec 生成 helper (`round_up_to_next_minute`, -//! `compute_safe_minute_for_park_signal`) -//! - 順位 209 で導入された PARK signal format (`format_review_park_signal`) - -use crate::state::PrMonitorState; - -use super::PollContext; - -/// 順位 209: PARK signal の cron spec round-UP rule (= Constraint 1)。 -/// -/// `unix_secs` の秒部分が `0` でなければ次の完全な分に round-UP した unix seconds を返す。 -/// `~/.claude/rules/common/development-workflow.md` § Cron スケジューリングの秒 → 分 round-UP の -/// Constraint 1 (= scheduling minimum lead time) のみを実装。 -/// -/// Constraint 2 (= execution jitter ≤90s pre-fire / minute `:00`・`:30` 回避) は local TZ -/// awareness が必要で fractional-hour offset (例: IST +5:30) で正しく適用するには -/// AI agent consumer 側での処理が安全。本関数は UTC pure arithmetic に閉じる設計とし、 -/// PARK signal の ACTION REQUIRED block で Step 2 として AI agent に明示する。 -/// -/// 由来: PR #210 セッション (2026-06-16) で実観測した cron timing race。秒解像度 timestamp を -/// 分単位 cron に round-DOWN 変換した結果、`should_resume_wakeup` が `wakeup_at > now` で false -/// 判定 → fresh path に倒れて recheck_count が前進せず、2 回の無駄 wakeup が発生した root cause。 -pub(crate) fn round_up_to_next_minute(unix_secs: i64) -> i64 { - let sec_in_minute = unix_secs.rem_euclid(60); - if sec_in_minute == 0 { - unix_secs - } else { - unix_secs - sec_in_minute + 60 - } -} - -/// 順位 209: PARK signal 用に Constraint 1 (秒 → 分 round-UP) を適用した -/// safe minute の unix seconds と UTC ISO 8601 文字列を返す。 -/// -/// `wakeup_unix == 0` (未設定) のとき `(0, "?")` を返す sentinel 値を維持し、 -/// `format_review_park_signal` 出力の "?" plain string 互換を保つ。 -fn compute_safe_minute_for_park_signal(wakeup_unix: i64) -> (i64, String) { - if wakeup_unix <= 0 { - return (0, "?".into()); - } - let safe_unix = round_up_to_next_minute(wakeup_unix); - let safe_iso = lib_pending_file::epoch_secs_to_iso8601(safe_unix as u64); - (safe_unix, safe_iso) -} - -struct ReviewParkSignalFields { - safe_minute_unix: i64, - safe_minute_iso_utc: String, - pr: String, - repo: String, - wakeup_unix: i64, - wakeup_iso: String, - wait_secs: i64, - exe: String, - cwd: String, - recheck: u32, - max_rechecks: u32, -} - -fn collect_review_park_fields( - state: &PrMonitorState, - ctx: &PollContext<'_>, -) -> ReviewParkSignalFields { - let pr = ctx - .pr_info - .pr_number - .map(|n| n.to_string()) - .unwrap_or_else(|| "?".into()); - let repo = ctx.pr_info.repo.clone().unwrap_or_else(|| "?".into()); - let wakeup_unix = state.next_wakeup_at_unix.unwrap_or(0); - let wakeup_iso = if wakeup_unix > 0 { - lib_pending_file::epoch_secs_to_iso8601(wakeup_unix as u64) - } else { - "?".into() - }; - let (safe_minute_unix, safe_minute_iso_utc) = compute_safe_minute_for_park_signal(wakeup_unix); - let now_unix = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs() as i64) - .unwrap_or(0); - let wait_secs = (wakeup_unix - now_unix).max(0); - let exe = std::env::current_exe() - .map(|p| p.display().to_string()) - .unwrap_or_else(|_| format!("cli-pr-monitor{}", std::env::consts::EXE_SUFFIX)); - let cwd = std::env::current_dir() - .map(|p| p.display().to_string()) - .unwrap_or_else(|_| ".".into()); - - ReviewParkSignalFields { - safe_minute_unix, - safe_minute_iso_utc, - pr, - repo, - wakeup_unix, - wakeup_iso, - wait_secs, - exe, - cwd, - recheck: state.review_recheck_count, - max_rechecks: ctx.max_review_rechecks, - } -} - -/// Bb-2: 初回 push 後の review_recheck park signal を生成する。 -/// -/// `format_park_signal` (rate_limit_retry) と同じ envelope `[PR_MONITOR_PARK]` を使い、 -/// `reason: review_recheck` で discriminate する。Claude Code 側のパーサは両 signal を -/// 同じ format で読める。 -pub(super) fn format_review_park_signal(state: &PrMonitorState, ctx: &PollContext<'_>) -> String { - let f = collect_review_park_fields(state, ctx); - format!( - "[PR_MONITOR_PARK] -reason: review_recheck -pr: {pr} -repo: {repo} -next_wakeup_at_unix: {wakeup_unix} -next_wakeup_at_iso_utc: {wakeup_iso} -safe_minute_at_unix: {safe_unix} -safe_minute_at_iso_utc: {safe_iso} -wait_total_seconds: {wait_secs} -recheck_count: {recheck} -max_rechecks: {max} -exe: {exe} -cwd: {cwd} - -ACTION REQUIRED: please schedule one-shot wakeup using CronCreate. - -Cron spec derivation (apply 2 orthogonal constraints sequentially): - Step 1 (round-UP, already applied at the source): use `safe_minute_at_iso_utc` - (= next_wakeup_at_iso_utc with seconds rounded UP to next full minute). - Step 2 (avoid :00 / :30 minute due to 90s pre-fire jitter): convert - `safe_minute_at_iso_utc` to LOCAL TZ, then bump the minute by +1 if it - lands on :00 or :30. Use the resulting `HH:MM` as the cron field. - Reference: ~/.claude/rules/common/development-workflow.md - § Cron スケジューリングの秒 → 分 round-UP - -CronCreate({{ - cron: \"\", - recurring: false, - durable: true, - prompt: \"Wakeup: review recheck for PR #{pr} ({repo}). cd \\\"{cwd}\\\" && \\\"{exe}\\\" --monitor-only\" -}}) -[/PR_MONITOR_PARK]", - pr = f.pr, - repo = f.repo, - wakeup_unix = f.wakeup_unix, - wakeup_iso = f.wakeup_iso, - safe_unix = f.safe_minute_unix, - safe_iso = f.safe_minute_iso_utc, - wait_secs = f.wait_secs, - recheck = f.recheck, - max = f.max_rechecks, - exe = f.exe, - cwd = f.cwd, - ) -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::config::{ClassifierConfig, RateLimitConfig}; - - #[test] - fn round_up_to_next_minute_keeps_value_when_seconds_already_zero() { - let aligned = 1_775_044_800; - assert_eq!(round_up_to_next_minute(aligned), aligned); - } - - #[test] - fn round_up_to_next_minute_rounds_up_when_seconds_present() { - let unaligned = 1_775_044_819; - assert_eq!(round_up_to_next_minute(unaligned), 1_775_044_860); - } - - #[test] - fn round_up_to_next_minute_rounds_up_one_second_before_next_minute() { - let one_sec_before = 1_775_044_859; - assert_eq!(round_up_to_next_minute(one_sec_before), 1_775_044_860); - } - - #[test] - fn round_up_to_next_minute_one_second_past_minute_rounds_up_to_next_full_minute() { - let one_sec_past = 1_775_044_801; - assert_eq!(round_up_to_next_minute(one_sec_past), 1_775_044_860); - } - - #[test] - fn round_up_to_next_minute_handles_zero_input_as_minute_zero() { - assert_eq!(round_up_to_next_minute(0), 0); - } - - #[test] - fn compute_safe_minute_returns_sentinel_when_input_zero() { - let (safe_unix, safe_iso) = compute_safe_minute_for_park_signal(0); - assert_eq!(safe_unix, 0); - assert_eq!(safe_iso, "?"); - } - - #[test] - fn compute_safe_minute_returns_sentinel_when_input_negative() { - let (safe_unix, safe_iso) = compute_safe_minute_for_park_signal(-1); - assert_eq!(safe_unix, 0); - assert_eq!(safe_iso, "?"); - } - - #[test] - fn compute_safe_minute_rounds_up_and_formats_iso_when_input_unaligned() { - let (safe_unix, safe_iso) = compute_safe_minute_for_park_signal(1_775_044_819); - assert_eq!(safe_unix, 1_775_044_860); - assert_eq!(safe_iso, "2026-04-01T12:01:00Z"); - } - - #[test] - fn compute_safe_minute_preserves_iso_when_input_already_aligned() { - let (safe_unix, safe_iso) = compute_safe_minute_for_park_signal(1_775_044_800); - assert_eq!(safe_unix, 1_775_044_800); - assert_eq!(safe_iso, "2026-04-01T12:00:00Z"); - } - - /// Bb-3 (順位 55): `max_review_rechecks` の config 化が実際に PARK signal に - /// 反映されることを machine-enforce する (default 3 ではなく custom 値が出力されること)。 - #[test] - fn format_review_park_signal_uses_configured_max_rechecks() { - let state = - PrMonitorState::new(Some(42), Some("o/r".into()), "2026-05-01T00:00:00Z".into()); - let pr_info = crate::util::PrInfo { - pr_number: Some(42), - repo: Some("o/r".into()), - push_time: Some("2026-05-01T00:00:00Z".into()), - head_commit: None, - fix_push_time: None, - }; - let checker = std::path::PathBuf::from("dummy"); - let rate_limit_config = RateLimitConfig::default(); - let classifier_config = ClassifierConfig::default(); - let ctx = PollContext { - checker: &checker, - state_path: std::path::Path::new("unused"), - push_time: "2026-05-01T00:00:00Z", - fix_push_time: None, - pr_info: &pr_info, - rate_limit_config: &rate_limit_config, - classifier_config: &classifier_config, - start: std::time::Instant::now(), - max_duration: 600, - skip_ci: false, - skip_coderabbit: false, - initial_review_wait_secs: 120, - review_recheck_wait_secs: 240, - max_review_rechecks: 7, - }; - - let signal = format_review_park_signal(&state, &ctx); - - assert!( - signal.contains("max_rechecks: 7"), - "PARK signal に config 値 (max_rechecks: 7) が反映されること: {}", - signal - ); - assert!( - !signal.contains("max_rechecks: 3"), - "default 値 3 が hard-coded で残っていないこと: {}", - signal - ); - } - - #[test] - fn format_review_park_signal_includes_safe_minute_iso_utc_field() { - let mut state = - PrMonitorState::new(Some(99), Some("o/r".into()), "2026-04-01T00:00:00Z".into()); - state.next_wakeup_at_unix = Some(1_775_044_819); - let pr_info = crate::util::PrInfo { - pr_number: Some(99), - repo: Some("o/r".into()), - push_time: Some("2026-04-01T00:00:00Z".into()), - head_commit: None, - fix_push_time: None, - }; - let checker = std::path::PathBuf::from("dummy"); - let rate_limit_config = RateLimitConfig::default(); - let classifier_config = ClassifierConfig::default(); - let ctx = PollContext { - checker: &checker, - state_path: std::path::Path::new("unused"), - 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 - ); - } -} diff --git a/src/cli-pr-monitor/src/state.rs b/src/cli-pr-monitor/src/state.rs index 78bcc3ba..89cec811 100644 --- a/src/cli-pr-monitor/src/state.rs +++ b/src/cli-pr-monitor/src/state.rs @@ -40,45 +40,21 @@ 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) で導入。 + /// state 記録時点の PR head commit OID (CR Major #1 fix, Bb-2 PR #114 review で導入)。 /// - /// 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" / "review_recheck"). Bb-1 で導入、Bb-2 で値追加。 - /// - /// Bb-2 (review 完了待ち) で `"review_recheck"` 経路を追加。Bb-3 (SessionStart catch-up) で - /// 複数の wakeup 経路を識別するための discriminator。 - #[serde(default, skip_serializing_if = "Option::is_none")] - pub(crate) wakeup_reason: Option, - /// review 完了待ちの recheck 回数 (Bb-2 で導入)。 - /// - /// `parked_review_recheck` 経路で wakeup が発火するたびにインクリメントされる。 - /// `max_review_rechecks` (config) 到達で `action_required` 経路に抜ける (review が想定時間内に - /// 完了していない通知)。新規 push で `PrMonitorState::new` により 0 にリセット、wakeup 経路で - /// は build_state_for_iteration が既存値を保持する。 - #[serde(default)] - pub(crate) review_recheck_count: u32, - /// park 時点の PR head commit OID (CR Major #1 fix, Bb-2 PR #114 review)。 - /// - /// `detect_wakeup_resume` が wakeup 判定時に「同 PR への新 push で head が変わって - /// いないか」を検証するために使う。state の pr / repo / next_wakeup_at_unix が - /// 一致しても head が変われば fresh push として扱い、stale state (started_at / - /// review_recheck_count) を新 commit に持ち込まない。値は `gh pr view --json - /// headRefOid` で取得した SHA。legacy state (本フィールド未設定) は wakeup 不一致 - /// 扱い (= fresh push 経路) で安全側に倒す。 + /// 同一 PR への連続 invocation で state (started_at = 時刻窓アンカー) を継続してよいかの + /// 判定に使う (`should_continue_state`)。pr / repo が一致しても head が変われば新 push と + /// して扱い、stale な時刻窓を新 commit に持ち込まない。値は `gh pr view --json headRefOid` + /// で取得した SHA。legacy state (本フィールド未設定) は不一致扱い (= fresh 初期化) で + /// 安全側に倒す。 #[serde(default, skip_serializing_if = "Option::is_none")] pub(crate) head_commit: Option, /// fresh push 時の固定 push_time (順位 141 実装、CR rate-limit detection bug 修正)。 /// - /// `started_at` は wakeup ごとに更新されるため、CR walkthrough overlay の `updated_at` - /// が `started_at` より過去になると `parse_rate_limit` の `event_time >= push_time` - /// filter で除外され検出失敗する。本 field は fresh push (= head_commit 確定時) の - /// 時刻で固定し、wakeup recheck 経路でも上書きしないことで「fix push 直後の overlay - /// は確実に検出」される invariant を保証する。 + /// CR walkthrough overlay の `updated_at` が時刻窓アンカーより過去になると + /// `parse_rate_limit` の `event_time >= push_time` filter で除外され検出失敗する。 + /// 本 field は fresh push (= head_commit 確定時) の時刻で固定し、後続 invocation でも + /// 上書きしないことで「fix push 直後の overlay は確実に検出」される invariant を保証する。 /// 値は ISO 8601 UTC (`utc_now_iso8601()` で生成)。`check-ci-coderabbit` への /// `--push-time` 引数として渡される。legacy state (本フィールド未設定) は `started_at` /// に fallback して挙動を維持する。 @@ -144,13 +120,24 @@ impl PrMonitorState { rate_limit: None, rate_limit_retries: 0, rate_limit_last_retriggered_at: None, - next_wakeup_at_unix: None, - wakeup_reason: None, - review_recheck_count: 0, head_commit: None, fix_push_time: None, } } + + /// 現在の PR head OID を state へ記録する (terminal 化の全経路が呼ぶ)。 + /// + /// **`Some` のときだけ上書きする**。`gh pr view --json headRefOid` は API 障害等で + /// `None` を返しうるため (`util::get_pr_head_commit` は失敗を None に潰す)、無条件代入に + /// すると取得失敗のたびに保存済み OID が消える。その状態で次回 `--monitor-only` を実行 + /// すると `should_continue_state` が `None` を見て継続を拒否し、時刻窓アンカーが「今」へ + /// リセットされて間に届いた CR コメントを取りこぼす (PR #237/#307/#309 と同型)。 + /// 取得失敗は「head が変わった」の証拠ではないため、既存値の保持が正しい (fail-safe)。 + pub(crate) fn record_head_commit(&mut self, current: Option<&str>) { + if let Some(oid) = current { + self.head_commit = Some(oid.to_string()); + } + } } /// state file の保存パスを返す (本番デフォルト = `/pr-monitor-state.json`)。 @@ -269,9 +256,6 @@ mod tests { rate_limit: None, rate_limit_retries: 0, rate_limit_last_retriggered_at: None, - next_wakeup_at_unix: None, - wakeup_reason: None, - review_recheck_count: 0, head_commit: None, fix_push_time: None, }; @@ -281,6 +265,36 @@ mod tests { assert_eq!(state, deserialized); } + /// CodeRabbit #353: `record_head_commit(None)` は保存済み OID を消さないこと。 + /// + /// `get_pr_head_commit` は gh 失敗を None に潰すため、無条件代入だと API 障害の + /// たびに継続判定 (`should_continue_state`) が壊れ、時刻窓アンカーがリセットされる。 + /// 取得失敗は「head が変わった」の証拠ではない。 + #[test] + fn record_head_commit_keeps_existing_when_current_unavailable() { + let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); + state.record_head_commit(Some("abc1234")); + + state.record_head_commit(None); + + assert_eq!( + state.head_commit.as_deref(), + Some("abc1234"), + "取得失敗 (None) で保存済み head_commit を消さないこと" + ); + } + + /// 新しい OID が取れたときは上書きすること (新 commit の push を反映する)。 + #[test] + fn record_head_commit_overwrites_when_current_available() { + let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); + state.record_head_commit(Some("abc1234")); + + state.record_head_commit(Some("def5678")); + + assert_eq!(state.head_commit.as_deref(), Some("def5678")); + } + #[test] fn state_serialize_roundtrip_with_head_commit() { let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); @@ -311,10 +325,7 @@ mod tests { }"#; let state: PrMonitorState = serde_json::from_str(legacy_json).unwrap(); - assert_eq!(state.review_recheck_count, 0); assert!(state.head_commit.is_none()); - assert!(state.next_wakeup_at_unix.is_none()); - assert!(state.wakeup_reason.is_none()); assert_eq!(state.rate_limit_retries, 0); assert!( state.fix_push_time.is_none(), @@ -322,6 +333,34 @@ mod tests { ); } + /// PR 3 (wakeup 廃止): 旧 state file に残る wakeup fields (next_wakeup_at_unix / + /// wakeup_reason / review_recheck_count) は unknown field として無視され、 + /// deserialize が失敗しないこと (park モデル時代の state file との前方互換)。 + #[test] + fn state_with_removed_wakeup_fields_still_deserializes() { + let park_era_json = r#"{ + "pr": 42, + "repo": "owner/repo", + "started_at": "2026-08-01T00:00:00Z", + "last_checked": null, + "ci": null, + "coderabbit": null, + "action": "parked_review_recheck", + "summary": "review check を 300s 後に予約", + "findings": [], + "notified": false, + "daemon_pid": null, + "daemon_status": "running", + "next_wakeup_at_unix": 1785732397, + "wakeup_reason": "review_recheck", + "review_recheck_count": 2 + }"#; + + let state: PrMonitorState = serde_json::from_str(park_era_json).unwrap(); + assert_eq!(state.pr, Some(42)); + assert_eq!(state.action, "parked_review_recheck"); + } + /// 順位 141 (PR #169 follow-up): `fix_push_time` field の round-trip 検証。 /// JSON への serialize / deserialize で値が保持されることを machine-enforce する。 #[test] @@ -350,66 +389,6 @@ mod tests { assert!(!json.contains("fix_push_time")); } - #[test] - fn state_serialize_roundtrip_with_review_recheck_count() { - let mut state = PrMonitorState::new(Some(42), Some("o/r".into()), "t".into()); - state.review_recheck_count = 2; - state.next_wakeup_at_unix = Some(1_775_088_000); - state.wakeup_reason = Some("review_recheck".into()); - - let json = serde_json::to_string(&state).unwrap(); - assert!(json.contains("review_recheck_count")); - assert!(json.contains("review_recheck")); - - let deserialized: PrMonitorState = serde_json::from_str(&json).unwrap(); - assert_eq!(deserialized.review_recheck_count, 2); - assert_eq!( - deserialized.wakeup_reason.as_deref(), - Some("review_recheck") - ); - } - - #[test] - fn state_default_review_recheck_count_is_zero() { - let state = PrMonitorState::new(Some(1), None, "t".into()); - assert_eq!(state.review_recheck_count, 0); - } - - #[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 = diff --git a/src/cli-pr-monitor/src/util.rs b/src/cli-pr-monitor/src/util.rs index 07d5d7e7..d04a733b 100644 --- a/src/cli-pr-monitor/src/util.rs +++ b/src/cli-pr-monitor/src/util.rs @@ -10,13 +10,13 @@ pub(crate) struct PrInfo { pub(crate) push_time: Option, /// 現在の PR head commit OID (CR Major #1 fix, Bb-2 PR #114 review)。 /// - /// `gh pr view --json headRefOid` で取得した SHA。`detect_wakeup_resume` が - /// state の head_commit と比較し、新 commit が push されていれば fresh push + /// `gh pr view --json headRefOid` で取得した SHA。`detect_state_continuity` が + /// state の head_commit と比較し、新 commit が push されていれば fresh 初期化 /// 経路に分岐させる。`pr_number` が None の段階では None。 pub(crate) head_commit: Option, /// fresh push 時の固定 push_time (順位 141 実装、CR rate-limit detection bug 修正)。 /// - /// `push_time` は wakeup 経路で `state.started_at` を再利用するため CR overlay の + /// `push_time` は state 継続経路で `state.started_at` を再利用するため CR overlay の /// `updated_at` が「過去扱い」で除外される構造バグがあった。本 field は fresh push /// 時刻を固定保持し、`check-ci-coderabbit` への `--push-time` 引数として優先される。 /// None の場合は `push_time` に fallback (legacy 互換)。 @@ -76,7 +76,7 @@ fn find_pr_via_jj_bookmarks() -> Option { /// CR Major #1 fix (Bb-2 PR #114 review): PR の現在 head commit OID を `gh pr view` /// で取得する。失敗時は None を返す (caller は head_commit None を「不明」として扱い、 -/// detect_wakeup_resume 側で fresh push 経路に倒す)。 +/// detect_state_continuity 側で fresh 初期化経路に倒す)。 pub(crate) fn get_pr_head_commit(pr_number: u64, repo: Option<&str>) -> Option { let pr_str = pr_number.to_string(); let mut args: Vec<&str> = vec![ diff --git a/src/cli-telemetry-report/src/config.rs b/src/cli-telemetry-report/src/config.rs index c5d9997c..f2f1d9f0 100644 --- a/src/cli-telemetry-report/src/config.rs +++ b/src/cli-telemetry-report/src/config.rs @@ -41,7 +41,7 @@ pub struct TelemetryReportConfig { /// `[telemetry_report.registry]` の設定値。自動列挙元が無い hook / nudge 発火 id を静的に列挙する /// (設計決定 1 § hook / nudge)。id は hook 名と一致しない例がある (`jj-op-verify` / -/// `pr_monitor_catchup` / `hooks-stop-tool-call-leak/prompt-recovery` 等、各 hook の +/// `weekly_review_reminder` / `hooks-stop-tool-call-leak/prompt-recovery` 等、各 hook の /// `lib_telemetry::record` 実装で確認)。 #[derive(Debug, Default, Clone, Deserialize)] pub struct RegistryConfig { diff --git a/src/hooks-session-start/src/main.rs b/src/hooks-session-start/src/main.rs index 16ceb9aa..5006f6e5 100644 --- a/src/hooks-session-start/src/main.rs +++ b/src/hooks-session-start/src/main.rs @@ -1,14 +1,16 @@ -//! SessionStart hook — セッション ID を環境変数とファイルに伝播する + PR monitor catch-up +//! SessionStart hook — セッション ID を環境変数とファイルに伝播する + 各種 nudge //! //! Claude Code の SessionStart イベントで発火し、以下の経路で session 起動準備を行う: //! //! 1. $CLAUDE_ENV_FILE に export 文を追記 → Bash ツールから参照可能 //! 2. .claude/.session-id ファイルに書き出し → 子プロセス (exe) から参照可能 -//! 3. PR monitor catch-up: `pr_monitor` module -//! 4. Orphan run reaper (ADR-030 §L2): `reaper` module -//! 5. Working copy staleness nudge: `staleness` module -//! 6. Weekly review reminder (ADR-031 Phase C): `weekly_review` module -//! 7. Monthly review reminder (ADR-062, WP-12 step 2/3): `monthly_review` module +//! 3. Orphan run reaper (ADR-030 §L2): `reaper` module +//! 4. Working copy staleness nudge: `staleness` module +//! 5. Weekly review reminder (ADR-031 Phase C): `weekly_review` module +//! 6. Monthly review reminder (ADR-062, WP-12 step 2/3): `monthly_review` module +//! +//! 旧 3. PR monitor catch-up (Bb-3) は WP-17 PR 3 で撤去 — park/wakeup モデルの廃止に伴い +//! 「park 失効の救済」という役割ごと dead code になった (ADR-018 amendment 参照)。 //! //! 各 nudge の発火は `lib-telemetry` (ADR-055) に `warn` として記録され、ROI 棚卸しの //! 観測基盤 (`.claude/telemetry/firings-*.jsonl`) に載る (fail-open)。 @@ -26,14 +28,12 @@ mod hooks_config; mod jj_helpers; mod monthly_review; mod past_time; -mod pr_monitor; mod reaper; mod staleness; mod weekly_review; use hooks_config::read_hooks_config; use monthly_review::compute_monthly_review_reminder_nudge; -use pr_monitor::{compute_catchup_nudge, pr_monitor_state_path, read_parked_state}; use reaper::compute_reaper_nudge; use staleness::{compute_staleness_nudge, compute_workspace_stale_nudge}; use weekly_review::compute_weekly_review_reminder_nudge; @@ -109,7 +109,7 @@ fn main() { emit_session_start_output(&session_id); } -/// `additionalContext` (session_id + 任意の nudge 群: PR monitor catch-up / reaper / staleness / +/// `additionalContext` (session_id + 任意の nudge 群: reaper / staleness / /// workspace_stale / weekly review) と任意の `systemMessage` を組み立て、Claude Code に返す JSON を /// stdout に書き出す。各 nudge の追記と telemetry 記録はヘルパーに委譲する。 /// serde_json で組み立てることで session_id 内の特殊文字を安全にエスケープする。 @@ -117,7 +117,6 @@ fn emit_session_start_output(session_id: &str) { let mut context = format!("CLAUDE_CODE_SESSION_ID={}", session_id); let mut system_message: Option = None; let now_unix = current_unix_secs(); - append_pr_monitor_catchup_nudge(&mut context, session_id, now_unix); if let Ok(cwd) = std::env::current_dir() { system_message = append_cwd_nudges(&mut context, session_id, &cwd, now_unix); } @@ -125,18 +124,6 @@ fn emit_session_start_output(session_id: &str) { println!("{}", output); } -/// PR monitor catch-up nudge を `context` に追記し、発火時は telemetry に記録する。 -/// この nudge は cwd に依存せず parked state ファイルのみを見るため独立したヘルパーにする。 -fn append_pr_monitor_catchup_nudge(context: &mut String, session_id: &str, now_unix: i64) { - if let Some(state) = read_parked_state(&pr_monitor_state_path()) { - if let Some(nudge) = compute_catchup_nudge(&state, now_unix) { - context.push_str("\n\n"); - context.push_str(&nudge); - record_nudge_firing("pr_monitor_catchup", session_id); - } - } -} - /// cwd 依存の nudge 群 (reaper / staleness / workspace_stale / weekly review / monthly review) を /// `context` に追記し、発火時は telemetry に記録する。weekly / monthly review はユーザー可視の /// systemMessage を伴い、両方発火した場合は 1 行に合成して返す (systemMessage スロットは 1 つのため。 @@ -224,7 +211,7 @@ fn combine_system_messages(messages: Vec) -> Option, - pub(crate) repo: Option, - pub(crate) next_wakeup_at_unix: Option, - pub(crate) wakeup_reason: Option, - /// 監視ステータス。`"parked_*"` (parked_rate_limit / parked_review_recheck) のみ - /// catch-up nudge の対象。`"action_required"` 等の terminal 値では - /// next_wakeup_at_unix が古い park 由来で残っていても nudge を抑制する。 - #[serde(default)] - pub(crate) action: String, -} - -/// cli-pr-monitor の state file パス (`/pr-monitor-state.json`)。 -/// hooks-session-start.exe は cli-pr-monitor.exe と同じ `.claude/` 配下に配置される -/// 前提 (deploy:hooks スクリプトで保証)。 -pub(crate) fn pr_monitor_state_path() -> PathBuf { - std::env::current_exe() - .unwrap_or_default() - .parent() - .unwrap_or(Path::new(".")) - .join("pr-monitor-state.json") -} - -/// `next_wakeup_at_unix` が現在時刻以前なら catch-up nudge メッセージを返す。 -/// -/// session が park 中に終了 (CronCreate 発火前) し、後で再開された場合、 -/// CronCreate スケジュールが消えているため自動 wakeup は起こらない。 -/// このとき手動で監視継続するための指示を Claude に渡す。 -/// -/// 返り値: Some(message) なら additionalContext に注入する文字列、None なら何もしない。 -/// -/// 抑制条件: action が `"parked_*"` でない (= terminal 状態) 場合、`next_wakeup_at_unix` -/// が残っていても false-positive nudge を出さない。terminal 経路では cli-pr-monitor が -/// `next_wakeup_at_unix` を明示クリアしないため、action ベースの guard が必要。 -pub(crate) fn compute_catchup_nudge(state: &ParkedStatePartial, now_unix: i64) -> Option { - if !state.action.starts_with("parked_") { - return None; - } - let wakeup_at = state.next_wakeup_at_unix?; - if wakeup_at > now_unix { - return None; - } - let pr = state - .pr - .map(|n| format!("#{}", n)) - .unwrap_or_else(|| "?".into()); - let repo = state.repo.as_deref().unwrap_or("?"); - let reason = state.wakeup_reason.as_deref().unwrap_or("unknown"); - Some(format!( - "[PR_MONITOR_CATCHUP]\n\ - pending wakeup detected for PR {pr} ({repo}), reason={reason}, scheduled_at_unix={wakeup_at}, now={now_unix}.\n\ - CronCreate may have expired during session downtime. If the PR is still relevant, run `{cmd}` to resume monitoring.", - cmd = RESUME_MONITORING_COMMAND - )) -} - -pub(crate) fn read_parked_state(path: &Path) -> Option { - let content = std::fs::read_to_string(path).ok()?; - serde_json::from_str(&content).ok() -} - -#[cfg(test)] -mod tests { - use super::*; - - fn parked_state( - pr: Option, - repo: Option<&str>, - wakeup_at: Option, - reason: Option<&str>, - action: &str, - ) -> ParkedStatePartial { - ParkedStatePartial { - pr, - repo: repo.map(String::from), - next_wakeup_at_unix: wakeup_at, - wakeup_reason: reason.map(String::from), - action: action.into(), - } - } - - #[test] - fn catchup_nudge_none_when_no_wakeup_scheduled() { - let state = parked_state( - Some(42), - Some("o/r"), - None, - Some("review_recheck"), - "parked_review_recheck", - ); - assert!(compute_catchup_nudge(&state, 1_775_088_000).is_none()); - } - - #[test] - fn catchup_nudge_none_when_wakeup_in_future() { - let state = parked_state( - Some(42), - Some("o/r"), - Some(1_775_088_000), - Some("review_recheck"), - "parked_review_recheck", - ); - let now = 1_775_087_999; - assert!(compute_catchup_nudge(&state, now).is_none()); - } - - #[test] - fn catchup_nudge_emitted_when_wakeup_passed() { - let state = parked_state( - Some(42), - Some("owner/repo"), - Some(1_775_088_000), - Some("review_recheck"), - "parked_review_recheck", - ); - let now = 1_775_088_001; - let msg = compute_catchup_nudge(&state, now).expect("nudge should be emitted"); - assert!(msg.contains("[PR_MONITOR_CATCHUP]")); - assert!(msg.contains("PR #42")); - assert!(msg.contains("owner/repo")); - assert!(msg.contains("review_recheck")); - assert!( - msg.contains(RESUME_MONITORING_COMMAND), - "nudge は const RESUME_MONITORING_COMMAND を hint として埋め込むこと (pre-push-review #115 [B] 対策、コマンド名 rename 時に test が落ちて drift を catch)" - ); - } - - #[test] - fn catchup_nudge_handles_missing_optional_fields() { - let state = parked_state(None, None, Some(0), None, "parked_review_recheck"); - let msg = compute_catchup_nudge(&state, 1).expect("nudge should still be emitted"); - assert!(msg.contains("PR ?")); - assert!(msg.contains("(?)")); - assert!(msg.contains("reason=unknown")); - } - - /// terminal 経路では `next_wakeup_at_unix` が古い park 由来で残っていても - /// false-positive nudge を出さない (advisor 指摘: 順位 55 review)。 - #[test] - fn catchup_nudge_suppressed_for_terminal_action_required() { - let state = parked_state( - Some(42), - Some("o/r"), - Some(1_775_088_000), - Some("review_recheck"), - "action_required", - ); - let now = 1_775_088_001; - assert!( - compute_catchup_nudge(&state, now).is_none(), - "terminal 経路 (action_required) では nudge を抑制すること" - ); - } - - #[test] - fn catchup_nudge_suppressed_for_continue_monitoring() { - let state = parked_state( - Some(42), - Some("o/r"), - Some(1_775_088_000), - None, - "continue_monitoring", - ); - let now = 1_775_088_001; - assert!(compute_catchup_nudge(&state, now).is_none()); - } - - #[test] - fn catchup_nudge_emitted_for_parked_rate_limit() { - let state = parked_state( - Some(42), - Some("o/r"), - Some(1_775_088_000), - Some("rate_limit_retry"), - "parked_rate_limit", - ); - let msg = compute_catchup_nudge(&state, 1_775_088_001) - .expect("parked_rate_limit should emit nudge"); - assert!(msg.contains("rate_limit_retry")); - } - - #[test] - fn read_parked_state_returns_none_when_file_missing() { - let tmp = - std::env::temp_dir().join(format!("test-parked-state-missing-{}", std::process::id())); - let _ = std::fs::remove_file(&tmp); - assert!(read_parked_state(&tmp).is_none()); - } - - #[test] - fn read_parked_state_parses_partial_fields() { - let tmp = - std::env::temp_dir().join(format!("test-parked-state-partial-{}", std::process::id())); - let json = r#"{ - "pr": 42, - "repo": "owner/repo", - "started_at": "2026-05-01T00:00:00Z", - "action": "parked_review_recheck", - "summary": "...", - "notified": false, - "daemon_pid": null, - "daemon_status": "active", - "next_wakeup_at_unix": 1775088000, - "wakeup_reason": "review_recheck" - }"#; - std::fs::write(&tmp, json).unwrap(); - - let state = read_parked_state(&tmp).expect("should parse"); - assert_eq!(state.pr, Some(42)); - assert_eq!(state.repo.as_deref(), Some("owner/repo")); - assert_eq!(state.next_wakeup_at_unix, Some(1_775_088_000)); - assert_eq!(state.wakeup_reason.as_deref(), Some("review_recheck")); - assert_eq!(state.action, "parked_review_recheck"); - - let _ = std::fs::remove_file(&tmp); - } - - #[test] - fn pr_monitor_state_path_ends_with_filename() { - let path = pr_monitor_state_path(); - assert!(path.to_string_lossy().ends_with("pr-monitor-state.json")); - } -} diff --git a/templates/pr-monitor-config.toml b/templates/pr-monitor-config.toml index d6387d20..36a1c897 100644 --- a/templates/pr-monitor-config.toml +++ b/templates/pr-monitor-config.toml @@ -5,10 +5,10 @@ [monitor] enabled = true -poll_interval_secs = 120 -max_duration_secs = 600 check_ci = true check_coderabbit = true +# (旧 poll_interval_secs / max_duration_secs は廃止済み — single-shot 化 (WP-17 PR 3) に +# よりポーリングループ自体が消滅。残っていても unknown field として無視される) # takt セクションを有効にすると、ポーリング完了後に # CodeRabbit 指摘の AI 分析を実行する。