Skip to content
Merged
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
22 changes: 22 additions & 0 deletions docs/adr/adr-064-monitor-success-positive-evidence.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,28 @@ monitor 側に判定を分散させず、**`decide()` に rate_limit を渡し
wakeup 再 trigger) は、セッション中にレート制限が自然発生しなかったため未実測 (ステータス
欄の検証残)。monitor 側の分岐順序は本変更で触っておらず既存実装の性質に依存する。

## Amendment (2026-08-01): CI 側は「観測できていない」ことに気付けていなかった

本 ADR は CodeRabbit 側の判定に陽性証拠を要求したが、**CI 側の入力が恒久的に欠測している**
ことは検知できていなかった。`fetch_ci` は `git branch --show-current` でブランチ名を解決して
`gh run list --branch` を叩いていたが、本リポジトリは jj colocated で **git HEAD が detached**
のためブランチ名が常に空になり、早期 return で CI が常に `pending` / `runs: []` になっていた。

**これが表面化しなかった理由**が本質的である: 当時 PR に status check を出す workflow が
存在せず (`release-binaries.yml` は master push 限定、`pr-monitor.yml` は意図的に
`pull_request` を使わない)、「CI 未設定」と「CI を観測できない」が**同じ出力**になっていた。
ADR-065 (CI matrix、PR #342 で新設) が PR 単位の check を初めて生んだ瞬間に、実際には
failure だった Windows leg を `pending` と報告し続けることで露見した。

対処として `gh pr view <pr> --json statusCheckRollup` に切り替えた。ブランチ解決が不要になり、
併せて **`gh run list --branch X --limit 5` がブランチ上の全 SHA の run を返す**問題
(push 後に前 commit の結論を現在の結論として報告しうる = 本 ADR が排除した silent success と
同型) も構造的に閉じる。空 rollup は `success` ではなく `pending` として扱う。

**教訓**: 「証拠を要求する」判定は、**証拠の入力経路自体が沈黙していないか**を別途担保しない
と成立しない。欠測と正常が同じ出力になる構成 (ここでは「CI が無い」と「CI が見えない」) は、
観測対象が現れた瞬間まで誰も気付けない。

## 教訓 (同種修正のセルフチェック)

1. 修正が**本番 config の経路で実行される**ことをテストで固定する (旧初版は skip 構成でしか
Expand Down
62 changes: 25 additions & 37 deletions src/check-ci-coderabbit/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use crate::models::{
CheckResult, CiRunSummary, CiStatus, CodeRabbitStatus, ListFindingsOutput,
};
use crate::parsers::{
parse_actionable_comments, parse_ci_runs, parse_coderabbit_status, parse_new_comments,
parse_actionable_comments, parse_ci_rollup, parse_coderabbit_status, parse_new_comments,
parse_unresolved_threads, parse_walkthrough_clean_marker,
};
use crate::rate_limit::parse_rate_limit;
Expand Down Expand Up @@ -213,26 +213,6 @@ fn auto_detect_pr() -> Result<u64, String> {
.map_err(|_| format!("PR番号のパースに失敗: {}", output))
}

fn get_current_branch() -> Result<String, String> {
let child = Command::new("git")
.args(["branch", "--show-current"])
.stdout(std::process::Stdio::piped())
.stderr(std::process::Stdio::null())
.spawn()
.map_err(|e| format!("git branch の起動に失敗: {}", e))?;
let output = child
.wait_with_output()
.map_err(|e| format!("git branch の実行に失敗: {}", e))?;
// Note: wait_with_output 自体にはタイムアウトがないが、
// 呼び出し元の CronCreate ジョブ全体にタイムアウトがあるため実用上問題ない
let branch = String::from_utf8_lossy(&output.stdout).trim().to_string();
if branch.is_empty() {
Err("現在のブランチを取得できませんでした".to_string())
} else {
Ok(branch)
}
}

/// 解決済み repo/PR から head SHA を取得する (順位258 harm #2 fix)。
///
/// 旧実装は無指定 `gh pr view` で cwd の branch auto-detection に依存していたが、
Expand Down Expand Up @@ -272,7 +252,7 @@ fn run_check(args: CliArgs) -> CheckResult {
Err(error_result) => return *error_result,
};

let ci = fetch_ci(&get_current_branch().unwrap_or_default());
let ci = fetch_ci(&repo, pr);
let cr_state = fetch_coderabbit_commit_state(&repo, pr);

let comments_json = fetch_issue_comments_json(&repo, pr);
Expand Down Expand Up @@ -357,24 +337,32 @@ fn build_init_error_result(
}
}

fn fetch_ci(branch: &str) -> CiStatus {
if branch.is_empty() {
return CiStatus {
overall: "pending".to_string(),
runs: vec![],
};
}
/// PR head に紐づく check を `statusCheckRollup` から取得する。
///
/// **ブランチ名を使わない**理由 (2026-08-01、PR #342 で実観測): 旧実装は
/// `git branch --show-current` でブランチ名を解決して `gh run list --branch` を叩いて
/// いたが、本リポジトリは jj colocated で **git HEAD が detached** のためブランチ名が
/// 常に空になり、`fetch_ci("")` が早期 return して CI が恒久的に `pending` になっていた。
/// PR に status check を出す workflow が存在しなかった間は「CI 未設定」と区別できず、
/// この blind spot は表面化していなかった。
///
/// 併せて **古い SHA の run 混入**も解消する: `gh run list --branch X --limit 5` は
/// ブランチ上の全 SHA の run を返すため、push 後に前 commit の結論を現在の結論として
/// 報告しうる (ADR-064 が排除した陽性証拠なき success と同型)。rollup は PR head SHA に
/// 紐づく check だけを返すため、この経路が構造的に閉じる。
fn fetch_ci(repo: &str, pr: u64) -> CiStatus {
match run_gh(&[
"run",
"list",
"--branch",
branch,
"--limit",
"5",
"pr",
"view",
&pr.to_string(),
"--repo",
repo,
"--json",
"name,conclusion",
"statusCheckRollup",
"-q",
".statusCheckRollup",
]) {
Ok(ci_json) => parse_ci_runs(&ci_json),
Ok(ci_json) => parse_ci_rollup(&ci_json),
Err(e) => {
eprintln!("[check-ci-coderabbit] CI 取得エラー (pending 扱い): {}", e);
CiStatus {
Expand Down
18 changes: 18 additions & 0 deletions src/check-ci-coderabbit/src/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,24 @@ pub(crate) struct GhRunItem {
pub(crate) conclusion: Option<String>,
}

/// `gh pr view --json statusCheckRollup` の 1 要素。
///
/// rollup は 2 種類のノードが混在する GraphQL union で、フィールド名が異なる:
/// - `CheckRun` (GitHub Actions 等): `name` + `status` + `conclusion`
/// - `StatusContext` (旧 commit status API): `context` + `state`
///
/// どちらが来ても取りこぼさないよう両方を `Option` で受け、[`crate::parsers`] 側で
/// 正規化する。値は GraphQL 由来で大文字 (`SUCCESS` / `IN_PROGRESS`) のため、
/// 正規化時に小文字化して `gh run list` 時代の wire format と互換を保つ。
#[derive(Deserialize)]
pub(crate) struct GhRollupItem {
pub(crate) name: Option<String>,
pub(crate) context: Option<String>,
pub(crate) status: Option<String>,
pub(crate) conclusion: Option<String>,
pub(crate) state: Option<String>,
}

#[derive(Deserialize)]
pub(crate) struct GhStatusItem {
pub(crate) context: Option<String>,
Expand Down
Loading
Loading