Skip to content

fix(review-request): 冪等ガードの偽陽性を直す — 投稿者と本文の厳密一致で判定する - #496

Merged
aloekun merged 1 commit into
masterfrom
fix/review-request-idempotency-guard
Sep 11, 2026
Merged

aloekun merged 1 commit into
masterfrom
fix/review-request-idempotency-guard

Conversation

@aloekun

@aloekun aloekun commented Sep 11, 2026

Copy link
Copy Markdown
Owner

問題

review-request.yml の冪等ガードが、既存のレビュー要求を本文の部分一致で探していた。

select(.body | contains("@coderabbitai review"))

これはコマンド名に言及しているだけのコメントも拾う。PR #494 で実際に 2 件誤マッチした。

誤マッチしたコメント 該当箇所
CodeRabbit の skip 通知 「To trigger a single review, invoke the @coderabbitai review command.」
pr-monitor の分析コメント @coderabbitai review が投稿できていないため」

どちらもレビュー要求ではない。 誤認すると「既に要求済み」と判断して投稿を飛ばし、レビューが永久に走らない

skip 通知は bot 作成 PR に必ず付き、しかも本 workflow の起動と数秒差で投稿される (2026-09-10 の run は起動 18:14:33 / 通知 18:14:34)。部分一致のままでは競合の結果次第で挙動が変わる。pr-monitor の分析コメントは後から付くため、再実行では必ず誤マッチする。

修正

投稿者と本文の厳密一致で判定する。

  • 本文が前後の空白を除いて @coderabbitai review と完全一致すること
  • 投稿者が Bot でないこと (本 workflow は PAT の人間 identity で投稿する。誤マッチした 2 件はどちらも Bot だった)

空白の除去に \s を使わない。 jq は \s を文字列エスケープとして受け付けず、sub("^\\s+";"") は parse エラーで全件 0 件に落ちる。0 件は「既存要求なし」と読まれるため、壊れていても投稿は続き失敗が静かになる。実装中に実際に踏んだので、POSIX 文字クラスを使う理由をコード内コメントに残した。

検証 (実 PR に対する実測)

新旧の判定を実 PR に当てて比較した。

PR 内訳
#494 2 1 偽陽性 2 件を除外、本物のトリガーのみ
#461 1 1 本物のトリガー、非退行
#477 2 1 偽陽性を除外
#459 2 1 偽陽性を除外
#483 2 2 両方 aloekun の本物 (手動再試行を含む)
#487 2 2 同上

本物のトリガーは 1 件も落ちていない。偽陽性 2 件を個別に新判定へ通し、どちらも出力が空 (除外) であることも確認した。

run: ブロックをファイルから切り出して bash -e で実行し、#494#461 で本物のトリガーを検出して投稿せず exit 0 すること、since_id が正しく出ることを実測した (この job は bot 作成 PR でしか起動しないため、GHA -e convention に記載のローカル再現手段を使う)。

pnpm lint:workflows (契約検査 3 を含む) green。

補足: 過去の失敗の分類を訂正

調査の過程で、以前「PAT は 2026-09-06 に失効した」とした認識が誤りだったことが判明した。各 run の失敗 step を確認すると、09-06 と 09-07 は投稿 step が success で Verify CodeRabbit actually reviewed が失敗しており、認証エラーは 09-10 の #494 が初出だった。

日付 PR 失敗した step 性質
2026-09-06 #483 Verify CodeRabbit actually reviewed 投稿成功、レビュー未着
2026-09-07 #487 Verify CodeRabbit actually reviewed 投稿成功、レビュー未着
2026-09-10 #494 Post the review request 認証エラー (401)

対象外

  • security review が非ブロッキング警告を 1 件出している。第三者が同一本文の非 Bot コメントを先行投稿すればレビュー要求をスキップさせ得るという指摘で、旧実装より誤マッチ範囲は狭く、本 diff が持ち込んだ経路ではないため対応しない

🤖 Generated with Claude Code

Summary by CodeRabbit

  • バグ修正
    • CodeRabbitへのレビュー要求コメントを、投稿者がBotではなく、本文が空白除去後に「@coderabbitai review」と完全一致する場合のみ既存要求として認識するよう変更。
    • スキップ通知や分析コメントが、レビュー要求として誤認される問題を修正。

@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 65e7c579-1d5f-4824-978e-99eaa7d3851f

📥 Commits

Reviewing files that changed from the base of the PR and between b6db53f and b529a07.

📒 Files selected for processing (1)
  • .github/workflows/review-request.yml

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

既存レビュー要求コメントの検出条件を厳密化しました。Bot 投稿と部分一致を除外し、空白除去後に @coderabbitai review と完全一致するコメントだけを検出します。

Changes

レビュー要求検出

Layer / File(s) Summary
レビュー要求コメントの完全一致判定
.github/workflows/review-request.yml
投稿者が Bot ではないことを確認し、本文の前後空白を除去して @coderabbitai review と完全一致する場合だけ既存要求として扱います。[[:space:]] を使用して jq の解析エラーを回避します。

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to b529a

The stricter review-request detection is ready to merge.

🚥 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 タイトルは、投稿者と本文の厳密一致によってレビュー要求の偽陽性を修正する主変更を正確に示しています。簡潔で具体的です。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/review-request-idempotency-guard

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

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

@github-actions

Copy link
Copy Markdown
Contributor

🤖 PR Monitor 分析 (GitHub Actions バックストップ)

  • トリガー: issue_comment (created) / 実行 run
  • CI: rust (ubuntu-latest) pending / rust (windows-latest) pending / request: skipping / CodeRabbit: pending (Review in progress)
  • レビュー状況: 未実施 (陽性証拠なし) — pulls/496/reviews は 0 件、インライン指摘も 0 件。会話コメントは CodeRabbit の "Currently processing new changes... please wait" (処理中通知) のみで、要約/walkthrough 完了コメントではない。head (b529a07d) に対する完了済みレビューの証拠が無いため、CI 状態や mergeStateStatus: BLOCKED を根拠に approved とはしない。

レビュー指摘 0 件のため、CI状態と diff 概要のみ

  • 変更ファイル: .github/workflows/review-request.yml (1 ファイル)
  • 変更内容: 既存レビュー要求コメントの重複判定を contains("@coderabbitai review") の部分一致から、user.type != "Bot" かつ本文の前後空白除去後の厳密一致 (sub("^[[:space:]]+";"") / sub("[[:space:]]+$";"")) に変更。PR fix(jj-op-verify): commit message 内の文言を実行と誤認しない (nightly-todo 順位 476) #494 で観測された誤マッチ (CodeRabbit の skip 通知、pr-monitor 分析コメントがコマンド名を引用するケース) を防ぐ修正。
  • コメントで \s ではなく POSIX 文字クラス [[:space:]] を使う理由 (jq\s をエスケープとして受け付けず全件 0 件に落ちる、という 2026-09-11 の実測) が明記されており、設計判断の根拠は diff 内に閉じている。
  • CodeRabbit のレビューは処理中のため、この時点でのインライン指摘は無い。

次のアクション

  • CodeRabbit のレビュー完了 (walkthrough/summary コメント、またはインライン指摘) を待ってから再評価する。
  • rust (ubuntu/windows) の CI 結果が出た時点で failure がないか確認する。

@aloekun
aloekun merged commit 1793207 into master Sep 11, 2026
4 checks passed
@aloekun
aloekun deleted the fix/review-request-idempotency-guard branch September 11, 2026 19:29
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