From 296e6ceff1c3527b5c5680b19cfeeb9f050877d0 Mon Sep 17 00:00:00 2001 From: aloekun Date: Wed, 22 Apr 2026 17:47:23 +0900 Subject: [PATCH 1/3] =?UTF-8?q?feat(cli-pr-monitor):=20observer=20mode=20?= =?UTF-8?q?=E3=81=A7=20post-pr=20review=20=E3=82=92=E4=B8=A6=E8=A1=8C?= =?UTF-8?q?=E9=80=9A=E7=9F=A5=E5=8C=96=20(task=202)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 主要変更 - `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 の両分岐確認 --- docs/adr/adr-018-pr-monitor-takt-migration.md | 28 +++ docs/todo.md | 130 ++------------ package.json | 2 + src/cli-pr-monitor/src/main.rs | 12 +- src/cli-pr-monitor/src/stages/mod.rs | 2 + src/cli-pr-monitor/src/stages/monitor.rs | 13 ++ src/cli-pr-monitor/src/stages/observe.rs | 163 ++++++++++++++++++ src/cli-pr-monitor/src/stages/poll.rs | 10 +- src/cli-pr-monitor/src/state.rs | 1 - 9 files changed, 241 insertions(+), 120 deletions(-) create mode 100644 src/cli-pr-monitor/src/stages/observe.rs diff --git a/docs/adr/adr-018-pr-monitor-takt-migration.md b/docs/adr/adr-018-pr-monitor-takt-migration.md index 79ab4134..fa78dcd3 100644 --- a/docs/adr/adr-018-pr-monitor-takt-migration.md +++ b/docs/adr/adr-018-pr-monitor-takt-migration.md @@ -102,3 +102,31 @@ extra_args = ["--pipeline", "--skip-git"] - **Phase 2: fix loop + re-push**: takt ワークフローに fix ステップを追加し、CodeRabbit 指摘の自動修正 + re-push まで一気通貫で処理 - **push-runner との共通化**: fix loop / report ロジックの共通 takt instruction 化 + +## 追記 (2026-04-22): observer モードで並行通知化 + +### 背景 + +takt fix の自動修正 + re-push が BG で走る間、Claude Code は state 更新を +リアルタイムで追えず、ユーザーが「未対応レビューをリストアップして」と +重複依頼するケースが発生した。 + +### 追加 + +- `cli-pr-monitor --observe` サブコマンドを新設 (read-only 観測パス) + - `pr-monitor-state.json` を 5 秒間隔ポーリング + - `action != "continue_monitoring"` を検出したら state 全文を stdout に出して exit + - `notified=true` はサイレント exit (Claude Code 再起動時の重複防止) + - 10 分タイムアウトで exit 1 (orphan OK) +- `pnpm observe-pr` / `pnpm mark-notified` スクリプトを復活 +- `poll.rs`: iteration を跨いで `notified` フラグを preserve +- `start_monitoring` 冒頭で state を初期化 (新セッション開始時の reset) + +### 設計原則 (崩さないこと) + +- **主フロー (cli-pr-monitor 本体) は 100% 機械的**に detect → fix → re-push を完了させる。 + observer や Claude Code の判断を gate にしない +- **observer は read-only な side effect**。state file を読むだけで主フローには影響しない + +Claude Code が Task A (`pnpm create-pr`) と Task B (`pnpm observe-pr`) を +並行 BG 起動することで、observer は早期に終端状態を報告できる (ADR-022 責務分離原則と整合)。 diff --git a/docs/todo.md b/docs/todo.md index f7f6bd3e..cdd1dc7a 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -23,58 +23,20 @@ - SessionStart hook は master に実装済み (`src/hooks-session-start/`)。セッション引継ぎ設計は session ID → jsonl transcript 紐付けの ADR が必要 - takt-test-vc での試験運用を先に行い、本プロジェクトに反映 -### 2. post-pr review フローの並行通知化 (観測用 BG タスク追加) - -- **やろうとしたこと**: `pnpm create-pr` 実行中に CodeRabbit 指摘検出 → takt 自動修正 → re-push が BG で進行するが、ユーザーは完了通知まで状況を把握できない。Claude Code にも進捗が届かず、すでに自動修正されている指摘についてユーザーから「未対応レビューをリストアップして」と重複依頼が発生する。これを早期通知で解消する -- **現在地**: 設計確定、未実装 -- **背景**: - - 現状フロー (cli-pr-monitor が一気通貫): `poll → detect → takt fix → re-push → report → exit` - - ユーザーは exit 時点まで中間状態を見られない - - 深刻度別の扱い: - - **Critical / Major**: 既存の `auto_push_severity` 設定で自動修正 - - **Minor 以下**: 並行してユーザーにヒアリング。主フローの外で判断 -- **設計原則 (絶対に崩さないこと)**: - - **主フロー (detect → fix → re-push) は 100% 機械的**。Claude Code の判断や通知受領を gate にしない。セッション切断や AI スキップでフローが止まると、ハーネスとしての「必ず修正まで到達する」保証が崩れる - - **通知は side effect**。主フローの成否に影響しない観測用パス。cli-pr-monitor 自体には手を入れない -- **実装内容 (並行タスク方式)**: - - ``` - Claude Code が 2 つの BG タスクを同時起動: - - Task A (主フロー): pnpm create-pr - → cli-pr-monitor 既存フローをそのまま実行 - → 変更不要 - - Task B (通知用): scripts/observe-pr-state (新設) - → /pr-monitor-state.json をポーリング（cli-pr-monitor.exe と同じディレクトリ） - → action=action_required を検出したら state 内容を stdout に出して exit - → Claude Code が完了通知を受領 → ユーザーにレポート表示 + Minor ヒアリング - ``` - -- **タスク分解**: - - [ ] `scripts/observe-pr-state.ps1` (Windows 用) 新設 - - `/pr-monitor-state.json`（cli-pr-monitor.exe と同じディレクトリ）を 5-10s 間隔ポーリング - - `action == "action_required"` または `action == "approved"` 検出で state 全文を出力して exit 0 - - 10 分のタイムアウト (ADR-016 準拠) 後は exit 1 - - 起動時に `notified` フラグを確認し、`true` であれば出力をスキップして exit 0（Claude Code 再起動時の重複レポート防止） - - `notified=true` への書き込みは `cli-pr-monitor` の `mark_notified` ステージ（`src/cli-pr-monitor/src/stages/mark_notified.rs`）が担う - - `notified=false` へのリセットは新しい監視セッション開始時に `cli-pr-monitor` 側で行う（observer は stateless/single-shot のため自身ではリセットしない） - - [ ] `package.json` に `"observe-pr"` スクリプト追加 (PowerShell 起動) - - [ ] `post-pr-create-review-check` スキルを修正 - - 現状: daemon 起動後に state file を一度読んで報告する一段構成 - - 変更後: `pnpm create-pr` と `pnpm observe-pr` を並行 BG 起動 - - observer exit 時に state を整形してユーザーに提示 - - Minor 指摘があれば AskUserQuestion で対応方針をヒアリング - - [ ] E2E 検証: CodeRabbit Major ありの PR で通知タイミングを確認 -- **詰まっている箇所**: - - **Windows 依存**: Bash 経由では PowerShell 起動が二重シェルで動作不安定。pnpm script で `pwsh -File` 直接呼び出しが必要か要調査 - - **Minor ヒアリングの UX**: observer 完了時に Claude Code に state が届くが、並行して Task A の fix も進行中。Task A が Minor を `user_decision` verdict で止めた場合と action_required で進んでいる場合の挙動差を確認 -- **考慮事項**: - - observer は read-only (state file 読み取りのみ)。Task A に影響しない - - Task A と Task B 両方タイムアウト時のクリーンアップ (observer 側は orphan OK、Task A 側は既存タイムアウト機構に委ねる) -- **参照 ADR**: - - ADR-018 (post-pr-monitor takt 化): 既存フローの前提 - - ADR-016 (長時間コマンド): observer の 10 分タイムアウト設計 +### 2. post-pr review フローの並行通知化 (E2E 検証待ち) + +- **やろうとしたこと**: `pnpm create-pr` 実行中に CodeRabbit 指摘検出 → takt 自動修正 → re-push が BG で進行する間、Claude Code が中間状態を受け取れず「未対応レビューをリストアップして」の重複依頼が発生していた。observer パスで早期通知して解消する +- **現在地**: 実装完了 (ADR-018 追記セクションに仕組み反映済み)。残るは実 PR での E2E 観察のみ + - [x] `src/cli-pr-monitor/src/stages/observe.rs` 新設 (Rust exe サブコマンド、PowerShell 廃案) + - [x] `cli-pr-monitor --observe` ハンドラを `main.rs` に配線 + observe stage の unit test 7 件 + - [x] `poll.rs`: iteration を跨いで `notified` flag を preserve (`PrMonitorState::new` が毎回 reset する挙動を修正) + - [x] `start_monitoring` 冒頭で state を明示初期化 (新セッション開始時の reset) + - [x] `package.json` に `observe-pr` / `mark-notified` スクリプト追加 + - [x] `~/.claude/skills/post-pr-create-review-check/skill.md` を並行 BG 構成に更新 (stale な daemon/CronCreate 記述を除去) + - [ ] 実 PR での E2E 検証: CodeRabbit Major ありの PR で、Claude Code が `pnpm create-pr` と `pnpm observe-pr` を並行 BG 起動し、observer の早期通知で Minor ヒアリングが走ることを確認 +- **参照**: + - ADR-018 追記 (2026-04-22) — observer モードと責務分離原則 + - ADR-022 — 「主フローは 100% 機械的 / 通知は read-only side effect」の境界 ### 3. cli-pr-monitor の auto re-push に bookmark 自動前進を移植 @@ -92,70 +54,6 @@ - PR #63 (takt fix のコミット分離、完了済) - ADR-024 (共通 jj helper、試験運用) -### 4. post-pr-review workflow の verdict に push 反映確認を追加 - -- **やろうとしたこと**: post-pr-review workflow が local file の状態だけを見て `approved` を判定する設計のため、auto re-push が失敗していても `approved` になる gap を埋める。PR #53 で「local 修正済み + bookmark 未前進 → workflow approved」の食い違いを実測 -- **現在地**: 設計段階、未着手。task 3 (bookmark 自動前進) の defense-in-depth 的位置付け -- **実装内容案**: - - [ ] workflow の analyze (or 終了前) ステップで `gh api` を叩いて remote の最新 commit SHA を取得し、local の fix commit SHA と比較 - - [ ] 一致しなければ `verdict = action_required` にダウングレードし「remote 未反映」のメッセージを出す -- **詰まっている箇所**: - - **task 3 実装後の優先度再評価**: task 3 が入れば「auto re-push が失敗しない限り approved は妥当」となるため、本タスクの優先度は大きく下がる。実装可否は task 3 完了後に判断 - - **takt workflow の YAML から外部コマンド呼び出しが可能か**: facets instruction 内でシェルが呼べるかは要調査 (ADR-019/020 の facets 設計内) -- **参照 ADR**: - - ADR-018 (post-pr-monitor takt 化) - - ADR-019 (CodeRabbit レビュー運用ハイブリッド) - - 関連: task 3 (bookmark auto-advance) - -### 5. prepare-pr skill の前提条件を ADR-022 v3 に整合 - -- **やろうとしたこと**: `prepare-pr` skill が前提不成立で停止する運用痛を解消する。現状の前提 3「jj working copy は `pnpm push` 完了済」は ADR-022 v1 時代 (Claude が commit description / bookmark / push に触れない前提) の書き方で、ADR-022 v3 で確立した「Claude が草案生成 → 承認 → 実行」フローと噛み合っていない -- **背景**: - - PR #64 セッション冒頭で発生: 「PR を作成して」と依頼された際、skill が前提 3 で停止し、ユーザーに「commit description 書いて pnpm push まで先にやれ」と返した。しかし ADR-022 v3 の承認ゲート表では commit description / bookmark / pnpm push はすべて Claude に委譲される想定 - - 結果として skill の責務境界が実運用とズレており、ADR-022 v3 を書く契機になった - - ADR-022 v3 側は修正済み (PR #64)。skill 側が取り残されている -- **現在地**: 設計段階、未着手。skill の本体は global + `$CLAUDE_SKILLS_REPO` (`C:\Users\HIROKI\.claude\skills\prepare-pr\` / `E:\work\claude-code-skills` 配下)。本リポジトリ外 -- **作業内容**: - - [ ] `SKILL.md` の「前提条件」セクション書き換え: - - 旧: 前提 3「jj working copy は pnpm push 完了済」 - - 新: 「commit description / bookmark / push が完了している。未完了なら skill 外で Claude が順次実行してから起動する」(or skill 内 Step 0 で実行) - - [ ] 「実行手順 > Step 1: 現状確認」のチェック項目も前提変更に合わせて書き換え。`master..@` 差分空 / bookmark なし / remote 未反映は「skill の責務外のはずだが、運用中に前提未達なら Claude に fallback を促す」方針に - - [ ] `draft.md` の入力セクション (`jj log -r 'master..@'` 等) が「空」を返す場合の fallback 記述を追加 - - [ ] ADR-022 v3 承認ゲート表 への参照リンクを skill 冒頭に明記 - - [ ] evals/evals.json の scenario 2-4 (前提未達 → 早期終了) を新設計に合わせて更新 -- **完了条件**: - - 「PR を作成して」依頼で skill が止まらず、PR 作成まで抜ける happy path を 1 回検証 - - `$CLAUDE_SKILLS_REPO` と global deploy 先 (`~/.claude/skills/prepare-pr/`) の同期を `/skill-sync-check` で確認 -- **参照**: - - ADR-022 v3 (PR #64) — 承認ゲート表 / 原則 1 再構築 - - PR #62 (task 7) — skill を global + skill repo に分割した経緯 - - memory `feedback_bookmark_auto_naming.md` — Claude の権限境界 - - PR #64 の本セッション記録 — 前提不成立で停止した実例 - -### 6. cli-pr-monitor の空 fix commit cleanup で @ を PR tip に re-parent - -- **やろうとしたこと**: takt fix が NoChange で空 child commit を abandon した後、`@` (working copy) が孤児位置に残る問題を解消。結果として次の `jj new` が孤児 commit の上に積まれ、手動で `jj abandon` + `jj new -r ` の修正が必要になる -- **背景**: - - PR #64 / PR #66 (docs-adr-028-axis-boundary) セッションで連続発生。毎回 `jj abandon ` → `jj abandon ` → `jj new -r ` の 3 ステップ手修正が必要だった - - `cli-pr-monitor/src/stages/` 配下で空 commit を abandon する処理はあるが、abandon 後の `@` 再配置までは実装されていなかった -- **現在地**: 実装 + 統合テスト完了。実 PR での E2E 検証を残すのみ - - [x] `fix_commit.rs::try_abandon_empty_fix_commit` の abandon 成功後に `reparent_at_to_pr_tip` を呼び出すフローを追加 - - [x] `push_jj_bookmark.rs::resolve_pr_tip_commit_id` を新設: 単一の非 trunk local bookmark を PR tip として解決 (0 件 / 複数件は fail-safe で None) - - [x] 既に `@-` が PR tip と一致する場合は redundant な空 commit を作らずスキップ (`parent_commit_id_is`) - - [x] 統合テスト `integration_try_abandon_reparents_at_to_pr_tip_after_cleanup`: `C1 (bookmark) ← C1' (pnpm push の空) ← Y (fix)` → cleanup 後 `@-` が `C1` に戻ることを実 jj で確認 - - [x] 統合テスト `integration_try_abandon_skips_reparent_with_multiple_bookmarks`: 複数 bookmark (stacked PR 想定) では reparent をスキップし jj default 配置に任せる fail-safe 挙動を確認 - - [ ] 実 PR での E2E 検証: 次の CodeRabbit NoChange 修正ループで `@-` が自動で PR tip に戻ることを目視確認 (本 PR マージ後のリリース) -- **詰まっている箇所**: 解消済 - - **PR tip の確定方法**: `jj bookmark list` ベースで非 trunk local bookmark を解決する方式を採用。単一 bookmark 限定で stacked PR は fail-safe スキップ (task 3 の dispatch パターンに合わせた) -- **関連ファイル**: - - `src/cli-pr-monitor/src/fix_commit.rs` (reparent 実装、統合テスト) - - `src/cli-pr-monitor/src/stages/push_jj_bookmark.rs` (`resolve_pr_tip_commit_id` 追加) - - `src/cli-pr-monitor/src/stages/mod.rs` (`push_jj_bookmark` を `pub(crate)` 化、fix_commit.rs から参照するため) -- **参照**: - - ADR-022 原則 5 (PR 包含 changeset の不変性) — 本問題は原則 5 実装の副作用の一つ - - PR #63 (takt fix の child commit 分離) — 空 commit が出るようになった起点 - - PR #64 / PR #66 の実地記録 — 連続発生した事例 - --- ## スコープ外だが将来検討 diff --git a/package.json b/package.json index b79bc3cf..c92c28ff 100644 --- a/package.json +++ b/package.json @@ -19,6 +19,8 @@ "build:all": "pnpm build:hooks-session-start && pnpm build:hooks-pre-tool-validate && pnpm build:hooks-post-tool-linter && pnpm build:hooks-stop-quality && pnpm build:cli-push-runner && pnpm build:cli-pr-monitor && pnpm build:cli-merge-pipeline && pnpm build:check-ci-coderabbit && pnpm build:hooks-settings", "push": ".\\.claude\\cli-push-runner.exe && .\\.claude\\cli-pr-monitor.exe --monitor-only", "create-pr": ".\\.claude\\cli-pr-monitor.exe", + "observe-pr": ".\\.claude\\cli-pr-monitor.exe --observe", + "mark-notified": ".\\.claude\\cli-pr-monitor.exe --mark-notified", "merge-pr": ".\\.claude\\cli-merge-pipeline.exe", "check-ci": ".\\.claude\\check-ci-coderabbit.exe", "prepare-pr-body": "pwsh -NoProfile -File scripts/prepare-pr-body.ps1", diff --git a/src/cli-pr-monitor/src/main.rs b/src/cli-pr-monitor/src/main.rs index 22835902..2e9cc07f 100644 --- a/src/cli-pr-monitor/src/main.rs +++ b/src/cli-pr-monitor/src/main.rs @@ -14,9 +14,13 @@ //! --mark-notified: state file の notified フラグを true にする //! Claude が結果を処理した後に呼ばれる //! +//! --observe: state file をポーリングし、終端状態 (action != continue_monitoring) +//! で state 全文を stdout に出して exit する (todo.md task 2)。 +//! 主フロー (pnpm create-pr) と並行起動される読み取り専用の観測パス。 +//! //! 終了コード: //! 0 - 正常終了 -//! 1 - gh pr create 失敗 (PR 作成モードのみ) +//! 1 - gh pr create 失敗 (PR 作成モードのみ) / observer タイムアウト mod config; mod fix_commit; @@ -26,7 +30,7 @@ mod stages; mod state; mod util; -use stages::{run_create_pr, run_mark_notified, run_monitor_only}; +use stages::{run_create_pr, run_mark_notified, run_monitor_only, run_observe}; fn main() { let args: Vec = std::env::args().collect(); @@ -35,6 +39,10 @@ fn main() { std::process::exit(run_mark_notified()); } + if args.iter().any(|a| a == "--observe") { + std::process::exit(run_observe()); + } + if args.iter().any(|a| a == "--monitor-only") { std::process::exit(run_monitor_only()); } diff --git a/src/cli-pr-monitor/src/stages/mod.rs b/src/cli-pr-monitor/src/stages/mod.rs index 02417d66..081db829 100644 --- a/src/cli-pr-monitor/src/stages/mod.rs +++ b/src/cli-pr-monitor/src/stages/mod.rs @@ -2,6 +2,7 @@ pub(crate) mod collect; mod create_pr; mod mark_notified; mod monitor; +mod observe; pub(crate) mod poll; pub(crate) mod push; pub(crate) mod push_jj_bookmark; @@ -11,3 +12,4 @@ pub(crate) mod takt; pub(crate) use create_pr::run_create_pr; pub(crate) use mark_notified::run_mark_notified; pub(crate) use monitor::run_monitor_only; +pub(crate) use observe::run_observe; diff --git a/src/cli-pr-monitor/src/stages/monitor.rs b/src/cli-pr-monitor/src/stages/monitor.rs index 9991613d..b313c442 100644 --- a/src/cli-pr-monitor/src/stages/monitor.rs +++ b/src/cli-pr-monitor/src/stages/monitor.rs @@ -5,6 +5,7 @@ use crate::stages::collect::collect_findings; use crate::stages::poll::run_poll_loop; use crate::stages::repush::execute_repush_flow; use crate::stages::takt::run_takt; +use crate::state::{write_state, PrMonitorState}; use crate::util::{get_pr_info, utc_now_iso8601, PrInfo}; // ─── 監視開始 (sequential chain) ─── @@ -24,6 +25,18 @@ pub(crate) fn start_monitoring(pr_info: &PrInfo) -> i32 { log_info(&format!("{} の監視を開始", pr_label)); + // セッション開始時の state 初期化: notified=false にリセットして + // observer (todo.md task 2) が過去セッションの通知履歴を引きずらないようにする。 + // poll_loop 内では iteration を跨いで notified を preserve する。 + let init_state = PrMonitorState::new( + pr_info.pr_number, + pr_info.repo.clone(), + pr_info.push_time.clone().unwrap_or_default(), + ); + if let Err(e) = write_state(&init_state) { + log_info(&format!("[state] 初期化書き込み失敗 (継続): {}", e)); + } + // Stage 1: poll_loop (in-process, blocking) let poll_result = run_poll_loop(&config.monitor, pr_info); diff --git a/src/cli-pr-monitor/src/stages/observe.rs b/src/cli-pr-monitor/src/stages/observe.rs new file mode 100644 index 00000000..1c08f215 --- /dev/null +++ b/src/cli-pr-monitor/src/stages/observe.rs @@ -0,0 +1,163 @@ +//! observer mode: state file をポーリングして終端状態を検出したら +//! state 全文を stdout に出して exit する。 +//! +//! # 役割 (todo.md task 2) +//! +//! `pnpm create-pr` (主フロー) と並行して Claude Code が BG 起動する通知用パス。 +//! 主フローは 100% 機械的に detect → fix → re-push を完了させ、observer は +//! read-only に state file を観測するだけ。主フローの成否には影響しない。 +//! +//! # 終了条件 +//! +//! - `state.action != "continue_monitoring"` (= 終端状態: action_required / +//! stop_monitoring_success / stop_monitoring_failure / timed_out / error): +//! state の JSON を stdout に出力し exit 0 +//! - `state.notified == true`: Claude Code 再起動時の重複レポート防止のため、 +//! 何も出力せず exit 0 +//! - state file 不在: poll 継続 (主フローがまだ state を作っていない初期段階) +//! - `MAX_DURATION_SECS` 経過: exit 1 (主フローも別経路でタイムアウト監視するため +//! orphan OK) +use std::thread::sleep; +use std::time::{Duration, Instant}; + +use crate::log::log_info; +use crate::state::{read_state_from, state_file_path, PrMonitorState}; + +const POLL_INTERVAL_SECS: u64 = 5; +const MAX_DURATION_SECS: u64 = 600; + +#[derive(Debug, PartialEq, Eq)] +pub(crate) enum ObserveDecision { + /// 終端状態を検出した: state を stdout に出して exit + NotifyAndExit, + /// 既に notified=true: 何も出さず exit + SilentExit, + /// まだ継続監視中: poll 続行 + Continue, +} + +/// state の内容から observer の次の挙動を判定する pure function。 +/// +/// 優先順位: +/// 1. `notified == true` → 既通知なので `SilentExit` +/// (Claude Code 再起動時の重複レポート防止) +/// 2. `action != "continue_monitoring"` → 終端状態なので `NotifyAndExit` +/// 対象アクション: `action_required` / `stop_monitoring_success` / +/// `stop_monitoring_failure` / `timed_out` / `error` +/// 3. 上記以外 → `Continue` +pub(crate) fn decide(state: &PrMonitorState) -> ObserveDecision { + if state.notified { + return ObserveDecision::SilentExit; + } + if state.action != "continue_monitoring" { + return ObserveDecision::NotifyAndExit; + } + ObserveDecision::Continue +} + +fn emit_terminal_state(state: &PrMonitorState) -> i32 { + match serde_json::to_string_pretty(state) { + Ok(json) => { + println!("{}", json); + log_info(&format!( + "observer: 終端状態を検出 (action={}) — 通知して exit", + state.action + )); + 0 + } + Err(e) => { + log_info(&format!("state シリアライズ失敗: {}", e)); + 1 + } + } +} + +pub(crate) fn run_observe() -> i32 { + let state_path = state_file_path(); + let deadline = Instant::now() + Duration::from_secs(MAX_DURATION_SECS); + + log_info(&format!( + "observer 開始: state={}, interval={}s, timeout={}s", + state_path.display(), + POLL_INTERVAL_SECS, + MAX_DURATION_SECS + )); + + loop { + if let Some(state) = read_state_from(&state_path) { + match decide(&state) { + ObserveDecision::SilentExit => { + log_info("state.notified=true (既通知): 何も出力せず exit"); + return 0; + } + ObserveDecision::NotifyAndExit => return emit_terminal_state(&state), + ObserveDecision::Continue => {} + } + } + + if Instant::now() >= deadline { + log_info(&format!( + "observer タイムアウト ({}秒) — orphan exit", + MAX_DURATION_SECS + )); + return 1; + } + + sleep(Duration::from_secs(POLL_INTERVAL_SECS)); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn state_with(action: &str, notified: bool) -> PrMonitorState { + let mut s = PrMonitorState::new(Some(1), None, "2026-01-01T00:00:00Z".into()); + s.action = action.to_string(); + s.notified = notified; + s + } + + #[test] + fn decide_continue_when_still_monitoring() { + let s = state_with("continue_monitoring", false); + assert_eq!(decide(&s), ObserveDecision::Continue); + } + + #[test] + fn decide_notify_on_action_required() { + let s = state_with("action_required", false); + assert_eq!(decide(&s), ObserveDecision::NotifyAndExit); + } + + #[test] + fn decide_notify_on_stop_monitoring_success() { + let s = state_with("stop_monitoring_success", false); + assert_eq!(decide(&s), ObserveDecision::NotifyAndExit); + } + + #[test] + fn decide_notify_on_stop_monitoring_failure() { + let s = state_with("stop_monitoring_failure", false); + assert_eq!(decide(&s), ObserveDecision::NotifyAndExit); + } + + #[test] + fn decide_notify_on_timed_out() { + let s = state_with("timed_out", false); + assert_eq!(decide(&s), ObserveDecision::NotifyAndExit); + } + + #[test] + fn decide_silent_exit_takes_priority_over_terminal_action() { + // 既に通知済みなら、終端状態でも SilentExit を優先する + let s = state_with("action_required", true); + assert_eq!(decide(&s), ObserveDecision::SilentExit); + } + + #[test] + fn decide_silent_exit_when_notified_and_still_continuing() { + let s = state_with("continue_monitoring", true); + assert_eq!(decide(&s), ObserveDecision::SilentExit); + } +} diff --git a/src/cli-pr-monitor/src/stages/poll.rs b/src/cli-pr-monitor/src/stages/poll.rs index 7dc42bd5..4025faae 100644 --- a/src/cli-pr-monitor/src/stages/poll.rs +++ b/src/cli-pr-monitor/src/stages/poll.rs @@ -5,7 +5,8 @@ use crate::config::{MonitorConfig, DEFAULT_CHECK_TIMEOUT_SECS}; use crate::log::{log_info, truncate_safe}; use crate::runner::{checker_exe_path, run_cmd_direct}; use crate::state::{ - update_state_from_check_result, write_state, CiState, CodeRabbitState, PrMonitorState, + read_state, update_state_from_check_result, write_state, CiState, CodeRabbitState, + PrMonitorState, }; use crate::util::{utc_now_iso8601, PrInfo}; @@ -106,6 +107,13 @@ pub(crate) fn run_poll_loop(config: &MonitorConfig, pr_info: &PrInfo) -> PollRes ); update_state_from_check_result(&mut state, &result); + // `PrMonitorState::new` は毎回 notified=false で初期化するため、既存 state + // から runtime-updated な notified を読み戻す。新規セッションでは + // start_monitoring 冒頭で reset 済み。 + if let Some(existing) = read_state() { + state.notified = existing.notified; + } + // Skip handling: skipped なチェックを成功扱いにした後、action を再計算する if skip_ci { state.ci = Some(CiState { diff --git a/src/cli-pr-monitor/src/state.rs b/src/cli-pr-monitor/src/state.rs index 4f1f299b..34c340a0 100644 --- a/src/cli-pr-monitor/src/state.rs +++ b/src/cli-pr-monitor/src/state.rs @@ -85,7 +85,6 @@ pub(crate) fn write_state(state: &PrMonitorState) -> Result<(), String> { write_state_to(&state_file_path(), state) } -#[allow(dead_code)] pub(crate) fn read_state() -> Option { read_state_from(&state_file_path()) } From b37900730ef1a9cbac6baa2fa36ad751399b5b97 Mon Sep 17 00:00:00 2001 From: aloekun Date: Wed, 22 Apr 2026 22:23:23 +0900 Subject: [PATCH 2/3] fix(review): apply CodeRabbit fixes for #68 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolved findings: - [Major] src/cli-pr-monitor/src/stages/monitor.rs:38 observer 起動前に stale state を読ませない初期化位置にしてください。 --- src/cli-pr-monitor/src/stages/create_pr.rs | 9 +++++++++ src/cli-pr-monitor/src/stages/monitor.rs | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/cli-pr-monitor/src/stages/create_pr.rs b/src/cli-pr-monitor/src/stages/create_pr.rs index 4238b38c..bc5a76f3 100644 --- a/src/cli-pr-monitor/src/stages/create_pr.rs +++ b/src/cli-pr-monitor/src/stages/create_pr.rs @@ -4,6 +4,7 @@ use crate::config::DEFAULT_STEP_TIMEOUT_SECS; use crate::log::log_info; use crate::runner::{run_cmd_direct, run_gh_quiet}; use crate::stages::monitor::start_monitoring; +use crate::state::{write_state, PrMonitorState}; use crate::util::{ get_jj_bookmarks, get_pr_info, parse_pr_number_from_url, utc_now_iso8601, PrInfo, }; @@ -170,6 +171,14 @@ fn ensure_head_arg(args: Vec, bookmarks: &[String]) -> Vec { pub(crate) fn run_create_pr(gh_args: &[String]) -> i32 { log_info("PR 作成モード"); + // observer が前セッションの stale state を読まないよう、gh pr create 実行前に + // 早期 reset を書き込む。PR 番号・repo は未確定なので None で初期化する。 + // action="continue_monitoring" / notified=false により observer は Continue 判定になる。 + let early_state = PrMonitorState::new(None, None, utc_now_iso8601()); + if let Err(e) = write_state(&early_state) { + log_info(&format!("[state] 早期 reset 書き込み失敗 (継続): {}", e)); + } + // Windows の pnpm/cmd.exe 経由で --body の複数行テキストが分割される問題の対策 let reassembled = reassemble_split_body(gh_args); diff --git a/src/cli-pr-monitor/src/stages/monitor.rs b/src/cli-pr-monitor/src/stages/monitor.rs index b313c442..3c86939c 100644 --- a/src/cli-pr-monitor/src/stages/monitor.rs +++ b/src/cli-pr-monitor/src/stages/monitor.rs @@ -25,8 +25,8 @@ pub(crate) fn start_monitoring(pr_info: &PrInfo) -> i32 { log_info(&format!("{} の監視を開始", pr_label)); - // セッション開始時の state 初期化: notified=false にリセットして - // observer (todo.md task 2) が過去セッションの通知履歴を引きずらないようにする。 + // 早期 reset は run_create_pr 冒頭で実施済み (gh pr create 実行前)。 + // ここは run_monitor_only 経路および冪等化のための最終 reset。 // poll_loop 内では iteration を跨いで notified を preserve する。 let init_state = PrMonitorState::new( pr_info.pr_number, From f5ac40336297da1c9b27669a5c50b6417fdf6bef Mon Sep 17 00:00:00 2001 From: aloekun Date: Thu, 23 Apr 2026 01:22:03 +0900 Subject: [PATCH 3/3] =?UTF-8?q?refactor(cli-pr-monitor):=20started=5Fat=20?= =?UTF-8?q?=E3=81=AE=20fallback=20=E3=82=92=E7=8F=BE=E5=9C=A8=E6=99=82?= =?UTF-8?q?=E5=88=BB=E3=81=AB=E3=81=99=E3=82=8B=20(CodeRabbit=20nitpick)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 変更 - `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 --- src/cli-pr-monitor/src/stages/monitor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli-pr-monitor/src/stages/monitor.rs b/src/cli-pr-monitor/src/stages/monitor.rs index 3c86939c..0a4c71d1 100644 --- a/src/cli-pr-monitor/src/stages/monitor.rs +++ b/src/cli-pr-monitor/src/stages/monitor.rs @@ -31,7 +31,7 @@ pub(crate) fn start_monitoring(pr_info: &PrInfo) -> i32 { let init_state = PrMonitorState::new( pr_info.pr_number, pr_info.repo.clone(), - pr_info.push_time.clone().unwrap_or_default(), + pr_info.push_time.clone().unwrap_or_else(utc_now_iso8601), ); if let Err(e) = write_state(&init_state) { log_info(&format!("[state] 初期化書き込み失敗 (継続): {}", e));