Skip to content

feat(cli-pr-monitor): 重複起動 file lock + ポーリング間隔延長 (Phase 3 / 順位 12) + Bundle V 登録#96

Merged
aloekun merged 3 commits intomasterfrom
feat/cli-pr-monitor-lock-and-poll-extension
Apr 30, 2026
Merged

feat(cli-pr-monitor): 重複起動 file lock + ポーリング間隔延長 (Phase 3 / 順位 12) + Bundle V 登録#96
aloekun merged 3 commits intomasterfrom
feat/cli-pr-monitor-lock-and-poll-extension

Conversation

@aloekun
Copy link
Copy Markdown
Owner

@aloekun aloekun commented Apr 30, 2026

Summary

トークン削減ロードマップ Phase 3 (順位 12: cli-pr-monitor ポーリング延長 + 重複起動ロック) の本実装に加え、PR #95 post-merge-feedback 由来の Bundle V 登録 (順位 31-33) を同梱します。両者とも token-reduction 系で論理的近接、PR 数最小化方針 (MEMORY.md: feedback_minimize_pr_count_during_rate_limit) により同 PR で land。

Why

PR #88 で実証された rate-limit 浪費 (Claude Code Max を 1 時間で 40% 消費) への直接対策。複数セッションが同時に cli-pr-monitor の polling を回すと、Claude API への並走 polling で rate-limit を急速消費する事象が観測されていました。

本 PR で rate-limit 抑制の 3 層のうち 2 層を完成させます:

