diff --git a/docs/adr/adr-018-pr-monitor-takt-migration.md b/docs/adr/adr-018-pr-monitor-takt-migration.md index 76e420c..c58cdaf 100644 --- a/docs/adr/adr-018-pr-monitor-takt-migration.md +++ b/docs/adr/adr-018-pr-monitor-takt-migration.md @@ -173,3 +173,37 @@ Bundle b 設計時に再分析して判明した CronCreate の特性: - PR #114 (Bb-2): review_recheck CronCreate park + observer 撤廃 - PR #115 (Bb-3): config 拡張 + SessionStart catch-up nudge + sanitize() (CR Major #1+#2 fold-in) - PR #116: Bb-3 post-merge-feedback 採用 3 件 (順位 76/77/78) を todo に登録 + +## 追記 (2026-05-08): 順位 80 — auto-retry の transient failure scope 明文化 + +PR #120 (cli-finding-classifier 統合) の dogfood で観測された **「rate-limit auto-retry が `RateLimitOutcome::Posted` 経路で park 予約せず silent exit する」** 事象を契機に、auto-retry の対象 transient failure pattern を本 ADR で明文化。 + +### 対象 transient failure の分類 + +| Pattern | Detection 基準 | Auto-retry 状態 | Notes | +|---|---|---|---| +| **Rate limit (待機型)** | `Rate limit exceeded` + 未来 reset_time | ✅ 実装済 (`RateLimitOutcome::Parked`) | reset まで `until_unix_secs` で park、再投稿不要 | +| **Rate limit (即時型)** | `Rate limit exceeded` + 過去 reset_time | ✅ 実装済 (`RateLimitOutcome::Posted` + 順位 80 fix) | `@coderabbitai review` 投稿後、`review_recheck_wait_secs` で park (順位 80 fix で導入、silent exit 防止) | +| **CR 投稿エラー** (`Failed to post review comments`) | walkthrough overlay の error message | ⏳ 未実装 (順位 81、1 観測のみ低頻度) | 頻度が確認できるまで実装を defer (memory: `feedback_no_unenforced_rules` 系の判断) | +| **Wakeup 未予約 fallback** | rate-limit 検出ありで polling 終端時に next_wakeup_at_unix 未設定 | ⏳ 将来候補 | 順位 80 fix で `Posted` 経路は塞がれた、他経路の同型 silent exit が再観測されたら追加 | + +### 順位 80 fix の実装ポイント (PR #129 / 2026-05-08) + +`finalize_posted_retrigger` (poll.rs) を以下のように変更: + +- **Before**: 投稿後 `write_state` のみして `None` を返す → polling 継続 → max_duration timeout で `make_timeout_result` (silent exit) +- **After**: 投稿後 `state.action = "parked_review_recheck"` + `next_wakeup_at_unix = now + review_recheck_wait_secs` を設定し、`format_park_signal` で PARK signal を stdout 出力、`Some(park_poll_result)` を返す → CronCreate wakeup で再開 + +これにより、rate-limit detection 後の **全経路で必ず PARK signal が emit される** ことが構造的に保証される。 + +### 順位 81 (CR 投稿エラー) を defer した理由 + +- **Frequency**: 1 観測 (PR #120 のみ) で systemic 性が未確認 +- **Risk**: detection 拡張は false positive リスク (`Failed to post review comments` が本当に transient か、permanent failure かの判別が難しい) +- **Decision**: ユーザー方針 `feedback_no_unenforced_rules` (機械検知不可なら何もしない方がマシ) と整合、**3 PR 観測** の閾値到達まで実装を defer +- **Re-trigger 条件**: 同型の error message が別 PR で 2 件以上観測されたら順位 81 を再活性化 + +### 関連 PR / commit (本追記) + +- 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 diff --git a/docs/local-llm-offload-analysis.md b/docs/local-llm-offload-analysis.md index e526583..d89257c 100644 --- a/docs/local-llm-offload-analysis.md +++ b/docs/local-llm-offload-analysis.md @@ -418,7 +418,7 @@ P-1 (Bundle g-1): PR #125, merged 2026-05-07, findings: 0 (CR APPROVE no comm P-2 (順位 47): PR #126, merged 2026-05-07, findings: 1 (Nitpick, CR review body 内 `
` block), agreement: 1/1 (100%, 私評価=human_review と一致), latency: 6.4s/件 (>5s 目標), fallback: 1/1 (normalized_issue length 100>80) - 既知 gap: check-ci-coderabbit が review body の `
` block 内 Nitpick を抽出しない (post-pr-monitor が classifier に渡せず、手動で synthetic finding 構築して classifier 実行) P-3 (順位 7): PR #127, merged 2026-05-08, findings: rate-limit blocked (CR が 27 min wait の rate-limit overlay で formal review 投稿不可)、classifier 未起動、計測 N/A — dogfood 不発 (但し CR rate-limit 経路が dogfood の阻害要因として観測された、§A-2 §6 注意事項 #1 を実証) -P-4 (順位 76+77): PR #___, merged ___, findings: __, agreement: __/__, token Δ: __, latency: __s/件, fallback: __/__ +P-4 (順位 76+77): PR #128, merged 2026-05-08, findings: 1 (CR Nitpick: cross_module_* 命名 misleading)、手動 synthetic finding で classifier 実行 → action=human_review / action_confidence=**0.9** (P-2 の 0.0 から大幅改善、length contract pass)、latency: 6.6s/件、fallback: **0/1** (P-2 の 1/1 から改善、normalized_issue 50 chars Japanese で contract pass)、agreement: 1/1 (100%、私評価=human_review と一致) P-5 (Bundle f-1): PR #___, merged ___, findings: __, agreement: __/__, token Δ: __, latency: __s/件, fallback: __/__ 集計: diff --git a/docs/todo.md b/docs/todo.md index 706be61..907452d 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -67,9 +67,7 @@ | 69 | 💎 Tier 3 | **`no-ephemeral-todo-reference` の `yaml`/`yml` extensions 追加理由をコメントで明記 (PR #110 T3-1 採用) ★ Bundle d** | todo5.md | XS | なし (rule⑥ コメント欄に 1-2 行追記、設計 doc と実装の経緯保存、git blame 不要化) | | 78 | 💎 Tier 3 | **ADR-038 (Rust timestamp arithmetic safety) + CLAUDE.md security 拡充 (PR #115 T3-1 採用) ★ Bb-3 follow-up** | todo5.md | S | なし (config が user-editable system boundary のとき `sanitize()` 値域検証を必須化し dependent arithmetic に `// SAFETY: により上限保証` コメントを要求するパターンを ADR + CLAUDE.md に codify、Rust 固有の checked_add + MAX_SAFE capping + time-dependent test の 3 層を明文化) | | 79 | 💎 Tier 3 | **`docs-governance.md` § Retirement Workflow に「残タスクの lifecycle 整合」要件明記 (PR #117 T3-1 採用)** | todo5.md | XS | なし (PR #117 で順位 15 を Bb-3 で吸収済として削除した際、現 Step 2「残タスクを priority table に登録」が priority table から除外するケース = 完了/deprioritize/defer を未定義だった実証。除外時の commit/PR で 3 値のいずれかを明示する要件を追加して将来の同型 ambiguity を構造的に防ぐ) | -| 80 | 🚀 Tier 1 | **cli-pr-monitor: rate-limit auto-retry wakeup 予約ロジックの整理 (PR #120 T1-1 採用) ★ Bundle f** | todo5.md | M | なし (PR #120 dogfood で `auto_retry_enabled=true` でも wakeup park が予約されず exit する事象を観測、ADR-018 設計の実装落ち) | -| 81 | 🚀 Tier 1 | **cli-pr-monitor: CR 投稿エラー (`Failed to post review comments`) auto-retry 拡張 (PR #120 T1-2 採用) ★ Bundle f** | todo5.md | M | 順位 80 と同 PR (Bundle f コア層、failure detection と retry path 共通化、rate-limit と統合) | -| 82 | 💎 Tier 3 | **ADR-018 update: rate-limit 以外の transient failure auto-retry 設計の明文化 (PR #120 T3-2 採用) ★ Bundle f** | todo5.md | S | 順位 80 / 81 と同 PR (実装と仕様の整合確保、Bundle f) | +| 81 | 🚀 Tier 1 | **cli-pr-monitor: CR 投稿エラー (`Failed to post review comments`) auto-retry 拡張 (PR #120 T1-2 採用) ★ Bundle f (defer)** | todo5.md | M | 1 観測のみで systemic 性未確認 (§A-2 P-5 PR で defer 判断、ADR-018 §追記 2026-05-08 で re-trigger 条件 = 2 件以上の同型観測を規定) | | 83 | 🔧 Tier 2 | **cli-pr-monitor: 複合 AND guard の各条件を独立テストで検証 (PR #120 T2-1 採用)** | todo5.md | S | なし (PR #120 W-001、`enrich_with_classifier_skips_when_disabled` の test setup で複合条件分離不全) | | 84 | 💎 Tier 3 | **グローバルルール: code-review.md に「early-return guard テスト分離」チェックリスト追記 (PR #120 T3-1 採用)** | todo5.md | XS | なし (順位 83 の知見を global rule に codify、`~/.claude/rules/common/code-review.md`、独立並列実施可) | | 85 | 🚀 Tier 1 | **cli-pr-monitor: monitor state machine guard 強化 (`review_state: not_found && findings: []` を pending 据置) (PR #121 T1-1 採用) ★ Bundle g** | todo5.md | S | なし (PR #119/#120/#121 で 3 PR 連続観測、Frequency Medium 閾値到達済み、Severity High = 誤 approved リスク、Bundle f #80 と関連だが verdict logic 側) | diff --git a/docs/todo5.md b/docs/todo5.md index 380fc83..e41c579 100644 --- a/docs/todo5.md +++ b/docs/todo5.md @@ -745,66 +745,27 @@ --- -### cli-pr-monitor: rate-limit auto-retry wakeup 予約ロジックの整理 (PR #120 T1-1 採用) ★ Bundle f - -> **動機**: PR #120 dogfood で「`rate_limit_config.auto_retry_enabled = true / max_retries = 3` でも rate-limit 検出後に wakeup park が予約されず exit する」事象を観測。手動 `@coderabbitai review` + `CronCreate` 投入でリカバリ。ADR-018 で設計された auto-retry に実装落ちが疑われる。 -> -> **参照**: PR #120 dogfood 観測、`.claude/feedback-reports/120.md` Tier 1 #1 -> -> **実行優先度**: 🚀 **Tier 1** — Effort M。`cli-pr-monitor/src/stages/poll.rs::handle_rate_limit_branch` 周辺と state.rate_limit_retries 制御の整合性確認。 - -#### 作業計画 - -- [ ] `auto_retry_enabled = true` 経路で rate-limit 検出時に確実に `parked_review_recheck` (or 専用 reason) で park signal を emit する -- [ ] `rate_limit_retries` がインクリメントされない / 上限到達後の挙動を test infra で再現 -- [ ] 順位 81 / 82 と同 PR で land (Bundle f コア実装) - -#### 完了基準 - -- rate-limit 検出時に常に PARK signal が emit される (silent exit が消える) -- regression test (auto_retry_enabled=true で rate-limit 注入 → park 予約) が green - ---- - -### cli-pr-monitor: CR 投稿エラー (`Failed to post review comments`) auto-retry 拡張 (PR #120 T1-2 採用) ★ Bundle f +### cli-pr-monitor: CR 投稿エラー (`Failed to post review comments`) auto-retry 拡張 (PR #120 T1-2 採用) ★ Bundle f (defer) > **動機**: PR #120 dogfood で CR walkthrough overlay が `Failed to post review comments` (rate-limit ではない transient failure) を表示するも `parse_rate_limit_status` が detected せず、auto-retry が発火しなかった。1 観測だが auto-retry の silent failure として機能不全。 > -> **参照**: PR #120 walkthrough comment (16:41Z 投稿)、`.claude/feedback-reports/120.md` Tier 1 #2 +> **参照**: PR #120 walkthrough comment (16:41Z 投稿)、`.claude/feedback-reports/120.md` Tier 1 #2、[ADR-018 §追記 2026-05-08](adr/adr-018-pr-monitor-takt-migration.md) +> +> **実行優先度**: 🚀 **Tier 1 (defer)** — §A-2 P-5 PR (2026-05-08) で Defer 判定。1 観測のみで systemic 性未確認のため、ユーザー方針 `feedback_no_unenforced_rules` (機械検知不可なら何もしない方がマシ) と整合させて 3 PR 観測閾値到達まで待つ。 > -> **実行優先度**: 🚀 **Tier 1** — Effort M。`check-ci-coderabbit/src/main.rs` の failure detection 拡張 + cli-pr-monitor の retry path 共通化。 +> **Re-trigger 条件**: `Failed to post review comments` (またはそれに類する rate-limit 以外の CR transient failure) が他の PR で 1 件以上追加観測 (合計 2 件以上) されたら本タスクを再活性化、実装に着手。 -#### 作業計画 +#### 作業計画 (defer 中、参考) - [ ] `Review failed` / `Failed to post review comments` 等の transient failure pattern を detection に追加 - [ ] rate-limit 系と統合する場合は state field を `transient_failure: Option` に一般化検討 -- [ ] 順位 80 と同 PR で land (Bundle f) +- [ ] ADR-018 §追記 2026-05-08 の「対象 transient failure 分類」表を「⏳ 未実装」→「✅ 実装済」に更新 #### 完了基準 - `Failed to post review comments` を含む walkthrough overlay 検出時に auto-retry が発火する - regression test (failure pattern 注入 → auto-retry 発火) が green - ---- - -### ADR-018 update: rate-limit 以外の transient failure auto-retry 設計の明文化 (PR #120 T3-2 採用) ★ Bundle f - -> **動機**: 順位 80 / 81 で実装する設計方針 (rate-limit 以外の transient failure も auto-retry 範囲) を ADR-018 に明記しないと、将来の同型実装落ちが構造的に再発するリスク。 -> -> **参照**: PR #120 T1-1 / T1-2 観測、`.claude/feedback-reports/120.md` Tier 3 #2、[ADR-018](adr/adr-018-pr-monitor-takt-migration.md) -> -> **実行優先度**: 💎 **Tier 3** — Effort S。順位 80 / 81 と同 PR で同時更新 (実装と仕様の整合確保)。 - -#### 作業計画 - -- [ ] ADR-018 に「auto-retry の対象 transient failure pattern」セクション追加 -- [ ] rate-limit + 投稿エラー + (将来候補) wakeup 未予約を列挙 -- [ ] 順位 80 / 81 と同 PR で land - -#### 完了基準 - -- ADR-018 に対象 failure pattern が enumerable 形式で列挙される -- 順位 80 / 81 の実装が ADR-018 から逆引き可能 +- ADR-018 §追記 2026-05-08 と整合 --- diff --git a/src/cli-pr-monitor/src/stages/poll.rs b/src/cli-pr-monitor/src/stages/poll.rs index 2178323..acd9ca8 100644 --- a/src/cli-pr-monitor/src/stages/poll.rs +++ b/src/cli-pr-monitor/src/stages/poll.rs @@ -116,9 +116,13 @@ 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, &result) - { + if let Some(terminal) = handle_rate_limit_branch( + &mut state, + ctx.rate_limit_config, + ctx.pr_info, + ctx.review_recheck_wait_secs, + &result, + ) { return Some(terminal); } @@ -293,6 +297,7 @@ fn handle_rate_limit_branch( state: &mut PrMonitorState, rate_limit_config: &RateLimitConfig, pr_info: &PrInfo, + review_recheck_wait_secs: u64, result: &serde_json::Value, ) -> Option { let rl = state.rate_limit.clone()?; @@ -319,7 +324,14 @@ fn handle_rate_limit_branch( return None; } - dispatch_rate_limit_outcome(state, &rl, pr_info, rate_limit_config.max_retries, result) + dispatch_rate_limit_outcome( + state, + &rl, + pr_info, + rate_limit_config.max_retries, + review_recheck_wait_secs, + result, + ) } fn dispatch_rate_limit_outcome( @@ -327,10 +339,18 @@ fn dispatch_rate_limit_outcome( rl: &crate::state::RateLimitState, pr_info: &PrInfo, max_retries: u32, + review_recheck_wait_secs: u64, result: &serde_json::Value, ) -> Option { match handle_rate_limit_retry(rl, state, pr_info, max_retries) { - RateLimitOutcome::Posted => finalize_posted_retrigger(state, rl, result), + RateLimitOutcome::Posted => finalize_posted_retrigger( + state, + rl, + pr_info, + review_recheck_wait_secs, + max_retries, + result, + ), RateLimitOutcome::Parked { wakeup_at_unix } => Some(finalize_parked( state, rl, @@ -356,9 +376,28 @@ fn dispatch_rate_limit_outcome( fn finalize_posted_retrigger( state: &mut PrMonitorState, rl: &crate::state::RateLimitState, + pr_info: &PrInfo, + review_recheck_wait_secs: u64, + max_retries: u32, result: &serde_json::Value, ) -> Option { 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.summary = format!( + "rate-limit retrigger 後の review 完了待ちを {}s 後に予約 (順位 80 fix: silent exit 防止)", + review_recheck_wait_secs + ); + if let Err(e) = write_state(state) { log_info(&format!( "[rate_limit] retrigger 後の state 永続化失敗、自動 retry を停止: {}", @@ -373,7 +412,11 @@ fn finalize_posted_retrigger( ), )); } - None + + let signal = format_park_signal(state, rl, pr_info, max_retries); + println!("{}", signal); + + Some(make_park_poll_result(state.clone())) } fn finalize_parked( @@ -1429,4 +1472,80 @@ mod tests { "sibling parity (review_recheck ↔ initial_review)" ); } + + 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()), + }; + (state, rl, pr_info) + } + + #[test] + fn finalize_posted_retrigger_schedules_park_after_post() { + let _guard = env_override_lock(); + let tmp = tempfile::tempdir().unwrap(); + let state_path = tmp.path().join("state.json"); + std::env::set_var("PR_MONITOR_STATE_FILE_OVERRIDE", &state_path); + + let (mut state, rl, pr_info) = setup_posted_retrigger_fixture(); + let result = finalize_posted_retrigger(&mut state, &rl, &pr_info, 300, 3, &serde_json::Value::Null); + + std::env::remove_var("PR_MONITOR_STATE_FILE_OVERRIDE"); + + 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 _guard = env_override_lock(); + let bad_path = unwritable_state_path(); + std::env::set_var("PR_MONITOR_STATE_FILE_OVERRIDE", &bad_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, + }; + + let result = finalize_posted_retrigger(&mut state, &rl, &pr_info, 300, 3, &serde_json::Value::Null); + + std::env::remove_var("PR_MONITOR_STATE_FILE_OVERRIDE"); + + assert!(result.is_some()); + assert_eq!( + result.unwrap().action, + "action_required", + "write_state 失敗時は action_required で抜ける (sibling parity with finalize_parked)" + ); + } }