feat(cli-pr-monitor): observer mode で post-pr review を並行通知化 (task 2) - #68
Conversation
## 主要変更 - `cli-pr-monitor --observe` サブコマンドを新設 (read-only 観測パス) - `pr-monitor-state.json` を 5 秒間隔ポーリング - `action != continue_monitoring` 検出で state 全文を stdout に出して exit 0 - `notified=true` はサイレント exit (Claude Code 再起動時の重複防止) - 10 分タイムアウトで exit 1 (orphan OK) - `decide()` を pure function として切り出し、7 パターンの unit test を追加 - `poll.rs`: iteration を跨いで `notified` flag を preserve (`PrMonitorState::new` が毎回 false リセットする挙動を修正) - `start_monitoring` 冒頭で state を明示初期化 (新セッション開始時の reset) - `package.json` に `observe-pr` / `mark-notified` スクリプト復活 - ADR-018 に 2026-04-22 追記で仕組みを反映 ## 付随: todo.md cleanup - task 4 (post-pr-review workflow の push 反映確認) を削除 理由: task 3 (PR #61) + task 6 (PR #67) の bookmark auto-advance で gap 解消済み ADR-022 の「主フロー vs 観測」責務分離原則とも整合 - task 2 を実装完了状態に縮退 (E2E 検証のみ残置) ## 参照 - ADR-018 追記 — observer モード仕様 - ADR-022 — 主フロー 100% 機械的 / observer は read-only side effect - docs/todo.md task 2 ## テスト - `cargo test -p cli-pr-monitor`: 97 passed / 0 failed / 7 ignored - `cargo clippy --release`: warning なし - 手動検証: 終端状態検出 (exit 0) / notified=true サイレント exit の両分岐確認
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 40 minutes and 21 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughPR Monitor に読み取り専用の「Observer Mode」を追加。 Changes
Sequence Diagram(s)sequenceDiagram
participant Observer as Observer CLI (--observe)
participant FS as State file (.claude/pr-monitor-state.json)
participant Monitor as Main monitor (takt run)
Observer->>FS: read_state() (poll)
alt state.notified == true
Observer->>Observer: SilentExit (exit 0)
else state.action != "continue_monitoring"
Observer->>Observer: emit_terminal_state (stdout JSON) and Exit (0)
else
Observer->>Observer: sleep & retry until timeout (10min) then exit 1
end
Monitor->>FS: write_state(...) (initial / updates / notified=true)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli-pr-monitor/src/stages/poll.rs (1)
159-169:⚠️ Potential issue | 🟠 Majortimeout を state file に終端状態として書き戻してください。
ここでは
PollResultだけtimed_outにして返すため、直前に保存された state はcontinue_monitoringのまま残ります。observer はそのまま自分の timeout まで待って exit 1 になり、timed_outstate を stdout 通知できません。🐛 修正案
if start.elapsed() >= Duration::from_secs(max_duration) { log_info(&format!("監視タイムアウト ({}秒)", max_duration)); + state.action = "timed_out".into(); + state.summary = format!("監視タイムアウト ({}秒)", max_duration); + state.last_checked = Some(utc_now_iso8601()); + let _ = write_state(&state); return PollResult { - action: "timed_out".into(), - summary: format!("監視タイムアウト ({}秒)", max_duration), + action: state.action, + summary: state.summary, ci: state.ci, coderabbit: state.coderabbit, findings: state.findings,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli-pr-monitor/src/stages/poll.rs` around lines 159 - 169, Within the timeout branch (inside the if start.elapsed() >= Duration::from_secs(max_duration) block) update the in-memory state from the ongoing/continue_monitoring status to a terminal "timed_out" status and persist that change to the state file before returning the PollResult; specifically, set the state's terminal field (e.g., state.status or state.state) to "timed_out" and call the existing state persistence function (e.g., save_state, write_state_file, or whatever persists state in this module) so the observer sees the terminal state, then return the PollResult as currently done.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cli-pr-monitor/src/stages/monitor.rs`:
- Around line 28-38: 現在の初期化 (PrMonitorState::new ... と write_state の呼び出し) が
observer 起動後に実行されるため、並列で pnpm create-pr と pnpm observe-pr を実行すると古い
pr-monitor-state.json を observer が読み誤って誤通知を起こします。修正は2案いずれかを採ってください：1) PR
作成フローの最初の observable point（例えば run_create_pr の先頭）へ PrMonitorState::new と
write_state の初期化処理を移動して observer が起動する前に必ずリセットする、または 2) state にユニークな session_id
を追加して PrMonitorState に保持し、observer 側（poll_loop 内の状態読み取り）で読み込んだ state.session_id
が現在のセッションと一致しない場合は無視するようにする。いずれかを実装して pr-monitor-state.json
の古いセッションによる誤検知を防いでください。
---
Outside diff comments:
In `@src/cli-pr-monitor/src/stages/poll.rs`:
- Around line 159-169: Within the timeout branch (inside the if start.elapsed()
>= Duration::from_secs(max_duration) block) update the in-memory state from the
ongoing/continue_monitoring status to a terminal "timed_out" status and persist
that change to the state file before returning the PollResult; specifically, set
the state's terminal field (e.g., state.status or state.state) to "timed_out"
and call the existing state persistence function (e.g., save_state,
write_state_file, or whatever persists state in this module) so the observer
sees the terminal state, then return the PollResult as currently done.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ed61bcc5-bce8-4dd7-996e-41ade90a1f31
📒 Files selected for processing (9)
docs/adr/adr-018-pr-monitor-takt-migration.mddocs/todo.mdpackage.jsonsrc/cli-pr-monitor/src/main.rssrc/cli-pr-monitor/src/stages/mod.rssrc/cli-pr-monitor/src/stages/monitor.rssrc/cli-pr-monitor/src/stages/observe.rssrc/cli-pr-monitor/src/stages/poll.rssrc/cli-pr-monitor/src/state.rs
💤 Files with no reviewable changes (1)
- src/cli-pr-monitor/src/state.rs
Resolved findings: - [Major] src/cli-pr-monitor/src/stages/monitor.rs:38 observer 起動前に stale state を読ませない初期化位置にしてください。
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/cli-pr-monitor/src/stages/monitor.rs (1)
28-38: 二段 reset の意図は妥当ですが、コメントの整合性を確認してください。
run_create_pr経由ではearly_state (pr=None, repo=None)→gh pr create→ ここでpr_infoを反映した再 reset、という二段構えになっており、run_monitor_only経路の初期化も兼ねるため現実装は妥当です。一点、
pr_info.push_timeがNoneの場合unwrap_or_default()によりstarted_atが空文字列になります。run_monitor_onlyでは直前でSome(utc_now_iso8601())が設定されるため現状は到達しませんが、将来start_monitoringの直接呼び出しが増えた場合に observability を損なう恐れがあります。utc_now_iso8601()をフォールバックにすることも検討いただけると安全です。♻️ 参考 diff
- pr_info.push_time.clone().unwrap_or_default(), + pr_info + .push_time + .clone() + .unwrap_or_else(utc_now_iso8601),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli-pr-monitor/src/stages/monitor.rs` around lines 28 - 38, The initialization currently sets started_at via pr_info.push_time.unwrap_or_default(), which yields an empty string if None; update the creation of init_state (PrMonitorState::new call) to fallback to utc_now_iso8601() when pr_info.push_time is None (e.g., use pr_info.push_time.clone().unwrap_or_else(|| utc_now_iso8601())), keeping the write_state(...) and logging behavior unchanged; this ensures run_create_pr, run_monitor_only and any direct start_monitoring calls preserve observability by recording a real timestamp.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/cli-pr-monitor/src/stages/monitor.rs`:
- Around line 28-38: The initialization currently sets started_at via
pr_info.push_time.unwrap_or_default(), which yields an empty string if None;
update the creation of init_state (PrMonitorState::new call) to fallback to
utc_now_iso8601() when pr_info.push_time is None (e.g., use
pr_info.push_time.clone().unwrap_or_else(|| utc_now_iso8601())), keeping the
write_state(...) and logging behavior unchanged; this ensures run_create_pr,
run_monitor_only and any direct start_monitoring calls preserve observability by
recording a real timestamp.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ffaa6412-8993-4181-9ef9-1a25a903ad1f
📒 Files selected for processing (2)
src/cli-pr-monitor/src/stages/create_pr.rssrc/cli-pr-monitor/src/stages/monitor.rs
…nitpick) ## 変更 - `start_monitoring` の init_state で `pr_info.push_time.clone().unwrap_or_default()` を `unwrap_or_else(utc_now_iso8601)` に置き換え - `push_time=None` だと `started_at=""` になる観測性の問題を防御的に修正 ## 背景 PR #68 CodeRabbit 2 回目レビュー (Nitpick) の提案: 現状の `run_monitor_only` 経路では直前で `Some(utc_now_iso8601())` が設定されるため 到達しないが、将来 `start_monitoring` が他経路から直接呼ばれた場合に started_at が 空文字列になり observability を損なう恐れあり。`run_create_pr` 冒頭の早期 reset と 同じフォールバック (utc_now_iso8601) に揃える。 ## テスト - `cargo test -p cli-pr-monitor`: 97 pass
1 participant
Summary
cli-pr-monitor --observeサブコマンドを新設:pr-monitor-state.jsonを 5s 間隔ポーリングし、終端状態 (action != continue_monitoring) を検出したら state JSON を stdout に出して exit (read-only、主フローに影響しない)decide()pure function 切り出し + 7 パターンの unit test (notified 優先 / terminal actions / continue)poll.rsが iteration を跨いでnotifiedflag を preserve するよう修正 (PrMonitorState::newが毎回 false リセットする既存挙動の fix)start_monitoring冒頭で state を明示初期化 (新セッション境界で notified=false にリセット)package.jsonにobserve-pr/mark-notifiedスクリプト追加Context
Why:
pnpm create-pr実行中に CodeRabbit 指摘検出 → takt 自動修正 → re-push が BG で進行する間、Claude Code が中間状態を受け取れず、すでに自動修正されている指摘についてユーザーから「未対応レビューをリストアップして」と重複依頼が発生していた。observer パスで早期通知して解消する。Trigger: docs/todo.md task 2 (post-pr review の並行通知化)。実装言語は todo.md 原案の PowerShell から Rust exe サブコマンドに差し替え — ADR-018「機械的ステップは Rust」原則と整合、既存
state.rs型の再利用、Windows 依存 (pwsh 二重シェル) の解消のため。Scope decision: 主フロー (cli-pr-monitor の既存 detect → fix → re-push) には手を入れず、read-only な observer を並行 BG タスクとして追加。Claude Code が 2 つの BG タスク (
pnpm create-pr+pnpm observe-pr) を起動する構成 (ADR-022 の「主フロー 100% 機械的 / 通知は read-only side effect」責務分離)。付随削除の task 4 は task 3 (PR #61) + task 6 (PR #67) の bookmark auto-advance で gap が原因側で閉じたため、workflow 側に追加の push 反映確認を入れる動機が消失。ADR-022 責務分離原則とも整合。
Validation
cargo test -p cli-pr-monitor: 97 pass / 0 fail / 7 ignoredcargo clippy --release: warning なし.claude/cli-pr-monitor.exe --observeで state file に terminal state を書いた場合 (exit 0 + JSON 出力) とnotified=trueの場合 (サイレント exit) の両分岐を確認pnpm pushpre-push review: verdict=APPROVE (初回 3 iterations / 7m58s で simplicity fix 2 件を takt が適用 → 再 push は 1 iteration / 5m4s で APPROVE)pnpm create-prとpnpm observe-prを並行 BG 起動し、observer の早期通知 → Minor ヒアリングが走ることを確認 (本 PR マージ後)References
feedback_bookmark_auto_naming.md— pnpm create-pr 実行ゲートSummary by CodeRabbit
新機能
バグ修正
ドキュメント