対策 状態
1. Claude 側 polling 禁止 (preventive) Polling anti-pattern 検出 hook (PR #86 T1-1) ✅ 完了済
2. tool 側ポーリング頻度削減 (corrective) 本 PR: 重複起動 file lock + 間隔延長 ✅ 本 PR で完成
3. review 単位の自動再トリガー post-pr-review に rate-limit 自動検出 + 再トリガー Tier 2 残

Scope (2 論理ユニット同梱)

Phase 3 本実装 (順位 12)

重複起動 file lock — 新 module src/cli-pr-monitor/src/lock.rs

設計要素 内容
取得方式 OpenOptions::create_new による atomic create-or-fail (TOCTOU race を排除)
Busy 判定 AlreadyExists 時のみ既存 lock の stale 判定にフォールバック
Stale 閾値 1800s (max_duration_secs 600s の 3x 安全マージン)
Stale 時挙動 overwrite で takeover (複数 takeover 並走しても last write wins、 cleanup は確実)
Cleanup RAII Drop guard (ADR-025 同型パターン) で正常終了時削除
Crash recovery Drop が走らなくても次インスタンスが stale 経由で takeover
Guard 範囲 start_monitoring のみ。--observe (read-only) と --mark-notified (one-shot) は対象外で並走可

ポーリング間隔延長 src/cli-pr-monitor/src/config.rs

  • DEFAULT_POLL_INTERVAL: 120s → 180s
  • 単独セッションの polling 回数を 5 → ~3 サイクル/監視に削減 (約 40% 削減)
  • max_duration_secs (600s) は維持 (検出窓は変えない)

ISO 8601 parser (lock.rs 内蔵)

chrono 依存を増やさず、手書き parser で start_time の age を計算。util::utc_now_iso8601() 出力との format alignment は test で round-trip 検証済 (advisor 指摘反映)。

Bundle V 登録 (PR #95 post-merge-feedback)

docs/todo.md table への 順位 31-33 追加 + docs/todo3.md に詳細 3 件追加。実装は別 PR で予定。

順位 内容
31 review-security.md docs-only fast-approve から .takt/**.claude/** を明示除外
32 docs/todo.md ヘッダの「新規タスクは追加しない」表記を実態整合
33 code-review.md に新 verdict 経路追加時の 3 点チェックリスト追記

Test plan

  • cargo test -p cli-pr-monitor: 106 passed (lock 9 件含む)
  • cargo fmt --check / cargo clippy --all-targets -- -D warnings: 全 clean
  • release ビルド成功 + .claude/cli-pr-monitor.exe 配備
  • takt pre-push-review: 3 iterations / 11m 26s / aggregate=approved
  • concurrent_acquire_only_one_wins: 8 thread 同時 acquire で 1 つだけが Acquired になることを検証 (advisor 指摘の TOCTOU race を真に検証する test)
  • advisor pre-push 検証で TOCTOU race 指摘を反映、OpenOptions::create_new に書き換え

Design notes

  • stale 閾値 1800s の根拠: max_duration 600s の 3x マージン。プロセス crash 後 30 分待てば takeover できる。長すぎる場合は pr-monitor-config.toml で将来 override 可能 (本 PR では非導入)。
  • PID liveness check は非導入: timestamp-based stale で十分カバー。Windows/Unix の platform-specific 実装を避け、依存追加もなし。30 min 待機が許容できないユースケース観測時に follow-up で検討。
  • fail-open ポリシー: lock I/O エラー時は監視を継続 (権限不足等で lock が機能しなくても polling 自体は走る)。

Phase

Token 削減 Phase 3 完了。次は Phase 4 (順位 13: post-pr-review に rate-limit 自動検出 + 再トリガー、rate-limit critical 系の最終 1 つ)。

Summary by CodeRabbit

  • バグ修正

    • 同一リポジトリでの重複起動を防ぐプロセスロックを導入しました。既存ホルダー情報を通知して重複時は正常終了、古いロックは上書きで復旧。ロック取得不能時はロック無しで継続します。
    • ポーリング間隔の既定値を120秒から180秒に調整しました。
  • ドキュメント

    • 優先度表と戦略記述を更新(Tier表の再編、"rate-limit 抑制の3層"モデルへの変更)と、複数ドキュメントの整合性修正・チェックリスト追加を行いました。

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 30, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 80a44c16-20d5-401f-96d2-99541e279eca

📥 Commits

Reviewing files that changed from the base of the PR and between 0a6b2cd and 66246c1.

📒 Files selected for processing (2)
  • src/cli-pr-monitor/src/lock.rs
  • src/cli-pr-monitor/src/stages/monitor.rs

📝 Walkthrough

Walkthrough

cli-pr-monitor のデフォルトポーリング間隔を120秒→180秒に延長し、プロセス重複を防ぐファイルベースのリポジトリロックを追加。関連ドキュメントの優先度表記を更新し、Tier 3 のドキュメント整合タスクを追加。

Changes

Cohort / File(s) Summary
Documentation Updates
docs/todo.md, docs/todo3.md
recommended-order/Tier 表を再構成して PR#88 完了を反映し、PR#95 による3件のTier 3 ドキュメント整合タスクを追加。
Polling Interval Configuration
src/cli-pr-monitor/src/config.rs
DEFAULT_POLL_INTERVAL120180 秒に変更し、doc comment を追加。
Repository Lock Implementation
src/cli-pr-monitor/src/lock.rs
.claude/pr-monitor.lock を用いるファイルベースのロック実装を追加(PID/UTC start_time/mode を TOML 書込、Acquired/Busy/Unavailable 列挙、RAII 解除、古いロックの上書きロジック、ISO8601 パーサとテスト群)。
Lock Integration
src/cli-pr-monitor/src/main.rs, src/cli-pr-monitor/src/stages/monitor.rs
mod lock; を導入し、start_monitoring がリポジトリロックを取得するよう変更。Busy の場合はホルダー情報をログして早期終了、Unavailable の場合はロック無しで継続、Acquired の場合はスコープ保持(_lock_guard)。

Sequence Diagram(s)

sequenceDiagram
    participant CLI as "CLI / main"
    participant Lock as "lock module"
    participant FS as "Filesystem (.claude/pr-monitor.lock)"
    participant Monitor as "start_monitoring"

    CLI->>Lock: acquire("start_monitoring")
    Lock->>FS: create_new() or read lock file
    alt created (Acquired)
        FS-->>Lock: wrote lock (PID, start_time, mode)
        Lock-->>CLI: Acquired(guard)
        CLI->>Monitor: proceed with monitoring (holds lock)
        Monitor-->>Lock: release on Drop at end
    else exists and fresh (Busy)
        FS-->>Lock: read holder PID + age
        Lock-->>CLI: Busy(holder_pid, age)
        CLI->>CLI: log holder and exit (code 0)
    else stale or corrupt (Overwrite / takeover)
        FS-->>Lock: overwrite with new lock
        Lock-->>CLI: Acquired(guard)
        CLI->>Monitor: proceed with monitoring (holds lock)
    else io error (Unavailable)
        FS-->>Lock: I/O error
        Lock-->>CLI: Unavailable(reason)
        CLI->>Monitor: proceed without lock
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PRタイトルは、重複起動ファイルロック機能、ポーリング間隔延長、およびBundle V登録という3つの主要な変更を正確に反映しており、変更セット全体の核となる部分を明確に要約しています。
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/lock.rs`:
- Around line 167-205: parse_iso8601 currently forwards parsed
month/day/hour/minute/second into days_from_epoch/unix_timestamp without
validating ranges, causing panics for malformed timestamps like
"2026-99-01T00:00:00Z"; update parse_iso8601 to validate that month is 1..=12,
hour is 0..=23, minute and second are 0..=59, and that day is within the valid
number of days for the parsed month (accounting for is_leap for February), and
return None if any check fails so callers treat invalid parse as stale instead
of panicking; keep days_from_epoch/unix_timestamp unchanged but rely on
parse_iso8601 to guarantee valid inputs.
- Around line 75-109: The code currently returns LockResult::Acquired even when
filesystem ops fail (create_dir_all, write_all, create_new, or takeover write),
which falsely signals a held lock; change this to return a distinct failure
state (e.g. add LockResult::Unavailable or LockResult::IoError) instead of
Acquired in all error branches where we couldn't create or write the lock (refer
to functions/builders: build_lock_content, read_fresh_lock, MonitorLock and the
match arms handling OpenOptions::new().create_new and std::fs::write); update
the match arms so any I/O error paths (create_dir_all failure, f.write_all
error, create_new non-AlreadyExists errors, and takeover write failure) return
the new LockResult variant, and then update the caller in monitor.rs to treat
that variant as "skip/explicit failure" rather than "Acquired."
🪄 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: 57f5b89e-8257-4e4b-8b8b-eecde6288f2e

📥 Commits

Reviewing files that changed from the base of the PR and between c299aa2 and 073de72.

📒 Files selected for processing (6)
  • docs/todo.md
  • docs/todo3.md
  • src/cli-pr-monitor/src/config.rs
  • src/cli-pr-monitor/src/lock.rs
  • src/cli-pr-monitor/src/main.rs
  • src/cli-pr-monitor/src/stages/monitor.rs

Comment thread src/cli-pr-monitor/src/lock.rs Outdated
Comment thread src/cli-pr-monitor/src/lock.rs
PR #88 で実証された rate-limit 浪費 (Claude Code Max を 1 時間で 40% 消費) への
直接対策。tool 側の polling 動作を 2 軸で改善し、Polling anti-pattern 検出 (PR
#86 T1-1, 完了済) と組み合わせて 3 層の rate-limit 抑制を成立させる。

## 順位 12 本実装

### 重複起動 file lock (新 module: src/cli-pr-monitor/src/lock.rs)

- `start_monitoring` の polling + takt 並走を `.claude/pr-monitor.lock` で
  1 アクティブ監視にゲート
- atomic create-or-fail (`OpenOptions::create_new`) で TOCTOU race を排除
- AlreadyExists 時のみ既存 lock の stale 判定にフォールバック
  - fresh (start_time が threshold 内): Busy 返却 → caller は no-op exit
  - stale (threshold 超過 / parse 不能): overwrite で takeover
- stale_threshold = 1800s (max_duration_secs 600s の 3x 安全マージン)
- Drop guard で正常終了時 cleanup、crash 時は次インスタンスが stale 経由で takeover
- guard 対象は `start_monitoring` のみ。`--observe` (read-only) と
  `--mark-notified` (one-shot mutation) は対象外で並走可能

### ポーリング間隔延長 (config.rs)

- DEFAULT_POLL_INTERVAL を 120s → 180s
- 単独セッションでも polling 回数を 5 → ~3 サイクル/監視に削減 (~40% 削減)
- max_duration_secs (600s) は維持

### ISO 8601 parser (lock.rs 内蔵)

- chrono 依存を増やさず手書き parse で start_time の age を計算
- うるう年・月日・時分秒を正確にハンドル (`is_leap` / `days_from_epoch` テスト済)
- util::utc_now_iso8601() の出力 format と round-trip することを test で検証

## テスト

- 9 件の lock test 追加 (cli-pr-monitor 全体 106 passed):
  - acquire / drop の基本動作
  - fresh lock が second acquire を block
  - stale lock の takeover (1980 timestamp で確実に stale)
  - corrupt lock (parse 不能) の takeover
  - **concurrent_acquire_only_one_wins**: 8 thread 同時 acquire で 1 つだけが
    Acquired になることを検証 (advisor 指摘の TOCTOU race を真に検証する test)
  - lock_format_matches_util_iso8601: util との format alignment 確認
  - parse_iso8601 / is_leap 単体動作

## docs/todo 系列の更新 (Bundle V 登録 + Phase 3 完了反映、同 PR 同梱)

ユーザー方針 (memory: feedback_minimize_pr_count_during_rate_limit) により、
直前 session の Bundle V 登録 (順位 31-33 の table 追加 + 詳細エントリ) と本
Phase 3 完了反映 (順位 12 削除 + narrative 整合化) を同 PR で land する。

- docs/todo.md: 順位 12 削除 + Bundle V 順位 31-33 追加 + narrative 三段構え
  (Polling anti-pattern + cli-pr-monitor lock + post-pr-review rate-limit) に更新
- docs/todo3.md: 順位 12 詳細エントリ削除 + Bundle V 詳細 3 件追加

## 期待効果

- 重複起動セッション間の polling 並走を確実に防止
- DEFAULT_POLL_INTERVAL 延長で単独セッションの polling 総回数を ~40% 削減
- rate-limit 抑制 3 層 (Claude 側 polling 禁止 + tool 側ポーリング頻度削減 +
  review 単位の自動再トリガー [Tier 2 残]) のうち 2 層を完成
aloekun added a commit that referenced this pull request Apr 30, 2026
Resolved findings:
- [Major] src/cli-pr-monitor/src/lock.rs:109 `Acquired` を「実際には lock できていない」ケースにも返しています。
- [Major] src/cli-pr-monitor/src/lock.rs:205 壊れた `start_time` で panic します。
@aloekun aloekun force-pushed the feat/cli-pr-monitor-lock-and-poll-extension branch from 073de72 to 0a6b2cd Compare April 30, 2026 04:23
Resolved findings:
- [Major] src/cli-pr-monitor/src/lock.rs:109 `Acquired` を「実際には lock できていない」ケースにも返しています。
- [Major] src/cli-pr-monitor/src/lock.rs:205 壊れた `start_time` で panic します。
@aloekun
Copy link
Copy Markdown
Owner Author

aloekun commented Apr 30, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 30, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@aloekun
Copy link
Copy Markdown
Owner Author

aloekun commented Apr 30, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 30, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@aloekun
Copy link
Copy Markdown
Owner Author

aloekun commented Apr 30, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 30, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
src/cli-pr-monitor/src/lock.rs (1)

90-110: ⚠️ Potential issue | 🟠 Major

書き込み失敗後も Acquired を返してしまっています。

Line 95 / Line 110 では、実際には lock file を正常に残せていないのに caller が「lock 保持中」と解釈します。空/部分書き込みの file は次回 parse 失敗で stale 扱いになるので、重複起動防止がここで抜けます。write_all / takeover write の失敗は Unavailable に寄せ、新規作成側は部分 file を削除して戻す方が安全です。

差分案
         Ok(mut f) => {
             if let Err(e) = f.write_all(content.as_bytes()) {
-                log_info(&format!("[lock] 新規 lock 書き込み失敗 (継続): {}", e));
+                log_info(&format!("[lock] 新規 lock 書き込み失敗: {}", e));
+                let _ = std::fs::remove_file(&path);
+                return LockResult::Unavailable {
+                    reason: e.to_string(),
+                };
             }
             LockResult::Acquired(MonitorLock { path })
         }
@@
             if let Err(e) = std::fs::write(&path, content) {
-                log_info(&format!("[lock] takeover 書き込み失敗 (継続): {}", e));
+                log_info(&format!("[lock] takeover 書き込み失敗: {}", e));
+                return LockResult::Unavailable {
+                    reason: e.to_string(),
+                };
             }
             LockResult::Acquired(MonitorLock { path })
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli-pr-monitor/src/lock.rs` around lines 90 - 110, The code returns
LockResult::Acquired even when writes fail (in the OpenOptions branch using
f.write_all and in the takeover branch using std::fs::write), which can leave a
corrupt/empty lock file; change the logic so that any write error causes the
function to return LockResult::Unavailable (or equivalent) instead of Acquired,
and for the new-create branch delete the partial file if write_all fails before
returning Unavailable; keep the existing fresh-check using
read_fresh_lock/stale_threshold_secs and only return MonitorLock with
LockResult::Acquired after a successful write.
🤖 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/lock.rs`:
- Around line 354-356: このテストは thread 内で matches!(acquire_at(p, "concurrent",
1800), LockResult::Acquired(_)) を直接評価しているため取得した MonitorLock が即時 drop されて flaky
です。修正するには各 thread が acquire_at の戻り値(LockResult)を保持してから全 thread を join し、その後で各
LockResult を調べて exactly one が Acquired であることをアサートしてください; 該当する識別子は acquire_at,
LockResult::Acquired, MonitorLock, handles, thread::spawn,
b.wait()(および同様のブロックが存在する 359–369 範囲)です。
- Around line 163-170: The current parse_age_secs function uses
now.saturating_sub(then) which produces age=0 for future timestamps and prevents
treating future/broken timestamps as stale; modify parse_age_secs (and its use
of parse_iso8601) to detect if then > now and return None in that case,
otherwise return Some(now - then) as i64—ensure you compare the same integer
types (after converting SystemTime to i64 seconds) and avoid saturating_sub so
future timestamps are treated as parse failures/stale.

---

Duplicate comments:
In `@src/cli-pr-monitor/src/lock.rs`:
- Around line 90-110: The code returns LockResult::Acquired even when writes
fail (in the OpenOptions branch using f.write_all and in the takeover branch
using std::fs::write), which can leave a corrupt/empty lock file; change the
logic so that any write error causes the function to return
LockResult::Unavailable (or equivalent) instead of Acquired, and for the
new-create branch delete the partial file if write_all fails before returning
Unavailable; keep the existing fresh-check using
read_fresh_lock/stale_threshold_secs and only return MonitorLock with
LockResult::Acquired after a successful write.
🪄 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: 23020d50-2f44-416f-9f41-79d2abd5b487

📥 Commits

Reviewing files that changed from the base of the PR and between 073de72 and 0a6b2cd.

📒 Files selected for processing (6)
  • docs/todo.md
  • docs/todo3.md
  • src/cli-pr-monitor/src/config.rs
  • src/cli-pr-monitor/src/lock.rs
  • src/cli-pr-monitor/src/main.rs
  • src/cli-pr-monitor/src/stages/monitor.rs
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/cli-pr-monitor/src/main.rs
  • src/cli-pr-monitor/src/stages/monitor.rs
  • docs/todo.md

Comment thread src/cli-pr-monitor/src/lock.rs Outdated
Comment thread src/cli-pr-monitor/src/lock.rs Outdated
Finding D (Major, parse_age_secs): 未来日時 (時計巻き戻し / 破損 future timestamp)
が saturating_sub で age=0 となり永続 fresh 扱いになる bug を修正。明示的に
then > now を None (stale 扱い) で返し、crash recovery が機能するようにする。
test future_timestamp_lock_is_taken_over を追加。

Finding E (Minor, concurrent test flakiness): 1 barrier 構成だと先行 thread の
guard が判定後に即 drop されて後続 thread が逐次 Acquired する race window が
存在した。start barrier + finish barrier の 2 barrier 構成にし、全 thread が
判定終了まで Acquired guard を保持するよう修正。

両者とも CR の suggestion 準拠。lock test 全 13 件 pass、clippy clean。
@aloekun aloekun force-pushed the feat/cli-pr-monitor-lock-and-poll-extension branch from 0a6b2cd to 66246c1 Compare April 30, 2026 05:21
@aloekun
Copy link
Copy Markdown
Owner Author

aloekun commented Apr 30, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 30, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@aloekun aloekun merged commit 5f5c79b into master Apr 30, 2026
1 check passed
@aloekun aloekun deleted the feat/cli-pr-monitor-lock-and-poll-extension branch April 30, 2026 08:54
aloekun added a commit that referenced this pull request Apr 30, 2026
cli-pr-monitor の重複起動防止 lock file (PR #88 / #96 で導入) が
.gitignore 漏れで PR snapshot に混入する事故を防ぐ。
aloekun added a commit that referenced this pull request Apr 30, 2026
CR review @ commit e7b8d49 で検出された 3 Major + 1 Nitpick を修正。

## 修正内容

### Finding 1 (Major): parse_rate_limit に push_time フィルタ追加
過去セッションの rate-limit comment を新セッション開始時に誤って拾うバグを修正。
`comment.created_at >= push_time` フィルタを追加し、parse_new_comments / parse_findings
と同じ規則に揃える。これにより本 PR 検証中に観測された 22 分遅延の根因の一つが解消される。

### Finding 2 (Major): parse_new_comments で rate-limit comment 除外
rate-limit comment が new_comments にカウントされると `decide()` が action_required
を早期 return し、poll.rs の rate-limit retry 経路に入らずに監視終了するバグ。
"Rate limit exceeded" を含む comment を新規コメント集計から除外する。22 分遅延のもう
一つの根因。

### Finding 3 (Major): write_state 失敗時の自動 retry 停止
retrigger 後の state 永続化に失敗すると、次 iteration で `rate_limit_retries` と
`rate_limit_last_retriggered_at` の復元に失敗し、dedup と max_retries が壊れて
`@coderabbitai review` 重複投稿の可能性がある。失敗時は action_required で抜ける。

### Finding 4 (Nitpick): docs/todo.md 順位 絶対参照削除
ADR-033 の「絶対番号は table のみに保持」原則に従い、Bundle W/X 説明文から
`(順位 34)` `(順位 35)` 等の絶対参照を削除しタスク名参照に置換。

### .gitignore: pr-monitor.lock 除外
cli-pr-monitor の重複起動防止 lock file (PR #88 / #96 で導入) が .gitignore
漏れで PR snapshot に混入する事故を防ぐ。

## テスト

- check-ci-coderabbit: 59 tests pass (rate_limit_filters_out_past_session_comments,
  rate_limit_includes_comment_at_exact_push_time, comments_excludes_rate_limit を追加)
- cli-pr-monitor: 118 tests pass
- clippy + fmt clean (変更パッケージ)
aloekun added a commit that referenced this pull request Apr 30, 2026
CR review @ commit 79b7c3d で検出された 3 Major + 1 Minor のうち
Finding 1 (updated_at) を除く 3 件を修正。

## 修正内容

### Finding 2 (Major): rate-limit sleep を max_duration で cap
`handle_rate_limit_retry` に `remaining_monitor_secs` 引数を追加し、
sleep が監視残り予算を超える場合は Err を返して retry を停止する。
これまでは max_duration を素通りして 30 分超ブロックする可能性があった。

### Finding 3 (Major): handle_rate_limit_retry を Result 化
gh pr comment 投稿失敗 / PR 番号未確定の場合に retries++ や
last_retriggered_at 更新を実施しないよう、関数を Result<(), String> に
変更。caller は Ok 時のみ dedup key を更新し、Err 時は action_required
で抜ける。これにより失敗 retrigger が dedup で perma-skip 化する
silent failure を防ぐ。

### Finding 4 (Minor): docs/todo.md 第2層 PR 参照に #96 追記
"(PR #88 T2-4、完了済)" → "(PR #88 T2-4 / #96、完了済)" に履歴整合。

## テスト追加 (2 件)

- rate_limit_retry_returns_err_when_sleep_exceeds_budget:
  remaining=60s だが sleep=600s 必要なケースで Err を返し state 不変を確認
- rate_limit_retry_returns_err_when_pr_number_missing:
  PR 番号未確定で Err を返し state 不変を確認

## 未対応 (任意)

Finding 1 (updated_at): CR が rate-limit comment を編集する挙動は実観測なし。
将来的な堅牢性向上のため todo 化候補。
aloekun added a commit that referenced this pull request Apr 30, 2026
* feat(cli-pr-monitor): rate-limit 自動検出 + 再トリガーロジック (Phase 4 / 順位 13)

PR #89 T2-1 の自動化。CodeRabbit が `Rate limit exceeded` コメントを投稿した
場合、reset 時刻 + 60s buffer まで sleep し `@coderabbitai review` を再投稿する。

## 実装

- check-ci-coderabbit: rate-limit comment を検出し reset 時刻を計算する
  `parse_rate_limit` を追加。`Please wait N minutes M seconds` の正規表現抽出 +
  ISO 8601 → unix epoch 秒の手動パース (chrono 依存追加なし)。

- cli-pr-monitor: poll loop に rate-limit retry ブランチを追加
  - `RateLimitConfig`: auto_retry_enabled (default true) + max_retries (default 3)
  - `state.rate_limit_retries`: 累積 retry 回数を state.json で persist
  - `state.rate_limit_last_retriggered_at`: dedup key (advisor 指摘対応)
  - `handle_rate_limit_retry`: sleep → gh pr comment 投稿 → counter++

## dedup の必要性 (advisor finding)

`comment_created_at` で dedup しないと、同一 rate-limit comment が iteration を
跨いで PR コメント一覧に残り、`(until_unix_secs - now).max(0) = 0` となって
即時 retrigger を繰り返す。結果 max_retries=3 が数秒で消費され誤って
`action_required` で抜ける。`rate_limit_last_retriggered_at` で同じ created_at
を skip し、CR が新しい rate-limit comment を投稿した時点で再度対象にする。

## テスト

- check-ci-coderabbit: rate-limit detection / parsing 11 tests 追加 (56 passed)
- cli-pr-monitor: poll dedup + state persistence 4 tests 追加 (116 passed)

## 完了タスク

- 順位 13 を docs/todo.md table から削除、todo3.md detail 削除
- 順位 19 (REJECT-ESCALATE) が rate-limit critical 系の最後の Tier 2 残

* fix(cli-pr-monitor): CodeRabbit 指摘 (PR #97) 反映 + 誤混入 lock 削除

- handle_rate_limit_retry の log format 修正: retry={}/{} の 2 番目に
  rl.until_unix_secs (unix 秒) が渡されていたバグ。max_retries に修正。
  以前は "retry=1/1735689600" のような意味不明な出力になっていた。
- RateLimitConfig の parse test 追加 (defaults + custom)。既存
  config_fix_defaults / config_fix_custom と同パターンで網羅。
- 前 commit で誤って snapshot された .claude/pr-monitor.lock を削除
  (.gitignore 追加は後続 commit で実施)。

* fix(cli-pr-monitor): CodeRabbit round 2 指摘 (PR #97) 反映 + .gitignore 整備

CR review @ commit e7b8d49 で検出された 3 Major + 1 Nitpick を修正。

## 修正内容

### Finding 1 (Major): parse_rate_limit に push_time フィルタ追加
過去セッションの rate-limit comment を新セッション開始時に誤って拾うバグを修正。
`comment.created_at >= push_time` フィルタを追加し、parse_new_comments / parse_findings
と同じ規則に揃える。これにより本 PR 検証中に観測された 22 分遅延の根因の一つが解消される。

### Finding 2 (Major): parse_new_comments で rate-limit comment 除外
rate-limit comment が new_comments にカウントされると `decide()` が action_required
を早期 return し、poll.rs の rate-limit retry 経路に入らずに監視終了するバグ。
"Rate limit exceeded" を含む comment を新規コメント集計から除外する。22 分遅延のもう
一つの根因。

### Finding 3 (Major): write_state 失敗時の自動 retry 停止
retrigger 後の state 永続化に失敗すると、次 iteration で `rate_limit_retries` と
`rate_limit_last_retriggered_at` の復元に失敗し、dedup と max_retries が壊れて
`@coderabbitai review` 重複投稿の可能性がある。失敗時は action_required で抜ける。

### Finding 4 (Nitpick): docs/todo.md 順位 絶対参照削除
ADR-033 の「絶対番号は table のみに保持」原則に従い、Bundle W/X 説明文から
`(順位 34)` `(順位 35)` 等の絶対参照を削除しタスク名参照に置換。

### .gitignore: pr-monitor.lock 除外
cli-pr-monitor の重複起動防止 lock file (PR #88 / #96 で導入) が .gitignore
漏れで PR snapshot に混入する事故を防ぐ。

## テスト

- check-ci-coderabbit: 59 tests pass (rate_limit_filters_out_past_session_comments,
  rate_limit_includes_comment_at_exact_push_time, comments_excludes_rate_limit を追加)
- cli-pr-monitor: 118 tests pass
- clippy + fmt clean (変更パッケージ)

* fix(cli-pr-monitor): CodeRabbit round 3 指摘 (PR #97) 反映

CR review @ commit 79b7c3d で検出された 3 Major + 1 Minor のうち
Finding 1 (updated_at) を除く 3 件を修正。

## 修正内容

### Finding 2 (Major): rate-limit sleep を max_duration で cap
`handle_rate_limit_retry` に `remaining_monitor_secs` 引数を追加し、
sleep が監視残り予算を超える場合は Err を返して retry を停止する。
これまでは max_duration を素通りして 30 分超ブロックする可能性があった。

### Finding 3 (Major): handle_rate_limit_retry を Result 化
gh pr comment 投稿失敗 / PR 番号未確定の場合に retries++ や
last_retriggered_at 更新を実施しないよう、関数を Result<(), String> に
変更。caller は Ok 時のみ dedup key を更新し、Err 時は action_required
で抜ける。これにより失敗 retrigger が dedup で perma-skip 化する
silent failure を防ぐ。

### Finding 4 (Minor): docs/todo.md 第2層 PR 参照に #96 追記
"(PR #88 T2-4、完了済)" → "(PR #88 T2-4 / #96、完了済)" に履歴整合。

## テスト追加 (2 件)

- rate_limit_retry_returns_err_when_sleep_exceeds_budget:
  remaining=60s だが sleep=600s 必要なケースで Err を返し state 不変を確認
- rate_limit_retry_returns_err_when_pr_number_missing:
  PR 番号未確定で Err を返し state 不変を確認

## 未対応 (任意)

Finding 1 (updated_at): CR が rate-limit comment を編集する挙動は実観測なし。
将来的な堅牢性向上のため todo 化候補。

* fix(check-ci-coderabbit): rate-limit 計算を updated_at 基準に変更 (PR #97 round 3 Finding 1 実観測対応)

CR が rate-limit comment を編集して wait 時間を更新するケースが本 PR の dogfood で
実観測された (created_at=2026-04-30T11:11:51Z, updated_at=2026-04-30T14:38:32Z で
"wait 21 minutes" に編集)。created_at 基準だと reset 時刻を 3 時間以上前として誤算定し、
premature retrigger → CR 再 rate-limit → comment edit のループに陥る危険があった。

## 修正内容

- GhComment に updated_at フィールドを追加
- rate_limit_event_time(): updated_at fallback created_at を返すヘルパー追加
- parse_rate_limit: 計算基準を event_time に変更 (フィルタ / sort / until 計算)
- RateLimitInfo.comment_created_at: 値の意味を「event_time」に拡張 (フィールド名は維持)
- state.rs RateLimitState.comment_created_at の doc 更新

## テスト追加 (3 件)

- rate_limit_uses_updated_at_when_present:
  実観測ケース (created_at != updated_at) で updated_at 基準計算を確認
- rate_limit_falls_back_to_created_at_when_updated_at_missing:
  既存挙動の後方互換性を確認
- rate_limit_edited_comment_yields_new_dedup_key:
  編集前後で dedup key が変化することを確認 (新 wait 時間で再 trigger 可能)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant