From 60e3a6acc673e80b89a4a1164d4fbc8aa372e7fc Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 08:32:07 +0000 Subject: [PATCH 1/3] =?UTF-8?q?feat:=20=E3=83=81=E3=82=A7=E3=83=83?= =?UTF-8?q?=E3=82=AF=E3=81=8C=E6=88=90=E5=8A=9F=E3=81=97=E3=81=A6=E3=81=84?= =?UTF-8?q?=E3=81=AA=E3=81=84=20PR=20=E3=82=92=E3=83=A1=E3=83=B3=E3=82=B7?= =?UTF-8?q?=E3=83=A7=E3=83=B3=E5=AF=BE=E8=B1=A1=E3=81=8B=E3=82=89=E5=A4=96?= =?UTF-8?q?=E3=81=99=EF=BC=88#105=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI・自動チェックが失敗中/実行中の PR はレビューできる状態になっていない ことが多く、@channel / @everyone で急かす必要がない。 - ChecksState を追加し、check runs と commit status の 2 系統を厳しい方に 畳んで判定する - 判定対象は PullRequest かつ機械的に発生する reason のみ。人が明示的に 呼んだ mention / team_mention はチェック状態に関わらずメンションする - チェック未設定・取得失敗はメンションを維持する（見逃しより誤メンションを 許容する安全側の判断） 通知そのものは従来どおり送り、メンションの有無だけを変える。 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MoGxJDiZa8c2E3PVAnxbK8 --- spec/github/models_spec.cr | 112 +++++++++++++++++++++++++++++++++- spec/github/usecase_spec.cr | 75 ++++++++++++++++++++++- src/github/models.cr | 118 ++++++++++++++++++++++++++++++++++++ src/github/repository.cr | 73 ++++++++++++++++++++++ src/github/usecase.cr | 14 ++++- 5 files changed, 386 insertions(+), 6 deletions(-) diff --git a/spec/github/models_spec.cr b/spec/github/models_spec.cr index 6f2449e..ce6c7df 100644 --- a/spec/github/models_spec.cr +++ b/spec/github/models_spec.cr @@ -141,6 +141,24 @@ describe Github::Notification do end end + describe "#checks_gated?" do + it "is true for pull request notifications with a machine-driven reason" do + notification_from("review_requested", type: "PullRequest").checks_gated?.should be_true + notification_from("assign", type: "PullRequest").checks_gated?.should be_true + notification_from("author", type: "PullRequest").checks_gated?.should be_true + end + + it "is false when a human explicitly mentioned the user" do + notification_from("mention", type: "PullRequest").checks_gated?.should be_false + notification_from("team_mention", type: "PullRequest").checks_gated?.should be_false + end + + it "is false for non pull request subjects" do + notification_from("review_requested", type: "Issue").checks_gated?.should be_false + notification_from("author", type: "Commit").checks_gated?.should be_false + end + end + it "parses a GitHub notifications API payload" do notifications = Array(Github::Notification).from_json(NOTIFICATIONS_FIXTURE) notifications.size.should eq 1 @@ -155,10 +173,10 @@ describe Github::Notification do end end -private def notification_from(reason : String) +private def notification_from(reason : String, type = "Issue") Github::Notification.from_json({ reason: reason, - subject: {type: "Issue", title: "title"}, + subject: {type: type, title: "title"}, repository: {owner: {login: "octocat"}}, updated_at: "2026-07-14T00:00:00Z", }.to_json) @@ -173,6 +191,96 @@ private def notification_with(url = "", repo_html_url : String? = nil) }.to_json) end +describe Github::ChecksState do + describe "#merge" do + it "takes the stricter state of the two" do + Github::ChecksState::Success.merge(Github::ChecksState::Failure).should eq Github::ChecksState::Failure + Github::ChecksState::Pending.merge(Github::ChecksState::Failure).should eq Github::ChecksState::Failure + Github::ChecksState::Success.merge(Github::ChecksState::Pending).should eq Github::ChecksState::Pending + Github::ChecksState::NoChecks.merge(Github::ChecksState::Success).should eq Github::ChecksState::Success + end + + it "keeps the other state when one side could not be fetched" do + Github::ChecksState::Unknown.merge(Github::ChecksState::Success).should eq Github::ChecksState::Success + Github::ChecksState::Failure.merge(Github::ChecksState::Unknown).should eq Github::ChecksState::Failure + end + + it "stays unknown when neither side could be fetched" do + Github::ChecksState::Unknown.merge(Github::ChecksState::Unknown).should eq Github::ChecksState::Unknown + end + + it "stays no-checks when neither side has any check" do + Github::ChecksState::NoChecks.merge(Github::ChecksState::NoChecks).should eq Github::ChecksState::NoChecks + end + end + + describe "#blocks_mention?" do + it "blocks while checks are failing or still running" do + Github::ChecksState::Failure.blocks_mention?.should be_true + Github::ChecksState::Pending.blocks_mention?.should be_true + end + + it "does not block on success, no checks, or a failed lookup" do + Github::ChecksState::Success.blocks_mention?.should be_false + Github::ChecksState::NoChecks.blocks_mention?.should be_false + Github::ChecksState::Unknown.blocks_mention?.should be_false + end + end +end + +describe Github::CheckRuns do + describe "#checks_state" do + it "is success when every run completed without a blocking conclusion" do + check_runs_from([ + {status: "completed", conclusion: "success"}, + {status: "completed", conclusion: "skipped"}, + {status: "completed", conclusion: "neutral"}, + ]).checks_state.should eq Github::ChecksState::Success + end + + it "is failure when a completed run has a blocking conclusion" do + check_runs_from([ + {status: "completed", conclusion: "success"}, + {status: "completed", conclusion: "failure"}, + ]).checks_state.should eq Github::ChecksState::Failure + end + + it "is pending while a run has not completed" do + check_runs_from([ + {status: "completed", conclusion: "success"}, + {status: "in_progress", conclusion: nil}, + ]).checks_state.should eq Github::ChecksState::Pending + end + + it "is no-checks when the commit has no check run" do + check_runs_from([] of NamedTuple(status: String, conclusion: String?)).checks_state.should eq Github::ChecksState::NoChecks + end + end +end + +describe Github::CombinedStatus do + describe "#checks_state" do + it "maps the combined state" do + combined_status_from("success", 2).checks_state.should eq Github::ChecksState::Success + combined_status_from("failure", 2).checks_state.should eq Github::ChecksState::Failure + combined_status_from("error", 2).checks_state.should eq Github::ChecksState::Failure + combined_status_from("pending", 2).checks_state.should eq Github::ChecksState::Pending + end + + it "is no-checks when the commit has no status, even though the api reports pending" do + combined_status_from("pending", 0).checks_state.should eq Github::ChecksState::NoChecks + end + end +end + +private def check_runs_from(runs) + Github::CheckRuns.from_json({check_runs: runs}.to_json) +end + +private def combined_status_from(state : String, total_count : Int32) + Github::CombinedStatus.from_json({state: state, total_count: total_count}.to_json) +end + NOTIFICATIONS_FIXTURE = <<-JSON [ { diff --git a/spec/github/usecase_spec.cr b/spec/github/usecase_spec.cr index 029214e..4c84be1 100644 --- a/spec/github/usecase_spec.cr +++ b/spec/github/usecase_spec.cr @@ -2,25 +2,33 @@ require "../spec_helper" require "../../src/github/repository" require "../../src/github/usecase" -# HTTP を張らずに、あらかじめ用意した Comment を返すリポジトリ。 +# HTTP を張らずに、あらかじめ用意した Comment とチェック状態を返すリポジトリ。 private class StubRepo < Github::NotificationRepository - def initialize(@comment : Github::Comment) + getter checks_calls = 0 + + def initialize(@comment : Github::Comment, @checks_state : Github::ChecksState = Github::ChecksState::NoChecks) super("token") end def find_comment_by_url(url : String) : Github::Comment @comment end + + def find_checks_state(notify : Github::Notification) : Github::ChecksState + @checks_calls += 1 + @checks_state + end end private def notification( url = "https://api.github.com/repos/octocat/Hello-World/issues/42", reason = "review_requested", repo_html_url : String? = "https://github.com/octocat/Hello-World", + type = "Issue", ) Github::Notification.from_json({ reason: reason, - subject: {type: "Issue", title: "Spurious failure", url: url}, + subject: {type: type, title: "Spurious failure", url: url}, repository: {full_name: "octocat/Hello-World", html_url: repo_html_url, owner: {login: "octocat"}}, updated_at: "2026-07-14T00:00:00Z", }.to_json) @@ -67,4 +75,65 @@ describe Github::Usecase do build(notification, comment(body: nil)).text.should be_nil end end + + describe "#build_message mention" do + it "mentions on a pull request whose checks all succeeded" do + message = build_with_checks(pull_request, Github::ChecksState::Success) + message.mention?.should be_true + end + + it "does not mention while a pull request has failing checks" do + build_with_checks(pull_request, Github::ChecksState::Failure).mention?.should be_false + end + + it "does not mention while a pull request still has running checks" do + build_with_checks(pull_request, Github::ChecksState::Pending).mention?.should be_false + end + + it "mentions when the pull request has no checks configured" do + build_with_checks(pull_request, Github::ChecksState::NoChecks).mention?.should be_true + end + + it "mentions when the checks state could not be fetched" do + build_with_checks(pull_request, Github::ChecksState::Unknown).mention?.should be_true + end + + it "still mentions on a failing pull request when a human mentioned the user" do + notify = pull_request(reason: "mention") + build_with_checks(notify, Github::ChecksState::Failure).mention?.should be_true + end + + it "keeps non-mention reasons unmentioned regardless of the checks state" do + notify = pull_request(reason: "subscribed") + build_with_checks(notify, Github::ChecksState::Success).mention?.should be_false + end + + it "does not look up checks for non pull request subjects" do + repo = StubRepo.new(comment, Github::ChecksState::Failure) + message = Github::Usecase.new(repo).build_message(notification(reason: "review_requested")) + + message.mention?.should be_true + repo.checks_calls.should eq 0 + end + + it "does not look up checks for reasons that never mention" do + repo = StubRepo.new(comment, Github::ChecksState::Failure) + message = Github::Usecase.new(repo).build_message(pull_request(reason: "subscribed")) + + message.mention?.should be_false + repo.checks_calls.should eq 0 + end + end +end + +private def pull_request(reason = "review_requested") + notification( + url: "https://api.github.com/repos/octocat/Hello-World/pulls/42", + reason: reason, + type: "PullRequest", + ) +end + +private def build_with_checks(notify, checks_state) + Github::Usecase.new(StubRepo.new(comment, checks_state)).build_message(notify) end diff --git a/src/github/models.cr b/src/github/models.cr index 15fc9ab..9db110b 100644 --- a/src/github/models.cr +++ b/src/github/models.cr @@ -1,6 +1,35 @@ require "json" module Github + # PR の CI・自動チェックの集計状態（issue #105）。 + # check runs（GitHub Actions 等）と commit status の 2 系統をまとめて表す。 + enum ChecksState + Success # 全て完了し、ブロックする結果が無い + Pending # 未完了のチェックがある + Failure # 失敗したチェックがある + NoChecks # チェックが 1 つも設定されていない + Unknown # 取得できなかった + + # 2 系統の結果を 1 つに畳む。厳しい方（Failure > Pending > Success > + # NoChecks）を採る。Unknown は「情報が無い」だけなので、もう片方が + # 取得できていればそちらの結果を活かす。 + def merge(other : ChecksState) : ChecksState + return other if unknown? + return self if other.unknown? + return Failure if failure? || other.failure? + return Pending if pending? || other.pending? + return Success if success? || other.success? + NoChecks + end + + # メンションを抑止すべき状態か。 + # 成功・チェック未設定・取得失敗ではメンションする。取得できなかった場合に + # 抑止すると通知の見逃しにつながるため、安全側（誤メンションを許容）に倒す。 + def blocks_mention? : Bool + failure? || pending? + end + end + class Notification include JSON::Serializable @@ -34,6 +63,14 @@ module Github GENERIC_MESSAGE = "なにかあったみたいです。確認してみましょう！" + # 人が明示的に呼んだことを表す reason。CI の状態に関わらずメンションする + # （呼ばれている以上、チェックの成否とは無関係に見てほしいはずなので + # 抑止しない / issue #105）。 + HUMAN_MENTION_REASONS = { + "mention", + "team_mention", + } + getter subject : Subject getter reason : String getter repository : Repository @@ -50,6 +87,14 @@ module Github REASON_MESSAGES[reason]? || GENERIC_MESSAGE end + # CI・自動チェックの状態でメンションを抑止する対象か（issue #105）。 + # レビューできる状態になっていない PR で `@channel` / `@everyone` を撃たない + # ことが目的なので、PR 以外（Issue / Commit 等）と、人が明示的に呼んだ + # mention 系は対象外にする。 + def checks_gated? : Bool + subject.type == Subject::Type::PULL_REQUEST && !reason.in?(HUMAN_MENTION_REASONS) + end + # 通知の pretext（botのセリフ）。`[] ` 形式。 def pretext : String "[#{subject.type}] #{reason_message}" @@ -180,6 +225,79 @@ module Github end end + # チェック状態の判定に使う PR 情報。head の SHA だけ参照する（issue #105）。 + class PullRequest + include JSON::Serializable + + getter head : Head + + class Head + include JSON::Serializable + + getter sha : String + end + end + + # GET /repos/:owner/:repo/commits/:ref/check-runs のレスポンス（issue #105）。 + class CheckRuns + include JSON::Serializable + + # ブロックしない conclusion。neutral / skipped は「実行された上で + # 通していい」結果なので成功側に含める。 + PASSING_CONCLUSIONS = { + "success", + "neutral", + "skipped", + } + + getter check_runs : Array(CheckRun) = [] of CheckRun + + def checks_state : ChecksState + return ChecksState::NoChecks if check_runs.empty? + return ChecksState::Pending unless check_runs.all?(&.completed?) + + check_runs.all?(&.passing?) ? ChecksState::Success : ChecksState::Failure + end + + class CheckRun + include JSON::Serializable + + getter status : String = "" + getter conclusion : String? + + def completed? : Bool + status == "completed" + end + + def passing? : Bool + conclusion.in?(PASSING_CONCLUSIONS) + end + end + end + + # GET /repos/:owner/:repo/commits/:ref/status のレスポンス（issue #105）。 + # check runs とは別系統の commit status（外部 CI 等）を表す。 + class CombinedStatus + include JSON::Serializable + + getter state : String = "" + getter total_count : Int32 = 0 + + def checks_state : ChecksState + # status が 1 件も無いと state は "pending" で返るため、件数で先に弾く。 + return ChecksState::NoChecks if total_count.zero? + + case state + when "success" + ChecksState::Success + when "failure", "error" + ChecksState::Failure + else + ChecksState::Pending + end + end + end + class Error include JSON::Serializable diff --git a/src/github/repository.cr b/src/github/repository.cr index c621c05..f074902 100644 --- a/src/github/repository.cr +++ b/src/github/repository.cr @@ -132,6 +132,79 @@ module Github end end + # チェック状態を見るときに一度に取得する check runs の件数。 + # これを超える数のチェックがある PR は一部しか見えないが、GitHub の上限 + # 100 を超えるチェックは想定しにくいためページングはしない（issue #105）。 + CHECKS_PER_PAGE = 100 + + # PR の CI・自動チェックの集計状態を返す（issue #105）。 + # + # check runs（GitHub Actions 等）と commit status（外部 CI 等）は別系統で、 + # 片方にしか結果が出ないことがあるため両方を見て厳しい方を採る。 + # + # 取得できなかった場合は Unknown を返し、呼び出し側でメンションを維持させる。 + # 通知の見逃しより誤メンションの方が軽い、という判断（安全側に倒す）。 + def find_checks_state(notify : Notification) : ChecksState + return ChecksState::Unknown unless repo = notify.repository.full_name.try(&.presence) + return ChecksState::Unknown unless number = notify.subject.number + return ChecksState::Unknown unless sha = find_head_sha "/repos/#{repo}/pulls/#{number}" + + runs = find_check_runs_state "/repos/#{repo}/commits/#{sha}/check-runs?per_page=#{CHECKS_PER_PAGE}" + runs.merge find_combined_status_state("/repos/#{repo}/commits/#{sha}/status?per_page=#{CHECKS_PER_PAGE}") + end + + private def find_head_sha(path : String) : String? + return nil unless body = get_body path + + begin + PullRequest.from_json(body).head.sha.presence + rescue + Serverless::Lambda.print_log "failed parse pull request data" + nil + end + end + + private def find_check_runs_state(path : String) : ChecksState + return ChecksState::Unknown unless body = get_body path + + begin + CheckRuns.from_json(body).checks_state + rescue + Serverless::Lambda.print_log "failed parse check runs data" + ChecksState::Unknown + end + end + + private def find_combined_status_state(path : String) : ChecksState + return ChecksState::Unknown unless body = get_body path + + begin + CombinedStatus.from_json(body).checks_state + rescue + Serverless::Lambda.print_log "failed parse combined status data" + ChecksState::Unknown + end + end + + # チェック状態の取得用 GET。1 件の取得失敗で通知全体を巻き添えにしないよう、 + # 例外・エラー応答はログだけ残して nil を返す（issue #105）。 + private def get_body(path : String) : String? + res = + begin + @github.get path + rescue ex + Serverless::Lambda.print_log "failed to get #{path}: #{ex.message}" + return nil + end + + unless res.success? + Serverless::Lambda.print_log "return #{res.status_code} from #{path}" + return nil + end + + res.body + end + # 通知を既読化する。last_read_at は排他的境界で、その時刻より前 # （updated_at < last_read_at）に更新された通知だけが既読化される。 # 等値（updated_at == last_read_at）は未読のまま残るため、送信済み通知を diff --git a/src/github/usecase.cr b/src/github/usecase.cr index b2574a7..b2f8900 100644 --- a/src/github/usecase.cr +++ b/src/github/usecase.cr @@ -14,7 +14,7 @@ module Github def build_message(notify : Notification) : Notify::Message comment = @repo.find_comment_by_url notify.subject.comment_url Notify::Message.new( - mention: notify.mention?, + mention: mention?(notify), author_name: comment.user.login, author_icon: comment.user.avatar_url, author_link: comment.user.html_url, @@ -28,6 +28,18 @@ module Github ) end + # メンション（`@channel` / `@everyone`）を付けるか。 + # + # mention 系 reason であることに加え、PR は CI・自動チェックが失敗中・実行中 + # でないことを条件にする。まだレビューできる状態ではない PR でチャンネル全体を + # 叩かないため（issue #105）。通知そのものは抑止しない。 + private def mention?(notify : Notification) : Bool + return false unless notify.mention? + return true unless notify.checks_gated? + + !@repo.find_checks_state(notify).blocks_mention? + end + private def truncate_body(body : String?) : String? return nil unless text = body.try(&.presence) text.size > BODY_LIMIT ? "#{text[0, BODY_LIMIT]}…" : text From f97327c9ed0f125162fb807c60270d6a711a696d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 08:47:40 +0000 Subject: [PATCH 2/3] =?UTF-8?q?style:=20ameba=20=E3=81=AE=20Style/Redundan?= =?UTF-8?q?tNilInControlExpression=20=E3=81=AB=E5=AF=BE=E5=BF=9C=EF=BC=88#?= =?UTF-8?q?105=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit チェック状態取得まわりで追加した `return nil` を `return` にする。 いずれも戻り値が nilable なメソッドで、挙動は変わらない。 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MoGxJDiZa8c2E3PVAnxbK8 --- src/github/repository.cr | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/github/repository.cr b/src/github/repository.cr index f074902..f7cd4ef 100644 --- a/src/github/repository.cr +++ b/src/github/repository.cr @@ -154,7 +154,7 @@ module Github end private def find_head_sha(path : String) : String? - return nil unless body = get_body path + return unless body = get_body path begin PullRequest.from_json(body).head.sha.presence @@ -194,12 +194,12 @@ module Github @github.get path rescue ex Serverless::Lambda.print_log "failed to get #{path}: #{ex.message}" - return nil + return end unless res.success? Serverless::Lambda.print_log "return #{res.status_code} from #{path}" - return nil + return end res.body From 5d86066c3718fae574197d7479d1ae3c9fce6859 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 16:09:37 +0000 Subject: [PATCH 3/3] =?UTF-8?q?fix:=20mention=20=E7=B3=BB=20reason=20?= =?UTF-8?q?=E3=82=82=E3=83=81=E3=82=A7=E3=83=83=E3=82=AF=E7=8A=B6=E6=85=8B?= =?UTF-8?q?=E3=81=A7=E3=83=A1=E3=83=B3=E3=82=B7=E3=83=A7=E3=83=B3=E3=82=92?= =?UTF-8?q?=E5=88=B6=E5=BE=A1=E3=81=99=E3=82=8B=EF=BC=88#105=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit reason は購読理由であってイベント種別ではないため、一度メンションされた PR はその後の push やコメントでも reason=mention のまま届く。これを CI ゲートの 対象外にすると、最も関与している PR でこそチェックが赤いまま @channel を 撃つことになり本末転倒だった。 mention / team_mention の例外を削除し、PR の通知はすべてチェック状態で 制御する。メンションされた通知自体は従来どおり届き、@channel が付かなく なるだけ。 PR #107 のレビュー指摘対応。 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MoGxJDiZa8c2E3PVAnxbK8 --- spec/github/models_spec.cr | 22 +++++++++++++--------- spec/github/usecase_spec.cr | 12 ++++++++++-- src/github/models.cr | 23 ++++++++++++----------- 3 files changed, 35 insertions(+), 22 deletions(-) diff --git a/spec/github/models_spec.cr b/spec/github/models_spec.cr index 3b6f7ad..c75cc84 100644 --- a/spec/github/models_spec.cr +++ b/spec/github/models_spec.cr @@ -211,19 +211,23 @@ describe Github::Notification do end describe "#checks_gated?" do - it "is true for pull request notifications with a machine-driven reason" do - notification_from("review_requested", type: "PullRequest").checks_gated?.should be_true - notification_from("assign", type: "PullRequest").checks_gated?.should be_true - notification_from("author", type: "PullRequest").checks_gated?.should be_true - end - - it "is false when a human explicitly mentioned the user" do - notification_from("mention", type: "PullRequest").checks_gated?.should be_false - notification_from("team_mention", type: "PullRequest").checks_gated?.should be_false + it "is true for every pull request notification regardless of reason" do + # reason は購読理由であってイベント種別ではないため、mention 系も + # 「今回の更新がメンションだった」ことを意味しない。よって例外にしない。 + [ + "review_requested", + "assign", + "author", + "mention", + "team_mention", + ].each do |reason| + notification_from(reason, type: "PullRequest").checks_gated?.should be_true + end end it "is false for non pull request subjects" do notification_from("review_requested", type: "Issue").checks_gated?.should be_false + notification_from("mention", type: "Issue").checks_gated?.should be_false notification_from("author", type: "Commit").checks_gated?.should be_false end end diff --git a/spec/github/usecase_spec.cr b/spec/github/usecase_spec.cr index c619e6f..419d364 100644 --- a/spec/github/usecase_spec.cr +++ b/spec/github/usecase_spec.cr @@ -107,9 +107,17 @@ describe Github::Usecase do build_with_checks(pull_request, Github::ChecksState::Unknown).mention?.should be_true end - it "still mentions on a failing pull request when a human mentioned the user" do + it "does not mention on a failing pull request even for a mention reason" do + # reason=mention は一度メンションされた PR に永続するため、その後の + # push やコメントでも維持される。例外にすると赤い PR でチャンネル全体を + # 叩いてしまうので、mention 系もチェック状態で制御する。 notify = pull_request(reason: "mention") - build_with_checks(notify, Github::ChecksState::Failure).mention?.should be_true + build_with_checks(notify, Github::ChecksState::Failure).mention?.should be_false + end + + it "mentions on a passing pull request for a mention reason" do + notify = pull_request(reason: "mention") + build_with_checks(notify, Github::ChecksState::Success).mention?.should be_true end it "keeps non-mention reasons unmentioned regardless of the checks state" do diff --git a/src/github/models.cr b/src/github/models.cr index 199542d..712c1af 100644 --- a/src/github/models.cr +++ b/src/github/models.cr @@ -79,14 +79,6 @@ module Github GENERIC_MESSAGE = "なにかあったみたいです。確認してみましょう！" - # 人が明示的に呼んだことを表す reason。CI の状態に関わらずメンションする - # （呼ばれている以上、チェックの成否とは無関係に見てほしいはずなので - # 抑止しない / issue #105）。 - HUMAN_MENTION_REASONS = { - "mention", - "team_mention", - } - getter subject : Subject getter reason : String getter repository : Repository @@ -110,10 +102,19 @@ module Github # CI・自動チェックの状態でメンションを抑止する対象か（issue #105）。 # レビューできる状態になっていない PR で `@channel` / `@everyone` を撃たない - # ことが目的なので、PR 以外（Issue / Commit 等）と、人が明示的に呼んだ - # mention 系は対象外にする。 + # ことが目的なので、PR の通知はすべて対象にする。 + # + # 当初は mention / team_mention を「人が明示的に呼んだ」ものとして対象外に + # していたが、reason は購読理由であってイベント種別ではないため（FOLLOWUP_MESSAGES + # のコメント参照）、一度メンションされた PR はその後の push やコメントでも + # reason=mention のまま届く。これを対象外にすると、最も関与している PR でこそ + # チェックが赤いままチャンネル全体を叩いてしまい本末転倒なので、reason による + # 例外は設けない（PR #107 レビュー指摘）。 + # + # 実際に今回の更新がメンションだったかは通知 payload からは判別できない。 + # メンションされた通知自体は従来どおり届き、`@channel` が付かなくなるだけ。 def checks_gated? : Bool - subject.type == Subject::Type::PULL_REQUEST && !reason.in?(HUMAN_MENTION_REASONS) + subject.type == Subject::Type::PULL_REQUEST end # 通知の pretext（botのセリフ）。`[] ` 形式。