fix(hooks): CodeRabbit レビュー対応 + ポーリングロジック修正 - #11
Merged
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughCodeRabbitのレビュー判定でコメントフィルタと制御フローが変更され、CI「pending」判定が Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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/hooks-post-pr-monitor/src/main.rs (2)
89-91: 正規表現パターンに^\s*アンカーがありません。他のパターン（
PAT_GH_PR_CREATE、PAT_GIT_PUSH、PAT_JJ_GIT_PUSH）はすべて^\s*で始まっていますが、PAT_PNPM_PUSHにはこのアンカーがありません。これにより、echo pnpm pushや# pnpm pushのようなコマンドでも誤検出する可能性があります。♻️ 他のパターンと一貫性を持たせる修正案
-/// pnpm push / npm push / pnpm run push (パイプライン経由の push) -const PAT_PNPM_PUSH: &str = r"(?:pnpm|npm)\s+(?:run\s+)?push(\s|$)"; +/// pnpm push / npm push / pnpm run push (パイプライン経由の push) +const PAT_PNPM_PUSH: &str = r"^\s*(?:pnpm|npm)\s+(?:run\s+)?push(\s|$)";🤖 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 89 - 91, PAT_PNPM_PUSH is missing the leading anchor used by the other patterns, causing false positives (e.g., "echo pnpm push" or commented lines); update the constant PAT_PNPM_PUSH to start with the same anchor sequence (^\s*) as PAT_GH_PR_CREATE, PAT_GIT_PUSH, and PAT_JJ_GIT_PUSH so the regex only matches when the command appears at the start of a line (optionally preceded by whitespace).
469-491: テストカバレッジは良好ですが、エッジケースの追加を検討してください。正の検出ケース（
pnpm push、pnpm run push、npm push）と負のケース（pnpm build）をカバーしています。先ほど指摘した正規表現のアンカー修正後、echo pnpm pushのような誤検出を防ぐテストを追加すると、パターンの堅牢性を確認できます。🧪 追加テストの提案
#[test] fn no_trigger_echo_pnpm_push() { let patterns = default_patterns(); assert!(!is_trigger_command("echo pnpm push", &patterns)); }🤖 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 469 - 491, Add a negative edge-case test to ensure commands like "echo pnpm push" don't falsely match the trigger regex: create a test function (e.g., no_trigger_echo_pnpm_push) that calls default_patterns() and asserts !is_trigger_command("echo pnpm push", &patterns); place it alongside the existing tests (trigger_pnpm_push, trigger_pnpm_run_push, trigger_npm_push, no_trigger_pnpm_build) to verify the anchored regex change prevents this false positive.
🤖 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/hooks-post-pr-monitor/src/main.rs:
- Around line 89-91: PAT_PNPM_PUSH is missing the leading anchor used by the
other patterns, causing false positives (e.g., "echo pnpm push" or commented
lines); update the constant PAT_PNPM_PUSH to start with the same anchor sequence
(^\s*) as PAT_GH_PR_CREATE, PAT_GIT_PUSH, and PAT_JJ_GIT_PUSH so the regex only
matches when the command appears at the start of a line (optionally preceded by
whitespace).
- Around line 469-491: Add a negative edge-case test to ensure commands like
"echo pnpm push" don't falsely match the trigger regex: create a test function
(e.g., no_trigger_echo_pnpm_push) that calls default_patterns() and asserts
!is_trigger_command("echo pnpm push", &patterns); place it alongside the
existing tests (trigger_pnpm_push, trigger_pnpm_run_push, trigger_npm_push,
no_trigger_pnpm_build) to verify the anchored regex change prevents this false
positive.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e49d252e-8f28-4509-a629-eee1e79f8454
⛔ Files ignored due to path filters (1)
.claude/post-pr-monitor-debug.logis excluded by!**/*.log
📒 Files selected for processing (2)
.claude/check-ci-coderabbit/src/main.rs.claude/hooks-post-pr-monitor/src/main.rs
aloekun
force-pushed
the
fix/post-pr-monitor-review
branch
from
April 2, 2026 13:44
a58b41b to
e5a5b4b
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 381-387: The current logic treats an empty ci.runs as "no_ci",
which conflates a true no-CI repo with failures from `gh run list`; add a
distinct ci_fetch_failed state and ensure the code that calls `gh run list` sets
ci_fetch_failed on API/timeout/error instead of returning an empty runs Vec,
keep no_ci only when you’ve positively determined there are no runs, and update
the evaluation logic (symbols: ci, ci_pending, cr_pending, no_ci,
ci_fetch_failed, ci.overall, ci.runs) so that ci_pending only considers runs
when they were successfully fetched and does not skip monitoring when
ci_fetch_failed is true.
- Around line 372-378: The JSON `action` and human `summary` diverge when
cr.review_state == "not_found" and has_actionable is true; update build_summary
to consider has_actionable for the not_found case (the same condition checked
where cr.review_state and has_actionable are evaluated) so the produced summary
reflects actionable comments (e.g., treat not_found+has_actionable as "action
required" or append an actionable note) and keep the unique symbols: adjust
build_summary to inspect cr.review_state == "not_found" && has_actionable and
return a matching human-readable summary to align with the earlier
action_required return.
In @.claude/hooks-post-pr-monitor/src/main.rs:
- Around line 137-141: The detect_command_type function incorrectly returns
"pnpm push" whenever PAT_PNPM_PUSH matches, causing npm push to be
misclassified; update the logic in detect_command_type to distinguish npm vs
pnpm by checking the exact package manager token (e.g., test the command string
for "pnpm" vs "npm" or refine PAT_PNPM_PUSH to only match pnpm invocations) and
return "npm push" or "pnpm push" accordingly; apply the same fix to the
duplicate block referenced around the other instance (the block at the lines
mentioned) and add/adjust unit tests to assert both "npm push" and "pnpm push"
are returned correctly by detect_command_type.
- Around line 89-90: The code introduced a new combined default trigger constant
PAT_PNPM_PUSH but the configuration docs/sample in .claude/hooks-config.toml
were not updated; update the documentation and example to reflect the new
default trigger (PAT_PNPM_PUSH) and its combined regex (pnpm/npm (run )?push) so
users who copy the explicit settings don't miss the new pattern, and also adjust
any descriptive text that previously referenced the three separate triggers to
mention the consolidated default (see the constant name PAT_PNPM_PUSH and the
defaultization around the earlier change).
🪄 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: d8626f4b-439e-4fa9-afea-61b29a363178
⛔ Files ignored due to path filters (1)
.claude/post-pr-monitor-debug.logis excluded by!**/*.log
📒 Files selected for processing (2)
.claude/check-ci-coderabbit/src/main.rs.claude/hooks-post-pr-monitor/src/main.rs
Comment on lines
+89
to
+90
| /// pnpm push / npm push / pnpm run push (パイプライン経由の push) | ||
| const PAT_PNPM_PUSH: &str = r"^\s*(?:pnpm|npm)\s+(?:run\s+)?push(\s|$)"; |
There was a problem hiding this comment.
デフォルトトリガー追加に対して設定ドキュメントが未追従です
Line 97 で PAT_PNPM_PUSH をデフォルト化した一方、.claude/hooks-config.toml の説明（Line 106）とサンプル（Line 118）が旧3種のままです。明示設定をコピペした利用者が新トリガーを落とす可能性があります。
Also applies to: 97-98
🤖 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 89 - 90, The code
introduced a new combined default trigger constant PAT_PNPM_PUSH but the
configuration docs/sample in .claude/hooks-config.toml were not updated; update
the documentation and example to reflect the new default trigger (PAT_PNPM_PUSH)
and its combined regex (pnpm/npm (run )?push) so users who copy the explicit
settings don't miss the new pattern, and also adjust any descriptive text that
previously referenced the three separate triggers to mention the consolidated
default (see the constant name PAT_PNPM_PUSH and the defaultization around the
earlier change).
CR#1: build_summary で not_found + actionable 時にコメント数を表示 CR#2: gh run list の API エラー時に runs 空ではなく pending (runs非空) を返す → API エラーとCI未設定を区別し、誤った CI スキップを防止 CR#3: hooks-config.toml のコメント/サンプルに pnpm push を追加 → poll_interval_secs を 30 → 120 (2分) に変更 CR#4: detect_command_type で npm push を pnpm push と区別 テスト: 77 (36 + 41) パス Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
aloekun
force-pushed
the
fix/post-pr-monitor-review
branch
from
April 2, 2026 15:03
e5a5b4b to
79292f4
Compare
This was referenced Apr 2, 2026
6 tasks
6 tasks
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
CodeRabbit レビュー対応 (PR feat(hooks): Post-PR Monitor — CI・CodeRabbit自動監視 #10 の未解決3件)
run_gh_quietをCommand::new("gh")に統一 (cmd /c除去)debug_log関数を削除しeprintln!に置き換えポーリングロジック修正
review_state: not_foundでもコメント/スレッドがあればaction_requiredを返すマージ漏れ修正
PAT_PNPM_PUSHトリガーパターン +detect_command_type追加Test plan
cargo test— hooks-post-pr-monitor 34テスト通過cargo test— check-ci-coderabbit 40テスト通過pnpm build:hooks— 全 exe ビルド成功pnpm push— push パイプライン (test → review → push) 通過🤖 Generated with Claude Code
Summary by CodeRabbit
新機能
改善
テスト
その他