Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions docs/harness-improvement-plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ Anthropic 公式のハーネスエンジニアリング指針(決定論的基
| WP-12 | 2 | 発火テレメトリ + ハーネス ROI 棚卸し | M | なし | 実装済(step1 収集層のみ: ADR-055 + lib-telemetry + 6 hook 計装。step2-3〔集計 pre-step / 卒業判定機械化〕は 28 日 warm-up 後着手のため todo 順位 307/308 へ移管) |
| WP-13 | 3 | EXE_SUFFIX 抽象化 | M | なし | 実装済(build/実行 scripts を deploy-artifacts.mjs / run-artifact.mjs 経由に、settings を `/` 区切り + `{{EXE_SUFFIX}}` 化、Rust の機能的 exe 解決を EXE_SUFFIX 化。ADR-005 amendment。cargo test 全 pass・build:all/deploy:hooks/lint:docs 退行なし実測。config TOML の cmd.exe 依存は WP-15 へ。`完了` は初回 push/PR で launcher 経路の実走確認後) |
| WP-14 | 3 | PowerShell 3 本の Rust 化 | S-M ×2 | なし | 実装済(3 本すべて Rust 化: fix-metrics-check→comment-lint `--fix-metrics-check` / prepare-pr-body→cli-pr-monitor サブコマンド / analyze-takt-timings→新規 cli-takt-timings crate。cargo test カバレッジ下・実データで旧 ps1 と出力一致確認。`完了` は初回 push/PR で fix step metrics-check と prepare-pr-body 経路の実走確認後) |
| WP-15 | 3 | Linux バイナリビルド + クラウド setup script | M | WP-13, 14 | 実装済(release-binaries.yml〔master push → rolling `nightly` prerelease に単一 tarball〕+ scripts/cloud-setup.sh 新設。前提として Linux 実行時に壊れる可搬性欠陥を修正: `cmd /c` 決め打ちの唯一の shell spawn 点を `shell_command`〔Windows=cmd /c / 他=sh -c〕へ集約、taskkill のみだった timeout kill に unix 分岐、cmd.exe 構文テストの OS 中立化、config の `.exe`/backslash 依存を `{{CLAUDE_DIR}}`/`{{EXE_SUFFIX}}` 展開へ。**WSL Ubuntu 24.04 で実測**: cargo test --workspace 全 pass・ignored 含め全 pass・clippy clean・hooks 実発火・push pipeline が sh -c 経路で完走。Linux 実測により lock の同時取得レース〔8 中 6 取得〕も発見・修正。`完了` release 実生成 + 実クラウドセッションでの cloud-setup.sh 実走確認後) |
| WP-15 | 3 | Linux バイナリビルド + クラウド setup script | M | WP-13, 14 | 実装済(release-binaries.yml〔master push → rolling `nightly` prerelease に単一 tarball〕+ scripts/cloud-setup.sh 新設。前提として Linux 実行時に壊れる可搬性欠陥を修正: `cmd /c` 決め打ちの唯一の shell spawn 点を `shell_command`〔Windows=cmd /c / 他=sh -c〕へ集約、taskkill のみだった timeout kill に unix 分岐、cmd.exe 構文テストの OS 中立化、config の `.exe`/backslash 依存を `{{CLAUDE_DIR}}`/`{{EXE_SUFFIX}}` 展開へ。**WSL Ubuntu 24.04 で実測**: cargo test --workspace 全 pass・ignored 含め全 pass・clippy clean・hooks 実発火・push pipeline が sh -c 経路で完走。Linux 実測により lock の同時取得レース〔8 中 6 取得〕も発見・修正。**`完了` 条件 (1) 達成 (2026-07-20)**: PR #307 / #308 マージ後に release-binaries.yml が実走し `nightly` prerelease を生成。公開された実物の artifact を認証なし curl で取得 → checksum 一致 → 展開 → **release バイナリそのもので Linux 上の hooks 実発火**まで確認。残るは実クラウドセッションでの cloud-setup.sh 実走のみ) |
| WP-16 | 3 | CI matrix(移植退行防止) | S | WP-13, 14 | 未着手 |
| WP-17 | 4 | イベント駆動バックボーン完成(Phase B + routines 移行) | M | WP-09, 10, 11 | 未着手 |
| WP-18 | 4 | 夜間 todo 消化ループ | M-L | WP-15, 17 | 未着手 |
Expand Down Expand Up @@ -267,7 +267,11 @@ Anthropic 公式のハーネスエンジニアリング指針(決定論的基
>
> **Linux 実測で発見した副次不具合**: `cli-pr-monitor` の lock が**同時取得**を許していた (8 スレッド中 6 つが取得)。`create_new` は atomic だが直後のファイルは空で、その窓を読んだ側が TOML parse 失敗を一律「stale」と扱って全員 takeover していた。Windows ではスケジューリング差で顕在化していなかっただけで欠陥は同じ。parse 失敗を内容で 2 分 (空 = 書き込み中 → busy / 非空の不正 = 破損 → takeover) して修正。**「Windows だけで回していると気付けない設計欠陥が実在した」= Linux 実測と WP-16 (CI matrix) の価値を裏づける実例。**
>
> **`完了` 条件**: (1) 本変更が master に入り release-binaries.yml が実走して `nightly` release が生成されること、(2) 実際の claude.ai/code セッションで `cloud-setup.sh` を走らせ hooks 発火と `cargo test` 通過を確認すること。いずれも本セッションでは実施不能 (release 未生成 / クラウド環境未使用) のため `実装済` に留める。**Linux 側の未検証領域**: `#[cfg(windows)]` ガードのテスト (pump_child_io の deadlock 保護、run_cmd_capture の stdout/stderr 分離) は Linux で skip されるため、WP-16 の CI matrix で扱う。以下は当初ステップ (記録用)。
> **`完了` 条件 (1) 達成 (2026-07-20、PR #307 / #308 マージ後)**: release-binaries.yml が master で実走し `nightly` prerelease を生成 (tarball 9.72 MB + sha256、commit `541adde1`)。**公開された実物の artifact で end-to-end 検証済み**: 認証なしの素の curl で取得 → checksum 一致 → 17 ファイル展開 (バイナリ 16 + BUILD_INFO) → `file` が `ELF 64-bit LSB pie executable, x86-64 ... stripped` を確認 → **release バイナリそのもので Linux 上の hooks が実発火** (PreToolUse が危険コマンドを exit 2 でブロックし通常コマンドは exit 0、SessionStart が additionalContext JSON を出力、tree-sitter の comment-lint が違反検出)。これにより「public release は gh 認証不要」という設計判断も実証された。
>
> **初回実行は失敗した (PR #308 で修正)**: #307 マージ直後の初回 run は `hooks-stop-tool-call-leak` の E2E が Broken pipe で落ちて赤になった。kill-switch 経路の hook が stdin を読まずに即 return するため、親の `write_all` が Unix で EPIPE になる競合で、Windows では payload がパイプバッファに収まり顕在化していなかった。**WSL では修正前でも 60/60 pass してしまい再現できず、ubuntu-22.04 の CI が初めて捕捉した** (遅延注入で EPIPE を強制して修正前 FAIL / 修正後 PASS を実測)。**WSL 検証はスケジューリング依存の race に対しては CI の代替にならない**という限定的な教訓であり、WP-16 (CI matrix) の必要性を裏づける実例。
>
> **残る `完了` 条件 (2)**: 実際の claude.ai/code セッションで `cloud-setup.sh` を走らせ hooks 発火と `cargo test` 通過を確認すること (クラウド環境未使用のため本セッションでは実施不能)。**Linux 側の未検証領域**: `#[cfg(windows)]` ガードのテスト (pump_child_io の deadlock 保護、run_cmd_capture の stdout/stderr 分離) は Linux で skip されるため、WP-16 の CI matrix で扱う。以下は当初ステップ (記録用)。

- **目的**: 使い捨てのクラウドセッションで 19 crate をビルドせずにハーネスを即時有効化する。
- **ステップ**:
Expand Down
7 changes: 7 additions & 0 deletions src/check-ci-coderabbit/src/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ pub(crate) struct RateLimitInfo {
pub(crate) comment_event_time: String,
pub(crate) wait_minutes: u64,
pub(crate) wait_seconds: u64,
/// 待ち時間を comment 本文から**実際に読み取れたか**。
///
/// `false` = rate-limit comment とは判定できたが、CR の文面が既知のどの書式にも
/// 一致せず既定値で代替したことを意味する (CR の書式変更を検知した状態)。
/// 下流はこれを見て「wakeup 時刻は当てにならない」と扱い、書式追加が必要な事実を
/// 可視化する。ADR-034 § CR rate-limit format evolution 参照。
pub(crate) wait_time_parsed: bool,
}

#[derive(Serialize, Default)]
Expand Down
149 changes: 139 additions & 10 deletions src/check-ci-coderabbit/src/rate_limit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,24 @@
use crate::markers::{is_rate_limit_comment, rate_limit_event_time};
use crate::models::{GhComment, RateLimitInfo};

/// CodeRabbit rate-limit comment を検出し、reset 時刻 (unix epoch) を返す。
pub(crate) fn parse_rate_limit(json: &str, push_time: &str) -> Option<RateLimitInfo> {
let comments: Vec<GhComment> = serde_json::from_str(json).ok()?;

/// 待ち時間を読み取れなかった rate-limit comment に充てる既定の待ち分数。
///
/// CR が書式を変えても「レート制限は起きている」という事実は marker で判っている。
/// ここで `None` を返して**制限なし扱いにするのが最悪の失敗**なので (実観測: 監視が
/// 「CodeRabbit 指摘なし」と誤報告して success 判定した)、既定値で park させる。
///
/// 短すぎても実害は小さい: wakeup 後に再 poll し、まだ制限中なら再度 park する
/// (self-correcting)。過去の実観測値は 5 / 15 / 21 / 30 / 36 分。
const FALLBACK_WAIT_MINUTES: u64 = 15;

/// push 以降に CR が投稿した rate-limit comment のうち**最も新しいもの**を返す。
///
/// 新しい順で選ぶのは、CR が同一 comment を編集して待ち時間を更新するため
/// (`rate_limit_event_time` が `updated_at` を優先する理由と対)。
fn latest_rate_limit_comment<'a>(
comments: &'a [GhComment],
push_time: &str,
) -> Option<&'a GhComment> {
let mut candidates: Vec<&GhComment> = comments
.iter()
.filter(|c| {
Expand All @@ -28,20 +42,44 @@ pub(crate) fn parse_rate_limit(json: &str, push_time: &str) -> Option<RateLimitI
.unwrap_or("")
.cmp(rate_limit_event_time(a).unwrap_or(""))
});
let latest = candidates.first()?;
candidates.first().copied()
}

/// CodeRabbit rate-limit comment を検出し、reset 時刻 (unix epoch) を返す。
///
/// **marker が一致したら必ず `Some` を返す**。待ち時間のパースに失敗しても
/// `FALLBACK_WAIT_MINUTES` で代替し `wait_time_parsed: false` を立てる。
/// 「rate-limit と判っているのに待ち時間が読めないから制限なしとして扱う」のは
/// fail-open であり、CR の書式変更のたびに silent regression を生む (ADR-034)。
pub(crate) fn parse_rate_limit(json: &str, push_time: &str) -> Option<RateLimitInfo> {
let comments: Vec<GhComment> = serde_json::from_str(json).ok()?;
let latest = latest_rate_limit_comment(&comments, push_time)?;

let body = latest.body.as_deref()?;
let event_time = rate_limit_event_time(latest)?;
let (minutes, seconds) = extract_wait_time(body)?;
let comment_unix = parse_iso8601_to_unix(event_time)?;

let (minutes, seconds, wait_time_parsed) = match extract_wait_time(body) {
Some((m, s)) => (m, s, true),
None => {
eprintln!(
"[check-ci] Warning: rate-limit comment を検出しましたが待ち時間の書式が未知です。\
既定値 {}分で park します。CR が書式を変更した可能性があるため \
extract_wait_time に新書式の追加が必要です (ADR-034)。",
FALLBACK_WAIT_MINUTES
);
(FALLBACK_WAIT_MINUTES, 0, false)
}
};

let until_unix_secs = comment_unix + (minutes as i64) * 60 + (seconds as i64) + 60;

Some(RateLimitInfo {
until_unix_secs,
comment_event_time: event_time.to_string(),
wait_minutes: minutes,
wait_seconds: seconds,
wait_time_parsed,
})
}

Expand Down Expand Up @@ -76,9 +114,36 @@ pub(crate) fn extract_new_format_wait_time(body: &str) -> Option<(u64, u64)> {
Some((m, 0))
}

/// 旧 / 新どちらかの format に一致すれば `(minutes, seconds)` を返す。旧 → 新の順で試行。
/// 現行 format (`**Next review available in:** **N minutes**`) を抽出。
/// PR #307 (2026-07-20) で実観測した CR の 3 番目の書式。
///
/// markdown の強調 (`**`) が `in:` の直後と数値の直前の両方に入るため、
/// `\**` と `\s*` で任意個の `*` と空白を吸収する。
pub(crate) fn extract_current_format_wait_time(body: &str) -> Option<(u64, u64)> {
let re_full = regex::Regex::new(
r"Next review available in:\**\s*\**(\d+) minutes? and \**(\d+) seconds?",
)
.ok()?;
if let Some(caps) = re_full.captures(body) {
let m: u64 = caps.get(1)?.as_str().parse().ok()?;
let s: u64 = caps.get(2)?.as_str().parse().ok()?;
return Some((m, s));
}
let re_min = regex::Regex::new(r"Next review available in:\**\s*\**(\d+) minutes?").ok()?;
let caps = re_min.captures(body)?;
let m: u64 = caps.get(1)?.as_str().parse().ok()?;
Some((m, 0))
}

/// 既知の 3 書式のいずれかに一致すれば `(minutes, seconds)` を返す。旧 → 新 → 現行の順で試行。
///
/// **書式を足すときは必ず実観測した本文で test を書くこと**。ここに落ちると
/// `parse_rate_limit` が既定値へフォールバックし、wakeup 時刻が実際の reset と
/// ズレる (制限なし扱いにはならないが、無駄な poll が増える)。
pub(crate) fn extract_wait_time(body: &str) -> Option<(u64, u64)> {
extract_old_format_wait_time(body).or_else(|| extract_new_format_wait_time(body))
extract_old_format_wait_time(body)
.or_else(|| extract_new_format_wait_time(body))
.or_else(|| extract_current_format_wait_time(body))
}

/// ISO 8601 (`YYYY-MM-DDTHH:MM:SSZ` 形式) を unix epoch 秒に変換する。
Expand Down Expand Up @@ -239,14 +304,21 @@ mod tests {
assert!(parse_rate_limit(json, "2026-04-29T00:00:00Z").is_none());
}

/// **契約変更 (PR #307 incident、2026-07-20)**: 旧実装はこのケースで `None`
/// (= rate-limit なし) を返しており、本 test はその挙動を固定していた。
/// しかしそれこそが「CR が書式を変えると監視が誤って success を返す」原因だった
/// ため、marker 一致時は既定値付きで必ず `Some` を返す契約へ改めた。
#[test]
fn rate_limit_no_match_when_no_wait_time() {
fn rate_limit_reported_with_fallback_when_wait_time_format_is_unknown() {
let json = r#"[{
"user": {"login": "coderabbitai[bot]"},
"body": "Rate limit exceeded but format is unusual",
"created_at": "2026-04-30T00:00:00Z"
}]"#;
assert!(parse_rate_limit(json, "2026-04-29T00:00:00Z").is_none());
let result = parse_rate_limit(json, "2026-04-29T00:00:00Z")
.expect("marker 一致なら待ち時間が読めなくても rate-limit として報告する");
assert!(!result.wait_time_parsed);
assert_eq!(result.wait_minutes, FALLBACK_WAIT_MINUTES);
}

#[test]
Expand Down Expand Up @@ -351,6 +423,63 @@ mod tests {
assert_eq!(result.wait_seconds, 0);
}

/// PR #307 (2026-07-20) の incident 再現 (bad): CR が 3 番目の書式に変えたため
/// `extract_wait_time` が None になり、`parse_rate_limit` が **rate-limit なし**を
/// 返していた。結果、監視が「CodeRabbit 指摘なし」= success と誤報告した。
///
/// body は実際に PR #307 へ投稿された comment から採取したもの (ADR-049 の流儀:
/// incident 由来の fixture は実データを使う)。
#[test]
fn rate_limit_detected_from_current_format_next_review_available_in() {
let json = r#"[{
"user": {"login": "coderabbitai[bot]"},
"body": "<!-- This is an auto-generated comment: summarize by coderabbit.ai -->\n<!-- This is an auto-generated comment: rate limited by coderabbit.ai -->\n\n> [!WARNING]\n> ## Review limit reached\n> \n> `@aloekun`, you've reached your PR review limit, so we couldn't start this review.\n> \n> **Next review available in:** **15 minutes**\n",
"created_at": "2026-07-20T10:06:43Z"
}]"#;
let result = parse_rate_limit(json, "2026-07-20T10:06:34Z")
.expect("現行書式の rate-limit comment を検出できること (PR #307 incident)");
assert_eq!(result.wait_minutes, 15);
assert_eq!(result.wait_seconds, 0);
assert!(
result.wait_time_parsed,
"書式を追加したので実測値として読めていること",
);
}

/// 構造的な回帰防止 (good): marker は一致するが待ち時間の書式が**未知**でも
/// `None` (= rate-limit なし) を返さないこと。
///
/// これが本 incident の本質。書式追加だけでは 4 回目の変更で同じ silent regression が
/// 再発するため、「marker 一致 → 必ず rate-limit として扱う」を契約として固定する。
#[test]
fn unknown_wait_time_format_still_reports_rate_limit_with_fallback() {
let json = r#"[{
"user": {"login": "coderabbitai[bot]"},
"body": "<!-- This is an auto-generated comment: rate limited by coderabbit.ai -->\n\nReviews resume at some point in the future.",
"created_at": "2026-07-20T10:06:43Z"
}]"#;
let result = parse_rate_limit(json, "2026-07-20T10:00:00Z").expect(
"待ち時間が読めなくても rate-limit として報告すること (None は制限なし扱い = 誤 success の原因)",
);
assert!(
!result.wait_time_parsed,
"既定値で代替したことを下流へ伝えること",
);
assert_eq!(result.wait_minutes, FALLBACK_WAIT_MINUTES);
let base = parse_iso8601_to_unix("2026-07-20T10:06:43Z").unwrap();
assert_eq!(
result.until_unix_secs,
base + (FALLBACK_WAIT_MINUTES as i64) * 60 + 60,
);
}

/// 現行書式の分・秒併記 variant (CR が秒を付け足した場合に備える)。
#[test]
fn current_format_with_minutes_and_seconds() {
let body = "**Next review available in:** **3 minutes and 20 seconds**";
assert_eq!(extract_wait_time(body), Some((3, 20)));
}

#[test]
fn rate_limit_picks_latest_when_mixed_old_and_new_formats() {
let json = r#"[
Expand Down
Loading