fix(monitor): 不完全な信号から success を導く fail-open 2 件を修正 (rate-limit 検知 / 判定文) - #309
Closed
aloekun wants to merge 4 commits into
Closed
fix(monitor): 不完全な信号から success を導く fail-open 2 件を修正 (rate-limit 検知 / 判定文)#309aloekun wants to merge 4 commits into
aloekun wants to merge 4 commits into
Conversation
PR #307 で実発火した誤報告の修正。CodeRabbit がレート制限でレビューを開始 できなかったにもかかわらず、監視が「CodeRabbit 指摘なし」= stop_monitoring_success と報告した (人間が PR コメントを直接読むまで誰も気付けない状態だった)。 原因は 2 段構えの検知のうち後段だけが失敗し、それが fail-open だったこと: - is_rate_limit_comment ("rate limited by coderabbit.ai") は一致していた - extract_wait_time が既知 2 書式のいずれにも一致せず None - parse_rate_limit は `extract_wait_time(body)?` で早期 return するため、 **レート制限と判っているのに「制限なし」を返していた** CR は書式を 3 度目の変更をしていた: 旧: Please wait **N minutes and M seconds** 新: More reviews will be available in N minutes and M seconds 現行: **Next review available in:** **15 minutes** ← 未対応だった 修正は 2 層: 1. **構造 (本質)**: marker が一致したら待ち時間が読めなくても必ず rate-limit として 報告する。既定値 15 分で park し、`wait_time_parsed: false` を下流へ伝えて summary / ログに「書式が未知」と明示する。これにより 4 回目の書式変更が起きても silent success ではなく「park + 書式追加が必要という可視の警告」に着地する。 短すぎる既定値は self-correcting (wakeup 後に再 poll し、まだ制限中なら再 park)。 2. **書式追加**: 現行書式の regex を追加。markdown 強調 (`**`) を `\**` と `\s*` で吸収。 既存 test `rate_limit_no_match_when_no_wait_time` は**旧来の誤挙動そのもの**を 固定していたため、新契約に合わせて書き換えた (改名 + 理由を doc に明記)。 ADR-034 が「CR は format を時間経過で変更するため multi-variant 配列で対応する (PR #182/#184 で silent regression を実体観測)」と警告していた事象の再発であり、 regex を足すだけでは同じことが繰り返される。構造側を直したのはそのため。 実測: Windows で cargo test --workspace 全 pass / clippy clean。Linux (WSL) でも check-ci-coderabbit rate_limit 28/28・cli-pr-monitor state 23/23 pass。 incident 由来 fixture は PR #307 に実投稿された comment 本文を使用 (ADR-049)。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PR #307 の recheck で実観測: 同じレポート内で「未解決スレッド2件」 「action: action_required」と表示した直後に「**判定**: 問題は見つかりませんでした」 と結論していた。人間はレポート末尾の判定行を読んで判断するため、未解決の CodeRabbit 指摘を見落とす経路になっていた。 原因は compute_verdict が result.findings の件数だけで分岐していたこと。 再チェックでは新規コメントが無いため findings は空になるが、未解決スレッドは 残り続ける。action は正しく action_required になっていたので、**判定文だけが 不完全な信号から安心させる結論を出していた**。 findings が空かつ未解決スレッドがある場合の分岐を追加した。findings がある場合は 従来どおり severity ベースの判定を優先し、未解決スレッドの文言で severity 情報を 潰さない。unresolved_threads が None (不明) の場合は 0 と同じ扱いで従来挙動を維持。 これは同日に修正した rate-limit 検知の silent regression と同じ family (不完全な信号から success を導出する fail-open) で、post-merge feedback の analyzer も独立に Tier 1 High として検出していた。 test は ADR-049 の流儀で incident 再現 (bad) + 退行防止 (good) を対で追加: - findings 空 + 未解決 2 件 → 「問題なし」と結論しないこと (incident 再現) - 未解決 0 件 → 従来どおり「問題なし」 - unresolved_threads = None → 0 と同じ扱い - findings あり + 未解決あり → severity ベース判定を優先 実測: Windows で cli-pr-monitor 262 pass (257→262、本 test 4 件 + helper 追加)。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PR #307 / #308 マージ後に release-binaries.yml が master で実走し `nightly` prerelease を生成した (tarball 9.72 MB + sha256、commit 541adde)。 公開された実物の artifact で end-to-end 検証した結果を記録する: - 認証なしの素の curl で取得 (= 「public release は gh 認証不要」の設計判断を実証) - checksum 一致 / 17 ファイル展開 (バイナリ 16 + BUILD_INFO) - file が ELF 64-bit LSB pie executable, x86-64 ... stripped を確認 - **release バイナリそのもので Linux 上の hooks が実発火** (PreToolUse が危険 コマンドを exit 2 でブロックし通常コマンドは exit 0、SessionStart が additionalContext JSON、tree-sitter の comment-lint が違反検出) あわせて初回 run が赤だった経緯 (PR #308 で修正) も記録した。WSL では修正前でも 60/60 pass して再現できず ubuntu-22.04 の CI が初めて捕捉したため、 **「WSL 検証はスケジューリング依存の race に対しては CI の代替にならない」**という 限定的な教訓として書いた。WP-16 (CI matrix) の必要性を裏づける実例になる。 過度な一般化 (「WSL は CI の代替にならない」) は避けた。post-merge feedback の analyzer が「既存メモと矛盾しうる」と指摘したとおり、WSL 検証自体は cargo test / clippy / hooks 発火の確認には有効であり、限界は race に限られる。 残る `完了` 条件 (2) は実クラウドセッションでの cloud-setup.sh 実走のみ。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 57 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✨ 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 バックストップ)
Applicable Findings (Critical / High / Major)該当なし (指摘 0 件) Applicable Findings (Medium 以下)該当なし Filtered (not applicable)該当なし Diff 概要 (軽量サマリー)
次のアクション
|
本 PR の先行 2 コミットだけでは**観測される症状が変わらなかった**ため追加する。 rate-limit の検出は直ったが、検出結果が action 判定で捨てられていた。 3 層構造だった: 層1: 待ち時間パース失敗 → 「制限なし」扱い (先行コミットで修正済み) 層2: cr_ok が「コメント0件」を「レビューclean」と解釈 ← 本コミット 層3: action 確定後に terminal 短絡し rate-limit 分岐に到達しない ← 本コミット 層2 が本質。CodeRabbit がレート制限でレビューを**開始できなかった**場合もコメントは 0 件になるため、`new_comments == 0 && unresolved_threads == 0` だけを見ると **レビュー未実施と clean レビューが区別できない**。そこに ci_ok が重なり stop_monitoring_success に倒れていた。 層3 は iteration.rs の `action != "continue_monitoring"` で即 terminal を返す短絡。 層2 で success が確定すると、せっかく検出した state.rate_limit を使う handle_rate_limit_branch に一度も到達せず、park も `@coderabbitai review` の 再トリガーも起きなかった。 pending 判定を `checks_still_outstanding` として切り出し、rate_limit を 「まだ結論を出せない」条件に加えた。これで continue_monitoring に倒れ、既存の park + 再トリガー経路へ流れる。skip_coderabbit 指定時は rate-limit も判断材料から 外す (skip の意味と整合)。 **実環境での検証**: 同じ PR #309・同じレート制限状態で修正前後を実測。 修正前: action=stop_monitoring_success / 「問題は見つかりませんでした」 修正後: action=parked_review_recheck / 「review 完了待ちのため wakeup を予約」 checker 単体では既に rate_limit を正しく検出できていた (wait_minutes=23, wait_time_parsed=true) ことも確認済みで、捨てていたのが監視側だと特定できている。 test は ADR-049 の流儀で incident 再現 (bad) + 退行防止 (good) を対で追加: - rate-limit 中は「コメント0件」でも continue_monitoring にすること (incident 再現) - rate-limit 無しなら従来どおり stop_monitoring_success に到達すること - skip_coderabbit = true なら rate-limit も判断材料から外すこと 実測: Windows で cli-pr-monitor 265 pass (262→265) / clippy clean。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Contributor
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)該当なし (指摘 0 件) Applicable Findings (Medium 以下)該当なし Filtered (not applicable)該当なし Diff 概要 (軽量サマリー)
次のアクション
|
Owner
Author
|
本 PR は 再利用せず全破棄 します（マージしません）。 理由4 コミット目の初版が「本番 config では一度も実行されない誤修正 + 実エントリポイントを迂回して pass するテスト」であり、pre-push review の High REJECT → fix step の自動書き直しを経た合成物になっていました。コミットメッセージには無効と判明した検証主張が残存し、実装も症状側への多層パッチ（monitor 側 2 箇所に分散）です。 中途半端な状態を引き継ぐと、それ自体がより良い実装を制約するため、健全に見えるコミットも含めて一切引き継がずにゼロから再構築します。 後継同じ要件（R1〜R5）を なお本 PR に残る CodeRabbit の rate-limit コメント（2026-07-20T12:10:47Z 投稿、第 3 世代書式 |
aloekun
added a commit
that referenced
this pull request
Jul 20, 2026
## 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 の退役手順で本ファイルごと削除する前提のため見送る。
aloekun
added a commit
that referenced
this pull request
Jul 21, 2026
…#311) * docs(plan): WP-15 追補 — 監視 fail-open 修正のゼロ再構築方針 (旧 PR #309 全破棄) 旧 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> * fix(check-ci): CR rate-limit 書式の第3世代対応と未知書式 fallback (WP-15 追補 R2) 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) を修正。 * fix(check-ci): decide() に rate_limit を渡し silent success を排除 (WP-15 追補 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 の実観測値から構成。 * fix(pr-monitor): 判定文が未確定要素を無視して断定しないよう修正 (WP-15 追補 R3) 監視レポートの人間向け判定文が、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。 * docs(plan): WP-15 完了条件 (1) 達成の記録と追補の実装状況を反映 (R5) ## 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 の退役手順で本ファイルごと削除する前提のため見送る。 * docs(check-ci): wait_time_parsed の doc が実装と食い違う記述を修正 セルフレビュー (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 で行う。 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
aloekun
added a commit
that referenced
this pull request
Aug 3, 2026
CodeRabbit 指摘「2c の workflow に一致する chain 宣言に更新してください」への対応。 ## 経緯 — takt fix が真の記述を偽に書き換えた 指摘を受けた post-pr-review の fix step は、宣言の「2c の実体で step 名・パス・引数を 照合済み」を「実装済みの実体と照合済みではない」へ書き換えて auto-push した。これは **事実に反する**。2c の実装はローカルの未 land コミットとして存在し、step 名 `Gate fix push (deterministic, 4-axis AND)`・exe パス・4 引数はいずれも実体と照合済み である (照合は 2b 着手時に実施)。 一方 CodeRabbit の懸念自体は正当だった: 本 PR の diff だけを見るレビュアーには、その 照合の主張を検証する手段がない。fix 後の記述は「偽だが検証可能」、修正前の記述は 「真だが検証不能」で、どちらも宣言として不十分だった。 ## 修正内容 主張を検証可能性で 2 つに分ける: - **diff 内で照合できるもの**: 引数 4 種と exe 名 (`main.rs` の `parse_args` / `USAGE`)。 ADR-069 § 決定 1 の名前一致要件は、この部分で本 diff 内に閉じて満たされる。 - **diff 外の主張**: step 名と exe パス。2c の未 land 実装と照合済みであることを述べつつ、 **本 PR の diff だけでは検証できない主張である**と明示し、名前一致の最終確認は 2c の diff レビューで行うと書く。 真である事実を落とさず、レビュアーが何を確認できて何を確認できないかを判別できる形に した。宣言の強度は落ちない — ADR-069 が要求する名前一致は diff 内で閉じている。 なお本件は「fix step の出力を実測検証する」(#309 / ADR-068) が docs 領域でも必要である ことの実例になった。WP-17 完了後の feedback 採否で扱う。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
aloekun
added a commit
that referenced
this pull request
Aug 3, 2026
* refactor(lib-scope-guard): ADR-054 scope 判定コアを lib へ抽出 (WP-17 PR 2) Phase B (CI 側の無人 fix push) が同じ scope 検証を必要とするため、cli-pr-monitor に 閉じていた判定コアを lib 化する。挙動は不変。 抽出根拠は ADR-044 層 1 の「3+ crate 重複」ではなく **判定の同一性**。同一 ADR (ADR-054) の防御がローカル経路と CI 経路に分岐すると、片方だけ緩んだ時点で injection 防御が無効化される。重複数ではなくこの drift リスクを根拠に、2 呼び手の時点で抽出する (lib-docs-policy が ADR-035 の path 基準を単一実装へ集約したのと同じ理由)。 lib に置いたもの (純粋な文字列処理のみ): - normalize_path / allowlist_from_paths / parse_changed_files / find_out_of_scope - ALWAYS_ALLOWED (.takt/review-diff.txt) 呼び出し側に残したもの: diff の取得 (jj / git)、mode 判定 (enforce/observe)、 kill-switch、ログ出力。lib は「変更ファイル集合が許可集合に収まるか」だけを答える。 allowlist の入力を Finding 型ではなくパス文字列の iterator にしたのは、findings の 表現が経路ごとに異なるため (ローカル = lib_report_formatter::Finding、CI = JSON)。 これにより lib は依存ゼロを保てる。 テストも移設し、lib 側に 11 件 (境界値の網羅: 空 summary / 空白行 / tab 区切り / 空白を含むパス / 空 allowlist)。cli-pr-monitor 側には Finding→パスの glue と mode/統合テストのみ残す。tab 区切り拒否のテストを新規追加した — git diff --name-status は tab 区切りで、CI 経路が正規化を怠ると fail-closed に倒れることを固定するため。 検証: cargo clippy -D warnings 緑、cli-pr-monitor 260 件 + lib-scope-guard 11 件 pass。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(lib-autonomy-policy): kill-switch 判定コアを lib へ抽出 (WP-17 PR 2a) 後続 PR (WP-17 PR 2b) の fix push 直前ゲート `cli-fix-push-gate` が kill-switch を 含む全軸を 1 回で評価できるようにするため、cli-autonomy-gate 内の decision.rs / sources.rs を lib へ移す。挙動は不変 (テスト 21 件は PR 1 と同数を維持)。 **本 PR の時点で 2 つ目の呼び手はまだ存在しない。** ADR-044 層 1 は「2 つ目の使用例が 出た時点で extract」と定めるが、後から昇格すると kill-switch 判定が一時的に 2 箇所へ 分岐する期間ができる。その期間を作らないための**明示的な前倒し判断**であり、 層 1 の厳密な充足ではない。この位置づけは各 module doc にも記載した (pre-push review の simplicity facet が「存在しない 2nd caller を根拠にしている」と 指摘し、未来形へ言い換える fix が入った。指摘は妥当で、4 箇所すべてが修正済み)。 exe 連鎖 (`cli-autonomy-gate && cli-fix-push-gate`) を採らなかった理由: workflow 側で `&&` を書き忘れると kill-switch を通り越して fix push が走る。 「1 つでも欠けたら停止」の fail-closed 合成が呼び手のミスで壊れる経路を残さないため、 ライブラリ共有にして fix push ゲートが構造的に kill-switch を含む形にする。 cli-autonomy-gate は CLI 面 (引数解析・loud 出力・exit コード・telemetry) だけを持つ 薄い exe になり、汎用の kill-switch 判定 (pnpm autonomy-status / 単体 drill) を担う。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(cli-fix-push-gate): Phase B の fix push 直前ゲートを追加 (WP-17 PR 2) ADR-052 自動実行可クラスの 4 軸を 1 回の呼び出しで AND 評価する決定論ゲート。 GitHub Actions の Phase B job が push 直前に呼び、非ゼロなら push を止めて Phase A 相当 (分析コメントのみ) へ degrade する。 4 軸と根拠: - kill-switch (ADR-066 / ADR-052 原則 5): --config + env AUTONOMY_ENABLED - target 軸 (ADR-052 原則 2): --branch が claude/ prefix であること - 内容軸 (ADR-052 原則 2 / ADR-035): fix diff が docs-only であること - scope (ADR-054): 変更ファイルが findings 由来 allowlist に収まること 各軸の基準は既存の単一実装 (lib-autonomy-policy / lib-docs-policy / lib-scope-guard) を借り、本 crate 固有のロジックは AND 合成と判定順序だけに留めた。基準の再実装は ADR-035 / ADR-054 が防ごうとした drift の再生産にあたるため。 判定順は kill-switch → ブランチ → 空 diff → 内容軸 → scope。空 diff を内容軸より先に 見るのは、is_docs_only_summary が空入力へ false を返す仕様で、そのままだと「変更なし」が 「docs-only ではない」と誤報告されるため (drill 6 で実確認)。 findings の出所に関する契約を inputs.rs の doc に明記した: findings は fix を書いた エージェント自身ではなく、先行する読み取り専用の分析 step の出力でなければならない。 同一エージェントが両方を出すと scope guard が自己申告の追認になり ADR-054 の防御が 成立しない (ローカル経路で review facet と fix step が別エージェントなのと同じ分離)。 loud 出力は deny 理由 1 つに絞らず 4 軸すべての状態を出す。drill 3 (非 claude ブランチ) では autonomy=allowed branch=not-isolated content=docs-only scope=in-scope と出て、 ブランチだけが原因と 1 行で読める。 引数は 4 つとも必須。省略で軸が無検査になる fail-open を作らないため、どれか 1 つでも 落とすと exit 2 になることをテストで固定した。 テスト 22 件 + 実 exe drill 7 シナリオ (全軸 OK → exit 0、kill-switch / 非 claude ブランチ / scope violation / code 変更 / 空 diff / rename fail-closed → 全て exit 1)。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(wp17): lib module doc を呼び手実在の現実に合わせる + 2b の chain 宣言 (WP-17 2b) 先行 3 コミット (lib-scope-guard / lib-autonomy-policy 抽出、cli-fix-push-gate 追加) は incident 前に「抽出だけの PR」として書かれており、module doc が呼び手を「計画中・本 diff の時点では未実装」と説明していた。本 PR は抽出と最初の呼び手を同一 PR に入れるため (ADR-069 § 決定 3-1)、その前提が成立しない。文言を現実に合わせる。 ## 時制修正 (4 ファイル) - `lib-autonomy-policy/src/lib.rs`: 呼び手 2 件 (`cli-autonomy-gate` / `cli-fix-push-gate`) を実在として記載。「前倒しで lib 化した (ADR-044 層 1 の厳密な充足ではない)」という 但し書きは、2 呼び手が同一 PR に揃った今は不要なので落とし、層 1 充足と書く。 - `lib-scope-guard/src/lib.rs`: 同上。「CI 側の呼び手はまだ実装されていない」を削除。 lib 化の根拠 (重複数ではなく判定の同一性) は ADR-054 の防御に関わるので残す。 - `cli-autonomy-gate/src/main.rs`: `cli-fix-push-gate` を実在として参照。 - `cli-pr-monitor/src/stages/scope_guard.rs`: 「将来 CI 経路が追加された際に」を現在形へ。 ## chain 宣言の精緻化 (ADR-069 初回 dogfood) 計画書の「2b の chain 宣言」を、ADR-069 § 決定 1 の 3 要件に照らして具体化した: - **未消費は 1 つだけ**であることを明示。`lib-scope-guard` / `lib-autonomy-policy` は どちらも呼び手 2 件が本 PR の diff 内に揃っており missing-consumer ではない。 宣言が要るのは `cli-fix-push-gate` の workflow 呼び手のみ。 - **名前一致要件**を満たすため、2c の実体 (未 land コミット lqxzpvuw) と照合して step 名を 実名 `Gate fix push (deterministic, 4-axis AND)` に修正した (旧記載 `Gate fix push` は 前方一致にすぎず、ADR-069 の「矛盾する宣言は降格根拠にならない」に触れうる)。 exe パスと 4 引数も実体と突き合わせ済み。 宣言付き先頭 PR が missing-consumer で REJECT されないことが ADR-069 試験運用の decision trigger (a) の初回実測になる。結果は ADR-069 の判断基準へ記帳する。 ## 計画書の状態行 2a を完了 (PR #350) に、2b を実施中に更新。この更新自体が計画書を本 PR の diff に載せ、 ADR-069 § 決定 1「置き場所 = diff 内の計画文書のみ」を満たす手段でもある。 検証: cargo test --workspace 1945 件 pass、clippy --workspace --all-targets -D warnings 緑、 pnpm lint:docs / lint:md 0 error。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(review): apply CodeRabbit fixes for #351 Resolved findings: - [Minor] docs/harness-improvement-plan.md:185 2c の workflow に一致する chain 宣言に更新してください。 * docs(wp17): chain 宣言の検証状態を diff 内/外で分けて明記 (CodeRabbit #351) CodeRabbit 指摘「2c の workflow に一致する chain 宣言に更新してください」への対応。 ## 経緯 — takt fix が真の記述を偽に書き換えた 指摘を受けた post-pr-review の fix step は、宣言の「2c の実体で step 名・パス・引数を 照合済み」を「実装済みの実体と照合済みではない」へ書き換えて auto-push した。これは **事実に反する**。2c の実装はローカルの未 land コミットとして存在し、step 名 `Gate fix push (deterministic, 4-axis AND)`・exe パス・4 引数はいずれも実体と照合済み である (照合は 2b 着手時に実施)。 一方 CodeRabbit の懸念自体は正当だった: 本 PR の diff だけを見るレビュアーには、その 照合の主張を検証する手段がない。fix 後の記述は「偽だが検証可能」、修正前の記述は 「真だが検証不能」で、どちらも宣言として不十分だった。 ## 修正内容 主張を検証可能性で 2 つに分ける: - **diff 内で照合できるもの**: 引数 4 種と exe 名 (`main.rs` の `parse_args` / `USAGE`)。 ADR-069 § 決定 1 の名前一致要件は、この部分で本 diff 内に閉じて満たされる。 - **diff 外の主張**: step 名と exe パス。2c の未 land 実装と照合済みであることを述べつつ、 **本 PR の diff だけでは検証できない主張である**と明示し、名前一致の最終確認は 2c の diff レビューで行うと書く。 真である事実を落とさず、レビュアーが何を確認できて何を確認できないかを判別できる形に した。宣言の強度は落ちない — ADR-069 が要求する名前一致は diff 内で閉じている。 なお本件は「fix step の出力を実測検証する」(#309 / ADR-068) が docs 領域でも必要である ことの実例になった。WP-17 完了後の feedback 採否で扱う。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
aloekun
added a commit
that referenced
this pull request
Aug 3, 2026
…bit #353) PR #353 の CodeRabbit 指摘 3 件への対応。うち 2 件は本 PR が持ち込んだ実バグだった。 ## 指摘 2 (Minor): terminal 化 3 経路が head_commit を None で上書きする `finalize_pending_review` / `finalize_waiting_reset` / `finalize_posted_retrigger` が `state.head_commit = pr_info.head_commit.clone()` と**無条件代入**していた。 `util::get_pr_head_commit` は gh 失敗を None に潰すため、API 障害のたびに保存済み OID が 消える。その状態で次回 `--monitor-only` を実行すると `should_continue_state` が None を 見て継続を拒否し、時刻窓アンカーが「今」へリセットされて間に届いた CR コメントを 取りこぼす — 本 PR が動機に挙げた incident class (#237/#307/#309) と同型。 CodeRabbit が「共通の根本原因」と指摘したとおり 3 経路に同じ欠陥があったため、 `PrMonitorState::record_head_commit(Option<&str>)` を state.rs に置き、Some のときだけ 上書きする単一実装へ集約した。取得失敗は「head が変わった」の証拠ではないので既存値の 保持が正しい (fail-safe)。回帰テスト 2 件 (None で消えない / Some で上書きする) を追加。 ## 指摘 3 (Minor): テストが実 gh CLI を呼ぶ `finalize_waiting_reset` は内部で `fetch_mergeable_status` → `run_gh_quiet` を呼ぶため、 前コミットで追加した回帰テストが実 gh プロセスを起動していた (ネットワーク依存・低速)。 `finalize_waiting_reset_with(..., fetch_mergeable: impl FnOnce(&PrInfo) -> Option<..>)` へ本体を切り出し、production は `fetch_mergeable_status` を、テストは closure を渡す形に した (cli-push-runner の `verify_diff_covers_pr_range` と同じ注入の流儀)。 あわせて「注入したものが実際に使われている」ことを確認するテストを追加した — `|_| None` を渡すだけのテストでは、fetcher を無視する実装でも通ってしまい注入の正しさを判別できない。 Cell フラグで呼び出し自体を観測する。 ## 指摘 1 (Minor): 計画書の stale な park 検証残 「WP-15 追補残: レート制限 park の実観測」は park 廃止で (a) が moot になっていた。 見出しを「レート制限時の保留保証 (GitHub Actions 経路)」へ改め、(a) の終了と (b) の 引き継ぎ先を明記した (ADR-018 amendment / ADR-064 ステータス欄と同じ内容を計画書側にも 反映し、3 箇所の記述を揃える)。 検証: cargo test --workspace 1905 件 pass (新規 4 件)、clippy --workspace -D warnings 緑、 pnpm lint:docs / lint:md 0 error。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
aloekun
added a commit
that referenced
this pull request
Aug 3, 2026
…d 回帰復元 (CodeRabbit #353) PR #353 のレビュー指摘への対応。CodeRabbit 3 件 + pre-push simplicity 1 件で、 **4 件中 3 件は本 PR が持ち込んだ実バグ / カバレッジ欠落**だった。 ## CodeRabbit 指摘 2 (Minor): terminal 化 3 経路が head_commit を None で上書きする `finalize_pending_review` / `finalize_waiting_reset` / `finalize_posted_retrigger` が `state.head_commit = pr_info.head_commit.clone()` と**無条件代入**していた。 `util::get_pr_head_commit` は gh 失敗を None に潰すため、API 障害のたびに保存済み OID が 消える。その状態で次回 `--monitor-only` を実行すると `should_continue_state` が None を 見て継続を拒否し、時刻窓アンカーが「今」へリセットされて間に届いた CR コメントを 取りこぼす — 本 PR が動機に挙げた incident class (#237/#307/#309) と同型。 CodeRabbit が「共通の根本原因」と指摘したとおり 3 経路に同じ欠陥があったため、 `PrMonitorState::record_head_commit(Option<&str>)` を state.rs に置き、Some のときだけ 上書きする単一実装へ集約した。取得失敗は「head が変わった」の証拠ではないので既存値の 保持が正しい (fail-safe)。回帰テスト 2 件 (None で消えない / Some で上書きする) を追加。 ## CodeRabbit 指摘 3 (Minor): テストが実 gh CLI を呼ぶ `finalize_waiting_reset` は内部で `fetch_mergeable_status` → `run_gh_quiet` を呼ぶため、 前コミットで追加した回帰テストが実 gh プロセスを起動していた (ネットワーク依存・低速)。 `finalize_waiting_reset_with(..., fetch_mergeable: impl FnOnce(&PrInfo) -> Option<..>)` へ本体を切り出し、production は `fetch_mergeable_status` を、テストは closure を渡す形に した (cli-push-runner の `verify_diff_covers_pr_range` と同じ注入の流儀)。 あわせて「注入したものが実際に使われている」ことを確認するテストを追加した — `|_| None` を渡すだけのテストでは、fetcher を無視する実装でも通ってしまい注入の正しさを判別できない。 Cell フラグで呼び出し自体を観測する。 ## pre-push simplicity 指摘 (SIM-NEW-rate_limit-L122, Medium): fail-closed 回帰の消失 tests.rs のファイル分割時に `finalize_posted_retrigger_action_required_when_write_state_fails` だけが欠落していた。この関数は 2 つの sibling (`finalize_waiting_reset` / `finalize_pending_review`) と**逆に fail-closed** で、理由は `@coderabbitai review` 投稿と いう副作用を伴うため state 永続化の成否で重複投稿を防ぐ必要があるから。fail-open 側 2 件の 回帰テストは移行されていたのに、意図的に非対称な唯一の経路のテストが落ちていた。復元し、 非対称の理由を doc comment に明記した。 ## CodeRabbit 指摘 1 (Minor): 計画書の stale な park 検証残 「WP-15 追補残: レート制限 park の実観測」は park 廃止で (a) が moot になっていた。 見出しを「レート制限時の保留保証 (GitHub Actions 経路)」へ改め、(a) の終了と (b) の 引き継ぎ先を明記した (ADR-018 amendment / ADR-064 ステータス欄と同じ内容を計画書側にも 反映し、3 箇所の記述を揃える)。 検証: cargo test --workspace 1906 件 pass (新規 5 件)、clippy --workspace -D warnings 緑、 pnpm lint:docs / lint:md 0 error。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
aloekun
added a commit
that referenced
this pull request
Aug 3, 2026
… 3) (#353) * refactor(pr-monitor): CronCreate park モデルを撤去し single-shot 化 (WP-17 PR 3) cli-pr-monitor を「1 回 check して必ず terminal 報告で終了する」モデルへ移行する。 旧 Bb-1/Bb-2 の park モデル (未確定なら state に wakeup 時刻を書き、[PR_MONITOR_PARK] envelope で Claude に CronCreate 予約を依頼し、発火時の再 invoke で継続する) を撤去した。 ## なぜ廃止するか (ADR-018 amendment) - wakeup はローカルセッションの寿命に依存する。PR #237 でセッション終了による CronCreate 失効 = 監視の取りこぼしを実観測した。 - WP-17 で GitHub Actions 経路 (pr-monitor workflow の Phase A/B) が常設になった。 CodeRabbit のレビュー到着・後続コメントがそのままトリガーになるため、ローカルの 時限 wakeup は冗長になった。 ## 挙動の変化 - 未確定 (review 未完) → 旧: park + wakeup 予約 / 新: terminal `pending_review` 報告 (「後続は GitHub Actions 経路が処理」を明示。ADR-064 の陽性証拠原則どおり pending を silent success に見せない) - rate-limit reset 待ち → 旧: park / 新: terminal `rate_limited` 報告 (ADR-064 (b) の 保留判定文を維持)。reset 経過後の即時 retrigger 投稿と comment dedup は残す - fresh push → 旧: checker を呼ばず initial park / 新: 即 1 回 check して報告 ## 残したもの (park の付随物ではないため) - **時刻窓アンカーの継続** (`should_continue_state`): 同一 PR + 同一 head なら state.started_at / fix_push_time を維持する。これを落とすと再実行のたびに `--push-time` が「今」になり、push 後に届いた CR コメントが新着判定から漏れる。 旧 `should_resume_wakeup` から wakeup 時刻経過の条件だけを外した形 - rate_limit_retries / rate_limit_last_retriggered_at (dedup と retry 上限は invocation 跨ぎで引き続き意味を持つ) - **順位 141 の mergeable shortcut** (`[RATE_LIMIT_BUT_MERGEABLE]` signal): 「rate-limit 中でも既に mergeable なら即 merge を選べる」独立機能で、park の 付随物ではない。初版 diff は rate_limit_signal.rs の削除に巻き込んで silent に 消しており、pre-push simplicity review が REJECT (SIM-NEW-src-cli-pr-monitor-rate_limit-L138: 他の撤去物は全て ADR/計画書に 列挙されているのに shortcut だけ無言で消えている = 意図の疑い) → fix step が rate_limit.rs へテストごと復元した (実測検証済み)。terminal 化した `finalize_waiting_reset` から引き続き発火する。signal 文面の選択肢 B は single-shot 後の実態 (Actions 経路 / --monitor-only 再実行) に合わせて更新 - **rate-limit 系 terminal での head_commit 保存**: 2 回目の pre-push review が、 `finalize_waiting_reset` / `finalize_posted_retrigger` で head_commit 保存が 落ちている回帰を REJECT で検出 (SIM-NEW-rate_limit-L154: 保存が無いと次回 `--monitor-only` の継続判定が fresh 初期化に倒れ、時刻窓リセットで CR コメントを 取りこぼす — 本 PR 自身が動機に挙げた incident class の再導入)。fix step が 両関数への保存 + 回帰テスト 2 件を追加した (実測検証済み) ## 撤去したもの - state: next_wakeup_at_unix / wakeup_reason / review_recheck_count (旧 state file に 残っていても unknown field として無視される — 前方互換テストを追加) - poll: review_recheck.rs / review_recheck_signal.rs (park scheduling と PARK envelope 整形の全量)。rate_limit_signal.rs はファイルとしては削除したが、上記のとおり shortcut 部分は rate_limit.rs へ移設して維持 - config: [review_recheck] セクションと sanitize / overflow 検査 (wakeup 時刻の加算 演算ごと消滅)、monitor.max_duration_secs (ループ上限)。旧 config は unknown field/section として無視される — 前方互換テストを追加 検証: cargo test -p cli-pr-monitor 226 件 pass、workspace 全緑、 clippy --workspace -D warnings 緑。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(hooks-session-start): pr_monitor catch-up nudge を撤去 (WP-17 PR 3) pr_monitor catch-up nudge (Bb-3) は「park 中にセッションが終了して CronCreate が失効 した場合の救済」だった。park/wakeup モデルの廃止 (同 PR の cli-pr-monitor 変更) に伴い 救済対象の状態が存在しなくなるため、機構ごと撤去する。 - hooks-session-start: pr_monitor.rs module 削除 + main.rs の配線解除 - .claude/hooks-config.toml: telemetry registry から `pr_monitor_catchup` id を除去 (過去の firings jsonl には残るが現役 id ではない)。あわせて、どのコードも読んで いなかった daemon 時代 (ADR-009) の死に設定 [post_pr_monitor] セクションを撤去 (現行の監視設定はリポジトリルートの pr-monitor-config.toml が正) - cli-telemetry-report: doc comment の例示 id を現役のものへ差し替え 検証: cargo test --workspace 1893 件 pass、clippy --workspace -D warnings 緑。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(adr): ADR-018 amendment (park モデル廃止) + ADR-064/034 整合 + 計画書 (WP-17 PR 3) - ADR-018 追記 (2026-08-03): park モデル廃止の決定・根拠・維持したもの (時刻窓アンカー の state 継続 / rate-limit dedup) を記録。ADR-064 検証残の移し替え ((a) park 実観測は moot / (b) 判定文の保留保証は Actions 経路の検証残へ) を明記 - ADR-064 ステータス欄: 同じ移し替えを検証残の側からも記載 (両方に記録して検証の穴を 残さない — 計画書の着手前決定 2) - ADR-034: Bundle b の CronCreate park モデル記述に「廃止済み、歴史記録として読む」 注記を追加 - 計画書: PR 3 を実施中に更新し、実装で確定した設計判断 (should_continue_state を残す 理由) と「本 PR の PR がスモーク段 1 の観測対象を兼ねる」ことを記載 検証: pnpm lint:docs / lint:md 0 error。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(pr-monitor): head_commit の None 上書き防止 + テストの gh 依存除去 + fail-closed 回帰復元 (CodeRabbit #353) PR #353 のレビュー指摘への対応。CodeRabbit 3 件 + pre-push simplicity 1 件で、 **4 件中 3 件は本 PR が持ち込んだ実バグ / カバレッジ欠落**だった。 ## CodeRabbit 指摘 2 (Minor): terminal 化 3 経路が head_commit を None で上書きする `finalize_pending_review` / `finalize_waiting_reset` / `finalize_posted_retrigger` が `state.head_commit = pr_info.head_commit.clone()` と**無条件代入**していた。 `util::get_pr_head_commit` は gh 失敗を None に潰すため、API 障害のたびに保存済み OID が 消える。その状態で次回 `--monitor-only` を実行すると `should_continue_state` が None を 見て継続を拒否し、時刻窓アンカーが「今」へリセットされて間に届いた CR コメントを 取りこぼす — 本 PR が動機に挙げた incident class (#237/#307/#309) と同型。 CodeRabbit が「共通の根本原因」と指摘したとおり 3 経路に同じ欠陥があったため、 `PrMonitorState::record_head_commit(Option<&str>)` を state.rs に置き、Some のときだけ 上書きする単一実装へ集約した。取得失敗は「head が変わった」の証拠ではないので既存値の 保持が正しい (fail-safe)。回帰テスト 2 件 (None で消えない / Some で上書きする) を追加。 ## CodeRabbit 指摘 3 (Minor): テストが実 gh CLI を呼ぶ `finalize_waiting_reset` は内部で `fetch_mergeable_status` → `run_gh_quiet` を呼ぶため、 前コミットで追加した回帰テストが実 gh プロセスを起動していた (ネットワーク依存・低速)。 `finalize_waiting_reset_with(..., fetch_mergeable: impl FnOnce(&PrInfo) -> Option<..>)` へ本体を切り出し、production は `fetch_mergeable_status` を、テストは closure を渡す形に した (cli-push-runner の `verify_diff_covers_pr_range` と同じ注入の流儀)。 あわせて「注入したものが実際に使われている」ことを確認するテストを追加した — `|_| None` を渡すだけのテストでは、fetcher を無視する実装でも通ってしまい注入の正しさを判別できない。 Cell フラグで呼び出し自体を観測する。 ## pre-push simplicity 指摘 (SIM-NEW-rate_limit-L122, Medium): fail-closed 回帰の消失 tests.rs のファイル分割時に `finalize_posted_retrigger_action_required_when_write_state_fails` だけが欠落していた。この関数は 2 つの sibling (`finalize_waiting_reset` / `finalize_pending_review`) と**逆に fail-closed** で、理由は `@coderabbitai review` 投稿と いう副作用を伴うため state 永続化の成否で重複投稿を防ぐ必要があるから。fail-open 側 2 件の 回帰テストは移行されていたのに、意図的に非対称な唯一の経路のテストが落ちていた。復元し、 非対称の理由を doc comment に明記した。 ## CodeRabbit 指摘 1 (Minor): 計画書の stale な park 検証残 「WP-15 追補残: レート制限 park の実観測」は park 廃止で (a) が moot になっていた。 見出しを「レート制限時の保留保証 (GitHub Actions 経路)」へ改め、(a) の終了と (b) の 引き継ぎ先を明記した (ADR-018 amendment / ADR-064 ステータス欄と同じ内容を計画書側にも 反映し、3 箇所の記述を揃える)。 検証: cargo test --workspace 1906 件 pass (新規 5 件)、clippy --workspace -D warnings 緑、 pnpm lint:docs / lint:md 0 error。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.
Summary
**Next review available in:** **15 minutes**の regex を追加unresolved_threadsを無視し、未解決スレッドが残っていても「問題は見つかりませんでした」と結論していた fail-open を修正完了条件 (1) 達成 (release 実生成 + 実 artifact での検証) を plan に記録Context
いずれも本日の運用中に実発火した不具合。 2 件のコード修正は「不完全な信号から安心させる結論を出す」という同じ family で、人間がレポート末尾だけ読むと見落とす経路になっていた。
① rate-limit 検知 (PR #307 で実発火): CodeRabbit がレート制限でレビューを開始できなかったのに、監視は
stop_monitoring_success/ 「CodeRabbit 指摘なし」と報告した。検知は 2 段構えで、marker 判定 (rate limited by coderabbit.ai) は一致していたがextract_wait_timeが既知 2 書式のいずれにも一致せずNoneを返し、parse_rate_limitがextract_wait_time(body)?で早期 return していた。CR は書式を 3 度変えている:Please wait **N minutes and M seconds**More reviews will be available in N minutes and M seconds**Next review available in:** **15 minutes**ADR-034 が「CR は format を時間経過で変更するため multi-variant 配列で対応する (PR #182/#184 で silent regression を実体観測)」と警告していた事象の再発であり、regex を足すだけでは 4 回目で同じことが起きる。そこで marker 一致時は既定値 15 分で必ず park し、
wait_time_parsed: falseを下流へ伝えて summary / ログに「書式が未知」と明示する構造に変えた。既定値が短すぎても self-correcting (wakeup 後に再 poll し、まだ制限中なら再 park)。② 判定文 (PR #307 の recheck で実発火): 同じレポート内で「未解決スレッド2件」「action: action_required」と表示した直後に「判定: 問題は見つかりませんでした」と結論していた。
compute_verdictがfindingsの件数だけで分岐しており、再チェックでは新規コメントが無くfindingsが空になる一方で未解決スレッドは残り続けるため。actionは正しくaction_requiredになっていたので、判定文だけが不完全だった。post-merge feedback の analyzer も独立に Tier 1 High として検出している。スコープ: master を最短で緑に戻す必要があった E2E の競合修正 (PR #308) は先行させ、本 PR は監視系 fail-open 2 件 + 記録に限定した。
Validation
cargo test --workspace全 pass /clippy --workspace --all-targets --all-features -- -D warningscleancli-pr-monitor257 → 262 (判定文の test 4 件 + helper 追加)、check-ci-coderabbit97 passrate_limit_no_match_when_no_wait_timeは旧来の誤挙動そのものを固定していたため、新契約に合わせて改名・書き換え (理由を doc に明記)pnpm lint:docs/lint:mdpass、pre-push reviewverdict=APPROVEReferences
完了条件 (1) 達成もこの PR 由来