feat(hooks): Post-PR Monitor — CI・CodeRabbit自動監視 - #10
Merged
Conversation
📝 WalkthroughWalkthroughPRは、プッシュ/PR作成後に自動でCIとCodeRabbitを監視するワークフローを導入するため、2つのRust実行ファイルと関連設定・ビルド・デプロイ・ドキュメントを追加します（フック検出→Cron登録→定期チェック→JSON結果出力）。 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Claude
participant Hook as hooks-post-pr-monitor.exe
participant Scheduler as CronCreate
participant Monitor as check-ci-coderabbit.exe
participant GH as GitHub CLI/API
User->>Claude: git push / gh pr create
Claude->>Hook: PostToolUse event (tool_input.command)
Hook->>GH: gh pr view / gh repo view (suppress stderr)
GH-->>Hook: repo, pr, timestamp
Hook->>Scheduler: emit CronCreate (additionalContext with checker cmd)
Scheduler->>Monitor: invoke checker (--push-time, --repo, --pr)
Monitor->>GH: gh run list / statuses / GraphQL threads / comments
GH-->>Monitor: CI runs, commit statuses, comments, threads
Monitor-->>Claude: JSON {status, action, ci, coderabbit, summary}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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.
🧹 Nitpick comments (2)
.claude/check-ci-coderabbit/src/main.rs (2)
608-621: 引数エラー時の出力形式について確認を推奨します。
--push-timeが欠けている場合、JSON ではなく stderr にエラーメッセージを出力して exit(1) します。hooks-post-pr-monitor が常に--push-timeを渡すため実運用上は問題ありませんが、手動デバッグ時やエラー発生時の一貫性のため、エラー時も JSON 形式で出力する選択肢も検討できます。💡 エラー時も JSON を出力する例
fn main() { let args = match parse_args() { Ok(a) => a, Err(e) => { let result = CheckResult { status: "error".to_string(), action: "stop_monitoring_failure".to_string(), ci: CiStatus { overall: "error".to_string(), runs: vec![] }, coderabbit: CodeRabbitStatus { review_state: "error".to_string(), ..Default::default() }, summary: format!("引数エラー: {}", e), }; let json = serde_json::to_string_pretty(&result).unwrap_or_else(|_| "{}".to_string()); println!("{}", json); std::process::exit(1); } }; // ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/check-ci-coderabbit/src/main.rs around lines 608 - 621, When parse_args() fails in main, instead of printing a plain stderr string and exiting, build a CheckResult error payload and print it as JSON so error output stays consistent; update the Err(e) arm in main (where parse_args() is matched) to construct a CheckResult with status or overall marked "error", populate CiStatus and CodeRabbitStatus with minimal error indicators, set summary to include the parse error message, serialize with serde_json::to_string_pretty and println! the JSON, then exit(1). Reference parse_args, main, CheckResult, CiStatus, and CodeRabbitStatus to locate and modify the error-handling branch.
74-91: タイマースレッドの join により不要な待機が発生する可能性があります。
ghコマンドが素早く完了した場合でも、timer.join()はタイマースレッドが 30 秒の sleep を完了するまで待機します。実用上は CronCreate ジョブ全体のタイムアウトがあるため大きな問題にはなりませんが、改善の余地があります。♻️ タイマースレッドを join しない方法
- // タイマースレッドを停止 (join するが結果は無視) - let _ = timer.join(); + // タイマースレッドは join しない (プロセス終了時に自動終了) + drop(timer);または、
std::sync::Condvarを使用してタイマースレッドに早期終了を通知する方法もあります。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/check-ci-coderabbit/src/main.rs around lines 74 - 91, The timer thread (timer spawned with std::thread::spawn using flag_clone) is being joined after child.wait_with_output, which can block up to the timer sleep duration; instead make the timer truly detachable and cooperative: remove the blocking timer.join() and change the timer closure to poll an Arc<AtomicBool> (flag/flag_clone) in short sleep increments (or wait on a Condvar/channel) so it exits early when the main thread sets the flag after child.wait_with_output returns; keep the existing kill logic (taskkill) in the timer when the timeout elapses, but do not join timer — let it exit on its own when the flag is set.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.claude/check-ci-coderabbit/src/main.rs:
- Around line 608-621: When parse_args() fails in main, instead of printing a
plain stderr string and exiting, build a CheckResult error payload and print it
as JSON so error output stays consistent; update the Err(e) arm in main (where
parse_args() is matched) to construct a CheckResult with status or overall
marked "error", populate CiStatus and CodeRabbitStatus with minimal error
indicators, set summary to include the parse error message, serialize with
serde_json::to_string_pretty and println! the JSON, then exit(1). Reference
parse_args, main, CheckResult, CiStatus, and CodeRabbitStatus to locate and
modify the error-handling branch.
- Around line 74-91: The timer thread (timer spawned with std::thread::spawn
using flag_clone) is being joined after child.wait_with_output, which can block
up to the timer sleep duration; instead make the timer truly detachable and
cooperative: remove the blocking timer.join() and change the timer closure to
poll an Arc<AtomicBool> (flag/flag_clone) in short sleep increments (or wait on
a Condvar/channel) so it exits early when the main thread sets the flag after
child.wait_with_output returns; keep the existing kill logic (taskkill) in the
timer when the timeout elapses, but do not join timer — let it exit on its own
when the flag is set.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5281045c-71c8-4a6b-848b-d731974637f7
⛔ Files ignored due to path filters (2)
.claude/check-ci-coderabbit/Cargo.lockis excluded by!**/*.lock.claude/hooks-post-pr-monitor/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
.claude/check-ci-coderabbit/Cargo.toml.claude/check-ci-coderabbit/src/main.rs.claude/hooks-config.toml.claude/hooks-post-pr-monitor/Cargo.toml.claude/hooks-post-pr-monitor/src/main.rs.claude/settings.local.json.template.gitignoreCLAUDE.mddocs/adr/adr-009-post-pr-monitor.mdpackage.jsonscripts/deploy-hooks.tstemplates/hooks-config-python.tomltemplates/hooks-config-typescript.toml
- H1: GraphQL を -F 変数渡しに変更 (インジェクション防止) - H2: run_gh デッドロック防止 (try_wait → wait_with_output + タイムアウトスレッド) - H3: repo/head_sha の入力値検証追加 - H4: serde_json パースエラーの eprintln ログ追加 (4箇所) - H5: post-pr-monitor stdin/JSON エラーログ追加 - H6: 無効な正規表現パターンの警告ログ追加 - H7: auto_detect 失敗理由を summary に反映 - regex クレートを check-ci-coderabbit に追加 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
aloekun
force-pushed
the
feat/post-pr-monitor
branch
from
April 2, 2026 12:25
120d2d3 to
6fad2aa
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
.claude/hooks-post-pr-monitor/src/main.rs (1)
100-114: 正規表現の毎回コンパイルは非効率
is_trigger_commandはパターンごとにRegex::new(pat)を呼び出しています。デフォルトで 3 パターン × 毎回の hook 呼び出しでコンパイルが発生します。♻️ コンパイル済み正規表現をキャッシュする修正案
+use std::collections::HashMap; +use std::sync::Mutex; +use std::sync::LazyLock; + +static REGEX_CACHE: LazyLock<Mutex<HashMap<String, Regex>>> = + LazyLock::new(|| Mutex::new(HashMap::new())); + +fn get_or_compile_regex(pattern: &str) -> Option<Regex> { + let mut cache = REGEX_CACHE.lock().ok()?; + if let Some(re) = cache.get(pattern) { + return Some(re.clone()); + } + match Regex::new(pattern) { + Ok(re) => { + cache.insert(pattern.to_string(), re.clone()); + Some(re) + } + Err(e) => { + eprintln!("[post-pr-monitor] 無効な正規表現パターン \"{}\": {}", pattern, e); + None + } + } +} + fn is_trigger_command(command: &str, patterns: &[String]) -> bool { for pat in patterns { - match Regex::new(pat) { - Ok(re) => { - if re.is_match(command) { - return true; - } - } - Err(e) => { - eprintln!("[post-pr-monitor] 無効な正規表現パターン \"{}\": {}", pat, e); + if let Some(re) = get_or_compile_regex(pat) { + if re.is_match(command) { + return true; } } } false }または、
detect_command_typeと共通化してデフォルトパターンはLazyLock<Regex>で静的に保持することも検討してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/hooks-post-pr-monitor/src/main.rs around lines 100 - 114, is_trigger_command currently compiles each pattern on every call via Regex::new(pat) which is inefficient; fix it by precompiling and reusing Regex objects instead: change the API to accept compiled patterns (e.g. &[Regex]) or build a one-time cache of compiled Regexes and look them up (use once_cell::sync::LazyLock or a static HashMap) so Regex::new is not called per invocation; update callers (including detect_command_type) to pass the compiled Regex slice or rely on the shared LazyLock defaults for the three default patterns..claude/check-ci-coderabbit/src/main.rs (4)
276-280: ISO 8601 文字列比較の前提条件に注意
t > push_timeによる文字列比較は、両方のタイムスタンプが同一フォーマット（Zulu 時間、ミリ秒なし）であることを前提としています。GitHub API が2026-04-01T12:00:00.123Zのようにミリ秒を含む場合、比較が不正確になる可能性があります。現時点では GitHub API のレスポンスが一貫した形式を返すため実用上問題ないと思われますが、堅牢性のためにタイムスタンプのパースと数値比較を検討してください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/check-ci-coderabbit/src/main.rs around lines 276 - 280, The current string comparison (created_at.as_deref().map(|t| t > push_time)) can be wrong when timestamps include milliseconds; instead parse both timestamps to chrono DateTime (e.g., DateTime<Utc> via DateTime::parse_from_rfc3339 or chrono::DateTime::parse_from_rfc3339) and compare the resulting DateTime values; update the code around after_push_time, parsing push_time once to a DateTime prior to the map, parse created_at inside the map (handle parse errors by treating as false or logging), and add the chrono import and error handling so comparisons are numeric/temporal rather than string-based.
495-495:.map(|r| r)は不要な identity 操作
repo_result.map(|r| r)は何もしていません。♻️ 簡略化
- let repo = repo_result.map(|r| r).unwrap_or_default(); + let repo = repo_result.unwrap_or_default();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/check-ci-coderabbit/src/main.rs at line 495, repo_result.map(|r| r) は不要な identity 操作なので単に unwrap_or_default を呼ぶように簡略化してください：置換対象は変数 repo を作る式（repo_result.map(|r| r).unwrap_or_default() または類似のパターン）で、map(|r| r) を削除して let repo = repo_result.unwrap_or_default(); としてください。
443-461:get_current_branchにタイムアウトがない
run_ghは 30 秒のタイムアウトを実装していますが、get_current_branchのgit branch --show-currentにはタイムアウトがありません。コメントで「CronCreate ジョブ全体にタイムアウトがあるため問題ない」と記載されていますが、一貫性のためにもrun_ghと同様のタイムアウト機構を共通化することを検討してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/check-ci-coderabbit/src/main.rs around lines 443 - 461, get_current_branch spawns git without a timeout, unlike run_gh which enforces 30s; add the same timeout behavior by extracting/using the existing run_gh (or its underlying helper) to execute "git branch --show-current" so the call times out consistently, or factor out a new helper (e.g., run_command_with_timeout) and have both run_gh and get_current_branch call it; update get_current_branch to return an error if the helper reports a timeout or nonzero exit, and keep the same UTF-8/trimming logic on success.
470-473: Regex の毎回コンパイルは非効率
is_valid_repoは呼び出しごとに正規表現をコンパイルしています。この関数はrun_check内で複数回呼ばれる可能性があり、パフォーマンスに影響します。♻️ `once_cell::sync::Lazy` または `std::sync::LazyLock` (Rust 1.80+) を使用した修正案
+use std::sync::LazyLock; + +static REPO_REGEX: LazyLock<regex::Regex> = LazyLock::new(|| { + regex::Regex::new(r"^[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+$").unwrap() +}); + fn is_valid_repo(repo: &str) -> bool { - let re = regex::Regex::new(r"^[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+$").unwrap(); - re.is_match(repo) + REPO_REGEX.is_match(repo) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/check-ci-coderabbit/src/main.rs around lines 470 - 473, The is_valid_repo function currently compiles the regex on every call; replace that with a static, precompiled Regex (e.g., using once_cell::sync::Lazy or std::sync::LazyLock) so the pattern r"^[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+$" is compiled once at startup and reused; update is_valid_repo to reference the static (e.g., RE.is_match(repo)) and add the necessary use/import for Lazy and regex::Regex.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/check-ci-coderabbit/src/main.rs:
- Around line 77-93: The timeout handler currently only kills the child on
Windows; update the thread closure where flag_clone.store(...) is called (the
std::thread::spawn block using done_clone, flag_clone and child_id) to also
terminate the child on Unix by adding a cfg(unix) branch that sends SIGKILL
(e.g., invoke "kill -9 <pid>" via Command or call nix::sys::signal::kill or
libc::kill) after setting the timeout flag, ensuring the gh child process is
terminated so wait_with_output doesn't block; alternatively document
Windows-only support if you choose not to implement Unix handling.
In @.claude/hooks-post-pr-monitor/src/main.rs:
- Around line 185-204: The current run_gh_quiet uses Windows-only
Command::new("cmd").args(["/c", "gh"]) which breaks on Linux/macOS; change it to
invoke gh directly like the run_gh in check-ci-coderabbit by replacing the
Windows wrapper with Command::new("gh").args(args) while preserving the
stdout/stderr piping, output().ok()? handling, and the existing logic that
returns Some(trimmed_string) on success or None on failure.
- Around line 283-295: Remove the test-only debug_log function and replace its
usages with eprintln! (or remove the calls if redundant): delete the debug_log
function definition and update all places that call debug_log(...) to either
call eprintln!("[post-pr-monitor] {}", ... ) with the same message expression or
remove those calls entirely so no file writes occur in production; search for
debug_log invocations to update each call site accordingly.
---
Nitpick comments:
In @.claude/check-ci-coderabbit/src/main.rs:
- Around line 276-280: The current string comparison
(created_at.as_deref().map(|t| t > push_time)) can be wrong when timestamps
include milliseconds; instead parse both timestamps to chrono DateTime (e.g.,
DateTime<Utc> via DateTime::parse_from_rfc3339 or
chrono::DateTime::parse_from_rfc3339) and compare the resulting DateTime values;
update the code around after_push_time, parsing push_time once to a DateTime
prior to the map, parse created_at inside the map (handle parse errors by
treating as false or logging), and add the chrono import and error handling so
comparisons are numeric/temporal rather than string-based.
- Line 495: repo_result.map(|r| r) は不要な identity 操作なので単に unwrap_or_default
を呼ぶように簡略化してください：置換対象は変数 repo を作る式（repo_result.map(|r| r).unwrap_or_default()
または類似のパターン）で、map(|r| r) を削除して let repo = repo_result.unwrap_or_default();
としてください。
- Around line 443-461: get_current_branch spawns git without a timeout, unlike
run_gh which enforces 30s; add the same timeout behavior by extracting/using the
existing run_gh (or its underlying helper) to execute "git branch
--show-current" so the call times out consistently, or factor out a new helper
(e.g., run_command_with_timeout) and have both run_gh and get_current_branch
call it; update get_current_branch to return an error if the helper reports a
timeout or nonzero exit, and keep the same UTF-8/trimming logic on success.
- Around line 470-473: The is_valid_repo function currently compiles the regex
on every call; replace that with a static, precompiled Regex (e.g., using
once_cell::sync::Lazy or std::sync::LazyLock) so the pattern
r"^[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+$" is compiled once at startup and reused;
update is_valid_repo to reference the static (e.g., RE.is_match(repo)) and add
the necessary use/import for Lazy and regex::Regex.
In @.claude/hooks-post-pr-monitor/src/main.rs:
- Around line 100-114: is_trigger_command currently compiles each pattern on
every call via Regex::new(pat) which is inefficient; fix it by precompiling and
reusing Regex objects instead: change the API to accept compiled patterns (e.g.
&[Regex]) or build a one-time cache of compiled Regexes and look them up (use
once_cell::sync::LazyLock or a static HashMap) so Regex::new is not called per
invocation; update callers (including detect_command_type) to pass the compiled
Regex slice or rely on the shared LazyLock defaults for the three default
patterns.
🪄 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: a1083153-ff85-40ab-9155-b6d3e22fe399
⛔ Files ignored due to path filters (3)
.claude/check-ci-coderabbit/Cargo.lockis excluded by!**/*.lock.claude/hooks-post-pr-monitor/Cargo.lockis excluded by!**/*.lock.claude/post-pr-monitor-debug.logis excluded by!**/*.log
📒 Files selected for processing (13)
.claude/check-ci-coderabbit/Cargo.toml.claude/check-ci-coderabbit/src/main.rs.claude/hooks-config.toml.claude/hooks-post-pr-monitor/Cargo.toml.claude/hooks-post-pr-monitor/src/main.rs.claude/settings.local.json.template.gitignoreCLAUDE.mddocs/adr/adr-009-post-pr-monitor.mdpackage.jsonscripts/deploy-hooks.tstemplates/hooks-config-python.tomltemplates/hooks-config-typescript.toml
✅ Files skipped from review due to trivial changes (8)
- CLAUDE.md
- scripts/deploy-hooks.ts
- templates/hooks-config-python.toml
- .claude/check-ci-coderabbit/Cargo.toml
- .gitignore
- .claude/hooks-post-pr-monitor/Cargo.toml
- templates/hooks-config-typescript.toml
- docs/adr/adr-009-post-pr-monitor.md
🚧 Files skipped from review as they are similar to previous changes (2)
- .claude/settings.local.json.template
- package.json
Comment on lines
+77
to
+93
| std::thread::spawn(move || { | ||
| let deadline = std::time::Instant::now() + Duration::from_secs(30); | ||
| while std::time::Instant::now() < deadline { | ||
| if done_clone.load(std::sync::atomic::Ordering::Relaxed) { | ||
| return; // プロセス完了 → スレッド即終了 | ||
| } | ||
| std::thread::sleep(Duration::from_millis(100)); | ||
| } | ||
| // タイムアウト到達 | ||
| flag_clone.store(true, std::sync::atomic::Ordering::Relaxed); | ||
| #[cfg(target_os = "windows")] | ||
| { | ||
| let _ = Command::new("taskkill") | ||
| .args(["/F", "/PID", &child_id.to_string()]) | ||
| .output(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
タイムアウト処理が Windows 専用になっている
タイムアウト発生時のプロセス強制終了が #[cfg(target_os = "windows")] ブロック内にのみ実装されています。Linux/macOS 環境では timeout_flag は設定されますが、gh プロセスは kill されず、wait_with_output がブロックし続ける可能性があります。
🔧 Unix 対応を追加する修正案
// タイムアウト到達
flag_clone.store(true, std::sync::atomic::Ordering::Relaxed);
#[cfg(target_os = "windows")]
{
let _ = Command::new("taskkill")
.args(["/F", "/PID", &child_id.to_string()])
.output();
}
+ #[cfg(unix)]
+ {
+ // SIGKILL を送信
+ unsafe {
+ libc::kill(child_id as i32, libc::SIGKILL);
+ }
+ }あるいは、nix クレートを使用するか、この実行ファイルが Windows 専用であることをドキュメント化してください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/check-ci-coderabbit/src/main.rs around lines 77 - 93, The timeout
handler currently only kills the child on Windows; update the thread closure
where flag_clone.store(...) is called (the std::thread::spawn block using
done_clone, flag_clone and child_id) to also terminate the child on Unix by
adding a cfg(unix) branch that sends SIGKILL (e.g., invoke "kill -9 <pid>" via
Command or call nix::sys::signal::kill or libc::kill) after setting the timeout
flag, ensuring the gh child process is terminated so wait_with_output doesn't
block; alternatively document Windows-only support if you choose not to
implement Unix handling.
Comment on lines
+185
to
+204
| fn run_gh_quiet(args: &[&str]) -> Option<String> { | ||
| let output = Command::new("cmd") | ||
| .args(["/c", "gh"]) | ||
| .args(args) | ||
| .stdout(std::process::Stdio::piped()) | ||
| .stderr(std::process::Stdio::null()) | ||
| .output() | ||
| .ok()?; | ||
|
|
||
| if output.status.success() { | ||
| let s = String::from_utf8_lossy(&output.stdout).trim().to_string(); | ||
| if s.is_empty() { | ||
| None | ||
| } else { | ||
| Some(s) | ||
| } | ||
| } else { | ||
| None | ||
| } | ||
| } |
There was a problem hiding this comment.
run_gh_quiet が Windows 専用になっている
Command::new("cmd").args(["/c", "gh"]) は Windows 固有の実装です。Linux/macOS では cmd コマンドが存在しないため失敗します。
🔧 クロスプラットフォーム対応の修正案
fn run_gh_quiet(args: &[&str]) -> Option<String> {
- let output = Command::new("cmd")
- .args(["/c", "gh"])
+ let output = Command::new("gh")
.args(args)
.stdout(std::process::Stdio::piped())
.stderr(std::process::Stdio::null())
.output()
.ok()?;check-ci-coderabbit/src/main.rs の run_gh は Command::new("gh") を直接使用しており、そちらと一貫性を持たせてください。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fn run_gh_quiet(args: &[&str]) -> Option<String> { | |
| let output = Command::new("cmd") | |
| .args(["/c", "gh"]) | |
| .args(args) | |
| .stdout(std::process::Stdio::piped()) | |
| .stderr(std::process::Stdio::null()) | |
| .output() | |
| .ok()?; | |
| if output.status.success() { | |
| let s = String::from_utf8_lossy(&output.stdout).trim().to_string(); | |
| if s.is_empty() { | |
| None | |
| } else { | |
| Some(s) | |
| } | |
| } else { | |
| None | |
| } | |
| } | |
| fn run_gh_quiet(args: &[&str]) -> Option<String> { | |
| let output = Command::new("gh") | |
| .args(args) | |
| .stdout(std::process::Stdio::piped()) | |
| .stderr(std::process::Stdio::null()) | |
| .output() | |
| .ok()?; | |
| if output.status.success() { | |
| let s = String::from_utf8_lossy(&output.stdout).trim().to_string(); | |
| if s.is_empty() { | |
| None | |
| } else { | |
| Some(s) | |
| } | |
| } else { | |
| None | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/hooks-post-pr-monitor/src/main.rs around lines 185 - 204, The
current run_gh_quiet uses Windows-only Command::new("cmd").args(["/c", "gh"])
which breaks on Linux/macOS; change it to invoke gh directly like the run_gh in
check-ci-coderabbit by replacing the Windows wrapper with
Command::new("gh").args(args) while preserving the stdout/stderr piping,
output().ok()? handling, and the existing logic that returns
Some(trimmed_string) on success or None on failure.
Comment on lines
+283
to
+295
| /// デバッグログをファイルに追記 (テスト用 — 本番では削除) | ||
| fn debug_log(msg: &str) { | ||
| use std::io::Write; | ||
| let log_path = std::env::current_exe() | ||
| .unwrap_or_default() | ||
| .parent() | ||
| .unwrap_or(Path::new(".")) | ||
| .join("post-pr-monitor-debug.log"); | ||
| if let Ok(mut f) = std::fs::OpenOptions::new().create(true).append(true).open(&log_path) { | ||
| let _ = writeln!(f, "[{}] {}", utc_now_iso8601(), msg); | ||
| } | ||
| eprintln!("[post-pr-monitor] {}", msg); | ||
| } |
There was a problem hiding this comment.
本番コードにデバッグ用関数が残っている
コメントに「テスト用 — 本番では削除」と明記されていますが、debug_log 関数とその呼び出し（lines 298, 303, 307, 313, 322, 327, 335, 346, 351）が残っています。
ファイル書き込みを伴うデバッグログは本番環境では不要であり、eprintln! での標準エラー出力のみで十分です。
🧹 デバッグコードの削除
debug_log 関数を削除し、呼び出し箇所を eprintln! に置き換えるか、完全に削除してください。
-/// デバッグログをファイルに追記 (テスト用 — 本番では削除)
-fn debug_log(msg: &str) {
- use std::io::Write;
- let log_path = std::env::current_exe()
- .unwrap_or_default()
- .parent()
- .unwrap_or(Path::new("."))
- .join("post-pr-monitor-debug.log");
- if let Ok(mut f) = std::fs::OpenOptions::new().create(true).append(true).open(&log_path) {
- let _ = writeln!(f, "[{}] {}", utc_now_iso8601(), msg);
- }
- eprintln!("[post-pr-monitor] {}", msg);
-}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /// デバッグログをファイルに追記 (テスト用 — 本番では削除) | |
| fn debug_log(msg: &str) { | |
| use std::io::Write; | |
| let log_path = std::env::current_exe() | |
| .unwrap_or_default() | |
| .parent() | |
| .unwrap_or(Path::new(".")) | |
| .join("post-pr-monitor-debug.log"); | |
| if let Ok(mut f) = std::fs::OpenOptions::new().create(true).append(true).open(&log_path) { | |
| let _ = writeln!(f, "[{}] {}", utc_now_iso8601(), msg); | |
| } | |
| eprintln!("[post-pr-monitor] {}", msg); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/hooks-post-pr-monitor/src/main.rs around lines 283 - 295, Remove the
test-only debug_log function and replace its usages with eprintln! (or remove
the calls if redundant): delete the debug_log function definition and update all
places that call debug_log(...) to either call eprintln!("[post-pr-monitor] {}",
... ) with the same message expression or remove those calls entirely so no file
writes occur in production; search for debug_log invocations to update each call
site accordingly.
4 tasks
This was referenced Apr 2, 2026
This was referenced Apr 15, 2026
6 tasks
aloekun
added a commit
that referenced
this pull request
Aug 6, 2026
…DR-071) (#361) * feat(autonomy-policy): 未マージ draft 数の背圧を判定コアの入力にする WP-18 PR 1 (1/5)。ADR-052 原則 5 が自動実行可クラスの前提条件とする背圧を、 判定コア `lib-autonomy-policy` が入力として受け取れるようにする。 ## 変更の要点 `Operation::backpressure_connected()` の `DraftPr => false` 固定を廃し、背圧の **指標の要求**と**指標の実測値**を別の層へ分けた: - `Operation::requires_draft_backpressure()` — 操作クラスが「未マージ draft 数」を 要求するかだけを持つ静的分類。状態は持たない - `GateInputs::open_draft_prs` / `max_open_draft_prs` — 実測値と閾値。どちらか一方でも `None` なら `backpressure-unavailable` で deny (fail-closed) 背圧の状態を enum 側にも持たせると `GateInputs` と二重管理になり判定経路が分岐するため、 状態の保持先は `GateInputs` 1 箇所に限定した (計画書 WP-18 PR 1 の明示要件)。 `FixPush` が draft 数を要求しないのは背圧が無いからではなく、その背圧が cli-pr-monitor の 有界 retry (`max_retries`) で、呼び出しごとに gate へ渡す状態を持たないため。この非対称を doc コメントとテスト (`fix_push_is_unaffected_by_draft_backpressure_inputs`) の両方で固定した。 ## 追加した deny 理由 `DenyReason::BackpressureSaturated { open, limit }` (code = `backpressure-saturated`)。 飽和判定は `>=`。閾値は「これ以上は積まない」上限であり、`limit = 0` は「draft を 1 件も 作らない」= 実質停止を意味する。 `describe_sources` の背圧表記は `structural` / `ok(N/M)` / `saturated(N/M)` / `unavailable` の 4 状態で、実数を必ず併記する。run log 1 行で「数え損ねて止まった」と「積み過ぎて 止まった」を切り分けられるようにするため。 ## テスト 判定コアのテストを 16 → 22 件へ。網羅走査テストは背圧 4 パターンを軸に加えて 54 → 216 組合せになり、許可される組合せ数も式で固定した (truthy 表記数 × (fix-push 4 + draft-pr 1))。 境界 (`open == limit` で止まる)、`limit = 0`、kill-switch が背圧より先に効くこと、 背圧入力の欠落 3 パターンをそれぞれ独立に pin している。 ## 本コミットの範囲 呼び手 2 件 (`cli-autonomy-gate` / `cli-fix-push-gate`) は本コミットでは `None` を渡す。 `draft-pr` は従来どおり deny のままで、運用挙動は変わらない。config からの閾値読み取りは 2/5、実測 draft 数の受け口は 3/5 で接続する。 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(autonomy-policy): 背圧閾値 max_open_draft_prs を config から 1 回の read で取り込む WP-18 PR 1 (2/5)。背圧の閾値を `autonomy-config.toml` の `[autonomy] max_open_draft_prs` に置き、kill-switch フラグと同じ信頼境界 (master ref の写し) と 同じ「欠損 → 停止」極性に乗せる。 ## read を 1 回に集約した理由 `read_repo_config_enabled(path) -> Option<bool>` を `read_repo_config(path) -> RepoConfig` へ置き換えた。キーごとに読み取り関数を生やすとファイルを 2 回読むことになり、2 回の read の 間に config が差し替わると「kill-switch は旧世代・閾値は新世代」という混成状態で判定しうる。 1 回の read から派生した値だけを使う。 ## 半壊 config は全フィールドが停止側へ倒れる toml の parse はファイル単位なので、`max_open_draft_prs` が型違い (文字列 / 負値 / 小数) だと `enabled` も含めて全フィールドが `None` になる。これは意図した挙動で、config の一部が壊れた 状態を「kill-switch だけ有効」で運転させない (ADR-043 fail-closed)。テストで明示的に固定した。 閾値キー欠落時に既定値 (3 など) へ倒さないのも同じ理由。書き忘れが「勝手に 3 件まで作る」 という fail-open にならないよう、欠落は背圧未接続 = draft PR deny とする。 一方で未知キーは無視する (`unknown_keys_are_ignored`)。config に新しい設定を足したときに、 未更新のバイナリが parse 失敗で全停止するのは安全側に振りすぎるため。 ## テスト sources のテストを 6 → 11 件へ。閾値の正常読み取り (0 を含む)、キー欠落が既定値でないこと、 型違い 3 種が kill-switch フラグごと停止側へ倒すこと、未知キー無視を追加した。 ## 本コミットの範囲 `cli-autonomy-gate` は閾値を `GateInputs` へ渡すようになったが、実測 draft 数の受け口 (`--open-draft-prs`) はまだ無いため `draft-pr` は `backpressure-unavailable` で deny のまま。 接続は 3/5 で行う。`cli-fix-push-gate` は `enabled` しか使わないため呼び出し形の追従のみ。 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(autonomy-gate): --open-draft-prs で実測 draft 数を受け取り背圧を接続する WP-18 PR 1 (3/5)。判定コア (1/5) と閾値 (2/5) に続き、背圧の残り 1 入力である **実測件数**の受け口を CLI に開ける。これで `draft-pr` が構造的 deny を脱し、 ADR-052 原則 5 の契約を満たした状態でのみ許可されるようになる。 ## 引数の扱い `--open-draft-prs <count>` は省略可能。`fix-push` では判定に使わないためで、`draft-pr` で 省略した場合は `None` = 背圧未接続として deny に倒れる。**省略が許可へ倒れる経路は無い**。 値のパースは `u32` で、空文字 / 負値 / 小数 / 非数値 / 末尾空白は引数不正 (exit 2) として 弾く。呼び手の `gh api` が失敗したときの出力 (空文字など) を 0 件と読み違えて「draft が 1 件も無いので作ってよい」に倒れるのが最悪の failure mode なので、ここは黙って `None` へ 潰さず loud に落とす。 `--config` / `--operation` の必須性は従来どおり。引数ループは値取得を各 arm へ寄せ、 未知フラグの判定を値の有無より先に行う既存の順序を保った。 ## テスト 引数解析のテストを 4 → 7 件へ。省略時が `None` であること、`0` が正しく `Some(0)` として 読まれること (「0 件」と「数えられなかった」を型で区別する契約)、不正値 5 種が引数不正で あることを固定した。 ## 本コミットの範囲 exe 単体では背圧が接続された。実測値を渡す呼び手 (夜間 workflow の `gh api` step) は **本 PR には含まれず PR 3 で追加する** (ADR-069 chain 宣言 → 計画書 § WP-18 と ADR-071 を 5/5 で更新)。実 exe による drill は 4/5。 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(adr-071): 未マージ draft PR 数による背圧を起票し drill 12 シナリオを記録 WP-18 PR 1 (4/5)。1/5〜3/5 で実装した背圧の設計判断・試験運用条件・実測を永続化する。 ## 記録した決定 5 件 1. 指標は「未マージ draft PR 数 (claude/ prefix)」。run 回数や経過時間のような代理指標 ではなく、止めたい事象 (人間が捌けない量の未処理成果物) を直接数える 2. 背圧の状態は GateInputs だけが持つ。enum 側にも状態を置くと判定経路が二股に分かれ、 「テストは通るが実運用では別の枝を通る」drift を生む 3. 閾値は autonomy-config.toml の max_open_draft_prs、判定は >=。0 は draft-pr クラス だけの停止、キー欠落は既定値ではなく deny 4. 数えるのは呼び手 (workflow step の gh api)、判断するのは gate。exe が gh に依存すると ローカル drill が GitHub 到達性に依存し、安全装置の再現可能な検証ができなくなる 5. 数えられなかったことは 0 件ではない。不正値は exit 2 で loud に落とす 決定 4 の「呼び手が数を偽れる」問題は、schedule イベントが default branch の workflow 定義を 使うという GitHub の仕様で担保される (ADR-066 の config master ref 契約と同じ信頼境界)。 ## 外部 SaaS の課金・上限事実を移管 (計画書 § 2 の退役条件 2) 2026-08-06 に最新値を再確認して永続化した。 - public リポジトリ + standard runner の Actions 実行は無料・分数無制限 (GitHub 公式 docs で 原文確認)。GitHub Free の 2,000 分/月は private のみ - claude-code-action@v1 は claude_code_oauth_token で Max 枠を消費。OAuth token は個人 サブスクに紐づき、自動化の消費が対話作業のレート枠を圧迫しうる 要約: Actions の実行時間は無料だが Max 枠は有限。背圧の経済的根拠はここにある。 ## 検証記録 release build の実 exe で drill 12 シナリオを実施し全て設計どおりを確認した。#6/#7 の対で >= 境界、#8 で limit=0 の停止、#9 で fix push が draft 数から独立していること、#10 で 半壊 config が enabled ごと停止することを実バイナリ上で確認している。 unit test は判定コア 22 / sources 11 / 引数解析 7 件。網羅走査 1 件が 216 組合せを走査する。 ## bounded lifetime decision trigger (b)「閾値到達で実際に次の run が止まること」は本 ADR 固有の観測点で、 exe 単体 drill では作れない状態 (夜間ループが実際に 3 件積む必要がある)。期限は 2026-11-06。 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(harness-plan): WP-18 PR 1 の完了を記帳し PR 3 への chain 宣言と SaaS 事実の移管を行う WP-18 PR 1 (5/5)。計画書側の 3 種類の更新。 ## 1. PR 1 完了の記帳 - WP-18 節の見出しと全体像表を「着手可」→「着手中（PR 1 実装済み）」へ - PR 構成 1 に実装時の設計判断を確定記録: 閾値判定の層は (a) gate 内を採用。 backpressure_connected() は廃し requires_draft_backpressure()（指標の要求のみ）と GateInputs の 2 フィールド（状態）へ分離した - 着手前決定 2 の記述を過去形へ（backpressure_connected() は既に存在しない名前のため、 現在形のまま残すと ADR-069 決定 1 の「名前一致」要件に反する） - WP-19 ステップ 2 を「前倒し済み」→「land 済み」へ ## 2. PR chain 宣言（ADR-069 決定 1） PR 1 が導入した 3 点（--open-draft-prs フラグ / max_open_draft_prs キー / DraftPr の許可 経路）は PR 3 まで呼び手を持たない。抽出↔呼び手のペアリングを表で具体名指定した。 順序を逆にできない根拠も明記した — ADR-052 原則 5 が背圧の接続を draft-pr クラス有効化の 前提条件としているため、背圧が先に land する必要がある（WP-17 の kill-switch 先行と同構造）。 なお PR 1 単体では draft-pr は deny のままで運用挙動は変わらない。 ## 3. SaaS 課金・上限事実の移管（退役条件 2 / 順位 117 の 3 ステップ原則） permanent 側（ADR-071 § 外部 SaaS の課金・上限事実）を先に作成 → 本ファイルの § 2 から GitHub Actions 課金 2 点と claude-code-action の OAuth 認証を削除し ADR-071 への参照へ 置き換えた。cloud routines の事実群は WP-19 / ADR-070 の担当範囲のため本節に残している。 runner 単価の相対比も private 化時のみ関係するため移管対象外とした。 pnpm lint:docs / markdownlint ともに green。 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: CodeRabbit 指摘 2 件に対応 (主張の適用範囲を限定 / 移管済み方針の更新) PR #361 への CodeRabbit レビュー (Minor 2 件) の反映。どちらも「書いた内容が実態より 広い/古い」型の指摘で、妥当。 ## 1. 「draft-pr は deny のまま」の適用範囲を限定 (2 箇所) 指摘は「**両文書**が PR 3 まで draft-pr に許可経路が無いように読める」というもの。実際には ADR-071 § 検証記録の drill #6 が示すとおり、--open-draft-prs と有効な config を渡せば exe は allow になる。 正確には**リポジトリ内の自動化経路に --open-draft-prs を渡す呼び手が 1 つも無いため** deny に なる、が正しい。「運用挙動は変わらない」の根拠を exe の挙動ではなく呼び手の不在へ置き直した。 根拠を取り違えたまま PR 3 で呼び手が入ると、主張だけが stale に残る。 修正箇所は 2 つ: - docs/adr/adr-071-*.md § 残課題 - docs/harness-improvement-plan.md § WP-18 の PR chain 宣言 計画書側から ADR-071 § 残課題への参照も付け、同じ限定が 1 箇所に集約されるようにした。 ## 2. 計画書 § 2 preamble: 移管済みの事実と矛盾する旧方針を更新 旧文は「現時点では ADR 化せず本ファイルに保持する」と書いていたが、同じ blockquote の 次の行で「ADR-071 へ移管済み」と述べており矛盾していた。 方針文を「担当 WP の ADR 起票時に移管し本節から削除する」という恒常ルールへ書き換え、 移管済み / 未移管を別項目に分けた。未移管の内訳 (cloud routines の daily cap / webhook 上限 / GitHub App 必須 / 緑ステータスの意味) も具体名で列挙し、次の担当 WP が何を移管 すべきか読み取れるようにした。 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
aloekun
added a commit
that referenced
this pull request
Aug 8, 2026
#366) * feat(nightly-todo): 改ざん検知を red 化し決定 10 の色分けと決定 6 の列挙基準を明文化する (ADR-072) #363 の最終 push が pre-push security review の REJECT で止まったため master に 載らなかった 3 点を、master 上に載せ直す。いずれも可観測性と文書の改善で、 fail-closed 自体は master 版でも成立している。 ## 改ざん検知を green から red へ (workflow) ゲート資産の改ざん検知 step から continue-on-error を除去する。 fail-closed は元から成立していた — continue-on-error: true でも下流の `if: steps.integrity.outcome == 'success'` で push は止まる。問題は**色**で、 green で終わるため run 一覧上「何もすることが無かった夜」と区別が付かなかった。 毎晩回る無人ループでは、この 2 つが混ざった時点で run 一覧が読まれなくなる。 改ざん検知は「何かがゲートを無効化しようとした」という、この系が出しうる最も 大きい信号である。red で落ちても後続 step は if: の評価前に skip されるため push には到達せず、Report outcome は if: '!cancelled()' なので診断行は出る。 ## 決定 10 に色分け表を追加 (ADR-072) 「設計された停止」と「インフラ障害」の 2 分類を表にしたところ、**改ざん検知が どちらにも入っていない**ことが露出した。分類を明文化すると分類に入らない結末が 可視になる、の実例として § 静的レビューが捕捉した件 の #10 に記録した。 見落としていたのは安全性ではなく可観測性の側だった。 ## 決定 6 に列挙基準を追加 (ADR-072) 禁止リストの基準は「危険か」ではなく「**将来の無人 run のゲートを緩めるか**」で ある。security review が挙げた Cargo.toml / Cargo.lock の欠落を採らない根拠が これで、通常の diff は人間の PR レビューとマージという既存の防衛線が効く。 基準を持たないと禁止リストは「怪しいもの全部」へ膨らみ正当なタスクを弾き始める。 ## 適用方法 保持していたローカル bookmark (wp18/unpushed-improvements) は #363 マージ前の スタックのため、そのまま復元すると **#364 で入れた ADR-072 § 残課題 の追記 (外部設定の実体が未記録 / 秘密値は記録しない) を巻き戻す**。したがって workflow は ファイル単位で restore し、ADR-072 は追加分 4 箇所のみ手で適用した。適用後に lpzvttwu との差分が #364 の 1 行だけであることを実測確認している。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(nightly-todo): 決定 10 の red 分類を残り 3 step へ適用する (#366 CodeRabbit 指摘) 自動 fix 経路が作った commit を、内容を実測検証したうえで採用したもの。 **push 自体は scope guard (ADR-054) が BLOCK した** — finding の anchor が docs/adr/adr-072 なのに fix が .github/workflows/ を触ったため。指摘の remedy が anchor と別ファイルにある典型で、guard の設計どおりの挙動だが本件は injection では ないため誤検知にあたる (WP-11 の enforce 期間の観測データとして計上すべき)。 ## 指摘の妥当性 同 PR で追加した決定 10 の色分け表は「gh / network / clone の失敗 → red」と 定めているのに、その 3 経路が continue-on-error: true で green に落ちていた。 表を追加した PR 自身が作った不整合であり、妥当と判断して採用する。 - Prepare a clean publish tree — git clone (ネットワーク I/O) - Mint App token — GitHub API 呼び出し (secret 誤設定・GitHub 障害) - Push branch and open draft PR — git push / gh pr create ## 実測検証 (fix の出力を鵜呑みにしない) - 下流の if: はいずれも `steps.<id>.outcome == 'success'` 形式のため、失敗時は 後続が skip され push へ到達しない (fail-closed は維持) - Report outcome は `if: '!cancelled()'` なので red でも診断行は出る - dry_run=true では app-token / publish は if: により **skipped** (failed ではない) ため、dry_run の run は green のまま ## ADR への記帳 表を書いた著者自身は 1 件 (改ざん検知) しか見つけられず、残り 3 件は他者の レビューで出た。**分類の明文化は露出の必要条件であって十分条件ではない**ことの 実例として決定 10 へ追記した。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
gh pr create/git push/jj git pushを検出し、CronCreate で定期監視を開始 (29テスト)[post_pr_monitor]セクション追加（ポーリング間隔、最大監視時間、CI/CodeRabbit 個別有効化）レビュー指摘対応 (Critical 1件 + High 10件)
Test plan
cargo test— check-ci-coderabbit 37テスト通過cargo test— hooks-post-pr-monitor 29テスト通過pnpm build:hooks— 全 exe ビルド成功pnpm push— push パイプライン (test → review → push) 通過🤖 Generated with Claude Code
Summary by CodeRabbit
新機能
ドキュメント
Chores