fix(monitor): レート制限中のレビュー未実施を success と誤判定する fail-open を修正 (WP-15 追補) - #311
Merged
Conversation
旧 PR #309 (fix/monitor-fail-open-signals) は、4 コミット目の初版が本番 config で 一度も実行されない誤修正 + 実エントリポイントを迂回して pass するテストであり、 takt の High REJECT → fix step の自動書き直しを経た合成物となった。続修より ゼロ再構築が速いとのユーザー判断 (2026-07-20) に基づき、再利用なしの全破棄を 決定。健全に見えるコミットも含めて引き継がない (中途半端な状態の引き継ぎと、 それによる実装の制約を排除するため)。 追補には、新実装が単独セッションで着手できるよう以下を自己完結で記録した: - 破棄対象と手順 (PR close / branch 削除 / local abandon — 本コミット時点で未実施) - 要件 R1-R5 (What のみ。実装方式は新実装の裁量、旧コードは参照しない) - コード実読で検証済みの根本原因チェーン 6 段 (再調査不要) - 検証要件 (両 OS 全スイート / incident 実データでの checker 単体実測 / E2E カバレッジの正直な申告 / 経路同一性を確認してから実測を主張する規律) - 旧作業の教訓 (本番経路での発火をテストで固定・実エントリポイント・実データ fixture) 本コミットはドキュメント更新のみ。#309 の close・破棄・新実装には着手していない。 lint:docs / lint:md pass。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CodeRabbit が rate-limit comment の待機時間書式を 3 度目に変更 (PR #309、 2026-07-20 実観測) し、`parse_rate_limit` が None を返して rate-limit 検知が 沈黙した。ADR-034 が予告していた再発事案 (PR #182/#184 に次ぐ 2 度目の 書式変更起因 regression)。 - 第 3 世代書式 `**Next review available in:** **57 minutes**` の抽出を追加。 ラベルと数値の間に markdown 強調と `:` が挟まるため区切りを `[:*\s]*` で 吸収し、CR が強調記法を変えても壊れないようにした。 - **書式追随を前提にしない fail-closed 化**: marker (`rate limited by coderabbit.ai`) が一致したのに待機時間をどの既知書式でも読めない場合、 従来は None = 「rate-limit ではない」に倒れていた。marker 一致を制限の 根拠として採用し、待機時間だけを既定 30 分で埋める方式に変更。既定値が 実 reset より短ければ wakeup 後に再検出されて再 park されるだけで、 retry は max_retries で有界。 - 既定値適用時は checker が stderr に警告 (cli-pr-monitor がログ転送) し、 「30 分」を CR の申告値と誤読させない + 書式再変更の検知シグナルを兼ねる。 fixture は PR #309 の実 comment body を出典付きで埋め込み (ADR-049)。この comment は walkthrough header marker を同一 body に併せ持つため、clean walkthrough と誤認しない排他も併せて固定した。 ADR-034 の既知 format 一覧に第 3 世代行と fallback 方針を追記し、更新手順の stale なファイル参照 (main.rs → markers.rs / rate_limit.rs) を修正。
… R1/R4) CodeRabbit がレート制限でレビューを開始できないまま、監視が「レビュー済み・ 指摘なし」と報告する silent success を、判定ロジック側で塞ぐ。 ## 根本原因 (2026-07-20 コード実読 + PR #307/#309 実観測) CR はレート制限中も commit check を pass にする (外部 SaaS 挙動)。checker は これを review_state に採用する一方、parse_rate_limit の結果は出力 JSON に 添付されるだけで decide() には渡っていなかった。結果、decide() は 「review_state = success かつ指摘ゼロ」= 完了と読み、stop_monitoring_success を返していた。monitor 側の terminal 短絡は rate-limit branch より先に発火する ため、park / 再 trigger 機構は一度も呼ばれない。 症状は monitor 側に出るが、原因は action の算出そのものにある。旧 PR #309 は 症状側 (monitor 2 箇所) への多層パッチで High REJECT を受けたため、本実装では 算出点である decide() に一本化した。 ## 変更 - decide() / build_summary() に rate_limit を渡す。 - R1: rate-limit 検出中かつ「レビュー実施の陽性証拠」が無ければ continue_monitoring を返し、判断を monitor の rate-limit branch (park / 再 trigger、既存・有界) に委ねる。has_actionable 分岐より前に置くことで、 過去サイクル由来の未解決スレッドで action_required に抜ける穴も塞ぐ。 - R4: rate-limit を検出できなかった場合の backstop として、陽性証拠が無い限り stop_monitoring_success を出さない。CR が marker 文言自体を変えても silent success には戻らず、最悪 max_duration までの監視継続 (timed_out) に倒れる。 - build_summary は rate-limit 中に「CodeRabbit指摘なし」と断定せず 「レート制限中 (レビュー未実施)」を出す。 ## 陽性証拠の定義 review_state (commit status) は制限中でも pass になるため証拠に使わない。 push_time で絞られた「今サイクルの CR 出力そのもの」= walkthrough_clean / actionable_comments が読めた (Some(0) 含む) / new_comments > 0 のみを採用する。 unresolved_threads は push_time で絞られず過去サイクルの残骸を含み得るので 除外した。 ## 残存リスク 陽性証拠を一切残さない clean レビュー経路が CR 側に存在した場合、監視が max_duration まで走って timed_out 報告になる。silent success より安全側だが 遅くなる。既存 fixture の範囲では walkthrough_clean か actionable_comments の いずれかが必ず立つことを確認済み。 既存 decide/summary テスト 100 件は無改修で pass (新 gate が確立済み挙動を 乱していないことの確認)。incident 再現テストは PR #309 の実観測値から構成。
監視レポートの人間向け判定文が、findings が空というだけで「問題は見つかり ませんでした」と断定していた。PR #307/#309 では「未解決スレッド2件」を表示 しながら同一レポート内で「問題は見つかりませんでした」と結論する矛盾が実観測 されている。 findings が空であることは「見るべきものが無かった」の十分条件ではない。 レート制限でレビューが走っていない場合も、未解決スレッドが残っている場合も 空になり得る。 - rate-limit 検出中は保留判定文を出す (R1 で checker から rate_limit が 届くようになったため、monitor 側で判別可能になった)。 - 未解決スレッドが残っている間は「問題なし」「重大な問題なし」のいずれも 出さず、件数を添えて保留する。重大な指摘がある場合は「修正が必要」を優先。 - 判定順を「未確定 → 重大 → 未解決 → 軽微 → 問題なし」に整理し、未確定要素を findings の有無より先に評価する。断定文へ落ちる経路を構造的に塞ぐ形。 compute_verdict は未確定判定 (verdict_for_unsettled_review) と findings 判定 (verdict_for_findings) に分割し、「断定文はどの guard を通過して初めて出せる のか」を関数境界で表現した (50 行ガイドラインにも整合)。 既存の verdict テスト 13 件は無改修で pass。
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughCodeRabbit の第3世代 rate-limit 書式、未知書式の待機時間フォールバック、レビュー証拠に基づく判定、未確定シグナルを優先する monitor verdict と関連テスト・文書を更新した。 ChangesRate-limit monitoring
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CodeRabbit
participant parse_rate_limit
participant run_check
participant decide
participant compute_verdict
CodeRabbit->>parse_rate_limit: rate-limit comment
parse_rate_limit->>run_check: RateLimitInfo
run_check->>decide: CI, review status, rate-limit
decide->>run_check: pending or completed action
run_check->>compute_verdict: monitoring result
compute_verdict->>compute_verdict: prioritize unsettled signals
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
軽量サマリー (レビュー指摘 0 件のため)変更概要 (7 ファイル、+637 / -50):
変更の性質: PR タイトルどおり、監視系の fail-open 修正 (WP-15 追補)。plan ( 次のアクション
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/check-ci-coderabbit/src/rate_limit.rs (1)
116-141: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
extract_old/new/next_review_format_wait_timeの重複ロジックを共通化する余地あり3つの
extract_*_format_wait_timeはいずれも「full pattern（分+秒）→ minutes-only pattern」の同一構造を繰り返しています。ADR-034 にも記載の通り CR は書式をこれまで既に2回変更し、今回で3世代目です。今後も同様の追加が予想されるため、(full_regex, minutes_only_regex)のペアをテーブル化し、共通ヘルパーで走査する形にすると、新書式追加時の実装コストと重複を下げられます。♻️ 共通化の方向性（イメージ）
+fn try_extract(body: &str, full_re: &str, min_re: &str) -> Option<(u64, u64)> { + if let Some(caps) = regex::Regex::new(full_re).ok()?.captures(body) { + let m: u64 = caps.get(1)?.as_str().parse().ok()?; + let s: u64 = caps.get(2)?.as_str().parse().ok()?; + return Some((m, s)); + } + let caps = regex::Regex::new(min_re).ok()?.captures(body)?; + let m: u64 = caps.get(1)?.as_str().parse().ok()?; + Some((m, 0)) +} + pub(crate) fn extract_wait_time(body: &str) -> Option<(u64, u64)> { - extract_old_format_wait_time(body) - .or_else(|| extract_new_format_wait_time(body)) - .or_else(|| extract_next_review_format_wait_time(body)) + const FORMATS: &[(&str, &str)] = &[ + (r"Please wait \*?\*?(\d+) minutes? and (\d+) seconds?", r"Please wait \*?\*?(\d+) minutes?"), + (r"More reviews will be available in (\d+) minutes? and (\d+) seconds?", r"More reviews will be available in (\d+) minutes?"), + (r"Next review available in[:*\s]*(\d+) minutes?[*\s]*and[:*\s]*(\d+) seconds?", r"Next review available in[:*\s]*(\d+) minutes?"), + ]; + FORMATS.iter().find_map(|(full, min)| try_extract(body, full, min)) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/check-ci-coderabbit/src/rate_limit.rs` around lines 116 - 141, 共通の「分+秒」形式を試し、失敗時に「分のみ」形式へフォールバックする処理をヘルパーへ抽出し、extract_old_format_wait_time、extract_new_format_wait_time、extract_next_review_format_wait_time から再利用してください。各形式の正規表現ペアはテーブル化して extract_wait_time から順番に走査し、既存の世代順と戻り値 `(minutes, seconds)`（分のみは秒を0）を維持してください。src/cli-pr-monitor/src/stages/monitor.rs (1)
641-723: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win未確定シグナル間の優先順位テストを追加する余地あり。
verdict_critical_takes_precedence_over_unresolved_threads（Line 711-714）で「重大指摘 vs 未解決スレッド」の優先順位はテストされていますが、「rate_limit=Someかつ critical finding あり」という組み合わせ（verdict_for_unsettled_reviewがverdict_for_findingsより先に評価される、という設計上の前提）を直接検証するテストはありません。ドキュメント上の意図（Line 324: 「未確定 → 重大」の順）を将来のリファクタで壊さないためのロック用テストとして追加を検討してください。♻️ 追加テストの例
#[test] fn verdict_rate_limit_takes_precedence_over_critical_findings() { let r = poll_result_with_unsettled( Some(0), Some(pr309_rate_limit()), vec![finding("critical")], ); let verdict = compute_verdict(&r); assert!(verdict.contains("レート制限"), "verdict={}", verdict); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli-pr-monitor/src/stages/monitor.rs` around lines 641 - 723, Add a test beside verdict_critical_takes_precedence_over_unresolved_threads that builds a PollResult with pr309_rate_limit() and a critical finding, then verifies compute_verdict returns wording containing “レート制限”. This must lock in that the rate-limit/unsettled-review verdict is evaluated before critical findings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/check-ci-coderabbit/src/rate_limit.rs`:
- Around line 116-141:
共通の「分+秒」形式を試し、失敗時に「分のみ」形式へフォールバックする処理をヘルパーへ抽出し、extract_old_format_wait_time、extract_new_format_wait_time、extract_next_review_format_wait_time
から再利用してください。各形式の正規表現ペアはテーブル化して extract_wait_time から順番に走査し、既存の世代順と戻り値 `(minutes,
seconds)`（分のみは秒を0）を維持してください。
In `@src/cli-pr-monitor/src/stages/monitor.rs`:
- Around line 641-723: Add a test beside
verdict_critical_takes_precedence_over_unresolved_threads that builds a
PollResult with pr309_rate_limit() and a critical finding, then verifies
compute_verdict returns wording containing “レート制限”. This must lock in that the
rate-limit/unsettled-review verdict is evaluated before critical findings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d3c11468-2e05-4f84-9bab-6813e7799311
📒 Files selected for processing (7)
docs/adr/adr-034-coderabbit-auto-monitoring.mddocs/harness-improvement-plan.mdsrc/check-ci-coderabbit/src/decide.rssrc/check-ci-coderabbit/src/main.rssrc/check-ci-coderabbit/src/models.rssrc/check-ci-coderabbit/src/rate_limit.rssrc/cli-pr-monitor/src/stages/monitor.rs
Contributor
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)該当なし Applicable Findings (Medium 以下)
Filtered (not applicable)該当なし 次のアクション
|
## R5: WP-15 `完了` 条件 (1) の達成記録 PR #307 マージ時の release-binaries.yml run は build job が失敗しており (master が赤で #308 の E2E 修正が必要だった)、成功したのは #308 マージ後の run (commit 541adde)。この経緯も含めて記録した。 生成物は本セッションで再実測している: WSL Ubuntu 24.04 から素の curl で tarball (9,721,643 bytes) + .sha256 を**認証なしで**取得 → sha256sum -c 一致 → 展開して 16 バイナリ + BUILD_INFO 確認 → release バイナリそのもので hooks 実発火 (pre-tool-validate が破壊的削除コマンドを exit 2 でブロックし無害な echo を exit 0 で通す / session-start が additionalContext JSON を出力)。 これで WP-15 ④ の「public リポジトリの Release asset は素の HTTPS で取得 できるため gh CLI 認証は不要」という設計判断が実 URL・実 asset で裏づけられた。 旧 PR #309 の docs コミットの主張を引き写すのではなく、自分で再実測した結果を 記録している。 ## 追補の実装状況 R1〜R4 の実装内容、破棄の実施結果、検証の実測値を反映。E2E カバレッジは 正直に申告した: 担保できたのは全 gate のユニット検証 / checker 実エントリ ポイントの実データ実走 / 修正前後の差分実測の 3 点で、cli-pr-monitor 側の 統合経路 (park → PARK signal) と wakeup → 再 trigger 経路は、CR レート制限が 本セッション中に自然発生しなかったため未実測である旨を明記した。 なお本ファイルは master 時点で既に 59,798 bytes と file_size_check の 50KB 閾値を超過しており (non-blocking 警告)、本変更で 73,781 bytes になった。 分割は § 9 の退役手順で本ファイルごと削除する前提のため見送る。
セルフレビュー (pre-push-review simplicity facet) の指摘。 `wait_time_parsed` の doc は「監視側はこれを見て『実測』と『既定値』を区別 して報告する」と書いていたが、実装では monitor 側への配線を見送っており (cli-pr-monitor の RateLimitState は本 field を持たない)、記述と実装が 食い違っていた。既定値適用を運用者に伝える経路は実際には checker の stderr 警告 (monitor がログ転送) が担っている。 doc を実態に合わせ、あわせて「なぜ typed 化を見送ったか」(全 struct literal の 更新コストに対し得られるのが park summary の文言精度という副次的利得)と 「値がどこから参照できるか」(monitor が保持する checker の生 JSON) を明記した。 なお本指摘は、PR 全体 (master..@) を対象にセルフレビューを再実行して初めて 検出された。push 時のパイプラインは [diff] stage が tip コミットのみを レビュアーに渡すため (push-runner-config.toml の `command = "jj diff -r @"`)、 models.rs を含む祖先コミットがレビュー対象外だった。この構造的欠陥は docs/todo-summary2.md 順位 288 (Tier 1、Severity High で 3 連続再発) として 既知で、本 PR で 4 回目の再発となった。修正は独立 PR で行う。
aloekun
force-pushed
the
fix/monitor-review-evidence-gate
branch
from
July 20, 2026 17:46
ded8999 to
36c5d18
Compare
aloekun
added a commit
that referenced
this pull request
Jul 21, 2026
## 問題 `[diff] command` に `jj diff -r @` が直書きされており、AI レビュアーには **tip コミットの diff しか渡っていなかった**。祖先コミットは pre-push のセルフレビューを 一度も経ずに merge される。 同じパイプライン内で `pr_size_check` と `docs_only_routing` は `<base>..@` (PR 範囲) を見ており、`[diff]` だけが非対称だった。実際 PR #311 では 695 行の PR に対して 37 行だけがレビュー対象になり、レビュアーは渡された 37 行を見て正しく「docs-only」と 判定していた。**レビュアー側からは「渡された diff が PR 全体か」を検証できない**ため、 この誤りは誰にも検知されない。 docs/todo-summary2.md 順位 288 として既知で、Severity High で PR #268/#300/#301 に 続き #311 が 4 回目の再発。 ## 変更 - **範囲の真実源を 1 箇所に**: top-level `default_branch` を新設し、`diff` / `docs_only_routing` / `pr_size_check` の 3 stage が `Config::resolve_base_branch` 経由で同じ値を使う。従来は section ごとに独立した `default_branch` を持ち、 「値を同期する義務」を config コメントで課していた (docs_only_routing.rs の doc に 明記されていた) が、義務はコード上の不変条件ではないため非対称を許していた。 section 側は後方互換の override として残す (派生プロジェクトの既存 config 対策)。 - **config から revset を排除**: `[diff] command` は `{{PR_RANGE}}` プレースホルダを 使う。push-runner が `<base>..@` に展開するため、狭い範囲を書く余地が無くなる。 - **範囲カバレッジ検査 (fail-closed)**: 生成した diff が PR 範囲の全変更ファイルを 含むか `jj diff --summary` と突き合わせ、不足があれば exit 5 で中断する。config の 書き方に依存せず「レビュー範囲 < PR 範囲」を検知できるため、未更新の派生プロジェクト config も捕まえる。summary 取得失敗・`diff --git` ヘッダ不在 (= 収録ファイルを特定 できない) も「網羅している」に倒さずエラーにする (ADR-043)。 - **`--git` 形式へ切替 (順位 264)**: 範囲検査がヘッダを読む要件に加え、jj 既定形式は 色を落とすと `+`/`-` が消えて LLM レビュアーが削除を追加と誤読する (PR #256 で simplicity-review が todo 25 行の削除を追加と誤読し false positive REJECT、約 19 分 浪費)。 - `templates/push-runner-config.toml` も同時修正 (deploy:hooks で派生配布されるため)。 ## 実測で見つけた副次バグ 範囲 summary の取得を当初 shell 経由 (`[diff] command` と同じ経路) で実装したが、 実シェルで叩いたところ **cmd.exe はクォートを除去せず jj に渡す**ため `Revision '"<base>..@"' doesn't exist` で必ず失敗した。sh は除去するので Linux だけ 通る = Windows の全 push が fail-closed で止まる形。ユニットテストは summary 取得を 注入していたため検出できなかった。direct args 呼び出し (docs_only_routing の既存 `run_jj_diff_summary`) を共有する形に修正し、doc に理由を残した。両 stage は 「PR 範囲の変更ファイル一覧」という同一の問いを扱うため、別実装にすると本 PR が 排除した非対称を再導入することになる。 ## 検証 cargo test --workspace 全 pass / clippy clean。範囲検査は incident 形状 (PR 2 ファイル 変更に対し diff は tip の 1 ファイルのみ) の再現テストで固定し、fail-closed 経路 (summary 取得失敗 / ヘッダ不在) と過剰検知しない経路 (空 PR 範囲 / Windows パス 区切りの正規化) も併せて固定した。base branch 解決は 3 stage が同一範囲に解決される ことと override の優先順位を machine-enforce している。
aloekun
added a commit
that referenced
this pull request
Jul 21, 2026
) * fix(push-runner): AI レビュー対象 diff を PR 全体に修正し範囲を機械検査する (順位 288/264) ## 問題 `[diff] command` に `jj diff -r @` が直書きされており、AI レビュアーには **tip コミットの diff しか渡っていなかった**。祖先コミットは pre-push のセルフレビューを 一度も経ずに merge される。 同じパイプライン内で `pr_size_check` と `docs_only_routing` は `<base>..@` (PR 範囲) を見ており、`[diff]` だけが非対称だった。実際 PR #311 では 695 行の PR に対して 37 行だけがレビュー対象になり、レビュアーは渡された 37 行を見て正しく「docs-only」と 判定していた。**レビュアー側からは「渡された diff が PR 全体か」を検証できない**ため、 この誤りは誰にも検知されない。 docs/todo-summary2.md 順位 288 として既知で、Severity High で PR #268/#300/#301 に 続き #311 が 4 回目の再発。 ## 変更 - **範囲の真実源を 1 箇所に**: top-level `default_branch` を新設し、`diff` / `docs_only_routing` / `pr_size_check` の 3 stage が `Config::resolve_base_branch` 経由で同じ値を使う。従来は section ごとに独立した `default_branch` を持ち、 「値を同期する義務」を config コメントで課していた (docs_only_routing.rs の doc に 明記されていた) が、義務はコード上の不変条件ではないため非対称を許していた。 section 側は後方互換の override として残す (派生プロジェクトの既存 config 対策)。 - **config から revset を排除**: `[diff] command` は `{{PR_RANGE}}` プレースホルダを 使う。push-runner が `<base>..@` に展開するため、狭い範囲を書く余地が無くなる。 - **範囲カバレッジ検査 (fail-closed)**: 生成した diff が PR 範囲の全変更ファイルを 含むか `jj diff --summary` と突き合わせ、不足があれば exit 5 で中断する。config の 書き方に依存せず「レビュー範囲 < PR 範囲」を検知できるため、未更新の派生プロジェクト config も捕まえる。summary 取得失敗・`diff --git` ヘッダ不在 (= 収録ファイルを特定 できない) も「網羅している」に倒さずエラーにする (ADR-043)。 - **`--git` 形式へ切替 (順位 264)**: 範囲検査がヘッダを読む要件に加え、jj 既定形式は 色を落とすと `+`/`-` が消えて LLM レビュアーが削除を追加と誤読する (PR #256 で simplicity-review が todo 25 行の削除を追加と誤読し false positive REJECT、約 19 分 浪費)。 - `templates/push-runner-config.toml` も同時修正 (deploy:hooks で派生配布されるため)。 ## 実測で見つけた副次バグ 範囲 summary の取得を当初 shell 経由 (`[diff] command` と同じ経路) で実装したが、 実シェルで叩いたところ **cmd.exe はクォートを除去せず jj に渡す**ため `Revision '"<base>..@"' doesn't exist` で必ず失敗した。sh は除去するので Linux だけ 通る = Windows の全 push が fail-closed で止まる形。ユニットテストは summary 取得を 注入していたため検出できなかった。direct args 呼び出し (docs_only_routing の既存 `run_jj_diff_summary`) を共有する形に修正し、doc に理由を残した。両 stage は 「PR 範囲の変更ファイル一覧」という同一の問いを扱うため、別実装にすると本 PR が 排除した非対称を再導入することになる。 ## 検証 cargo test --workspace 全 pass / clippy clean。範囲検査は incident 形状 (PR 2 ファイル 変更に対し diff は tip の 1 ファイルのみ) の再現テストで固定し、fail-closed 経路 (summary 取得失敗 / ヘッダ不在) と過剰検知しない経路 (空 PR 範囲 / Windows パス 区切りの正規化) も併せて固定した。base branch 解決は 3 stage が同一範囲に解決される ことと override の優先順位を machine-enforce している。 * docs: ADR-027 に「diff 局所 = 観点の限定であって範囲の限定ではない」を明記 (順位 288/264) ## ADR-027 amendment ADR-027 が狭めたのは reviewer が使う criteria (cross-file 探索を要求しない) であり、レビュー対象に含めるコミットの範囲ではなかった。この 2 つが混同され、 `[diff] command` が tip コミット限定のまま運用されて 4 回の再発を招いたため、 射程を明文化した。 「レビュー対象は PR 範囲全体」と決定した根拠も併記: - 速度は理由にならない。同一 PR でレビュー対象を 37 行 → 1011 行 (27 倍) に 広げても 4m32s → 4m43s の +11 秒。ADR-027 の速度改善は arch-review facet の 除去 (219-270s/iter) によるもので、範囲縮小は寄与していなかった。 - 範囲が狭いことによる見落としはレビュアー側から検知できない (渡された diff が PR 全体かを検証する手段が無い)。 - CodeRabbit backstop はセルフレビューを省く理由にならない。独立した層として併用する。 ## todo 更新 - 順位 264 (`--git` 切替): 完了につきエントリと table 行を削除。 - 順位 288: `[diff]` 範囲修正の部分のみ完了として記録。**残タスク** (post-merge feedback の全 run 集約、bookmark_check.rs の祖先未レビュー穴の検証) は明示して 残す。エントリ全体を消すと未着手部分が失われるため削除しない。 * fix(push-runner): pre-push 範囲検査の欠陥修正 + CodeRabbit 指摘5件対応 (#313)
4 tasks
aloekun
added a commit
that referenced
this pull request
Jul 22, 2026
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
CodeRabbit がレート制限でレビューを開始できないまま、監視が「レビュー済み・指摘なし」と報告する silent success を修正します。
docs/harness-improvement-plan.mdの「WP-15 追補: 監視 fail-open 修正のゼロ再構築」の実装です。旧 PR #309 は、4 コミット目が「本番 config では一度も実行されない誤修正 + 実エントリポイントを迂回して pass するテスト」の合成物だったため、健全に見えるコミットも含めて全破棄しました（コード・テストは一切参照していません）。本 PR はゼロからの再構築です。
根本原因
CodeRabbit はレート制限中も commit check を pass にします（外部 SaaS 挙動、実観測）。checker はこれを
review_stateに採用する一方、parse_rate_limitの結果は出力 JSON に添付されるだけでdecide()には渡っていませんでした。結果、decide()は「review_state = success かつ指摘ゼロ」= 完了と読みstop_monitoring_successを返します。monitor 側の terminal 短絡は rate-limit branch より先に発火するため、park / 再 trigger 機構は一度も呼ばれません。症状は monitor 側に出ますが、原因は action の算出そのものにあります。旧 #309 は症状側（monitor 2 箇所）への多層パッチで High REJECT を受けたため、本実装では算出点である
decide()に一本化しました。変更内容
decide()に rate_limit を渡し silent success を排除R2: 書式追随を前提にしない fail-closed 化
第 3 世代書式
**Next review available in:** **57 minutes**の抽出を追加（区切りを[:*\s]*で吸収し強調記法の変化に耐える）。加えて、marker が一致したのに待機時間をどの既知書式でも読めない場合、従来はNone=「rate-limit ではない」に倒れていたものを、marker 一致を制限の根拠として採用し待機時間だけを既定 30 分で埋める方式に変更しました。既定値が実 reset より短ければ wakeup 後に再検出されて再 park されるだけで、retry はmax_retriesで有界です。既定値適用時は stderr に警告を出し、書式再変更の検知シグナルを兼ねます。R1/R4: 陽性証拠の要求
continue_monitoringを返し、判断を monitor 既存の rate-limit branch に委ねます。has_actionable分岐より前に置くのが要点で、これが無いと過去サイクル由来の未解決スレッドでaction_requiredに抜け、レビューが走っていないのに監視が終了します。stop_monitoring_successを出さないため、CR が marker 文言自体を変えても silent success には戻らず、最悪max_durationまでの監視継続（timed_out）に倒れます。陽性証拠の定義:
review_state（commit status）は制限中でも pass になるため証拠に使わず、push_timeで絞られた「今サイクルの CR 出力そのもの」=walkthrough_clean/actionable_commentsが読めた（Some(0)含む）/new_comments > 0のみを採用。unresolved_threadsは push_time で絞られず過去サイクルの残骸を含み得るため除外しました。R3: 判定文
判定順を「未確定 → 重大 → 未解決 → 軽微 → 問題なし」に整理し、未確定要素（park / rate-limit / review 未完了 / 未解決スレッド）を findings の有無より先に評価します。実観測では「未解決スレッド2件」を表示しながら同一レポートで「問題は見つかりませんでした」と断定する矛盾が出ていました。
検証
cargo test --workspace全 pass・clippy --workspace --all-targets --all-features -- -D warningscleanincident 実データでの実測（実エントリポイント経由）
close 後の PR #309 に残る実 rate-limit コメント（2026-07-20T12:10:47Z 投稿 / 12:38:33Z 編集、第 3 世代書式）に対し、実 exe を
--push-time 2026-07-20T12:37:00Zで実走させました（この push_time は rate-limit コメントのupdated_atを含みつつ、後から投稿された CR の「Review finished」コメントを除外するため incident 当時と同形の入力になります）。continue_monitoringstop_monitoring_success修正前バイナリは
.claude/にデプロイ済みだった旧 #309 branch 由来のもの（= R2 相当の検知修正は入っているがdecide()統合は無い状態）です。rate-limit を検知できていてもdecide()に渡っていなければ silent success になるという根本原因が実データで直接裏づけられ、同時に症状側パッチでは不十分だったことの実証にもなっています。E2E カバレッジの正直な申告
担保できたのは (a) 全 gate のユニット検証、(b) checker の実エントリポイントを実データで通した単体実測、(c) 修正前後の差分の実測 — の 3 点です。
未実測:
continue_monitoring受領 →handle_rate_limit_branchで park → PARK signal 出力）は、CR レート制限が本セッション中に自然発生しなかったため実走させていませんcontinue_monitoringを返せば branch に到達することは既存実装の性質に依存しています#[cfg(windows)]ガードのテストは Linux で skip されます（WP-16 の CI matrix で扱う既存ギャップ）pre-push review の適用範囲: push-runner の takt AI レビューは
jj diff -r @（単一コミット）が対象のため、本 PR では 5 コミット目（docs）のみが AI レビュー対象となり、R1〜R4 のコード変更は AI レビューを通っていません。決定論層（lint / test / build / clippy / cargo test --ignored）は PR 全体の統合状態に対して実行され全 PASS しています。補足
main.rs→markers.rs/rate_limit.rs）も修正しましたdocs/harness-improvement-plan.mdは master 時点で既に 59,798 bytes とfile_size_checkの 50KB 閾値を超過しており（non-blocking 警告）、本変更で 73,781 bytes になります。分割は § 9 の退役手順で本ファイルごと削除する前提のため見送っています🤖 Generated with Claude Code
Summary by CodeRabbit
バグ修正
ドキュメント