refactor(pr-monitor): CronCreate park モデルを撤去し single-shot 化 (WP-17 PR 3) - #353
Merged
Conversation
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>
…R 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>
…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>
|
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 Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughCronCreateのpark/wakeup方式を廃止しました。 ChangesPR監視single-shot移行
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Monitor
participant PollLoop
participant StateFile
participant GitHubActions
CLI->>Monitor: single-shot checkを開始する
Monitor->>StateFile: 同一PR・repository・headのstateを確認する
Monitor->>PollLoop: 継続stateで監視を実行する
PollLoop->>StateFile: pending_reviewまたはrate_limitedを保存する
PollLoop->>GitHubActions: 後続イベント経路を報告する
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 概要)
次のアクション
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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.
Inline comments:
In `@docs/harness-improvement-plan.md`:
- Around line 205-213: 同じ計画書の旧 park 検証残を更新し、rate-limit 時の park 観測項目を moot
として閉じてください。該当する WP-17 または rate-limit 検証残の記載では、削除済みの park/wakeup 機構への作業指示を除き、PR
イベントを担う GitHub Actions 経路の検証項目だけを残してください。
- Around line 205-213: GitHub Actions 経路の rate-limit
保留判定を実走行で検証し、pull_request_review、issue_comment、rate-limit 中の
retry、coderabbitai[bot] の dedup 判定を含む smoke/integration 結果を
docs/adr/adr-064-monitor-success-positive-evidence.md に追記して検証残を完了させる。既存の wakeup
廃止方針や関連する ADR 記録は変更せず、Actions 側の実行結果と判定を ADR-064 に反映する。
In `@src/cli-pr-monitor/src/stages/poll/mod.rs`:
- Around line 103-106: Update the head_commit assignments in
finalize_pending_review (src/cli-pr-monitor/src/stages/poll/mod.rs, lines
103-106), finalize_posted_retrigger
(src/cli-pr-monitor/src/stages/poll/rate_limit.rs, line 115), and
finalize_waiting_reset (src/cli-pr-monitor/src/stages/poll/rate_limit.rs, line
178) to update state only when ctx.pr_info.head_commit is Some, matching the
existing write-once behavior of fix_push_time; preserve the existing state value
when it is None.
In `@src/cli-pr-monitor/src/stages/poll/rate_limit/tests.rs`:
- Around line 213-248: テスト関数 finalize_waiting_reset_persists_head_commit の
pr_info で pr_number を Some(42) ではなく None に変更し、emit_shortcut_signal_if_eligible
が実際の gh CLI を呼び出さず早期 return するようにしてください。head_commit
の状態更新と永続化に関する既存の検証はそのまま維持してください。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c3357712-b61c-45e3-a220-82602a2253a9
📒 Files selected for processing (24)
.claude/hooks-config.tomldocs/adr/adr-018-pr-monitor-takt-migration.mddocs/adr/adr-034-coderabbit-auto-monitoring.mddocs/adr/adr-064-monitor-success-positive-evidence.mddocs/harness-improvement-plan.mddocs/todo18.mdpr-monitor-config.tomlsrc/check-ci-coderabbit/src/main.rssrc/cli-pr-monitor/src/config.rssrc/cli-pr-monitor/src/main.rssrc/cli-pr-monitor/src/stages/monitor.rssrc/cli-pr-monitor/src/stages/poll/iteration.rssrc/cli-pr-monitor/src/stages/poll/mod.rssrc/cli-pr-monitor/src/stages/poll/rate_limit.rssrc/cli-pr-monitor/src/stages/poll/rate_limit/tests.rssrc/cli-pr-monitor/src/stages/poll/rate_limit_signal.rssrc/cli-pr-monitor/src/stages/poll/review_recheck.rssrc/cli-pr-monitor/src/stages/poll/review_recheck_signal.rssrc/cli-pr-monitor/src/state.rssrc/cli-pr-monitor/src/util.rssrc/cli-telemetry-report/src/config.rssrc/hooks-session-start/src/main.rssrc/hooks-session-start/src/pr_monitor.rstemplates/pr-monitor-config.toml
💤 Files with no reviewable changes (4)
- src/cli-pr-monitor/src/stages/poll/rate_limit_signal.rs
- src/cli-pr-monitor/src/stages/poll/review_recheck_signal.rs
- src/cli-pr-monitor/src/stages/poll/review_recheck.rs
- src/hooks-session-start/src/pr_monitor.rs
Comment on lines
+205
to
+213
| #### 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 で確認する）。 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
旧 park 検証残を同じ計画書から更新してください。
Line [205]-[213] で park/wakeup の廃止を記録しています。しかし、同じファイルの Line [90]-[93] は、rate-limit 時の park 観測を現在の残作業として残しています。削除済み機構への作業指示になります。park の観測を moot として閉じ、GitHub Actions 経路の検証残だけを記載してください。
修正案
-### WP-15 追補残: レート制限 park の実観測
+### WP-15 追補残: GitHub Actions 経路の検証
-- 残作業: 実 push/PR サイクルで CodeRabbit レート制限が自然発生した際に
- (a) 監視が success で終わらず park することを実観測したら完了。
+- `park` の実観測は WP-17 PR 3 で機構が廃止されたため moot として閉じる。
+- 残作業: GitHub Actions 経路で保留判定と後続処理を確認する。🤖 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 `@docs/harness-improvement-plan.md` around lines 205 - 213, 同じ計画書の旧 park
検証残を更新し、rate-limit 時の park 観測項目を moot として閉じてください。該当する WP-17 または rate-limit
検証残の記載では、削除済みの park/wakeup 機構への作業指示を除き、PR イベントを担う GitHub Actions
経路の検証項目だけを残してください。
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
workflow="$(fd -t f -a 'pr-monitor.yml' . | head -n 1)"
test -n "$workflow"
rg -n -C 8 \
'pull_request_review|issue_comment|workflow_dispatch|pr_number|cli-pr-monitor|monitor-only|pending_review|rate_limited' \
"$workflow"
rg -n -C 6 \
'head_commit|dedup|rate.limit|pending_review|rate_limited' \
src/cli-pr-monitor \
docs/adr/adr-064-monitor-success-positive-evidence.mdRepository: aloekun/claude-code-hook-test
Length of output: 167
🏁 Script executed:
#!/bin/bash
set -euopipefail
printf '\n=== tracked candidate files ===\n'
git ls-files | rg '(^docs/harness-improvement-plan\.md$|^docs/adr/adr-064|pr-monitor|cli-pr-monitor)' || true
printf '\n=== harness lines 190-225 ===\n'
sed -n '190,225p' docs/harness-improvement-plan.md | nl -ba -v190
printf '\n=== adr-064 file size and headings ===\n'
fd -t f 'adr-064' docs src || true
while IFS= read -r f; do
echo "--- $f"
wc -l "$f"
sed -n '1,120p' "$f"
done < <(fd -t f 'adr-064' docs)
printf '\n=== repo-wide searches ===\n'
rg -n -C 4 \
'pull_request_review|issue_comment|workflow_dispatch|pr_number|cli-pr-monitor|monitor-only|pending_review|rate_limited|head_commit|dedup|rate.limit|next_wakeup_at_unix|wakeup_reason|CronCreate' \
. || trueRepository: aloekun/claude-code-hook-test
Length of output: 820
🏁 Script executed:
#!/bin/bash
set -e
printf '\n=== tracked candidate files ===\n'
git ls-files | rg '(^docs/harness-improvement-plan\.md$|^docs/adr/adr-064|pr-monitor|cli-pr-monitor)' || true
printf '\n=== harness lines 190-225 ===\n'
sed -n '190,225p' docs/harness-improvement-plan.md | nl -ba -v190
printf '\n=== adr-064 related files ===\n'
git ls-files | rg '(^docs/adr/adr-064|--064--|adr-[0-9]+.*monitor)' || true
while IFS= read -r f; do
echo "--- $f ($(wc -l < "$f") lines)"
sed -n '1,160p' "$f"
done < <(git ls-files | rg '(^docs/adr/adr-064|--064--|adr-[0-9]+.*monitor)')
printf '\n=== repo-wide searches ===\n'
rg -n -C 4 \
'pull_request_review|issue_comment|workflow_dispatch|pr_number|cli-pr-monitor|monitor-only|pending_review|rate_limited|head_commit|dedup|rate\.limit|next_wakeup_at_unix|wakeup_reason|CronCreate' \
. || trueRepository: aloekun/claude-code-hook-test
Length of output: 1787
🏁 Script executed:
#!/bin/bash
set -e
printf '\n=== harness lines 190-225 ===\n'
sed -n '190,225p' docs/harness-improvement-plan.md | awk '{printf "%4d\t%s\n", NR+189, $0}'
printf '\n=== adr-064 evidence excerpt ===\n'
sed -n '1,120p' docs/adr/adr-064-monitor-success-positive-evidence.md | awk '{printf "%4d\t%s\n", NR, $0}'
printf '\n=== workflow relevant contents ===\n'
sed -n '1,260p' .github/workflows/pr-monitor.yml | awk '{printf "%4d\t%s\n", NR, $0}'
printf '\n=== key references in source/docs ===\n'
rg -n -C 4 'pull_request_review|issue_comment|workflow_dispatch|pr_number|cli-pr-monitor|monitor-only|pending_review|rate_limited|head_commit|dedup|rate\.limit|next_wakeup_at_unix|wakeup_reason|CronCreate|actions' \
.github/workflows/pr-monitor.yml \
docs/adr/adr-064-monitor-success-positive-evidence.md \
src/cli-pr-monitor \
docs/harness-improvement-plan.md \
--glob '!**/target/**' \
|| trueRepository: aloekun/claude-code-hook-test
Length of output: 50386
GitHub Actions 経路での rate-limit 保留判定を実走行してください。
wakeup 廃止で PR が Actions 経路に依存するため、docs/adr/adr-064-monitor-success-positive-evidence.md の検証残は Actions 側で閉じてください。pull_request_review・issue_comment・レート制限中の retry・coderabbitai[bot] の dedup 判定を含む smoke/integration 実行結果を ADR-064 に追記してください。
🤖 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 `@docs/harness-improvement-plan.md` around lines 205 - 213, GitHub Actions 経路の
rate-limit 保留判定を実走行で検証し、pull_request_review、issue_comment、rate-limit 中の
retry、coderabbitai[bot] の dedup 判定を含む smoke/integration 結果を
docs/adr/adr-064-monitor-success-positive-evidence.md に追記して検証残を完了させる。既存の wakeup
廃止方針や関連する ADR 記録は変更せず、Actions 側の実行結果と判定を ADR-064 に反映する。
Comment on lines
+103
to
+106
| 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)); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
terminal 化の 3 経路が head_commit を None で上書きします。 共通の根本原因は、pr_info.head_commit の値を条件なしで state へ代入していることです。gh pr view --json headRefOid の取得が失敗すると pr_info.head_commit は None になり、既存 state の継続アンカーが消えます。その結果、次回 --monitor-only 実行時に should_continue_state が false となり、時刻窓が「今」にリセットされ、その間に届いた CodeRabbit コメントを取りこぼします。fix_push_time と同じ write-once 方針へ揃えてください。
src/cli-pr-monitor/src/stages/poll/mod.rs#L103-L106:finalize_pending_reviewでctx.pr_info.head_commitが Some のときだけstate.head_commitを更新する。src/cli-pr-monitor/src/stages/poll/rate_limit.rs#L115-L115:finalize_posted_retriggerで同じ条件付き更新にする。src/cli-pr-monitor/src/stages/poll/rate_limit.rs#L178-L178:finalize_waiting_resetで同じ条件付き更新にする。
📍 Affects 2 files
src/cli-pr-monitor/src/stages/poll/mod.rs#L103-L106(this comment)src/cli-pr-monitor/src/stages/poll/rate_limit.rs#L115-L115src/cli-pr-monitor/src/stages/poll/rate_limit.rs#L178-L178
🤖 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/poll/mod.rs` around lines 103 - 106, Update the
head_commit assignments in finalize_pending_review
(src/cli-pr-monitor/src/stages/poll/mod.rs, lines 103-106),
finalize_posted_retrigger (src/cli-pr-monitor/src/stages/poll/rate_limit.rs,
line 115), and finalize_waiting_reset
(src/cli-pr-monitor/src/stages/poll/rate_limit.rs, line 178) to update state
only when ctx.pr_info.head_commit is Some, matching the existing write-once
behavior of fix_push_time; preserve the existing state value when it is None.
Comment on lines
+213
to
+248
| 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( | ||
| &mut state, | ||
| &rl, | ||
| &pr_info, | ||
| &serde_json::Value::Null, | ||
| &state_path, | ||
| ); | ||
|
|
||
| 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 初期化に倒れる" | ||
| ); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
このテストは実 gh CLI を呼び出します。
pr_info に pr_number: Some(42) を設定しているため、finalize_waiting_reset 内の emit_shortcut_signal_if_eligible が fetch_mergeable_status を実行し、gh pr view を呼び出します。ネットワークと認証に依存するため、CI で遅延または不安定になります。このテストの検証対象は head_commit の persist だけです。pr_number を None にして shortcut 経路を早期 return させてください。
♻️ 提案する修正
let pr_info = crate::util::PrInfo {
- pr_number: Some(42),
- repo: Some("o/r".into()),
+ pr_number: None,
+ repo: None,
push_time: None,
head_commit: Some("deadbeef".into()),
fix_push_time: None,
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 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( | |
| &mut state, | |
| &rl, | |
| &pr_info, | |
| &serde_json::Value::Null, | |
| &state_path, | |
| ); | |
| 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 初期化に倒れる" | |
| ); | |
| } | |
| 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: None, | |
| repo: None, | |
| push_time: None, | |
| head_commit: Some("deadbeef".into()), | |
| fix_push_time: None, | |
| }; | |
| let outcome = finalize_waiting_reset( | |
| &mut state, | |
| &rl, | |
| &pr_info, | |
| &serde_json::Value::Null, | |
| &state_path, | |
| ); | |
| 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 初期化に倒れる" | |
| ); | |
| } |
🤖 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/poll/rate_limit/tests.rs` around lines 213 -
248, テスト関数 finalize_waiting_reset_persists_head_commit の pr_info で pr_number を
Some(42) ではなく None に変更し、emit_shortcut_signal_if_eligible が実際の gh CLI を呼び出さず早期
return するようにしてください。head_commit の状態更新と永続化に関する既存の検証はそのまま維持してください。
Contributor
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)該当なし Applicable Findings (Medium 以下)
Filtered (not applicable)
次のアクション
|
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>
…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
force-pushed
the
feat/wp17-pr3-wakeup-removal
branch
from
August 3, 2026 16:10
f626224 to
c4bc65e
Compare
Contributor
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)該当なし Applicable Findings (Medium 以下)該当なし（前回分析の #1〜#3 はいずれも Filtered (not applicable)該当なし 前回指摘の対応状況（参考・diff で確認済み）
次のアクション
|
This was referenced Aug 4, 2026
aloekun
added a commit
that referenced
this pull request
Aug 4, 2026
スモーク段 2 の 2 回目の実走 (2026-08-04) で `Extract findings JSON` step が exit 1 で失敗し、
Phase B が findings 取得直後で停止した。
## 原因: 出力形式の保証を指示層だけに委ねていた
findings agent は仕込んだ docs 指摘 3 件を**過不足なく検出**していたが、出力を ```json で
囲んで返した:
jq: parse error: Invalid numeric literal at line 2, column 0
[FIX_PUSH_DENY] findings agent の出力が JSON 配列ではありません
```json
[
{"file": "docs/phase-b-smoke-test.md", ...},
...
]
```
prompt は「前置き・後書き・**コードフェンスを一切含まない** JSON 配列のみ」と明示していたが、
agent はこれに従わなかった。**指示層だけで出力形式を保証する設計では、モデルの気まぐれで
経路全体が止まる** (ADR-042: ルールでなく仕組みで守る)。
## 修正: 決定論層でフェンス行を落とす
`Extract findings JSON` で jq に渡す前に sed でフェンス行のみを除去する:
sed -e 's/^[[:space:]]*```[[:alnum:]]*[[:space:]]*$//' "$RAW_FILE" > "$FINDINGS_FILE"
設計上の線引き:
- **フェンスを剥がす以上のことはしない**。中身の修復 (引用符の補完、部分抽出等) は試みない。
agent が全く違う形式を返した場合は従来どおり落とすべきで、ヒューリスティックな救済は
「壊れた入力をそれらしく通す」経路を作る
- **fail-closed は維持**。剥がした後も配列でなければ下の guard が exit 1 で止める
- 診断出力を `$RAW_FILE` (剥がす前) に変更。フェンス以外が原因で落ちた場合に、agent が実際に
何を返したかがログに残る
prompt 側も二層目の防御として残し、「フェンスがあると後続がパースに失敗する」という理由と
実際に発生した日付を明記した (指示は保険であり主たる担保ではない旨も併記)。
## 検証
- **実際に落ちた入力でロジックを実測**: ```json で囲まれた配列 → 除去後にパース成功
(isArray: true、要素 2 件)。フェンスなしの素の配列 → 副作用なくパース成功
- js-yaml パースで 13 step 構造と claude_args 全行のフラグ形式を確認
- 実走検証はスモーク段 2 (PR #355) の 3 回目の dispatch が兼ねる
## 併せて観測できたこと (段 2 の中間成果)
- findings agent が仕込んだ不整合 3 点を正確に検出 = 観測装置の設計は妥当
- 決定論的著者フィルタ (#352) と gh api 修正 (#356) が実走で正常動作
- degrade 分岐 (#353 で修正) が `GATE_OUTCOME: skipped` を正しく判別し、
「gate step に到達しませんでした」を出力 = fail-closed と診断が設計どおり機能
Co-authored-by: Claude Opus 5 (1M context) <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.
概要
WP-17 PR 3(wakeup 機構の廃止)です。cli-pr-monitor を「1 回 check して必ず terminal
報告で終了する」single-shot モデルへ移行し、CronCreate park モデル(Bb-1/Bb-2)と
hooks-session-start の catch-up nudge を撤去します。
3 コミット / 22 ファイル / +575 -2503 行。diff の大半が削除の撤去 PR で、分割すると
park モデルが中途半端に残る壊れた中間状態を作るため、PR size gate は ADR-069 § 決定 3-3
の正規経路(
PR_SIZE_CHECK_OVERRIDE=1+ 本欄での理由明記)でバイパスしました。なぜ廃止するか
CronCreate 失効 = 監視の取りこぼしを実観測(catch-up nudge はその症状への救済層で、
根治ではなかった)
レビュー到着・後続コメントがそのままトリガーになるため、ローカルの時限 wakeup は冗長
挙動の変化
pending_review報告rate_limited報告(ADR-064 (b) の保留判定文を維持)いずれも「後続は GitHub Actions 経路が処理」を明示し、pending を silent success に
見せません(ADR-064 陽性証拠原則)。
残したもの(park の付随物ではないため)
should_continue_state): 同一 PR + 同一 head ならstarted_at/fix_push_timeを維持。落とすと再実行のたびに CR コメントの新着判定窓がリセットされる
[RATE_LIMIT_BUT_MERGEABLE]): rate-limit 中でもmergeable なら即 merge を提案する独立機能。
rate_limit.rsへ移設して維持pre-push レビューが検出した回帰 2 件(両方修正済み)
本 PR は 2 ラウンドの REJECT を受け、どちらも実在の回帰でした:
shortcut を巻き込んでいた。「他の撤去物は全て ADR に列挙されているのに、これだけ無言で
消えている」という検出 → テストごと復元
次回実行の継続判定が fresh 初期化に倒れて時刻窓がリセットされる回帰。本 PR 自身が
動機に挙げた incident class(docs: PR-W5 (#234) land 反映 + post-merge-feedback 採用 3 件を todo 登録 (順位 248-250) #237/feat: Linux バイナリビルド + クラウド setup script (WP-15) #307/fix(monitor): 不完全な信号から success を導く fail-open 2 件を修正 (rate-limit 検知 / 判定文) #309)の再導入だった → 保存 + 回帰テスト 2 件
前方互換
旧 state file(wakeup fields)・旧 config(
[review_recheck]/max_duration_secs)が残っていても unknown field として無視されます(前方互換テストで固定)。
ADR 記帳
検証
cargo test --workspace1901 件 pass(park 系テスト約 52 件削除、新モデル + 回帰テスト追加)cargo clippy --workspace --all-targets -- -D warnings緑、lint:docs / lint:md 0 errorこの PR 自身がスモーク段 1 の観測対象です
AUTONOMY_ENABLED再設定済み・master 版 workflow・非claude/ブランチという構成なので、CodeRabbit レビュー到着時に fix job が起動し prefix 層で deny(
[FIX_PUSH_DENY] branch=... claude/ prefix ではない、job は緑)となるのが期待動作です。観測結果はADR-067 の検証記録に記帳します。
マージ後の運用変更
pnpm pushチェイン末尾の監視は「その時点の状態報告」になります。レビュー未着ならpending_reviewで終わり、後続は Actions 経路のコメントか手動--monitor-only再実行で把握します(Claude セッションが CronCreate で wakeup を予約する運用は終了)
.claude/の exe 再ビルドが必要です(cli-pr-monitor/hooks-session-start/cli-push-runner。配備乖離の再発防止は WP-17 完了後の feedback 採否で扱う予定)🤖 Generated with Claude Code
Summary by CodeRabbit
変更点
ドキュメント