Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion docs/architecture/adr/0012-comment-gate-exemption-set.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ We will narrow #1813's "every PR issue comment" scope to every PR issue comment
From any other author the marker does nothing.
5. **A registered clean info-status bot comment** (#1918): a registered
reviewer source whose body matches its `info_status_pattern` and carries no
finding-bearing section (#2008).
finding-bearing section (#2008). The author must be a GitHub App
(`author.__typename` is `Bot`), because a User account can hold a registered
bot's bare login.

We will never exempt by author login alone, because `don-petry` is also the
account dev-lead and the personas post from. A steering comment stays in scope
Expand Down
8 changes: 7 additions & 1 deletion docs/pr-review-agent/maintainer-comment-gate.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,14 @@ is minimized `RESOLVED`, **unless** it is in the exemption set recorded in
reviewer source that declares an `info_status_pattern` in the reviewer-source
registry (`scripts/lib/reviewer-sources.tsv`), and whose body matches that pattern,
is a clean status report carrying no finding and is treated as **addressed**. The
author must be the GitHub App itself (`author.__typename` is `Bot`): an ordinary
User account can hold a registered bot's bare login (a User named `sonarqubecloud`
exists), so the same text from it still blocks, and a comment whose author type is
unknown still blocks. pr-review reads the author type in the same GraphQL pass that
reads edit times (`maintainer_gate_merge_edit_times`). The
canonical case is SonarCloud's `Quality Gate passed` comment, which it **re-posts on
every push**; its pattern pins the `**Quality Gate passed**` headline **and** the
every push**; its pattern is anchored to the passed-badge image the comment opens
with and pins the `**Quality Gate passed**` headline **and** the
`[0 New issues]` and `[0 Security Hotspots]` lines, so a `Quality Gate failed`
comment, or one still reporting new issues or hotspots, does **not** match and still
blocks. An unreadable registry degrades to "clear nothing", never to clearing
Expand Down
95 changes: 70 additions & 25 deletions scripts/lib/maintainer-comment-gate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@
# optional `.`/`!`, optionally the Claude Code footer) and nothing looser;
# • from an OWNER/MEMBER/COLLABORATOR only: the explicit opt-out marker
# `<!-- maintainer:not-a-finding -->`;
# • a registered clean info-status bot comment (#1918).
# • a registered clean info-status bot comment (#1918), from a GitHub App
# author (`author.__typename` "Bot") only.
# No author is exempt by login alone: don-petry is also the account dev-lead and
# the personas post from (ADR-0008). A steering comment stays in scope unless
# the maintainer opts it out. The definition is _MAINTAINER_GATE_SCOPE_JQ_DEFS,
Expand Down Expand Up @@ -124,6 +125,29 @@ readonly _MAINTAINER_GATE_SCOPE_JQ_DEFS='
and (opted_out | not);
'

# The known-clean info-status classifier (#1918) as a jq definition, shared by
# check_maintainer_comments and maintainer_gate_open_comment_ids so the gate and
# dev-lead's candidate filter cannot disagree. The caller binds $infopatterns
# (_maintainer_gate_info_patterns_json) and $findre (_maintainer_gate_finding_re).
# info_status_clear — ALL of: the author is a GitHub App bot
# (author.__typename "Bot"); its login, with any [bot] suffix
# stripped, has a registered info_status_pattern; the body
# matches that pattern; and the body carries no finding-bearing
# section (#2008). The login alone is not enough: an ordinary
# User account can hold a registered bot's bare login (a User
# named `sonarqubecloud` exists), so the same text from it must
# still block. A missing __typename is not "Bot" (fail closed).
readonly _MAINTAINER_GATE_INFO_JQ_DEFS='
def info_status_clear:
((.author?.__typename // "") == "Bot")
and ((.author?.login // "" | tostring | if endswith("[bot]") then .[0:-5] else . end) as $lbare
| ($infopatterns[$lbare] // null) as $p
| ((.body // "") | tostring) as $b
| ($p != null)
and (($b | test($findre)) | not)
and ($b | test($p)));
'

# This library's own directory, resolved ONCE at source time. dev-lead-fix-reviews.sh
# is run by a relative path (`bash .dev-lead/scripts/...`) and later cds into the
# PR worktree, so a function resolving BASH_SOURCE at call time gets a path that
Expand Down Expand Up @@ -309,6 +333,9 @@ log_warn() {
# that comment WITHOUT a lastEditedAt key. The gate then fails closed (rc 2) for
# any edited RESOLVED bot comment. The snapshot is never fabricated: on any
# failure it is echoed unchanged.
# The same read also merges each comment's author.__typename, which `gh pr view`
# does not expose either. info_status_clear needs it ("Bot"); a comment without it
# is never cleared as info-status, so a failed lookup fails closed there too.
maintainer_gate_merge_edit_times() {
local pr_url="${1:-}" snapshot="${2:-}"
[[ -n "$snapshot" ]] || return 0
Expand All @@ -317,7 +344,7 @@ maintainer_gate_merge_edit_times() {
return 0
fi
# shellcheck disable=SC2016 # $url/$cursor are GraphQL variables, not shell
local _gql='query($url:URI!,$cursor:String){resource(url:$url){...on PullRequest{comments(first:100,after:$cursor){pageInfo{hasNextPage endCursor} nodes{id lastEditedAt}}}}}'
local _gql='query($url:URI!,$cursor:String){resource(url:$url){...on PullRequest{comments(first:100,after:$cursor){pageInfo{hasNextPage endCursor} nodes{id lastEditedAt author{__typename}}}}}}'
local edits="[]" page nodes has_next="true" cursor="" pages=0
local -a cursor_args=()
while [[ "$has_next" == "true" ]]; do
Expand Down Expand Up @@ -359,10 +386,18 @@ maintainer_gate_merge_edit_times() {
done
printf '%s' "$snapshot" | jq -c --argjson edits "$edits" '
([ $edits[] | objects | select(.id != null) | {key: .id, value: .lastEditedAt} ] | from_entries) as $m
| ([ $edits[] | objects | select(.id != null)
| select(((.author?.__typename // null) | type) == "string")
| {key: .id, value: .author.__typename} ] | from_entries) as $t
| if (.comments | type) == "array" then
.comments |= map(if (type == "object") and ((.id // null) | type) == "string"
and (.id as $id | $m | has($id))
then . + {lastEditedAt: $m[.id]} else . end)
then . + {lastEditedAt: $m[.id]}
| if (.id as $id | $t | has($id))
and (((.author // null) | type) == "object" or .author == null)
then .author = ((.author // {}) + {__typename: $t[.id]})
else . end
else . end)
else . end
' 2>/dev/null || printf '%s' "$snapshot"
}
Expand Down Expand Up @@ -484,9 +519,11 @@ maintainer_gate_head_committer_date() {
# node id per line for every comment in the gate's scope (in_gate_scope) that is
# not minimized RESOLVED, so the harness never tries to disposition a comment the
# gate does not count. <comments_array_json> is the harness's issue-comment nodes,
# each {id, author{login}, authorAssociation, body, isMinimized, minimizedReason}.
# Filters out clean registered info-status comments matching check_maintainer_comments
# so the gate and candidate filter never disagree. Unreadable input echoes nothing
# each {id, author{login,__typename}, authorAssociation, body, isMinimized,
# minimizedReason}. Filters out clean registered info-status comments with the same
# info_status_clear definition check_maintainer_comments uses, so the gate and
# candidate filter never disagree. A node without author.__typename "Bot" is never
# filtered as info-status. Unreadable input echoes nothing
# and returns non-zero.
maintainer_gate_open_comment_ids() {
local comments="${1:-}" bot_user="${2:-donpetry-bot}" info_patterns finding_re
Expand All @@ -496,16 +533,11 @@ maintainer_gate_open_comment_ids() {
--arg botuser "$bot_user" \
--arg markers "$_MAINTAINER_GATE_AGENT_MARKERS" \
--arg findre "$finding_re" \
--argjson infopatterns "$info_patterns" "$_MAINTAINER_GATE_SCOPE_JQ_DEFS"'
--argjson infopatterns "$info_patterns" "$_MAINTAINER_GATE_SCOPE_JQ_DEFS$_MAINTAINER_GATE_INFO_JQ_DEFS"'
if type != "array" then error("not an array") else . end
| [ .[] | objects
| select(in_gate_scope)
| (.author?.login // "" | tostring | if endswith("[bot]") then .[0:-5] else . end) as $lbare
| ((.body // "") | tostring | test($findre)) as $findings
| select(
($infopatterns[$lbare] // null) as $p
| ($p == null) or $findings or (((.body // "") | test($p)) | not)
)
| select(info_status_clear | not)
| select(((.isMinimized // false) == true)
and (((.minimizedReason // "") | ascii_downcase) == "resolved") | not)
| .id ]
Expand All @@ -530,7 +562,8 @@ check_maintainer_comments() {
# Data-driven info-status classifier (#1918): {login: pattern} of KNOWN CLEAN
# informational status comments (e.g. sonarqubecloud → "Quality Gate passed"),
# read from the reviewer-source registry. A comment authored by a listed login
# whose body matches its pattern carries no finding and is treated as addressed.
# (a Bot author only, see info_status_clear) whose body matches its pattern
# carries no finding and is treated as addressed.
# An unreadable registry yields "{}" → no comment is auto-cleared (fail closed).
local info_patterns
info_patterns="$(_maintainer_gate_info_patterns_json)"
Expand All @@ -542,7 +575,8 @@ check_maintainer_comments() {
registered="$(_maintainer_gate_registered_logins_json)"

# Per non-agent comment, classify: "clear" | "block" | "unreadable".
# • A KNOWN CLEAN info-status comment (registered login + its pattern) is clear,
# • A KNOWN CLEAN info-status comment (Bot author + registered login + its
# pattern) is clear,
# unless its body carries a finding-bearing section (#2008). A rate-limit or
# status notice never clears the findings beside it.
# • A comment not minimized RESOLVED blocks.
Expand All @@ -562,21 +596,18 @@ check_maintainer_comments() {
--arg botuser "$bot_user" \
--arg findre "$finding_re" \
--argjson registered "$registered" \
--argjson infopatterns "$info_patterns" "$_MAINTAINER_GATE_DISP_JQ_DEFS$_MAINTAINER_GATE_SCOPE_JQ_DEFS"'
--argjson infopatterns "$info_patterns" "$_MAINTAINER_GATE_DISP_JQ_DEFS$_MAINTAINER_GATE_SCOPE_JQ_DEFS$_MAINTAINER_GATE_INFO_JQ_DEFS"'
(.comments // []) as $all
| [ $all[] | objects
| (.author?.login // "" | tostring | if endswith("[bot]") then .[0:-5] else . end) as $lbare
# #2209: our login, registered markers, review requests, opt-outs.
| select(in_gate_scope)
| ((.body // "") | tostring | test($findre)) as $findings
# Drop KNOWN CLEAN info-status comments: author is a registered source, its
# body matches that source pattern, and it carries no finding-bearing
# section. Anything else (a failing status, an unrecognised body, a bot
# with no pattern, a human) is still evaluated.
| select(
($infopatterns[$lbare] // null) as $p
| ($p == null) or $findings or (((.body // "") | test($p)) | not)
)
# Drop KNOWN CLEAN info-status comments: a Bot author that is a registered
# source, its body matches that source pattern, and it carries no
# finding-bearing section. Anything else (a failing status, an
# unrecognised body, a bot with no pattern, a human, a User account
# holding a bot login) is still evaluated.
| select(info_status_clear | not)
| if ((.isMinimized // false) == true)
and (((.minimizedReason // "") | ascii_downcase) == "resolved") | not
then "block"
Expand All @@ -591,6 +622,20 @@ check_maintainer_comments() {
return 2
}

# A registered info-status login with no author type can never clear as
# info-status (info_status_clear fails closed). Say so, so a stall on clean
# notices (e.g. the edit-time/author-type lookup failed) is diagnosable.
local untyped
untyped=$(printf '%s' "$json" | jq -r --argjson infopatterns "$info_patterns" '
[ (.comments // [])[]? | objects
| select(((.author?.__typename // null) | type) != "string")
| (.author?.login // "" | tostring | if endswith("[bot]") then .[0:-5] else . end) as $l
| select($infopatterns[$l] != null) ] | length
' 2>/dev/null || echo 0)
if [[ "$untyped" =~ ^[0-9]+$ && "$untyped" -gt 0 ]]; then
log_warn "$untyped comment(s) from a registered info-status login carry no author type — not cleared as info-status (author-type lookup missing or failed)"
fi

local blockers unreadable
blockers=$(printf '%s' "$verdict" | jq -r '.blockers // empty' 2>/dev/null || true)
unreadable=$(printf '%s' "$verdict" | jq -r '.unreadable // empty' 2>/dev/null || true)
Expand Down
2 changes: 1 addition & 1 deletion scripts/lib/reviewer-sources.tsv
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ copilot-pull-request-reviewer yes yes no - - Inline review comments; dev-lead ad
gemini-code-assist yes yes yes - - Inline review comments; dev-lead addresses the findings.
chatgpt-codex-connector yes yes no - ^(You have reached your Codex usage limits?( for code reviews)?\.(\s+You can see your limits in the (\[Codex usage dashboard\]\([^)\s]*\)|Codex usage dashboard)\.)?(\s+To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your \[settings\]\([^)\s]*\)\.)?|To use Codex here, (\[create an environment for this repo\]\([^)\s]*\)|create an environment for this repo)\.)\s*$ Inline review comments (pull_request_review_comment); dev-lead addresses the findings. Its out-of-quota "You have reached your Codex usage limit[s for code reviews …]" notice — short and long forms alike — carries no finding, so the gate auto-clears it (#1995, #1993). Its "To use Codex here, create an environment for this repo." notice carries no finding either and clears the same way (#2255). Anchored to the body start and to the end of the notice sentence, so neither a comment that merely quotes the notice nor one that opens with its words and goes on to a finding can clear; a real review body does not match and still blocks. Dropped from the approval-wait set (advisory_gate=no, #1997) because zero reviews were observed across eight consecutive PRs (#1888, #1887, #1873, #1799, #1796, #1794, #1793, #1770), measured 2026-09-23. Stays dev-lead-trusted and on the #1220 scorecard; it leaves only the approval-wait set.
coderabbitai yes yes no - ^\s*(<!--[^>]*-->\s*)*Review limit reached — you have used up your prepaid credits\.?\s*$ Inline review comments; tracked by the scorecard and rate-limit detector but not part of the advisory approval-wait set. The pattern matches only a STANDALONE "Review limit reached — you have used up your prepaid credits" notice, a documented form that has NOT been observed live (#1995). The notice CodeRabbit actually posts is the OSS "Review limit reached / You've used all free OSS reviews…" block, embedded in its full summary comment beside the separately throttled Security Architecture Review. That summary can carry real findings while the code review is rate-limited (PR #2000), so the pattern deliberately does NOT match the OSS notice: matching it would clear the whole summary, findings included (#2008; the #1995 "do not widen" rule). The summary is dispositioned per section through dev-lead's flow instead, and no CodeRabbit comment carrying a finding-bearing section (reviewer_sources_finding_section_pattern) can be auto-cleared. A real review body does not match and still blocks.
sonarqubecloud no yes yes - \*\*Quality Gate passed\*\*[\s\S]*\[0 New issues\][\s\S]*\[0 Security Hotspots\] Top-level quality-gate comment only — no inline review threads; trusted so dev-lead acts on the comment via the issue-comment path. Its "Quality Gate passed" status is re-posted on every push and carries no finding, so the gate auto-clears it (#1918); "Quality Gate failed" does not match and still blocks.
sonarqubecloud no yes yes - ^## \[!\[Quality Gate Passed\]\(https://sonarsource\.github\.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px\.png[\s\S]*\*\*Quality Gate passed\*\*[\s\S]*\[0 New issues\][\s\S]*\[0 Security Hotspots\] Top-level quality-gate comment only — no inline review threads; trusted so dev-lead acts on the comment via the issue-comment path. Its "Quality Gate passed" status is re-posted on every push and carries no finding, so the gate auto-clears it (#1918); "Quality Gate failed" does not match and still blocks. The pattern is anchored to the body start at the passed-badge image SonarQube Cloud opens every gate comment with, so the words quoted inside another comment never clear it.
qodo-code-review yes yes no - ^\s*(<!--[^>]*-->\s*)*((\*\*)?(ⓘ\s*)?Qodo reviews are paused because your trial has ended\.(\*\*)?(\s+Ask your workspace admin to add credits to resume reviews\.(\s+\[Manage billing\]\([^)\s]*\))?)?|Qodo Merge has reached your monthly usage limit for pull-request reviews on this repository\.(\s+Reviews will resume when the limit resets\.)?)\s*$ Inline review comments (#1355 recognized it for gating/scorecard; trusted for acting per #1425). Its trial-ended and monthly-usage-limit notices carry no finding, so the gate auto-clears either (#1995); a real review body does not match and still blocks. Dropped from the approval-wait set (advisory_gate=no, #1997) because zero reviews were observed across eight consecutive PRs (#1888, #1887, #1873, #1799, #1796, #1794, #1793, #1770), measured 2026-09-23. Stays dev-lead-trusted and on the #1220 scorecard; it leaves only the approval-wait set.
codeant-ai yes yes yes - - Inline review comments (#1355 recognized it for gating/scorecard; trusted for acting per #1425).
graphite-app yes yes yes Graphite / AI Reviews - Inline review comments AND clean passes reported ONLY as the "Graphite / AI Reviews" check run (no PR review) — the scorecard must read that check run or every clean pass is miscounted as No response (#1908). Root cause of the 2026-08-02 unclearable-thread deadlock — trusted so dev-lead can act on its findings (#1425). Kept at advisory_gate=yes under #1997: its zero-review count is a measurement artefact (a clean pass posts only the check run, invisible to the reviews API), not an absence of signal; its 2026-08-21 silence was a lapsed 30-day trial and the org moved to the paid plan on 2026-09-22, so do NOT re-drop it. The advisory gate counts a successful check run at the current head as graphite's clean submission (#2005), so a clean PR reaches quorum without waiting out the head-age timeout.
Expand Down
Loading
Loading