feat: チェックが成功していない PR をメンション対象から外す（#105） - #107
Conversation
CI・自動チェックが失敗中/実行中の PR はレビューできる状態になっていない ことが多く、@channel / @everyone で急かす必要がない。 - ChecksState を追加し、check runs と commit status の 2 系統を厳しい方に 畳んで判定する - 判定対象は PullRequest かつ機械的に発生する reason のみ。人が明示的に 呼んだ mention / team_mention はチェック状態に関わらずメンションする - チェック未設定・取得失敗はメンションを維持する（見逃しより誤メンションを 許容する安全側の判断） 通知そのものは従来どおり送り、メンションの有無だけを変える。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MoGxJDiZa8c2E3PVAnxbK8
|
CI が赤ですが、本 PR の差分とは無関係の既存問題です。
#108 に切り出し、修正 PR を #109 で用意しました（Crystal 1.21.0 固定 + ameba を対応コミットに固定）。#109 がマージされたら本 PR に master を取り込んで CI を確認します。 Generated by Claude Code |
…-policy-to-issues-vdp4s3-105
|
#109（CI 修正）が green になったので、本ブランチに取り込んで CI を回しています。 そのため現時点の diff には #109 のコミットが含まれています。#109 が master にマージされれば差分から消えるので、レビューは本 PR 固有のコミット（ Generated by Claude Code |
チェック状態取得まわりで追加した `return nil` を `return` にする。 いずれも戻り値が nilable なメソッドで、挙動は変わらない。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MoGxJDiZa8c2E3PVAnxbK8
…o-issues-vdp4s3-105
#106（issue #104）のマージにより master へ入った通知文言の変更を取り込む。 競合はテストヘルパーの引数追加が両側で起きたもので、双方の引数を残して解消: - spec/github/models_spec.cr: notification_from に type と url/latest_comment_url - spec/github/usecase_spec.cr: notification に type と latest_comment_url src/github/models.cr は変更箇所が異なるため自動マージ（#104 の FOLLOWUP_MESSAGES / commented? と #105 の ChecksState / checks_gated? が併存）。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MoGxJDiZa8c2E3PVAnxbK8
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f56df0784
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
reason は購読理由であってイベント種別ではないため、一度メンションされた PR はその後の push やコメントでも reason=mention のまま届く。これを CI ゲートの 対象外にすると、最も関与している PR でこそチェックが赤いまま @channel を 撃つことになり本末転倒だった。 mention / team_mention の例外を削除し、PR の通知はすべてチェック状態で 制御する。メンションされた通知自体は従来どおり届き、@channel が付かなく なるだけ。 PR #107 のレビュー指摘対応。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MoGxJDiZa8c2E3PVAnxbK8
2 participants
close #105
課題
mention 系 reason の通知は
@channel/@everyoneでチャンネル全体を叩くが、CI・自動チェックがまだ成功していない PR はレビューできる状態になっていないことが多く、その時点で急かす必要がない。変更内容
通知そのものは従来どおり送り、メンションの有無だけを変える。
Github::ChecksState（Success/Pending/Failure/NoChecks/Unknown）を追加#mergeで厳しい方を採る#blocks_mention?がFailure/Pendingのときだけメンションを抑止するNotificationRepository#find_checks_stateを追加。PR の head SHA を取得し、check-runs と combined status を引くUsecase#build_messageのmentionをnotify.mention?からチェック状態込みの判定に変更判定ルール
Failure/PendingSuccessNoChecks）Unknown）判断の軸として、通知の見逃しより誤メンションの方が軽いと考え、状態が分からない場合は必ずメンション側に倒している。チェック未設定のリポジトリを巻き込まないよう、
NoChecksも抑止しない。neutral/skippedの conclusion は「実行された上で通していい」結果として成功側に含めている。commit status が 0 件のときは API がstate: "pending"を返すため、total_countで先にNoChecksへ振り分けている。reason による例外を設けていない理由
当初は
mention/team_mentionを「人が明示的に呼んだ」ものとしてチェックゲートの対象外にしていたが、レビュー指摘を受けて取り下げた。reasonは購読理由であってイベント種別ではないため、一度メンションされた PR はその後の push やコメントでもreason=mentionのまま届く。例外にすると、最も関与している PR でこそチェックが赤いままチャンネル全体を叩くことになり、本 PR の目的と正反対だった。「今回の更新が実際にメンションだったか」を判別する案（コメント本文の
@loginを見る等）も検討したが、自分の login を得る追加 API が必要・team_mention（@org/team）は判定不可・今回取得したコメント以外のメンションを拾えない、という制約があるため見送っている。メンションされた通知自体は従来どおり届き、
@channelが付かなくなるだけなので見逃しにはならない。（レビュー指摘対応: #discussion_r3744388810）
API コスト
抑止判定の対象になる PR 通知 1 件あたり +3 リクエスト（PR / check-runs / status）。未読通知のみが対象なので許容範囲と判断した。同一 PR に複数通知が来た場合はその都度引き直すため、頻度が問題になるならキャッシュを別途検討する。
check runs は
per_page=100の 1 ページのみ取得し、ページングはしていない（100 を超えるチェックは想定しにくいため）。テスト
ChecksState#merge/#blocks_mention?の各組み合わせCheckRuns#checks_state（成功 / 失敗 / 実行中 / チェック無し）CombinedStatus#checks_state（success / failure / error / pending / 0 件）Notification#checks_gated?（PR は reason によらず true、PR 以外は false）Usecase#build_messageのメンション判定と、対象外のときにチェックを引かないこと