From 0dd29300c2f739124eca74504bf6e31ddd418202 Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Wed, 4 Nov 2026 14:14:57 -0600 Subject: [PATCH 1/3] fix(maintainer-gate): bind info-status exemptions to the Bot author and body prefix The info-status exemption (#1918, #1995) cleared a comment on its bare login and pattern alone. A User account can hold a registered bot's login (a User named `sonarqubecloud` exists), so the SonarQube Cloud "Quality Gate passed" text from that account cleared the gate. The Sonar pattern was also unanchored. - Add info_status_clear, a shared jq definition used by both check_maintainer_comments and maintainer_gate_open_comment_ids. It requires author.__typename "Bot" (missing type fails closed), the registered pattern, and no finding-bearing section. - maintainer_gate_merge_edit_times also merges author.__typename from its existing GraphQL pass, since gh pr view does not expose it. - Anchor the sonarqubecloud pattern to the passed-badge prefix. - Tests use the verbatim combsmith/.github#1421 notices: both clear from the Bot; a failed Sonar gate, a Codex review, a User or different bot posting the same text, and a missing author type all still block. Co-Authored-By: Claude Opus 5.5 --- .../adr/0012-comment-gate-exemption-set.md | 4 +- .../maintainer-comment-gate.md | 8 +- scripts/lib/maintainer-comment-gate.sh | 81 +++++++---- scripts/lib/reviewer-sources.tsv | 2 +- .../unit/test_maintainer_comment_gate.bats | 129 ++++++++++++++++-- .../unit/test_maintainer_resolve_comment.bats | 5 +- 6 files changed, 184 insertions(+), 45 deletions(-) diff --git a/docs/architecture/adr/0012-comment-gate-exemption-set.md b/docs/architecture/adr/0012-comment-gate-exemption-set.md index 64d8ec56f..7fc34d930 100644 --- a/docs/architecture/adr/0012-comment-gate-exemption-set.md +++ b/docs/architecture/adr/0012-comment-gate-exemption-set.md @@ -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 diff --git a/docs/pr-review-agent/maintainer-comment-gate.md b/docs/pr-review-agent/maintainer-comment-gate.md index 9b5afdd85..394c192b5 100644 --- a/docs/pr-review-agent/maintainer-comment-gate.md +++ b/docs/pr-review-agent/maintainer-comment-gate.md @@ -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 diff --git a/scripts/lib/maintainer-comment-gate.sh b/scripts/lib/maintainer-comment-gate.sh index 12ed76330..bedd00be5 100755 --- a/scripts/lib/maintainer-comment-gate.sh +++ b/scripts/lib/maintainer-comment-gate.sh @@ -40,7 +40,8 @@ # optional `.`/`!`, optionally the Claude Code footer) and nothing looser; # • from an OWNER/MEMBER/COLLABORATOR only: the explicit opt-out marker # ``; -# • 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, @@ -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 @@ -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 @@ -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 @@ -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" } @@ -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. 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 @@ -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 ] @@ -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)" @@ -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. @@ -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" diff --git a/scripts/lib/reviewer-sources.tsv b/scripts/lib/reviewer-sources.tsv index b6eec36ff..68033cfb8 100644 --- a/scripts/lib/reviewer-sources.tsv +++ b/scripts/lib/reviewer-sources.tsv @@ -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. diff --git a/tests/dev-lead/unit/test_maintainer_comment_gate.bats b/tests/dev-lead/unit/test_maintainer_comment_gate.bats index aedb688f4..43bd0603d 100644 --- a/tests/dev-lead/unit/test_maintainer_comment_gate.bats +++ b/tests/dev-lead/unit/test_maintainer_comment_gate.bats @@ -199,11 +199,13 @@ _run_check() { # #1918 — data-driven info-status classifier (SonarCloud clean passes) # ──────────────────────────────────────────────────────────────────── -# SonarCloud's real "Quality Gate passed" comment shape (trimmed): the headline plus -# per-metric lines. `_sonar_body ` renders it with the given -# counts; `_sonar_json [reviews_json]` wraps it in a gate snapshot. +# SonarCloud's real "Quality Gate passed" comment shape (trimmed): the badge +# headline plus per-metric lines. `_sonar_body ` renders it +# with the given counts; `_sonar_json [reviews_json] [typename]` wraps +# it in a gate snapshot whose author is a Bot unless [typename] says otherwise. +_SONAR_BADGE='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge' _sonar_body() { - printf '%s' "## [![Quality Gate Passed](https://sonarsource.github.io/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard) **Quality Gate passed** + printf '%s' "## [![Quality Gate Passed]($_SONAR_BADGE/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard) **Quality Gate passed** Issues ![](https://sonarsource.github.io/passed-16px.png '') [$1 New issues](https://sonarcloud.io/project/issues) ![](https://sonarsource.github.io/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues) @@ -213,8 +215,8 @@ Measures ![](https://sonarsource.github.io/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures)" } _sonar_json() { - jq -cn --arg l "$1" --arg b "$2" --argjson r "${3:-[]}" \ - '{reviews:$r, comments:[{author:{login:$l}, body:$b, isMinimized:false, minimizedReason:""}]}' + jq -cn --arg l "$1" --arg b "$2" --argjson r "${3:-[]}" --arg t "${4:-Bot}" \ + '{reviews:$r, comments:[{author:{login:$l, __typename:$t}, body:$b, isMinimized:false, minimizedReason:""}]}' } # AC1: a clean SonarCloud "Quality Gate passed" status comment (0 new issues, 0 @@ -257,7 +259,7 @@ _sonar_json() { # AC2: a SonarCloud "Quality Gate failed" comment still blocks — it carries findings. @test "AC2: SonarCloud 'Quality Gate failed' comment blocks → 1" { - local json='{"comments":[{"author":{"login":"sonarqubecloud"},"body":"## Quality Gate failed\n\n2 new issues.","isMinimized":false,"minimizedReason":""}]}' + local json='{"comments":[{"author":{"login":"sonarqubecloud","__typename":"Bot"},"body":"## Quality Gate failed\n\n2 new issues.","isMinimized":false,"minimizedReason":""}]}' _run_check "$json" [ "$status" -eq 1 ] } @@ -265,7 +267,7 @@ _sonar_json() { # AC2: an unclassifiable SonarCloud comment (author registered, body does NOT match # its info-status pattern) fails closed and blocks. @test "AC2: unclassifiable SonarCloud comment blocks (fail closed) → 1" { - local json='{"comments":[{"author":{"login":"sonarqubecloud"},"body":"Analysis in progress…","isMinimized":false,"minimizedReason":""}]}' + local json='{"comments":[{"author":{"login":"sonarqubecloud","__typename":"Bot"},"body":"Analysis in progress…","isMinimized":false,"minimizedReason":""}]}' _run_check "$json" [ "$status" -eq 1 ] } @@ -273,7 +275,7 @@ _sonar_json() { # AC2: the pattern is per-author — a DIFFERENT bot with no info-status pattern is # NOT cleared even if its body happens to contain "Quality Gate passed". @test "AC2: a bot with no info-status pattern is not cleared by matching text → 1" { - local json='{"comments":[{"author":{"login":"codeant-ai"},"body":"Quality Gate passed","isMinimized":false,"minimizedReason":""}]}' + local json='{"comments":[{"author":{"login":"codeant-ai","__typename":"Bot"},"body":"Quality Gate passed","isMinimized":false,"minimizedReason":""}]}' _run_check "$json" [ "$status" -eq 1 ] } @@ -304,9 +306,10 @@ _sonar_json() { # ──────────────────────────────────────────────────────────────────── _notice_json() { - # _notice_json — a one-comment, un-minimized gate snapshot. - jq -cn --arg l "$1" --arg b "$2" \ - '{reviews:[], comments:[{author:{login:$l}, body:$b, isMinimized:false, minimizedReason:""}]}' + # _notice_json [typename] — a one-comment, un-minimized gate + # snapshot whose author is a Bot unless [typename] says otherwise. + jq -cn --arg l "$1" --arg b "$2" --arg t "${3:-Bot}" \ + '{reviews:[], comments:[{author:{login:$l, __typename:$t}, body:$b, isMinimized:false, minimizedReason:""}]}' } @test "AC1(#1995): Codex usage-limit notice clears → 0" { @@ -775,8 +778,8 @@ _FOOTER=$'\n\n---\n_Generated by [Claude Code](https://claude.ai/code)_' @test "#2209: maintainer_gate_open_comment_ids drops a clean info-status comment and keeps a finding-bearing one, as the gate does" { local json json=$(jq -cn --arg clean "$(_sonar_body 0 0)" --arg dirty "$(_sonar_body 2 0)" '{comments:[ - {id:"IC_clean", author:{login:"sonarqubecloud"}, authorAssociation:"NONE", body:$clean, isMinimized:false, minimizedReason:""}, - {id:"IC_dirty", author:{login:"sonarqubecloud"}, authorAssociation:"NONE", body:$dirty, isMinimized:false, minimizedReason:""}, + {id:"IC_clean", author:{login:"sonarqubecloud", __typename:"Bot"}, authorAssociation:"NONE", body:$clean, isMinimized:false, minimizedReason:""}, + {id:"IC_dirty", author:{login:"sonarqubecloud", __typename:"Bot"}, authorAssociation:"NONE", body:$dirty, isMinimized:false, minimizedReason:""}, {id:"IC_h", author:{login:"don-petry"}, authorAssociation:"OWNER", body:"Fix X.", isMinimized:false, minimizedReason:""}]}') _open_ids "$json" [ "$status" -eq 0 ] @@ -788,7 +791,7 @@ _FOOTER=$'\n\n---\n_Generated by [Claude Code](https://claude.ai/code)_' # With only the clean status comment, the helper lists nothing and the gate is clear. json=$(jq -cn --arg clean "$(_sonar_body 0 0)" '{comments:[ - {id:"IC_clean", author:{login:"sonarqubecloud"}, authorAssociation:"NONE", body:$clean, isMinimized:false, minimizedReason:""}]}') + {id:"IC_clean", author:{login:"sonarqubecloud", __typename:"Bot"}, authorAssociation:"NONE", body:$clean, isMinimized:false, minimizedReason:""}]}') _open_ids "$json" [ "$status" -eq 0 ] [ -z "$output" ] @@ -796,6 +799,102 @@ _FOOTER=$'\n\n---\n_Generated by [Claude Code](https://claude.ai/code)_' [ "$status" -eq 0 ] } +# ──────────────────────────────────────────────────────────────────── +# Info-status notices are bound to the Bot author AND the body prefix. On +# combsmith/.github PR 1421 the two no-finding notices below had to be minimized by +# hand. Both must clear with no disposition, in the gate and in dev-lead's candidate +# filter alike. The SAME text from a User account holding the bot's login (a User +# named `sonarqubecloud` exists on GitHub), from a different bot, or with no author +# type must still block. +# ──────────────────────────────────────────────────────────────────── + +# The live PR 1421 bodies, verbatim. +_SONAR_1421_BODY=$'## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png \'Quality Gate Passed\')](https://sonarcloud.io/dashboard?id=combsmith_.github&pullRequest=1421) **Quality Gate passed** \nIssues \n![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png \'\') [0 New issues](https://sonarcloud.io/project/issues?id=combsmith_.github&pullRequest=1421&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true) \n![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png \'\') [0 Accepted issues](https://sonarcloud.io/project/issues?id=combsmith_.github&pullRequest=1421&issueStatuses=ACCEPTED)\n\nMeasures \n![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png \'\') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=combsmith_.github&pullRequest=1421&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true) \n![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png \'\') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=combsmith_.github&pullRequest=1421&metric=new_coverage&view=list) \n![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png \'\') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=combsmith_.github&pullRequest=1421&metric=new_duplicated_lines_density&view=list) \n \n\n[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=combsmith_.github&pullRequest=1421)\n\n' +_CODEX_1421_BODY=$'You have reached your Codex usage limits for code reviews. You can see your limits in the [Codex usage dashboard](https://chatgpt.com/codex/cloud/settings/usage).\nTo continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your [settings](https://chatgpt.com/settings/code-review).' +# SonarQube Cloud's failed-gate shape (live form, project id generalized). +_SONAR_FAILED_BODY=$'## [![Quality Gate Failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-failed-20px.png \'Quality Gate Failed\')](https://sonarcloud.io/dashboard?id=o_r&pullRequest=1) **Quality Gate failed** \nFailed conditions \n![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png \'\') [2 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=o_r&pullRequest=1) \n \n[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=o_r&pullRequest=1)\n' + +# _expect_clear — gate → 0 and no dev-lead candidate. +_expect_clear() { + local json + json=$(jq -cn --arg l "$1" --arg t "$2" --arg b "$3" \ + '{comments:[{id:"IC_n", author:{login:$l, __typename:$t}, authorAssociation:"NONE", body:$b, isMinimized:false, minimizedReason:""}]}') + _run_check "$json" + [ "$status" -eq 0 ] + _open_ids "$json" + [ "$status" -eq 0 ] + [ -z "$output" ] +} + +# _expect_open — gate → 1 and the comment is a dev-lead candidate. +_expect_open() { + local json + json=$(jq -cn --argjson a "$1" --arg b "$2" \ + '{comments:[{id:"IC_n", author:$a, authorAssociation:"NONE", body:$b, isMinimized:false, minimizedReason:""}]}') + _run_check "$json" + [ "$status" -eq 1 ] + _open_ids "$json" + [ "$status" -eq 0 ] + [ "$output" = "IC_n" ] +} + +@test "PR 1421: the live SonarQube Cloud 'Quality Gate passed' summary from the Bot needs no disposition" { + _expect_clear sonarqubecloud Bot "$_SONAR_1421_BODY" + _expect_clear 'sonarqubecloud[bot]' Bot "$_SONAR_1421_BODY" +} + +@test "PR 1421: the live Codex usage-limit notice from the Bot needs no disposition" { + _expect_clear chatgpt-codex-connector Bot "$_CODEX_1421_BODY" + _expect_clear 'chatgpt-codex-connector[bot]' Bot "$_CODEX_1421_BODY" +} + +@test "PR 1421: a failed SonarQube Cloud quality gate from the Bot still needs a disposition" { + _expect_open '{"login":"sonarqubecloud","__typename":"Bot"}' "$_SONAR_FAILED_BODY" +} + +@test "PR 1421: a Codex review summary (not the usage-limit notice) from the Bot still needs a disposition" { + _expect_open '{"login":"chatgpt-codex-connector","__typename":"Bot"}' $'\n### 💡 Codex Review\n\n**P1** The merge gate trusts an unchecked author — guard it before approving.' +} + +@test "PR 1421: a User account holding the Sonar login posting the passed summary still needs a disposition" { + _expect_open '{"login":"sonarqubecloud","__typename":"User"}' "$_SONAR_1421_BODY" +} + +@test "PR 1421: a User account holding the Codex login posting the usage-limit notice still needs a disposition" { + _expect_open '{"login":"chatgpt-codex-connector","__typename":"User"}' "$_CODEX_1421_BODY" +} + +@test "PR 1421: a different bot posting either notice still needs a disposition" { + _expect_open '{"login":"github-actions","__typename":"Bot"}' "$_SONAR_1421_BODY" + _expect_open '{"login":"sonarqubecloud","__typename":"Bot"}' "$_CODEX_1421_BODY" + _expect_open '{"login":"chatgpt-codex-connector","__typename":"Bot"}' "$_SONAR_1421_BODY" +} + +@test "PR 1421: a notice with no author type still needs a disposition (fail closed)" { + _expect_open '{"login":"sonarqubecloud"}' "$_SONAR_1421_BODY" + _expect_open '{"login":"chatgpt-codex-connector"}' "$_CODEX_1421_BODY" +} + +@test "PR 1421: the Sonar passed summary that does not open the body still needs a disposition" { + _expect_open '{"login":"sonarqubecloud","__typename":"Bot"}' $'Re-posting the earlier status:\n\n'"$_SONAR_1421_BODY" +} + +@test "maintainer_gate_merge_edit_times merges author.__typename by comment id" { + local bin="$BATS_TEST_TMPDIR/bin"; mkdir -p "$bin" + cat > "$bin/gh" <<'SHIM' +#!/usr/bin/env bash +printf '%s' '{"data":{"resource":{"comments":{"pageInfo":{"hasNextPage":false,"endCursor":null},"nodes":[{"id":"IC_b","lastEditedAt":null,"author":{"__typename":"Bot"}},{"id":"IC_u","lastEditedAt":null,"author":{"__typename":"User"}},{"id":"IC_g","lastEditedAt":null,"author":null}]}}}}' +SHIM + chmod +x "$bin/gh" + run env PATH="$bin:$PATH" bash -c "source '$GATE'; maintainer_gate_merge_edit_times https://github.com/o/r/pull/1 \"\$1\"" _ \ + '{"comments":[{"id":"IC_b","author":{"login":"sonarqubecloud"}},{"id":"IC_u","author":{"login":"sonarqubecloud"}},{"id":"IC_g","author":null},{"id":"IC_y","author":{"login":"x"}}]}' + [ "$status" -eq 0 ] + [ "$(printf '%s' "$output" | jq -c '.comments[0].author')" = '{"login":"sonarqubecloud","__typename":"Bot"}' ] + [ "$(printf '%s' "$output" | jq -r '.comments[1].author.__typename')" = "User" ] + [ "$(printf '%s' "$output" | jq -c '.comments[2].author')" = 'null' ] + [ "$(printf '%s' "$output" | jq -r '.comments[3].author | has("__typename")')" = "false" ] +} + @test "#2209: maintainer_gate_open_comment_ids fails on unreadable input" { run bash -c "source '$GATE'; maintainer_gate_open_comment_ids 'not json' donpetry-bot" [ "$status" -ne 0 ] diff --git a/tests/dev-lead/unit/test_maintainer_resolve_comment.bats b/tests/dev-lead/unit/test_maintainer_resolve_comment.bats index 2f3478dd4..3ebe0eefa 100644 --- a/tests/dev-lead/unit/test_maintainer_resolve_comment.bats +++ b/tests/dev-lead/unit/test_maintainer_resolve_comment.bats @@ -299,8 +299,9 @@ _mrc_minimized_reason() { # #1918 review: the registered-bot path must require a GitHub App actor. A PERSON # whose login happens to equal a registered bot login (e.g. a user account named # `sonarqubecloud`) must be refused like any other human — never minimized. -# A SonarCloud clean-status body that matches its registered info_status_pattern. -SONAR_PASS_JSON_BODY='**Quality Gate passed**\n[0 New issues]\n[0 Security Hotspots]' +# A SonarCloud clean-status body that matches its registered info_status_pattern, +# which is anchored to the passed-badge image the body opens with. +SONAR_PASS_JSON_BODY='## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png)](https://sonarcloud.io/dashboard) **Quality Gate passed**\n[0 New issues]\n[0 Security Hotspots]' _mrc_fake_gh() { # $1 = author __typename, $2 = author login (default sonarqubecloud), $3 = the From f3f70d9fff493b400da0deabecdee0da1ceb1273 Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Wed, 4 Nov 2026 14:14:57 -0600 Subject: [PATCH 2/3] fix(maintainer-gate): warn on untyped info-status notices; pin the lookup query in tests - check_maintainer_comments logs a warning when a comment from a registered info-status login carries no author type, so a stall on clean notices (author-type lookup missing or failed) is diagnosable. - The merge-helper test's gh shim answers only a query that selects author{__typename}, so dropping the selection fails the test (CodeRabbit). - A wiring test pins dev-lead's issue-comment fetch to select __typename. Co-Authored-By: Claude Opus 5.5 --- scripts/lib/maintainer-comment-gate.sh | 14 +++++++++++++ .../unit/test_maintainer_comment_gate.bats | 20 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/scripts/lib/maintainer-comment-gate.sh b/scripts/lib/maintainer-comment-gate.sh index bedd00be5..4be4aad7f 100755 --- a/scripts/lib/maintainer-comment-gate.sh +++ b/scripts/lib/maintainer-comment-gate.sh @@ -622,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) diff --git a/tests/dev-lead/unit/test_maintainer_comment_gate.bats b/tests/dev-lead/unit/test_maintainer_comment_gate.bats index 43bd0603d..62398fb64 100644 --- a/tests/dev-lead/unit/test_maintainer_comment_gate.bats +++ b/tests/dev-lead/unit/test_maintainer_comment_gate.bats @@ -875,14 +875,34 @@ _expect_open() { _expect_open '{"login":"chatgpt-codex-connector"}' "$_CODEX_1421_BODY" } +@test "PR 1421: a registered notice with no author type logs why it was not cleared" { + local json + json=$(jq -cn --arg b "$_SONAR_1421_BODY" \ + '{comments:[{id:"IC_n", author:{login:"sonarqubecloud"}, authorAssociation:"NONE", body:$b, isMinimized:false, minimizedReason:""}]}') + _run_check "$json" + [ "$status" -eq 1 ] + [[ "$output" == *"1 comment(s) from a registered info-status login carry no author type"* ]] + # A typed Bot notice logs nothing of the sort. + _run_check "$(_sonar_json sonarqubecloud "$_SONAR_1421_BODY")" + [ "$status" -eq 0 ] + [[ "$output" != *"carry no author type"* ]] +} + +@test "Wiring: dev-lead's issue-comment fetch selects the author type info_status_clear needs" { + grep -q 'nodes{ id author{login __typename} authorAssociation body' "$SCRIPT_DIR/dev-lead-fix-reviews.sh" +} + @test "PR 1421: the Sonar passed summary that does not open the body still needs a disposition" { _expect_open '{"login":"sonarqubecloud","__typename":"Bot"}' $'Re-posting the earlier status:\n\n'"$_SONAR_1421_BODY" } @test "maintainer_gate_merge_edit_times merges author.__typename by comment id" { local bin="$BATS_TEST_TMPDIR/bin"; mkdir -p "$bin" + # The shim answers only a query that selects the author type, so dropping it + # from the lookup fails this test instead of silently stalling clean notices. cat > "$bin/gh" <<'SHIM' #!/usr/bin/env bash +[[ "$*" == *'author{__typename}'* ]] || exit 1 printf '%s' '{"data":{"resource":{"comments":{"pageInfo":{"hasNextPage":false,"endCursor":null},"nodes":[{"id":"IC_b","lastEditedAt":null,"author":{"__typename":"Bot"}},{"id":"IC_u","lastEditedAt":null,"author":{"__typename":"User"}},{"id":"IC_g","lastEditedAt":null,"author":null}]}}}}' SHIM chmod +x "$bin/gh" From d27c67fcb72d0f2b03f0aa82f603d5e4880bbff9 Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Wed, 4 Nov 2026 14:15:21 -0600 Subject: [PATCH 3/3] test(feature-ideation): assert the redispatch uses the job token (#2444) #2444 switched the redispatch job to GH_TOKEN: ${{ github.token }}. The job token can start workflow_dispatch runs, and GH_PAT_WORKFLOWS is being retired (#1326). The test still required secrets.GH_PAT_WORKFLOWS, so bats has failed on main since #2444. It now asserts the redispatch job sets GH_TOKEN from github.token and that the workflow no longer references GH_PAT_WORKFLOWS. Co-Authored-By: Claude Opus 5.5 --- tests/test_feature_ideation.bats | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/test_feature_ideation.bats b/tests/test_feature_ideation.bats index bec64189c..b04643dc0 100644 --- a/tests/test_feature_ideation.bats +++ b/tests/test_feature_ideation.bats @@ -103,10 +103,18 @@ setup() { grep -qE "github\.event\.discussion\.user\.type[[:space:]]*!=[[:space:]]*['\"]Bot['\"]" "$FEATURE_IDEATION_YML" } -@test "feature-ideation.yml redispatch requires GH_PAT_WORKFLOWS (GITHUB_TOKEN won't start a run)" { - # A workflow_dispatch fired with GITHUB_TOKEN is accepted but never starts a - # run (loop prevention) — same constraint as the initiative-planner bridge. - grep -qF "secrets.GH_PAT_WORKFLOWS" "$FEATURE_IDEATION_YML" +@test "feature-ideation.yml redispatch uses the job token, not GH_PAT_WORKFLOWS (#2444)" { + # workflow_dispatch and repository_dispatch are the two events the job token + # can start runs with, so the redispatch needs no personal token (#2444), and + # GH_PAT_WORKFLOWS is being retired (#1326). + local job + job=$(awk '/^ redispatch:/{f=1; print; next} f && /^ [A-Za-z0-9_-]+:/{f=0} f' "$FEATURE_IDEATION_YML") + [ -n "$job" ] + grep -qE 'GH_TOKEN:[[:space:]]*\$\{\{[[:space:]]*github\.token[[:space:]]*\}\}' <<<"$job" + if grep -qF "secrets.GH_PAT_WORKFLOWS" "$FEATURE_IDEATION_YML"; then + echo "feature-ideation.yml still references secrets.GH_PAT_WORKFLOWS" >&2 + return 1 + fi } @test "feature-ideation.yml redispatch forwards target_discussion via workflow_dispatch" {